From f56d7276cf6b7d0cd3a5bace7478038b195691c9 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Tue, 20 Oct 2026 18:17:45 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=B1=20make=20spec.collisionProtection?= =?UTF-8?q?=20required=20in=20ClusterObjectSet=20and=20ClusterObjectDeploy?= =?UTF-8?q?ment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ClusterObjectSetSpec.CollisionProtection already carried +required but its json tag still had omitempty, letting Go serialise an empty string silently. Remove omitempty and clarify the doc comment to remove "When set" phrasing that implied optionality. - ClusterObjectDeploymentTemplateSpec.CollisionProtection was +optional. Promote it to +required, drop omitempty, and update its doc comment. - Regenerate all derived artifacts: CRD YAMLs, manifests, applyconfigurations, and API reference docs. collisionProtection now appears in the required list for spec.template.spec in the ClusterObjectDeployment CRD. - Update tests: newTestClusterObjectDeployment now supplies a default CollisionProtectionPrevent value; add empty-value rejection cases to the create and update validity tables; replace the DoesNotDefaultTemplate test (which relied on omitting the field) with CollisionProtectionIsRequired; remove the now-invalid update path that cleared the field to "". Co-Authored-By: Claude Sonnet 4.6 (1M context) Signed-off-by: Per G. da Silva --- api/v1/clusterobjectdeployment_types.go | 10 +++---- api/v1/clusterobjectdeployment_types_test.go | 30 +++++++++++-------- api/v1/clusterobjectset_types.go | 12 ++++---- .../v1/clusterobjectdeploymenttemplatespec.go | 6 ++-- .../api/v1/clusterobjectsetspec.go | 10 +++---- docs/api-reference/olmv1-api-reference.md | 2 +- ...framework.io_clusterobjectdeployments.yaml | 7 ++--- ...peratorframework.io_clusterobjectsets.yaml | 10 +++---- manifests/experimental-e2e.yaml | 17 +++++------ manifests/experimental.yaml | 17 +++++------ 10 files changed, 55 insertions(+), 66 deletions(-) diff --git a/api/v1/clusterobjectdeployment_types.go b/api/v1/clusterobjectdeployment_types.go index 7596b94fb7..a2a28dcd0b 100644 --- a/api/v1/clusterobjectdeployment_types.go +++ b/api/v1/clusterobjectdeployment_types.go @@ -68,13 +68,11 @@ type ClusterObjectDeploymentTemplateSpec struct { // +listType=atomic ProgressionProbes []ProgressionProbe `json:"progressionProbes,omitempty"` - // collisionProtection is the default strategy for objects in the revision. - // Objects and phases may override it. When omitted the controller uses Prevent - // in the generated ClusterObjectSet. Omission remains distinct from explicit - // Prevent for template identity; admission does not default this field. - // +optional + // collisionProtection is the required default collision protection strategy for objects in the revision. + // Objects and phases may override it. + // +required // +kubebuilder:validation:Enum=Prevent;IfNoController;None - CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"` + CollisionProtection CollisionProtection `json:"collisionProtection"` } // ClusterObjectDeploymentStatus reports rollout progress. diff --git a/api/v1/clusterobjectdeployment_types_test.go b/api/v1/clusterobjectdeployment_types_test.go index 138048d696..1340f1116f 100644 --- a/api/v1/clusterobjectdeployment_types_test.go +++ b/api/v1/clusterobjectdeployment_types_test.go @@ -34,6 +34,7 @@ func TestClusterObjectDeploymentValidity(t *testing.T) { {name: "collision protection IfNoController", mutate: codCollisionProtection(CollisionProtectionIfNoController), valid: true}, {name: "collision protection None", mutate: codCollisionProtection(CollisionProtectionNone), valid: true}, {name: "collision protection invalid", mutate: codCollisionProtection("Invalid")}, + {name: "collision protection empty", mutate: codCollisionProtection("")}, {name: "phases omitted", mutate: func(cod *ClusterObjectDeployment) { cod.Spec.Template.Spec.Phases = nil }}, {name: "phases empty", mutate: func(cod *ClusterObjectDeployment) { cod.Spec.Template.Spec.Phases = []ClusterObjectSetPhase{} }}, {name: "20 phases", mutate: codPhases(20), valid: true}, @@ -104,24 +105,28 @@ func TestClusterObjectDeploymentValidity(t *testing.T) { } } -func TestClusterObjectDeploymentDoesNotDefaultTemplate(t *testing.T) { +func TestClusterObjectDeploymentCollisionProtectionIsRequired(t *testing.T) { c := newClient(t) - cod := newTestClusterObjectDeployment("cod-no-defaults") + + // collisionProtection missing rejects the object. + cod := newTestClusterObjectDeployment("cod-cp-required") + cod.Spec.Template.Spec.CollisionProtection = "" + err := c.Create(t.Context(), cod) + require.True(t, apierrors.IsInvalid(err), "expected missing collisionProtection to be invalid, got: %v", err) + + // Valid values round-trip correctly; phase and object collisionProtection remain optional. + cod = newTestClusterObjectDeployment("cod-cp-roundtrip") require.NoError(t, c.Create(t.Context(), cod)) cod = getTestClusterObjectDeployment(t, c, cod.Name) - require.Empty(t, cod.Spec.Template.Spec.CollisionProtection) + require.Equal(t, CollisionProtectionPrevent, cod.Spec.Template.Spec.CollisionProtection) require.Empty(t, cod.Spec.Template.Spec.Phases[0].CollisionProtection) require.Empty(t, cod.Spec.Template.Spec.Phases[0].Objects[0].CollisionProtection) - // Omitted and explicitly supplied Prevent remain distinguishable templates. - cod.Spec.Template.Spec.CollisionProtection = CollisionProtectionPrevent - require.NoError(t, c.Update(t.Context(), cod)) - cod = getTestClusterObjectDeployment(t, c, cod.Name) - require.Equal(t, CollisionProtectionPrevent, cod.Spec.Template.Spec.CollisionProtection) - cod.Spec.Template.Spec.CollisionProtection = "" + // The field can be changed to another valid value. + cod.Spec.Template.Spec.CollisionProtection = CollisionProtectionNone require.NoError(t, c.Update(t.Context(), cod)) cod = getTestClusterObjectDeployment(t, c, cod.Name) - require.Empty(t, cod.Spec.Template.Spec.CollisionProtection) + require.Equal(t, CollisionProtectionNone, cod.Spec.Template.Spec.CollisionProtection) } func TestClusterObjectDeploymentTemplateIsMutable(t *testing.T) { @@ -154,7 +159,6 @@ func TestClusterObjectDeploymentTemplateIsMutable(t *testing.T) { // Optional fields can also be removed after being set. cod.Spec.Template.Spec.ProgressionProbes = nil - cod.Spec.Template.Spec.CollisionProtection = "" require.NoError(t, c.Update(t.Context(), cod)) } @@ -166,6 +170,7 @@ func TestClusterObjectDeploymentUpdateValidation(t *testing.T) { }{ {name: "no phases", mutate: func(cod *ClusterObjectDeployment) { cod.Spec.Template.Spec.Phases = nil }}, {name: "invalid collision protection", mutate: codCollisionProtection("Invalid")}, + {name: "empty collision protection", mutate: codCollisionProtection("")}, } { t.Run(tc.name, func(t *testing.T) { cod := newTestClusterObjectDeployment(fmt.Sprintf("cod-update-validity-%d", i)) @@ -306,7 +311,8 @@ func newTestClusterObjectDeployment(name string) *ClusterObjectDeployment { Spec: ClusterObjectDeploymentSpec{ Template: ClusterObjectDeploymentTemplate{ Spec: ClusterObjectDeploymentTemplateSpec{ - Phases: []ClusterObjectSetPhase{{Name: "deploy", Objects: []ClusterObjectSetObject{{Object: configMap()}}}}, + CollisionProtection: CollisionProtectionPrevent, + Phases: []ClusterObjectSetPhase{{Name: "deploy", Objects: []ClusterObjectSetObject{{Object: configMap()}}}}, }, }, }, diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index d869143c77..377aa81633 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -142,18 +142,16 @@ type ClusterObjectSetSpec struct { // ProgressionProbes []ProgressionProbe `json:"progressionProbes,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. + // collisionProtection is a required field that specifies the default collision protection + // strategy for all objects in this revision. Individual phases or objects can override this value. // - // The resolution order is: object > phase > spec + // 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"` + CollisionProtection CollisionProtection `json:"collisionProtection"` } // ProgressionProbe provides a custom probe definition, consisting of an object selection method and assertions. diff --git a/applyconfigurations/api/v1/clusterobjectdeploymenttemplatespec.go b/applyconfigurations/api/v1/clusterobjectdeploymenttemplatespec.go index 745f2b128e..8639fa7656 100644 --- a/applyconfigurations/api/v1/clusterobjectdeploymenttemplatespec.go +++ b/applyconfigurations/api/v1/clusterobjectdeploymenttemplatespec.go @@ -34,10 +34,8 @@ type ClusterObjectDeploymentTemplateSpecApplyConfiguration struct { // progressionProbes supplies custom readiness probes for objects in every phase. // The existing ClusterObjectSet selector and assertion semantics apply. ProgressionProbes []ProgressionProbeApplyConfiguration `json:"progressionProbes,omitempty"` - // collisionProtection is the default strategy for objects in the revision. - // Objects and phases may override it. When omitted the controller uses Prevent - // in the generated ClusterObjectSet. Omission remains distinct from explicit - // Prevent for template identity; admission does not default this field. + // collisionProtection is the required default collision protection strategy for objects in the revision. + // Objects and phases may override it. CollisionProtection *apiv1.CollisionProtection `json:"collisionProtection,omitempty"` } diff --git a/applyconfigurations/api/v1/clusterobjectsetspec.go b/applyconfigurations/api/v1/clusterobjectsetspec.go index 1698ab7208..5dfb42e09b 100644 --- a/applyconfigurations/api/v1/clusterobjectsetspec.go +++ b/applyconfigurations/api/v1/clusterobjectsetspec.go @@ -93,13 +93,11 @@ type ClusterObjectSetSpecApplyConfiguration struct { // // ProgressionProbes []ProgressionProbeApplyConfiguration `json:"progressionProbes,omitempty"` - // collisionProtection specifies the default collision protection strategy for all objects - // in this revision. Individual phases or objects can override this value. + // collisionProtection is a required field that 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 + // 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. CollisionProtection *apiv1.CollisionProtection `json:"collisionProtection,omitempty"` } diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 2a46ebfa25..8d2df41777 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -492,7 +492,7 @@ _Appears in:_ | --- | --- | --- | --- | | `phases` _[ClusterObjectSetPhase](#clusterobjectsetphase) array_ | phases is an ordered list of 1 to 20 uniquely named phases. Each phase contains
at most 50 objects using the existing inline object or immutable Secret reference
representation. Phase, object, and probe order participate in template identity. | | MaxItems: 20
MinItems: 1
Required: \{\}
| | `progressionProbes` _[ProgressionProbe](#progressionprobe) array_ | progressionProbes supplies custom readiness probes for objects in every phase.
The existing ClusterObjectSet selector and assertion semantics apply. | | MaxItems: 20
MinItems: 1
Optional: \{\}
| -| `collisionProtection` _[CollisionProtection](#collisionprotection)_ | collisionProtection is the default strategy for objects in the revision.
Objects and phases may override it. When omitted the controller uses Prevent
in the generated ClusterObjectSet. Omission remains distinct from explicit
Prevent for template identity; admission does not default this field. | | Enum: [Prevent IfNoController None]
Optional: \{\}
| +| `collisionProtection` _[CollisionProtection](#collisionprotection)_ | collisionProtection is the required default collision protection strategy for objects in the revision.
Objects and phases may override it. | | Enum: [Prevent IfNoController None]
Required: \{\}
| diff --git a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml index 8ecf889a6c..68f1e2ad42 100644 --- a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml +++ b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml @@ -63,10 +63,8 @@ spec: properties: collisionProtection: description: |- - collisionProtection is the default strategy for objects in the revision. - Objects and phases may override it. When omitted the controller uses Prevent - in the generated ClusterObjectSet. Omission remains distinct from explicit - Prevent for template identity; admission does not default this field. + collisionProtection is the required default collision protection strategy for objects in the revision. + Objects and phases may override it. enum: - Prevent - IfNoController @@ -469,6 +467,7 @@ spec: type: array x-kubernetes-list-type: atomic required: + - collisionProtection - phases type: object required: diff --git a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml index ec3177584a..2c8bd4ab83 100644 --- a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml +++ b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml @@ -76,13 +76,11 @@ spec: properties: collisionProtection: description: |- - collisionProtection specifies the default collision protection strategy for all objects - in this revision. Individual phases or objects can override this value. + collisionProtection is a required field that 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 + 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 diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 03b089db9e..d79111109d 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1474,10 +1474,8 @@ spec: properties: collisionProtection: description: |- - collisionProtection is the default strategy for objects in the revision. - Objects and phases may override it. When omitted the controller uses Prevent - in the generated ClusterObjectSet. Omission remains distinct from explicit - Prevent for template identity; admission does not default this field. + collisionProtection is the required default collision protection strategy for objects in the revision. + Objects and phases may override it. enum: - Prevent - IfNoController @@ -1880,6 +1878,7 @@ spec: type: array x-kubernetes-list-type: atomic required: + - collisionProtection - phases type: object required: @@ -2048,13 +2047,11 @@ spec: properties: collisionProtection: description: |- - collisionProtection specifies the default collision protection strategy for all objects - in this revision. Individual phases or objects can override this value. + collisionProtection is a required field that 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 + 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 diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 01c3ec9438..31f9c4232e 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1435,10 +1435,8 @@ spec: properties: collisionProtection: description: |- - collisionProtection is the default strategy for objects in the revision. - Objects and phases may override it. When omitted the controller uses Prevent - in the generated ClusterObjectSet. Omission remains distinct from explicit - Prevent for template identity; admission does not default this field. + collisionProtection is the required default collision protection strategy for objects in the revision. + Objects and phases may override it. enum: - Prevent - IfNoController @@ -1841,6 +1839,7 @@ spec: type: array x-kubernetes-list-type: atomic required: + - collisionProtection - phases type: object required: @@ -2009,13 +2008,11 @@ spec: properties: collisionProtection: description: |- - collisionProtection specifies the default collision protection strategy for all objects - in this revision. Individual phases or objects can override this value. + collisionProtection is a required field that 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 + 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