Add NamespacedCloudProfile support to Shoot validation. (#834)

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-openstack/charts/application/templates/rbac.yaml

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

docs/usage/usage.md

@@ -196,7 +196,8 @@ metadata:
   name: johndoe-openstack
   namespace: garden-dev
 spec:
-  cloudProfileName: openstack
+  cloudProfile:
+    name: openstack
   region: europe-1
   secretBindingName: core-openstack
   provider:

pkg/admission/validator/shoot.go

@@ -13,6 +13,7 @@ import (
 	gardencorehelper "github.com/gardener/gardener/pkg/apis/core/helper"
 	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	securityv1alpha1 "github.com/gardener/gardener/pkg/apis/security/v1alpha1"
+	"github.com/gardener/gardener/pkg/utils/gardener"
 	kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -48,7 +49,7 @@ type validationContext struct {
 	shoot              *core.Shoot
 	infraConfig        *api.InfrastructureConfig
 	cpConfig           *api.ControlPlaneConfig
-	cloudProfile       *gardencorev1beta1.CloudProfile
+	cloudProfileSpec   *gardencorev1beta1.CloudProfileSpec
 	cloudProfileConfig *api.CloudProfileConfig
 }
 
@@ -86,19 +87,32 @@ func (s *shoot) Validate(ctx context.Context, new, old client.Object) error {
 		}
 	}
 
+	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, credentials)
+		return s.validateShootUpdate(oldShoot, shoot, credentials, &cloudProfile.Spec)
 	}
 
-	return s.validateShootCreation(ctx, shoot, credentials)
+	return s.validateShootCreation(shoot, credentials, &cloudProfile.Spec)
 }
 
-func (s *shoot) validateShootCreation(ctx context.Context, shoot *core.Shoot, credentials *openstack.Credentials) error {
-	valContext, err := newValidationContext(ctx, s.decoder, s.client, shoot)
+func (s *shoot) validateShootCreation(shoot *core.Shoot, credentials *openstack.Credentials, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec) error {
+	valContext, err := newValidationContext(s.decoder, shoot, cloudProfileSpec)
 	if err != nil {
 		return err
 	}
@@ -113,13 +127,13 @@ func (s *shoot) validateShootCreation(ctx context.Context, shoot *core.Shoot, cr
 	return allErrs.ToAggregate()
 }
 
-func (s *shoot) validateShootUpdate(ctx context.Context, oldShoot, shoot *core.Shoot, credentials *openstack.Credentials) error {
-	oldValContext, err := newValidationContext(ctx, s.lenientDecoder, s.client, oldShoot)
+func (s *shoot) validateShootUpdate(oldShoot, shoot *core.Shoot, credentials *openstack.Credentials, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec) error {
+	oldValContext, err := newValidationContext(s.lenientDecoder, oldShoot, cloudProfileSpec)
 	if err != nil {
 		return err
 	}
 
-	valContext, err := newValidationContext(ctx, s.decoder, s.client, shoot)
+	valContext, err := newValidationContext(s.decoder, shoot, cloudProfileSpec)
 	if err != nil {
 		return err
 	}
@@ -168,7 +182,7 @@ func (s *shoot) validateShoot(context *validationContext) field.ErrorList {
 	return allErrs
 }
 
-func newValidationContext(ctx context.Context, decoder runtime.Decoder, c client.Client, shoot *core.Shoot) (*validationContext, error) {
+func newValidationContext(decoder runtime.Decoder, shoot *core.Shoot, cloudProfileSpec *gardencorev1beta1.CloudProfileSpec) (*validationContext, error) {
 	if shoot.Spec.Provider.InfrastructureConfig == nil {
 		return nil, field.Required(infraConfigPath, "infrastructureConfig must be set for OpenStack shoots")
 	}
@@ -185,29 +199,19 @@ func newValidationContext(ctx context.Context, decoder runtime.Decoder, c client
 		return nil, fmt.Errorf("error decoding controlPlaneConfig: %v", err)
 	}
 
-	cloudProfile := &gardencorev1beta1.CloudProfile{}
-
-	if shoot.Spec.CloudProfile == nil {
-		return nil, fmt.Errorf("shoot.spec.cloudprofile must not be nil <nil>")
-	}
-
-	if err := c.Get(ctx, client.ObjectKey{Name: shoot.Spec.CloudProfile.Name}, cloudProfile); err != nil {
-		return nil, err
-	}
-
-	if cloudProfile.Spec.ProviderConfig == nil {
-		return nil, fmt.Errorf("providerConfig is not given for cloud profile %q", cloudProfile.Name)
+	if cloudProfileSpec.ProviderConfig == nil {
+		return nil, fmt.Errorf("providerConfig is not given for cloud profile %q", shoot.Spec.CloudProfile)
 	}
-	cloudProfileConfig, err := decodeCloudProfileConfig(decoder, cloudProfile.Spec.ProviderConfig)
+	cloudProfileConfig, err := decodeCloudProfileConfig(decoder, cloudProfileSpec.ProviderConfig)
 	if err != nil {
-		return nil, fmt.Errorf("an error occurred while reading the cloud profile %q: %v", cloudProfile.Name, err)
+		return nil, fmt.Errorf("an error occurred while reading the cloud profile %q: %v", shoot.Spec.CloudProfile, err)
 	}
 
 	return &validationContext{
 		shoot:              shoot,
 		infraConfig:        infraConfig,
 		cpConfig:           cpConfig,
-		cloudProfile:       cloudProfile,
+		cloudProfileSpec:   cloudProfileSpec,
 		cloudProfileConfig: cloudProfileConfig,
 	}, nil
 }

pkg/admission/validator/shoot_test.go

@@ -6,6 +6,7 @@ package validator_test
 
 import (
 	"context"
+	"encoding/json"
 
 	extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook"
 	"github.com/gardener/gardener/pkg/apis/core"
@@ -18,8 +19,12 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/gardener/gardener-extension-provider-openstack/pkg/admission/validator"
+	apisopenstack "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
+	apisopenstackv1alpha "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/v1alpha1"
+	openstackv1alpha1 "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/v1alpha1"
 )
 
 var _ = Describe("Shoot validator", func() {
@@ -35,13 +40,20 @@ var _ = Describe("Shoot validator", func() {
 			apiReader *mockclient.MockReader
 			shoot     *core.Shoot
 
-			ctx = context.TODO()
+			ctx = context.Background()
+
+			regionName   string
+			imageName    string
+			imageVersion string
+			architecture *string
 		)
 
 		BeforeEach(func() {
 			ctrl = gomock.NewController(GinkgoT())
 
 			scheme := runtime.NewScheme()
+			Expect(apisopenstack.AddToScheme(scheme)).To(Succeed())
+			Expect(apisopenstackv1alpha.AddToScheme(scheme)).To(Succeed())
 			Expect(gardencorev1beta1.AddToScheme(scheme)).To(Succeed())
 
 			c = mockclient.NewMockClient(ctrl)
@@ -53,20 +65,63 @@ var _ = Describe("Shoot validator", func() {
 			mgr.EXPECT().GetAPIReader().Return(apiReader)
 			shootValidator = validator.NewShootValidator(mgr)
 
+			regionName = "eu-de-1"
+			imageName = "Foo"
+			imageVersion = "1.0.0"
+			architecture = ptr.To("analog")
+
 			shoot = &core.Shoot{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "foo",
 					Namespace: namespace,
 				},
 				Spec: core.ShootSpec{
 					CloudProfile: &core.CloudProfileReference{
+						Kind: "CloudProfile",
 						Name: "cloudProfile",
 					},
 					Provider: core.Provider{
-						Type:    "openstack",
-						Workers: []core.Worker{},
+						Type: "openstack",
+						Workers: []core.Worker{
+							{
+								Name: "worker-1",
+								Volume: &core.Volume{
+									VolumeSize: "50Gi",
+									Type:       ptr.To("volumeType"),
+								},
+								Zones: []string{"zone1"},
+								Machine: core.Machine{
+									Image: &core.ShootMachineImage{
+										Name:    imageName,
+										Version: imageVersion,
+									},
+									Architecture: architecture,
+								},
+							},
+						},
+						InfrastructureConfig: &runtime.RawExtension{
+							Raw: encode(&openstackv1alpha1.InfrastructureConfig{
+								TypeMeta: metav1.TypeMeta{
+									APIVersion: openstackv1alpha1.SchemeGroupVersion.String(),
+									Kind:       "InfrastructureConfig",
+								},
+								Networks: openstackv1alpha1.Networks{
+									Workers: "10.250.0.0/19",
+								},
+								FloatingPoolName: "pool-1",
+							}),
+						},
+						ControlPlaneConfig: &runtime.RawExtension{
+							Raw: encode(&openstackv1alpha1.ControlPlaneConfig{
+								TypeMeta: metav1.TypeMeta{
+									APIVersion: openstackv1alpha1.SchemeGroupVersion.String(),
+									Kind:       "ControlPlaneConfig",
+								},
+								LoadBalancerProvider: "haproxy",
+							}),
+						},
 					},
-					Region: "us-west",
+					Region: "eu-de-1",
 					Networking: &core.Networking{
 						Nodes: ptr.To("10.250.0.0/16"),
 					},
@@ -84,5 +139,133 @@ var _ = Describe("Shoot validator", func() {
 				Expect(err).NotTo(HaveOccurred())
 			})
 		})
+
+		Context("Shoot creation", func() {
+			var (
+				cloudProfileKey           client.ObjectKey
+				namespacedCloudProfileKey client.ObjectKey
+
+				cloudProfile           *gardencorev1beta1.CloudProfile
+				namespacedCloudProfile *gardencorev1beta1.NamespacedCloudProfile
+			)
+
+			BeforeEach(func() {
+				cloudProfileKey = client.ObjectKey{Name: "openstack"}
+				namespacedCloudProfileKey = client.ObjectKey{Name: "openstack-nscpfl", Namespace: namespace}
+
+				cloudProfile = &gardencorev1beta1.CloudProfile{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "openstack",
+					},
+					Spec: gardencorev1beta1.CloudProfileSpec{
+						Regions: []gardencorev1beta1.Region{
+							{
+								Name: regionName,
+								Zones: []gardencorev1beta1.AvailabilityZone{
+									{
+										Name: "zone1",
+									},
+									{
+										Name: "zone2",
+									},
+								},
+							},
+						},
+						ProviderConfig: &runtime.RawExtension{
+							Raw: encode(&apisopenstackv1alpha.CloudProfileConfig{
+								TypeMeta: metav1.TypeMeta{
+									APIVersion: apisopenstackv1alpha.SchemeGroupVersion.String(),
+									Kind:       "CloudProfileConfig",
+								},
+								MachineImages: []apisopenstackv1alpha.MachineImages{
+									{
+										Name: imageName,
+										Versions: []apisopenstackv1alpha.MachineImageVersion{
+											{
+												Version: imageVersion,
+												Regions: []apisopenstackv1alpha.RegionIDMapping{
+													{
+														Name:         regionName,
+														ID:           "Bar",
+														Architecture: architecture,
+													},
+												},
+											},
+										},
+									},
+								},
+							}),
+						},
+					},
+				}
+
+				namespacedCloudProfile = &gardencorev1beta1.NamespacedCloudProfile{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "openstack-nscpfl",
+					},
+					Spec: gardencorev1beta1.NamespacedCloudProfileSpec{
+						Parent: gardencorev1beta1.CloudProfileReference{
+							Kind: "CloudProfile",
+							Name: "openstack",
+						},
+					},
+					Status: gardencorev1beta1.NamespacedCloudProfileStatus{
+						CloudProfileSpec: cloudProfile.Spec,
+					},
+				}
+			})
+
+			It("should work for CloudProfile referenced from Shoot", func() {
+				shoot.Spec.CloudProfile = &core.CloudProfileReference{
+					Kind: "CloudProfile",
+					Name: "openstack",
+				}
+				c.EXPECT().Get(ctx, cloudProfileKey, &gardencorev1beta1.CloudProfile{}).SetArg(2, *cloudProfile)
+
+				err := shootValidator.Validate(ctx, shoot, nil)
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("should work for CloudProfile referenced from cloudProfileName", func() {
+				shoot.Spec.CloudProfileName = ptr.To("openstack")
+				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 work for NamespacedCloudProfile referenced from Shoot", func() {
+				shoot.Spec.CloudProfile = &core.CloudProfileReference{
+					Kind: "NamespacedCloudProfile",
+					Name: "openstack-nscpfl",
+				}
+				c.EXPECT().Get(ctx, namespacedCloudProfileKey, &gardencorev1beta1.NamespacedCloudProfile{}).SetArg(2, *namespacedCloudProfile)
+
+				err := shootValidator.Validate(ctx, shoot, nil)
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("should fail for a missing cloud profile provider config", func() {
+				shoot.Spec.CloudProfile = &core.CloudProfileReference{
+					Kind: "NamespacedCloudProfile",
+					Name: "openstack-nscpfl",
+				}
+				namespacedCloudProfile.Status.CloudProfileSpec.ProviderConfig = nil
+				c.EXPECT().Get(ctx, namespacedCloudProfileKey, &gardencorev1beta1.NamespacedCloudProfile{}).SetArg(2, *namespacedCloudProfile)
+
+				err := shootValidator.Validate(ctx, shoot, nil)
+				Expect(err).To(MatchError(And(
+					ContainSubstring("providerConfig is not given for cloud profile"),
+					ContainSubstring("NamespacedCloudProfile"),
+					ContainSubstring("openstack-nscpfl"),
+				)))
+			})
+		})
 	})
 })
+
+func encode(obj runtime.Object) []byte {
+	data, _ := json.Marshal(obj)
+	return data
+}

pkg/apis/openstack/helper/scheme.go

@@ -71,9 +71,13 @@ func InfrastructureStatusFromRaw(raw *runtime.RawExtension) (*api.Infrastructure
 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