From cc8f29c30b5dfbbe621b1b6b5aac0561a0644aee Mon Sep 17 00:00:00 2001 From: Kipp Morris <117932707+kimorris27@users.noreply.github.com> Date: Thu, 7 Nov 2024 07:45:22 -0800 Subject: [PATCH] Fix flaky unit test cases (#3945) Tests now account for the fact that the actual code iterates over a map, so the order can differ between test executions --- pkg/validate/dynamic/dynamic_test.go | 14 ++++++++++---- test/util/error/error.go | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pkg/validate/dynamic/dynamic_test.go b/pkg/validate/dynamic/dynamic_test.go index 2ffa5ca1d..7c24dc15a 100644 --- a/pkg/validate/dynamic/dynamic_test.go +++ b/pkg/validate/dynamic/dynamic_test.go @@ -1573,7 +1573,7 @@ func TestValidatePreconfiguredNSGPermissions(t *testing.T) { platformIdentityMap map[string][]string checkAccessMocks func(context.CancelFunc, *mock_checkaccess.MockRemotePDPClient, *mock_azcore.MockTokenCredential, *mock_env.MockInterface) vnetMocks func(*mock_network.MockVirtualNetworksClient, mgmtnetwork.VirtualNetwork) - wantErr string + wantErrs []string }{ { name: "pass: skip when preconfiguredNSG is not enabled", @@ -1616,7 +1616,10 @@ func TestValidatePreconfiguredNSGPermissions(t *testing.T) { }, ).AnyTimes() }, - wantErr: "400: InvalidServicePrincipalPermissions: : The cluster service principal (Application ID: fff51942-b1f9-4119-9453-aaa922259eb7) does not have Network Contributor role on network security group '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/networkSecurityGroups/aro-node-nsg'. This is required when the enable-preconfigured-nsg option is specified.", + wantErrs: []string{ + "400: InvalidServicePrincipalPermissions: : The cluster service principal (Application ID: fff51942-b1f9-4119-9453-aaa922259eb7) does not have Network Contributor role on network security group '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/networkSecurityGroups/aro-node-nsg'. This is required when the enable-preconfigured-nsg option is specified.", + "400: InvalidServicePrincipalPermissions: : The cluster service principal (Application ID: fff51942-b1f9-4119-9453-aaa922259eb7) does not have Network Contributor role on network security group '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/networkSecurityGroups/aro-controlplane-nsg'. This is required when the enable-preconfigured-nsg option is specified.", + }, }, { name: "Fail - MIWI Cluster - permissions don't exist on all nsg", @@ -1657,7 +1660,10 @@ func TestValidatePreconfiguredNSGPermissions(t *testing.T) { platformIdentityMap: map[string][]string{ "Dummy": platformIdentity1SubnetActions, }, - wantErr: "400: InvalidWorkloadIdentityPermissions: : The Dummy platform managed identity does not have required permissions on network security group '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/networkSecurityGroups/aro-node-nsg'. This is required when the enable-preconfigured-nsg option is specified.", + wantErrs: []string{ + "400: InvalidWorkloadIdentityPermissions: : The Dummy platform managed identity does not have required permissions on network security group '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/networkSecurityGroups/aro-node-nsg'. This is required when the enable-preconfigured-nsg option is specified.", + "400: InvalidWorkloadIdentityPermissions: : The Dummy platform managed identity does not have required permissions on network security group '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/networkSecurityGroups/aro-controlplane-nsg'. This is required when the enable-preconfigured-nsg option is specified.", + }, }, { name: "pass: sp has the required permission on the NSG", @@ -1827,7 +1833,7 @@ func TestValidatePreconfiguredNSGPermissions(t *testing.T) { } err := dv.ValidatePreConfiguredNSGs(ctx, oc, subnets) - utilerror.AssertErrorMessage(t, err, tt.wantErr) + utilerror.AssertOneOfErrorMessages(t, err, tt.wantErrs) }) } } diff --git a/test/util/error/error.go b/test/util/error/error.go index 1e1b1fc4a..c83b95dee 100644 --- a/test/util/error/error.go +++ b/test/util/error/error.go @@ -1,6 +1,9 @@ package error -import "testing" +import ( + "slices" + "testing" +) // Copyright (c) Microsoft Corporation. // Licensed under the Apache License 2.0. @@ -16,3 +19,15 @@ func AssertErrorMessage(t *testing.T, err error, wantMsg string) { t.Errorf("got error '%v', but wanted error '%v'", err, wantMsg) } } + +// AssertOneOfErrorMessages asserts that err.Error() is in wantMsgs. +func AssertOneOfErrorMessages(t *testing.T, err error, wantMsgs []string) { + t.Helper() + if err == nil && len(wantMsgs) > 0 { + t.Errorf("did not get an error, but wanted one of these errors: '%v'", wantMsgs) + } + + if err != nil && !slices.Contains(wantMsgs, err.Error()) { + t.Errorf("got error '%v', but wanted one of these errors: '%v'", err, wantMsgs) + } +}