From b236d93edb3c066d8eeafc6dd02ac60750ae2df0 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 15 Nov 2021 15:36:42 +0800 Subject: [PATCH 01/19] basic shim layer support --- internal/services/storage/client/client.go | 113 +++++++++-- .../storage/shim/containers_mgmt_plane.go | 132 ++++++++++++ internal/services/storage/shim/queues.go | 4 - .../storage/shim/queues_mgmt_plane.go | 85 ++++++++ .../storage/shim/shares_mgmt_plane.go | 189 ++++++++++++++++++ .../storage/shim/tables_mgmt_plane.go | 60 ++++++ internal/services/storage/shim/types.go | 25 +++ .../storage/storage_account_resource.go | 17 +- 8 files changed, 597 insertions(+), 28 deletions(-) create mode 100644 internal/services/storage/shim/containers_mgmt_plane.go create mode 100644 internal/services/storage/shim/queues_mgmt_plane.go create mode 100644 internal/services/storage/shim/shares_mgmt_plane.go create mode 100644 internal/services/storage/shim/tables_mgmt_plane.go create mode 100644 internal/services/storage/shim/types.go diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index 52d3ecc8f78b..30106ea98ad2 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -29,19 +29,27 @@ type Client struct { FileSystemsClient *filesystems.Client ADLSGen2PathsClient *paths.Client ManagementPoliciesClient *storage.ManagementPoliciesClient - BlobServicesClient *storage.BlobServicesClient BlobInventoryPoliciesClient *legacystorage.BlobInventoryPoliciesClient CloudEndpointsClient *storagesync.CloudEndpointsClient EncryptionScopesClient *storage.EncryptionScopesClient Environment az.Environment - FileServicesClient *storage.FileServicesClient ObjectReplicationClient *storage.ObjectReplicationPoliciesClient SyncServiceClient *storagesync.ServicesClient SyncGroupsClient *storagesync.SyncGroupsClient SubscriptionId string + BlobServicesClient *storage.BlobServicesClient + FileServicesClient *storage.FileServicesClient + QueueServicesClient *storage.QueueServicesClient + TableServicesClient *storage.TableServicesClient + resourceManagerAuthorizer autorest.Authorizer storageAdAuth *autorest.Authorizer + + // useResourceManager specifies whether to use the mgmt plane API for resources that have both data plane and mgmt plane support. + // Currently, only the following resources are affected: blob container, file share, queue. + // TODO: data lake filesystem/path, table. + useResourceManager bool } func NewClient(options *common.ClientOptions) *Client { @@ -57,9 +65,6 @@ func NewClient(options *common.ClientOptions) *Client { managementPoliciesClient := storage.NewManagementPoliciesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) options.ConfigureClient(&managementPoliciesClient.Client, options.ResourceManagerAuthorizer) - blobServicesClient := storage.NewBlobServicesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) - options.ConfigureClient(&blobServicesClient.Client, options.ResourceManagerAuthorizer) - blobInventoryPoliciesClient := legacystorage.NewBlobInventoryPoliciesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) options.ConfigureClient(&blobInventoryPoliciesClient.Client, options.ResourceManagerAuthorizer) @@ -69,9 +74,6 @@ func NewClient(options *common.ClientOptions) *Client { encryptionScopesClient := storage.NewEncryptionScopesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) options.ConfigureClient(&encryptionScopesClient.Client, options.ResourceManagerAuthorizer) - fileServicesClient := storage.NewFileServicesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) - options.ConfigureClient(&fileServicesClient.Client, options.ResourceManagerAuthorizer) - objectReplicationPolicyClient := storage.NewObjectReplicationPoliciesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) options.ConfigureClient(&objectReplicationPolicyClient.Client, options.ResourceManagerAuthorizer) @@ -81,6 +83,18 @@ func NewClient(options *common.ClientOptions) *Client { syncGroupsClient := storagesync.NewSyncGroupsClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) options.ConfigureClient(&syncGroupsClient.Client, options.ResourceManagerAuthorizer) + blobServicesClient := storage.NewBlobServicesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) + options.ConfigureClient(&blobServicesClient.Client, options.ResourceManagerAuthorizer) + + fileServicesClient := storage.NewFileServicesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) + options.ConfigureClient(&fileServicesClient.Client, options.ResourceManagerAuthorizer) + + queueServiceClient := storage.NewQueueServicesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) + options.ConfigureClient(&queueServiceClient.Client, options.ResourceManagerAuthorizer) + + tableServiceClient := storage.NewTableServicesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) + options.ConfigureClient(&tableServiceClient.Client, options.ResourceManagerAuthorizer) + // TODO: switch Storage Containers to using the storage.BlobContainersClient // (which should fix #2977) when the storage clients have been moved in here client := Client{ @@ -88,18 +102,23 @@ func NewClient(options *common.ClientOptions) *Client { FileSystemsClient: &fileSystemsClient, ADLSGen2PathsClient: &adlsGen2PathsClient, ManagementPoliciesClient: &managementPoliciesClient, - BlobServicesClient: &blobServicesClient, BlobInventoryPoliciesClient: &blobInventoryPoliciesClient, CloudEndpointsClient: &cloudEndpointsClient, EncryptionScopesClient: &encryptionScopesClient, Environment: options.Environment, - FileServicesClient: &fileServicesClient, ObjectReplicationClient: &objectReplicationPolicyClient, SubscriptionId: options.SubscriptionId, SyncServiceClient: &syncServiceClient, SyncGroupsClient: &syncGroupsClient, + BlobServicesClient: &blobServicesClient, + FileServicesClient: &fileServicesClient, + QueueServicesClient: &queueServiceClient, + TableServicesClient: &tableServiceClient, + resourceManagerAuthorizer: options.ResourceManagerAuthorizer, + + useResourceManager: false, // TODO: feature toggleable } if options.StorageUseAzureAD { @@ -154,11 +173,24 @@ func (client Client) BlobsClient(ctx context.Context, account accountDetails) (* } func (client Client) ContainersClient(ctx context.Context, account accountDetails) (shim.StorageContainerWrapper, error) { + if client.useResourceManager { + rmClient := storage.NewBlobContainersClientWithBaseURI(client.Environment.ResourceManagerEndpoint, client.SubscriptionId) + rmClient.Client.Authorizer = client.resourceManagerAuthorizer + return shim.NewMgmtPlaneStorageContainerWrapper(&rmClient), nil + } + + containersClient, err := client.ContainersDataPlaneClient(ctx, account) + if err != nil { + return nil, err + } + return shim.NewDataPlaneStorageContainerWrapper(containersClient), nil +} + +func (client Client) ContainersDataPlaneClient(ctx context.Context, account accountDetails) (*containers.Client, error) { if client.storageAdAuth != nil { containersClient := containers.NewWithEnvironment(client.Environment) containersClient.Client.Authorizer = *client.storageAdAuth - shim := shim.NewDataPlaneStorageContainerWrapper(&containersClient) - return shim, nil + return &containersClient, nil } accountKey, err := account.AccountKey(ctx, client) @@ -173,9 +205,7 @@ func (client Client) ContainersClient(ctx context.Context, account accountDetail containersClient := containers.NewWithEnvironment(client.Environment) containersClient.Client.Authorizer = storageAuth - - shim := shim.NewDataPlaneStorageContainerWrapper(&containersClient) - return shim, nil + return &containersClient, nil } func (client Client) FileShareDirectoriesClient(ctx context.Context, account accountDetails) (*directories.Client, error) { @@ -215,6 +245,20 @@ func (client Client) FileShareFilesClient(ctx context.Context, account accountDe } func (client Client) FileSharesClient(ctx context.Context, account accountDetails) (shim.StorageShareWrapper, error) { + if client.useResourceManager { + sharesClient := storage.NewFileSharesClientWithBaseURI(client.Environment.ResourceManagerEndpoint, client.SubscriptionId) + sharesClient.Client.Authorizer = client.resourceManagerAuthorizer + return shim.NewMgmtPlaneStorageShareWrapper(&sharesClient), nil + } + + sharesClient, err := client.FileSharesDataPlaneClient(ctx, account) + if err != nil { + return nil, err + } + return shim.NewDataPlaneStorageShareWrapper(sharesClient), nil +} + +func (client Client) FileSharesDataPlaneClient(ctx context.Context, account accountDetails) (*shares.Client, error) { // NOTE: Files do not support AzureAD Authentication accountKey, err := account.AccountKey(ctx, client) @@ -229,15 +273,28 @@ func (client Client) FileSharesClient(ctx context.Context, account accountDetail sharesClient := shares.NewWithEnvironment(client.Environment) sharesClient.Client.Authorizer = storageAuth - shim := shim.NewDataPlaneStorageShareWrapper(&sharesClient) - return shim, nil + return &sharesClient, nil } func (client Client) QueuesClient(ctx context.Context, account accountDetails) (shim.StorageQueuesWrapper, error) { + if client.useResourceManager { + queueClient := storage.NewQueueClient(client.SubscriptionId) + queueClient.Client.Authorizer = client.resourceManagerAuthorizer + return shim.NewMgmtPlaneStorageQueueWrapper(&queueClient), nil + } + + queuesClient, err := client.QueuesDataPlaneClient(ctx, account) + if err != nil { + return nil, err + } + return shim.NewDataPlaneStorageQueueWrapper(queuesClient), nil +} + +func (client Client) QueuesDataPlaneClient(ctx context.Context, account accountDetails) (*queues.Client, error) { if client.storageAdAuth != nil { queueClient := queues.NewWithEnvironment(client.Environment) queueClient.Client.Authorizer = *client.storageAdAuth - return shim.NewDataPlaneStorageQueueWrapper(&queueClient), nil + return &queueClient, nil } accountKey, err := account.AccountKey(ctx, client) @@ -252,7 +309,7 @@ func (client Client) QueuesClient(ctx context.Context, account accountDetails) ( queuesClient := queues.NewWithEnvironment(client.Environment) queuesClient.Client.Authorizer = storageAuth - return shim.NewDataPlaneStorageQueueWrapper(&queuesClient), nil + return &queuesClient, nil } func (client Client) TableEntityClient(ctx context.Context, account accountDetails) (*entities.Client, error) { @@ -274,6 +331,21 @@ func (client Client) TableEntityClient(ctx context.Context, account accountDetai } func (client Client) TablesClient(ctx context.Context, account accountDetails) (shim.StorageTableWrapper, error) { + //TODO: once mgmt API got ACL support, we can uncomment below + //if client.useResourceManager { + // tableClient := storage.NewTableClient(client.SubscriptionId) + // tableClient.Client.Authorizer = client.resourceManagerAuthorizer + // return shim.NewMgmtPlaneStorageTableWrapper(&tableClient), nil + //} + + tablesClient, err := client.TablesDataPlaneClient(ctx, account) + if err != nil { + return nil, err + } + return shim.NewDataPlaneStorageTableWrapper(tablesClient), nil +} + +func (client Client) TablesDataPlaneClient(ctx context.Context, account accountDetails) (*tables.Client, error) { // NOTE: Tables do not support AzureAD Authentication accountKey, err := account.AccountKey(ctx, client) @@ -288,6 +360,5 @@ func (client Client) TablesClient(ctx context.Context, account accountDetails) ( tablesClient := tables.NewWithEnvironment(client.Environment) tablesClient.Client.Authorizer = storageAuth - shim := shim.NewDataPlaneStorageTableWrapper(&tablesClient) - return shim, nil + return &tablesClient, nil } diff --git a/internal/services/storage/shim/containers_mgmt_plane.go b/internal/services/storage/shim/containers_mgmt_plane.go new file mode 100644 index 000000000000..5e126021bddb --- /dev/null +++ b/internal/services/storage/shim/containers_mgmt_plane.go @@ -0,0 +1,132 @@ +package shim + +import ( + "context" + "fmt" + + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/hashicorp/terraform-provider-azurerm/utils" + "github.com/tombuildsstuff/giovanni/storage/2019-12-12/blob/containers" +) + +type ResourceManagerStorageContainerWrapper struct { + client *storage.BlobContainersClient +} + +func NewMgmtPlaneStorageContainerWrapper(client *storage.BlobContainersClient) StorageContainerWrapper { + return ResourceManagerStorageContainerWrapper{ + client: client, + } +} + +func (w ResourceManagerStorageContainerWrapper) Create(ctx context.Context, resourceGroup, accountName, containerName string, input containers.CreateInput) error { + rmInput := storage.BlobContainer{ + ContainerProperties: &storage.ContainerProperties{ + PublicAccess: w.mapAccessLevel(input.AccessLevel), + Metadata: mapStringToMapStringPtr(input.MetaData), + }, + } + _, err := w.client.Create(ctx, resourceGroup, accountName, containerName, rmInput) + return err +} + +func (w ResourceManagerStorageContainerWrapper) Delete(ctx context.Context, resourceGroup, accountName, containerName string) error { + resp, err := w.client.Delete(ctx, resourceGroup, accountName, containerName) + if err != nil { + if utils.ResponseWasNotFound(resp) { + return nil + } + + return err + } + return nil +} + +func (w ResourceManagerStorageContainerWrapper) Exists(ctx context.Context, resourceGroup, accountName, containerName string) (*bool, error) { + container, err := w.client.Get(ctx, resourceGroup, accountName, containerName) + if err != nil { + if utils.ResponseWasNotFound(container.Response) { + return utils.Bool(false), nil + } + + return nil, err + } + + return utils.Bool(container.ContainerProperties != nil), nil +} + +func (w ResourceManagerStorageContainerWrapper) Get(ctx context.Context, resourceGroup, accountName, containerName string) (*StorageContainerProperties, error) { + container, err := w.client.Get(ctx, resourceGroup, accountName, containerName) + if err != nil { + if utils.ResponseWasNotFound(container.Response) { + return nil, nil + } + + return nil, err + } + + if container.ContainerProperties == nil { + return nil, fmt.Errorf("`properties` is null in the API response") + } + + var nullableBoolToBool = func(input *bool) bool { + if input == nil { + return false + } + + return *input + } + + output := StorageContainerProperties{ + AccessLevel: w.mapPublicAccess(container.ContainerProperties.PublicAccess), + HasLegalHold: nullableBoolToBool(container.ContainerProperties.HasLegalHold), + HasImmutabilityPolicy: nullableBoolToBool(container.ContainerProperties.HasImmutabilityPolicy), + MetaData: mapStringPtrToMapString(container.ContainerProperties.Metadata), + } + return &output, nil +} + +func (w ResourceManagerStorageContainerWrapper) UpdateAccessLevel(ctx context.Context, resourceGroup, accountName, containerName string, level containers.AccessLevel) error { + rmInput := storage.BlobContainer{ + ContainerProperties: &storage.ContainerProperties{ + PublicAccess: w.mapAccessLevel(level), + }, + } + _, err := w.client.Update(ctx, resourceGroup, accountName, containerName, rmInput) + return err +} + +func (w ResourceManagerStorageContainerWrapper) UpdateMetaData(ctx context.Context, resourceGroup, accountName, containerName string, metaData map[string]string) error { + rmInput := storage.BlobContainer{ + ContainerProperties: &storage.ContainerProperties{ + Metadata: mapStringToMapStringPtr(metaData), + }, + } + _, err := w.client.Update(ctx, resourceGroup, accountName, containerName, rmInput) + return err +} + +func (w ResourceManagerStorageContainerWrapper) mapAccessLevel(input containers.AccessLevel) storage.PublicAccess { + switch input { + case containers.Blob: + return storage.PublicAccessBlob + + case containers.Container: + return storage.PublicAccessContainer + } + + // this is an empty string, or _could_ be a value going forwards - it's easier to default this + return storage.PublicAccessNone +} + +func (w ResourceManagerStorageContainerWrapper) mapPublicAccess(input storage.PublicAccess) containers.AccessLevel { + switch input { + case storage.PublicAccessBlob: + return containers.Blob + + case storage.PublicAccessContainer: + return containers.Container + } + + return containers.Private +} diff --git a/internal/services/storage/shim/queues.go b/internal/services/storage/shim/queues.go index 8a444b6b3e63..f76a18cbc36c 100644 --- a/internal/services/storage/shim/queues.go +++ b/internal/services/storage/shim/queues.go @@ -2,8 +2,6 @@ package shim import ( "context" - - "github.com/tombuildsstuff/giovanni/storage/2019-12-12/queue/queues" ) type StorageQueuesWrapper interface { @@ -11,9 +9,7 @@ type StorageQueuesWrapper interface { Delete(ctx context.Context, resourceGroup, accountName, queueName string) error Exists(ctx context.Context, resourceGroup, accountName, queueName string) (*bool, error) Get(ctx context.Context, resourceGroup, accountName, queueName string) (*StorageQueueProperties, error) - GetServiceProperties(ctx context.Context, resourceGroup, accountName string) (*queues.StorageServiceProperties, error) UpdateMetaData(ctx context.Context, resourceGroup, accountName, queueName string, metaData map[string]string) error - UpdateServiceProperties(ctx context.Context, resourceGroup, accountName string, properties queues.StorageServiceProperties) error } type StorageQueueProperties struct { diff --git a/internal/services/storage/shim/queues_mgmt_plane.go b/internal/services/storage/shim/queues_mgmt_plane.go new file mode 100644 index 000000000000..5b9ae843066e --- /dev/null +++ b/internal/services/storage/shim/queues_mgmt_plane.go @@ -0,0 +1,85 @@ +package shim + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-provider-azurerm/utils" + + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" +) + +type ResourceManagerStorageQueueWrapper struct { + client *storage.QueueClient +} + +func NewMgmtPlaneStorageQueueWrapper(client *storage.QueueClient) StorageQueuesWrapper { + return ResourceManagerStorageQueueWrapper{ + client: client, + } +} + +func (w ResourceManagerStorageQueueWrapper) Create(ctx context.Context, resourceGroup, accountName, queueName string, metaData map[string]string) error { + rmInput := storage.Queue{ + QueueProperties: &storage.QueueProperties{ + Metadata: mapStringToMapStringPtr(metaData), + }, + } + _, err := w.client.Create(ctx, resourceGroup, accountName, queueName, rmInput) + return err +} + +func (w ResourceManagerStorageQueueWrapper) Delete(ctx context.Context, resourceGroup, accountName, queueName string) error { + resp, err := w.client.Delete(ctx, resourceGroup, accountName, queueName) + if err != nil { + if utils.ResponseWasNotFound(resp) { + return nil + } + + return err + } + return nil +} + +func (w ResourceManagerStorageQueueWrapper) Exists(ctx context.Context, resourceGroup, accountName, queueName string) (*bool, error) { + queue, err := w.client.Get(ctx, resourceGroup, accountName, queueName) + if err != nil { + if utils.ResponseWasNotFound(queue.Response) { + return utils.Bool(false), nil + } + + return nil, err + } + + return utils.Bool(queue.QueueProperties != nil), nil +} + +func (w ResourceManagerStorageQueueWrapper) Get(ctx context.Context, resourceGroup, accountName, queueName string) (*StorageQueueProperties, error) { + queue, err := w.client.Get(ctx, resourceGroup, accountName, queueName) + if err != nil { + if utils.ResponseWasNotFound(queue.Response) { + return nil, nil + } + + return nil, err + } + + if queue.QueueProperties == nil { + return nil, fmt.Errorf("`properties` is null in the API response") + } + + output := StorageQueueProperties{ + MetaData: mapStringPtrToMapString(queue.QueueProperties.Metadata), + } + return &output, nil +} + +func (w ResourceManagerStorageQueueWrapper) UpdateMetaData(ctx context.Context, resourceGroup, accountName, queueName string, metaData map[string]string) error { + rmInput := storage.Queue{ + QueueProperties: &storage.QueueProperties{ + Metadata: mapStringToMapStringPtr(metaData), + }, + } + _, err := w.client.Update(ctx, resourceGroup, accountName, queueName, rmInput) + return err +} diff --git a/internal/services/storage/shim/shares_mgmt_plane.go b/internal/services/storage/shim/shares_mgmt_plane.go new file mode 100644 index 000000000000..bb4d63e1cf89 --- /dev/null +++ b/internal/services/storage/shim/shares_mgmt_plane.go @@ -0,0 +1,189 @@ +package shim + +import ( + "context" + "fmt" + "time" + + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/hashicorp/terraform-provider-azurerm/utils" + "github.com/tombuildsstuff/giovanni/storage/2020-08-04/file/shares" + + "github.com/Azure/go-autorest/autorest/date" +) + +type ResourceManagerStorageShareWrapper struct { + client *storage.FileSharesClient +} + +func NewMgmtPlaneStorageShareWrapper(client *storage.FileSharesClient) StorageShareWrapper { + return ResourceManagerStorageShareWrapper{ + client: client, + } +} + +func (w ResourceManagerStorageShareWrapper) Create(ctx context.Context, resourceGroup, accountName, shareName string, input shares.CreateInput) error { + rmInput := storage.FileShare{ + FileShareProperties: &storage.FileShareProperties{ + ShareQuota: utils.Int32(int32(input.QuotaInGB)), + EnabledProtocols: storage.EnabledProtocols(input.EnabledProtocol), + Metadata: mapStringToMapStringPtr(input.MetaData), + }, + } + _, err := w.client.Create(ctx, resourceGroup, accountName, shareName, rmInput, "") + return err +} + +func (w ResourceManagerStorageShareWrapper) Delete(ctx context.Context, resourceGroup, accountName, shareName string) error { + resp, err := w.client.Delete(ctx, resourceGroup, accountName, shareName, "", "") + if err != nil { + if utils.ResponseWasNotFound(resp) { + return nil + } + + return err + } + return nil +} + +func (w ResourceManagerStorageShareWrapper) Exists(ctx context.Context, resourceGroup, accountName, shareName string) (*bool, error) { + share, err := w.client.Get(ctx, resourceGroup, accountName, shareName, "", "") + if err != nil { + if utils.ResponseWasNotFound(share.Response) { + return utils.Bool(false), nil + } + + return nil, err + } + + return utils.Bool(share.FileShareProperties != nil), nil +} + +func (w ResourceManagerStorageShareWrapper) Get(ctx context.Context, resourceGroup, accountName, shareName string) (*StorageShareProperties, error) { + share, err := w.client.Get(ctx, resourceGroup, accountName, shareName, "", "") + if err != nil { + if utils.ResponseWasNotFound(share.Response) { + return nil, nil + } + + return nil, err + } + + if share.FileShareProperties == nil { + return nil, fmt.Errorf("`properties` is null in the API response") + } + + quotaGB := 0 + if quotaPtr := share.FileShareProperties.ShareQuota; quotaPtr != nil { + quotaGB = int(*quotaPtr) + } + + output := StorageShareProperties{ + ACLs: w.mapToACLs(share.FileShareProperties.SignedIdentifiers), + MetaData: mapStringPtrToMapString(share.FileShareProperties.Metadata), + QuotaGB: quotaGB, + EnabledProtocol: shares.ShareProtocol(share.FileShareProperties.EnabledProtocols), + } + return &output, nil +} + +func (w ResourceManagerStorageShareWrapper) UpdateACLs(ctx context.Context, resourceGroup, accountName, shareName string, acls []shares.SignedIdentifier) error { + identifiers, err := w.mapFromACLs(acls) + if err != nil { + return err + } + rmInput := storage.FileShare{ + FileShareProperties: &storage.FileShareProperties{ + SignedIdentifiers: identifiers, + }, + } + _, err = w.client.Update(ctx, resourceGroup, accountName, shareName, rmInput) + return err +} + +func (w ResourceManagerStorageShareWrapper) UpdateMetaData(ctx context.Context, resourceGroup, accountName, shareName string, metaData map[string]string) error { + rmInput := storage.FileShare{ + FileShareProperties: &storage.FileShareProperties{ + Metadata: mapStringToMapStringPtr(metaData), + }, + } + _, err := w.client.Update(ctx, resourceGroup, accountName, shareName, rmInput) + return err +} + +func (w ResourceManagerStorageShareWrapper) UpdateQuota(ctx context.Context, resourceGroup, accountName, shareName string, quotaGB int) error { + rmInput := storage.FileShare{ + FileShareProperties: &storage.FileShareProperties{ + ShareQuota: utils.Int32(int32(quotaGB)), + }, + } + _, err := w.client.Update(ctx, resourceGroup, accountName, shareName, rmInput) + return err +} + +func (w ResourceManagerStorageShareWrapper) mapToACLs(input *[]storage.SignedIdentifier) []shares.SignedIdentifier { + if input == nil { + return nil + } + var output []shares.SignedIdentifier + for _, identifier := range *input { + var id string + if identifier.ID != nil { + id = *identifier.ID + } + + var policy shares.AccessPolicy + if identifier.AccessPolicy != nil { + var expiry string + if identifier.AccessPolicy.Expiry != nil { + expiry = identifier.AccessPolicy.Expiry.String() + } + var start string + if identifier.AccessPolicy.Start != nil { + start = identifier.AccessPolicy.Start.String() + } + var permission string + if identifier.AccessPolicy.Permission != nil { + permission = *identifier.AccessPolicy.Permission + } + policy = shares.AccessPolicy{ + Start: start, + Expiry: expiry, + Permission: permission, + } + } + output = append(output, shares.SignedIdentifier{ + Id: id, + AccessPolicy: policy, + }) + } + return output +} + +func (w ResourceManagerStorageShareWrapper) mapFromACLs(input []shares.SignedIdentifier) (*[]storage.SignedIdentifier, error) { + if len(input) == 0 { + return nil, nil + } + + var output []storage.SignedIdentifier + for _, identifier := range input { + policy := identifier.AccessPolicy + start, err := date.ParseTime(time.RFC3339, policy.Start) + if err != nil { + return nil, fmt.Errorf("parsing start time of the ACL %q: %w", policy.Start, err) + } + expiry, err := date.ParseTime(time.RFC3339, policy.Expiry) + if err != nil { + return nil, fmt.Errorf("parsing expiry time of the ACL %q: %w", policy.Expiry, err) + } + output = append(output, storage.SignedIdentifier{ + ID: &identifier.Id, + AccessPolicy: &storage.AccessPolicy{ + Start: &date.Time{Time: start}, + Expiry: &date.Time{Time: expiry}, + Permission: &policy.Permission, + }, + }) + } + return &output, nil +} diff --git a/internal/services/storage/shim/tables_mgmt_plane.go b/internal/services/storage/shim/tables_mgmt_plane.go new file mode 100644 index 000000000000..7efd0396761f --- /dev/null +++ b/internal/services/storage/shim/tables_mgmt_plane.go @@ -0,0 +1,60 @@ +package shim + +import ( + "context" + + "github.com/hashicorp/terraform-provider-azurerm/utils" + + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/tombuildsstuff/giovanni/storage/2019-12-12/table/tables" +) + +type ResourceManagerStorageTableWrapper struct { + client *storage.TableClient +} + +func NewMgmtPlaneStorageTableWrapper(client *storage.TableClient) StorageTableWrapper { + return ResourceManagerStorageTableWrapper{ + client: client, + } +} + +func (w ResourceManagerStorageTableWrapper) Create(ctx context.Context, resourceGroup string, accountName string, tableName string) error { + _, err := w.client.Create(ctx, resourceGroup, accountName, tableName) + return err +} + +func (w ResourceManagerStorageTableWrapper) Delete(ctx context.Context, resourceGroup string, accountName string, tableName string) error { + resp, err := w.client.Delete(ctx, resourceGroup, accountName, tableName) + if err != nil { + if utils.ResponseWasNotFound(resp) { + return nil + } + + return err + } + return nil +} + +func (w ResourceManagerStorageTableWrapper) Exists(ctx context.Context, resourceGroup string, accountName string, tableName string) (*bool, error) { + table, err := w.client.Get(ctx, resourceGroup, accountName, tableName) + if err != nil { + if utils.ResponseWasNotFound(table.Response) { + return utils.Bool(false), nil + } + + return nil, err + } + + return utils.Bool(table.TableProperties != nil), nil +} + +func (w ResourceManagerStorageTableWrapper) GetACLs(ctx context.Context, resourceGroup string, accountName string, tableName string) (*[]tables.SignedIdentifier, error) { + // TODO @magodo: support ACLs once API is available + panic("implement me") +} + +func (w ResourceManagerStorageTableWrapper) UpdateACLs(ctx context.Context, resourceGroup string, accountName string, tableName string, acls []tables.SignedIdentifier) error { + // TODO @magodo: support ACLs once API is available + panic("implement me") +} diff --git a/internal/services/storage/shim/types.go b/internal/services/storage/shim/types.go new file mode 100644 index 000000000000..9ea198157a53 --- /dev/null +++ b/internal/services/storage/shim/types.go @@ -0,0 +1,25 @@ +package shim + +func mapStringPtrToMapString(input map[string]*string) map[string]string { + output := make(map[string]string, len(input)) + + for k, v := range input { + if v == nil { + continue + } + + output[k] = *v + } + + return output +} + +func mapStringToMapStringPtr(input map[string]string) map[string]*string { + output := make(map[string]*string, len(input)) + + for k, v := range input { + output[k] = &v + } + + return output +} diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 72c143b0890e..ff4fed4af2ec 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -9,6 +9,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/shim" azautorest "github.com/Azure/go-autorest/autorest" autorestAzure "github.com/Azure/go-autorest/autorest/azure" "github.com/hashicorp/go-azure-helpers/lang/response" @@ -1079,10 +1080,13 @@ func resourceStorageAccountCreate(d *pluginsdk.ResourceData, meta interface{}) e return fmt.Errorf("Unable to locate Storage Account %q!", storageAccountName) } - queueClient, err := storageClient.QueuesClient(ctx, *account) + // Currently, only the data plane API of queues fully support the exported properties of the `queue_properties`. + // TODO @magodo: once the mgmt plane API catches up, switch following client to the mgmt plane. + queuesDataPlaneClient, err := storageClient.QueuesDataPlaneClient(ctx, *account) if err != nil { return fmt.Errorf("building Queues Client: %s", err) } + queueClient := shim.NewDataPlaneStorageQueueWrapper(queuesDataPlaneClient).(shim.DataPlaneStorageQueueWrapper) queueProperties, err := expandQueueProperties(val.([]interface{})) if err != nil { @@ -1453,10 +1457,13 @@ func resourceStorageAccountUpdate(d *pluginsdk.ResourceData, meta interface{}) e return fmt.Errorf("Unable to locate Storage Account %q!", storageAccountName) } - queueClient, err := storageClient.QueuesClient(ctx, *account) + // Currently, only the data plane API of queues fully support the exported properties of the `queue_properties`. + // TODO @magodo: once the mgmt plane API catches up, switch following client to the mgmt plane. + queuesDataPlaneClient, err := storageClient.QueuesDataPlaneClient(ctx, *account) if err != nil { return fmt.Errorf("building Queues Client: %s", err) } + queueClient := shim.NewDataPlaneStorageQueueWrapper(queuesDataPlaneClient).(shim.DataPlaneStorageQueueWrapper) queueProperties, err := expandQueueProperties(d.Get("queue_properties").([]interface{})) if err != nil { @@ -1742,10 +1749,14 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err if resp.Sku.Tier == storage.SkuTierStandard { if resp.Kind == storage.KindStorage || resp.Kind == storage.KindStorageV2 { - queueClient, err := storageClient.QueuesClient(ctx, *account) + // Currently, only the data plane API of queues fully support the exported properties of the `queue_properties`. + // TODO @magodo: once the mgmt plane API catches up, switch following client to the mgmt plane. + queuesDataPlaneClient, err := storageClient.QueuesDataPlaneClient(ctx, *account) if err != nil { return fmt.Errorf("building Queues Client: %s", err) } + queueClient := shim.NewDataPlaneStorageQueueWrapper(queuesDataPlaneClient).(shim.DataPlaneStorageQueueWrapper) + queueProps, err := queueClient.GetServiceProperties(ctx, account.ResourceGroup, storageAccountName) if err != nil { From 907ca7b9055c734d3700391bbbd1b229f4056f74 Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 17 Nov 2021 11:12:23 +0800 Subject: [PATCH 02/19] provider feature: add `storage` for the `use_resource_manager` toggle --- internal/features/defaults.go | 3 + internal/features/user_flags.go | 5 ++ internal/provider/features.go | 24 +++++++ internal/provider/features_test.go | 84 ++++++++++++++++++++++ internal/services/storage/client/client.go | 2 +- 5 files changed, 117 insertions(+), 1 deletion(-) diff --git a/internal/features/defaults.go b/internal/features/defaults.go index 8f16083fe539..4a7e0dfc016a 100644 --- a/internal/features/defaults.go +++ b/internal/features/defaults.go @@ -35,5 +35,8 @@ func Default() UserFeatures { RollInstancesWhenRequired: true, ScaleToZeroOnDelete: true, }, + Storage: StorageFeatures{ + UseResourceManager: false, + }, } } diff --git a/internal/features/user_flags.go b/internal/features/user_flags.go index e9fc0c0f1353..241705e41019 100644 --- a/internal/features/user_flags.go +++ b/internal/features/user_flags.go @@ -10,6 +10,7 @@ type UserFeatures struct { TemplateDeployment TemplateDeploymentFeatures LogAnalyticsWorkspace LogAnalyticsWorkspaceFeatures ResourceGroup ResourceGroupFeatures + Storage StorageFeatures } type CognitiveAccountFeatures struct { @@ -52,3 +53,7 @@ type ResourceGroupFeatures struct { type ApiManagementFeatures struct { PurgeSoftDeleteOnDestroy bool } + +type StorageFeatures struct { + UseResourceManager bool +} diff --git a/internal/provider/features.go b/internal/provider/features.go index 1b5338679eb2..36bf8bf453d9 100644 --- a/internal/provider/features.go +++ b/internal/provider/features.go @@ -157,6 +157,20 @@ func schemaFeatures(supportLegacyTestSuite bool) *pluginsdk.Schema { }, }, }, + + "storage": { + Type: pluginsdk.TypeList, + Optional: true, + MaxItems: 1, + Elem: &pluginsdk.Resource{ + Schema: map[string]*schema.Schema{ + "use_resource_manager": { + Type: pluginsdk.TypeBool, + Optional: true, + }, + }, + }, + }, } // this is a temporary hack to enable us to gradually add provider blocks to test configurations @@ -297,5 +311,15 @@ func expandFeatures(input []interface{}) features.UserFeatures { } } + if raw, ok := val["storage"]; ok { + items := raw.([]interface{}) + if len(items) > 0 { + storageRaw := items[0].(map[string]interface{}) + if v, ok := storageRaw["use_resource_manager"]; ok { + features.Storage.UseResourceManager = v.(bool) + } + } + } + return features } diff --git a/internal/provider/features_test.go b/internal/provider/features_test.go index a3c0fde7f64e..cc807d10153d 100644 --- a/internal/provider/features_test.go +++ b/internal/provider/features_test.go @@ -50,6 +50,9 @@ func TestExpandFeatures(t *testing.T) { ResourceGroup: features.ResourceGroupFeatures{ PreventDeletionIfContainsResources: false, }, + Storage: features.StorageFeatures{ + UseResourceManager: false, + }, }, }, { @@ -106,6 +109,11 @@ func TestExpandFeatures(t *testing.T) { "scale_to_zero_before_deletion": true, }, }, + "storage": []interface{}{ + map[string]interface{}{ + "use_resource_manager": true, + }, + }, }, }, Expected: features.UserFeatures{ @@ -141,6 +149,9 @@ func TestExpandFeatures(t *testing.T) { ForceDelete: true, ScaleToZeroOnDelete: true, }, + Storage: features.StorageFeatures{ + UseResourceManager: true, + }, }, }, { @@ -197,6 +208,11 @@ func TestExpandFeatures(t *testing.T) { "scale_to_zero_before_deletion": false, }, }, + "storage": []interface{}{ + map[string]interface{}{ + "use_resource_manager": false, + }, + }, }, }, Expected: features.UserFeatures{ @@ -232,6 +248,9 @@ func TestExpandFeatures(t *testing.T) { RollInstancesWhenRequired: false, ScaleToZeroOnDelete: false, }, + Storage: features.StorageFeatures{ + UseResourceManager: false, + }, }, }, } @@ -935,3 +954,68 @@ func TestExpandFeaturesResourceGroup(t *testing.T) { } } } + +func TestExpandFeaturesStorage(t *testing.T) { + testData := []struct { + Name string + Input []interface{} + EnvVars map[string]interface{} + Expected features.UserFeatures + }{ + { + Name: "Empty Block", + Input: []interface{}{ + map[string]interface{}{ + "use_resource_manager": []interface{}{}, + }, + }, + Expected: features.UserFeatures{ + Storage: features.StorageFeatures{ + UseResourceManager: false, + }, + }, + }, + { + Name: "Use Resource Manager Enabled", + Input: []interface{}{ + map[string]interface{}{ + "storage": []interface{}{ + map[string]interface{}{ + "use_resource_manager": true, + }, + }, + }, + }, + Expected: features.UserFeatures{ + Storage: features.StorageFeatures{ + UseResourceManager: true, + }, + }, + }, + { + Name: "Use Resource Manager Disabled", + Input: []interface{}{ + map[string]interface{}{ + "storage": []interface{}{ + map[string]interface{}{ + "use_resource_manager": false, + }, + }, + }, + }, + Expected: features.UserFeatures{ + Storage: features.StorageFeatures{ + UseResourceManager: false, + }, + }, + }, + } + + for _, testCase := range testData { + t.Logf("[DEBUG] Test Case: %q", testCase.Name) + result := expandFeatures(testCase.Input) + if !reflect.DeepEqual(result.Storage, testCase.Expected.Storage) { + t.Fatalf("Expected %+v but got %+v", result.Storage, testCase.Expected.Storage) + } + } +} diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index 30106ea98ad2..411a5016d3a1 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -118,7 +118,7 @@ func NewClient(options *common.ClientOptions) *Client { resourceManagerAuthorizer: options.ResourceManagerAuthorizer, - useResourceManager: false, // TODO: feature toggleable + useResourceManager: options.Features.Storage.UseResourceManager, } if options.StorageUseAzureAD { From 3ac3a60b28b1f0d2714535391bc0e0975ad74408 Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 17 Nov 2021 15:37:44 +0800 Subject: [PATCH 03/19] pass tests for container/queue/shares --- .../storage/shim/shares_mgmt_plane.go | 9 +- internal/services/storage/shim/types.go | 1 + .../storage_container_resource_test.go | 139 ++++++++++- .../storage/storage_queue_resource_test.go | 74 +++++- .../storage/storage_share_resource_test.go | 219 +++++++++++++++++- 5 files changed, 426 insertions(+), 16 deletions(-) diff --git a/internal/services/storage/shim/shares_mgmt_plane.go b/internal/services/storage/shim/shares_mgmt_plane.go index bb4d63e1cf89..84b03540328e 100644 --- a/internal/services/storage/shim/shares_mgmt_plane.go +++ b/internal/services/storage/shim/shares_mgmt_plane.go @@ -78,11 +78,18 @@ func (w ResourceManagerStorageShareWrapper) Get(ctx context.Context, resourceGro quotaGB = int(*quotaPtr) } + // Currently, the service won't return the `enabledProtocols` in the GET response when set to `SMB`. See: https://github.com/Azure/azure-rest-api-specs/issues/16782. + // Once above issue being addressed, we can remove below code. + protocol := shares.SMB + if share.FileShareProperties.EnabledProtocols != "" { + protocol = shares.ShareProtocol(share.FileShareProperties.EnabledProtocols) + } + output := StorageShareProperties{ ACLs: w.mapToACLs(share.FileShareProperties.SignedIdentifiers), MetaData: mapStringPtrToMapString(share.FileShareProperties.Metadata), QuotaGB: quotaGB, - EnabledProtocol: shares.ShareProtocol(share.FileShareProperties.EnabledProtocols), + EnabledProtocol: protocol, } return &output, nil } diff --git a/internal/services/storage/shim/types.go b/internal/services/storage/shim/types.go index 9ea198157a53..bb7deea34c4c 100644 --- a/internal/services/storage/shim/types.go +++ b/internal/services/storage/shim/types.go @@ -18,6 +18,7 @@ func mapStringToMapStringPtr(input map[string]string) map[string]*string { output := make(map[string]*string, len(input)) for k, v := range input { + v := v output[k] = &v } diff --git a/internal/services/storage/storage_container_resource_test.go b/internal/services/storage/storage_container_resource_test.go index 9cdad510a31b..2fb2f7c320ca 100644 --- a/internal/services/storage/storage_container_resource_test.go +++ b/internal/services/storage/storage_container_resource_test.go @@ -15,7 +15,9 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" ) -type StorageContainerResource struct{} +type StorageContainerResource struct { + useResourceManager bool +} func TestAccStorageContainer_basic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") @@ -183,6 +185,133 @@ func TestAccStorageContainer_web(t *testing.T) { }) } +func TestAccStorageContainer_mgmtBasic(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") + r := StorageContainerResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageContainer_mgmtUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") + r := StorageContainerResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.update(data, "private"), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("container_access_type").HasValue("private"), + ), + }, + data.ImportStep(), + { + Config: r.update(data, "container"), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("container_access_type").HasValue("container"), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageContainer_mgmtMetaData(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") + r := StorageContainerResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.metaData(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.metaDataUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.metaDataEmpty(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +// TODO: The mgmt plane API is forbidden to destroy a root container. +// See: https://github.com/Azure/azure-rest-api-specs/issues/16783 +//func TestAccStorageContainer_mgmtRoot(t *testing.T) { +// data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") +// r := StorageContainerResource{useResourceManager: true} +// +// data.ResourceTest(t, r, []acceptance.TestStep{ +// { +// Config: r.root(data), +// Check: acceptance.ComposeTestCheckFunc( +// check.That(data.ResourceName).ExistsInAzure(r), +// check.That(data.ResourceName).Key("name").HasValue("$root"), +// ), +// }, +// data.ImportStep(), +// }) +//} + +func TestAccStorageContainer_mgmtWeb(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") + r := StorageContainerResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.web(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("name").HasValue("$web"), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageContainer_crossPlaneUpdate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") + r := StorageContainerResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.update(data, "private"), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + PreConfig: func() { + r.useResourceManager = true + }, + Config: r.update(data, "container"), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func (r StorageContainerResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := parse.StorageContainerDataPlaneID(state.ID) if err != nil { @@ -379,7 +508,11 @@ resource "azurerm_storage_container" "test" { func (r StorageContainerResource) template(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -399,7 +532,7 @@ resource "azurerm_storage_account" "test" { environment = "staging" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomString) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString) } func TestValidateStorageContainerName(t *testing.T) { diff --git a/internal/services/storage/storage_queue_resource_test.go b/internal/services/storage/storage_queue_resource_test.go index 378c365402d9..2a6c9f8a19ac 100644 --- a/internal/services/storage/storage_queue_resource_test.go +++ b/internal/services/storage/storage_queue_resource_test.go @@ -13,7 +13,9 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" ) -type StorageQueueResource struct{} +type StorageQueueResource struct { + useResourceManager bool +} func TestAccStorageQueue_basic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") @@ -82,6 +84,68 @@ func TestAccStorageQueue_metaData(t *testing.T) { }) } +func TestAccStorageQueue_mgmtBasic(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") + r := StorageQueueResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageQueue_mgmtMetaData(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") + r := StorageQueueResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.metaData(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.metaDataUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageQueue_crossPlaneMetaData(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") + r := StorageQueueResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.metaData(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + PreConfig: func() { + r.useResourceManager = true + }, + Config: r.metaDataUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func (r StorageQueueResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := parse.StorageQueueDataPlaneID(state.ID) if err != nil { @@ -196,7 +260,11 @@ resource "azurerm_storage_queue" "test" { func (r StorageQueueResource) template(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -215,5 +283,5 @@ resource "azurerm_storage_account" "test" { environment = "staging" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomString) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString) } diff --git a/internal/services/storage/storage_share_resource_test.go b/internal/services/storage/storage_share_resource_test.go index aa3bef39bd83..1fecbc29700e 100644 --- a/internal/services/storage/storage_share_resource_test.go +++ b/internal/services/storage/storage_share_resource_test.go @@ -13,7 +13,9 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" ) -type StorageShareResource struct{} +type StorageShareResource struct { + useResourceManager bool +} func TestAccStorageShare_basic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") @@ -200,6 +202,189 @@ func TestAccStorageShare_nfsProtocol(t *testing.T) { }) } +func TestAccStorageShare_mgmtBasic(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_mgmtMetaData(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.metaData(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.metaDataUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +// TODO: uncomment following two test cases after https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed. +//func TestAccStorageShare_mgmtAcl(t *testing.T) { +// data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") +// r := StorageShareResource{useResourceManager: true} +// +// data.ResourceTest(t, r, []acceptance.TestStep{ +// { +// Config: r.acl(data), +// Check: acceptance.ComposeTestCheckFunc( +// check.That(data.ResourceName).ExistsInAzure(r), +// ), +// }, +// data.ImportStep(), +// { +// Config: r.aclUpdated(data), +// Check: acceptance.ComposeTestCheckFunc( +// check.That(data.ResourceName).ExistsInAzure(r), +// ), +// }, +// data.ImportStep(), +// }) +//} +// +//func TestAccStorageShare_mgmtAclGhostedRecall(t *testing.T) { +// data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") +// r := StorageShareResource{useResourceManager: true} +// +// data.ResourceTest(t, r, []acceptance.TestStep{ +// { +// Config: r.aclGhostedRecall(data), +// Check: acceptance.ComposeTestCheckFunc( +// check.That(data.ResourceName).ExistsInAzure(r), +// ), +// }, +// data.ImportStep(), +// }) +//} + +func TestAccStorageShare_mgmtUpdateQuota(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + { + Config: r.updateQuota(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("quota").HasValue("5"), + ), + }, + }) +} + +func TestAccStorageShare_mgmtLargeQuota(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.largeQuota(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.largeQuotaUpdate(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_mgmtNfsProtocol(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.nfsProtocol(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_crossPlaneMetaData(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.metaData(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + PreConfig: func() { + r.useResourceManager = true + }, + Config: r.metaDataUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_crossPlaneAcl(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.acl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + PreConfig: func() { + r.useResourceManager = true + }, + Config: r.aclUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func (r StorageShareResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := parse.StorageShareDataPlaneID(state.ID) if err != nil { @@ -395,7 +580,11 @@ resource "azurerm_storage_share" "test" { func (r StorageShareResource) largeQuota(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -421,13 +610,17 @@ resource "azurerm_storage_share" "test" { storage_account_name = azurerm_storage_account.test.name quota = 6000 } -`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) } func (r StorageShareResource) largeQuotaUpdate(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -453,13 +646,17 @@ resource "azurerm_storage_share" "test" { storage_account_name = azurerm_storage_account.test.name quota = 10000 } -`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) } func (r StorageShareResource) nfsProtocol(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -481,13 +678,17 @@ resource "azurerm_storage_share" "test" { storage_account_name = azurerm_storage_account.test.name enabled_protocol = "NFS" } -`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString) } func (r StorageShareResource) template(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -506,5 +707,5 @@ resource "azurerm_storage_account" "test" { environment = "staging" } } -`, data.RandomInteger, data.Locations.Primary, data.RandomString) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString) } From 02b9bb2efd34e004c2067c199f818affb3c8088f Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 17 Nov 2021 15:46:57 +0800 Subject: [PATCH 04/19] change comment --- internal/services/storage/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index 411a5016d3a1..f61bd0425feb 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -48,7 +48,7 @@ type Client struct { // useResourceManager specifies whether to use the mgmt plane API for resources that have both data plane and mgmt plane support. // Currently, only the following resources are affected: blob container, file share, queue. - // TODO: data lake filesystem/path, table. + // TODO: table. useResourceManager bool } From c1d3357a7a905a7b16d3811fb41a9210c2d4b64a Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 17 Nov 2021 16:05:28 +0800 Subject: [PATCH 05/19] fmt --- internal/services/storage/storage_account_resource.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index ff4fed4af2ec..f4b1c43dbf2c 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -9,7 +9,6 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" - "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/shim" azautorest "github.com/Azure/go-autorest/autorest" autorestAzure "github.com/Azure/go-autorest/autorest/azure" "github.com/hashicorp/go-azure-helpers/lang/response" @@ -24,6 +23,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/internal/services/network" "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/migration" "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/parse" + "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/shim" "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tags" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" @@ -1757,7 +1757,6 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err } queueClient := shim.NewDataPlaneStorageQueueWrapper(queuesDataPlaneClient).(shim.DataPlaneStorageQueueWrapper) - queueProps, err := queueClient.GetServiceProperties(ctx, account.ResourceGroup, storageAccountName) if err != nil { return fmt.Errorf("reading queue properties for AzureRM Storage Account %q: %+v", storageAccountName, err) From 9fc467b0009435f6b400d45e8335e7b16bcd4786 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 6 Dec 2021 10:48:25 +0800 Subject: [PATCH 06/19] resolve comments --- internal/services/storage/client/client.go | 13 +- .../storage/shim/containers_mgmt_plane.go | 13 +- .../storage/shim/queues_mgmt_plane.go | 13 +- .../storage/shim/shares_mgmt_plane.go | 13 +- .../storage/shim/tables_mgmt_plane.go | 18 +-- internal/services/storage/shim/types.go | 9 +- .../storage_container_resource_test.go | 79 +++++++---- .../storage/storage_queue_resource_test.go | 34 ++++- .../storage/storage_share_resource_test.go | 128 +++++++++++------- 9 files changed, 197 insertions(+), 123 deletions(-) diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index f61bd0425feb..f9abd8d12e5b 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -128,6 +128,11 @@ func NewClient(options *common.ClientOptions) *Client { return &client } +// A setter to manipulate the "useResourceManager" feature flag. This is only meant for the acc test. +func (client *Client) UseResourceManager(t bool) { + client.useResourceManager = t +} + func (client Client) AccountsDataPlaneClient(ctx context.Context, account accountDetails) (*accounts.Client, error) { if client.storageAdAuth != nil { accountsClient := accounts.NewWithEnvironment(client.Environment) @@ -176,7 +181,7 @@ func (client Client) ContainersClient(ctx context.Context, account accountDetail if client.useResourceManager { rmClient := storage.NewBlobContainersClientWithBaseURI(client.Environment.ResourceManagerEndpoint, client.SubscriptionId) rmClient.Client.Authorizer = client.resourceManagerAuthorizer - return shim.NewMgmtPlaneStorageContainerWrapper(&rmClient), nil + return shim.NewManagementPlaneStorageContainerWrapper(&rmClient), nil } containersClient, err := client.ContainersDataPlaneClient(ctx, account) @@ -248,7 +253,7 @@ func (client Client) FileSharesClient(ctx context.Context, account accountDetail if client.useResourceManager { sharesClient := storage.NewFileSharesClientWithBaseURI(client.Environment.ResourceManagerEndpoint, client.SubscriptionId) sharesClient.Client.Authorizer = client.resourceManagerAuthorizer - return shim.NewMgmtPlaneStorageShareWrapper(&sharesClient), nil + return shim.NewManagementPlaneStorageShareWrapper(&sharesClient), nil } sharesClient, err := client.FileSharesDataPlaneClient(ctx, account) @@ -280,7 +285,7 @@ func (client Client) QueuesClient(ctx context.Context, account accountDetails) ( if client.useResourceManager { queueClient := storage.NewQueueClient(client.SubscriptionId) queueClient.Client.Authorizer = client.resourceManagerAuthorizer - return shim.NewMgmtPlaneStorageQueueWrapper(&queueClient), nil + return shim.NewManagementPlaneStorageQueueWrapper(&queueClient), nil } queuesClient, err := client.QueuesDataPlaneClient(ctx, account) @@ -335,7 +340,7 @@ func (client Client) TablesClient(ctx context.Context, account accountDetails) ( //if client.useResourceManager { // tableClient := storage.NewTableClient(client.SubscriptionId) // tableClient.Client.Authorizer = client.resourceManagerAuthorizer - // return shim.NewMgmtPlaneStorageTableWrapper(&tableClient), nil + // return shim.NewManagementPlaneStorageTableWrapper(&tableClient), nil //} tablesClient, err := client.TablesDataPlaneClient(ctx, account) diff --git a/internal/services/storage/shim/containers_mgmt_plane.go b/internal/services/storage/shim/containers_mgmt_plane.go index 5e126021bddb..42bf5c5d50f1 100644 --- a/internal/services/storage/shim/containers_mgmt_plane.go +++ b/internal/services/storage/shim/containers_mgmt_plane.go @@ -13,7 +13,7 @@ type ResourceManagerStorageContainerWrapper struct { client *storage.BlobContainersClient } -func NewMgmtPlaneStorageContainerWrapper(client *storage.BlobContainersClient) StorageContainerWrapper { +func NewManagementPlaneStorageContainerWrapper(client *storage.BlobContainersClient) StorageContainerWrapper { return ResourceManagerStorageContainerWrapper{ client: client, } @@ -31,15 +31,8 @@ func (w ResourceManagerStorageContainerWrapper) Create(ctx context.Context, reso } func (w ResourceManagerStorageContainerWrapper) Delete(ctx context.Context, resourceGroup, accountName, containerName string) error { - resp, err := w.client.Delete(ctx, resourceGroup, accountName, containerName) - if err != nil { - if utils.ResponseWasNotFound(resp) { - return nil - } - - return err - } - return nil + _, err := w.client.Delete(ctx, resourceGroup, accountName, containerName) + return err } func (w ResourceManagerStorageContainerWrapper) Exists(ctx context.Context, resourceGroup, accountName, containerName string) (*bool, error) { diff --git a/internal/services/storage/shim/queues_mgmt_plane.go b/internal/services/storage/shim/queues_mgmt_plane.go index 5b9ae843066e..9df3e6720ee5 100644 --- a/internal/services/storage/shim/queues_mgmt_plane.go +++ b/internal/services/storage/shim/queues_mgmt_plane.go @@ -13,7 +13,7 @@ type ResourceManagerStorageQueueWrapper struct { client *storage.QueueClient } -func NewMgmtPlaneStorageQueueWrapper(client *storage.QueueClient) StorageQueuesWrapper { +func NewManagementPlaneStorageQueueWrapper(client *storage.QueueClient) StorageQueuesWrapper { return ResourceManagerStorageQueueWrapper{ client: client, } @@ -30,15 +30,8 @@ func (w ResourceManagerStorageQueueWrapper) Create(ctx context.Context, resource } func (w ResourceManagerStorageQueueWrapper) Delete(ctx context.Context, resourceGroup, accountName, queueName string) error { - resp, err := w.client.Delete(ctx, resourceGroup, accountName, queueName) - if err != nil { - if utils.ResponseWasNotFound(resp) { - return nil - } - - return err - } - return nil + _, err := w.client.Delete(ctx, resourceGroup, accountName, queueName) + return err } func (w ResourceManagerStorageQueueWrapper) Exists(ctx context.Context, resourceGroup, accountName, queueName string) (*bool, error) { diff --git a/internal/services/storage/shim/shares_mgmt_plane.go b/internal/services/storage/shim/shares_mgmt_plane.go index 84b03540328e..0342bb27d605 100644 --- a/internal/services/storage/shim/shares_mgmt_plane.go +++ b/internal/services/storage/shim/shares_mgmt_plane.go @@ -16,7 +16,7 @@ type ResourceManagerStorageShareWrapper struct { client *storage.FileSharesClient } -func NewMgmtPlaneStorageShareWrapper(client *storage.FileSharesClient) StorageShareWrapper { +func NewManagementPlaneStorageShareWrapper(client *storage.FileSharesClient) StorageShareWrapper { return ResourceManagerStorageShareWrapper{ client: client, } @@ -35,15 +35,8 @@ func (w ResourceManagerStorageShareWrapper) Create(ctx context.Context, resource } func (w ResourceManagerStorageShareWrapper) Delete(ctx context.Context, resourceGroup, accountName, shareName string) error { - resp, err := w.client.Delete(ctx, resourceGroup, accountName, shareName, "", "") - if err != nil { - if utils.ResponseWasNotFound(resp) { - return nil - } - - return err - } - return nil + _, err := w.client.Delete(ctx, resourceGroup, accountName, shareName, "", "") + return err } func (w ResourceManagerStorageShareWrapper) Exists(ctx context.Context, resourceGroup, accountName, shareName string) (*bool, error) { diff --git a/internal/services/storage/shim/tables_mgmt_plane.go b/internal/services/storage/shim/tables_mgmt_plane.go index 7efd0396761f..097803e69adb 100644 --- a/internal/services/storage/shim/tables_mgmt_plane.go +++ b/internal/services/storage/shim/tables_mgmt_plane.go @@ -2,6 +2,7 @@ package shim import ( "context" + "errors" "github.com/hashicorp/terraform-provider-azurerm/utils" @@ -13,7 +14,7 @@ type ResourceManagerStorageTableWrapper struct { client *storage.TableClient } -func NewMgmtPlaneStorageTableWrapper(client *storage.TableClient) StorageTableWrapper { +func NewManagementPlaneStorageTableWrapper(client *storage.TableClient) StorageTableWrapper { return ResourceManagerStorageTableWrapper{ client: client, } @@ -25,15 +26,8 @@ func (w ResourceManagerStorageTableWrapper) Create(ctx context.Context, resource } func (w ResourceManagerStorageTableWrapper) Delete(ctx context.Context, resourceGroup string, accountName string, tableName string) error { - resp, err := w.client.Delete(ctx, resourceGroup, accountName, tableName) - if err != nil { - if utils.ResponseWasNotFound(resp) { - return nil - } - - return err - } - return nil + _, err := w.client.Delete(ctx, resourceGroup, accountName, tableName) + return err } func (w ResourceManagerStorageTableWrapper) Exists(ctx context.Context, resourceGroup string, accountName string, tableName string) (*bool, error) { @@ -51,10 +45,10 @@ func (w ResourceManagerStorageTableWrapper) Exists(ctx context.Context, resource func (w ResourceManagerStorageTableWrapper) GetACLs(ctx context.Context, resourceGroup string, accountName string, tableName string) (*[]tables.SignedIdentifier, error) { // TODO @magodo: support ACLs once API is available - panic("implement me") + return nil, errors.New("Storage Table management plane API doesn't support ACLs now") } func (w ResourceManagerStorageTableWrapper) UpdateACLs(ctx context.Context, resourceGroup string, accountName string, tableName string, acls []tables.SignedIdentifier) error { // TODO @magodo: support ACLs once API is available - panic("implement me") + return errors.New("Storage Table management plane API doesn't support ACLs now") } diff --git a/internal/services/storage/shim/types.go b/internal/services/storage/shim/types.go index bb7deea34c4c..35957c672b81 100644 --- a/internal/services/storage/shim/types.go +++ b/internal/services/storage/shim/types.go @@ -1,7 +1,9 @@ package shim +import "github.com/hashicorp/terraform-provider-azurerm/utils" + func mapStringPtrToMapString(input map[string]*string) map[string]string { - output := make(map[string]string, len(input)) + output := make(map[string]string, 0) for k, v := range input { if v == nil { @@ -15,11 +17,10 @@ func mapStringPtrToMapString(input map[string]*string) map[string]string { } func mapStringToMapStringPtr(input map[string]string) map[string]*string { - output := make(map[string]*string, len(input)) + output := make(map[string]*string, 0) for k, v := range input { - v := v - output[k] = &v + output[k] = utils.String(v) } return output diff --git a/internal/services/storage/storage_container_resource_test.go b/internal/services/storage/storage_container_resource_test.go index 2fb2f7c320ca..1256dc9a1bdc 100644 --- a/internal/services/storage/storage_container_resource_test.go +++ b/internal/services/storage/storage_container_resource_test.go @@ -185,7 +185,7 @@ func TestAccStorageContainer_web(t *testing.T) { }) } -func TestAccStorageContainer_mgmtBasic(t *testing.T) { +func TestAccStorageContainer_resourceManangerBasic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -200,7 +200,7 @@ func TestAccStorageContainer_mgmtBasic(t *testing.T) { }) } -func TestAccStorageContainer_mgmtUpdate(t *testing.T) { +func TestAccStorageContainer_resourceManangerUpdate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -224,7 +224,7 @@ func TestAccStorageContainer_mgmtUpdate(t *testing.T) { }) } -func TestAccStorageContainer_mgmtMetaData(t *testing.T) { +func TestAccStorageContainer_resourceManangerMetaData(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -253,25 +253,28 @@ func TestAccStorageContainer_mgmtMetaData(t *testing.T) { }) } -// TODO: The mgmt plane API is forbidden to destroy a root container. -// See: https://github.com/Azure/azure-rest-api-specs/issues/16783 -//func TestAccStorageContainer_mgmtRoot(t *testing.T) { -// data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") -// r := StorageContainerResource{useResourceManager: true} -// -// data.ResourceTest(t, r, []acceptance.TestStep{ -// { -// Config: r.root(data), -// Check: acceptance.ComposeTestCheckFunc( -// check.That(data.ResourceName).ExistsInAzure(r), -// check.That(data.ResourceName).Key("name").HasValue("$root"), -// ), -// }, -// data.ImportStep(), -// }) -//} - -func TestAccStorageContainer_mgmtWeb(t *testing.T) { +func TestAccStorageContainer_resourceManangerRoot(t *testing.T) { + // TODO: The mgmt plane API is forbidden to destroy a root container. + // See: https://github.com/Azure/azure-rest-api-specs/issues/16783 + // Once that issue is fixed, we can enable this test case. + t.Skip() + + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") + r := StorageContainerResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.root(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("name").HasValue("$root"), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageContainer_resourceManangerWeb(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -287,7 +290,7 @@ func TestAccStorageContainer_mgmtWeb(t *testing.T) { }) } -func TestAccStorageContainer_crossPlaneUpdate(t *testing.T) { +func TestAccStorageContainer_dataPlaneThenResourceManager(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{} @@ -312,6 +315,31 @@ func TestAccStorageContainer_crossPlaneUpdate(t *testing.T) { }) } +func TestAccStorageContainer_resourceManagerThenDataPlane(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") + r := StorageContainerResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.update(data, "private"), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + PreConfig: func() { + r.useResourceManager = false + }, + Config: r.update(data, "container"), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func (r StorageContainerResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := parse.StorageContainerDataPlaneID(state.ID) if err != nil { @@ -325,6 +353,8 @@ func (r StorageContainerResource) Exists(ctx context.Context, client *clients.Cl return nil, fmt.Errorf("unable to locate Storage Account %q", id.AccountName) } + client.Storage.UseResourceManager(r.useResourceManager) + containersClient, err := client.Storage.ContainersClient(ctx, *account) if err != nil { return nil, fmt.Errorf("building Containers Client: %+v", err) @@ -348,6 +378,9 @@ func (r StorageContainerResource) Destroy(ctx context.Context, client *clients.C if account == nil { return nil, fmt.Errorf("unable to locate Storage Account %q", id.AccountName) } + + client.Storage.UseResourceManager(r.useResourceManager) + containersClient, err := client.Storage.ContainersClient(ctx, *account) if err != nil { return nil, fmt.Errorf("building Containers Client: %+v", err) diff --git a/internal/services/storage/storage_queue_resource_test.go b/internal/services/storage/storage_queue_resource_test.go index 2a6c9f8a19ac..c49e8651312f 100644 --- a/internal/services/storage/storage_queue_resource_test.go +++ b/internal/services/storage/storage_queue_resource_test.go @@ -84,7 +84,7 @@ func TestAccStorageQueue_metaData(t *testing.T) { }) } -func TestAccStorageQueue_mgmtBasic(t *testing.T) { +func TestAccStorageQueue_resourceManagerBasic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} @@ -99,7 +99,7 @@ func TestAccStorageQueue_mgmtBasic(t *testing.T) { }) } -func TestAccStorageQueue_mgmtMetaData(t *testing.T) { +func TestAccStorageQueue_resourceManagerMetaData(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} @@ -121,7 +121,7 @@ func TestAccStorageQueue_mgmtMetaData(t *testing.T) { }) } -func TestAccStorageQueue_crossPlaneMetaData(t *testing.T) { +func TestAccStorageQueue_dataPlaneThenResourceManagerMetaData(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{} @@ -146,6 +146,31 @@ func TestAccStorageQueue_crossPlaneMetaData(t *testing.T) { }) } +func TestAccStorageQueue_resourceManagerThenDataPlaneMetaData(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") + r := StorageQueueResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.metaData(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + PreConfig: func() { + r.useResourceManager = false + }, + Config: r.metaDataUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func (r StorageQueueResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := parse.StorageQueueDataPlaneID(state.ID) if err != nil { @@ -158,6 +183,9 @@ func (r StorageQueueResource) Exists(ctx context.Context, client *clients.Client if account == nil { return nil, fmt.Errorf("unable to determine Resource Group for Storage Queue %q (Account %q)", id.Name, id.AccountName) } + + client.Storage.UseResourceManager(r.useResourceManager) + queuesClient, err := client.Storage.QueuesClient(ctx, *account) if err != nil { return nil, fmt.Errorf("building Queues Client: %+v", err) diff --git a/internal/services/storage/storage_share_resource_test.go b/internal/services/storage/storage_share_resource_test.go index 1fecbc29700e..7ca1bcaa44a7 100644 --- a/internal/services/storage/storage_share_resource_test.go +++ b/internal/services/storage/storage_share_resource_test.go @@ -202,7 +202,7 @@ func TestAccStorageShare_nfsProtocol(t *testing.T) { }) } -func TestAccStorageShare_mgmtBasic(t *testing.T) { +func TestAccStorageShare_resourceManagerBasic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -217,7 +217,7 @@ func TestAccStorageShare_mgmtBasic(t *testing.T) { }) } -func TestAccStorageShare_mgmtMetaData(t *testing.T) { +func TestAccStorageShare_resourceManagerMetaData(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -239,45 +239,50 @@ func TestAccStorageShare_mgmtMetaData(t *testing.T) { }) } -// TODO: uncomment following two test cases after https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed. -//func TestAccStorageShare_mgmtAcl(t *testing.T) { -// data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") -// r := StorageShareResource{useResourceManager: true} -// -// data.ResourceTest(t, r, []acceptance.TestStep{ -// { -// Config: r.acl(data), -// Check: acceptance.ComposeTestCheckFunc( -// check.That(data.ResourceName).ExistsInAzure(r), -// ), -// }, -// data.ImportStep(), -// { -// Config: r.aclUpdated(data), -// Check: acceptance.ComposeTestCheckFunc( -// check.That(data.ResourceName).ExistsInAzure(r), -// ), -// }, -// data.ImportStep(), -// }) -//} -// -//func TestAccStorageShare_mgmtAclGhostedRecall(t *testing.T) { -// data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") -// r := StorageShareResource{useResourceManager: true} -// -// data.ResourceTest(t, r, []acceptance.TestStep{ -// { -// Config: r.aclGhostedRecall(data), -// Check: acceptance.ComposeTestCheckFunc( -// check.That(data.ResourceName).ExistsInAzure(r), -// ), -// }, -// data.ImportStep(), -// }) -//} - -func TestAccStorageShare_mgmtUpdateQuota(t *testing.T) { +func TestAccStorageShare_resourceManagerAcl(t *testing.T) { + // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. + t.Skip() + + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.acl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.aclUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_resourceManagerAclGhostedRecall(t *testing.T) { + // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. + t.Skip() + + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.aclGhostedRecall(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_resourceManagerUpdateQuota(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -298,7 +303,7 @@ func TestAccStorageShare_mgmtUpdateQuota(t *testing.T) { }) } -func TestAccStorageShare_mgmtLargeQuota(t *testing.T) { +func TestAccStorageShare_resourceManagerLargeQuota(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -320,7 +325,7 @@ func TestAccStorageShare_mgmtLargeQuota(t *testing.T) { }) } -func TestAccStorageShare_mgmtNfsProtocol(t *testing.T) { +func TestAccStorageShare_resourceManagerNfsProtocol(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -335,7 +340,7 @@ func TestAccStorageShare_mgmtNfsProtocol(t *testing.T) { }) } -func TestAccStorageShare_crossPlaneMetaData(t *testing.T) { +func TestAccStorageShare_dataPlaneThenResourceManagerMetaData(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{} @@ -360,9 +365,34 @@ func TestAccStorageShare_crossPlaneMetaData(t *testing.T) { }) } -func TestAccStorageShare_crossPlaneAcl(t *testing.T) { +func TestAccStorageShare_resourceManagerThenDataPlaneMetaData(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") - r := StorageShareResource{} + r := StorageShareResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.metaData(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + PreConfig: func() { + r.useResourceManager = false + }, + Config: r.metaDataUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_resourceManagerThenDataPlaneAcl(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{useResourceManager: true} data.ResourceTest(t, r, []acceptance.TestStep{ { @@ -374,7 +404,7 @@ func TestAccStorageShare_crossPlaneAcl(t *testing.T) { data.ImportStep(), { PreConfig: func() { - r.useResourceManager = true + r.useResourceManager = false }, Config: r.aclUpdated(data), Check: acceptance.ComposeTestCheckFunc( @@ -399,6 +429,8 @@ func (r StorageShareResource) Exists(ctx context.Context, client *clients.Client return nil, fmt.Errorf("unable to determine Account %q for Storage Share %q", id.AccountName, id.Name) } + client.Storage.UseResourceManager(r.useResourceManager) + sharesClient, err := client.Storage.FileSharesClient(ctx, *account) if err != nil { return nil, fmt.Errorf("building File Share Client for Storage Account %q (Resource Group %q): %+v", id.AccountName, account.ResourceGroup, err) @@ -425,6 +457,8 @@ func (r StorageShareResource) Destroy(ctx context.Context, client *clients.Clien return nil, fmt.Errorf("unable to determine Account %q for Storage Share %q", id.AccountName, id.Name) } + client.Storage.UseResourceManager(r.useResourceManager) + sharesClient, err := client.Storage.FileSharesClient(ctx, *account) if err != nil { return nil, fmt.Errorf("building File Share Client for Storage Account %q (Resource Group %q): %+v", id.AccountName, account.ResourceGroup, err) From 64fec6bc9a6c9e2d3335f4c75354ff62317643f1 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 6 Dec 2021 10:57:39 +0800 Subject: [PATCH 07/19] typo --- .../storage/storage_container_resource_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/services/storage/storage_container_resource_test.go b/internal/services/storage/storage_container_resource_test.go index 1256dc9a1bdc..5f6c0a8e4c28 100644 --- a/internal/services/storage/storage_container_resource_test.go +++ b/internal/services/storage/storage_container_resource_test.go @@ -185,7 +185,7 @@ func TestAccStorageContainer_web(t *testing.T) { }) } -func TestAccStorageContainer_resourceManangerBasic(t *testing.T) { +func TestAccStorageContainer_resourceManagerBasic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -200,7 +200,7 @@ func TestAccStorageContainer_resourceManangerBasic(t *testing.T) { }) } -func TestAccStorageContainer_resourceManangerUpdate(t *testing.T) { +func TestAccStorageContainer_resourceManagerUpdate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -224,7 +224,7 @@ func TestAccStorageContainer_resourceManangerUpdate(t *testing.T) { }) } -func TestAccStorageContainer_resourceManangerMetaData(t *testing.T) { +func TestAccStorageContainer_resourceManagerMetaData(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -253,7 +253,7 @@ func TestAccStorageContainer_resourceManangerMetaData(t *testing.T) { }) } -func TestAccStorageContainer_resourceManangerRoot(t *testing.T) { +func TestAccStorageContainer_resourceManagerRoot(t *testing.T) { // TODO: The mgmt plane API is forbidden to destroy a root container. // See: https://github.com/Azure/azure-rest-api-specs/issues/16783 // Once that issue is fixed, we can enable this test case. @@ -274,7 +274,7 @@ func TestAccStorageContainer_resourceManangerRoot(t *testing.T) { }) } -func TestAccStorageContainer_resourceManangerWeb(t *testing.T) { +func TestAccStorageContainer_resourceManagerWeb(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} From 65429fd3a814f5fe7d9e7c0071df8e39046ebe29 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 6 Dec 2021 11:51:45 +0800 Subject: [PATCH 08/19] fix the cross plane test --- .../storage_container_resource_test.go | 12 ++--- .../storage/storage_queue_resource_test.go | 12 ++--- .../storage/storage_share_resource_test.go | 49 +++++++++++++++---- 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/internal/services/storage/storage_container_resource_test.go b/internal/services/storage/storage_container_resource_test.go index 5f6c0a8e4c28..443ac74ef7ac 100644 --- a/internal/services/storage/storage_container_resource_test.go +++ b/internal/services/storage/storage_container_resource_test.go @@ -303,10 +303,10 @@ func TestAccStorageContainer_dataPlaneThenResourceManager(t *testing.T) { }, data.ImportStep(), { - PreConfig: func() { + Config: func() string { r.useResourceManager = true - }, - Config: r.update(data, "container"), + return r.update(data, "container") + }(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -328,10 +328,10 @@ func TestAccStorageContainer_resourceManagerThenDataPlane(t *testing.T) { }, data.ImportStep(), { - PreConfig: func() { + Config: func() string { r.useResourceManager = false - }, - Config: r.update(data, "container"), + return r.update(data, "container") + }(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), diff --git a/internal/services/storage/storage_queue_resource_test.go b/internal/services/storage/storage_queue_resource_test.go index c49e8651312f..a7a8ef02afef 100644 --- a/internal/services/storage/storage_queue_resource_test.go +++ b/internal/services/storage/storage_queue_resource_test.go @@ -134,10 +134,10 @@ func TestAccStorageQueue_dataPlaneThenResourceManagerMetaData(t *testing.T) { }, data.ImportStep(), { - PreConfig: func() { + Config: func() string { r.useResourceManager = true - }, - Config: r.metaDataUpdated(data), + return r.metaDataUpdated(data) + }(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -159,10 +159,10 @@ func TestAccStorageQueue_resourceManagerThenDataPlaneMetaData(t *testing.T) { }, data.ImportStep(), { - PreConfig: func() { + Config: func() string { r.useResourceManager = false - }, - Config: r.metaDataUpdated(data), + return r.metaDataUpdated(data) + }(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), diff --git a/internal/services/storage/storage_share_resource_test.go b/internal/services/storage/storage_share_resource_test.go index 7ca1bcaa44a7..7cd9f166f192 100644 --- a/internal/services/storage/storage_share_resource_test.go +++ b/internal/services/storage/storage_share_resource_test.go @@ -353,10 +353,10 @@ func TestAccStorageShare_dataPlaneThenResourceManagerMetaData(t *testing.T) { }, data.ImportStep(), { - PreConfig: func() { + Config: func() string { r.useResourceManager = true - }, - Config: r.metaDataUpdated(data), + return r.metaDataUpdated(data) + }(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -378,10 +378,38 @@ func TestAccStorageShare_resourceManagerThenDataPlaneMetaData(t *testing.T) { }, data.ImportStep(), { - PreConfig: func() { + Config: func() string { r.useResourceManager = false - }, - Config: r.metaDataUpdated(data), + return r.metaDataUpdated(data) + }(), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageShare_dataPlaneThenResourceManagerAcl(t *testing.T) { + // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. + t.Skip() + + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") + r := StorageShareResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.acl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: func() string { + r.useResourceManager = true + return r.aclUpdated(data) + }(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -391,6 +419,9 @@ func TestAccStorageShare_resourceManagerThenDataPlaneMetaData(t *testing.T) { } func TestAccStorageShare_resourceManagerThenDataPlaneAcl(t *testing.T) { + // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. + t.Skip() + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -403,10 +434,10 @@ func TestAccStorageShare_resourceManagerThenDataPlaneAcl(t *testing.T) { }, data.ImportStep(), { - PreConfig: func() { + Config: func() string { r.useResourceManager = false - }, - Config: r.aclUpdated(data), + return r.aclUpdated(data) + }(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), From e86d3e5e188dc504060d054fc3f59baafee93fab Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 6 Dec 2021 12:41:09 +0800 Subject: [PATCH 09/19] linting --- internal/services/storage/shim/types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/services/storage/shim/types.go b/internal/services/storage/shim/types.go index 35957c672b81..3af5ccc40cbc 100644 --- a/internal/services/storage/shim/types.go +++ b/internal/services/storage/shim/types.go @@ -3,7 +3,7 @@ package shim import "github.com/hashicorp/terraform-provider-azurerm/utils" func mapStringPtrToMapString(input map[string]*string) map[string]string { - output := make(map[string]string, 0) + output := make(map[string]string) for k, v := range input { if v == nil { @@ -17,7 +17,7 @@ func mapStringPtrToMapString(input map[string]*string) map[string]string { } func mapStringToMapStringPtr(input map[string]string) map[string]*string { - output := make(map[string]*string, 0) + output := make(map[string]*string) for k, v := range input { output[k] = utils.String(v) From 3dbb0128da0c2c1bb8dcd07ff0d590cb8928d0c3 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 6 Dec 2021 14:36:13 +0800 Subject: [PATCH 10/19] linter --- internal/services/storage/client/client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index 8bf2eef682cd..4a9bdd6717fb 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -342,12 +342,12 @@ func (client Client) TableEntityClient(ctx context.Context, account accountDetai } func (client Client) TablesClient(ctx context.Context, account accountDetails) (shim.StorageTableWrapper, error) { - //TODO: once mgmt API got ACL support, we can uncomment below - //if client.useResourceManager { + // TODO: once mgmt API got ACL support, we can uncomment below + // if client.useResourceManager { // tableClient := storage.NewTableClient(client.SubscriptionId) // tableClient.Client.Authorizer = client.resourceManagerAuthorizer // return shim.NewManagementPlaneStorageTableWrapper(&tableClient), nil - //} + // } tablesClient, err := client.TablesDataPlaneClient(ctx, account) if err != nil { From 446802b9e38929ddcb382a5d5a765a7a122fce6c Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 8 Dec 2021 10:48:47 +0800 Subject: [PATCH 11/19] make the feature toggle functional for 3.0-beta --- internal/provider/features.go | 22 +++++++----- internal/provider/features_test.go | 16 +++++---- .../storage_container_resource_test.go | 22 ++++++++++++ .../storage/storage_queue_resource_test.go | 13 +++++++ .../storage/storage_share_resource_test.go | 34 +++++++++++++++++++ 5 files changed, 93 insertions(+), 14 deletions(-) diff --git a/internal/provider/features.go b/internal/provider/features.go index df634a92f2d9..26ef04b7d7c4 100644 --- a/internal/provider/features.go +++ b/internal/provider/features.go @@ -164,12 +164,7 @@ func schemaFeatures(supportLegacyTestSuite bool) *pluginsdk.Schema { Optional: true, MaxItems: 1, Elem: &pluginsdk.Resource{ - Schema: map[string]*schema.Schema{ - "use_resource_manager": { - Type: pluginsdk.TypeBool, - Optional: true, - }, - }, + Schema: map[string]*schema.Schema{}, }, }, } @@ -225,6 +220,14 @@ func schemaFeatures(supportLegacyTestSuite bool) *pluginsdk.Schema { } } + if features.ThreePointOhBetaResources() { + f := featuresMap["storage"].Elem.(*pluginsdk.Resource) + f.Schema["use_resource_manager"] = &pluginsdk.Schema{ + Type: pluginsdk.TypeBool, + Optional: true, + } + } + // this is a temporary hack to enable us to gradually add provider blocks to test configurations // rather than doing it as a big-bang and breaking all open PR's if supportLegacyTestSuite { @@ -398,8 +401,11 @@ func expandFeatures(input []interface{}) features.UserFeatures { items := raw.([]interface{}) if len(items) > 0 { storageRaw := items[0].(map[string]interface{}) - if v, ok := storageRaw["use_resource_manager"]; ok { - featuresMap.Storage.UseResourceManager = v.(bool) + + if features.ThreePointOhBetaResources() { + if v, ok := storageRaw["use_resource_manager"]; ok { + featuresMap.Storage.UseResourceManager = v.(bool) + } } } } diff --git a/internal/provider/features_test.go b/internal/provider/features_test.go index 7cc2b21dfdca..ebfab9ba580f 100644 --- a/internal/provider/features_test.go +++ b/internal/provider/features_test.go @@ -123,7 +123,8 @@ func TestExpandFeatures(t *testing.T) { }, "storage": []interface{}{ map[string]interface{}{ - "use_resource_manager": true, + // TODO 3.0: uncomment this + //"use_resource_manager": true, }, }, }, @@ -168,7 +169,7 @@ func TestExpandFeatures(t *testing.T) { ScaleToZeroOnDelete: true, }, Storage: features.StorageFeatures{ - UseResourceManager: true, + UseResourceManager: false, }, }, }, @@ -234,7 +235,8 @@ func TestExpandFeatures(t *testing.T) { }, "storage": []interface{}{ map[string]interface{}{ - "use_resource_manager": false, + // TODO 3.0: uncomment this + //"use_resource_manager": false, }, }, }, @@ -1041,14 +1043,15 @@ func TestExpandFeaturesStorage(t *testing.T) { map[string]interface{}{ "storage": []interface{}{ map[string]interface{}{ - "use_resource_manager": true, + // TODO 3.0: uncomment this + //"use_resource_manager": true, }, }, }, }, Expected: features.UserFeatures{ Storage: features.StorageFeatures{ - UseResourceManager: true, + UseResourceManager: false, }, }, }, @@ -1058,7 +1061,8 @@ func TestExpandFeaturesStorage(t *testing.T) { map[string]interface{}{ "storage": []interface{}{ map[string]interface{}{ - "use_resource_manager": false, + // TODO 3.0: uncomment this + //"use_resource_manager": false, }, }, }, diff --git a/internal/services/storage/storage_container_resource_test.go b/internal/services/storage/storage_container_resource_test.go index 443ac74ef7ac..d7d2c0bc02e2 100644 --- a/internal/services/storage/storage_container_resource_test.go +++ b/internal/services/storage/storage_container_resource_test.go @@ -3,6 +3,7 @@ package storage_test import ( "context" "fmt" + "os" "strings" "testing" @@ -186,6 +187,9 @@ func TestAccStorageContainer_web(t *testing.T) { } func TestAccStorageContainer_resourceManagerBasic(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -201,6 +205,9 @@ func TestAccStorageContainer_resourceManagerBasic(t *testing.T) { } func TestAccStorageContainer_resourceManagerUpdate(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -225,6 +232,9 @@ func TestAccStorageContainer_resourceManagerUpdate(t *testing.T) { } func TestAccStorageContainer_resourceManagerMetaData(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -259,6 +269,9 @@ func TestAccStorageContainer_resourceManagerRoot(t *testing.T) { // Once that issue is fixed, we can enable this test case. t.Skip() + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -275,6 +288,9 @@ func TestAccStorageContainer_resourceManagerRoot(t *testing.T) { } func TestAccStorageContainer_resourceManagerWeb(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -291,6 +307,9 @@ func TestAccStorageContainer_resourceManagerWeb(t *testing.T) { } func TestAccStorageContainer_dataPlaneThenResourceManager(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{} @@ -316,6 +335,9 @@ func TestAccStorageContainer_dataPlaneThenResourceManager(t *testing.T) { } func TestAccStorageContainer_resourceManagerThenDataPlane(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} diff --git a/internal/services/storage/storage_queue_resource_test.go b/internal/services/storage/storage_queue_resource_test.go index a7a8ef02afef..e894d5468e8a 100644 --- a/internal/services/storage/storage_queue_resource_test.go +++ b/internal/services/storage/storage_queue_resource_test.go @@ -3,6 +3,7 @@ package storage_test import ( "context" "fmt" + "os" "testing" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" @@ -85,6 +86,9 @@ func TestAccStorageQueue_metaData(t *testing.T) { } func TestAccStorageQueue_resourceManagerBasic(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} @@ -100,6 +104,9 @@ func TestAccStorageQueue_resourceManagerBasic(t *testing.T) { } func TestAccStorageQueue_resourceManagerMetaData(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} @@ -122,6 +129,9 @@ func TestAccStorageQueue_resourceManagerMetaData(t *testing.T) { } func TestAccStorageQueue_dataPlaneThenResourceManagerMetaData(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{} @@ -147,6 +157,9 @@ func TestAccStorageQueue_dataPlaneThenResourceManagerMetaData(t *testing.T) { } func TestAccStorageQueue_resourceManagerThenDataPlaneMetaData(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} diff --git a/internal/services/storage/storage_share_resource_test.go b/internal/services/storage/storage_share_resource_test.go index 7cd9f166f192..449ce9eb2b37 100644 --- a/internal/services/storage/storage_share_resource_test.go +++ b/internal/services/storage/storage_share_resource_test.go @@ -3,6 +3,7 @@ package storage_test import ( "context" "fmt" + "os" "testing" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" @@ -203,6 +204,9 @@ func TestAccStorageShare_nfsProtocol(t *testing.T) { } func TestAccStorageShare_resourceManagerBasic(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -218,6 +222,9 @@ func TestAccStorageShare_resourceManagerBasic(t *testing.T) { } func TestAccStorageShare_resourceManagerMetaData(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -243,6 +250,9 @@ func TestAccStorageShare_resourceManagerAcl(t *testing.T) { // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. t.Skip() + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -268,6 +278,9 @@ func TestAccStorageShare_resourceManagerAclGhostedRecall(t *testing.T) { // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. t.Skip() + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -283,6 +296,9 @@ func TestAccStorageShare_resourceManagerAclGhostedRecall(t *testing.T) { } func TestAccStorageShare_resourceManagerUpdateQuota(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -304,6 +320,9 @@ func TestAccStorageShare_resourceManagerUpdateQuota(t *testing.T) { } func TestAccStorageShare_resourceManagerLargeQuota(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -326,6 +345,9 @@ func TestAccStorageShare_resourceManagerLargeQuota(t *testing.T) { } func TestAccStorageShare_resourceManagerNfsProtocol(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -341,6 +363,9 @@ func TestAccStorageShare_resourceManagerNfsProtocol(t *testing.T) { } func TestAccStorageShare_dataPlaneThenResourceManagerMetaData(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{} @@ -366,6 +391,9 @@ func TestAccStorageShare_dataPlaneThenResourceManagerMetaData(t *testing.T) { } func TestAccStorageShare_resourceManagerThenDataPlaneMetaData(t *testing.T) { + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} @@ -394,6 +422,9 @@ func TestAccStorageShare_dataPlaneThenResourceManagerAcl(t *testing.T) { // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. t.Skip() + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{} @@ -422,6 +453,9 @@ func TestAccStorageShare_resourceManagerThenDataPlaneAcl(t *testing.T) { // TODO: Once https://github.com/Azure/azure-rest-api-specs/issues/16782 is addressed, we can enable this test case. t.Skip() + // The storage "use_resource_manager" feature is only available in 3.0 + os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") + data := acceptance.BuildTestData(t, "azurerm_storage_share", "test") r := StorageShareResource{useResourceManager: true} From cc7089b2ff455b050382f7126d6ce941af464d14 Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 8 Dec 2021 11:03:00 +0800 Subject: [PATCH 12/19] storage account: directly use queue data plane client --- internal/services/storage/storage_account_resource.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 6e102279946d..e83db34a8c51 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -1535,14 +1535,13 @@ func resourceStorageAccountUpdate(d *pluginsdk.ResourceData, meta interface{}) e if err != nil { return fmt.Errorf("building Queues Client: %s", err) } - queueClient := shim.NewDataPlaneStorageQueueWrapper(queuesDataPlaneClient).(shim.DataPlaneStorageQueueWrapper) queueProperties, err := expandQueueProperties(d.Get("queue_properties").([]interface{})) if err != nil { return fmt.Errorf("expanding `queue_properties` for Azure Storage Account %q: %+v", id.Name, err) } - if err = queueClient.UpdateServiceProperties(ctx, account.ResourceGroup, id.Name, queueProperties); err != nil { + if _, err := queuesDataPlaneClient.SetServiceProperties(ctx, id.Name, queueProperties); err != nil { return fmt.Errorf("updating Queue Properties for Storage Account %q: %+v", id.Name, err) } } @@ -1842,14 +1841,13 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err if err != nil { return fmt.Errorf("building Queues Client: %s", err) } - queueClient := shim.NewDataPlaneStorageQueueWrapper(queuesDataPlaneClient).(shim.DataPlaneStorageQueueWrapper) - queueProps, err := queueClient.GetServiceProperties(ctx, account.ResourceGroup, id.Name) + resp, err := queuesDataPlaneClient.GetServiceProperties(ctx, id.Name) if err != nil { return fmt.Errorf("reading queue properties for AzureRM Storage Account %q: %+v", id.Name, err) } - if err := d.Set("queue_properties", flattenQueueProperties(queueProps)); err != nil { + if err := d.Set("queue_properties", flattenQueueProperties(&resp.StorageServiceProperties)); err != nil { return fmt.Errorf("setting `queue_properties`: %+v", err) } } From d232cf01e3851f287f5699b660b113ebf472f0de Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 8 Dec 2021 12:09:04 +0800 Subject: [PATCH 13/19] lint --- internal/provider/features_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/provider/features_test.go b/internal/provider/features_test.go index ebfab9ba580f..f3c5da6a17b1 100644 --- a/internal/provider/features_test.go +++ b/internal/provider/features_test.go @@ -124,7 +124,7 @@ func TestExpandFeatures(t *testing.T) { "storage": []interface{}{ map[string]interface{}{ // TODO 3.0: uncomment this - //"use_resource_manager": true, + // "use_resource_manager": true, }, }, }, @@ -236,7 +236,7 @@ func TestExpandFeatures(t *testing.T) { "storage": []interface{}{ map[string]interface{}{ // TODO 3.0: uncomment this - //"use_resource_manager": false, + // "use_resource_manager": false, }, }, }, @@ -1044,7 +1044,7 @@ func TestExpandFeaturesStorage(t *testing.T) { "storage": []interface{}{ map[string]interface{}{ // TODO 3.0: uncomment this - //"use_resource_manager": true, + // "use_resource_manager": true, }, }, }, @@ -1062,7 +1062,7 @@ func TestExpandFeaturesStorage(t *testing.T) { "storage": []interface{}{ map[string]interface{}{ // TODO 3.0: uncomment this - //"use_resource_manager": false, + // "use_resource_manager": false, }, }, }, From 7b44e48cca7eae1b5a5ee53a45f946aad793d5d3 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 11 Jul 2022 15:34:13 +0800 Subject: [PATCH 14/19] Fix after merge --- internal/provider/features_test.go | 16 +++++--------- internal/services/storage/client/client.go | 7 +----- .../storage/shim/shares_mgmt_plane.go | 10 +++++++++ .../storage_container_resource_test.go | 22 ------------------- .../storage/storage_queue_resource_test.go | 13 ----------- 5 files changed, 17 insertions(+), 51 deletions(-) diff --git a/internal/provider/features_test.go b/internal/provider/features_test.go index 4beb954bc412..bde78a47c188 100644 --- a/internal/provider/features_test.go +++ b/internal/provider/features_test.go @@ -132,8 +132,7 @@ func TestExpandFeatures(t *testing.T) { }, "storage": []interface{}{ map[string]interface{}{ - // TODO 3.0: uncomment this - // "use_resource_manager": true, + "use_resource_manager": true, }, }, }, @@ -180,7 +179,7 @@ func TestExpandFeatures(t *testing.T) { ScaleToZeroOnDelete: true, }, Storage: features.StorageFeatures{ - UseResourceManager: false, + UseResourceManager: true, }, }, }, @@ -253,8 +252,7 @@ func TestExpandFeatures(t *testing.T) { }, "storage": []interface{}{ map[string]interface{}{ - // TODO 3.0: uncomment this - // "use_resource_manager": false, + "use_resource_manager": false, }, }, }, @@ -1073,15 +1071,14 @@ func TestExpandFeaturesStorage(t *testing.T) { map[string]interface{}{ "storage": []interface{}{ map[string]interface{}{ - // TODO 3.0: uncomment this - // "use_resource_manager": true, + "use_resource_manager": true, }, }, }, }, Expected: features.UserFeatures{ Storage: features.StorageFeatures{ - UseResourceManager: false, + UseResourceManager: true, }, }, }, @@ -1091,8 +1088,7 @@ func TestExpandFeaturesStorage(t *testing.T) { map[string]interface{}{ "storage": []interface{}{ map[string]interface{}{ - // TODO 3.0: uncomment this - // "use_resource_manager": false, + "use_resource_manager": false, }, }, }, diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index c75c348122d9..fba3b89c94e4 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -21,7 +21,7 @@ import ( "github.com/tombuildsstuff/giovanni/storage/2019-12-12/table/tables" "github.com/tombuildsstuff/giovanni/storage/2020-08-04/file/directories" "github.com/tombuildsstuff/giovanni/storage/2020-08-04/file/files" - "github.com/tombuildsstuff/giovanni/storage/2020-08-04/file/shares") + "github.com/tombuildsstuff/giovanni/storage/2020-08-04/file/shares" ) type Client struct { @@ -31,7 +31,6 @@ type Client struct { ManagementPoliciesClient *storage.ManagementPoliciesClient BlobInventoryPoliciesClient *storage.BlobInventoryPoliciesClient CloudEndpointsClient *storagesync.CloudEndpointsClient - DisksPoolsClient *storagepool.DiskPoolsClient EncryptionScopesClient *storage.EncryptionScopesClient Environment az.Environment ObjectReplicationClient *objectreplicationpolicies.ObjectReplicationPoliciesClient @@ -75,9 +74,6 @@ func NewClient(options *common.ClientOptions) *Client { encryptionScopesClient := storage.NewEncryptionScopesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) options.ConfigureClient(&encryptionScopesClient.Client, options.ResourceManagerAuthorizer) - disksPoolsClient := storagepool.NewDiskPoolsClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) - options.ConfigureClient(&disksPoolsClient.Client, options.ResourceManagerAuthorizer) - objectReplicationPolicyClient := objectreplicationpolicies.NewObjectReplicationPoliciesClientWithBaseURI(options.ResourceManagerEndpoint) options.ConfigureClient(&objectReplicationPolicyClient.Client, options.ResourceManagerAuthorizer) @@ -108,7 +104,6 @@ func NewClient(options *common.ClientOptions) *Client { ManagementPoliciesClient: &managementPoliciesClient, BlobInventoryPoliciesClient: &blobInventoryPoliciesClient, CloudEndpointsClient: &cloudEndpointsClient, - DisksPoolsClient: &disksPoolsClient, EncryptionScopesClient: &encryptionScopesClient, Environment: options.Environment, ObjectReplicationClient: &objectReplicationPolicyClient, diff --git a/internal/services/storage/shim/shares_mgmt_plane.go b/internal/services/storage/shim/shares_mgmt_plane.go index 0342bb27d605..b7019203bc4a 100644 --- a/internal/services/storage/shim/shares_mgmt_plane.go +++ b/internal/services/storage/shim/shares_mgmt_plane.go @@ -121,6 +121,16 @@ func (w ResourceManagerStorageShareWrapper) UpdateQuota(ctx context.Context, res return err } +func (w ResourceManagerStorageShareWrapper) UpdateTier(ctx context.Context, resourceGroup string, accountName string, shareName string, tier shares.AccessTier) error { + rmInput := storage.FileShare{ + FileShareProperties: &storage.FileShareProperties{ + AccessTier: storage.ShareAccessTier(tier), + }, + } + _, err := w.client.Update(ctx, resourceGroup, accountName, shareName, rmInput) + return err +} + func (w ResourceManagerStorageShareWrapper) mapToACLs(input *[]storage.SignedIdentifier) []shares.SignedIdentifier { if input == nil { return nil diff --git a/internal/services/storage/storage_container_resource_test.go b/internal/services/storage/storage_container_resource_test.go index 5100e8dbafb5..fa497bef2d44 100644 --- a/internal/services/storage/storage_container_resource_test.go +++ b/internal/services/storage/storage_container_resource_test.go @@ -3,7 +3,6 @@ package storage_test import ( "context" "fmt" - "os" "strings" "testing" @@ -187,9 +186,6 @@ func TestAccStorageContainer_web(t *testing.T) { } func TestAccStorageContainer_resourceManagerBasic(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -205,9 +201,6 @@ func TestAccStorageContainer_resourceManagerBasic(t *testing.T) { } func TestAccStorageContainer_resourceManagerUpdate(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -232,9 +225,6 @@ func TestAccStorageContainer_resourceManagerUpdate(t *testing.T) { } func TestAccStorageContainer_resourceManagerMetaData(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -269,9 +259,6 @@ func TestAccStorageContainer_resourceManagerRoot(t *testing.T) { // Once that issue is fixed, we can enable this test case. t.Skip() - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -288,9 +275,6 @@ func TestAccStorageContainer_resourceManagerRoot(t *testing.T) { } func TestAccStorageContainer_resourceManagerWeb(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} @@ -307,9 +291,6 @@ func TestAccStorageContainer_resourceManagerWeb(t *testing.T) { } func TestAccStorageContainer_dataPlaneThenResourceManager(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{} @@ -335,9 +316,6 @@ func TestAccStorageContainer_dataPlaneThenResourceManager(t *testing.T) { } func TestAccStorageContainer_resourceManagerThenDataPlane(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_container", "test") r := StorageContainerResource{useResourceManager: true} diff --git a/internal/services/storage/storage_queue_resource_test.go b/internal/services/storage/storage_queue_resource_test.go index e894d5468e8a..a7a8ef02afef 100644 --- a/internal/services/storage/storage_queue_resource_test.go +++ b/internal/services/storage/storage_queue_resource_test.go @@ -3,7 +3,6 @@ package storage_test import ( "context" "fmt" - "os" "testing" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" @@ -86,9 +85,6 @@ func TestAccStorageQueue_metaData(t *testing.T) { } func TestAccStorageQueue_resourceManagerBasic(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} @@ -104,9 +100,6 @@ func TestAccStorageQueue_resourceManagerBasic(t *testing.T) { } func TestAccStorageQueue_resourceManagerMetaData(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} @@ -129,9 +122,6 @@ func TestAccStorageQueue_resourceManagerMetaData(t *testing.T) { } func TestAccStorageQueue_dataPlaneThenResourceManagerMetaData(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{} @@ -157,9 +147,6 @@ func TestAccStorageQueue_dataPlaneThenResourceManagerMetaData(t *testing.T) { } func TestAccStorageQueue_resourceManagerThenDataPlaneMetaData(t *testing.T) { - // The storage "use_resource_manager" feature is only available in 3.0 - os.Setenv("ARM_THREEPOINTZERO_BETA_RESOURCES", "true") - data := acceptance.BuildTestData(t, "azurerm_storage_queue", "test") r := StorageQueueResource{useResourceManager: true} From f31d1fdb0ca88f01644528f6adaa3dfde58716a4 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 11 Jul 2022 15:50:58 +0800 Subject: [PATCH 15/19] terrafmt --- internal/services/storage/storage_share_resource_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/services/storage/storage_share_resource_test.go b/internal/services/storage/storage_share_resource_test.go index c75b5a7d071e..f0ebb5e74360 100644 --- a/internal/services/storage/storage_share_resource_test.go +++ b/internal/services/storage/storage_share_resource_test.go @@ -718,11 +718,11 @@ resource "azurerm_storage_share" "test" { func (r StorageShareResource) largeQuota(data acceptance.TestData) string { return fmt.Sprintf(` - features { - storage { - use_resource_manager = %t - } +features { + storage { + use_resource_manager = %t } +} resource "azurerm_resource_group" "test" { name = "acctestRG-storageshare-%d" From fddedfc955913620f8d20fea1cdab511156c1ba4 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 11 Jul 2022 16:52:41 +0800 Subject: [PATCH 16/19] Clean up some code and add links to known issues --- internal/services/storage/client/client.go | 3 ++- .../services/storage/shim/shares_mgmt_plane.go | 17 ++++++++++++++++- .../services/storage/shim/tables_mgmt_plane.go | 4 ++-- .../storage/storage_account_resource.go | 6 +++--- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index fba3b89c94e4..20c731c0c58b 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -336,7 +336,8 @@ func (client Client) TableEntityClient(ctx context.Context, account accountDetai } func (client Client) TablesClient(ctx context.Context, account accountDetails) (shim.StorageTableWrapper, error) { - // TODO: once mgmt API got ACL support, we can uncomment below + // TODO: Once mgmt API got ACL support, continue implementing the table mgmt wrapper and uncomment below. Issue: https://github.com/Azure/azure-rest-api-specs/issues/17007 + // if client.useResourceManager { // tableClient := storage.NewTableClient(client.SubscriptionId) // tableClient.Client.Authorizer = client.resourceManagerAuthorizer diff --git a/internal/services/storage/shim/shares_mgmt_plane.go b/internal/services/storage/shim/shares_mgmt_plane.go index b7019203bc4a..f51bcfb1aa52 100644 --- a/internal/services/storage/shim/shares_mgmt_plane.go +++ b/internal/services/storage/shim/shares_mgmt_plane.go @@ -124,7 +124,7 @@ func (w ResourceManagerStorageShareWrapper) UpdateQuota(ctx context.Context, res func (w ResourceManagerStorageShareWrapper) UpdateTier(ctx context.Context, resourceGroup string, accountName string, shareName string, tier shares.AccessTier) error { rmInput := storage.FileShare{ FileShareProperties: &storage.FileShareProperties{ - AccessTier: storage.ShareAccessTier(tier), + AccessTier: w.mapToTier(tier), }, } _, err := w.client.Update(ctx, resourceGroup, accountName, shareName, rmInput) @@ -197,3 +197,18 @@ func (w ResourceManagerStorageShareWrapper) mapFromACLs(input []shares.SignedIde } return &output, nil } + +func (w ResourceManagerStorageShareWrapper) mapToTier(input shares.AccessTier) storage.ShareAccessTier { + switch input { + case shares.CoolAccessTier: + return storage.ShareAccessTierCool + case shares.HotAccessTier: + return storage.ShareAccessTierHot + case shares.PremiumAccessTier: + return storage.ShareAccessTierPremium + case shares.TransactionOptimizedAccessTier: + return storage.ShareAccessTierTransactionOptimized + default: + return "" + } +} diff --git a/internal/services/storage/shim/tables_mgmt_plane.go b/internal/services/storage/shim/tables_mgmt_plane.go index 097803e69adb..2f73cc8930ff 100644 --- a/internal/services/storage/shim/tables_mgmt_plane.go +++ b/internal/services/storage/shim/tables_mgmt_plane.go @@ -44,11 +44,11 @@ func (w ResourceManagerStorageTableWrapper) Exists(ctx context.Context, resource } func (w ResourceManagerStorageTableWrapper) GetACLs(ctx context.Context, resourceGroup string, accountName string, tableName string) (*[]tables.SignedIdentifier, error) { - // TODO @magodo: support ACLs once API is available + // TODO: Implement this once the following issue is resolved: https://github.com/Azure/azure-rest-api-specs/issues/17007 return nil, errors.New("Storage Table management plane API doesn't support ACLs now") } func (w ResourceManagerStorageTableWrapper) UpdateACLs(ctx context.Context, resourceGroup string, accountName string, tableName string, acls []tables.SignedIdentifier) error { - // TODO @magodo: support ACLs once API is available + // TODO: Implement this once the following issue is resolved: https://github.com/Azure/azure-rest-api-specs/issues/17007 return errors.New("Storage Table management plane API doesn't support ACLs now") } diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 7e7bfb6428fa..5d2103770339 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -1195,7 +1195,7 @@ func resourceStorageAccountCreate(d *pluginsdk.ResourceData, meta interface{}) e } // Currently, only the data plane API of queues fully support the exported properties of the `queue_properties`. - // TODO @magodo: once the mgmt plane API catches up, switch following client to the mgmt plane. + // TODO: Switch following client to the mgmt plane once the following issue got resolved: https://github.com/Azure/azure-rest-api-specs/issues/17006 queuesDataPlaneClient, err := storageClient.QueuesDataPlaneClient(ctx, *account) if err != nil { return fmt.Errorf("building Queues Client: %s", err) @@ -1596,7 +1596,7 @@ func resourceStorageAccountUpdate(d *pluginsdk.ResourceData, meta interface{}) e } // Currently, only the data plane API of queues fully support the exported properties of the `queue_properties`. - // TODO @magodo: once the mgmt plane API catches up, switch following client to the mgmt plane. + // TODO: Switch following client to the mgmt plane once the following issue got resolved: https://github.com/Azure/azure-rest-api-specs/issues/17006 queuesDataPlaneClient, err := storageClient.QueuesDataPlaneClient(ctx, *account) if err != nil { return fmt.Errorf("building Queues Client: %s", err) @@ -1914,7 +1914,7 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err if resp.Sku.Tier == storage.SkuTierStandard { if resp.Kind == storage.KindStorage || resp.Kind == storage.KindStorageV2 { // Currently, only the data plane API of queues fully support the exported properties of the `queue_properties`. - // TODO @magodo: once the mgmt plane API catches up, switch following client to the mgmt plane. + // TODO: Switch following client to the mgmt plane once the following issue got resolved: https://github.com/Azure/azure-rest-api-specs/issues/17006 queuesDataPlaneClient, err := storageClient.QueuesDataPlaneClient(ctx, *account) if err != nil { return fmt.Errorf("building Queues Client: %s", err) From e1aa42d1a9b77191c56b5b8298fe8bc7048360cf Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 11 Jul 2022 16:55:33 +0800 Subject: [PATCH 17/19] Fix incorrect testcase tpl --- internal/services/storage/storage_share_resource_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/services/storage/storage_share_resource_test.go b/internal/services/storage/storage_share_resource_test.go index f0ebb5e74360..c4e20036927b 100644 --- a/internal/services/storage/storage_share_resource_test.go +++ b/internal/services/storage/storage_share_resource_test.go @@ -718,12 +718,13 @@ resource "azurerm_storage_share" "test" { func (r StorageShareResource) largeQuota(data acceptance.TestData) string { return fmt.Sprintf(` -features { - storage { - use_resource_manager = %t +provider "azurerm" { + features { + storage { + use_resource_manager = %t + } } } - resource "azurerm_resource_group" "test" { name = "acctestRG-storageshare-%d" location = "%s" From 8b6fa000de4942db16693e08c0f6ae99fd7dcd28 Mon Sep 17 00:00:00 2001 From: magodo Date: Sat, 8 Oct 2022 10:31:39 +0800 Subject: [PATCH 18/19] Use the correct api version --- .../services/storage/shim/containers_mgmt_plane.go | 2 +- .../services/storage/shim/queues_mgmt_plane.go | 2 +- .../services/storage/shim/shares_mgmt_plane.go | 14 +++++++------- .../services/storage/shim/tables_mgmt_plane.go | 5 +++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/internal/services/storage/shim/containers_mgmt_plane.go b/internal/services/storage/shim/containers_mgmt_plane.go index 42bf5c5d50f1..1d2a97c1d594 100644 --- a/internal/services/storage/shim/containers_mgmt_plane.go +++ b/internal/services/storage/shim/containers_mgmt_plane.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" "github.com/hashicorp/terraform-provider-azurerm/utils" "github.com/tombuildsstuff/giovanni/storage/2019-12-12/blob/containers" ) diff --git a/internal/services/storage/shim/queues_mgmt_plane.go b/internal/services/storage/shim/queues_mgmt_plane.go index 9df3e6720ee5..db519a8c6790 100644 --- a/internal/services/storage/shim/queues_mgmt_plane.go +++ b/internal/services/storage/shim/queues_mgmt_plane.go @@ -6,7 +6,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" - "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" ) type ResourceManagerStorageQueueWrapper struct { diff --git a/internal/services/storage/shim/shares_mgmt_plane.go b/internal/services/storage/shim/shares_mgmt_plane.go index f51bcfb1aa52..3777e3834629 100644 --- a/internal/services/storage/shim/shares_mgmt_plane.go +++ b/internal/services/storage/shim/shares_mgmt_plane.go @@ -5,7 +5,7 @@ import ( "fmt" "time" - "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" "github.com/hashicorp/terraform-provider-azurerm/utils" "github.com/tombuildsstuff/giovanni/storage/2020-08-04/file/shares" @@ -145,12 +145,12 @@ func (w ResourceManagerStorageShareWrapper) mapToACLs(input *[]storage.SignedIde var policy shares.AccessPolicy if identifier.AccessPolicy != nil { var expiry string - if identifier.AccessPolicy.Expiry != nil { - expiry = identifier.AccessPolicy.Expiry.String() + if identifier.AccessPolicy.ExpiryTime != nil { + expiry = identifier.AccessPolicy.ExpiryTime.String() } var start string - if identifier.AccessPolicy.Start != nil { - start = identifier.AccessPolicy.Start.String() + if identifier.AccessPolicy.StartTime != nil { + start = identifier.AccessPolicy.StartTime.String() } var permission string if identifier.AccessPolicy.Permission != nil { @@ -189,8 +189,8 @@ func (w ResourceManagerStorageShareWrapper) mapFromACLs(input []shares.SignedIde output = append(output, storage.SignedIdentifier{ ID: &identifier.Id, AccessPolicy: &storage.AccessPolicy{ - Start: &date.Time{Time: start}, - Expiry: &date.Time{Time: expiry}, + StartTime: &date.Time{Time: start}, + ExpiryTime: &date.Time{Time: expiry}, Permission: &policy.Permission, }, }) diff --git a/internal/services/storage/shim/tables_mgmt_plane.go b/internal/services/storage/shim/tables_mgmt_plane.go index 2f73cc8930ff..1886a455f0c9 100644 --- a/internal/services/storage/shim/tables_mgmt_plane.go +++ b/internal/services/storage/shim/tables_mgmt_plane.go @@ -6,7 +6,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" - "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-04-01/storage" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" "github.com/tombuildsstuff/giovanni/storage/2019-12-12/table/tables" ) @@ -21,7 +21,8 @@ func NewManagementPlaneStorageTableWrapper(client *storage.TableClient) StorageT } func (w ResourceManagerStorageTableWrapper) Create(ctx context.Context, resourceGroup string, accountName string, tableName string) error { - _, err := w.client.Create(ctx, resourceGroup, accountName, tableName) + // TODO: Consider the correct parameter to use once the following issue is resolved: https://github.com/Azure/azure-rest-api-specs/issues/17007 + _, err := w.client.Create(ctx, resourceGroup, accountName, tableName, nil) return err } From 3a4c5bdcef376f9df6e64fd63c46494ffb52e664 Mon Sep 17 00:00:00 2001 From: magodo Date: Sat, 8 Oct 2022 12:28:03 +0800 Subject: [PATCH 19/19] Adding the table to using resource manager --- internal/services/storage/client/client.go | 12 +- .../storage/shim/tables_mgmt_plane.go | 97 +++++++++++- .../storage/storage_table_resource_test.go | 139 ++++++++++++++++-- 3 files changed, 221 insertions(+), 27 deletions(-) diff --git a/internal/services/storage/client/client.go b/internal/services/storage/client/client.go index 7f948a0a3345..7bd335928eee 100644 --- a/internal/services/storage/client/client.go +++ b/internal/services/storage/client/client.go @@ -336,13 +336,11 @@ func (client Client) TableEntityClient(ctx context.Context, account accountDetai } func (client Client) TablesClient(ctx context.Context, account accountDetails) (shim.StorageTableWrapper, error) { - // TODO: Once mgmt API got ACL support, continue implementing the table mgmt wrapper and uncomment below. Issue: https://github.com/Azure/azure-rest-api-specs/issues/17007 - - // if client.useResourceManager { - // tableClient := storage.NewTableClient(client.SubscriptionId) - // tableClient.Client.Authorizer = client.resourceManagerAuthorizer - // return shim.NewManagementPlaneStorageTableWrapper(&tableClient), nil - // } + if client.useResourceManager { + tableClient := storage.NewTableClient(client.SubscriptionId) + tableClient.Client.Authorizer = client.resourceManagerAuthorizer + return shim.NewManagementPlaneStorageTableWrapper(&tableClient), nil + } tablesClient, err := client.TablesDataPlaneClient(ctx, account) if err != nil { diff --git a/internal/services/storage/shim/tables_mgmt_plane.go b/internal/services/storage/shim/tables_mgmt_plane.go index 1886a455f0c9..803056cc18c2 100644 --- a/internal/services/storage/shim/tables_mgmt_plane.go +++ b/internal/services/storage/shim/tables_mgmt_plane.go @@ -2,11 +2,13 @@ package shim import ( "context" - "errors" + "fmt" + "time" "github.com/hashicorp/terraform-provider-azurerm/utils" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" + "github.com/Azure/go-autorest/autorest/date" "github.com/tombuildsstuff/giovanni/storage/2019-12-12/table/tables" ) @@ -21,8 +23,7 @@ func NewManagementPlaneStorageTableWrapper(client *storage.TableClient) StorageT } func (w ResourceManagerStorageTableWrapper) Create(ctx context.Context, resourceGroup string, accountName string, tableName string) error { - // TODO: Consider the correct parameter to use once the following issue is resolved: https://github.com/Azure/azure-rest-api-specs/issues/17007 - _, err := w.client.Create(ctx, resourceGroup, accountName, tableName, nil) + _, err := w.client.Create(ctx, resourceGroup, accountName, tableName, &storage.Table{}) return err } @@ -45,11 +46,93 @@ func (w ResourceManagerStorageTableWrapper) Exists(ctx context.Context, resource } func (w ResourceManagerStorageTableWrapper) GetACLs(ctx context.Context, resourceGroup string, accountName string, tableName string) (*[]tables.SignedIdentifier, error) { - // TODO: Implement this once the following issue is resolved: https://github.com/Azure/azure-rest-api-specs/issues/17007 - return nil, errors.New("Storage Table management plane API doesn't support ACLs now") + table, err := w.client.Get(ctx, resourceGroup, accountName, tableName) + if err != nil { + return nil, err + } + if prop := table.TableProperties; prop != nil { + identifiers := w.mapToACLs(prop.SignedIdentifiers) + return &identifiers, nil + } + return nil, nil } func (w ResourceManagerStorageTableWrapper) UpdateACLs(ctx context.Context, resourceGroup string, accountName string, tableName string, acls []tables.SignedIdentifier) error { - // TODO: Implement this once the following issue is resolved: https://github.com/Azure/azure-rest-api-specs/issues/17007 - return errors.New("Storage Table management plane API doesn't support ACLs now") + identifiers, err := w.mapFromACLs(acls) + if err != nil { + return err + } + _, err = w.client.Update(ctx, resourceGroup, accountName, tableName, &storage.Table{ + TableProperties: &storage.TableProperties{ + SignedIdentifiers: identifiers, + }, + }) + return err +} + +func (w ResourceManagerStorageTableWrapper) mapToACLs(input *[]storage.TableSignedIdentifier) []tables.SignedIdentifier { + if input == nil { + return nil + } + var output []tables.SignedIdentifier + for _, identifier := range *input { + var id string + if identifier.ID != nil { + id = *identifier.ID + } + + var policy tables.AccessPolicy + if identifier.AccessPolicy != nil { + var expiry string + if identifier.AccessPolicy.ExpiryTime != nil { + expiry = identifier.AccessPolicy.ExpiryTime.String() + } + var start string + if identifier.AccessPolicy.StartTime != nil { + start = identifier.AccessPolicy.StartTime.String() + } + var permission string + if identifier.AccessPolicy.Permission != nil { + permission = *identifier.AccessPolicy.Permission + } + policy = tables.AccessPolicy{ + Start: start, + Expiry: expiry, + Permission: permission, + } + } + output = append(output, tables.SignedIdentifier{ + Id: id, + AccessPolicy: policy, + }) + } + return output +} + +func (w ResourceManagerStorageTableWrapper) mapFromACLs(input []tables.SignedIdentifier) (*[]storage.TableSignedIdentifier, error) { + if len(input) == 0 { + return nil, nil + } + + var output []storage.TableSignedIdentifier + for _, identifier := range input { + policy := identifier.AccessPolicy + start, err := date.ParseTime(time.RFC3339, policy.Start) + if err != nil { + return nil, fmt.Errorf("parsing start time of the ACL %q: %w", policy.Start, err) + } + expiry, err := date.ParseTime(time.RFC3339, policy.Expiry) + if err != nil { + return nil, fmt.Errorf("parsing expiry time of the ACL %q: %w", policy.Expiry, err) + } + output = append(output, storage.TableSignedIdentifier{ + ID: &identifier.Id, + AccessPolicy: &storage.TableAccessPolicy{ + StartTime: &date.Time{Time: start}, + ExpiryTime: &date.Time{Time: expiry}, + Permission: &policy.Permission, + }, + }) + } + return &output, nil } diff --git a/internal/services/storage/storage_table_resource_test.go b/internal/services/storage/storage_table_resource_test.go index ca30ce08a799..714a325bb441 100644 --- a/internal/services/storage/storage_table_resource_test.go +++ b/internal/services/storage/storage_table_resource_test.go @@ -13,7 +13,105 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/utils" ) -type StorageTableResource struct{} +type StorageTableResource struct { + useResourceManager bool +} + +func TestAccStorageTable_resourceManagerBasic(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_table", "test") + r := StorageTableResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageTable_resourceManagerAcl(t *testing.T) { + // TODO: The API is problematic: https://github.com/Azure/azure-rest-api-specs/issues/17007#issuecomment-1272222690 + t.Skip() + + data := acceptance.BuildTestData(t, "azurerm_storage_table", "test") + r := StorageTableResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.acl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.aclUpdated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageTable_dataPlaneThenResourceManagerAcl(t *testing.T) { + // TODO: The API is problematic: https://github.com/Azure/azure-rest-api-specs/issues/17007#issuecomment-1272222690 + t.Skip() + + data := acceptance.BuildTestData(t, "azurerm_storage_table", "test") + r := StorageTableResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.acl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: func() string { + r.useResourceManager = true + return r.aclUpdated(data) + }(), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccStorageTable_resourceManagerThenDataPlaneAcl(t *testing.T) { + // TODO: The API is problematic: https://github.com/Azure/azure-rest-api-specs/issues/17007#issuecomment-1272222690 + t.Skip() + + data := acceptance.BuildTestData(t, "azurerm_storage_table", "test") + r := StorageTableResource{useResourceManager: true} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.acl(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: func() string { + r.useResourceManager = false + return r.aclUpdated(data) + }(), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} func TestAccStorageTable_basic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_table", "test") @@ -91,6 +189,9 @@ func (r StorageTableResource) Exists(ctx context.Context, client *clients.Client if account == nil { return nil, fmt.Errorf("unable to determine Resource Group for Storage Storage Table %q (Account %q)", id.Name, id.AccountName) } + + client.Storage.UseResourceManager(r.useResourceManager) + tablesClient, err := client.Storage.TablesClient(ctx, *account) if err != nil { return nil, fmt.Errorf("building Table Client: %+v", err) @@ -132,7 +233,11 @@ func (r StorageTableResource) Destroy(ctx context.Context, client *clients.Clien func (r StorageTableResource) basic(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -156,7 +261,7 @@ resource "azurerm_storage_table" "test" { name = "acctestst%d" storage_account_name = azurerm_storage_account.test.name } -`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) } func (r StorageTableResource) requiresImport(data acceptance.TestData) string { @@ -174,7 +279,11 @@ resource "azurerm_storage_table" "import" { func (r StorageTableResource) acl(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -202,18 +311,22 @@ resource "azurerm_storage_table" "test" { access_policy { permissions = "raud" - start = "2020-11-26T08:49:37.0000000Z" - expiry = "2020-11-27T08:49:37.0000000Z" + start = "2020-11-26T08:49:37Z" + expiry = "2020-11-27T08:49:37Z" } } } -`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) } func (r StorageTableResource) aclUpdated(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { - features {} + features { + storage { + use_resource_manager = %t + } + } } resource "azurerm_resource_group" "test" { @@ -242,8 +355,8 @@ resource "azurerm_storage_table" "test" { access_policy { permissions = "raud" - start = "2020-11-26T08:49:37.0000000Z" - expiry = "2020-11-27T08:49:37.0000000Z" + start = "2020-11-26T08:49:37Z" + expiry = "2020-11-27T08:49:37Z" } } acl { @@ -251,10 +364,10 @@ resource "azurerm_storage_table" "test" { access_policy { permissions = "raud" - start = "2019-07-02T09:38:21.0000000Z" - expiry = "2019-07-02T10:38:21.0000000Z" + start = "2019-07-02T09:38:21Z" + expiry = "2019-07-02T10:38:21Z" } } } -`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) +`, r.useResourceManager, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) }