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 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. + """