From 2711915843b15262128260265980017919200769 Mon Sep 17 00:00:00 2001 From: josvaz Date: Fri, 12 Jul 2024 12:43:06 +0200 Subject: [PATCH] Draft triage (#1684) Signed-off-by: jose.vazquez --- Makefile | 3 +- internal/mocks/atlas/integrations.go | 35 +++ pkg/controller/atlasproject/integrations.go | 55 +--- .../atlasproject/integrations_test.go | 296 +++++++++++++++++- 4 files changed, 347 insertions(+), 42 deletions(-) create mode 100644 internal/mocks/atlas/integrations.go diff --git a/Makefile b/Makefile index 92b0380fd9..d0e83c4248 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,7 @@ GOMOD_LICENSES_SHA := $(shell cat $(LICENSES_GOMOD_SHA_FILE)) OPERATOR_NAMESPACE=atlas-operator OPERATOR_POD_NAME=mongodb-atlas-operator RUN_YAML= # Set to the YAML to run when calling make run +RUN_LOG_LEVEL ?= debug LOCAL_IMAGE=mongodb-atlas-kubernetes-operator:compiled CONTAINER_SPEC=.spec.template.spec.containers[0] @@ -533,7 +534,7 @@ ifdef RUN_YAML endif OPERATOR_POD_NAME=$(OPERATOR_POD_NAME) \ OPERATOR_NAMESPACE=$(OPERATOR_NAMESPACE) \ - bin/manager --object-deletion-protection=false --log-level=debug \ + bin/manager --object-deletion-protection=false --log-level=$(RUN_LOG_LEVEL) \ --atlas-domain=$(ATLAS_DOMAIN) \ --global-api-secret-name=$(ATLAS_KEY_SECRET_NAME) diff --git a/internal/mocks/atlas/integrations.go b/internal/mocks/atlas/integrations.go new file mode 100644 index 0000000000..f4035fb09b --- /dev/null +++ b/internal/mocks/atlas/integrations.go @@ -0,0 +1,35 @@ +package atlas + +import ( + "context" + + "go.mongodb.org/atlas/mongodbatlas" +) + +type IntegrationsMock struct { + CreateFunc func(ctx context.Context, projectID string, integrationType string, integration *mongodbatlas.ThirdPartyIntegration) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) + ReplaceFunc func(ctx context.Context, projectID string, integrationType string, integration *mongodbatlas.ThirdPartyIntegration) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) + DeleteFunc func(ctx context.Context, projectID string, integrationType string) (*mongodbatlas.Response, error) + GetFunc func(ctx context.Context, projectID string, integrationType string) (*mongodbatlas.ThirdPartyIntegration, *mongodbatlas.Response, error) + ListFunc func(ctx context.Context, projectID string) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) +} + +func (im *IntegrationsMock) Create(ctx context.Context, projectID string, integrationType string, integration *mongodbatlas.ThirdPartyIntegration) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) { + return im.CreateFunc(ctx, projectID, integrationType, integration) +} + +func (im *IntegrationsMock) Replace(ctx context.Context, projectID string, integrationType string, integration *mongodbatlas.ThirdPartyIntegration) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) { + return im.ReplaceFunc(ctx, projectID, integrationType, integration) +} + +func (im *IntegrationsMock) Delete(ctx context.Context, projectID string, integrationType string) (*mongodbatlas.Response, error) { + return im.DeleteFunc(ctx, projectID, integrationType) +} + +func (im *IntegrationsMock) Get(ctx context.Context, projectID string, integrationType string) (*mongodbatlas.ThirdPartyIntegration, *mongodbatlas.Response, error) { + return im.GetFunc(ctx, projectID, integrationType) +} + +func (im *IntegrationsMock) List(ctx context.Context, projectID string) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) { + return im.ListFunc(ctx, projectID) +} diff --git a/pkg/controller/atlasproject/integrations.go b/pkg/controller/atlasproject/integrations.go index f4823b69df..5cac1c4a1a 100644 --- a/pkg/controller/atlasproject/integrations.go +++ b/pkg/controller/atlasproject/integrations.go @@ -4,7 +4,6 @@ import ( "fmt" "net/http" "net/url" - "reflect" "go.mongodb.org/atlas/mongodbatlas" @@ -82,11 +81,11 @@ func (r *AtlasProjectReconciler) updateIntegrationsAtlas(ctx *workflow.Context, ctx.Log.Warnw("Update Integrations", "Can not convert kube integration", err) return workflow.Terminate(workflow.ProjectIntegrationInternal, "Update Integrations: Can not convert kube integration") } - t := mongodbatlas.ThirdPartyIntegration(atlasIntegration) - if &t != kubeIntegration { + specIntegration := (*aliasThirdPartyIntegration)(kubeIntegration) + if !areIntegrationsEqual(specIntegration, &atlasIntegration) { ctx.Log.Debugf("Try to update integration: %s", kubeIntegration.Type) if _, _, err := ctx.Client.Integrations.Replace(ctx.Context, projectID, kubeIntegration.Type, kubeIntegration); err != nil { - return workflow.Terminate(workflow.ProjectIntegrationRequest, "Can not convert integration") + return workflow.Terminate(workflow.ProjectIntegrationRequest, fmt.Sprintf("Can not apply integration: %v", err)) } } } @@ -136,7 +135,7 @@ func (r *AtlasProjectReconciler) checkIntegrationsReady(ctx *workflow.Context, n } else { specAsAtlas, _ := spec.ToAtlas(ctx.Context, r.Client, namespace) specAlias := aliasThirdPartyIntegration(*specAsAtlas) - areEqual = AreIntegrationsEqual(&atlas, &specAlias) + areEqual = integrationsApplied(&atlas, &specAlias) } ctx.Log.Debugw("checkIntegrationsReady", "atlas", atlas, "spec", spec, "areEqual", areEqual) @@ -148,41 +147,21 @@ func (r *AtlasProjectReconciler) checkIntegrationsReady(ctx *workflow.Context, n return true } -func AreIntegrationsEqual(atlas, specAsAtlas *aliasThirdPartyIntegration) bool { - return reflect.DeepEqual(cleanCopyToCompare(atlas), cleanCopyToCompare(specAsAtlas)) -} - -func cleanCopyToCompare(input *aliasThirdPartyIntegration) *aliasThirdPartyIntegration { - if input == nil { - return input - } - - result := *input - keepLastFourChars(&result.APIKey) - keepLastFourChars(&result.APIToken) - keepLastFourChars(&result.LicenseKey) - keepLastFourChars(&result.Password) - keepLastFourChars(&result.ReadToken) - keepLastFourChars(&result.RoutingKey) - keepLastFourChars(&result.Secret) - keepLastFourChars(&result.ServiceKey) - keepLastFourChars(&result.WriteToken) - - return &result +func integrationsApplied(_, _ *aliasThirdPartyIntegration) bool { + // As integration secrets are redacted from Alas, we cannot properly compare them, + // so as a simple fix here we assume changes were applied correctly as we would + // have otherwise errored out as are always needed + // TODO: remove and replace calls to this with areIntegrationsEqual when + // that code is properly comparing fields + return true } -func keepLastFourChars(strPtr *string) { - if strPtr == nil { - return - } - - charCount := 4 - str := *strPtr - if len(str) <= charCount { - return - } - - *strPtr = str[len(str)-charCount:] +func areIntegrationsEqual(_, _ *aliasThirdPartyIntegration) bool { + // As integration secrets are redacted from Alas, we cannot properly compare them, + // so as a simple fix we assume changes are always needed + // TODO: Compare using Atlas redacted fields with checksums if accepted OR + // move to implicit state checks if Atlas cannot help with this. + return false } type aliasThirdPartyIntegration mongodbatlas.ThirdPartyIntegration diff --git a/pkg/controller/atlasproject/integrations_test.go b/pkg/controller/atlasproject/integrations_test.go index 324893fda9..68f69d4e84 100644 --- a/pkg/controller/atlasproject/integrations_test.go +++ b/pkg/controller/atlasproject/integrations_test.go @@ -1,12 +1,28 @@ package atlasproject import ( + "context" + "fmt" "testing" "github.com/stretchr/testify/assert" "go.mongodb.org/atlas/mongodbatlas" + "go.uber.org/zap" + + "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/atlas" + "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/set" + "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/project" + "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/workflow" +) + +const ( + testProjectID = "project-id" + + testNamespace = "some-namespace" ) +var errTest = fmt.Errorf("fake test error") + func TestToAlias(t *testing.T) { sample := []*mongodbatlas.ThirdPartyIntegration{{ Type: "DATADOG", @@ -31,10 +47,284 @@ func TestAreIntegrationsEqual(t *testing.T) { Region: "EU", } - areEqual := AreIntegrationsEqual(&atlasDef, &specDef) + areEqual := integrationsApplied(&atlasDef, &specDef) assert.True(t, areEqual, "Identical objects should be equal") - specDef.APIKey = "non-equal-id************1234" - areEqual = AreIntegrationsEqual(&atlasDef, &specDef) + areEqual = areIntegrationsEqual(&atlasDef, &specDef) assert.False(t, areEqual, "Should fail if the last 4 characters of APIKey do not match") } + +func TestUpdateIntegrationsAtlas(t *testing.T) { + calls := 0 + for _, tc := range []struct { + title string + toUpdate [][]set.Identifiable + client *mongodbatlas.Client + expectedResult workflow.Result + expectedCalls int + }{ + { + title: "nil list does nothing", + expectedResult: workflow.OK(), + }, + + { + title: "empty list does nothing", + toUpdate: [][]set.Identifiable{}, + expectedResult: workflow.OK(), + }, + + { + title: "different integrations get updated", + toUpdate: set.Intersection( + []aliasThirdPartyIntegration{ + { + Type: "MICROSOFT_TEAMS", + Name: testNamespace, + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }, + []project.Integration{ + { + Type: "MICROSOFT_TEAMS", + MicrosoftTeamsWebhookURL: "https://somehost/some-otherpath/some-othersecret", + Enabled: true, + }, + }), + client: &mongodbatlas.Client{ + Integrations: &atlas.IntegrationsMock{ + ReplaceFunc: func(ctx context.Context, projectID string, integrationType string, integration *mongodbatlas.ThirdPartyIntegration) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) { + calls += 1 + return nil, nil, nil + }, + }, + }, + expectedResult: workflow.OK(), + expectedCalls: 1, + }, + + { + title: "matching integrations get updated anyway", + toUpdate: set.Intersection( + []aliasThirdPartyIntegration{ + { + Type: "MICROSOFT_TEAMS", + Name: testNamespace, + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }, + []project.Integration{ + { + Type: "MICROSOFT_TEAMS", + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }), + client: &mongodbatlas.Client{ + Integrations: &atlas.IntegrationsMock{ + ReplaceFunc: func(ctx context.Context, projectID string, integrationType string, integration *mongodbatlas.ThirdPartyIntegration) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) { + calls += 1 + return nil, nil, nil + }, + }, + }, + expectedResult: workflow.OK(), + expectedCalls: 1, + }, + + { + title: "integrations fail to update and return error", + toUpdate: set.Intersection( + []aliasThirdPartyIntegration{ + { + Type: "MICROSOFT_TEAMS", + Name: testNamespace, + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }, + []project.Integration{ + { + Type: "MICROSOFT_TEAMS", + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }), + client: &mongodbatlas.Client{ + Integrations: &atlas.IntegrationsMock{ + ReplaceFunc: func(ctx context.Context, projectID string, integrationType string, integration *mongodbatlas.ThirdPartyIntegration) (*mongodbatlas.ThirdPartyIntegrations, *mongodbatlas.Response, error) { + calls += 1 + return nil, nil, errTest + }, + }, + }, + expectedResult: workflow.Terminate(workflow.ProjectIntegrationRequest, fmt.Sprintf("Can not apply integration: %v", errTest)), + expectedCalls: 1, + }, + } { + t.Run(tc.title, func(t *testing.T) { + workflowCtx := &workflow.Context{ + Context: context.Background(), + Log: zap.S(), + Client: tc.client, + } + r := AtlasProjectReconciler{} + calls = 0 + result := r.updateIntegrationsAtlas(workflowCtx, testProjectID, tc.toUpdate, testNamespace) + assert.Equal(t, tc.expectedResult, result) + assert.Equal(t, tc.expectedCalls, calls) + }) + } +} + +func TestCheckIntegrationsReady(t *testing.T) { + for _, tc := range []struct { + title string + toCheck [][]set.Identifiable + requested []project.Integration + expected bool + }{ + { + title: "nil list does nothing", + expected: true, + }, + + { + title: "empty list does nothing", + toCheck: [][]set.Identifiable{}, + requested: []project.Integration{}, + expected: true, + }, + + { + title: "when requested list differs in length it bails early", + toCheck: [][]set.Identifiable{}, + requested: []project.Integration{{}}, + expected: false, + }, + + { + title: "matching integrations are considered applied", + toCheck: set.Intersection( + []aliasThirdPartyIntegration{ + { + Type: "MICROSOFT_TEAMS", + Name: testNamespace, + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }, + []project.Integration{ + { + Type: "MICROSOFT_TEAMS", + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }), + requested: []project.Integration{{}}, + expected: true, + }, + + { + title: "different integrations are considered also applied", + toCheck: set.Intersection( + []aliasThirdPartyIntegration{ + { + Type: "MICROSOFT_TEAMS", + Name: testNamespace, + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + }, + []project.Integration{ + { + Type: "MICROSOFT_TEAMS", + MicrosoftTeamsWebhookURL: "https://somehost/some-otherpath/some-othersecret", + Enabled: true, + }, + }), + requested: []project.Integration{{}}, + expected: true, + }, + + { + title: "matching integrations including prometheus are considered applied", + toCheck: set.Intersection( + []aliasThirdPartyIntegration{ + { + Type: "MICROSOFT_TEAMS", + Name: testNamespace, + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + { + Type: "PROMETHEUS", + UserName: "prometheus", + ServiceDiscovery: "http", + Enabled: true, + }, + }, + []project.Integration{ + { + Type: "MICROSOFT_TEAMS", + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + { + Type: "PROMETHEUS", + UserName: "prometheus", + ServiceDiscovery: "http", + Enabled: true, + }, + }), + requested: []project.Integration{{}, {}}, + expected: true, + }, + + { + title: "matching integrations with a differing prometheus are considered different", + toCheck: set.Intersection( + []aliasThirdPartyIntegration{ + { + Type: "MICROSOFT_TEAMS", + Name: testNamespace, + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + { + Type: "PROMETHEUS", + UserName: "prometheus", + ServiceDiscovery: "http", + Enabled: true, + }, + }, + []project.Integration{ + { + Type: "MICROSOFT_TEAMS", + MicrosoftTeamsWebhookURL: "https://somehost/somepath/somesecret", + Enabled: true, + }, + { + Type: "PROMETHEUS", + UserName: "zeus", + ServiceDiscovery: "file", + Enabled: true, + }, + }), + requested: []project.Integration{{}, {}}, + expected: false, + }, + } { + t.Run(tc.title, func(t *testing.T) { + workflowCtx := &workflow.Context{ + Context: context.Background(), + Log: zap.S(), + } + r := AtlasProjectReconciler{} + result := r.checkIntegrationsReady(workflowCtx, testNamespace, tc.toCheck, tc.requested) + assert.Equal(t, tc.expected, result) + }) + } +}