Skip to content

Commit e005c99

Browse files
andyzhangxandyzhangx
committed
fix: add fallback to sas token on azcopy copy command
1 parent 077a33b commit e005c99

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

@@ -775,7 +785,7 @@ func (d *Driver) copyBlobContainer(ctx context.Context, req *csi.CreateVolumeReq
775785
SubscriptionID: srcSubscriptionID,
776786
GetLatestAccountKey: accountOptions.GetLatestAccountKey,
777787
}
778-
if srcAccountSasToken, _, err = d.getAzcopyAuth(ctx, srcAccountName, "", storageEndpointSuffix, srcAccountOptions, nil, "", secretNamespace); err != nil {
788+
if srcAccountSasToken, _, err = d.getAzcopyAuth(ctx, srcAccountName, "", storageEndpointSuffix, srcAccountOptions, nil, "", secretNamespace, true); err != nil {
779789
return err
780790
}
781791
}
@@ -868,11 +878,10 @@ func (d *Driver) authorizeAzcopyWithIdentity() ([]string, error) {
868878
// getAzcopyAuth will only generate sas token for azcopy in following conditions:
869879
// 1. secrets is not empty
870880
// 2. driver is not using managed identity and service principal
871-
// 3. azcopy returns AuthorizationPermissionMismatch error when using service principal or managed identity
872-
func (d *Driver) getAzcopyAuth(ctx context.Context, accountName, accountKey, storageEndpointSuffix string, accountOptions *azure.AccountOptions, secrets map[string]string, secretName, secretNamespace string) (string, []string, error) {
881+
// 3. parameter useSasToken is true
882+
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) {
873883
var authAzcopyEnv []string
874884
var err error
875-
useSasToken := false
876885
if !d.useDataPlaneAPI("", accountName) && len(secrets) == 0 && len(secretName) == 0 {
877886
// search in cache first
878887
if cache, err := d.azcopySasTokenCache.Get(accountName, azcache.CacheReadTypeDefault); err == nil && cache != nil {
@@ -883,17 +892,6 @@ func (d *Driver) getAzcopyAuth(ctx context.Context, accountName, accountKey, sto
883892
authAzcopyEnv, err = d.authorizeAzcopyWithIdentity()
884893
if err != nil {
885894
klog.Warningf("failed to authorize azcopy with identity, error: %v", err)
886-
} else {
887-
if len(authAzcopyEnv) > 0 {
888-
out, testErr := d.azcopy.TestListJobs(accountName, storageEndpointSuffix, authAzcopyEnv)
889-
if testErr != nil {
890-
return "", nil, fmt.Errorf("azcopy list command failed with error(%v): %v", testErr, out)
891-
}
892-
if strings.Contains(out, authorizationPermissionMismatch) {
893-
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)
894-
useSasToken = true
895-
}
896-
}
897895
}
898896
}
899897

‎pkg/blob/controllerserver_test.go

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

855-
m := util.NewMockEXEC(ctrl)
856-
857855
clientFactoryMock := mock_azclient.NewMockClientFactory(ctrl)
858856
blobClientMock := mock_blobcontainerclient.NewMockInterface(ctrl)
859857
blobClientMock.EXPECT().CreateContainer(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil)
860858
clientFactoryMock.EXPECT().GetBlobContainerClientForSub(gomock.Any()).Return(blobClientMock, nil)
861859
d.clientFactory = clientFactoryMock
862860

863-
listStr := "no error"
864-
m.EXPECT().RunCommand(gomock.Any(), gomock.Any()).Return(listStr, nil)
865-
d.azcopy.ExecCmd = m
866-
867861
expectedErr := status.Errorf(codes.NotFound, "error parsing volume id: \"unit-test\", should at least contain two #")
868862
_, err := d.CreateVolume(context.Background(), req)
869863
if !reflect.DeepEqual(err, expectedErr) {
@@ -2024,7 +2018,7 @@ func TestGetAzcopyAuth(t *testing.T) {
20242018
ctx := context.Background()
20252019
expectedAccountSASToken := ""
20262020
expectedErr := fmt.Errorf("could not find accountkey or azurestorageaccountkey field in secrets")
2027-
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
2021+
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
20282022
if !reflect.DeepEqual(err, expectedErr) || !reflect.DeepEqual(accountSASToken, expectedAccountSASToken) {
20292023
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
20302024
}
@@ -2047,7 +2041,7 @@ func TestGetAzcopyAuth(t *testing.T) {
20472041
defaultSecretAccountName: "accountName",
20482042
defaultSecretAccountKey: "YWNjb3VudGtleQo=",
20492043
}
2050-
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
2044+
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
20512045
if !reflect.DeepEqual(err, nil) || !strings.Contains(accountSASToken, "?se=") {
20522046
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
20532047
}
@@ -2073,7 +2067,7 @@ func TestGetAzcopyAuth(t *testing.T) {
20732067

20742068
expectedAccountSASToken := ""
20752069
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"))
2076-
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
2070+
accountSASToken, _, err := d.getAzcopyAuth(context.Background(), "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
20772071
if !reflect.DeepEqual(err, expectedErr) || !reflect.DeepEqual(accountSASToken, expectedAccountSASToken) {
20782072
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
20792073
}
@@ -2094,7 +2088,7 @@ func TestGetAzcopyAuth(t *testing.T) {
20942088
ctx := context.Background()
20952089
expectedAccountSASToken := ""
20962090
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"))
2097-
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace")
2091+
accountSASToken, _, err := d.getAzcopyAuth(ctx, "accountName", "", "core.windows.net", &azure.AccountOptions{}, secrets, "secretsName", "secretsNamespace", false)
20982092
if !reflect.DeepEqual(err, expectedErr) || !reflect.DeepEqual(accountSASToken, expectedAccountSASToken) {
20992093
t.Errorf("Unexpected accountSASToken: %s, Unexpected error: %v", accountSASToken, err)
21002094
}

0 commit comments

Comments
 (0)