From 31f9d427b3c3491440700dd9a3c4a24e44562ca3 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Mon, 5 Aug 2024 07:48:42 +0000 Subject: [PATCH] fix: VirtualNetworkRule match issue during account search fix fix --- pkg/provider/azure_storageaccount.go | 17 ++-- pkg/provider/azure_storageaccount_test.go | 104 +++++++++++++++++++++- 2 files changed, 112 insertions(+), 9 deletions(-) diff --git a/pkg/provider/azure_storageaccount.go b/pkg/provider/azure_storageaccount.go index fa75f00a6b..13c72390ea 100644 --- a/pkg/provider/azure_storageaccount.go +++ b/pkg/provider/azure_storageaccount.go @@ -842,17 +842,18 @@ func AreVNetRulesEqual(account storage.Account, accountOptions *AccountOptions) return false } - found := false for _, subnetID := range accountOptions.VirtualNetworkResourceIDs { + found := false for _, rule := range *account.AccountProperties.NetworkRuleSet.VirtualNetworkRules { if strings.EqualFold(ptr.Deref(rule.VirtualNetworkResourceID, ""), subnetID) && rule.Action == storage.ActionAllow { found = true break } } - } - if !found { - return false + if !found { + klog.V(2).Infof("subnetID(%s) not found in account(%s) virtual network rules", subnetID, ptr.Deref(account.Name, "")) + return false + } } } return true @@ -872,7 +873,7 @@ func isTaggedWithSkip(account storage.Account) bool { if account.Tags != nil { // skip account with SkipMatchingTag tag if _, ok := account.Tags[SkipMatchingTag]; ok { - klog.V(2).Infof("found %s tag for account %s, skip matching", SkipMatchingTag, *account.Name) + klog.V(2).Infof("found %s tag for account %s, skip matching", SkipMatchingTag, ptr.Deref(account.Name, "")) return false } } @@ -963,7 +964,7 @@ func (az *Cloud) isMultichannelEnabledEqual(ctx context.Context, account storage return false, nil } - prop, err := az.getFileServicePropertiesCache(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, *account.Name) + prop, err := az.getFileServicePropertiesCache(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, ptr.Deref(account.Name, "")) if err != nil { return false, err } @@ -988,7 +989,7 @@ func (az *Cloud) isDisableFileServiceDeleteRetentionPolicyEqual(ctx context.Cont return false, nil } - prop, err := az.FileClient.WithSubscriptionID(accountOptions.SubscriptionID).GetServiceProperties(ctx, accountOptions.ResourceGroup, *account.Name) + prop, err := az.FileClient.WithSubscriptionID(accountOptions.SubscriptionID).GetServiceProperties(ctx, accountOptions.ResourceGroup, ptr.Deref(account.Name, "")) if err != nil { return false, err } @@ -1010,7 +1011,7 @@ func (az *Cloud) isEnableBlobDataProtectionEqual(ctx context.Context, account st return true, nil } - property, err := az.BlobClient.GetServiceProperties(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, *account.Name) + property, err := az.BlobClient.GetServiceProperties(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, ptr.Deref(account.Name, "")) if err != nil { return false, err } diff --git a/pkg/provider/azure_storageaccount_test.go b/pkg/provider/azure_storageaccount_test.go index ce7ba2ee3c..f69a2f1f30 100644 --- a/pkg/provider/azure_storageaccount_test.go +++ b/pkg/provider/azure_storageaccount_test.go @@ -1844,7 +1844,7 @@ func TestIsDisableFileServiceDeleteRetentionPolicyEqual(t *testing.T) { } } -func Test_isSoftDeleteBlobsEqual(t *testing.T) { +func TestIsSoftDeleteBlobsEqual(t *testing.T) { type args struct { property storage.BlobServiceProperties accountOptions *AccountOptions @@ -2094,3 +2094,105 @@ func TestParseServiceAccountToken(t *testing.T) { t.Errorf("ParseServiceAccountToken(%s) = %s, want %s", saTokens, token, expectedToken) } } + +func TestAreVNetRulesEqual(t *testing.T) { + type args struct { + account storage.Account + accountOption *AccountOptions + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "account option is empty", + args: args{ + account: storage.Account{ + AccountProperties: &storage.AccountProperties{}, + }, + accountOption: &AccountOptions{ + VirtualNetworkResourceIDs: []string{}, + }, + }, + want: true, + }, + { + name: "VirtualNetworkRules are equal", + args: args{ + account: storage.Account{ + AccountProperties: &storage.AccountProperties{ + NetworkRuleSet: &storage.NetworkRuleSet{ + VirtualNetworkRules: &[]storage.VirtualNetworkRule{ + { + VirtualNetworkResourceID: ptr.To("id"), + Action: storage.ActionAllow, + State: "state", + }, + }, + }, + }, + }, + accountOption: &AccountOptions{ + VirtualNetworkResourceIDs: []string{"id"}, + }, + }, + want: true, + }, + { + name: "VirtualNetworkRules are equal with multiple NetworkRules", + args: args{ + account: storage.Account{ + AccountProperties: &storage.AccountProperties{ + NetworkRuleSet: &storage.NetworkRuleSet{ + VirtualNetworkRules: &[]storage.VirtualNetworkRule{ + { + VirtualNetworkResourceID: ptr.To("id1"), + Action: storage.ActionAllow, + }, + { + VirtualNetworkResourceID: ptr.To("id2"), + Action: storage.ActionAllow, + }, + }, + }, + }, + }, + accountOption: &AccountOptions{ + VirtualNetworkResourceIDs: []string{"id2"}, + }, + }, + want: true, + }, + { + name: "VirtualNetworkRules not equal", + args: args{ + account: storage.Account{ + AccountProperties: &storage.AccountProperties{ + NetworkRuleSet: &storage.NetworkRuleSet{ + VirtualNetworkRules: &[]storage.VirtualNetworkRule{ + { + VirtualNetworkResourceID: ptr.To("id1"), + Action: storage.ActionAllow, + State: "state", + }, + }, + }, + }, + }, + accountOption: &AccountOptions{ + VirtualNetworkResourceIDs: []string{"id2"}, + }, + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := AreVNetRulesEqual(tt.args.account, tt.args.accountOption); got != tt.want { + t.Errorf("areVNetRulesEqual() = %v, want %v", got, tt.want) + } + }) + } +}