From 4718475fc638050f338cd0bbe7c450acabfaf444 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 20 Dec 2024 09:02:29 +0100 Subject: [PATCH] Make use of gardener utils --- .../validator/namespacedcloudprofile.go | 21 +----- pkg/apis/azure/validation/cloudprofile.go | 67 ++++++++++--------- .../azure/validation/cloudprofile_test.go | 4 +- 3 files changed, 41 insertions(+), 51 deletions(-) diff --git a/pkg/admission/validator/namespacedcloudprofile.go b/pkg/admission/validator/namespacedcloudprofile.go index ace739832..d0ac116a6 100644 --- a/pkg/admission/validator/namespacedcloudprofile.go +++ b/pkg/admission/validator/namespacedcloudprofile.go @@ -111,7 +111,7 @@ func (p *namespacedCloudProfile) validateMachineImages(providerConfig *api.Cloud profileImages := util.NewCoreImagesContext(machineImages) parentImages := util.NewV1beta1ImagesContext(parentSpec.MachineImages) - providerImages := newProviderImagesContext(providerConfig.MachineImages) + providerImages := validation.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. @@ -126,7 +126,7 @@ func (p *namespacedCloudProfile) validateMachineImages(providerConfig *api.Cloud for _, version := range machineImage.Versions { _, existsInParent := parentImages.GetImageVersion(machineImage.Name, version.Version) for _, expectedArchitecture := range version.Architectures { - if _, exists := providerImages.GetImageVersion(machineImage.Name, versionArchitectureKey(version.Version, expectedArchitecture)); !existsInParent && !exists { + if _, exists := providerImages.GetImageVersion(machineImage.Name, validation.VersionArchitectureKey(version.Version, expectedArchitecture)); !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), @@ -178,23 +178,6 @@ func (p *namespacedCloudProfile) validateMachineImages(providerConfig *api.Cloud return allErrs } -func providerMachineImageKey(v api.MachineImageVersion) string { - return versionArchitectureKey(v.Version, ptr.Deref(v.Architecture, constants.ArchitectureAMD64)) -} - -func versionArchitectureKey(version, architecture string) string { - return version + "-" + architecture -} - -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 providerMachineImageKey(v) }) - }, - ) -} - func (p *namespacedCloudProfile) validateMachineTypes(providerConfig *api.CloudProfileConfig, machineTypes []core.MachineType, parentSpec gardencorev1beta1.CloudProfileSpec) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/azure/validation/cloudprofile.go b/pkg/apis/azure/validation/cloudprofile.go index fb42e986a..f2e6d2736 100644 --- a/pkg/apis/azure/validation/cloudprofile.go +++ b/pkg/apis/azure/validation/cloudprofile.go @@ -8,6 +8,7 @@ import ( "fmt" "strings" + "github.com/gardener/gardener/extensions/pkg/util" "github.com/gardener/gardener/pkg/apis/core" v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" "github.com/gardener/gardener/pkg/utils" @@ -37,7 +38,7 @@ func ValidateCloudProfileConfig(cpConfig *apisazure.CloudProfileConfig, machineI allErrs = append(allErrs, ValidateProviderMachineImage(idxPath, machineImage)...) } - allErrs = append(allErrs, validateProviderImagesMapping(cpConfig, machineImages, machineImagesPath)...) + allErrs = append(allErrs, validateProviderImagesMapping(cpConfig.MachineImages, machineImages, machineImagesPath)...) return allErrs } @@ -105,40 +106,27 @@ func ValidateProviderMachineImage(validationPath *field.Path, machineImage apisa } // verify that for each cp image a provider image exists -func validateProviderImagesMapping(cpConfig *apisazure.CloudProfileConfig, machineImages []core.MachineImage, fldPath *field.Path) field.ErrorList { +func validateProviderImagesMapping(cpConfigImages []apisazure.MachineImages, machineImages []core.MachineImage, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - providerImageMap := utils.CreateMapFromSlice(cpConfig.MachineImages, func(mi apisazure.MachineImages) string { return mi.Name }) - for idx, image := range machineImages { - providerImage, found := providerImageMap[image.Name] - if found { - allErrs = append(allErrs, validateVersionsMapping(providerImage.Versions, image.Versions, fldPath.Index(idx).Child("versions"))...) - } else if len(image.Versions) > 0 { - allErrs = append(allErrs, field.Required(fldPath, fmt.Sprintf("must provide a provider image mapping for image %q", image.Name))) - } - } - - return allErrs -} - -// validate that for each image version and architecture a corresponding provider image exists -func validateVersionsMapping(providerVersions []apisazure.MachineImageVersion, versions []core.MachineImageVersion, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} + images := util.NewCoreImagesContext(machineImages) + providerImages := NewProviderImagesContext(cpConfigImages) - isVersionArchAvailable := func(version string, architecture string) bool { - for _, providerVersion := range providerVersions { - if architecture == ptr.Deref(providerVersion.Architecture, v1beta1constants.ArchitectureAMD64) && version == providerVersion.Version { - return true - } + // for each image in the CloudProfile, check if it exists in the CloudProfileConfig + for _, machineImage := range images.Images { + if _, existsInParent := providerImages.GetImage(machineImage.Name); !existsInParent { + allErrs = append(allErrs, field.Required(fldPath, fmt.Sprintf("must provide a provider image mapping for image %q", machineImage.Name))) + continue } - return false - } - for _, version := range versions { - for _, architecture := range version.Architectures { - if !isVersionArchAvailable(version.Version, architecture) { - errorMessage := fmt.Sprintf("must provide an image mapping for version %q and architecture: %s", version.Version, architecture) - allErrs = append(allErrs, field.Required(fldPath, errorMessage)) + // validate that for each version and architecture of an image in the cloud profile a + // corresponding provider specific image in the cloud profile config exists + for _, version := range machineImage.Versions { + for _, expectedArchitecture := range version.Architectures { + if _, exists := providerImages.GetImageVersion(machineImage.Name, VersionArchitectureKey(version.Version, expectedArchitecture)); !exists { + allErrs = append(allErrs, field.Required(fldPath.Child("versions"), + fmt.Sprintf("must provide an image mapping for version %q and architecture: %s", version.Version, expectedArchitecture))) + } } } } @@ -196,3 +184,22 @@ func validateDomainCount(domainCount []apisazure.DomainCount, fldPath *field.Pat return allErrs } + +func providerMachineImageKey(v apisazure.MachineImageVersion) string { + return VersionArchitectureKey(v.Version, ptr.Deref(v.Architecture, v1beta1constants.ArchitectureAMD64)) +} + +// VersionArchitectureKey returns a key for a version and architecture. +func VersionArchitectureKey(version, architecture string) string { + return version + "-" + architecture +} + +// NewProviderImagesContext creates a new images context for provider images. +func NewProviderImagesContext(providerImages []apisazure.MachineImages) *util.ImagesContext[apisazure.MachineImages, apisazure.MachineImageVersion] { + return util.NewImagesContext( + utils.CreateMapFromSlice(providerImages, func(mi apisazure.MachineImages) string { return mi.Name }), + func(mi apisazure.MachineImages) map[string]apisazure.MachineImageVersion { + return utils.CreateMapFromSlice(mi.Versions, func(v apisazure.MachineImageVersion) string { return providerMachineImageKey(v) }) + }, + ) +} diff --git a/pkg/apis/azure/validation/cloudprofile_test.go b/pkg/apis/azure/validation/cloudprofile_test.go index 8cec7e8c1..66d407da6 100644 --- a/pkg/apis/azure/validation/cloudprofile_test.go +++ b/pkg/apis/azure/validation/cloudprofile_test.go @@ -115,7 +115,7 @@ var _ = Describe("CloudProfileConfig validation", func() { errorList := ValidateCloudProfileConfig(cloudProfileConfig, cloudProfileMachineImages, root) Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeRequired), - "Field": Equal("root.machineImages[0].versions"), + "Field": Equal("root.machineImages.versions"), })), PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeNotSupported), "Field": Equal("root.machineImages[0].versions[0].architecture"), @@ -275,7 +275,7 @@ var _ = Describe("CloudProfileConfig validation", func() { Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeRequired), - "Field": Equal("root.machineImages[0].versions"), + "Field": Equal("root.machineImages.versions"), })))) }) })