From 6b3ac75a36b0d03fb0d7e2d14dbd98d53d479716 Mon Sep 17 00:00:00 2001 From: "jose.vazquez" Date: Fri, 12 Jul 2024 18:26:13 +0200 Subject: [PATCH] Revert "Draft triage (#1684)" This reverts commit 2711915843b15262128260265980017919200769. --- Makefile | 3 +- internal/mocks/atlas/integrations.go | 35 --- pkg/controller/atlasproject/integrations.go | 55 +++- .../atlasproject/integrations_test.go | 296 +----------------- 4 files changed, 42 insertions(+), 347 deletions(-) delete mode 100644 internal/mocks/atlas/integrations.go diff --git a/Makefile b/Makefile index d0e83c4248..92b0380fd9 100644 --- a/Makefile +++ b/Makefile @@ -105,7 +105,6 @@ 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] @@ -534,7 +533,7 @@ ifdef RUN_YAML endif OPERATOR_POD_NAME=$(OPERATOR_POD_NAME) \ OPERATOR_NAMESPACE=$(OPERATOR_NAMESPACE) \ - bin/manager --object-deletion-protection=false --log-level=$(RUN_LOG_LEVEL) \ + bin/manager --object-deletion-protection=false --log-level=debug \ --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 deleted file mode 100644 index f4035fb09b..0000000000 --- a/internal/mocks/atlas/integrations.go +++ /dev/null @@ -1,35 +0,0 @@ -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 5cac1c4a1a..f4823b69df 100644 --- a/pkg/controller/atlasproject/integrations.go +++ b/pkg/controller/atlasproject/integrations.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/url" + "reflect" "go.mongodb.org/atlas/mongodbatlas" @@ -81,11 +82,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") } - specIntegration := (*aliasThirdPartyIntegration)(kubeIntegration) - if !areIntegrationsEqual(specIntegration, &atlasIntegration) { + t := mongodbatlas.ThirdPartyIntegration(atlasIntegration) + if &t != kubeIntegration { 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, fmt.Sprintf("Can not apply integration: %v", err)) + return workflow.Terminate(workflow.ProjectIntegrationRequest, "Can not convert integration") } } } @@ -135,7 +136,7 @@ func (r *AtlasProjectReconciler) checkIntegrationsReady(ctx *workflow.Context, n } else { specAsAtlas, _ := spec.ToAtlas(ctx.Context, r.Client, namespace) specAlias := aliasThirdPartyIntegration(*specAsAtlas) - areEqual = integrationsApplied(&atlas, &specAlias) + areEqual = AreIntegrationsEqual(&atlas, &specAlias) } ctx.Log.Debugw("checkIntegrationsReady", "atlas", atlas, "spec", spec, "areEqual", areEqual) @@ -147,21 +148,41 @@ func (r *AtlasProjectReconciler) checkIntegrationsReady(ctx *workflow.Context, n return true } -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 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 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 +func keepLastFourChars(strPtr *string) { + if strPtr == nil { + return + } + + charCount := 4 + str := *strPtr + if len(str) <= charCount { + return + } + + *strPtr = str[len(str)-charCount:] } type aliasThirdPartyIntegration mongodbatlas.ThirdPartyIntegration diff --git a/pkg/controller/atlasproject/integrations_test.go b/pkg/controller/atlasproject/integrations_test.go index 68f69d4e84..324893fda9 100644 --- a/pkg/controller/atlasproject/integrations_test.go +++ b/pkg/controller/atlasproject/integrations_test.go @@ -1,28 +1,12 @@ 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", @@ -47,284 +31,10 @@ func TestAreIntegrationsEqual(t *testing.T) { Region: "EU", } - areEqual := integrationsApplied(&atlasDef, &specDef) + areEqual := AreIntegrationsEqual(&atlasDef, &specDef) assert.True(t, areEqual, "Identical objects should be equal") - areEqual = areIntegrationsEqual(&atlasDef, &specDef) + specDef.APIKey = "non-equal-id************1234" + 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) - }) - } -}