Skip to content

Commit

Permalink
fix issue with dnsrecord on CN not having a region
Browse files Browse the repository at this point in the history
  • Loading branch information
kon-angelo committed Nov 19, 2024
1 parent 301fd7f commit 0cb7e16
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 36 deletions.
13 changes: 12 additions & 1 deletion pkg/azure/client/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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...)
}

Expand All @@ -66,6 +75,8 @@ func NewAzureClientFactory(authCredentials *internal.ClientAuth, options ...Azur
option(factory)
}

fmt.Printf("AAAAAAAAAa %v", factory.clientOpts.Cloud)

return *factory, nil
}

Expand Down
14 changes: 13 additions & 1 deletion pkg/azure/client/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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):
Expand Down
4 changes: 4 additions & 0 deletions pkg/azure/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/controlplane/valuesprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
18 changes: 0 additions & 18 deletions pkg/controller/dnsrecord/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/infrastructure/flow_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
18 changes: 9 additions & 9 deletions pkg/internal/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -58,36 +58,36 @@ 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{
SubscriptionID: string(subscriptionID),
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
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/internal/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
Expand All @@ -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))
Expand All @@ -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())
Expand All @@ -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))
Expand All @@ -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))
Expand Down

0 comments on commit 0cb7e16

Please sign in to comment.