From b8917f1acee2e6f92d0dbe84c3b6f7bbf7c9a7f2 Mon Sep 17 00:00:00 2001 From: Pablo Rodriguez Nava Date: Tue, 3 Mar 2026 11:50:04 +0100 Subject: [PATCH] MCO-2142: OSImageStream.spec.defaultStream validation Add a validation to the OSImageStream resource to ensure that the .spec.defaultStream fiel, when set, points to a value given in the .status.availableStreams. If the resourece has no status yet the validation should just pass and the MCO operator should take care of handling such spec. This only happens at cluster bootstrapping. Signed-off-by: Pablo Rodriguez Nava --- .../OSStreams.yaml | 117 ++++++++++++++++++ .../v1alpha1/types_osimagestream.go | 8 ++ ..._machine-config_01_osimagestreams.crd.yaml | 11 ++ .../OSStreams.yaml | 11 ++ .../zz_generated.swagger_doc_generated.go | 2 +- .../generated_openapi/zz_generated.openapi.go | 2 +- ..._machine-config_01_osimagestreams.crd.yaml | 11 ++ 7 files changed, 160 insertions(+), 2 deletions(-) diff --git a/machineconfiguration/v1alpha1/tests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml b/machineconfiguration/v1alpha1/tests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml index cc75fb5e52d..40768e36f33 100644 --- a/machineconfiguration/v1alpha1/tests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml +++ b/machineconfiguration/v1alpha1/tests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml @@ -305,6 +305,123 @@ tests: osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + - name: Should accept updating spec.defaultStream to a valid stream when status is populated + initial: | + apiVersion: machineconfiguration.openshift.io/v1alpha1 + kind: OSImageStream + metadata: + name: cluster + spec: + defaultStream: rhel-coreos + status: + defaultStream: rhel-coreos + availableStreams: + - name: rhel-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a1b2 + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:f9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b3a2f1e0d9c8b7a6f5e4d3c2b1a0f9e8 + - name: rhel10-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + updated: | + apiVersion: machineconfiguration.openshift.io/v1alpha1 + kind: OSImageStream + metadata: + name: cluster + spec: + defaultStream: rhel10-coreos + status: + defaultStream: rhel-coreos + availableStreams: + - name: rhel-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a1b2 + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:f9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b3a2f1e0d9c8b7a6f5e4d3c2b1a0f9e8 + - name: rhel10-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + expected: | + apiVersion: machineconfiguration.openshift.io/v1alpha1 + kind: OSImageStream + metadata: + name: cluster + spec: + defaultStream: rhel10-coreos + status: + defaultStream: rhel-coreos + availableStreams: + - name: rhel-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a1b2 + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:f9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b3a2f1e0d9c8b7a6f5e4d3c2b1a0f9e8 + - name: rhel10-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + + - name: Should reject updating spec.defaultStream to a stream not in status.availableStreams + initial: | + apiVersion: machineconfiguration.openshift.io/v1alpha1 + kind: OSImageStream + metadata: + name: cluster + spec: + defaultStream: rhel-coreos + status: + defaultStream: rhel-coreos + availableStreams: + - name: rhel-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a1b2 + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:f9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b3a2f1e0d9c8b7a6f5e4d3c2b1a0f9e8 + - name: rhel10-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + updated: | + apiVersion: machineconfiguration.openshift.io/v1alpha1 + kind: OSImageStream + metadata: + name: cluster + spec: + defaultStream: non-existent-stream + status: + defaultStream: rhel-coreos + availableStreams: + - name: rhel-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a1b2 + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:f9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b3a2f1e0d9c8b7a6f5e4d3c2b1a0f9e8 + - name: rhel10-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + expectedError: "spec.defaultStream must reference an existing stream name from status.availableStreams" + + - name: Should reject removing a stream from status.availableStreams that spec.defaultStream references + initial: | + apiVersion: machineconfiguration.openshift.io/v1alpha1 + kind: OSImageStream + metadata: + name: cluster + spec: + defaultStream: rhel-coreos + status: + defaultStream: rhel-coreos + availableStreams: + - name: rhel-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a1b2 + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:f9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b3a2f1e0d9c8b7a6f5e4d3c2b1a0f9e8 + - name: rhel10-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + updated: | + apiVersion: machineconfiguration.openshift.io/v1alpha1 + kind: OSImageStream + metadata: + name: cluster + spec: + defaultStream: rhel-coreos + status: + defaultStream: rhel10-coreos + availableStreams: + - name: rhel10-coreos + osImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b + osExtensionsImage: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e + expectedStatusError: "spec.defaultStream must reference an existing stream name from status.availableStreams" + - name: Should reject an status update without defaultStream initial: | apiVersion: machineconfiguration.openshift.io/v1alpha1 diff --git a/machineconfiguration/v1alpha1/types_osimagestream.go b/machineconfiguration/v1alpha1/types_osimagestream.go index 20cc963896c..f91a47f738b 100644 --- a/machineconfiguration/v1alpha1/types_osimagestream.go +++ b/machineconfiguration/v1alpha1/types_osimagestream.go @@ -23,6 +23,7 @@ import ( // +openshift:enable:FeatureGate=OSStreams // +kubebuilder:metadata:labels=openshift.io/operator-managed= // +kubebuilder:validation:XValidation:rule="self.metadata.name == 'cluster'",message="osimagestream is a singleton, .metadata.name must be 'cluster'" +// +kubebuilder:validation:XValidation:rule="!has(self.spec.defaultStream) || !has(self.status) || self.spec.defaultStream in self.status.availableStreams.map(s, s.name)",message="spec.defaultStream must reference an existing stream name from status.availableStreams" type OSImageStream struct { metav1.TypeMeta `json:",inline"` @@ -84,6 +85,13 @@ type OSImageStreamSpec struct { // status.availableStreams to apply as the default for MachineConfigPools // that do not specify a stream override. // + // When status.availableStreams has been populated by the operator, this field + // must reference the name of one of the streams in status.availableStreams. + // During initial creation, before the operator has populated status, any + // valid value is accepted. + // + // When omitted, the operator determines the default stream automatically. + // // It must be a valid RFC 1123 subdomain between 1 and 253 characters in length, // consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.'). // diff --git a/machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_osimagestreams.crd.yaml b/machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_osimagestreams.crd.yaml index 50a36f06d4d..39fdd152c5c 100644 --- a/machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_osimagestreams.crd.yaml +++ b/machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_osimagestreams.crd.yaml @@ -62,6 +62,13 @@ spec: status.availableStreams to apply as the default for MachineConfigPools that do not specify a stream override. + When status.availableStreams has been populated by the operator, this field + must reference the name of one of the streams in status.availableStreams. + During initial creation, before the operator has populated status, any + valid value is accepted. + + When omitted, the operator determines the default stream automatically. + It must be a valid RFC 1123 subdomain between 1 and 253 characters in length, consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.'). maxLength: 253 @@ -184,6 +191,10 @@ spec: x-kubernetes-validations: - message: osimagestream is a singleton, .metadata.name must be 'cluster' rule: self.metadata.name == 'cluster' + - message: spec.defaultStream must reference an existing stream name from + status.availableStreams + rule: '!has(self.spec.defaultStream) || !has(self.status) || self.spec.defaultStream + in self.status.availableStreams.map(s, s.name)' served: true storage: true subresources: diff --git a/machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml b/machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml index 67d07f7fab1..6c4efc08c99 100644 --- a/machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml +++ b/machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/osimagestreams.machineconfiguration.openshift.io/OSStreams.yaml @@ -62,6 +62,13 @@ spec: status.availableStreams to apply as the default for MachineConfigPools that do not specify a stream override. + When status.availableStreams has been populated by the operator, this field + must reference the name of one of the streams in status.availableStreams. + During initial creation, before the operator has populated status, any + valid value is accepted. + + When omitted, the operator determines the default stream automatically. + It must be a valid RFC 1123 subdomain between 1 and 253 characters in length, consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.'). maxLength: 253 @@ -184,6 +191,10 @@ spec: x-kubernetes-validations: - message: osimagestream is a singleton, .metadata.name must be 'cluster' rule: self.metadata.name == 'cluster' + - message: spec.defaultStream must reference an existing stream name from + status.availableStreams + rule: '!has(self.spec.defaultStream) || !has(self.status) || self.spec.defaultStream + in self.status.availableStreams.map(s, s.name)' served: true storage: true subresources: diff --git a/machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.go b/machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.go index 574d524ec92..e527758b4d1 100644 --- a/machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.go +++ b/machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.go @@ -186,7 +186,7 @@ func (OSImageStreamSet) SwaggerDoc() map[string]string { var map_OSImageStreamSpec = map[string]string{ "": "OSImageStreamSpec defines the desired state of a OSImageStream.", - "defaultStream": "defaultStream is the desired name of the stream that should be used as the default when no specific stream is requested by a MachineConfigPool.\n\nThis field is set by the installer during installation. Users may need to update it if the currently selected stream is no longer available, for example when the stream has reached its End of Life. The MachineConfigOperator uses this value to determine which stream from status.availableStreams to apply as the default for MachineConfigPools that do not specify a stream override.\n\nIt must be a valid RFC 1123 subdomain between 1 and 253 characters in length, consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.').", + "defaultStream": "defaultStream is the desired name of the stream that should be used as the default when no specific stream is requested by a MachineConfigPool.\n\nThis field is set by the installer during installation. Users may need to update it if the currently selected stream is no longer available, for example when the stream has reached its End of Life. The MachineConfigOperator uses this value to determine which stream from status.availableStreams to apply as the default for MachineConfigPools that do not specify a stream override.\n\nWhen status.availableStreams has been populated by the operator, this field must reference the name of one of the streams in status.availableStreams. During initial creation, before the operator has populated status, any valid value is accepted.\n\nWhen omitted, the operator determines the default stream automatically.\n\nIt must be a valid RFC 1123 subdomain between 1 and 253 characters in length, consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.').", } func (OSImageStreamSpec) SwaggerDoc() map[string]string { diff --git a/openapi/generated_openapi/zz_generated.openapi.go b/openapi/generated_openapi/zz_generated.openapi.go index c2746f05d0b..e89936d8b6c 100644 --- a/openapi/generated_openapi/zz_generated.openapi.go +++ b/openapi/generated_openapi/zz_generated.openapi.go @@ -45672,7 +45672,7 @@ func schema_openshift_api_machineconfiguration_v1alpha1_OSImageStreamSpec(ref co Properties: map[string]spec.Schema{ "defaultStream": { SchemaProps: spec.SchemaProps{ - Description: "defaultStream is the desired name of the stream that should be used as the default when no specific stream is requested by a MachineConfigPool.\n\nThis field is set by the installer during installation. Users may need to update it if the currently selected stream is no longer available, for example when the stream has reached its End of Life. The MachineConfigOperator uses this value to determine which stream from status.availableStreams to apply as the default for MachineConfigPools that do not specify a stream override.\n\nIt must be a valid RFC 1123 subdomain between 1 and 253 characters in length, consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.').", + Description: "defaultStream is the desired name of the stream that should be used as the default when no specific stream is requested by a MachineConfigPool.\n\nThis field is set by the installer during installation. Users may need to update it if the currently selected stream is no longer available, for example when the stream has reached its End of Life. The MachineConfigOperator uses this value to determine which stream from status.availableStreams to apply as the default for MachineConfigPools that do not specify a stream override.\n\nWhen status.availableStreams has been populated by the operator, this field must reference the name of one of the streams in status.availableStreams. During initial creation, before the operator has populated status, any valid value is accepted.\n\nWhen omitted, the operator determines the default stream automatically.\n\nIt must be a valid RFC 1123 subdomain between 1 and 253 characters in length, consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.').", Type: []string{"string"}, Format: "", }, diff --git a/payload-manifests/crds/0000_80_machine-config_01_osimagestreams.crd.yaml b/payload-manifests/crds/0000_80_machine-config_01_osimagestreams.crd.yaml index 50a36f06d4d..39fdd152c5c 100644 --- a/payload-manifests/crds/0000_80_machine-config_01_osimagestreams.crd.yaml +++ b/payload-manifests/crds/0000_80_machine-config_01_osimagestreams.crd.yaml @@ -62,6 +62,13 @@ spec: status.availableStreams to apply as the default for MachineConfigPools that do not specify a stream override. + When status.availableStreams has been populated by the operator, this field + must reference the name of one of the streams in status.availableStreams. + During initial creation, before the operator has populated status, any + valid value is accepted. + + When omitted, the operator determines the default stream automatically. + It must be a valid RFC 1123 subdomain between 1 and 253 characters in length, consisting of lowercase alphanumeric characters, hyphens ('-'), and periods ('.'). maxLength: 253 @@ -184,6 +191,10 @@ spec: x-kubernetes-validations: - message: osimagestream is a singleton, .metadata.name must be 'cluster' rule: self.metadata.name == 'cluster' + - message: spec.defaultStream must reference an existing stream name from + status.availableStreams + rule: '!has(self.spec.defaultStream) || !has(self.status) || self.spec.defaultStream + in self.status.availableStreams.map(s, s.name)' served: true storage: true subresources: