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.

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

[release-1.29] fix: VirtualNetworkRule match issue during account search #6743

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions pkg/provider/azure_storageaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -839,17 +839,18 @@
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, ""))

Check failure on line 851 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

undefined: ptr

Check failure on line 851 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr

Check failure on line 851 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr
return false
}
}
}
return true
Expand All @@ -869,7 +870,7 @@
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, ""))

Check failure on line 873 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

undefined: ptr

Check failure on line 873 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr

Check failure on line 873 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr
return false
}
}
Expand Down Expand Up @@ -960,7 +961,7 @@
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, ""))

Check failure on line 964 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

undefined: ptr

Check failure on line 964 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr

Check failure on line 964 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr
if err != nil {
return false, err
}
Expand All @@ -985,7 +986,7 @@
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, ""))

Check failure on line 989 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

undefined: ptr

Check failure on line 989 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr
if err != nil {
return false, err
}
Expand All @@ -1007,7 +1008,7 @@
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, ""))

Check failure on line 1011 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

undefined: ptr

Check failure on line 1011 in pkg/provider/azure_storageaccount.go

View workflow job for this annotation

GitHub Actions / Lint

undefined: ptr) (typecheck)
if err != nil {
return false, err
}
Expand Down
104 changes: 103 additions & 1 deletion pkg/provider/azure_storageaccount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
})
}
}
Loading