From 0cb7e16bed0dba96e11c36561ec4a0fe5ba74cfb Mon Sep 17 00:00:00 2001 From: Konstantinos Angelopoulos Date: Tue, 19 Nov 2024 14:08:55 +0100 Subject: [PATCH] fix issue with dnsrecord on CN not having a region --- pkg/azure/client/factory.go | 13 ++++++++++++- pkg/azure/client/helper.go | 14 +++++++++++++- pkg/azure/types.go | 4 ++++ pkg/controller/controlplane/valuesprovider.go | 2 +- pkg/controller/dnsrecord/actuator.go | 18 ------------------ .../infrastructure/flow_reconciler.go | 2 +- pkg/internal/auth.go | 18 +++++++++--------- pkg/internal/auth_test.go | 10 +++++----- 8 files changed, 45 insertions(+), 36 deletions(-) diff --git a/pkg/azure/client/factory.go b/pkg/azure/client/factory.go index dafa2c35f..0582e5a3c 100644 --- a/pkg/azure/client/factory.go +++ b/pkg/azure/client/factory.go @@ -6,6 +6,7 @@ package client import ( "context" + "fmt" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" @@ -41,10 +42,18 @@ func NewAzureClientFactoryFromSecret( isDNSSecret bool, options ...AzureFactoryOption, ) (Factory, error) { - auth, err := internal.GetClientAuthData(ctx, client, secretRef, isDNSSecret) + auth, secret, err := internal.GetClientAuthData(ctx, client, secretRef, isDNSSecret) if err != nil { return nil, err } + if isDNSSecret { + acc, err := cloudConfigurationFromSecret(secret) + if err != nil { + return nil, err + } + // prepend the cloud configuration from the secret in favor of the explicit ones that may be passed from options. + options = append([]AzureFactoryOption{WithCloudConfiguration(acc)}, options...) + } return NewAzureClientFactory(auth, options...) } @@ -66,6 +75,8 @@ func NewAzureClientFactory(authCredentials *internal.ClientAuth, options ...Azur option(factory) } + fmt.Printf("AAAAAAAAAa %v", factory.clientOpts.Cloud) + return *factory, nil } diff --git a/pkg/azure/client/helper.go b/pkg/azure/client/helper.go index feceeccaa..660c241e3 100644 --- a/pkg/azure/client/helper.go +++ b/pkg/azure/client/helper.go @@ -15,8 +15,10 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/go-autorest/autorest" azerrors "github.com/AzureAD/microsoft-authentication-library-for-go/apps/errors" + corev1 "k8s.io/api/core/v1" "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure" + azuretypes "github.com/gardener/gardener-extension-provider-azure/pkg/azure" ) // FilterNotFoundError returns nil for NotFound errors. @@ -89,6 +91,17 @@ func AzureCloudConfiguration(cloudConfiguration *azure.CloudConfiguration, regio return AzureCloudConfigurationFromCloudConfiguration(cloudConf) } +// cloudConfigurationFromRegion returns a matching cloudConfiguration corresponding to a well known cloud instance for the given region +func cloudConfigurationFromSecret(secret *corev1.Secret) (cloud.Configuration, error) { + if v, ok := secret.Data[azuretypes.AzureCloud]; ok { + return AzureCloudConfigurationFromCloudConfiguration(&azure.CloudConfiguration{Name: string(v)}) + } + if v, ok := secret.Data[azuretypes.DNSAzureCloud]; ok { + return AzureCloudConfigurationFromCloudConfiguration(&azure.CloudConfiguration{Name: string(v)}) + } + return AzureCloudConfigurationFromCloudConfiguration(nil) +} + // cloudConfigurationFromRegion returns a matching cloudConfiguration corresponding to a well known cloud instance for the given region func cloudConfigurationFromRegion(region string) *azure.CloudConfiguration { switch { @@ -106,7 +119,6 @@ func AzureCloudConfigurationFromCloudConfiguration(cloudConfiguration *azure.Clo if cloudConfiguration == nil { return cloud.AzurePublic, nil } - cloudConfigurationName := cloudConfiguration.Name switch { case strings.EqualFold(cloudConfigurationName, azure.AzurePublicCloudName): diff --git a/pkg/azure/types.go b/pkg/azure/types.go index 97b9701c6..075d22beb 100644 --- a/pkg/azure/types.go +++ b/pkg/azure/types.go @@ -57,6 +57,8 @@ const ( ClientIDKey = "clientID" // ClientSecretKey is the key for the client secret. ClientSecretKey = "clientSecret" + // AzureCloud is the key for the cloud configuration in the DNS Secret. + AzureCloud = "azureCloud" // #nosec G101 -- No credential. // DNSSubscriptionIDKey is the key for the subscription ID in DNS secrets. DNSSubscriptionIDKey = "AZURE_SUBSCRIPTION_ID" @@ -66,6 +68,8 @@ const ( DNSClientIDKey = "AZURE_CLIENT_ID" // DNSClientSecretKey is the key for the client secret in DNS secrets. DNSClientSecretKey = "AZURE_CLIENT_SECRET" // #nosec G101 -- No credential. + // DNSAzureCloud is the key for the cloud configuration in the DNS Secret + DNSAzureCloud = "AZURE_CLOUD" // #nosec G101 -- No credential. // StorageAccount is a constant for the key in a cloud provider secret and backup secret that holds the Azure account name. StorageAccount = "storageAccount" diff --git a/pkg/controller/controlplane/valuesprovider.go b/pkg/controller/controlplane/valuesprovider.go index 515dab372..0ceb475a2 100644 --- a/pkg/controller/controlplane/valuesprovider.go +++ b/pkg/controller/controlplane/valuesprovider.go @@ -328,7 +328,7 @@ func (vp *valuesProvider) GetConfigChartValues(ctx context.Context, cp *extensio } // Get client auth - auth, err := internal.GetClientAuthData(ctx, vp.client, cp.Spec.SecretRef, false) + auth, _, err := internal.GetClientAuthData(ctx, vp.client, cp.Spec.SecretRef, false) if err != nil { return nil, fmt.Errorf("could not get service account from secret '%s/%s': %w", cp.Spec.SecretRef.Namespace, cp.Spec.SecretRef.Name, err) } diff --git a/pkg/controller/dnsrecord/actuator.go b/pkg/controller/dnsrecord/actuator.go index 544450a24..d7865b472 100644 --- a/pkg/controller/dnsrecord/actuator.go +++ b/pkg/controller/dnsrecord/actuator.go @@ -47,29 +47,11 @@ func NewActuator(mgr manager.Manager) dnsrecord.Actuator { // Reconcile reconciles the DNSRecord. func (a *actuator) Reconcile(ctx context.Context, log logr.Logger, dns *extensionsv1alpha1.DNSRecord, _ *extensionscontroller.Cluster) error { - dnsRecordConfig, err := helper.DNSRecordConfigFromDNSRecord(dns) - if err != nil { - return err - } - - var azCloudConfiguration cloud.Configuration - // Unlike for the other actuators, both of the values used to determine the cloud instance might be nil - usually the region would not be. - // Default to the public cloud in this case. - if dnsRecordConfig.CloudConfiguration != nil || dns.Spec.Region != nil { - azCloudConfiguration, err = azureclient.AzureCloudConfiguration(dnsRecordConfig.CloudConfiguration, dns.Spec.Region) - if err != nil { - return err - } - } else { - azCloudConfiguration = cloud.AzurePublic - } - clientFactory, err := DefaultAzureClientFactoryFunc( ctx, a.client, dns.Spec.SecretRef, true, - azureclient.WithCloudConfiguration(azCloudConfiguration), ) if err != nil { return err diff --git a/pkg/controller/infrastructure/flow_reconciler.go b/pkg/controller/infrastructure/flow_reconciler.go index 798b2d118..cc223f68e 100644 --- a/pkg/controller/infrastructure/flow_reconciler.go +++ b/pkg/controller/infrastructure/flow_reconciler.go @@ -64,7 +64,7 @@ func (f *FlowReconciler) Reconcile(ctx context.Context, infra *extensionsv1alpha } } - auth, err := internal.GetClientAuthData(ctx, f.client, infra.Spec.SecretRef, false) + auth, _, err := internal.GetClientAuthData(ctx, f.client, infra.Spec.SecretRef, false) if err != nil { return err } diff --git a/pkg/internal/auth.go b/pkg/internal/auth.go index 8cf47e9ff..49a8627e6 100644 --- a/pkg/internal/auth.go +++ b/pkg/internal/auth.go @@ -37,17 +37,17 @@ func (clientAuth ClientAuth) GetAzClientCredentials() (*azidentity.ClientSecretC } // GetClientAuthData retrieves the client auth data specified by the secret reference. -func GetClientAuthData(ctx context.Context, c client.Client, secretRef corev1.SecretReference, allowDNSKeys bool) (*ClientAuth, error) { +func GetClientAuthData(ctx context.Context, c client.Client, secretRef corev1.SecretReference, allowDNSKeys bool) (*ClientAuth, *corev1.Secret, error) { secret, err := extensionscontroller.GetSecretByReference(ctx, c, &secretRef) if err != nil { - return nil, err + return nil, nil, err } return NewClientAuthDataFromSecret(secret, allowDNSKeys) } // NewClientAuthDataFromSecret reads the client auth details from the given secret. -func NewClientAuthDataFromSecret(secret *corev1.Secret, allowDNSKeys bool) (*ClientAuth, error) { +func NewClientAuthDataFromSecret(secret *corev1.Secret, allowDNSKeys bool) (*ClientAuth, *corev1.Secret, error) { var altSubscriptionIDIDKey, altTenantIDKey, altClientIDKey, altClientSecretKey *string if allowDNSKeys { altSubscriptionIDIDKey = ptr.To(azure.DNSSubscriptionIDKey) @@ -58,22 +58,22 @@ func NewClientAuthDataFromSecret(secret *corev1.Secret, allowDNSKeys bool) (*Cli subscriptionID, ok := getSecretDataValue(secret, azure.SubscriptionIDKey, altSubscriptionIDIDKey) if !ok { - return nil, fmt.Errorf("secret %s/%s doesn't have a subscription ID", secret.Namespace, secret.Name) + return nil, nil, fmt.Errorf("secret %s/%s doesn't have a subscription ID", secret.Namespace, secret.Name) } tenantID, ok := getSecretDataValue(secret, azure.TenantIDKey, altTenantIDKey) if !ok { - return nil, fmt.Errorf("secret %s/%s doesn't have a tenant ID", secret.Namespace, secret.Name) + return nil, nil, fmt.Errorf("secret %s/%s doesn't have a tenant ID", secret.Namespace, secret.Name) } clientID, ok := getSecretDataValue(secret, azure.ClientIDKey, altClientIDKey) if !ok { - return nil, fmt.Errorf("secret %s/%s doesn't have a client ID", secret.Namespace, secret.Name) + return nil, nil, fmt.Errorf("secret %s/%s doesn't have a client ID", secret.Namespace, secret.Name) } clientSecret, ok := getSecretDataValue(secret, azure.ClientSecretKey, altClientSecretKey) if !ok { - return nil, fmt.Errorf("secret %s/%s doesn't have a client secret", secret.Namespace, secret.Name) + return nil, nil, fmt.Errorf("secret %s/%s doesn't have a client secret", secret.Namespace, secret.Name) } return &ClientAuth{ @@ -81,13 +81,13 @@ func NewClientAuthDataFromSecret(secret *corev1.Secret, allowDNSKeys bool) (*Cli TenantID: string(tenantID), ClientID: string(clientID), ClientSecret: string(clientSecret), - }, nil + }, secret, nil } // GetAuthorizerAndSubscriptionIDFromSecretRef retrieves the client auth data specified by the secret reference // to create and return an Azure Authorizer and a subscription id. func GetAuthorizerAndSubscriptionIDFromSecretRef(ctx context.Context, c client.Client, secretRef corev1.SecretReference, allowDNSKeys bool) (azureautorest.Authorizer, string, error) { - clientAuth, err := GetClientAuthData(ctx, c, secretRef, allowDNSKeys) + clientAuth, _, err := GetClientAuthData(ctx, c, secretRef, allowDNSKeys) if err != nil { return nil, "", err } diff --git a/pkg/internal/auth_test.go b/pkg/internal/auth_test.go index fe24098d6..3e143d7a8 100644 --- a/pkg/internal/auth_test.go +++ b/pkg/internal/auth_test.go @@ -75,7 +75,7 @@ var _ = Describe("Azure Auth", func() { Describe("#NewClientAuthDataFromSecret", func() { It("should read the client auth data from the secret", func() { - actual, err := NewClientAuthDataFromSecret(secret, false) + actual, _, err := NewClientAuthDataFromSecret(secret, false) Expect(err).NotTo(HaveOccurred()) Expect(actual).To(Equal(clientAuth)) }) @@ -91,7 +91,7 @@ var _ = Describe("Azure Auth", func() { return nil }) - actual, err := GetClientAuthData(ctx, c, secretRef, false) + actual, _, err := GetClientAuthData(ctx, c, secretRef, false) Expect(err).NotTo(HaveOccurred()) Expect(actual).To(Equal(clientAuth)) @@ -105,7 +105,7 @@ var _ = Describe("Azure Auth", func() { return nil }) - actual, err := GetClientAuthData(ctx, c, secretRef, false) + actual, _, err := GetClientAuthData(ctx, c, secretRef, false) Expect(err).To(HaveOccurred()) Expect(actual).To(BeNil()) @@ -121,7 +121,7 @@ var _ = Describe("Azure Auth", func() { return nil }) - actual, err := GetClientAuthData(ctx, c, secretRef, true) + actual, _, err := GetClientAuthData(ctx, c, secretRef, true) Expect(err).NotTo(HaveOccurred()) Expect(actual).To(Equal(clientAuth)) @@ -135,7 +135,7 @@ var _ = Describe("Azure Auth", func() { return nil }) - actual, err := GetClientAuthData(ctx, c, secretRef, true) + actual, _, err := GetClientAuthData(ctx, c, secretRef, true) Expect(err).NotTo(HaveOccurred()) Expect(actual).To(Equal(clientAuth))