Skip to content

Commit 11ccb33

Browse files
pedjakclaude
pedjak
and
claude
committed
Add collisionProtection inheritance hierarchy to ClusterExtensionRevision
Add collisionProtection as a required field at the spec level and an optional field at the phase level, with inheritance resolution: object > phase > spec > default ("Prevent"). This reduces verbosity by letting users set a single spec-level default instead of repeating the value on every object. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 130c987 commit 11ccb33

File tree

10 files changed

+313
-28
lines changed

10 files changed

+313
-28
lines changed

‎api/v1/clusterextensionrevision_types.go‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,19 @@ type ClusterExtensionRevisionSpec struct {
105105
// +optional
106106
// <opcon:experimental>
107107
ProgressDeadlineMinutes int32 `json:"progressDeadlineMinutes,omitempty"`
108+
109+
// collisionProtection specifies the default collision protection strategy for all objects
110+
// in this revision. Individual phases or objects can override this value.
111+
//
112+
// When set, this value is used as the default for any phase or object that does not
113+
// explicitly specify its own collisionProtection.
114+
//
115+
// The resolution order is: object > phase > spec > default ("Prevent").
116+
//
117+
// +required
118+
// +kubebuilder:validation:Enum=Prevent;IfNoController;None
119+
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="collisionProtection is immutable"
120+
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
108121
}
109122

110123
// ClusterExtensionRevisionLifecycleState specifies the lifecycle state of the ClusterExtensionRevision.
@@ -144,6 +157,18 @@ type ClusterExtensionRevisionPhase struct {
144157
// +required
145158
// +kubebuilder:validation:MaxItems=50
146159
Objects []ClusterExtensionRevisionObject `json:"objects"`
160+
161+
// collisionProtection specifies the default collision protection strategy for all objects
162+
// in this phase. Individual objects can override this value.
163+
//
164+
// When set, this value is used as the default for any object in this phase that does not
165+
// explicitly specify its own collisionProtection.
166+
//
167+
// The resolution order is: object > phase > spec > default ("Prevent").
168+
//
169+
// +optional
170+
// +kubebuilder:validation:Enum=Prevent;IfNoController;None
171+
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
147172
}
148173

149174
// ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part
@@ -174,7 +199,9 @@ type ClusterExtensionRevisionObject struct {
174199
// Use this setting with extreme caution as it may cause multiple controllers to fight over
175200
// the same resource, resulting in increased load on the API server and etcd.
176201
//
177-
// +required
202+
// When omitted, the value is inherited from the phase, then spec, then defaults to "Prevent".
203+
//
204+
// +optional
178205
// +kubebuilder:validation:Enum=Prevent;IfNoController;None
179206
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
180207
}

‎api/v1/clusterextensionrevision_types_test.go‎

Lines changed: 103 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/stretchr/testify/require"
99
"k8s.io/apimachinery/pkg/api/errors"
1010
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1112
)
1213

1314
func TestClusterExtensionRevisionImmutability(t *testing.T) {
@@ -21,18 +22,20 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
2122
}{
2223
"revision is immutable": {
2324
spec: ClusterExtensionRevisionSpec{
24-
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
25-
Revision: 1,
25+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
26+
Revision: 1,
27+
CollisionProtection: CollisionProtectionPrevent,
2628
},
2729
updateFunc: func(cer *ClusterExtensionRevision) {
2830
cer.Spec.Revision = 2
2931
},
3032
},
3133
"phases may be initially empty": {
3234
spec: ClusterExtensionRevisionSpec{
33-
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
34-
Revision: 1,
35-
Phases: []ClusterExtensionRevisionPhase{},
35+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
36+
Revision: 1,
37+
CollisionProtection: CollisionProtectionPrevent,
38+
Phases: []ClusterExtensionRevisionPhase{},
3639
},
3740
updateFunc: func(cer *ClusterExtensionRevision) {
3841
cer.Spec.Phases = []ClusterExtensionRevisionPhase{
@@ -46,8 +49,9 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
4649
},
4750
"phases may be initially unset": {
4851
spec: ClusterExtensionRevisionSpec{
49-
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
50-
Revision: 1,
52+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
53+
Revision: 1,
54+
CollisionProtection: CollisionProtectionPrevent,
5155
},
5256
updateFunc: func(cer *ClusterExtensionRevision) {
5357
cer.Spec.Phases = []ClusterExtensionRevisionPhase{
@@ -61,8 +65,9 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
6165
},
6266
"phases are immutable if not empty": {
6367
spec: ClusterExtensionRevisionSpec{
64-
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
65-
Revision: 1,
68+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
69+
Revision: 1,
70+
CollisionProtection: CollisionProtectionPrevent,
6671
Phases: []ClusterExtensionRevisionPhase{
6772
{
6873
Name: "foo",
@@ -79,6 +84,16 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
7984
}
8085
},
8186
},
87+
"spec collisionProtection is immutable": {
88+
spec: ClusterExtensionRevisionSpec{
89+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
90+
Revision: 1,
91+
CollisionProtection: CollisionProtectionPrevent,
92+
},
93+
updateFunc: func(cer *ClusterExtensionRevision) {
94+
cer.Spec.CollisionProtection = CollisionProtectionNone
95+
},
96+
},
8297
} {
8398
t.Run(name, func(t *testing.T) {
8499
cer := &ClusterExtensionRevision{
@@ -124,8 +139,9 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
124139
},
125140
"revision must be positive": {
126141
spec: ClusterExtensionRevisionSpec{
127-
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
128-
Revision: 1,
142+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
143+
Revision: 1,
144+
CollisionProtection: CollisionProtectionPrevent,
129145
},
130146
valid: true,
131147
},
@@ -192,6 +208,70 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
192208
},
193209
valid: false,
194210
},
211+
"spec collisionProtection accepts Prevent": {
212+
spec: ClusterExtensionRevisionSpec{
213+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
214+
Revision: 1,
215+
CollisionProtection: CollisionProtectionPrevent,
216+
},
217+
valid: true,
218+
},
219+
"spec collisionProtection accepts IfNoController": {
220+
spec: ClusterExtensionRevisionSpec{
221+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
222+
Revision: 1,
223+
CollisionProtection: CollisionProtectionIfNoController,
224+
},
225+
valid: true,
226+
},
227+
"spec collisionProtection accepts None": {
228+
spec: ClusterExtensionRevisionSpec{
229+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
230+
Revision: 1,
231+
CollisionProtection: CollisionProtectionNone,
232+
},
233+
valid: true,
234+
},
235+
"spec collisionProtection is required": {
236+
spec: ClusterExtensionRevisionSpec{
237+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
238+
Revision: 1,
239+
},
240+
valid: false,
241+
},
242+
"spec collisionProtection rejects invalid values": {
243+
spec: ClusterExtensionRevisionSpec{
244+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
245+
Revision: 1,
246+
CollisionProtection: CollisionProtection("Invalid"),
247+
},
248+
valid: false,
249+
},
250+
"spec collisionProtection must be set": {
251+
spec: ClusterExtensionRevisionSpec{
252+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
253+
Revision: 1,
254+
},
255+
valid: false,
256+
},
257+
"object collisionProtection is optional": {
258+
spec: ClusterExtensionRevisionSpec{
259+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
260+
Revision: 1,
261+
CollisionProtection: CollisionProtectionPrevent,
262+
Phases: []ClusterExtensionRevisionPhase{
263+
{
264+
Name: "deploy",
265+
Objects: []ClusterExtensionRevisionObject{
266+
{
267+
Object: configMap(),
268+
},
269+
},
270+
},
271+
},
272+
},
273+
valid: true,
274+
},
195275
} {
196276
t.Run(name, func(t *testing.T) {
197277
cer := &ClusterExtensionRevision{
@@ -211,3 +291,15 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
211291
})
212292
}
213293
}
294+
295+
func configMap() unstructured.Unstructured {
296+
return unstructured.Unstructured{
297+
Object: map[string]interface{}{
298+
"apiVersion": "v1",
299+
"kind": "ConfigMap",
300+
"metadata": map[string]interface{}{
301+
"name": "test-cm",
302+
},
303+
},
304+
}
305+
}

‎api/v1/validation_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ func TestValidate(t *testing.T) {
3636
}
3737
defaultRevisionSpec := func(s *ClusterExtensionRevisionSpec) *ClusterExtensionRevisionSpec {
3838
s.Revision = 1
39+
s.CollisionProtection = CollisionProtectionPrevent
3940
return s
4041
}
4142
c := newClient(t)

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,23 @@ spec:
5656
spec:
5757
description: spec defines the desired state of the ClusterExtensionRevision.
5858
properties:
59+
collisionProtection:
60+
description: |-
61+
collisionProtection specifies the default collision protection strategy for all objects
62+
in this revision. Individual phases or objects can override this value.
63+
64+
When set, this value is used as the default for any phase or object that does not
65+
explicitly specify its own collisionProtection.
66+
67+
The resolution order is: object > phase > spec > default ("Prevent").
68+
enum:
69+
- Prevent
70+
- IfNoController
71+
- None
72+
type: string
73+
x-kubernetes-validations:
74+
- message: collisionProtection is immutable
75+
rule: self == oldSelf
5976
lifecycleState:
6077
description: |-
6178
lifecycleState specifies the lifecycle state of the ClusterExtensionRevision.
@@ -102,6 +119,20 @@ spec:
102119
ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered
103120
complete only after all objects pass their status probes.
104121
properties:
122+
collisionProtection:
123+
description: |-
124+
collisionProtection specifies the default collision protection strategy for all objects
125+
in this phase. Individual objects can override this value.
126+
127+
When set, this value is used as the default for any object in this phase that does not
128+
explicitly specify its own collisionProtection.
129+
130+
The resolution order is: object > phase > spec > default ("Prevent").
131+
enum:
132+
- Prevent
133+
- IfNoController
134+
- None
135+
type: string
105136
name:
106137
description: |-
107138
name is a required identifier for this phase.
@@ -149,6 +180,8 @@ spec:
149180
owned by another controller.
150181
Use this setting with extreme caution as it may cause multiple controllers to fight over
151182
the same resource, resulting in increased load on the API server and etcd.
183+
184+
When omitted, the value is inherited from the phase, then spec, then defaults to "Prevent".
152185
enum:
153186
- Prevent
154187
- IfNoController
@@ -163,7 +196,6 @@ spec:
163196
x-kubernetes-embedded-resource: true
164197
x-kubernetes-preserve-unknown-fields: true
165198
required:
166-
- collisionProtection
167199
- object
168200
type: object
169201
maxItems: 50
@@ -205,6 +237,7 @@ spec:
205237
- message: revision is immutable
206238
rule: self == oldSelf
207239
required:
240+
- collisionProtection
208241
- lifecycleState
209242
- revision
210243
type: object

‎internal/operator-controller/applier/boxcutter.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,7 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
7575
sanitizedUnstructured(ctx, &obj)
7676

7777
objs = append(objs, ocv1.ClusterExtensionRevisionObject{
78-
Object: obj,
79-
CollisionProtection: ocv1.CollisionProtectionNone, // allow to adopt objects from previous release
78+
Object: obj,
8079
})
8180
}
8281

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

@@ -147,11 +147,12 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
147147
sanitizedUnstructured(ctx, &unstr)
148148

149149
objs = append(objs, ocv1.ClusterExtensionRevisionObject{
150-
Object: unstr,
151-
CollisionProtection: ocv1.CollisionProtectionPrevent,
150+
Object: unstr,
152151
})
153152
}
154-
return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil
153+
rev := r.buildClusterExtensionRevision(objs, ext, revisionAnnotations)
154+
rev.Spec.CollisionProtection = ocv1.CollisionProtectionPrevent
155+
return rev, nil
155156
}
156157

157158
// 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‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,9 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
114114
},
115115
},
116116
Spec: ocv1.ClusterExtensionRevisionSpec{
117-
LifecycleState: ocv1.ClusterExtensionRevisionLifecycleStateActive,
118-
Revision: 1,
117+
LifecycleState: ocv1.ClusterExtensionRevisionLifecycleStateActive,
118+
CollisionProtection: ocv1.CollisionProtectionNone,
119+
Revision: 1,
119120
Phases: []ocv1.ClusterExtensionRevisionPhase{
120121
{
121122
Name: "deploy",
@@ -132,7 +133,6 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
132133
},
133134
},
134135
},
135-
CollisionProtection: ocv1.CollisionProtectionNone,
136136
},
137137
{
138138
Object: unstructured.Unstructured{
@@ -146,7 +146,6 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
146146
},
147147
},
148148
},
149-
CollisionProtection: ocv1.CollisionProtectionNone,
150149
},
151150
},
152151
},
@@ -215,6 +214,8 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
215214
}, rev.Labels)
216215
t.Log("by checking the revision number is 0")
217216
require.Equal(t, int64(0), rev.Spec.Revision)
217+
t.Log("by checking the spec-level collisionProtection is set")
218+
require.Equal(t, ocv1.CollisionProtectionPrevent, rev.Spec.CollisionProtection)
218219
t.Log("by checking the rendered objects are present in the correct phases")
219220
require.Equal(t, []ocv1.ClusterExtensionRevisionPhase{
220221
{
@@ -231,7 +232,6 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
231232
"spec": map[string]interface{}{},
232233
},
233234
},
234-
CollisionProtection: ocv1.CollisionProtectionPrevent,
235235
},
236236
{
237237
Object: unstructured.Unstructured{
@@ -260,7 +260,6 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
260260
},
261261
},
262262
},
263-
CollisionProtection: ocv1.CollisionProtectionPrevent,
264263
},
265264
},
266265
},

0 commit comments

Comments
 (0)