Add NamespacedCloudProfile support to Shoot validation.

Switch from CloudProfile to CloudProfileSpec as a generic foundation for CloudProfile and NamespacedCloudProfile.

Improve log message for possibly artificially crafted CloudProfile from NamespacedCloudProfile in Cluster resource.

charts/gardener-extension-admission-aws/charts/application/templates/rbac.yaml

@@ -10,6 +10,7 @@ rules:
   - core.gardener.cloud
   resources:
   - cloudprofiles
+  - namespacedcloudprofiles
   verbs:
   - get
   - list

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:

pkg/admission/validator/shoot.go

@@ -13,6 +13,7 @@ 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"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -27,7 +28,6 @@ import (
 func NewShootValidator(mgr manager.Manager) extensionswebhook.Validator {
 	return &shoot{
 		client:         mgr.GetClient(),
-		scheme:         mgr.GetScheme(),
 		decoder:        serializer.NewCodecFactory(mgr.GetScheme(), serializer.EnableStrict).UniversalDecoder(),
 		lenientDecoder: serializer.NewCodecFactory(mgr.GetScheme()).UniversalDecoder(),
 	}
@@ -37,7 +37,6 @@ type shoot struct {
 	client         client.Client
 	decoder        runtime.Decoder
 	lenientDecoder runtime.Decoder
-	scheme         *runtime.Scheme
 }
 
 // Validate validates the given shoot object.
@@ -52,15 +51,28 @@ func (s *shoot) Validate(ctx context.Context, new, old client.Object) error {
 		return nil
 	}
 
+	shootV1Beta1 := &gardencorev1beta1.Shoot{}
+	err := gardencorev1beta1.Convert_core_Shoot_To_v1beta1_Shoot(shoot, shootV1Beta1, nil)
+	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.Spec)
 	}
 
-	return s.validateShootCreation(ctx, shoot)
+	return s.validateShootCreation(ctx, shoot, &cloudProfile.Spec)
 }
 
 func (s *shoot) validateShoot(_ context.Context, shoot *core.Shoot) error {
@@ -121,7 +133,7 @@ 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, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec) error {
 	var (
 		fldPath            = field.NewPath("spec", "provider")
 		infraConfigFldPath = fldPath.Child("infrastructureConfig")
@@ -136,7 +148,7 @@ func (s *shoot) validateShootUpdate(ctx context.Context, oldShoot, shoot *core.S
 		return err
 	}
 
-	awsCloudProfile, infraConfig, err := s.baseShootValidation(ctx, shoot, oldInfraConfig, fldPath)
+	awsCloudProfile, infraConfig, err := s.baseShootValidation(ctx, shoot, cloudProfileSpec, oldInfraConfig, fldPath)
 	if err != nil {
 		return err
 	}
@@ -158,12 +170,12 @@ 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, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec) error {
 	var (
 		fldPath = field.NewPath("spec", "provider")
 	)
 
-	awsCloudProfile, _, err := s.baseShootValidation(ctx, shoot, nil, fldPath)
+	awsCloudProfile, _, err := s.baseShootValidation(ctx, shoot, cloudProfileSpec, nil, fldPath)
 	if err != nil {
 		return err
 	}
@@ -175,9 +187,8 @@ func (s *shoot) validateShootCreation(ctx context.Context, shoot *core.Shoot) er
 	return s.validateShoot(ctx, shoot)
 }
 
-func (s *shoot) baseShootValidation(ctx context.Context, shoot *core.Shoot, oldInfraConfig *api.InfrastructureConfig, fldPath *field.Path) (*api.CloudProfileConfig, *api.InfrastructureConfig, error) {
+func (s *shoot) baseShootValidation(ctx context.Context, shoot *core.Shoot, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec, oldInfraConfig *api.InfrastructureConfig, fldPath *field.Path) (*api.CloudProfileConfig, *api.InfrastructureConfig, error) {
 	var (
-		commonCloudProfile = &gardencorev1beta1.CloudProfile{}
 		infraConfigFldPath = fldPath.Child("infrastructureConfig")
 	)
 
@@ -190,28 +201,23 @@ func (s *shoot) baseShootValidation(ctx context.Context, shoot *core.Shoot, oldI
 		return nil, nil, err
 	}
 
-	if shoot.Spec.CloudProfile == nil {
+	if cloudProfileSpec == nil {
 		return nil, nil, fmt.Errorf("shoot.spec.cloudprofile must not be nil <nil>")
 	}
-
-	if err := s.client.Get(ctx, client.ObjectKey{Name: shoot.Spec.CloudProfile.Name}, commonCloudProfile); err != nil {
-		return nil, nil, err
-	}
-
-	awsCloudProfile, err := decodeCloudProfileConfig(s.decoder, commonCloudProfile.Spec.ProviderConfig)
+	awsCloudProfile, err := decodeCloudProfileConfig(s.decoder, cloudProfileSpec.ProviderConfig)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	if err = s.validateAgainstCloudProfile(ctx, shoot, oldInfraConfig, infraConfig, commonCloudProfile, infraConfigFldPath); err != nil {
+	if err = s.validateAgainstCloudProfile(ctx, shoot, oldInfraConfig, infraConfig, cloudProfileSpec, infraConfigFldPath); err != nil {
 		return nil, nil, err
 	}
 
 	return awsCloudProfile, infraConfig, nil
 }
 
-func (s *shoot) validateAgainstCloudProfile(_ context.Context, shoot *core.Shoot, oldInfraConfig, infraConfig *api.InfrastructureConfig, cloudProfile *gardencorev1beta1.CloudProfile, fldPath *field.Path) error {
-	if errList := awsvalidation.ValidateInfrastructureConfigAgainstCloudProfile(oldInfraConfig, infraConfig, shoot, cloudProfile, fldPath); len(errList) != 0 {
+func (s *shoot) validateAgainstCloudProfile(_ context.Context, shoot *core.Shoot, oldInfraConfig, infraConfig *api.InfrastructureConfig, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec, fldPath *field.Path) error {
+	if errList := awsvalidation.ValidateInfrastructureConfigAgainstCloudProfile(oldInfraConfig, infraConfig, shoot, cloudProfileSpec, fldPath); len(errList) != 0 {
 		return errList.ToAggregate()
 	}
 

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.Background()
+			cloudProfileKey           = client.ObjectKey{Name: "aws"}
+			namespacedCloudProfileKey = client.ObjectKey{Name: "aws-nscpfl", Namespace: namespace}
+			gp2type                   = string(apisaws.VolumeTypeGP2)
 
 			regionName   = "us-west"
 			imageName    = "Foo"
@@ -63,7 +65,7 @@ var _ = Describe("Shoot validator", func() {
 			c = mockclient.NewMockClient(ctrl)
 			mgr = mockmanager.NewMockManager(ctrl)
 
-			mgr.EXPECT().GetScheme().Return(scheme).Times(3)
+			mgr.EXPECT().GetScheme().Return(scheme).Times(2)
 			mgr.EXPECT().GetClient().Return(c)
 
 			shootValidator = validator.NewShootValidator(mgr)
@@ -114,13 +116,29 @@ 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,
+				},
+			}
+
 			shoot = &core.Shoot{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "foo",
 					Namespace: namespace,
 				},
 				Spec: core.ShootSpec{
 					CloudProfile: &core.CloudProfileReference{
+						Kind: "CloudProfile",
 						Name: cloudProfile.Name,
 					},
 					Provider: core.Provider{
@@ -178,6 +196,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)
@@ -188,6 +207,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)
@@ -366,6 +386,26 @@ var _ = Describe("Shoot validator", func() {
 				err := shootValidator.Validate(ctx, shoot, nil)
 				Expect(err).NotTo(HaveOccurred())
 			})
+
+			It("should also work for CloudProfileName instead of CloudProfile reference in Shoot", func() {
+				shoot.Spec.CloudProfileName = ptr.To("aws")
+				shoot.Spec.CloudProfile = nil
+				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.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() {

pkg/apis/aws/helper/scheme.go

@@ -36,9 +36,13 @@ func init() {
 func CloudProfileConfigFromCluster(cluster *controller.Cluster) (*api.CloudProfileConfig, error) {
 	var cloudProfileConfig *api.CloudProfileConfig
 	if cluster != nil && cluster.CloudProfile != nil && cluster.CloudProfile.Spec.ProviderConfig != nil && cluster.CloudProfile.Spec.ProviderConfig.Raw != nil {
+		cloudProfileSpecifier := fmt.Sprintf("cloudProfile '%q'", k8sclient.ObjectKeyFromObject(cluster.CloudProfile))
+		if cluster.Shoot != nil && cluster.Shoot.Spec.CloudProfile != nil {
+			cloudProfileSpecifier = fmt.Sprintf("%s '%s/%s'", cluster.Shoot.Spec.CloudProfile.Kind, cluster.Shoot.Namespace, cluster.Shoot.Spec.CloudProfile.Name)
+		}
 		cloudProfileConfig = &api.CloudProfileConfig{}
 		if _, _, err := decoder.Decode(cluster.CloudProfile.Spec.ProviderConfig.Raw, nil, cloudProfileConfig); err != nil {
-			return nil, fmt.Errorf("could not decode providerConfig of cloudProfile for '%s': %w", k8sclient.ObjectKeyFromObject(cluster.CloudProfile), err)
+			return nil, fmt.Errorf("could not decode providerConfig of %s: %w", cloudProfileSpecifier, err)
 		}
 	}
 	return cloudProfileConfig, nil

pkg/apis/aws/validation/infrastructure.go

@@ -23,11 +23,11 @@ import (
 var gatewayEndpointPattern = regexp.MustCompile(`^\w+(\.\w+)*$`)
 
 // ValidateInfrastructureConfigAgainstCloudProfile validates the given `InfrastructureConfig` against the given `CloudProfile`.
-func ValidateInfrastructureConfigAgainstCloudProfile(oldInfra, infra *apisaws.InfrastructureConfig, shoot *core.Shoot, cloudProfile *gardencorev1beta1.CloudProfile, fldPath *field.Path) field.ErrorList {
+func ValidateInfrastructureConfigAgainstCloudProfile(oldInfra, infra *apisaws.InfrastructureConfig, shoot *core.Shoot, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	shootRegion := shoot.Spec.Region
-	for _, region := range cloudProfile.Spec.Regions {
+	for _, region := range cloudProfileSpec.Regions {
 		if region.Name == shootRegion {
 			allErrs = append(allErrs, validateInfrastructureConfigZones(oldInfra, infra, region.Zones, fldPath.Child("network"))...)
 			break

pkg/apis/aws/validation/infrastructure_test.go

@@ -101,14 +101,14 @@ var _ = Describe("InfrastructureConfig validation", func() {
 			})
 
 			It("should pass because zone is configured in CloudProfile", func() {
-				errorList := ValidateInfrastructureConfigAgainstCloudProfile(nil, infrastructureConfig, shoot, cloudProfile, &field.Path{})
+				errorList := ValidateInfrastructureConfigAgainstCloudProfile(nil, infrastructureConfig, shoot, &cloudProfile.Spec, &field.Path{})
 
 				Expect(errorList).To(BeEmpty())
 			})
 
 			It("should forbid because zone is not specified in CloudProfile", func() {
 				infrastructureConfig.Networks.Zones[0].Name = "not-available"
-				errorList := ValidateInfrastructureConfigAgainstCloudProfile(nil, infrastructureConfig, shoot, cloudProfile, field.NewPath("spec"))
+				errorList := ValidateInfrastructureConfigAgainstCloudProfile(nil, infrastructureConfig, shoot, &cloudProfile.Spec, field.NewPath("spec"))
 
 				Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
 					"Type":  Equal(field.ErrorTypeNotSupported),
@@ -118,7 +118,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
 
 			It("should forbid because zone is duplicate", func() {
 				infrastructureConfig.Networks.Zones = append(infrastructureConfig.Networks.Zones, infrastructureConfig.Networks.Zones[0])
-				errorList := ValidateInfrastructureConfigAgainstCloudProfile(nil, infrastructureConfig, shoot, cloudProfile, field.NewPath("spec"))
+				errorList := ValidateInfrastructureConfigAgainstCloudProfile(nil, infrastructureConfig, shoot, &cloudProfile.Spec, field.NewPath("spec"))
 
 				Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
 					"Type":  Equal(field.ErrorTypeDuplicate),
@@ -129,7 +129,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
 			It("should forbid zone update because zone is duplicate", func() {
 				oldInfra := infrastructureConfig.DeepCopy()
 				infrastructureConfig.Networks.Zones = append(infrastructureConfig.Networks.Zones, infrastructureConfig.Networks.Zones[0])
-				errorList := ValidateInfrastructureConfigAgainstCloudProfile(oldInfra, infrastructureConfig, shoot, cloudProfile, field.NewPath("spec"))
+				errorList := ValidateInfrastructureConfigAgainstCloudProfile(oldInfra, infrastructureConfig, shoot, &cloudProfile.Spec, field.NewPath("spec"))
 
 				Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
 					"Type":  Equal(field.ErrorTypeDuplicate),
@@ -141,7 +141,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
 				infrastructureConfig.Networks.Zones[0].Name = "not-available"
 				oldInfrastructureConfig := infrastructureConfig.DeepCopy()
 
-				errorList := ValidateInfrastructureConfigAgainstCloudProfile(oldInfrastructureConfig, infrastructureConfig, shoot, cloudProfile, field.NewPath("spec"))
+				errorList := ValidateInfrastructureConfigAgainstCloudProfile(oldInfrastructureConfig, infrastructureConfig, shoot, &cloudProfile.Spec, field.NewPath("spec"))
 
 				Expect(errorList).To(BeEmpty())
 			})
@@ -150,7 +150,7 @@ var _ = Describe("InfrastructureConfig validation", func() {
 				oldInfrastructureConfig := infrastructureConfig.DeepCopy()
 				infrastructureConfig.Networks.Zones[0].Name = "not-available"
 
-				errorList := ValidateInfrastructureConfigAgainstCloudProfile(oldInfrastructureConfig, infrastructureConfig, shoot, cloudProfile, field.NewPath("spec"))
+				errorList := ValidateInfrastructureConfigAgainstCloudProfile(oldInfrastructureConfig, infrastructureConfig, shoot, &cloudProfile.Spec, field.NewPath("spec"))
 
 				Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
 					"Type":  Equal(field.ErrorTypeNotSupported),