From e7c849b0fa39580a8ec14e31a9a7af6b542bdcd3 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 16 May 2022 17:58:24 -0700 Subject: [PATCH 1/7] fix windows intersection --- npm/pkg/dataplane/dataplane.go | 7 +-- npm/pkg/dataplane/dataplane_windows.go | 30 +++++---- npm/pkg/dataplane/ipsets/ipset.go | 51 +++++++++++---- npm/pkg/dataplane/ipsets/ipsetmanager.go | 3 +- .../dataplane/ipsets/ipsetmanager_windows.go | 63 ++++++++++--------- npm/pkg/dataplane/policies/policymanager.go | 1 + .../policies/policymanager_windows.go | 2 +- npm/util/const.go | 4 +- 8 files changed, 100 insertions(+), 61 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 9bad183f3d..5ef5e4000f 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -13,12 +13,7 @@ import ( "k8s.io/klog" ) -const ( - // AzureNetworkName is default network Azure CNI creates - AzureNetworkName = "azure" - - reconcileTimeInMinutes = 5 -) +const reconcileTimeInMinutes = 5 type PolicyMode string diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index b8df5c400c..4d2a3f2f75 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -1,11 +1,13 @@ package dataplane import ( + "errors" "fmt" "strings" "time" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" + "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "github.com/Microsoft/hcsshim/hcn" "k8s.io/klog" @@ -14,21 +16,23 @@ import ( const ( maxNoNetRetryCount int = 240 // max wait time 240*5 == 20 mins maxNoNetSleepTime int = 5 // in seconds + unspecifiedPodKey = "" ) -func (dp *DataPlane) setPolicyMode() { - dp.PolicyMode = policies.IPSetPolicyMode - err := hcn.SetPolicySupported() - if err != nil { - dp.PolicyMode = policies.IPPolicyMode - } -} +var errPolicyModeUnsupported = errors.New("only IPSet policy mode is supported") // initializeDataPlane will help gather network and endpoint details func (dp *DataPlane) initializeDataPlane() error { klog.Infof("[DataPlane] Initializing dataplane for windows") + if dp.PolicyMode == "" { - dp.setPolicyMode() + dp.PolicyMode = policies.IPSetPolicyMode + } + if dp.PolicyMode != policies.IPSetPolicyMode { + return errPolicyModeUnsupported + } + if err := hcn.SetPolicySupported(); err != nil { + return npmerrors.SimpleErrorWrapper("[DataPlane] kernel does not support SetPolicies", err) } err := dp.getNetworkInfo() @@ -51,7 +55,7 @@ func (dp *DataPlane) getNetworkInfo() error { var err error for ; true; <-ticker.C { - err = dp.setNetworkIDByName(AzureNetworkName) + err = dp.setNetworkIDByName(util.AzureNetworkName) if err == nil || !isNetworkNotFoundErr(err) { return err } @@ -60,7 +64,7 @@ func (dp *DataPlane) getNetworkInfo() error { break } klog.Infof("[DataPlane Windows] Network with name %s not found. Retrying in %d seconds, Current retry number %d, max retries: %d", - AzureNetworkName, + util.AzureNetworkName, maxNoNetSleepTime, retryNumber, maxNoNetRetryCount, @@ -169,7 +173,7 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { continue } // TODO Also check if the endpoint reference in policy for this Ip is right - netpolSelectorIPs, err := dp.getSelectorIPsByPolicyName(policyKey) + netpolSelectorIPs, err := dp.getSelectorIPsByPolicyKey(policyKey) if err != nil { return err } @@ -198,7 +202,7 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { return nil } -func (dp *DataPlane) getSelectorIPsByPolicyName(policyKey string) (map[string]struct{}, error) { +func (dp *DataPlane) getSelectorIPsByPolicyKey(policyKey string) (map[string]struct{}, error) { policy, ok := dp.policyMgr.GetPolicy(policyKey) if !ok { return nil, fmt.Errorf("policy with name %s does not exist", policyKey) @@ -328,5 +332,5 @@ func (dp *DataPlane) getAllEndpointIDs() []string { } func isNetworkNotFoundErr(err error) bool { - return strings.Contains(err.Error(), fmt.Sprintf("Network name \"%s\" not found", AzureNetworkName)) + return strings.Contains(err.Error(), fmt.Sprintf("Network name \"%s\" not found", util.AzureNetworkName)) } diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index b87b21d689..a8c838b821 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -372,21 +372,50 @@ func (set *IPSet) hasMember(memberName string) bool { return isMember } -func (set *IPSet) getSetIntersection(existingIntersection map[string]struct{}) (map[string]struct{}, error) { - if !set.canSetBeSelectorIPSet() { - return nil, npmerrors.Errorf( - npmerrors.IPSetIntersection, - false, - fmt.Sprintf("[IPSet] Selector IPSet cannot be of type %s", set.Type.String())) +// affiliatedIPs returns the IPs for a hash set or the IPs of the member sets for a list set +// This method, intersectAffiliatedIPs, and GetSetContents are good examples of how the +// ipset struct may have been better designed as an interface with hash and list implementations. +// Not worth it to redesign though. +func (set *IPSet) affiliatedIPs() map[string]struct{} { + if set.Kind == HashSet { + result := make(map[string]struct{}, len(set.IPPodKey)) + for ip := range set.IPPodKey { + result[ip] = struct{}{} + } + return result } - newIntersectionMap := make(map[string]struct{}) - for ip := range set.IPPodKey { - if _, ok := existingIntersection[ip]; ok { - newIntersectionMap[ip] = struct{}{} + + // number of member ipsets usually underestimates the final map size + result := make(map[string]struct{}, len(set.MemberIPSets)) + for _, memberSet := range set.MemberIPSets { + for ip := range memberSet.IPPodKey { + result[ip] = struct{}{} } } + return result +} + +// intersectAffiliatedIPs removes IPs that aren't affiliated with this set +func (set *IPSet) intersectAffiliatedIPs(existingIPs map[string]struct{}) { + for ip := range existingIPs { + isAffiliated := false + if set.Kind == HashSet { + _, ok := set.IPPodKey[ip] + isAffiliated = ok + } else { + for _, memberSet := range set.MemberIPSets { + _, ok := memberSet.IPPodKey[ip] + if ok { + isAffiliated = true + break + } + } + } - return newIntersectionMap, nil + if !isAffiliated { + delete(existingIPs, ip) + } + } } func (set *IPSet) canSetBeSelectorIPSet() bool { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 800f2fe3fc..a6a3f473ba 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -42,7 +42,8 @@ type IPSetManager struct { } type IPSetManagerCfg struct { - IPSetMode IPSetMode + IPSetMode IPSetMode + // NetworkName can be left empty or set to 'azure' (the only supported network) NetworkName string } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 06954b223d..6435c5b59d 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -2,11 +2,12 @@ package ipsets import ( "encoding/json" + "errors" "fmt" "strings" "github.com/Azure/azure-container-networking/npm/util" - "github.com/Azure/azure-container-networking/npm/util/errors" + npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "github.com/Microsoft/hcsshim/hcn" "k8s.io/klog" ) @@ -19,6 +20,8 @@ const ( donotResetIPSets = false ) +var errUnsupportedNetwork = errors.New("only 'azure' network is supported") + type networkPolicyBuilder struct { toAddSets map[string]*hcn.SetPolicySetting toUpdateSets map[string]*hcn.SetPolicySetting @@ -33,40 +36,42 @@ func (iMgr *IPSetManager) GetIPsFromSelectorIPSets(setList map[string]struct{}) iMgr.Lock() defer iMgr.Unlock() - setintersections := make(map[string]struct{}) - var err error + ips := make(map[string]struct{}) firstLoop := true for setName := range setList { if !iMgr.exists(setName) { - return nil, errors.Errorf( - errors.GetSelectorReference, + return nil, npmerrors.Errorf( + npmerrors.GetSelectorReference, false, fmt.Sprintf("[ipset manager] selector ipset %s does not exist", setName)) } + set := iMgr.setMap[setName] + if !set.canSetBeSelectorIPSet() { + return nil, npmerrors.Errorf( + npmerrors.IPSetIntersection, + false, + fmt.Sprintf("[IPSet] Selector IPSet cannot be of type %s", set.Type.String())) + } + if firstLoop { - intialSetIPs := set.IPPodKey - for k := range intialSetIPs { - setintersections[k] = struct{}{} - } + ips = set.affiliatedIPs() firstLoop = false - } - klog.Infof("set [%s] has ippodkey: %+v", set.Name, set.IPPodKey) // FIXME remove after debugging - setintersections, err = set.getSetIntersection(setintersections) - if err != nil { - return nil, err + } else { + set.intersectAffiliatedIPs() } } - klog.Infof("setintersection for getIPsFromSelectorIPSets %+v", setintersections) // FIXME remove after debugging - return setintersections, err + + klog.Infof("IPs in selector IPSets: %+v", ips) // FIXME remove after debugging + return ips, nil } func (iMgr *IPSetManager) GetSelectorReferencesBySet(setName string) (map[string]struct{}, error) { iMgr.Lock() defer iMgr.Unlock() if !iMgr.exists(setName) { - return nil, errors.Errorf( - errors.GetSelectorReference, + return nil, npmerrors.Errorf( + npmerrors.GetSelectorReference, false, fmt.Sprintf("[ipset manager] selector ipset %s does not exist", setName)) } @@ -81,7 +86,6 @@ func (iMgr *IPSetManager) resetIPSets() error { return err } - // TODO delete 2nd level sets first and then 1st level sets _, toDeleteSets := iMgr.segregateSetPolicies(network.Policies, resetIPSetsTrue) if len(toDeleteSets) == 0 { @@ -143,7 +147,7 @@ func (iMgr *IPSetManager) applyIPSets() error { // calculateNewSetPolicies will take in existing setPolicies on network in HNS and the dirty cache, will return back // networkPolicyBuild which contains the new setPolicies to be added, updated and deleted -// TODO: This function is not thread safe. +// Assumes that the dirty cache is locked (or equivalently, the ipsetmanager itself). // toAddSets: // this function will loop through the dirty cache and adds non-existing sets to toAddSets // toUpdateSets: @@ -210,9 +214,12 @@ func (iMgr *IPSetManager) calculateNewSetPolicies(networkPolicies []hcn.NetworkP func (iMgr *IPSetManager) getHCnNetwork() (*hcn.HostComputeNetwork, error) { if iMgr.iMgrCfg.NetworkName == "" { - iMgr.iMgrCfg.NetworkName = "azure" + iMgr.iMgrCfg.NetworkName = util.AzureNetworkName + } + if iMgr.iMgrCfg.NetworkName != util.AzureNetworkName { + return nil, errUnsupportedNetwork } - network, err := iMgr.ioShim.Hns.GetNetworkByName("azure") + network, err := iMgr.ioShim.Hns.GetNetworkByName(iMgr.iMgrCfg.NetworkName) if err != nil { return nil, err } @@ -243,8 +250,7 @@ func (iMgr *IPSetManager) modifySetPolicies(network *hcn.HostComputeNetwork, ope } if policyRequest == nil { - klog.Infof("[IPSetManager Windows] No Policies to apply") - return nil + continue } requestMessage := &hcn.ModifyNetworkSettingRequest{ @@ -300,10 +306,6 @@ func (setPolicyBuilder *networkPolicyBuilder) setNameExists(setName string) bool } func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicySetting, policyType hcn.SetPolicyType) ([]byte, error) { - if len(setPolicySettings) == 0 { - klog.Info("[Dataplane Windows] no set policies to apply on network") - return nil, nil - } klog.Infof("[Dataplane Windows] marshalling %s type of sets", policyType) policyNetworkRequest := &hcn.PolicyNetworkRequest{ Policies: make([]hcn.NetworkPolicy, 0), @@ -327,6 +329,11 @@ func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicyS ) } + if len(policyNetworkRequest.Policies) == 0 { + klog.Info("[Dataplane Windows] no %s type of sets to apply", policyType) + return nil, nil + } + policyReqSettings, err := json.Marshal(policyNetworkRequest) if err != nil { return nil, err diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 8c578dce59..92b7484d37 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -19,6 +19,7 @@ const ( // IPSetPolicyMode will references IPSets in policies IPSetPolicyMode PolicyManagerMode = "IPSet" // IPPolicyMode will replace ipset names with their value IPs in policies + // NOTE: this is currently unimplemented IPPolicyMode PolicyManagerMode = "IP" // this number is based on the implementation in chain-management_linux.go diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index c2c77d69ea..2d40529793 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -43,7 +43,7 @@ func (pMgr *PolicyManager) bootup(epIDs []string) error { } func (pMgr *PolicyManager) reconcile() { - // TODO + // not implemented } func (pMgr *PolicyManager) addPolicy(policy *NPMNetworkPolicy, endpointList map[string]string) error { diff --git a/npm/util/const.go b/npm/util/const.go index 2f5c58de72..126adc6700 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -104,7 +104,6 @@ const ( IptablesAzureEgressPolicyChainPrefix string = "AZURE-NPM-EGRESS" // Below chain exists only in NPM before v1.2.6 - // TODO delete this below set while cleaning up IptablesAzureTargetSetsChain string = "AZURE-NPM-TARGET-SETS" // Below chain existing only in NPM before v1.2.7 IptablesAzureIngressWrongDropsChain string = "AZURE-NPM-INRGESS-DROPS" @@ -224,6 +223,9 @@ const ( ErrorValue float64 = 1 ) +// AzureNetworkName is the default network Azure CNI creates +const AzureNetworkName = "azure" + // These ID represents where did the error log generate from. // It's for better query purpose. In Kusto these value are used in // OperationID column From 8683dba6c9ec2d177248f400f232803940a6d0ca Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Tue, 17 May 2022 10:21:24 -0700 Subject: [PATCH 2/7] test intersection --- npm/pkg/dataplane/ipsets/ipset_test.go | 48 ++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/npm/pkg/dataplane/ipsets/ipset_test.go b/npm/pkg/dataplane/ipsets/ipset_test.go index 2a30e1bb7f..4e1cabe781 100644 --- a/npm/pkg/dataplane/ipsets/ipset_test.go +++ b/npm/pkg/dataplane/ipsets/ipset_test.go @@ -6,6 +6,54 @@ import ( "github.com/stretchr/testify/require" ) +func TestAffiliatedIPs(t *testing.T) { + s1 := NewIPSet(NewIPSetMetadata("test-set1", Namespace)) + s1.IPPodKey["1.1.1.1"] = "pod-a" + s1.IPPodKey["2.2.2.2"] = "pod-b" + s2 := NewIPSet(NewIPSetMetadata("test-set2", Namespace)) + s2.IPPodKey["3.3.3.3"] = "pod-c" + s2.IPPodKey["4.4.4.4"] = "pod-d" + + // 1 IP from each set above + s3 := NewIPSet(NewIPSetMetadata("test-set3", Namespace)) + s3.IPPodKey["1.1.1.1"] = "pod-a" + s3.IPPodKey["4.4.4.4"] = "pod-d" + + l := NewIPSet(NewIPSetMetadata("test-list", KeyLabelOfNamespace)) + l.MemberIPSets[s1.Name] = s1 + l.MemberIPSets[s2.Name] = s2 + + expected := map[string]struct{}{ + "1.1.1.1": {}, + "2.2.2.2": {}, + } + require.Equal(t, expected, s1.affiliatedIPs(), "unexpected affiliated IPs for set 1") + + expected = map[string]struct{}{ + "1.1.1.1": {}, + "2.2.2.2": {}, + "3.3.3.3": {}, + "4.4.4.4": {}, + } + require.Equal(t, expected, l.affiliatedIPs(), "unexpected affiliated IPs for list") + + intersection := s3.affiliatedIPs() + l.intersectAffiliatedIPs(intersection) + expected = map[string]struct{}{ + "1.1.1.1": {}, + "4.4.4.4": {}, + } + require.Equal(t, expected, intersection, "unexpected intersection (direction #1)") + + intersection = l.affiliatedIPs() + s3.intersectAffiliatedIPs(intersection) + expected = map[string]struct{}{ + "1.1.1.1": {}, + "4.4.4.4": {}, + } + require.Equal(t, expected, intersection, "unexpected intersection (direction #2)") +} + func TestShouldBeInKernelAndCanDelete(t *testing.T) { s := &IPSetMetadata{"test-set", Namespace} l := &IPSetMetadata{"test-list", KeyLabelOfNamespace} From 576b86b241efa0eeaba157dca5868fa004d752eb Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Tue, 17 May 2022 13:09:32 -0700 Subject: [PATCH 3/7] fix build --- npm/pkg/dataplane/ipsets/ipsetmanager_windows.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 6435c5b59d..53dec5b7c9 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -58,7 +58,7 @@ func (iMgr *IPSetManager) GetIPsFromSelectorIPSets(setList map[string]struct{}) ips = set.affiliatedIPs() firstLoop = false } else { - set.intersectAffiliatedIPs() + set.intersectAffiliatedIPs(ips) } } @@ -177,7 +177,7 @@ func (iMgr *IPSetManager) calculateNewSetPolicies(networkPolicies []hcn.NetworkP for setName := range toAddUpdateSetNames { set, exists := iMgr.setMap[setName] // check if the Set exists if !exists { - return nil, errors.Errorf(errors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) + return nil, npmerrors.Errorf(npmerrors.AppendIPSet, false, fmt.Sprintf("ipset %s does not exist", setName)) } setPol, err := convertToSetPolicy(set) From 0dd8c55b2c11b60e181cc1575fd22b1ba64eab5f Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 19 May 2022 13:22:23 -0700 Subject: [PATCH 4/7] separate out children of pod selector and add translate UTs --- .../translation/translatePolicy.go | 49 +- .../translation/translatePolicy_test.go | 532 ++++++++++++++++-- npm/pkg/dataplane/dataplane.go | 4 +- npm/pkg/dataplane/ipsets/ipset_test.go | 12 +- .../dataplane/ipsets/ipsetmanager_windows.go | 2 +- npm/pkg/dataplane/policies/policy.go | 13 +- 6 files changed, 543 insertions(+), 69 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 49c2707cfe..7043530e9e 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -214,42 +214,46 @@ func ipBlockRule(policyName, ns string, direction policies.Direction, matchType return ipBlockIPSet, setInfo } -// PodSelector translates podSelector of NetworkPolicyPeer field in networkpolicy object to translatedIPSet and SetInfo. +// PodSelector translates podSelector of NetworkPolicyPeer field in networkpolicy object to translatedIPSets, children of translated IPSets, and SetInfo. +// Children are members of a list-type IPSet. // This function is called only when the NetworkPolicyPeer has namespaceSelector field. -func podSelector(policyKey string, matchType policies.MatchType, selector *metav1.LabelSelector) ([]*ipsets.TranslatedIPSet, []policies.SetInfo, error) { +func podSelector(policyKey string, matchType policies.MatchType, selector *metav1.LabelSelector) (psSets, childPSSets []*ipsets.TranslatedIPSet, psList []policies.SetInfo, err error) { podSelectors, err := parsePodSelector(policyKey, selector) if err != nil { - return nil, nil, err + return } lenOfPodSelectors := len(podSelectors) - podSelectorIPSets := []*ipsets.TranslatedIPSet{} - podSelectorList := make([]policies.SetInfo, lenOfPodSelectors) + psSets = make([]*ipsets.TranslatedIPSet, 0) + childPSSets = make([]*ipsets.TranslatedIPSet, 0) + psList = make([]policies.SetInfo, lenOfPodSelectors) for i := 0; i < lenOfPodSelectors; i++ { ps := podSelectors[i] - podSelectorIPSets = append(podSelectorIPSets, ipsets.NewTranslatedIPSet(ps.setName, ps.setType, ps.members...)) + psSets = append(psSets, ipsets.NewTranslatedIPSet(ps.setName, ps.setType, ps.members...)) + // if value is nested value, create translatedIPSet with the nested value for j := 0; j < len(ps.members); j++ { - podSelectorIPSets = append(podSelectorIPSets, ipsets.NewTranslatedIPSet(ps.members[j], ipsets.KeyValueLabelOfPod)) + childPSSets = append(childPSSets, ipsets.NewTranslatedIPSet(ps.members[j], ipsets.KeyValueLabelOfPod)) } - podSelectorList[i] = policies.NewSetInfo(ps.setName, ps.setType, ps.include, matchType) + psList[i] = policies.NewSetInfo(ps.setName, ps.setType, ps.include, matchType) } - return podSelectorIPSets, podSelectorList, nil + return } -// podSelectorWithNS translates podSelector of spec and NetworkPolicyPeer in networkpolicy object to translatedIPSet and SetInfo. +// podSelectorWithNS translates podSelector of spec and NetworkPolicyPeer in networkpolicy object to translatedIPSets, children of translated IPSets, and SetInfo. +// Children are members of a list-type IPSet. // This function is called only when the NetworkPolicyPeer does not have namespaceSelector field. -func podSelectorWithNS(policyKey, ns string, matchType policies.MatchType, selector *metav1.LabelSelector) ([]*ipsets.TranslatedIPSet, []policies.SetInfo, error) { - podSelectorIPSets, podSelectorList, err := podSelector(policyKey, matchType, selector) +func podSelectorWithNS(policyKey, ns string, matchType policies.MatchType, selector *metav1.LabelSelector) (psSets, childPSSets []*ipsets.TranslatedIPSet, psList []policies.SetInfo, err error) { + psSets, childPSSets, psList, err = podSelector(policyKey, matchType, selector) if err != nil { - return nil, nil, err + return } // Add translatedIPSet and SetInfo based on namespace - podSelectorIPSets = append(podSelectorIPSets, ipsets.NewTranslatedIPSet(ns, ipsets.Namespace)) - podSelectorList = append(podSelectorList, policies.NewSetInfo(ns, ipsets.Namespace, included, matchType)) - return podSelectorIPSets, podSelectorList, nil + psSets = append(psSets, ipsets.NewTranslatedIPSet(ns, ipsets.Namespace)) + psList = append(psList, policies.NewSetInfo(ns, ipsets.Namespace, included, matchType)) + return } // nameSpaceSelector translates namespaceSelector of NetworkPolicyPeer in networkpolicy object to translatedIPSet and SetInfo. @@ -397,11 +401,12 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire // #2.3 handle podSelector and port if exist if peer.PodSelector != nil && peer.NamespaceSelector == nil { - podSelectorIPSets, podSelectorList, err := podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, matchType, peer.PodSelector) + podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err := podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, matchType, peer.PodSelector) if err != nil { return err } npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, podSelectorIPSets...) + npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, childPodSelectorIPSets...) err = peerAndPortRule(npmNetPol, direction, ports, podSelectorList) if err != nil { return err @@ -418,11 +423,12 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire } // #2.4 handle namespaceSelector and podSelector and port if exist - podSelectorIPSets, podSelectorList, err := podSelector(npmNetPol.PolicyKey, matchType, peer.PodSelector) + podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err := podSelector(npmNetPol.PolicyKey, matchType, peer.PodSelector) if err != nil { return err } npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, podSelectorIPSets...) + npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, childPodSelectorIPSets...) // Before translating NamespaceSelector, flattenNameSpaceSelector function call should be called // to handle multiple values in matchExpressions spec. @@ -542,7 +548,12 @@ func TranslatePolicy(npObj *networkingv1.NetworkPolicy) (*policies.NPMNetworkPol // podSelector in spec.PodSelector is common for ingress and egress. // Process this podSelector first. var err error - npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, &npObj.Spec.PodSelector) + npmNetPol.PodSelectorIPSets, npmNetPol.ChildPodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS( + npmNetPol.PolicyKey, + npmNetPol.Namespace, + policies.EitherMatch, + &npObj.Spec.PodSelector, + ) if err != nil { return nil, err } diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index c096addad1..6894c49a1d 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -539,12 +539,13 @@ func TestPodSelector(t *testing.T) { policyKey := "test-ns/test-policy" policyKeyWithDash := policyKey + "-" tests := []struct { - name string - namespace string - matchType policies.MatchType - labelSelector *metav1.LabelSelector - podSelectorIPSets []*ipsets.TranslatedIPSet - podSelectorList []policies.SetInfo + name string + namespace string + matchType policies.MatchType + labelSelector *metav1.LabelSelector + podSelectorIPSets []*ipsets.TranslatedIPSet + childPodSelectorIPSets []*ipsets.TranslatedIPSet + podSelectorList []policies.SetInfo }{ { name: "all pods selector in default namespace in ingress", @@ -556,6 +557,7 @@ func TestPodSelector(t *testing.T) { podSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, podSelectorList: []policies.SetInfo{ policies.NewSetInfo(defaultNS, ipsets.Namespace, included, matchType), }, @@ -570,6 +572,7 @@ func TestPodSelector(t *testing.T) { podSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("test", ipsets.Namespace), }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, podSelectorList: []policies.SetInfo{ policies.NewSetInfo("test", ipsets.Namespace, included, matchType), }, @@ -585,6 +588,7 @@ func TestPodSelector(t *testing.T) { podSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, podSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, matchType), }, @@ -607,6 +611,7 @@ func TestPodSelector(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("label", ipsets.KeyLabelOfPod), }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, podSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, matchType), policies.NewSetInfo("label", ipsets.KeyLabelOfPod, included, matchType), @@ -633,6 +638,7 @@ func TestPodSelector(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("labelIn:src", ipsets.KeyValueLabelOfPod), }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, podSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, matchType), policies.NewSetInfo("labelIn:src", ipsets.KeyValueLabelOfPod, included, matchType), @@ -659,6 +665,7 @@ func TestPodSelector(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("labelNotIn:src", ipsets.KeyValueLabelOfPod), }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, podSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, matchType), policies.NewSetInfo("labelNotIn:src", ipsets.KeyValueLabelOfPod, nonIncluded, matchType), @@ -690,14 +697,57 @@ func TestPodSelector(t *testing.T) { podSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(policyKeyWithDash+"k1:v10:v11", ipsets.NestedLabelOfPod, []string{"k1:v10", "k1:v11"}...), + ipsets.NewTranslatedIPSet("k2", ipsets.KeyLabelOfPod), + }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), + }, + podSelectorList: []policies.SetInfo{ + policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, matchType), + policies.NewSetInfo(policyKeyWithDash+"k1:v10:v11", ipsets.NestedLabelOfPod, included, matchType), + policies.NewSetInfo("k2", ipsets.KeyLabelOfPod, nonIncluded, matchType), + }, + }, + { + name: "target pod Selector with three labels AND a namespace (one included value, one non-included value, and one included netest value) for acl in ingress", + namespace: defaultNS, + matchType: matchType, + labelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k0": "v0", + }, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "k1", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "v10", + "v11", + }, + }, + { + Key: "k2", + Operator: metav1.LabelSelectorOpDoesNotExist, + Values: []string{}, + }, + }, + }, + podSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet(policyKeyWithDash+"k1:v10:v11", ipsets.NestedLabelOfPod, []string{"k1:v10", "k1:v11"}...), ipsets.NewTranslatedIPSet("k2", ipsets.KeyLabelOfPod), + ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), + }, + childPodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), }, podSelectorList: []policies.SetInfo{ policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, matchType), policies.NewSetInfo(policyKeyWithDash+"k1:v10:v11", ipsets.NestedLabelOfPod, included, matchType), policies.NewSetInfo("k2", ipsets.KeyLabelOfPod, nonIncluded, matchType), + policies.NewSetInfo(defaultNS, ipsets.Namespace, included, matchType), }, }, } @@ -707,16 +757,18 @@ func TestPodSelector(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() var podSelectorIPSets []*ipsets.TranslatedIPSet + var childPodSelectorIPSets []*ipsets.TranslatedIPSet var podSelectorList []policies.SetInfo var err error if tt.namespace == "" { - podSelectorIPSets, podSelectorList, err = podSelector(policyKey, tt.matchType, tt.labelSelector) + podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err = podSelector(policyKey, tt.matchType, tt.labelSelector) } else { // technically, the policyKey prefix would contain the namespace, but it might not for these tests - podSelectorIPSets, podSelectorList, err = podSelectorWithNS(policyKey, tt.namespace, tt.matchType, tt.labelSelector) + podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err = podSelectorWithNS(policyKey, tt.namespace, tt.matchType, tt.labelSelector) } require.NoError(t, err) require.Equal(t, tt.podSelectorIPSets, podSelectorIPSets) + require.Equal(t, tt.childPodSelectorIPSets, childPodSelectorIPSets) require.Equal(t, tt.podSelectorList, podSelectorList) }) } @@ -1300,14 +1352,16 @@ func TestIngressPolicy(t *testing.T) { emptyString := intstr.FromString("") // TODO(jungukcho): add test cases with more complex rules tests := []struct { - name string - targetSelector *metav1.LabelSelector - rules []networkingv1.NetworkPolicyIngressRule - npmNetPol *policies.NPMNetworkPolicy - wantErr bool + name string + isNewNwPolicyVer bool + targetSelector *metav1.LabelSelector + rules []networkingv1.NetworkPolicyIngressRule + npmNetPol *policies.NPMNetworkPolicy + wantErr bool }{ { - name: "only port in ingress rules", + name: "only port in ingress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1330,6 +1384,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), @@ -1349,7 +1404,8 @@ func TestIngressPolicy(t *testing.T) { }, }, { - name: "only ipBlock in ingress rules", + name: "only ipBlock in ingress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1375,6 +1431,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), @@ -1395,7 +1452,8 @@ func TestIngressPolicy(t *testing.T) { }, }, { - name: "only peer podSelector in ingress rules", + name: "only peer podSelector in ingress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1422,6 +1480,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), @@ -1444,7 +1503,8 @@ func TestIngressPolicy(t *testing.T) { }, }, { - name: "only peer nameSpaceSelector in ingress rules", + name: "only peer nameSpaceSelector in ingress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1471,6 +1531,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), @@ -1491,7 +1552,8 @@ func TestIngressPolicy(t *testing.T) { }, }, { - name: "peer nameSpaceSelector and ipblock in ingress rules", + name: "peer nameSpaceSelector and ipblock in ingress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1529,6 +1591,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet(defaultNS, ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo(defaultNS, ipsets.Namespace, included, targetPodMatchType), @@ -1565,7 +1628,8 @@ func TestIngressPolicy(t *testing.T) { }, }, { - name: "error", + name: "error", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1589,6 +1653,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1605,7 +1670,8 @@ func TestIngressPolicy(t *testing.T) { wantErr: true, }, { - name: "allow all ingress rules", + name: "allow all ingress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1622,6 +1688,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1635,7 +1702,8 @@ func TestIngressPolicy(t *testing.T) { }, }, { - name: "deny all in ingress rules", + name: "deny all in ingress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "src", @@ -1650,6 +1718,7 @@ func TestIngressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:src", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:src", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1659,19 +1728,204 @@ func TestIngressPolicy(t *testing.T) { }, }, }, + { + name: "multi-value pod/target selector", + isNewNwPolicyVer: true, + targetSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k0": "v0", + }, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "k1", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "v10", + "v11", + }, + }, + { + Key: "k2", + Operator: metav1.LabelSelectorOpDoesNotExist, + Values: []string{}, + }, + }, + }, + rules: []networkingv1.NetworkPolicyIngressRule{ + {}, + }, + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, "k1:v10", "k1:v11"), + ipsets.NewTranslatedIPSet("k2", ipsets.KeyLabelOfPod), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), + }, + PodSelectorList: []policies.SetInfo{ + policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("k2", ipsets.KeyLabelOfPod, nonIncluded, targetPodMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + }, + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + }, + }, + }, + }, + { + name: "multi-value pod/peer selector", + isNewNwPolicyVer: true, + targetSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k0": "v0", + }, + }, + rules: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "k1", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "v10", + "v11", + }, + }, + }, + }, + }, + }, + }, + }, + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, + PodSelectorList: []policies.SetInfo{ + policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + }, + RuleIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, "k1:v10", "k1:v11"), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), + }, + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcList: []policies.SetInfo{ + policies.NewSetInfo("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, included, peerMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, peerMatchType), + }, + }, + { + Target: policies.Dropped, + Direction: policies.Ingress, + }, + }, + }, + }, + { + name: "multi-value pod/peer selector with namespace selector in same peer rule", + isNewNwPolicyVer: true, + targetSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k0": "v0", + }, + }, + rules: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "peer-nsselector-kay": "peer-nsselector-value", + }, + }, + PodSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "k1", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "v10", + "v11", + }, + }, + }, + }, + }, + }, + }, + }, + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, + PodSelectorList: []policies.SetInfo{ + policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + }, + RuleIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, "k1:v10", "k1:v11"), + ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace), + }, + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcList: []policies.SetInfo{ + policies.NewSetInfo("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace, included, peerMatchType), + policies.NewSetInfo("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, included, peerMatchType), + }, + }, + { + Target: policies.Dropped, + Direction: policies.Ingress, + }, + }, + }, + }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - t.Parallel() + util.IsNewNwPolicyVerFlag = tt.isNewNwPolicyVer npmNetPol := &policies.NPMNetworkPolicy{ Namespace: tt.npmNetPol.Namespace, PolicyKey: tt.npmNetPol.PolicyKey, ACLPolicyID: tt.npmNetPol.ACLPolicyID, } var err error - npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) + npmNetPol.PodSelectorIPSets, npmNetPol.ChildPodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) require.NoError(t, err) splitPolicyKey := strings.Split(npmNetPol.PolicyKey, "/") require.Len(t, splitPolicyKey, 2, "policy key must include name") @@ -1692,14 +1946,16 @@ func TestEgressPolicy(t *testing.T) { targetPodMatchType := policies.EitherMatch peerMatchType := policies.DstMatch tests := []struct { - name string - targetSelector *metav1.LabelSelector - rules []networkingv1.NetworkPolicyEgressRule - npmNetPol *policies.NPMNetworkPolicy - wantErr bool + name string + isNewNwPolicyVer bool + targetSelector *metav1.LabelSelector + rules []networkingv1.NetworkPolicyEgressRule + npmNetPol *policies.NPMNetworkPolicy + wantErr bool }{ { - name: "only port in egress rules", + name: "only port in egress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -1722,6 +1978,7 @@ func TestEgressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1741,7 +1998,8 @@ func TestEgressPolicy(t *testing.T) { }, }, { - name: "only ipBlock in egress rules", + name: "only ipBlock in egress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -1771,6 +2029,7 @@ func TestEgressPolicy(t *testing.T) { policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, RuleIPSets: []*ipsets.TranslatedIPSet{ ipsets.NewTranslatedIPSet("only-ipblock-in-ns-default-0-0OUT", ipsets.CIDRBlocks, []string{"172.17.0.0/16", "172.17.1.0/24 nomatch"}...), }, @@ -1787,7 +2046,8 @@ func TestEgressPolicy(t *testing.T) { }, }, { - name: "only peer podSelector in egress rules", + name: "only peer podSelector in egress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -1814,6 +2074,7 @@ func TestEgressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1836,7 +2097,8 @@ func TestEgressPolicy(t *testing.T) { }, }, { - name: "only peer nameSpaceSelector in egress rules", + name: "only peer nameSpaceSelector in egress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -1863,6 +2125,7 @@ func TestEgressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1883,7 +2146,8 @@ func TestEgressPolicy(t *testing.T) { }, }, { - name: "deny all in egress rules", + name: "deny all in egress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -1898,6 +2162,7 @@ func TestEgressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1908,7 +2173,8 @@ func TestEgressPolicy(t *testing.T) { }, }, { - name: "allow all egress rules", + name: "allow all egress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -1925,6 +2191,7 @@ func TestEgressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -1938,7 +2205,8 @@ func TestEgressPolicy(t *testing.T) { }, }, { - name: "peer nameSpaceSelector and ipblock in egress rules", + name: "peer nameSpaceSelector and ipblock in egress rules", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -1976,6 +2244,7 @@ func TestEgressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -2012,7 +2281,8 @@ func TestEgressPolicy(t *testing.T) { }, }, { - name: "error", + name: "error", + isNewNwPolicyVer: true, targetSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "label": "dst", @@ -2036,6 +2306,7 @@ func TestEgressPolicy(t *testing.T) { ipsets.NewTranslatedIPSet("label:dst", ipsets.KeyValueLabelOfPod), ipsets.NewTranslatedIPSet("default", ipsets.Namespace), }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, PodSelectorList: []policies.SetInfo{ policies.NewSetInfo("label:dst", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), @@ -2051,19 +2322,204 @@ func TestEgressPolicy(t *testing.T) { }, wantErr: true, }, + { + name: "multi-value pod/target selector", + isNewNwPolicyVer: true, + targetSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k0": "v0", + }, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "k1", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "v10", + "v11", + }, + }, + { + Key: "k2", + Operator: metav1.LabelSelectorOpDoesNotExist, + Values: []string{}, + }, + }, + }, + rules: []networkingv1.NetworkPolicyEgressRule{ + {}, + }, + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, "k1:v10", "k1:v11"), + ipsets.NewTranslatedIPSet("k2", ipsets.KeyLabelOfPod), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), + }, + PodSelectorList: []policies.SetInfo{ + policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("k2", ipsets.KeyLabelOfPod, nonIncluded, targetPodMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + }, + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + }, + }, + }, + }, + { + name: "multi-value pod/peer selector", + isNewNwPolicyVer: true, + targetSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k0": "v0", + }, + }, + rules: []networkingv1.NetworkPolicyEgressRule{ + { + To: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "k1", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "v10", + "v11", + }, + }, + }, + }, + }, + }, + }, + }, + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, + PodSelectorList: []policies.SetInfo{ + policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + }, + RuleIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, "k1:v10", "k1:v11"), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), + }, + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstList: []policies.SetInfo{ + policies.NewSetInfo("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, included, peerMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, peerMatchType), + }, + }, + { + Target: policies.Dropped, + Direction: policies.Egress, + }, + }, + }, + }, + { + name: "multi-value pod/peer selector with namespace selector in same peer rule", + isNewNwPolicyVer: true, + targetSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k0": "v0", + }, + }, + rules: []networkingv1.NetworkPolicyEgressRule{ + { + To: []networkingv1.NetworkPolicyPeer{ + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "peer-nsselector-kay": "peer-nsselector-value", + }, + }, + PodSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "k1", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "v10", + "v11", + }, + }, + }, + }, + }, + }, + }, + }, + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: "default", + PolicyKey: "default/serve-tcp", + ACLPolicyID: "azure-acl-default-serve-tcp", + PodSelectorIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("k0:v0", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("default", ipsets.Namespace), + }, + ChildPodSelectorIPSets: []*ipsets.TranslatedIPSet{}, + PodSelectorList: []policies.SetInfo{ + policies.NewSetInfo("k0:v0", ipsets.KeyValueLabelOfPod, included, targetPodMatchType), + policies.NewSetInfo("default", ipsets.Namespace, included, targetPodMatchType), + }, + RuleIPSets: []*ipsets.TranslatedIPSet{ + ipsets.NewTranslatedIPSet("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, "k1:v10", "k1:v11"), + ipsets.NewTranslatedIPSet("k1:v10", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("k1:v11", ipsets.KeyValueLabelOfPod), + ipsets.NewTranslatedIPSet("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace), + }, + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstList: []policies.SetInfo{ + policies.NewSetInfo("peer-nsselector-kay:peer-nsselector-value", ipsets.KeyValueLabelOfNamespace, included, peerMatchType), + policies.NewSetInfo("default/serve-tcp-k1:v10:v11", ipsets.NestedLabelOfPod, included, peerMatchType), + }, + }, + { + Target: policies.Dropped, + Direction: policies.Egress, + }, + }, + }, + }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - t.Parallel() + util.IsNewNwPolicyVerFlag = tt.isNewNwPolicyVer npmNetPol := &policies.NPMNetworkPolicy{ Namespace: tt.npmNetPol.Namespace, PolicyKey: tt.npmNetPol.PolicyKey, ACLPolicyID: tt.npmNetPol.ACLPolicyID, } var err error - npmNetPol.PodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) + npmNetPol.PodSelectorIPSets, npmNetPol.ChildPodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) require.NoError(t, err) splitPolicyKey := strings.Split(npmNetPol.PolicyKey, "/") require.Len(t, splitPolicyKey, 2, "policy key must include name") diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 5ef5e4000f..20f7208f98 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -203,7 +203,7 @@ func (dp *DataPlane) ApplyDataPlane() error { func (dp *DataPlane) AddPolicy(policy *policies.NPMNetworkPolicy) error { klog.Infof("[DataPlane] Add Policy called for %s", policy.PolicyKey) // Create and add references for Selector IPSets first - err := dp.createIPSetsAndReferences(policy.PodSelectorIPSets, policy.PolicyKey, ipsets.SelectorType) + err := dp.createIPSetsAndReferences(policy.AllPodSelectorIPSets(), policy.PolicyKey, ipsets.SelectorType) if err != nil { klog.Infof("[DataPlane] error while adding Selector IPSet references: %s", err.Error()) return fmt.Errorf("[DataPlane] error while adding Selector IPSet references: %w", err) @@ -254,7 +254,7 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { } // Remove references for Selector IPSets - err = dp.deleteIPSetsAndReferences(policy.PodSelectorIPSets, policy.PolicyKey, ipsets.SelectorType) + err = dp.deleteIPSetsAndReferences(policy.AllPodSelectorIPSets(), policy.PolicyKey, ipsets.SelectorType) if err != nil { return err } diff --git a/npm/pkg/dataplane/ipsets/ipset_test.go b/npm/pkg/dataplane/ipsets/ipset_test.go index 4e1cabe781..fc426f82b1 100644 --- a/npm/pkg/dataplane/ipsets/ipset_test.go +++ b/npm/pkg/dataplane/ipsets/ipset_test.go @@ -8,16 +8,16 @@ import ( func TestAffiliatedIPs(t *testing.T) { s1 := NewIPSet(NewIPSetMetadata("test-set1", Namespace)) - s1.IPPodKey["1.1.1.1"] = "pod-a" - s1.IPPodKey["2.2.2.2"] = "pod-b" + s1.IPPodKey["1.1.1.1"] = "pod-w" + s1.IPPodKey["2.2.2.2"] = "pod-x" s2 := NewIPSet(NewIPSetMetadata("test-set2", Namespace)) - s2.IPPodKey["3.3.3.3"] = "pod-c" - s2.IPPodKey["4.4.4.4"] = "pod-d" + s2.IPPodKey["3.3.3.3"] = "pod-y" + s2.IPPodKey["4.4.4.4"] = "pod-z" // 1 IP from each set above s3 := NewIPSet(NewIPSetMetadata("test-set3", Namespace)) - s3.IPPodKey["1.1.1.1"] = "pod-a" - s3.IPPodKey["4.4.4.4"] = "pod-d" + s3.IPPodKey["1.1.1.1"] = "pod-w" + s3.IPPodKey["4.4.4.4"] = "pod-z" l := NewIPSet(NewIPSetMetadata("test-list", KeyLabelOfNamespace)) l.MemberIPSets[s1.Name] = s1 diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 53dec5b7c9..788c8f55d9 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -330,7 +330,7 @@ func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicyS } if len(policyNetworkRequest.Policies) == 0 { - klog.Info("[Dataplane Windows] no %s type of sets to apply", policyType) + klog.Infof("[Dataplane Windows] no %s type of sets to apply", policyType) return nil, nil } diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index d29ddd1553..b1a113e33c 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -17,13 +17,16 @@ type NPMNetworkPolicy struct { PolicyKey string // ACLPolicyID is only used in Windows. See aclPolicyID() in policy_windows.go for more info ACLPolicyID string - // PodSelectorIPSets holds all the IPSets generated from Pod Selector + // TODO get rid of PodSelectorIPSets in favor of PodSelectorList (exact same except need to add members field to SetInfo) + // PodSelectorIPSets holds the IPSets for the Pod Selector PodSelectorIPSets []*ipsets.TranslatedIPSet + // ChildPodSelectorIPSets holds the IPSets that are members of any ipset in PodSelectorIPSets + ChildPodSelectorIPSets []*ipsets.TranslatedIPSet // TODO change to slice of pointers - // PodSelectorList holds target pod information to avoid duplicatoin in SrcList and DstList fields in ACLs + // PodSelectorList holds the ipsets from PodSelectorIPSets and info about them to avoid duplication in SrcList and DstList fields in ACLs PodSelectorList []SetInfo // RuleIPSets holds all IPSets generated from policy's rules - // and not from pod selector IPSets + // and not from pod selector IPSets, including children of a NestedLabelOfPod ipset RuleIPSets []*ipsets.TranslatedIPSet ACLs []*ACLPolicy // podIP is key and endpoint ID as value @@ -39,6 +42,10 @@ func NewNPMNetworkPolicy(netPolName, netPolNamespace string) *NPMNetworkPolicy { } } +func (netPol *NPMNetworkPolicy) AllPodSelectorIPSets() []*ipsets.TranslatedIPSet { + return append(netPol.PodSelectorIPSets, netPol.ChildPodSelectorIPSets...) +} + func (netPol *NPMNetworkPolicy) numACLRulesProducedInKernel() int { numRules := 0 hasIngress := false From 7a3237cb4ce16fdbae5f67591fd82ee1c114e35d Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 19 May 2022 13:32:18 -0700 Subject: [PATCH 5/7] address comments --- npm/pkg/dataplane/ipsets/ipset.go | 3 +-- npm/pkg/dataplane/ipsets/ipsetmanager_windows.go | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index a8c838b821..a248a4916c 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -385,8 +385,7 @@ func (set *IPSet) affiliatedIPs() map[string]struct{} { return result } - // number of member ipsets usually underestimates the final map size - result := make(map[string]struct{}, len(set.MemberIPSets)) + result := make(map[string]struct{}) for _, memberSet := range set.MemberIPSets { for ip := range memberSet.IPPodKey { result[ip] = struct{}{} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 788c8f55d9..99b83adce2 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -306,6 +306,10 @@ func (setPolicyBuilder *networkPolicyBuilder) setNameExists(setName string) bool } func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicySetting, policyType hcn.SetPolicyType) ([]byte, error) { + if len(setPolicySettings) == 0 { + klog.Info("[Dataplane Windows] no set policies to apply on network") + return nil, nil + } klog.Infof("[Dataplane Windows] marshalling %s type of sets", policyType) policyNetworkRequest := &hcn.PolicyNetworkRequest{ Policies: make([]hcn.NetworkPolicy, 0), From ed104970c79d69205dea12763d62704669d10969 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 20 May 2022 10:08:25 -0700 Subject: [PATCH 6/7] address named return comment --- .../translation/translatePolicy.go | 68 ++++++++++--------- .../translation/translatePolicy_test.go | 29 ++++---- 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 7043530e9e..7f36b4a4f1 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -27,6 +27,12 @@ var ( ErrUnsupportedNegativeMatch = errors.New("unsupported NotExist operator translation features used on windows") ) +type podSelectorResult struct { + psSets []*ipsets.TranslatedIPSet + childPSSets []*ipsets.TranslatedIPSet + psList []policies.SetInfo +} + type netpolPortType string const ( @@ -217,43 +223,44 @@ func ipBlockRule(policyName, ns string, direction policies.Direction, matchType // PodSelector translates podSelector of NetworkPolicyPeer field in networkpolicy object to translatedIPSets, children of translated IPSets, and SetInfo. // Children are members of a list-type IPSet. // This function is called only when the NetworkPolicyPeer has namespaceSelector field. -func podSelector(policyKey string, matchType policies.MatchType, selector *metav1.LabelSelector) (psSets, childPSSets []*ipsets.TranslatedIPSet, psList []policies.SetInfo, err error) { +func podSelector(policyKey string, matchType policies.MatchType, selector *metav1.LabelSelector) (*podSelectorResult, error) { podSelectors, err := parsePodSelector(policyKey, selector) if err != nil { - return + return nil, err } - lenOfPodSelectors := len(podSelectors) - psSets = make([]*ipsets.TranslatedIPSet, 0) - childPSSets = make([]*ipsets.TranslatedIPSet, 0) - psList = make([]policies.SetInfo, lenOfPodSelectors) + lenOfPodSelectors := len(podSelectors) + psResult := &podSelectorResult{ + psSets: make([]*ipsets.TranslatedIPSet, 0), + childPSSets: make([]*ipsets.TranslatedIPSet, 0), + psList: make([]policies.SetInfo, lenOfPodSelectors), + } for i := 0; i < lenOfPodSelectors; i++ { ps := podSelectors[i] - psSets = append(psSets, ipsets.NewTranslatedIPSet(ps.setName, ps.setType, ps.members...)) + psResult.psSets = append(psResult.psSets, ipsets.NewTranslatedIPSet(ps.setName, ps.setType, ps.members...)) // if value is nested value, create translatedIPSet with the nested value for j := 0; j < len(ps.members); j++ { - childPSSets = append(childPSSets, ipsets.NewTranslatedIPSet(ps.members[j], ipsets.KeyValueLabelOfPod)) + psResult.childPSSets = append(psResult.childPSSets, ipsets.NewTranslatedIPSet(ps.members[j], ipsets.KeyValueLabelOfPod)) } - psList[i] = policies.NewSetInfo(ps.setName, ps.setType, ps.include, matchType) + psResult.psList[i] = policies.NewSetInfo(ps.setName, ps.setType, ps.include, matchType) } - - return + return psResult, nil } // podSelectorWithNS translates podSelector of spec and NetworkPolicyPeer in networkpolicy object to translatedIPSets, children of translated IPSets, and SetInfo. // Children are members of a list-type IPSet. // This function is called only when the NetworkPolicyPeer does not have namespaceSelector field. -func podSelectorWithNS(policyKey, ns string, matchType policies.MatchType, selector *metav1.LabelSelector) (psSets, childPSSets []*ipsets.TranslatedIPSet, psList []policies.SetInfo, err error) { - psSets, childPSSets, psList, err = podSelector(policyKey, matchType, selector) +func podSelectorWithNS(policyKey, ns string, matchType policies.MatchType, selector *metav1.LabelSelector) (*podSelectorResult, error) { + psResult, err := podSelector(policyKey, matchType, selector) if err != nil { - return + return nil, err } // Add translatedIPSet and SetInfo based on namespace - psSets = append(psSets, ipsets.NewTranslatedIPSet(ns, ipsets.Namespace)) - psList = append(psList, policies.NewSetInfo(ns, ipsets.Namespace, included, matchType)) - return + psResult.psSets = append(psResult.psSets, ipsets.NewTranslatedIPSet(ns, ipsets.Namespace)) + psResult.psList = append(psResult.psList, policies.NewSetInfo(ns, ipsets.Namespace, included, matchType)) + return psResult, nil } // nameSpaceSelector translates namespaceSelector of NetworkPolicyPeer in networkpolicy object to translatedIPSet and SetInfo. @@ -401,13 +408,13 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire // #2.3 handle podSelector and port if exist if peer.PodSelector != nil && peer.NamespaceSelector == nil { - podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err := podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, matchType, peer.PodSelector) + psResult, err := podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, matchType, peer.PodSelector) if err != nil { return err } - npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, podSelectorIPSets...) - npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, childPodSelectorIPSets...) - err = peerAndPortRule(npmNetPol, direction, ports, podSelectorList) + npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, psResult.psSets...) + npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, psResult.childPSSets...) + err = peerAndPortRule(npmNetPol, direction, ports, psResult.psList) if err != nil { return err } @@ -423,12 +430,12 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire } // #2.4 handle namespaceSelector and podSelector and port if exist - podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err := podSelector(npmNetPol.PolicyKey, matchType, peer.PodSelector) + psResult, err := podSelector(npmNetPol.PolicyKey, matchType, peer.PodSelector) if err != nil { return err } - npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, podSelectorIPSets...) - npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, childPodSelectorIPSets...) + npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, psResult.psSets...) + npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, psResult.childPSSets...) // Before translating NamespaceSelector, flattenNameSpaceSelector function call should be called // to handle multiple values in matchExpressions spec. @@ -436,7 +443,7 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire for i := range flattenNSSelector { nsSelectorIPSets, nsSelectorList := nameSpaceSelector(matchType, &flattenNSSelector[i]) npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, nsSelectorIPSets...) - nsSelectorList = append(nsSelectorList, podSelectorList...) + nsSelectorList = append(nsSelectorList, psResult.psList...) err := peerAndPortRule(npmNetPol, direction, ports, nsSelectorList) if err != nil { return err @@ -547,16 +554,13 @@ func TranslatePolicy(npObj *networkingv1.NetworkPolicy) (*policies.NPMNetworkPol // podSelector in spec.PodSelector is common for ingress and egress. // Process this podSelector first. - var err error - npmNetPol.PodSelectorIPSets, npmNetPol.ChildPodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS( - npmNetPol.PolicyKey, - npmNetPol.Namespace, - policies.EitherMatch, - &npObj.Spec.PodSelector, - ) + psResult, err := podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, &npObj.Spec.PodSelector) if err != nil { return nil, err } + npmNetPol.PodSelectorIPSets = psResult.psSets + npmNetPol.ChildPodSelectorIPSets = psResult.childPSSets + npmNetPol.PodSelectorList = psResult.psList // Each NetworkPolicy includes a policyTypes list which may include either Ingress, Egress, or both. // If no policyTypes are specified on a NetworkPolicy then by default Ingress will always be set diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index 6894c49a1d..f7f5276526 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -756,20 +756,21 @@ func TestPodSelector(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - var podSelectorIPSets []*ipsets.TranslatedIPSet - var childPodSelectorIPSets []*ipsets.TranslatedIPSet - var podSelectorList []policies.SetInfo + var psResult *podSelectorResult var err error if tt.namespace == "" { - podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err = podSelector(policyKey, tt.matchType, tt.labelSelector) + psResult, err = podSelector(policyKey, tt.matchType, tt.labelSelector) } else { // technically, the policyKey prefix would contain the namespace, but it might not for these tests - podSelectorIPSets, childPodSelectorIPSets, podSelectorList, err = podSelectorWithNS(policyKey, tt.namespace, tt.matchType, tt.labelSelector) + psResult, err = podSelectorWithNS(policyKey, tt.namespace, tt.matchType, tt.labelSelector) + } + if psResult == nil { + psResult = &podSelectorResult{} } require.NoError(t, err) - require.Equal(t, tt.podSelectorIPSets, podSelectorIPSets) - require.Equal(t, tt.childPodSelectorIPSets, childPodSelectorIPSets) - require.Equal(t, tt.podSelectorList, podSelectorList) + require.Equal(t, tt.podSelectorIPSets, psResult.psSets) + require.Equal(t, tt.childPodSelectorIPSets, psResult.childPSSets) + require.Equal(t, tt.podSelectorList, psResult.psList) }) } } @@ -1924,8 +1925,10 @@ func TestIngressPolicy(t *testing.T) { PolicyKey: tt.npmNetPol.PolicyKey, ACLPolicyID: tt.npmNetPol.ACLPolicyID, } - var err error - npmNetPol.PodSelectorIPSets, npmNetPol.ChildPodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) + psResult, err := podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) + npmNetPol.PodSelectorIPSets = psResult.psSets + npmNetPol.ChildPodSelectorIPSets = psResult.childPSSets + npmNetPol.PodSelectorList = psResult.psList require.NoError(t, err) splitPolicyKey := strings.Split(npmNetPol.PolicyKey, "/") require.Len(t, splitPolicyKey, 2, "policy key must include name") @@ -2518,8 +2521,10 @@ func TestEgressPolicy(t *testing.T) { PolicyKey: tt.npmNetPol.PolicyKey, ACLPolicyID: tt.npmNetPol.ACLPolicyID, } - var err error - npmNetPol.PodSelectorIPSets, npmNetPol.ChildPodSelectorIPSets, npmNetPol.PodSelectorList, err = podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) + psResult, err := podSelectorWithNS(npmNetPol.PolicyKey, npmNetPol.Namespace, policies.EitherMatch, tt.targetSelector) + npmNetPol.PodSelectorIPSets = psResult.psSets + npmNetPol.ChildPodSelectorIPSets = psResult.childPSSets + npmNetPol.PodSelectorList = psResult.psList require.NoError(t, err) splitPolicyKey := strings.Split(npmNetPol.PolicyKey, "/") require.Len(t, splitPolicyKey, 2, "policy key must include name") From 18d5a883f499c435b00a3a3f35b987902ca2fc3a Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 20 May 2022 16:21:27 -0700 Subject: [PATCH 7/7] optimize space/time for checking if an ip satisfies a pod selector --- npm/pkg/dataplane/dataplane_windows.go | 39 +++--- npm/pkg/dataplane/ipsets/ipset.go | 47 ++----- npm/pkg/dataplane/ipsets/ipset_test.go | 48 ------- .../dataplane/ipsets/ipsetmanager_windows.go | 118 +++++++++++++++--- 4 files changed, 123 insertions(+), 129 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 4d2a3f2f75..f8742cd3df 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -172,22 +172,23 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { if _, ok := endpoint.NetPolReference[policyKey]; ok { continue } + // TODO Also check if the endpoint reference in policy for this Ip is right - netpolSelectorIPs, err := dp.getSelectorIPsByPolicyKey(policyKey) + policy, ok := dp.policyMgr.GetPolicy(policyKey) + if !ok { + return fmt.Errorf("policy with name %s does not exist", policyKey) + } + + selectorIPSets := dp.getSelectorIPSets(policy) + ok, err := dp.ipsetMgr.DoesIPSatisfySelectorIPSets(pod.PodIP, selectorIPSets) if err != nil { return err } - - if _, ok := netpolSelectorIPs[pod.PodIP]; !ok { + if !ok { continue } // Apply the network policy - policy, ok := dp.policyMgr.GetPolicy(policyKey) - if !ok { - return fmt.Errorf("policy with name %s does not exist", policyKey) - } - endpointList := map[string]string{ endpoint.IP: endpoint.ID, } @@ -202,24 +203,13 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error { return nil } -func (dp *DataPlane) getSelectorIPsByPolicyKey(policyKey string) (map[string]struct{}, error) { - policy, ok := dp.policyMgr.GetPolicy(policyKey) - if !ok { - return nil, fmt.Errorf("policy with name %s does not exist", policyKey) - } - - return dp.getSelectorIPsByPolicy(policy) -} - -func (dp *DataPlane) getSelectorIPsByPolicy(policy *policies.NPMNetworkPolicy) (map[string]struct{}, error) { +func (dp *DataPlane) getSelectorIPSets(policy *policies.NPMNetworkPolicy) map[string]struct{} { selectorIpSets := make(map[string]struct{}) for _, ipset := range policy.PodSelectorIPSets { selectorIpSets[ipset.Metadata.GetPrefixName()] = struct{}{} } - klog.Infof("policy %s has policy selector: %+v", policy.PolicyKey, selectorIpSets) // FIXME remove after debugging - - return dp.ipsetMgr.GetIPsFromSelectorIPSets(selectorIpSets) + return selectorIpSets } func (dp *DataPlane) getEndpointsToApplyPolicy(policy *policies.NPMNetworkPolicy) (map[string]string, error) { @@ -229,8 +219,8 @@ func (dp *DataPlane) getEndpointsToApplyPolicy(policy *policies.NPMNetworkPolicy return nil, err } - // TODO need to calculate all existing selector - netpolSelectorIPs, err := dp.getSelectorIPsByPolicy(policy) + selectorIPSets := dp.getSelectorIPSets(policy) + netpolSelectorIPs, err := dp.ipsetMgr.GetIPsFromSelectorIPSets(selectorIPSets) if err != nil { return nil, err } @@ -239,8 +229,7 @@ func (dp *DataPlane) getEndpointsToApplyPolicy(policy *policies.NPMNetworkPolicy for ip := range netpolSelectorIPs { endpoint, ok := dp.endpointCache[ip] if !ok { - // this endpoint might not be in this particular Node. - klog.Infof("[DataPlane] Ignoring endpoint with IP %s. Not found in endpointCache", ip) + klog.Infof("[DataPlane] Ignoring endpoint with IP %s since it was not found in the endpoint cache. This IP might not be in the HNS network", ip) continue } endpointList[ip] = endpoint.ID diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index a248a4916c..9b860bf225 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -372,49 +372,22 @@ func (set *IPSet) hasMember(memberName string) bool { return isMember } -// affiliatedIPs returns the IPs for a hash set or the IPs of the member sets for a list set -// This method, intersectAffiliatedIPs, and GetSetContents are good examples of how the -// ipset struct may have been better designed as an interface with hash and list implementations. -// Not worth it to redesign though. -func (set *IPSet) affiliatedIPs() map[string]struct{} { +// isIPAffiliated determines whether an IP belongs to the set or its member sets in the case of a list set. +// This method and GetSetContents are good examples of how the ipset struct may have been better designed +// as an interface with hash and list implementations. Not worth it to redesign though. +func (set *IPSet) isIPAffiliated(ip string) bool { if set.Kind == HashSet { - result := make(map[string]struct{}, len(set.IPPodKey)) - for ip := range set.IPPodKey { - result[ip] = struct{}{} + if _, ok := set.IPPodKey[ip]; ok { + return true } - return result } - - result := make(map[string]struct{}) for _, memberSet := range set.MemberIPSets { - for ip := range memberSet.IPPodKey { - result[ip] = struct{}{} - } - } - return result -} - -// intersectAffiliatedIPs removes IPs that aren't affiliated with this set -func (set *IPSet) intersectAffiliatedIPs(existingIPs map[string]struct{}) { - for ip := range existingIPs { - isAffiliated := false - if set.Kind == HashSet { - _, ok := set.IPPodKey[ip] - isAffiliated = ok - } else { - for _, memberSet := range set.MemberIPSets { - _, ok := memberSet.IPPodKey[ip] - if ok { - isAffiliated = true - break - } - } - } - - if !isAffiliated { - delete(existingIPs, ip) + _, ok := memberSet.IPPodKey[ip] + if ok { + return true } } + return false } func (set *IPSet) canSetBeSelectorIPSet() bool { diff --git a/npm/pkg/dataplane/ipsets/ipset_test.go b/npm/pkg/dataplane/ipsets/ipset_test.go index fc426f82b1..2a30e1bb7f 100644 --- a/npm/pkg/dataplane/ipsets/ipset_test.go +++ b/npm/pkg/dataplane/ipsets/ipset_test.go @@ -6,54 +6,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestAffiliatedIPs(t *testing.T) { - s1 := NewIPSet(NewIPSetMetadata("test-set1", Namespace)) - s1.IPPodKey["1.1.1.1"] = "pod-w" - s1.IPPodKey["2.2.2.2"] = "pod-x" - s2 := NewIPSet(NewIPSetMetadata("test-set2", Namespace)) - s2.IPPodKey["3.3.3.3"] = "pod-y" - s2.IPPodKey["4.4.4.4"] = "pod-z" - - // 1 IP from each set above - s3 := NewIPSet(NewIPSetMetadata("test-set3", Namespace)) - s3.IPPodKey["1.1.1.1"] = "pod-w" - s3.IPPodKey["4.4.4.4"] = "pod-z" - - l := NewIPSet(NewIPSetMetadata("test-list", KeyLabelOfNamespace)) - l.MemberIPSets[s1.Name] = s1 - l.MemberIPSets[s2.Name] = s2 - - expected := map[string]struct{}{ - "1.1.1.1": {}, - "2.2.2.2": {}, - } - require.Equal(t, expected, s1.affiliatedIPs(), "unexpected affiliated IPs for set 1") - - expected = map[string]struct{}{ - "1.1.1.1": {}, - "2.2.2.2": {}, - "3.3.3.3": {}, - "4.4.4.4": {}, - } - require.Equal(t, expected, l.affiliatedIPs(), "unexpected affiliated IPs for list") - - intersection := s3.affiliatedIPs() - l.intersectAffiliatedIPs(intersection) - expected = map[string]struct{}{ - "1.1.1.1": {}, - "4.4.4.4": {}, - } - require.Equal(t, expected, intersection, "unexpected intersection (direction #1)") - - intersection = l.affiliatedIPs() - s3.intersectAffiliatedIPs(intersection) - expected = map[string]struct{}{ - "1.1.1.1": {}, - "4.4.4.4": {}, - } - require.Equal(t, expected, intersection, "unexpected intersection (direction #2)") -} - func TestShouldBeInKernelAndCanDelete(t *testing.T) { s := &IPSetMetadata{"test-set", Namespace} l := &IPSetMetadata{"test-list", KeyLabelOfNamespace} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 99b83adce2..04021b057b 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -28,6 +28,28 @@ type networkPolicyBuilder struct { toDeleteSets map[string]*hcn.SetPolicySetting } +func (iMgr *IPSetManager) DoesIPSatisfySelectorIPSets(ip string, setList map[string]struct{}) (bool, error) { + if len(setList) == 0 { + klog.Infof("[ipset manager] unexpectedly encountered empty selector list") + return true, nil + } + iMgr.Lock() + defer iMgr.Unlock() + + if err := iMgr.validateSelectorIPSets(setList); err != nil { + return false, err + } + + for setName := range setList { + set := iMgr.setMap[setName] + if !set.isIPAffiliated(ip) { + return false, nil + } + } + + return true, nil +} + // GetIPsFromSelectorIPSets will take in a map of prefixedSetNames and return an intersection of IPs func (iMgr *IPSetManager) GetIPsFromSelectorIPSets(setList map[string]struct{}) (map[string]struct{}, error) { if len(setList) == 0 { @@ -36,32 +58,71 @@ func (iMgr *IPSetManager) GetIPsFromSelectorIPSets(setList map[string]struct{}) iMgr.Lock() defer iMgr.Unlock() - ips := make(map[string]struct{}) - firstLoop := true + if err := iMgr.validateSelectorIPSets(setList); err != nil { + return nil, err + } + + // the following is a space/time optimized way to get the intersection of IPs from the selector sets + // we should always take the hash set branch because a pod selector always includes a namespace ipset, + // which is a hash set, and we favor hash sets for firstSet + var firstSet *IPSet for setName := range setList { - if !iMgr.exists(setName) { - return nil, npmerrors.Errorf( - npmerrors.GetSelectorReference, - false, - fmt.Sprintf("[ipset manager] selector ipset %s does not exist", setName)) + set := iMgr.setMap[setName] + if set.Kind == HashSet || firstSet == nil { + // firstSet can be any set, but ideally is a hash set for efficiency (compare the branch for hash sets to the one for lists below) + firstSet = set } + } + ips := make(map[string]struct{}) + if firstSet.Kind == HashSet { + // include every IP in firstSet that is also affiliated with every other selector set + for ip := range firstSet.IPPodKey { + isAffiliated := true + for otherSetName := range setList { + if otherSetName == firstSet.Name { + continue + } + otherSet := iMgr.setMap[otherSetName] + if !otherSet.isIPAffiliated(ip) { + isAffiliated = false + break + } + } - set := iMgr.setMap[setName] - if !set.canSetBeSelectorIPSet() { - return nil, npmerrors.Errorf( - npmerrors.IPSetIntersection, - false, - fmt.Sprintf("[IPSet] Selector IPSet cannot be of type %s", set.Type.String())) + if isAffiliated { + ips[ip] = struct{}{} + } + } + } else { + // should never reach this branch (see note above) + // include every IP affiliated with firstSet that is also affiliated with every other selector set + // identical to the hash set case, except we have to make space for all IPs affiliated with firstSet + + // only loop over the unique affiliated IPs + for _, memberSet := range firstSet.MemberIPSets { + for ip := range memberSet.IPPodKey { + ips[ip] = struct{}{} + } } + for ip := range ips { + // identical to the hash set case + isAffiliated := true + for otherSetName := range setList { + if otherSetName == firstSet.Name { + continue + } + otherSet := iMgr.setMap[otherSetName] + if !otherSet.isIPAffiliated(ip) { + isAffiliated = false + break + } + } - if firstLoop { - ips = set.affiliatedIPs() - firstLoop = false - } else { - set.intersectAffiliatedIPs(ips) + if !isAffiliated { + delete(ips, ip) + } } } - klog.Infof("IPs in selector IPSets: %+v", ips) // FIXME remove after debugging return ips, nil } @@ -79,6 +140,25 @@ func (iMgr *IPSetManager) GetSelectorReferencesBySet(setName string) (map[string return set.SelectorReference, nil } +func (iMgr *IPSetManager) validateSelectorIPSets(setList map[string]struct{}) error { + for setName := range setList { + if !iMgr.exists(setName) { + return npmerrors.Errorf( + npmerrors.GetSelectorReference, + false, + fmt.Sprintf("[ipset manager] selector ipset %s does not exist", setName)) + } + set := iMgr.setMap[setName] + if !set.canSetBeSelectorIPSet() { + return npmerrors.Errorf( + npmerrors.IPSetIntersection, + false, + fmt.Sprintf("[IPSet] Selector IPSet cannot be of type %s", set.Type.String())) + } + } + return nil +} + func (iMgr *IPSetManager) resetIPSets() error { klog.Infof("[IPSetManager Windows] Resetting Dataplane") network, err := iMgr.getHCnNetwork()