diff --git a/api/v1beta1/common_validate.go b/api/v1beta1/common_validate.go index 3e1fa770a59b..de776e4a23fb 100644 --- a/api/v1beta1/common_validate.go +++ b/api/v1beta1/common_validate.go @@ -1,5 +1,5 @@ /* -Copyright 2021 The Kubernetes Authors. +Copyright 2023 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,7 +17,7 @@ limitations under the License. package v1beta1 import ( - metavalidation "k8s.io/apimachinery/pkg/api/validation" + apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -28,7 +28,7 @@ func (metadata *ObjectMeta) Validate(parent *field.Path) field.ErrorList { metadata.Labels, parent.Child("labels"), ) - allErrs = append(allErrs, metavalidation.ValidateAnnotations( + allErrs = append(allErrs, apivalidation.ValidateAnnotations( metadata.Annotations, parent.Child("annotations"), )...) 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_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_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_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_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_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/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_test.go b/test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook_test.go index 9284e4cb68f0..65ebacd68420 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,59 @@ 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{}, + }, + }, + } + if tt.expectErr { + g.Expect(dct.ValidateCreate()).NotTo(Succeed()) + } else { + g.Expect(dct.ValidateCreate()).To(Succeed()) + } + }) + } +} 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) {