Added unreachable rules check, v0.6.0

CHANGELOG.md

@@ -1,8 +1,8 @@
-## Unreleased
+## v0.6.0 [2016-04-22]
 
 - Refactoring - moved all generic functions from checks to lib/acl and lib/backend
 - New ACL type to keep ACLs negation and other attributes
-
+- Added unreachable rules check
 
 ## v0.5.0 [2016-04-19]
 

README.md

@@ -98,6 +98,7 @@ executed, as they just duplicate HAProxy's own checks.
 | 005 | warning  | rule order masking real evaluation precedence | no                            |
 | 006 | warning  | duplicate rules found                         | yes                           |
 | 007 | warning  | deprecated keywords found                     | no                            |
+| 008 | warning  | unreachable rule found                        | yes                           |
 
 ## Contributing
 

checks/checks.go

@@ -21,6 +21,7 @@ func Run(sections []*lib.Section, extrasOnly bool) []lib.Problem {
 		{CheckPrecedence, false},
 		{CheckDuplicates, true},
 		{CheckDeprecations, false},
+		{CheckUnreachableRules, true},
 	}
 	var globalChecks = []globCheck{
 		{CheckUnusedBackends, true},

checks/checks_test.go

@@ -14,8 +14,8 @@ func TestRun(t *testing.T) {
 	sections := lib.GetSections(lines)
 	problems := Run(sections, false)
 
-	if len(problems) != 9 {
-		t.Errorf("Expected %d problems, got %d", 9, len(problems))
+	if len(problems) != 11 {
+		t.Errorf("Expected %d problems, got %d", 11, len(problems))
 	}
 }
 

checks/unreachable.go

@@ -0,0 +1,73 @@
+package checks
+
+import (
+	"fmt"
+	"sort"
+
+	"github.com/abulimov/haproxy-lint/lib"
+	A "github.com/abulimov/haproxy-lint/lib/acl"
+)
+
+// CheckUnreachableRules checks if we have rules that will never be
+// triggered because their acls are less generic than similar acls
+// declared before.
+func CheckUnreachableRules(s *lib.Section) []lib.Problem {
+	checkableKWs := map[string]bool{
+		"use_backend": true,
+		"redirect":    true,
+		"use-server":  true,
+	}
+
+	var problems []lib.Problem
+	var lineNums []int
+	aclsMap := make(map[int][]*A.ACL)
+	kwMap := make(map[int]string)
+	for i, line := range s.Content {
+		acls := A.ParseACLs(line)
+		if len(acls) > 0 {
+			lineNums = append(lineNums, i)
+			aclsMap[i] = acls
+			kwMap[i] = lib.GetKeyword(line)
+		}
+	}
+	sort.Ints(lineNums)
+	usedACLs := make(map[A.ACL]int)
+	// for each line from start of section
+	for _, i := range lineNums {
+		curACLs := aclsMap[i]
+		curKW := kwMap[i]
+		// for each acl in current acls
+		for _, acl := range curACLs {
+			// if we already seen some of current acls
+			if prevLine, found := usedACLs[*acl]; found {
+				// if current keyword is same as in line when this ACL was used
+				if _, ok := checkableKWs[curKW]; ok && curKW == kwMap[prevLine] {
+					// if previous acl line is more generic than current
+					if A.In(aclsMap[prevLine], curACLs) {
+						problems = append(
+							problems,
+							lib.Problem{
+								Line:     prevLine,
+								Col:      0,
+								Severity: "warning",
+								Message:  fmt.Sprintf("This line shadows rule '%s' on line %d", s.Content[i], i),
+							},
+						)
+						problems = append(
+							problems,
+							lib.Problem{
+								Line:     i,
+								Col:      0,
+								Severity: "warning",
+								Message:  fmt.Sprintf("Unreachable rule - '%s' on line %d will match first", s.Content[prevLine], prevLine),
+							},
+						)
+					}
+				}
+			}
+			// add current acl to used acls map
+			usedACLs[*acl] = i
+		}
+	}
+	return problems
+}

checks/unreachable_test.go

@@ -0,0 +1,20 @@
+package checks
+
+import (
+	"testing"
+
+	"github.com/abulimov/haproxy-lint/lib"
+)
+
+func TestCheckUnreachableRules(t *testing.T) {
+	lines, err := lib.GetConfig("../testdata/haproxy.cfg", "")
+	if err != nil {
+		t.Fatalf("Failed to read test data: %v", err)
+	}
+	sections := lib.GetSection("frontend", lines)
+	problems := CheckUnreachableRules(sections[1])
+
+	if len(problems) != 2 {
+		t.Errorf("Expected %d problems, got %d", 2, len(problems))
+	}
+}

haproxy-lint.go

@@ -28,7 +28,7 @@ import (
 	"github.com/abulimov/haproxy-lint/lib"
 )
 
-var version = "0.5.0"
+var version = "0.6.0"
 
 func myUsage() {
 	fmt.Printf("Usage: %s [OPTIONS] haproxy.cfg\n", os.Args[0])

testdata/haproxy.cfg

@@ -32,6 +32,9 @@ frontend https-in
     redirect scheme https code 301 if !h_test or h_some
 
     use_backend undefined-servers if h_test
+
+    use_backend servers if h_test { ssl_fc }
+
     redirect scheme https if !{ ssl_fc }
     default_backend other-servers