Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add GetLatestAccountKey in account key fetch #4067

Merged
merged 1 commit into from
Jun 9, 2023
Merged

feat: add GetLatestAccountKey in account key fetch #4067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/Azure/azure-sdk-for-go v68.0.0+incompatible
github.com/Azure/go-autorest/autorest v0.11.29
github.com/Azure/go-autorest/autorest/adal v0.9.23
github.com/Azure/go-autorest/autorest/date v0.3.0
github.com/Azure/go-autorest/autorest/mocks v0.4.2
github.com/Azure/go-autorest/tracing v0.6.0
github.com/evanphx/json-patch v5.6.0+incompatible
Expand Down Expand Up @@ -45,7 +46,6 @@ require (
github.com/Azure/azure-storage-queue-go v0.0.0-20191125232315-636801874cdd // indirect
github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
github.com/Azure/go-autorest v14.2.0+incompatible // indirect
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
github.com/Azure/go-autorest/autorest/to v0.4.0 // indirect
github.com/Azure/go-autorest/autorest/validation v0.3.1 // indirect
github.com/Azure/go-autorest/logger v0.2.1 // indirect
Expand Down
37 changes: 32 additions & 5 deletions pkg/provider/azure_storageaccount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"math/big"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network"
"github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns"
Expand Down Expand Up @@ -74,6 +75,7 @@ type AccountOptions struct {
SubnetName string
AccessTier string
MatchTags bool
GetLatestAccountKey bool
EnableBlobVersioning *bool
SoftDeleteBlobs int32
SoftDeleteContainers int32
Expand Down Expand Up @@ -125,7 +127,8 @@ func (az *Cloud) getStorageAccounts(ctx context.Context, accountOptions *Account
}

// GetStorageAccesskey gets the storage account access key
func (az *Cloud) GetStorageAccesskey(ctx context.Context, subsID, account, resourceGroup string) (string, error) {
// getLatestAccountKey: get the latest account key per CreationTime if true, otherwise get the first account key
func (az *Cloud) GetStorageAccesskey(ctx context.Context, subsID, account, resourceGroup string, getLatestAccountKey bool) (string, error) {
if az.StorageAccountClient == nil {
return "", fmt.Errorf("StorageAccountClient is nil")
}
Expand All @@ -138,16 +141,40 @@ func (az *Cloud) GetStorageAccesskey(ctx context.Context, subsID, account, resou
return "", fmt.Errorf("empty keys")
}

var key string
var creationTime time.Time

for _, k := range *result.Keys {
if k.Value != nil && *k.Value != "" {
v := *k.Value
if ind := strings.LastIndex(v, " "); ind >= 0 {
v = v[(ind + 1):]
}
return v, nil
if !getLatestAccountKey {
// get first key
return v, nil
}
// get account key with latest CreationTime
if key == "" {
key = v
if k.CreationTime != nil {
creationTime = k.CreationTime.ToTime()
}
klog.V(2).Infof("got storage account key with creation time: %v", creationTime)
andyzhangx marked this conversation as resolved.
Show resolved Hide resolved
} else {
if k.CreationTime != nil && creationTime.Before(k.CreationTime.ToTime()) {
key = v
creationTime = k.CreationTime.ToTime()
klog.V(2).Infof("got storage account key with latest creation time: %v", creationTime)
}
}
}
}
return "", fmt.Errorf("no valid keys")

if key == "" {
return "", fmt.Errorf("no valid keys")
}
return key, nil
}

// EnsureStorageAccount search storage account, create one storage account(with genAccountNamePrefix) if not found, return accountName, accountKey
Expand Down Expand Up @@ -239,7 +266,7 @@ func (az *Cloud) EnsureStorageAccount(ctx context.Context, accountOptions *Accou
createNewAccount = false
if accountOptions.CreateAccount {
// check whether account exists
if _, err := az.GetStorageAccesskey(ctx, subsID, accountName, resourceGroup); err != nil {
if _, err := az.GetStorageAccesskey(ctx, subsID, accountName, resourceGroup, accountOptions.GetLatestAccountKey); err != nil {
klog.V(2).Infof("get storage key for storage account %s returned with %v", accountName, err)
createNewAccount = true
}
Expand Down Expand Up @@ -469,7 +496,7 @@ func (az *Cloud) EnsureStorageAccount(ctx context.Context, accountOptions *Accou
}

// find the access key with this account
accountKey, err := az.GetStorageAccesskey(ctx, subsID, accountName, resourceGroup)
accountKey, err := az.GetStorageAccesskey(ctx, subsID, accountName, resourceGroup, accountOptions.GetLatestAccountKey)
if err != nil {
return "", "", fmt.Errorf("could not get storage key for storage account %s: %w", accountName, err)
}
Expand Down
48 changes: 41 additions & 7 deletions pkg/provider/azure_storageaccount_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ import (
"fmt"
"strings"
"testing"
"time"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network"
"github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns"
"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage"
"github.com/Azure/go-autorest/autorest/date"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"k8s.io/utils/pointer"
Expand All @@ -51,20 +53,27 @@ func TestGetStorageAccessKeys(t *testing.T) {

cloud := &Cloud{}
value := "foo bar"
oldTime := date.Time{}
oldTime.Time = time.Now().Add(-time.Hour)
newValue := "newkey"
newTime := date.Time{}
newTime.Time = time.Now()

tests := []struct {
results storage.AccountListKeysResult
expectedKey string
expectErr bool
err error
results storage.AccountListKeysResult
getLatestAccountKey bool
expectedKey string
expectErr bool
err error
}{
{storage.AccountListKeysResult{}, "", true, nil},
{storage.AccountListKeysResult{}, false, "", true, nil},
{
storage.AccountListKeysResult{
Keys: &[]storage.AccountKey{
{Value: &value},
},
},
false,
"bar",
false,
nil,
Expand All @@ -76,18 +85,43 @@ func TestGetStorageAccessKeys(t *testing.T) {
{Value: &value},
},
},
false,
"bar",
false,
nil,
},
{
storage.AccountListKeysResult{
Keys: &[]storage.AccountKey{
{Value: &value, CreationTime: &oldTime},
{Value: &newValue, CreationTime: &newTime},
},
},
true,
"newkey",
false,
nil,
},
{
storage.AccountListKeysResult{
Keys: &[]storage.AccountKey{
{Value: &value, CreationTime: &oldTime},
{Value: &newValue, CreationTime: &newTime},
},
},
false,
"bar",
false,
nil,
},
{storage.AccountListKeysResult{}, "", true, fmt.Errorf("test error")},
{storage.AccountListKeysResult{}, false, "", true, fmt.Errorf("test error")},
}

for _, test := range tests {
mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl)
cloud.StorageAccountClient = mockStorageAccountsClient
mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), "", "rg", gomock.Any()).Return(test.results, nil).AnyTimes()
key, err := cloud.GetStorageAccesskey(ctx, "", "acct", "rg")
key, err := cloud.GetStorageAccesskey(ctx, "", "acct", "rg", test.getLatestAccountKey)
if test.expectErr && err == nil {
t.Errorf("Unexpected non-error")
continue
Expand Down