Skip to content

Commit b1ca59f

Browse files
andyzhangxk8s-infra-cherrypick-robot
andyzhangx
authored and
k8s-infra-cherrypick-robot
committed
fix: add fallback to sas token on azcopy copy command
1 parent c15ec89 commit b1ca59f

File tree

2 files changed

+20
-28
lines changed

2 files changed

+20
-28
lines changed

‎pkg/blob/controllerserver.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -436,12 +436,22 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
436436
return nil, status.Errorf(codes.Internal, "failed to create container(%s) on account(%s) type(%s) rg(%s) location(%s) size(%d), error: %v", validContainerName, accountName, storageAccountType, resourceGroup, location, requestGiB, err)
437437
}
438438
if volContentSource != nil {
439-
accountSASToken, authAzcopyEnv, err := d.getAzcopyAuth(ctx, accountName, accountKey, storageEndpointSuffix, accountOptions, secrets, secretName, secretNamespace)
439+
accountSASToken, authAzcopyEnv, err := d.getAzcopyAuth(ctx, accountName, accountKey, storageEndpointSuffix, accountOptions, secrets, secretName, secretNamespace, false)
440440
if err != nil {
441441
return nil, status.Errorf(codes.Internal, "failed to getAzcopyAuth on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
442442
}
443-
if err := d.copyVolume(ctx, req, accountName, accountSASToken, authAzcopyEnv, validContainerName, secretNamespace, accountOptions, storageEndpointSuffix); err != nil {
444-
return nil, err
443+
var copyErr error
444+
copyErr = d.copyVolume(ctx, req, accountName, accountSASToken, authAzcopyEnv, validContainerName, secretNamespace, accountOptions, storageEndpointSuffix)
445+
if accountSASToken == "" && copyErr != nil && strings.Contains(copyErr.Error(), authorizationPermissionMismatch) {
446+
klog.Warningf("azcopy copy failed with AuthorizationPermissionMismatch error, should assign \"Storage Blob Data Contributor\" role to controller identity, fall back to use sas token, original error: %v", copyErr)
447+
accountSASToken, authAzcopyEnv, err := d.getAzcopyAuth(ctx, accountName, accountKey, storageEndpointSuffix, accountOptions, secrets, secretName, secretNamespace, true)
448+
if err != nil {
449+
return nil, status.Errorf(codes.Internal, "failed to getAzcopyAuth on account(%s) rg(%s), error: %v", accountOptions.Name, accountOptions.ResourceGroup, err)
450+
}
451+
copyErr = d.copyVolume(ctx, req, accountName, accountSASToken, authAzcopyEnv, validContainerName, secretNamespace, accountOptions, storageEndpointSuffix)
452+
}
453+
if copyErr != nil {
454+
return nil, copyErr
445455
}
446456
}
447457

@@ -770,7 +780,7 @@ func (d *Driver) copyBlobContainer(ctx context.Context, req *csi.CreateVolumeReq
770780
SubscriptionID: srcSubscriptionID,
771781
GetLatestAccountKey: accountOptions.GetLatestAccountKey,
772782
}
773-
if srcAccountSasToken, _, err = d.getAzcopyAuth(ctx, srcAccountName, "", storageEndpointSuffix, srcAccountOptions, nil, "", secretNamespace); err != nil {
783+
if srcAccountSasToken, _, err = d.getAzcopyAuth(ctx, srcAccountName, "", storageEndpointSuffix, srcAccountOptions, nil, "", secretNamespace, true); err != nil {
774784
return err
775785
}
776786
}
@@ -863,11 +873,10 @@ func (d *Driver) authorizeAzcopyWithIdentity() ([]string, error) {
863873
// getAzcopyAuth will only generate sas token for azcopy in following conditions:
864874
// 1. secrets is not empty
865875
// 2. driver is not using managed identity and service principal
866-
// 3. azcopy returns AuthorizationPermissionMismatch error when using service principal or managed identity
867-
func (d *Driver) getAzcopyAuth(ctx context.Context, accountName, accountKey, storageEndpointSuffix string, accountOptions *azure.AccountOptions, secrets map[string]string, secretName, secretNamespace string) (string, []string, error) {
876+
// 3. parameter useSasToken is true
877+
func (d *Driver) getAzcopyAuth(ctx context.Context, accountName, accountKey, storageEndpointSuffix string, accountOptions *azure.AccountOptions, secrets map[string]string, secretName, secretNamespace string, useSasToken bool) (string, []string, error) {
868878
var authAzcopyEnv []string
869879
var err error
870-
useSasToken := false
871880
if !d.useDataPlaneAPI("", accountName) && len(secrets) == 0 && len(secretName) == 0 {
872881
// search in cache first
873882
if cache, err := d.azcopySasTokenCache.Get(accountName, azcache.CacheReadTypeDefault); err == nil && cache != nil {
@@ -878,17 +887,6 @@ func (d *Driver) getAzcopyAuth(ctx context.Context, accountName, accountKey, sto
878887
authAzcopyEnv, err = d.authorizeAzcopyWithIdentity()
879888
if err != nil {
880889
klog.Warningf("failed to authorize azcopy with identity, error: %v", err)
881-
} else {
882-
if len(authAzcopyEnv) > 0 {
883-
out, testErr := d.azcopy.TestListJobs(accountName, storageEndpointSuffix, authAzcopyEnv)
884-
if testErr != nil {
885-
return "", nil, fmt.Errorf("azcopy list command failed with error(%v): %v", testErr, out)
886-
}
887-
if strings.Contains(out, authorizationPermissionMismatch) {
888-
klog.Warningf("azcopy list failed with AuthorizationPermissionMismatch error, should assign \"Storage Blob Data Contributor\" role to controller identity, fall back to use sas token, original output: %v", out)
889-
useSasToken = true
890-
}
891-
}
892890
}
893891
}
894892

‎pkg/blob/controllerserver_test.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -794,18 +794,12 @@ func TestCreateVolume(t *testing.T) {
794794
ctrl := gomock.NewController(t)
795795
defer ctrl.Finish()
796796

797-
m := util.NewMockEXEC(ctrl)
798-
799797
clientFactoryMock := mock_azclient.NewMockClientFactory(ctrl)
800798
blobClientMock := mock_blobcontainerclient.NewMockInterface(ctrl)
801799
blobClientMock.EXPECT().CreateContainer(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil)
802800
clientFactoryMock.EXPECT().GetBlobContainerClientForSub(gomock.Any()).Return(blobClientMock, nil)
803801
d.clientFactory = clientFactoryMock
804802

805-
listStr := "no error"
806-
m.EXPECT().RunCommand(gomock.Any(), gomock.Any()).Return(listStr, nil)
807-
d.azcopy.ExecCmd = m
808-
809803
expectedErr := status.Errorf(codes.NotFound, "error parsing volume id: \"unit-test\", should at least contain two #")
810804
_, err := d.CreateVolume(context.Background(), req)
811805
if !reflect.DeepEqual(err, expectedErr) {
@@ -1966,7 +1960,7 @@ func TestGetAzcopyAuth(t *testing.T) {
19661960
ctx := context.Background()
19671961
expectedAccountSASToken := ""
19681962
expectedErr := fmt.Errorf("could not find accountkey or azurestorageaccountkey field in secrets")
1969-
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
1963+
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
19701964
if !reflect.DeepEqual(err, expectedErr) || !reflect.DeepEqual(accountSASToken, expectedAccountSASToken) {
19711965
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
19721966
}
@@ -1989,7 +1983,7 @@ func TestGetAzcopyAuth(t *testing.T) {
19891983
defaultSecretAccountName: "accountName",
19901984
defaultSecretAccountKey: "YWNjb3VudGtleQo=",
19911985
}
1992-
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
1986+
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
19931987
if !reflect.DeepEqual(err, nil) || !strings.Contains(accountSASToken, "?se=") {
19941988
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
19951989
}
@@ -2015,7 +2009,7 @@ func TestGetAzcopyAuth(t *testing.T) {
20152009

20162010
expectedAccountSASToken := ""
20172011
expectedErr := status.Errorf(codes.Internal, fmt.Sprintf("failed to generate sas token in creating new shared key credential, accountName: %s, err: %s", "accountName", "decode account key: illegal base64 data at input byte 8"))
2018-
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
2012+
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
20192013
if !reflect.DeepEqual(err, expectedErr) || !reflect.DeepEqual(accountSASToken, expectedAccountSASToken) {
20202014
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
20212015
}
@@ -2036,7 +2030,7 @@ func TestGetAzcopyAuth(t *testing.T) {
20362030
ctx := context.Background()
20372031
expectedAccountSASToken := ""
20382032
expectedErr := status.Errorf(codes.Internal, fmt.Sprintf("failed to generate sas token in creating new shared key credential, accountName: %s, err: %s", "accountName", "decode account key: illegal base64 data at input byte 8"))
2039-
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
2033+
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
20402034
if !reflect.DeepEqual(err, expectedErr) || !reflect.DeepEqual(accountSASToken, expectedAccountSASToken) {
20412035
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
20422036
}

0 commit comments

Comments
 (0)