From 71d65c358feb25a30f02fba64e1b9b6335473b62 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 19:58:49 -0700 Subject: [PATCH 1/4] make connectivity table easier to read for loopback traffic Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- cmd/policy-assistant/pkg/connectivity/probe/connectivity.go | 4 ++++ cmd/policy-assistant/pkg/connectivity/probe/jobrunner.go | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/cmd/policy-assistant/pkg/connectivity/probe/connectivity.go b/cmd/policy-assistant/pkg/connectivity/probe/connectivity.go index 05048cce..b7f7e307 100644 --- a/cmd/policy-assistant/pkg/connectivity/probe/connectivity.go +++ b/cmd/policy-assistant/pkg/connectivity/probe/connectivity.go @@ -11,6 +11,8 @@ const ( ConnectivityInvalidPortProtocol Connectivity = "invalidportprotocol" ConnectivityBlocked Connectivity = "blocked" ConnectivityAllowed Connectivity = "allowed" + // ConnectivityUndefined e.g. for loopback traffic + ConnectivityUndefined Connectivity = "undefined" ) var AllConnectivity = []Connectivity{ @@ -36,6 +38,8 @@ func (p Connectivity) ShortString() string { return "P" case ConnectivityInvalidPortProtocol: return "N" + case ConnectivityUndefined: + return "#" default: panic(errors.Errorf("invalid Connectivity value: %+v", p)) } diff --git a/cmd/policy-assistant/pkg/connectivity/probe/jobrunner.go b/cmd/policy-assistant/pkg/connectivity/probe/jobrunner.go index 53f40e87..227e4a3e 100644 --- a/cmd/policy-assistant/pkg/connectivity/probe/jobrunner.go +++ b/cmd/policy-assistant/pkg/connectivity/probe/jobrunner.go @@ -76,6 +76,11 @@ func (s *SimulatedJobRunner) RunJobs(jobs []*Job) []*JobResult { } func (s *SimulatedJobRunner) RunJob(job *Job) *JobResult { + if job.FromKey == job.ToKey { + connUndefined := ConnectivityUndefined + return &JobResult{Job: job, Ingress: &connUndefined, Egress: &connUndefined, Combined: ConnectivityUndefined} + } + allowed := s.Policies.IsTrafficAllowed(job.Traffic()) // TODO could also keep the whole `allowed` struct somewhere From 7141a519596ee1e8ab4f14c6b2b3c9bf8a721efc Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 20:07:38 -0700 Subject: [PATCH 2/4] enhance displays with (b)anp rule names Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- cmd/policy-assistant/pkg/matcher/explain.go | 13 ++++++------ .../pkg/matcher/peermatcherv2.go | 20 ++++++++++++------- cmd/policy-assistant/pkg/matcher/policy.go | 14 ++++++------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/cmd/policy-assistant/pkg/matcher/explain.go b/cmd/policy-assistant/pkg/matcher/explain.go index 3654d666..b7ae3413 100644 --- a/cmd/policy-assistant/pkg/matcher/explain.go +++ b/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" @@ -26,7 +27,7 @@ func (p *peerProtocolGroup) Matches(subject, peer *TrafficPeer, portInt int, por } type anpGroup struct { - name string + ruleName string priority int effects []string kind PolicyKind @@ -129,9 +130,9 @@ func (s *SliceBuilder) peerProtocolGroupTableLines(t *peerProtocolGroup) { }) for _, v := range anps { if len(v.effects) > 1 { - actions = append(actions, fmt.Sprintf(" pri=%d (%s): %s (ineffective rules: %s)", v.priority, v.name, v.effects[0], strings.Join(v.effects[1:], ", "))) + actions = append(actions, fmt.Sprintf(" pri=%d (%s): %s (ineffective rules: %s)", v.priority, v.ruleName, v.effects[0], strings.Join(v.effects[1:], ", "))) } else { - actions = append(actions, fmt.Sprintf(" pri=%d (%s): %s", v.priority, v.name, v.effects[0])) + actions = append(actions, fmt.Sprintf(" pri=%d (%s): %s", v.priority, v.ruleName, v.effects[0])) } } } @@ -199,10 +200,10 @@ func groupAnbAndBanp(p []PeerMatcher) []PeerMatcher { policies: map[string]*anpGroup{}, } } - kg := t.Name + kg := t.PolicyName if _, ok := groups[k].policies[kg]; !ok { groups[k].policies[kg] = &anpGroup{ - name: t.Name, + ruleName: t.PolicyName, priority: t.effectFromMatch.Priority, effects: []string{}, kind: t.effectFromMatch.PolicyKind, diff --git a/cmd/policy-assistant/pkg/matcher/peermatcherv2.go b/cmd/policy-assistant/pkg/matcher/peermatcherv2.go index 960359a4..efbd9a52 100644 --- a/cmd/policy-assistant/pkg/matcher/peermatcherv2.go +++ b/cmd/policy-assistant/pkg/matcher/peermatcherv2.go @@ -10,16 +10,19 @@ import ( // This is because ANP and BANP only deal with Pod to Pod traffic, and do not deal with external IPs. type PeerMatcherAdmin struct { *PodPeerMatcher - Name string + PolicyName string + RuleName string effectFromMatch Effect } // NewPeerMatcherANP creates a PeerMatcherAdmin for an ANP rule -func NewPeerMatcherANP(peer *PodPeerMatcher, v Verdict, priority int, source string) *PeerMatcherAdmin { +func NewPeerMatcherANP(peer *PodPeerMatcher, v Verdict, priority int, policyName, ruleName string) *PeerMatcherAdmin { return &PeerMatcherAdmin{ PodPeerMatcher: peer, - Name: source, + PolicyName: policyName, + RuleName: ruleName, effectFromMatch: Effect{ + RuleName: ruleName, PolicyKind: AdminNetworkPolicy, Priority: priority, Verdict: v, @@ -28,11 +31,13 @@ func NewPeerMatcherANP(peer *PodPeerMatcher, v Verdict, priority int, source str } // NewPeerMatcherBANP creates a new PeerMatcherAdmin for a BANP rule -func NewPeerMatcherBANP(peer *PodPeerMatcher, v Verdict, source string) *PeerMatcherAdmin { +func NewPeerMatcherBANP(peer *PodPeerMatcher, v Verdict, policyName, ruleName string) *PeerMatcherAdmin { return &PeerMatcherAdmin{ PodPeerMatcher: peer, - Name: source, + PolicyName: policyName, + RuleName: ruleName, effectFromMatch: Effect{ + RuleName: ruleName, PolicyKind: BaselineAdminNetworkPolicy, Verdict: v, }, @@ -41,6 +46,7 @@ func NewPeerMatcherBANP(peer *PodPeerMatcher, v Verdict, source string) *PeerMat // Effect models the effect of one or more v1/v2 NetPol rules on a peer type Effect struct { + RuleName string PolicyKind // Priority is only used for ANP (there can only be one BANP) Priority int @@ -57,9 +63,9 @@ const ( func NewV1Effect(allow bool) Effect { if allow { - return Effect{NetworkPolicyV1, 0, Allow} + return Effect{"", NetworkPolicyV1, 0, Allow} } - return Effect{NetworkPolicyV1, 0, None} + return Effect{"", NetworkPolicyV1, 0, None} } type Verdict string diff --git a/cmd/policy-assistant/pkg/matcher/policy.go b/cmd/policy-assistant/pkg/matcher/policy.go index 1aa3fc8d..73a9b2c4 100644 --- a/cmd/policy-assistant/pkg/matcher/policy.go +++ b/cmd/policy-assistant/pkg/matcher/policy.go @@ -120,15 +120,15 @@ func (d DirectionResult) Flow() string { flows := make([]string, 0) if anp != nil { if anp.Verdict == Allow { - return "[ANP] Allow" + return fmt.Sprintf("[ANP] Allow (%s)", anp.RuleName) } if anp.Verdict == Deny { - return "[ANP] Deny" + return fmt.Sprintf("[ANP] Deny (%s)", anp.RuleName) } if anp.Verdict == Pass { - flows = append(flows, "[ANP] Pass") + flows = append(flows, fmt.Sprintf("[ANP] Pass (%s)", anp.RuleName)) } else { flows = append(flows, "[ANP] No-Op") } @@ -136,9 +136,9 @@ func (d DirectionResult) Flow() string { if npv1 != nil { if npv1.Verdict == Allow { - flows = append(flows, "[NPv1] Allow") + flows = append(flows, fmt.Sprintf("[NPv1] Allow (%s)", npv1.RuleName)) } else { - flows = append(flows, "[NPv1] Dropped") + flows = append(flows, fmt.Sprintf("[NPv1] Dropped (%s)", npv1.RuleName)) } return strings.Join(flows, " -> ") @@ -146,9 +146,9 @@ func (d DirectionResult) Flow() string { if banp != nil { if banp.Verdict == Allow { - flows = append(flows, "[BANP] Allow") + flows = append(flows, fmt.Sprintf("[BANP] Allow (%s)", banp.RuleName)) } else if banp.Verdict == Deny { - flows = append(flows, "[BANP] Deny") + flows = append(flows, fmt.Sprintf("[BANP] Deny (%s)", banp.RuleName)) } else { flows = append(flows, "[BANP] No-Op") } From a7efe53d10ed22d39c0e7b726abb6761cb12696f Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 20:09:24 -0700 Subject: [PATCH 3/4] don't print an extra line when egress is empty Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- cmd/policy-assistant/pkg/matcher/explain.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/policy-assistant/pkg/matcher/explain.go b/cmd/policy-assistant/pkg/matcher/explain.go index b7ae3413..21e22f78 100644 --- a/cmd/policy-assistant/pkg/matcher/explain.go +++ b/cmd/policy-assistant/pkg/matcher/explain.go @@ -55,8 +55,10 @@ func (p *Policy) ExplainTable() string { ingresses, egresses := p.SortedTargets() builder.TargetsTableLines(ingresses, true) - builder.Elements = append(builder.Elements, []string{"", "", "", "", "", ""}) - builder.TargetsTableLines(egresses, false) + if len(egresses) > 0 { + builder.Elements = append(builder.Elements, []string{"", "", "", "", "", ""}) + builder.TargetsTableLines(egresses, false) + } table.AppendBulk(builder.Elements) From ec8037961642df9a386538ccbead91568a6a8d33 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 20:15:13 -0700 Subject: [PATCH 4/4] fix: use rule name instead of policy name Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- cmd/policy-assistant/pkg/matcher/explain.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/policy-assistant/pkg/matcher/explain.go b/cmd/policy-assistant/pkg/matcher/explain.go index 21e22f78..dc28bbef 100644 --- a/cmd/policy-assistant/pkg/matcher/explain.go +++ b/cmd/policy-assistant/pkg/matcher/explain.go @@ -205,7 +205,7 @@ func groupAnbAndBanp(p []PeerMatcher) []PeerMatcher { kg := t.PolicyName if _, ok := groups[k].policies[kg]; !ok { groups[k].policies[kg] = &anpGroup{ - ruleName: t.PolicyName, + ruleName: t.RuleName, priority: t.effectFromMatch.Priority, effects: []string{}, kind: t.effectFromMatch.PolicyKind,