diff --git a/pkg/agent/controller/networkpolicy/reconciler.go b/pkg/agent/controller/networkpolicy/reconciler.go index 46b7116c548..8436d995511 100644 --- a/pkg/agent/controller/networkpolicy/reconciler.go +++ b/pkg/agent/controller/networkpolicy/reconciler.go @@ -748,7 +748,7 @@ func groupPodsByServices(services []v1beta1.Service, pods v1beta1.GroupMemberPod resolvedServices := make([]v1beta1.Service, len(services)) for podKey, pod := range pods { for i := range services { - resolvedServices[i] = *resolveService(&services[i], *pod.ToGroupMember()) + resolvedServices[i] = *resolveService(&services[i], pod.ToGroupMember()) } svcKey := normalizeServices(resolvedServices) if _, exists := podsByServicesMap[svcKey]; !exists { @@ -788,7 +788,7 @@ func groupMembersByServices(services []v1beta1.Service, memberSet v1beta1.GroupM resolvedServices := make([]v1beta1.Service, len(services)) for memberKey, member := range memberSet { for i := range services { - resolvedServices[i] = *resolveService(&services[i], *member) + resolvedServices[i] = *resolveService(&services[i], member) } svcKey := normalizeServices(resolvedServices) if _, exists := membersByServicesMap[svcKey]; !exists { @@ -886,7 +886,7 @@ func filterUnresolvablePort(in []v1beta1.Service) []v1beta1.Service { // resolveService resolves the port name of the provided service to a port number // for the provided groupMember. -func resolveService(service *v1beta1.Service, member v1beta1.GroupMember) *v1beta1.Service { +func resolveService(service *v1beta1.Service, member *v1beta1.GroupMember) *v1beta1.Service { // If port is not specified or is already a number, return it as is. if service.Port == nil || service.Port.Type == intstr.Int { return service diff --git a/pkg/apis/controlplane/helper.go b/pkg/apis/controlplane/helper.go new file mode 100644 index 00000000000..20ddb4d9c0f --- /dev/null +++ b/pkg/apis/controlplane/helper.go @@ -0,0 +1,33 @@ +// Copyright 2020 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controlplane + +// Conversion functions between GroupMember and GroupMemberPod +func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod { + return &GroupMemberPod{ + Pod: g.Pod, + IP: g.Endpoints[0].IP, + Ports: g.Endpoints[0].Ports, + } +} + +func (p *GroupMemberPod) ToGroupMember() *GroupMember { + return &GroupMember{ + Pod: p.Pod, + Endpoints: []Endpoint{ + {IP: p.IP, Ports: p.Ports}, + }, + } +} diff --git a/pkg/apis/controlplane/sets.go b/pkg/apis/controlplane/sets.go index 02f91bd67bc..9a2d341fffd 100644 --- a/pkg/apis/controlplane/sets.go +++ b/pkg/apis/controlplane/sets.go @@ -213,21 +213,3 @@ func (s GroupMemberSet) Items() []*GroupMember { } return res } - -// Conversion functions -func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod { - return &GroupMemberPod{ - Pod: g.Pod, - IP: g.Endpoints[0].IP, - Ports: g.Endpoints[0].Ports, - } -} - -func (p *GroupMemberPod) ToGroupMember() *GroupMember { - return &GroupMember{ - Pod: p.Pod, - Endpoints: []Endpoint{ - {IP: p.IP, Ports: p.Ports}, - }, - } -} diff --git a/pkg/apis/controlplane/v1beta1/helper.go b/pkg/apis/controlplane/v1beta1/helper.go new file mode 100644 index 00000000000..5a3d4c2dd2c --- /dev/null +++ b/pkg/apis/controlplane/v1beta1/helper.go @@ -0,0 +1,33 @@ +// Copyright 2020 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1beta1 + +// Conversion functions between GroupMember and GroupMemberPod +func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod { + return &GroupMemberPod{ + Pod: g.Pod, + IP: g.Endpoints[0].IP, + Ports: g.Endpoints[0].Ports, + } +} + +func (p *GroupMemberPod) ToGroupMember() *GroupMember { + return &GroupMember{ + Pod: p.Pod, + Endpoints: []Endpoint{ + {IP: p.IP, Ports: p.Ports}, + }, + } +} diff --git a/pkg/apis/controlplane/v1beta1/sets.go b/pkg/apis/controlplane/v1beta1/sets.go index 275c7466b24..41b716ba1e1 100644 --- a/pkg/apis/controlplane/v1beta1/sets.go +++ b/pkg/apis/controlplane/v1beta1/sets.go @@ -224,21 +224,3 @@ func (s GroupMemberSet) Items() []*GroupMember { } return res } - -// Conversion functions -func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod { - return &GroupMemberPod{ - Pod: g.Pod, - IP: g.Endpoints[0].IP, - Ports: g.Endpoints[0].Ports, - } -} - -func (p *GroupMemberPod) ToGroupMember() *GroupMember { - return &GroupMember{ - Pod: p.Pod, - Endpoints: []Endpoint{ - {IP: p.IP, Ports: p.Ports}, - }, - } -} diff --git a/pkg/controller/networkpolicy/antreanetworkpolicy_test.go b/pkg/controller/networkpolicy/antreanetworkpolicy_test.go index 8a49206efcd..34458a0aa6a 100644 --- a/pkg/controller/networkpolicy/antreanetworkpolicy_test.go +++ b/pkg/controller/networkpolicy/antreanetworkpolicy_test.go @@ -383,3 +383,40 @@ func getANP() *secv1alpha1.NetworkPolicy { } return npObj } + +func getEETestANP(selectorAppliedTo, selectorIn, selectorOut metav1.LabelSelector) *secv1alpha1.NetworkPolicy { + allowAction := secv1alpha1.RuleActionAllow + return &secv1alpha1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "anpA", + Namespace: "nsA", + }, + Spec: secv1alpha1.NetworkPolicySpec{ + AppliedTo: []secv1alpha1.NetworkPolicyPeer{ + { + PodSelector: &selectorAppliedTo, + }, + }, + Ingress: []secv1alpha1.Rule{ + { + From: []secv1alpha1.NetworkPolicyPeer{ + { + ExternalEntitySelector: &selectorIn, + }, + }, + Action: &allowAction, + }, + }, + Egress: []secv1alpha1.Rule{ + { + To: []secv1alpha1.NetworkPolicyPeer{ + { + ExternalEntitySelector: &selectorOut, + }, + }, + Action: &allowAction, + }, + }, + }, + } +} diff --git a/pkg/controller/networkpolicy/networkpolicy_controller.go b/pkg/controller/networkpolicy/networkpolicy_controller.go index 6cb5a943f9c..2ca40d10926 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller.go @@ -148,7 +148,6 @@ type NetworkPolicyController struct { // anpLister is able to list/get AntreaNetworkPolicies and is populated by the shared informer passed to // NewNetworkPolicyController. anpLister seclisters.NetworkPolicyLister - // anpListerSynced is a function which returns true if the AntreaNetworkPolicies shared informer has been synced at least once. anpListerSynced cache.InformerSynced @@ -915,10 +914,10 @@ func (n *NetworkPolicyController) updateExternalEntity(oldObj, curObj interface{ return } // Find groups matching the old ExternalEntity's labels. + oldAppliedToGroupKeySet := n.filterAppliedToGroupsForPodOrExternalEntity(oldEE) oldAddressGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(oldEE) - oldAppliedToGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(oldEE) // Find groups matching the new ExternalEntity's labels. - curAppliedToGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(curEE) + curAppliedToGroupKeySet := n.filterAppliedToGroupsForPodOrExternalEntity(curEE) curAddressGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(curEE) // Create set to hold the group keys to enqueue. var appliedToGroupKeys sets.String @@ -1351,9 +1350,9 @@ func podToMemberPod(pod *v1.Pod, includeIP, includePodRef bool) *controlplane.Gr func externalEntityToGroupMember(ee *v1alpha1.ExternalEntity) *controlplane.GroupMember { memberEntity := &controlplane.GroupMember{} for _, endpoint := range ee.Spec.Endpoints { - var networkingPorts []controlplane.NamedPort + var namedPorts []controlplane.NamedPort for _, port := range endpoint.Ports { - networkingPorts = append(networkingPorts, controlplane.NamedPort{ + namedPorts = append(namedPorts, controlplane.NamedPort{ Port: port.Port, Name: port.Name, Protocol: controlplane.Protocol(port.Protocol), @@ -1361,7 +1360,7 @@ func externalEntityToGroupMember(ee *v1alpha1.ExternalEntity) *controlplane.Grou } ep := controlplane.Endpoint{ IP: ipStrToIPAddress(endpoint.IP), - Ports: networkingPorts, + Ports: namedPorts, } memberEntity.Endpoints = append(memberEntity.Endpoints, ep) } @@ -1396,13 +1395,11 @@ func (n *NetworkPolicyController) processSelector(groupSelector antreatypes.Grou } } } else if groupSelector.NamespaceSelector != nil { - // All the Pods and EEs from Namespaces matching the nsSelector must be selected. + // All the Pods from Namespaces matching the nsSelector must be selected. namespaces, _ := n.namespaceLister.List(groupSelector.NamespaceSelector) for _, ns := range namespaces { nsPods, _ := n.podLister.Pods(ns.Name).List(labels.Everything()) pods = append(pods, nsPods...) - nsExtEntities, _ := n.externalEntityLister.ExternalEntities(ns.Name).List(labels.Everything()) - externalEntities = append(externalEntities, nsExtEntities...) } } else if groupSelector.PodSelector != nil { // Lack of Namespace and NamespaceSelector indicates Pods must be selected diff --git a/pkg/controller/networkpolicy/networkpolicy_controller_test.go b/pkg/controller/networkpolicy/networkpolicy_controller_test.go index 89b009f51fd..b3cee0aa46f 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller_test.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller_test.go @@ -38,7 +38,6 @@ import ( "github.com/vmware-tanzu/antrea/pkg/apis/controlplane" "github.com/vmware-tanzu/antrea/pkg/apis/core/v1alpha1" - secv1alpha1 "github.com/vmware-tanzu/antrea/pkg/apis/security/v1alpha1" "github.com/vmware-tanzu/antrea/pkg/apiserver/storage" fakeversioned "github.com/vmware-tanzu/antrea/pkg/client/clientset/versioned/fake" crdinformers "github.com/vmware-tanzu/antrea/pkg/client/informers/externalversions" @@ -1226,7 +1225,6 @@ func TestAddExternalEntity(t *testing.T) { selectorOut := metav1.LabelSelector{ MatchLabels: map[string]string{"outGroup": "outAddress"}, } - allowAction := secv1alpha1.RuleActionAllow appliedPod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "podA", @@ -1251,39 +1249,159 @@ func TestAddExternalEntity(t *testing.T) { PodIP: "1.2.3.4", }, } - testANPObj := &secv1alpha1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "anpA", - Namespace: "nsA", - }, - Spec: secv1alpha1.NetworkPolicySpec{ - AppliedTo: []secv1alpha1.NetworkPolicyPeer{ - { - PodSelector: &selectorSpec, + testANPObj := getEETestANP(selectorSpec, selectorIn, selectorOut) + tests := []struct { + name string + addedExternalEntity *v1alpha1.ExternalEntity + inAddressGroupMatch bool + outAddressGroupMatch bool + }{ + { + "no-match-ee", + &v1alpha1.ExternalEntity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eeA", + Namespace: "nsA", + Labels: map[string]string{"group": "none"}, }, }, - Ingress: []secv1alpha1.Rule{ - { - From: []secv1alpha1.NetworkPolicyPeer{ - { - ExternalEntitySelector: &selectorIn, - }, - }, - Action: &allowAction, + false, + false, + }, + { + "ee-match-ingress", + &v1alpha1.ExternalEntity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eeB", + Namespace: "nsA", + Labels: map[string]string{"inGroup": "inAddress"}, }, }, - Egress: []secv1alpha1.Rule{ - { - To: []secv1alpha1.NetworkPolicyPeer{ - { - ExternalEntitySelector: &selectorOut, - }, + true, + false, + }, + { + "ee-match-ingress-egress", + &v1alpha1.ExternalEntity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eeC", + Namespace: "nsA", + Labels: map[string]string{ + "inGroup": "inAddress", + "outGroup": "outAddress", }, - Action: &allowAction, }, }, + true, + true, }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, npc := newController() + npc.addANP(testANPObj) + npc.podStore.Add(appliedPod) + npc.externalEntityStore.Add(tt.addedExternalEntity) + appGroupID := getNormalizedUID(toGroupSelector("nsA", &selectorSpec, nil, nil).NormalizedName) + inGroupID := getNormalizedUID(toGroupSelector("nsA", nil, nil, &selectorIn).NormalizedName) + outGroupID := getNormalizedUID(toGroupSelector("nsA", nil, nil, &selectorOut).NormalizedName) + + npc.addPod(appliedPod) + npc.addExternalEntity(tt.addedExternalEntity) + atGroups, addrGroups := getQueuedGroups(npc) + assert.Equal(t, true, atGroups.Has(appGroupID)) + assert.Equal(t, tt.inAddressGroupMatch, addrGroups.Has(inGroupID)) + assert.Equal(t, tt.outAddressGroupMatch, addrGroups.Has(outGroupID)) + + npc.syncAppliedToGroup(appGroupID) + npc.syncAddressGroup(inGroupID) + npc.syncAddressGroup(outGroupID) + updatedInAddrGroupObj, _, _ := npc.addressGroupStore.Get(inGroupID) + updatedInAddrGroup := updatedInAddrGroupObj.(*antreatypes.AddressGroup) + updatedOutAddrGroupObj, _, _ := npc.addressGroupStore.Get(outGroupID) + updatedOutAddrGroup := updatedOutAddrGroupObj.(*antreatypes.AddressGroup) + member := externalEntityToGroupMember(tt.addedExternalEntity) + assert.Equal(t, tt.inAddressGroupMatch, updatedInAddrGroup.GroupMembers.Has(member)) + assert.Equal(t, tt.outAddressGroupMatch, updatedOutAddrGroup.GroupMembers.Has(member)) + }) + } +} + +func TestUpdateExternalEntity(t *testing.T) { + selectorSpec := metav1.LabelSelector{ + MatchLabels: map[string]string{"group": "appliedTo"}, + } + selectorIn := metav1.LabelSelector{ + MatchLabels: map[string]string{"inGroup": "inAddress"}, + } + selectorOut := metav1.LabelSelector{ + MatchLabels: map[string]string{"outGroup": "outAddress"}, + } + selectorOut2 := metav1.LabelSelector{ + MatchLabels: map[string]string{"outGroup": "outAddress2"}, + } + testANPObj := getEETestANP(selectorSpec, selectorIn, selectorOut) + testANPObj2 := getEETestANP(selectorSpec, selectorIn, selectorOut2) + ee1 := &v1alpha1.ExternalEntity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eeC", + Namespace: "nsA", + Labels: map[string]string{ + "outGroup": "outAddress", + }, + }, + } + ee2 := &v1alpha1.ExternalEntity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eeC", + Namespace: "nsA", + Labels: map[string]string{ + "outGroup": "outAddress2", + }, + }, + } + ee3 := &v1alpha1.ExternalEntity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eeC", + Namespace: "nsA", + Labels: map[string]string{ + "outGroup": "outAddress3", + }, + }, + } + outGroupID := getNormalizedUID(toGroupSelector("nsA", nil, nil, &selectorOut).NormalizedName) + outGroupID2 := getNormalizedUID(toGroupSelector("nsA", nil, nil, &selectorOut2).NormalizedName) + _, npc := newController() + npc.addANP(testANPObj) + npc.addANP(testANPObj2) + npc.updateExternalEntity(ee3, ee1) + _, addrGroups := getQueuedGroups(npc) + assert.Equal(t, true, addrGroups.Has(outGroupID)) + assert.Equal(t, false, addrGroups.Has(outGroupID2)) + // outGroupID and outGroupID2 should both be queued (EE removed and EE added) + npc.updateExternalEntity(ee1, ee2) + _, addrGroups = getQueuedGroups(npc) + assert.Equal(t, true, addrGroups.Has(outGroupID)) + assert.Equal(t, true, addrGroups.Has(outGroupID2)) + // only outGroupID2 should be queued (EE removed) + npc.updateExternalEntity(ee2, ee3) + _, addrGroups = getQueuedGroups(npc) + assert.Equal(t, false, addrGroups.Has(outGroupID)) + assert.Equal(t, true, addrGroups.Has(outGroupID2)) + +} + +func TestDeleteExternalEntity(t *testing.T) { + selectorSpec := metav1.LabelSelector{ + MatchLabels: map[string]string{"group": "appliedTo"}, + } + selectorIn := metav1.LabelSelector{ + MatchLabels: map[string]string{"inGroup": "inAddress"}, + } + selectorOut := metav1.LabelSelector{ + MatchLabels: map[string]string{"outGroup": "outAddress"}, + } + testANPObj := getEETestANP(selectorSpec, selectorIn, selectorOut) tests := []struct { name string addedExternalEntity *v1alpha1.ExternalEntity @@ -1334,12 +1452,15 @@ func TestAddExternalEntity(t *testing.T) { t.Run(tt.name, func(t *testing.T) { _, npc := newController() npc.addANP(testANPObj) - npc.podStore.Add(appliedPod) npc.externalEntityStore.Add(tt.addedExternalEntity) - appGroupID := getNormalizedUID(toGroupSelector("nsA", &selectorSpec, nil, nil).NormalizedName) + npc.addExternalEntity(tt.addedExternalEntity) inGroupID := getNormalizedUID(toGroupSelector("nsA", nil, nil, &selectorIn).NormalizedName) outGroupID := getNormalizedUID(toGroupSelector("nsA", nil, nil, &selectorOut).NormalizedName) - npc.syncAppliedToGroup(appGroupID) + npc.syncAddressGroup(inGroupID) + npc.syncAddressGroup(outGroupID) + + npc.externalEntityStore.Delete(tt.addedExternalEntity) + npc.deleteExternalEntity(tt.addedExternalEntity) npc.syncAddressGroup(inGroupID) npc.syncAddressGroup(outGroupID) updatedInAddrGroupObj, _, _ := npc.addressGroupStore.Get(inGroupID) @@ -1347,8 +1468,13 @@ func TestAddExternalEntity(t *testing.T) { updatedOutAddrGroupObj, _, _ := npc.addressGroupStore.Get(outGroupID) updatedOutAddrGroup := updatedOutAddrGroupObj.(*antreatypes.AddressGroup) member := externalEntityToGroupMember(tt.addedExternalEntity) - assert.Equal(t, tt.inAddressGroupMatch, updatedInAddrGroup.GroupMembers.Has(member)) - assert.Equal(t, tt.outAddressGroupMatch, updatedOutAddrGroup.GroupMembers.Has(member)) + + if tt.inAddressGroupMatch { + assert.Equal(t, false, updatedInAddrGroup.GroupMembers.Has(member)) + } + if tt.outAddressGroupMatch { + assert.Equal(t, false, updatedOutAddrGroup.GroupMembers.Has(member)) + } }) } } @@ -2725,6 +2851,22 @@ func TestDeleteFinalStateUnknownNetworkPolicy(t *testing.T) { assert.True(t, ok, "Missing event on channel") } +func getQueuedGroups(npc *networkPolicyController) (atGroups, addrGroups sets.String) { + atGroups, addrGroups = sets.NewString(), sets.NewString() + atLen, addrLen := npc.appliedToGroupQueue.Len(), npc.addressGroupQueue.Len() + for i := 0; i < atLen; i++ { + id, _ := npc.appliedToGroupQueue.Get() + atGroups.Insert(id.(string)) + npc.appliedToGroupQueue.Done(id) + } + for i := 0; i < addrLen; i++ { + id, _ := npc.addressGroupQueue.Get() + addrGroups.Insert(id.(string)) + npc.addressGroupQueue.Done(id) + } + return +} + func getK8sNetworkPolicyObj() *networkingv1.NetworkPolicy { ns := metav1.NamespaceDefault npName := "testing-1" diff --git a/pkg/controller/types/networkpolicy.go b/pkg/controller/types/networkpolicy.go index b740177117b..8ac68f38f8b 100644 --- a/pkg/controller/types/networkpolicy.go +++ b/pkg/controller/types/networkpolicy.go @@ -47,7 +47,7 @@ const ( // GroupSelector describes how to select Pods. type GroupSelector struct { - // The normalized name is calculated from Namespace, PodSelector, and NamespaceSelector. + // The normalized name is calculated from Namespace, PodSelector, ExternalEntitySelector and NamespaceSelector. // If multiple policies have same selectors, they should share this group by comparing NormalizedName. // It's also used to generate Name and UUID of group. NormalizedName string