Skip to content

Commit

Permalink
More delicate handling of intersection of ingress and egress connecti…
Browse files Browse the repository at this point in the history
…ons (to preserve explainability data from both directions).

Updating testing data accordingly.
  • Loading branch information
tanyaveksler committed Dec 10, 2024
1 parent 4684411 commit 8217a4b
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 49 deletions.
4 changes: 1 addition & 3 deletions pkg/netpol/connlist/explanation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,10 @@ var explainTests = []struct {
// testDirName: "anp_test_10",
// },
{
testDirName: "anp_banp_blog_demo",
focusWorkload: "my-monitoring",
testDirName: "anp_banp_blog_demo",
},
{
testDirName: "anp_banp_blog_demo_2",
// focusWorkload: "my-monitoring",
},
// {
// testDirName: "ipblockstest",
Expand Down
49 changes: 36 additions & 13 deletions pkg/netpol/internal/common/augmented_intervalset.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,32 @@ func (rules *ImplyingRulesType) Union(other ImplyingRulesType, collectRules bool
rules.Egress.Union(other.Egress, collectRules)
}

func (rules *ImplyingRulesType) onlyIngressDirection() bool {
return !rules.Ingress.Empty() && rules.Egress.Empty()
}

func (rules *ImplyingRulesType) onlyEgressDirection() bool {
return rules.Ingress.Empty() && !rules.Egress.Empty()
}

// OverrideUnlessOppositeDirections checks whether rules and other contain only rules of opposite directions
// (one of them only Ingress and another only Egress).
// This happens when performing intersection between ingress and egress connections.
// In this case the function preserves implying rules of both directions (for detailed explainability report).
// If this is not the case of 'opposite durections' scenario, the function overrides current implying rules by others'.
func (rules *ImplyingRulesType) OverrideUnlessOppositeDirections(other ImplyingRulesType) {
if rules.onlyIngressDirection() && other.onlyEgressDirection() {

Check failure on line 266 in pkg/netpol/internal/common/augmented_intervalset.go

View workflow job for this annotation

GitHub Actions / golangci-lint

ifElseChain: rewrite if-else to switch statement (gocritic)
// opposite directions (Ingress in rules and Egress in other) -> keep Ingress, copy Egress
rules.Egress = other.Egress.Copy()
} else if rules.onlyEgressDirection() && other.onlyIngressDirection() {
// opposite directions (Egress in rules and Ingress in other) -> keep Egress, copy Ingress
rules.Ingress = other.Ingress.Copy()
} else {
// this is not the case of opposite directions -> override everything
*rules = other.Copy()
}
}

func (rules ImplyingRulesType) mayBeUpdatedBy(other ImplyingRulesType, collectRules bool) bool {
return rules.Ingress.mayBeUpdatedBy(other.Ingress, collectRules) || rules.Egress.mayBeUpdatedBy(other.Egress, collectRules)
}
Expand Down Expand Up @@ -453,12 +479,11 @@ func (c *AugmentedCanonicalSet) AddAugmentedInterval(v AugmentedInterval, collec
// split set[left] into two intervals, while the implying rules of the second interval should get the new value (from v)
new1 := AugmentedInterval{interval: interval.New(set[left].interval.Start(), v.interval.Start()-1),
inSet: set[left].inSet, implyingRules: set[left].implyingRules.Copy()}
var newImplyingRules ImplyingRulesType
newImplyingRules := set[left].implyingRules.Copy()
if set[left].inSet == v.inSet {
newImplyingRules = set[left].implyingRules.Copy()
newImplyingRules.Union(v.implyingRules, collectRules)
} else {
newImplyingRules = v.implyingRules.Copy()
newImplyingRules.OverrideUnlessOppositeDirections(v.implyingRules)
}
new2 := AugmentedInterval{interval: interval.New(v.interval.Start(), min(set[left].interval.End(), v.interval.End())),
inSet: v.inSet, implyingRules: newImplyingRules}
Expand All @@ -470,14 +495,13 @@ func (c *AugmentedCanonicalSet) AddAugmentedInterval(v AugmentedInterval, collec
(set[right].inSet != v.inSet || set[right].implyingRules.mayBeUpdatedBy(v.implyingRules, collectRules)) {
break // this is the corner case handled following the loop below
}
var newImplyingRules ImplyingRulesType
newImplyingRules := set[ind].implyingRules.Copy()
if set[ind].inSet == v.inSet {
// this interval is not impacted by v;
// however, its implying rules may be updated by those of v.
newImplyingRules = set[ind].implyingRules.Copy()
newImplyingRules.Union(v.implyingRules, collectRules)
} else {
newImplyingRules = v.implyingRules.Copy()
newImplyingRules.OverrideUnlessOppositeDirections(v.implyingRules)
}
result = append(result, AugmentedInterval{interval: set[ind].interval, inSet: v.inSet, implyingRules: newImplyingRules})
}
Expand All @@ -488,12 +512,11 @@ func (c *AugmentedCanonicalSet) AddAugmentedInterval(v AugmentedInterval, collec
if left < right || (left == right && v.interval.Start() == set[left].interval.Start()) {
// a special case when left==right (i.e., v is included in one interval from set) was already handled
// at the left-hand side of the intersection of v with set
var newImplyingRules ImplyingRulesType
newImplyingRules := set[right].implyingRules.Copy()
if set[right].inSet == v.inSet {
newImplyingRules = set[right].implyingRules.Copy()
newImplyingRules.Union(v.implyingRules, collectRules)
} else {
newImplyingRules = v.implyingRules.Copy()
newImplyingRules.OverrideUnlessOppositeDirections(v.implyingRules)
}
new1 := AugmentedInterval{interval: interval.New(set[right].interval.Start(), v.interval.End()),
inSet: v.inSet, implyingRules: newImplyingRules}
Expand Down Expand Up @@ -537,12 +560,12 @@ func (c *AugmentedCanonicalSet) Union(other *AugmentedCanonicalSet, collectRules
res := NewAugmentedCanonicalSet(c.MinValue(), c.MaxValue(), false)
for _, left := range c.intervalSet {
if !left.inSet {
res.AddAugmentedInterval(left, false)
res.AddAugmentedInterval(left, collectRules)
}
}
for _, right := range other.intervalSet {
if !right.inSet {
res.AddAugmentedInterval(right, false)
res.AddAugmentedInterval(right, collectRules)
}
}
for _, left := range c.intervalSet {
Expand Down Expand Up @@ -627,12 +650,12 @@ func (c *AugmentedCanonicalSet) Intersect(other *AugmentedCanonicalSet) *Augment
}
for _, left := range c.intervalSet {
if !left.inSet {
res.AddAugmentedInterval(left, false)
res.AddAugmentedInterval(left, true) // collect implying rules allowed by both sets
}
}
for _, right := range other.intervalSet {
if !right.inSet {
res.AddAugmentedInterval(right, false)
res.AddAugmentedInterval(right, true) // collect implying rules allowed by both sets
}
}
return res
Expand Down
4 changes: 4 additions & 0 deletions test_outputs/connlist/anp_banp_blog_demo_2_explain_output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ ALLOWED SCTP:1-65535 the system default (Allow all)
ALLOWED UDP:1-65535 the system default (Allow all)

DENIED TCP:1-1233 due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [BANP] default//Ingress rule deny-ingress-from-all-namespaces (Deny)

Expand All @@ -39,15 +40,18 @@ ALLOWED TCP:1234 due to the following policies//rules:
1) [ANP] allow-monitoring//Ingress rule allow-ingress-from-monitoring (Allow)

DENIED TCP:1235-8079 due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [BANP] default//Ingress rule deny-ingress-from-all-namespaces (Deny)

DENIED TCP:8080 due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [ANP] pass-monitoring//Ingress rule pass-ingress-from-monitoring (Pass)
2) [BANP] default//Ingress rule deny-ingress-from-all-namespaces (Deny)

DENIED TCP:8081-9000 due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [BANP] default//Ingress rule deny-ingress-from-all-namespaces (Deny)

Expand Down
80 changes: 80 additions & 0 deletions test_outputs/connlist/anp_banp_blog_demo_explain_output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN 0.0.0.0-255.255.255.255 => foo/my-foo[Pod]:

No Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [NP] foo/allow-monitoring//Ingress (captured but not selected by any Ingress rule)

----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN bar/my-bar[Pod] => foo/my-foo[Pod]:

No Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [NP] foo/allow-monitoring//Ingress (captured but not selected by any Ingress rule)

----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN baz/my-baz[Pod] => bar/my-bar[Pod]:

No Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [BANP] default//Ingress rule deny-ingress-from-all-namespaces (Deny)

----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN baz/my-baz[Pod] => foo/my-foo[Pod]:

No Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [NP] foo/allow-monitoring//Ingress (captured but not selected by any Ingress rule)

----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN foo/my-foo[Pod] => bar/my-bar[Pod]:

No Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [BANP] default//Ingress rule deny-ingress-from-all-namespaces (Deny)

----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN monitoring/my-monitoring[Pod] => bar/my-bar[Pod]:

No Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (DENIED)
1) [ANP] pass-monitoring//Ingress rule pass-ingress-from-monitoring (Pass)
2) [BANP] default//Ingress rule deny-ingress-from-all-namespaces (Deny)

----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN monitoring/my-monitoring[Pod] => baz/my-baz[Pod]:

All Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (ALLOWED)
1) [ANP] allow-monitoring//Ingress rule allow-ingress-from-monitoring (Allow)

----------------------------------------------------------------------------------------------------------------------------------------------------------------
CONNECTIONS BETWEEN monitoring/my-monitoring[Pod] => foo/my-foo[Pod]:

All Connections due to the following policies//rules:
EGRESS DIRECTION (ALLOWED) due to the system default (Allow all)
INGRESS DIRECTION (ALLOWED)
1) [ANP] pass-monitoring//Ingress rule pass-ingress-from-monitoring (Pass)
2) [NP] foo/allow-monitoring//Ingress rule #1

----------------------------------------------------------------------------------------------------------------------------------------------------------------
The following nodes are connected due to the system default (Allow all):
0.0.0.0-255.255.255.255 => bar/my-bar[Pod]
0.0.0.0-255.255.255.255 => baz/my-baz[Pod]
0.0.0.0-255.255.255.255 => monitoring/my-monitoring[Pod]
bar/my-bar[Pod] => 0.0.0.0-255.255.255.255
bar/my-bar[Pod] => baz/my-baz[Pod]
bar/my-bar[Pod] => monitoring/my-monitoring[Pod]
baz/my-baz[Pod] => 0.0.0.0-255.255.255.255
baz/my-baz[Pod] => monitoring/my-monitoring[Pod]
foo/my-foo[Pod] => 0.0.0.0-255.255.255.255
foo/my-foo[Pod] => baz/my-baz[Pod]
foo/my-foo[Pod] => monitoring/my-monitoring[Pod]
monitoring/my-monitoring[Pod] => 0.0.0.0-255.255.255.255

This file was deleted.

0 comments on commit 8217a4b

Please sign in to comment.