adding bad path tests (#437)

* adding ANP to parser.k8sobj * fixing gocritic rangeValCopy by indexing * w.i.p. anp support - first commit * more examples (2 ANPs/ ANP+NP) * fixing references * new_test that ensures rule ordering in ANP is respected * update the conn representation as complement in case it is shorter (all but: udp 5353 instead of SCTP 1-65535,TCP 1-65535,UDP 1-5352,5354-65535) * test with swapped rules from another test + diff test * more-tests * fixing conns computations and a test with multiple ANPs * extending output formats of existing tests * tiny fix * fixing a tinu bug in ruleConnections func * tiny doc updte * tiny doc update * a @todo tbd while review * return error if ANPs are without name or not unique names * remove redundant lines * reverting the changes adding complement string representation (all but) for connectionSet * Merge github.com:np-guard/netpol-analyzer into support_admin_netpolicy * minor updates to netpol_errors * currently disabling exposure-analysis when there are admin-network-policies in the input resources * some organizations (mainly comments updates) * updating some todo messages * updating some todo messages/questions * todo question * removing a todo that had an answer for, will add some tests on that case * fixing single anp conns compute when ingress and egress are intersected (not fully matched) * Update pkg/internal/netpolerrors/netpol_errors.go Co-authored-by: Tanya <tatyana@il.ibm.com> * Update pkg/netpol/eval/internal/k8s/adminnetpol.go Co-authored-by: Tanya <tatyana@il.ibm.com> * update todo msg * some fixes to anp so it matches latest apis * fixing port-set union func * Update pkg/netpol/connlist/connlist.go Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * Update pkg/netpol/eval/internal/k8s/adminnetpol.go Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * Update pkg/netpol/internal/common/connectionset.go Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * Update pkg/netpol/eval/internal/k8s/adminnetpol.go Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * Update pkg/netpol/eval/internal/k8s/adminnetpol.go Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * go.mod + lint fix * adding todo comment * fixes in subtract * one line func eliminated * uniqueness names are required for netpols and admin-netpols * hasNetpols considers ANPs too * Tests for AdminNetworkPolicy (#388) * Added some ANP tests from policy-assistant. Fixed a small bug in handling named ports in ANP * fixing lint errors * Fixing lint error * Reorganized testing infrastructure from for tests fro parsed resources - creating pod and namespace resources per test; reading expected results from file. Added more tests from policy assistant. * fixing lint errors * return error if ANPs are without name or not unique names * Revert "return error if ANPs are without name or not unique names" This reverts commit 1805549. * Added ANP/BANP names in tests. Added more tests, including BANP tests, currently commented out. * Fixed lint errors. * Fixed lint errors * Added eval parsed resources tests (along with connlist tests). Moved all parsed resources tests to a separate file. * fixing lint errors * fixing lint errors * Added testing of CheckIfAllowed and CheckIfAllowedNew * fixing lint errors * making linter happy * Reorganized eval ANP tests, to not depend on connlist. * Small fixes. * small fixes * Changed expected results to not use "all but" expressions. * making linter happy * making linter happy * making lint happy * making linter happy * make linter happy * Creating k8sObjects during a test run, rather then in a test creation. * making lint happy * make lint happy * linter * shutting up linter * Moved to parsed_resources_tests some functions used only there. * Added fake pod status IP fields * Avoiding unnecessary exports; Fixing lint errors. * Making linter happy * Update pkg/internal/testutils/parsed_resources_tests.go Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * Update pkg/internal/testutils/parsed_resources_tests.go Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * Fixed typos; removed unneeded change. --------- Co-authored-by: shireenf-ibm <shireenf@il.ibm.com> Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> * updating some todo comment which were updated in BANP PR * sort anps only once before allowed-conns computes (#402) * sort anps only once before allowed-conns computes * support_banp (#403) * support_banp+tests * removing lint note * fix merge errors * why failed to use generics for duplicated code in egressRuleSelectsPeer and ingressRuleSelectsPeer * banp tests with swapped rules * integrating Tanya's tests with BANP + adding results; results were compared to policy-assistant, all good * pass action is not defined for BANP * more code enhancement, + could not use generics * adding banp to policy kinds * adding comment on priority range * Update pkg/internal/netpolerrors/netpol_errors.go Co-authored-by: Tanya <tatyana@il.ibm.com> * Update pkg/netpol/eval/internal/k8s/adminnetpol.go Co-authored-by: Tanya <tatyana@il.ibm.com> * Update pkg/netpol/eval/internal/k8s/adminnetpol.go Co-authored-by: Tanya <tatyana@il.ibm.com> * Update pkg/netpol/eval/resources.go Co-authored-by: Tanya <tatyana@il.ibm.com> * Update pkg/netpol/eval/internal/k8s/policy_connections.go Co-authored-by: Tanya <tatyana@il.ibm.com> * some fixes + a new test * tiny doc update * demo test * tiny change to getPoliciesSelectingPod func and deleting the "deprecated" if statements in "getAllAllowedXgressConnsFromNetpols" * removing redundant if statements * new parsed tests with expected outputs and a fix to the func computing "intersection" between ANP's egress-ingress * fixing implementing approach + some more parsed tests * tiny doc update * renaming func * comment changed * removing comment * changing const names * fixing if else * code optimizations and re-org * moving parsed_resources_tests file + some re-orgs * optimizing collect from banp + fixing one test output * optimize + fix + tests confirming results - tested with policy-assistant * deny examples parallel to the allow examples added previously * switch * policy conns * collect from banp * updating outputs with empty line at eof * adding bad path tests * add anp_banp_blog_demo example Signed-off-by: adisos <adisos@il.ibm.com> * update example Signed-off-by: adisos <adisos@il.ibm.com> * tiny fix * update example - add another workload and ns Signed-off-by: adisos <adisos@il.ibm.com> * update example Signed-off-by: adisos <adisos@il.ibm.com> * min-max priority consts * moving consts * renaming some tests + adding blog_test to the connlist_test * test updates * updating test * adding references * updating test anp_test_6_swapping_rules * test update * test update * add test details Signed-off-by: adisos <adisos@il.ibm.com> --------- Signed-off-by: adisos <adisos@il.ibm.com> Co-authored-by: Tanya <tatyana@il.ibm.com> Co-authored-by: Adi Sosnovich <82078442+adisos@users.noreply.github.com> Co-authored-by: adisos <adisos@il.ibm.com>
np-guard · Dec 1, 2024 · c4811e0 · c4811e0
1 parent 16eb79f
commit c4811e0
Show file tree

Hide file tree

Showing 67 changed files with 8,233 additions and 8 deletions.
diff --git a/pkg/internal/netpolerrors/netpol_errors.go b/pkg/internal/netpolerrors/netpol_errors.go
@@ -146,10 +146,11 @@ func ConcatErrors(err1, err2 string) string {
 	return err1 + colonSep + err2
 }
 
+const PriorityErrExplain = "Two policies are considered to be conflicting if they are assigned the same priority."
+
 // SamePriorityErr returns the error message if a priority appears more than once in different admin-network-policies
 func SamePriorityErr(name1, name2 string) string {
-	return "Admin Network Policies: " + name1 + " and " + name2 + " have same priority;" +
-		"Two policies are considered to be conflicting if they are assigned the same priority."
+	return "Admin Network Policies: " + name1 + " and " + name2 + " have same priority;" + PriorityErrExplain
 }
 
 // PriorityValueErr returns error message of invalid priority value in an admin-network-policy

diff --git a/pkg/netpol/connlist/connlist_test.go b/pkg/netpol/connlist/connlist_test.go
@@ -167,6 +167,162 @@ func TestConnlistAnalyzeFatalErrors(t *testing.T) {
 			dirName:          "semanticDiff-same-topologies-illegal-podlist",
 			errorStrContains: netpolerrors.NotSupportedPodResourcesErrorStr("demo/cog-agents"),
 		},
+		{
+			name:             "Input_dir_has_two_netpols_with_same_name_in_a_namespace_should_return_fatal_error_of_existing_object",
+			dirName:          "np_bad_path_test_1",
+			errorStrContains: netpolerrors.NPWithSameNameError("default/backend-netpol"),
+		},
+		// anp & banp bad path tests
+		{
+			name:             "Input_dir_has_two_admin_netpols_with_same_priority_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_1",
+			errorStrContains: netpolerrors.PriorityErrExplain,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_invalid_priority_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_2",
+			errorStrContains: netpolerrors.PriorityValueErr("invalid-priority", 1001),
+		},
+		{
+			name:             "Input_dir_has_two_admin_netpols_with_same_name_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_3",
+			errorStrContains: netpolerrors.ANPsWithSameNameErr("same-name"),
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_empty_subject_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_4",
+			errorStrContains: netpolerrors.OneFieldSetSubjectErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_invalid_subject_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_5",
+			errorStrContains: netpolerrors.OneFieldSetSubjectErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_empty_egress_rule_peer_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_6",
+			errorStrContains: netpolerrors.ANPEgressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_invalid_egress_rule_peer_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_7",
+			errorStrContains: netpolerrors.OneFieldSetRulePeerErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_invalid_egress_rule_port_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_8",
+			errorStrContains: netpolerrors.ANPPortsError,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_invalid_egress_rule_action_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_9",
+			errorStrContains: netpolerrors.UnknownRuleActionErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_missing_egress_rule_peer_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_10",
+			errorStrContains: netpolerrors.ANPEgressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_missing_egress_rule_action_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_12",
+			errorStrContains: netpolerrors.UnknownRuleActionErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_missing_ingress_rule_peer_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_14",
+			errorStrContains: netpolerrors.ANPIngressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_empty_ingress_rule_peer_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_15",
+			errorStrContains: netpolerrors.ANPIngressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_invalid_ingress_rule_peer_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_16",
+			errorStrContains: netpolerrors.OneFieldSetRulePeerErr,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_invalid_ingress_rule_port_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_17",
+			errorStrContains: netpolerrors.ANPPortsError,
+		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_invalid_ingress_rule_action_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_18",
+			errorStrContains: netpolerrors.UnknownRuleActionErr,
+		},
+		{
+			name:             "Input_dir_has_more_than_one_baseline_admin_netpol_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_1",
+			errorStrContains: netpolerrors.BANPAlreadyExists,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_name_not_default_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_2",
+			errorStrContains: netpolerrors.BANPNameAssertion,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_empty_subject_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_3",
+			errorStrContains: netpolerrors.OneFieldSetSubjectErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_invalid_subject_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_4",
+			errorStrContains: netpolerrors.OneFieldSetSubjectErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_empty_egress_rule_peer_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_5",
+			errorStrContains: netpolerrors.ANPEgressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_missing_egress_rule_peer_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_6",
+			errorStrContains: netpolerrors.ANPEgressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_invalid_egress_rule_peer_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_7",
+			errorStrContains: netpolerrors.OneFieldSetRulePeerErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_invalid_egress_rule_action_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_8",
+			errorStrContains: netpolerrors.UnknownRuleActionErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_invalid_egress_rule_port_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_9",
+			errorStrContains: netpolerrors.ANPPortsError,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_missing_ingress_rule_peer_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_10",
+			errorStrContains: netpolerrors.ANPIngressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_empty_ingress_rule_peer_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_11",
+			errorStrContains: netpolerrors.ANPIngressRulePeersErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_invalid_ingress_rule_peer_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_12",
+			errorStrContains: netpolerrors.OneFieldSetRulePeerErr,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_invalid_ingress_rule_port_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_13",
+			errorStrContains: netpolerrors.ANPPortsError,
+		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_invalid_ingress_rule_action_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_14",
+			errorStrContains: netpolerrors.UnknownRuleActionErr,
+		},
 	}
 	for _, tt := range cases {
 		tt := tt

diff --git a/pkg/netpol/eval/internal/k8s/adminnetpol.go b/pkg/netpol/eval/internal/k8s/adminnetpol.go
@@ -223,11 +223,11 @@ func onlyOnePortFieldsSet(anpPort apisv1a.AdminNetworkPolicyPort) bool {
 }
 
 // subjectSelectsPeer returns true iff the given subject of the (baseline)adminNetworkPolicy selects the given peer
-func subjectSelectsPeer(anpSubject apisv1a.AdminNetworkPolicySubject, p Peer) (bool, error) {
+func subjectSelectsPeer(anpSubject apisv1a.AdminNetworkPolicySubject, p Peer, errTitle string) (bool, error) {
 	if (anpSubject.Namespaces == nil) == (anpSubject.Pods == nil) {
 		// (Baseline)AdminNetworkPolicySubject should contain exactly one field
 		// (https://github.com/kubernetes-sigs/network-policy-api/blob/v0.1.5/apis/v1alpha1/shared_types.go#L27))
-		return false, errors.New(netpolerrors.OneFieldSetSubjectErr)
+		return false, errors.New(errTitle + netpolerrors.OneFieldSetSubjectErr)
 	}
 	if anpSubject.Namespaces != nil {
 		return doesNamespacesFieldMatchPeer(anpSubject.Namespaces, p)
@@ -378,7 +378,8 @@ func (anp *AdminNetworkPolicy) Selects(p Peer, isIngress bool) (bool, error) {
 		return false, nil
 	}
 	// check if the subject selects the given peer
-	return subjectSelectsPeer(anp.Spec.Subject, p)
+	errTitle := fmt.Sprintf("%s %q: ", anpErrTitle, anp.Name)
+	return subjectSelectsPeer(anp.Spec.Subject, p, errTitle)
 }
 
 // adminPolicyAffectsDirection returns whether the anp affects the given direction or not.
@@ -392,9 +393,11 @@ func (anp *AdminNetworkPolicy) adminPolicyAffectsDirection(isIngress bool) bool
 	return len(anp.Spec.Egress) > 0
 }
 
+const anpErrTitle = "admin network policy"
+
 // anpErr returns string format of an error in a rule in admin netpol
 func (anp *AdminNetworkPolicy) anpRuleErr(ruleName, description string) error {
-	return fmt.Errorf("admin network policy %q: %s %q: %s", anp.Name, ruleErrTitle, ruleName, description)
+	return fmt.Errorf("%s %q: %s %q: %s", anpErrTitle, anp.Name, ruleErrTitle, ruleName, description)
 }
 
 // GetIngressPolicyConns returns the connections from the ingress rules selecting the src in spec of the adminNetworkPolicy

diff --git a/pkg/netpol/eval/internal/k8s/baseline_admin_netpol.go b/pkg/netpol/eval/internal/k8s/baseline_admin_netpol.go
@@ -30,7 +30,7 @@ func (banp *BaselineAdminNetworkPolicy) Selects(p Peer, isIngress bool) (bool, e
 		return false, nil
 	}
 	// check if the subject selects the given peer
-	return subjectSelectsPeer(banp.Spec.Subject, p)
+	return subjectSelectsPeer(banp.Spec.Subject, p, banpErrTitle)
 }
 
 // baselineAdminPolicyAffectsDirection returns whether the banp affects the given direction or not.
@@ -44,9 +44,11 @@ func (banp *BaselineAdminNetworkPolicy) baselineAdminPolicyAffectsDirection(isIn
 	return len(banp.Spec.Egress) > 0
 }
 
+const banpErrTitle = "default baseline admin network policy: "
+
 // banpRuleErr returns string format of an err in a rule in baseline-admin netpol
 func banpRuleErr(ruleName, description string) error {
-	return fmt.Errorf("default baseline admin network policy: %s %q: %s", ruleErrTitle, ruleName, description)
+	return fmt.Errorf("%s%s %q: %s", banpErrTitle, ruleErrTitle, ruleName, description)
 }
 
 // GetEgressPolicyConns returns the connections from the egress rules selecting the dst in spec of the baselineAdminNetworkPolicy

diff --git a/pkg/netpol/eval/resources.go b/pkg/netpol/eval/resources.go
@@ -185,6 +185,9 @@ func (pe *PolicyEngine) addObjectsByKind(objects []parser.K8sObject) error {
 // since the priority of policies is critical for computing the conns between peers
 func (pe *PolicyEngine) sortAdminNetpolsByPriority() error {
 	var err error
+	if len(pe.sortedAdminNetpols) == 1 && !pe.sortedAdminNetpols[0].HasValidPriority() {
+		return errors.New(netpolerrors.PriorityValueErr(pe.sortedAdminNetpols[0].Name, pe.sortedAdminNetpols[0].Spec.Priority))
+	}
 	sort.Slice(pe.sortedAdminNetpols, func(i, j int) bool {
 		// outcome is non-deterministic if there are two AdminNetworkPolicies at the same priority
 		if pe.sortedAdminNetpols[i].Spec.Priority == pe.sortedAdminNetpols[j].Spec.Priority {

diff --git a/tests/anp_bad_path_test_1/anps.yaml b/tests/anp_bad_path_test_1/anps.yaml
@@ -0,0 +1,74 @@
+apiVersion: policy.networking.k8s.io/v1alpha1
+kind: AdminNetworkPolicy
+metadata:
+  name: priority-50-example
+spec:
+  priority: 50
+  subject:
+    pods:
+      namespaceSelector:
+        matchLabels:
+          conformance-house: gryffindor
+      podSelector:
+        matchLabels:
+          conformance-house: gryffindor
+  ingress:
+  - name: "deny-all-ingress-from-slytherin"
+    action: "Deny"
+    from:
+    - pods:
+        namespaceSelector:
+          matchLabels:
+            conformance-house: slytherin
+        podSelector:
+          matchLabels:
+            conformance-house: slytherin
+  egress:
+  - name: "deny-all-egress-to-slytherin"
+    action: "Deny"
+    to:
+    - pods:
+        namespaceSelector:
+          matchLabels:
+            conformance-house: slytherin
+        podSelector:
+          matchLabels:
+            conformance-house: slytherin
+---
+apiVersion: policy.networking.k8s.io/v1alpha1
+kind: AdminNetworkPolicy
+metadata:
+  name: same-priority
+spec:
+  priority: 50   # priority error : two ANPs with same priority
+  subject: 
+    pods:
+      namespaceSelector:
+        matchLabels:
+          conformance-house: gryffindor
+      podSelector:
+        matchLabels:
+          conformance-house: gryffindor
+  ingress:
+  - name: "pass-all-ingress-from-slytherin"
+    action: "Pass"
+    from:
+    - pods:
+        namespaceSelector:
+            matchLabels:
+              conformance-house: slytherin
+        podSelector:
+          matchLabels:
+            conformance-house: slytherin
+  egress:
+  - name: "pass-all-egress-to-slytherin"
+    action: "Pass"
+    to:
+    - pods:
+        namespaceSelector:
+          matchLabels:
+              conformance-house: slytherin
+        podSelector:
+          matchLabels:
+            conformance-house: slytherin
+---