Merge pull request #1202 from k8s-infra-cherrypick-robot/cherry-pick-1201-to-release-1.23

[release-1.23] cleanup: add cache in sastoken fallback

Author: andyzhangx

pkg/blob/blob.go

@@ -212,6 +212,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
@@ -256,6 +258,9 @@ func NewDriver(options *DriverOptions) *Driver {
 	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

@@ -91,6 +91,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
 	assert.Equal(t, driver, fakedriver)
 }

pkg/blob/controllerserver.go

@@ -828,18 +828,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

@@ -1789,7 +1789,7 @@ func TestCopyVolume(t *testing.T) {
 	}
 }
 
-func Test_parseDays(t *testing.T) {
+func TestParseDays(t *testing.T) {
 	type args struct {
 		dayStr string
 	}
@@ -1838,7 +1838,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
@@ -1876,7 +1876,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)
@@ -1995,7 +1995,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)