Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GEP-25] Enable Shoot cloudProfile reference as complement to cloudProfileName #834

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ rules:
- core.gardener.cloud
resources:
- cloudprofiles
- namespacedcloudprofiles
- secretbindings
verbs:
- get
Expand Down
3 changes: 2 additions & 1 deletion docs/usage/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ metadata:
name: johndoe-openstack
namespace: garden-dev
spec:
cloudProfileName: openstack
cloudProfile:
name: openstack
region: europe-1
secretBindingName: core-openstack
provider:
Expand Down
52 changes: 28 additions & 24 deletions pkg/admission/validator/shoot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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")
}
Expand All @@ -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
}
Expand Down
191 changes: 187 additions & 4 deletions pkg/admission/validator/shoot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand All @@ -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)
Expand All @@ -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"),
},
Expand All @@ -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
}
6 changes: 5 additions & 1 deletion pkg/apis/openstack/helper/scheme.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading