🌱 Add collisionProtection inheritance hierarchy to ClusterExtensionRevision

api/v1/clusterextensionrevision_types.go

@@ -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.
@@ -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
@@ -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"`
 }

api/v1/clusterextensionrevision_types_test.go

@@ -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) {
@@ -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{
@@ -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{
@@ -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",
@@ -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{
@@ -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,
  },
@@ -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": {
+ 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{
@@ -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",
+ },
+ },
+	}
+}

api/v1/validation_test.go

@@ -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)

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml

@@ -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.
@@ -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.
@@ -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
@@ -163,7 +197,6 @@ spec:
  x-kubernetes-embedded-resource: true
  x-kubernetes-preserve-unknown-fields: true
  required:
- - collisionProtection
  - object
  type: object
  maxItems: 50
@@ -205,6 +238,7 @@ spec:
  - message: revision is immutable
  rule: self == oldSelf
  required:
+ - collisionProtection
  - lifecycleState
  - revision
  type: object

internal/operator-controller/applier/boxcutter.go

@@ -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,
  })
 	}
 
@@ -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
 }
 
@@ -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.

internal/operator-controller/applier/boxcutter_test.go

@@ -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",
@@ -132,7 +133,6 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
  },
  },
  },
- CollisionProtection: ocv1.CollisionProtectionNone,
  },
  {
  Object: unstructured.Unstructured{
@@ -146,7 +146,6 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
  },
  },
  },
- CollisionProtection: ocv1.CollisionProtectionNone,
  },
  },
  },
@@ -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{
  {
@@ -231,7 +232,6 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
  "spec": map[string]interface{}{},
  },
  },
- CollisionProtection: ocv1.CollisionProtectionPrevent,
  },
  {
  Object: unstructured.Unstructured{
@@ -260,7 +260,6 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
  },
  },
  },
- CollisionProtection: ocv1.CollisionProtectionPrevent,
  },
  },
  },