Merge pull request #8431 from LuBingtan/add-validation-metadata

✨  Add validation to nested ObjectMeta fields

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
+}

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
 	}

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())
+			}
+		})
+	}
+}

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
 	}

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())
+			}
+		})
+	}
+}

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

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())
 		})
 	}

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"))
 	}

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 {

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
 }