Merge pull request #240 from huntergregory/fix-211

[Policy Assistant] fix: npv1 deny-all simulation

Author: k8s-ci-robot

cmd/policy-assistant/pkg/matcher/builder.go

@@ -81,6 +81,10 @@ func BuildTarget(netpol *networkingv1.NetworkPolicy) (*Target, *Target) {
 }
 
 func BuildIngressMatcher(policyNamespace string, ingresses []networkingv1.NetworkPolicyIngressRule) []PeerMatcher {
+	if len(ingresses) == 0 {
+		return []PeerMatcher{&NoMatcher{}}
+	}
+
 	var matchers []PeerMatcher
 	for _, ingress := range ingresses {
 		matchers = append(matchers, BuildPeerMatcher(policyNamespace, ingress.Ports, ingress.From)...)
@@ -89,6 +93,10 @@ func BuildIngressMatcher(policyNamespace string, ingresses []networkingv1.Networ
 }
 
 func BuildEgressMatcher(policyNamespace string, egresses []networkingv1.NetworkPolicyEgressRule) []PeerMatcher {
+	if len(egresses) == 0 {
+		return []PeerMatcher{&NoMatcher{}}
+	}
+
 	var matchers []PeerMatcher
 	for _, egress := range egresses {
 		matchers = append(matchers, BuildPeerMatcher(policyNamespace, egress.Ports, egress.To)...)

cmd/policy-assistant/pkg/matcher/explain.go

@@ -2,11 +2,12 @@ package matcher
 
 import (
 	"fmt"
+	"strings"
+
 	"github.com/mattfenwick/collections/pkg/json"
 	"github.com/mattfenwick/collections/pkg/slice"
 	"golang.org/x/exp/slices"
 	v1 "k8s.io/api/core/v1"
-	"strings"
 
 	"github.com/mattfenwick/cyclonus/pkg/kube"
 	"github.com/olekukonko/tablewriter"
@@ -80,23 +81,25 @@ func (s *SliceBuilder) TargetsTableLines(targets []*Target, isIngress bool) {
 		rules := strings.Join(sourceRulesStrings, "\n")
 		s.Prefix = []string{ruleType, target.TargetString(), rules}
 
-		if len(target.Peers) == 0 {
-			s.Append("no pods, no ips", "NPv1: All peers allowed", "no ports, no protocols")
-			continue
+		if len(target.Peers) == 1 {
+			if _, ok := target.Peers[0].(*NoMatcher); ok {
+				s.Append("no peers", "NPv1:\n   Allow any peers", "none")
+				continue
+			}
 		}
 
 		peers := groupAnbAndBanp(target.Peers)
 		for _, p := range slice.SortOn(func(p PeerMatcher) string { return json.MustMarshalToString(p) }, peers) {
 			switch t := p.(type) {
 			case *AllPeersMatcher:
-				s.Append("all pods, all ips", "NPv1: All peers allowed", "all ports, all protocols")
+				s.Append("all pods, all ips", "NPv1:\n   Allow any peers", "all ports, all protocols")
 			case *PortsForAllPeersMatcher:
 				pps := PortMatcherTableLines(t.Port, NetworkPolicyV1)
-				s.Append("all pods, all ips", "NPv1: All peers allowed", strings.Join(pps, "\n"))
+				s.Append("all pods, all ips", "NPv1:\n   Allow any peers", strings.Join(pps, "\n"))
 			case *IPPeerMatcher:
 				s.IPPeerMatcherTableLines(t)
 			case *PodPeerMatcher:
-				s.Append(resolveSubject(t), "NPv1: All peers allowed", strings.Join(PortMatcherTableLines(t.Port, NewV1Effect(true).PolicyKind), "\n"))
+				s.Append(resolveSubject(t), "NPv1:\n   Allow any peers", strings.Join(PortMatcherTableLines(t.Port, NewV1Effect(true).PolicyKind), "\n"))
 			case *peerProtocolGroup:
 				s.peerProtocolGroupTableLines(t)
 			default:
@@ -110,7 +113,7 @@ func (s *SliceBuilder) TargetsTableLines(targets []*Target, isIngress bool) {
 func (s *SliceBuilder) IPPeerMatcherTableLines(ip *IPPeerMatcher) {
 	peer := ip.IPBlock.CIDR + "\n" + fmt.Sprintf("except %+v", ip.IPBlock.Except)
 	pps := PortMatcherTableLines(ip.Port, NetworkPolicyV1)
-	s.Append(peer, "NPv1: All peers allowed", strings.Join(pps, "\n"))
+	s.Append(peer, "NPv1:\n   Allow any peers", strings.Join(pps, "\n"))
 }
 
 func (s *SliceBuilder) peerProtocolGroupTableLines(t *peerProtocolGroup) {

cmd/policy-assistant/pkg/matcher/peermatcher.go

@@ -18,8 +18,9 @@ These are the original PeerMatcher implementations made for v1 NetPol:
 - PortsForAllPeersMatcher
 - IPPeerMatcher
 - PodPeerMatcher
+- NoMatcher
 
-All PeerMatcher implementations (except AllPeersMatcher) use a PortMatcher.
+All PeerMatcher implementations (except AllPeersMatcher and NoMatcher) use a PortMatcher.
 If the traffic doesn't match the port matcher, then Matches() will be false.
 
 Now we also have PeerMatcherAdmin, a wrapper for PodPeerMatcher to model ANP and BANP.
@@ -29,6 +30,13 @@ type PeerMatcher interface {
 	Matches(subject, peer *TrafficPeer, portInt int, portName string, protocol v1.Protocol) bool
 }
 
+// NoMatcher matches no traffic.
+type NoMatcher struct{}
+
+func (p *NoMatcher) Matches(subject, peer *TrafficPeer, portInt int, portName string, protocol v1.Protocol) bool {
+	return false
+}
+
 // AllPeerMatcher matches all pod to pod traffic.
 type AllPeersMatcher struct{}
 

cmd/policy-assistant/pkg/matcher/simplifier.go

@@ -33,6 +33,22 @@ func SimplifyV1(matchers []PeerMatcher) []PeerMatcher {
 		}
 	}
 
+	if len(v1Matchers) == 0 {
+		return nil
+	}
+
+	onlyNoMatchers := true
+	for _, matcher := range v1Matchers {
+		if _, ok := matcher.(*NoMatcher); !ok {
+			onlyNoMatchers = false
+			break
+		}
+	}
+
+	if onlyNoMatchers {
+		return []PeerMatcher{&NoMatcher{}}
+	}
+
 	matchesAll := false
 	var portsForAllPeersMatchers []*PortsForAllPeersMatcher
 	var ips []*IPPeerMatcher
@@ -47,6 +63,8 @@ func SimplifyV1(matchers []PeerMatcher) []PeerMatcher {
 			ips = append(ips, a)
 		case *PodPeerMatcher:
 			pods = append(pods, a)
+		case *NoMatcher:
+			// nothing to do
 		default:
 			panic(errors.Errorf("invalid matcher type %T", matcher))
 		}

cmd/policy-assistant/pkg/matcher/simplifier_tests.go

@@ -109,6 +109,49 @@ func RunSimplifierTests() {
 			}
 			Expect(Simplify([]PeerMatcher{dns, somePod})).To(Equal([]PeerMatcher{dns, somePod}))
 		})
+
+		It("should simplify no matchers to one no matcher", func() {
+			no1 := &NoMatcher{}
+			no2 := &NoMatcher{}
+
+			Expect(Simplify([]PeerMatcher{no1, no2})).To(Equal([]PeerMatcher{&NoMatcher{}}))
+		})
+
+		It("ignore no matchers amongst others", func() {
+			no1 := &NoMatcher{}
+
+			Expect(Simplify([]PeerMatcher{no1, all, allOnTCP80, ip, allPodsAllPorts, allPodsTCP103})).To(Equal([]PeerMatcher{all}))
+		})
+
+		It("don't simplify (b)anp", func() {
+			anpDenyAll := &PeerMatcherAdmin{
+				PodPeerMatcher: &PodPeerMatcher{
+					Namespace: &AllNamespaceMatcher{},
+					Pod:       &AllPodMatcher{},
+					Port:      &AllPortMatcher{},
+				},
+				effectFromMatch: Effect{
+					PolicyKind: AdminNetworkPolicy,
+					Priority:   5,
+					Verdict:    Deny,
+				},
+				Name: "anp",
+			}
+			banpAllowAll := &PeerMatcherAdmin{
+				PodPeerMatcher: &PodPeerMatcher{
+					Namespace: &AllNamespaceMatcher{},
+					Pod:       &AllPodMatcher{},
+					Port:      &AllPortMatcher{},
+				},
+				effectFromMatch: Effect{
+					PolicyKind: BaselineAdminNetworkPolicy,
+					Verdict:    Allow,
+				},
+				Name: "banp",
+			}
+			Expect(Simplify([]PeerMatcher{all, allOnTCP80, ip, allPodsAllPorts, banpAllowAll, allPodsTCP103, anpDenyAll})).To(Equal([]PeerMatcher{banpAllowAll, anpDenyAll, all}))
+
+		})
 	})
 
 	Describe("Port Simplifier", func() {

cmd/policy-assistant/test/integration/integration_test.go

@@ -57,6 +57,26 @@ type flow struct {
 
 func TestNetPolV1Connectivity(t *testing.T) {
 	tests := []connectivityTest{
+		{
+			name:                   "deny all",
+			defaultIngressBehavior: probe.ConnectivityBlocked,
+			defaultEgressBehavior:  probe.ConnectivityAllowed,
+			args: args{
+				resources: getResources(t, []string{"x"}, []string{"a", "b"}, []int{80}, []v1.Protocol{v1.ProtocolTCP}),
+				netpols: []*networkingv1.NetworkPolicy{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: "x",
+							Name:      "base",
+						},
+						Spec: networkingv1.NetworkPolicySpec{
+							PodSelector: metav1.LabelSelector{},
+							PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
+						},
+					},
+				},
+			},
+		},
 		{
 			name:                   "ingress port allowed",
 			defaultIngressBehavior: probe.ConnectivityAllowed,