diff --git a/.github/renovate.json5 b/.github/renovate.json5 index 626a7e8e7..ec18e5c4e 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -14,7 +14,6 @@ // PLEASE UPDATE THIS WHEN RELEASING. "baseBranches": [ 'main', - 'release-1.18', 'release-1.19', 'release-1.20', 'release-2.0', diff --git a/apis/common/policies.go b/apis/common/policies.go index c2e2cfec5..326feb2b3 100644 --- a/apis/common/policies.go +++ b/apis/common/policies.go @@ -22,7 +22,7 @@ type ManagementPolicies []ManagementAction // A ManagementAction represents an action that the Crossplane controllers // can take on an external resource. -// +kubebuilder:validation:Enum=Observe;Create;Update;Delete;LateInitialize;* +// +kubebuilder:validation:Enum=Observe;Create;Update;Delete;LateInitialize;*;Orphan type ManagementAction string const ( @@ -49,6 +49,10 @@ const ( // ManagementActionAll means that all of the above actions will be taken // by the Crossplane controllers. ManagementActionAll ManagementAction = "*" + + // ManagementActionOrphan means that all actions except Delete will be taken + // by the Crossplane controllers. + ManagementActionOrphan ManagementAction = "Orphan" ) // A DeletionPolicy determines what should happen to the underlying external diff --git a/apis/common/v1/policies.go b/apis/common/v1/policies.go index d51120dfa..ae5e28de7 100644 --- a/apis/common/v1/policies.go +++ b/apis/common/v1/policies.go @@ -52,6 +52,10 @@ const ( // ManagementActionAll means that all of the above actions will be taken // by the Crossplane controllers. ManagementActionAll = common.ManagementActionAll + + // ManagementActionOrphan means that all actions except Delete will be taken + // by the Crossplane controllers. + ManagementActionOrphan ManagementAction = common.ManagementActionOrphan ) // A DeletionPolicy determines what should happen to the underlying external diff --git a/pkg/reconciler/managed/policies.go b/pkg/reconciler/managed/policies.go index 6bd1fd67a..b160f4ef5 100644 --- a/pkg/reconciler/managed/policies.go +++ b/pkg/reconciler/managed/policies.go @@ -106,6 +106,8 @@ func defaultSupportedManagementPolicies() []sets.Set[xpv1.ManagementAction] { // Useful when external resource lifecycle is managed elsewhere but you want // to allow Crossplane to make updates and discover state changes. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionUpdate, xpv1.ManagementActionLateInitialize), + // Orphan allows all actions except Delete. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionOrphan), } } @@ -187,7 +189,7 @@ func (m *ManagementPoliciesResolver) ShouldCreate() bool { return true } - return m.managementPolicies.HasAny(xpv1.ManagementActionCreate, xpv1.ManagementActionAll) + return m.managementPolicies.HasAny(xpv1.ManagementActionCreate, xpv1.ManagementActionAll, xpv1.ManagementActionOrphan) } // ShouldUpdate returns true if the Update action is allowed. @@ -197,7 +199,7 @@ func (m *ManagementPoliciesResolver) ShouldUpdate() bool { return true } - return m.managementPolicies.HasAny(xpv1.ManagementActionUpdate, xpv1.ManagementActionAll) + return m.managementPolicies.HasAny(xpv1.ManagementActionUpdate, xpv1.ManagementActionAll, xpv1.ManagementActionOrphan) } // ShouldLateInitialize returns true if the LateInitialize action is allowed. @@ -207,7 +209,7 @@ func (m *ManagementPoliciesResolver) ShouldLateInitialize() bool { return true } - return m.managementPolicies.HasAny(xpv1.ManagementActionLateInitialize, xpv1.ManagementActionAll) + return m.managementPolicies.HasAny(xpv1.ManagementActionLateInitialize, xpv1.ManagementActionAll, xpv1.ManagementActionOrphan) } // ShouldOnlyObserve returns true if the Observe action is allowed and all diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index 4a5d3f365..8db657e5c 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -1107,9 +1107,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotDisconnect, err)) } - if err := external.Disconnect(ctx); err != nil { - log.Debug("Cannot disconnect from provider", "error", err) - record.Event(managed, event.Warning(reasonCannotDisconnect, err)) + if external != nil { + if err := external.Disconnect(ctx); err != nil { + log.Debug("Cannot disconnect from provider", "error", err) + record.Event(managed, event.Warning(reasonCannotDisconnect, err)) + } } }() diff --git a/pkg/reconciler/managed/reconciler_legacy_test.go b/pkg/reconciler/managed/reconciler_legacy_test.go index b6f272807..2fd7fe49e 100644 --- a/pkg/reconciler/managed/reconciler_legacy_test.go +++ b/pkg/reconciler/managed/reconciler_legacy_test.go @@ -86,7 +86,7 @@ func TestReconciler(t *testing.T) { }, want: want{result: reconcile.Result{}}, }, - "UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn": { + "UnpublishConnectionDetailsDeletionPolicyDeleteOrphan": { reason: "Errors unpublishing connection details should trigger a requeue after a short wait.", args: args{ m: &fake.Manager{ @@ -1735,6 +1735,45 @@ func TestReconciler(t *testing.T) { }, want: want{result: reconcile.Result{Requeue: true}}, }, + "ManagementPolicyOrphanCreateSuccessful": { + reason: "Successful managed resource creation using management policy Orphan should trigger a requeue after a short wait.", + args: args{ + m: &fake.Manager{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + mg := asLegacyManaged(obj, 42) + mg.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + return nil + }), + MockUpdate: test.NewMockUpdateFn(nil), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + want := newLegacyManaged(42) + want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + meta.SetExternalCreatePending(want, time.Now()) + meta.SetExternalCreateSucceeded(want, time.Now()) + want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42)) + want.SetConditions(xpv1.Creating().WithObservedGeneration(42)) + if diff := cmp.Diff(want, obj, test.EquateConditions(), cmpopts.EquateApproxTime(1*time.Second)); diff != "" { + reason := "Successful managed resource creation should be reported as a conditioned status." + t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff) + } + return nil + }), + }, + Scheme: fake.SchemeWith(&fake.LegacyManaged{}), + }, + mg: resource.ManagedKind(fake.GVK(&fake.LegacyManaged{})), + o: []ReconcilerOption{ + WithInitializers(), + WithManagementPolicies(), + WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), + WithExternalConnector(&NopConnector{}), + WithCriticalAnnotationUpdater(CriticalAnnotationUpdateFn(func(_ context.Context, _ client.Object) error { return nil })), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + }, + }, + want: want{result: reconcile.Result{Requeue: true}}, + }, "ManagementPolicyCreateCreateSuccessful": { reason: "Successful managed resource creation using management policy Create should trigger a requeue after a short wait.", args: args{ @@ -1869,6 +1908,53 @@ func TestReconciler(t *testing.T) { }, want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}}, }, + "ManagementPolicyOrphanUpdateSuccessful": { + reason: "A successful managed resource update using management policies should trigger a requeue after a long wait.", + args: args{ + m: &fake.Manager{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + mg := asLegacyManaged(obj, 42) + mg.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + return nil + }), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + want := newLegacyManaged(42) + want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42).WithObservedGeneration(42)) + if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" { + reason := "A successful managed resource update should be reported as a conditioned status." + t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff) + } + return nil + }), + }, + Scheme: fake.SchemeWith(&fake.LegacyManaged{}), + }, + mg: resource.ManagedKind(fake.GVK(&fake.LegacyManaged{})), + o: []ReconcilerOption{ + WithInitializers(), + WithManagementPolicies(), + WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), + WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + c := &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true, ResourceUpToDate: false}, nil + }, + UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { + return ExternalUpdate{}, nil + }, + DisconnectFn: func(_ context.Context) error { + return nil + }, + } + return c, nil + })), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + }, + }, + want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}}, + }, "ManagementPolicyUpdateUpdateSuccessful": { reason: "A successful managed resource update using management policies should trigger a requeue after a long wait.", args: args{ @@ -2175,6 +2261,14 @@ func TestLegacyManagementPoliciesResolverShouldCreate(t *testing.T) { }, want: true, }, + "ManagementPoliciesEnabledHasCreateOrphan": { + reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", + args: args{ + managementPoliciesEnabled: true, + policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + want: true, + }, "ManagementPoliciesEnabledActionNotAllowed": { reason: "Should return false if management policies are enabled and managementPolicies does not have Create", args: args{ @@ -2236,6 +2330,14 @@ func TestLegacyManagementPoliciesResolverShouldUpdate(t *testing.T) { }, want: false, }, + "ManagementPoliciesEnabledHasUpdateOrphan": { + reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", + args: args{ + managementPoliciesEnabled: true, + policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + want: true, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -2289,6 +2391,14 @@ func TestLegacyManagementPoliciesResolverShouldLateInitialize(t *testing.T) { }, want: false, }, + "ManagementPoliciesEnabledHasLateInitializeOrphan": { + reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", + args: args{ + managementPoliciesEnabled: true, + policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + want: true, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -2459,6 +2569,33 @@ func TestLegacyShouldDelete(t *testing.T) { }, want: want{delete: false}, }, + "DeletionDeleteManagementActionOrphan": { + reason: "Should orphan if management policies are enabled and deletion policy is set to Delete and management policy does not have action Delete.", + args: args{ + managementPoliciesEnabled: true, + managed: &fake.LegacyManaged{ + Orphanable: fake.Orphanable{ + Policy: xpv1.DeletionDelete, + }, + Manageable: fake.Manageable{ + Policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + }, + }, + want: want{delete: false}, + }, + "DeletionManagementActionOrphan": { + reason: "Should orphan if management policies are enabled and deletion policy is not set and management policy does not have action Delete.", + args: args{ + managementPoliciesEnabled: true, + managed: &fake.LegacyManaged{ + Manageable: fake.Manageable{ + Policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + }, + }, + want: want{delete: false}, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { diff --git a/pkg/reconciler/managed/reconciler_modern_test.go b/pkg/reconciler/managed/reconciler_modern_test.go index 5b668b0a9..fd804cb04 100644 --- a/pkg/reconciler/managed/reconciler_modern_test.go +++ b/pkg/reconciler/managed/reconciler_modern_test.go @@ -1741,6 +1741,45 @@ func TestModernReconciler(t *testing.T) { }, want: want{result: reconcile.Result{Requeue: true}}, }, + "ManagementPolicyOrphanCreateSuccessful": { + reason: "Successful managed resource creation using management policy Orphan should trigger a requeue after a short wait.", + args: args{ + m: &fake.Manager{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + mg := asModernManaged(obj, 42) + mg.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + return nil + }), + MockUpdate: test.NewMockUpdateFn(nil), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + want := newModernManaged(42) + want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + meta.SetExternalCreatePending(want, time.Now()) + meta.SetExternalCreateSucceeded(want, time.Now()) + want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42)) + want.SetConditions(xpv1.Creating().WithObservedGeneration(42)) + if diff := cmp.Diff(want, obj, test.EquateConditions(), cmpopts.EquateApproxTime(1*time.Second)); diff != "" { + reason := "Successful managed resource creation should be reported as a conditioned status." + t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff) + } + return nil + }), + }, + Scheme: fake.SchemeWith(&fake.ModernManaged{}), + }, + mg: resource.ManagedKind(fake.GVK(&fake.ModernManaged{})), + o: []ReconcilerOption{ + WithInitializers(), + WithManagementPolicies(), + WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), + WithExternalConnector(&NopConnector{}), + WithCriticalAnnotationUpdater(CriticalAnnotationUpdateFn(func(_ context.Context, _ client.Object) error { return nil })), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + }, + }, + want: want{result: reconcile.Result{Requeue: true}}, + }, "ManagementPolicyCreateCreateSuccessful": { reason: "Successful managed resource creation using management policy Create should trigger a requeue after a short wait.", args: args{ @@ -1875,6 +1914,53 @@ func TestModernReconciler(t *testing.T) { }, want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}}, }, + "ManagementPolicyOrphanUpdateSuccessful": { + reason: "A successful managed resource update using management policy Orphan should trigger a requeue after a long wait.", + args: args{ + m: &fake.Manager{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + mg := asModernManaged(obj, 42) + mg.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + return nil + }), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + want := newModernManaged(42) + want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}) + want.SetConditions(xpv1.ReconcileSuccess().WithObservedGeneration(42).WithObservedGeneration(42)) + if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" { + reason := "A successful managed resource update should be reported as a conditioned status." + t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff) + } + return nil + }), + }, + Scheme: fake.SchemeWith(&fake.ModernManaged{}), + }, + mg: resource.ManagedKind(fake.GVK(&fake.ModernManaged{})), + o: []ReconcilerOption{ + WithInitializers(), + WithManagementPolicies(), + WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), + WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + c := &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true, ResourceUpToDate: false}, nil + }, + UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { + return ExternalUpdate{}, nil + }, + DisconnectFn: func(_ context.Context) error { + return nil + }, + } + return c, nil + })), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + }, + }, + want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}}, + }, "ManagementPolicyUpdateUpdateSuccessful": { reason: "A successful managed resource update using management policies should trigger a requeue after a long wait.", args: args{ @@ -2181,6 +2267,14 @@ func TestManagementPoliciesResolverShouldCreate(t *testing.T) { }, want: true, }, + "ManagementPoliciesEnabledHasCreateOrphan": { + reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", + args: args{ + managementPoliciesEnabled: true, + policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + want: true, + }, "ManagementPoliciesEnabledActionNotAllowed": { reason: "Should return false if management policies are enabled and managementPolicies does not have Create", args: args{ @@ -2242,6 +2336,14 @@ func TestManagementPoliciesResolverShouldUpdate(t *testing.T) { }, want: false, }, + "ManagementPoliciesEnabledHasUpdateOrphan": { + reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", + args: args{ + managementPoliciesEnabled: true, + policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + want: true, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -2295,6 +2397,14 @@ func TestManagementPoliciesResolverShouldLateInitialize(t *testing.T) { }, want: false, }, + "ManagementPoliciesEnabledHasLateInitializeOrphan": { + reason: "Should return true if management policies are enabled and managementPolicies has action Orphan", + args: args{ + managementPoliciesEnabled: true, + policy: xpv1.ManagementPolicies{xpv1.ManagementActionOrphan}, + }, + want: true, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) {