From e56f0d3639f02f98fa811e7ac76f778143e3da8a Mon Sep 17 00:00:00 2001 From: Konstantinos Angelopoulos Date: Thu, 1 Aug 2024 00:30:18 +0200 Subject: [PATCH 1/2] update backupbucket fields --- hack/api-reference/api.md | 10 ++-- pkg/apis/azure/helper/scheme.go | 8 +-- pkg/apis/azure/register.go | 2 +- pkg/apis/azure/types_backup.go | 4 +- pkg/apis/azure/v1alpha1/register.go | 2 +- pkg/apis/azure/v1alpha1/types_backup.go | 4 +- .../azure/v1alpha1/zz_generated.conversion.go | 24 ++++---- .../azure/v1alpha1/zz_generated.deepcopy.go | 10 ++-- pkg/apis/azure/zz_generated.deepcopy.go | 10 ++-- pkg/azure/client/factory.go | 41 ------------- pkg/azure/client/storage.go | 58 ++++++++++++++----- pkg/azure/types.go | 2 + pkg/controller/backupbucket/actuator.go | 14 +++-- pkg/controller/backupbucket/secret.go | 3 +- pkg/controller/backupentry/actuator.go | 9 +-- pkg/internal/infrastructure/terraform.go | 2 +- 16 files changed, 99 insertions(+), 104 deletions(-) diff --git a/hack/api-reference/api.md b/hack/api-reference/api.md index 18678edd6..83ce3d708 100644 --- a/hack/api-reference/api.md +++ b/hack/api-reference/api.md @@ -10,7 +10,7 @@

Resource Types: -

BackupConfig +

BackupBucketConfig

-

BackupConfig is the provider-specific configuration for backup buckets/entries

+

BackupBucketConfig is the provider-specific configuration for backup buckets/entries

@@ -52,7 +52,7 @@ azure.provider.extensions.gardener.cloud/v1alpha1 kind
string - +
BackupConfigBackupBucketConfig
@@ -605,7 +605,7 @@ string

(Appears on: -BackupConfig, +BackupBucketConfig, CloudProfileConfig, DNSRecordConfig)

diff --git a/pkg/apis/azure/helper/scheme.go b/pkg/apis/azure/helper/scheme.go index 62b2865f2..025c5e274 100644 --- a/pkg/apis/azure/helper/scheme.go +++ b/pkg/apis/azure/helper/scheme.go @@ -83,8 +83,8 @@ func CloudProfileConfigFromCluster(cluster *controller.Cluster) (*api.CloudProfi } // BackupConfigFromBackupBucket decodes the provider specific config from a given BackupBucket object. -func BackupConfigFromBackupBucket(backupBucket *extensionsv1alpha1.BackupBucket) (api.BackupConfig, error) { - backupConfig := api.BackupConfig{} +func BackupConfigFromBackupBucket(backupBucket *extensionsv1alpha1.BackupBucket) (api.BackupBucketConfig, error) { + backupConfig := api.BackupBucketConfig{} if backupBucket != nil && backupBucket.Spec.ProviderConfig != nil { bucketJson, err := backupBucket.Spec.ProviderConfig.MarshalJSON() if err != nil { @@ -99,8 +99,8 @@ func BackupConfigFromBackupBucket(backupBucket *extensionsv1alpha1.BackupBucket) } // BackupConfigFromBackupEntry decodes the provider specific config from a given BackupEntry object. -func BackupConfigFromBackupEntry(backupEntry *extensionsv1alpha1.BackupEntry) (api.BackupConfig, error) { - backupConfig := api.BackupConfig{} +func BackupConfigFromBackupEntry(backupEntry *extensionsv1alpha1.BackupEntry) (api.BackupBucketConfig, error) { + backupConfig := api.BackupBucketConfig{} if backupEntry != nil && backupEntry.Spec.DefaultSpec.ProviderConfig != nil { entryJson, err := backupEntry.Spec.ProviderConfig.MarshalJSON() if err != nil { diff --git a/pkg/apis/azure/register.go b/pkg/apis/azure/register.go index faf91b146..5b763de1b 100644 --- a/pkg/apis/azure/register.go +++ b/pkg/apis/azure/register.go @@ -42,7 +42,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ControlPlaneConfig{}, &WorkerStatus{}, &WorkerConfig{}, - &BackupConfig{}, + &BackupBucketConfig{}, &DNSRecordConfig{}, ) return nil diff --git a/pkg/apis/azure/types_backup.go b/pkg/apis/azure/types_backup.go index eb4f79764..586058af2 100644 --- a/pkg/apis/azure/types_backup.go +++ b/pkg/apis/azure/types_backup.go @@ -10,8 +10,8 @@ import ( // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object -// BackupConfig is the provider-specific configuration for backup buckets/entries -type BackupConfig struct { +// BackupBucketConfig is the provider-specific configuration for backup buckets/entries +type BackupBucketConfig struct { metav1.TypeMeta // CloudConfiguration contains config that controls which cloud to connect to. CloudConfiguration *CloudConfiguration diff --git a/pkg/apis/azure/v1alpha1/register.go b/pkg/apis/azure/v1alpha1/register.go index d688ec420..987615f86 100644 --- a/pkg/apis/azure/v1alpha1/register.go +++ b/pkg/apis/azure/v1alpha1/register.go @@ -36,7 +36,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ControlPlaneConfig{}, &WorkerConfig{}, &WorkerStatus{}, - &BackupConfig{}, + &BackupBucketConfig{}, &DNSRecordConfig{}, ) return nil diff --git a/pkg/apis/azure/v1alpha1/types_backup.go b/pkg/apis/azure/v1alpha1/types_backup.go index 387dfe766..b829bf437 100644 --- a/pkg/apis/azure/v1alpha1/types_backup.go +++ b/pkg/apis/azure/v1alpha1/types_backup.go @@ -11,8 +11,8 @@ import ( // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object -// BackupConfig is the provider-specific configuration for backup buckets/entries -type BackupConfig struct { +// BackupBucketConfig is the provider-specific configuration for backup buckets/entries +type BackupBucketConfig struct { metav1.TypeMeta `json:",inline"` // CloudConfiguration contains config that controls which cloud to connect to. // +optional diff --git a/pkg/apis/azure/v1alpha1/zz_generated.conversion.go b/pkg/apis/azure/v1alpha1/zz_generated.conversion.go index b1bc9742d..d89a84b41 100644 --- a/pkg/apis/azure/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/azure/v1alpha1/zz_generated.conversion.go @@ -45,13 +45,13 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*BackupConfig)(nil), (*azure.BackupConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha1_BackupConfig_To_azure_BackupConfig(a.(*BackupConfig), b.(*azure.BackupConfig), scope) + if err := s.AddGeneratedConversionFunc((*BackupBucketConfig)(nil), (*azure.BackupBucketConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_BackupBucketConfig_To_azure_BackupBucketConfig(a.(*BackupBucketConfig), b.(*azure.BackupBucketConfig), scope) }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*azure.BackupConfig)(nil), (*BackupConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_azure_BackupConfig_To_v1alpha1_BackupConfig(a.(*azure.BackupConfig), b.(*BackupConfig), scope) + if err := s.AddGeneratedConversionFunc((*azure.BackupBucketConfig)(nil), (*BackupBucketConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_azure_BackupBucketConfig_To_v1alpha1_BackupBucketConfig(a.(*azure.BackupBucketConfig), b.(*BackupBucketConfig), scope) }); err != nil { return err } @@ -438,24 +438,24 @@ func Convert_azure_AzureResource_To_v1alpha1_AzureResource(in *azure.AzureResour return autoConvert_azure_AzureResource_To_v1alpha1_AzureResource(in, out, s) } -func autoConvert_v1alpha1_BackupConfig_To_azure_BackupConfig(in *BackupConfig, out *azure.BackupConfig, s conversion.Scope) error { +func autoConvert_v1alpha1_BackupBucketConfig_To_azure_BackupBucketConfig(in *BackupBucketConfig, out *azure.BackupBucketConfig, s conversion.Scope) error { out.CloudConfiguration = (*azure.CloudConfiguration)(unsafe.Pointer(in.CloudConfiguration)) return nil } -// Convert_v1alpha1_BackupConfig_To_azure_BackupConfig is an autogenerated conversion function. -func Convert_v1alpha1_BackupConfig_To_azure_BackupConfig(in *BackupConfig, out *azure.BackupConfig, s conversion.Scope) error { - return autoConvert_v1alpha1_BackupConfig_To_azure_BackupConfig(in, out, s) +// Convert_v1alpha1_BackupBucketConfig_To_azure_BackupBucketConfig is an autogenerated conversion function. +func Convert_v1alpha1_BackupBucketConfig_To_azure_BackupBucketConfig(in *BackupBucketConfig, out *azure.BackupBucketConfig, s conversion.Scope) error { + return autoConvert_v1alpha1_BackupBucketConfig_To_azure_BackupBucketConfig(in, out, s) } -func autoConvert_azure_BackupConfig_To_v1alpha1_BackupConfig(in *azure.BackupConfig, out *BackupConfig, s conversion.Scope) error { +func autoConvert_azure_BackupBucketConfig_To_v1alpha1_BackupBucketConfig(in *azure.BackupBucketConfig, out *BackupBucketConfig, s conversion.Scope) error { out.CloudConfiguration = (*CloudConfiguration)(unsafe.Pointer(in.CloudConfiguration)) return nil } -// Convert_azure_BackupConfig_To_v1alpha1_BackupConfig is an autogenerated conversion function. -func Convert_azure_BackupConfig_To_v1alpha1_BackupConfig(in *azure.BackupConfig, out *BackupConfig, s conversion.Scope) error { - return autoConvert_azure_BackupConfig_To_v1alpha1_BackupConfig(in, out, s) +// Convert_azure_BackupBucketConfig_To_v1alpha1_BackupBucketConfig is an autogenerated conversion function. +func Convert_azure_BackupBucketConfig_To_v1alpha1_BackupBucketConfig(in *azure.BackupBucketConfig, out *BackupBucketConfig, s conversion.Scope) error { + return autoConvert_azure_BackupBucketConfig_To_v1alpha1_BackupBucketConfig(in, out, s) } func autoConvert_v1alpha1_CloudConfiguration_To_azure_CloudConfiguration(in *CloudConfiguration, out *azure.CloudConfiguration, s conversion.Scope) error { diff --git a/pkg/apis/azure/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/azure/v1alpha1/zz_generated.deepcopy.go index 8b3449a78..79edfbd6b 100644 --- a/pkg/apis/azure/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/azure/v1alpha1/zz_generated.deepcopy.go @@ -57,7 +57,7 @@ func (in *AzureResource) DeepCopy() *AzureResource { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *BackupConfig) DeepCopyInto(out *BackupConfig) { +func (in *BackupBucketConfig) DeepCopyInto(out *BackupBucketConfig) { *out = *in out.TypeMeta = in.TypeMeta if in.CloudConfiguration != nil { @@ -68,18 +68,18 @@ func (in *BackupConfig) DeepCopyInto(out *BackupConfig) { return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupConfig. -func (in *BackupConfig) DeepCopy() *BackupConfig { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupBucketConfig. +func (in *BackupBucketConfig) DeepCopy() *BackupBucketConfig { if in == nil { return nil } - out := new(BackupConfig) + out := new(BackupBucketConfig) in.DeepCopyInto(out) return out } // DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *BackupConfig) DeepCopyObject() runtime.Object { +func (in *BackupBucketConfig) DeepCopyObject() runtime.Object { if c := in.DeepCopy(); c != nil { return c } diff --git a/pkg/apis/azure/zz_generated.deepcopy.go b/pkg/apis/azure/zz_generated.deepcopy.go index eb0bf6b75..752f00019 100644 --- a/pkg/apis/azure/zz_generated.deepcopy.go +++ b/pkg/apis/azure/zz_generated.deepcopy.go @@ -57,7 +57,7 @@ func (in *AzureResource) DeepCopy() *AzureResource { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *BackupConfig) DeepCopyInto(out *BackupConfig) { +func (in *BackupBucketConfig) DeepCopyInto(out *BackupBucketConfig) { *out = *in out.TypeMeta = in.TypeMeta if in.CloudConfiguration != nil { @@ -68,18 +68,18 @@ func (in *BackupConfig) DeepCopyInto(out *BackupConfig) { return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupConfig. -func (in *BackupConfig) DeepCopy() *BackupConfig { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupBucketConfig. +func (in *BackupBucketConfig) DeepCopy() *BackupBucketConfig { if in == nil { return nil } - out := new(BackupConfig) + out := new(BackupBucketConfig) in.DeepCopyInto(out) return out } // DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *BackupConfig) DeepCopyObject() runtime.Object { +func (in *BackupBucketConfig) DeepCopyObject() runtime.Object { if c := in.DeepCopy(); c != nil { return c } diff --git a/pkg/azure/client/factory.go b/pkg/azure/client/factory.go index f7732edbc..dafa2c35f 100644 --- a/pkg/azure/client/factory.go +++ b/pkg/azure/client/factory.go @@ -6,8 +6,6 @@ package client import ( "context" - "fmt" - "strings" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" @@ -15,7 +13,6 @@ import ( corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure" "github.com/gardener/gardener-extension-provider-azure/pkg/internal" ) @@ -166,41 +163,3 @@ func (f azureFactory) ManagedUserIdentity() (ManagedUserIdentity, error) { func (f azureFactory) VirtualMachineImages() (VirtualMachineImages, error) { return NewVirtualMachineImagesClient(f.auth, f.tokenCredential, f.clientOpts) } - -// NewBlobStorageClient reads the secret from the passed reference and return an Azure (blob) storage client. -func NewBlobStorageClient(ctx context.Context, c client.Client, secretRef corev1.SecretReference, cloudConfiguration *azure.CloudConfiguration) (BlobStorage, error) { - var storageDomain string - - // Unfortunately the valid values for storage domains run by Microsoft do not seem to be part of any sdk module. They might be queryable from the cloud configuration, - // but I also haven't been able to find a documented list of proper ServiceName values. - // Furthermore, it seems there is still no unified way of specifying the cloud instance to connect to as the domain remains part of the storage account URL while - // the new options _also_ allow configuring the cloud instance. - // For the time being (until further testing can be done) I assume that the instance that is pointed at by the URL wins so let's keep the old logic for building the - // service URL. - if cloudConfiguration == nil { - storageDomain = "blob.core.windows.net" - } else { - cloudConfigurationName := cloudConfiguration.Name - switch { - case strings.EqualFold(cloudConfigurationName, "AzurePublic"): - storageDomain = "blob.core.windows.net" - case strings.EqualFold(cloudConfigurationName, "AzureGovernment"): - // Note: This differs from the one mentioned in the docs ("blob.core.govcloudapi.net") but should be the right one. - // ref.: https://github.com/google/go-cloud/blob/be1b4aee38955e1b8cd1c46f8f47fb6f9d820a9b/blob/azureblob/azureblob.go#L162 - storageDomain = "blob.core.usgovcloudapi.net" - case strings.EqualFold(cloudConfigurationName, "AzureChina"): - // This is an educated guess - storageDomain = "blob.core.chinacloudapi.cn" - - default: - return nil, fmt.Errorf("unknown cloud configuration name '%s'", cloudConfigurationName) - } - } - - blobStorageClient, err := newStorageClient(ctx, c, &secretRef, storageDomain) - if err != nil { - return nil, err - } - - return blobStorageClient, nil -} diff --git a/pkg/azure/client/storage.go b/pkg/azure/client/storage.go index 267a6ee99..9717e7129 100644 --- a/pkg/azure/client/storage.go +++ b/pkg/azure/client/storage.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "net/url" + "strings" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob" @@ -17,6 +18,7 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + azureapi "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure" "github.com/gardener/gardener-extension-provider-azure/pkg/azure" ) @@ -28,9 +30,46 @@ type BlobStorageClient struct { client *azblob.Client } -// newStorageClient creates a client for an Azure Blob storage by reading auth information from secret reference. Requires passing the storage domain (formerly +// BlobStorageDomainFromCloudConfiguration returns the storage service domain given a known cloudConfiguration. +func BlobStorageDomainFromCloudConfiguration(cloudConfiguration *azureapi.CloudConfiguration) (string, error) { + // Unfortunately the valid values for storage domains run by Microsoft do not seem to be part of any sdk module. They might be queryable from the cloud configuration, + // but I also haven't been able to find a documented list of proper ServiceName values. + // Furthermore, it seems there is still no unified way of specifying the cloud instance to connect to as the domain remains part of the storage account URL while + // the new options _also_ allow configuring the cloud instance. + switch { + case cloudConfiguration == nil: + return "blob.core.windows.net", nil + case strings.EqualFold(cloudConfiguration.Name, "AzurePublic"): + return "blob.core.windows.net", nil + case strings.EqualFold(cloudConfiguration.Name, "AzureGovernment"): + // Note: This differs from the one mentioned in the docs ("blob.core.govcloudapi.net") but should be the right one. + // ref.: https://github.com/google/go-cloud/blob/be1b4aee38955e1b8cd1c46f8f47fb6f9d820a9b/blob/azureblob/azureblob.go#L162 + return "blob.core.usgovcloudapi.net", nil + case strings.EqualFold(cloudConfiguration.Name, "AzureChina"): + // source: https://learn.microsoft.com/en-us/azure/china/resources-developer-guide#check-endpoints-in-azure + return "blob.core.chinacloudapi.cn", nil + } + return "", fmt.Errorf("unknown cloud configuration name '%s'", cloudConfiguration.Name) +} + +// NewStorageClient creates a blob storage client. +func NewStorageClient(_ context.Context, storageAccountName, storageAccountKey, storageDomain string) (*BlobStorageClient, error) { + credentials, err := azblob.NewSharedKeyCredential(storageAccountName, storageAccountKey) + if err != nil { + return nil, fmt.Errorf("failed to create shared key credentials: %v", err) + } + + storageEndpointURL, err := url.Parse(fmt.Sprintf("https://%s.%s", storageAccountName, storageDomain)) + if err != nil { + return nil, fmt.Errorf("failed to parse service url: %v", err) + } + blobclient, err := azblob.NewClientWithSharedKeyCredential(storageEndpointURL.String(), credentials, nil) + return &BlobStorageClient{blobclient}, err +} + +// NewStorageClientFromSecretRef creates a client for an Azure Blob storage by reading auth information from secret reference. Requires passing the storage domain (formerly // blobstorage host name) to determine the endpoint to build the service url for. -func newStorageClient(ctx context.Context, client client.Client, secretRef *corev1.SecretReference, storageDomain string) (*BlobStorageClient, error) { +func NewStorageClientFromSecretRef(ctx context.Context, client client.Client, secretRef *corev1.SecretReference) (*BlobStorageClient, error) { secret, err := extensionscontroller.GetSecretByReference(ctx, client, secretRef) if err != nil { return nil, err @@ -45,19 +84,12 @@ func newStorageClient(ctx context.Context, client client.Client, secretRef *core return nil, fmt.Errorf("secret %s/%s doesn't have a storage key", secret.Namespace, secret.Name) } - credentials, err := azblob.NewSharedKeyCredential(string(storageAccountName), string(storageAccountKey)) - if err != nil { - return nil, fmt.Errorf("failed to create shared key credentials: %v", err) - } - - storageAccountURL, err := url.Parse(fmt.Sprintf("https://%s.%s", storageAccountName, storageDomain)) - if err != nil { - return nil, fmt.Errorf("failed to parse service url: %v", err) + storageDomain := azure.AzureBlobStorageHostName + if v, ok := secret.Data[azure.StorageDomain]; ok { + storageDomain = string(v) } - blobclient, err := azblob.NewClientWithSharedKeyCredential(storageAccountURL.String(), credentials, nil) - return &BlobStorageClient{blobclient}, err - + return NewStorageClient(ctx, string(storageAccountName), string(storageAccountKey), storageDomain) } // DeleteObjectsWithPrefix deletes the blob objects with the specific from . diff --git a/pkg/azure/types.go b/pkg/azure/types.go index b4dd8dd3b..ad4168f04 100644 --- a/pkg/azure/types.go +++ b/pkg/azure/types.go @@ -71,6 +71,8 @@ const ( StorageAccount = "storageAccount" // StorageKey is a constant for the key in a cloud provider secret and backup secret that holds the Azure secret storage access key. StorageKey = "storageKey" + // StorageDomain is a constant for the key in a backup secret that holds the domain for the Azure blob storage service. + StorageDomain = "domain" // AzureBlobStorageHostName is the host name for azure blob storage service. AzureBlobStorageHostName = "blob.core.windows.net" diff --git a/pkg/controller/backupbucket/actuator.go b/pkg/controller/backupbucket/actuator.go index 9017641e4..68cb4e3d5 100644 --- a/pkg/controller/backupbucket/actuator.go +++ b/pkg/controller/backupbucket/actuator.go @@ -6,6 +6,7 @@ package backupbucket import ( "context" + "fmt" "github.com/gardener/gardener/extensions/pkg/controller/backupbucket" "github.com/gardener/gardener/extensions/pkg/util" @@ -21,7 +22,7 @@ import ( var ( // DefaultBlobStorageClient is the default function to get a backupbucket client. Can be overridden for tests. - DefaultBlobStorageClient = azureclient.NewBlobStorageClient + DefaultBlobStorageClient = azureclient.NewStorageClientFromSecretRef ) type actuator struct { @@ -64,13 +65,18 @@ func (a *actuator) Reconcile(ctx context.Context, _ logr.Logger, backupBucket *e if err != nil { return util.DetermineError(err, helper.KnownCodes) } + + storageDomain, err := azureclient.BlobStorageDomainFromCloudConfiguration(backupConfig.CloudConfiguration) + if err != nil { + return fmt.Errorf("failed to determine blob storage service domain: %w", err) + } // Create the generated backupbucket secret. - if err := a.createBackupBucketGeneratedSecret(ctx, backupBucket, storageAccountName, storageAccountKey); err != nil { + if err := a.createBackupBucketGeneratedSecret(ctx, backupBucket, storageAccountName, storageAccountKey, storageDomain); err != nil { return util.DetermineError(err, helper.KnownCodes) } } - storageClient, err := azureclient.NewBlobStorageClient(ctx, a.client, *backupBucket.Status.GeneratedSecretRef, backupConfig.CloudConfiguration) + storageClient, err := DefaultBlobStorageClient(ctx, a.client, backupBucket.Status.GeneratedSecretRef) if err != nil { return util.DetermineError(err, helper.KnownCodes) } @@ -115,7 +121,7 @@ func (a *actuator) delete(ctx context.Context, _ logr.Logger, backupBucket *exte if secret != nil { // Get a storage account client to delete the backup container in the storage account. - storageClient, err := azureclient.NewBlobStorageClient(ctx, a.client, *backupBucket.Status.GeneratedSecretRef, cloudConfiguration) + storageClient, err := DefaultBlobStorageClient(ctx, a.client, backupBucket.Status.GeneratedSecretRef) if err != nil { return err } diff --git a/pkg/controller/backupbucket/secret.go b/pkg/controller/backupbucket/secret.go index 111c6773a..c81e82daa 100644 --- a/pkg/controller/backupbucket/secret.go +++ b/pkg/controller/backupbucket/secret.go @@ -18,7 +18,7 @@ import ( "github.com/gardener/gardener-extension-provider-azure/pkg/azure" ) -func (a *actuator) createBackupBucketGeneratedSecret(ctx context.Context, backupBucket *extensionsv1alpha1.BackupBucket, storageAccountName, storageKey string) error { +func (a *actuator) createBackupBucketGeneratedSecret(ctx context.Context, backupBucket *extensionsv1alpha1.BackupBucket, storageAccountName, storageKey, storageDomain string) error { var generatedSecret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("generated-bucket-%s", backupBucket.Name), @@ -30,6 +30,7 @@ func (a *actuator) createBackupBucketGeneratedSecret(ctx context.Context, backup generatedSecret.Data = map[string][]byte{ azure.StorageAccount: []byte(storageAccountName), azure.StorageKey: []byte(storageKey), + azure.StorageDomain: []byte(storageDomain), } return nil }); err != nil { diff --git a/pkg/controller/backupentry/actuator.go b/pkg/controller/backupentry/actuator.go index 18e320ef7..93f621a95 100644 --- a/pkg/controller/backupentry/actuator.go +++ b/pkg/controller/backupentry/actuator.go @@ -23,7 +23,7 @@ import ( var ( // DefaultBlobStorageClient is the default function to get a backupbucket client. Can be overridden for tests. - DefaultBlobStorageClient = azureclient.NewBlobStorageClient + DefaultBlobStorageClient = azureclient.NewStorageClientFromSecretRef ) type actuator struct { @@ -41,12 +41,7 @@ func (a *actuator) GetETCDSecretData(_ context.Context, _ logr.Logger, _ *extens } func (a *actuator) Delete(ctx context.Context, _ logr.Logger, backupEntry *extensionsv1alpha1.BackupEntry) error { - backupConfig, err := helper.BackupConfigFromBackupEntry(backupEntry) - if err != nil { - return err - } - - storageClient, err := DefaultBlobStorageClient(ctx, a.client, backupEntry.Spec.SecretRef, backupConfig.CloudConfiguration) + storageClient, err := DefaultBlobStorageClient(ctx, a.client, &backupEntry.Spec.SecretRef) if err != nil { return util.DetermineError(err, helper.KnownCodes) } diff --git a/pkg/internal/infrastructure/terraform.go b/pkg/internal/infrastructure/terraform.go index e69e56ca5..7a276836e 100644 --- a/pkg/internal/infrastructure/terraform.go +++ b/pkg/internal/infrastructure/terraform.go @@ -560,7 +560,7 @@ func findDomainCounts(cluster *controller.Cluster, infra *extensionsv1alpha1.Inf } // Take values from the availability set status. - // Domain counts can still be nil, esp. if the status was written by an earlier version of this provider extension. + // StorageDomain counts can still be nil, esp. if the status was written by an earlier version of this provider extension. if nodesAvailabilitySet != nil { faultDomainCount = nodesAvailabilitySet.CountFaultDomains updateDomainCount = nodesAvailabilitySet.CountUpdateDomains From 9392acef04e6a47fdbb4afd989c702a30abad518 Mon Sep 17 00:00:00 2001 From: Konstantinos Angelopoulos Date: Thu, 1 Aug 2024 15:06:52 +0200 Subject: [PATCH 2/2] rename func + introduce consts --- pkg/azure/client/storage.go | 22 ++++++++++------------ pkg/azure/types.go | 8 ++++++-- pkg/controller/backupbucket/actuator.go | 6 +++--- pkg/controller/backupentry/actuator.go | 2 +- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/pkg/azure/client/storage.go b/pkg/azure/client/storage.go index 9717e7129..ef640af82 100644 --- a/pkg/azure/client/storage.go +++ b/pkg/azure/client/storage.go @@ -37,23 +37,21 @@ func BlobStorageDomainFromCloudConfiguration(cloudConfiguration *azureapi.CloudC // Furthermore, it seems there is still no unified way of specifying the cloud instance to connect to as the domain remains part of the storage account URL while // the new options _also_ allow configuring the cloud instance. switch { - case cloudConfiguration == nil: - return "blob.core.windows.net", nil - case strings.EqualFold(cloudConfiguration.Name, "AzurePublic"): - return "blob.core.windows.net", nil + case cloudConfiguration == nil || strings.EqualFold(cloudConfiguration.Name, "AzurePublic"): + return azure.AzureBlobStorageDomain, nil case strings.EqualFold(cloudConfiguration.Name, "AzureGovernment"): // Note: This differs from the one mentioned in the docs ("blob.core.govcloudapi.net") but should be the right one. // ref.: https://github.com/google/go-cloud/blob/be1b4aee38955e1b8cd1c46f8f47fb6f9d820a9b/blob/azureblob/azureblob.go#L162 - return "blob.core.usgovcloudapi.net", nil + return azure.AzureUSGovBlobStorageDomain, nil case strings.EqualFold(cloudConfiguration.Name, "AzureChina"): // source: https://learn.microsoft.com/en-us/azure/china/resources-developer-guide#check-endpoints-in-azure - return "blob.core.chinacloudapi.cn", nil + return azure.AzureChinaBlobStorageDomain, nil } return "", fmt.Errorf("unknown cloud configuration name '%s'", cloudConfiguration.Name) } -// NewStorageClient creates a blob storage client. -func NewStorageClient(_ context.Context, storageAccountName, storageAccountKey, storageDomain string) (*BlobStorageClient, error) { +// NewBlobStorageClient creates a blob storage client. +func NewBlobStorageClient(_ context.Context, storageAccountName, storageAccountKey, storageDomain string) (*BlobStorageClient, error) { credentials, err := azblob.NewSharedKeyCredential(storageAccountName, storageAccountKey) if err != nil { return nil, fmt.Errorf("failed to create shared key credentials: %v", err) @@ -67,9 +65,9 @@ func NewStorageClient(_ context.Context, storageAccountName, storageAccountKey, return &BlobStorageClient{blobclient}, err } -// NewStorageClientFromSecretRef creates a client for an Azure Blob storage by reading auth information from secret reference. Requires passing the storage domain (formerly +// NewBlobStorageClientFromSecretRef creates a client for an Azure Blob storage by reading auth information from secret reference. Requires passing the storage domain (formerly // blobstorage host name) to determine the endpoint to build the service url for. -func NewStorageClientFromSecretRef(ctx context.Context, client client.Client, secretRef *corev1.SecretReference) (*BlobStorageClient, error) { +func NewBlobStorageClientFromSecretRef(ctx context.Context, client client.Client, secretRef *corev1.SecretReference) (*BlobStorageClient, error) { secret, err := extensionscontroller.GetSecretByReference(ctx, client, secretRef) if err != nil { return nil, err @@ -84,12 +82,12 @@ func NewStorageClientFromSecretRef(ctx context.Context, client client.Client, se return nil, fmt.Errorf("secret %s/%s doesn't have a storage key", secret.Namespace, secret.Name) } - storageDomain := azure.AzureBlobStorageHostName + storageDomain := azure.AzureBlobStorageDomain if v, ok := secret.Data[azure.StorageDomain]; ok { storageDomain = string(v) } - return NewStorageClient(ctx, string(storageAccountName), string(storageAccountKey), storageDomain) + return NewBlobStorageClient(ctx, string(storageAccountName), string(storageAccountKey), storageDomain) } // DeleteObjectsWithPrefix deletes the blob objects with the specific from . diff --git a/pkg/azure/types.go b/pkg/azure/types.go index ad4168f04..3ab6bf8e7 100644 --- a/pkg/azure/types.go +++ b/pkg/azure/types.go @@ -74,8 +74,12 @@ const ( // StorageDomain is a constant for the key in a backup secret that holds the domain for the Azure blob storage service. StorageDomain = "domain" - // AzureBlobStorageHostName is the host name for azure blob storage service. - AzureBlobStorageHostName = "blob.core.windows.net" + // AzureBlobStorageDomain is the host name for azure blob storage service. + AzureBlobStorageDomain = "blob.core.windows.net" + // AzureChinaBlobStorageDomain is the host name for azure blob storage service for the Chinese regions. + AzureChinaBlobStorageDomain = "blob.core.chinacloudapi.cn" + // AzureUSGovBlobStorageDomain is the host name for azure blob storage service for the US Government regions. + AzureUSGovBlobStorageDomain = "blob.core.usgovcloudapi.net" // MachineSetTagKey is the name of the infrastructure resource tag for machine sets. MachineSetTagKey = "machineset.azure.extensions.gardener.cloud" diff --git a/pkg/controller/backupbucket/actuator.go b/pkg/controller/backupbucket/actuator.go index 68cb4e3d5..b0214b6d1 100644 --- a/pkg/controller/backupbucket/actuator.go +++ b/pkg/controller/backupbucket/actuator.go @@ -22,7 +22,7 @@ import ( var ( // DefaultBlobStorageClient is the default function to get a backupbucket client. Can be overridden for tests. - DefaultBlobStorageClient = azureclient.NewStorageClientFromSecretRef + DefaultBlobStorageClient = azureclient.NewBlobStorageClientFromSecretRef ) type actuator struct { @@ -76,11 +76,11 @@ func (a *actuator) Reconcile(ctx context.Context, _ logr.Logger, backupBucket *e } } - storageClient, err := DefaultBlobStorageClient(ctx, a.client, backupBucket.Status.GeneratedSecretRef) + blobStorageClient, err := DefaultBlobStorageClient(ctx, a.client, backupBucket.Status.GeneratedSecretRef) if err != nil { return util.DetermineError(err, helper.KnownCodes) } - return util.DetermineError(storageClient.CreateContainerIfNotExists(ctx, backupBucket.Name), helper.KnownCodes) + return util.DetermineError(blobStorageClient.CreateContainerIfNotExists(ctx, backupBucket.Name), helper.KnownCodes) } func (a *actuator) Delete(ctx context.Context, logger logr.Logger, backupBucket *extensionsv1alpha1.BackupBucket) error { diff --git a/pkg/controller/backupentry/actuator.go b/pkg/controller/backupentry/actuator.go index 93f621a95..a70cf81fc 100644 --- a/pkg/controller/backupentry/actuator.go +++ b/pkg/controller/backupentry/actuator.go @@ -23,7 +23,7 @@ import ( var ( // DefaultBlobStorageClient is the default function to get a backupbucket client. Can be overridden for tests. - DefaultBlobStorageClient = azureclient.NewStorageClientFromSecretRef + DefaultBlobStorageClient = azureclient.NewBlobStorageClientFromSecretRef ) type actuator struct {