From 319cc6aa7ce76529c3e8514d07272841c8d95703 Mon Sep 17 00:00:00 2001 From: Jaeryn Date: Fri, 20 Dec 2019 23:24:11 +0000 Subject: [PATCH] We need to allow external instead of all-namespaces when ingress/egress rules only contain {} --- npm/translatePolicy.go | 14 ++++++++---- npm/translatePolicy_test.go | 43 +++++++++++++++++++------------------ 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/npm/translatePolicy.go b/npm/translatePolicy.go index 3c93850333..7c04a69638 100644 --- a/npm/translatePolicy.go +++ b/npm/translatePolicy.go @@ -162,8 +162,9 @@ func translateIngress(ns string, targetSelector metav1.LabelSelector, rules []ne targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false) for _, rule := range rules { - allowExternal, portRuleExists, fromRuleExists := false, false, false - portRuleExists = rule.Ports != nil && len(rule.Ports) > 0 + allowExternal := false + portRuleExists := rule.Ports != nil && len(rule.Ports) > 0 + fromRuleExists := false addedPortEntry = addedPortEntry || portRuleExists if rule.From != nil { @@ -180,6 +181,8 @@ func translateIngress(ns string, targetSelector metav1.LabelSelector, rules []ne break } } + } else if !portRuleExists { + allowExternal = true } if !portRuleExists && !fromRuleExists && !allowExternal { @@ -644,8 +647,9 @@ func translateEgress(ns string, targetSelector metav1.LabelSelector, rules []net targetSelectorIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesSrcFlag, false) targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false) for _, rule := range rules { - allowExternal, portRuleExists, toRuleExists := false, false, false - portRuleExists = rule.Ports != nil && len(rule.Ports) > 0 + allowExternal := false + portRuleExists := rule.Ports != nil && len(rule.Ports) > 0 + toRuleExists := false addedPortEntry = addedPortEntry || portRuleExists if rule.To != nil { @@ -662,6 +666,8 @@ func translateEgress(ns string, targetSelector metav1.LabelSelector, rules []net break } } + } else if !portRuleExists { + allowExternal = true } if !portRuleExists && !toRuleExists && !allowExternal { diff --git a/npm/translatePolicy_test.go b/npm/translatePolicy_test.go index ba8f839322..e0e3cfd332 100644 --- a/npm/translatePolicy_test.go +++ b/npm/translatePolicy_test.go @@ -1248,7 +1248,7 @@ func TestTranslatePolicy(t *testing.T) { } allowToFrontendPolicy := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ - Name: "ALLOW-all-TO-app:frontend-FROM-all-namespaces-policy", + Name: "ALLOW-all-TO-app:frontend-policy", Namespace: "testnamespace", }, Spec: networkingv1.NetworkPolicySpec{ @@ -1269,16 +1269,14 @@ func TestTranslatePolicy(t *testing.T) { "ns-testnamespace", } if !reflect.DeepEqual(sets, expectedSets) { - t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-FROM-all-namespaces-policy sets comparison") + t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-policy sets comparison") t.Errorf("sets: %v", sets) t.Errorf("expectedSets: %v", expectedSets) } - expectedLists = []string{ - util.KubeAllNamespacesFlag, - } + expectedLists = []string{} if !reflect.DeepEqual(lists, expectedLists) { - t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-FROM-all-namespaces-policy lists comparison") + t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-policy lists comparison") t.Errorf("lists: %v", lists) t.Errorf("expectedLists: %v", expectedLists) } @@ -1289,11 +1287,6 @@ func TestTranslatePolicy(t *testing.T) { &iptm.IptEntry{ Chain: util.IptablesAzureIngressPortChain, Specs: []string{ - util.IptablesModuleFlag, - util.IptablesSetModuleFlag, - util.IptablesMatchSetFlag, - util.GetHashedName(util.KubeAllNamespacesFlag), - util.IptablesSrcFlag, util.IptablesModuleFlag, util.IptablesSetModuleFlag, util.IptablesMatchSetFlag, @@ -1304,14 +1297,14 @@ func TestTranslatePolicy(t *testing.T) { util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-ALL-TO-app:frontend-FROM-all-namespaces", + "ALLOW-ALL-TO-app:frontend", }, }, } expectedIptEntries = append(expectedIptEntries, nonKubeSystemEntries...) expectedIptEntries = append(expectedIptEntries, getDefaultDropEntries("testnamespace", targetSelector, false, false)...) if !reflect.DeepEqual(iptEntries, expectedIptEntries) { - t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-FROM-all-namespaces-policy policy comparison") + t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-policy policy comparison") marshalledIptEntries, _ := json.Marshal(iptEntries) marshalledExpectedIptEntries, _ := json.Marshal(expectedIptEntries) t.Errorf("iptEntries: %s", marshalledIptEntries) @@ -2633,9 +2626,7 @@ func TestTranslatePolicy(t *testing.T) { t.Errorf("expectedSets: %v", expectedSets) } - expectedLists = []string{ - util.KubeAllNamespacesFlag, - } + expectedLists = []string{} if !reflect.DeepEqual(lists, expectedLists) { t.Errorf("translatedPolicy failed @ ALLOW-all-FROM-app:backend-policy lists comparison") t.Errorf("lists: %v", lists) @@ -2652,18 +2643,28 @@ func TestTranslatePolicy(t *testing.T) { util.IptablesMatchSetFlag, util.GetHashedName("app:backend"), util.IptablesSrcFlag, + util.IptablesJumpFlag, + util.IptablesAccept, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "ALLOW-ALL-FROM-app:backend", + }, + }, + &iptm.IptEntry{ + Chain: util.IptablesAzureEgressPortChain, + Specs: []string{ util.IptablesModuleFlag, util.IptablesSetModuleFlag, util.IptablesMatchSetFlag, - util.GetHashedName(util.KubeAllNamespacesFlag), - util.IptablesDstFlag, + util.GetHashedName("app:backend"), + util.IptablesSrcFlag, util.IptablesJumpFlag, - util.IptablesAccept, + util.IptablesAzureTargetSetsChain, util.IptablesModuleFlag, util.IptablesCommentModuleFlag, util.IptablesCommentFlag, - "ALLOW-ALL-FROM-app:backend-TO-" + - util.KubeAllNamespacesFlag, + "ALLOW-ALL-FROM-app:backend-TO-JUMP-TO-AZURE-NPM-TARGET-SETS", }, }, }