From 7eb27f755c776fe409435ceed731363eb639ed7c Mon Sep 17 00:00:00 2001 From: Dyanngg Date: Mon, 9 Jan 2023 15:33:09 -0800 Subject: [PATCH] Add same-labels e2e testcase Signed-off-by: Dyanngg --- .../antrea-multicluster-leader-global.yml | 96 +++++++++++--- ...cluster.crd.antrea.io_resourceexports.yaml | 48 +++++-- ...cluster.crd.antrea.io_resourceimports.yaml | 48 +++++-- .../networkpolicy/clusternetworkpolicy.go | 2 +- pkg/controller/networkpolicy/validate.go | 3 + test/e2e/antreapolicy_test.go | 117 +++++++++++++++++- test/e2e/reachability.go | 20 +++ 7 files changed, 295 insertions(+), 39 deletions(-) diff --git a/multicluster/build/yamls/antrea-multicluster-leader-global.yml b/multicluster/build/yamls/antrea-multicluster-leader-global.yml index b361d645ec4..39a50966c66 100644 --- a/multicluster/build/yamls/antrea-multicluster-leader-global.yml +++ b/multicluster/build/yamls/antrea-multicluster-leader-global.yml @@ -935,9 +935,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1315,9 +1323,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1823,9 +1839,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -2203,9 +2227,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -3627,9 +3659,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -4007,9 +4047,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -4515,9 +4563,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -4895,9 +4951,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the diff --git a/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceexports.yaml b/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceexports.yaml index fc5d680b9a5..2cad5eedb2a 100644 --- a/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceexports.yaml +++ b/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceexports.yaml @@ -630,9 +630,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1010,9 +1018,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1518,9 +1534,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1898,9 +1922,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the diff --git a/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceimports.yaml b/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceimports.yaml index 1e0dc7a17c2..1f74cdb1802 100644 --- a/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceimports.yaml +++ b/multicluster/config/crd/bases/multicluster.crd.antrea.io_resourceimports.yaml @@ -628,9 +628,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1008,9 +1016,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1516,9 +1532,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the @@ -1896,9 +1920,17 @@ spec: ingress/egress rules. Cannot be set with NamespaceSelector.' properties: match: - description: NamespaceMatchType describes Namespace - matching strategy. + description: Selects from the same Namespace of + the appliedTo workloads. type: string + sameLabels: + description: Selects Namespaces that share the + same values for the given set of label keys + with the appliedTo Namespace. Namespaces must + have all the label keys. + items: + type: string + type: array type: object nodeSelector: description: Select certain Nodes which match the diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy.go b/pkg/controller/networkpolicy/clusternetworkpolicy.go index cfb56766358..748751d82fe 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy.go +++ b/pkg/controller/networkpolicy/clusternetworkpolicy.go @@ -117,7 +117,7 @@ func (n *NetworkPolicyController) filterPerNamespaceRuleACNPsByNSLabels(nsLabels peerNamespacesSelectorExists := func(peers []crdv1alpha1.NetworkPolicyPeer) bool { for _, peer := range peers { - if peer.Namespaces != nil && peer.Namespaces.Match == crdv1alpha1.NamespaceMatchSelf { + if peer.Namespaces != nil { return true } } diff --git a/pkg/controller/networkpolicy/validate.go b/pkg/controller/networkpolicy/validate.go index a76a4188de7..516ede384f3 100644 --- a/pkg/controller/networkpolicy/validate.go +++ b/pkg/controller/networkpolicy/validate.go @@ -561,6 +561,9 @@ func (v *antreaPolicyValidator) validatePeers(ingress, egress []crdv1alpha1.Rule if peer.NamespaceSelector != nil && peer.Namespaces != nil { return "namespaces and namespaceSelector cannot be set at the same time for a single NetworkPolicyPeer", false } + if peer.Namespaces != nil && numFieldsSetInStruct(*peer.Namespaces) > 1 { + return "only one matching criteria can be specified in a single peer namespaces field", false + } peerFieldsNum := numFieldsSetInStruct(peer) if peer.Group != "" && peerFieldsNum > 1 { return "group cannot be set with other peers in rules", false diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go index 4e7e68a6305..2efcd59a1cc 100644 --- a/test/e2e/antreapolicy_test.go +++ b/test/e2e/antreapolicy_test.go @@ -138,10 +138,10 @@ func initNamespaceMeta(formFactor string) map[string]TestNamespaceMeta { } allNamespaceMeta["dev"+strconv.Itoa(i)] = devNS } - allNamespaceMeta["no-tier-label"] = TestNamespaceMeta{ - Name: "no-tier-label-" + suffix, + allNamespaceMeta["no-tier"] = TestNamespaceMeta{ + Name: "no-tier-" + suffix, Labels: map[string]string{ - "purpose": "test", + "purpose": "test-exclusion", }, } } else if formFactor == formFactorNormal { @@ -2640,7 +2640,7 @@ func testAuditLoggingBasic(t *testing.T, data *TestData) { return false, nil } - destinations := []string{getNS("z") + "/a", getNS("z") + "/b", getNS("z") + "/c"} + destinations := []string{getPodName("z", "a"), getPodName("z", "b"), getPodName("z", "c")} srcIPs, _ := podIPs[getPodName("x", "a")] var expectedNumEntries, actualNumEntries int for _, d := range destinations { @@ -2732,7 +2732,7 @@ func testAuditLoggingEnableNP(t *testing.T, data *TestData) { } var expectedNumEntries, actualNumEntries int - srcPods := []string{getNS("x") + "/b", getNS("x") + "/c"} + srcPods := []string{getPodName("x", "b"), getPodName("x", "c")} expectedLogPrefix := []string{npRef + " Allow [0-9]+ ", "K8sNetworkPolicy Drop "} destIPs, _ := podIPs[getPodName("x", "a")] for i := 0; i < len(srcPods); i++ { @@ -3185,7 +3185,93 @@ func testACNPStrictNamespacesIsolation(t *testing.T) { } testCase := []*TestCase{ - {"ACNP strict Namespace isolation for all namespaces", []*TestStep{testStep1, testStep2}}, + {"ACNP strict Namespace isolation for all Namespaces", []*TestStep{testStep1, testStep2}}, + } + executeTests(t, testCase) +} + +func testACNPStrictNamespacesIsolationByLabels(t *testing.T) { + samePurposeTierLabels := &crdv1alpha1.PeerNamespaces{ + SameLabels: []string{"purpose", "tier"}, + } + builder := &ClusterNetworkPolicySpecBuilder{} + builder = builder.SetName("test-acnp-strict-ns-isolation-by-labels"). + SetTier("securityops"). + SetPriority(1.0). + SetAppliedToGroup([]ACNPAppliedToSpec{{NSSelector: map[string]string{}}}) + builder.AddIngress(ProtocolTCP, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, + samePurposeTierLabels, nil, crdv1alpha1.RuleActionPass, "", "", nil) + builder.AddIngress(ProtocolTCP, nil, nil, nil, nil, nil, nil, nil, nil, nil, map[string]string{}, nil, nil, + nil, nil, crdv1alpha1.RuleActionDrop, "", "", nil) + // prod1 and prod2 Namespaces should be able to connect to each other. The same goes for dev1 and + // dev2 Namespaces. However, any prod Namespace should not be able to connect to any dev Namespace + // due to different "tier" label values. For the "no-tier" Namespace, the first ingress rule will + // have no effect because the Namespace does not have a "tier" label. So every Pod in that Namespace + // will be isolated according to the second rule of the ACNP. + reachability := NewReachability(allPods, Dropped) + reachability.ExpectNamespaceIngressFromNamespace(getNS("prod1"), getNS("prod2"), Connected) + reachability.ExpectNamespaceEgressToNamespace(getNS("prod1"), getNS("prod2"), Connected) + reachability.ExpectNamespaceIngressFromNamespace(getNS("prod2"), getNS("prod1"), Connected) + reachability.ExpectNamespaceEgressToNamespace(getNS("prod2"), getNS("prod1"), Connected) + reachability.ExpectNamespaceIngressFromNamespace(getNS("dev1"), getNS("dev2"), Connected) + reachability.ExpectNamespaceEgressToNamespace(getNS("dev1"), getNS("dev2"), Connected) + reachability.ExpectNamespaceIngressFromNamespace(getNS("dev2"), getNS("dev1"), Connected) + reachability.ExpectNamespaceEgressToNamespace(getNS("dev2"), getNS("dev1"), Connected) + reachability.ExpectAllSelfNamespace(Connected) + reachability.ExpectSelfNamespace(getNS("no-tier"), Dropped) + reachability.ExpectSelf(allPods, Connected) + + testStep1 := &TestStep{ + "Namespace isolation by label, Port 80", + reachability, + []metav1.Object{builder.Get()}, + []int32{80}, + ProtocolTCP, + 0, + nil, + } + testCase := []*TestCase{ + {"ACNP strict Namespace isolation by Namespace purpose and tier labels", []*TestStep{testStep1}}, + } + executeTests(t, testCase) +} + +func testACNPStrictNamespacesIsolationBySingleLabel(t *testing.T) { + samePurposeTierLabels := &crdv1alpha1.PeerNamespaces{ + SameLabels: []string{"purpose"}, + } + builder := &ClusterNetworkPolicySpecBuilder{} + builder = builder.SetName("test-acnp-strict-ns-isolation-by-single-label"). + SetTier("securityops"). + SetPriority(1.0). + SetAppliedToGroup([]ACNPAppliedToSpec{{NSSelector: map[string]string{}}}) + builder.AddIngress(ProtocolTCP, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, + samePurposeTierLabels, nil, crdv1alpha1.RuleActionPass, "", "", nil) + builder.AddIngress(ProtocolTCP, nil, nil, nil, nil, nil, nil, nil, nil, nil, map[string]string{}, nil, nil, + nil, nil, crdv1alpha1.RuleActionDrop, "", "", nil) + // Namespaces are split into two logical groups, purpose=test (prod1,2 and dev1,2) and purpose=test-exclusion + // (no-tier). The two groups of Namespace should not be able to connect to each other. + reachability := NewReachability(allPods, Connected) + reachability.ExpectNamespaceEgressToNamespace(getNS("prod1"), getNS("no-tier"), Dropped) + reachability.ExpectNamespaceEgressToNamespace(getNS("prod2"), getNS("no-tier"), Dropped) + reachability.ExpectNamespaceEgressToNamespace(getNS("dev1"), getNS("no-tier"), Dropped) + reachability.ExpectNamespaceEgressToNamespace(getNS("dev2"), getNS("no-tier"), Dropped) + reachability.ExpectNamespaceIngressFromNamespace(getNS("prod1"), getNS("no-tier"), Dropped) + reachability.ExpectNamespaceIngressFromNamespace(getNS("prod2"), getNS("no-tier"), Dropped) + reachability.ExpectNamespaceIngressFromNamespace(getNS("dev1"), getNS("no-tier"), Dropped) + reachability.ExpectNamespaceIngressFromNamespace(getNS("dev2"), getNS("no-tier"), Dropped) + + testStep1 := &TestStep{ + "Namespace isolation by single label, Port 80", + reachability, + []metav1.Object{builder.Get()}, + []int32{80}, + ProtocolTCP, + 0, + nil, + } + testCase := []*TestCase{ + {"ACNP strict Namespace isolation by Namespace purpose label", []*TestStep{testStep1}}, } executeTests(t, testCase) } @@ -4343,6 +4429,25 @@ func TestAntreaPolicy(t *testing.T) { k8sUtils.Cleanup(namespaces) } +func TestAntreaPolicyExtendedNamespaces(t *testing.T) { + skipIfHasWindowsNodes(t) + skipIfAntreaPolicyDisabled(t) + + data, err := setupTest(t) + if err != nil { + t.Fatalf("Error when setting up test: %v", err) + } + defer teardownTest(t, data) + + initialize(t, data, formFactorLarge) + + t.Run("TestGroupACNPNamespaceLabelSelections", func(t *testing.T) { + t.Run("Case=ACNPStrictNamespacesIsolationByLabels", func(t *testing.T) { testACNPStrictNamespacesIsolationByLabels(t) }) + t.Run("Case=ACNPStrictNamespacesIsolationBySingleLabel", func(t *testing.T) { testACNPStrictNamespacesIsolationBySingleLabel(t) }) + }) + k8sUtils.Cleanup(namespaces) +} + func TestAntreaPolicyStatus(t *testing.T) { skipIfHasWindowsNodes(t) skipIfAntreaPolicyDisabled(t) diff --git a/test/e2e/reachability.go b/test/e2e/reachability.go index 4a781307030..8acd9b60c89 100644 --- a/test/e2e/reachability.go +++ b/test/e2e/reachability.go @@ -312,6 +312,26 @@ func (r *Reachability) ExpectEgressToNamespace(pod Pod, namespace string, connec } } +func (r *Reachability) ExpectNamespaceIngressFromNamespace(dstNamespace, srcNamespace string, connectivity PodConnectivityMark) { + dstPods, ok := r.PodsByNamespace[dstNamespace] + if !ok { + panic(fmt.Errorf("destination Namespace %s is not found", dstNamespace)) + } + for _, p := range dstPods { + r.ExpectIngressFromNamespace(p, srcNamespace, connectivity) + } +} + +func (r *Reachability) ExpectNamespaceEgressToNamespace(srcNamespace, dstNamespace string, connectivity PodConnectivityMark) { + srcPods, ok := r.PodsByNamespace[srcNamespace] + if !ok { + panic(fmt.Errorf("src Namespace %s is not found", srcNamespace)) + } + for _, p := range srcPods { + r.ExpectEgressToNamespace(p, dstNamespace, connectivity) + } +} + func (r *Reachability) Observe(pod1 Pod, pod2 Pod, connectivity PodConnectivityMark) { r.Observed.Set(string(pod1), string(pod2), connectivity) }