From f4bbc238cba128337c7a9191dafff65b5d3c6d16 Mon Sep 17 00:00:00 2001 From: Luca Bernstein Date: Mon, 21 Oct 2024 16:31:18 +0200 Subject: [PATCH] Add `NamespacedCloudProfile` validation and mutation --- .../mutator/namespacedcloudprofile.go | 103 +++++++ .../mutator/namespacedcloudprofile_test.go | 144 ++++++++++ pkg/admission/mutator/webhook.go | 5 +- .../validator/namespacedcloudprofile.go | 203 ++++++++++++++ .../validator/namespacedcloudprofile_test.go | 263 ++++++++++++++++++ pkg/admission/validator/webhook.go | 11 +- pkg/apis/azure/validation/cloudprofile.go | 96 ++++--- 7 files changed, 774 insertions(+), 51 deletions(-) create mode 100644 pkg/admission/mutator/namespacedcloudprofile.go create mode 100644 pkg/admission/mutator/namespacedcloudprofile_test.go create mode 100644 pkg/admission/validator/namespacedcloudprofile.go create mode 100644 pkg/admission/validator/namespacedcloudprofile_test.go diff --git a/pkg/admission/mutator/namespacedcloudprofile.go b/pkg/admission/mutator/namespacedcloudprofile.go new file mode 100644 index 000000000..b936b0a52 --- /dev/null +++ b/pkg/admission/mutator/namespacedcloudprofile.go @@ -0,0 +1,103 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package mutator + +import ( + "context" + "encoding/json" + "fmt" + "maps" + "slices" + + extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook" + gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/gardener/gardener/pkg/utils" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/v1alpha1" +) + +// NewNamespacedCloudProfileMutator returns a new instance of a NamespacedCloudProfile mutator. +func NewNamespacedCloudProfileMutator(mgr manager.Manager) extensionswebhook.Mutator { + return &namespacedCloudProfile{ + client: mgr.GetClient(), + decoder: serializer.NewCodecFactory(mgr.GetScheme(), serializer.EnableStrict).UniversalDecoder(), + } +} + +type namespacedCloudProfile struct { + client client.Client + decoder runtime.Decoder +} + +// Mutate mutates the given NamespacedCloudProfile object. +func (p *namespacedCloudProfile) Mutate(_ context.Context, newObj, _ client.Object) error { + profile, ok := newObj.(*gardencorev1beta1.NamespacedCloudProfile) + if !ok { + return fmt.Errorf("wrong object type %T", newObj) + } + + // Ignore NamespacedCloudProfiles being deleted and wait for core mutator to patch the status. + if profile.DeletionTimestamp != nil || profile.Generation != profile.Status.ObservedGeneration || + profile.Spec.ProviderConfig == nil || profile.Status.CloudProfileSpec.ProviderConfig == nil { + return nil + } + + specConfig := &v1alpha1.CloudProfileConfig{} + if _, _, err := p.decoder.Decode(profile.Spec.ProviderConfig.Raw, nil, specConfig); err != nil { + return fmt.Errorf("could not decode providerConfig of namespacedCloudProfile spec for '%s': %w", profile.Name, err) + } + statusConfig := &v1alpha1.CloudProfileConfig{} + if _, _, err := p.decoder.Decode(profile.Status.CloudProfileSpec.ProviderConfig.Raw, nil, statusConfig); err != nil { + return fmt.Errorf("could not decode providerConfig of namespacedCloudProfile status for '%s': %w", profile.Name, err) + } + + statusConfig.MachineImages = mergeMachineImages(specConfig.MachineImages, statusConfig.MachineImages) + statusConfig.MachineTypes = mergeMachineTypes(specConfig.MachineTypes, statusConfig.MachineTypes) + + modifiedStatusConfig, err := json.Marshal(statusConfig) + if err != nil { + return err + } + profile.Status.CloudProfileSpec.ProviderConfig.Raw = modifiedStatusConfig + + return nil +} + +func mergeMachineImages(specMachineImages, statusMachineImages []v1alpha1.MachineImages) []v1alpha1.MachineImages { + specImages := utils.CreateMapFromSlice(specMachineImages, func(mi v1alpha1.MachineImages) string { return mi.Name }) + statusImages := utils.CreateMapFromSlice(statusMachineImages, func(mi v1alpha1.MachineImages) string { return mi.Name }) + for _, specMachineImage := range specImages { + if _, exists := statusImages[specMachineImage.Name]; !exists { + statusImages[specMachineImage.Name] = specMachineImage + } else { + statusImageVersions := utils.CreateMapFromSlice(statusImages[specMachineImage.Name].Versions, func(v v1alpha1.MachineImageVersion) string { return v.Version }) + specImageVersions := utils.CreateMapFromSlice(specImages[specMachineImage.Name].Versions, func(v v1alpha1.MachineImageVersion) string { return v.Version }) + for _, version := range specImageVersions { + statusImageVersions[version.Version] = version + } + + statusImages[specMachineImage.Name] = v1alpha1.MachineImages{ + Name: specMachineImage.Name, + Versions: slices.Collect(maps.Values(statusImageVersions)), + } + } + } + return slices.Collect(maps.Values(statusImages)) +} + +func mergeMachineTypes(specMachineTypes, statusMachineTypes []v1alpha1.MachineType) []v1alpha1.MachineType { + specImages := utils.CreateMapFromSlice(specMachineTypes, func(mi v1alpha1.MachineType) string { return mi.Name }) + statusImages := utils.CreateMapFromSlice(statusMachineTypes, func(mi v1alpha1.MachineType) string { return mi.Name }) + for _, specMachineImage := range specImages { + if _, exists := statusImages[specMachineImage.Name]; !exists { + statusImages[specMachineImage.Name] = specMachineImage + } + } + return slices.Collect(maps.Values(statusImages)) +} diff --git a/pkg/admission/mutator/namespacedcloudprofile_test.go b/pkg/admission/mutator/namespacedcloudprofile_test.go new file mode 100644 index 000000000..51bd7b254 --- /dev/null +++ b/pkg/admission/mutator/namespacedcloudprofile_test.go @@ -0,0 +1,144 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package mutator_test + +import ( + "context" + + "github.com/gardener/gardener/extensions/pkg/util" + extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook" + "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/gardener/gardener/pkg/utils/test" + . "github.com/gardener/gardener/pkg/utils/test/matchers" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/gardener/gardener-extension-provider-azure/pkg/admission/mutator" + api "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure" + "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/install" +) + +var _ = Describe("NamespacedCloudProfile Mutator", func() { + var ( + fakeClient client.Client + fakeManager manager.Manager + namespace string + ctx = context.Background() + decoder runtime.Decoder + + namespacedCloudProfileMutator extensionswebhook.Mutator + namespacedCloudProfile *v1beta1.NamespacedCloudProfile + ) + + BeforeEach(func() { + scheme := runtime.NewScheme() + utilruntime.Must(install.AddToScheme(scheme)) + utilruntime.Must(v1beta1.AddToScheme(scheme)) + fakeClient = fakeclient.NewClientBuilder().WithScheme(scheme).Build() + fakeManager = &test.FakeManager{ + Client: fakeClient, + Scheme: scheme, + } + namespace = "garden-dev" + decoder = serializer.NewCodecFactory(fakeManager.GetScheme(), serializer.EnableStrict).UniversalDecoder() + + namespacedCloudProfileMutator = mutator.NewNamespacedCloudProfileMutator(fakeManager) + namespacedCloudProfile = &v1beta1.NamespacedCloudProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "profile-1", + Namespace: namespace, + }, + } + }) + + Describe("#Mutate", func() { + It("should succeed for NamespacedCloudProfile without provider config", func() { + Expect(namespacedCloudProfileMutator.Mutate(ctx, namespacedCloudProfile, nil)).To(Succeed()) + }) + + It("should skip if NamespacedCloudProfile is in deletion phase", func() { + namespacedCloudProfile.DeletionTimestamp = ptr.To(metav1.Now()) + expectedProfile := namespacedCloudProfile.DeepCopy() + + Expect(namespacedCloudProfileMutator.Mutate(ctx, namespacedCloudProfile, nil)).To(Succeed()) + + Expect(namespacedCloudProfile).To(DeepEqual(expectedProfile)) + }) + + Describe("merge the provider configurations from a NamespacedCloudProfile and the parent CloudProfile", func() { + It("should correctly merge extended machineImages", func() { + namespacedCloudProfile.Status.CloudProfileSpec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineImages":[ + {"name":"image-1","versions":[{"version":"1.0","id":"local/image:1.0"}]} +]}`)} + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineImages":[ + {"name":"image-1","versions":[{"version":"1.1","id":"local/image:1.1"}]}, + {"name":"image-2","versions":[{"version":"2.0","id":"local/image:2.0"}]} +]}`)} + + Expect(namespacedCloudProfileMutator.Mutate(ctx, namespacedCloudProfile, nil)).To(Succeed()) + + mergedConfig, err := decodeCloudProfileConfig(decoder, namespacedCloudProfile.Status.CloudProfileSpec.ProviderConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(mergedConfig.MachineImages).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "Name": Equal("image-1"), + "Versions": ContainElements( + api.MachineImageVersion{Version: "1.0", ID: ptr.To("local/image:1.0"), Architecture: ptr.To("amd64")}, + api.MachineImageVersion{Version: "1.1", ID: ptr.To("local/image:1.1"), Architecture: ptr.To("amd64")}, + ), + }), + MatchFields(IgnoreExtras, Fields{ + "Name": Equal("image-2"), + "Versions": ContainElements(api.MachineImageVersion{Version: "2.0", ID: ptr.To("local/image:2.0"), Architecture: ptr.To("amd64")}), + }), + )) + }) + + It("should correctly merge added machineTypes", func() { + namespacedCloudProfile.Status.CloudProfileSpec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineTypes":[{"name":"type-1"}]}`)} + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineTypes":[{"name":"type-2","acceleratedNetworking":true}]}`)} + + Expect(namespacedCloudProfileMutator.Mutate(ctx, namespacedCloudProfile, nil)).To(Succeed()) + + mergedConfig, err := decodeCloudProfileConfig(decoder, namespacedCloudProfile.Status.CloudProfileSpec.ProviderConfig) + Expect(err).ToNot(HaveOccurred()) + var boolNil *bool + Expect(mergedConfig.MachineTypes).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{"Name": Equal("type-1"), "AcceleratedNetworking": BeEquivalentTo(boolNil)}), + MatchFields(IgnoreExtras, Fields{"Name": Equal("type-2"), "AcceleratedNetworking": BeEquivalentTo(ptr.To(true))}), + )) + }) + }) + }) +}) + +func decodeCloudProfileConfig(decoder runtime.Decoder, config *runtime.RawExtension) (*api.CloudProfileConfig, error) { + cloudProfileConfig := &api.CloudProfileConfig{} + if err := util.Decode(decoder, config.Raw, cloudProfileConfig); err != nil { + return nil, err + } + return cloudProfileConfig, nil +} diff --git a/pkg/admission/mutator/webhook.go b/pkg/admission/mutator/webhook.go index 53fcf688f..45c86c713 100644 --- a/pkg/admission/mutator/webhook.go +++ b/pkg/admission/mutator/webhook.go @@ -21,7 +21,7 @@ const ( var logger = log.Log.WithName("azure-mutator-webhook") -// New creates a new webhook that mutates Shoot resources. +// New creates a new webhook that mutates Shoot and NamespacedCloudProfile resources. func New(mgr manager.Manager) (*extensionswebhook.Webhook, error) { logger.Info("Setting up webhook", "name", Name) @@ -30,7 +30,8 @@ func New(mgr manager.Manager) (*extensionswebhook.Webhook, error) { Name: Name, Path: "/webhooks/mutate", Mutators: map[extensionswebhook.Mutator][]extensionswebhook.Type{ - NewShootMutator(mgr): {{Obj: &gardencorev1beta1.Shoot{}}}, + NewShootMutator(mgr): {{Obj: &gardencorev1beta1.Shoot{}}}, + NewNamespacedCloudProfileMutator(mgr): {{Obj: &gardencorev1beta1.NamespacedCloudProfile{}}}, }, Target: extensionswebhook.TargetSeed, ObjectSelector: &metav1.LabelSelector{ diff --git a/pkg/admission/validator/namespacedcloudprofile.go b/pkg/admission/validator/namespacedcloudprofile.go new file mode 100644 index 000000000..479f639fd --- /dev/null +++ b/pkg/admission/validator/namespacedcloudprofile.go @@ -0,0 +1,203 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validator + +import ( + "context" + "fmt" + + "github.com/gardener/gardener/extensions/pkg/util" + extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook" + "github.com/gardener/gardener/pkg/apis/core" + gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" + "github.com/gardener/gardener/pkg/utils" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + + api "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure" + "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/validation" +) + +// NewNamespacedCloudProfileValidator returns a new instance of a namespaced cloud profile validator. +func NewNamespacedCloudProfileValidator(mgr manager.Manager) extensionswebhook.Validator { + return &namespacedCloudProfile{ + client: mgr.GetClient(), + decoder: serializer.NewCodecFactory(mgr.GetScheme(), serializer.EnableStrict).UniversalDecoder(), + } +} + +type namespacedCloudProfile struct { + client client.Client + decoder runtime.Decoder +} + +// Validate validates the given NamespacedCloudProfile objects. +func (p *namespacedCloudProfile) Validate(ctx context.Context, newObj, _ client.Object) error { + profile, ok := newObj.(*core.NamespacedCloudProfile) + if !ok { + return fmt.Errorf("wrong object type %T", newObj) + } + + if profile.DeletionTimestamp != nil { + return nil + } + + cpConfig := &api.CloudProfileConfig{} + if profile.Spec.ProviderConfig != nil { + var err error + cpConfig, err = decodeCloudProfileConfig(p.decoder, profile.Spec.ProviderConfig) + if err != nil { + return err + } + } + + parentCloudProfile := profile.Spec.Parent + if parentCloudProfile.Kind != constants.CloudProfileReferenceKindCloudProfile { + return fmt.Errorf("parent reference must be of kind CloudProfile (unsupported kind: %s)", parentCloudProfile.Kind) + } + parentProfile := &gardencorev1beta1.CloudProfile{} + if err := p.client.Get(ctx, client.ObjectKey{Name: parentCloudProfile.Name}, parentProfile); err != nil { + return err + } + + return p.validateNamespacedCloudProfileProviderConfig(cpConfig, profile.Spec, parentProfile.Spec).ToAggregate() +} + +// validateNamespacedCloudProfileProviderConfig validates the CloudProfileConfig passed with a NamespacedCloudProfile. +func (p *namespacedCloudProfile) validateNamespacedCloudProfileProviderConfig(providerConfig *api.CloudProfileConfig, profileSpec core.NamespacedCloudProfileSpec, parentSpec gardencorev1beta1.CloudProfileSpec) field.ErrorList { + allErrs := field.ErrorList{} + + if providerConfig.CloudConfiguration != nil { + allErrs = append(allErrs, field.Forbidden( + field.NewPath("spec.providerConfig.cloudConfiguration"), + "cloud configuration is not allowed in a NamespacedCloudProfile providerConfig", + )) + } + if len(providerConfig.CountUpdateDomains) > 0 { + allErrs = append(allErrs, field.Forbidden( + field.NewPath("spec.providerConfig.countUpdateDomains"), + "count update domains is not allowed in a NamespacedCloudProfile providerConfig", + )) + } + if len(providerConfig.CountFaultDomains) > 0 { + allErrs = append(allErrs, field.Forbidden( + field.NewPath("spec.providerConfig.countFaultDomains"), + "count fault domains is not allowed in a NamespacedCloudProfile providerConfig", + )) + } + + allErrs = append(allErrs, p.validateMachineImages(providerConfig, profileSpec.MachineImages, parentSpec)...) + allErrs = append(allErrs, p.validateMachineTypes(providerConfig, profileSpec.MachineTypes, parentSpec)...) + + return allErrs +} + +func (p *namespacedCloudProfile) validateMachineImages(providerConfig *api.CloudProfileConfig, machineImages []core.MachineImage, parentSpec gardencorev1beta1.CloudProfileSpec) field.ErrorList { + allErrs := field.ErrorList{} + + machineImagesPath := field.NewPath("spec.providerConfig.machineImages") + for i, machineImage := range providerConfig.MachineImages { + idxPath := machineImagesPath.Index(i) + allErrs = append(allErrs, validation.ValidateMachineImage(idxPath, machineImage)...) + } + + profileImages := util.NewCoreImagesContext(machineImages) + parentImages := util.NewV1beta1ImagesContext(parentSpec.MachineImages) + providerImages := newProviderImagesContext(providerConfig.MachineImages) + + for _, machineImage := range profileImages.Images { + // Check that for each new image version defined in the NamespacedCloudProfile, the image is also defined in the providerConfig. + _, existsInParent := parentImages.GetImage(machineImage.Name) + if _, existsInProvider := providerImages.GetImage(machineImage.Name); !existsInParent && !existsInProvider { + allErrs = append(allErrs, field.Required( + field.NewPath("spec.providerConfig.machineImages"), + fmt.Sprintf("machine image %s is not defined in the NamespacedCloudProfile providerConfig", machineImage.Name), + )) + continue + } + for _, version := range machineImage.Versions { + _, existsInParent := parentImages.GetImageVersion(machineImage.Name, version.Version) + if _, exists := providerImages.GetImageVersion(machineImage.Name, version.Version); !existsInParent && !exists { + allErrs = append(allErrs, field.Required( + field.NewPath("spec.providerConfig.machineImages"), + fmt.Sprintf("machine image version %s@%s is not defined in the NamespacedCloudProfile providerConfig", machineImage.Name, version.Version), + )) + } + } + } + for imageIdx, machineImage := range providerConfig.MachineImages { + // Check that the machine image version is not already defined in the parent CloudProfile. + if _, exists := parentImages.GetImage(machineImage.Name); exists { + for versionIdx, version := range machineImage.Versions { + if _, exists := parentImages.GetImageVersion(machineImage.Name, version.Version); exists { + allErrs = append(allErrs, field.Forbidden( + field.NewPath("spec.providerConfig.machineImages").Index(imageIdx).Child("versions").Index(versionIdx), + fmt.Sprintf("machine image version %s@%s is already defined in the parent CloudProfile", machineImage.Name, version.Version), + )) + } + } + } + // Check that the machine image version is defined in the NamespacedCloudProfile. + if _, exists := profileImages.GetImage(machineImage.Name); !exists { + allErrs = append(allErrs, field.Required( + field.NewPath("spec.providerConfig.machineImages").Index(imageIdx), + fmt.Sprintf("machine image %s is not defined in the NamespacedCloudProfile .spec.machineImages", machineImage.Name), + )) + continue + } + for versionIdx, version := range machineImage.Versions { + if _, exists := profileImages.GetImageVersion(machineImage.Name, version.Version); !exists { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec.providerConfig.machineImages").Index(imageIdx).Child("versions").Index(versionIdx), + fmt.Sprintf("%s@%s", machineImage.Name, version.Version), + "machine image version is not defined in the NamespacedCloudProfile", + )) + } + } + } + + return allErrs +} + +func newProviderImagesContext(providerImages []api.MachineImages) *util.ImagesContext[api.MachineImages, api.MachineImageVersion] { + return util.NewImagesContext( + utils.CreateMapFromSlice(providerImages, func(mi api.MachineImages) string { return mi.Name }), + func(mi api.MachineImages) map[string]api.MachineImageVersion { + return utils.CreateMapFromSlice(mi.Versions, func(v api.MachineImageVersion) string { return v.Version }) + }, + ) +} + +func (p *namespacedCloudProfile) validateMachineTypes(providerConfig *api.CloudProfileConfig, machineTypes []core.MachineType, parentSpec gardencorev1beta1.CloudProfileSpec) field.ErrorList { + allErrs := field.ErrorList{} + + profileTypes := utils.CreateMapFromSlice(machineTypes, func(mi core.MachineType) string { return mi.Name }) + parentTypes := utils.CreateMapFromSlice(parentSpec.MachineTypes, func(mi gardencorev1beta1.MachineType) string { return mi.Name }) + + for typeIdx, machineType := range providerConfig.MachineTypes { + // Check that the machine type is not already defined in the parent CloudProfile. + if _, exists := parentTypes[machineType.Name]; exists { + allErrs = append(allErrs, field.Forbidden( + field.NewPath("spec.providerConfig.machineTypes").Index(typeIdx), + fmt.Sprintf("machine type %s is already defined in the parent CloudProfile", machineType.Name), + )) + } + // Check that the machine type is defined in the NamespacedCloudProfile. + _, exists := profileTypes[machineType.Name] + if !exists { + allErrs = append(allErrs, field.Required( + field.NewPath("spec.providerConfig.machineTypes").Index(typeIdx), + fmt.Sprintf("machine type %s is not defined in the NamespacedCloudProfile .spec.machineTypes", machineType.Name), + )) + continue + } + } + + return allErrs +} diff --git a/pkg/admission/validator/namespacedcloudprofile_test.go b/pkg/admission/validator/namespacedcloudprofile_test.go new file mode 100644 index 000000000..4a8fb3367 --- /dev/null +++ b/pkg/admission/validator/namespacedcloudprofile_test.go @@ -0,0 +1,263 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validator_test + +import ( + "context" + + extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook" + "github.com/gardener/gardener/pkg/apis/core" + "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/gardener/gardener/pkg/utils/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/gardener/gardener-extension-provider-azure/pkg/admission/validator" + "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/install" +) + +var _ = Describe("NamespacedCloudProfile Validator", func() { + var ( + fakeClient client.Client + fakeManager manager.Manager + namespace string + ctx = context.Background() + + namespacedCloudProfileValidator extensionswebhook.Validator + namespacedCloudProfile *core.NamespacedCloudProfile + cloudProfile *v1beta1.CloudProfile + ) + + BeforeEach(func() { + scheme := runtime.NewScheme() + utilruntime.Must(install.AddToScheme(scheme)) + utilruntime.Must(v1beta1.AddToScheme(scheme)) + fakeClient = fakeclient.NewClientBuilder().WithScheme(scheme).Build() + fakeManager = &test.FakeManager{ + Client: fakeClient, + Scheme: scheme, + } + namespace = "garden-dev" + + namespacedCloudProfileValidator = validator.NewNamespacedCloudProfileValidator(fakeManager) + namespacedCloudProfile = &core.NamespacedCloudProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "profile-1", + Namespace: namespace, + }, + Spec: core.NamespacedCloudProfileSpec{ + Parent: core.CloudProfileReference{ + Name: "cloud-profile", + Kind: "CloudProfile", + }, + }, + } + cloudProfile = &v1beta1.CloudProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-profile", + }, + } + }) + + Describe("#Validate", func() { + It("should succeed for NamespacedCloudProfile without provider config", func() { + Expect(fakeClient.Create(ctx, cloudProfile)).To(Succeed()) + Expect(namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil)).To(Succeed()) + }) + + It("should succeed if NamespacedCloudProfile is in deletion phase", func() { + namespacedCloudProfile.DeletionTimestamp = ptr.To(metav1.Now()) + + Expect(namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil)).To(Succeed()) + }) + + It("should succeed if the NamespacedCloudProfile correctly defines new machine images and types", func() { + cloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineImages":[{"name":"image-1","versions":[{"version":"1.0"}]}], +"machineTypes":[{"name":"type-1"}] +}`)} + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineImages":[ + {"name":"image-1","versions":[{"version":"1.1","id":"image-1-1-1"}]}, + {"name":"image-2","versions":[{"version":"2.0","id":"image-2-1-0"}]} +], +"machineTypes":[{"name":"type-2"}] +}`)} + namespacedCloudProfile.Spec.MachineImages = []core.MachineImage{ + { + Name: "image-1", + Versions: []core.MachineImageVersion{{ExpirableVersion: core.ExpirableVersion{Version: "1.1"}}}, + }, + { + Name: "image-2", + Versions: []core.MachineImageVersion{{ExpirableVersion: core.ExpirableVersion{Version: "2.0"}}}, + }, + } + namespacedCloudProfile.Spec.MachineTypes = []core.MachineType{ + {Name: "type-2"}, + } + Expect(fakeClient.Create(ctx, cloudProfile)).To(Succeed()) + + Expect(namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil)).To(Succeed()) + }) + + It("should fail for NamespacedCloudProfile with invalid parent kind", func() { + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig" +}`)} + namespacedCloudProfile.Spec.Parent = core.CloudProfileReference{ + Name: "cloud-profile", + Kind: "NamespacedCloudProfile", + } + + Expect(namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil)).To(MatchError(ContainSubstring("parent reference must be of kind CloudProfile"))) + }) + + It("should fail for NamespacedCloudProfile trying to override an already existing machine image version or type", func() { + cloudProfile.Spec.MachineImages = []v1beta1.MachineImage{ + {Name: "image-1", Versions: []v1beta1.MachineImageVersion{{ExpirableVersion: v1beta1.ExpirableVersion{Version: "1.0"}}}}, + } + cloudProfile.Spec.MachineTypes = []v1beta1.MachineType{{Name: "type-1"}} + + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineImages":[ + {"name":"image-1","versions":[{"version":"1.0","id":"image-1-1-0"}]} +], +"machineTypes":[{"name":"type-1"}] +}`)} + namespacedCloudProfile.Spec.MachineImages = []core.MachineImage{ + { + Name: "image-1", + Versions: []core.MachineImageVersion{ + {ExpirableVersion: core.ExpirableVersion{Version: "1.0"}}, + }, + }, + } + namespacedCloudProfile.Spec.MachineTypes = []core.MachineType{{Name: "type-1"}} + + Expect(fakeClient.Create(ctx, cloudProfile)).To(Succeed()) + + err := namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil) + Expect(err).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeForbidden), + "Field": Equal("spec.providerConfig.machineImages[0].versions[0]"), + "Detail": Equal("machine image version image-1@1.0 is already defined in the parent CloudProfile"), + })), PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeForbidden), + "Field": Equal("spec.providerConfig.machineTypes[0]"), + "Detail": Equal("machine type type-1 is already defined in the parent CloudProfile"), + })))) + }) + + It("should fail for NamespacedCloudProfile specifying provider config without the according version in the spec.machineImages", func() { + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineImages":[ + {"name":"image-1","versions":[{"version":"1.1","id":"image-1-1-1"}]} +] +}`)} + namespacedCloudProfile.Spec.MachineImages = []core.MachineImage{ + { + Name: "image-1", + Versions: []core.MachineImageVersion{ + {ExpirableVersion: core.ExpirableVersion{Version: "1.2"}}, + }, + }, + } + + Expect(fakeClient.Create(ctx, cloudProfile)).To(Succeed()) + + err := namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil) + Expect(err).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("spec.providerConfig.machineImages"), + "Detail": Equal("machine image version image-1@1.2 is not defined in the NamespacedCloudProfile providerConfig"), + })), PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("spec.providerConfig.machineImages[0].versions[0]"), + "BadValue": Equal("image-1@1.1"), + "Detail": Equal("machine image version is not defined in the NamespacedCloudProfile"), + })))) + }) + + It("should fail for NamespacedCloudProfile specifying provider config without the according version in the spec.machineTypes", func() { + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig", +"machineTypes":[{"name":"type-1"}] +}`)} + + Expect(fakeClient.Create(ctx, cloudProfile)).To(Succeed()) + + err := namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil) + Expect(err).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("spec.providerConfig.machineTypes[0]"), + "Detail": Equal("machine type type-1 is not defined in the NamespacedCloudProfile .spec.machineTypes"), + })))) + }) + + It("should fail for NamespacedCloudProfile specifying new spec.machineImages without the according version in the provider config", func() { + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig" +}`)} + namespacedCloudProfile.Spec.MachineImages = []core.MachineImage{ + { + Name: "image-3", + Versions: []core.MachineImageVersion{ + {ExpirableVersion: core.ExpirableVersion{Version: "3.0"}}, + }, + }, + } + + Expect(fakeClient.Create(ctx, cloudProfile)).To(Succeed()) + + err := namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil) + Expect(err).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("spec.providerConfig.machineImages"), + "Detail": Equal("machine image image-3 is not defined in the NamespacedCloudProfile providerConfig"), + })))) + }) + + It("should succeed for NamespacedCloudProfile specifying new spec.machineTypes without an according entry in the provider config", func() { + // By default, project administrators should be able to define new machine types in the NamespacedCloudProfile. + // This should not be dependent on them being able to edit the provider config. + + namespacedCloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: []byte(`{ +"apiVersion":"azure.provider.extensions.gardener.cloud/v1alpha1", +"kind":"CloudProfileConfig" +}`)} + namespacedCloudProfile.Spec.MachineTypes = []core.MachineType{ + { + Name: "type-2", + }, + } + + Expect(fakeClient.Create(ctx, cloudProfile)).To(Succeed()) + + err := namespacedCloudProfileValidator.Validate(ctx, namespacedCloudProfile, nil) + Expect(err).To(BeNil()) + }) + }) +}) diff --git a/pkg/admission/validator/webhook.go b/pkg/admission/validator/webhook.go index f6901dea6..4d5b14e81 100644 --- a/pkg/admission/validator/webhook.go +++ b/pkg/admission/validator/webhook.go @@ -25,7 +25,7 @@ const ( var logger = log.Log.WithName("azure-validator-webhook") -// New creates a new webhook that validates Shoot, CloudProfile, SecretBinding and CredentialsBinding resources. +// New creates a new webhook that validates Shoot, CloudProfile, NamespacedCloudProfile, SecretBinding and CredentialsBinding resources. func New(mgr manager.Manager) (*extensionswebhook.Webhook, error) { logger.Info("Setting up webhook", "name", Name) @@ -34,10 +34,11 @@ func New(mgr manager.Manager) (*extensionswebhook.Webhook, error) { Name: Name, Path: "/webhooks/validate", Validators: map[extensionswebhook.Validator][]extensionswebhook.Type{ - NewShootValidator(mgr): {{Obj: &core.Shoot{}}}, - NewCloudProfileValidator(mgr): {{Obj: &core.CloudProfile{}}}, - NewSecretBindingValidator(mgr): {{Obj: &core.SecretBinding{}}}, - NewCredentialsBindingValidator(mgr): {{Obj: &security.CredentialsBinding{}}}, + NewShootValidator(mgr): {{Obj: &core.Shoot{}}}, + NewCloudProfileValidator(mgr): {{Obj: &core.CloudProfile{}}}, + NewNamespacedCloudProfileValidator(mgr): {{Obj: &core.NamespacedCloudProfile{}}}, + NewSecretBindingValidator(mgr): {{Obj: &core.SecretBinding{}}}, + NewCredentialsBindingValidator(mgr): {{Obj: &security.CredentialsBinding{}}}, }, Target: extensionswebhook.TargetSeed, ObjectSelector: &metav1.LabelSelector{ diff --git a/pkg/apis/azure/validation/cloudprofile.go b/pkg/apis/azure/validation/cloudprofile.go index a729afc51..0da29fd7e 100644 --- a/pkg/apis/azure/validation/cloudprofile.go +++ b/pkg/apis/azure/validation/cloudprofile.go @@ -28,60 +28,68 @@ func ValidateCloudProfileConfig(cloudProfile *apisazure.CloudProfileConfig, fldP } for i, machineImage := range cloudProfile.MachineImages { idxPath := machineImagesPath.Index(i) + allErrs = append(allErrs, ValidateMachineImage(idxPath, machineImage)...) + } - if len(machineImage.Name) == 0 { - allErrs = append(allErrs, field.Required(idxPath.Child("name"), "must provide a name")) - } + return allErrs +} - if len(machineImage.Versions) == 0 { - allErrs = append(allErrs, field.Required(idxPath.Child("versions"), fmt.Sprintf("must provide at least one version for machine image %q", machineImage.Name))) - } - for j, version := range machineImage.Versions { - jdxPath := idxPath.Child("versions").Index(j) +// ValidateMachineImage validates a CloudProfileConfig MachineImages entry. +func ValidateMachineImage(validationPath *field.Path, machineImage apisazure.MachineImages) field.ErrorList { + allErrs := field.ErrorList{} - if len(version.Version) == 0 { - allErrs = append(allErrs, field.Required(jdxPath.Child("version"), "must provide a version")) - } + if len(machineImage.Name) == 0 { + allErrs = append(allErrs, field.Required(validationPath.Child("name"), "must provide a name")) + } + + if len(machineImage.Versions) == 0 { + allErrs = append(allErrs, field.Required(validationPath.Child("versions"), fmt.Sprintf("must provide at least one version for machine image %q", machineImage.Name))) + } + for j, version := range machineImage.Versions { + jdxPath := validationPath.Child("versions").Index(j) - allErrs = append(allErrs, validateProvidedImageIdCount(version, jdxPath)...) + if len(version.Version) == 0 { + allErrs = append(allErrs, field.Required(jdxPath.Child("version"), "must provide a version")) + } - if version.URN != nil { - if len(*version.URN) == 0 { - allErrs = append(allErrs, field.Required(jdxPath.Child("urn"), "urn cannot be empty when defined")) - } else if len(strings.Split(*version.URN, ":")) != 4 { - allErrs = append(allErrs, field.Invalid(jdxPath.Child("urn"), version.URN, "please use the format `Publisher:Offer:Sku:Version` for the urn")) - } - } - if version.ID != nil && len(*version.ID) == 0 { - allErrs = append(allErrs, field.Required(jdxPath.Child("id"), "id cannot be empty when defined")) + allErrs = append(allErrs, validateProvidedImageIdCount(version, jdxPath)...) + + if version.URN != nil { + if len(*version.URN) == 0 { + allErrs = append(allErrs, field.Required(jdxPath.Child("urn"), "urn cannot be empty when defined")) + } else if len(strings.Split(*version.URN, ":")) != 4 { + allErrs = append(allErrs, field.Invalid(jdxPath.Child("urn"), version.URN, "please use the format `Publisher:Offer:Sku:Version` for the urn")) } - if version.CommunityGalleryImageID != nil { - if len(*version.CommunityGalleryImageID) == 0 { - allErrs = append(allErrs, field.Required(jdxPath.Child("communityGalleryImageID"), "communityGalleryImageID cannot be empty when defined")) - } else if len(strings.Split(*version.CommunityGalleryImageID, "/")) != 7 { - allErrs = append(allErrs, field.Invalid(jdxPath.Child("communityGalleryImageID"), - version.CommunityGalleryImageID, "please use the format `/CommunityGalleries//Images//versions/` for the communityGalleryImageID")) - } else if !strings.EqualFold(strings.Split(*version.CommunityGalleryImageID, "/")[1], "CommunityGalleries") { - allErrs = append(allErrs, field.Invalid(jdxPath.Child("communityGalleryImageID"), - version.CommunityGalleryImageID, "communityGalleryImageID must start with '/CommunityGalleries/' prefix")) - } + } + if version.ID != nil && len(*version.ID) == 0 { + allErrs = append(allErrs, field.Required(jdxPath.Child("id"), "id cannot be empty when defined")) + } + if version.CommunityGalleryImageID != nil { + if len(*version.CommunityGalleryImageID) == 0 { + allErrs = append(allErrs, field.Required(jdxPath.Child("communityGalleryImageID"), "communityGalleryImageID cannot be empty when defined")) + } else if len(strings.Split(*version.CommunityGalleryImageID, "/")) != 7 { + allErrs = append(allErrs, field.Invalid(jdxPath.Child("communityGalleryImageID"), + version.CommunityGalleryImageID, "please use the format `/CommunityGalleries//Images//versions/` for the communityGalleryImageID")) + } else if !strings.EqualFold(strings.Split(*version.CommunityGalleryImageID, "/")[1], "CommunityGalleries") { + allErrs = append(allErrs, field.Invalid(jdxPath.Child("communityGalleryImageID"), + version.CommunityGalleryImageID, "communityGalleryImageID must start with '/CommunityGalleries/' prefix")) } + } - if version.SharedGalleryImageID != nil { - if len(*version.SharedGalleryImageID) == 0 { - allErrs = append(allErrs, field.Required(jdxPath.Child("sharedGalleryImageID"), "SharedGalleryImageID cannot be empty when defined")) - } else if len(strings.Split(*version.SharedGalleryImageID, "/")) != 7 { - allErrs = append(allErrs, field.Invalid(jdxPath.Child("sharedGalleryImageID"), - version.SharedGalleryImageID, "please use the format `/SharedGalleries//Images//Versions/` for the SharedGalleryImageID")) - } else if !strings.EqualFold(strings.Split(*version.SharedGalleryImageID, "/")[1], "SharedGalleries") { - allErrs = append(allErrs, field.Invalid(jdxPath.Child("sharedGalleryImageID"), - version.SharedGalleryImageID, "SharedGalleryImageID must start with '/SharedGalleries/' prefix")) - } + if version.SharedGalleryImageID != nil { + if len(*version.SharedGalleryImageID) == 0 { + allErrs = append(allErrs, field.Required(jdxPath.Child("sharedGalleryImageID"), "SharedGalleryImageID cannot be empty when defined")) + } else if len(strings.Split(*version.SharedGalleryImageID, "/")) != 7 { + allErrs = append(allErrs, field.Invalid(jdxPath.Child("sharedGalleryImageID"), + version.SharedGalleryImageID, "please use the format `/SharedGalleries//Images//Versions/` for the SharedGalleryImageID")) + } else if !strings.EqualFold(strings.Split(*version.SharedGalleryImageID, "/")[1], "SharedGalleries") { + allErrs = append(allErrs, field.Invalid(jdxPath.Child("sharedGalleryImageID"), + version.SharedGalleryImageID, "SharedGalleryImageID must start with '/SharedGalleries/' prefix")) } + } - if !slices.Contains(v1beta1constants.ValidArchitectures, *version.Architecture) { - allErrs = append(allErrs, field.NotSupported(jdxPath.Child("architecture"), *version.Architecture, v1beta1constants.ValidArchitectures)) - } + if !slices.Contains(v1beta1constants.ValidArchitectures, *version.Architecture) { + allErrs = append(allErrs, field.NotSupported(jdxPath.Child("architecture"), *version.Architecture, v1beta1constants.ValidArchitectures)) } }