From b5b18ab831ad8fa1a6009de1534232563c426549 Mon Sep 17 00:00:00 2001 From: Bob Haddleton Date: Fri, 12 Sep 2025 14:06:16 -0500 Subject: [PATCH 1/2] Add MustCreate management policy Signed-off-by: Bob Haddleton --- apis/common/policies.go | 4 + apis/common/v1/policies.go | 4 + pkg/reconciler/managed/policies.go | 41 ++++++++- pkg/reconciler/managed/reconciler.go | 15 ++++ .../managed/reconciler_legacy_test.go | 87 +++++++++++++++++++ .../managed/reconciler_modern_test.go | 87 +++++++++++++++++++ 6 files changed, 237 insertions(+), 1 deletion(-) diff --git a/apis/common/policies.go b/apis/common/policies.go index c2e2cfec5..f303ee015 100644 --- a/apis/common/policies.go +++ b/apis/common/policies.go @@ -49,6 +49,10 @@ const ( // ManagementActionAll means that all of the above actions will be taken // by the Crossplane controllers. ManagementActionAll ManagementAction = "*" + + // ManagementActionMustCreate means that the external resource MUST be created + // by the managed resource and if it already exists an error condition is raised. + ManagementActionMustCreate ManagementAction = "MustCreate" ) // 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..53c2da3f9 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 + + // ManagementActionMustCreate means that the external resource must be created + // by this MR and if it already exists an error condition is raised and no further processing is executed. + ManagementActionMustCreate = common.ManagementActionMustCreate ) // 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..46ca4b664 100644 --- a/pkg/reconciler/managed/policies.go +++ b/pkg/reconciler/managed/policies.go @@ -59,6 +59,8 @@ func defaultSupportedManagementPolicies() []sets.Set[xpv1.ManagementAction] { sets.New[xpv1.ManagementAction](xpv1.ManagementActionAll), // All actions explicitly set, the same as default. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionLateInitialize, xpv1.ManagementActionDelete), + // All actions explicitly set with MustCreate instead of Create. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionLateInitialize, xpv1.ManagementActionDelete), // ObserveOnly, just observe action is done, the external resource is // considered as read-only. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve), @@ -68,29 +70,55 @@ func defaultSupportedManagementPolicies() []sets.Set[xpv1.ManagementAction] { // No LateInitialize filling in the spec.forProvider, allowing some // external resource fields to be managed externally. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionDelete), + // No LateInitialize filling in the spec.forProvider, allowing some + // external resource fields to be managed externally. With MustCreate. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionDelete), // No Delete, the external resource is not deleted when the managed // resource is deleted. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionLateInitialize), + // No Delete, the external resource is not deleted when the managed + // resource is deleted. With MustCreate. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionLateInitialize), // No Delete and no LateInitialize, the external resource is not deleted // when the managed resource is deleted and the spec.forProvider is not // late initialized. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionUpdate), + // No Delete and no LateInitialize, the external resource is not deleted + // when the managed resource is deleted and the spec.forProvider is not + // late initialized. With MustCreate. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionUpdate), // No Update, the external resource is not updated when the managed // resource is updated. Useful for immutable external resources. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionDelete, xpv1.ManagementActionLateInitialize), + // No Update, the external resource is not updated when the managed + // resource is updated. Useful for immutable external resources. With MustCreate. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete, xpv1.ManagementActionLateInitialize), // No Update and no Delete, the external resource is not updated // when the managed resource is updated and the external resource // is not deleted when the managed resource is deleted. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionLateInitialize), + // No Update and no Delete, the external resource is not updated + // when the managed resource is updated and the external resource + // is not deleted when the managed resource is deleted. With MustCreate. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionLateInitialize), // No Update and no LateInitialize, the external resource is not updated // when the managed resource is updated and the spec.forProvider is not // late initialized. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionDelete), + // No Update and no LateInitialize, the external resource is not updated + // when the managed resource is updated and the spec.forProvider is not + // late initialized. With MustCreate. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete), // No Update, no Delete and no LateInitialize, the external resource is // not updated when the managed resource is updated, the external resource // is not deleted when the managed resource is deleted and the // spec.forProvider is not late initialized. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate), + // No Update, no Delete and no LateInitialize, the external resource is + // not updated when the managed resource is updated, the external resource + // is not deleted when the managed resource is deleted and the + // spec.forProvider is not late initialized. With MustCreate. + sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate), // Like ObserveOnly, but the external resource is deleted when the // managed resource is deleted. sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionDelete), @@ -187,7 +215,18 @@ func (m *ManagementPoliciesResolver) ShouldCreate() bool { return true } - return m.managementPolicies.HasAny(xpv1.ManagementActionCreate, xpv1.ManagementActionAll) + return m.managementPolicies.HasAny(xpv1.ManagementActionCreate, xpv1.ManagementActionAll, xpv1.ManagementActionMustCreate) +} + +// MustCreate returns true if the Create action is required. If the resource already exists an error will +// be raised in the reconciler. +// If the management policy feature is disabled, it returns true. +func (m *ManagementPoliciesResolver) MustCreate() bool { + if !m.enabled { + return false + } + + return m.managementPolicies.Has(xpv1.ManagementActionMustCreate) } // ShouldUpdate returns true if the Update action is allowed. diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index 8db657e5c..a033639e1 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -70,6 +70,8 @@ const ( errExternalResourceNotExist = "external resource does not exist" errManagedNotImplemented = "managed resource does not implement connection details" + + errMustCreate = "managed resource has mustcreate policy but external resource already exists" ) // Event reasons. @@ -115,6 +117,8 @@ type ManagementPoliciesChecker interface { //nolint:interfacebloat // This has t ShouldOnlyObserve() bool // ShouldCreate returns true if the Create action is allowed. ShouldCreate() bool + // MustCreate returns true if the create action is required to be executed + MustCreate() bool // ShouldLateInitialize returns true if the LateInitialize action is // allowed. ShouldLateInitialize() bool @@ -1144,6 +1148,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) } + never := time.Time{} + // If the resource already exists, the MustCreate policy is set, and there are no create annotations then + // this MR did not create the resource and an error is raised. + if observation.ResourceExists && policy.MustCreate() && meta.GetExternalCreatePending(managed).Equal(never) && meta.GetExternalCreateSucceeded(managed).Equal(never) && meta.GetExternalCreateFailed(managed).Equal(never) { + log.Debug(errMustCreate) + record.Event(managed, event.Warning(reasonCannotCreate, errors.New(errMustCreate))) + status.MarkConditions(xpv1.Creating(), xpv1.ReconcileError(errors.New(errMustCreate))) + + return reconcile.Result{Requeue: false}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + } + // If this resource has a non-zero creation grace period we want to wait // for that period to expire before we trust that the resource really // doesn't exist. This is because some external APIs are eventually diff --git a/pkg/reconciler/managed/reconciler_legacy_test.go b/pkg/reconciler/managed/reconciler_legacy_test.go index b6f272807..3b5fff138 100644 --- a/pkg/reconciler/managed/reconciler_legacy_test.go +++ b/pkg/reconciler/managed/reconciler_legacy_test.go @@ -1774,6 +1774,93 @@ func TestReconciler(t *testing.T) { }, want: want{result: reconcile.Result{Requeue: true}}, }, + "ManagementPolicyMustCreateCreateSuccessful": { + reason: "Successful managed resource creation using management policy MustCreate 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.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + 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.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + 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}}, + }, + "ResourceExistsMustCreateError": { + reason: "When a resource exists and the policy is MustCreate an error condition should be raised.", + 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.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + return nil + }), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + want := newLegacyManaged(42) + want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + want.SetConditions(xpv1.Creating().WithObservedGeneration(42), xpv1.ReconcileError(errors.New(errMustCreate)).WithObservedGeneration(42).WithObservedGeneration(42)) + if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" { + reason := "With MustCreate, a successful managed resource observation with no creation annotations should be reported as an error condition." + 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(), + WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + c := &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true}, nil + }, + DisconnectFn: func(_ context.Context) error { + return nil + }, + } + return c, nil + })), + withLocalConnectionPublishers(LocalConnectionPublisherFns{ + PublishConnectionFn: func(_ context.Context, _ resource.LocalConnectionSecretOwner, _ ConnectionDetails) (bool, error) { + return false, nil + }, + }), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + }, + }, + want: want{result: reconcile.Result{Requeue: false}}, + }, "ManagementPolicyImmutable": { reason: "Successful reconciliation skipping update should trigger a requeue after a long wait.", args: args{ diff --git a/pkg/reconciler/managed/reconciler_modern_test.go b/pkg/reconciler/managed/reconciler_modern_test.go index 5b668b0a9..85941ecef 100644 --- a/pkg/reconciler/managed/reconciler_modern_test.go +++ b/pkg/reconciler/managed/reconciler_modern_test.go @@ -1780,6 +1780,93 @@ func TestModernReconciler(t *testing.T) { }, want: want{result: reconcile.Result{Requeue: true}}, }, + "ManagementPolicyMustCreateCreateSuccessful": { + reason: "Successful managed resource creation using management policy MustCreate 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.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + 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.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + 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}}, + }, + "ResourceExistsMustCreateError": { + reason: "When a resource exists and the policy is MustCreate an error condition should be raised.", + 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.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + return nil + }), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + want := newModernManaged(42) + want.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionDelete}) + want.SetConditions(xpv1.Creating().WithObservedGeneration(42), xpv1.ReconcileError(errors.New(errMustCreate)).WithObservedGeneration(42).WithObservedGeneration(42)) + if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" { + reason := "With MustCreate, a successful managed resource observation with no creation annotations should be reported as an error condition." + 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(), + WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + c := &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true}, nil + }, + DisconnectFn: func(_ context.Context) error { + return nil + }, + } + return c, nil + })), + withLocalConnectionPublishers(LocalConnectionPublisherFns{ + PublishConnectionFn: func(_ context.Context, _ resource.LocalConnectionSecretOwner, _ ConnectionDetails) (bool, error) { + return false, nil + }, + }), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + }, + }, + want: want{result: reconcile.Result{Requeue: false}}, + }, "ManagementPolicyImmutable": { reason: "Successful reconciliation skipping update should trigger a requeue after a long wait.", args: args{ From 597a4dfd49b82de6a1f91893c29211c4de5adfc0 Mon Sep 17 00:00:00 2001 From: Bob Haddleton Date: Fri, 12 Sep 2025 17:21:14 -0500 Subject: [PATCH 2/2] Add MustCreate to kubebuilder list Signed-off-by: Bob Haddleton --- apis/common/policies.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/common/policies.go b/apis/common/policies.go index f303ee015..65e598d40 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;*;MustCreate type ManagementAction string const (