From 77d5a2069cf4be95d7a7486a9c826b6b528c476e Mon Sep 17 00:00:00 2001 From: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:09:03 +0100 Subject: [PATCH 1/4] Use MutateAndPatchStatus/Spec in MCPOIDCConfig controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Status writes now flow through controllerutil.MutateAndPatchStatus and finalizer writes through controllerutil.MutateAndPatchSpec, matching the MCPAuthzConfig controller migration in #4777. The previous r.Status().Update calls sent full PUT bodies that would clobber conditions written by any disjoint owner of Status.Conditions on this CRD; the previous r.Update calls had no optimistic-lock guard around the finalizer array. Condition and field mutations move inside the helper closures so the pre-mutate snapshot reflects the live state rather than already containing the change — a MutateAndPatchStatus prerequisite. A small helper, setOIDCConfigValidTrueCondition, factors out the Valid=True transition. Add a PreservesForeignConditions regression test mirroring the MCPAuthzConfig guard, asserting a foreign-owned condition survives a reconcile. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../controllers/mcpoidcconfig_controller.go | 113 +++++++++++------- .../mcpoidcconfig_controller_test.go | 66 ++++++++++ 2 files changed, 133 insertions(+), 46 deletions(-) diff --git a/cmd/thv-operator/controllers/mcpoidcconfig_controller.go b/cmd/thv-operator/controllers/mcpoidcconfig_controller.go index c02bdaf33f..5bb6af1d91 100644 --- a/cmd/thv-operator/controllers/mcpoidcconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpoidcconfig_controller.go @@ -70,10 +70,14 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques return r.handleDeletion(ctx, oidcConfig) } - // Add finalizer if it doesn't exist + // Add finalizer if it doesn't exist. + // MutateAndPatchSpec wraps an optimistic-lock merge patch: any concurrent + // finalizer additions land on the live object via the apiserver, and our + // patch only carries the field we changed. See .claude/rules/operator.md. if !controllerutil.ContainsFinalizer(oidcConfig, OIDCConfigFinalizerName) { - controllerutil.AddFinalizer(oidcConfig, OIDCConfigFinalizerName) - if err := r.Update(ctx, oidcConfig); err != nil { + if err := ctrlutil.MutateAndPatchSpec(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { + controllerutil.AddFinalizer(c, OIDCConfigFinalizerName) + }); err != nil { logger.Error(err, "Failed to add finalizer") return ctrl.Result{}, err } @@ -83,28 +87,20 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Validate spec configuration early if err := oidcConfig.Validate(); err != nil { logger.Error(err, "MCPOIDCConfig spec validation failed") - meta.SetStatusCondition(&oidcConfig.Status.Conditions, metav1.Condition{ - Type: mcpv1beta1.ConditionTypeOIDCConfigValid, - Status: metav1.ConditionFalse, - Reason: mcpv1beta1.ConditionReasonOIDCConfigInvalid, - Message: err.Error(), - ObservedGeneration: oidcConfig.Generation, - }) - if updateErr := r.Status().Update(ctx, oidcConfig); updateErr != nil { + if updateErr := ctrlutil.MutateAndPatchStatus(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { + meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeOIDCConfigValid, + Status: metav1.ConditionFalse, + Reason: mcpv1beta1.ConditionReasonOIDCConfigInvalid, + Message: err.Error(), + ObservedGeneration: c.Generation, + }) + }); updateErr != nil { logger.Error(updateErr, "Failed to update status after validation error") } return ctrl.Result{}, nil // Don't requeue on validation errors - user must fix spec } - // Validation succeeded - set Valid=True condition - conditionChanged := meta.SetStatusCondition(&oidcConfig.Status.Conditions, metav1.Condition{ - Type: mcpv1beta1.ConditionTypeOIDCConfigValid, - Status: metav1.ConditionTrue, - Reason: mcpv1beta1.ConditionReasonOIDCConfigValid, - Message: "Spec validation passed", - ObservedGeneration: oidcConfig.Generation, - }) - // Calculate the hash of the current configuration configHash := r.calculateConfigHash(oidcConfig.Spec) @@ -115,10 +111,11 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques "oldHash", oidcConfig.Status.ConfigHash, "newHash", configHash) - oidcConfig.Status.ConfigHash = configHash - oidcConfig.Status.ObservedGeneration = oidcConfig.Generation - - if err := r.Status().Update(ctx, oidcConfig); err != nil { + if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { + setOIDCConfigValidTrueCondition(c) + c.Status.ConfigHash = configHash + c.Status.ObservedGeneration = c.Generation + }); err != nil { logger.Error(err, "Failed to update MCPOIDCConfig status") return ctrl.Result{}, err } @@ -129,24 +126,46 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques referencingWorkloads, err := r.findReferencingWorkloads(ctx, oidcConfig) if err != nil { logger.Error(err, "Failed to find referencing workloads") - } else if !ctrlutil.WorkloadRefsEqual(oidcConfig.Status.ReferencingWorkloads, referencingWorkloads) || - oidcConfig.Status.ReferenceCount != workloadReferenceCount(referencingWorkloads) { - oidcConfig.Status.ReferencingWorkloads = referencingWorkloads - oidcConfig.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) - conditionChanged = true + // Fall through: the status patch below is best-effort and still ensures + // the Valid=True condition is set even when the reference refresh fails. } - // Update condition if it changed (even without hash change) - if conditionChanged { - if err := r.Status().Update(ctx, oidcConfig); err != nil { - logger.Error(err, "Failed to update MCPOIDCConfig status after condition change") - return ctrl.Result{}, err + // Single status patch covering the steady-state success path: ensure the + // Valid=True condition is set, and refresh the references list if it + // changed. MutateAndPatchStatus short-circuits on an empty diff so the + // no-op case still skips the wire call (SteadyStateNoOp behaviour is + // preserved). + if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { + setOIDCConfigValidTrueCondition(c) + if referencingWorkloads != nil && + (!ctrlutil.WorkloadRefsEqual(c.Status.ReferencingWorkloads, referencingWorkloads) || + c.Status.ReferenceCount != workloadReferenceCount(referencingWorkloads)) { + c.Status.ReferencingWorkloads = referencingWorkloads + c.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) } + }); err != nil { + logger.Error(err, "Failed to update MCPOIDCConfig status") + return ctrl.Result{}, err } return ctrl.Result{}, nil } +// setOIDCConfigValidTrueCondition stamps ConditionTypeOIDCConfigValid=True onto +// the supplied object. It is callable inside a MutateAndPatchStatus closure: the +// closure receives the freshly-snapshotted object, and SetStatusCondition only +// mutates Conditions when the desired state differs, so a no-op reconcile +// produces an empty patch body that the helper skips. +func setOIDCConfigValidTrueCondition(c *mcpv1beta1.MCPOIDCConfig) { + meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeOIDCConfigValid, + Status: metav1.ConditionTrue, + Reason: mcpv1beta1.ConditionReasonOIDCConfigValid, + Message: "Spec validation passed", + ObservedGeneration: c.Generation, + }) +} + // calculateConfigHash calculates a hash of the MCPOIDCConfig spec using Kubernetes utilities func (*MCPOIDCConfigReconciler) calculateConfigHash(spec mcpv1beta1.MCPOIDCConfigSpec) string { return ctrlutil.CalculateConfigHash(spec) @@ -175,16 +194,17 @@ func (r *MCPOIDCConfigReconciler) handleDeletion( "oidcConfig", oidcConfig.Name, "referencingWorkloads", referencingWorkloads) - meta.SetStatusCondition(&oidcConfig.Status.Conditions, metav1.Condition{ - Type: mcpv1beta1.ConditionTypeDeletionBlocked, - Status: metav1.ConditionTrue, - Reason: "ReferencedByWorkloads", - Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), - ObservedGeneration: oidcConfig.Generation, - }) - oidcConfig.Status.ReferencingWorkloads = referencingWorkloads - oidcConfig.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) - if updateErr := r.Status().Update(ctx, oidcConfig); updateErr != nil { + if updateErr := ctrlutil.MutateAndPatchStatus(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { + meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeDeletionBlocked, + Status: metav1.ConditionTrue, + Reason: "ReferencedByWorkloads", + Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), + ObservedGeneration: c.Generation, + }) + c.Status.ReferencingWorkloads = referencingWorkloads + c.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) + }); updateErr != nil { logger.Error(updateErr, "Failed to update status during deletion block") } @@ -192,8 +212,9 @@ func (r *MCPOIDCConfigReconciler) handleDeletion( return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } - controllerutil.RemoveFinalizer(oidcConfig, OIDCConfigFinalizerName) - if err := r.Update(ctx, oidcConfig); err != nil { + if err := ctrlutil.MutateAndPatchSpec(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { + controllerutil.RemoveFinalizer(c, OIDCConfigFinalizerName) + }); err != nil { logger.Error(err, "Failed to remove finalizer") return ctrl.Result{}, err } diff --git a/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go b/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go index cef0fe7071..552cbd663b 100644 --- a/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -636,3 +637,68 @@ func TestMCPOIDCConfigReconciler_ReferenceCountUpdatedWithWorkloads(t *testing.T assert.Empty(t, updatedConfig.Status.ReferencingWorkloads) assert.EqualValues(t, 0, updatedConfig.Status.ReferenceCount) } + +// TestMCPOIDCConfigReconciler_PreservesForeignConditions guards the +// MutateAndPatchStatus migration: a condition owned by a hypothetical disjoint +// writer of this resource's Status.Conditions must survive a reconcile. A raw +// r.Status().Update sends a full PUT that would clobber the conditions array; +// the merge-patch helper carries only the controller's own condition diff. +func TestMCPOIDCConfigReconciler_PreservesForeignConditions(t *testing.T) { + t.Parallel() + + ctx := t.Context() + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + oidcConfig := &mcpv1beta1.MCPOIDCConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "test-config", Namespace: "default", Generation: 1}, + Spec: mcpv1beta1.MCPOIDCConfigSpec{ + Type: mcpv1beta1.MCPOIDCConfigTypeInline, + Inline: &mcpv1beta1.InlineOIDCSharedConfig{ + Issuer: "https://accounts.google.com", + ClientID: "test-client", + }, + }, + Status: mcpv1beta1.MCPOIDCConfigStatus{ + Conditions: []metav1.Condition{ + { + Type: "ForeignControllerSays", + Status: metav1.ConditionTrue, + Reason: "ExternallySet", + Message: "set by a hypothetical sibling owner of this resource", + LastTransitionTime: metav1.Now(), + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(oidcConfig). + WithStatusSubresource(&mcpv1beta1.MCPOIDCConfig{}). + Build() + r := &MCPOIDCConfigReconciler{Client: fakeClient, Scheme: scheme} + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: oidcConfig.Name, Namespace: oidcConfig.Namespace}} + + // First reconcile adds the finalizer; second runs the success path and + // writes Valid=True without touching any foreign condition. + _, err := r.Reconcile(ctx, req) + require.NoError(t, err) + _, err = r.Reconcile(ctx, req) + require.NoError(t, err) + + var after mcpv1beta1.MCPOIDCConfig + require.NoError(t, fakeClient.Get(ctx, req.NamespacedName, &after)) + + foreign := meta.FindStatusCondition(after.Status.Conditions, "ForeignControllerSays") + require.NotNil(t, foreign, + "foreign condition must survive an MCPOIDCConfig reconcile — otherwise merge-patch is replacing the conditions array wholesale") + assert.Equal(t, metav1.ConditionTrue, foreign.Status, "foreign condition value must not be modified") + assert.Equal(t, "ExternallySet", foreign.Reason) + + // And our own Valid=True landed. + own := meta.FindStatusCondition(after.Status.Conditions, mcpv1beta1.ConditionTypeOIDCConfigValid) + require.NotNil(t, own, "controller-owned Valid condition must land") + assert.Equal(t, metav1.ConditionTrue, own.Status) +} From 7b9b123bf5a51cf086d87d5e8b75f0b0287b2027 Mon Sep 17 00:00:00 2001 From: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:16:02 +0100 Subject: [PATCH 2/4] Use MutateAndPatchStatus/Spec in MCPExternalAuthConfig controller Status writes now flow through controllerutil.MutateAndPatchStatus and finalizer writes through controllerutil.MutateAndPatchSpec, matching the MCPAuthzConfig and MCPOIDCConfig controllers. The previous r.Status().Update calls sent full PUT bodies that would clobber conditions owned by any disjoint writer of Status.Conditions on this CRD; the previous r.Update calls had no optimistic-lock guard around the finalizer array. The IdentitySynthesized advisory was previously computed once in memory before validation. Because MutateAndPatchStatus snapshots the object on entry and any pre-mutate change is dropped from the merge patch, that upfront mutation is removed; the advisory is now recomputed inside each status-write closure via a new setValidTrueAndSynthesized helper (and directly on the validation-failure and setInvalid paths). applyIdentitySynthesizedCondition is idempotent on the same spec, so this preserves the advisory transition on every path. The setInvalid doc comment is updated to drop the stale upfront-mutation reference. Add a PreservesForeignConditions regression test mirroring the sibling guards. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcpexternalauthconfig_controller.go | 211 +++++++++--------- .../mcpexternalauthconfig_controller_test.go | 80 +++++++ .../mcpoidcconfig_controller_test.go | 20 +- 3 files changed, 201 insertions(+), 110 deletions(-) diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go index 7e0a653527..213a31b25c 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go @@ -74,10 +74,14 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr return r.handleDeletion(ctx, externalAuthConfig) } - // Add finalizer if it doesn't exist + // Add finalizer if it doesn't exist. + // MutateAndPatchSpec wraps an optimistic-lock merge patch: any concurrent + // finalizer additions land on the live object via the apiserver, and our + // patch only carries the field we changed. See .claude/rules/operator.md. if !controllerutil.ContainsFinalizer(externalAuthConfig, ExternalAuthConfigFinalizerName) { - controllerutil.AddFinalizer(externalAuthConfig, ExternalAuthConfigFinalizerName) - if err := r.Update(ctx, externalAuthConfig); err != nil { + if err := ctrlutil.MutateAndPatchSpec(ctx, r.Client, externalAuthConfig, func(c *mcpv1beta1.MCPExternalAuthConfig) { + controllerutil.AddFinalizer(c, ExternalAuthConfigFinalizerName) + }); err != nil { logger.Error(err, "Failed to add finalizer") return ctrl.Result{}, err } @@ -85,38 +89,28 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr return ctrl.Result{RequeueAfter: externalAuthConfigRequeueDelay}, nil } - // Compute the IdentitySynthesized advisory upfront, before validation. - // The advisory is a pure function of the upstream provider field shape - // (specifically, which OAuth2 upstreams have nil userInfo) and does not - // depend on issuer URL validity or other Validate() concerns. Computing - // it before validation ensures the advisory tracks the current spec on - // every reconcile — including the validation-failure path — so a broken - // edit cannot leave a stale True/upstream-name dangling. - // - // Note: the OBO failure path routes through setInvalid, which discards - // this in-memory mutation (its MutateAndPatchStatus call re-fetches and - // re-applies the advisory inside its patch closure). The in-memory - // mutation here is therefore load-bearing only on the validation-failure - // path (the r.Status().Update on the ValidationFailed return) and the - // Valid=True path (the r.Status().Update on the conditionChanged write). - // The function is idempotent on the same spec, so the double computation - // on the OBO path is benign. - syntheticChanged := r.applyIdentitySynthesizedCondition(externalAuthConfig) - // Validate spec configuration early if err := externalAuthConfig.Validate(); err != nil { logger.Error(err, "MCPExternalAuthConfig spec validation failed") - // Update status with validation error. The synthesis condition mutated - // above is part of the same in-memory Conditions slice and will land - // in this same write. - meta.SetStatusCondition(&externalAuthConfig.Status.Conditions, metav1.Condition{ - Type: mcpv1beta1.ConditionTypeValid, - Status: metav1.ConditionFalse, - Reason: "ValidationFailed", - Message: err.Error(), - ObservedGeneration: externalAuthConfig.Generation, - }) - if updateErr := r.Status().Update(ctx, externalAuthConfig); updateErr != nil { + // Fold the IdentitySynthesized advisory into the same patch as the + // Valid=False write so a broken edit cannot leave a stale advisory + // (True/upstream-name) dangling. Both mutations happen inside the + // closure: MutateAndPatchStatus snapshots the object on entry, so any + // pre-mutate change would land in both halves of the diff and be + // silently dropped. applyIdentitySynthesizedCondition is a pure + // function of the current spec, so it recomputes the advisory even on + // the validation-failure path. + if updateErr := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig, + func(c *mcpv1beta1.MCPExternalAuthConfig) { + r.applyIdentitySynthesizedCondition(c) + meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeValid, + Status: metav1.ConditionFalse, + Reason: "ValidationFailed", + Message: err.Error(), + ObservedGeneration: c.Generation, + }) + }); updateErr != nil { logger.Error(updateErr, "Failed to update status after validation error") } return ctrl.Result{}, nil // Don't requeue on validation errors - user must fix spec @@ -125,25 +119,15 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr // Dispatch OBO-typed configs through the registered handler. The default // handler returns obo.ErrEnterpriseRequired so upstream-only builds surface // Valid=False / Reason=EnterpriseRequired here rather than failing later - // inside a consumer reconciler with a generic "unsupported" error. + // inside a consumer reconciler with a generic "unsupported" error. The OBO + // failure path routes through setInvalid, which applies the advisory inside + // its own patch closure. if externalAuthConfig.Spec.Type == mcpv1beta1.ExternalAuthTypeOBO { if handled, err := r.triageOBOValidation(ctx, externalAuthConfig); handled { return ctrl.Result{}, err } } - // Validation succeeded - set Valid=True condition - conditionChanged := meta.SetStatusCondition(&externalAuthConfig.Status.Conditions, metav1.Condition{ - Type: mcpv1beta1.ConditionTypeValid, - Status: metav1.ConditionTrue, - Reason: "ValidationSucceeded", - Message: "Spec validation passed", - ObservedGeneration: externalAuthConfig.Generation, - }) - if syntheticChanged { - conditionChanged = true - } - // Calculate the hash of the current configuration configHash := r.calculateConfigHash(externalAuthConfig.Spec) @@ -153,19 +137,29 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr return r.handleConfigHashChange(ctx, externalAuthConfig, configHash) } - // Update condition if it changed (even without hash change) - if conditionChanged { - if err := r.Status().Update(ctx, externalAuthConfig); err != nil { - logger.Error(err, "Failed to update MCPExternalAuthConfig status after condition change") - return ctrl.Result{}, err - } - } - - // Even when hash hasn't changed, update referencing workloads list. - // This ensures ReferencingWorkloads is updated when MCPServers are created or deleted. + // Steady-state success path: ensure Valid=True and the IdentitySynthesized + // advisory are set, and refresh the referencing-workloads list, in a single + // status patch. return r.updateReferencingWorkloads(ctx, externalAuthConfig) } +// setValidTrueAndSynthesized stamps ConditionTypeValid=True and refreshes the +// IdentitySynthesized advisory on the supplied object. It is callable inside a +// MutateAndPatchStatus closure: applyIdentitySynthesizedCondition is idempotent +// on the same spec and SetStatusCondition only mutates Conditions on a real +// change, so a no-op reconcile produces an empty patch body that the helper +// skips. +func (r *MCPExternalAuthConfigReconciler) setValidTrueAndSynthesized(c *mcpv1beta1.MCPExternalAuthConfig) { + r.applyIdentitySynthesizedCondition(c) + meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeValid, + Status: metav1.ConditionTrue, + Reason: "ValidationSucceeded", + Message: "Spec validation passed", + ObservedGeneration: c.Generation, + }) +} + // calculateConfigHash calculates a hash of the MCPExternalAuthConfig spec using Kubernetes utilities func (*MCPExternalAuthConfigReconciler) calculateConfigHash(spec mcpv1beta1.MCPExternalAuthConfigSpec) string { return ctrlutil.CalculateConfigHash(spec) @@ -253,16 +247,13 @@ func (r *MCPExternalAuthConfigReconciler) triageOBOValidation( // an out-of-tree handler must be registered) for this branch to clear, so // requeuing buys nothing. // -// Callers in Reconcile() may have already mutated cfg.Status.Conditions in -// memory (notably applyIdentitySynthesizedCondition). MutateAndPatchStatus -// diffs the post-mutate object against the snapshot it takes at the start of -// the call, so any mutation present in cfg before the helper runs lands in -// both halves of the diff and silently disappears from the merge patch. To -// avoid losing the IdentitySynthesized advisory transition (e.g., when a user -// switches a config from embeddedAuthServer to obo), this helper re-fetches -// the object from the apiserver and re-applies the synthesized-condition -// computation inside the patch closure so both the advisory transition and -// the Valid=False condition land in the same patch. +// The IdentitySynthesized advisory is recomputed inside the patch closure so +// both the advisory transition (e.g., when a user switches a config from +// embeddedAuthServer to obo) and the Valid=False condition land in the same +// merge patch. The object is re-fetched first so the closure mutates a clean +// snapshot: MutateAndPatchStatus diffs the post-mutate object against the +// snapshot it takes on entry, so any mutation already present before the +// helper runs would land in both halves of the diff and be dropped. func (r *MCPExternalAuthConfigReconciler) setInvalid( ctx context.Context, cfg *mcpv1beta1.MCPExternalAuthConfig, @@ -281,14 +272,10 @@ func (r *MCPExternalAuthConfigReconciler) setInvalid( } return ctrlutil.MutateAndPatchStatus(ctx, r.Client, fresh, func(c *mcpv1beta1.MCPExternalAuthConfig) { // applyIdentitySynthesizedCondition is idempotent on the same spec; - // re-applying it inside the closure folds the advisory transition - // into the same patch as the Valid=False write below. This - // re-invocation is load-bearing: removing it would cause - // MutateAndPatchStatus to silently drop the IdentitySynthesized - // transition because the pre-mutate snapshot already contains the - // in-memory mutation from line 95. See - // TestMCPExternalAuthConfigReconciler_OBO_ClearsStaleIdentitySynthesized - // for the regression guard. + // re-applying it inside the closure folds the advisory transition into + // the same patch as the Valid=False write below. See + // TestMCPExternalAuthConfigReconciler_IdentitySynthesizedTransitionsOnValidationFailure + // for the related validation-path regression guard. r.applyIdentitySynthesizedCondition(c) meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ Type: mcpv1beta1.ConditionTypeValid, @@ -311,10 +298,6 @@ func (r *MCPExternalAuthConfigReconciler) handleConfigHashChange( "oldHash", externalAuthConfig.Status.ConfigHash, "newHash", configHash) - // Update the status with the new hash - externalAuthConfig.Status.ConfigHash = configHash - externalAuthConfig.Status.ObservedGeneration = externalAuthConfig.Generation - // Find all MCPServers that reference this MCPExternalAuthConfig referencingServers, err := r.findReferencingMCPServers(ctx, externalAuthConfig) if err != nil { @@ -322,17 +305,25 @@ func (r *MCPExternalAuthConfigReconciler) handleConfigHashChange( return ctrl.Result{}, fmt.Errorf("failed to find referencing MCPServers: %w", err) } - // Update the status with the list of referencing workloads + // Build the list of referencing workloads refs := make([]mcpv1beta1.WorkloadReference, 0, len(referencingServers)) for _, server := range referencingServers { refs = append(refs, mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPServer, Name: server.Name}) } ctrlutil.SortWorkloadRefs(refs) - externalAuthConfig.Status.ReferencingWorkloads = refs - externalAuthConfig.Status.ReferenceCount = workloadReferenceCount(refs) - // Update the MCPExternalAuthConfig status - if err := r.Status().Update(ctx, externalAuthConfig); err != nil { + // Single status patch covering the hash-change success path: the new hash + // and generation, the refreshed reference list, and the Valid=True / + // IdentitySynthesized conditions. All mutations happen inside the closure so + // the pre-mutate snapshot stays clean (a MutateAndPatchStatus prerequisite). + if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig, + func(c *mcpv1beta1.MCPExternalAuthConfig) { + r.setValidTrueAndSynthesized(c) + c.Status.ConfigHash = configHash + c.Status.ObservedGeneration = c.Generation + c.Status.ReferencingWorkloads = refs + c.Status.ReferenceCount = workloadReferenceCount(refs) + }); err != nil { logger.Error(err, "Failed to update MCPExternalAuthConfig status") return ctrl.Result{}, err } @@ -377,16 +368,18 @@ func (r *MCPExternalAuthConfigReconciler) handleDeletion( "externalAuthConfig", externalAuthConfig.Name, "referencingWorkloads", referencingWorkloads) - meta.SetStatusCondition(&externalAuthConfig.Status.Conditions, metav1.Condition{ - Type: mcpv1beta1.ConditionTypeDeletionBlocked, - Status: metav1.ConditionTrue, - Reason: "ReferencedByWorkloads", - Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), - ObservedGeneration: externalAuthConfig.Generation, - }) - externalAuthConfig.Status.ReferencingWorkloads = referencingWorkloads - externalAuthConfig.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) - if updateErr := r.Status().Update(ctx, externalAuthConfig); updateErr != nil { + if updateErr := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig, + func(c *mcpv1beta1.MCPExternalAuthConfig) { + meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeDeletionBlocked, + Status: metav1.ConditionTrue, + Reason: "ReferencedByWorkloads", + Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), + ObservedGeneration: c.Generation, + }) + c.Status.ReferencingWorkloads = referencingWorkloads + c.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) + }); updateErr != nil { logger.Error(updateErr, "Failed to update status during deletion block") } @@ -395,8 +388,10 @@ func (r *MCPExternalAuthConfigReconciler) handleDeletion( } // No references, safe to remove finalizer and allow deletion - controllerutil.RemoveFinalizer(externalAuthConfig, ExternalAuthConfigFinalizerName) - if err := r.Update(ctx, externalAuthConfig); err != nil { + if err := ctrlutil.MutateAndPatchSpec(ctx, r.Client, externalAuthConfig, + func(c *mcpv1beta1.MCPExternalAuthConfig) { + controllerutil.RemoveFinalizer(c, ExternalAuthConfigFinalizerName) + }); err != nil { logger.Error(err, "Failed to remove finalizer") return ctrl.Result{}, err } @@ -662,27 +657,33 @@ func (r *MCPExternalAuthConfigReconciler) mapMCPRemoteProxyToExternalAuthConfig( return requests } -// updateReferencingWorkloads finds referencing workloads and updates the status if the list changed +// updateReferencingWorkloads writes the steady-state success status in a single +// patch: it ensures Valid=True and the IdentitySynthesized advisory are set and +// refreshes the referencing-workloads list. MutateAndPatchStatus short-circuits +// on an empty diff, so a no-op reconcile skips the wire call. func (r *MCPExternalAuthConfigReconciler) updateReferencingWorkloads( ctx context.Context, externalAuthConfig *mcpv1beta1.MCPExternalAuthConfig, ) (ctrl.Result, error) { + logger := log.FromContext(ctx) + refs, err := r.findReferencingWorkloads(ctx, externalAuthConfig) if err != nil { - logger := log.FromContext(ctx) logger.Error(err, "Failed to find referencing workloads") return ctrl.Result{}, fmt.Errorf("failed to find referencing workloads: %w", err) } - if !ctrlutil.WorkloadRefsEqual(externalAuthConfig.Status.ReferencingWorkloads, refs) || - externalAuthConfig.Status.ReferenceCount != workloadReferenceCount(refs) { - externalAuthConfig.Status.ReferencingWorkloads = refs - externalAuthConfig.Status.ReferenceCount = workloadReferenceCount(refs) - if err := r.Status().Update(ctx, externalAuthConfig); err != nil { - logger := log.FromContext(ctx) - logger.Error(err, "Failed to update MCPExternalAuthConfig status") - return ctrl.Result{}, err - } + if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig, + func(c *mcpv1beta1.MCPExternalAuthConfig) { + r.setValidTrueAndSynthesized(c) + if !ctrlutil.WorkloadRefsEqual(c.Status.ReferencingWorkloads, refs) || + c.Status.ReferenceCount != workloadReferenceCount(refs) { + c.Status.ReferencingWorkloads = refs + c.Status.ReferenceCount = workloadReferenceCount(refs) + } + }); err != nil { + logger.Error(err, "Failed to update MCPExternalAuthConfig status") + return ctrl.Result{}, err } return ctrl.Result{}, nil diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go index 194378fabb..c540855b05 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go @@ -1773,3 +1773,83 @@ func TestMCPExternalAuthConfigReconciler_OBO_ErrorTriageInReconcile(t *testing.T }) } } + +// TestMCPExternalAuthConfigReconciler_PreservesForeignConditions locks in the +// property the MutateAndPatchStatus migration is meant to protect: the +// controller's snapshot-and-diff machinery does not erase Status.Conditions +// entries it doesn't own when building its patch body. +// +// The test fails if anyone mutates conditions outside the MutateAndPatchStatus +// closure (the snapshot would already contain the in-memory mutation, the diff +// would be empty, and the controller-owned Valid condition would never land — +// the assertion at the bottom catches that). +// +// It does NOT catch a regression to r.Status().Update on its own: under the +// fake client there is no concurrent writer between Get and Patch, so a full +// PUT of the in-memory object (which still includes the foreign condition) +// would persist successfully. Exercising the merge-patch-vs-PUT difference for +// concurrent writers requires a WithInterceptorFuncs-backed scenario. +func TestMCPExternalAuthConfigReconciler_PreservesForeignConditions(t *testing.T) { + t.Parallel() + + ctx := t.Context() + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + externalAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "test-config", Namespace: "default", Generation: 1}, + Spec: mcpv1beta1.MCPExternalAuthConfigSpec{ + Type: mcpv1beta1.ExternalAuthTypeTokenExchange, + TokenExchange: &mcpv1beta1.TokenExchangeConfig{ + TokenURL: "https://oauth.example.com/token", + ClientID: "test-client", + ClientSecretRef: &mcpv1beta1.SecretKeyRef{ + Name: "test-secret", + Key: "client-secret", + }, + Audience: "backend-service", + }, + }, + Status: mcpv1beta1.MCPExternalAuthConfigStatus{ + Conditions: []metav1.Condition{ + { + Type: "ForeignControllerSays", + Status: metav1.ConditionTrue, + Reason: "ExternallySet", + Message: "set by a hypothetical sibling owner of this resource", + LastTransitionTime: metav1.Now(), + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(externalAuthConfig). + WithStatusSubresource(&mcpv1beta1.MCPExternalAuthConfig{}). + Build() + r := &MCPExternalAuthConfigReconciler{Client: fakeClient, Scheme: scheme} + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: externalAuthConfig.Name, Namespace: externalAuthConfig.Namespace}} + + // First reconcile adds the finalizer; second runs the success path and + // writes Valid=True without touching any foreign condition. + _, err := r.Reconcile(ctx, req) + require.NoError(t, err) + _, err = r.Reconcile(ctx, req) + require.NoError(t, err) + + var after mcpv1beta1.MCPExternalAuthConfig + require.NoError(t, fakeClient.Get(ctx, req.NamespacedName, &after)) + + foreign := findCondition(after.Status.Conditions, "ForeignControllerSays") + require.NotNil(t, foreign, + "foreign condition must survive an MCPExternalAuthConfig reconcile — otherwise merge-patch is replacing the conditions array wholesale") + assert.Equal(t, metav1.ConditionTrue, foreign.Status, "foreign condition value must not be modified") + assert.Equal(t, "ExternallySet", foreign.Reason) + + // And our own Valid=True landed. + own := findCondition(after.Status.Conditions, mcpv1beta1.ConditionTypeValid) + require.NotNil(t, own, "controller-owned Valid condition must land") + assert.Equal(t, metav1.ConditionTrue, own.Status) +} diff --git a/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go b/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go index 552cbd663b..eb29c01001 100644 --- a/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go @@ -638,11 +638,21 @@ func TestMCPOIDCConfigReconciler_ReferenceCountUpdatedWithWorkloads(t *testing.T assert.EqualValues(t, 0, updatedConfig.Status.ReferenceCount) } -// TestMCPOIDCConfigReconciler_PreservesForeignConditions guards the -// MutateAndPatchStatus migration: a condition owned by a hypothetical disjoint -// writer of this resource's Status.Conditions must survive a reconcile. A raw -// r.Status().Update sends a full PUT that would clobber the conditions array; -// the merge-patch helper carries only the controller's own condition diff. +// TestMCPOIDCConfigReconciler_PreservesForeignConditions locks in the property +// the MutateAndPatchStatus migration is meant to protect: the controller's +// snapshot-and-diff machinery does not erase Status.Conditions entries it +// doesn't own when building its patch body. +// +// The test fails if anyone mutates conditions outside the MutateAndPatchStatus +// closure (the snapshot would already contain the in-memory mutation, the diff +// would be empty, and the controller-owned Valid condition would never land — +// the assertion at the bottom catches that). +// +// It does NOT catch a regression to r.Status().Update on its own: under the +// fake client there is no concurrent writer between Get and Patch, so a full +// PUT of the in-memory object (which still includes the foreign condition) +// would persist successfully. Exercising the merge-patch-vs-PUT difference for +// concurrent writers requires a WithInterceptorFuncs-backed scenario. func TestMCPOIDCConfigReconciler_PreservesForeignConditions(t *testing.T) { t.Parallel() From b497c54a9f0bcabd212639dd24c8b8d0ea5c7615 Mon Sep 17 00:00:00 2001 From: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:24:22 +0100 Subject: [PATCH 3/4] Drop unused bool return from applyIdentitySynthesizedCondition The advisory helper's bool return was consumed only by the upfront in-memory call removed in the MutateAndPatchStatus migration; every remaining caller ignores it inside a status-write closure. Drop the return value to satisfy the unparam linter and refresh the doc comment. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcpexternalauthconfig_controller.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go index 213a31b25c..be4a4f538e 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go @@ -168,27 +168,30 @@ func (*MCPExternalAuthConfigReconciler) calculateConfigHash(spec mcpv1beta1.MCPE // applyIdentitySynthesizedCondition sets ConditionTypeIdentitySynthesized // True when any OAuth2 upstream has nil userInfo, False when every upstream // has userInfo configured, and removes it for non-embeddedAuthServer types -// where the question is moot. Returns true if the in-memory condition list -// changed so the caller can fold this into the next status write. +// where the question is moot. It is idempotent on the same spec and is called +// inside the status-write closures so the advisory is recomputed on every +// status patch. func (*MCPExternalAuthConfigReconciler) applyIdentitySynthesizedCondition( cfg *mcpv1beta1.MCPExternalAuthConfig, -) bool { +) { if cfg.Spec.Type != mcpv1beta1.ExternalAuthTypeEmbeddedAuthServer || cfg.Spec.EmbeddedAuthServer == nil { - return meta.RemoveStatusCondition(&cfg.Status.Conditions, mcpv1beta1.ConditionTypeIdentitySynthesized) + meta.RemoveStatusCondition(&cfg.Status.Conditions, mcpv1beta1.ConditionTypeIdentitySynthesized) + return } syntheticUpstreams := cfg.Spec.EmbeddedAuthServer.SyntheticIdentityUpstreams() if len(syntheticUpstreams) == 0 { - return meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ + meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ Type: mcpv1beta1.ConditionTypeIdentitySynthesized, Status: metav1.ConditionFalse, Reason: mcpv1beta1.ConditionReasonIdentitySynthesizedInactive, Message: "All OAuth2 upstreams have userInfo configured; user identity is resolved from the upstream", ObservedGeneration: cfg.Generation, }) + return } - return meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ + meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ Type: mcpv1beta1.ConditionTypeIdentitySynthesized, Status: metav1.ConditionTrue, Reason: mcpv1beta1.ConditionReasonIdentitySynthesizedActive, From b9b2e38c09d2c61f7329f9b8f6e6913e4c25dcd2 Mon Sep 17 00:00:00 2001 From: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:50:32 +0100 Subject: [PATCH 4/4] Harden migrated OIDC paths per review feedback Address review feedback on the status-write migration: - Gate the OIDC steady-state reference refresh on a captured findErr == nil rather than the referencingWorkloads != nil sentinel, decoupling the guard from findReferencingWorkloads' nil-on-error / non-nil-empty-on-success contract. - Assert the migrated DeletionBlocked condition (and reference bookkeeping) is persisted in the OIDC blocking-deletion test, which previously only checked the requeue and finalizer retention. - Use meta.FindStatusCondition in the externalauth PreservesForeignConditions test for parity with the OIDC and MCPAuthzConfig sibling guards. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcpexternalauthconfig_controller_test.go | 5 +++-- .../controllers/mcpoidcconfig_controller.go | 22 +++++++++---------- .../controllers/mcpserver_oidcconfig_test.go | 11 ++++++++++ 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go index c540855b05..7b6bc220b6 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -1842,14 +1843,14 @@ func TestMCPExternalAuthConfigReconciler_PreservesForeignConditions(t *testing.T var after mcpv1beta1.MCPExternalAuthConfig require.NoError(t, fakeClient.Get(ctx, req.NamespacedName, &after)) - foreign := findCondition(after.Status.Conditions, "ForeignControllerSays") + foreign := meta.FindStatusCondition(after.Status.Conditions, "ForeignControllerSays") require.NotNil(t, foreign, "foreign condition must survive an MCPExternalAuthConfig reconcile — otherwise merge-patch is replacing the conditions array wholesale") assert.Equal(t, metav1.ConditionTrue, foreign.Status, "foreign condition value must not be modified") assert.Equal(t, "ExternallySet", foreign.Reason) // And our own Valid=True landed. - own := findCondition(after.Status.Conditions, mcpv1beta1.ConditionTypeValid) + own := meta.FindStatusCondition(after.Status.Conditions, mcpv1beta1.ConditionTypeValid) require.NotNil(t, own, "controller-owned Valid condition must land") assert.Equal(t, metav1.ConditionTrue, own.Status) } diff --git a/cmd/thv-operator/controllers/mcpoidcconfig_controller.go b/cmd/thv-operator/controllers/mcpoidcconfig_controller.go index 5bb6af1d91..8c609a3e94 100644 --- a/cmd/thv-operator/controllers/mcpoidcconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpoidcconfig_controller.go @@ -122,22 +122,22 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } - // Refresh ReferencingWorkloads list - referencingWorkloads, err := r.findReferencingWorkloads(ctx, oidcConfig) - if err != nil { - logger.Error(err, "Failed to find referencing workloads") - // Fall through: the status patch below is best-effort and still ensures - // the Valid=True condition is set even when the reference refresh fails. + // Refresh ReferencingWorkloads list. On error, fall through with the lookup + // result skipped: the status patch below is best-effort and still ensures + // the Valid=True condition is set even when the reference refresh fails. + referencingWorkloads, findErr := r.findReferencingWorkloads(ctx, oidcConfig) + if findErr != nil { + logger.Error(findErr, "Failed to find referencing workloads") } // Single status patch covering the steady-state success path: ensure the - // Valid=True condition is set, and refresh the references list if it - // changed. MutateAndPatchStatus short-circuits on an empty diff so the - // no-op case still skips the wire call (SteadyStateNoOp behaviour is - // preserved). + // Valid=True condition is set, and refresh the references list when the + // lookup succeeded and the list changed. MutateAndPatchStatus short-circuits + // on an empty diff so the no-op case still skips the wire call + // (SteadyStateNoOp behaviour is preserved). if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { setOIDCConfigValidTrueCondition(c) - if referencingWorkloads != nil && + if findErr == nil && (!ctrlutil.WorkloadRefsEqual(c.Status.ReferencingWorkloads, referencingWorkloads) || c.Status.ReferenceCount != workloadReferenceCount(referencingWorkloads)) { c.Status.ReferencingWorkloads = referencingWorkloads diff --git a/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go b/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go index c0044f40f2..96de0cb2e1 100644 --- a/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go +++ b/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go @@ -363,6 +363,17 @@ func TestMCPOIDCConfigReconciler_handleDeletion_BlocksWhenReferenced(t *testing. assert.Greater(t, result.RequeueAfter, time.Duration(0), "should requeue while referenced") assert.Contains(t, cfg.Finalizers, OIDCConfigFinalizerName, "finalizer must remain") + + // The DeletionBlocked condition is written through MutateAndPatchStatus; + // re-fetch to confirm it (and the referencing-workload bookkeeping) was + // persisted rather than only mutated in memory. + var after mcpv1beta1.MCPOIDCConfig + require.NoError(t, fc.Get(ctx, client.ObjectKeyFromObject(cfg), &after)) + blocked := meta.FindStatusCondition(after.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) + require.NotNil(t, blocked, "DeletionBlocked condition must be set while referenced") + assert.Equal(t, metav1.ConditionTrue, blocked.Status) + assert.Equal(t, "ReferencedByWorkloads", blocked.Reason) + assert.EqualValues(t, 1, after.Status.ReferenceCount, "referencing workload must be recorded") } func TestMCPOIDCConfigReconciler_handleDeletion_AllowsWhenNotReferenced(t *testing.T) {