From fe3b5eb5a69e26dfc80ba1994f5d0963de6ed504 Mon Sep 17 00:00:00 2001 From: Rafael Franzke Date: Wed, 4 Dec 2019 16:17:07 +0100 Subject: [PATCH] Allow region-ized fields in OpenStack CloudProfileConfig --- .../provider-azure/pkg/internal/scheme.go | 70 ---------- .../docs/usage-as-operator.md | 18 ++- .../hack/api-reference/api.md | 80 +++++++++++ .../pkg/apis/openstack/helper/helper.go | 17 +++ .../pkg/apis/openstack/helper/helper_test.go | 20 +++ .../pkg/apis/openstack/types_cloudprofile.go | 14 ++ .../openstack/v1alpha1/types_cloudprofile.go | 20 ++- .../v1alpha1/zz_generated.conversion.go | 38 ++++++ .../v1alpha1/zz_generated.deepcopy.go | 35 ++++- .../apis/openstack/validation/cloudprofile.go | 55 +++++++- .../openstack/validation/cloudprofile_test.go | 127 +++++++++++++++--- .../apis/openstack/validation/controlplane.go | 40 ++++-- .../openstack/validation/controlplane_test.go | 105 ++++++++++++++- .../openstack/validation/infrastructure.go | 40 ++++-- .../validation/infrastructure_test.go | 95 +++++++++++-- .../apis/openstack/zz_generated.deepcopy.go | 35 ++++- .../controller/controlplane/valuesprovider.go | 60 ++++++--- .../controlplane/valuesprovider_test.go | 75 ++++++----- .../pkg/controller/infrastructure/actuator.go | 8 +- .../pkg/controller/worker/machines.go | 17 ++- .../pkg/controller/worker/machines_test.go | 21 +-- .../pkg/internal/infrastructure/terraform.go | 45 +++---- .../internal/infrastructure/terraform_test.go | 16 ++- .../provider-openstack/pkg/internal/scheme.go | 70 ---------- 24 files changed, 808 insertions(+), 313 deletions(-) delete mode 100644 controllers/provider-azure/pkg/internal/scheme.go delete mode 100644 controllers/provider-openstack/pkg/internal/scheme.go diff --git a/controllers/provider-azure/pkg/internal/scheme.go b/controllers/provider-azure/pkg/internal/scheme.go deleted file mode 100644 index 991d673de..000000000 --- a/controllers/provider-azure/pkg/internal/scheme.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (c) 2019 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package internal - -import ( - "fmt" - - "github.com/gardener/gardener-extensions/controllers/provider-azure/pkg/apis/azure/install" - azurev1alpha1 "github.com/gardener/gardener-extensions/controllers/provider-azure/pkg/apis/azure/v1alpha1" - - gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" - extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" -) - -var ( - // Scheme is a scheme with the types relevant for Azure actuators. - Scheme *runtime.Scheme - - decoder runtime.Decoder -) - -func init() { - Scheme = runtime.NewScheme() - utilruntime.Must(install.AddToScheme(Scheme)) - - decoder = serializer.NewCodecFactory(Scheme).UniversalDecoder() -} - -// InfrastructureConfigFromInfrastructure extracts the InfrastructureConfig from the -// ProviderConfig section of the given Infrastructure. -func InfrastructureConfigFromInfrastructure(infra *extensionsv1alpha1.Infrastructure) (*azurev1alpha1.InfrastructureConfig, error) { - config := &azurev1alpha1.InfrastructureConfig{} - if infra.Spec.ProviderConfig != nil && infra.Spec.ProviderConfig.Raw != nil { - if _, _, err := decoder.Decode(infra.Spec.ProviderConfig.Raw, nil, config); err != nil { - return nil, err - } - - return config, nil - } - return nil, fmt.Errorf("infrastructure config is not set on the infrastructure resource") -} - -// CloudProfileConfigFromCloudProfile extracts the CloudProfileConfig from the -// ProviderConfig section of the given CloudProfile. -func CloudProfileConfigFromCloudProfile(cloudProfile *gardencorev1beta1.CloudProfile) (*azurev1alpha1.CloudProfileConfig, error) { - config := &azurev1alpha1.CloudProfileConfig{} - if cloudProfile.Spec.ProviderConfig != nil && cloudProfile.Spec.ProviderConfig.Raw != nil { - if _, _, err := decoder.Decode(cloudProfile.Spec.ProviderConfig.Raw, nil, config); err != nil { - return nil, err - } - - return config, nil - } - return nil, fmt.Errorf("cloud profile config is not set on the cloudprofile resource") -} diff --git a/controllers/provider-openstack/docs/usage-as-operator.md b/controllers/provider-openstack/docs/usage-as-operator.md index aa2eb85bb..9fbb85c80 100644 --- a/controllers/provider-openstack/docs/usage-as-operator.md +++ b/controllers/provider-openstack/docs/usage-as-operator.md @@ -28,7 +28,12 @@ machineImages: - name: coreos version: 2135.6.0 image: coreos-2135.6.0 -keystoneURL: https://url-to-keystone/v3/ +# keystoneURL: https://url-to-keystone/v3/ +# keystoneURLs: +# - region: europe +# url: https://europe.example.com/v3/ +# - region: asia +# url: https://asia.example.com/v3/ # dnsServers: # - 10.10.10.11 # - 10.10.10.12 @@ -36,6 +41,7 @@ keystoneURL: https://url-to-keystone/v3/ constraints: floatingPools: - name: fp-pool-1 +# region: europe # loadBalancerClasses: # - name: lb-class-1 # floatingSubnetID: "1234" @@ -43,8 +49,18 @@ constraints: # subnetID: "7890" loadBalancerProviders: - name: haproxy +# region: europe +# - name: f5 +# region: asia ``` +Please note that it is possible to configure a region mapping for keystone URLs, floating pools, and load balancer providers. +The default behavior is that, if found, the regional entry is taken. +If no entry for the given region exists then the fallback value is the first entry in the list without a `region` field (or the `keystoneURL` value for the keystone URLs). +Some OpenStack environments don't need these regional mappings, hence, the `region` and `keystoneURLs` fields are optional. +If your OpenStack environment only has regional values and it doesn't make sense to provide a (non-regional) fallback then simply +omit `keystoneURL` and always specify `region`. + ## Example `CloudProfile` manifest Please find below an example `CloudProfile` manifest: diff --git a/controllers/provider-openstack/hack/api-reference/api.md b/controllers/provider-openstack/hack/api-reference/api.md index 4ffe955ea..954f3b4b2 100644 --- a/controllers/provider-openstack/hack/api-reference/api.md +++ b/controllers/provider-openstack/hack/api-reference/api.md @@ -95,11 +95,26 @@ string +(Optional)

KeyStoneURL is the URL for auth{n,z} in OpenStack (pointing to KeyStone).

+keystoneURLs
+ + +[]KeyStoneURL + + + + +(Optional) +

KeyStoneURLs is a region-URL mapping for auth{n,z} in OpenStack (pointing to KeyStone).

+ + + + machineImages
@@ -420,6 +435,18 @@ string +region
+ +string + + + +(Optional) +

Region is the region name.

+ + + + loadBalancerClasses
 @@ -529,6 +556,47 @@ NodeStatus +

KeyStoneURL +

+

+(Appears on: +CloudProfileConfig) +

+

+

KeyStoneURL is a region-URL mapping for auth{n,z} in OpenStack (pointing to KeyStone).

+

+ + + + + + + + + + + + + + + + + +
FieldDescription
+region
+ +string + +		 +

Region is the name of the region.

+
+url
+ +string + +		 +

URL is the keystone URL.

+

LoadBalancerClass

@@ -625,6 +693,18 @@ string

Name is the name of the load balancer provider.

+ + +region
+ +string + + + +(Optional) +

Region is the region name.

+ +

MachineImage diff --git a/controllers/provider-openstack/pkg/apis/openstack/helper/helper.go b/controllers/provider-openstack/pkg/apis/openstack/helper/helper.go index afce2d7e3..0266d820a 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/helper/helper.go +++ b/controllers/provider-openstack/pkg/apis/openstack/helper/helper.go @@ -91,3 +91,20 @@ func FindImageFromCloudProfile(cloudProfileConfig *api.CloudProfileConfig, image return nil, fmt.Errorf("could not find an image for name %q in version %q for region %q", imageName, imageVersion, regionName) } + +// FindKeyStoneURL takes a list of keystone URLs and tries to find the first entry +// whose region matches with the given region. If no such entry is found then it tries to use the non-regional +// keystone URL. If this is not specified then an error will be returned. +func FindKeyStoneURL(keyStoneURLs []api.KeyStoneURL, keystoneURL, region string) (string, error) { + for _, keyStoneURL := range keyStoneURLs { + if keyStoneURL.Region == region { + return keyStoneURL.URL, nil + } + } + + if len(keystoneURL) > 0 { + return keystoneURL, nil + } + + return "", fmt.Errorf("cannot find keystone URL for region %q", region) +} diff --git a/controllers/provider-openstack/pkg/apis/openstack/helper/helper_test.go b/controllers/provider-openstack/pkg/apis/openstack/helper/helper_test.go index 369ea8d73..b6955ee36 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/helper/helper_test.go +++ b/controllers/provider-openstack/pkg/apis/openstack/helper/helper_test.go @@ -97,6 +97,26 @@ var _ = Describe("Helper", func() { Entry("profile region entry", makeProfileRegionMachineImages("ubuntu", "1", "image-1234", regionName), "ubuntu", "1", regionName, "image-1234"), Entry("profile region not found", makeProfileRegionMachineImages("ubuntu", "1", "image-1234", regionName+"x"), "ubuntu", "1", regionName, ""), ) + + DescribeTable("#FindKeyStoneURL", + func(keyStoneURLs []api.KeyStoneURL, keystoneURL, region, expectedKeyStoneURL string, expectErr bool) { + result, err := FindKeyStoneURL(keyStoneURLs, keystoneURL, region) + + if !expectErr { + Expect(result).To(Equal(expectedKeyStoneURL)) + Expect(err).NotTo(HaveOccurred()) + } else { + Expect(result).To(BeEmpty()) + Expect(err).To(HaveOccurred()) + } + }, + + Entry("list is nil", nil, "default", "europe", "default", false), + Entry("empty list", []api.KeyStoneURL{}, "default", "europe", "default", false), + Entry("region not found", []api.KeyStoneURL{{URL: "bar", Region: "asia"}}, "default", "europe", "default", false), + Entry("region exists", []api.KeyStoneURL{{URL: "bar", Region: "europe"}}, "default", "europe", "bar", false), + Entry("no default URL", []api.KeyStoneURL{{URL: "bar", Region: "europe"}}, "", "asia", "", true), + ) }) func makeProfileMachineImages(name, version, image string) []api.MachineImages { diff --git a/controllers/provider-openstack/pkg/apis/openstack/types_cloudprofile.go b/controllers/provider-openstack/pkg/apis/openstack/types_cloudprofile.go index 759887aa7..bcd3ab4c5 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/types_cloudprofile.go +++ b/controllers/provider-openstack/pkg/apis/openstack/types_cloudprofile.go @@ -33,6 +33,8 @@ type CloudProfileConfig struct { DHCPDomain *string // KeyStoneURL is the URL for auth{n,z} in OpenStack (pointing to KeyStone). KeyStoneURL string + // KeyStoneURLs is a region-URL mapping for auth{n,z} in OpenStack (pointing to KeyStone). + KeyStoneURLs []KeyStoneURL // MachineImages is the list of machine images that are understood by the controller. It maps // logical names and versions to provider-specific identifiers. MachineImages []MachineImages @@ -52,10 +54,20 @@ type Constraints struct { type FloatingPool struct { // Name is the name of the floating pool. Name string + // Region is the region name. + Region *string // LoadBalancerClasses contains a list of supported labeled load balancer network settings. LoadBalancerClasses []LoadBalancerClass } +// KeyStoneURL is a region-URL mapping for auth{n,z} in OpenStack (pointing to KeyStone). +type KeyStoneURL struct { + // Region is the name of the region. + Region string + // URL is the keystone URL. + URL string +} + // LoadBalancerClass defines a restricted network setting for generic LoadBalancer classes. type LoadBalancerClass struct { // Name is the name of the LB class @@ -73,6 +85,8 @@ type LoadBalancerClass struct { type LoadBalancerProvider struct { // Name is the name of the load balancer provider. Name string + // Region is the region name. + Region *string } // MachineImages is a mapping from logical names and versions to provider-specific identifiers. diff --git a/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/types_cloudprofile.go b/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/types_cloudprofile.go index d44fd6dcc..b56a99a93 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/types_cloudprofile.go +++ b/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/types_cloudprofile.go @@ -35,7 +35,11 @@ type CloudProfileConfig struct { // +optional DHCPDomain *string `json:"dhcpDomain,omitempty"` // KeyStoneURL is the URL for auth{n,z} in OpenStack (pointing to KeyStone). - KeyStoneURL string `json:"keystoneURL"` + // +optional + KeyStoneURL string `json:"keystoneURL,omitempty"` + // KeyStoneURLs is a region-URL mapping for auth{n,z} in OpenStack (pointing to KeyStone). + // +optional + KeyStoneURLs []KeyStoneURL `json:"keystoneURLs,omitempty"` // MachineImages is the list of machine images that are understood by the controller. It maps // logical names and versions to provider-specific identifiers. MachineImages []MachineImages `json:"machineImages"` @@ -56,11 +60,22 @@ type Constraints struct { type FloatingPool struct { // Name is the name of the floating pool. Name string `json:"name"` + // Region is the region name. + // +optional + Region *string `json:"region,omitempty"` // LoadBalancerClasses contains a list of supported labeled load balancer network settings. // +optional LoadBalancerClasses []LoadBalancerClass `json:"loadBalancerClasses,omitempty"` } +// KeyStoneURL is a region-URL mapping for auth{n,z} in OpenStack (pointing to KeyStone). +type KeyStoneURL struct { + // Region is the name of the region. + Region string `json:"region"` + // URL is the keystone URL. + URL string `json:"url"` +} + // LoadBalancerClass defines a restricted network setting for generic LoadBalancer classes. type LoadBalancerClass struct { // Name is the name of the LB class @@ -81,6 +96,9 @@ type LoadBalancerClass struct { type LoadBalancerProvider struct { // Name is the name of the load balancer provider. Name string `json:"name"` + // Region is the region name. + // +optional + Region *string `json:"region,omitempty"` } // MachineImages is a mapping from logical names and versions to provider-specific identifiers. diff --git a/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go b/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go index 755311fd7..67c6ceb00 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go +++ b/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go @@ -115,6 +115,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*KeyStoneURL)(nil), (*openstack.KeyStoneURL)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_KeyStoneURL_To_openstack_KeyStoneURL(a.(*KeyStoneURL), b.(*openstack.KeyStoneURL), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*openstack.KeyStoneURL)(nil), (*KeyStoneURL)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_openstack_KeyStoneURL_To_v1alpha1_KeyStoneURL(a.(*openstack.KeyStoneURL), b.(*KeyStoneURL), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*LoadBalancerClass)(nil), (*openstack.LoadBalancerClass)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha1_LoadBalancerClass_To_openstack_LoadBalancerClass(a.(*LoadBalancerClass), b.(*openstack.LoadBalancerClass), scope) }); err != nil { @@ -285,6 +295,7 @@ func autoConvert_v1alpha1_CloudProfileConfig_To_openstack_CloudProfileConfig(in out.DNSServers = *(*[]string)(unsafe.Pointer(&in.DNSServers)) out.DHCPDomain = (*string)(unsafe.Pointer(in.DHCPDomain)) out.KeyStoneURL = in.KeyStoneURL + out.KeyStoneURLs = *(*[]openstack.KeyStoneURL)(unsafe.Pointer(&in.KeyStoneURLs)) out.MachineImages = *(*[]openstack.MachineImages)(unsafe.Pointer(&in.MachineImages)) out.RequestTimeout = (*string)(unsafe.Pointer(in.RequestTimeout)) return nil @@ -302,6 +313,7 @@ func autoConvert_openstack_CloudProfileConfig_To_v1alpha1_CloudProfileConfig(in out.DNSServers = *(*[]string)(unsafe.Pointer(&in.DNSServers)) out.DHCPDomain = (*string)(unsafe.Pointer(in.DHCPDomain)) out.KeyStoneURL = in.KeyStoneURL + out.KeyStoneURLs = *(*[]KeyStoneURL)(unsafe.Pointer(&in.KeyStoneURLs)) out.MachineImages = *(*[]MachineImages)(unsafe.Pointer(&in.MachineImages)) out.RequestTimeout = (*string)(unsafe.Pointer(in.RequestTimeout)) return nil @@ -362,6 +374,7 @@ func Convert_openstack_ControlPlaneConfig_To_v1alpha1_ControlPlaneConfig(in *ope func autoConvert_v1alpha1_FloatingPool_To_openstack_FloatingPool(in *FloatingPool, out *openstack.FloatingPool, s conversion.Scope) error { out.Name = in.Name + out.Region = (*string)(unsafe.Pointer(in.Region)) out.LoadBalancerClasses = *(*[]openstack.LoadBalancerClass)(unsafe.Pointer(&in.LoadBalancerClasses)) return nil } @@ -373,6 +386,7 @@ func Convert_v1alpha1_FloatingPool_To_openstack_FloatingPool(in *FloatingPool, o func autoConvert_openstack_FloatingPool_To_v1alpha1_FloatingPool(in *openstack.FloatingPool, out *FloatingPool, s conversion.Scope) error { out.Name = in.Name + out.Region = (*string)(unsafe.Pointer(in.Region)) out.LoadBalancerClasses = *(*[]LoadBalancerClass)(unsafe.Pointer(&in.LoadBalancerClasses)) return nil } @@ -462,6 +476,28 @@ func Convert_openstack_InfrastructureStatus_To_v1alpha1_InfrastructureStatus(in return autoConvert_openstack_InfrastructureStatus_To_v1alpha1_InfrastructureStatus(in, out, s) } +func autoConvert_v1alpha1_KeyStoneURL_To_openstack_KeyStoneURL(in *KeyStoneURL, out *openstack.KeyStoneURL, s conversion.Scope) error { + out.Region = in.Region + out.URL = in.URL + return nil +} + +// Convert_v1alpha1_KeyStoneURL_To_openstack_KeyStoneURL is an autogenerated conversion function. +func Convert_v1alpha1_KeyStoneURL_To_openstack_KeyStoneURL(in *KeyStoneURL, out *openstack.KeyStoneURL, s conversion.Scope) error { + return autoConvert_v1alpha1_KeyStoneURL_To_openstack_KeyStoneURL(in, out, s) +} + +func autoConvert_openstack_KeyStoneURL_To_v1alpha1_KeyStoneURL(in *openstack.KeyStoneURL, out *KeyStoneURL, s conversion.Scope) error { + out.Region = in.Region + out.URL = in.URL + return nil +} + +// Convert_openstack_KeyStoneURL_To_v1alpha1_KeyStoneURL is an autogenerated conversion function. +func Convert_openstack_KeyStoneURL_To_v1alpha1_KeyStoneURL(in *openstack.KeyStoneURL, out *KeyStoneURL, s conversion.Scope) error { + return autoConvert_openstack_KeyStoneURL_To_v1alpha1_KeyStoneURL(in, out, s) +} + func autoConvert_v1alpha1_LoadBalancerClass_To_openstack_LoadBalancerClass(in *LoadBalancerClass, out *openstack.LoadBalancerClass, s conversion.Scope) error { out.Name = in.Name out.FloatingSubnetID = (*string)(unsafe.Pointer(in.FloatingSubnetID)) @@ -490,6 +526,7 @@ func Convert_openstack_LoadBalancerClass_To_v1alpha1_LoadBalancerClass(in *opens func autoConvert_v1alpha1_LoadBalancerProvider_To_openstack_LoadBalancerProvider(in *LoadBalancerProvider, out *openstack.LoadBalancerProvider, s conversion.Scope) error { out.Name = in.Name + out.Region = (*string)(unsafe.Pointer(in.Region)) return nil } @@ -500,6 +537,7 @@ func Convert_v1alpha1_LoadBalancerProvider_To_openstack_LoadBalancerProvider(in func autoConvert_openstack_LoadBalancerProvider_To_v1alpha1_LoadBalancerProvider(in *openstack.LoadBalancerProvider, out *LoadBalancerProvider, s conversion.Scope) error { out.Name = in.Name + out.Region = (*string)(unsafe.Pointer(in.Region)) return nil } diff --git a/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go b/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go index f5eefcdde..97913ddc1 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go +++ b/controllers/provider-openstack/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go @@ -62,6 +62,11 @@ func (in *CloudProfileConfig) DeepCopyInto(out *CloudProfileConfig) { *out = new(string) **out = **in } + if in.KeyStoneURLs != nil { + in, out := &in.KeyStoneURLs, &out.KeyStoneURLs + *out = make([]KeyStoneURL, len(*in)) + copy(*out, *in) + } if in.MachineImages != nil { in, out := &in.MachineImages, &out.MachineImages *out = make([]MachineImages, len(*in)) @@ -108,7 +113,9 @@ func (in *Constraints) DeepCopyInto(out *Constraints) { if in.LoadBalancerProviders != nil { in, out := &in.LoadBalancerProviders, &out.LoadBalancerProviders *out = make([]LoadBalancerProvider, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -163,6 +170,11 @@ func (in *ControlPlaneConfig) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FloatingPool) DeepCopyInto(out *FloatingPool) { *out = *in + if in.Region != nil { + in, out := &in.Region, &out.Region + *out = new(string) + **out = **in + } if in.LoadBalancerClasses != nil { in, out := &in.LoadBalancerClasses, &out.LoadBalancerClasses *out = make([]LoadBalancerClass, len(*in)) @@ -257,6 +269,22 @@ func (in *InfrastructureStatus) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *KeyStoneURL) DeepCopyInto(out *KeyStoneURL) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KeyStoneURL. +func (in *KeyStoneURL) DeepCopy() *KeyStoneURL { + if in == nil { + return nil + } + out := new(KeyStoneURL) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancerClass) DeepCopyInto(out *LoadBalancerClass) { *out = *in @@ -291,6 +319,11 @@ func (in *LoadBalancerClass) DeepCopy() *LoadBalancerClass { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancerProvider) DeepCopyInto(out *LoadBalancerProvider) { *out = *in + if in.Region != nil { + in, out := &in.Region, &out.Region + *out = new(string) + **out = **in + } return } diff --git a/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile.go b/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile.go index e1ef7cba7..069af9e77 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile.go +++ b/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile.go @@ -19,22 +19,35 @@ import ( "net" "time" - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" "k8s.io/apimachinery/pkg/util/validation/field" ) // ValidateCloudProfileConfig validates a CloudProfileConfig object. -func ValidateCloudProfileConfig(cloudProfile *apisopenstack.CloudProfileConfig) field.ErrorList { +func ValidateCloudProfileConfig(cloudProfile *api.CloudProfileConfig) field.ErrorList { allErrs := field.ErrorList{} floatingPoolPath := field.NewPath("constraints", "floatingPools") if len(cloudProfile.Constraints.FloatingPools) == 0 { allErrs = append(allErrs, field.Required(floatingPoolPath, "must provide at least one floating pool")) } + + regionsFound := map[string]struct{}{} for i, pool := range cloudProfile.Constraints.FloatingPools { + idxPath := floatingPoolPath.Index(i) if len(pool.Name) == 0 { - allErrs = append(allErrs, field.Required(floatingPoolPath.Index(i).Child("name"), "must provide a name")) + allErrs = append(allErrs, field.Required(idxPath.Child("name"), "must provide a name")) + } + + if pool.Region != nil { + if len(*pool.Region) == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("region"), "must provide a region if key is present")) + } + if _, ok := regionsFound[*pool.Region]; ok { + allErrs = append(allErrs, field.Duplicate(idxPath.Child("region"), *pool.Region)) + } + regionsFound[*pool.Region] = struct{}{} } } @@ -42,9 +55,23 @@ func ValidateCloudProfileConfig(cloudProfile *apisopenstack.CloudProfileConfig) if len(cloudProfile.Constraints.LoadBalancerProviders) == 0 { allErrs = append(allErrs, field.Required(loadBalancerProviderPath, "must provide at least one load balancer provider")) } + + regionsFound = map[string]struct{}{} for i, pool := range cloudProfile.Constraints.LoadBalancerProviders { + idxPath := loadBalancerProviderPath.Index(i) + if len(pool.Name) == 0 { - allErrs = append(allErrs, field.Required(loadBalancerProviderPath.Index(i).Child("name"), "must provide a name")) + allErrs = append(allErrs, field.Required(idxPath.Child("name"), "must provide a name")) + } + + if pool.Region != nil { + if len(*pool.Region) == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("region"), "must provide a region if key is present")) + } + if _, ok := regionsFound[*pool.Region]; ok { + allErrs = append(allErrs, field.Duplicate(idxPath.Child("region"), *pool.Region)) + } + regionsFound[*pool.Region] = struct{}{} } } @@ -74,10 +101,28 @@ func ValidateCloudProfileConfig(cloudProfile *apisopenstack.CloudProfileConfig) } } - if len(cloudProfile.KeyStoneURL) == 0 { + if len(cloudProfile.KeyStoneURL) == 0 && len(cloudProfile.KeyStoneURLs) == 0 { allErrs = append(allErrs, field.Required(field.NewPath("keyStoneURL"), "must provide the URL to KeyStone")) } + regionsFound = map[string]struct{}{} + for i, val := range cloudProfile.KeyStoneURLs { + idxPath := field.NewPath("keyStoneURLs").Index(i) + + if len(val.Region) == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("region"), "must provide a region")) + } + + if len(val.URL) == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("url"), "must provide an url")) + } + + if _, ok := regionsFound[val.Region]; ok { + allErrs = append(allErrs, field.Duplicate(idxPath.Child("region"), val.Region)) + } + regionsFound[val.Region] = struct{}{} + } + for i, ip := range cloudProfile.DNSServers { if net.ParseIP(ip) == nil { allErrs = append(allErrs, field.Invalid(field.NewPath("dnsServers").Index(i), ip, "must provide a valid IP")) diff --git a/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile_test.go b/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile_test.go index e803d7e48..f23c17d78 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile_test.go +++ b/controllers/provider-openstack/pkg/apis/openstack/validation/cloudprofile_test.go @@ -15,7 +15,7 @@ package validation_test import ( - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" . "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/validation" . "github.com/onsi/ginkgo" @@ -26,15 +26,15 @@ import ( var _ = Describe("CloudProfileConfig validation", func() { Describe("#ValidateCloudProfileConfig", func() { - var cloudProfileConfig *apisopenstack.CloudProfileConfig + var cloudProfileConfig *api.CloudProfileConfig BeforeEach(func() { - cloudProfileConfig = &apisopenstack.CloudProfileConfig{ - Constraints: apisopenstack.Constraints{ - FloatingPools: []apisopenstack.FloatingPool{ + cloudProfileConfig = &api.CloudProfileConfig{ + Constraints: api.Constraints{ + FloatingPools: []api.FloatingPool{ {Name: "MY-POOL"}, }, - LoadBalancerProviders: []apisopenstack.LoadBalancerProvider{ + LoadBalancerProviders: []api.LoadBalancerProvider{ {Name: "haproxy"}, }, }, @@ -43,10 +43,10 @@ var _ = Describe("CloudProfileConfig validation", func() { "5.6.7.8", }, KeyStoneURL: "http://url-to-keystone/v3", - MachineImages: []apisopenstack.MachineImages{ + MachineImages: []api.MachineImages{ { Name: "ubuntu", - Versions: []apisopenstack.MachineImageVersion{ + Versions: []api.MachineImageVersion{ { Version: "1.2.3", Image: "ubuntu-1.2.3", @@ -59,7 +59,7 @@ var _ = Describe("CloudProfileConfig validation", func() { Context("floating pools constraints", func() { It("should enforce that at least one pool has been defined", func() { - cloudProfileConfig.Constraints.FloatingPools = []apisopenstack.FloatingPool{} + cloudProfileConfig.Constraints.FloatingPools = []api.FloatingPool{} errorList := ValidateCloudProfileConfig(cloudProfileConfig) @@ -69,9 +69,12 @@ var _ = Describe("CloudProfileConfig validation", func() { })))) }) - It("should forbid unsupported providers", func() { - cloudProfileConfig.Constraints.FloatingPools = []apisopenstack.FloatingPool{ - {Name: ""}, + It("should forbid unsupported pools", func() { + cloudProfileConfig.Constraints.FloatingPools = []api.FloatingPool{ + { + Name: "", + Region: makeStringPointer(""), + }, } errorList := ValidateCloudProfileConfig(cloudProfileConfig) @@ -79,13 +82,36 @@ var _ = Describe("CloudProfileConfig validation", func() { Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeRequired), "Field": Equal("constraints.floatingPools[0].name"), + })), PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("constraints.floatingPools[0].region"), + })))) + }) + + It("should forbid duplicates regions in pools", func() { + cloudProfileConfig.Constraints.FloatingPools = []api.FloatingPool{ + { + Name: "foo", + Region: makeStringPointer("foo"), + }, + { + Name: "foo", + Region: makeStringPointer("foo"), + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeDuplicate), + "Field": Equal("constraints.floatingPools[1].region"), })))) }) }) Context("load balancer provider constraints", func() { It("should enforce that at least one provider has been defined", func() { - cloudProfileConfig.Constraints.LoadBalancerProviders = []apisopenstack.LoadBalancerProvider{} + cloudProfileConfig.Constraints.LoadBalancerProviders = []api.LoadBalancerProvider{} errorList := ValidateCloudProfileConfig(cloudProfileConfig) @@ -96,8 +122,11 @@ var _ = Describe("CloudProfileConfig validation", func() { }) It("should forbid unsupported providers", func() { - cloudProfileConfig.Constraints.LoadBalancerProviders = []apisopenstack.LoadBalancerProvider{ - {Name: ""}, + cloudProfileConfig.Constraints.LoadBalancerProviders = []api.LoadBalancerProvider{ + { + Name: "", + Region: makeStringPointer(""), + }, } errorList := ValidateCloudProfileConfig(cloudProfileConfig) @@ -105,6 +134,29 @@ var _ = Describe("CloudProfileConfig validation", func() { Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeRequired), "Field": Equal("constraints.loadBalancerProviders[0].name"), + })), PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("constraints.loadBalancerProviders[0].region"), + })))) + }) + + It("should forbid duplicates regions in providers", func() { + cloudProfileConfig.Constraints.LoadBalancerProviders = []api.LoadBalancerProvider{ + { + Name: "foo", + Region: makeStringPointer("foo"), + }, + { + Name: "foo", + Region: makeStringPointer("foo"), + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeDuplicate), + "Field": Equal("constraints.loadBalancerProviders[1].region"), })))) }) }) @@ -112,6 +164,7 @@ var _ = Describe("CloudProfileConfig validation", func() { Context("keystone url validation", func() { It("should forbid keystone urls with unsupported format", func() { cloudProfileConfig.KeyStoneURL = "" + cloudProfileConfig.KeyStoneURLs = nil errorList := ValidateCloudProfileConfig(cloudProfileConfig) @@ -120,6 +173,42 @@ var _ = Describe("CloudProfileConfig validation", func() { "Field": Equal("keyStoneURL"), })))) }) + + It("should forbid keystone urls with missing keys", func() { + cloudProfileConfig.KeyStoneURL = "" + cloudProfileConfig.KeyStoneURLs = []api.KeyStoneURL{{}} + + errorList := ValidateCloudProfileConfig(cloudProfileConfig) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("keyStoneURLs[0].region"), + })), PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("keyStoneURLs[0].url"), + })))) + }) + + It("should forbid duplicate regions for keystone urls", func() { + cloudProfileConfig.KeyStoneURL = "" + cloudProfileConfig.KeyStoneURLs = []api.KeyStoneURL{ + { + Region: "foo", + URL: "bar", + }, + { + Region: "foo", + URL: "bar", + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeDuplicate), + "Field": Equal("keyStoneURLs[1].region"), + })))) + }) }) Context("dns server validation", func() { @@ -163,7 +252,7 @@ var _ = Describe("CloudProfileConfig validation", func() { Context("machine image validation", func() { It("should enforce that at least one machine image has been defined", func() { - cloudProfileConfig.MachineImages = []apisopenstack.MachineImages{} + cloudProfileConfig.MachineImages = []api.MachineImages{} errorList := ValidateCloudProfileConfig(cloudProfileConfig) @@ -174,7 +263,7 @@ var _ = Describe("CloudProfileConfig validation", func() { }) It("should forbid unsupported machine image configuration", func() { - cloudProfileConfig.MachineImages = []apisopenstack.MachineImages{{}} + cloudProfileConfig.MachineImages = []api.MachineImages{{}} errorList := ValidateCloudProfileConfig(cloudProfileConfig) @@ -188,10 +277,10 @@ var _ = Describe("CloudProfileConfig validation", func() { }) It("should forbid unsupported machine image version configuration", func() { - cloudProfileConfig.MachineImages = []apisopenstack.MachineImages{ + cloudProfileConfig.MachineImages = []api.MachineImages{ { Name: "abc", - Versions: []apisopenstack.MachineImageVersion{{}}, + Versions: []api.MachineImageVersion{{}}, }, } diff --git a/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane.go b/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane.go index 289548968..84cdf970c 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane.go +++ b/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane.go @@ -15,7 +15,7 @@ package validation import ( - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -23,12 +23,12 @@ import ( ) // ValidateControlPlaneConfig validates a ControlPlaneConfig object. -func ValidateControlPlaneConfig(controlPlaneConfig *apisopenstack.ControlPlaneConfig, region string, regions []gardencorev1beta1.Region, constraints apisopenstack.Constraints) field.ErrorList { +func ValidateControlPlaneConfig(controlPlaneConfig *api.ControlPlaneConfig, region string, regions []gardencorev1beta1.Region, constraints api.Constraints) field.ErrorList { allErrs := field.ErrorList{} if len(controlPlaneConfig.LoadBalancerProvider) == 0 { allErrs = append(allErrs, field.Required(field.NewPath("loadBalancerProvider"), "must provide the name of a load balancer provider")) - } else if ok, validLoadBalancerProviders := validateLoadBalancerProviderConstraints(constraints.LoadBalancerProviders, controlPlaneConfig.LoadBalancerProvider, ""); !ok { + } else if ok, validLoadBalancerProviders := validateLoadBalancerProviderConstraints(constraints.LoadBalancerProviders, region, controlPlaneConfig.LoadBalancerProvider, ""); !ok { allErrs = append(allErrs, field.NotSupported(field.NewPath("loadBalancerProvider"), controlPlaneConfig.LoadBalancerProvider, validLoadBalancerProviders)) } @@ -42,7 +42,7 @@ func ValidateControlPlaneConfig(controlPlaneConfig *apisopenstack.ControlPlaneCo } // ValidateControlPlaneConfigUpdate validates a ControlPlaneConfig object. -func ValidateControlPlaneConfigUpdate(oldConfig, newConfig *apisopenstack.ControlPlaneConfig, region string, regions []gardencorev1beta1.Region, constraints apisopenstack.Constraints) field.ErrorList { +func ValidateControlPlaneConfigUpdate(oldConfig, newConfig *api.ControlPlaneConfig, region string, regions []gardencorev1beta1.Region, constraints api.Constraints) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.Zone, oldConfig.Zone, field.NewPath("zone"))...) @@ -50,16 +50,40 @@ func ValidateControlPlaneConfigUpdate(oldConfig, newConfig *apisopenstack.Contro return allErrs } -func validateLoadBalancerProviderConstraints(providers []apisopenstack.LoadBalancerProvider, provider, oldProvider string) (bool, []string) { +func validateLoadBalancerProviderConstraints(providers []api.LoadBalancerProvider, region, provider, oldProvider string) (bool, []string) { if provider == oldProvider { return true, nil } - validValues := []string{} + var ( + validValues = []string{} + fallback *api.LoadBalancerProvider + ) for _, p := range providers { - validValues = append(validValues, p.Name) - if p.Name == provider { + // store the first non-regional image for fallback value if no load balancer provider for the given + // region was found + if p.Region == nil && fallback == nil { + v := p + fallback = &v + continue + } + + // load balancer provider for the given region found, validate it + if p.Region != nil && *p.Region == region { + validValues = append(validValues, p.Name) + if p.Name == provider { + return true, nil + } else { + return false, validValues + } + } + } + + // no load balancer provider for the given region found yet, check if the non-regional fallback is used + if fallback != nil { + validValues = append(validValues, fallback.Name) + if fallback.Name == provider { return true, nil } } diff --git a/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane_test.go b/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane_test.go index 6d46b449c..eb64df157 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane_test.go +++ b/controllers/provider-openstack/pkg/apis/openstack/validation/controlplane_test.go @@ -15,7 +15,7 @@ package validation_test import ( - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" . "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/validation" gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" @@ -40,18 +40,18 @@ var _ = Describe("ControlPlaneConfig validation", func() { }, } - constraints = apisopenstack.Constraints{ - LoadBalancerProviders: []apisopenstack.LoadBalancerProvider{ + constraints = api.Constraints{ + LoadBalancerProviders: []api.LoadBalancerProvider{ {Name: lbProvider1}, }, } - controlPlane *apisopenstack.ControlPlaneConfig + controlPlane *api.ControlPlaneConfig ) Describe("#ValidateControlPlaneConfig", func() { BeforeEach(func() { - controlPlane = &apisopenstack.ControlPlaneConfig{ + controlPlane = &api.ControlPlaneConfig{ LoadBalancerProvider: lbProvider1, Zone: "some-zone", } @@ -83,6 +83,101 @@ var _ = Describe("ControlPlaneConfig validation", func() { })))) }) + It("should forbid using a load balancer provider for different region", func() { + differentRegion := "asia" + constraints := api.Constraints{ + LoadBalancerProviders: []api.LoadBalancerProvider{ + { + Name: lbProvider1, + Region: ®ion, + }, + { + Name: "other", + Region: &differentRegion, + }, + }, + } + regions := []gardencorev1beta1.Region{ + { + Name: differentRegion, + Zones: []gardencorev1beta1.AvailabilityZone{ + {Name: zone}, + }, + }, + } + controlPlane.LoadBalancerProvider = lbProvider1 + + errorList := ValidateControlPlaneConfig(controlPlane, differentRegion, regions, constraints) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeNotSupported), + "Field": Equal("loadBalancerProvider"), + })))) + }) + + It("should forbid using the non-regional load balancer provider name if region is specified", func() { + differentRegion := "asia" + lbProvider2 := "lb2" + + constraints := api.Constraints{ + LoadBalancerProviders: []api.LoadBalancerProvider{ + { + Name: lbProvider2, + }, + { + Name: lbProvider1, + Region: &differentRegion, + }, + }, + } + regions := []gardencorev1beta1.Region{ + { + Name: region, + Zones: []gardencorev1beta1.AvailabilityZone{ + {Name: zone}, + }, + }, + } + controlPlane.LoadBalancerProvider = lbProvider1 + + errorList := ValidateControlPlaneConfig(controlPlane, region, regions, constraints) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeNotSupported), + "Field": Equal("loadBalancerProvider"), + })))) + }) + + It("should allow using the non-regional load balancer provider name if region not specified", func() { + differentRegion := "asia" + lbProvider2 := "lb2" + + constraints := api.Constraints{ + LoadBalancerProviders: []api.LoadBalancerProvider{ + { + Name: lbProvider2, + }, + { + Name: lbProvider1, + Region: ®ion, + }, + }, + } + regions := []gardencorev1beta1.Region{ + { + Name: differentRegion, + Zones: []gardencorev1beta1.AvailabilityZone{ + {Name: zone}, + }, + }, + } + controlPlane.LoadBalancerProvider = lbProvider2 + + errorList := ValidateControlPlaneConfig(controlPlane, differentRegion, regions, constraints) + + Expect(errorList).To(BeEmpty()) + }) + It("should require the name of a zone", func() { controlPlane.Zone = "" diff --git a/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure.go b/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure.go index 31657cbfa..53d7292ec 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure.go +++ b/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure.go @@ -15,7 +15,7 @@ package validation import ( - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" cidrvalidation "github.com/gardener/gardener/pkg/utils/validation/cidr" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -23,12 +23,12 @@ import ( ) // ValidateInfrastructureConfig validates a InfrastructureConfig object. -func ValidateInfrastructureConfig(infra *apisopenstack.InfrastructureConfig, constraints apisopenstack.Constraints, nodesCIDR *string) field.ErrorList { +func ValidateInfrastructureConfig(infra *api.InfrastructureConfig, constraints api.Constraints, region string, nodesCIDR *string) field.ErrorList { allErrs := field.ErrorList{} if len(infra.FloatingPoolName) == 0 { allErrs = append(allErrs, field.Required(field.NewPath("floatingPoolName"), "must provide the name of a floating pool")) - } else if ok, validFloatingPoolNames := validateFloatingPoolNameConstraints(constraints.FloatingPools, infra.FloatingPoolName, ""); !ok { + } else if ok, validFloatingPoolNames := validateFloatingPoolNameConstraints(constraints.FloatingPools, region, infra.FloatingPoolName, ""); !ok { allErrs = append(allErrs, field.NotSupported(field.NewPath("floatingPoolName"), infra.FloatingPoolName, validFloatingPoolNames)) } @@ -58,7 +58,7 @@ func ValidateInfrastructureConfig(infra *apisopenstack.InfrastructureConfig, con } // ValidateInfrastructureConfigUpdate validates a InfrastructureConfig object. -func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *apisopenstack.InfrastructureConfig, constraints apisopenstack.Constraints, nodesCIDR *string) field.ErrorList { +func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *api.InfrastructureConfig, constraints api.Constraints, nodesCIDR *string) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, apivalidation.ValidateImmutableField(newConfig.Networks, oldConfig.Networks, field.NewPath("networks"))...) @@ -66,16 +66,40 @@ func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *apisopenstack.Infr return allErrs } -func validateFloatingPoolNameConstraints(names []apisopenstack.FloatingPool, name, oldName string) (bool, []string) { +func validateFloatingPoolNameConstraints(names []api.FloatingPool, region string, name, oldName string) (bool, []string) { if name == oldName { return true, nil } - validValues := []string{} + var ( + validValues = []string{} + fallback *api.FloatingPool + ) for _, n := range names { - validValues = append(validValues, n.Name) - if n.Name == name { + // store the first non-regional image for fallback value if no floating pool for the given + // region was found + if n.Region == nil && fallback == nil { + v := n + fallback = &v + continue + } + + // floating pool for the given region found, validate it + if n.Region != nil && *n.Region == region { + validValues = append(validValues, n.Name) + if n.Name == name { + return true, nil + } else { + return false, validValues + } + } + } + + // no floating pool for the given region found yet, check if the non-regional fallback is used + if fallback != nil { + validValues = append(validValues, fallback.Name) + if fallback.Name == name { return true, nil } } diff --git a/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure_test.go b/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure_test.go index 36d07fc20..ac31d50b7 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure_test.go +++ b/controllers/provider-openstack/pkg/apis/openstack/validation/infrastructure_test.go @@ -15,7 +15,7 @@ package validation_test import ( - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" . "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/validation" . "github.com/gardener/gardener/pkg/utils/validation/gomega" @@ -28,24 +28,25 @@ import ( var _ = Describe("InfrastructureConfig validation", func() { var ( floatingPoolName1 = "foo" + region = "europe" - constraints = apisopenstack.Constraints{ - FloatingPools: []apisopenstack.FloatingPool{ + constraints = api.Constraints{ + FloatingPools: []api.FloatingPool{ {Name: floatingPoolName1}, }, } - infrastructureConfig *apisopenstack.InfrastructureConfig + infrastructureConfig *api.InfrastructureConfig nodes = "10.250.0.0/16" invalidCIDR = "invalid-cidr" ) BeforeEach(func() { - infrastructureConfig = &apisopenstack.InfrastructureConfig{ + infrastructureConfig = &api.InfrastructureConfig{ FloatingPoolName: floatingPoolName1, - Networks: apisopenstack.Networks{ - Router: &apisopenstack.Router{ + Networks: api.Networks{ + Router: &api.Router{ ID: "hugo", }, Worker: "10.250.0.0/16", @@ -57,7 +58,7 @@ var _ = Describe("InfrastructureConfig validation", func() { It("should forbid invalid floating pool name configuration", func() { infrastructureConfig.FloatingPoolName = "" - errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, &nodes) + errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, region, &nodes) Expect(errorList).To(ConsistOfFields(Fields{ "Type": Equal(field.ErrorTypeRequired), @@ -65,11 +66,81 @@ var _ = Describe("InfrastructureConfig validation", func() { })) }) + It("should forbid using a floating pool name for different region", func() { + differentRegion := "asia" + constraints := api.Constraints{ + FloatingPools: []api.FloatingPool{ + { + Name: floatingPoolName1, + Region: ®ion, + }, + { + Name: "other", + Region: &differentRegion, + }, + }, + } + infrastructureConfig.FloatingPoolName = floatingPoolName1 + + errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, differentRegion, &nodes) + + Expect(errorList).To(ConsistOfFields(Fields{ + "Type": Equal(field.ErrorTypeNotSupported), + "Field": Equal("floatingPoolName"), + })) + }) + + It("should forbid using the non-regional floating pool name if region is specified", func() { + floatingPoolName2 := "fp2" + + constraints := api.Constraints{ + FloatingPools: []api.FloatingPool{ + { + Name: floatingPoolName2, + }, + { + Name: floatingPoolName1, + Region: ®ion, + }, + }, + } + infrastructureConfig.FloatingPoolName = floatingPoolName2 + + errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, region, &nodes) + + Expect(errorList).To(ConsistOfFields(Fields{ + "Type": Equal(field.ErrorTypeNotSupported), + "Field": Equal("floatingPoolName"), + })) + }) + + It("should allow using the non-regional floating pool name if region not specified", func() { + differentRegion := "asia" + floatingPoolName2 := "fp2" + + constraints := api.Constraints{ + FloatingPools: []api.FloatingPool{ + { + Name: floatingPoolName2, + }, + { + Name: floatingPoolName1, + Region: ®ion, + }, + }, + } + infrastructureConfig.FloatingPoolName = floatingPoolName2 + + errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, differentRegion, &nodes) + + Expect(errorList).To(BeEmpty()) + }) + Context("CIDR", func() { It("should forbid invalid workers CIDR", func() { infrastructureConfig.Networks.Worker = invalidCIDR - errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, &nodes) + errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, region, &nodes) Expect(errorList).To(ConsistOfFields(Fields{ "Type": Equal(field.ErrorTypeInvalid), @@ -81,7 +152,7 @@ var _ = Describe("InfrastructureConfig validation", func() { It("should forbid workers CIDR which are not in Nodes CIDR", func() { infrastructureConfig.Networks.Worker = "1.1.1.1/32" - errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, &nodes) + errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, region, &nodes) Expect(errorList).To(ConsistOfFields(Fields{ "Type": Equal(field.ErrorTypeInvalid), @@ -95,7 +166,7 @@ var _ = Describe("InfrastructureConfig validation", func() { infrastructureConfig.Networks.Worker = "10.250.3.8/24" - errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, &nodeCIDR) + errorList := ValidateInfrastructureConfig(infrastructureConfig, constraints, region, &nodeCIDR) Expect(errorList).To(HaveLen(1)) Expect(errorList).To(ConsistOfFields(Fields{ @@ -114,7 +185,7 @@ var _ = Describe("InfrastructureConfig validation", func() { It("should forbid changing the network section", func() { newInfrastructureConfig := infrastructureConfig.DeepCopy() - newInfrastructureConfig.Networks.Router = &apisopenstack.Router{ID: "name"} + newInfrastructureConfig.Networks.Router = &api.Router{ID: "name"} errorList := ValidateInfrastructureConfigUpdate(infrastructureConfig, newInfrastructureConfig, constraints, &nodes) diff --git a/controllers/provider-openstack/pkg/apis/openstack/zz_generated.deepcopy.go b/controllers/provider-openstack/pkg/apis/openstack/zz_generated.deepcopy.go index 6d4b0c5cc..5dd6f856c 100644 --- a/controllers/provider-openstack/pkg/apis/openstack/zz_generated.deepcopy.go +++ b/controllers/provider-openstack/pkg/apis/openstack/zz_generated.deepcopy.go @@ -62,6 +62,11 @@ func (in *CloudProfileConfig) DeepCopyInto(out *CloudProfileConfig) { *out = new(string) **out = **in } + if in.KeyStoneURLs != nil { + in, out := &in.KeyStoneURLs, &out.KeyStoneURLs + *out = make([]KeyStoneURL, len(*in)) + copy(*out, *in) + } if in.MachineImages != nil { in, out := &in.MachineImages, &out.MachineImages *out = make([]MachineImages, len(*in)) @@ -108,7 +113,9 @@ func (in *Constraints) DeepCopyInto(out *Constraints) { if in.LoadBalancerProviders != nil { in, out := &in.LoadBalancerProviders, &out.LoadBalancerProviders *out = make([]LoadBalancerProvider, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -163,6 +170,11 @@ func (in *ControlPlaneConfig) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FloatingPool) DeepCopyInto(out *FloatingPool) { *out = *in + if in.Region != nil { + in, out := &in.Region, &out.Region + *out = new(string) + **out = **in + } if in.LoadBalancerClasses != nil { in, out := &in.LoadBalancerClasses, &out.LoadBalancerClasses *out = make([]LoadBalancerClass, len(*in)) @@ -257,6 +269,22 @@ func (in *InfrastructureStatus) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *KeyStoneURL) DeepCopyInto(out *KeyStoneURL) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KeyStoneURL. +func (in *KeyStoneURL) DeepCopy() *KeyStoneURL { + if in == nil { + return nil + } + out := new(KeyStoneURL) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancerClass) DeepCopyInto(out *LoadBalancerClass) { *out = *in @@ -291,6 +319,11 @@ func (in *LoadBalancerClass) DeepCopy() *LoadBalancerClass { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancerProvider) DeepCopyInto(out *LoadBalancerProvider) { *out = *in + if in.Region != nil { + in, out := &in.Region, &out.Region + *out = new(string) + **out = **in + } return } diff --git a/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider.go b/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider.go index e841ea2b1..49367159d 100644 --- a/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider.go +++ b/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider.go @@ -16,9 +16,10 @@ package controlplane import ( "context" + "fmt" "path/filepath" - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/helper" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/internal" openstacktypes "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/openstack" @@ -135,14 +136,14 @@ func (vp *valuesProvider) GetConfigChartValues( cp *extensionsv1alpha1.ControlPlane, cluster *extensionscontroller.Cluster, ) (map[string]interface{}, error) { - cpConfig := &apisopenstack.ControlPlaneConfig{} + cpConfig := &api.ControlPlaneConfig{} if cp.Spec.ProviderConfig != nil { if _, _, err := vp.Decoder().Decode(cp.Spec.ProviderConfig.Raw, nil, cpConfig); err != nil { return nil, errors.Wrapf(err, "could not decode providerConfig of controlplane '%s'", util.ObjectName(cp)) } } - infraStatus := &apisopenstack.InfrastructureStatus{} + infraStatus := &api.InfrastructureStatus{} if _, _, err := vp.Decoder().Decode(cp.Spec.InfrastructureProviderStatus.Raw, nil, infraStatus); err != nil { return nil, errors.Wrapf(err, "could not decode infrastructureProviderStatus of controlplane '%s'", util.ObjectName(cp)) } @@ -171,7 +172,7 @@ func (vp *valuesProvider) GetControlPlaneChartValues( scaledDown bool, ) (map[string]interface{}, error) { // Decode providerConfig - cpConfig := &apisopenstack.ControlPlaneConfig{} + cpConfig := &api.ControlPlaneConfig{} if cp.Spec.ProviderConfig != nil { if _, _, err := vp.Decoder().Decode(cp.Spec.ProviderConfig.Raw, nil, cpConfig); err != nil { return nil, errors.Wrapf(err, "could not decode providerConfig of controlplane '%s'", util.ObjectName(cp)) @@ -189,7 +190,7 @@ func (vp *valuesProvider) GetStorageClassesChartValues( cluster *extensionscontroller.Cluster, ) (map[string]interface{}, error) { // Decode providerConfig - cpConfig := &apisopenstack.ControlPlaneConfig{} + cpConfig := &api.ControlPlaneConfig{} if cp.Spec.ProviderConfig != nil { if _, _, err := vp.Decoder().Decode(cp.Spec.ProviderConfig.Raw, nil, cpConfig); err != nil { return nil, errors.Wrapf(err, "could not decode providerConfig of controlplane '%s'", util.ObjectName(cp)) @@ -203,15 +204,15 @@ func (vp *valuesProvider) GetStorageClassesChartValues( // getConfigChartValues collects and returns the configuration chart values. func getConfigChartValues( - cpConfig *apisopenstack.ControlPlaneConfig, - infraStatus *apisopenstack.InfrastructureStatus, - cloudProfileConfig *apisopenstack.CloudProfileConfig, + cpConfig *api.ControlPlaneConfig, + infraStatus *api.InfrastructureStatus, + cloudProfileConfig *api.CloudProfileConfig, cp *extensionsv1alpha1.ControlPlane, c *internal.Credentials, cluster *extensionscontroller.Cluster, ) (map[string]interface{}, error) { // Get the first subnet with purpose "nodes" - subnet, err := helper.FindSubnetByPurpose(infraStatus.Networks.Subnets, apisopenstack.PurposeNodes) + subnet, err := helper.FindSubnetByPurpose(infraStatus.Networks.Subnets, api.PurposeNodes) if err != nil { return nil, errors.Wrapf(err, "could not determine subnet from infrastructureProviderStatus of controlplane '%s'", util.ObjectName(cp)) } @@ -222,10 +223,16 @@ func getConfigChartValues( requestTimeout *string ) - if cloudProfileConfig != nil { - keyStoneURL = cloudProfileConfig.KeyStoneURL - dhcpDomain = cloudProfileConfig.DHCPDomain - requestTimeout = cloudProfileConfig.RequestTimeout + if cloudProfileConfig == nil { + return nil, fmt.Errorf("cloud profile config is nil - cannot determine keystone URL and other parameters") + } + + dhcpDomain = cloudProfileConfig.DHCPDomain + requestTimeout = cloudProfileConfig.RequestTimeout + + keyStoneURL, err = helper.FindKeyStoneURL(cloudProfileConfig.KeyStoneURLs, cloudProfileConfig.KeyStoneURL, cp.Spec.Region) + if err != nil { + return nil, err } // Collect config chart values @@ -244,18 +251,27 @@ func getConfigChartValues( } if cpConfig.LoadBalancerClasses == nil { - if cloudProfileConfig != nil { - for _, pool := range cloudProfileConfig.Constraints.FloatingPools { - if pool.Name == infraStatus.Networks.FloatingPool.Name { - cpConfig.LoadBalancerClasses = pool.LoadBalancerClasses - break - } + var fallback *api.FloatingPool + + for _, pool := range cloudProfileConfig.Constraints.FloatingPools { + if pool.Region == nil && fallback == nil { + v := pool + fallback = &v + } + + if pool.Region != nil && *pool.Region == cp.Spec.Region && pool.Name == infraStatus.Networks.FloatingPool.Name { + cpConfig.LoadBalancerClasses = pool.LoadBalancerClasses + break } } + + if cpConfig.LoadBalancerClasses == nil && fallback != nil { + cpConfig.LoadBalancerClasses = fallback.LoadBalancerClasses + } } for _, class := range cpConfig.LoadBalancerClasses { - if class.Name == apisopenstack.DefaultLoadBalancerClass { + if class.Name == api.DefaultLoadBalancerClass { utils.SetStringValue(values, "floatingNetworkID", class.FloatingNetworkID) utils.SetStringValue(values, "floatingSubnetID", class.FloatingSubnetID) utils.SetStringValue(values, "subnetID", class.SubnetID) @@ -263,7 +279,7 @@ func getConfigChartValues( } } for _, class := range cpConfig.LoadBalancerClasses { - if class.Name == apisopenstack.PrivateLoadBalancerClass { + if class.Name == api.PrivateLoadBalancerClass { utils.SetStringValue(values, "subnetID", class.SubnetID) break } @@ -292,7 +308,7 @@ func getConfigChartValues( // getCCMChartValues collects and returns the CCM chart values. func getCCMChartValues( - cpConfig *apisopenstack.ControlPlaneConfig, + cpConfig *api.ControlPlaneConfig, cp *extensionsv1alpha1.ControlPlane, cluster *extensionscontroller.Cluster, checksums map[string]string, diff --git a/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider_test.go b/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider_test.go index cf2a2949a..5c8a933a8 100644 --- a/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider_test.go +++ b/controllers/provider-openstack/pkg/controller/controlplane/valuesprovider_test.go @@ -18,8 +18,8 @@ import ( "context" "encoding/json" - "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" - openstacktypes "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/openstack" extensionscontroller "github.com/gardener/gardener-extensions/pkg/controller" mockclient "github.com/gardener/gardener-extensions/pkg/mock/controller-runtime/client" "github.com/gardener/gardener-extensions/pkg/util" @@ -49,16 +49,17 @@ var requestTimeout = util.StringPtr("2s") func defaultControlPlane() *extensionsv1alpha1.ControlPlane { return controlPlane( "floating-network-id", - &openstack.ControlPlaneConfig{ + &api.ControlPlaneConfig{ LoadBalancerProvider: "load-balancer-provider", - CloudControllerManager: &openstack.CloudControllerManagerConfig{ + CloudControllerManager: &api.CloudControllerManagerConfig{ FeatureGates: map[string]bool{ "CustomResourceValidation": true, }, }, }) } -func controlPlane(floatingPoolID string, cfg *openstack.ControlPlaneConfig) *extensionsv1alpha1.ControlPlane { + +func controlPlane(floatingPoolID string, cfg *api.ControlPlaneConfig) *extensionsv1alpha1.ControlPlane { return &extensionsv1alpha1.ControlPlane{ ObjectMeta: metav1.ObjectMeta{ Name: "control-plane", @@ -73,15 +74,15 @@ func controlPlane(floatingPoolID string, cfg *openstack.ControlPlaneConfig) *ext Raw: encode(cfg), }, InfrastructureProviderStatus: &runtime.RawExtension{ - Raw: encode(&openstack.InfrastructureStatus{ - Networks: openstack.NetworkStatus{ - FloatingPool: openstack.FloatingPoolStatus{ + Raw: encode(&api.InfrastructureStatus{ + Networks: api.NetworkStatus{ + FloatingPool: api.FloatingPoolStatus{ ID: floatingPoolID, }, - Subnets: []openstack.Subnet{ + Subnets: []api.Subnet{ { ID: "subnet-acbd1234", - Purpose: openstack.PurposeNodes, + Purpose: api.PurposeNodes, }, }, }, @@ -97,12 +98,12 @@ var _ = Describe("ValuesProvider", func() { // Build scheme scheme = runtime.NewScheme() - _ = openstack.AddToScheme(scheme) + _ = api.AddToScheme(scheme) cp = defaultControlPlane() cidr = "10.250.0.0/19" - cloudProfileConfig = &openstack.CloudProfileConfig{ + cloudProfileConfig = &api.CloudProfileConfig{ KeyStoneURL: authURL, DHCPDomain: dhcpDomain, RequestTimeout: requestTimeout, @@ -146,10 +147,10 @@ var _ = Describe("ValuesProvider", func() { } checksums = map[string]string{ - v1beta1constants.SecretNameCloudProvider: "8bafb35ff1ac60275d62e1cbd495aceb511fb354f74a20f7d06ecb48b3a68432", - "cloud-controller-manager": "3d791b164a808638da9a8df03924be2a41e34cd664e42231c00fe369e3588272", - "cloud-controller-manager-server": "6dff2a2e6f14444b66d8e4a351c049f7e89ee24ba3eaab95dbec40ba6bdebb52", - openstacktypes.CloudProviderConfigCloudControllerManagerName: "08a7bc7fe8f59b055f173145e211760a83f02cf89635cef26ebb351378635606", + v1beta1constants.SecretNameCloudProvider: "8bafb35ff1ac60275d62e1cbd495aceb511fb354f74a20f7d06ecb48b3a68432", + "cloud-controller-manager": "3d791b164a808638da9a8df03924be2a41e34cd664e42231c00fe369e3588272", + "cloud-controller-manager-server": "6dff2a2e6f14444b66d8e4a351c049f7e89ee24ba3eaab95dbec40ba6bdebb52", + openstack.CloudProviderConfigCloudControllerManagerName: "08a7bc7fe8f59b055f173145e211760a83f02cf89635cef26ebb351378635606", } configChartValues = map[string]interface{}{ @@ -240,17 +241,17 @@ var _ = Describe("ValuesProvider", func() { err = vp.(inject.Client).InjectClient(client) Expect(err).NotTo(HaveOccurred()) - fnid := "4711" - fnid2 := "pub" + floatingNetworkID := "4711" + floatingNetworkID2 := "pub" fsid := "0815" - fsid2 := "pub0815" - psid := "priv" - dfsid := "default-floating-subnet-id" + floatingSubnetID2 := "pub0815" + subnetID := "priv" + floatingSubnetID := "default-floating-subnet-id" cp := controlPlane( - fnid, - &openstack.ControlPlaneConfig{ + floatingNetworkID, + &api.ControlPlaneConfig{ LoadBalancerProvider: "load-balancer-provider", - LoadBalancerClasses: []openstack.LoadBalancerClass{ + LoadBalancerClasses: []api.LoadBalancerClass{ { Name: "test", FloatingSubnetID: &fsid, @@ -258,21 +259,21 @@ var _ = Describe("ValuesProvider", func() { }, { Name: "default", - FloatingSubnetID: &dfsid, + FloatingSubnetID: &floatingSubnetID, SubnetID: nil, }, { Name: "public", - FloatingSubnetID: &fsid2, - FloatingNetworkID: &fnid2, + FloatingSubnetID: &floatingSubnetID2, + FloatingNetworkID: &floatingNetworkID2, SubnetID: nil, }, { Name: "other", - SubnetID: &psid, + SubnetID: &subnetID, }, }, - CloudControllerManager: &openstack.CloudControllerManagerConfig{ + CloudControllerManager: &api.CloudControllerManagerConfig{ FeatureGates: map[string]bool{ "CustomResourceValidation": true, }, @@ -288,27 +289,27 @@ var _ = Describe("ValuesProvider", func() { "password": "password", "subnetID": "subnet-acbd1234", "lbProvider": "load-balancer-provider", - "floatingNetworkID": fnid, - "floatingSubnetID": dfsid, + "floatingNetworkID": floatingNetworkID, + "floatingSubnetID": floatingSubnetID, "floatingClasses": []map[string]interface{}{ { "name": "test", - "floatingNetworkID": fnid, + "floatingNetworkID": floatingNetworkID, "floatingSubnetID": fsid, }, { "name": "default", - "floatingNetworkID": fnid, - "floatingSubnetID": dfsid, + "floatingNetworkID": floatingNetworkID, + "floatingSubnetID": floatingSubnetID, }, { "name": "public", - "floatingNetworkID": fnid2, - "floatingSubnetID": fsid2, + "floatingNetworkID": floatingNetworkID2, + "floatingSubnetID": floatingSubnetID2, }, { "name": "other", - "subnetID": psid, + "subnetID": subnetID, }, }, "authUrl": authURL, diff --git a/controllers/provider-openstack/pkg/controller/infrastructure/actuator.go b/controllers/provider-openstack/pkg/controller/infrastructure/actuator.go index eef091031..72139b801 100644 --- a/controllers/provider-openstack/pkg/controller/infrastructure/actuator.go +++ b/controllers/provider-openstack/pkg/controller/infrastructure/actuator.go @@ -17,21 +17,17 @@ package infrastructure import ( "context" - "github.com/gardener/gardener-extensions/pkg/controller/common" - "k8s.io/client-go/util/retry" - api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" infrainternal "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/internal/infrastructure" extensionscontroller "github.com/gardener/gardener-extensions/pkg/controller" + "github.com/gardener/gardener-extensions/pkg/controller/common" "github.com/gardener/gardener-extensions/pkg/controller/infrastructure" "github.com/gardener/gardener-extensions/pkg/terraformer" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/runtime" - + "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/log" ) diff --git a/controllers/provider-openstack/pkg/controller/worker/machines.go b/controllers/provider-openstack/pkg/controller/worker/machines.go index 8476943a0..f7849379c 100644 --- a/controllers/provider-openstack/pkg/controller/worker/machines.go +++ b/controllers/provider-openstack/pkg/controller/worker/machines.go @@ -19,10 +19,8 @@ import ( "fmt" "path/filepath" - apisopenstack "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" - openstackapi "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" + api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/helper" - openstackapihelper "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/helper" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/internal" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/openstack" extensionscontroller "github.com/gardener/gardener-extensions/pkg/controller" @@ -73,8 +71,13 @@ func (w *workerDelegate) generateMachineClassSecretData(ctx context.Context) (ma return nil, err } + keyStoneURL, err := helper.FindKeyStoneURL(cloudProfileConfig.KeyStoneURLs, cloudProfileConfig.KeyStoneURL, w.worker.Spec.Region) + if err != nil { + return nil, err + } + return map[string][]byte{ - machinev1alpha1.OpenStackAuthURL: []byte(cloudProfileConfig.KeyStoneURL), + machinev1alpha1.OpenStackAuthURL: []byte(keyStoneURL), machinev1alpha1.OpenStackInsecure: []byte("true"), machinev1alpha1.OpenStackDomainName: []byte(credentials.DomainName), machinev1alpha1.OpenStackTenantName: []byte(credentials.TenantName), @@ -87,7 +90,7 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { var ( machineDeployments = worker.MachineDeployments{} machineClasses []map[string]interface{} - machineImages []apisopenstack.MachineImage + machineImages []api.MachineImage ) machineClassSecretData, err := w.generateMachineClassSecretData(ctx) @@ -95,12 +98,12 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { return err } - infrastructureStatus := &openstackapi.InfrastructureStatus{} + infrastructureStatus := &api.InfrastructureStatus{} if _, _, err := w.Decoder().Decode(w.worker.Spec.InfrastructureProviderStatus.Raw, nil, infrastructureStatus); err != nil { return err } - nodesSecurityGroup, err := openstackapihelper.FindSecurityGroupByPurpose(infrastructureStatus.SecurityGroups, openstackapi.PurposeNodes) + nodesSecurityGroup, err := helper.FindSecurityGroupByPurpose(infrastructureStatus.SecurityGroups, api.PurposeNodes) if err != nil { return err } diff --git a/controllers/provider-openstack/pkg/controller/worker/machines_test.go b/controllers/provider-openstack/pkg/controller/worker/machines_test.go index ca46dd5bf..6c95f97ca 100644 --- a/controllers/provider-openstack/pkg/controller/worker/machines_test.go +++ b/controllers/provider-openstack/pkg/controller/worker/machines_test.go @@ -18,7 +18,6 @@ import ( "context" "encoding/json" "fmt" - "github.com/gardener/gardener-extensions/pkg/controller/common" "path/filepath" api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" @@ -26,6 +25,7 @@ import ( . "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/controller/worker" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/openstack" extensionscontroller "github.com/gardener/gardener-extensions/pkg/controller" + "github.com/gardener/gardener-extensions/pkg/controller/common" "github.com/gardener/gardener-extensions/pkg/controller/worker" mockclient "github.com/gardener/gardener-extensions/pkg/mock/controller-runtime/client" mockkubernetes "github.com/gardener/gardener-extensions/pkg/mock/gardener/client/kubernetes" @@ -124,6 +124,8 @@ var _ = Describe("Machines", func() { shootVersion string scheme *runtime.Scheme decoder runtime.Decoder + cloudProfileConfig *api.CloudProfileConfig + cloudProfileConfigJSON []byte clusterWithoutImages *extensionscontroller.Cluster cluster *extensionscontroller.Cluster w *extensionsv1alpha1.Worker @@ -172,14 +174,15 @@ var _ = Describe("Machines", func() { shootVersionMajorMinor = "1.2" shootVersion = shootVersionMajorMinor + ".3" - cloudProfileConfig := &apiv1alpha1.CloudProfileConfig{ + cloudProfileConfig = &api.CloudProfileConfig{ TypeMeta: metav1.TypeMeta{ - APIVersion: apiv1alpha1.SchemeGroupVersion.String(), + APIVersion: api.SchemeGroupVersion.String(), Kind: "CloudProfileConfig", }, KeyStoneURL: openstackAuthURL, } - cloudProfileConfigJSON, _ := json.Marshal(cloudProfileConfig) + cloudProfileConfigJSON, _ = json.Marshal(cloudProfileConfig) + clusterWithoutImages = &extensionscontroller.Cluster{ CloudProfile: &gardencorev1beta1.CloudProfile{ ObjectMeta: metav1.ObjectMeta{ @@ -205,14 +208,14 @@ var _ = Describe("Machines", func() { }, } - cloudProfileConfig.MachineImages = []apiv1alpha1.MachineImages{ - apiv1alpha1.MachineImages{ + cloudProfileConfig.MachineImages = []api.MachineImages{ + api.MachineImages{ Name: machineImageName, - Versions: []apiv1alpha1.MachineImageVersion{ - apiv1alpha1.MachineImageVersion{ + Versions: []api.MachineImageVersion{ + api.MachineImageVersion{ Version: machineImageVersion, Image: machineImage, - Regions: []apiv1alpha1.RegionIDMapping{ + Regions: []api.RegionIDMapping{ { Name: regionWithImages, ID: machineImageID, diff --git a/controllers/provider-openstack/pkg/internal/infrastructure/terraform.go b/controllers/provider-openstack/pkg/internal/infrastructure/terraform.go index ac9c66bb0..bf33d3467 100644 --- a/controllers/provider-openstack/pkg/internal/infrastructure/terraform.go +++ b/controllers/provider-openstack/pkg/internal/infrastructure/terraform.go @@ -19,14 +19,13 @@ import ( api "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/helper" - openstackv1alpha1 "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/v1alpha1" + apiv1alpha1 "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/v1alpha1" "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/internal" "github.com/gardener/gardener-extensions/pkg/controller" "github.com/gardener/gardener-extensions/pkg/terraformer" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" "github.com/gardener/gardener/pkg/chartrenderer" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -58,7 +57,7 @@ var ( InternalChartsPath = filepath.Join(ChartsPath, "internal") // StatusTypeMeta is the TypeMeta of the GCP InfrastructureStatus StatusTypeMeta = metav1.TypeMeta{ - APIVersion: openstackv1alpha1.SchemeGroupVersion.String(), + APIVersion: apiv1alpha1.SchemeGroupVersion.String(), Kind: "InfrastructureStatus", } ) @@ -71,8 +70,8 @@ func ComputeTerraformerChartValues( cluster *controller.Cluster, ) (map[string]interface{}, error) { var ( - routerID = DefaultRouterID createRouter = true + routerID = DefaultRouterID ) if router := config.Networks.Router; router != nil { @@ -80,17 +79,15 @@ func ComputeTerraformerChartValues( routerID = router.ID } - var ( - keyStoneURL string - dnsServers []string - ) - cloudProfileConfig, err := helper.CloudProfileConfigFromCluster(cluster) if err != nil { return nil, err } - keyStoneURL = cloudProfileConfig.KeyStoneURL - dnsServers = cloudProfileConfig.DNSServers + + keyStoneURL, err := helper.FindKeyStoneURL(cloudProfileConfig.KeyStoneURLs, cloudProfileConfig.KeyStoneURL, infra.Spec.Region) + if err != nil { + return nil, err + } return map[string]interface{}{ "openstack": map[string]interface{}{ @@ -103,7 +100,7 @@ func ComputeTerraformerChartValues( "create": map[string]interface{}{ "router": createRouter, }, - "dnsServers": dnsServers, + "dnsServers": cloudProfileConfig.DNSServers, "sshPublicKey": string(infra.Spec.SSHPublicKey), "router": map[string]interface{}{ "id": routerID, @@ -205,36 +202,36 @@ func ExtractTerraformState(tf terraformer.Terraformer, config *api.Infrastructur // StatusFromTerraformState computes an InfrastructureStatus from the given // Terraform variables. -func StatusFromTerraformState(state *TerraformState) *openstackv1alpha1.InfrastructureStatus { +func StatusFromTerraformState(state *TerraformState) *apiv1alpha1.InfrastructureStatus { var ( - status = &openstackv1alpha1.InfrastructureStatus{ + status = &apiv1alpha1.InfrastructureStatus{ TypeMeta: metav1.TypeMeta{ - APIVersion: openstackv1alpha1.SchemeGroupVersion.String(), + APIVersion: apiv1alpha1.SchemeGroupVersion.String(), Kind: "InfrastructureStatus", }, - Networks: openstackv1alpha1.NetworkStatus{ + Networks: apiv1alpha1.NetworkStatus{ ID: state.NetworkID, - FloatingPool: openstackv1alpha1.FloatingPoolStatus{ + FloatingPool: apiv1alpha1.FloatingPoolStatus{ ID: state.FloatingNetworkID, }, - Router: openstackv1alpha1.RouterStatus{ + Router: apiv1alpha1.RouterStatus{ ID: state.RouterID, }, - Subnets: []openstackv1alpha1.Subnet{ + Subnets: []apiv1alpha1.Subnet{ { - Purpose: openstackv1alpha1.PurposeNodes, + Purpose: apiv1alpha1.PurposeNodes, ID: state.SubnetID, }, }, }, - SecurityGroups: []openstackv1alpha1.SecurityGroup{ + SecurityGroups: []apiv1alpha1.SecurityGroup{ { - Purpose: openstackv1alpha1.PurposeNodes, + Purpose: apiv1alpha1.PurposeNodes, ID: state.SecurityGroupID, Name: state.SecurityGroupName, }, }, - Node: openstackv1alpha1.NodeStatus{ + Node: apiv1alpha1.NodeStatus{ KeyName: state.SSHKeyName, }, } @@ -244,7 +241,7 @@ func StatusFromTerraformState(state *TerraformState) *openstackv1alpha1.Infrastr } // ComputeStatus computes the status based on the Terraformer and the given InfrastructureConfig. -func ComputeStatus(tf terraformer.Terraformer, config *api.InfrastructureConfig) (*openstackv1alpha1.InfrastructureStatus, error) { +func ComputeStatus(tf terraformer.Terraformer, config *api.InfrastructureConfig) (*apiv1alpha1.InfrastructureStatus, error) { state, err := ExtractTerraformState(tf, config) if err != nil { return nil, err diff --git a/controllers/provider-openstack/pkg/internal/infrastructure/terraform_test.go b/controllers/provider-openstack/pkg/internal/infrastructure/terraform_test.go index 78141b0f8..a9f4fddf5 100644 --- a/controllers/provider-openstack/pkg/internal/infrastructure/terraform_test.go +++ b/controllers/provider-openstack/pkg/internal/infrastructure/terraform_test.go @@ -33,10 +33,12 @@ import ( var _ = Describe("Terraform", func() { var ( - infra *extensionsv1alpha1.Infrastructure - config *api.InfrastructureConfig - cluster *controller.Cluster - credentials *internal.Credentials + infra *extensionsv1alpha1.Infrastructure + cloudProfileConfig *api.CloudProfileConfig + cloudProfileConfigJSON []byte + config *api.InfrastructureConfig + cluster *controller.Cluster + credentials *internal.Credentials keystoneURL = "foo-bar.com" dnsServers = []string{"a", "b"} @@ -73,15 +75,15 @@ var _ = Describe("Terraform", func() { podsCIDR := "11.0.0.0/16" servicesCIDR := "12.0.0.0/16" - cloudProfileConfig := &apiv1alpha1.CloudProfileConfig{ + cloudProfileConfig = &api.CloudProfileConfig{ TypeMeta: metav1.TypeMeta{ - APIVersion: apiv1alpha1.SchemeGroupVersion.String(), + APIVersion: api.SchemeGroupVersion.String(), Kind: "CloudProfileConfig", }, DNSServers: dnsServers, KeyStoneURL: keystoneURL, } - cloudProfileConfigJSON, _ := json.Marshal(cloudProfileConfig) + cloudProfileConfigJSON, _ = json.Marshal(cloudProfileConfig) cluster = &controller.Cluster{ CloudProfile: &gardencorev1beta1.CloudProfile{ Spec: gardencorev1beta1.CloudProfileSpec{ diff --git a/controllers/provider-openstack/pkg/internal/scheme.go b/controllers/provider-openstack/pkg/internal/scheme.go deleted file mode 100644 index fec216e12..000000000 --- a/controllers/provider-openstack/pkg/internal/scheme.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (c) 2019 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package internal - -import ( - "fmt" - - "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/install" - openstackv1alpha1 "github.com/gardener/gardener-extensions/controllers/provider-openstack/pkg/apis/openstack/v1alpha1" - - gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" - extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" -) - -var ( - // Scheme is a scheme with the types relevant for OpenStack actuators. - Scheme *runtime.Scheme - - decoder runtime.Decoder -) - -func init() { - Scheme = runtime.NewScheme() - utilruntime.Must(install.AddToScheme(Scheme)) - - decoder = serializer.NewCodecFactory(Scheme).UniversalDecoder() -} - -// InfrastructureConfigFromInfrastructure extracts the InfrastructureConfig from the -// ProviderConfig section of the given Infrastructure. -func InfrastructureConfigFromInfrastructure(infra *extensionsv1alpha1.Infrastructure) (*openstackv1alpha1.InfrastructureConfig, error) { - config := &openstackv1alpha1.InfrastructureConfig{} - if infra.Spec.ProviderConfig != nil && infra.Spec.ProviderConfig.Raw != nil { - if _, _, err := decoder.Decode(infra.Spec.ProviderConfig.Raw, nil, config); err != nil { - return nil, err - } - - return config, nil - } - return nil, fmt.Errorf("infrastructure config is not set on the infrastructure resource") -} - -// CloudProfileConfigFromCloudProfile extracts the CloudProfileConfig from the -// ProviderConfig section of the given CloudProfile. -func CloudProfileConfigFromCloudProfile(infra *gardencorev1beta1.CloudProfile) (*openstackv1alpha1.CloudProfileConfig, error) { - config := &openstackv1alpha1.CloudProfileConfig{} - if infra.Spec.ProviderConfig != nil && infra.Spec.ProviderConfig.Raw != nil { - if _, _, err := decoder.Decode(infra.Spec.ProviderConfig.Raw, nil, config); err != nil { - return nil, err - } - - return config, nil - } - return nil, fmt.Errorf("cloudprofile config is not set on the cloudprofile resource") -}