From f1129eb781ae2b002ae8afa7b0dc4a392ce3537f Mon Sep 17 00:00:00 2001 From: Bingtan Lu Date: Wed, 5 Apr 2023 16:55:12 +0800 Subject: [PATCH] Add unit test for metadata validation --- api/v1beta1/common_validate.go | 6 +- api/v1beta1/machinedeployment_webhook_test.go | 46 +++++++++++++++ api/v1beta1/machineset_webhook_test.go | 46 +++++++++++++++ .../kubeadmconfigtemplate_webhook_test.go | 29 +++++++++- .../kubeadm_control_plane_webhook_test.go | 34 +++++++++++ ...ubeadmcontrolplanetemplate_webhook_test.go | 39 +++++++++++++ exp/api/v1beta1/machinepool_webhook_test.go | 46 +++++++++++++++ internal/webhooks/clusterclass_test.go | 39 +++++++++++++ .../dockerclustertemplate_webhook_test.go | 58 +++++++++++++++++++ .../dockermachinetemplate_webhook_test.go | 20 +++++++ 10 files changed, 357 insertions(+), 6 deletions(-) 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 23d3b9124c19..3ec055ff3580 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" @@ -545,3 +546,48 @@ 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 { + g.Expect(md.ValidateCreate()).NotTo(Succeed()) + g.Expect(md.ValidateUpdate(md)).NotTo(Succeed()) + } else { + g.Expect(md.ValidateCreate()).To(Succeed()) + g.Expect(md.ValidateUpdate(md)).To(Succeed()) + } + }) + } +} diff --git a/api/v1beta1/machineset_webhook_test.go b/api/v1beta1/machineset_webhook_test.go index 43fd79a5f31b..74795e585a0a 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" @@ -218,3 +219,48 @@ func TestMachineSetVersionValidation(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 { + g.Expect(ms.ValidateCreate()).NotTo(Succeed()) + g.Expect(ms.ValidateUpdate(ms)).NotTo(Succeed()) + } else { + g.Expect(ms.ValidateCreate()).To(Succeed()) + g.Expect(ms.ValidateUpdate(ms)).To(Succeed()) + } + }) + } +} diff --git a/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook_test.go b/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook_test.go index c35a4b438f32..d038b94811e2 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 { @@ -68,8 +86,13 @@ func TestKubeadmConfigTemplateValidation(t *testing.T) { t.Run(name, func(t *testing.T) { g := NewWithT(t) - g.Expect(tt.in.ValidateCreate()).To(Succeed()) - g.Expect(tt.in.ValidateUpdate(nil)).To(Succeed()) + if tt.expectErr { + g.Expect(tt.in.ValidateCreate()).NotTo(Succeed()) + g.Expect(tt.in.ValidateUpdate(nil)).NotTo(Succeed()) + } else { + g.Expect(tt.in.ValidateCreate()).To(Succeed()) + g.Expect(tt.in.ValidateUpdate(nil)).To(Succeed()) + } }) } } 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 6ae774051a0f..a3fc14ff3739 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 { @@ -669,6 +686,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 @@ -1014,6 +1041,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 64ebebe6a0e0..e92849405ae5 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" @@ -106,3 +108,40 @@ func TestKubeadmControlPlaneTemplateValidationFeatureGateDisabled(t *testing.T) g.Expect(kcpTemplate.ValidateCreate()).NotTo(Succeed()) }) } + +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", + }, + }, + }, + }, + }, + }, + } + g.Expect(kcpTemplate.ValidateCreate()).ShouldNot(Succeed()) + }) +} diff --git a/exp/api/v1beta1/machinepool_webhook_test.go b/exp/api/v1beta1/machinepool_webhook_test.go index 7fd1eca0ac98..6e76a681e29f 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" @@ -293,3 +294,48 @@ 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 { + g.Expect(mp.ValidateCreate()).NotTo(Succeed()) + g.Expect(mp.ValidateUpdate(mp)).NotTo(Succeed()) + } else { + g.Expect(mp.ValidateCreate()).To(Succeed()) + g.Expect(mp.ValidateUpdate(mp)).To(Succeed()) + } + }) + } +} 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 d32279166f4a..ed41d2cdefaa 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" ) @@ -64,3 +66,59 @@ func TestDockerClusterTemplateValidationFeatureGateDisabled(t *testing.T) { g.Expect(dct.ValidateCreate()).NotTo(Succeed()) }) } + +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 2d54f8c3a61b..276cbf21a189 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" admissionv1 "k8s.io/api/admission/v1" @@ -41,6 +42,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 @@ -83,6 +96,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) {