Test coverage

go/pkg/pass1/check-identical-services.go

@@ -85,18 +85,17 @@ func (c *spoc) checkIdenticalServices(sRules *serviceRules) {
 			// Sort riList, because we use attributes of first element
 			// to build hash key from.
 			slices.SortFunc(riList, func(a, b *ruleInfo) int {
-				if a.deny != b.deny {
-					if a.deny {
+				cmpBool1 := func(a bool) int {
+					if a {
 						return -1
 					}
-					// Uncoverable, deny rules have been put in front of list.
-					//return 1
+					return 1
+				}
+				if a.deny != b.deny {
+					return cmpBool1(a.deny)
 				}
 				if a.objIsSrc != b.objIsSrc {
-					if !a.objIsSrc {
-						return -1
-					}
-					return 1
+					return cmpBool1(!a.objIsSrc)
 				}
 				return slices.Compare(a.names, b.names)
 			})

go/pkg/pass2/cisco.go

@@ -343,26 +343,24 @@ func moveRulesEspAh(
 		}
 		return cmp.Compare(a.Bits(), b.Bits())
 	}
+	hasLog = true
 	slices.SortStableFunc(rules, func(a, b *ciscoRule) int {
-		if a.deny && b.deny {
-			return 0
-		}
-		if a.deny {
-			return -1
-		}
-		if b.deny {
+		cmpBool := func(a, b bool) int {
+			if a == b {
+				return 0
+			}
+			if a {
+				return -1
+			}
 			return 1
 		}
+		if a.deny || b.deny {
+			return cmpBool(a.deny, b.deny)
+		}
 		sa := needSort(a)
 		sb := needSort(b)
-		if !sa && !sb {
-			return 0
-		}
-		if !sa {
-			return 1
-		}
-		if !sb {
-			return -1
+		if !sa || !sb {
+			return cmpBool(!sb, !sa)
 		}
 		if cmp := strings.Compare(a.prt.protocol, b.prt.protocol); cmp != 0 {
 			return cmp

go/testdata/identical-services.t

@@ -337,6 +337,27 @@ Warning: These services have identical rule definitions.
  - service:s2
 =OPTIONS=--check_identical_services=warn
 
+############################################################
+=TITLE=Changed order of equal rules (3)
+=INPUT=
+[[topo]]
+service:s1 = {
+ user = network:n2;
+ deny   src = host:h10; dst = user; prt = tcp 22;
+ permit src = network:n1; dst = user; prt = tcp 22;
+}
+service:s2 = {
+ user = interface:r1.n1;
+ permit src = network:n1; dst = user; prt = tcp 22;
+ deny   src = host:h10; dst = user; prt = tcp 22;
+}
+=WARNING=
+Warning: These services have identical rule definitions.
+ A single service should be created instead, with merged users.
+ - service:s1
+ - service:s2
+=OPTIONS=--check_identical_services=warn --check_duplicate_rules=0
+
 ############################################################
 =TITLE=Similar service, but changed src/dst
 =INPUT=

go/testdata/ipv6/identical-services_ipv6.t

@@ -353,6 +353,28 @@ Warning: These services have identical rule definitions.
  - service:s2
 =OPTIONS=--check_identical_services=warn
 
+############################################################
+=TITLE=Changed order of equal rules (3)
+=PARAMS=--ipv6
+=INPUT=
+[[topo]]
+service:s1 = {
+ user = network:n2;
+ deny   src = host:h10; dst = user; prt = tcp 22;
+ permit src = network:n1; dst = user; prt = tcp 22;
+}
+service:s2 = {
+ user = interface:r1.n1;
+ permit src = network:n1; dst = user; prt = tcp 22;
+ deny   src = host:h10; dst = user; prt = tcp 22;
+}
+=WARNING=
+Warning: These services have identical rule definitions.
+ A single service should be created instead, with merged users.
+ - service:s1
+ - service:s2
+=OPTIONS=--check_identical_services=warn --check_duplicate_rules=0
+
 ############################################################
 =TITLE=Similar service, but changed src/dst
 =PARAMS=--ipv6