From bcdc61a8646f7a0e2ce6d62e26641a19d5f87d5e Mon Sep 17 00:00:00 2001 From: Urvashi Date: Thu, 10 Jul 2025 10:17:58 -0400 Subject: [PATCH] MachineOSConfig name should match MachineConfigPool Add a restriction where the name of the MachineOSConfig object must be the same as the MachineConfigPool it is created for. This will ensure that we only have one MOSC per MCP. Signed-off-by: Urvashi --- .../OnClusterBuild.yaml | 188 +++++++++++++++++- .../v1/types_machineosconfig.go | 1 + ...achine-config_01_machineosconfigs.crd.yaml | 6 + .../OnClusterBuild.yaml | 6 + ...achine-config_01_machineosconfigs.crd.yaml | 6 + 5 files changed, 199 insertions(+), 8 deletions(-) diff --git a/machineconfiguration/v1/tests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml b/machineconfiguration/v1/tests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml index d55a1d4dff6..5a174b88dd8 100644 --- a/machineconfiguration/v1/tests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml +++ b/machineconfiguration/v1/tests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml @@ -9,7 +9,7 @@ tests: apiVersion: machineconfiguration.openshift.io/v1 kind: MachineOSConfig metadata: - name: foobar + name: worker spec: machineConfigPool: name: worker @@ -24,7 +24,7 @@ tests: apiVersion: machineconfiguration.openshift.io/v1 kind: MachineOSConfig metadata: - name: foobar + name: worker spec: machineConfigPool: name: worker @@ -40,7 +40,7 @@ tests: apiVersion: machineconfiguration.openshift.io/v1 kind: MachineOSConfig metadata: - name: foobar + name: worker spec: machineConfigPool: name: worker @@ -55,7 +55,7 @@ tests: apiVersion: machineconfiguration.openshift.io/v1 kind: MachineOSConfig metadata: - name: foobar + name: worker spec: machineConfigPool: name: worker @@ -71,7 +71,7 @@ tests: apiVersion: machineconfiguration.openshift.io/v1 kind: MachineOSConfig metadata: - name: foobar + name: worker spec: machineConfigPool: name: worker @@ -88,7 +88,7 @@ tests: apiVersion: machineconfiguration.openshift.io/v1 kind: MachineOSConfig metadata: - name: foobar + name: worker spec: machineConfigPool: name: worker @@ -101,12 +101,184 @@ tests: apiVersion: machineconfiguration.openshift.io/v1 kind: MachineOSConfig metadata: - name: foobar + name: worker spec: machineConfigPool: name: worker - imageBuilder: + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + - name: Should succeed when MachineOSConfig name matches MachineConfigPool name + initial: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: worker + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + expected: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: worker + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + - name: Should fail when MachineOSConfig name does not match MachineConfigPool name + initial: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: different-name + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + expectedError: "MachineOSConfig name must match the referenced MachineConfigPool name; can only have one MachineOSConfig per MachineConfigPool" + onUpdate: + - name: Should allow changing other fields when a persisted value is no longer valid (mismatched names) + initialCRDPatches: + - op: remove + path: /spec/versions/0/schema/openAPIV3Schema/x-kubernetes-validations # Remove the name matching validation + initial: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: different-name + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + updated: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: different-name + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:v2.0 + containerFile: + - containerfileArch: AMD64 + content: | + FROM configs AS final + RUN rpm-ostree install tree && \ + ostree container commit + expected: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: different-name + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:v2.0 + containerFile: + - containerfileArch: AMD64 + content: | + FROM configs AS final + RUN rpm-ostree install tree && \ + ostree container commit + - name: Should allow updating a persisted value that is no longer valid to a valid value (fix mismatched names) + initialCRDPatches: + - op: remove + path: /spec/versions/0/schema/openAPIV3Schema/x-kubernetes-validations # Remove the name matching validation + initial: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: worker + spec: + machineConfigPool: + name: different-pool + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + updated: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: worker + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + expected: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: worker + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + - name: Should not allow updating a persisted value that is no longer valid to a still invalid value (different mismatched MCP names) + initialCRDPatches: + - op: remove + path: /spec/versions/0/schema/openAPIV3Schema/x-kubernetes-validations # Remove the name matching validation + initial: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: different-name + spec: + machineConfigPool: + name: worker + imageBuilder: + imageBuilderType: Job + renderedImagePushSecret: + name: foo + renderedImagePushSpec: quay.io/mco/renderedImg:latest + updated: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: MachineOSConfig + metadata: + name: different-name + spec: + machineConfigPool: + name: worker-different + imageBuilder: imageBuilderType: Job renderedImagePushSecret: name: foo renderedImagePushSpec: quay.io/mco/renderedImg:latest + expectedError: "MachineOSConfig name must match the referenced MachineConfigPool name; can only have one MachineOSConfig per MachineConfigPool" diff --git a/machineconfiguration/v1/types_machineosconfig.go b/machineconfiguration/v1/types_machineosconfig.go index 0bc5984b818..2c1d73a0185 100644 --- a/machineconfiguration/v1/types_machineosconfig.go +++ b/machineconfiguration/v1/types_machineosconfig.go @@ -18,6 +18,7 @@ import ( // MachineOSConfig describes the configuration for a build process managed by the MCO // Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer). // +openshift:compatibility-gen:level=1 +// +kubebuilder:validation:XValidation:rule="self.metadata.name == self.spec.machineConfigPool.name || oldSelf.hasValue() && oldSelf.spec.machineConfigPool.name.value() == self.spec.machineConfigPool.name",optionalOldSelf=true,message="MachineOSConfig name must match the referenced MachineConfigPool name; can only have one MachineOSConfig per MachineConfigPool" type MachineOSConfig struct { metav1.TypeMeta `json:",inline"` diff --git a/machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineosconfigs.crd.yaml b/machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineosconfigs.crd.yaml index 6e8dd52e54b..d1515713bdb 100644 --- a/machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineosconfigs.crd.yaml +++ b/machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineosconfigs.crd.yaml @@ -345,6 +345,12 @@ spec: required: - spec type: object + x-kubernetes-validations: + - message: MachineOSConfig name must match the referenced MachineConfigPool + name; can only have one MachineOSConfig per MachineConfigPool + optionalOldSelf: true + rule: self.metadata.name == self.spec.machineConfigPool.name || oldSelf.hasValue() + && oldSelf.spec.machineConfigPool.name.value() == self.spec.machineConfigPool.name served: true storage: true subresources: diff --git a/machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml b/machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml index 4bcf5b7e24e..3322d69280f 100644 --- a/machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml +++ b/machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineosconfigs.machineconfiguration.openshift.io/OnClusterBuild.yaml @@ -346,6 +346,12 @@ spec: required: - spec type: object + x-kubernetes-validations: + - message: MachineOSConfig name must match the referenced MachineConfigPool + name; can only have one MachineOSConfig per MachineConfigPool + optionalOldSelf: true + rule: self.metadata.name == self.spec.machineConfigPool.name || oldSelf.hasValue() + && oldSelf.spec.machineConfigPool.name.value() == self.spec.machineConfigPool.name served: true storage: true subresources: diff --git a/payload-manifests/crds/0000_80_machine-config_01_machineosconfigs.crd.yaml b/payload-manifests/crds/0000_80_machine-config_01_machineosconfigs.crd.yaml index 6e8dd52e54b..d1515713bdb 100644 --- a/payload-manifests/crds/0000_80_machine-config_01_machineosconfigs.crd.yaml +++ b/payload-manifests/crds/0000_80_machine-config_01_machineosconfigs.crd.yaml @@ -345,6 +345,12 @@ spec: required: - spec type: object + x-kubernetes-validations: + - message: MachineOSConfig name must match the referenced MachineConfigPool + name; can only have one MachineOSConfig per MachineConfigPool + optionalOldSelf: true + rule: self.metadata.name == self.spec.machineConfigPool.name || oldSelf.hasValue() + && oldSelf.spec.machineConfigPool.name.value() == self.spec.machineConfigPool.name served: true storage: true subresources: