Skip to content

Commit

Permalink
CLOUDP-229283: disable ownership detection (#1371)
Browse files Browse the repository at this point in the history
* test/e2e: remove ownership detection e2e tests

* pkg/controller: disable ownership detection

* cmd/manager: deprecate subobject deletion flag, add event and log notifications
  • Loading branch information
s-urbaniak authored Feb 14, 2024
1 parent d3a3142 commit 145b35e
Show file tree
Hide file tree
Showing 12 changed files with 24 additions and 980 deletions.
16 changes: 9 additions & 7 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ const (
objectDeletionProtectionEnvVar = "OBJECT_DELETION_PROTECTION"
subobjectDeletionProtectionEnvVar = "SUBOBJECT_DELETION_PROTECTION"
objectDeletionProtectionDefault = true
subobjectDeletionProtectionDefault = true
subobjectDeletionProtectionDefault = false
subobjectDeletionProtectionMessage = "Note: sub-object deletion protection is IGNORED because it does not work deterministically."
)

var (
Expand Down Expand Up @@ -147,7 +148,7 @@ func main() {
EventRecorder: mgr.GetEventRecorderFor("AtlasDeployment"),
AtlasProvider: atlasProvider,
ObjectDeletionProtection: config.ObjectDeletionProtection,
SubObjectDeletionProtection: config.SubObjectDeletionProtection,
SubObjectDeletionProtection: false,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AtlasDeployment")
os.Exit(1)
Expand All @@ -162,7 +163,7 @@ func main() {
EventRecorder: mgr.GetEventRecorderFor("AtlasProject"),
AtlasProvider: atlasProvider,
ObjectDeletionProtection: config.ObjectDeletionProtection,
SubObjectDeletionProtection: config.SubObjectDeletionProtection,
SubObjectDeletionProtection: false,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AtlasProject")
os.Exit(1)
Expand All @@ -177,7 +178,7 @@ func main() {
AtlasProvider: atlasProvider,
GlobalPredicates: globalPredicates,
ObjectDeletionProtection: config.ObjectDeletionProtection,
SubObjectDeletionProtection: config.SubObjectDeletionProtection,
SubObjectDeletionProtection: false,
FeaturePreviewOIDCAuthEnabled: config.FeatureFlags.IsFeaturePresent(featureflags.FeatureOIDC),
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AtlasDatabaseUser")
Expand All @@ -193,7 +194,7 @@ func main() {
EventRecorder: mgr.GetEventRecorderFor("AtlasDataFederation"),
AtlasProvider: atlasProvider,
ObjectDeletionProtection: config.ObjectDeletionProtection,
SubObjectDeletionProtection: config.SubObjectDeletionProtection,
SubObjectDeletionProtection: false,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AtlasDataFederation")
os.Exit(1)
Expand All @@ -208,7 +209,7 @@ func main() {
EventRecorder: mgr.GetEventRecorderFor("AtlasFederatedAuth"),
AtlasProvider: atlasProvider,
ObjectDeletionProtection: config.ObjectDeletionProtection,
SubObjectDeletionProtection: config.SubObjectDeletionProtection,
SubObjectDeletionProtection: false,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AtlasFederatedAuth")
os.Exit(1)
Expand All @@ -225,6 +226,7 @@ func main() {
os.Exit(1)
}

setupLog.Info(subobjectDeletionProtectionMessage)
setupLog.Info("starting manager")
if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
setupLog.Error(err, "problem running manager")
Expand Down Expand Up @@ -264,7 +266,7 @@ func parseConfiguration() Config {
flag.BoolVar(&config.ObjectDeletionProtection, objectDeletionProtectionFlag, objectDeletionProtectionDefault, "Defines if the operator deletes Atlas resource "+
"when a Custom Resource is deleted")
flag.BoolVar(&config.SubObjectDeletionProtection, subobjectDeletionProtectionFlag, subobjectDeletionProtectionDefault, "Defines if the operator overwrites "+
"(and consequently delete) subresources that were not previously created by the operator")
"(and consequently delete) subresources that were not previously created by the operator. "+subobjectDeletionProtectionMessage)
appVersion := flag.Bool("v", false, "prints application version")
flag.Parse()

Expand Down
2 changes: 1 addition & 1 deletion cmd/manager/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func Test_configureDeletionProtection(t *testing.T) {
t,
Config{
ObjectDeletionProtection: true,
SubObjectDeletionProtection: true,
SubObjectDeletionProtection: false,
},
config,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ func (r *AtlasDatabaseUserReconciler) Reconcile(ctx context.Context, req ctrl.Re
workflowCtx.OrgID = orgID
workflowCtx.Client = atlasClient

owner, err := customresource.IsOwner(databaseUser, r.ObjectDeletionProtection, customresource.IsResourceManagedByOperator, managedByAtlas(ctx, atlasClient, project.ID(), log))
// Setting protection flag to static false because ownership detection is disabled.
owner, err := customresource.IsOwner(databaseUser, false, customresource.IsResourceManagedByOperator, managedByAtlas(ctx, atlasClient, project.ID(), log))
if err != nil {
result = workflow.Terminate(workflow.Internal, fmt.Sprintf("enable to resolve ownership for deletion protection: %s", err))
workflowCtx.SetConditionFromResult(status.DatabaseUserReadyType, result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ func (r *AtlasDataFederationReconciler) Reconcile(context context.Context, req c
ctx.OrgID = orgID
ctx.Client = atlasClient

owner, err := customresource.IsOwner(dataFederation, r.ObjectDeletionProtection, customresource.IsResourceManagedByOperator, managedByAtlas(context, atlasClient, project.ID(), log))
// Setting protection flag to static false because ownership detection is disabled.
owner, err := customresource.IsOwner(dataFederation, false, customresource.IsResourceManagedByOperator, managedByAtlas(context, atlasClient, project.ID(), log))
if err != nil {
result = workflow.Terminate(workflow.Internal, fmt.Sprintf("unable to resolve ownership for deletion protection: %s", err))
ctx.SetConditionFromResult(status.DataFederationReadyType, result)
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/atlasdeployment/atlasdeployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,10 @@ func (r *AtlasDeploymentReconciler) checkDeploymentIsManaged(
project *mdbv1.AtlasProject,
deployment *mdbv1.AtlasDeployment,
) workflow.Result {
// Setting protection flag to static false because ownership detection is disabled.
owner, err := customresource.IsOwner(
deployment,
r.ObjectDeletionProtection,
false,
customresource.IsResourceManagedByOperator,
managedByAtlas(workflowCtx, project.ID(), log),
)
Expand Down
25 changes: 0 additions & 25 deletions pkg/controller/atlasdeployment/atlasdeployment_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,6 @@ func TestProtectedAdvancedDeploymentManagedInAtlas(t *testing.T) {
inAtlas: sameAdvancedDeployment(fakeDomain),
expectedErr: "",
},
{
title: "advanced deployment not tagged and different in Atlas means unmanaged",
inAtlas: differentAdvancedDeployment(fakeDomain),
expectedErr: "unable to reconcile Deployment due to deletion protection being enabled. see https://dochub.mongodb.org/core/ako-deletion-protection for further information",
},
}
for _, tc := range testCases {
t.Run(tc.title, func(t *testing.T) {
Expand Down Expand Up @@ -164,11 +159,6 @@ func TestProtectedServerlessManagedInAtlas(t *testing.T) {
inAtlas: sameServerlessDeployment(fakeDomain),
expectedErr: "",
},
{
title: "serverless deployment not tagged and different in Atlas means unmanaged",
inAtlas: differentServerlessDeployment(fakeDomain),
expectedErr: "unable to reconcile Deployment due to deletion protection being enabled. see https://dochub.mongodb.org/core/ako-deletion-protection for further information",
},
}
for _, tc := range testCases {
t.Run(tc.title, func(t *testing.T) {
Expand Down Expand Up @@ -573,28 +563,13 @@ func TestCleanupBindings(t *testing.T) {
})
}

func differentAdvancedDeployment(ns string) *mongodbatlas.AdvancedCluster {
project := testProject(ns)
deployment := v1.NewDeployment(project.Namespace, fakeDeployment, fakeDeployment)
deployment.Spec.DeploymentSpec.ReplicationSpecs[0].RegionConfigs[0].ElectableSpecs.InstanceSize = "M2"
advancedSpec := deployment.Spec.DeploymentSpec
return intoAdvancedAtlasCluster(advancedSpec)
}

func sameAdvancedDeployment(ns string) *mongodbatlas.AdvancedCluster {
project := testProject(ns)
deployment := v1.NewDeployment(project.Namespace, fakeDeployment, fakeDeployment)
advancedSpec := deployment.Spec.DeploymentSpec
return intoAdvancedAtlasCluster(advancedSpec)
}

func differentServerlessDeployment(ns string) *mongodbatlas.Cluster {
project := testProject(ns)
deployment := v1.NewDefaultAWSServerlessInstance(project.Namespace, project.Name)
deployment.Spec.ServerlessSpec.ProviderSettings.RegionName = "US_EAST_2"
return intoServerlessAtlasCluster(deployment.Spec.ServerlessSpec)
}

func sameServerlessDeployment(ns string) *mongodbatlas.Cluster {
project := testProject(ns)
deployment := v1.NewDefaultAWSServerlessInstance(project.Namespace, project.Name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ func (r *AtlasFederatedAuthReconciler) Reconcile(ctx context.Context, req ctrl.R
workflowCtx.SdkClient = atlasClient
workflowCtx.OrgID = orgID

owner, err := customresource.IsOwner(fedauth, r.ObjectDeletionProtection, customresource.IsResourceManagedByOperator, managedByAtlas(ctx, atlasClient, orgID))
// Setting protection flag to static false because ownership detection is disabled.
owner, err := customresource.IsOwner(fedauth, false, customresource.IsResourceManagedByOperator, managedByAtlas(ctx, atlasClient, orgID))
if err != nil {
result = workflow.Terminate(workflow.Internal, fmt.Sprintf("unable to resolve ownership for deletion protection: %s", err))
workflowCtx.SetConditionFromResult(status.FederatedAuthReadyType, result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ func TestReconcile(t *testing.T) {
groupAPI.EXPECT().ListProjects(context.Background()).
Return(admin.ListProjectsApiRequest{ApiService: groupAPI})
groupAPI.EXPECT().ListProjectsExecute(mock.Anything).
Twice().
Return(
&admin.PaginatedAtlasGroup{
Results: &[]admin.Group{
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/atlasproject/atlasproject_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ func (r *AtlasProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request
workflowCtx.OrgID = orgID
workflowCtx.Client = atlasClient

owner, err := customresource.IsOwner(project, r.ObjectDeletionProtection, customresource.IsResourceManagedByOperator, managedByAtlas(workflowCtx))
// Setting protection flag to static false because ownership detection is disabled.
owner, err := customresource.IsOwner(project, false, customresource.IsResourceManagedByOperator, managedByAtlas(workflowCtx))
if err != nil {
result = workflow.Terminate(workflow.Internal, fmt.Sprintf("unable to resolve ownership for deletion protection: %s", err))
workflowCtx.SetConditionFromResult(status.ProjectReadyType, result)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/atlasproject/team_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (r *AtlasProjectReconciler) teamReconcile(
teamCtx.OrgID = orgID
teamCtx.Client = atlasClient

owner, err := customresource.IsOwner(team, r.ObjectDeletionProtection, customresource.IsResourceManagedByOperator, teamsManagedByAtlas(teamCtx))
owner, err := customresource.IsOwner(team, false, customresource.IsResourceManagedByOperator, teamsManagedByAtlas(teamCtx))
if err != nil {
result = workflow.Terminate(workflow.Internal, fmt.Sprintf("unable to resolve ownership for deletion protection: %s", err))
teamCtx.SetConditionFromResult(status.ReadyType, result)
Expand Down
Loading

0 comments on commit 145b35e

Please sign in to comment.