Skip to content

Commit af6733e

Browse files
pedjakclaude
pedjak
and
claude
authored
Add collisionProtection inheritance hierarchy to ClusterExtensionRevision (#2513)
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 1fd37fa commit af6733e

File tree

10 files changed

+317
-28
lines changed

10 files changed

+317
-28
lines changed

‎api/v1/clusterextensionrevision_types.go‎

Lines changed: 29 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
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,19 @@ 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+
// When omitted, we use .spec.collistionProtection as the default for any object in this phase that does not
168+
// explicitly specify its own collisionProtection.
169+
//
170+
// +optional
171+
// +kubebuilder:validation:Enum=Prevent;IfNoController;None
172+
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
147173
}
148174

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

‎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: 35 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
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,21 @@ 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+
When omitted, we use .spec.collistionProtection as the default for any object in this phase that does not
131+
explicitly specify its own collisionProtection.
132+
enum:
133+
- Prevent
134+
- IfNoController
135+
- None
136+
type: string
105137
name:
106138
description: |-
107139
name is a required identifier for this phase.
@@ -149,6 +181,8 @@ spec:
149181
owned by another controller.
150182
Use this setting with extreme caution as it may cause multiple controllers to fight over
151183
the same resource, resulting in increased load on the API server and etcd.
184+
185+
When omitted, the value is inherited from the phase, then spec.
152186
enum:
153187
- Prevent
154188
- IfNoController
@@ -163,7 +197,6 @@ spec:
163197
x-kubernetes-embedded-resource: true
164198
x-kubernetes-preserve-unknown-fields: true
165199
required:
166-
- collisionProtection
167200
- object
168201
type: object
169202
maxItems: 50
@@ -205,6 +238,7 @@ spec:
205238
- message: revision is immutable
206239
rule: self == oldSelf
207240
required:
241+
- collisionProtection
208242
- lifecycleState
209243
- revision
210244
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)