Skip to content

Commit b80c637

Browse files
andyzhangxk8s-infra-cherrypick-robot
authored andcommitted
feat: add getLatestAccountKey parameter in storage class
1 parent 0250441 commit b80c637

File tree

6 files changed

+113
-13
lines changed

6 files changed

+113
-13
lines changed

docs/driver-parameters.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ useDataPlaneAPI | specify whether use data plane API for blob container create/d
2828
--- | **Following parameters are only for blobfuse** | --- | --- |
2929
subscriptionID | specify Azure subscription ID in which blob storage directory will be created | Azure subscription ID | No | if not empty, `resourceGroup` must be provided
3030
storeAccountKey | whether store account key to k8s secret <br><br> Note: <br> `false` means driver would leverage kubelet identity to get account key | `true`,`false` | No | `true`
31+
getLatestAccountKey | whether getting the latest account key based on the creation time, this driver would get the first key by default | `true`,`false` | No | `false`
3132
secretName | specify secret name to store account key | | No |
3233
secretNamespace | specify the namespace of secret to store account key | `default`,`kube-system`, etc | No | pvc namespace
3334
isHnsEnabled | enable `Hierarchical namespace` for Azure DataLake storage account | `true`,`false` | No | `false`
@@ -80,6 +81,7 @@ volumeAttributes.protocol | specify blobfuse, blobfuse2 or NFSv3 mount (blobfuse
8081
--- | **Following parameters are only for blobfuse** | --- | --- |
8182
volumeAttributes.secretName | secret name that stores storage account name and key(only applies for SMB) | | No |
8283
volumeAttributes.secretNamespace | secret namespace | `default`,`kube-system`, etc | No | pvc namespace
84+
volumeAttributes.getLatestAccountKey | whether getting the latest account key based on the creation time, this driver would get the first key by default | `true`,`false` | No | `false`
8385
nodeStageSecretRef.name | secret name that stores(check below examples):<br>`azurestorageaccountkey`<br>`azurestorageaccountsastoken`<br>`msisecret`<br>`azurestoragespnclientsecret` | existing Kubernetes secret name | No |
8486
nodeStageSecretRef.namespace | secret namespace | k8s namespace | Yes |
8587
--- | **Following parameters are only for NFS protocol** | --- | --- |

pkg/blob/blob.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package blob
1919
import (
2020
"fmt"
2121
"os"
22+
"strconv"
2223
"strings"
2324
"sync"
2425
"time"
@@ -70,6 +71,7 @@ const (
7071
containerNameField = "containername"
7172
containerNamePrefixField = "containernameprefix"
7273
storeAccountKeyField = "storeaccountkey"
74+
getLatestAccountKeyField = "getlatestaccountkey"
7375
isHnsEnabledField = "ishnsenabled"
7476
softDeleteBlobsField = "softdeleteblobs"
7577
softDeleteContainersField = "softdeletecontainers"
@@ -381,6 +383,7 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
381383
azureStorageAuthType string
382384
authEnv []string
383385
getAccountKeyFromSecret bool
386+
getLatestAccountKey bool
384387
)
385388

386389
for k, v := range attrib {
@@ -426,6 +429,10 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
426429
storageSPNTenantID = v
427430
case "azurestorageaadendpoint":
428431
authEnv = append(authEnv, "AZURE_STORAGE_AAD_ENDPOINT="+v)
432+
case getLatestAccountKeyField:
433+
if getLatestAccountKey, err = strconv.ParseBool(v); err != nil {
434+
return rgName, accountName, accountKey, containerName, authEnv, fmt.Errorf("invalid %s: %s in volume context", getLatestAccountKeyField, v)
435+
}
429436
}
430437
}
431438
klog.V(2).Infof("volumeID(%s) authEnv: %s", volumeID, authEnv)
@@ -485,7 +492,7 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
485492
if err != nil && !getAccountKeyFromSecret && (azureStorageAuthType == "" || strings.EqualFold(azureStorageAuthType, "key")) {
486493
klog.V(2).Infof("get account(%s) key from secret(%s, %s) failed with error: %v, use cluster identity to get account key instead",
487494
accountName, secretNamespace, secretName, err)
488-
accountKey, err = d.cloud.GetStorageAccesskey(ctx, subsID, accountName, rgName)
495+
accountKey, err = d.cloud.GetStorageAccesskey(ctx, subsID, accountName, rgName, getLatestAccountKey)
489496
if err != nil {
490497
return rgName, accountName, accountKey, containerName, authEnv, fmt.Errorf("no key for storage account(%s) under resource group(%s), err %w", accountName, rgName, err)
491498
}
@@ -567,6 +574,7 @@ func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID str
567574
keyVaultURL string
568575
keyVaultSecretName string
569576
keyVaultSecretVersion string
577+
getLatestAccountKey bool
570578
err error
571579
)
572580

@@ -586,6 +594,10 @@ func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID str
586594
accountName = v
587595
case storageAccountNameField: // for compatibility
588596
accountName = v
597+
case getLatestAccountKeyField:
598+
if getLatestAccountKey, err = strconv.ParseBool(v); err != nil {
599+
return "", "", "", "", fmt.Errorf("invalid %s: %s in volume context", getLatestAccountKeyField, v)
600+
}
589601
}
590602
}
591603

@@ -614,7 +626,7 @@ func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID str
614626
rgName = d.cloud.ResourceGroup
615627
}
616628

617-
accountKey, err = d.cloud.GetStorageAccesskey(ctx, subsID, accountName, rgName)
629+
accountKey, err = d.cloud.GetStorageAccesskey(ctx, subsID, accountName, rgName, getLatestAccountKey)
618630
if err != nil {
619631
return "", "", "", "", fmt.Errorf("no key for storage account(%s) under resource group(%s), err %w", accountName, rgName, err)
620632
}
@@ -784,7 +796,7 @@ func (d *Driver) GetStorageAccesskey(ctx context.Context, accountOptions *azure.
784796
_, accountKey, _, _, _, _, _, err := d.GetInfoFromSecret(ctx, secretName, secretNamespace) //nolint
785797
if err != nil {
786798
klog.V(2).Infof("could not get account(%s) key from secret(%s) namespace(%s), error: %v, use cluster identity to get account key instead", accountOptions.Name, secretName, secretNamespace, err)
787-
accountKey, err = d.cloud.GetStorageAccesskey(ctx, accountOptions.SubscriptionID, accountOptions.Name, accountOptions.ResourceGroup)
799+
accountKey, err = d.cloud.GetStorageAccesskey(ctx, accountOptions.SubscriptionID, accountOptions.Name, accountOptions.ResourceGroup, accountOptions.GetLatestAccountKey)
788800
}
789801
return accountOptions.Name, accountKey, err
790802
}

pkg/blob/blob_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,37 @@ func TestGetAuthEnv(t *testing.T) {
551551
}
552552
},
553553
},
554+
{
555+
name: "invalid getLatestAccountKey value",
556+
testFunc: func(t *testing.T) {
557+
d := NewFakeDriver()
558+
attrib := map[string]string{
559+
getLatestAccountKeyField: "invalid",
560+
}
561+
secret := make(map[string]string)
562+
volumeID := "rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41"
563+
d.cloud = &azure.Cloud{}
564+
ctrl := gomock.NewController(t)
565+
defer ctrl.Finish()
566+
mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl)
567+
d.cloud.StorageAccountClient = mockStorageAccountsClient
568+
s := "unit-test"
569+
accountkey := storage.AccountKey{
570+
Value: &s,
571+
}
572+
accountkeylist := []storage.AccountKey{}
573+
accountkeylist = append(accountkeylist, accountkey)
574+
list := storage.AccountListKeysResult{
575+
Keys: &accountkeylist,
576+
}
577+
mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(list, nil).AnyTimes()
578+
_, _, _, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
579+
expectedErr := fmt.Errorf("invalid getlatestaccountkey: %s in volume context", "invalid")
580+
if !reflect.DeepEqual(err, expectedErr) {
581+
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
582+
}
583+
},
584+
},
554585
{
555586
name: "secret not empty",
556587
testFunc: func(t *testing.T) {
@@ -720,6 +751,37 @@ func TestGetStorageAccountAndContainer(t *testing.T) {
720751
}
721752
},
722753
},
754+
{
755+
name: "invalid getLatestAccountKey value",
756+
testFunc: func(t *testing.T) {
757+
d := NewFakeDriver()
758+
attrib := map[string]string{
759+
getLatestAccountKeyField: "invalid",
760+
}
761+
secret := make(map[string]string)
762+
volumeID := "rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41"
763+
d.cloud = &azure.Cloud{}
764+
ctrl := gomock.NewController(t)
765+
defer ctrl.Finish()
766+
mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl)
767+
d.cloud.StorageAccountClient = mockStorageAccountsClient
768+
s := "unit-test"
769+
accountkey := storage.AccountKey{
770+
Value: &s,
771+
}
772+
accountkeylist := []storage.AccountKey{}
773+
accountkeylist = append(accountkeylist, accountkey)
774+
list := storage.AccountListKeysResult{
775+
Keys: &accountkeylist,
776+
}
777+
mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(list, nil).AnyTimes()
778+
_, _, _, _, err := d.GetStorageAccountAndContainer(context.TODO(), volumeID, attrib, secret)
779+
expectedErr := fmt.Errorf("invalid getlatestaccountkey: %s in volume context", "invalid")
780+
if !reflect.DeepEqual(err, expectedErr) {
781+
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
782+
}
783+
},
784+
},
723785
}
724786
for _, tc := range testCases {
725787
t.Run(tc.name, tc.testFunc)

pkg/blob/controllerserver.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,9 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
7575
var storageAccountType, subsID, resourceGroup, location, account, containerName, containerNamePrefix, protocol, customTags, secretName, secretNamespace, pvcNamespace string
7676
var isHnsEnabled, requireInfraEncryption, enableBlobVersioning *bool
7777
var vnetResourceGroup, vnetName, subnetName, accessTier, networkEndpointType, storageEndpointSuffix string
78-
var matchTags, useDataPlaneAPI bool
78+
var matchTags, useDataPlaneAPI, getLatestAccountKey bool
7979
var softDeleteBlobs, softDeleteContainers int32
80+
var err error
8081
// set allowBlobPublicAccess as false by default
8182
allowBlobPublicAccess := pointer.Bool(false)
8283

@@ -137,6 +138,10 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
137138
if strings.EqualFold(v, falseValue) {
138139
storeAccountKey = false
139140
}
141+
case getLatestAccountKeyField:
142+
if getLatestAccountKey, err = strconv.ParseBool(v); err != nil {
143+
return nil, status.Errorf(codes.InvalidArgument, "invalid %s: %s in volume context", getLatestAccountKeyField, v)
144+
}
140145
case allowBlobPublicAccessField:
141146
if strings.EqualFold(v, trueValue) {
142147
allowBlobPublicAccess = pointer.Bool(true)
@@ -302,6 +307,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
302307
EnableBlobVersioning: enableBlobVersioning,
303308
SoftDeleteBlobs: softDeleteBlobs,
304309
SoftDeleteContainers: softDeleteContainers,
310+
GetLatestAccountKey: getLatestAccountKey,
305311
}
306312

307313
var accountKey string

pkg/blob/controllerserver_test.go

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -229,15 +229,9 @@ func TestCreateVolume(t *testing.T) {
229229
testFunc: func(t *testing.T) {
230230
d := NewFakeDriver()
231231
d.cloud = &azure.Cloud{}
232-
mp := make(map[string]string)
233-
mp[protocolField] = "unit-test"
234-
mp[skuNameField] = "unit-test"
235-
mp[storageAccountTypeField] = "unit-test"
236-
mp[locationField] = "unit-test"
237-
mp[storageAccountField] = "unit-test"
238-
mp[resourceGroupField] = "unit-test"
239-
mp[containerNameField] = "unit-test"
240-
mp[mountPermissionsField] = "0750"
232+
mp := map[string]string{
233+
protocolField: "unit-test",
234+
}
241235
req := &csi.CreateVolumeRequest{
242236
Name: "unit-test",
243237
VolumeCapabilities: stdVolumeCapabilities,
@@ -253,6 +247,29 @@ func TestCreateVolume(t *testing.T) {
253247
}
254248
},
255249
},
250+
{
251+
name: "invalid getLatestAccountKey value",
252+
testFunc: func(t *testing.T) {
253+
d := NewFakeDriver()
254+
d.cloud = &azure.Cloud{}
255+
mp := map[string]string{
256+
getLatestAccountKeyField: "invalid",
257+
}
258+
req := &csi.CreateVolumeRequest{
259+
Name: "unit-test",
260+
VolumeCapabilities: stdVolumeCapabilities,
261+
Parameters: mp,
262+
}
263+
d.Cap = []*csi.ControllerServiceCapability{
264+
controllerServiceCapability,
265+
}
266+
_, err := d.CreateVolume(context.Background(), req)
267+
expectedErr := status.Errorf(codes.InvalidArgument, "invalid %s: %s in volume context", getLatestAccountKeyField, "invalid")
268+
if !reflect.DeepEqual(err, expectedErr) {
269+
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
270+
}
271+
},
272+
},
256273
{
257274
name: "storageAccount and matchTags conflict",
258275
testFunc: func(t *testing.T) {

test/e2e/dynamic_provisioning_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ var _ = ginkgo.Describe("[blob-csi-e2e] Dynamic Provisioning", func() {
8282
"secretNamespace": "default",
8383
// make sure this is the first test case due to storeAccountKey is set as false
8484
"storeAccountKey": "false",
85+
"getLatestAccountKey": "true",
8586
"requireInfraEncryption": "true",
8687
"accessTier": "Hot",
8788
},

0 commit comments

Comments
 (0)