Skip to content

🌱 Add collisionProtection inheritance hierarchy to ClusterExtensionRevision #2513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Feb 19, 2026
Merged

🌱 Add collisionProtection inheritance hierarchy to ClusterExtensionRevision #2513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 29 additions & 1 deletion api/v1/clusterextensionrevision_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,19 @@ type ClusterExtensionRevisionSpec struct {
// +optional
// <opcon:experimental>
ProgressDeadlineMinutes int32 `json:"progressDeadlineMinutes,omitempty"`

// collisionProtection specifies the default collision protection strategy for all objects
// in this revision. Individual phases or objects can override this value.
//
// When set, this value is used as the default for any phase or object that does not
// explicitly specify its own collisionProtection.
//
// The resolution order is: object > phase > spec
//
// +required
// +kubebuilder:validation:Enum=Prevent;IfNoController;None
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="collisionProtection is immutable"
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
}

// ClusterExtensionRevisionLifecycleState specifies the lifecycle state of the ClusterExtensionRevision.
Expand Down Expand Up @@ -144,6 +157,19 @@ type ClusterExtensionRevisionPhase struct {
// +required
// +kubebuilder:validation:MaxItems=50
Objects []ClusterExtensionRevisionObject `json:"objects"`

// collisionProtection specifies the default collision protection strategy for all objects
// in this phase. Individual objects can override this value.
//
// When set, this value is used as the default for any object in this phase that does not
// explicitly specify its own collisionProtection.
//
// When omitted, we use .spec.collistionProtection as the default for any object in this phase that does not
// explicitly specify its own collisionProtection.
//
// +optional
// +kubebuilder:validation:Enum=Prevent;IfNoController;None
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
}

// ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part
Expand Down Expand Up @@ -174,7 +200,9 @@ type ClusterExtensionRevisionObject struct {
// Use this setting with extreme caution as it may cause multiple controllers to fight over
// the same resource, resulting in increased load on the API server and etcd.
//
// +required
// When omitted, the value is inherited from the phase, then spec.
//
// +optional
// +kubebuilder:validation:Enum=Prevent;IfNoController;None
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
}
Expand Down
114 changes: 103 additions & 11 deletions api/v1/clusterextensionrevision_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

func TestClusterExtensionRevisionImmutability(t *testing.T) {
Expand All @@ -21,18 +22,20 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
}{
"revision is immutable": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
},
updateFunc: func(cer *ClusterExtensionRevision) {
cer.Spec.Revision = 2
},
},
"phases may be initially empty": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
Phases: []ClusterExtensionRevisionPhase{},
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
Phases: []ClusterExtensionRevisionPhase{},
},
updateFunc: func(cer *ClusterExtensionRevision) {
cer.Spec.Phases = []ClusterExtensionRevisionPhase{
Expand All @@ -46,8 +49,9 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
},
"phases may be initially unset": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
},
updateFunc: func(cer *ClusterExtensionRevision) {
cer.Spec.Phases = []ClusterExtensionRevisionPhase{
Expand All @@ -61,8 +65,9 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
},
"phases are immutable if not empty": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
Phases: []ClusterExtensionRevisionPhase{
{
Name: "foo",
Expand All @@ -79,6 +84,16 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
}
},
},
"spec collisionProtection is immutable": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
},
updateFunc: func(cer *ClusterExtensionRevision) {
cer.Spec.CollisionProtection = CollisionProtectionNone
},
},
} {
t.Run(name, func(t *testing.T) {
cer := &ClusterExtensionRevision{
Expand Down Expand Up @@ -124,8 +139,9 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
},
"revision must be positive": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
},
valid: true,
},
Expand Down Expand Up @@ -192,6 +208,70 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
},
valid: false,
},
"spec collisionProtection accepts Prevent": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
},
valid: true,
},
"spec collisionProtection accepts IfNoController": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionIfNoController,
},
valid: true,
},
"spec collisionProtection accepts None": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionNone,
},
valid: true,
},
"spec collisionProtection is required": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
},
valid: false,
},
"spec collisionProtection rejects invalid values": {
Copy link
Contributor

@perdasilva perdasilva Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going with omitempty and "" for unset, maybe it's also worth checking that "" is rejected?

spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtection("Invalid"),
},
valid: false,
},
"spec collisionProtection must be set": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
},
valid: false,
},
"object collisionProtection is optional": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
CollisionProtection: CollisionProtectionPrevent,
Phases: []ClusterExtensionRevisionPhase{
{
Name: "deploy",
Objects: []ClusterExtensionRevisionObject{
{
Object: configMap(),
},
},
},
},
},
valid: true,
},
} {
t.Run(name, func(t *testing.T) {
cer := &ClusterExtensionRevision{
Expand All @@ -211,3 +291,15 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
})
}
}

func configMap() unstructured.Unstructured {
return unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
"kind": "ConfigMap",
"metadata": map[string]interface{}{
"name": "test-cm",
},
},
}
}
1 change: 1 addition & 0 deletions api/v1/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestValidate(t *testing.T) {
}
defaultRevisionSpec := func(s *ClusterExtensionRevisionSpec) *ClusterExtensionRevisionSpec {
s.Revision = 1
s.CollisionProtection = CollisionProtectionPrevent
return s
}
c := newClient(t)
Expand Down
36 changes: 35 additions & 1 deletion ...rator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@ spec:
spec:
description: spec defines the desired state of the ClusterExtensionRevision.
properties:
collisionProtection:
description: |-
collisionProtection specifies the default collision protection strategy for all objects
in this revision. Individual phases or objects can override this value.

When set, this value is used as the default for any phase or object that does not
explicitly specify its own collisionProtection.

The resolution order is: object > phase > spec
enum:
- Prevent
- IfNoController
- None
type: string
x-kubernetes-validations:
- message: collisionProtection is immutable
rule: self == oldSelf
lifecycleState:
description: |-
lifecycleState specifies the lifecycle state of the ClusterExtensionRevision.
Expand Down Expand Up @@ -102,6 +119,21 @@ spec:
ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered
complete only after all objects pass their status probes.
properties:
collisionProtection:
description: |-
collisionProtection specifies the default collision protection strategy for all objects
in this phase. Individual objects can override this value.

When set, this value is used as the default for any object in this phase that does not
explicitly specify its own collisionProtection.

When omitted, we use .spec.collistionProtection as the default for any object in this phase that does not
explicitly specify its own collisionProtection.
enum:
- Prevent
- IfNoController
- None
type: string
name:
description: |-
name is a required identifier for this phase.
Expand Down Expand Up @@ -149,6 +181,8 @@ spec:
owned by another controller.
Use this setting with extreme caution as it may cause multiple controllers to fight over
the same resource, resulting in increased load on the API server and etcd.

When omitted, the value is inherited from the phase, then spec.
enum:
- Prevent
- IfNoController
Expand All @@ -163,7 +197,6 @@ spec:
x-kubernetes-embedded-resource: true
x-kubernetes-preserve-unknown-fields: true
required:
- collisionProtection
- object
type: object
maxItems: 50
Expand Down Expand Up @@ -205,6 +238,7 @@ spec:
- message: revision is immutable
rule: self == oldSelf
required:
- collisionProtection
- lifecycleState
- revision
type: object
Expand Down
11 changes: 6 additions & 5 deletions internal/operator-controller/applier/boxcutter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
sanitizedUnstructured(ctx, &obj)

objs = append(objs, ocv1.ClusterExtensionRevisionObject{
Object: obj,
CollisionProtection: ocv1.CollisionProtectionNone, // allow to adopt objects from previous release
Object: obj,
})
}

Expand All @@ -88,6 +87,7 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
})
rev.Name = fmt.Sprintf("%s-1", ext.Name)
rev.Spec.Revision = 1
rev.Spec.CollisionProtection = ocv1.CollisionProtectionNone // allow to adopt objects from previous release
return rev, nil
}

Expand Down Expand Up @@ -147,11 +147,12 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
sanitizedUnstructured(ctx, &unstr)

objs = append(objs, ocv1.ClusterExtensionRevisionObject{
Object: unstr,
CollisionProtection: ocv1.CollisionProtectionPrevent,
Object: unstr,
})
}
return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil
rev := r.buildClusterExtensionRevision(objs, ext, revisionAnnotations)
rev.Spec.CollisionProtection = ocv1.CollisionProtectionPrevent
return rev, nil
}

// sanitizedUnstructured takes an unstructured obj, removes status if present, and returns a sanitized copy containing only the allowed metadata entries set below.
Expand Down
11 changes: 5 additions & 6 deletions internal/operator-controller/applier/boxcutter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
LifecycleState: ocv1.ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
LifecycleState: ocv1.ClusterExtensionRevisionLifecycleStateActive,
CollisionProtection: ocv1.CollisionProtectionNone,
Revision: 1,
Phases: []ocv1.ClusterExtensionRevisionPhase{
{
Name: "deploy",
Expand All @@ -132,7 +133,6 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
},
},
},
CollisionProtection: ocv1.CollisionProtectionNone,
},
{
Object: unstructured.Unstructured{
Expand All @@ -146,7 +146,6 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
},
},
},
CollisionProtection: ocv1.CollisionProtectionNone,
},
},
},
Expand Down Expand Up @@ -215,6 +214,8 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
}, rev.Labels)
t.Log("by checking the revision number is 0")
require.Equal(t, int64(0), rev.Spec.Revision)
t.Log("by checking the spec-level collisionProtection is set")
require.Equal(t, ocv1.CollisionProtectionPrevent, rev.Spec.CollisionProtection)
t.Log("by checking the rendered objects are present in the correct phases")
require.Equal(t, []ocv1.ClusterExtensionRevisionPhase{
{
Expand All @@ -231,7 +232,6 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
"spec": map[string]interface{}{},
},
},
CollisionProtection: ocv1.CollisionProtectionPrevent,
},
{
Object: unstructured.Unstructured{
Expand Down Expand Up @@ -260,7 +260,6 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
},
},
},
CollisionProtection: ocv1.CollisionProtectionPrevent,
},
},
},
Expand Down
Loading
Loading