From 386a673fbac9c3c8cd1dde780157f00d7b7a27c3 Mon Sep 17 00:00:00 2001 From: Daniel Franz Date: Wed, 28 Oct 2026 13:37:47 +0900 Subject: [PATCH] Align ClusterObjectSet types with kubeapilinter conventions Adds omitempty and omitzero tags where appropriate to align our API with conventions set by kubernetes and openshift. Signed-off-by: Daniel Franz --- api/v1/clusterobjectdeployment_types.go | 2 +- api/v1/clusterobjectset_types.go | 38 +++++++++++-------- api/v1/clusterobjectset_types_test.go | 8 ++-- .../api/v1/clusterobjectset.go | 2 + hack/api-lint-diff/run.sh | 1 + ...framework.io_clusterobjectdeployments.yaml | 1 + ...peratorframework.io_clusterobjectsets.yaml | 9 ++++- manifests/experimental-e2e.yaml | 10 ++++- manifests/experimental.yaml | 10 ++++- 9 files changed, 54 insertions(+), 27 deletions(-) diff --git a/api/v1/clusterobjectdeployment_types.go b/api/v1/clusterobjectdeployment_types.go index 86d02882bf..fd87c52d4e 100644 --- a/api/v1/clusterobjectdeployment_types.go +++ b/api/v1/clusterobjectdeployment_types.go @@ -134,7 +134,7 @@ type ClusterObjectDeploymentTemplateSpec struct { // Objects and phases may override it. // +required // +kubebuilder:validation:Enum=Prevent;IfNoController;None - CollisionProtection CollisionProtection `json:"collisionProtection"` + CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"` } // ClusterObjectDeploymentStatus reports rollout progress. diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index c23a5328b5..d9d544f96c 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -87,7 +87,7 @@ type ClusterObjectSetSpec struct { // +kubebuilder:validation:Minimum:=1 // +kubebuilder:validation:Maximum:=9999999999 // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="revision is immutable" - Revision int64 `json:"revision"` + Revision int64 `json:"revision,omitempty"` // phases is an optional, immutable list of phases that group objects to be applied together. // @@ -112,7 +112,7 @@ type ClusterObjectSetSpec struct { // +listType=map // +listMapKey=name // +optional - Phases []ClusterObjectSetPhase `json:"phases,omitempty"` + Phases []ClusterObjectSetPhase `json:"phases,omitempty"` //nolint:kubeapilinter // progressDeadlineMinutes is an optional field that defines the maximum period // of time in minutes after which an installation should be considered failed and @@ -157,7 +157,7 @@ type ClusterObjectSetSpec struct { // +required // +kubebuilder:validation:Enum=Prevent;IfNoController;None // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="collisionProtection is immutable" - CollisionProtection CollisionProtection `json:"collisionProtection"` + CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"` } // ProgressionProbe provides a custom probe definition, consisting of an object selection method and assertions. @@ -388,12 +388,13 @@ type ClusterObjectSetPhase struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 // +kubebuilder:validation:XValidation:rule=`!format.dns1123Label().validate(self).hasValue()`,message="the value must consist of only lowercase alphanumeric characters and hyphens, and must start and end with an alphanumeric character." - Name string `json:"name"` + Name string `json:"name,omitempty"` // objects is a required list of all Kubernetes objects that belong to this phase. // // All objects in this list are applied to the cluster in no particular order. The maximum number of objects per phase is 50. // +required + // +listType=atomic // +kubebuilder:validation:MaxItems=50 Objects []ClusterObjectSetObject `json:"objects"` @@ -469,7 +470,7 @@ type ObjectSourceRef struct { // +required // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 - Name string `json:"name"` + Name string `json:"name,omitempty"` // namespace is the namespace of the referenced Secret. // When empty, defaults to the OLM system namespace during ref resolution. @@ -485,7 +486,7 @@ type ObjectSourceRef struct { // +required // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 - Key string `json:"key"` + Key string `json:"key,omitempty"` } // CollisionProtection specifies if and how ownership collisions are prevented. @@ -536,7 +537,9 @@ type ClusterObjectSetStatus struct { // +listType=map // +listMapKey=type // +optional - Conditions []metav1.Condition `json:"conditions,omitempty"` + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` //nolint:kubeapilinter // This CRD uses structural list-map validation, not protobuf serialization. // objectCounts reports aggregate object counts across all phases. // Values are sums of the per-phase counts in observedPhases. @@ -555,7 +558,7 @@ type ClusterObjectSetStatus struct { // +listType=map // +listMapKey=name // +optional - ObservedPhases []ObservedPhase `json:"observedPhases,omitempty"` + ObservedPhases []ObservedPhase `json:"observedPhases,omitempty"` //nolint:kubeapilinter // completedAt is the timestamp at which the revision was first observed to be // ready, meaning it had successfully rolled out and all of its objects passed @@ -575,13 +578,13 @@ type ObservedPhase struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 // +kubebuilder:validation:XValidation:rule=`!format.dns1123Label().validate(self).hasValue()`,message="the value must consist of only lowercase alphanumeric characters and hyphens, and must start and end with an alphanumeric character." - Name string `json:"name"` + Name string `json:"name,omitempty"` // status is the current state of this phase in the rollout. Must be one of // InvalidSpecification, PreconditionsNotMet, Pending, Reconciling, // Stabilizing, Ready, Unknown, Superseded, Archiving, or Archived. // +required - Status PhaseStatus `json:"status"` + Status PhaseStatus `json:"status,omitempty"` // completedAt is the timestamp when this phase first became Ready. // Set once and never modified. Nil means the phase has never been @@ -597,7 +600,7 @@ type ObservedPhase struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=1024 // +optional - Message string `json:"message,omitzero"` + Message string `json:"message,omitempty"` // digest is the digest of the phase's resolved object content // at first successful resolution, in the format ":". @@ -607,7 +610,7 @@ type ObservedPhase struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=256 // +kubebuilder:validation:XValidation:rule=`self.matches('^[a-z0-9]+:[a-f0-9]+$')`,message="digest must be in the format ':'" - Digest string `json:"digest"` + Digest string `json:"digest,omitempty"` // objectCounts reports the number of objects in this phase by state. // +optional @@ -678,14 +681,14 @@ type ObjectStatus struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 // +required - Version string `json:"version"` + Version string `json:"version,omitempty"` // kind is the kind of the object. Must be between 1 and 63 // characters. // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 // +required - Kind string `json:"kind"` + Kind string `json:"kind,omitempty"` // namespace is the namespace of the object. Empty for cluster-scoped // resources. At most 253 characters (DNS subdomain max). @@ -698,7 +701,7 @@ type ObjectStatus struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 // +required - Name string `json:"name"` + Name string `json:"name,omitempty"` // messages lists the failure reasons for this object. The maximum // of 18 entries accounts for up to 16 assertion probe failures, @@ -794,6 +797,7 @@ const ( // Set metadata.name explicitly to -, using decimal // notation without leading zeros. This ensures revision uniqueness within a group. // Name generation through metadata.generateName is not supported. +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.status) || has(self.status)",message="status cannot be removed once set" type ClusterObjectSet struct { metav1.TypeMeta `json:",inline"` @@ -807,8 +811,10 @@ type ClusterObjectSet struct { Spec ClusterObjectSetSpec `json:"spec,omitzero"` // status is optional and defines the observed state of the ClusterObjectSet. + // Cannot be removed once set; this is to prevent immutable fields such as completedAt + // from being cleared by an update which omits the status. // +optional - Status ClusterObjectSetStatus `json:"status,omitempty"` + Status ClusterObjectSetStatus `json:"status,omitzero"` } // +kubebuilder:object:root=true diff --git a/api/v1/clusterobjectset_types_test.go b/api/v1/clusterobjectset_types_test.go index 232bc1b58e..c1a8d7ecac 100644 --- a/api/v1/clusterobjectset_types_test.go +++ b/api/v1/clusterobjectset_types_test.go @@ -297,7 +297,7 @@ func TestClusterObjectSetValidity(t *testing.T) { }, }, valid: false, - expectedError: "spec.phases[0].name: Invalid value", + expectedError: "spec.phases[0].name: Required value", }, "phases entry names cannot start with symbols": { spec: ClusterObjectSetSpec{ @@ -400,7 +400,7 @@ func TestClusterObjectSetValidity(t *testing.T) { Revision: 1, }, valid: false, - expectedError: "spec.collisionProtection: Unsupported value", + expectedError: "spec.collisionProtection: Required value", }, "spec collisionProtection rejects invalid values": { spec: ClusterObjectSetSpec{ @@ -472,12 +472,12 @@ func TestClusterObjectSetValidity(t *testing.T) { "object ref requires a name": { spec: refSpec("validity-case-21", ObjectSourceRef{Key: "my-key"}), valid: false, - expectedError: "ref.name: Invalid value", + expectedError: "ref.name: Required value", }, "object ref requires a key": { spec: refSpec("validity-case-22", ObjectSourceRef{Name: "my-secret"}), valid: false, - expectedError: "ref.key: Invalid value", + expectedError: "ref.key: Required value", }, "object ref name cannot exceed 253 characters": { spec: refSpec("validity-case-23", ObjectSourceRef{Name: strings.Repeat("a", 254), Key: "my-key"}), diff --git a/applyconfigurations/api/v1/clusterobjectset.go b/applyconfigurations/api/v1/clusterobjectset.go index d77a178bde..7b5447e8e0 100644 --- a/applyconfigurations/api/v1/clusterobjectset.go +++ b/applyconfigurations/api/v1/clusterobjectset.go @@ -47,6 +47,8 @@ type ClusterObjectSetApplyConfiguration struct { // spec defines the desired state of the ClusterObjectSet. Spec *ClusterObjectSetSpecApplyConfiguration `json:"spec,omitempty"` // status is optional and defines the observed state of the ClusterObjectSet. + // Cannot be removed once set; this is to prevent immutable fields such as completedAt + // from being cleared by an update which omits the status. Status *ClusterObjectSetStatusApplyConfiguration `json:"status,omitempty"` } diff --git a/hack/api-lint-diff/run.sh b/hack/api-lint-diff/run.sh index 75b422cb95..e039f3e32d 100755 --- a/hack/api-lint-diff/run.sh +++ b/hack/api-lint-diff/run.sh @@ -90,6 +90,7 @@ linters: optionalfields: pointers: preference: WhenRequired + policy: Warn run: timeout: 5m 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 1879b3eb4f..6058620b65 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 @@ -245,6 +245,7 @@ spec: == 1 maxItems: 50 type: array + x-kubernetes-list-type: atomic required: - name - objects 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 8661270922..71a5ee1224 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 @@ -272,6 +272,7 @@ spec: rule: (has(self.object)?1:0)+(has(self.ref)?1:0) == 1 maxItems: 50 type: array + x-kubernetes-list-type: atomic required: - name - objects @@ -580,8 +581,10 @@ spec: - revision type: object status: - description: status is optional and defines the observed state of the - ClusterObjectSet. + description: |- + status is optional and defines the observed state of the ClusterObjectSet. + Cannot be removed once set; this is to prevent immutable fields such as completedAt + from being cleared by an update which omits the status. properties: completedAt: description: |- @@ -925,6 +928,8 @@ spec: x-kubernetes-validations: - message: metadata.name must be - rule: self.metadata.name == '%s-%d'.format([self.spec.group, self.spec.revision]) + - message: status cannot be removed once set + rule: '!has(oldSelf.status) || has(self.status)' selectableFields: - jsonPath: .spec.group served: true diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 941c81db1b..f99f147bb9 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1574,6 +1574,7 @@ spec: == 1 maxItems: 50 type: array + x-kubernetes-list-type: atomic required: - name - objects @@ -2230,6 +2231,7 @@ spec: rule: (has(self.object)?1:0)+(has(self.ref)?1:0) == 1 maxItems: 50 type: array + x-kubernetes-list-type: atomic required: - name - objects @@ -2538,8 +2540,10 @@ spec: - revision type: object status: - description: status is optional and defines the observed state of the - ClusterObjectSet. + description: |- + status is optional and defines the observed state of the ClusterObjectSet. + Cannot be removed once set; this is to prevent immutable fields such as completedAt + from being cleared by an update which omits the status. properties: completedAt: description: |- @@ -2883,6 +2887,8 @@ spec: x-kubernetes-validations: - message: metadata.name must be - rule: self.metadata.name == '%s-%d'.format([self.spec.group, self.spec.revision]) + - message: status cannot be removed once set + rule: '!has(oldSelf.status) || has(self.status)' selectableFields: - jsonPath: .spec.group served: true diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 1f56f63d51..0abcd6082d 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1535,6 +1535,7 @@ spec: == 1 maxItems: 50 type: array + x-kubernetes-list-type: atomic required: - name - objects @@ -2191,6 +2192,7 @@ spec: rule: (has(self.object)?1:0)+(has(self.ref)?1:0) == 1 maxItems: 50 type: array + x-kubernetes-list-type: atomic required: - name - objects @@ -2499,8 +2501,10 @@ spec: - revision type: object status: - description: status is optional and defines the observed state of the - ClusterObjectSet. + description: |- + status is optional and defines the observed state of the ClusterObjectSet. + Cannot be removed once set; this is to prevent immutable fields such as completedAt + from being cleared by an update which omits the status. properties: completedAt: description: |- @@ -2844,6 +2848,8 @@ spec: x-kubernetes-validations: - message: metadata.name must be - rule: self.metadata.name == '%s-%d'.format([self.spec.group, self.spec.revision]) + - message: status cannot be removed once set + rule: '!has(oldSelf.status) || has(self.status)' selectableFields: - jsonPath: .spec.group served: true