Repository: kubernetes-sigs/cloud-provider-azure
Pull request #784 (2021-08-31 04:00:30)

feat: add AllowBlobPublicAccess setting in storage account creation
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide/first-contribution.md#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/sig-release/release.md#issuepr-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/sig-testing/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

#### What type of PR is this?
/kind feature

#### What this PR does / why we need it:
feat: add AllowBlobPublicAccess setting in storage account creation
This PR also add `ctx context.Context` parameter in some functions.

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

#### Special notes for your reviewer:

#### Does this PR introduce a user-facing change?
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

For more information on release notes see: https://git.k8s.io/community/contributors/guide/release-notes.md
-->
```release-note
feat: add AllowBlobPublicAccess setting in storage account creation
```

#### Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

<!--
This section can be blank if this pull request does not require a release note.

When adding links which point to resources within git repositories, like
KEPs or supporting documentation, please reference a specific commit and avoid
linking directly to the master branch. This ensures that links reference a
specific point in time, rather than a document that may change over time.

See here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files

Please use the following format for linking documentation:
- [KEP]: <link>
- [Usage]: <link>
- [Other doc]: <link>
-->
```docs
feat: add AllowBlobPublicAccess setting in storage account creation
```



Patch:
diff --git a/pkg/provider/azure_blobDiskController.go b/pkg/provider/azure_blobDiskController.go
index 4e4d96e73..410c8c08a 100644
--- a/pkg/provider/azure_blobDiskController.go
+++ b/pkg/provider/azure_blobDiskController.go
@@ -80,7 +80,7 @@ func (c *BlobDiskController) initStorageAccounts() {
 // CreateVolume creates a VHD blob in a storage account that has storageType and location using the given storage account.
 // If no storage account is given, search all the storage accounts associated with the resource group and pick one that
 // fits storage type and location.
-func (c *BlobDiskController) CreateVolume(blobName, accountName, accountType, location string, requestGB int) (string, string, int, error) {
+func (c *BlobDiskController) CreateVolume(ctx context.Context, blobName, accountName, accountType, location string, requestGB int) (string, string, int, error) {
 	accountOptions := &AccountOptions{
 		Name:                   accountName,
 		Type:                   accountType,
@@ -89,7 +89,7 @@ func (c *BlobDiskController) CreateVolume(blobName, accountName, accountType, lo
 		Location:               location,
 		EnableHTTPSTrafficOnly: true,
 	}
-	account, key, err := c.common.cloud.EnsureStorageAccount(accountOptions, consts.DedicatedDiskAccountNamePrefix)
+	account, key, err := c.common.cloud.EnsureStorageAccount(ctx, accountOptions, consts.DedicatedDiskAccountNamePrefix)
 	if err != nil {
 		return "", "", 0, fmt.Errorf("could not get storage key for storage account %s: %w", accountName, err)
 	}
diff --git a/pkg/provider/azure_storage.go b/pkg/provider/azure_storage.go
index db1ad994b..1d00b9659 100644
--- a/pkg/provider/azure_storage.go
+++ b/pkg/provider/azure_storage.go
@@ -17,6 +17,7 @@ limitations under the License.
 package provider
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-02-01/storage"
@@ -29,7 +30,7 @@ import (
 
 // CreateFileShare creates a file share, using a matching storage account type, account kind, etc.
 // storage account will be created if specified account is not found
-func (az *Cloud) CreateFileShare(accountOptions *AccountOptions, shareOptions *fileclient.ShareOptions) (string, string, error) {
+func (az *Cloud) CreateFileShare(ctx context.Context, accountOptions *AccountOptions, shareOptions *fileclient.ShareOptions) (string, string, error) {
 	if accountOptions == nil {
 		return "", "", fmt.Errorf("account options is nil")
 	}
@@ -45,7 +46,7 @@ func (az *Cloud) CreateFileShare(accountOptions *AccountOptions, shareOptions *f
 		accountOptions.EnableHTTPSTrafficOnly = false
 	}
 
-	accountName, accountKey, err := az.EnsureStorageAccount(accountOptions, consts.FileShareAccountNamePrefix)
+	accountName, accountKey, err := az.EnsureStorageAccount(ctx, accountOptions, consts.FileShareAccountNamePrefix)
 	if err != nil {
 		return "", "", fmt.Errorf("could not get storage key for storage account %s: %w", accountOptions.Name, err)
 	}
diff --git a/pkg/provider/azure_storageaccount.go b/pkg/provider/azure_storageaccount.go
index cfc981fa0..225bd36bc 100644
--- a/pkg/provider/azure_storageaccount.go
+++ b/pkg/provider/azure_storageaccount.go
@@ -50,6 +50,7 @@ type AccountOptions struct {
 	DisableFileServiceDeleteRetentionPolicy bool
 	IsHnsEnabled                            *bool
 	EnableNfsV3                             *bool
+	AllowBlobPublicAccess                   *bool
 	Tags                                    map[string]string
 	VirtualNetworkResourceIDs               []string
 }
@@ -119,7 +120,7 @@ func (az *Cloud) GetStorageAccesskey(account, resourceGroup string) (string, err
 }
 
 // EnsureStorageAccount search storage account, create one storage account(with genAccountNamePrefix) if not found, return accountName, accountKey
-func (az *Cloud) EnsureStorageAccount(accountOptions *AccountOptions, genAccountNamePrefix string) (string, string, error) {
+func (az *Cloud) EnsureStorageAccount(ctx context.Context, accountOptions *AccountOptions, genAccountNamePrefix string) (string, string, error) {
 	if accountOptions == nil {
 		return "", "", fmt.Errorf("account options is nil")
 	}
@@ -207,15 +208,17 @@ func (az *Cloud) EnsureStorageAccount(accountOptions *AccountOptions, genAccount
 				Location: &location}
 
 			if accountOptions.EnableLargeFileShare {
-				klog.V(2).Infof("Enabling LargeFileShare for the storage account")
+				klog.V(2).Infof("Enabling LargeFileShare for storage account(%s)", accountName)
 				cp.AccountPropertiesCreateParameters.LargeFileSharesState = storage.LargeFileSharesStateEnabled
 			}
+			if accountOptions.AllowBlobPublicAccess != nil {
+				klog.V(2).Infof("set AllowBlobPublicAccess(%v) for storage account(%s)", *accountOptions.AllowBlobPublicAccess, accountName)
+				cp.AccountPropertiesCreateParameters.AllowBlobPublicAccess = accountOptions.AllowBlobPublicAccess
+			}
 			if az.StorageAccountClient == nil {
 				return "", "", fmt.Errorf("StorageAccountClient is nil")
 			}
 
-			ctx, cancel := getContextWithCancel()
-			defer cancel()
 			if rerr := az.StorageAccountClient.Create(ctx, resourceGroup, accountName, cp); rerr != nil {
 				return "", "", fmt.Errorf("failed to create storage account %s, error: %v", accountName, rerr)
 			}


Test patch:
diff --git a/pkg/provider/azure_blobDiskController_test.go b/pkg/provider/azure_blobDiskController_test.go
index 9cacc6cd5..951c841a9 100644
--- a/pkg/provider/azure_blobDiskController_test.go
+++ b/pkg/provider/azure_blobDiskController_test.go
@@ -79,11 +79,14 @@ func TestCreateVolume(t *testing.T) {
 	defer ctrl.Finish()
 	b := GetTestBlobDiskController(t)
 
+	ctx, cancel := getContextWithCancel()
+	defer cancel()
+
 	mockSAClient := mockstorageaccountclient.NewMockInterface(ctrl)
 	mockSAClient.EXPECT().ListKeys(gomock.Any(), b.common.resourceGroup, "testsa").Return(storage.AccountListKeysResult{}, &retryError500)
 	b.common.cloud.StorageAccountClient = mockSAClient
 
-	diskName, diskURI, requestGB, err := b.CreateVolume("testBlob", "testsa", "type", b.common.location, 10)
+	diskName, diskURI, requestGB, err := b.CreateVolume(ctx, "testBlob", "testsa", "type", b.common.location, 10)
 	var nilErr error
 	rawErr := fmt.Errorf("%w", nilErr)
 	retryErr := fmt.Errorf("Retriable: false, RetryAfter: 0s, HTTPStatusCode: 500, RawError: %w", rawErr)
@@ -102,7 +105,7 @@ func TestCreateVolume(t *testing.T) {
 			},
 		},
 	}, nil)
-	diskName, diskURI, requestGB, err = b.CreateVolume("testBlob", "testsa", "type", b.common.location, 10)
+	diskName, diskURI, requestGB, err = b.CreateVolume(ctx, "testBlob", "testsa", "type", b.common.location, 10)
 	expectedErrStr := "failed to put page blob testBlob.vhd in container vhds: storage: service returned error: StatusCode=403, ErrorCode=AccountIsDisabled, ErrorMessage=The specified account is disabled."
 	assert.Error(t, err)
 	assert.True(t, strings.Contains(err.Error(), expectedErrStr))
diff --git a/pkg/provider/azure_storage_test.go b/pkg/provider/azure_storage_test.go
index e1ab74832..714cb7531 100644
--- a/pkg/provider/azure_storage_test.go
+++ b/pkg/provider/azure_storage_test.go
@@ -32,6 +32,9 @@ func TestCreateFileShare(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
+	ctx, cancel := getContextWithCancel()
+	defer cancel()
+
 	cloud := &Cloud{controllerCommon: &controllerCommon{resourceGroup: "rg"}}
 	name := "baz"
 	sku := "sku"
@@ -168,7 +171,7 @@ func TestCreateFileShare(t *testing.T) {
 			RequestGiB: test.gb,
 		}
 
-		account, key, err := cloud.CreateFileShare(mockAccount, mockFileShare)
+		account, key, err := cloud.CreateFileShare(ctx, mockAccount, mockFileShare)
 		if test.expectErr && err == nil {
 			t.Errorf("unexpected non-error")
 			continue
diff --git a/pkg/provider/azure_storageaccount_test.go b/pkg/provider/azure_storageaccount_test.go
index bf91c957c..7dfb5be9f 100644
--- a/pkg/provider/azure_storageaccount_test.go
+++ b/pkg/provider/azure_storageaccount_test.go
@@ -323,6 +323,9 @@ func TestEnsureStorageAccountWithPrivateEndpoint(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
+	ctx, cancel := getContextWithCancel()
+	defer cancel()
+
 	resourceGroup := "ResourceGroup"
 	vnetName := "VnetName"
 	vnetResourceGroup := "VnetResourceGroup"
@@ -387,7 +390,7 @@ func TestEnsureStorageAccountWithPrivateEndpoint(t *testing.T) {
 		CreatePrivateEndpoint: true,
 	}
 
-	if _, _, err := cloud.EnsureStorageAccount(testAccountOptions, "test"); err != nil {
+	if _, _, err := cloud.EnsureStorageAccount(ctx, testAccountOptions, "test"); err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
 }