Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions internal/operator-controller/controllers/common_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ||
Expand Down Expand Up @@ -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
Expand Down
86 changes: 86 additions & 0 deletions test/e2e/features/cos_static_validation.feature
Original file line number Diff line number Diff line change
@@ -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.
"""
Loading