From 9851e2ef07a49aeeaf37652ffe0a5ed91aa5a4e9 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 8 Aug 2022 09:01:31 -0700 Subject: [PATCH 1/2] validation to prevent cidr except in windows --- .../translation/translatePolicy.go | 33 +++++++++++++------ .../translation/translatePolicy_test.go | 10 +++--- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index bfc52fff38..bd8a60adca 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -24,6 +24,8 @@ var ( ErrUnsupportedNamedPort = errors.New("unsupported namedport translation features used on windows") // ErrUnsupportedNegativeMatch is returned when negative match translation feature is used in windows. ErrUnsupportedNegativeMatch = errors.New("unsupported NotExist operator translation features used on windows") + // ErrUnsupportedExceptCIDR is returned when Except CIDR block translation feature is used in windows. + ErrUnsupportedExceptCIDR = errors.New("unsupported Except CIDR block translation features used on windows") // ErrUnsupportedSCTP is returned when SCTP protocol is used in windows. ErrUnsupportedSCTP = errors.New("unsupported SCTP protocol used on windows") ) @@ -156,14 +158,19 @@ func deDuplicateExcept(exceptInIPBlock []string) []string { } // ipBlockIPSet return translatedIPSet based based on ipBlockRule. -func ipBlockIPSet(policyName, ns string, direction policies.Direction, ipBlockSetIndex, ipBlockPeerIndex int, ipBlockRule *networkingv1.IPBlock) *ipsets.TranslatedIPSet { +func ipBlockIPSet(policyName, ns string, direction policies.Direction, ipBlockSetIndex, ipBlockPeerIndex int, ipBlockRule *networkingv1.IPBlock) (*ipsets.TranslatedIPSet, error) { if ipBlockRule == nil || ipBlockRule.CIDR == "" { - return nil + return nil, nil } // de-duplicated Except if there are redundance elements. deDupExcepts := deDuplicateExcept(ipBlockRule.Except) lenOfDeDupExcepts := len(deDupExcepts) + + if util.IsWindowsDP() && lenOfDeDupExcepts > 0 { + return nil, ErrUnsupportedExceptCIDR + } + var members []string indexOfMembers := 0 // Ipset doesn't allow 0.0.0.0/0 to be added. @@ -203,21 +210,23 @@ func ipBlockIPSet(policyName, ns string, direction policies.Direction, ipBlockSe ipBlockIPSetName := ipBlockSetName(policyName, ns, direction, ipBlockSetIndex, ipBlockPeerIndex) ipBlockIPSet := ipsets.NewTranslatedIPSet(ipBlockIPSetName, ipsets.CIDRBlocks, members...) - return ipBlockIPSet + return ipBlockIPSet, nil } // ipBlockRule translates IPBlock field in networkpolicy object to translatedIPSet and SetInfo. // ipBlockSetIndex parameter is used to diffentiate ipBlock fields in one networkpolicy object. func ipBlockRule(policyName, ns string, direction policies.Direction, matchType policies.MatchType, ipBlockSetIndex, ipBlockPeerIndex int, - ipBlockRule *networkingv1.IPBlock) (*ipsets.TranslatedIPSet, policies.SetInfo) { - + ipBlockRule *networkingv1.IPBlock) (*ipsets.TranslatedIPSet, policies.SetInfo, error) { //nolint // gofumpt if ipBlockRule == nil || ipBlockRule.CIDR == "" { - return nil, policies.SetInfo{} + return nil, policies.SetInfo{}, nil } - ipBlockIPSet := ipBlockIPSet(policyName, ns, direction, ipBlockSetIndex, ipBlockPeerIndex, ipBlockRule) + ipBlockIPSet, err := ipBlockIPSet(policyName, ns, direction, ipBlockSetIndex, ipBlockPeerIndex, ipBlockRule) + if err != nil { + return nil, policies.SetInfo{}, err + } setInfo := policies.NewSetInfo(ipBlockIPSet.Metadata.Name, ipsets.CIDRBlocks, included, matchType) - return ipBlockIPSet, setInfo + return ipBlockIPSet, setInfo, err } // PodSelector translates podSelector of NetworkPolicyPeer field in networkpolicy object to translatedIPSets, children of translated IPSets, and SetInfo. @@ -373,9 +382,13 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire // #2.1 Handle IPBlock and port if exist if peer.IPBlock != nil { if len(peer.IPBlock.CIDR) > 0 { - ipBlockIPSet, ipBlockSetInfo := ipBlockRule(netPolName, npmNetPol.Namespace, direction, matchType, ruleIndex, peerIdx, peer.IPBlock) + ipBlockIPSet, ipBlockSetInfo, err := ipBlockRule(netPolName, npmNetPol.Namespace, direction, matchType, ruleIndex, peerIdx, peer.IPBlock) + if err != nil { + return err + } npmNetPol.RuleIPSets = append(npmNetPol.RuleIPSets, ipBlockIPSet) - err := peerAndPortRule(npmNetPol, direction, ports, []policies.SetInfo{ipBlockSetInfo}) + + err = peerAndPortRule(npmNetPol, direction, ports, []policies.SetInfo{ipBlockSetInfo}) if err != nil { return err } diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index ab5b578547..1fd1484236 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -33,7 +33,6 @@ func TestPortType(t *testing.T) { name string portRule networkingv1.NetworkPolicyPort want netpolPortType - wantErr bool }{ { name: "empty", @@ -179,7 +178,6 @@ func TestNamedPortRuleInfo(t *testing.T) { name string portRule *networkingv1.NetworkPolicyPort want *namedPortOutput - wantErr bool }{ { name: "empty", @@ -239,7 +237,6 @@ func TestNamedPortRule(t *testing.T) { name string portRule *networkingv1.NetworkPolicyPort want *namedPortRuleOutput - wantErr bool }{ { name: "empty", @@ -249,7 +246,6 @@ func TestNamedPortRule(t *testing.T) { setInfo: policies.SetInfo{}, protocol: "", }, - wantErr: false, }, { name: "serve-tcp", @@ -461,7 +457,8 @@ func TestIPBlockIPSet(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := ipBlockIPSet(tt.policyName, tt.namemspace, tt.direction, tt.ipBlockSetIndex, tt.ipBlockPeerIndex, tt.ipBlockRule) + got, err := ipBlockIPSet(tt.policyName, tt.namemspace, tt.direction, tt.ipBlockSetIndex, tt.ipBlockPeerIndex, tt.ipBlockRule) + require.NoError(t, err) require.Equal(t, tt.translatedIPSet, got) }) } @@ -527,7 +524,8 @@ func TestIPBlockRule(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - translatedIPSet, setInfo := ipBlockRule(tt.policyName, tt.namemspace, tt.direction, tt.matchType, tt.ipBlockSetIndex, tt.ipBlockPeerIndex, tt.ipBlockRule) + translatedIPSet, setInfo, err := ipBlockRule(tt.policyName, tt.namemspace, tt.direction, tt.matchType, tt.ipBlockSetIndex, tt.ipBlockPeerIndex, tt.ipBlockRule) + require.NoError(t, err) require.Equal(t, tt.translatedIPSet, translatedIPSet) require.Equal(t, tt.setInfo, setInfo) }) From 89389326b7e779fae2ab9f4053d38f3774df96cf Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 8 Aug 2022 10:44:10 -0700 Subject: [PATCH 2/2] do not requeue on ErrUnsupportedExceptCIDR --- npm/pkg/controlplane/controllers/v2/networkPolicyController.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go index a5a1dc7913..0d33aef7d4 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go @@ -349,5 +349,6 @@ func (c *NetworkPolicyController) cleanUpNetworkPolicy(netPolKey string) error { func isUnsupportedWindowsTranslationErr(err error) bool { return errors.Is(err, translation.ErrUnsupportedNamedPort) || errors.Is(err, translation.ErrUnsupportedNegativeMatch) || - errors.Is(err, translation.ErrUnsupportedSCTP) + errors.Is(err, translation.ErrUnsupportedSCTP) || + errors.Is(err, translation.ErrUnsupportedExceptCIDR) }