Move mod-security logic from template to go code (#5009)

internal/ingress/controller/template/template.go

@@ -179,6 +179,7 @@ var (
 		"buildHTTPSListener":                 buildHTTPSListener,
 		"buildOpentracingForLocation":        buildOpentracingForLocation,
 		"shouldLoadOpentracingModule":        shouldLoadOpentracingModule,
+		"buildModSecurityForLocation":        buildModSecurityForLocation,
 	}
 )
 
@@ -1336,3 +1337,43 @@ func shouldLoadOpentracingModule(c interface{}, s interface{}) bool {
 
 	return false
 }
+
+func buildModSecurityForLocation(cfg config.Configuration, location *ingress.Location) string {
+	isMSEnabledInLoc := location.ModSecurity.Enable
+	isMSEnabled := cfg.EnableModsecurity
+
+	if !isMSEnabled && !isMSEnabledInLoc {
+		return ""
+	}
+
+	if !isMSEnabledInLoc {
+		return ""
+	}
+
+	var buffer bytes.Buffer
+
+	if !isMSEnabled {
+		buffer.WriteString(`modsecurity on;
+modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
+`)
+	}
+
+	if !cfg.EnableOWASPCoreRules && location.ModSecurity.OWASPRules {
+		buffer.WriteString(`modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf;
+`)
+	}
+
+	if location.ModSecurity.Snippet != "" {
+		buffer.WriteString(fmt.Sprintf(`modsecurity_rules '
+%v
+';
+`, location.ModSecurity.Snippet))
+	}
+
+	if location.ModSecurity.TransactionID != "" {
+		buffer.WriteString(fmt.Sprintf(`modsecurity_transaction_id "%v";
+`, location.ModSecurity.TransactionID))
+	}
+
+	return buffer.String()
+}

internal/ingress/controller/template/template_test.go

@@ -29,7 +29,6 @@ import (
 	"testing"
 
 	jsoniter "github.com/json-iterator/go"
-
 	apiv1 "k8s.io/api/core/v1"
 	networking "k8s.io/api/networking/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -1369,3 +1368,67 @@ func TestShouldLoadOpentracingModule(t *testing.T) {
 		t.Errorf("Expected '%v' but returned '%v'", expected, actual)
 	}
 }
+
+func TestModSecurityForLocation(t *testing.T) {
+	loadModule := `modsecurity on;
+modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
+`
+
+	modsecRule := `modsecurity_rules '
+#RULE#
+';
+`
+	owaspRules := `modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf;
+`
+
+	transactionID := "0000000"
+	transactionCfg := fmt.Sprintf(`modsecurity_transaction_id "%v";
+`, transactionID)
+
+	testRule := "#RULE#"
+
+	testCases := []struct {
+		description         string
+		isEnabledInCM       bool
+		isOwaspEnabledInCM  bool
+		isEnabledInLoc      bool
+		isOwaspEnabledInLoc bool
+		snippet             string
+		transactionID       string
+		expected            string
+	}{
+		{"configmap enabled, configmap OWASP disabled, without annotation, snippet or transaction ID", true, false, false, false, "", "", ""},
+		{"configmap enabled, configmap OWASP disabled, without annotation, snippet and with transaction ID", true, false, false, false, "", transactionID, ""},
+		{"configmap enabled, configmap OWASP  enabled, without annotation, OWASP enabled", true, true, false, false, "", "", ""},
+		{"configmap enabled, configmap OWASP  enabled, without annotation, OWASP disabled, with snippet and no transaction ID", true, true, true, false, testRule, "", modsecRule},
+		{"configmap enabled, configmap OWASP  enabled, without annotation, OWASP disabled, with snippet and transaction ID", true, true, true, false, testRule, transactionID, fmt.Sprintf("%v%v", modsecRule, transactionCfg)},
+		{"configmap enabled, with annotation, OWASP disabled", true, false, true, false, "", "", ""},
+		{"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, no snippet and no transaction ID", true, false, true, true, "", "", owaspRules},
+		{"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, with snippet and no transaction ID", true, false, true, true, "", "", owaspRules},
+		{"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, with snippet and transaction ID", true, false, true, true, "", transactionID, fmt.Sprintf("%v%v", owaspRules, transactionCfg)},
+		{"configmap enabled, OWASP configmap enabled, with annotation, OWASP disabled", true, true, true, false, "", "", ""},
+		{"configmap disabled, with annotation, OWASP disabled", false, false, true, false, "", "", loadModule},
+		{"configmap disabled, with annotation, OWASP disabled", false, false, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
+		{"configmap disabled, with annotation, OWASP enabled", false, false, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
+	}
+
+	for _, testCase := range testCases {
+		il := &ingress.Location{
+			ModSecurity: modsecurity.Config{
+				Enable:        testCase.isEnabledInLoc,
+				OWASPRules:    testCase.isOwaspEnabledInLoc,
+				Snippet:       testCase.snippet,
+				TransactionID: testCase.transactionID,
+			},
+		}
+
+		actual := buildModSecurityForLocation(config.Configuration{
+			EnableModsecurity:    testCase.isEnabledInCM,
+			EnableOWASPCoreRules: testCase.isOwaspEnabledInCM,
+		}, il)
+
+		if testCase.expected != actual {
+			t.Errorf("%v: expected '%v' but returned '%v'", testCase.description, testCase.expected, actual)
+		}
+	}
+}

rootfs/etc/nginx/template/nginx.tmpl

@@ -1029,25 +1029,7 @@ stream {
 
             set $proxy_alternative_upstream_name "";
 
-            {{ if (or $location.ModSecurity.Enable $all.Cfg.EnableModsecurity) }}
+            {{ buildModSecurityForLocation $all.Cfg $location }}
-            {{ if not $all.Cfg.EnableModsecurity }}
-            modsecurity on;
-
-            modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
-            {{ end }}
-
-            {{ if $location.ModSecurity.Snippet }}
-            modsecurity_rules '
-                {{ $location.ModSecurity.Snippet }}
-            ';
-            {{ else if (and (not $all.Cfg.EnableOWASPCoreRules) ($location.ModSecurity.OWASPRules))}}
-            modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf;
-            {{ end }}
-
-            {{ if (not (empty $location.ModSecurity.TransactionID)) }}
-            modsecurity_transaction_id {{ $location.ModSecurity.TransactionID | quote }};
-            {{ end }}
-            {{ end }}
 
             {{ if isLocationAllowed $location }}
             {{ if gt (len $location.Whitelist.CIDR) 0 }}