From d3c2f7972a71f77b43eaffd50d34067f1a4c73bf Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Tue, 31 Dec 2024 13:14:45 +0100 Subject: [PATCH 01/20] chore: wip commit --- pkg/client/clientset.go | 2 + pkg/client/dummy_clientset.go | 30 ++++++++-- pkg/deploy/deploy.go | 8 +++ pkg/deploy/internal/segment/segment.go | 83 ++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 pkg/deploy/internal/segment/segment.go diff --git a/pkg/client/clientset.go b/pkg/client/clientset.go index 700a857c0..e7aea73f4 100644 --- a/pkg/client/clientset.go +++ b/pkg/client/clientset.go @@ -170,6 +170,8 @@ type SegmentClient interface { List(ctx context.Context) (segments.Response, error) GetAll(ctx context.Context) ([]segments.Response, error) Delete(ctx context.Context, id string) (segments.Response, error) + Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) + Get(ctx context.Context, id string) (segments.Response, error) } var DefaultMonacoUserAgent = "Dynatrace Monitoring as Code/" + version.MonitoringAsCode + " " + (runtime.GOOS + " " + runtime.GOARCH) diff --git a/pkg/client/dummy_clientset.go b/pkg/client/dummy_clientset.go index 3b78c5b10..2f784ef5e 100644 --- a/pkg/client/dummy_clientset.go +++ b/pkg/client/dummy_clientset.go @@ -26,17 +26,19 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/automation" buckets "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" documents "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" + grailfiltersegment "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/grailfiltersegments" openpipeline "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/openpipeline" dtclient "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/client/dtclient" ) var DummyClientSet = ClientSet{ - ConfigClient: &dtclient.DummyConfigClient{}, - SettingsClient: &dtclient.DummySettingsClient{}, - AutClient: &DummyAutomationClient{}, - BucketClient: &DummyBucketClient{}, - DocumentClient: &DummyDocumentClient{}, - OpenPipelineClient: &DummyOpenPipelineClient{}, + ConfigClient: &dtclient.DummyConfigClient{}, + SettingsClient: &dtclient.DummySettingsClient{}, + AutClient: &DummyAutomationClient{}, + BucketClient: &DummyBucketClient{}, + DocumentClient: &DummyDocumentClient{}, + OpenPipelineClient: &DummyOpenPipelineClient{}, + GrailFilterSegmentClient: &DummySegmentsClient{}, } var _ AutomationClient = (*DummyAutomationClient)(nil) @@ -155,3 +157,19 @@ func (c *DummyOpenPipelineClient) GetAll(ctx context.Context) ([]coreapi.Respons func (c *DummyOpenPipelineClient) Update(_ context.Context, _ string, _ []byte) (openpipeline.Response, error) { return openpipeline.Response{}, nil } + +type DummySegmentsClient struct{} + +// GetAll implements GrailFilterSegmentClient +func (c *DummySegmentsClient) GetAll(_ context.Context) ([]coreapi.Response, error) { + panic("unimplemented") +} + +// Upsert implements GrailFilterSegmentClient +func (c *DummySegmentsClient) Upsert(_ context.Context, _ string, _ []byte) (grailfiltersegment.Response, error) { + return grailfiltersegment.Response{}, nil +} + +func (c *DummySegmentsClient) Get(_ context.Context, id string) (grailfiltersegment.Response, error) { + return grailfiltersegment.Response{}, nil +} diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go index 00f278c74..270d9d92d 100644 --- a/pkg/deploy/deploy.go +++ b/pkg/deploy/deploy.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy/internal/segment" "sync" "time" @@ -309,6 +310,13 @@ func deployConfig(ctx context.Context, c *config.Config, clientset *client.Clien deployErr = fmt.Errorf("unknown config-type (ID: %q)", c.Type.ID()) } + case config.GrailFilterSegment: + if featureflags.Temporary[featureflags.GrailFilterSegment].Enabled() { + resolvedEntity, deployErr = segment.Deploy(ctx, clientset.GrailFilterSegmentClient, properties, renderedConfig, c) + } else { + deployErr = fmt.Errorf("unknown config-type (ID: %q)", c.Type.ID()) + } + default: deployErr = fmt.Errorf("unknown config-type (ID: %q)", c.Type.ID()) } diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go new file mode 100644 index 000000000..46e89e62b --- /dev/null +++ b/pkg/deploy/internal/segment/segment.go @@ -0,0 +1,83 @@ +/* + * @license + * Copyright 2024 Dynatrace LLC + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package segment + +import ( + "context" + "errors" + "fmt" + "github.com/dynatrace/dynatrace-configuration-as-code-core/api" + segment "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/grailfiltersegments" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/log" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/entities" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/parameter" + deployErrors "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy/errors" + "github.com/go-logr/logr" + "net/http" + "time" +) + +type DeploySegmentClient interface { + Upsert(ctx context.Context, id string, data []byte) (segment.Response, error) + Get(ctx context.Context, id string) (segment.Response, error) +} + +func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { + externalId := c.Coordinate.String() + println(externalId) //@TODO remove this, as its only here for debug + + //create new context to carry logger + ctx = logr.NewContext(ctx, log.WithCtxFields(ctx).GetLogr()) + ctx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + + //Strategy 1 if OriginObjectId is set check on remote if an object can be found and update it, + //also update the externalId to the computed one we generate + if c.OriginObjectId != "" { + getResponse, err := client.Get(ctx, c.OriginObjectId) + if err != nil { + return entities.ResolvedEntity{}, err + } + if getResponse.StatusCode != http.StatusNotFound { + //@TODO how to set externalId here? unmarshall and set then marshall + _, err := client.Upsert(ctx, c.OriginObjectId, []byte(renderedConfig)) + if err != nil { + //println(updateResponse) + } + } + } + + //Strategy 2 is to generate external id and either update or create object + _, err := client.Upsert(ctx, externalId, []byte(renderedConfig)) + if err != nil { + var apiErr api.APIError + if errors.As(err, &apiErr) { + return entities.ResolvedEntity{}, fmt.Errorf("failed to upsert segment with segmentName %q: %w", externalId, err) + } + + return entities.ResolvedEntity{}, deployErrors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert segment with segmentName %q", externalId)).WithError(err) + } + // Set this to the upserte id returned by api + properties[config.IdParameter] = externalId + + return entities.ResolvedEntity{ + EntityName: externalId, + Coordinate: c.Coordinate, + Properties: properties, + }, nil +} From 12544b2ff885276f1e6c8467aa404bd4f66b995b Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Thu, 2 Jan 2025 14:23:33 +0100 Subject: [PATCH 02/20] feat: wip core implementation working --- pkg/client/clientset.go | 1 - pkg/client/dummy_clientset.go | 24 +++++------ pkg/deploy/deploy.go | 6 +-- pkg/deploy/internal/segment/segment.go | 59 +++++++++++++++++++------- 4 files changed, 56 insertions(+), 34 deletions(-) diff --git a/pkg/client/clientset.go b/pkg/client/clientset.go index e7aea73f4..db8738fb7 100644 --- a/pkg/client/clientset.go +++ b/pkg/client/clientset.go @@ -171,7 +171,6 @@ type SegmentClient interface { GetAll(ctx context.Context) ([]segments.Response, error) Delete(ctx context.Context, id string) (segments.Response, error) Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) - Get(ctx context.Context, id string) (segments.Response, error) } var DefaultMonacoUserAgent = "Dynatrace Monitoring as Code/" + version.MonitoringAsCode + " " + (runtime.GOOS + " " + runtime.GOARCH) diff --git a/pkg/client/dummy_clientset.go b/pkg/client/dummy_clientset.go index 2f784ef5e..cbe6841b6 100644 --- a/pkg/client/dummy_clientset.go +++ b/pkg/client/dummy_clientset.go @@ -26,19 +26,19 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/automation" buckets "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" documents "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" - grailfiltersegment "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/grailfiltersegments" openpipeline "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/openpipeline" + segments "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" dtclient "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/client/dtclient" ) var DummyClientSet = ClientSet{ - ConfigClient: &dtclient.DummyConfigClient{}, - SettingsClient: &dtclient.DummySettingsClient{}, - AutClient: &DummyAutomationClient{}, - BucketClient: &DummyBucketClient{}, - DocumentClient: &DummyDocumentClient{}, - OpenPipelineClient: &DummyOpenPipelineClient{}, - GrailFilterSegmentClient: &DummySegmentsClient{}, + ConfigClient: &dtclient.DummyConfigClient{}, + SettingsClient: &dtclient.DummySettingsClient{}, + AutClient: &DummyAutomationClient{}, + BucketClient: &DummyBucketClient{}, + DocumentClient: &DummyDocumentClient{}, + OpenPipelineClient: &DummyOpenPipelineClient{}, + SegmentClient: &DummySegmentsClient{}, } var _ AutomationClient = (*DummyAutomationClient)(nil) @@ -166,10 +166,6 @@ func (c *DummySegmentsClient) GetAll(_ context.Context) ([]coreapi.Response, err } // Upsert implements GrailFilterSegmentClient -func (c *DummySegmentsClient) Upsert(_ context.Context, _ string, _ []byte) (grailfiltersegment.Response, error) { - return grailfiltersegment.Response{}, nil -} - -func (c *DummySegmentsClient) Get(_ context.Context, id string) (grailfiltersegment.Response, error) { - return grailfiltersegment.Response{}, nil +func (c *DummySegmentsClient) Upsert(_ context.Context, _ string, _ []byte) (segments.Response, error) { + return segments.Response{}, nil } diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go index 270d9d92d..636892e31 100644 --- a/pkg/deploy/deploy.go +++ b/pkg/deploy/deploy.go @@ -310,9 +310,9 @@ func deployConfig(ctx context.Context, c *config.Config, clientset *client.Clien deployErr = fmt.Errorf("unknown config-type (ID: %q)", c.Type.ID()) } - case config.GrailFilterSegment: - if featureflags.Temporary[featureflags.GrailFilterSegment].Enabled() { - resolvedEntity, deployErr = segment.Deploy(ctx, clientset.GrailFilterSegmentClient, properties, renderedConfig, c) + case config.Segment: + if featureflags.Temporary[featureflags.Segments].Enabled() { + resolvedEntity, deployErr = segment.Deploy(ctx, clientset.SegmentClient, properties, renderedConfig, c) } else { deployErr = fmt.Errorf("unknown config-type (ID: %q)", c.Type.ID()) } diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index 46e89e62b..d000d021f 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -18,52 +18,79 @@ package segment import ( "context" + "encoding/json" "errors" "fmt" "github.com/dynatrace/dynatrace-configuration-as-code-core/api" - segment "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/grailfiltersegments" + segment "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/log" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/entities" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/parameter" deployErrors "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy/errors" "github.com/go-logr/logr" - "net/http" "time" ) type DeploySegmentClient interface { Upsert(ctx context.Context, id string, data []byte) (segment.Response, error) - Get(ctx context.Context, id string) (segment.Response, error) + GetAll(ctx context.Context) ([]segment.Response, error) } func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { - externalId := c.Coordinate.String() - println(externalId) //@TODO remove this, as its only here for debug - //create new context to carry logger ctx = logr.NewContext(ctx, log.WithCtxFields(ctx).GetLogr()) ctx, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() + // external id is generated from project-configType-configId + externalId := c.Coordinate.String() + + var request map[string]any + err := json.Unmarshal([]byte(renderedConfig), &request) + request["externalId"] = externalId + //Strategy 1 if OriginObjectId is set check on remote if an object can be found and update it, - //also update the externalId to the computed one we generate + //update the externalId to the computed one we generate if c.OriginObjectId != "" { - getResponse, err := client.Get(ctx, c.OriginObjectId) + request["uid"] = c.OriginObjectId + payload, _ := json.Marshal(request) + responseUpsert, err := client.Upsert(ctx, c.OriginObjectId, payload) if err != nil { return entities.ResolvedEntity{}, err } - if getResponse.StatusCode != http.StatusNotFound { - //@TODO how to set externalId here? unmarshall and set then marshall - _, err := client.Upsert(ctx, c.OriginObjectId, []byte(renderedConfig)) - if err != nil { - //println(updateResponse) - } + if responseUpsert.Data == nil { + properties[config.IdParameter] = c.OriginObjectId } + + return entities.ResolvedEntity{ + EntityName: externalId, + Coordinate: c.Coordinate, + Properties: properties, + }, nil } //Strategy 2 is to generate external id and either update or create object - _, err := client.Upsert(ctx, externalId, []byte(renderedConfig)) + segmentsResponses, err := client.GetAll(ctx) + if err != nil { + return entities.ResolvedEntity{}, err + } + + var response map[string]interface{} + for _, segmentResponse := range segmentsResponses { + err = json.Unmarshal(segmentResponse.Data, &response) + if err != nil { + return entities.ResolvedEntity{}, err + } + //In case of a match, the put needs additional fields + if response["externalId"] == externalId { + request["uid"] = response["uid"].(string) + request["owner"] = response["owner"].(string) + externalId = response["uid"].(string) + } + } + payload, _ := json.Marshal(request) + _, err = client.Upsert(ctx, externalId, payload) if err != nil { var apiErr api.APIError if errors.As(err, &apiErr) { @@ -72,7 +99,7 @@ func Deploy(ctx context.Context, client DeploySegmentClient, properties paramete return entities.ResolvedEntity{}, deployErrors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert segment with segmentName %q", externalId)).WithError(err) } - // Set this to the upserte id returned by api + properties[config.IdParameter] = externalId return entities.ResolvedEntity{ From bc6835f97e6b5f447ddea5a2b239b51ce259e337 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Tue, 7 Jan 2025 13:23:50 +0100 Subject: [PATCH 03/20] feat: Refactor and improve error handling of segments deploy --- pkg/deploy/internal/segment/segment.go | 134 ++++++++++++++++++------- 1 file changed, 97 insertions(+), 37 deletions(-) diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index d000d021f..89b994cb0 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -29,6 +29,7 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/parameter" deployErrors "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy/errors" "github.com/go-logr/logr" + "net/http" "time" ) @@ -38,73 +39,132 @@ type DeploySegmentClient interface { } func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { - //create new context to carry logger - ctx = logr.NewContext(ctx, log.WithCtxFields(ctx).GetLogr()) - ctx, cancel := context.WithTimeout(ctx, time.Minute) - defer cancel() - - // external id is generated from project-configType-configId - externalId := c.Coordinate.String() - var request map[string]any err := json.Unmarshal([]byte(renderedConfig), &request) + if err != nil { + return entities.ResolvedEntity{}, err + } + + //externalId is generated from [project-configType-configId] + externalId := c.Coordinate.String() request["externalId"] = externalId - //Strategy 1 if OriginObjectId is set check on remote if an object can be found and update it, - //update the externalId to the computed one we generate + //Strategy 1 when OriginObjectId is set we try to update the object first, if the object doesn't exist we create it. if c.OriginObjectId != "" { - request["uid"] = c.OriginObjectId - payload, _ := json.Marshal(request) - responseUpsert, err := client.Upsert(ctx, c.OriginObjectId, payload) + id, err := deployWithOriginObjectId(ctx, client, request, c) if err != nil { - return entities.ResolvedEntity{}, err - } - if responseUpsert.Data == nil { - properties[config.IdParameter] = c.OriginObjectId + return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with originObjectId: %s, with error: %w", c.OriginObjectId, err) } - return entities.ResolvedEntity{ - EntityName: externalId, - Coordinate: c.Coordinate, - Properties: properties, - }, nil + return createResolveEntity(id, externalId, properties, c), nil } - //Strategy 2 is to generate external id and either update or create object + //Strategy 2 is to try to find a match with external id and either update or create object + id, err := deployWithExternalId(ctx, client, request, c) + if err != nil { + return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s, with error: %w", externalId, err) + } + + return createResolveEntity(id, externalId, properties, c), nil +} + +func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config) (string, error) { + request["uid"] = c.OriginObjectId + payload, err := json.Marshal(request) + if err != nil { + return "", fmt.Errorf("failed to marshal segment request: %w", err) + } + + responseUpsert, err := deploy(ctx, client, c.OriginObjectId, payload, c) + if err != nil { + return "", fmt.Errorf("failed API request: %w", err) + } + + //When post was executed we get payload where we read the id from + if responseUpsert.StatusCode == http.StatusCreated { + return extractUidFromResponse(responseUpsert) + } + + return c.OriginObjectId, nil +} + +func deployWithExternalId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config) (string, error) { segmentsResponses, err := client.GetAll(ctx) if err != nil { - return entities.ResolvedEntity{}, err + return "", fmt.Errorf("failed to GET segments: %w", err) } - var response map[string]interface{} + var jsonResponse struct { + Id string `json:"uid"` + Owner string `json:"owner"` + } for _, segmentResponse := range segmentsResponses { - err = json.Unmarshal(segmentResponse.Data, &response) + err = json.Unmarshal(segmentResponse.Data, &jsonResponse) if err != nil { - return entities.ResolvedEntity{}, err + return "", err } //In case of a match, the put needs additional fields - if response["externalId"] == externalId { - request["uid"] = response["uid"].(string) - request["owner"] = response["owner"].(string) - externalId = response["uid"].(string) + if jsonResponse.Id == request["externalId"] { + request["uid"] = jsonResponse.Id + request["owner"] = jsonResponse.Owner + break } } - payload, _ := json.Marshal(request) - _, err = client.Upsert(ctx, externalId, payload) + + payload, err := json.Marshal(request) + if err != nil { + return "", fmt.Errorf("failed to marshal segment request: %w", err) + } + + responseUpsert, err := deploy(ctx, client, jsonResponse.Id, payload, c) + if err != nil { + return "", fmt.Errorf("failed API request: %w", err) + } + + //When post was executed we get payload where we read the id from + if responseUpsert.StatusCode == http.StatusCreated { + return extractUidFromResponse(responseUpsert) + } + + return jsonResponse.Id, nil +} + +func deploy(ctx context.Context, client DeploySegmentClient, id string, payload []byte, c *config.Config) (segment.Response, error) { + //create new context to carry logger + ctx = logr.NewContext(ctx, log.WithCtxFields(ctx).GetLogr()) + ctx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + + responseUpsert, err := client.Upsert(ctx, id, payload) if err != nil { var apiErr api.APIError if errors.As(err, &apiErr) { - return entities.ResolvedEntity{}, fmt.Errorf("failed to upsert segment with segmentName %q: %w", externalId, err) + return api.Response{}, fmt.Errorf("failed to upsert segment with id %q: %w", id, err) } - return entities.ResolvedEntity{}, deployErrors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert segment with segmentName %q", externalId)).WithError(err) + return api.Response{}, deployErrors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert segkent with id %q", id)).WithError(err) } - properties[config.IdParameter] = externalId + return responseUpsert, nil +} +func createResolveEntity(id string, externalId string, properties parameter.Properties, c *config.Config) entities.ResolvedEntity { + properties[config.IdParameter] = id return entities.ResolvedEntity{ EntityName: externalId, Coordinate: c.Coordinate, Properties: properties, - }, nil + } +} + +func extractUidFromResponse(response segment.Response) (string, error) { + var jsonResponse struct { + Id string `json:"uid"` + } + err := json.Unmarshal(response.Data, &jsonResponse) + if err != nil { + return "", fmt.Errorf("failed to unmarshal response: %w", err) + } + + return jsonResponse.Id, nil } From 8c3364ebaad47546747614d024a3be417897065c Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Tue, 7 Jan 2025 14:19:35 +0100 Subject: [PATCH 04/20] feat: Fix issue with originObject id being wrong and matching over externalId --- pkg/client/clientset.go | 1 + pkg/client/dummy_clientset.go | 4 ++++ pkg/deploy/internal/segment/segment.go | 32 ++++++++++++++++++-------- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/pkg/client/clientset.go b/pkg/client/clientset.go index db8738fb7..e7aea73f4 100644 --- a/pkg/client/clientset.go +++ b/pkg/client/clientset.go @@ -171,6 +171,7 @@ type SegmentClient interface { GetAll(ctx context.Context) ([]segments.Response, error) Delete(ctx context.Context, id string) (segments.Response, error) Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) + Get(ctx context.Context, id string) (segments.Response, error) } var DefaultMonacoUserAgent = "Dynatrace Monitoring as Code/" + version.MonitoringAsCode + " " + (runtime.GOOS + " " + runtime.GOARCH) diff --git a/pkg/client/dummy_clientset.go b/pkg/client/dummy_clientset.go index cbe6841b6..6e8bc0c1a 100644 --- a/pkg/client/dummy_clientset.go +++ b/pkg/client/dummy_clientset.go @@ -169,3 +169,7 @@ func (c *DummySegmentsClient) GetAll(_ context.Context) ([]coreapi.Response, err func (c *DummySegmentsClient) Upsert(_ context.Context, _ string, _ []byte) (segments.Response, error) { return segments.Response{}, nil } + +func (c *DummySegmentsClient) Get(_ context.Context, _ string) (segments.Response, error) { + return segments.Response{}, nil +} diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index 89b994cb0..d0b1bfc0b 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -36,6 +36,7 @@ import ( type DeploySegmentClient interface { Upsert(ctx context.Context, id string, data []byte) (segment.Response, error) GetAll(ctx context.Context) ([]segment.Response, error) + Get(ctx context.Context, id string) (segment.Response, error) } func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { @@ -49,18 +50,19 @@ func Deploy(ctx context.Context, client DeploySegmentClient, properties paramete externalId := c.Coordinate.String() request["externalId"] = externalId - //Strategy 1 when OriginObjectId is set we try to update the object first, if the object doesn't exist we create it. + //Strategy 1 when OriginObjectId is set we try to get the object if it exists we update it. if c.OriginObjectId != "" { id, err := deployWithOriginObjectId(ctx, client, request, c) if err != nil { - return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with originObjectId: %s, with error: %w", c.OriginObjectId, err) + return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s, with error: %w", externalId, err) + } + if id != "" { + return createResolveEntity(id, externalId, properties, c), nil } - - return createResolveEntity(id, externalId, properties, c), nil } - //Strategy 2 is to try to find a match with external id and either update or create object - id, err := deployWithExternalId(ctx, client, request, c) + //Strategy 2 is to try to find a match with external id and either update or create object if no match found. + id, err := deployWithExternalId(ctx, client, request, c, externalId) if err != nil { return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s, with error: %w", externalId, err) } @@ -69,6 +71,15 @@ func Deploy(ctx context.Context, client DeploySegmentClient, properties paramete } func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config) (string, error) { + _, err := client.Get(ctx, c.OriginObjectId) + if err != nil { + apiError := api.APIError{} + if errors.As(err, &apiError) && apiError.StatusCode == http.StatusNotFound { + return "", nil + } + return "", fmt.Errorf("failed to fetch segment object: %w", err) + } + request["uid"] = c.OriginObjectId payload, err := json.Marshal(request) if err != nil { @@ -88,15 +99,16 @@ func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, r return c.OriginObjectId, nil } -func deployWithExternalId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config) (string, error) { +func deployWithExternalId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config, externalId string) (string, error) { segmentsResponses, err := client.GetAll(ctx) if err != nil { return "", fmt.Errorf("failed to GET segments: %w", err) } var jsonResponse struct { - Id string `json:"uid"` - Owner string `json:"owner"` + Id string `json:"uid"` + Owner string `json:"owner"` + ExternalId string `json:"externalId"` } for _, segmentResponse := range segmentsResponses { err = json.Unmarshal(segmentResponse.Data, &jsonResponse) @@ -104,7 +116,7 @@ func deployWithExternalId(ctx context.Context, client DeploySegmentClient, reque return "", err } //In case of a match, the put needs additional fields - if jsonResponse.Id == request["externalId"] { + if jsonResponse.ExternalId == request["externalId"] { request["uid"] = jsonResponse.Id request["owner"] = jsonResponse.Owner break From 71214a7991488bc7148c7d308cfaa3b0a27cd4f8 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Tue, 7 Jan 2025 14:40:47 +0100 Subject: [PATCH 05/20] chore: Fix issue after rebase, remove duplicated code --- pkg/client/dummy_clientset.go | 18 ---------- pkg/deploy/deploy.go | 2 +- pkg/deploy/internal/segment/segment.go | 50 +++++++++++++------------- 3 files changed, 25 insertions(+), 45 deletions(-) diff --git a/pkg/client/dummy_clientset.go b/pkg/client/dummy_clientset.go index 6e8bc0c1a..3b78c5b10 100644 --- a/pkg/client/dummy_clientset.go +++ b/pkg/client/dummy_clientset.go @@ -27,7 +27,6 @@ import ( buckets "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" documents "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" openpipeline "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/openpipeline" - segments "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" dtclient "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/client/dtclient" ) @@ -38,7 +37,6 @@ var DummyClientSet = ClientSet{ BucketClient: &DummyBucketClient{}, DocumentClient: &DummyDocumentClient{}, OpenPipelineClient: &DummyOpenPipelineClient{}, - SegmentClient: &DummySegmentsClient{}, } var _ AutomationClient = (*DummyAutomationClient)(nil) @@ -157,19 +155,3 @@ func (c *DummyOpenPipelineClient) GetAll(ctx context.Context) ([]coreapi.Respons func (c *DummyOpenPipelineClient) Update(_ context.Context, _ string, _ []byte) (openpipeline.Response, error) { return openpipeline.Response{}, nil } - -type DummySegmentsClient struct{} - -// GetAll implements GrailFilterSegmentClient -func (c *DummySegmentsClient) GetAll(_ context.Context) ([]coreapi.Response, error) { - panic("unimplemented") -} - -// Upsert implements GrailFilterSegmentClient -func (c *DummySegmentsClient) Upsert(_ context.Context, _ string, _ []byte) (segments.Response, error) { - return segments.Response{}, nil -} - -func (c *DummySegmentsClient) Get(_ context.Context, _ string) (segments.Response, error) { - return segments.Response{}, nil -} diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go index 636892e31..6693b94a6 100644 --- a/pkg/deploy/deploy.go +++ b/pkg/deploy/deploy.go @@ -311,7 +311,7 @@ func deployConfig(ctx context.Context, c *config.Config, clientset *client.Clien } case config.Segment: - if featureflags.Temporary[featureflags.Segments].Enabled() { + if featureflags.Segments.Enabled() { resolvedEntity, deployErr = segment.Deploy(ctx, clientset.SegmentClient, properties, renderedConfig, c) } else { deployErr = fmt.Errorf("unknown config-type (ID: %q)", c.Type.ID()) diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index d0b1bfc0b..68e48730a 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -39,6 +39,12 @@ type DeploySegmentClient interface { Get(ctx context.Context, id string) (segment.Response, error) } +type jsonResponse struct { + UID string `json:"uid"` + Owner string `json:"owner"` + ExternalId string `json:"externalId"` +} + func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { var request map[string]any err := json.Unmarshal([]byte(renderedConfig), &request) @@ -86,16 +92,11 @@ func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, r return "", fmt.Errorf("failed to marshal segment request: %w", err) } - responseUpsert, err := deploy(ctx, client, c.OriginObjectId, payload, c) + _, err = deploy(ctx, client, c.OriginObjectId, payload, c) if err != nil { return "", fmt.Errorf("failed API request: %w", err) } - //When post was executed we get payload where we read the id from - if responseUpsert.StatusCode == http.StatusCreated { - return extractUidFromResponse(responseUpsert) - } - return c.OriginObjectId, nil } @@ -105,20 +106,16 @@ func deployWithExternalId(ctx context.Context, client DeploySegmentClient, reque return "", fmt.Errorf("failed to GET segments: %w", err) } - var jsonResponse struct { - Id string `json:"uid"` - Owner string `json:"owner"` - ExternalId string `json:"externalId"` - } + var responseData jsonResponse for _, segmentResponse := range segmentsResponses { - err = json.Unmarshal(segmentResponse.Data, &jsonResponse) + responseData, err = getJsonResponseFromSegmentsResponse(segmentResponse) if err != nil { return "", err } //In case of a match, the put needs additional fields - if jsonResponse.ExternalId == request["externalId"] { - request["uid"] = jsonResponse.Id - request["owner"] = jsonResponse.Owner + if responseData.ExternalId == request["externalId"] { + request["uid"] = responseData.UID + request["owner"] = responseData.Owner break } } @@ -128,17 +125,20 @@ func deployWithExternalId(ctx context.Context, client DeploySegmentClient, reque return "", fmt.Errorf("failed to marshal segment request: %w", err) } - responseUpsert, err := deploy(ctx, client, jsonResponse.Id, payload, c) + responseUpsert, err := deploy(ctx, client, responseData.UID, payload, c) if err != nil { return "", fmt.Errorf("failed API request: %w", err) } - //When post was executed we get payload where we read the id from + //For a POST we need to parse the response again to read out the ID if responseUpsert.StatusCode == http.StatusCreated { - return extractUidFromResponse(responseUpsert) + responseData, err = getJsonResponseFromSegmentsResponse(responseUpsert) + if err != nil { + return "", err + } } - return jsonResponse.Id, nil + return responseData.UID, nil } func deploy(ctx context.Context, client DeploySegmentClient, id string, payload []byte, c *config.Config) (segment.Response, error) { @@ -169,14 +169,12 @@ func createResolveEntity(id string, externalId string, properties parameter.Prop } } -func extractUidFromResponse(response segment.Response) (string, error) { - var jsonResponse struct { - Id string `json:"uid"` - } - err := json.Unmarshal(response.Data, &jsonResponse) +func getJsonResponseFromSegmentsResponse(rawResponse segment.Response) (jsonResponse, error) { + var response jsonResponse + err := json.Unmarshal(rawResponse.Data, &response) if err != nil { - return "", fmt.Errorf("failed to unmarshal response: %w", err) + return jsonResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) } - return jsonResponse.Id, nil + return response, nil } From 9a908a58b8695d5f16577b417686b4c66387257a Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Wed, 8 Jan 2025 13:12:11 +0100 Subject: [PATCH 06/20] chore: Fix issue after merge of `delete`, where wrong client is used in delete_test.go --- pkg/client/dummy_clientset.go | 24 ++++++++++ pkg/delete/delete_test.go | 86 ++++++++--------------------------- 2 files changed, 44 insertions(+), 66 deletions(-) diff --git a/pkg/client/dummy_clientset.go b/pkg/client/dummy_clientset.go index 3b78c5b10..c04cf3be2 100644 --- a/pkg/client/dummy_clientset.go +++ b/pkg/client/dummy_clientset.go @@ -27,6 +27,7 @@ import ( buckets "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" documents "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" openpipeline "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/openpipeline" + segments "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" dtclient "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/client/dtclient" ) @@ -37,6 +38,7 @@ var DummyClientSet = ClientSet{ BucketClient: &DummyBucketClient{}, DocumentClient: &DummyDocumentClient{}, OpenPipelineClient: &DummyOpenPipelineClient{}, + SegmentClient: &DummySegmentClient{}, } var _ AutomationClient = (*DummyAutomationClient)(nil) @@ -155,3 +157,25 @@ func (c *DummyOpenPipelineClient) GetAll(ctx context.Context) ([]coreapi.Respons func (c *DummyOpenPipelineClient) Update(_ context.Context, _ string, _ []byte) (openpipeline.Response, error) { return openpipeline.Response{}, nil } + +type DummySegmentClient struct{} + +func (c *DummySegmentClient) List(ctx context.Context) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (c *DummySegmentClient) GetAll(ctx context.Context) ([]segments.Response, error) { + return []segments.Response{}, fmt.Errorf("unimplemented") +} + +func (c *DummySegmentClient) Delete(ctx context.Context, id string) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (c *DummySegmentClient) Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (c *DummySegmentClient) Get(ctx context.Context, id string) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} diff --git a/pkg/delete/delete_test.go b/pkg/delete/delete_test.go index 42164c55c..144f430ac 100644 --- a/pkg/delete/delete_test.go +++ b/pkg/delete/delete_test.go @@ -37,7 +37,6 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/automation" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" - "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/idutils" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils/matcher" @@ -1149,93 +1148,48 @@ func TestDelete_Documents(t *testing.T) { }) } -type segmentStubClient struct { - called bool - list func() (segments.Response, error) - getAll func() ([]segments.Response, error) - delete func() (segments.Response, error) -} - -func (c *segmentStubClient) List(_ context.Context) (segments.Response, error) { - return c.list() -} - -func (c *segmentStubClient) GetAll(_ context.Context) ([]segments.Response, error) { - return c.getAll() -} - -func (c *segmentStubClient) Delete(_ context.Context, _ string) (segments.Response, error) { - c.called = true - return c.delete() -} - func TestDelete_Segments(t *testing.T) { - t.Run("simple case", func(t *testing.T) { - t.Setenv(featureflags.Segments.EnvName(), "true") - - c := segmentStubClient{ - delete: func() (segments.Response, error) { - return segments.Response{StatusCode: http.StatusOK}, nil + c := client.DummySegmentClient{} + given := delete.DeleteEntries{ + "segment": { + { + Type: "segment", + OriginObjectId: "originObjectID", }, - } + }, + } + + t.Run("With Enabled Segment FF", func(t *testing.T) { + t.Setenv(featureflags.Segments.EnvName(), "true") - given := delete.DeleteEntries{ - "segment": { - { - Type: "segment", - OriginObjectId: "originObjectID", - }, - }, - } err := delete.Configs(context.TODO(), client.ClientSet{SegmentClient: &c}, given) - assert.NoError(t, err) - assert.True(t, c.called, "delete should have been called") + //DummyClient returns unimplemented error on every execution of any method + assert.Error(t, err, "unimplemented") }) - t.Run("simple case with FF turned off", func(t *testing.T) { + t.Run("With Disabled Segment FF", func(t *testing.T) { t.Setenv(featureflags.Segments.EnvName(), "false") - c := segmentStubClient{} - - given := delete.DeleteEntries{ - "segment": { - { - Type: "segment", - OriginObjectId: "originObjectID", - }, - }, - } err := delete.Configs(context.TODO(), client.ClientSet{SegmentClient: &c}, given) assert.NoError(t, err) - assert.False(t, c.called, "delete should not have been called") }) } func TestDeleteAll_Segments(t *testing.T) { - t.Run("simple case", func(t *testing.T) { - t.Setenv(featureflags.Segments.EnvName(), "true") + c := client.DummySegmentClient{} - c := segmentStubClient{ - list: func() (segments.Response, error) { - return segments.Response{StatusCode: http.StatusOK, Data: []byte(`[{"uid": "uid_1"},{"uid": "uid_2"},{"uid": "uid_3"}]`)}, nil - }, - delete: func() (segments.Response, error) { - return segments.Response{StatusCode: http.StatusOK}, nil - }, - } + t.Run("With Enabled Segment FF", func(t *testing.T) { + t.Setenv(featureflags.Segments.EnvName(), "true") err := delete.All(context.TODO(), client.ClientSet{SegmentClient: &c}, api.APIs{}) - assert.NoError(t, err) - assert.True(t, c.called, "delete should have been called") + //Dummy client returns unimplemented error on every execution of any method + assert.Error(t, err, "unimplemented") }) - t.Run("FF is turned off", func(t *testing.T) { + t.Run("With Disabled Segment FF", func(t *testing.T) { t.Setenv(featureflags.Segments.EnvName(), "false") - c := segmentStubClient{} - err := delete.All(context.TODO(), client.ClientSet{SegmentClient: &c}, api.APIs{}) assert.NoError(t, err) - assert.False(t, c.called, "delete should not have been called") }) } From f3686e578b6383da6329ba7dbb5110900bb12742 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Thu, 9 Jan 2025 18:42:31 +0100 Subject: [PATCH 07/20] feat: Add tests for segments.go --- pkg/deploy/internal/segment/segment.go | 20 ++- pkg/deploy/internal/segment/segment_test.go | 171 ++++++++++++++++++++ 2 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 pkg/deploy/internal/segment/segment_test.go diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index 68e48730a..5353d31d9 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -77,7 +77,7 @@ func Deploy(ctx context.Context, client DeploySegmentClient, properties paramete } func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config) (string, error) { - _, err := client.Get(ctx, c.OriginObjectId) + jsonResponse, err := client.Get(ctx, c.OriginObjectId) if err != nil { apiError := api.APIError{} if errors.As(err, &apiError) && apiError.StatusCode == http.StatusNotFound { @@ -85,6 +85,15 @@ func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, r } return "", fmt.Errorf("failed to fetch segment object: %w", err) } + //Owner field is required in a PUT + value, ok := request["owner"] + if !ok || value == "" { + responseData, err := getJsonResponseFromSegmentsResponse(jsonResponse) + if err != nil { + return "", err + } + request["owner"] = responseData.Owner + } request["uid"] = c.OriginObjectId payload, err := json.Marshal(request) @@ -113,9 +122,12 @@ func deployWithExternalId(ctx context.Context, client DeploySegmentClient, reque return "", err } //In case of a match, the put needs additional fields - if responseData.ExternalId == request["externalId"] { + if responseData.ExternalId == externalId { request["uid"] = responseData.UID - request["owner"] = responseData.Owner + value, ok := request["owner"] + if !ok || value == "" { + request["owner"] = responseData.Owner + } break } } @@ -154,7 +166,7 @@ func deploy(ctx context.Context, client DeploySegmentClient, id string, payload return api.Response{}, fmt.Errorf("failed to upsert segment with id %q: %w", id, err) } - return api.Response{}, deployErrors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert segkent with id %q", id)).WithError(err) + return api.Response{}, deployErrors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert segment with id %q", id)).WithError(err) } return responseUpsert, nil diff --git a/pkg/deploy/internal/segment/segment_test.go b/pkg/deploy/internal/segment/segment_test.go new file mode 100644 index 000000000..8ee4f0854 --- /dev/null +++ b/pkg/deploy/internal/segment/segment_test.go @@ -0,0 +1,171 @@ +/* + * @license + * Copyright 2025 Dynatrace LLC + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package segment_test + +import ( + "context" + "encoding/json" + "fmt" + "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/entities" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/template" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy/internal/segment" + "github.com/stretchr/testify/assert" + "net/http" + "testing" +) + +type testClient struct { + upsertStub func() (segments.Response, error) + getStub func() (segments.Response, error) + getAllStub func() ([]segments.Response, error) +} + +func (tc *testClient) Upsert(_ context.Context, _ string, _ []byte) (segments.Response, error) { + return tc.upsertStub() +} + +func (tc *testClient) Get(_ context.Context, _ string) (segments.Response, error) { + return tc.getStub() +} + +func (tc *testClient) GetAll(_ context.Context) ([]segments.Response, error) { + return tc.getAllStub() +} + +func TestDeploy(t *testing.T) { + testCoordinate := coordinate.Coordinate{ + Project: "my-project", + Type: "segment", + ConfigId: "my-config-id", + } + tests := []struct { + name string + inputConfig config.Config + upsertStub func() (segments.Response, error) + getStub func() (segments.Response, error) + getAllStub func() ([]segments.Response, error) + expected entities.ResolvedEntity + expectErr bool + expectedErrMsg string + }{ + { + name: "deploy with objectOriginId - success PUT", + inputConfig: config.Config{ + Template: template.NewInMemoryTemplate("path/file.json", "{}"), + Coordinate: testCoordinate, + OriginObjectId: "my-object-id", + Type: config.Segment{}, + Parameters: config.Parameters{}, + Skip: false, + }, + upsertStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + }, nil + }, + getStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + Data: marshal(map[string]any{ + "uid": "JMhNaJ0Zbf9", + "name": "test-segment-post-match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "project_segment:segment:some-id", + }), + }, nil + }, + getAllStub: func() ([]segments.Response, error) { + panic("should not be called") + }, + expected: entities.ResolvedEntity{ + EntityName: testCoordinate.String(), + Coordinate: testCoordinate, + Properties: map[string]interface{}{ + "id": "my-object-id", + }, + Skip: false, + }, + expectErr: false, + }, + { + name: "deploy with objectOriginId - error PUT", + inputConfig: config.Config{ + Template: template.NewInMemoryTemplate("path/file.json", "{}"), + Coordinate: testCoordinate, + OriginObjectId: "my-object-id", + Type: config.Segment{}, + Parameters: config.Parameters{}, + Skip: false, + }, + upsertStub: func() (segments.Response, error) { + return segments.Response{}, fmt.Errorf("error") + }, + getStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + Data: marshal(map[string]any{ + "uid": "JMhNaJ0Zbf9", + "name": "test-segment-post-match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "project_segment:segment:some-id", + }), + }, nil + }, + getAllStub: func() ([]segments.Response, error) { + panic("should not be called") + }, + expectErr: true, + expectedErrMsg: "failed to deploy segment with externalId", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := testClient{upsertStub: tt.upsertStub, getStub: tt.getStub} + + props, errs := tt.inputConfig.ResolveParameterValues(entities.New()) + assert.Empty(t, errs) + + renderedConfig, err := tt.inputConfig.Render(props) + assert.NoError(t, err) + + resolvedEntity, err := segment.Deploy(context.Background(), &c, props, renderedConfig, &tt.inputConfig) + if tt.expectErr { + assert.ErrorContains(t, err, tt.expectedErrMsg) + } + if !tt.expectErr { + assert.NoError(t, err) + assert.Equal(t, resolvedEntity, tt.expected) + } + }) + } +} + +func marshal(object map[string]any) []byte { + payload, err := json.Marshal(object) + if err != nil { + panic(err) + } + return payload +} From 942131a1dbe10b9380a2482c2661c8b160a56d54 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Fri, 10 Jan 2025 09:13:49 +0100 Subject: [PATCH 08/20] feat: Add tests for segments.go --- pkg/deploy/internal/segment/segment_test.go | 179 +++++++++++++++++++- 1 file changed, 178 insertions(+), 1 deletion(-) diff --git a/pkg/deploy/internal/segment/segment_test.go b/pkg/deploy/internal/segment/segment_test.go index 8ee4f0854..e7fcd6b93 100644 --- a/pkg/deploy/internal/segment/segment_test.go +++ b/pkg/deploy/internal/segment/segment_test.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/dynatrace/dynatrace-configuration-as-code-core/api" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate" @@ -106,6 +107,80 @@ func TestDeploy(t *testing.T) { }, expectErr: false, }, + { + name: "deploy with objectOriginId, no object found on remote - success POST with externalId", + inputConfig: config.Config{ + Template: template.NewInMemoryTemplate("path/file.json", "{}"), + Coordinate: testCoordinate, + OriginObjectId: "my-object-id", + Type: config.Segment{}, + Parameters: config.Parameters{}, + Skip: false, + }, + upsertStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusCreated, + Data: marshal(map[string]any{ + "uid": "JMhNaJ0Zbf9", + "name": "test-segment-post-match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "project_segment:segment:some-id", + }), + }, nil + }, + getStub: func() (segments.Response, error) { + return segments.Response{}, api.APIError{ + StatusCode: http.StatusNotFound, + } + }, + getAllStub: func() ([]segments.Response, error) { + var response []segments.Response + return response, nil + }, + expected: entities.ResolvedEntity{ + EntityName: testCoordinate.String(), + Coordinate: testCoordinate, + Properties: map[string]interface{}{ + "id": "JMhNaJ0Zbf9", + }, + Skip: false, + }, + expectErr: false, + }, + { + name: "deploy with objectOriginId - error PUT", + inputConfig: config.Config{ + Template: template.NewInMemoryTemplate("path/file.json", "{}"), + Coordinate: testCoordinate, + OriginObjectId: "my-object-id", + Type: config.Segment{}, + Parameters: config.Parameters{}, + Skip: false, + }, + upsertStub: func() (segments.Response, error) { + return segments.Response{}, fmt.Errorf("error") + }, + getStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + Data: marshal(map[string]any{ + "uid": "JMhNaJ0Zbf9", + "name": "test-segment-post-match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "project_segment:segment:some-id", + }), + }, nil + }, + getAllStub: func() ([]segments.Response, error) { + panic("should not be called") + }, + expectErr: true, + expectedErrMsg: "failed to deploy segment with externalId", + }, { name: "deploy with objectOriginId - error PUT", inputConfig: config.Config{ @@ -138,11 +213,113 @@ func TestDeploy(t *testing.T) { expectErr: true, expectedErrMsg: "failed to deploy segment with externalId", }, + { + name: "deploy with externalId - success PUT", + inputConfig: config.Config{ + Template: template.NewInMemoryTemplate("path/file.json", "{}"), + Coordinate: testCoordinate, + Type: config.Segment{}, + Parameters: config.Parameters{}, + Skip: false, + }, + upsertStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + }, nil + }, + getStub: func() (segments.Response, error) { + panic("should not be called") + }, + getAllStub: func() ([]segments.Response, error) { + response := []segments.Response{ + { + StatusCode: http.StatusOK, + Data: marshal(map[string]any{ + "uid": "JMhNaJ0Zbf9", + "name": "no-match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "project_segment:segment:some-id-no-match", + }), + }, + { + StatusCode: http.StatusOK, + Data: marshal(map[string]any{ + "uid": "JMhNaJ0Zbf9", + "name": "match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "my-project:segment:my-config-id", + }), + }, + } + return response, nil + }, + expected: entities.ResolvedEntity{ + EntityName: testCoordinate.String(), + Coordinate: testCoordinate, + Properties: map[string]interface{}{ + "id": "JMhNaJ0Zbf9", + }, + Skip: false, + }, + expectErr: false, + }, + { + name: "deploy with externalId - error PUT 400", + inputConfig: config.Config{ + Template: template.NewInMemoryTemplate("path/file.json", "{}"), + Coordinate: testCoordinate, + Type: config.Segment{}, + Parameters: config.Parameters{}, + Skip: false, + }, + upsertStub: func() (segments.Response, error) { + return segments.Response{}, api.APIError{ + StatusCode: http.StatusBadRequest, + } + }, + getStub: func() (segments.Response, error) { + panic("should not be called") + }, + getAllStub: func() ([]segments.Response, error) { + var response []segments.Response + return response, nil + }, + expectErr: true, + expectedErrMsg: "failed to deploy segment with externalId", + }, + { + name: "deploy with externalId - error GET 400", + inputConfig: config.Config{ + Template: template.NewInMemoryTemplate("path/file.json", "{}"), + Coordinate: testCoordinate, + Type: config.Segment{}, + Parameters: config.Parameters{}, + Skip: false, + }, + upsertStub: func() (segments.Response, error) { + panic("should not be called") + }, + getStub: func() (segments.Response, error) { + panic("should not be called") + }, + getAllStub: func() ([]segments.Response, error) { + var response []segments.Response + return response, api.APIError{ + StatusCode: http.StatusBadRequest, + } + }, + expectErr: true, + expectedErrMsg: "failed to deploy segment with externalId", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := testClient{upsertStub: tt.upsertStub, getStub: tt.getStub} + c := testClient{upsertStub: tt.upsertStub, getStub: tt.getStub, getAllStub: tt.getAllStub} props, errs := tt.inputConfig.ResolveParameterValues(entities.New()) assert.Empty(t, errs) From 1f6fbdecc97f1b2069cadaa072613d60f6ac1ad7 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Fri, 10 Jan 2025 13:33:06 +0100 Subject: [PATCH 09/20] feat: Add tests for segment feature flag in deploy.go --- pkg/deploy/deploy_test.go | 54 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/pkg/deploy/deploy_test.go b/pkg/deploy/deploy_test.go index 2070bfca3..d4452f076 100644 --- a/pkg/deploy/deploy_test.go +++ b/pkg/deploy/deploy_test.go @@ -19,6 +19,7 @@ package deploy_test import ( "context" "fmt" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags" "testing" "github.com/stretchr/testify/assert" @@ -1298,3 +1299,56 @@ func TestDeployConfigGraph_CollectsAllErrors(t *testing.T) { }) } + +func TestDeployConfigFF(t *testing.T) { + dummyClientSet := client.DummyClientSet + c := dynatrace.EnvironmentClients{ + dynatrace.EnvironmentInfo{Name: "env"}: &dummyClientSet, + } + tests := []struct { + name string + projects []project.Project + featureFlag string + configType config.TypeID + expectedErrString string + }{ + { + name: "segments FF test", + projects: []project.Project{ + { + Configs: project.ConfigsPerTypePerEnvironments{ + "env": project.ConfigsPerType{ + "p1": { + config.Config{ + Type: config.Segment{}, + Environment: "env", + Coordinate: coordinate.Coordinate{ + Project: "p1", + Type: "type", + ConfigId: "config1", + }, + }, + }, + }, + }, + }, + }, + featureFlag: featureflags.Segments.EnvName(), + configType: config.SegmentID, + }, + } + + for _, tt := range tests { + t.Run(tt.name+" | FF Enabled", func(t *testing.T) { + t.Setenv(tt.featureFlag, "true") + err := deploy.Deploy(context.TODO(), tt.projects, c, deploy.DeployConfigsOptions{}) + //Dummy client returns unimplemented error on every execution of any method + assert.Errorf(t, err, "unimplemented") + }) + t.Run(tt.name+" | FF Disabled", func(t *testing.T) { + t.Setenv(tt.featureFlag, "false") + err := deploy.Deploy(context.TODO(), tt.projects, c, deploy.DeployConfigsOptions{}) + assert.Errorf(t, err, fmt.Sprintf("unknown config-type (ID: %q)", tt.configType)) + }) + } +} From c43285704175207c69ead753b4248ec2896beab2 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Fri, 17 Jan 2025 14:40:15 +0100 Subject: [PATCH 10/20] chore: update segments deploy after core lib update --- pkg/deploy/deploy_test.go | 7 +- pkg/deploy/internal/segment/segment.go | 131 +++++++++----------- pkg/deploy/internal/segment/segment_test.go | 71 +---------- 3 files changed, 72 insertions(+), 137 deletions(-) diff --git a/pkg/deploy/deploy_test.go b/pkg/deploy/deploy_test.go index d4452f076..ba4c3c0bf 100644 --- a/pkg/deploy/deploy_test.go +++ b/pkg/deploy/deploy_test.go @@ -19,9 +19,10 @@ package deploy_test import ( "context" "fmt" - "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags" "testing" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags" + "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -1341,13 +1342,13 @@ func TestDeployConfigFF(t *testing.T) { for _, tt := range tests { t.Run(tt.name+" | FF Enabled", func(t *testing.T) { t.Setenv(tt.featureFlag, "true") - err := deploy.Deploy(context.TODO(), tt.projects, c, deploy.DeployConfigsOptions{}) + err := deploy.Deploy(context.Background(), tt.projects, c, deploy.DeployConfigsOptions{}) //Dummy client returns unimplemented error on every execution of any method assert.Errorf(t, err, "unimplemented") }) t.Run(tt.name+" | FF Disabled", func(t *testing.T) { t.Setenv(tt.featureFlag, "false") - err := deploy.Deploy(context.TODO(), tt.projects, c, deploy.DeployConfigsOptions{}) + err := deploy.Deploy(context.Background(), tt.projects, c, deploy.DeployConfigsOptions{}) assert.Errorf(t, err, fmt.Sprintf("unknown config-type (ID: %q)", tt.configType)) }) } diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index 5353d31d9..eae28fb7f 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -21,22 +21,23 @@ import ( "encoding/json" "errors" "fmt" + "net/http" + "time" + "github.com/dynatrace/dynatrace-configuration-as-code-core/api" segment "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/idutils" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/log" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/entities" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/parameter" deployErrors "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy/errors" "github.com/go-logr/logr" - "net/http" - "time" ) type DeploySegmentClient interface { Upsert(ctx context.Context, id string, data []byte) (segment.Response, error) GetAll(ctx context.Context) ([]segment.Response, error) - Get(ctx context.Context, id string) (segment.Response, error) } type jsonResponse struct { @@ -46,120 +47,113 @@ type jsonResponse struct { } func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { - var request map[string]any - err := json.Unmarshal([]byte(renderedConfig), &request) + externalId := idutils.GenerateUUIDFromCoordinate(c.Coordinate) + requestPayload, err := addExternalId(externalId, renderedConfig) if err != nil { - return entities.ResolvedEntity{}, err + return entities.ResolvedEntity{}, fmt.Errorf("failed to add externalId to segments request payload: %w", err) } - //externalId is generated from [project-configType-configId] - externalId := c.Coordinate.String() - request["externalId"] = externalId - - //Strategy 1 when OriginObjectId is set we try to get the object if it exists we update it. + //Strategy 1 when OriginObjectId is set we try to get the object if it exists we update it else we create it. if c.OriginObjectId != "" { - id, err := deployWithOriginObjectId(ctx, client, request, c) + id, err := deployWithOriginObjectId(ctx, client, c, requestPayload) if err != nil { - return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s, with error: %w", externalId, err) - } - if id != "" { - return createResolveEntity(id, externalId, properties, c), nil + return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s : %w", externalId, err) } + + return createResolveEntity(id, externalId, properties, c), nil } //Strategy 2 is to try to find a match with external id and either update or create object if no match found. - id, err := deployWithExternalId(ctx, client, request, c, externalId) + id, err := deployWithExternalID(ctx, client, externalId, requestPayload, c) if err != nil { - return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s, with error: %w", externalId, err) + return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s : %w", externalId, err) } return createResolveEntity(id, externalId, properties, c), nil } -func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config) (string, error) { - jsonResponse, err := client.Get(ctx, c.OriginObjectId) +func addExternalId(externalId string, renderedConfig string) ([]byte, error) { + var request map[string]any + err := json.Unmarshal([]byte(renderedConfig), &request) if err != nil { - apiError := api.APIError{} - if errors.As(err, &apiError) && apiError.StatusCode == http.StatusNotFound { - return "", nil - } - return "", fmt.Errorf("failed to fetch segment object: %w", err) + return nil, err } - //Owner field is required in a PUT - value, ok := request["owner"] - if !ok || value == "" { - responseData, err := getJsonResponseFromSegmentsResponse(jsonResponse) - if err != nil { - return "", err - } - request["owner"] = responseData.Owner + request["externalId"] = externalId + return json.Marshal(request) +} + +func deployWithExternalID(ctx context.Context, client DeploySegmentClient, externalId string, requestPayload []byte, c *config.Config) (string, error) { + id := "" + responseData, match, err := findMatchOnRemote(ctx, client, externalId) + if err != nil { + return "", err + } + + if match { + id = responseData.UID } - request["uid"] = c.OriginObjectId - payload, err := json.Marshal(request) + responseUpsert, err := deploy(ctx, client, id, requestPayload, c) if err != nil { - return "", fmt.Errorf("failed to marshal segment request: %w", err) + return "", err } - _, err = deploy(ctx, client, c.OriginObjectId, payload, c) + id, err = resolveIdFromResponse(responseUpsert, id) if err != nil { - return "", fmt.Errorf("failed API request: %w", err) + return "", err } - return c.OriginObjectId, nil + return id, nil } -func deployWithExternalId(ctx context.Context, client DeploySegmentClient, request map[string]any, c *config.Config, externalId string) (string, error) { - segmentsResponses, err := client.GetAll(ctx) +func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, c *config.Config, requestPayload []byte) (string, error) { + responseUpsert, err := deploy(ctx, client, c.OriginObjectId, requestPayload, c) if err != nil { - return "", fmt.Errorf("failed to GET segments: %w", err) + return "", err } - var responseData jsonResponse - for _, segmentResponse := range segmentsResponses { - responseData, err = getJsonResponseFromSegmentsResponse(segmentResponse) + return resolveIdFromResponse(responseUpsert, c.OriginObjectId) +} + +func resolveIdFromResponse(responseUpsert segment.Response, id string) (string, error) { + //For a POST we need to parse the response again to read out the ID + if responseUpsert.StatusCode == http.StatusCreated { + responseData, err := getJsonResponseFromSegmentsResponse(responseUpsert) if err != nil { return "", err } - //In case of a match, the put needs additional fields - if responseData.ExternalId == externalId { - request["uid"] = responseData.UID - value, ok := request["owner"] - if !ok || value == "" { - request["owner"] = responseData.Owner - } - break - } - } - - payload, err := json.Marshal(request) - if err != nil { - return "", fmt.Errorf("failed to marshal segment request: %w", err) + return responseData.UID, nil } + return id, nil +} - responseUpsert, err := deploy(ctx, client, responseData.UID, payload, c) +func findMatchOnRemote(ctx context.Context, client DeploySegmentClient, externalId string) (jsonResponse, bool, error) { + segmentsResponses, err := client.GetAll(ctx) if err != nil { - return "", fmt.Errorf("failed API request: %w", err) + return jsonResponse{}, false, fmt.Errorf("failed to GET segments: %w", err) } - //For a POST we need to parse the response again to read out the ID - if responseUpsert.StatusCode == http.StatusCreated { - responseData, err = getJsonResponseFromSegmentsResponse(responseUpsert) + var responseData jsonResponse + for _, segmentResponse := range segmentsResponses { + responseData, err = getJsonResponseFromSegmentsResponse(segmentResponse) if err != nil { - return "", err + return jsonResponse{}, false, err + } + if responseData.ExternalId == externalId { + return responseData, true, nil } } - return responseData.UID, nil + return jsonResponse{}, false, nil } -func deploy(ctx context.Context, client DeploySegmentClient, id string, payload []byte, c *config.Config) (segment.Response, error) { +func deploy(ctx context.Context, client DeploySegmentClient, id string, requestPayload []byte, c *config.Config) (segment.Response, error) { //create new context to carry logger ctx = logr.NewContext(ctx, log.WithCtxFields(ctx).GetLogr()) ctx, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() - responseUpsert, err := client.Upsert(ctx, id, payload) + responseUpsert, err := client.Upsert(ctx, id, requestPayload) if err != nil { var apiErr api.APIError if errors.As(err, &apiErr) { @@ -175,7 +169,6 @@ func deploy(ctx context.Context, client DeploySegmentClient, id string, payload func createResolveEntity(id string, externalId string, properties parameter.Properties, c *config.Config) entities.ResolvedEntity { properties[config.IdParameter] = id return entities.ResolvedEntity{ - EntityName: externalId, Coordinate: c.Coordinate, Properties: properties, } diff --git a/pkg/deploy/internal/segment/segment_test.go b/pkg/deploy/internal/segment/segment_test.go index e7fcd6b93..57f2a738b 100644 --- a/pkg/deploy/internal/segment/segment_test.go +++ b/pkg/deploy/internal/segment/segment_test.go @@ -20,6 +20,9 @@ import ( "context" "encoding/json" "fmt" + "net/http" + "testing" + "github.com/dynatrace/dynatrace-configuration-as-code-core/api" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config" @@ -28,13 +31,10 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/template" "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy/internal/segment" "github.com/stretchr/testify/assert" - "net/http" - "testing" ) type testClient struct { upsertStub func() (segments.Response, error) - getStub func() (segments.Response, error) getAllStub func() ([]segments.Response, error) } @@ -42,10 +42,6 @@ func (tc *testClient) Upsert(_ context.Context, _ string, _ []byte) (segments.Re return tc.upsertStub() } -func (tc *testClient) Get(_ context.Context, _ string) (segments.Response, error) { - return tc.getStub() -} - func (tc *testClient) GetAll(_ context.Context) ([]segments.Response, error) { return tc.getAllStub() } @@ -60,7 +56,6 @@ func TestDeploy(t *testing.T) { name string inputConfig config.Config upsertStub func() (segments.Response, error) - getStub func() (segments.Response, error) getAllStub func() ([]segments.Response, error) expected entities.ResolvedEntity expectErr bool @@ -81,19 +76,6 @@ func TestDeploy(t *testing.T) { StatusCode: http.StatusOK, }, nil }, - getStub: func() (segments.Response, error) { - return segments.Response{ - StatusCode: http.StatusOK, - Data: marshal(map[string]any{ - "uid": "JMhNaJ0Zbf9", - "name": "test-segment-post-match", - "description": "post - update from monaco - change - 2", - "isPublic": false, - "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", - "externalId": "project_segment:segment:some-id", - }), - }, nil - }, getAllStub: func() ([]segments.Response, error) { panic("should not be called") }, @@ -108,7 +90,7 @@ func TestDeploy(t *testing.T) { expectErr: false, }, { - name: "deploy with objectOriginId, no object found on remote - success POST with externalId", + name: "deploy with objectOriginId, no object found on remote - success POST", inputConfig: config.Config{ Template: template.NewInMemoryTemplate("path/file.json", "{}"), Coordinate: testCoordinate, @@ -130,14 +112,8 @@ func TestDeploy(t *testing.T) { }), }, nil }, - getStub: func() (segments.Response, error) { - return segments.Response{}, api.APIError{ - StatusCode: http.StatusNotFound, - } - }, getAllStub: func() ([]segments.Response, error) { - var response []segments.Response - return response, nil + panic("should not be called") }, expected: entities.ResolvedEntity{ EntityName: testCoordinate.String(), @@ -162,19 +138,6 @@ func TestDeploy(t *testing.T) { upsertStub: func() (segments.Response, error) { return segments.Response{}, fmt.Errorf("error") }, - getStub: func() (segments.Response, error) { - return segments.Response{ - StatusCode: http.StatusOK, - Data: marshal(map[string]any{ - "uid": "JMhNaJ0Zbf9", - "name": "test-segment-post-match", - "description": "post - update from monaco - change - 2", - "isPublic": false, - "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", - "externalId": "project_segment:segment:some-id", - }), - }, nil - }, getAllStub: func() ([]segments.Response, error) { panic("should not be called") }, @@ -194,19 +157,6 @@ func TestDeploy(t *testing.T) { upsertStub: func() (segments.Response, error) { return segments.Response{}, fmt.Errorf("error") }, - getStub: func() (segments.Response, error) { - return segments.Response{ - StatusCode: http.StatusOK, - Data: marshal(map[string]any{ - "uid": "JMhNaJ0Zbf9", - "name": "test-segment-post-match", - "description": "post - update from monaco - change - 2", - "isPublic": false, - "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", - "externalId": "project_segment:segment:some-id", - }), - }, nil - }, getAllStub: func() ([]segments.Response, error) { panic("should not be called") }, @@ -227,9 +177,6 @@ func TestDeploy(t *testing.T) { StatusCode: http.StatusOK, }, nil }, - getStub: func() (segments.Response, error) { - panic("should not be called") - }, getAllStub: func() ([]segments.Response, error) { response := []segments.Response{ { @@ -281,9 +228,6 @@ func TestDeploy(t *testing.T) { StatusCode: http.StatusBadRequest, } }, - getStub: func() (segments.Response, error) { - panic("should not be called") - }, getAllStub: func() ([]segments.Response, error) { var response []segments.Response return response, nil @@ -303,9 +247,6 @@ func TestDeploy(t *testing.T) { upsertStub: func() (segments.Response, error) { panic("should not be called") }, - getStub: func() (segments.Response, error) { - panic("should not be called") - }, getAllStub: func() ([]segments.Response, error) { var response []segments.Response return response, api.APIError{ @@ -319,7 +260,7 @@ func TestDeploy(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := testClient{upsertStub: tt.upsertStub, getStub: tt.getStub, getAllStub: tt.getAllStub} + c := testClient{upsertStub: tt.upsertStub, getAllStub: tt.getAllStub} props, errs := tt.inputConfig.ResolveParameterValues(entities.New()) assert.Empty(t, errs) From 5c26976d8147447f885e120f8063056cc6d2fd38 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Fri, 17 Jan 2025 16:03:25 +0100 Subject: [PATCH 11/20] chore: fix tests --- pkg/deploy/internal/segment/segment_test.go | 43 ++++++++++++--------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/pkg/deploy/internal/segment/segment_test.go b/pkg/deploy/internal/segment/segment_test.go index 57f2a738b..33802143a 100644 --- a/pkg/deploy/internal/segment/segment_test.go +++ b/pkg/deploy/internal/segment/segment_test.go @@ -77,10 +77,10 @@ func TestDeploy(t *testing.T) { }, nil }, getAllStub: func() ([]segments.Response, error) { - panic("should not be called") + t.Fatalf("should not be called") + return nil, nil }, expected: entities.ResolvedEntity{ - EntityName: testCoordinate.String(), Coordinate: testCoordinate, Properties: map[string]interface{}{ "id": "my-object-id", @@ -109,14 +109,14 @@ func TestDeploy(t *testing.T) { "isPublic": false, "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", "externalId": "project_segment:segment:some-id", - }), + }, t), }, nil }, getAllStub: func() ([]segments.Response, error) { - panic("should not be called") + t.Fatalf("should not be called") + return nil, nil }, expected: entities.ResolvedEntity{ - EntityName: testCoordinate.String(), Coordinate: testCoordinate, Properties: map[string]interface{}{ "id": "JMhNaJ0Zbf9", @@ -126,7 +126,7 @@ func TestDeploy(t *testing.T) { expectErr: false, }, { - name: "deploy with objectOriginId - error PUT", + name: "deploy with objectOriginId - error PUT(error returned by upsert)", inputConfig: config.Config{ Template: template.NewInMemoryTemplate("path/file.json", "{}"), Coordinate: testCoordinate, @@ -139,13 +139,14 @@ func TestDeploy(t *testing.T) { return segments.Response{}, fmt.Errorf("error") }, getAllStub: func() ([]segments.Response, error) { - panic("should not be called") + t.Fatalf("should not be called") + return nil, nil }, expectErr: true, expectedErrMsg: "failed to deploy segment with externalId", }, { - name: "deploy with objectOriginId - error PUT", + name: "deploy with objectOriginId - error PUT(invalid response payload)", inputConfig: config.Config{ Template: template.NewInMemoryTemplate("path/file.json", "{}"), Coordinate: testCoordinate, @@ -155,10 +156,14 @@ func TestDeploy(t *testing.T) { Skip: false, }, upsertStub: func() (segments.Response, error) { - return segments.Response{}, fmt.Errorf("error") + return segments.Response{ + StatusCode: http.StatusCreated, + Data: []byte("invalid json"), + }, nil }, getAllStub: func() ([]segments.Response, error) { - panic("should not be called") + t.Fatalf("should not be called") + return nil, nil }, expectErr: true, expectedErrMsg: "failed to deploy segment with externalId", @@ -187,25 +192,24 @@ func TestDeploy(t *testing.T) { "description": "post - update from monaco - change - 2", "isPublic": false, "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", - "externalId": "project_segment:segment:some-id-no-match", - }), + "externalId": "e2320031-d6c6-3c83-9706-b3e82b834129", + }, t), }, { StatusCode: http.StatusOK, Data: marshal(map[string]any{ - "uid": "JMhNaJ0Zbf9", + "uid": "should-not-be-this-id", "name": "match", "description": "post - update from monaco - change - 2", "isPublic": false, "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", - "externalId": "my-project:segment:my-config-id", - }), + "externalId": "not-a-match", + }, t), }, } return response, nil }, expected: entities.ResolvedEntity{ - EntityName: testCoordinate.String(), Coordinate: testCoordinate, Properties: map[string]interface{}{ "id": "JMhNaJ0Zbf9", @@ -245,7 +249,8 @@ func TestDeploy(t *testing.T) { Skip: false, }, upsertStub: func() (segments.Response, error) { - panic("should not be called") + t.Fatalf("should not be called") + return segments.Response{}, nil }, getAllStub: func() ([]segments.Response, error) { var response []segments.Response @@ -280,10 +285,10 @@ func TestDeploy(t *testing.T) { } } -func marshal(object map[string]any) []byte { +func marshal(object map[string]any, t *testing.T) []byte { payload, err := json.Marshal(object) if err != nil { - panic(err) + t.Fatalf("error marshalling object: %v", err) } return payload } From 91837cdf1246bfbb1527f41a57e063ebd573da16 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Fri, 17 Jan 2025 16:20:10 +0100 Subject: [PATCH 12/20] chore: fix unused variable --- pkg/deploy/internal/segment/segment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index eae28fb7f..b549e87d1 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -166,7 +166,7 @@ func deploy(ctx context.Context, client DeploySegmentClient, id string, requestP return responseUpsert, nil } -func createResolveEntity(id string, externalId string, properties parameter.Properties, c *config.Config) entities.ResolvedEntity { +func createResolveEntity(id string, properties parameter.Properties, c *config.Config) entities.ResolvedEntity { properties[config.IdParameter] = id return entities.ResolvedEntity{ Coordinate: c.Coordinate, From 4aa32ff02f8b250a8c4a82bf6ef1eca6414a2363 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Fri, 17 Jan 2025 16:20:36 +0100 Subject: [PATCH 13/20] chore: fix unused variable --- pkg/deploy/internal/segment/segment.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index b549e87d1..f2c346bc0 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -60,7 +60,7 @@ func Deploy(ctx context.Context, client DeploySegmentClient, properties paramete return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s : %w", externalId, err) } - return createResolveEntity(id, externalId, properties, c), nil + return createResolveEntity(id, properties, c), nil } //Strategy 2 is to try to find a match with external id and either update or create object if no match found. @@ -69,7 +69,7 @@ func Deploy(ctx context.Context, client DeploySegmentClient, properties paramete return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s : %w", externalId, err) } - return createResolveEntity(id, externalId, properties, c), nil + return createResolveEntity(id, properties, c), nil } func addExternalId(externalId string, renderedConfig string) ([]byte, error) { From 5b46a35ee7a2509be2600fce1bf368b26e807ddc Mon Sep 17 00:00:00 2001 From: David Laubreiter Date: Tue, 21 Jan 2025 16:46:09 +0100 Subject: [PATCH 14/20] refactor: Split up DummyClient & fakeClient for testing --- pkg/client/dummy_clientset.go | 12 ++++++------ pkg/delete/delete_test.go | 29 +++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/pkg/client/dummy_clientset.go b/pkg/client/dummy_clientset.go index c04cf3be2..a0fa12956 100644 --- a/pkg/client/dummy_clientset.go +++ b/pkg/client/dummy_clientset.go @@ -17,18 +17,18 @@ package client import ( - context "context" + "context" "fmt" "net/http" coreapi "github.com/dynatrace/dynatrace-configuration-as-code-core/api" automationApi "github.com/dynatrace/dynatrace-configuration-as-code-core/api/clients/automation" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/automation" - buckets "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" - documents "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" - openpipeline "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/openpipeline" - segments "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" - dtclient "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/client/dtclient" + "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" + "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" + "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/openpipeline" + "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" + "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/client/dtclient" ) var DummyClientSet = ClientSet{ diff --git a/pkg/delete/delete_test.go b/pkg/delete/delete_test.go index 144f430ac..848f6630d 100644 --- a/pkg/delete/delete_test.go +++ b/pkg/delete/delete_test.go @@ -37,6 +37,7 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/automation" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" + "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/idutils" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils/matcher" @@ -1148,8 +1149,32 @@ func TestDelete_Documents(t *testing.T) { }) } +type fakeSegmentsClient struct{} + +var _ client.SegmentClient = (*fakeSegmentsClient)(nil) + +func (fakeSegmentsClient) List(ctx context.Context) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (fakeSegmentsClient) GetAll(ctx context.Context) ([]segments.Response, error) { + return []segments.Response{}, fmt.Errorf("unimplemented") +} + +func (fakeSegmentsClient) Delete(ctx context.Context, id string) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (fakeSegmentsClient) Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (fakeSegmentsClient) Get(ctx context.Context, id string) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + func TestDelete_Segments(t *testing.T) { - c := client.DummySegmentClient{} + c := fakeSegmentsClient{} given := delete.DeleteEntries{ "segment": { { @@ -1176,7 +1201,7 @@ func TestDelete_Segments(t *testing.T) { } func TestDeleteAll_Segments(t *testing.T) { - c := client.DummySegmentClient{} + c := fakeSegmentsClient{} t.Run("With Enabled Segment FF", func(t *testing.T) { t.Setenv(featureflags.Segments.EnvName(), "true") From 2bb60765866187de8350ab70d722ad30a56296ec Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Wed, 22 Jan 2025 09:06:29 +0100 Subject: [PATCH 15/20] chore: move `TestSegmentsClient` into separate file and reuse it in deploy and delete --- pkg/client/test_clientset.go | 46 ++++++++++++++++++++++++++++++++++++ pkg/delete/delete_test.go | 29 ++--------------------- pkg/deploy/deploy_test.go | 2 +- 3 files changed, 49 insertions(+), 28 deletions(-) create mode 100644 pkg/client/test_clientset.go diff --git a/pkg/client/test_clientset.go b/pkg/client/test_clientset.go new file mode 100644 index 000000000..790386e28 --- /dev/null +++ b/pkg/client/test_clientset.go @@ -0,0 +1,46 @@ +/* + * @license + * Copyright 2025 Dynatrace LLC + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package client + +import ( + "context" + "fmt" + + "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" +) + +type TestSegmentsClient struct{} + +func (TestSegmentsClient) List(ctx context.Context) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (TestSegmentsClient) GetAll(ctx context.Context) ([]segments.Response, error) { + return []segments.Response{}, fmt.Errorf("unimplemented") +} + +func (TestSegmentsClient) Delete(ctx context.Context, id string) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (TestSegmentsClient) Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} + +func (TestSegmentsClient) Get(ctx context.Context, id string) (segments.Response, error) { + return segments.Response{}, fmt.Errorf("unimplemented") +} diff --git a/pkg/delete/delete_test.go b/pkg/delete/delete_test.go index 848f6630d..a030ac47b 100644 --- a/pkg/delete/delete_test.go +++ b/pkg/delete/delete_test.go @@ -37,7 +37,6 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/automation" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" - "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/segments" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/idutils" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils/matcher" @@ -1149,32 +1148,8 @@ func TestDelete_Documents(t *testing.T) { }) } -type fakeSegmentsClient struct{} - -var _ client.SegmentClient = (*fakeSegmentsClient)(nil) - -func (fakeSegmentsClient) List(ctx context.Context) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") -} - -func (fakeSegmentsClient) GetAll(ctx context.Context) ([]segments.Response, error) { - return []segments.Response{}, fmt.Errorf("unimplemented") -} - -func (fakeSegmentsClient) Delete(ctx context.Context, id string) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") -} - -func (fakeSegmentsClient) Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") -} - -func (fakeSegmentsClient) Get(ctx context.Context, id string) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") -} - func TestDelete_Segments(t *testing.T) { - c := fakeSegmentsClient{} + c := client.TestSegmentsClient{} given := delete.DeleteEntries{ "segment": { { @@ -1201,7 +1176,7 @@ func TestDelete_Segments(t *testing.T) { } func TestDeleteAll_Segments(t *testing.T) { - c := fakeSegmentsClient{} + c := client.TestSegmentsClient{} t.Run("With Enabled Segment FF", func(t *testing.T) { t.Setenv(featureflags.Segments.EnvName(), "true") diff --git a/pkg/deploy/deploy_test.go b/pkg/deploy/deploy_test.go index ba4c3c0bf..24ac50a20 100644 --- a/pkg/deploy/deploy_test.go +++ b/pkg/deploy/deploy_test.go @@ -1302,7 +1302,7 @@ func TestDeployConfigGraph_CollectsAllErrors(t *testing.T) { } func TestDeployConfigFF(t *testing.T) { - dummyClientSet := client.DummyClientSet + dummyClientSet := client.ClientSet{SegmentClient: client.TestSegmentsClient{}} c := dynatrace.EnvironmentClients{ dynatrace.EnvironmentInfo{Name: "env"}: &dummyClientSet, } From 25a2958c37a3801b84e3f78eacddfa7a77da3f73 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Wed, 22 Jan 2025 09:07:05 +0100 Subject: [PATCH 16/20] chore: fix dry-run for `segments` and introduce unit test for it --- pkg/client/dummy_clientset.go | 20 ++++++++++---------- pkg/deploy/deploy_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/pkg/client/dummy_clientset.go b/pkg/client/dummy_clientset.go index a0fa12956..388b2b177 100644 --- a/pkg/client/dummy_clientset.go +++ b/pkg/client/dummy_clientset.go @@ -160,22 +160,22 @@ func (c *DummyOpenPipelineClient) Update(_ context.Context, _ string, _ []byte) type DummySegmentClient struct{} -func (c *DummySegmentClient) List(ctx context.Context) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") +func (c *DummySegmentClient) List(_ context.Context) (segments.Response, error) { + return segments.Response{}, nil } -func (c *DummySegmentClient) GetAll(ctx context.Context) ([]segments.Response, error) { - return []segments.Response{}, fmt.Errorf("unimplemented") +func (c *DummySegmentClient) GetAll(_ context.Context) ([]segments.Response, error) { + return []segments.Response{}, nil } -func (c *DummySegmentClient) Delete(ctx context.Context, id string) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") +func (c *DummySegmentClient) Delete(_ context.Context, _ string) (segments.Response, error) { + return segments.Response{}, nil } -func (c *DummySegmentClient) Upsert(ctx context.Context, id string, data []byte) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") +func (c *DummySegmentClient) Upsert(_ context.Context, _ string, _ []byte) (segments.Response, error) { + return segments.Response{}, nil } -func (c *DummySegmentClient) Get(ctx context.Context, id string) (segments.Response, error) { - return segments.Response{}, fmt.Errorf("unimplemented") +func (c *DummySegmentClient) Get(_ context.Context, _ string) (segments.Response, error) { + return segments.Response{}, nil } diff --git a/pkg/deploy/deploy_test.go b/pkg/deploy/deploy_test.go index 24ac50a20..a6402e67d 100644 --- a/pkg/deploy/deploy_test.go +++ b/pkg/deploy/deploy_test.go @@ -1353,3 +1353,34 @@ func TestDeployConfigFF(t *testing.T) { }) } } + +func TestDeployDryRun(t *testing.T) { + c := dynatrace.EnvironmentClients{ + dynatrace.EnvironmentInfo{Name: "env", Group: "group"}: &client.DummyClientSet, + } + projects := []project.Project{ + { + Configs: project.ConfigsPerTypePerEnvironments{ + "env": project.ConfigsPerType{ + "p1": { + config.Config{ + Type: config.Segment{}, + Environment: "env", + Coordinate: coordinate.Coordinate{ + Project: "p1", + Type: "segment", + ConfigId: "config1", + }, + Template: testutils.GenerateDummyTemplate(t), + }, + }, + }, + }, + }, + } + t.Setenv(featureflags.Segments.EnvName(), "true") + t.Run("dry-run", func(t *testing.T) { + err := deploy.Deploy(context.Background(), projects, c, deploy.DeployConfigsOptions{DryRun: true}) + assert.Empty(t, err) + }) +} From 55a82346e20980c214f83bf714a4c9a9213cdf78 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Thu, 23 Jan 2025 08:58:47 +0100 Subject: [PATCH 17/20] chore: switch externalId generator --- pkg/deploy/internal/segment/segment.go | 5 ++++- pkg/deploy/internal/segment/segment_test.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index f2c346bc0..8460afbe2 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -47,7 +47,10 @@ type jsonResponse struct { } func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { - externalId := idutils.GenerateUUIDFromCoordinate(c.Coordinate) + externalId, err := idutils.GenerateExternalIDForDocument(c.Coordinate) + if err != nil { + return entities.ResolvedEntity{}, err + } requestPayload, err := addExternalId(externalId, renderedConfig) if err != nil { return entities.ResolvedEntity{}, fmt.Errorf("failed to add externalId to segments request payload: %w", err) diff --git a/pkg/deploy/internal/segment/segment_test.go b/pkg/deploy/internal/segment/segment_test.go index 33802143a..a169e97c2 100644 --- a/pkg/deploy/internal/segment/segment_test.go +++ b/pkg/deploy/internal/segment/segment_test.go @@ -192,7 +192,7 @@ func TestDeploy(t *testing.T) { "description": "post - update from monaco - change - 2", "isPublic": false, "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", - "externalId": "e2320031-d6c6-3c83-9706-b3e82b834129", + "externalId": "monaco-e2320031-d6c6-3c83-9706-b3e82b834129", }, t), }, { From b52f10c3b42f47f82ff8eb02c035f1ae967d3e72 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Thu, 23 Jan 2025 09:22:37 +0100 Subject: [PATCH 18/20] chore: fix deploySegmentClient interface to not be exported --- pkg/deploy/internal/segment/segment.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index 8460afbe2..ebf6b7209 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -35,7 +35,7 @@ import ( "github.com/go-logr/logr" ) -type DeploySegmentClient interface { +type deploySegmentClient interface { Upsert(ctx context.Context, id string, data []byte) (segment.Response, error) GetAll(ctx context.Context) ([]segment.Response, error) } @@ -46,7 +46,7 @@ type jsonResponse struct { ExternalId string `json:"externalId"` } -func Deploy(ctx context.Context, client DeploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { +func Deploy(ctx context.Context, client deploySegmentClient, properties parameter.Properties, renderedConfig string, c *config.Config) (entities.ResolvedEntity, error) { externalId, err := idutils.GenerateExternalIDForDocument(c.Coordinate) if err != nil { return entities.ResolvedEntity{}, err @@ -85,7 +85,7 @@ func addExternalId(externalId string, renderedConfig string) ([]byte, error) { return json.Marshal(request) } -func deployWithExternalID(ctx context.Context, client DeploySegmentClient, externalId string, requestPayload []byte, c *config.Config) (string, error) { +func deployWithExternalID(ctx context.Context, client deploySegmentClient, externalId string, requestPayload []byte, c *config.Config) (string, error) { id := "" responseData, match, err := findMatchOnRemote(ctx, client, externalId) if err != nil { @@ -109,7 +109,7 @@ func deployWithExternalID(ctx context.Context, client DeploySegmentClient, exter return id, nil } -func deployWithOriginObjectId(ctx context.Context, client DeploySegmentClient, c *config.Config, requestPayload []byte) (string, error) { +func deployWithOriginObjectId(ctx context.Context, client deploySegmentClient, c *config.Config, requestPayload []byte) (string, error) { responseUpsert, err := deploy(ctx, client, c.OriginObjectId, requestPayload, c) if err != nil { return "", err @@ -130,7 +130,7 @@ func resolveIdFromResponse(responseUpsert segment.Response, id string) (string, return id, nil } -func findMatchOnRemote(ctx context.Context, client DeploySegmentClient, externalId string) (jsonResponse, bool, error) { +func findMatchOnRemote(ctx context.Context, client deploySegmentClient, externalId string) (jsonResponse, bool, error) { segmentsResponses, err := client.GetAll(ctx) if err != nil { return jsonResponse{}, false, fmt.Errorf("failed to GET segments: %w", err) @@ -150,7 +150,7 @@ func findMatchOnRemote(ctx context.Context, client DeploySegmentClient, external return jsonResponse{}, false, nil } -func deploy(ctx context.Context, client DeploySegmentClient, id string, requestPayload []byte, c *config.Config) (segment.Response, error) { +func deploy(ctx context.Context, client deploySegmentClient, id string, requestPayload []byte, c *config.Config) (segment.Response, error) { //create new context to carry logger ctx = logr.NewContext(ctx, log.WithCtxFields(ctx).GetLogr()) ctx, cancel := context.WithTimeout(ctx, time.Minute) From 150f36a29dd0373dbbf5f23734e6c4d99f955eb9 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Thu, 23 Jan 2025 13:13:11 +0100 Subject: [PATCH 19/20] fix: introduce a `get` call before upsert for deployment strategy in segments --- pkg/deploy/internal/segment/segment.go | 16 ++++-- pkg/deploy/internal/segment/segment_test.go | 59 +++++++++++++++++++-- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/pkg/deploy/internal/segment/segment.go b/pkg/deploy/internal/segment/segment.go index ebf6b7209..ef5d39483 100644 --- a/pkg/deploy/internal/segment/segment.go +++ b/pkg/deploy/internal/segment/segment.go @@ -38,6 +38,7 @@ import ( type deploySegmentClient interface { Upsert(ctx context.Context, id string, data []byte) (segment.Response, error) GetAll(ctx context.Context) ([]segment.Response, error) + Get(ctx context.Context, id string) (segment.Response, error) } type jsonResponse struct { @@ -58,12 +59,21 @@ func Deploy(ctx context.Context, client deploySegmentClient, properties paramete //Strategy 1 when OriginObjectId is set we try to get the object if it exists we update it else we create it. if c.OriginObjectId != "" { - id, err := deployWithOriginObjectId(ctx, client, c, requestPayload) + //@TODO this here is temporary code to enable deploy for segments + getResponse, err := client.Get(ctx, c.OriginObjectId) if err != nil { - return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s : %w", externalId, err) + return entities.ResolvedEntity{}, err } - return createResolveEntity(id, properties, c), nil + if getResponse.StatusCode != http.StatusNotFound { + + id, err := deployWithOriginObjectId(ctx, client, c, requestPayload) + if err != nil { + return entities.ResolvedEntity{}, fmt.Errorf("failed to deploy segment with externalId: %s : %w", externalId, err) + } + + return createResolveEntity(id, properties, c), nil + } } //Strategy 2 is to try to find a match with external id and either update or create object if no match found. diff --git a/pkg/deploy/internal/segment/segment_test.go b/pkg/deploy/internal/segment/segment_test.go index a169e97c2..965bc8441 100644 --- a/pkg/deploy/internal/segment/segment_test.go +++ b/pkg/deploy/internal/segment/segment_test.go @@ -36,6 +36,7 @@ import ( type testClient struct { upsertStub func() (segments.Response, error) getAllStub func() ([]segments.Response, error) + getStub func() (segments.Response, error) } func (tc *testClient) Upsert(_ context.Context, _ string, _ []byte) (segments.Response, error) { @@ -46,6 +47,10 @@ func (tc *testClient) GetAll(_ context.Context) ([]segments.Response, error) { return tc.getAllStub() } +func (tc *testClient) Get(_ context.Context, _ string) (segments.Response, error) { + return tc.getStub() +} + func TestDeploy(t *testing.T) { testCoordinate := coordinate.Coordinate{ Project: "my-project", @@ -56,6 +61,7 @@ func TestDeploy(t *testing.T) { name string inputConfig config.Config upsertStub func() (segments.Response, error) + getStub func() (segments.Response, error) getAllStub func() ([]segments.Response, error) expected entities.ResolvedEntity expectErr bool @@ -76,6 +82,11 @@ func TestDeploy(t *testing.T) { StatusCode: http.StatusOK, }, nil }, + getStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + }, nil + }, getAllStub: func() ([]segments.Response, error) { t.Fatalf("should not be called") return nil, nil @@ -90,7 +101,7 @@ func TestDeploy(t *testing.T) { expectErr: false, }, { - name: "deploy with objectOriginId, no object found on remote - success POST", + name: "deploy with objectOriginId, no object found on remote - success PUT wia externalId", inputConfig: config.Config{ Template: template.NewInMemoryTemplate("path/file.json", "{}"), Coordinate: testCoordinate, @@ -108,13 +119,41 @@ func TestDeploy(t *testing.T) { "description": "post - update from monaco - change - 2", "isPublic": false, "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", - "externalId": "project_segment:segment:some-id", + "externalId": "monaco-e2320031-d6c6-3c83-9706-b3e82b834129", }, t), }, nil }, + getStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusNotFound, + }, nil + }, getAllStub: func() ([]segments.Response, error) { - t.Fatalf("should not be called") - return nil, nil + response := []segments.Response{ + { + StatusCode: http.StatusOK, + Data: marshal(map[string]any{ + "uid": "JMhNaJ0Zbf9", + "name": "no-match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "monaco-e2320031-d6c6-3c83-9706-b3e82b834129", + }, t), + }, + { + StatusCode: http.StatusOK, + Data: marshal(map[string]any{ + "uid": "should-not-be-this-id", + "name": "match", + "description": "post - update from monaco - change - 2", + "isPublic": false, + "owner": "79a4c92e-379b-4cd7-96a3-78a601b6a69b", + "externalId": "not-a-match", + }, t), + }, + } + return response, nil }, expected: entities.ResolvedEntity{ Coordinate: testCoordinate, @@ -135,6 +174,11 @@ func TestDeploy(t *testing.T) { Parameters: config.Parameters{}, Skip: false, }, + getStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + }, nil + }, upsertStub: func() (segments.Response, error) { return segments.Response{}, fmt.Errorf("error") }, @@ -161,6 +205,11 @@ func TestDeploy(t *testing.T) { Data: []byte("invalid json"), }, nil }, + getStub: func() (segments.Response, error) { + return segments.Response{ + StatusCode: http.StatusOK, + }, nil + }, getAllStub: func() ([]segments.Response, error) { t.Fatalf("should not be called") return nil, nil @@ -265,7 +314,7 @@ func TestDeploy(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := testClient{upsertStub: tt.upsertStub, getAllStub: tt.getAllStub} + c := testClient{upsertStub: tt.upsertStub, getAllStub: tt.getAllStub, getStub: tt.getStub} props, errs := tt.inputConfig.ResolveParameterValues(entities.New()) assert.Empty(t, errs) From 2d7baa24f805910505b64b3ff00c07136a1e8ab1 Mon Sep 17 00:00:00 2001 From: "tomaz.puhar" Date: Thu, 23 Jan 2025 13:14:20 +0100 Subject: [PATCH 20/20] fix: fix typo in comment --- pkg/delete/delete_test.go | 2 +- pkg/deploy/deploy_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/delete/delete_test.go b/pkg/delete/delete_test.go index a030ac47b..db19b156f 100644 --- a/pkg/delete/delete_test.go +++ b/pkg/delete/delete_test.go @@ -1182,7 +1182,7 @@ func TestDeleteAll_Segments(t *testing.T) { t.Setenv(featureflags.Segments.EnvName(), "true") err := delete.All(context.TODO(), client.ClientSet{SegmentClient: &c}, api.APIs{}) - //Dummy client returns unimplemented error on every execution of any method + //fakeClient returns unimplemented error on every execution of any method assert.Error(t, err, "unimplemented") }) diff --git a/pkg/deploy/deploy_test.go b/pkg/deploy/deploy_test.go index a6402e67d..c93c638a1 100644 --- a/pkg/deploy/deploy_test.go +++ b/pkg/deploy/deploy_test.go @@ -1343,7 +1343,7 @@ func TestDeployConfigFF(t *testing.T) { t.Run(tt.name+" | FF Enabled", func(t *testing.T) { t.Setenv(tt.featureFlag, "true") err := deploy.Deploy(context.Background(), tt.projects, c, deploy.DeployConfigsOptions{}) - //Dummy client returns unimplemented error on every execution of any method + //fakeClient returns unimplemented error on every execution of any method assert.Errorf(t, err, "unimplemented") }) t.Run(tt.name+" | FF Disabled", func(t *testing.T) {