diff --git a/multicluster/test/e2e/antreapolicy_test.go b/multicluster/test/e2e/antreapolicy_test.go index 88df87af282..9bb2f5ad79b 100644 --- a/multicluster/test/e2e/antreapolicy_test.go +++ b/multicluster/test/e2e/antreapolicy_test.go @@ -54,8 +54,7 @@ func failOnError(err error, t *testing.T) { func initializeForPolicyTest(t *testing.T, data *MCTestData) { perNamespacePods = []string{"a", "b", "c"} perClusterNamespaces = make(map[string]antreae2e.TestNamespaceMeta) - nss := []string{"x", "y", "z"} - for _, ns := range nss { + for _, ns := range []string{"x", "y", "z"} { perClusterNamespaces[ns] = antreae2e.TestNamespaceMeta{Name: ns} } diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy.go b/pkg/controller/networkpolicy/clusternetworkpolicy.go index 99971bbd6b8..0387db11074 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy.go +++ b/pkg/controller/networkpolicy/clusternetworkpolicy.go @@ -16,6 +16,7 @@ package networkpolicy import ( "reflect" + "sort" "strings" v1 "k8s.io/api/core/v1" @@ -631,7 +632,9 @@ func groupNamespacesByLabelValue(affectedNSAndLabels map[string]labels.Set, labe func getLabelValues(labels map[string]string, labelKeys []string) string { key := "" for _, k := range labelKeys { - if v, ok := labels[k]; ok { + if v, ok := labels[k]; !ok { + return "" + } else { key += v + labelValueSeparator } } @@ -682,6 +685,7 @@ func (n *NetworkPolicyController) toAntreaPeerForSameLabelNamespaces(peer crdv1b LabelIdentities: labelIdentities, } var atgs []*antreatypes.AppliedToGroup + sort.Strings(namespacesByLabelValues) for _, ns := range namespacesByLabelValues { atgForNamespace, _ := atgPerAffectedNS[ns] atgs = append(atgs, atgForNamespace) diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy_test.go b/pkg/controller/networkpolicy/clusternetworkpolicy_test.go index 65a3462c401..b0164a081ce 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy_test.go +++ b/pkg/controller/networkpolicy/clusternetworkpolicy_test.go @@ -17,8 +17,6 @@ package networkpolicy import ( "fmt" "net" - "reflect" - "sort" "testing" "github.com/stretchr/testify/assert" @@ -36,48 +34,6 @@ import ( "antrea.io/antrea/pkg/util/k8s" ) -// ruleSemanticallyEqual compares two NetworkPolicyRule objects. It disregards -// the appliedToGroup slice element order as long as two rules' appliedToGroups -// have same elements. -func ruleSemanticallyEqual(a, b controlplane.NetworkPolicyRule) bool { - sort.Strings(a.AppliedToGroups) - sort.Strings(b.AppliedToGroups) - return reflect.DeepEqual(a, b) -} - -// diffNetworkPolicyRuleList checks if elements in two controlplane.NetworkPolicyRule -// slices are equal. If not, it returns the unmatched NetworkPolicyRules. -func diffNetworkPolicyRuleList(a, b []controlplane.NetworkPolicyRule) (extraA, extraB []controlplane.NetworkPolicyRule) { - if len(a) != len(b) { - return nil, nil - } - // Mark indexes in b that has already matched - visited := make([]bool, len(b)) - for i := 0; i < len(a); i++ { - found := false - for j := 0; j < len(b); j++ { - if visited[j] { - continue - } - if ruleSemanticallyEqual(a[i], b[j]) { - visited[j] = true - found = true - break - } - } - if !found { - extraA = append(extraA, a[i]) - } - } - for j := 0; j < len(b); j++ { - if visited[j] { - continue - } - extraB = append(extraB, b[j]) - } - return -} - func TestProcessClusterNetworkPolicy(t *testing.T) { p10 := float64(10) t10 := int32(10) @@ -1052,8 +1008,8 @@ func TestProcessClusterNetworkPolicy(t *testing.T) { { Direction: controlplane.DirectionIn, AppliedToGroups: []string{ - getNormalizedUID(antreatypes.NewGroupSelector("nsC", nil, nil, nil, nil).NormalizedName), getNormalizedUID(antreatypes.NewGroupSelector("nsB", nil, nil, nil, nil).NormalizedName), + getNormalizedUID(antreatypes.NewGroupSelector("nsC", nil, nil, nil, nil).NormalizedName), }, From: controlplane.NetworkPolicyPeer{ AddressGroups: []string{ @@ -1947,10 +1903,7 @@ func TestProcessClusterNetworkPolicy(t *testing.T) { assert.Equal(t, tt.expectedPolicy.Priority, actualPolicy.Priority) assert.Equal(t, tt.expectedPolicy.TierPriority, actualPolicy.TierPriority) assert.Equal(t, tt.expectedPolicy.AppliedToPerRule, actualPolicy.AppliedToPerRule) - missingExpectedRules, extraActualRules := diffNetworkPolicyRuleList(tt.expectedPolicy.Rules, actualPolicy.Rules) - if len(missingExpectedRules) > 0 || len(extraActualRules) > 0 { - t.Errorf("Unexpected rules in processed policy. Missing expected rules: %v. Extra actual rules: %v", missingExpectedRules, extraActualRules) - } + assert.ElementsMatch(t, tt.expectedPolicy.Rules, actualPolicy.Rules) assert.ElementsMatch(t, tt.expectedPolicy.AppliedToGroups, actualPolicy.AppliedToGroups) assert.Equal(t, tt.expectedAppliedToGroups, len(actualAppliedToGroups)) assert.Equal(t, tt.expectedAddressGroups, len(actualAddressGroups)) diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go index 47c210953df..7c4299eae78 100644 --- a/test/e2e/antreapolicy_test.go +++ b/test/e2e/antreapolicy_test.go @@ -49,7 +49,7 @@ var ( podsByNamespace map[string][]Pod k8sUtils *KubernetesUtils allTestList []*TestCase - pods []string + podsPerNamespace []string namespaces map[string]TestNamespaceMeta podIPs map[string][]string p80, p81, p8080, p8081, p8082, p8085, p6443 int32 @@ -66,11 +66,9 @@ const ( // Verification of deleting/creating resources timed out. timeout = 10 * time.Second // audit log directory on Antrea Agent - logDir = "/var/log/antrea/networkpolicy/" - logfileName = "np.log" - defaultTierName = "application" - formFactorNormal = "3by3PodWorkloads" - formFactorLarge = "extraNamespaces" + logDir = "/var/log/antrea/networkpolicy/" + logfileName = "np.log" + defaultTierName = "application" ) func failOnError(err error, t *testing.T) { @@ -105,56 +103,7 @@ func getPodName(ns, po string) string { return namespaces[ns].Name + "/" + po } -// initNamespaceMeta populates the test Namespaces metadata. -// There are two form factors for test workload Namespaces: -// -// Normal: three Namespaces x, y, z. -// Large: two "prod" Namespaces labeled purpose=test and tier=prod. -// two "dev" Namespaces labeled purpose=test and tier=dev. -// one "no-tier-label" Namespace labeled purpose=test. -// -// The large form factor workloads are used for testcases where advanced -// Namespace matching in policies are required. -func initNamespaceMeta(formFactor string) map[string]TestNamespaceMeta { - allNamespaceMeta := make(map[string]TestNamespaceMeta) - suffix := randName("") - if formFactor == formFactorLarge { - for i := 1; i < 3; i++ { - prodNS := TestNamespaceMeta{ - Name: "prod" + strconv.Itoa(i) + "-" + suffix, - Labels: map[string]string{ - "purpose": "test", - "tier": "prod", - }, - } - allNamespaceMeta["prod"+strconv.Itoa(i)] = prodNS - devNS := TestNamespaceMeta{ - Name: "dev" + strconv.Itoa(i) + "-" + suffix, - Labels: map[string]string{ - "purpose": "test", - "tier": "dev", - }, - } - allNamespaceMeta["dev"+strconv.Itoa(i)] = devNS - } - allNamespaceMeta["no-tier"] = TestNamespaceMeta{ - Name: "no-tier-" + suffix, - Labels: map[string]string{ - "purpose": "test-exclusion", - }, - } - } else if formFactor == formFactorNormal { - nss := []string{"x", "y", "z"} - for _, ns := range nss { - allNamespaceMeta[ns] = TestNamespaceMeta{ - Name: ns + "-" + suffix, - } - } - } - return allNamespaceMeta -} - -func initialize(t *testing.T, data *TestData, formFactor string) { +func initialize(t *testing.T, data *TestData, customNamespaces map[string]TestNamespaceMeta) { p80 = 80 p81 = 81 p8080 = 8080 @@ -164,15 +113,24 @@ func initialize(t *testing.T, data *TestData, formFactor string) { selfNamespace = &crdv1beta1.PeerNamespaces{ Match: crdv1beta1.NamespaceMatchSelf, } - pods = []string{"a", "b", "c"} - namespaces = initNamespaceMeta(formFactor) + namespaces = make(map[string]TestNamespaceMeta) + if customNamespaces != nil { + namespaces = customNamespaces + } else { + suffix := randName("") + for _, ns := range []string{"x", "y", "z"} { + namespaces[ns] = TestNamespaceMeta{ + Name: ns + "-" + suffix, + } + } + } // This function "initialize" will be used more than once, and variable "allPods" is global. // It should be empty every time when "initialize" is performed, otherwise there will be unexpected // results. allPods = []Pod{} podsByNamespace = make(map[string][]Pod) - - for _, podName := range pods { + podsPerNamespace = []string{"a", "b", "c"} + for _, podName := range podsPerNamespace { for _, ns := range namespaces { allPods = append(allPods, NewPod(ns.Name, podName)) podsByNamespace[ns.Name] = append(podsByNamespace[ns.Name], NewPod(ns.Name, podName)) @@ -184,7 +142,7 @@ func initialize(t *testing.T, data *TestData, formFactor string) { // k8sUtils is a global var k8sUtils, err = NewKubernetesUtils(data) failOnError(err, t) - ips, err := k8sUtils.Bootstrap(namespaces, pods, true, nil, nil) + ips, err := k8sUtils.Bootstrap(namespaces, podsPerNamespace, true, nil, nil) failOnError(err, t) podIPs = ips } @@ -4373,7 +4331,7 @@ func TestAntreaPolicy(t *testing.T) { } defer teardownTest(t, data) - initialize(t, data, formFactorNormal) + initialize(t, data, nil) // This test group only provides one case for each CR, including ACNP, ANNP, Tier, // ClusterGroup and Group to make sure the corresponding validation webhooks is @@ -4514,7 +4472,36 @@ func TestAntreaPolicyExtendedNamespaces(t *testing.T) { } defer teardownTest(t, data) - initialize(t, data, formFactorLarge) + extendedNamespaces := make(map[string]TestNamespaceMeta) + suffix := randName("") + // two "prod" Namespaces labeled purpose=test and tier=prod. + // two "dev" Namespaces labeled purpose=test and tier=dev. + // one "no-tier-label" Namespace labeled purpose=test. + for i := 1; i <= 2; i++ { + prodNS := TestNamespaceMeta{ + Name: "prod" + strconv.Itoa(i) + "-" + suffix, + Labels: map[string]string{ + "purpose": "test", + "tier": "prod", + }, + } + extendedNamespaces["prod"+strconv.Itoa(i)] = prodNS + devNS := TestNamespaceMeta{ + Name: "dev" + strconv.Itoa(i) + "-" + suffix, + Labels: map[string]string{ + "purpose": "test", + "tier": "dev", + }, + } + extendedNamespaces["dev"+strconv.Itoa(i)] = devNS + } + extendedNamespaces["no-tier"] = TestNamespaceMeta{ + Name: "no-tier-" + suffix, + Labels: map[string]string{ + "purpose": "test-exclusion", + }, + } + initialize(t, data, extendedNamespaces) t.Run("TestGroupACNPNamespaceLabelSelections", func(t *testing.T) { t.Run("Case=ACNPStrictNamespacesIsolationByLabels", func(t *testing.T) { testACNPStrictNamespacesIsolationByLabels(t) }) @@ -4658,7 +4645,7 @@ func TestAntreaPolicyStatusWithAppliedToUnsupportedGroup(t *testing.T) { } defer teardownTest(t, data) - initialize(t, data, formFactorNormal) + initialize(t, data, nil) testNamespace := getNS("x") // Build a Group with namespaceSelector selecting namespaces outside testNamespace. diff --git a/test/e2e/clustergroup_test.go b/test/e2e/clustergroup_test.go index 9308cd7b32f..6576546b1ae 100644 --- a/test/e2e/clustergroup_test.go +++ b/test/e2e/clustergroup_test.go @@ -320,7 +320,7 @@ func TestClusterGroup(t *testing.T) { } defer teardownTest(t, data) - initialize(t, data, formFactorNormal) + initialize(t, data, nil) t.Run("TestGroupClusterGroupValidate", func(t *testing.T) { t.Run("Case=IPBlockWithPodSelectorDenied", func(t *testing.T) { testInvalidCGIPBlockWithPodSelector(t) }) diff --git a/test/e2e/group_test.go b/test/e2e/group_test.go index 487d647047d..121c1a46627 100644 --- a/test/e2e/group_test.go +++ b/test/e2e/group_test.go @@ -263,7 +263,7 @@ func TestGroup(t *testing.T) { t.Fatalf("Error when setting up test: %v", err) } defer teardownTest(t, data) - initialize(t, data, formFactorNormal) + initialize(t, data, nil) t.Run("TestGroupNamespacedGroupValidate", func(t *testing.T) { t.Run("Case=IPBlockWithPodSelectorDenied", func(t *testing.T) { testInvalidGroupIPBlockWithPodSelector(t) }) diff --git a/test/e2e/k8s_util.go b/test/e2e/k8s_util.go index 71038752779..af3240d828b 100644 --- a/test/e2e/k8s_util.go +++ b/test/e2e/k8s_util.go @@ -1138,7 +1138,7 @@ func (k *KubernetesUtils) ValidateRemoteCluster(remoteCluster *KubernetesUtils, } } -func (k *KubernetesUtils) Bootstrap(namespaces map[string]TestNamespaceMeta, pods []string, createNamespaces bool, nodeNames map[string]string, hostNetworks map[string]bool) (map[string][]string, error) { +func (k *KubernetesUtils) Bootstrap(namespaces map[string]TestNamespaceMeta, podsPerNamespace []string, createNamespaces bool, nodeNames map[string]string, hostNetworks map[string]bool) (map[string][]string, error) { for key, ns := range namespaces { if createNamespaces { if ns.Labels == nil { @@ -1146,8 +1146,7 @@ func (k *KubernetesUtils) Bootstrap(namespaces map[string]TestNamespaceMeta, pod } // convenience label for testing ns.Labels["ns"] = ns.Name - _, err := k.CreateOrUpdateNamespace(ns.Name, ns.Labels) - if err != nil { + if _, err := k.CreateOrUpdateNamespace(ns.Name, ns.Labels); err != nil { return nil, fmt.Errorf("unable to create/update ns %s: %w", ns, err) } } @@ -1159,7 +1158,7 @@ func (k *KubernetesUtils) Bootstrap(namespaces map[string]TestNamespaceMeta, pod if hostNetworks != nil { hostNetwork = hostNetworks[key] } - for _, pod := range pods { + for _, pod := range podsPerNamespace { log.Infof("Creating/updating Pod '%s/%s'", ns, pod) deployment := ns.Name + pod _, err := k.CreateOrUpdateDeployment(ns.Name, deployment, 1, map[string]string{"pod": pod, "app": pod}, nodeName, hostNetwork) @@ -1169,8 +1168,8 @@ func (k *KubernetesUtils) Bootstrap(namespaces map[string]TestNamespaceMeta, pod } } var allPods []Pod - podIPs := make(map[string][]string, len(pods)*len(namespaces)) - for _, podName := range pods { + podIPs := make(map[string][]string, len(podsPerNamespace)*len(namespaces)) + for _, podName := range podsPerNamespace { for _, ns := range namespaces { allPods = append(allPods, NewPod(ns.Name, podName)) } diff --git a/test/e2e/nodenetworkpolicy_test.go b/test/e2e/nodenetworkpolicy_test.go index 592556d962c..5d88b53670f 100644 --- a/test/e2e/nodenetworkpolicy_test.go +++ b/test/e2e/nodenetworkpolicy_test.go @@ -37,7 +37,7 @@ func initializeAntreaNodeNetworkPolicy(t *testing.T, data *TestData, toHostNetwo p8082 = 8082 p8085 = 8085 pods = []string{"a"} - namespaces = initNamespaceMeta(formFactorNormal) + namespaces = initNamespaceMeta() nodes = make(map[string]string) nodes["x"] = controlPlaneNodeName() nodes["y"] = workerNodeName(1)