diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index 81c95efce0f0..017f7f675791 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -18,7 +18,10 @@ package v1beta1 import ( corev1 "k8s.io/api/core/v1" + apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/util/validation/field" ) const ( @@ -301,3 +304,16 @@ type ObjectMeta struct { // +optional Annotations map[string]string `json:"annotations,omitempty"` } + +// Validate validates the labels and annotations in ObjectMeta. +func (metadata *ObjectMeta) Validate(parent *field.Path) field.ErrorList { + allErrs := metav1validation.ValidateLabels( + metadata.Labels, + parent.Child("labels"), + ) + allErrs = append(allErrs, apivalidation.ValidateAnnotations( + metadata.Annotations, + parent.Child("annotations"), + )...) + return allErrs +} diff --git a/api/v1beta1/machinedeployment_webhook.go b/api/v1beta1/machinedeployment_webhook.go index 472c01afd5f5..c749f8c50913 100644 --- a/api/v1beta1/machinedeployment_webhook.go +++ b/api/v1beta1/machinedeployment_webhook.go @@ -266,6 +266,9 @@ func (m *MachineDeployment) validate(old *MachineDeployment) error { } } + // Validate the metadata of the template. + allErrs = append(allErrs, m.Spec.Template.ObjectMeta.Validate(specPath.Child("template", "metadata"))...) + if len(allErrs) == 0 { return nil } diff --git a/api/v1beta1/machinedeployment_webhook_test.go b/api/v1beta1/machinedeployment_webhook_test.go index 90688a101aa0..9231f0e64884 100644 --- a/api/v1beta1/machinedeployment_webhook_test.go +++ b/api/v1beta1/machinedeployment_webhook_test.go @@ -18,6 +18,7 @@ package v1beta1 import ( "context" + "strings" "testing" . "github.com/onsi/gomega" @@ -569,3 +570,56 @@ func defaultValidateTestCustomDefaulter(object admission.Validator, customDefaul }) } } + +func TestMachineDeploymentTemplateMetadataValidation(t *testing.T) { + tests := []struct { + name string + labels map[string]string + annotations map[string]string + expectErr bool + }{ + { + name: "should return error for invalid labels and annotations", + labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + annotations: map[string]string{ + "/invalid-key": "foo", + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + md := &MachineDeployment{ + Spec: MachineDeploymentSpec{ + Template: MachineTemplateSpec{ + ObjectMeta: ObjectMeta{ + Labels: tt.labels, + Annotations: tt.annotations, + }, + }, + }, + } + if tt.expectErr { + warnings, err := md.ValidateCreate() + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = md.ValidateUpdate(md) + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } else { + warnings, err := md.ValidateCreate() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = md.ValidateUpdate(md) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } + }) + } +} diff --git a/api/v1beta1/machineset_webhook.go b/api/v1beta1/machineset_webhook.go index 560ef68af59c..d9ea53dc3b63 100644 --- a/api/v1beta1/machineset_webhook.go +++ b/api/v1beta1/machineset_webhook.go @@ -152,6 +152,9 @@ func (m *MachineSet) validate(old *MachineSet) error { } } + // Validate the metadata of the template. + allErrs = append(allErrs, m.Spec.Template.ObjectMeta.Validate(specPath.Child("template", "metadata"))...) + if len(allErrs) == 0 { return nil } diff --git a/api/v1beta1/machineset_webhook_test.go b/api/v1beta1/machineset_webhook_test.go index 65b86348df82..e5d6ef89a36a 100644 --- a/api/v1beta1/machineset_webhook_test.go +++ b/api/v1beta1/machineset_webhook_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + "strings" "testing" . "github.com/onsi/gomega" @@ -307,3 +308,56 @@ func TestValidateSkippedMachineSetPreflightChecks(t *testing.T) { }) } } + +func TestMachineSetTemplateMetadataValidation(t *testing.T) { + tests := []struct { + name string + labels map[string]string + annotations map[string]string + expectErr bool + }{ + { + name: "should return error for invalid labels and annotations", + labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + annotations: map[string]string{ + "/invalid-key": "foo", + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + ms := &MachineSet{ + Spec: MachineSetSpec{ + Template: MachineTemplateSpec{ + ObjectMeta: ObjectMeta{ + Labels: tt.labels, + Annotations: tt.annotations, + }, + }, + }, + } + if tt.expectErr { + warnings, err := ms.ValidateCreate() + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = ms.ValidateUpdate(ms) + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } else { + warnings, err := ms.ValidateCreate() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = ms.ValidateUpdate(ms) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } + }) + } +} diff --git a/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go b/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go index 430923b14f81..cfab2f38a86c 100644 --- a/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go +++ b/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go @@ -63,6 +63,8 @@ func (r *KubeadmConfigTemplateSpec) validate(name string) error { var allErrs field.ErrorList allErrs = append(allErrs, r.Template.Spec.Validate(field.NewPath("spec", "template", "spec"))...) + // Validate the metadata of the template. + allErrs = append(allErrs, r.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...) if len(allErrs) == 0 { return nil diff --git a/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook_test.go b/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook_test.go index 258cb8f248a5..d7d3187f8067 100644 --- a/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook_test.go +++ b/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook_test.go @@ -17,12 +17,14 @@ limitations under the License. package v1beta1_test import ( + "strings" "testing" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" utildefaulting "sigs.k8s.io/cluster-api/util/defaulting" ) @@ -46,7 +48,8 @@ func TestKubeadmConfigTemplateDefault(t *testing.T) { func TestKubeadmConfigTemplateValidation(t *testing.T) { cases := map[string]struct { - in *bootstrapv1.KubeadmConfigTemplate + in *bootstrapv1.KubeadmConfigTemplate + expectErr bool }{ "valid configuration": { in: &bootstrapv1.KubeadmConfigTemplate{ @@ -61,6 +64,21 @@ func TestKubeadmConfigTemplateValidation(t *testing.T) { }, }, }, + "should return error for invalid labels and annotations": { + in: &bootstrapv1.KubeadmConfigTemplate{Spec: bootstrapv1.KubeadmConfigTemplateSpec{ + Template: bootstrapv1.KubeadmConfigTemplateResource{ObjectMeta: clusterv1.ObjectMeta{ + Labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + Annotations: map[string]string{ + "/invalid-key": "foo", + }, + }}, + }}, + expectErr: true, + }, } for name, tt := range cases { @@ -69,10 +87,18 @@ func TestKubeadmConfigTemplateValidation(t *testing.T) { t.Run(name, func(t *testing.T) { g := NewWithT(t) warnings, err := tt.in.ValidateCreate() - g.Expect(err).ToNot(HaveOccurred()) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } g.Expect(warnings).To(BeEmpty()) warnings, err = tt.in.ValidateUpdate(nil) - g.Expect(err).ToNot(HaveOccurred()) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } g.Expect(warnings).To(BeEmpty()) }) } diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go index 1120db1ce891..9db8198d29ce 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go @@ -330,6 +330,9 @@ func validateKubeadmControlPlaneSpec(s KubeadmControlPlaneSpec, namespace string ) } + // Validate the metadata of the MachineTemplate + allErrs = append(allErrs, s.MachineTemplate.ObjectMeta.Validate(pathPrefix.Child("machineTemplate", "metadata"))...) + if !version.KubeSemver.MatchString(s.Version) { allErrs = append(allErrs, field.Invalid(pathPrefix.Child("version"), s.Version, "must be a valid semantic version")) } diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go index b593798ee45f..cd230eb618de 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + "strings" "testing" "time" @@ -152,6 +153,16 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { validIgnitionConfiguration.Spec.KubeadmConfigSpec.Format = bootstrapv1.Ignition validIgnitionConfiguration.Spec.KubeadmConfigSpec.Ignition = &bootstrapv1.IgnitionSpec{} + invalidMetadata := valid.DeepCopy() + invalidMetadata.Spec.MachineTemplate.ObjectMeta.Labels = map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + } + invalidMetadata.Spec.MachineTemplate.ObjectMeta.Annotations = map[string]string{ + "/invalid-key": "foo", + } + tests := []struct { name string enableIgnitionFeature bool @@ -236,6 +247,12 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { expectErr: false, kcp: validIgnitionConfiguration, }, + { + name: "should return error for invalid metadata", + enableIgnitionFeature: true, + expectErr: true, + kcp: invalidMetadata, + }, } for _, tt := range tests { @@ -671,6 +688,16 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { {"/var/lib/testdir", "/var/lib/etcd/data"}, } + invalidMetadata := before.DeepCopy() + invalidMetadata.Spec.MachineTemplate.ObjectMeta.Labels = map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + } + invalidMetadata.Spec.MachineTemplate.ObjectMeta.Annotations = map[string]string{ + "/invalid-key": "foo", + } + tests := []struct { name string enableIgnitionFeature bool @@ -1016,6 +1043,13 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { before: before, kcp: switchFromCloudInitToIgnition, }, + { + name: "should return error for invalid metadata", + enableIgnitionFeature: true, + expectErr: true, + before: before, + kcp: invalidMetadata, + }, } for _, tt := range tests { diff --git a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go index 6ff3c43e36c8..02fe7da10fb3 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go @@ -69,6 +69,8 @@ func (r *KubeadmControlPlaneTemplate) ValidateCreate() (admission.Warnings, erro allErrs := validateKubeadmControlPlaneTemplateResourceSpec(spec, field.NewPath("spec", "template", "spec")) allErrs = append(allErrs, validateClusterConfiguration(spec.KubeadmConfigSpec.ClusterConfiguration, nil, field.NewPath("spec", "template", "spec", "kubeadmConfigSpec", "clusterConfiguration"))...) allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "template", "spec", "kubeadmConfigSpec"))...) + // Validate the metadata of the KubeadmControlPlaneTemplateResource + allErrs = append(allErrs, r.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...) if len(allErrs) > 0 { return nil, apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlaneTemplate").GroupKind(), r.Name, allErrs) } @@ -108,5 +110,10 @@ func validateKubeadmControlPlaneTemplateResourceSpec(s KubeadmControlPlaneTempla allErrs = append(allErrs, validateRolloutBefore(s.RolloutBefore, pathPrefix.Child("rolloutBefore"))...) allErrs = append(allErrs, validateRolloutStrategy(s.RolloutStrategy, nil, pathPrefix.Child("rolloutStrategy"))...) + if s.MachineTemplate != nil { + // Validate the metadata of the MachineTemplate + allErrs = append(allErrs, s.MachineTemplate.ObjectMeta.Validate(pathPrefix.Child("machineTemplate", "metadata"))...) + } + return allErrs } diff --git a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go index 7702184a1a2a..d02e46360715 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + "strings" "testing" "time" @@ -24,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilfeature "k8s.io/component-base/featuregate/testing" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/feature" utildefaulting "sigs.k8s.io/cluster-api/util/defaulting" @@ -110,3 +112,42 @@ func TestKubeadmControlPlaneTemplateValidationFeatureGateDisabled(t *testing.T) g.Expect(warnings).To(BeEmpty()) }) } + +func TestKubeadmControlPlaneTemplateValidationMetadata(t *testing.T) { + t.Run("create kubeadmcontrolplanetemplate should not pass if metadata is invalid", func(t *testing.T) { + g := NewWithT(t) + kcpTemplate := &KubeadmControlPlaneTemplate{ + Spec: KubeadmControlPlaneTemplateSpec{ + Template: KubeadmControlPlaneTemplateResource{ + ObjectMeta: clusterv1.ObjectMeta{ + Labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + Annotations: map[string]string{ + "/invalid-key": "foo", + }, + }, + Spec: KubeadmControlPlaneTemplateResourceSpec{ + MachineTemplate: &KubeadmControlPlaneTemplateMachineTemplate{ + ObjectMeta: clusterv1.ObjectMeta{ + Labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + Annotations: map[string]string{ + "/invalid-key": "foo", + }, + }, + }, + }, + }, + }, + } + warnings, err := kcpTemplate.ValidateCreate() + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + }) +} diff --git a/exp/api/v1beta1/machinepool_webhook.go b/exp/api/v1beta1/machinepool_webhook.go index 58dc05ef7758..8dfd0d6b07e9 100644 --- a/exp/api/v1beta1/machinepool_webhook.go +++ b/exp/api/v1beta1/machinepool_webhook.go @@ -152,6 +152,9 @@ func (m *MachinePool) validate(old *MachinePool) error { } } + // Validate the metadata of the MachinePool template. + allErrs = append(allErrs, m.Spec.Template.ObjectMeta.Validate(specPath.Child("template", "metadata"))...) + if len(allErrs) == 0 { return nil } diff --git a/exp/api/v1beta1/machinepool_webhook_test.go b/exp/api/v1beta1/machinepool_webhook_test.go index de3e1f4a1a22..3b115bd2d1c1 100644 --- a/exp/api/v1beta1/machinepool_webhook_test.go +++ b/exp/api/v1beta1/machinepool_webhook_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + "strings" "testing" . "github.com/onsi/gomega" @@ -320,3 +321,56 @@ func TestMachinePoolVersionValidation(t *testing.T) { }) } } + +func TestMachinePoolMetadataValidation(t *testing.T) { + tests := []struct { + name string + labels map[string]string + annotations map[string]string + expectErr bool + }{ + { + name: "should return error for invalid labels and annotations", + labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + annotations: map[string]string{ + "/invalid-key": "foo", + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + mp := &MachinePool{ + Spec: MachinePoolSpec{ + Template: clusterv1.MachineTemplateSpec{ + ObjectMeta: clusterv1.ObjectMeta{ + Labels: tt.labels, + Annotations: tt.annotations, + }, + }, + }, + } + if tt.expectErr { + warnings, err := mp.ValidateCreate() + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = mp.ValidateUpdate(mp) + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } else { + warnings, err := mp.ValidateCreate() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = mp.ValidateUpdate(mp) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } + }) + } +} diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index 9bce93a80e7a..2e0da4f579c7 100644 --- a/internal/webhooks/cluster.go +++ b/internal/webhooks/cluster.go @@ -255,6 +255,9 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu ) } + // metadata in topology should be valid + allErrs = append(allErrs, validateTopologyMetadata(newCluster.Spec.Topology, fldPath)...) + // upgrade concurrency should be a numeric value. if concurrency, ok := newCluster.Annotations[clusterv1.ClusterTopologyUpgradeConcurrencyAnnotation]; ok { concurrencyAnnotationField := field.NewPath("metadata", "annotations", clusterv1.ClusterTopologyUpgradeConcurrencyAnnotation) @@ -629,3 +632,16 @@ func clusterClassIsReconciled(clusterClass *clusterv1.ClusterClass) error { } return nil } + +func validateTopologyMetadata(topology *clusterv1.Topology, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + allErrs = append(allErrs, topology.ControlPlane.Metadata.Validate(fldPath.Child("controlPlane", "metadata"))...) + if topology.Workers != nil { + for idx, md := range topology.Workers.MachineDeployments { + allErrs = append(allErrs, md.Metadata.Validate( + fldPath.Child("workers", "machineDeployments").Index(idx).Child("metadata"), + )...) + } + } + return allErrs +} diff --git a/internal/webhooks/clusterclass.go b/internal/webhooks/clusterclass.go index a90fa33fb595..d802691d99b3 100644 --- a/internal/webhooks/clusterclass.go +++ b/internal/webhooks/clusterclass.go @@ -156,6 +156,9 @@ func (webhook *ClusterClass) validate(ctx context.Context, oldClusterClass, newC // Validate patches. allErrs = append(allErrs, validatePatches(newClusterClass)...) + // Validate metadata + allErrs = append(allErrs, validateClusterClassMetadata(newClusterClass)...) + // If this is an update run additional validation. if oldClusterClass != nil { // Ensure spec changes are compatible. @@ -365,3 +368,17 @@ func validateMachineHealthCheckClass(fldPath *field.Path, namepace string, m *cl return mhc.ValidateCommonFields(fldPath) } + +func validateClusterClassMetadata(clusterClass *clusterv1.ClusterClass) field.ErrorList { + var allErrs field.ErrorList + allErrs = append(allErrs, clusterClass.Spec.ControlPlane.Metadata.Validate(field.NewPath("spec", "controlPlane", "metadata"))...) + workerPath := field.NewPath("spec", "workers", "machineDeployments") + for idx, m := range clusterClass.Spec.Workers.MachineDeployments { + allErrs = append(allErrs, validateMachineDeploymentMetadata(m, workerPath.Index(idx))...) + } + return allErrs +} + +func validateMachineDeploymentMetadata(m clusterv1.MachineDeploymentClass, fldPath *field.Path) field.ErrorList { + return m.Template.Metadata.Validate(fldPath.Child("template", "metadata")) +} diff --git a/internal/webhooks/clusterclass_test.go b/internal/webhooks/clusterclass_test.go index 444e9268a058..702d453f23ed 100644 --- a/internal/webhooks/clusterclass_test.go +++ b/internal/webhooks/clusterclass_test.go @@ -17,6 +17,7 @@ limitations under the License. package webhooks import ( + "strings" "testing" "time" @@ -1140,6 +1141,30 @@ func TestClusterClassValidation(t *testing.T) { Build(), expectErr: true, }, + { + name: "should return error for invalid labels and annotations", + in: builder.ClusterClass(metav1.NamespaceDefault, "class1"). + WithInfrastructureClusterTemplate( + builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "infra1").Build()). + WithControlPlaneTemplate( + builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1"). + Build()). + WithControlPlaneInfrastructureMachineTemplate( + builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "cpInfra1"). + Build()). + WithWorkerMachineDeploymentClasses( + *builder.MachineDeploymentClass("aa"). + WithInfrastructureTemplate( + builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").Build()). + WithBootstrapTemplate( + builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). + WithLabels(invalidLabels()). + WithAnnotations(invalidAnnotations()). + Build()). + WithControlPlaneMetadata(invalidLabels(), invalidAnnotations()). + Build(), + expectErr: true, + }, } for _, tt := range tests { @@ -1641,3 +1666,17 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { }) } } + +func invalidLabels() map[string]string { + return map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + } +} + +func invalidAnnotations() map[string]string { + return map[string]string{ + "/invalid-key": "foo", + } +} diff --git a/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook.go b/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook.go index 46451cb0221c..5492ec952a4a 100644 --- a/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook.go +++ b/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook.go @@ -63,6 +63,10 @@ func (r *DockerClusterTemplate) ValidateCreate() (admission.Warnings, error) { } allErrs := validateDockerClusterSpec(r.Spec.Template.Spec) + + // Validate the metadata of the template. + allErrs = append(allErrs, r.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...) + if len(allErrs) > 0 { return nil, apierrors.NewInvalid(GroupVersion.WithKind("DockerClusterTemplate").GroupKind(), r.Name, allErrs) } diff --git a/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook_test.go b/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook_test.go index 9284e4cb68f0..4244684d4a68 100644 --- a/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook_test.go +++ b/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook_test.go @@ -17,12 +17,14 @@ limitations under the License. package v1beta1 import ( + "strings" "testing" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilfeature "k8s.io/component-base/featuregate/testing" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/feature" ) @@ -68,3 +70,62 @@ func TestDockerClusterTemplateValidationFeatureGateDisabled(t *testing.T) { g.Expect(warnings).To(BeEmpty()) }) } + +func TestDockerClusterTemplateValidationMetadata(t *testing.T) { + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)() + + tests := []struct { + name string + labels map[string]string + annotations map[string]string + expectErr bool + }{ + { + name: "should return error for invalid labels and annotations", + labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + annotations: map[string]string{ + "/invalid-key": "foo", + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + dct := &DockerClusterTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dockerclustertemplate-test", + Namespace: "test-namespace", + }, + Spec: DockerClusterTemplateSpec{ + Template: DockerClusterTemplateResource{ + ObjectMeta: clusterv1.ObjectMeta{ + Labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + Annotations: map[string]string{ + "/invalid-key": "foo", + }, + }, + Spec: DockerClusterSpec{}, + }, + }, + } + warnings, err := dct.ValidateCreate() + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } + }) + } +} diff --git a/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook.go b/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook.go index d7df886f80db..52d1e1f50a53 100644 --- a/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook.go +++ b/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook.go @@ -49,7 +49,16 @@ type DockerMachineTemplateWebhook struct{} var _ webhook.CustomValidator = &DockerMachineTemplateWebhook{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (*DockerMachineTemplateWebhook) ValidateCreate(_ context.Context, _ runtime.Object) (admission.Warnings, error) { +func (*DockerMachineTemplateWebhook) ValidateCreate(_ context.Context, raw runtime.Object) (admission.Warnings, error) { + obj, ok := raw.(*DockerMachineTemplate) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a DockerMachineTemplate but got a %T", raw)) + } + // Validate the metadata of the template. + allErrs := obj.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata")) + if len(allErrs) > 0 { + return nil, apierrors.NewInvalid(GroupVersion.WithKind("DockerClusterTemplate").GroupKind(), obj.Name, allErrs) + } return nil, nil } @@ -74,6 +83,9 @@ func (*DockerMachineTemplateWebhook) ValidateUpdate(ctx context.Context, oldRaw !reflect.DeepEqual(newObj.Spec.Template.Spec, oldObj.Spec.Template.Spec) { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec"), newObj, dockerMachineTemplateImmutableMsg)) } + // Validate the metadata of the template. + allErrs = append(allErrs, newObj.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...) + if len(allErrs) == 0 { return nil, nil } diff --git a/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook_test.go b/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook_test.go index b70fbfbd6ec0..0d11343c9a4e 100644 --- a/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook_test.go +++ b/test/infrastructure/docker/api/v1beta1/dockermachinetemplate_webhook_test.go @@ -18,6 +18,7 @@ package v1beta1 import ( "context" + "strings" "testing" . "github.com/onsi/gomega" @@ -42,6 +43,18 @@ func TestDockerMachineTemplateInvalid(t *testing.T) { newTemplateSkipImmutabilityAnnotationSet := newTemplate.DeepCopy() newTemplateSkipImmutabilityAnnotationSet.SetAnnotations(map[string]string{clusterv1.TopologyDryRunAnnotation: ""}) + newTemplateWithInvalidMetadata := newTemplate.DeepCopy() + newTemplateWithInvalidMetadata.Spec.Template.ObjectMeta = clusterv1.ObjectMeta{ + Labels: map[string]string{ + "foo": "$invalid-key", + "bar": strings.Repeat("a", 64) + "too-long-value", + "/invalid-key": "foo", + }, + Annotations: map[string]string{ + "/invalid-key": "foo", + }, + } + tests := []struct { name string newTemplate *DockerMachineTemplate @@ -84,6 +97,13 @@ func TestDockerMachineTemplateInvalid(t *testing.T) { req: &admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{DryRun: pointer.Bool(true)}}, wantError: false, }, + { + name: "don't allow invalid metadata", + newTemplate: newTemplateWithInvalidMetadata, + oldTemplate: newTemplate, + req: &admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{DryRun: pointer.Bool(true)}}, + wantError: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {