fix: VirtualNetworkRule match issue during account search

fix fix
k8s-infra-cherrypick-robot · Aug 5, 2024 · b8be616 · b8be616
1 parent e27dae9
commit b8be616
Show file tree

Hide file tree

Showing 2 changed files with 112 additions and 9 deletions.
diff --git 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(pointer.StringDeref(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
@@ -1842,7 +1842,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
@@ -2092,3 +2092,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)
+			}
+		})
+	}
+}