Skip to content

Commit 6cb06f9

Browse files
committed
chore: refine sastoken cache
fix
1 parent d140920 commit 6cb06f9

File tree

3 files changed

+23
-19
lines changed

3 files changed

+23
-19
lines changed

pkg/azurefile/azurefile.go

-1
Original file line numberDiff line numberDiff line change
@@ -1039,7 +1039,6 @@ func (d *Driver) copyFileShare(ctx context.Context, req *csi.CreateVolumeRequest
10391039
copyErr := fileutil.WaitForExecCompletion(time.Duration(d.waitForAzCopyTimeoutMinutes)*time.Minute, execFuncWithAuth, timeoutFunc)
10401040
if accountSASToken == "" && copyErr != nil && strings.Contains(copyErr.Error(), authorizationPermissionMismatch) {
10411041
klog.Warningf("azcopy list failed with AuthorizationPermissionMismatch error, should assign \"Storage File Data SMB Share Elevated Contributor\" role to controller identity, fall back to use sas token, original error: %v", copyErr)
1042-
d.azcopySasTokenCache.Set(accountName, "")
10431042
var sasToken string
10441043
if sasToken, _, err = d.getAzcopyAuth(ctx, accountName, "", storageEndpointSuffix, accountOptions, secrets, secretName, secretNamespace, true); err != nil {
10451044
return err

pkg/azurefile/controllerserver.go

+21-17
Original file line numberDiff line numberDiff line change
@@ -1230,42 +1230,44 @@ func (d *Driver) authorizeAzcopyWithIdentity() ([]string, error) {
12301230
// 4. parameter useSasToken is true
12311231
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) {
12321232
var authAzcopyEnv []string
1233+
var err error
12331234
if !useSasToken && len(secrets) == 0 && len(secretName) == 0 {
1234-
var err error
1235+
// search in cache first
1236+
if cache, err := d.azcopySasTokenCache.Get(accountName, azcache.CacheReadTypeDefault); err == nil && cache != nil {
1237+
klog.V(2).Infof("use sas token for account(%s) since this account is found in azcopySasTokenCache", accountName)
1238+
return cache.(string), nil, nil
1239+
}
12351240
authAzcopyEnv, err = d.authorizeAzcopyWithIdentity()
12361241
if err != nil {
12371242
klog.Warningf("failed to authorize azcopy with identity, error: %v", err)
1238-
} else {
1239-
if len(authAzcopyEnv) > 0 {
1240-
// search in cache first
1241-
cache, err := d.azcopySasTokenCache.Get(accountName, azcache.CacheReadTypeDefault)
1242-
if err != nil {
1243-
return "", nil, fmt.Errorf("get(%s) from azcopySasTokenCache failed with error: %v", accountName, err)
1244-
}
1245-
if cache != nil {
1246-
klog.V(2).Infof("use sas token for account(%s) since this account is found in azcopySasTokenCache", accountName)
1247-
useSasToken = true
1248-
}
1249-
}
12501243
}
12511244
}
12521245

12531246
if len(secrets) > 0 || len(secretName) > 0 || len(authAzcopyEnv) == 0 || useSasToken {
1254-
var err error
12551247
if accountKey == "" {
12561248
if accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, secrets, secretName, secretNamespace); err != nil {
12571249
return "", nil, err
12581250
}
12591251
}
12601252
klog.V(2).Infof("generate sas token for account(%s)", accountName)
1261-
sasToken, err := generateSASToken(accountName, accountKey, storageEndpointSuffix, d.sasTokenExpirationMinutes)
1253+
sasToken, err := d.generateSASToken(accountName, accountKey, storageEndpointSuffix, d.sasTokenExpirationMinutes)
12621254
return sasToken, nil, err
12631255
}
12641256
return "", authAzcopyEnv, nil
12651257
}
12661258

12671259
// generateSASToken generate a sas token for storage account
1268-
func generateSASToken(accountName, accountKey, storageEndpointSuffix string, expiryTime int) (string, error) {
1260+
func (d *Driver) generateSASToken(accountName, accountKey, storageEndpointSuffix string, expiryTime int) (string, error) {
1261+
// search in cache first
1262+
cache, err := d.azcopySasTokenCache.Get(accountName, azcache.CacheReadTypeDefault)
1263+
if err != nil {
1264+
return "", fmt.Errorf("get(%s) from azcopySasTokenCache failed with error: %v", accountName, err)
1265+
}
1266+
if cache != nil {
1267+
klog.V(2).Infof("use sas token for account(%s) since this account is found in azcopySasTokenCache", accountName)
1268+
return cache.(string), nil
1269+
}
1270+
12691271
credential, err := service.NewSharedKeyCredential(accountName, accountKey)
12701272
if err != nil {
12711273
return "", status.Errorf(codes.Internal, fmt.Sprintf("failed to generate sas token in creating new shared key credential, accountName: %s, err: %s", accountName, err.Error()))
@@ -1286,5 +1288,7 @@ func generateSASToken(accountName, accountKey, storageEndpointSuffix string, exp
12861288
if err != nil {
12871289
return "", err
12881290
}
1289-
return "?" + u.RawQuery, nil
1291+
sasToken := "?" + u.RawQuery
1292+
d.azcopySasTokenCache.Set(accountName, sasToken)
1293+
return sasToken, nil
12901294
}

pkg/azurefile/controllerserver_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -2713,6 +2713,7 @@ func TestSetAzureCredentials(t *testing.T) {
27132713
}
27142714

27152715
func TestGenerateSASToken(t *testing.T) {
2716+
d := NewFakeDriver()
27162717
storageEndpointSuffix := "core.windows.net"
27172718
tests := []struct {
27182719
name string
@@ -2738,7 +2739,7 @@ func TestGenerateSASToken(t *testing.T) {
27382739
}
27392740
for _, tt := range tests {
27402741
t.Run(tt.name, func(t *testing.T) {
2741-
sas, err := generateSASToken(tt.accountName, tt.accountKey, storageEndpointSuffix, 30)
2742+
sas, err := d.generateSASToken(tt.accountName, tt.accountKey, storageEndpointSuffix, 30)
27422743
if !reflect.DeepEqual(err, tt.expectedErr) {
27432744
t.Errorf("generateSASToken error = %v, expectedErr %v, sas token = %v, want %v", err, tt.expectedErr, sas, tt.want)
27442745
return

0 commit comments

Comments
 (0)