From 1b7c968ed4b69a627c6f0cdfe5085c1eca106f7c Mon Sep 17 00:00:00 2001 From: Luca Bernstein Date: Mon, 22 Jul 2024 15:24:56 +0200 Subject: [PATCH] Add support for cloudprofile field and validate namespacedcloudprofiles --- .../charts/application/templates/rbac.yaml | 1 + docs/usage/usage.md | 6 +- .../validator/namespacedcloudprofile.go | 51 +++++++++++++++ pkg/admission/validator/shoot.go | 40 +++++++----- pkg/admission/validator/shoot_test.go | 63 ++++++++++++++++--- pkg/admission/validator/webhook.go | 7 ++- 6 files changed, 138 insertions(+), 30 deletions(-) create mode 100644 pkg/admission/validator/namespacedcloudprofile.go diff --git a/charts/gardener-extension-admission-aws/charts/application/templates/rbac.yaml b/charts/gardener-extension-admission-aws/charts/application/templates/rbac.yaml index fb74c7711..a76285f47 100644 --- a/charts/gardener-extension-admission-aws/charts/application/templates/rbac.yaml +++ b/charts/gardener-extension-admission-aws/charts/application/templates/rbac.yaml @@ -10,6 +10,7 @@ rules: - core.gardener.cloud resources: - cloudprofiles + - namespacedcloudprofiles verbs: - get - list diff --git a/docs/usage/usage.md b/docs/usage/usage.md index baf4ca966..78a955a66 100644 --- a/docs/usage/usage.md +++ b/docs/usage/usage.md @@ -468,7 +468,8 @@ metadata: name: johndoe-aws namespace: garden-dev spec: - cloudProfileName: aws + cloudProfile: + name: aws region: eu-central-1 secretBindingName: core-aws provider: @@ -531,7 +532,8 @@ metadata: name: johndoe-aws namespace: garden-dev spec: - cloudProfileName: aws + cloudProfile: + name: aws region: eu-central-1 secretBindingName: core-aws provider: diff --git a/pkg/admission/validator/namespacedcloudprofile.go b/pkg/admission/validator/namespacedcloudprofile.go new file mode 100644 index 000000000..034c8d34a --- /dev/null +++ b/pkg/admission/validator/namespacedcloudprofile.go @@ -0,0 +1,51 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validator + +import ( + "context" + "fmt" + + extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook" + "github.com/gardener/gardener/pkg/apis/core" + "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" + + awsvalidation "github.com/gardener/gardener-extension-provider-aws/pkg/apis/aws/validation" +) + +// NewNamespacedCloudProfileValidator returns a new instance of a cloud profile validator. +func NewNamespacedCloudProfileValidator(mgr manager.Manager) extensionswebhook.Validator { + return &namespacedCloudProfile{ + decoder: serializer.NewCodecFactory(mgr.GetScheme(), serializer.EnableStrict).UniversalDecoder(), + } +} + +type namespacedCloudProfile struct { + decoder runtime.Decoder +} + +// Validate validates the given namespaced cloud profile objects. +func (cp *namespacedCloudProfile) Validate(_ context.Context, new, _ client.Object) error { + cloudProfile, ok := new.(*core.NamespacedCloudProfile) + if !ok { + return fmt.Errorf("wrong object type %T", new) + } + + providerConfigPath := field.NewPath("status", "cloudProfileSpec").Child("providerConfig") + if cloudProfile.Status.CloudProfileSpec.ProviderConfig == nil { + return field.Required(providerConfigPath, "providerConfig must be calculated for AWS namespaced cloud profiles") + } + + cpConfig, err := decodeCloudProfileConfig(cp.decoder, cloudProfile.Status.CloudProfileSpec.ProviderConfig) + if err != nil { + return err + } + + return awsvalidation.ValidateCloudProfileConfig(cpConfig, providerConfigPath).ToAggregate() +} diff --git a/pkg/admission/validator/shoot.go b/pkg/admission/validator/shoot.go index dd79d7b02..add3c710b 100644 --- a/pkg/admission/validator/shoot.go +++ b/pkg/admission/validator/shoot.go @@ -14,6 +14,8 @@ import ( "github.com/gardener/gardener/pkg/apis/core" gardencorehelper "github.com/gardener/gardener/pkg/apis/core/helper" gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/gardener/gardener/pkg/utils/gardener" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/validation/field" @@ -53,15 +55,33 @@ func (s *shoot) Validate(ctx context.Context, new, old client.Object) error { return nil } + shootV1Beta1 := &gardencorev1beta1.Shoot{ + TypeMeta: metav1.TypeMeta{ + APIVersion: gardencorev1beta1.SchemeGroupVersion.String(), + Kind: "Shoot", + }, + } + err := s.scheme.Convert(shoot, shootV1Beta1, ctx) + if err != nil { + return err + } + cloudProfile, err := gardener.GetCloudProfile(ctx, s.client, shootV1Beta1) + if err != nil { + return err + } + if cloudProfile == nil { + return fmt.Errorf("cloudprofile could not be found") + } + if old != nil { oldShoot, ok := old.(*core.Shoot) if !ok { return fmt.Errorf("wrong object type %T for old object", old) } - return s.validateShootUpdate(ctx, oldShoot, shoot) + return s.validateShootUpdate(ctx, oldShoot, shoot, cloudProfile) } - return s.validateShootCreation(ctx, shoot) + return s.validateShootCreation(ctx, shoot, cloudProfile) } func (s *shoot) validateShoot(_ context.Context, shoot *core.Shoot) error { @@ -122,11 +142,10 @@ func (s *shoot) validateShoot(_ context.Context, shoot *core.Shoot) error { return nil } -func (s *shoot) validateShootUpdate(ctx context.Context, oldShoot, shoot *core.Shoot) error { +func (s *shoot) validateShootUpdate(ctx context.Context, oldShoot, shoot *core.Shoot, cloudProfile *gardencorev1beta1.CloudProfile) error { var ( fldPath = field.NewPath("spec", "provider") infraConfigFldPath = fldPath.Child("infrastructureConfig") - cloudProfile = &gardencorev1beta1.CloudProfile{} ) // InfrastructureConfig update @@ -139,10 +158,6 @@ func (s *shoot) validateShootUpdate(ctx context.Context, oldShoot, shoot *core.S return err } - if err := s.client.Get(ctx, client.ObjectKey{Name: shoot.Spec.CloudProfileName}, cloudProfile); err != nil { - return err - } - if oldShoot.Spec.Provider.InfrastructureConfig == nil { return field.InternalError(infraConfigFldPath, errors.New("InfrastructureConfig is not available on old shoot")) } @@ -178,10 +193,9 @@ func (s *shoot) validateShootUpdate(ctx context.Context, oldShoot, shoot *core.S return s.validateShoot(ctx, shoot) } -func (s *shoot) validateShootCreation(ctx context.Context, shoot *core.Shoot) error { +func (s *shoot) validateShootCreation(ctx context.Context, shoot *core.Shoot, cloudProfile *gardencorev1beta1.CloudProfile) error { var ( - fldPath = field.NewPath("spec", "provider") - cloudProfile = &gardencorev1beta1.CloudProfile{} + fldPath = field.NewPath("spec", "provider") ) if shoot.Spec.Provider.InfrastructureConfig == nil { return field.Required(fldPath.Child("infrastructureConfig"), "InfrastructureConfig must be set for AWS shoots") @@ -192,10 +206,6 @@ func (s *shoot) validateShootCreation(ctx context.Context, shoot *core.Shoot) er return err } - if err := s.client.Get(ctx, client.ObjectKey{Name: shoot.Spec.CloudProfileName}, cloudProfile); err != nil { - return err - } - awsCloudProfile, err := decodeCloudProfileConfig(s.decoder, cloudProfile.Spec.ProviderConfig) if err != nil { return err diff --git a/pkg/admission/validator/shoot_test.go b/pkg/admission/validator/shoot_test.go index 36c6b1b94..2a2c3fcd5 100644 --- a/pkg/admission/validator/shoot_test.go +++ b/pkg/admission/validator/shoot_test.go @@ -36,15 +36,17 @@ var _ = Describe("Shoot validator", func() { var ( shootValidator extensionswebhook.Validator - ctrl *gomock.Controller - mgr *mockmanager.MockManager - c *mockclient.MockClient - cloudProfile *gardencorev1beta1.CloudProfile - shoot *core.Shoot - - ctx = context.TODO() - cloudProfileKey = client.ObjectKey{Name: "aws"} - gp2type = string(apisaws.VolumeTypeGP2) + ctrl *gomock.Controller + mgr *mockmanager.MockManager + c *mockclient.MockClient + cloudProfile *gardencorev1beta1.CloudProfile + namespacedCloudProfile *gardencorev1beta1.NamespacedCloudProfile + shoot *core.Shoot + + ctx = context.TODO() + cloudProfileKey = client.ObjectKey{Name: "aws"} + namespacedCloudProfileKey = client.ObjectKey{Name: "aws-nscpfl", Namespace: namespace} + gp2type = string(apisaws.VolumeTypeGP2) regionName = "us-west" imageName = "Foo" @@ -114,13 +116,28 @@ var _ = Describe("Shoot validator", func() { }, } + namespacedCloudProfile = &gardencorev1beta1.NamespacedCloudProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "aws-nscpfl", + }, + Spec: gardencorev1beta1.NamespacedCloudProfileSpec{ + Parent: gardencorev1beta1.CloudProfileReference{ + Kind: "CloudProfile", + Name: "aws", + }, + }, + Status: gardencorev1beta1.NamespacedCloudProfileStatus{ + CloudProfileSpec: cloudProfile.Spec, // TODO(LucaBernstein): Okay to mock like this? + }, + } + shoot = &core.Shoot{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: namespace, }, Spec: core.ShootSpec{ - CloudProfileName: cloudProfile.Name, + CloudProfileName: &cloudProfile.Name, Provider: core.Provider{ InfrastructureConfig: &runtime.RawExtension{ Raw: encode(&apisawsv1alpha1.InfrastructureConfig{ @@ -176,6 +193,7 @@ var _ = Describe("Shoot validator", func() { }) It("should return err when infrastructureConfig is nil", func() { + c.EXPECT().Get(ctx, cloudProfileKey, &gardencorev1beta1.CloudProfile{}).SetArg(2, *cloudProfile) shoot.Spec.Provider.InfrastructureConfig = nil err := shootValidator.Validate(ctx, shoot, nil) @@ -186,6 +204,7 @@ var _ = Describe("Shoot validator", func() { }) It("should return err when infrastructureConfig fails to be decoded", func() { + c.EXPECT().Get(ctx, cloudProfileKey, &gardencorev1beta1.CloudProfile{}).SetArg(2, *cloudProfile) shoot.Spec.Provider.InfrastructureConfig = &runtime.RawExtension{Raw: []byte("foo")} err := shootValidator.Validate(ctx, shoot, nil) @@ -364,6 +383,30 @@ var _ = Describe("Shoot validator", func() { err := shootValidator.Validate(ctx, shoot, nil) Expect(err).NotTo(HaveOccurred()) }) + + It("should also work for CloudProfile reference instead of cloudProfileName in Shoot", func() { + shoot.Spec.CloudProfileName = nil + shoot.Spec.CloudProfile = &core.CloudProfileReference{ + Kind: "CloudProfile", + Name: "aws", + } + c.EXPECT().Get(ctx, cloudProfileKey, &gardencorev1beta1.CloudProfile{}).SetArg(2, *cloudProfile) + + err := shootValidator.Validate(ctx, shoot, nil) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should also work for NamespacedCloudProfile referenced from Shoot", func() { + shoot.Spec.CloudProfileName = nil + shoot.Spec.CloudProfile = &core.CloudProfileReference{ + Kind: "NamespacedCloudProfile", + Name: "aws-nscpfl", + } + c.EXPECT().Get(ctx, namespacedCloudProfileKey, &gardencorev1beta1.NamespacedCloudProfile{}).SetArg(2, *namespacedCloudProfile) + + err := shootValidator.Validate(ctx, shoot, nil) + Expect(err).NotTo(HaveOccurred()) + }) }) Context("Workerless Shoot", func() { diff --git a/pkg/admission/validator/webhook.go b/pkg/admission/validator/webhook.go index 13788548b..2683c0aff 100644 --- a/pkg/admission/validator/webhook.go +++ b/pkg/admission/validator/webhook.go @@ -36,9 +36,10 @@ func New(mgr manager.Manager) (*extensionswebhook.Webhook, error) { Path: "/webhooks/validate", Predicates: []predicate.Predicate{extensionspredicate.GardenCoreProviderType(aws.Type)}, Validators: map[extensionswebhook.Validator][]extensionswebhook.Type{ - NewShootValidator(mgr): {{Obj: &core.Shoot{}}}, - NewCloudProfileValidator(mgr): {{Obj: &core.CloudProfile{}}}, - NewSecretBindingValidator(mgr): {{Obj: &core.SecretBinding{}}}, + NewShootValidator(mgr): {{Obj: &core.Shoot{}}}, + NewCloudProfileValidator(mgr): {{Obj: &core.CloudProfile{}}}, + NewNamespacedCloudProfileValidator(mgr): {{Obj: &core.NamespacedCloudProfile{}}}, + NewSecretBindingValidator(mgr): {{Obj: &core.SecretBinding{}}}, }, Target: extensionswebhook.TargetSeed, ObjectSelector: &metav1.LabelSelector{