diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 940b3eb671..f5308e66c0 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -133,14 +133,16 @@ func main() { } if err = (&atlasdeployment.AtlasDeploymentReconciler{ - Client: mgr.GetClient(), - Log: logger.Named("controllers").Named("AtlasDeployment").Sugar(), - Scheme: mgr.GetScheme(), - AtlasDomain: config.AtlasDomain, - GlobalAPISecret: config.GlobalAPISecret, - ResourceWatcher: watch.NewResourceWatcher(), - GlobalPredicates: globalPredicates, - EventRecorder: mgr.GetEventRecorderFor("AtlasDeployment"), + Client: mgr.GetClient(), + Log: logger.Named("controllers").Named("AtlasDeployment").Sugar(), + Scheme: mgr.GetScheme(), + AtlasDomain: config.AtlasDomain, + GlobalAPISecret: config.GlobalAPISecret, + ResourceWatcher: watch.NewResourceWatcher(), + GlobalPredicates: globalPredicates, + EventRecorder: mgr.GetEventRecorderFor("AtlasDeployment"), + ObjectDeletionProtection: config.ObjectDeletionProtection, + SubObjectDeletionProtection: config.SubObjectDeletionProtection, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "AtlasDeployment") os.Exit(1) diff --git a/pkg/controller/atlasdeployment/atlasdeployment_controller.go b/pkg/controller/atlasdeployment/atlasdeployment_controller.go index 3c3d2e292a..2a23112663 100644 --- a/pkg/controller/atlasdeployment/atlasdeployment_controller.go +++ b/pkg/controller/atlasdeployment/atlasdeployment_controller.go @@ -52,13 +52,17 @@ import ( // AtlasDeploymentReconciler reconciles an AtlasDeployment object type AtlasDeploymentReconciler struct { watch.ResourceWatcher - Client client.Client - Log *zap.SugaredLogger - Scheme *runtime.Scheme - AtlasDomain string - GlobalAPISecret client.ObjectKey - GlobalPredicates []predicate.Predicate - EventRecorder record.EventRecorder + Client client.Client + Log *zap.SugaredLogger + Scheme *runtime.Scheme + AtlasDomain string + GlobalAPISecret client.ObjectKey + GlobalPredicates []predicate.Predicate + EventRecorder record.EventRecorder + ObjectDeletionProtection bool + SubObjectDeletionProtection bool + + CustomClientFn func(string, atlas.Connection, *zap.SugaredLogger) (mongodbatlas.Client, error) } // +kubebuilder:rbac:groups=atlas.mongodb.com,resources=atlasdeployments,verbs=get;list;watch;create;update;patch;delete @@ -161,17 +165,23 @@ func (r *AtlasDeploymentReconciler) Reconcile(context context.Context, req ctrl. if !deployment.GetDeletionTimestamp().IsZero() { if customresource.HaveFinalizer(deployment, customresource.FinalizerLabel) { - if customresource.ResourceShouldBeLeftInAtlas(deployment) { - log.Infof("Not removing Atlas Deployment from Atlas as the '%s' annotation is set", customresource.ResourcePolicyAnnotation) + isProtected := customresource.IsResourceProtected(deployment, r.ObjectDeletionProtection) + log.Infow("RESOURCE PROTECTED", r.ObjectDeletionProtection, isProtected) + if isProtected { + log.Info("Not removing Atlas deployment from Atlas as per configuration") } else { - if err = r.deleteDeploymentFromAtlas(context, project, deployment, atlasClient, log); err != nil { - log.Errorf("failed to remove deployment from Atlas: %s", err) - result = workflow.Terminate(workflow.Internal, err.Error()) - ctx.SetConditionFromResult(status.DeploymentReadyType, result) - return result.ReconcileResult(), nil + if customresource.ResourceShouldBeLeftInAtlas(deployment) { + log.Infof("Not removing Atlas Deployment from Atlas as the '%s' annotation is set", customresource.ResourcePolicyAnnotation) + } else { + if err = r.deleteDeploymentFromAtlas(context, project, deployment, atlasClient, log); err != nil { + log.Errorf("failed to remove deployment from Atlas: %s", err) + result = workflow.Terminate(workflow.Internal, err.Error()) + ctx.SetConditionFromResult(status.DeploymentReadyType, result) + return result.ReconcileResult(), nil + } } } - err = r.removeDeletionFinalizer(context, deployment) + err = customresource.ManageFinalizer(context, r.Client, deployment, customresource.UnsetFinalizer) if err != nil { result = workflow.Terminate(workflow.Internal, err.Error()) log.Errorw("failed to remove finalizer", "error", err) diff --git a/pkg/controller/customresource/customresource.go b/pkg/controller/customresource/customresource.go index c23ff42acd..76b6722316 100644 --- a/pkg/controller/customresource/customresource.go +++ b/pkg/controller/customresource/customresource.go @@ -135,3 +135,13 @@ func ReconciliationShouldBeSkipped(resource mdbv1.AtlasCustomResource) bool { } return false } + +// SetAnnotation sets an annotation in resource while respecting the rest of annotations. +func SetAnnotation(resource mdbv1.AtlasCustomResource, key, value string) { + annot := resource.GetAnnotations() + if annot == nil { + annot = map[string]string{} + } + annot[key] = value + resource.SetAnnotations(annot) +} diff --git a/pkg/controller/customresource/protection_test.go b/pkg/controller/customresource/protection_test.go new file mode 100644 index 0000000000..77eb564aad --- /dev/null +++ b/pkg/controller/customresource/protection_test.go @@ -0,0 +1,149 @@ +package customresource_test + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + mdbv1 "github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1" + "github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/customresource" +) + +func sampleResource() *mdbv1.AtlasDatabaseUser { + return &mdbv1.AtlasDatabaseUser{ + Spec: mdbv1.AtlasDatabaseUserSpec{}, + } +} + +func taggedResource(tag, value string) *mdbv1.AtlasDatabaseUser { + dbUser := sampleResource() + annot := map[string]string{} + annot[tag] = value + dbUser.SetAnnotations(annot) + return dbUser +} + +func testOpChecker(reply bool) customresource.OperatorChecker { + return func(resource mdbv1.AtlasCustomResource) (bool, error) { + return reply, nil + } +} + +func testAtlasChecker(reply bool) customresource.AtlasChecker { + return func(resource mdbv1.AtlasCustomResource) (bool, error) { + return reply, nil + } +} + +var ErrOpChecker = fmt.Errorf("operator checker failed") + +func failedOpChecker(err error) customresource.OperatorChecker { + return func(resource mdbv1.AtlasCustomResource) (bool, error) { + return false, err + } +} + +var ErrAtlasChecker = fmt.Errorf("atlas checker failed") + +func failedAtlasChecker(err error) customresource.AtlasChecker { + return func(resource mdbv1.AtlasCustomResource) (bool, error) { + return false, err + } +} + +func TestWithoutProtectionIsOwned(t *testing.T) { + owned, err := customresource.IsOwner(sampleResource(), false, nil, nil) + assert.NoError(t, err) + assert.Equal(t, owned, true) +} + +func TestProtected(t *testing.T) { + tests := []struct { + title string + opChecker customresource.OperatorChecker + atlasChecker customresource.AtlasChecker + expectedOwned bool + }{ + {"managed is owned", testOpChecker(true), nil, true}, + {"unmanaged but not in Atlas is owned", testOpChecker(false), testAtlasChecker(false), true}, + {"unmanaged but in Atlas is NOT owned", testOpChecker(false), testAtlasChecker(true), false}, + } + for _, tc := range tests { + t.Run(fmt.Sprintf("Protected and %s", tc.title), func(t *testing.T) { + owned, err := customresource.IsOwner(sampleResource(), true, tc.opChecker, tc.atlasChecker) + assert.NoError(t, err) + assert.Equal(t, tc.expectedOwned, owned) + }) + } +} + +func TestProtectedFailures(t *testing.T) { + tests := []struct { + title string + opChecker customresource.OperatorChecker + atlasChecker customresource.AtlasChecker + expectedFailure error + }{ + {"When all checkers fail, operator checker fails first", failedOpChecker(ErrOpChecker), failedAtlasChecker(ErrAtlasChecker), ErrOpChecker}, + {"When unamanaged and atlas checker fails we get that its failure", testOpChecker(false), failedAtlasChecker(ErrAtlasChecker), ErrAtlasChecker}, + } + for _, tc := range tests { + t.Run(tc.title, func(t *testing.T) { + _, err := customresource.IsOwner(sampleResource(), true, tc.opChecker, tc.atlasChecker) + assert.Equal(t, tc.expectedFailure, err) + }) + } +} + +func TestIsResourceProtected(t *testing.T) { + tests := []struct { + title string + protectionFlag bool + resource mdbv1.AtlasCustomResource + expectedProtected bool + }{ + {"Resource without tags with the flag set is protected", true, sampleResource(), true}, + {"Resource without tags with the flag unset isn't protected", false, sampleResource(), false}, + { + "Resource with keep tag is protected", + false, + taggedResource(customresource.ResourcePolicyAnnotation, customresource.ResourcePolicyKeep), + true, + }, + { + "Resource with delete tag and protected flag set is NOT protected", + true, + taggedResource(customresource.ResourcePolicyAnnotation, customresource.ResourcePolicyDelete), + false, + }, + { + "Resource with delete tag and protected flag unset isn't protected", + false, + taggedResource(customresource.ResourcePolicyAnnotation, customresource.ResourcePolicyDelete), + false, + }, + } + for _, tc := range tests { + t.Run(tc.title, func(t *testing.T) { + assert.Equal(t, tc.expectedProtected, customresource.IsResourceProtected(tc.resource, tc.protectionFlag)) + }) + } +} + +func TestApplyLastConfigApplied(t *testing.T) { + resource := sampleResource() + resource.Spec.Username = "test-user" + + // ignore the error due to not configuring the fake client + // we are not checking that, we are only interested on a new annotation in resource + _ = customresource.ApplyLastConfigApplied(context.Background(), resource, fake.NewClientBuilder().Build()) + + annot := resource.GetAnnotations() + assert.NotEmpty(t, annot) + expectedConfig := `{"projectRef":{"name":"","namespace":""},"roles":null,"username":"test-user"}` + assert.Equal(t, annot[customresource.AnnotationLastAppliedConfiguration], expectedConfig) +} diff --git a/test/int/databaseuser_protected_test.go b/test/int/databaseuser_protected_test.go index d910da7f14..a51981f31f 100644 --- a/test/int/databaseuser_protected_test.go +++ b/test/int/databaseuser_protected_test.go @@ -58,6 +58,11 @@ var _ = Describe("Atlas Database User", Label("int", "AtlasDatabaseUser", "prote By("Creating a deployment", func() { testDeployment = mdbv1.DefaultAWSDeployment(testNamespace.Name, projectName).Lightweight() + customresource.SetAnnotation( // this test deployment must be deleted + testDeployment, + customresource.ResourcePolicyAnnotation, + customresource.ResourcePolicyDelete, + ) Expect(k8sClient.Create(context.TODO(), testDeployment)).To(Succeed()) Eventually(func() bool { diff --git a/test/int/deployment_test.go b/test/int/deployment_test.go index 3fea7fd9be..11687364cc 100644 --- a/test/int/deployment_test.go +++ b/test/int/deployment_test.go @@ -8,7 +8,9 @@ import ( "strconv" "time" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/connectionsecret" "github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/compat" @@ -1228,6 +1230,156 @@ var _ = Describe("AtlasDeployment", Label("int", "AtlasDeployment"), func() { }) }) +func prePropulatedTestProject(ns, name, secretName string) *mdbv1.AtlasProject { + return mdbv1.NewProject(ns, name, name). + WithConnectionSecret(secretName). + WithIPAccessList(project.NewIPAccessList().WithCIDR("0.0.0.0/0")). + WithAnnotations(map[string]string{ + customresource.ResourcePolicyAnnotation: customresource.ResourcePolicyDelete, + }) +} + +var _ = Describe("AtlasDeployment Deletion", Label("AtlasDeployment", "deployment-deletion-protection"), func() { + var testNamespace *corev1.Namespace + var stopManager context.CancelFunc + var connectionSecret corev1.Secret + var testProject *mdbv1.AtlasProject + + // This setupTestOperatorWithProject is NOT a BeforeEach because I could + // find no way to pass the protected argument to the BeforeEach call from + // each table test case run on the DescribeTables. + var setupTestOperatorWithProject = func(protected bool) { + By("Starting the operator", func() { + testNamespace, stopManager = prepareControllers(protected) + Expect(testNamespace).ToNot(BeNil()) + Expect(stopManager).ToNot(BeNil()) + }) + + By("Creating project connection secret", func() { + connectionSecret = buildConnectionSecret(fmt.Sprintf("%s-atlas-key", testNamespace.Name)) + Expect(k8sClient.Create(context.Background(), &connectionSecret)).To(Succeed()) + }) + + By("Creating a project", func() { + projectName := "test-project" + testProject = prePropulatedTestProject(testNamespace.Name, projectName, connectionSecret.Name) + Expect(k8sClient.Create(context.TODO(), testProject, &client.CreateOptions{})).To(Succeed()) + + Eventually(func() bool { + return testutil.CheckCondition(k8sClient, testProject, status.TrueCondition(status.ReadyType)) + }).WithTimeout(3 * time.Minute).WithPolling(PollingInterval).Should(BeTrue()) + }) + } + + AfterEach(func() { + By("Deleting project from k8s and atlas", func() { + Expect(k8sClient.Delete(context.TODO(), testProject, &client.DeleteOptions{})).To(Succeed()) + Eventually( + checkAtlasProjectRemoved(testProject.Status.ID), + ).WithTimeout(3 * time.Minute).WithPolling(PollingInterval).Should(BeTrue()) + }) + + By("Deleting project connection secret", func() { + Expect(k8sClient.Delete(context.Background(), &connectionSecret)).To(Succeed()) + }) + + By("Stopping the operator", func() { + stopManager() + err := k8sClient.Delete(context.Background(), testNamespace) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + DescribeTable("removing an already deleted deployment in the cluster when protection is", func(protected bool) { + setupTestOperatorWithProject(protected) + testDeployment := &mdbv1.AtlasDeployment{} + + By("Creating an already deleted deployment in the cluster", func() { + testDeployment = mdbv1.DefaultAWSDeployment(testNamespace.Name, testProject.Name) + Expect(k8sClient.Create(context.TODO(), testDeployment, &client.CreateOptions{})).To(Succeed()) + Expect(k8sClient.Delete(context.TODO(), testDeployment, &client.DeleteOptions{})).To(Succeed()) + + Eventually(func() bool { + deployment := mdbv1.AtlasDeployment{} + err := k8sClient.Get(context.TODO(), kube.ObjectKey(testNamespace.Name, testDeployment.Name), &deployment, &client.GetOptions{}) + return k8serrors.IsNotFound(err) + }).WithTimeout(5 * time.Minute).WithPolling(PollingInterval).Should(BeTrue()) + }) + }, + Entry("ON succeeds immediately", true), + Entry("OFF also succeeds immediately", false), + ) + + DescribeTable("removing from Kubernetes when protection is", func(protected bool, policy string, expectRemoval bool) { + setupTestOperatorWithProject(protected) + testDeployment := &mdbv1.AtlasDeployment{} + + By("Creating a deployment in the cluster with annotation set to delete", func() { + testDeployment = mdbv1.DefaultAWSDeployment(testNamespace.Name, testProject.Name).Lightweight() + if policy != "" { + customresource.SetAnnotation(testDeployment, customresource.ResourcePolicyAnnotation, policy) + } + Expect(k8sClient.Create(context.TODO(), testDeployment, &client.CreateOptions{})).To(Succeed()) + }) + + By("Waiting the deployment to settle in kubernetes", func() { + Eventually(func(g Gomega) bool { + return testutil.CheckCondition(k8sClient, testDeployment, status.TrueCondition(status.ReadyType), validateDeploymentUpdatingFunc(g)) + }).WithTimeout(30 * time.Minute).WithPolling(PollingInterval).Should(BeTrue()) + }) + + By("Deleting the deployment from Kubernetes", func() { + Expect(k8sClient.Delete(context.TODO(), testDeployment, &client.DeleteOptions{})).To(Succeed()) + Eventually(func() bool { + deployment := mdbv1.AtlasDeployment{} + err := k8sClient.Get(context.TODO(), kube.ObjectKey(testNamespace.Name, testDeployment.Name), &deployment, &client.GetOptions{}) + return k8serrors.IsNotFound(err) + }).WithTimeout(5 * time.Minute).WithPolling(PollingInterval).Should(BeTrue()) + }) + + By("Checking whether or not the Atlas deployment got also removed", func() { + deploymentAtlasName := testDeployment.Spec.DeploymentSpec.Name + if expectRemoval { + Expect(checkAtlasDeploymentRemoved(testProject.Name, deploymentAtlasName)()).To(BeTrue()) + } else { + Expect(checkAtlasDeploymentRemoved(testProject.Name, deploymentAtlasName)()).To(BeFalse()) + Expect(deleteAtlasDeployment(testProject.Name, deploymentAtlasName)).ToNot(HaveOccurred()) + } + }) + }, + Entry("ON and resource policy is NOT SET, should NOT remove the Atlas Deployment", + /* protected = */ true, + "", + /* expectRemoval = */ false, + ), + Entry("OFF and resource policy is NOT SET, should remove the Atlas Deployment", + /* protected = */ false, + "", + /* expectRemoval = */ true, + ), + Entry("ON and resource policy is 'delete', should remove the Atlas Deployment", + /* protected = */ true, + customresource.ResourcePolicyDelete, + /* expectRemoval = */ true, + ), + Entry("OFF and resource policy is 'delete' should remove the Atlas Deployment", + /* protected = */ false, + customresource.ResourcePolicyDelete, + /* expectRemoval = */ true, + ), + Entry("ON and resource policy is 'keep' should NOT remove the Atlas Deployment", + /* protected = */ true, + customresource.ResourcePolicyKeep, + /* expectRemoval = */ false, + ), + Entry("OFF and resource policy is 'keep' should NOT remove the Atlas Deployment", + /* protected = */ false, + customresource.ResourcePolicyKeep, + /* expectRemoval = */ false, + ), + ) +}) + func validateDeploymentCreatingFunc(g Gomega) func(a mdbv1.AtlasCustomResource) { startedCreation := false return func(a mdbv1.AtlasCustomResource) { diff --git a/test/int/integration_suite_test.go b/test/int/integration_suite_test.go index 0ad6fe1daa..76fe33eb96 100644 --- a/test/int/integration_suite_test.go +++ b/test/int/integration_suite_test.go @@ -230,13 +230,15 @@ func prepareControllers(deletionProtection bool) (*corev1.Namespace, context.Can Expect(err).ToNot(HaveOccurred()) err = (&atlasdeployment.AtlasDeploymentReconciler{ - Client: k8sManager.GetClient(), - Log: logger.Named("controllers").Named("AtlasDeployment").Sugar(), - AtlasDomain: atlasDomain, - ResourceWatcher: watch.NewResourceWatcher(), - GlobalAPISecret: kube.ObjectKey(namespace.Name, "atlas-operator-api-key"), - GlobalPredicates: globalPredicates, - EventRecorder: k8sManager.GetEventRecorderFor("AtlasDeployment"), + Client: k8sManager.GetClient(), + Log: logger.Named("controllers").Named("AtlasDeployment").Sugar(), + AtlasDomain: atlasDomain, + ResourceWatcher: watch.NewResourceWatcher(), + GlobalAPISecret: kube.ObjectKey(namespace.Name, "atlas-operator-api-key"), + GlobalPredicates: globalPredicates, + EventRecorder: k8sManager.GetEventRecorderFor("AtlasDeployment"), + ObjectDeletionProtection: deletionProtection, + SubObjectDeletionProtection: deletionProtection, }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred())