Skip to content

Commit

Permalink
fix: add cache in sastoken fallback
Browse files Browse the repository at this point in the history
fix
  • Loading branch information
andyzhangx authored and k8s-infra-cherrypick-robot committed Jan 3, 2024
1 parent 2df0715 commit 68ef58d
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 12 deletions.
5 changes: 5 additions & 0 deletions pkg/blob/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/blob/blob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
27 changes: 19 additions & 8 deletions pkg/blob/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/blob/controllerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 68ef58d

Please sign in to comment.