Skip to content

Commit

Permalink
Validate MachineImage architectures against providerConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
@LucaBernstein
LucaBernstein committed Nov 25, 2024
1 parent 7906e7f commit d593314
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 12 deletions.
35 changes: 28 additions & 7 deletions pkg/admission/validator/namespacedcloudprofile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package validator
import (
"context"
"fmt"
"slices"

"github.com/gardener/gardener/extensions/pkg/util"
extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook"
Expand All @@ -17,6 +18,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"

Expand Down Expand Up @@ -123,11 +125,13 @@ func (p *namespacedCloudProfile) validateMachineImages(providerConfig *api.Cloud
}
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 _, expectedArchitecture := range version.Architectures {
if _, exists := providerImages.GetImageVersion(machineImage.Name, 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),
))
}
}
}
}
Expand All @@ -152,24 +156,41 @@ func (p *namespacedCloudProfile) validateMachineImages(providerConfig *api.Cloud
continue
}
for versionIdx, version := range machineImage.Versions {
if _, exists := profileImages.GetImageVersion(machineImage.Name, version.Version); !exists {
profileImageVersion, exists := profileImages.GetImageVersion(machineImage.Name, version.Version)
if !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",
))
}
providerConfigArchitecture := ptr.Deref(version.Architecture, constants.ArchitectureAMD64)
if !slices.Contains(profileImageVersion.Architectures, providerConfigArchitecture) {
allErrs = append(allErrs, field.Forbidden(
field.NewPath("spec.providerConfig.machineImages"),
fmt.Sprintf("machine image version %s@%s has an excess entry for architecture %q, which is not defined in the machineImages spec",
machineImage.Name, version.Version, providerConfigArchitecture),
))
}
}
}

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 v.Version })
return utils.CreateMapFromSlice(mi.Versions, func(v api.MachineImageVersion) string { return providerMachineImageKey(v) })
},
)
}
Expand Down
55 changes: 50 additions & 5 deletions pkg/admission/validator/namespacedcloudprofile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ var _ = Describe("NamespacedCloudProfile Validator", func() {
namespacedCloudProfile.Spec.MachineImages = []core.MachineImage{
{
Name: "image-1",
Versions: []core.MachineImageVersion{{ExpirableVersion: core.ExpirableVersion{Version: "1.1"}}},
Versions: []core.MachineImageVersion{{ExpirableVersion: core.ExpirableVersion{Version: "1.1"}, Architectures: []string{"amd64"}}},
},
{
Name: "image-2",
Versions: []core.MachineImageVersion{{ExpirableVersion: core.ExpirableVersion{Version: "2.0"}}},
Versions: []core.MachineImageVersion{{ExpirableVersion: core.ExpirableVersion{Version: "2.0"}, Architectures: []string{"amd64"}}},
},
}
namespacedCloudProfile.Spec.MachineTypes = []core.MachineType{
Expand Down Expand Up @@ -147,7 +147,7 @@ var _ = Describe("NamespacedCloudProfile Validator", func() {
{
Name: "image-1",
Versions: []core.MachineImageVersion{
{ExpirableVersion: core.ExpirableVersion{Version: "1.0"}},
{ExpirableVersion: core.ExpirableVersion{Version: "1.0"}, Architectures: []string{"amd64"}},
},
},
}
Expand Down Expand Up @@ -179,7 +179,7 @@ var _ = Describe("NamespacedCloudProfile Validator", func() {
{
Name: "image-1",
Versions: []core.MachineImageVersion{
{ExpirableVersion: core.ExpirableVersion{Version: "1.2"}},
{ExpirableVersion: core.ExpirableVersion{Version: "1.2"}, Architectures: []string{"amd64"}},
},
},
}
Expand All @@ -196,6 +196,10 @@ var _ = Describe("NamespacedCloudProfile Validator", func() {
"Field": Equal("spec.providerConfig.machineImages[0].versions[0]"),
"BadValue": Equal("[email protected]"),
"Detail": Equal("machine image version is not defined in the NamespacedCloudProfile"),
})), PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeForbidden),
"Field": Equal("spec.providerConfig.machineImages"),
"Detail": Equal("machine image version [email protected] has an excess entry for architecture \"amd64\", which is not defined in the machineImages spec"),
}))))
})

Expand Down Expand Up @@ -225,7 +229,7 @@ var _ = Describe("NamespacedCloudProfile Validator", func() {
{
Name: "image-3",
Versions: []core.MachineImageVersion{
{ExpirableVersion: core.ExpirableVersion{Version: "3.0"}},
{ExpirableVersion: core.ExpirableVersion{Version: "3.0"}, Architectures: []string{"amd64"}},
},
},
}
Expand All @@ -240,6 +244,47 @@ var _ = Describe("NamespacedCloudProfile Validator", func() {
}))))
})

It("should fail for NamespacedCloudProfile specifying new spec.machineImages without the according version and architecture entries in the provider config", 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-id-1","architecture":"arm64"},
{"version":"1.1","id":"image-id-2","architecture":"amd64"},
{"version":"1.1-fallback","id":"image-id-3"}
]}
]
}`)}
namespacedCloudProfile.Spec.MachineImages = []core.MachineImage{
{
Name: "image-1",
Versions: []core.MachineImageVersion{
{ExpirableVersion: core.ExpirableVersion{Version: "1.1"}, Architectures: []string{"amd64", "arm64"}},
{ExpirableVersion: core.ExpirableVersion{Version: "1.1-fallback"}, Architectures: []string{"arm64"}},
{ExpirableVersion: core.ExpirableVersion{Version: "1.1-missing"}, Architectures: []string{"arm64"}},
},
},
}

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"),
"Detail": Equal("machine image version [email protected] has an excess entry for architecture \"amd64\", which is not defined in the machineImages spec"),
})), PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeRequired),
"Field": Equal("spec.providerConfig.machineImages"),
"Detail": Equal("machine image version [email protected] is not defined in the NamespacedCloudProfile providerConfig"),
})), PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeRequired),
"Field": Equal("spec.providerConfig.machineImages"),
"Detail": Equal("machine image version [email protected] 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.
Expand Down

0 comments on commit d593314

Please sign in to comment.