Merge pull request #1201 from andyzhangx/add-sastoken-get-cache

cleanup: add cache in sastoken fallback

Author: andyzhangx

pkg/blob/blob.go

@@ -219,6 +219,8 @@ type Driver struct {
 	accountSearchCache azcache.Resource
 	// a timed cache storing volume stats <volumeID, volumeStats>
 	volStatsCache azcache.Resource
+	// a timed cache storing account which should use sastoken for azcopy based volume cloning
+	azcopySasTokenCache azcache.Resource
 	// sas expiry time for azcopy in volume clone
 	sasTokenExpirationMinutes int
 	// azcopy for provide exec mock for ut
@@ -258,6 +260,9 @@ func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *p
 	if d.dataPlaneAPIVolCache, err = azcache.NewTimedCache(10*time.Minute, getter, false); err != nil {
 		klog.Fatalf("%v", err)
 	}
+	if d.azcopySasTokenCache, err = azcache.NewTimedCache(15*time.Minute, getter, false); err != nil {
+		klog.Fatalf("%v", err)
+	}
 
 	if options.VolStatsCacheExpireInMinutes <= 0 {
 		options.VolStatsCacheExpireInMinutes = 10 // default expire in 10 minutes

pkg/blob/blob_test.go

@@ -92,6 +92,7 @@ func TestNewDriver(t *testing.T) {
 	fakedriver.Version = driverVersion
 	fakedriver.accountSearchCache = driver.accountSearchCache
 	fakedriver.dataPlaneAPIVolCache = driver.dataPlaneAPIVolCache
+	fakedriver.azcopySasTokenCache = driver.azcopySasTokenCache
 	fakedriver.volStatsCache = driver.volStatsCache
 	fakedriver.cloud = driver.cloud
 	assert.Equal(t, driver, fakedriver)

pkg/blob/controllerserver.go

@@ -829,18 +829,29 @@ func (d *Driver) authorizeAzcopyWithIdentity() ([]string, error) {
 // 3. azcopy returns AuthorizationPermissionMismatch error when using service principal or managed identity
 func (d *Driver) getSASToken(ctx context.Context, accountName, accountKey, storageEndpointSuffix string, accountOptions *azure.AccountOptions, secrets map[string]string, secretName, secretNamespace string) (string, error) {
 	authAzcopyEnv, _ := d.authorizeAzcopyWithIdentity()
-	useSasTokenFallBack := false
+	useSasToken := false
 	if len(authAzcopyEnv) > 0 {
-		out, testErr := d.azcopy.TestListJobs(accountName, storageEndpointSuffix, authAzcopyEnv)
-		if testErr != nil {
-			return "", fmt.Errorf("azcopy list command failed with error(%v): %v", testErr, out)
+		// search in cache first
+		cache, err := d.azcopySasTokenCache.Get(accountName, azcache.CacheReadTypeDefault)
+		if err != nil {
+			return "", fmt.Errorf("get(%s) from azcopySasTokenCache failed with error: %v", accountName, err)
 		}
-		if strings.Contains(out, authorizationPermissionMismatch) {
-			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)
-			useSasTokenFallBack = true
+		if cache != nil {
+			klog.V(2).Infof("use sas token for account(%s) since this account is found in azcopySasTokenCache", accountName)
+			useSasToken = true
+		} else {
+			out, testErr := d.azcopy.TestListJobs(accountName, storageEndpointSuffix, authAzcopyEnv)
+			if testErr != nil {
+				return "", fmt.Errorf("azcopy list command failed with error(%v): %v", testErr, out)
+			}
+			if strings.Contains(out, authorizationPermissionMismatch) {
+				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)
+				d.azcopySasTokenCache.Set(accountName, "")
+				useSasToken = true
+			}
 		}
 	}
-	if len(secrets) > 0 || len(authAzcopyEnv) == 0 || useSasTokenFallBack {
+	if len(secrets) > 0 || len(authAzcopyEnv) == 0 || useSasToken {
 		var err error
 		if accountKey == "" {
 			if _, accountKey, err = d.GetStorageAccesskey(ctx, accountOptions, secrets, secretName, secretNamespace); err != nil {

pkg/blob/controllerserver_test.go

@@ -1748,7 +1748,7 @@ func TestCopyVolume(t *testing.T) {
 	}
 }
 
-func Test_parseDays(t *testing.T) {
+func TestParseDays(t *testing.T) {
 	type args struct {
 		dayStr string
 	}
@@ -1797,7 +1797,7 @@ func Test_parseDays(t *testing.T) {
 	}
 }
 
-func Test_generateSASToken(t *testing.T) {
+func TestGenerateSASToken(t *testing.T) {
 	storageEndpointSuffix := "core.windows.net"
 	tests := []struct {
 		name        string
@@ -1835,7 +1835,7 @@ func Test_generateSASToken(t *testing.T) {
 	}
 }
 
-func Test_authorizeAzcopyWithIdentity(t *testing.T) {
+func TestAuthorizeAzcopyWithIdentity(t *testing.T) {
 	testCases := []struct {
 		name     string
 		testFunc func(t *testing.T)
@@ -1966,7 +1966,7 @@ func Test_authorizeAzcopyWithIdentity(t *testing.T) {
 	}
 }
 
-func Test_getSASToken(t *testing.T) {
+func TestGetSASToken(t *testing.T) {
 	testCases := []struct {
 		name     string
 		testFunc func(t *testing.T)