diff --git a/.github/workflows/cyclonus-netpol-test.yaml b/.github/workflows/cyclonus-netpol-test.yaml index 5fce38aa16..a3a2ab88a7 100644 --- a/.github/workflows/cyclonus-netpol-test.yaml +++ b/.github/workflows/cyclonus-netpol-test.yaml @@ -40,7 +40,7 @@ jobs: - name: Install Azure NPM run: | - sed -i 's/mcr.microsoft.com\/containernetworking\/azure-npm:v1.3.1/acnpublic.azurecr.io\/azure-npm:cyclonus/' ./npm/azure-npm.yaml + sed -i 's/mcr.microsoft.com\/containernetworking\/azure-npm:v1.4.1/acnpublic.azurecr.io\/azure-npm:cyclonus/' ./npm/azure-npm.yaml kind load docker-image acnpublic.azurecr.io/azure-npm:cyclonus --name npm-kind kubectl apply -f ./npm/azure-npm.yaml diff --git a/npm/ipsm/ipsm.go b/npm/ipsm/ipsm.go index e74b776ee5..0dd57e64ca 100644 --- a/npm/ipsm/ipsm.go +++ b/npm/ipsm/ipsm.go @@ -17,6 +17,14 @@ import ( utilexec "k8s.io/utils/exec" ) +// ReferCountOperation is used to indicate whether ipset refer count should be increased or decreased. +type ReferCountOperation bool + +const ( + IncrementOp ReferCountOperation = true + DecrementOp ReferCountOperation = false +) + type ipsEntry struct { operationFlag string name string @@ -38,11 +46,20 @@ type Ipset struct { referCount int } +func (ipset *Ipset) incReferCount() { + ipset.referCount++ +} + +func (ipset *Ipset) decReferCount() { + ipset.referCount-- +} + // NewIpset creates a new instance for Ipset object. func NewIpset(setName string) *Ipset { return &Ipset{ - name: setName, - elements: make(map[string]string), + name: setName, + elements: make(map[string]string), + referCount: 0, } } @@ -73,6 +90,21 @@ func (ipsMgr *IpsetManager) Exists(listName string, setName string, kind string) return true } +// IpSetReferIncOrDec checks if an element exists in setMap/listMap and then increases or decreases this referCount. +func (ipsMgr *IpsetManager) IpSetReferIncOrDec(ipsetName string, kind string, countOperation ReferCountOperation) { + m := ipsMgr.SetMap + if kind == util.IpsetSetListFlag { + m = ipsMgr.ListMap + } + + switch countOperation { + case IncrementOp: + m[ipsetName].incReferCount() + case DecrementOp: + m[ipsetName].decReferCount() + } +} + // SetExists checks if an ipset exists, and returns the type func (ipsMgr *IpsetManager) SetExists(setName string) (bool, string) { _, exists := ipsMgr.SetMap[setName] @@ -122,6 +154,11 @@ func (ipsMgr *IpsetManager) DeleteList(listName string) error { set: util.GetHashedName(listName), } + if ipsMgr.ListMap[listName].referCount > 0 { + ipsMgr.IpSetReferIncOrDec(listName, util.IpsetSetListFlag, DecrementOp) + return nil + } + if errCode, err := ipsMgr.Run(entry); err != nil { if errCode == 1 { return nil @@ -428,6 +465,8 @@ func (ipsMgr *IpsetManager) DeleteFromSet(setName, ip, podKey string) error { return nil } +// TODO this below function is to be extended while improving ipset refer count +// support, if not used, please remove this stale function. // Clean removes all the empty sets & lists under the namespace. func (ipsMgr *IpsetManager) Clean() error { for setName, set := range ipsMgr.SetMap { diff --git a/npm/networkPolicyController.go b/npm/networkPolicyController.go index 5134f9ceec..ea6c7cbfa0 100644 --- a/npm/networkPolicyController.go +++ b/npm/networkPolicyController.go @@ -351,10 +351,24 @@ func (c *networkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 return fmt.Errorf("[syncAddAndUpdateNetPol] Error: creating ipset named port %s with err: %v", set, err) } } - for _, list := range lists { - if err = ipsMgr.CreateList(list); err != nil { - return fmt.Errorf("[syncAddAndUpdateNetPol] Error: creating ipset list %s with err: %v", list, err) + + // lists is a map with list name and members as value + // NPM will create the list first and increments the refer count + for listKey := range lists { + if err = ipsMgr.CreateList(listKey); err != nil { + return fmt.Errorf("[syncAddAndUpdateNetPol] Error: creating ipset list %s with err: %v", listKey, err) + } + ipsMgr.IpSetReferIncOrDec(listKey, util.IpsetSetListFlag, ipsm.IncrementOp) + } + // Then NPM will add members to the above list, this is to avoid members being added + // to lists before they are created. + for listKey, listLabelsMembers := range lists { + for _, listMember := range listLabelsMembers { + if err = ipsMgr.AddToList(listKey, listMember); err != nil { + return fmt.Errorf("[syncAddAndUpdateNetPol] Error: Adding ipset member %s to ipset list %s with err: %v", listMember, listKey, err) + } } + ipsMgr.IpSetReferIncOrDec(listKey, util.IpsetSetListFlag, ipsm.IncrementOp) } if err = c.createCidrsRule("in", netPolObj.ObjectMeta.Name, netPolObj.ObjectMeta.Namespace, ingressIPCidrs, ipsMgr); err != nil { @@ -385,7 +399,7 @@ func (c *networkPolicyController) cleanUpNetworkPolicy(netPolKey string, isSafeC ipsMgr := c.npMgr.NsMap[util.KubeAllNamespacesFlag].IpsMgr iptMgr := c.npMgr.NsMap[util.KubeAllNamespacesFlag].iptMgr // translate policy from "cachedNetPolObj" - _, _, _, ingressIPCidrs, egressIPCidrs, iptEntries := translatePolicy(cachedNetPolObj) + _, _, lists, ingressIPCidrs, egressIPCidrs, iptEntries := translatePolicy(cachedNetPolObj) var err error // delete iptables entries @@ -395,6 +409,17 @@ func (c *networkPolicyController) cleanUpNetworkPolicy(netPolKey string, isSafeC } } + // lists is a map with list name and members as value + for listKey := range lists { + // We do not have delete the members before deleting set as, + // 1. ipset allows deleting a ipset list with members + // 2. if the refer count is more than one we should not remove members + // 3. for reduced datapath operations + if err = ipsMgr.DeleteList(listKey); err != nil { + return fmt.Errorf("[syncAddAndUpdateNetPol] Error: creating ipset list %s with err: %v", listKey, err) + } + } + // delete ipset list related to ingress CIDRs if err = c.removeCidrsRule("in", cachedNetPolObj.Name, cachedNetPolObj.Namespace, ingressIPCidrs, ipsMgr); err != nil { return fmt.Errorf("[cleanUpNetworkPolicy] Error: removeCidrsRule in due to %v", err) diff --git a/npm/npm.go b/npm/npm.go index 99ed6138c9..f4dab0a135 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -164,7 +164,7 @@ func (npMgr *NetworkPolicyManager) backup() { time.Sleep(backupWaitTimeInSeconds * time.Second) if err = iptMgr.Save(util.IptablesConfigFile); err != nil { - metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to back up Azure-NPM states") + metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to back up Azure-NPM states %s", err.Error()) } } } diff --git a/npm/parseSelector.go b/npm/parseSelector.go index 4625e563a4..594b60e5c9 100644 --- a/npm/parseSelector.go +++ b/npm/parseSelector.go @@ -121,30 +121,161 @@ func sortSelector(selector *metav1.LabelSelector) { selector.MatchExpressions = sortedReqs } +// getSetNameForMultiValueSelector takes in label with multiple values without operator +// and returns a new 2nd level ipset name +func getSetNameForMultiValueSelector(key string, vals []string) string { + newIpSet := key + for _, val := range vals { + newIpSet = util.GetIpSetFromLabelKV(newIpSet, val) + } + return newIpSet +} + // HashSelector returns the hash value of the selector. func HashSelector(selector *metav1.LabelSelector) string { sortSelector(selector) return util.Hash(fmt.Sprintf("%v", selector)) } -// parseSelector takes a LabelSelector and returns a slice of processed labels, keys and values. -func parseSelector(selector *metav1.LabelSelector) ([]string, []string, []string) { +// flattenNameSpaceSelector will help flatten multiple NameSpace selector match Expressions values +// into multiple label selectors helping with the OR condition. +func FlattenNameSpaceSelector(nsSelector *metav1.LabelSelector) []metav1.LabelSelector { + /* + This function helps to create multiple labelSelectors when given a single multivalue nsSelector + Take below exmaple: this nsSelector has 2 values in a matchSelector. + - namespaceSelector: + matchExpressions: + - key: ns + operator: NotIn + values: + - netpol-x + - netpol-y + + goal is to convert this single nsSelector into multiple nsSelectors to preserve OR condition + between multiple values of the matchExpr i.e. this function will return + + - namespaceSelector: + matchExpressions: + - key: ns + operator: NotIn + values: + - netpol-x + - namespaceSelector: + matchExpressions: + - key: ns + operator: NotIn + values: + - netpol-y + + then, translate policy will replicate each of these nsSelectors to add two different rules in iptables, + resulting in OR condition between the values. + + Check TestFlattenNameSpaceSelector 2nd subcase for complex scenario + */ + + // To avoid any additional length checks, just return a slice of labelSelectors + // with original nsSelector + if nsSelector == nil { + return []metav1.LabelSelector{} + } + + if len(nsSelector.MatchExpressions) == 0 { + return []metav1.LabelSelector{*nsSelector} + } + + // create a baseSelector which needs to be same across all + // new labelSelectors + baseSelector := &metav1.LabelSelector{ + MatchLabels: nsSelector.MatchLabels, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + } + + multiValuePresent := false + multiValueMatchExprs := []metav1.LabelSelectorRequirement{} + for _, req := range nsSelector.MatchExpressions { + + // Only In and NotIn operators of matchExprs have multiple values + // NPM will ignore single value matchExprs of these operators. + // for multiple values, it will create a slice of them to be used for Zipping with baseSelector + // to create multiple nsSelectors to preserve OR condition across all labels and expressions + if (req.Operator == metav1.LabelSelectorOpIn) || (req.Operator == metav1.LabelSelectorOpNotIn) { + if len(req.Values) == 1 { + // for length 1, add the matchExpr to baseSelector + baseSelector.MatchExpressions = append(baseSelector.MatchExpressions, req) + } else { + multiValuePresent = true + multiValueMatchExprs = append(multiValueMatchExprs, req) + } + } else if (req.Operator == metav1.LabelSelectorOpExists) || (req.Operator == metav1.LabelSelectorOpDoesNotExist) { + // since Exists and NotExists do not contain any values, NPM can safely add them to the baseSelector + baseSelector.MatchExpressions = append(baseSelector.MatchExpressions, req) + } else { + log.Errorf("Invalid operator [%s] for selector [%v] requirement", req.Operator, *nsSelector) + } + } + + // If there are no multiValue NS selector match expressions + // return the original NsSelector + if !multiValuePresent { + return []metav1.LabelSelector{*nsSelector} + } + + // Now use the baseSelector and loop over multiValueMatchExprs to create all + // combinations of values + flatNsSelectors := []metav1.LabelSelector{ + *baseSelector.DeepCopy(), + } + for _, req := range multiValueMatchExprs { + flatNsSelectors = zipMatchExprs(flatNsSelectors, req) + } + + return flatNsSelectors +} + +// zipMatchExprs helps with zipping a given matchExpr with given baseLabelSelectors +// this func will loop over each baseSelector in the slice, +// deepCopies each baseSelector, combines with given matchExpr by looping over each value +// and creating a new LabelSelector with given baseSelector and value matchExpr +// then returns a new slice of these zipped LabelSelectors +func zipMatchExprs(baseSelectors []metav1.LabelSelector, matchExpr metav1.LabelSelectorRequirement) []metav1.LabelSelector { + zippedLabelSelectors := []metav1.LabelSelector{} + for _, selector := range baseSelectors { + for _, value := range matchExpr.Values { + tempBaseSelector := selector.DeepCopy() + tempBaseSelector.MatchExpressions = append( + tempBaseSelector.MatchExpressions, + metav1.LabelSelectorRequirement{ + Key: matchExpr.Key, + Operator: matchExpr.Operator, + Values: []string{value}, + }, + ) + zippedLabelSelectors = append(zippedLabelSelectors, *tempBaseSelector) + + } + } + return zippedLabelSelectors +} + +// parseSelector takes a LabelSelector and returns a slice of processed labels, Lists with members as values. +// this function returns a slice of all the label ipsets excluding multivalue matchExprs +// and a map of labelKeys and labelIpsetname for multivalue match exprs +// higher level functions will need to compute what sets or ipsets should be +// used from this map +func parseSelector(selector *metav1.LabelSelector) ([]string, map[string][]string) { var ( labels []string - keys []string - vals []string + vals map[string][]string ) + vals = make(map[string][]string) if selector == nil { - return labels, keys, vals + return labels, vals } if len(selector.MatchLabels) == 0 && len(selector.MatchExpressions) == 0 { labels = append(labels, "") - keys = append(keys, "") - vals = append(vals, "") - - return labels, keys, vals + return labels, vals } sortedKeys, sortedVals := util.SortMap(&selector.MatchLabels) @@ -152,42 +283,39 @@ func parseSelector(selector *metav1.LabelSelector) ([]string, []string, []string for i := range sortedKeys { labels = append(labels, sortedKeys[i]+":"+sortedVals[i]) } - keys = append(keys, sortedKeys...) - vals = append(vals, sortedVals...) for _, req := range selector.MatchExpressions { var k string switch op := req.Operator; op { case metav1.LabelSelectorOpIn: - for _, v := range req.Values { - k = req.Key - keys = append(keys, k) - vals = append(vals, v) - labels = append(labels, k+":"+v) + k = req.Key + if len(req.Values) == 1 { + labels = append(labels, k+":"+req.Values[0]) + } else { + // We are not adding the k:v to labels for multiple values, because, labels are used + // to contruct partial IptEntries and if these below labels are added then we are inducing + // AND condition on values of a match expression instead of OR + vals[k] = append(vals[k], req.Values...) } case metav1.LabelSelectorOpNotIn: - for _, v := range req.Values { - k = util.IptablesNotFlag + req.Key - keys = append(keys, k) - vals = append(vals, v) - labels = append(labels, k+":"+v) + k = util.IptablesNotFlag + req.Key + if len(req.Values) == 1 { + labels = append(labels, k+":"+req.Values[0]) + } else { + vals[k] = append(vals[k], req.Values...) } // Exists matches pods with req.Key as key case metav1.LabelSelectorOpExists: k = req.Key - keys = append(keys, req.Key) - vals = append(vals, "") labels = append(labels, k) // DoesNotExist matches pods without req.Key as key case metav1.LabelSelectorOpDoesNotExist: k = util.IptablesNotFlag + req.Key - keys = append(keys, k) - vals = append(vals, "") labels = append(labels, k) default: log.Errorf("Invalid operator [%s] for selector [%v] requirement", op, *selector) } } - return labels, keys, vals + return labels, vals } diff --git a/npm/parseSelector_test.go b/npm/parseSelector_test.go index 1ee671ef5a..cdfd6d2f77 100644 --- a/npm/parseSelector_test.go +++ b/npm/parseSelector_test.go @@ -301,17 +301,13 @@ func TestHashSelector(t *testing.T) { func TestParseSelector(t *testing.T) { var selector, expectedSelector *metav1.LabelSelector selector, expectedSelector = nil, nil - labels, keys, vals := parseSelector(selector) - expectedLabels, expectedKeys, expectedVals := []string{}, []string{}, []string{} + labels, vals := parseSelector(selector) + expectedLabels, expectedVals := []string{}, make(map[string][]string) if len(labels) != len(expectedLabels) { t.Errorf("TestparseSelector failed @ labels length comparison") } - if len(keys) != len(expectedKeys) { - t.Errorf("TestparseSelector failed @ keys length comparison") - } - if len(vals) != len(expectedVals) { t.Errorf("TestparseSelector failed @ vals length comparison") } @@ -321,16 +317,12 @@ func TestParseSelector(t *testing.T) { } selector = &metav1.LabelSelector{} - labels, keys, vals = parseSelector(selector) - expectedLabels, expectedKeys, expectedVals = []string{""}, []string{""}, []string{""} + labels, vals = parseSelector(selector) + expectedLabels = []string{""} if len(labels) != len(expectedLabels) { t.Errorf("TestparseSelector failed @ labels length comparison") } - if len(keys) != len(expectedKeys) { - t.Errorf("TestparseSelector failed @ keys length comparison") - } - if len(vals) != len(expectedVals) { t.Errorf("TestparseSelector failed @ vals length comparison") } @@ -348,38 +340,26 @@ func TestParseSelector(t *testing.T) { }, } - labels, keys, vals = parseSelector(selector) - expectedLabels = []string{ - "testIn:frontend", - "testIn:backend", - } - expectedKeys = []string{ - "testIn", - "testIn", - } - expectedVals = []string{ - "frontend", - "backend", + labels, vals = parseSelector(selector) + expectedLabels = []string{} + expectedVals = map[string][]string{ + "testIn": { + "frontend", + "backend", + }, } if len(labels) != len(expectedLabels) { t.Errorf("TestparseSelector failed @ labels length comparison") } - if len(keys) != len(expectedKeys) { - t.Errorf("TestparseSelector failed @ keys length comparison") - } - - if len(vals) != len(vals) { + if len(vals) != len(expectedVals) { t.Errorf("TestparseSelector failed @ vals length comparison") } - if !reflect.DeepEqual(labels, expectedLabels) { + if labels != nil { t.Errorf("TestparseSelector failed @ label comparison") } - if !reflect.DeepEqual(keys, expectedKeys) { - t.Errorf("TestparseSelector failed @ key comparison") - } if !reflect.DeepEqual(vals, expectedVals) { t.Errorf("TestparseSelector failed @ value comparison") } @@ -396,41 +376,31 @@ func TestParseSelector(t *testing.T) { me := &selector.MatchExpressions *me = append(*me, notIn) - labels, keys, vals = parseSelector(selector) - addedLabels := []string{ - "!testNotIn:frontend", - "!testNotIn:backend", - } - addedKeys := []string{ - "!testNotIn", - "!testNotIn", - } - addedVals := []string{ - "frontend", - "backend", + labels, vals = parseSelector(selector) + addedLabels := []string{} + addedVals := map[string][]string{ + "!testNotIn": { + "frontend", + "backend", + }, } + expectedLabels = append(expectedLabels, addedLabels...) - expectedKeys = append(expectedKeys, addedKeys...) - expectedVals = append(expectedVals, addedVals...) + for k, v := range addedVals { + expectedVals[k] = append(expectedVals[k], v...) + } if len(labels) != len(expectedLabels) { t.Errorf("TestparseSelector failed @ labels length comparison") } - if len(keys) != len(expectedKeys) { - t.Errorf("TestparseSelector failed @ keys length comparison") - } - - if len(vals) != len(vals) { + if len(vals) != len(expectedVals) { t.Errorf("TestparseSelector failed @ vals length comparison") } - if !reflect.DeepEqual(labels, expectedLabels) { + if labels != nil { t.Errorf("TestparseSelector failed @ label comparison") } - if !reflect.DeepEqual(keys, expectedKeys) { - t.Errorf("TestparseSelector failed @ key comparison") - } if !reflect.DeepEqual(vals, expectedVals) { t.Errorf("TestparseSelector failed @ value comparison") } @@ -443,38 +413,27 @@ func TestParseSelector(t *testing.T) { *me = append(*me, exists) - labels, keys, vals = parseSelector(selector) + labels, vals = parseSelector(selector) addedLabels = []string{ "testExists", } - addedKeys = []string{ - "testExists", - } - addedVals = []string{ - "", - } + addedVals = map[string][]string{} expectedLabels = append(expectedLabels, addedLabels...) - expectedKeys = append(expectedKeys, addedKeys...) - expectedVals = append(expectedVals, addedVals...) + for k, v := range addedVals { + expectedVals[k] = append(expectedVals[k], v...) + } if len(labels) != len(expectedLabels) { t.Errorf("TestparseSelector failed @ labels length comparison") } - if len(keys) != len(expectedKeys) { - t.Errorf("TestparseSelector failed @ keys length comparison") - } - - if len(vals) != len(vals) { + if len(vals) != len(expectedVals) { t.Errorf("TestparseSelector failed @ vals length comparison") } if !reflect.DeepEqual(labels, expectedLabels) { t.Errorf("TestparseSelector failed @ label comparison") } - if !reflect.DeepEqual(keys, expectedKeys) { - t.Errorf("TestparseSelector failed @ key comparison") - } if !reflect.DeepEqual(vals, expectedVals) { t.Errorf("TestparseSelector failed @ value comparison") } @@ -487,29 +446,21 @@ func TestParseSelector(t *testing.T) { *me = append(*me, doesNotExist) - labels, keys, vals = parseSelector(selector) + labels, vals = parseSelector(selector) addedLabels = []string{ "!testDoesNotExist", } - addedKeys = []string{ - "!testDoesNotExist", - } - addedVals = []string{ - "", - } + addedVals = map[string][]string{} expectedLabels = append(expectedLabels, addedLabels...) - expectedKeys = append(expectedKeys, addedKeys...) - expectedVals = append(expectedVals, addedVals...) + for k, v := range addedVals { + expectedVals[k] = append(expectedVals[k], v...) + } if len(labels) != len(expectedLabels) { t.Errorf("TestparseSelector failed @ labels length comparison") } - if len(keys) != len(expectedKeys) { - t.Errorf("TestparseSelector failed @ keys length comparison") - } - - if len(vals) != len(vals) { + if len(vals) != len(expectedVals) { t.Errorf("TestparseSelector failed @ vals length comparison") } @@ -517,11 +468,474 @@ func TestParseSelector(t *testing.T) { t.Errorf("TestparseSelector failed @ label comparison") } - if !reflect.DeepEqual(keys, expectedKeys) { - t.Errorf("TestparseSelector failed @ key comparison") - } - if !reflect.DeepEqual(vals, expectedVals) { t.Errorf("TestparseSelector failed @ value comparison") } } + +func TestFlattenNameSpaceSelectorCases(t *testing.T) { + firstSelector := &metav1.LabelSelector{} + + testSelectors := FlattenNameSpaceSelector(firstSelector) + if len(testSelectors) != 1 { + t.Errorf("TestFlattenNameSpaceSelectorCases failed @ 1st selector length check %+v", testSelectors) + } + + var secondSelector *metav1.LabelSelector + + testSelectors = FlattenNameSpaceSelector(secondSelector) + if len(testSelectors) > 0 { + t.Errorf("TestFlattenNameSpaceSelectorCases failed @ 1st selector length check %+v", testSelectors) + } + +} + +func TestFlattenNameSpaceSelector(t *testing.T) { + + commonMatchLabel := map[string]string{ + "c": "d", + "a": "b", + } + + firstSelector := &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + }, + }, + }, + MatchLabels: commonMatchLabel, + } + + testSelectors := FlattenNameSpaceSelector(firstSelector) + if len(testSelectors) != 1 { + t.Errorf("TestFlattenNameSpaceSelector failed @ 1st selector length check %+v", testSelectors) + } + + if !reflect.DeepEqual(testSelectors[0], *firstSelector) { + t.Errorf("TestFlattenNameSpaceSelector failed @ 1st selector deepEqual check.\n Expected: %+v \n Actual: %+v", *firstSelector, testSelectors[0]) + } + + secondSelector := &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + "frontend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + "b", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + "y", + }, + }, + }, + MatchLabels: commonMatchLabel, + } + + testSelectors = FlattenNameSpaceSelector(secondSelector) + if len(testSelectors) != 8 { + t.Errorf("TestFlattenNameSpaceSelector failed @ 2nd selector length check %+v", testSelectors) + } + + expectedSelectors := []metav1.LabelSelector{ + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "y", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "b", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "b", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "y", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "frontend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "frontend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "y", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "frontend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "b", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "frontend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "b", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "y", + }, + }, + }, + MatchLabels: commonMatchLabel, + }, + } + + if !reflect.DeepEqual(expectedSelectors, testSelectors) { + t.Errorf("TestFlattenNameSpaceSelector failed @ 2nd selector deepEqual check.\n Expected: %+v \n Actual: %+v", expectedSelectors, testSelectors) + } +} + +func TestFlattenNameSpaceSelectorWoMatchLabels(t *testing.T) { + firstSelector := &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + "y", + }, + }, + }, + } + + testSelectors := FlattenNameSpaceSelector(firstSelector) + if len(testSelectors) != 2 { + t.Errorf("TestFlattenNameSpaceSelector failed @ 1st selector length check %+v", testSelectors) + } + + expectedSelectors := []metav1.LabelSelector{ + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "t", + }, + }, + }, + }, + metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + metav1.LabelSelectorRequirement{ + Key: "testIn", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "backend", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "pod", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "a", + }, + }, + metav1.LabelSelectorRequirement{ + Key: "testExists", + Operator: metav1.LabelSelectorOpExists, + Values: []string{}, + }, + metav1.LabelSelectorRequirement{ + Key: "ns", + Operator: metav1.LabelSelectorOpIn, + Values: []string{ + "y", + }, + }, + }, + }, + } + + if !reflect.DeepEqual(testSelectors, expectedSelectors) { + t.Errorf("TestFlattenNameSpaceSelector failed @ 1st selector deepEqual check.\n Expected: %+v \n Actual: %+v", expectedSelectors, testSelectors) + } +} diff --git a/npm/testpolicies/allow-ns-y-z-pod-b-c.yaml b/npm/testpolicies/allow-ns-y-z-pod-b-c.yaml new file mode 100644 index 0000000000..71cb960a9b --- /dev/null +++ b/npm/testpolicies/allow-ns-y-z-pod-b-c.yaml @@ -0,0 +1,36 @@ +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: allow-ns-y-z-pod-b-c + namespace: netpol-4537-x +spec: + ingress: + - from: + - namespaceSelector: + matchExpressions: + - key: ns + operator: NotIn + values: + - netpol-4537-x + - netpol-4537-y + podSelector: + matchExpressions: + - key: pod + operator: In + values: + - b + - c + - key: app + operator: In + values: + - test + - int + podSelector: + matchExpressions: + - key: pod + operator: In + values: + - a + - x + policyTypes: + - Ingress diff --git a/npm/translatePolicy.go b/npm/translatePolicy.go index 8ced49254e..d63538b449 100644 --- a/npm/translatePolicy.go +++ b/npm/translatePolicy.go @@ -80,6 +80,7 @@ func craftPartialIptEntrySpecFromOpAndLabel(op, label, srcOrDstFlag string, isNa return util.DropEmptyFields(partialSpec) } +// TODO check this func references and change the label and op logic // craftPartialIptablesCommentFromSelector :- ns must be "" for namespace selectors func craftPartialIptEntrySpecFromOpsAndLabels(ns string, ops, labels []string, srcOrDstFlag string, isNamespaceSelector bool) []string { var spec []string @@ -113,6 +114,7 @@ func craftPartialIptEntrySpecFromOpsAndLabels(ns string, ops, labels []string, s } for i, _ := range ops { + // TODO need to change this logic, create a list of lsts here and have a single match against it spec = append(spec, craftPartialIptEntrySpecFromOpAndLabel(ops[i], labels[i], srcOrDstFlag, isNamespaceSelector)...) } @@ -120,10 +122,49 @@ func craftPartialIptEntrySpecFromOpsAndLabels(ns string, ops, labels []string, s } // craftPartialIptEntrySpecFromSelector :- ns must be "" for namespace selectors -func craftPartialIptEntrySpecFromSelector(ns string, selector *metav1.LabelSelector, srcOrDstFlag string, isNamespaceSelector bool) []string { - labelsWithOps, _, _ := parseSelector(selector) +// func helps in taking a labelSelector and converts it into corresponding matchSets +// to be a used in full iptable rules +// selector *metav1.LabelSelector: is used to create matchSets +// ns string: helps with adding a namespace name in case of empty (or all) selector +// srcOrDstFlag string: helps with determining if the src flag is to used in matchsets or dst flag, +// depending on ingress or egress translate policy +// isNamespaceSelector bool: helps in adding prefix for nameSpace ipsets +func craftPartialIptEntrySpecFromSelector(ns string, selector *metav1.LabelSelector, srcOrDstFlag string, isNamespaceSelector bool) ([]string, []string, map[string][]string) { + // parse the sector into labels and maps of multiVal match Exprs + labelsWithOps, nsLabelListKVs := parseSelector(selector) ops, labels := GetOperatorsAndLabels(labelsWithOps) - return craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, srcOrDstFlag, isNamespaceSelector) + + valueLabels := []string{} + listLabelsWithMembers := make(map[string][]string) + labelsForSpec := labels + // parseSelector returns a slice of processed label and a map of lists with members. + // now we need to compute the 2nd-level ipset names from lists and its members + // add use those 2nd level ipsets to be used to create the partial match set + for labelKeyWithOps, labelValueList := range nsLabelListKVs { + // look at each list and its members + op, labelKey := GetOperatorAndLabel(labelKeyWithOps) + // get the new 2nd level IpSet name + labelKVIpsetName := getSetNameForMultiValueSelector(labelKey, labelValueList) + if !util.StrExistsInSlice(labels, labelKVIpsetName) { + // Important: make sure length andordering of ops and labelsForSpec are same + // because craftPartialEntry loops over both of them at once and assumes + // a given position ops is to be applied on the same position label in labelsForSpec + ops = append(ops, op) + // TODO doubt check if this 2nd level needs to be added to the labels when labels are added to lists + // check if the 2nd level is already part of labels + labelsForSpec = append(labelsForSpec, labelKVIpsetName) + } + for _, labelValue := range labelValueList { + ipsetName := util.GetIpSetFromLabelKV(labelKey, labelValue) + valueLabels = append(valueLabels, ipsetName) + listLabelsWithMembers[labelKVIpsetName] = append(listLabelsWithMembers[labelKVIpsetName], ipsetName) + } + } + iptEntrySpecs := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labelsForSpec, srcOrDstFlag, isNamespaceSelector) + // only append valueLabels to labels after creating the Ipt Spec with valueLabels + // as 1D valueLabels are included in 2nd level labelKVIpsetName + labels = append(labels, valueLabels...) + return iptEntrySpecs, labels, listLabelsWithMembers } // craftPartialIptablesCommentFromSelector :- ns must be "" for namespace selectors @@ -140,8 +181,15 @@ func craftPartialIptablesCommentFromSelector(ns string, selector *metav1.LabelSe return "ns-" + ns } - labelsWithOps, _, _ := parseSelector(selector) + // TODO check if we are missing any crucial comment + labelsWithOps, labelKVs := parseSelector(selector) ops, labelsWithoutOps := GetOperatorsAndLabels(labelsWithOps) + for labelKeyWithOps, labelValueList := range labelKVs { + op, labelKey := GetOperatorAndLabel(labelKeyWithOps) + labelKVIpsetName := getSetNameForMultiValueSelector(labelKey, labelValueList) + labelsWithoutOps = append(labelsWithoutOps, labelKVIpsetName) + ops = append(ops, op) + } var comment, prefix, postfix string if isNamespaceSelector { @@ -160,11 +208,25 @@ func craftPartialIptablesCommentFromSelector(ns string, selector *metav1.LabelSe return comment[:len(comment)-len("-AND-")] + postfix } -func translateIngress(ns string, policyName string, targetSelector metav1.LabelSelector, rules []networkingv1.NetworkPolicyIngressRule) ([]string, []string, []string, [][]string, []*iptm.IptEntry) { +func appendSelectorLabelsToLists(lists, listLabelsWithMembers map[string][]string, isNamespaceSelector bool) { + for parsedListName, parsedListMembers := range listLabelsWithMembers { + if isNamespaceSelector { + parsedListName = util.GetNSNameWithPrefix(parsedListName) + } + for _, member := range parsedListMembers { + if isNamespaceSelector { + member = util.GetNSNameWithPrefix(member) + } + lists[parsedListName] = append(lists[parsedListName], member) + } + } +} + +func translateIngress(ns string, policyName string, targetSelector metav1.LabelSelector, rules []networkingv1.NetworkPolicyIngressRule) ([]string, []string, map[string][]string, [][]string, []*iptm.IptEntry) { var ( - sets []string // ipsets with type: net:hash - namedPorts []string // ipsets with type: hash:ip,port - lists []string // ipsets with type: list:set + netHashIPsets []string // ipsets with type: net:hash + namedPorts []string // ipsets with type: hash:ip,port + listIPsets map[string][]string // ipsets with type: list:set ipCidrs [][]string entries []*iptm.IptEntry fromRuleEntries []*iptm.IptEntry @@ -173,14 +235,15 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS ) log.Logf("started parsing ingress rule") - - labelsWithOps, _, _ := parseSelector(&targetSelector) - ops, labels := GetOperatorsAndLabels(labelsWithOps) - sets = append(sets, labels...) - sets = append(sets, "ns-"+ns) + netHashIPsets = append(netHashIPsets, "ns-"+ns) ipCidrs = make([][]string, len(rules)) + listIPsets = make(map[string][]string) - targetSelectorIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesDstFlag, false) + targetSelectorIptEntrySpec, labels, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector(ns, &targetSelector, util.IptablesDstFlag, false) + netHashIPsets = append(netHashIPsets, labels...) + for parsedListName, parsedListMembers := range listLabelsWithMembers { + listIPsets[parsedListName] = append(listIPsets[parsedListName], parsedListMembers...) + } targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false) for i, rule := range rules { @@ -235,7 +298,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS ) entries = append(entries, entry) - lists = append(lists, util.KubeAllNamespacesFlag) + listIPsets[util.KubeAllNamespacesFlag] = []string{} continue } @@ -421,21 +484,128 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS } if fromRule.PodSelector == nil && fromRule.NamespaceSelector != nil { - nsLabelsWithOps, _, _ := parseSelector(fromRule.NamespaceSelector) - _, nsLabelsWithoutOps := GetOperatorsAndLabels(nsLabelsWithOps) - if len(nsLabelsWithoutOps) == 1 && nsLabelsWithoutOps[0] == "" { - // Empty namespaceSelector. This selects all namespaces - nsLabelsWithoutOps[0] = util.KubeAllNamespacesFlag - } else { - for i, _ := range nsLabelsWithoutOps { - // Add namespaces prefix to distinguish namespace ipset lists and pod ipsets - nsLabelsWithoutOps[i] = "ns-" + nsLabelsWithoutOps[i] + for _, nsSelector := range FlattenNameSpaceSelector(fromRule.NamespaceSelector) { + iptPartialNsSpec, nsLabelsWithoutOps, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", &nsSelector, util.IptablesSrcFlag, true) + if len(nsLabelsWithoutOps) == 1 && nsLabelsWithoutOps[0] == "" { + // Empty namespaceSelector. This selects all namespaces + nsLabelsWithoutOps[0] = util.KubeAllNamespacesFlag + if _, ok := listIPsets[nsLabelsWithoutOps[0]]; !ok { + listIPsets[nsLabelsWithoutOps[0]] = nil + } + } else { + for i := range nsLabelsWithoutOps { + // Add namespaces prefix to distinguish namespace ipset lists and pod ipsets + nsLabelsWithoutOps[i] = util.GetNSNameWithPrefix(nsLabelsWithoutOps[i]) + if _, ok := listIPsets[nsLabelsWithoutOps[i]]; !ok { + listIPsets[nsLabelsWithoutOps[i]] = nil + } + } + appendSelectorLabelsToLists(listIPsets, listLabelsWithMembers, true) + } + iptPartialNsComment := craftPartialIptablesCommentFromSelector("", &nsSelector, true) + if portRuleExists { + for _, portRule := range rule.Ports { + switch portCheck := getPortType(portRule); portCheck { + case "namedport": + portName := util.NamedPortIPSetPrefix + portRule.Port.String() + namedPorts = append(namedPorts, portName) + entry := &iptm.IptEntry{ + Chain: util.IptablesAzureIngressPortChain, + Specs: append([]string(nil), targetSelectorIptEntrySpec...), + } + entry.Specs = append( + entry.Specs, + iptPartialNsSpec..., + ) + entry.Specs = append( + entry.Specs, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName(portName), + util.IptablesDstFlag+","+util.IptablesDstFlag, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureIngressMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-"+iptPartialNsComment+ + "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ + "-TO-"+targetSelectorComment, + ) + entries = append(entries, entry) + case "validport": + entry := &iptm.IptEntry{ + Chain: util.IptablesAzureIngressPortChain, + Specs: append([]string(nil), targetSelectorIptEntrySpec...), + } + entry.Specs = append( + entry.Specs, + iptPartialNsSpec..., + ) + entry.Specs = append( + entry.Specs, + craftPartialIptEntrySpecFromPort(portRule, util.IptablesDstPortFlag)..., + ) + entry.Specs = append( + entry.Specs, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureIngressMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-"+iptPartialNsComment+ + "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ + "-TO-"+targetSelectorComment, + ) + entries = append(entries, entry) + default: + log.Logf("Invalid NetworkPolicyPort.") + } + } + } else { + entry := &iptm.IptEntry{ + Chain: util.IptablesAzureIngressFromChain, + Specs: append([]string(nil), iptPartialNsSpec...), + } + entry.Specs = append( + entry.Specs, + targetSelectorIptEntrySpec..., + ) + entry.Specs = append( + entry.Specs, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureIngressMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-"+iptPartialNsComment+ + "-TO-"+targetSelectorComment, + ) + entries = append(entries, entry) } } - lists = append(lists, nsLabelsWithoutOps...) + continue + } - iptPartialNsSpec := craftPartialIptEntrySpecFromSelector("", fromRule.NamespaceSelector, util.IptablesSrcFlag, true) - iptPartialNsComment := craftPartialIptablesCommentFromSelector("", fromRule.NamespaceSelector, true) + if fromRule.PodSelector != nil && fromRule.NamespaceSelector == nil { + // TODO check old code if we need any ns- prefix for pod selectors + iptPartialPodSpec, podLabelsWithoutOps, listPodLabelsWithMembers := craftPartialIptEntrySpecFromSelector(ns, fromRule.PodSelector, util.IptablesSrcFlag, false) + if len(podLabelsWithoutOps) == 1 { + if podLabelsWithoutOps[0] == "" { + podLabelsWithoutOps[0] = util.GetNSNameWithPrefix(ns) + } + } + netHashIPsets = append(netHashIPsets, podLabelsWithoutOps...) + // TODO check this if ns- is needed here. + appendSelectorLabelsToLists(listIPsets, listPodLabelsWithMembers, false) + iptPartialPodComment := craftPartialIptablesCommentFromSelector(ns, fromRule.PodSelector, false) if portRuleExists { for _, portRule := range rule.Ports { switch portCheck := getPortType(portRule); portCheck { @@ -448,7 +618,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS } entry.Specs = append( entry.Specs, - iptPartialNsSpec..., + iptPartialPodSpec..., ) entry.Specs = append( entry.Specs, @@ -464,7 +634,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ + "ALLOW-"+iptPartialPodComment+ "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ "-TO-"+targetSelectorComment, ) @@ -476,7 +646,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS } entry.Specs = append( entry.Specs, - iptPartialNsSpec..., + iptPartialPodSpec..., ) entry.Specs = append( entry.Specs, @@ -491,7 +661,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ + "ALLOW-"+iptPartialPodComment+ "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ "-TO-"+targetSelectorComment, ) @@ -503,7 +673,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS } else { entry := &iptm.IptEntry{ Chain: util.IptablesAzureIngressFromChain, - Specs: append([]string(nil), iptPartialNsSpec...), + Specs: append([]string(nil), iptPartialPodSpec...), } entry.Specs = append( entry.Specs, @@ -518,7 +688,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ + "ALLOW-"+iptPartialPodComment+ "-TO-"+targetSelectorComment, ) entries = append(entries, entry) @@ -526,18 +696,28 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS continue } - if fromRule.PodSelector != nil && fromRule.NamespaceSelector == nil { - podLabelsWithOps, _, _ := parseSelector(fromRule.PodSelector) - _, podLabelsWithoutOps := GetOperatorsAndLabels(podLabelsWithOps) - if len(podLabelsWithoutOps) == 1 { - if podLabelsWithoutOps[0] == "" { - podLabelsWithoutOps[0] = "ns-" + ns + // fromRule has both namespaceSelector and podSelector set. + // We should match the selected pods in the selected namespaces. + // This allows traffic from podSelector intersects namespaceSelector + // This is only supported in kubernetes version >= 1.11 + if !util.IsNewNwPolicyVerFlag { + continue + } + for _, nsSelector := range FlattenNameSpaceSelector(fromRule.NamespaceSelector) { + // we pass empty ns for the podspec and comment here because it's a combo of both selectors and not limited to network policy namespace + iptPartialNsSpec, nsLabelsWithoutOps, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", &nsSelector, util.IptablesSrcFlag, true) // Add namespaces prefix to distinguish namespace ipsets and pod ipsets + for i := range nsLabelsWithoutOps { + nsLabelsWithoutOps[i] = util.GetNSNameWithPrefix(nsLabelsWithoutOps[i]) + if _, ok := listIPsets[nsLabelsWithoutOps[i]]; !ok { + listIPsets[nsLabelsWithoutOps[i]] = nil } } - sets = append(sets, podLabelsWithoutOps...) - - iptPartialPodSpec := craftPartialIptEntrySpecFromSelector(ns, fromRule.PodSelector, util.IptablesSrcFlag, false) - iptPartialPodComment := craftPartialIptablesCommentFromSelector(ns, fromRule.PodSelector, false) + appendSelectorLabelsToLists(listIPsets, listLabelsWithMembers, true) + iptPartialPodSpec, podLabelsWithoutOps, listPodLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", fromRule.PodSelector, util.IptablesSrcFlag, false) + netHashIPsets = append(netHashIPsets, podLabelsWithoutOps...) + appendSelectorLabelsToLists(listIPsets, listPodLabelsWithMembers, false) + iptPartialNsComment := craftPartialIptablesCommentFromSelector("", &nsSelector, true) + iptPartialPodComment := craftPartialIptablesCommentFromSelector("", fromRule.PodSelector, false) if portRuleExists { for _, portRule := range rule.Ports { switch portCheck := getPortType(portRule); portCheck { @@ -546,12 +726,16 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS namedPorts = append(namedPorts, portName) entry := &iptm.IptEntry{ Chain: util.IptablesAzureIngressPortChain, - Specs: append([]string(nil), targetSelectorIptEntrySpec...), + Specs: append([]string(nil), iptPartialNsSpec...), } entry.Specs = append( entry.Specs, iptPartialPodSpec..., ) + entry.Specs = append( + entry.Specs, + targetSelectorIptEntrySpec..., + ) entry.Specs = append( entry.Specs, util.IptablesModuleFlag, @@ -566,7 +750,8 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialPodComment+ + "ALLOW-"+iptPartialNsComment+ + "-AND-"+iptPartialPodComment+ "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ "-TO-"+targetSelectorComment, ) @@ -574,12 +759,16 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS case "validport": entry := &iptm.IptEntry{ Chain: util.IptablesAzureIngressPortChain, - Specs: append([]string(nil), targetSelectorIptEntrySpec...), + Specs: append([]string(nil), iptPartialNsSpec...), } entry.Specs = append( entry.Specs, iptPartialPodSpec..., ) + entry.Specs = append( + entry.Specs, + targetSelectorIptEntrySpec..., + ) entry.Specs = append( entry.Specs, craftPartialIptEntrySpecFromPort(portRule, util.IptablesDstPortFlag)..., @@ -593,7 +782,8 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialPodComment+ + "ALLOW-"+iptPartialNsComment+ + "-AND-"+iptPartialPodComment+ "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ "-TO-"+targetSelectorComment, ) @@ -605,11 +795,15 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS } else { entry := &iptm.IptEntry{ Chain: util.IptablesAzureIngressFromChain, - Specs: append([]string(nil), iptPartialPodSpec...), + Specs: append([]string(nil), targetSelectorIptEntrySpec...), } entry.Specs = append( entry.Specs, - targetSelectorIptEntrySpec..., + iptPartialNsSpec..., + ) + entry.Specs = append( + entry.Specs, + iptPartialPodSpec..., ) entry.Specs = append( entry.Specs, @@ -620,140 +814,12 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialPodComment+ + "ALLOW-"+iptPartialNsComment+ + "-AND-"+iptPartialPodComment+ "-TO-"+targetSelectorComment, ) entries = append(entries, entry) } - continue - } - - // fromRule has both namespaceSelector and podSelector set. - // We should match the selected pods in the selected namespaces. - // This allows traffic from podSelector intersects namespaceSelector - // This is only supported in kubernetes version >= 1.11 - if !util.IsNewNwPolicyVerFlag { - continue - } - - nsLabelsWithOps, _, _ := parseSelector(fromRule.NamespaceSelector) - _, nsLabelsWithoutOps := GetOperatorsAndLabels(nsLabelsWithOps) - // Add namespaces prefix to distinguish namespace ipsets and pod ipsets - for i, _ := range nsLabelsWithoutOps { - nsLabelsWithoutOps[i] = "ns-" + nsLabelsWithoutOps[i] - } - lists = append(lists, nsLabelsWithoutOps...) - - podLabelsWithOps, _, _ := parseSelector(fromRule.PodSelector) - _, podLabelsWithoutOps := GetOperatorsAndLabels(podLabelsWithOps) - sets = append(sets, podLabelsWithoutOps...) - - // we pass empty ns for the podspec and comment here because it's a combo of both selectors and not limited to network policy namespace - iptPartialNsSpec := craftPartialIptEntrySpecFromSelector("", fromRule.NamespaceSelector, util.IptablesSrcFlag, true) - iptPartialPodSpec := craftPartialIptEntrySpecFromSelector("", fromRule.PodSelector, util.IptablesSrcFlag, false) - iptPartialNsComment := craftPartialIptablesCommentFromSelector("", fromRule.NamespaceSelector, true) - iptPartialPodComment := craftPartialIptablesCommentFromSelector("", fromRule.PodSelector, false) - if portRuleExists { - for _, portRule := range rule.Ports { - switch portCheck := getPortType(portRule); portCheck { - case "namedport": - portName := util.NamedPortIPSetPrefix + portRule.Port.String() - namedPorts = append(namedPorts, portName) - entry := &iptm.IptEntry{ - Chain: util.IptablesAzureIngressPortChain, - Specs: append([]string(nil), iptPartialNsSpec...), - } - entry.Specs = append( - entry.Specs, - iptPartialPodSpec..., - ) - entry.Specs = append( - entry.Specs, - targetSelectorIptEntrySpec..., - ) - entry.Specs = append( - entry.Specs, - util.IptablesModuleFlag, - util.IptablesSetModuleFlag, - util.IptablesMatchSetFlag, - util.GetHashedName(portName), - util.IptablesDstFlag+","+util.IptablesDstFlag, - util.IptablesJumpFlag, - util.IptablesMark, - util.IptablesSetMarkFlag, - util.IptablesAzureIngressMarkHex, - util.IptablesModuleFlag, - util.IptablesCommentModuleFlag, - util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ - "-AND-"+iptPartialPodComment+ - "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ - "-TO-"+targetSelectorComment, - ) - entries = append(entries, entry) - case "validport": - entry := &iptm.IptEntry{ - Chain: util.IptablesAzureIngressPortChain, - Specs: append([]string(nil), iptPartialNsSpec...), - } - entry.Specs = append( - entry.Specs, - iptPartialPodSpec..., - ) - entry.Specs = append( - entry.Specs, - targetSelectorIptEntrySpec..., - ) - entry.Specs = append( - entry.Specs, - craftPartialIptEntrySpecFromPort(portRule, util.IptablesDstPortFlag)..., - ) - entry.Specs = append( - entry.Specs, - util.IptablesJumpFlag, - util.IptablesMark, - util.IptablesSetMarkFlag, - util.IptablesAzureIngressMarkHex, - util.IptablesModuleFlag, - util.IptablesCommentModuleFlag, - util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ - "-AND-"+iptPartialPodComment+ - "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ - "-TO-"+targetSelectorComment, - ) - entries = append(entries, entry) - default: - log.Logf("Invalid NetworkPolicyPort.") - } - } - } else { - entry := &iptm.IptEntry{ - Chain: util.IptablesAzureIngressFromChain, - Specs: append([]string(nil), targetSelectorIptEntrySpec...), - } - entry.Specs = append( - entry.Specs, - iptPartialNsSpec..., - ) - entry.Specs = append( - entry.Specs, - iptPartialPodSpec..., - ) - entry.Specs = append( - entry.Specs, - util.IptablesJumpFlag, - util.IptablesMark, - util.IptablesSetMarkFlag, - util.IptablesAzureIngressMarkHex, - util.IptablesModuleFlag, - util.IptablesCommentModuleFlag, - util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ - "-AND-"+iptPartialPodComment+ - "-TO-"+targetSelectorComment, - ) - entries = append(entries, entry) } } @@ -786,14 +852,14 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS } log.Logf("finished parsing ingress rule") - return util.DropEmptyFields(sets), util.DropEmptyFields(namedPorts), util.DropEmptyFields(lists), ipCidrs, entries + return util.DropEmptyFields(netHashIPsets), util.DropEmptyFields(namedPorts), listIPsets, ipCidrs, entries } -func translateEgress(ns string, policyName string, targetSelector metav1.LabelSelector, rules []networkingv1.NetworkPolicyEgressRule) ([]string, []string, []string, [][]string, []*iptm.IptEntry) { +func translateEgress(ns string, policyName string, targetSelector metav1.LabelSelector, rules []networkingv1.NetworkPolicyEgressRule) ([]string, []string, map[string][]string, [][]string, []*iptm.IptEntry) { var ( - sets []string // ipsets with type: net:hash - namedPorts []string // ipsets with type: hash:ip,port - lists []string // ipsets with type: list:set + netHashIPsets []string // ipsets with type: net:hash + namedPorts []string // ipsets with type: hash:ip,port + listIPsets map[string][]string // ipsets with type: list:set ipCidrs [][]string entries []*iptm.IptEntry toRuleEntries []*iptm.IptEntry @@ -802,14 +868,13 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe ) log.Logf("started parsing egress rule") - - labelsWithOps, _, _ := parseSelector(&targetSelector) - ops, labels := GetOperatorsAndLabels(labelsWithOps) - sets = append(sets, labels...) - sets = append(sets, "ns-"+ns) + netHashIPsets = append(netHashIPsets, "ns-"+ns) ipCidrs = make([][]string, len(rules)) + listIPsets = make(map[string][]string) - targetSelectorIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesSrcFlag, false) + targetSelectorIptEntrySpec, labels, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector(ns, &targetSelector, util.IptablesSrcFlag, false) + netHashIPsets = append(netHashIPsets, labels...) + appendSelectorLabelsToLists(listIPsets, listLabelsWithMembers, false) targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false) for i, rule := range rules { allowExternal := false @@ -860,7 +925,9 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe ) entries = append(entries, entry) - lists = append(lists, util.KubeAllNamespacesFlag) + if _, ok := listIPsets[util.KubeAllNamespacesFlag]; !ok { + listIPsets[util.KubeAllNamespacesFlag] = nil + } continue } @@ -1052,21 +1119,127 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe } if toRule.PodSelector == nil && toRule.NamespaceSelector != nil { - nsLabelsWithOps, _, _ := parseSelector(toRule.NamespaceSelector) - _, nsLabelsWithoutOps := GetOperatorsAndLabels(nsLabelsWithOps) - if len(nsLabelsWithoutOps) == 1 && nsLabelsWithoutOps[0] == "" { - // Empty namespaceSelector. This selects all namespaces - nsLabelsWithoutOps[0] = util.KubeAllNamespacesFlag - } else { - for i, _ := range nsLabelsWithoutOps { - // Add namespaces prefix to distinguish namespace ipset lists and pod ipsets - nsLabelsWithoutOps[i] = "ns-" + nsLabelsWithoutOps[i] + for _, nsSelector := range FlattenNameSpaceSelector(toRule.NamespaceSelector) { + iptPartialNsSpec, nsLabelsWithoutOps, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", &nsSelector, util.IptablesDstFlag, true) + if len(nsLabelsWithoutOps) == 1 && nsLabelsWithoutOps[0] == "" { + // Empty namespaceSelector. This selects all namespaces + nsLabelsWithoutOps[0] = util.KubeAllNamespacesFlag + if _, ok := listIPsets[nsLabelsWithoutOps[0]]; !ok { + listIPsets[nsLabelsWithoutOps[0]] = nil + } + } else { + for i := range nsLabelsWithoutOps { + // Add namespaces prefix to distinguish namespace ipset lists and pod ipsets + nsLabelsWithoutOps[i] = util.GetNSNameWithPrefix(nsLabelsWithoutOps[i]) + if _, ok := listIPsets[nsLabelsWithoutOps[i]]; !ok { + listIPsets[nsLabelsWithoutOps[i]] = nil + } + } + appendSelectorLabelsToLists(listIPsets, listLabelsWithMembers, true) + } + iptPartialNsComment := craftPartialIptablesCommentFromSelector("", &nsSelector, true) + if portRuleExists { + for _, portRule := range rule.Ports { + switch portCheck := getPortType(portRule); portCheck { + case "namedport": + portName := util.NamedPortIPSetPrefix + portRule.Port.String() + namedPorts = append(namedPorts, portName) + entry := &iptm.IptEntry{ + Chain: util.IptablesAzureEgressPortChain, + Specs: append([]string(nil), iptPartialNsSpec...), + } + entry.Specs = append( + entry.Specs, + targetSelectorIptEntrySpec..., + ) + entry.Specs = append( + entry.Specs, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName(portName), + util.IptablesDstFlag+","+util.IptablesDstFlag, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureEgressXMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-"+iptPartialNsComment+ + "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ + "-FROM-"+targetSelectorComment, + ) + entries = append(entries, entry) + case "validport": + entry := &iptm.IptEntry{ + Chain: util.IptablesAzureEgressPortChain, + Specs: append([]string(nil), iptPartialNsSpec...), + } + entry.Specs = append( + entry.Specs, + craftPartialIptEntrySpecFromPort(portRule, util.IptablesDstPortFlag)..., + ) + entry.Specs = append( + entry.Specs, + targetSelectorIptEntrySpec..., + ) + entry.Specs = append( + entry.Specs, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureEgressXMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-"+iptPartialNsComment+ + "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ + "-FROM-"+targetSelectorComment, + ) + entries = append(entries, entry) + default: + log.Logf("Invalid NetworkPolicyPort.") + } + } + } else { + entry := &iptm.IptEntry{ + Chain: util.IptablesAzureEgressToChain, + Specs: append([]string(nil), targetSelectorIptEntrySpec...), + } + entry.Specs = append( + entry.Specs, + iptPartialNsSpec..., + ) + entry.Specs = append( + entry.Specs, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureEgressXMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-"+targetSelectorComment+ + "-TO-"+iptPartialNsComment, + ) + entries = append(entries, entry) } } - lists = append(lists, nsLabelsWithoutOps...) + continue + } - iptPartialNsSpec := craftPartialIptEntrySpecFromSelector("", toRule.NamespaceSelector, util.IptablesDstFlag, true) - iptPartialNsComment := craftPartialIptablesCommentFromSelector("", toRule.NamespaceSelector, true) + if toRule.PodSelector != nil && toRule.NamespaceSelector == nil { + iptPartialPodSpec, podLabelsWithoutOps, listPodLabelsWithMembers := craftPartialIptEntrySpecFromSelector(ns, toRule.PodSelector, util.IptablesDstFlag, false) + if len(podLabelsWithoutOps) == 1 { + if podLabelsWithoutOps[0] == "" { + podLabelsWithoutOps[0] = util.GetNSNameWithPrefix(ns) + } + } + netHashIPsets = append(netHashIPsets, podLabelsWithoutOps...) + // TODO check if the ns- is needed here ? + appendSelectorLabelsToLists(listIPsets, listPodLabelsWithMembers, false) + iptPartialPodComment := craftPartialIptablesCommentFromSelector(ns, toRule.PodSelector, false) if portRuleExists { for _, portRule := range rule.Ports { switch portCheck := getPortType(portRule); portCheck { @@ -1075,7 +1248,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe namedPorts = append(namedPorts, portName) entry := &iptm.IptEntry{ Chain: util.IptablesAzureEgressPortChain, - Specs: append([]string(nil), iptPartialNsSpec...), + Specs: append([]string(nil), iptPartialPodSpec...), } entry.Specs = append( entry.Specs, @@ -1095,7 +1268,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ + "ALLOW-"+iptPartialPodComment+ "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ "-FROM-"+targetSelectorComment, ) @@ -1103,7 +1276,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe case "validport": entry := &iptm.IptEntry{ Chain: util.IptablesAzureEgressPortChain, - Specs: append([]string(nil), iptPartialNsSpec...), + Specs: append([]string(nil), iptPartialPodSpec...), } entry.Specs = append( entry.Specs, @@ -1122,7 +1295,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialNsComment+ + "ALLOW-"+iptPartialPodComment+ "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ "-FROM-"+targetSelectorComment, ) @@ -1138,7 +1311,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe } entry.Specs = append( entry.Specs, - iptPartialNsSpec..., + iptPartialPodSpec..., ) entry.Specs = append( entry.Specs, @@ -1150,25 +1323,36 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe util.IptablesCommentModuleFlag, util.IptablesCommentFlag, "ALLOW-"+targetSelectorComment+ - "-TO-"+iptPartialNsComment, + "-TO-"+iptPartialPodComment, ) entries = append(entries, entry) } continue } - if toRule.PodSelector != nil && toRule.NamespaceSelector == nil { - podLabelsWithOps, _, _ := parseSelector(toRule.PodSelector) - _, podLabelsWithoutOps := GetOperatorsAndLabels(podLabelsWithOps) - if len(podLabelsWithoutOps) == 1 { - if podLabelsWithoutOps[0] == "" { - podLabelsWithoutOps[0] = "ns-" + ns + // toRule has both namespaceSelector and podSelector set. + // We should match the selected pods in the selected namespaces. + // This allows traffic from podSelector intersects namespaceSelector + // This is only supported in kubernetes version >= 1.11 + if !util.IsNewNwPolicyVerFlag { + continue + } + for _, nsSelector := range FlattenNameSpaceSelector(toRule.NamespaceSelector) { + // we pass true for the podspec and comment here because it's a combo of both selectors and not limited to network policy namespace + iptPartialNsSpec, nsLabelsWithoutOps, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", &nsSelector, util.IptablesDstFlag, true) + // Add namespaces prefix to distinguish namespace ipsets and pod ipsets + for i := range nsLabelsWithoutOps { + nsLabelsWithoutOps[i] = "ns-" + nsLabelsWithoutOps[i] + if _, ok := listIPsets[nsLabelsWithoutOps[i]]; !ok { + listIPsets[nsLabelsWithoutOps[i]] = nil } } - sets = append(sets, podLabelsWithoutOps...) - - iptPartialPodSpec := craftPartialIptEntrySpecFromSelector(ns, toRule.PodSelector, util.IptablesDstFlag, false) - iptPartialPodComment := craftPartialIptablesCommentFromSelector(ns, toRule.PodSelector, false) + appendSelectorLabelsToLists(listIPsets, listLabelsWithMembers, true) + iptPartialPodSpec, podLabelsWithoutOps, listPodLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", toRule.PodSelector, util.IptablesDstFlag, false) + netHashIPsets = append(netHashIPsets, podLabelsWithoutOps...) + appendSelectorLabelsToLists(listIPsets, listPodLabelsWithMembers, false) + iptPartialNsComment := craftPartialIptablesCommentFromSelector("", &nsSelector, true) + iptPartialPodComment := craftPartialIptablesCommentFromSelector("", toRule.PodSelector, false) if portRuleExists { for _, portRule := range rule.Ports { switch portCheck := getPortType(portRule); portCheck { @@ -1177,11 +1361,15 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe namedPorts = append(namedPorts, portName) entry := &iptm.IptEntry{ Chain: util.IptablesAzureEgressPortChain, - Specs: append([]string(nil), iptPartialPodSpec...), + Specs: append([]string(nil), targetSelectorIptEntrySpec...), } entry.Specs = append( entry.Specs, - targetSelectorIptEntrySpec..., + iptPartialNsSpec..., + ) + entry.Specs = append( + entry.Specs, + iptPartialPodSpec..., ) entry.Specs = append( entry.Specs, @@ -1197,23 +1385,28 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialPodComment+ - "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ - "-FROM-"+targetSelectorComment, + "ALLOW-"+targetSelectorComment+ + "-TO-"+iptPartialNsComment+ + "-AND-"+iptPartialPodComment+ + "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag), ) entries = append(entries, entry) case "validport": entry := &iptm.IptEntry{ Chain: util.IptablesAzureEgressPortChain, - Specs: append([]string(nil), iptPartialPodSpec...), + Specs: append([]string(nil), targetSelectorIptEntrySpec...), } entry.Specs = append( entry.Specs, - craftPartialIptEntrySpecFromPort(portRule, util.IptablesDstPortFlag)..., + iptPartialNsSpec..., ) entry.Specs = append( entry.Specs, - targetSelectorIptEntrySpec..., + iptPartialPodSpec..., + ) + entry.Specs = append( + entry.Specs, + craftPartialIptEntrySpecFromPort(portRule, util.IptablesDstPortFlag)..., ) entry.Specs = append( entry.Specs, @@ -1224,9 +1417,10 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-"+iptPartialPodComment+ - "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag)+ - "-FROM-"+targetSelectorComment, + "ALLOW-"+targetSelectorComment+ + "-TO-"+iptPartialNsComment+ + "-AND-"+iptPartialPodComment+ + "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag), ) entries = append(entries, entry) default: @@ -1238,6 +1432,10 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe Chain: util.IptablesAzureEgressToChain, Specs: append([]string(nil), targetSelectorIptEntrySpec...), } + entry.Specs = append( + entry.Specs, + iptPartialNsSpec..., + ) entry.Specs = append( entry.Specs, iptPartialPodSpec..., @@ -1252,139 +1450,11 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe util.IptablesCommentModuleFlag, util.IptablesCommentFlag, "ALLOW-"+targetSelectorComment+ - "-TO-"+iptPartialPodComment, + "-TO-"+iptPartialNsComment+ + "-AND-"+iptPartialPodComment, ) entries = append(entries, entry) } - continue - } - - // toRule has both namespaceSelector and podSelector set. - // We should match the selected pods in the selected namespaces. - // This allows traffic from podSelector intersects namespaceSelector - // This is only supported in kubernetes version >= 1.11 - if !util.IsNewNwPolicyVerFlag { - continue - } - - nsLabelsWithOps, _, _ := parseSelector(toRule.NamespaceSelector) - _, nsLabelsWithoutOps := GetOperatorsAndLabels(nsLabelsWithOps) - // Add namespaces prefix to distinguish namespace ipsets and pod ipsets - for i, _ := range nsLabelsWithoutOps { - nsLabelsWithoutOps[i] = "ns-" + nsLabelsWithoutOps[i] - } - lists = append(lists, nsLabelsWithoutOps...) - - podLabelsWithOps, _, _ := parseSelector(toRule.PodSelector) - _, podLabelsWithoutOps := GetOperatorsAndLabels(podLabelsWithOps) - sets = append(sets, podLabelsWithoutOps...) - - // we pass true for the podspec and comment here because it's a combo of both selectors and not limited to network policy namespace - iptPartialNsSpec := craftPartialIptEntrySpecFromSelector("", toRule.NamespaceSelector, util.IptablesDstFlag, true) - iptPartialPodSpec := craftPartialIptEntrySpecFromSelector("", toRule.PodSelector, util.IptablesDstFlag, false) - iptPartialNsComment := craftPartialIptablesCommentFromSelector("", toRule.NamespaceSelector, true) - iptPartialPodComment := craftPartialIptablesCommentFromSelector("", toRule.PodSelector, false) - if portRuleExists { - for _, portRule := range rule.Ports { - switch portCheck := getPortType(portRule); portCheck { - case "namedport": - portName := util.NamedPortIPSetPrefix + portRule.Port.String() - namedPorts = append(namedPorts, portName) - entry := &iptm.IptEntry{ - Chain: util.IptablesAzureEgressPortChain, - Specs: append([]string(nil), targetSelectorIptEntrySpec...), - } - entry.Specs = append( - entry.Specs, - iptPartialNsSpec..., - ) - entry.Specs = append( - entry.Specs, - iptPartialPodSpec..., - ) - entry.Specs = append( - entry.Specs, - util.IptablesModuleFlag, - util.IptablesSetModuleFlag, - util.IptablesMatchSetFlag, - util.GetHashedName(portName), - util.IptablesDstFlag+","+util.IptablesDstFlag, - util.IptablesJumpFlag, - util.IptablesMark, - util.IptablesSetMarkFlag, - util.IptablesAzureEgressXMarkHex, - util.IptablesModuleFlag, - util.IptablesCommentModuleFlag, - util.IptablesCommentFlag, - "ALLOW-"+targetSelectorComment+ - "-TO-"+iptPartialNsComment+ - "-AND-"+iptPartialPodComment+ - "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag), - ) - entries = append(entries, entry) - case "validport": - entry := &iptm.IptEntry{ - Chain: util.IptablesAzureEgressPortChain, - Specs: append([]string(nil), targetSelectorIptEntrySpec...), - } - entry.Specs = append( - entry.Specs, - iptPartialNsSpec..., - ) - entry.Specs = append( - entry.Specs, - iptPartialPodSpec..., - ) - entry.Specs = append( - entry.Specs, - craftPartialIptEntrySpecFromPort(portRule, util.IptablesDstPortFlag)..., - ) - entry.Specs = append( - entry.Specs, - util.IptablesJumpFlag, - util.IptablesMark, - util.IptablesSetMarkFlag, - util.IptablesAzureEgressXMarkHex, - util.IptablesModuleFlag, - util.IptablesCommentModuleFlag, - util.IptablesCommentFlag, - "ALLOW-"+targetSelectorComment+ - "-TO-"+iptPartialNsComment+ - "-AND-"+iptPartialPodComment+ - "-AND-"+craftPartialIptablesCommentFromPort(portRule, util.IptablesDstPortFlag), - ) - entries = append(entries, entry) - default: - log.Logf("Invalid NetworkPolicyPort.") - } - } - } else { - entry := &iptm.IptEntry{ - Chain: util.IptablesAzureEgressToChain, - Specs: append([]string(nil), targetSelectorIptEntrySpec...), - } - entry.Specs = append( - entry.Specs, - iptPartialNsSpec..., - ) - entry.Specs = append( - entry.Specs, - iptPartialPodSpec..., - ) - entry.Specs = append( - entry.Specs, - util.IptablesJumpFlag, - util.IptablesMark, - util.IptablesSetMarkFlag, - util.IptablesAzureEgressXMarkHex, - util.IptablesModuleFlag, - util.IptablesCommentModuleFlag, - util.IptablesCommentFlag, - "ALLOW-"+targetSelectorComment+ - "-TO-"+iptPartialNsComment+ - "-AND-"+iptPartialPodComment, - ) - entries = append(entries, entry) } } @@ -1418,18 +1488,15 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe } log.Logf("finished parsing egress rule") - return util.DropEmptyFields(sets), util.DropEmptyFields(namedPorts), util.DropEmptyFields(lists), ipCidrs, entries + return util.DropEmptyFields(netHashIPsets), util.DropEmptyFields(namedPorts), listIPsets, ipCidrs, entries } // Drop all non-whitelisted packets. func getDefaultDropEntries(ns string, targetSelector metav1.LabelSelector, hasIngress, hasEgress bool) []*iptm.IptEntry { var entries []*iptm.IptEntry - labelsWithOps, _, _ := parseSelector(&targetSelector) - ops, labels := GetOperatorsAndLabels(labelsWithOps) - - targetSelectorIngressIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesDstFlag, false) - targetSelectorEgressIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesSrcFlag, false) + targetSelectorIngressIptEntrySpec, _, _ := craftPartialIptEntrySpecFromSelector(ns, &targetSelector, util.IptablesDstFlag, false) + targetSelectorEgressIptEntrySpec, _, _ := craftPartialIptEntrySpecFromSelector(ns, &targetSelector, util.IptablesSrcFlag, false) targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false) if hasIngress { @@ -1476,11 +1543,11 @@ func getDefaultDropEntries(ns string, targetSelector metav1.LabelSelector, hasIn // 1. ipset set names generated from all podSelectors // 2. ipset list names generated from all namespaceSelectors // 3. iptables entries generated from the input network policy object. -func translatePolicy(npObj *networkingv1.NetworkPolicy) ([]string, []string, []string, [][]string, [][]string, []*iptm.IptEntry) { +func translatePolicy(npObj *networkingv1.NetworkPolicy) ([]string, []string, map[string][]string, [][]string, [][]string, []*iptm.IptEntry) { var ( resultSets []string resultNamedPorts []string - resultLists []string + resultListMap map[string][]string resultIngressIPCidrs [][]string resultEgressIPCidrs [][]string entries []*iptm.IptEntry @@ -1490,7 +1557,7 @@ func translatePolicy(npObj *networkingv1.NetworkPolicy) ([]string, []string, []s defer func() { log.Logf("Finished translatePolicy") log.Logf("sets: %v", resultSets) - log.Logf("lists: %v", resultLists) + log.Logf("lists: %v", resultListMap) log.Logf("entries: ") for _, entry := range entries { log.Logf("entry: %+v", entry) @@ -1499,25 +1566,36 @@ func translatePolicy(npObj *networkingv1.NetworkPolicy) ([]string, []string, []s npNs := npObj.ObjectMeta.Namespace policyName := npObj.ObjectMeta.Name + resultListMap = make(map[string][]string) + // Since nested ipset list:sets are not allowed. We cannot use 2nd level Ipsets + // for NameSpaceSelectors with multiple values + // NPM will need to duplicate rules for each value in NSSelector if len(npObj.Spec.PolicyTypes) == 0 { ingressSets, ingressNamedPorts, ingressLists, ingressIPCidrs, ingressEntries := translateIngress(npNs, policyName, npObj.Spec.PodSelector, npObj.Spec.Ingress) resultSets = append(resultSets, ingressSets...) resultNamedPorts = append(resultNamedPorts, ingressNamedPorts...) - resultLists = append(resultLists, ingressLists...) + for resultListKey, resultLists := range ingressLists { + resultListMap[resultListKey] = append(resultListMap[resultListKey], resultLists...) + } entries = append(entries, ingressEntries...) egressSets, egressNamedPorts, egressLists, egressIPCidrs, egressEntries := translateEgress(npNs, policyName, npObj.Spec.PodSelector, npObj.Spec.Egress) resultSets = append(resultSets, egressSets...) resultNamedPorts = append(resultNamedPorts, egressNamedPorts...) - resultLists = append(resultLists, egressLists...) + for resultListKey, resultLists := range egressLists { + resultListMap[resultListKey] = append(resultListMap[resultListKey], resultLists...) + } entries = append(entries, egressEntries...) hasIngress = len(ingressSets) > 0 hasEgress = len(egressSets) > 0 entries = append(entries, getDefaultDropEntries(npNs, npObj.Spec.PodSelector, hasIngress, hasEgress)...) + for resultListKey, resultLists := range resultListMap { + resultListMap[resultListKey] = util.UniqueStrSlice(resultLists) + } - return util.UniqueStrSlice(resultSets), util.UniqueStrSlice(resultNamedPorts), util.UniqueStrSlice(resultLists), ingressIPCidrs, egressIPCidrs, entries + return util.UniqueStrSlice(resultSets), util.UniqueStrSlice(resultNamedPorts), resultListMap, ingressIPCidrs, egressIPCidrs, entries } for _, ptype := range npObj.Spec.PolicyTypes { @@ -1525,7 +1603,9 @@ func translatePolicy(npObj *networkingv1.NetworkPolicy) ([]string, []string, []s ingressSets, ingressNamedPorts, ingressLists, ingressIPCidrs, ingressEntries := translateIngress(npNs, policyName, npObj.Spec.PodSelector, npObj.Spec.Ingress) resultSets = append(resultSets, ingressSets...) resultNamedPorts = append(resultNamedPorts, ingressNamedPorts...) - resultLists = append(resultLists, ingressLists...) + for resultListKey, resultLists := range ingressLists { + resultListMap[resultListKey] = append(resultListMap[resultListKey], resultLists...) + } resultIngressIPCidrs = ingressIPCidrs entries = append(entries, ingressEntries...) @@ -1543,7 +1623,9 @@ func translatePolicy(npObj *networkingv1.NetworkPolicy) ([]string, []string, []s egressSets, egressNamedPorts, egressLists, egressIPCidrs, egressEntries := translateEgress(npNs, policyName, npObj.Spec.PodSelector, npObj.Spec.Egress) resultSets = append(resultSets, egressSets...) resultNamedPorts = append(resultNamedPorts, egressNamedPorts...) - resultLists = append(resultLists, egressLists...) + for resultListKey, resultLists := range egressLists { + resultListMap[resultListKey] = append(resultListMap[resultListKey], resultLists...) + } resultEgressIPCidrs = egressIPCidrs entries = append(entries, egressEntries...) @@ -1559,7 +1641,10 @@ func translatePolicy(npObj *networkingv1.NetworkPolicy) ([]string, []string, []s } entries = append(entries, getDefaultDropEntries(npNs, npObj.Spec.PodSelector, hasIngress, hasEgress)...) - resultSets, resultLists = util.UniqueStrSlice(resultSets), util.UniqueStrSlice(resultLists) + resultSets = util.UniqueStrSlice(resultSets) + for resultListKey, resultLists := range resultListMap { + resultListMap[resultListKey] = util.UniqueStrSlice(resultLists) + } - return resultSets, resultNamedPorts, resultLists, resultIngressIPCidrs, resultEgressIPCidrs, entries + return resultSets, resultNamedPorts, resultListMap, resultIngressIPCidrs, resultEgressIPCidrs, entries } diff --git a/npm/translatePolicy_test.go b/npm/translatePolicy_test.go index 589097357f..27d16018a4 100644 --- a/npm/translatePolicy_test.go +++ b/npm/translatePolicy_test.go @@ -266,7 +266,8 @@ func TestCraftPartialIptEntryFromSelector(t *testing.T) { }, } - iptEntrySpec := craftPartialIptEntrySpecFromSelector("testnamespace", srcSelector, util.IptablesSrcFlag, false) + // TODO add more test cases here form multi value + iptEntrySpec, _, _ := craftPartialIptEntrySpecFromSelector("testnamespace", srcSelector, util.IptablesSrcFlag, false) expectedIptEntrySpec := []string{ util.IptablesModuleFlag, util.IptablesSetModuleFlag, @@ -342,7 +343,7 @@ func TestCraftPartialIptablesCommentFromSelector(t *testing.T) { }, } comment = craftPartialIptablesCommentFromSelector("testnamespace", selector, false) - expectedComment = "k0:v0-AND-k1:v10-AND-k1:v11-AND-!k2-IN-ns-testnamespace" + expectedComment = "k0:v0-AND-!k2-AND-k1:v10:v11-IN-ns-testnamespace" if comment != expectedComment { t.Errorf("TestCraftPartialIptablesCommentFromSelector failed @ normal selector comparison") t.Errorf("comment:\n%v", comment) @@ -370,7 +371,7 @@ func TestCraftPartialIptablesCommentFromSelector(t *testing.T) { }, } comment = craftPartialIptablesCommentFromSelector("", nsSelector, true) - expectedComment = "ns-k0:v0-AND-ns-k1:v10-AND-ns-k1:v11-AND-ns-!k2" + expectedComment = "ns-k0:v0-AND-ns-!k2-AND-ns-k1:v10:v11" if comment != expectedComment { t.Errorf("TestCraftPartialIptablesCommentFromSelector failed @ namespace selector comparison") t.Errorf("comment:\n%v", comment) @@ -651,17 +652,17 @@ func TestTranslateIngress(t *testing.T) { "k", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedIngress failed @ sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{ - "ns-ns:dev", - "ns-testIn:frontendns", - "ns-planet:earth", - "ns-keyExists", + expectedLists := map[string][]string{ + "ns-ns:dev": nil, + "ns-testIn:frontendns": nil, + "ns-planet:earth": nil, + "ns-keyExists": nil, } if !reflect.DeepEqual(lists, expectedLists) { @@ -934,17 +935,17 @@ func TestTranslateEgress(t *testing.T) { "k", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedEgress failed @ sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{ - "ns-ns:dev", - "ns-testIn:frontendns", - "ns-planet:earth", - "ns-keyExists", + expectedLists := map[string][]string{ + "ns-ns:dev": nil, + "ns-testIn:frontendns": nil, + "ns-planet:earth": nil, + "ns-keyExists": nil, } if !reflect.DeepEqual(lists, expectedLists) { @@ -1132,13 +1133,13 @@ func TestDenyAllPolicy(t *testing.T) { sets, _, lists, _, _, iptEntries := translatePolicy(denyAllPolicy) expectedSets := []string{"ns-testnamespace"} - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ deny-all-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ deny-all-policy lists comparison") t.Errorf("lists: %v", lists) @@ -1168,13 +1169,13 @@ func TestAllowBackendToFrontend(t *testing.T) { "ns-testnamespace", "app:frontend", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -1261,13 +1262,13 @@ func TestAllowAllToAppFrontend(t *testing.T) { "app:frontend", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -1324,13 +1325,13 @@ func TestDenyAllToAppFrontend(t *testing.T) { "app:frontend", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-none-TO-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-none-TO-app:frontend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -1360,13 +1361,13 @@ func TestNamespaceToFrontend(t *testing.T) { "app:frontend", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-ns-testnamespace-TO-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-ns-testnamespace-TO-app:frontend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -1447,14 +1448,14 @@ func TestAllowAllNamespacesToAppFrontend(t *testing.T) { "app:frontend", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-all-namespaces-TO-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{ - util.KubeAllNamespacesFlag, + expectedLists := map[string][]string{ + util.KubeAllNamespacesFlag: {}, } if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-all-namespaces-TO-app:frontend-policy lists comparison") @@ -1537,19 +1538,19 @@ func TestAllowNamespaceDevToAppFrontend(t *testing.T) { "app:frontend", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { - t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0-AND-!ns-namespace:test1-TO-app:frontend-policy sets comparison") + if !util.CompareSlices(sets, expectedSets) { + t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0:test1-TO-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{ - "ns-namespace:dev", - "ns-namespace:test0", - "ns-namespace:test1", + expectedLists := map[string][]string{ + "ns-namespace:dev": {}, + "ns-namespace:test0": {}, + "ns-namespace:test1": {}, } if !reflect.DeepEqual(lists, expectedLists) { - t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0-AND-!ns-namespace:test1-TO-app:frontend-policy lists comparison") + t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0:test1-TO-app:frontend-policy lists comparison") t.Errorf("lists: %v", lists) t.Errorf("expectedLists: %v", expectedLists) } @@ -1572,6 +1573,34 @@ func TestAllowNamespaceDevToAppFrontend(t *testing.T) { util.IptablesSrcFlag, util.IptablesModuleFlag, util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("ns-testnamespace"), + util.IptablesDstFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("app:frontend"), + util.IptablesDstFlag, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureIngressMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-ns-namespace:dev-AND-ns-!namespace:test0-TO-app:frontend-IN-ns-testnamespace", + }, + }, + &iptm.IptEntry{ + Chain: util.IptablesAzureIngressFromChain, + Specs: []string{ + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("ns-namespace:dev"), + util.IptablesSrcFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, util.IptablesNotFlag, util.IptablesMatchSetFlag, util.GetHashedName("ns-namespace:test1"), @@ -1593,7 +1622,7 @@ func TestAllowNamespaceDevToAppFrontend(t *testing.T) { util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-ns-namespace:dev-AND-ns-!namespace:test0-AND-ns-!namespace:test1-TO-app:frontend-IN-ns-testnamespace", + "ALLOW-ns-namespace:dev-AND-ns-!namespace:test1-TO-app:frontend-IN-ns-testnamespace", }, }, &iptm.IptEntry{ @@ -1622,7 +1651,7 @@ func TestAllowNamespaceDevToAppFrontend(t *testing.T) { expectedIptEntries = append(expectedIptEntries, nonKubeSystemEntries...) expectedIptEntries = append(expectedIptEntries, getDefaultDropEntries("testnamespace", allowNsDevToFrontendPolicy.Spec.PodSelector, false, false)...) if !reflect.DeepEqual(iptEntries, expectedIptEntries) { - t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0-AND-!ns-namespace:test1-TO-app:frontend-policy policy comparison") + t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0:test1-TO-app:frontend-policy policy comparison") marshalledIptEntries, _ := json.Marshal(iptEntries) marshalledExpectedIptEntries, _ := json.Marshal(expectedIptEntries) t.Errorf("iptEntries: %s", marshalledIptEntries) @@ -1645,15 +1674,21 @@ func TestAllowAllToK0AndK1AndAppFrontend(t *testing.T) { "k1:v1", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ AllOW-ALL-TO-k0-AND-k1:v0-AND-k1:v1-AND-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{util.KubeAllNamespacesFlag} + expectedLists := map[string][]string{ + util.KubeAllNamespacesFlag: {}, + "k1:v0:v1": { + "k1:v0", + "k1:v1", + }, + } if !reflect.DeepEqual(lists, expectedLists) { - t.Errorf("translatedPolicy failed @ AllOW-ALL-TO-k0-AND-k1:v0-AND-k1:v1-AND-app:frontend-policy lists comparison") + t.Errorf("translatedPolicy failed @ AllOW-ALL-TO-k0-AND-k1:v0:v1-AND-app:frontend-policy lists comparison") t.Errorf("lists: %v", lists) t.Errorf("expectedLists: %v", expectedLists) } @@ -1687,12 +1722,7 @@ func TestAllowAllToK0AndK1AndAppFrontend(t *testing.T) { util.IptablesModuleFlag, util.IptablesSetModuleFlag, util.IptablesMatchSetFlag, - util.GetHashedName("k1:v0"), - util.IptablesDstFlag, - util.IptablesModuleFlag, - util.IptablesSetModuleFlag, - util.IptablesMatchSetFlag, - util.GetHashedName("k1:v1"), + util.GetHashedName("k1:v0:v1"), util.IptablesDstFlag, util.IptablesJumpFlag, util.IptablesMark, @@ -1701,7 +1731,7 @@ func TestAllowAllToK0AndK1AndAppFrontend(t *testing.T) { util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-all-namespaces-TO-app:frontend-AND-!k0-AND-k1:v0-AND-k1:v1-IN-ns-testnamespace", + "ALLOW-all-namespaces-TO-app:frontend-AND-!k0-AND-k1:v0:v1-IN-ns-testnamespace", }, }, &iptm.IptEntry{ @@ -1726,19 +1756,14 @@ func TestAllowAllToK0AndK1AndAppFrontend(t *testing.T) { util.IptablesModuleFlag, util.IptablesSetModuleFlag, util.IptablesMatchSetFlag, - util.GetHashedName("k1:v0"), - util.IptablesDstFlag, - util.IptablesModuleFlag, - util.IptablesSetModuleFlag, - util.IptablesMatchSetFlag, - util.GetHashedName("k1:v1"), + util.GetHashedName("k1:v0:v1"), util.IptablesDstFlag, util.IptablesJumpFlag, util.IptablesDrop, util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "DROP-ALL-TO-app:frontend-AND-!k0-AND-k1:v0-AND-k1:v1-IN-ns-testnamespace", + "DROP-ALL-TO-app:frontend-AND-!k0-AND-k1:v0:v1-IN-ns-testnamespace", }, }, } @@ -1746,7 +1771,7 @@ func TestAllowAllToK0AndK1AndAppFrontend(t *testing.T) { expectedIptEntries = append(expectedIptEntries, nonKubeSystemEntries...) expectedIptEntries = append(expectedIptEntries, getDefaultDropEntries("testnamespace", allowAllToFrontendPolicy.Spec.PodSelector, false, false)...) if !reflect.DeepEqual(iptEntries, expectedIptEntries) { - t.Errorf("translatedPolicy failed @ AllOW-all-TO-k0-AND-k1:v0-AND-k1:v1-AND-app:frontend-policy policy comparison") + t.Errorf("translatedPolicy failed @ AllOW-all-TO-k0-AND-k1:v0:v1-AND-app:frontend-policy policy comparison") marshalledIptEntries, _ := json.Marshal(iptEntries) marshalledExpectedIptEntries, _ := json.Marshal(expectedIptEntries) t.Errorf("iptEntries: %s", marshalledIptEntries) @@ -1768,14 +1793,14 @@ func TestAllowNsDevAndAppBackendToAppFrontend(t *testing.T) { "ns-testnamespace", "app:backend", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-ns-ns:dev-AND-app:backend-TO-app:frontend sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{ - "ns-ns:dev", + expectedLists := map[string][]string{ + "ns-ns:dev": {}, } if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-ns-ns:dev-AND-app:backend-TO-app:frontend lists comparison") @@ -1864,13 +1889,13 @@ func TestAllowInternalAndExternal(t *testing.T) { "app:backdoor", "ns-dangerous", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-ALL-TO-app:backdoor-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-ALL-TO-app:backdoor-policy lists comparison") t.Errorf("lists: %v", lists) @@ -1928,13 +1953,13 @@ func TestAllowBackendToFrontendPort8000(t *testing.T) { "ns-testnamespace", "app:backend", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-port-8000-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-port-8000-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2025,13 +2050,13 @@ func TestAllowBackendToFrontendWithMissingPort(t *testing.T) { "ns-testnamespace", "app:backend", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-port-8000-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-port-8000-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2124,13 +2149,13 @@ func TestAllowMultipleLabelsToMultipleLabels(t *testing.T) { "binary:cns", "group:container", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-program:cni-AND-team:acn-OR-binary:cns-AND-group:container-TO-app:k8s-AND-team:aks-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-program:cni-AND-team:acn-OR-binary:cns-AND-group:container-TO-app:k8s-AND-team:aks-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2276,13 +2301,13 @@ func TestDenyAllFromAppBackend(t *testing.T) { "app:backend", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-none-FROM-app:backend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-none-FROM-app:backend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2309,16 +2334,16 @@ func TestAllowAllFromAppBackend(t *testing.T) { sets, _, lists, _, _, iptEntries := translatePolicy(allowAllEgress) expectedSets := []string{ - "app:backend", "ns-testnamespace", + "app:backend", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-all-FROM-app:backend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-all-FROM-app:backend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2363,6 +2388,146 @@ func TestAllowAllFromAppBackend(t *testing.T) { } } +func TestAllowMultiplePodSelectors(t *testing.T) { + multiPodSlector, err := readPolicyYaml("testpolicies/allow-ns-y-z-pod-b-c.yaml") + if err != nil { + t.Fatal(err) + } + + util.IsNewNwPolicyVerFlag = true + + sets, _, lists, _, _, iptEntries := translatePolicy(multiPodSlector) + + expectedSets := []string{ + "ns-netpol-4537-x", + "pod:a", + "pod:x", + "pod:b", + "pod:c", + "app:test", + "app:int", + } + if !util.CompareSlices(sets, expectedSets) { + t.Errorf("translatedPolicy failed @ allow-ns-y-z-pod-b-c sets comparison") + t.Errorf("sets: %v", sets) + t.Errorf("expectedSets: %v", expectedSets) + } + + expectedLists := map[string][]string{ + "app:test:int": { + "app:test", + "app:int", + }, + "ns-ns:netpol-4537-x": {}, + "ns-ns:netpol-4537-y": {}, + "pod:a:x": { + "pod:a", + "pod:x", + }, + "pod:b:c": { + "pod:b", + "pod:c", + }, + } + if !reflect.DeepEqual(lists, expectedLists) { + t.Errorf("translatedPolicy failed @ allow-ns-y-z-pod-b-c lists comparison") + t.Errorf("lists: %v", lists) + t.Errorf("expectedLists: %v", expectedLists) + } + + expectedIptEntries := []*iptm.IptEntry{} + nonKubeSystemEntries := []*iptm.IptEntry{ + &iptm.IptEntry{ + Chain: util.IptablesAzureIngressFromChain, + Specs: []string{ + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("ns-netpol-4537-x"), + util.IptablesDstFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("pod:a:x"), + util.IptablesDstFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesNotFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("ns-ns:netpol-4537-x"), + util.IptablesSrcFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("pod:b:c"), + util.IptablesSrcFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("app:test:int"), + util.IptablesSrcFlag, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureIngressMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-ns-!ns:netpol-4537-x-AND-pod:b:c-AND-app:test:int-TO-pod:a:x-IN-ns-netpol-4537-x", + }, + }, + &iptm.IptEntry{ + Chain: util.IptablesAzureIngressFromChain, + Specs: []string{ + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("ns-netpol-4537-x"), + util.IptablesDstFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("pod:a:x"), + util.IptablesDstFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesNotFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("ns-ns:netpol-4537-y"), + util.IptablesSrcFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("pod:b:c"), + util.IptablesSrcFlag, + util.IptablesModuleFlag, + util.IptablesSetModuleFlag, + util.IptablesMatchSetFlag, + util.GetHashedName("app:test:int"), + util.IptablesSrcFlag, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureIngressMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-ns-!ns:netpol-4537-y-AND-pod:b:c-AND-app:test:int-TO-pod:a:x-IN-ns-netpol-4537-x", + }, + }, + } + expectedIptEntries = append(expectedIptEntries, nonKubeSystemEntries...) + // has egress, but empty map means allow all + expectedIptEntries = append(expectedIptEntries, getDefaultDropEntries("netpol-4537-x", multiPodSlector.Spec.PodSelector, true, false)...) + if !reflect.DeepEqual(iptEntries, expectedIptEntries) { + t.Errorf("translatedPolicy failed @ allow-ns-y-z-pod-b-c policy comparison") + marshalledIptEntries, _ := json.Marshal(iptEntries) + marshalledExpectedIptEntries, _ := json.Marshal(expectedIptEntries) + t.Errorf("iptEntries: %s", marshalledIptEntries) + t.Errorf("expectedIptEntries: %s", marshalledExpectedIptEntries) + } +} + func TestDenyAllFromNsUnsafe(t *testing.T) { denyAllFromNsUnsafePolicy, err := readPolicyYaml("testpolicies/deny-all-from-ns-unsafe.yaml") if err != nil { @@ -2373,12 +2538,12 @@ func TestDenyAllFromNsUnsafe(t *testing.T) { expectedSets := []string{ "ns-unsafe", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-none-FROM-ns-unsafe-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-none-FROM-app:backend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2408,14 +2573,14 @@ func TestAllowAppFrontendToTCPPort53UDPPort53Policy(t *testing.T) { "app:frontend", "ns-testnamespace", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-ALL-FROM-app:frontend-TCP-PORT-53-OR-UDP-PORT-53-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{ - util.KubeAllNamespacesFlag, + expectedLists := map[string][]string{ + util.KubeAllNamespacesFlag: {}, } if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-ALL-FROM-app:frontend-TCP-PORT-53-OR-UDP-PORT-53-policy lists comparison") @@ -2556,14 +2721,14 @@ func TestComplexPolicy(t *testing.T) { "ns-default", "role:frontend", } - if !reflect.DeepEqual(sets, expectedSets) || !reflect.DeepEqual(setsDiffOrder, expectedSets) { + if !util.CompareSlices(sets, expectedSets) || !util.CompareSlices(setsDiffOrder, expectedSets) { t.Errorf("translatedPolicy failed @ k8s-example-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{ - "ns-project:myproject", + expectedLists := map[string][]string{ + "ns-project:myproject": {}, } if !reflect.DeepEqual(lists, expectedLists) || !reflect.DeepEqual(listsDiffOrder, expectedLists) { t.Errorf("translatedPolicy failed @ k8s-example-policy lists comparison") @@ -2877,13 +3042,13 @@ func TestDropPrecedenceOverAllow(t *testing.T) { expectedSets := []string{ "ns-default", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ k8s-example-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ k8s-example-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2898,14 +3063,14 @@ func TestDropPrecedenceOverAllow(t *testing.T) { "testIn:pod-B", "testIn:pod-C", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ k8s-example-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists = []string{ - "all-namespaces", + expectedLists = map[string][]string{ + "all-namespaces": {}, } if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ k8s-example-policy lists comparison") @@ -3131,7 +3296,7 @@ func TestNamedPorts(t *testing.T) { "app:server", "ns-test", } - if !reflect.DeepEqual(sets, expectedSets) { + if !util.CompareSlices(sets, expectedSets) { t.Errorf("translatedPolicy failed @ ALLOW-ALL-TCP-PORT-serve-80-TO-app:server-IN-ns-test-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) @@ -3146,7 +3311,7 @@ func TestNamedPorts(t *testing.T) { t.Errorf("expectedSets: %v", expectedNamedPorts) } - expectedLists := []string{} + expectedLists := make(map[string][]string) if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-ALL-TCP-PORT-serve-80-TO-app:server-IN-ns-test-policy lists comparison") t.Errorf("lists: %v", lists) diff --git a/npm/util/util.go b/npm/util/util.go index bb73365eea..09d7b7d0d1 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -322,3 +322,22 @@ func GetLabelKVFromSet(ipsetName string) (string, string) { } return strSplit[0], "" } + +// StrExistsInSlice check if a string already exists in a given slice +func StrExistsInSlice(items []string, val string) bool { + for _, item := range items { + if item == val { + return true + } + } + return false +} + +func CompareSlices(list1, list2 []string) bool { + for _, item := range list1 { + if !StrExistsInSlice(list2, item) { + return false + } + } + return true +} diff --git a/npm/util/util_test.go b/npm/util/util_test.go index 0452d2b976..d1b5b51bfb 100644 --- a/npm/util/util_test.go +++ b/npm/util/util_test.go @@ -270,3 +270,59 @@ func TestParseResourceVersion(t *testing.T) { t.Errorf("TestParseResourceVersion failed @ inavlid RV gave no error") } } + +func TestCompareSlices(t *testing.T) { + list1 := []string{ + "a", + "b", + "c", + "d", + } + list2 := []string{ + "c", + "d", + "a", + "b", + } + + if !CompareSlices(list1, list2) { + t.Errorf("TestCompareSlices failed @ slice comparison 1") + } + + list2 = []string{ + "c", + "a", + "b", + } + + if CompareSlices(list1, list2) { + t.Errorf("TestCompareSlices failed @ slice comparison 2") + } + list1 = []string{ + "a", + "b", + "c", + "d", + "123", + "44", + } + list2 = []string{ + "c", + "44", + "d", + "a", + "b", + "123", + } + + if !CompareSlices(list1, list2) { + t.Errorf("TestCompareSlices failed @ slice comparison 3") + } + + list1 = []string{} + list2 = []string{} + + if !CompareSlices(list1, list2) { + t.Errorf("TestCompareSlices failed @ slice comparison 4") + } +}