Repository: kubernetes/ingress-nginx
Pull request #8051 (2021-12-18 13:36:47)

align bug report with feature request regarding kind documentation
<!--- Provide a general summary of your changes in the Title above --->
<!--- Please don't @-mention people in PR or commit messages (do so in an additional comment). --->




## What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

In the issue template for bugs we have a block that handles issues that are actually documentation related. We did not sync that with feature issue template back then. This PR aligns them. 

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] Documentation only

## Which issue/s this PR fixes
<!--
(optional, in `fixes #<issue number>` format, will close that issue when PR gets merged):

fixes #
-->

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->

## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.


k8s-ci-robot: @iamNoah1: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/ingress-nginx/pull/8051#" title="Author self-approved">iamNoah1</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **strongjz** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @strongjz` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fingress-nginx).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes/ingress-nginx/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["strongjz"]} -->
iamNoah1: /triage accepted 
/kind documentation 
/priority backlog

Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
index f266d0f29..9f8fd83ba 100644
--- a/.github/ISSUE_TEMPLATE/bug_report.md
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -131,6 +131,9 @@ kubectl exec -it -n ingress-nginx $POD_NAME -- curl -H 'Host: foo.bar' localhost
 
 **Anything else we need to know**:
 
-<!-- If this is actually about documentation, add `/kind documentation` below -->
+<!-- If this is actually about documentation, uncomment the following block -->
 
-/kind bug
+<!-- 
+/kind documentation
+/remove-kind bug
+-->
diff --git a/docs/index.md b/docs/index.md
index 182674947..b06141814 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -132,8 +132,10 @@ spec:
 ```
 And add the value "spec.ingressClassName=nginx" in your Ingress objects
 
-## I have multiple Ingress objects in my cluster. What should I do ?
-- If you don't care about ingress classes, or you have a lot of Ingress objects without any IngressClass configuration, you can run the ingress-controller with the flag `--watch-ingress-without-class=true`.
+
+## I have multiple ingress objects in my cluster. What should I do ?
+- If you have lot of ingress objects without ingressClass configuration, you can run the ingress-controller with the flag `--watch-ingress-without-class=true`.
+
 
 ### What is the flag '--watch-ingress-without-class' ?
 - Its a flag that is passed,as an argument, to the `nginx-ingress-controller` executable. In the configuration, it looks like this ;
diff --git a/docs/user-guide/multiple-ingress.md b/docs/user-guide/multiple-ingress.md
index b0125c3b2..f13574044 100644
--- a/docs/user-guide/multiple-ingress.md
+++ b/docs/user-guide/multiple-ingress.md
@@ -2,13 +2,14 @@
 
 By default, deploying multiple Ingress controllers (e.g., `ingress-nginx` & `gce`) will result in all controllers simultaneously racing to update Ingress status fields in confusing ways.
 
-To fix this problem, you can either use [IngressClasses](https://kubernetes.io/docs/concepts/services-networking/ingress/#ingress-class) (preferred) or use the `kubernetes.io/ingress.class` annotation (in deprecation).
+To fix this problem, use [IngressClasses](https://kubernetes.io/docs/concepts/services-networking/ingress/#ingress-class), the `kubernetes.io/ingress.class` annotation  is deprecated from kubernetes v1.22+.
 
 ## Using IngressClasses
 
 If all ingress controllers respect IngressClasses (e.g. multiple instances of ingress-nginx v1.0), you can deploy two Ingress controllers by granting them control over two different IngressClasses, then selecting one of the two IngressClasses with `ingressClassName`.
+When two or more 
 
-First, ensure the `--controller-class=` is set to something different on each ingress controller:
+First, ensure the `--controller-class=` and `--ingress-class` are set to something different on each ingress controller:
 
 ```yaml
 # ingress-nginx Deployment/Statfulset
@@ -19,7 +20,8 @@ spec:
          - name: ingress-nginx-internal-controller
            args:
              - /nginx-ingress-controller
-             - '--controller-class=k8s.io/internal-ingress-nginx'
+             - '--controller-class=k8s.io/internal-nginx'
+             - '--ingress-class=k8s.io/internal-nginx'
             ...
 ```
 
diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go
index ae5ec259a..e5be1490a 100644
--- a/internal/ingress/controller/template/template.go
+++ b/internal/ingress/controller/template/template.go
@@ -1537,7 +1537,7 @@ func buildModSecurityForLocation(cfg config.Configuration, location *ingress.Loc
 `, location.ModSecurity.TransactionID))
 	}
 
-	if !isMSEnabled {
+	if !isMSEnabled && location.ModSecurity.Snippet == "" {
 		buffer.WriteString(`modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
 `)
 	}
diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl
index c6e978ffe..0cc8d3cab 100755
--- a/rootfs/etc/nginx/template/nginx.tmpl
+++ b/rootfs/etc/nginx/template/nginx.tmpl
@@ -153,10 +153,10 @@ http {
     modsecurity_rules '
       {{ $all.Cfg.ModsecuritySnippet }}
     ';
-    {{ end }}
-
+    {{ else }}
     modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
-
+    {{ end }}
+    
     {{ if $all.Cfg.EnableOWASPCoreRules }}
     modsecurity_rules_file /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf;
     {{ end }}


Test patch:
diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go
index cfb65c08e..b65e33c32 100644
--- a/internal/ingress/controller/template/template_test.go
+++ b/internal/ingress/controller/template/template_test.go
@@ -1781,8 +1781,8 @@ func TestModSecurityForLocation(t *testing.T) {
 		{"configmap enabled, configmap OWASP enabled, annotation enabled, OWASP disabled", true, true, true, true, false, "", "", ""},
 		{"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, "", "", fmt.Sprintf("%v%v", loadModule, modSecCfg)},
 		{"configmap disabled, annotation disabled, OWASP disabled", false, false, false, true, false, "", "", ""},
-		{"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v%v", loadModule, modsecRule, modSecCfg)},
-		{"configmap disabled, annotation enabled, OWASP enabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v%v", loadModule, modsecRule, modSecCfg)},
+		{"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
+		{"configmap disabled, annotation enabled, OWASP enabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
 	}
 
 	for _, testCase := range testCases {
diff --git a/test/e2e/annotations/modsecurity/modsecurity.go b/test/e2e/annotations/modsecurity/modsecurity.go
index f88d6541e..4de85818d 100644
--- a/test/e2e/annotations/modsecurity/modsecurity.go
+++ b/test/e2e/annotations/modsecurity/modsecurity.go
@@ -342,4 +342,41 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
 			Expect().
 			Status(http.StatusOK)
 	})
+
+	ginkgo.It("should disable default modsecurity conf setting when modsecurity-snippet is specified", func() {
+		host := "modsecurity.foo.com"
+		nameSpace := f.Namespace
+
+		snippet := `SecRuleEngine On
+		SecRequestBodyAccess On
+		SecAuditEngine RelevantOnly
+		SecAuditLogParts ABIJDEFHZ
+		SecAuditLogType Concurrent
+		SecAuditLog /var/tmp/modsec_audit.log
+		SecAuditLogStorageDir /var/tmp/
+		SecRule REQUEST_HEADERS:User-Agent \"block-ua\" \"log,deny,id:107,status:403,msg:\'UA blocked\'\"`
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-modsecurity":  "true",
+			"nginx.ingress.kubernetes.io/modsecurity-snippet": snippet,
+		}
+		f.UpdateNginxConfigMapData("annotation-value-word-blocklist", "load_module, lua_package, _by_lua, location, root, {, }")
+		// Sleep a while just to guarantee that the configmap is applied
+		framework.Sleep()
+		ing := framework.NewSingleIngress(host, "/", host, nameSpace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return !strings.Contains(server, "modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;") &&
+					strings.Contains(server, "SecAuditLog /var/tmp/modsec_audit.log")
+			})
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			WithHeader("User-Agent", "block-ua").
+			Expect().
+			Status(http.StatusForbidden)
+	})
 })