From 08a44ee06a63dcff159c64fbf9487ee252f2490f Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Mon, 19 Oct 2026 16:45:00 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=8C=B1=20reword=20ClusterObjectSet=20?= =?UTF-8?q?preflight=20rejection=20messages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use "preflight" instead of "pre-apply" in the PreconditionsNotMet Ready messages for the ClusterObjectSet revision- and phase-level checks that run before objects are applied. "Preflight" is the term users already recognize for checks that run before an operation, and it matches the vocabulary already used in the controller comments, the Ready condition documentation, and the concepts docs. The condition reason (PreconditionsNotMet) is unchanged. Also align the explanatory comments in the ClusterExtension controller that describe this reason. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Per G. da Silva --- .../controllers/clusterobjectset_controller.go | 4 ++-- .../operator-controller/controllers/common_controller.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index 1962bb4758..94ffca3e4e 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -252,7 +252,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl // or a dependency CRD still propagating), and those inputs are not watched. if verr := rres.GetValidationError(); verr != nil { l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s") - markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("A pre-apply check rejected the revision: %v.", verr)) + markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("A preflight check rejected the revision: %v.", verr)) return ctrl.Result{RequeueAfter: validationRetryInterval}, nil } @@ -275,7 +275,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl continue } l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", pres.GetName()) - markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("A pre-apply check rejected phase %q: %v.", pres.GetName(), verr)) + markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("A preflight check rejected phase %q: %v.", pres.GetName(), verr)) return ctrl.Result{RequeueAfter: validationRetryInterval}, nil } diff --git a/internal/operator-controller/controllers/common_controller.go b/internal/operator-controller/controllers/common_controller.go index 327cc60275..9c037df3e7 100644 --- a/internal/operator-controller/controllers/common_controller.go +++ b/internal/operator-controller/controllers/common_controller.go @@ -152,7 +152,7 @@ func setProgressingFromReady(ext *ocv1.ClusterExtension, readyCond *metav1.Condi // Rationale: // - Failed: Semantically indicates an error prevented installation // - Absent: Semantically indicates "not there yet" (neutral state, e.g., during healthy rollout) -// - ObjectCollision (a foreign owner), PreconditionsNotMet (a pre-apply precondition not yet met), +// - ObjectCollision (a foreign owner), PreconditionsNotMet (a preflight precondition not yet met), // or InvalidSpecification (an invalid revision spec) on Ready indicates an error // - A transient error leaves the Ready condition unchanged (or absent), so it maps to Absent, not Failed // - Other Ready reasons indicate healthy progress or terminal states handled elsewhere @@ -168,7 +168,7 @@ func determineFailureReason(rollingRevisions []*RevisionMetadata) string { latestRevision := rollingRevisions[len(rollingRevisions)-1] readyCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterObjectSetTypeReady) // ObjectCollision (an object is owned by another revision/controller), - // PreconditionsNotMet (a pre-apply precondition not yet met), or InvalidSpecification + // PreconditionsNotMet (a preflight precondition not yet met), or InvalidSpecification // (an invalid revision spec) on the Ready condition all indicate an error prevented installation. if readyCond != nil && (readyCond.Reason == ocv1.ClusterObjectSetReasonObjectCollision || readyCond.Reason == ocv1.ClusterObjectSetReasonPreconditionsNotMet || @@ -280,7 +280,7 @@ func progressingFromReady(ready *metav1.Condition, completed bool) metav1.Condit cond.Status = metav1.ConditionFalse cond.Reason = ocv1.ReasonProgressDeadlineExceeded case ocv1.ClusterObjectSetReasonObjectCollision, ocv1.ClusterObjectSetReasonPreconditionsNotMet: - // A foreign-owned object (ObjectCollision) or an unmet pre-apply precondition + // A foreign-owned object (ObjectCollision) or an unmet preflight precondition // (PreconditionsNotMet) prevents the rollout from proceeding, but the revision keeps // retrying and recovers once the obstruction clears. cond.Reason = ocv1.ReasonRetrying From 0caedfc77f2481f391faeba13dc690ecbc9c9842 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Mon, 19 Oct 2026 16:45:01 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=8C=B1=20add=20ClusterObjectSet=20sta?= =?UTF-8?q?tic=20preflight=20validation=20e2e=20scenarios?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a cos_static_validation.feature suite covering the revision-level (static, non-dry-run) preflight checks, which surface on the ClusterObjectSet as Ready=False/PreconditionsNotMet: - the same object (GVK + namespace + name) appearing in more than one phase, and - an object manifest that carries its own metadata.ownerReferences. Both assert the controller's "A preflight check rejected the revision: ..." message. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Per G. da Silva --- .../features/cos_static_validation.feature | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 test/e2e/features/cos_static_validation.feature diff --git a/test/e2e/features/cos_static_validation.feature b/test/e2e/features/cos_static_validation.feature new file mode 100644 index 0000000000..42e332ed27 --- /dev/null +++ b/test/e2e/features/cos_static_validation.feature @@ -0,0 +1,86 @@ +@BoxcutterRuntime +Feature: ClusterObjectSet static preflight validation + + As an OLM user applying a ClusterObjectSet directly, I want the revision to be + rejected with an actionable PreconditionsNotMet condition when its objects fail + the preflight validation checks, so that malformed revisions surface clearly + instead of being silently accepted. + + Background: + Given OLM is available + + Scenario: ClusterObjectSet reports PreconditionsNotMet when the same object appears in multiple phases + Given namespace "${TEST_NAMESPACE}" is available + When ClusterObjectSet is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + name: ${COS_NAME} + spec: + group: ${NAME} + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: phase-a + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: dup-cm + namespace: ${TEST_NAMESPACE} + data: + k: v + - name: phase-b + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: dup-cm + namespace: ${TEST_NAMESPACE} + data: + k: v + revision: 1 + """ + Then ClusterObjectSet "${COS_NAME}" reports Ready as False with Reason PreconditionsNotMet and Message includes: + """ + A preflight check rejected the revision: revision "${COS_NAME}" (1): phase "phase-a": /v1, Kind=ConfigMap ${TEST_NAMESPACE}/dup-cm: duplicate object found in phases: phase-a, phase-b + """ + + Scenario: ClusterObjectSet reports PreconditionsNotMet when an object manifest carries ownerReferences + Given namespace "${TEST_NAMESPACE}" is available + When ClusterObjectSet is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + name: ${COS_NAME} + spec: + group: ${NAME} + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: phase-a + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: meta-cm + namespace: ${TEST_NAMESPACE} + ownerReferences: + - apiVersion: v1 + kind: ConfigMap + name: some-owner + uid: "11111111-1111-1111-1111-111111111111" + controller: true + data: + k: v + revision: 1 + """ + Then ClusterObjectSet "${COS_NAME}" reports Ready as False with Reason PreconditionsNotMet and Message includes: + """ + A preflight check rejected the revision: revision "${COS_NAME}" (1): phase "phase-a": /v1, Kind=ConfigMap ${TEST_NAMESPACE}/meta-cm: metadata.ownerReferences: Forbidden: must be empty. + """