Skip to content

Commit 5d5164e

Browse files
pedjakclaude
pedjak
and
claude
committed
Add collisionProtection inheritance hierarchy to ClusterExtensionRevision
Add collisionProtection as an optional field at the spec and phase levels, 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 5d5164e

File tree

9 files changed

+289
-17
lines changed

9 files changed

+289
-17
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+
// +optional
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: 79 additions & 0 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) {
@@ -79,6 +80,16 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
7980
}
8081
},
8182
},
83+
"spec collisionProtection is immutable": {
84+
spec: ClusterExtensionRevisionSpec{
85+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
86+
Revision: 1,
87+
CollisionProtection: CollisionProtectionPrevent,
88+
},
89+
updateFunc: func(cer *ClusterExtensionRevision) {
90+
cer.Spec.CollisionProtection = CollisionProtectionNone
91+
},
92+
},
8293
} {
8394
t.Run(name, func(t *testing.T) {
8495
cer := &ClusterExtensionRevision{
@@ -192,6 +203,62 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
192203
},
193204
valid: false,
194205
},
206+
"spec collisionProtection accepts Prevent": {
207+
spec: ClusterExtensionRevisionSpec{
208+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
209+
Revision: 1,
210+
CollisionProtection: CollisionProtectionPrevent,
211+
},
212+
valid: true,
213+
},
214+
"spec collisionProtection accepts IfNoController": {
215+
spec: ClusterExtensionRevisionSpec{
216+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
217+
Revision: 1,
218+
CollisionProtection: CollisionProtectionIfNoController,
219+
},
220+
valid: true,
221+
},
222+
"spec collisionProtection accepts None": {
223+
spec: ClusterExtensionRevisionSpec{
224+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
225+
Revision: 1,
226+
CollisionProtection: CollisionProtectionNone,
227+
},
228+
valid: true,
229+
},
230+
"spec collisionProtection can be omitted": {
231+
spec: ClusterExtensionRevisionSpec{
232+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
233+
Revision: 1,
234+
},
235+
valid: true,
236+
},
237+
"spec collisionProtection rejects invalid values": {
238+
spec: ClusterExtensionRevisionSpec{
239+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
240+
Revision: 1,
241+
CollisionProtection: CollisionProtection("Invalid"),
242+
},
243+
valid: false,
244+
},
245+
"object collisionProtection is optional": {
246+
spec: ClusterExtensionRevisionSpec{
247+
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
248+
Revision: 1,
249+
Phases: []ClusterExtensionRevisionPhase{
250+
{
251+
Name: "deploy",
252+
Objects: []ClusterExtensionRevisionObject{
253+
{
254+
Object: newMinimalUnstructured(),
255+
},
256+
},
257+
},
258+
},
259+
},
260+
valid: true,
261+
},
195262
} {
196263
t.Run(name, func(t *testing.T) {
197264
cer := &ClusterExtensionRevision{
@@ -211,3 +278,15 @@ func TestClusterExtensionRevisionValidity(t *testing.T) {
211278
})
212279
}
213280
}
281+
282+
func newMinimalUnstructured() unstructured.Unstructured {
283+
return unstructured.Unstructured{
284+
Object: map[string]interface{}{
285+
"apiVersion": "v1",
286+
"kind": "ConfigMap",
287+
"metadata": map[string]interface{}{
288+
"name": "test-cm",
289+
},
290+
},
291+
}
292+
}

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

Lines changed: 33 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

‎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
},

‎internal/operator-controller/controllers/clusterextensionrevision_controller.go‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,10 +464,11 @@ func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Con
464464
objLabels[labels.OwnerNameKey] = rev.Labels[labels.OwnerNameKey]
465465
obj.SetLabels(objLabels)
466466

467-
switch specObj.CollisionProtection {
467+
cp := EffectiveCollisionProtection(rev.Spec.CollisionProtection, specPhase.CollisionProtection, specObj.CollisionProtection)
468+
switch cp {
468469
case ocv1.CollisionProtectionIfNoController, ocv1.CollisionProtectionNone:
469470
opts = append(opts, boxcutter.WithObjectReconcileOptions(
470-
obj, boxcutter.WithCollisionProtection(specObj.CollisionProtection)))
471+
obj, boxcutter.WithCollisionProtection(cp)))
471472
}
472473

473474
phase.Objects = append(phase.Objects, *obj)
@@ -477,6 +478,21 @@ func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Con
477478
return r, opts, nil
478479
}
479480

481+
// EffectiveCollisionProtection resolves the collision protection value using
482+
// the inheritance hierarchy: object > phase > spec > default ("Prevent").
483+
func EffectiveCollisionProtection(specCP, phaseCP, objectCP ocv1.CollisionProtection) ocv1.CollisionProtection {
484+
if objectCP != "" {
485+
return objectCP
486+
}
487+
if phaseCP != "" {
488+
return phaseCP
489+
}
490+
if specCP != "" {
491+
return specCP
492+
}
493+
return ocv1.CollisionProtectionPrevent
494+
}
495+
480496
var (
481497
deploymentProbe = &probing.GroupKindSelector{
482498
GroupKind: schema.GroupKind{Group: appsv1.GroupName, Kind: "Deployment"},

‎internal/operator-controller/controllers/clusterextensionrevision_controller_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,6 +1266,60 @@ func (m *mockTrackingCache) Free(ctx context.Context, user client.Object) error
12661266
return nil
12671267
}
12681268

1269+
func Test_effectiveCollisionProtection(t *testing.T) {
1270+
for _, tc := range []struct {
1271+
name string
1272+
specCP ocv1.CollisionProtection
1273+
phaseCP ocv1.CollisionProtection
1274+
objectCP ocv1.CollisionProtection
1275+
expected ocv1.CollisionProtection
1276+
}{
1277+
{
1278+
name: "all empty defaults to Prevent",
1279+
expected: ocv1.CollisionProtectionPrevent,
1280+
},
1281+
{
1282+
name: "spec only",
1283+
specCP: ocv1.CollisionProtectionNone,
1284+
expected: ocv1.CollisionProtectionNone,
1285+
},
1286+
{
1287+
name: "phase overrides spec",
1288+
specCP: ocv1.CollisionProtectionPrevent,
1289+
phaseCP: ocv1.CollisionProtectionIfNoController,
1290+
expected: ocv1.CollisionProtectionIfNoController,
1291+
},
1292+
{
1293+
name: "object overrides phase",
1294+
specCP: ocv1.CollisionProtectionPrevent,
1295+
phaseCP: ocv1.CollisionProtectionIfNoController,
1296+
objectCP: ocv1.CollisionProtectionNone,
1297+
expected: ocv1.CollisionProtectionNone,
1298+
},
1299+
{
1300+
name: "object overrides spec",
1301+
specCP: ocv1.CollisionProtectionNone,
1302+
objectCP: ocv1.CollisionProtectionPrevent,
1303+
expected: ocv1.CollisionProtectionPrevent,
1304+
},
1305+
{
1306+
name: "phase only",
1307+
phaseCP: ocv1.CollisionProtectionNone,
1308+
expected: ocv1.CollisionProtectionNone,
1309+
},
1310+
{
1311+
name: "object only",
1312+
objectCP: ocv1.CollisionProtectionIfNoController,
1313+
expected: ocv1.CollisionProtectionIfNoController,
1314+
},
1315+
} {
1316+
t.Run(tc.name, func(t *testing.T) {
1317+
result := controllers.EffectiveCollisionProtection(tc.specCP, tc.phaseCP, tc.objectCP)
1318+
require.Equal(t, tc.expected, result)
1319+
})
1320+
}
1321+
}
1322+
12691323
func Test_ClusterExtensionRevisionReconciler_getScopedClient_Errors(t *testing.T) {
12701324
testScheme := newScheme(t)
12711325

0 commit comments

Comments
 (0)