Disable user snippets per default (#10393)

* Disable user snippets per default

* Enable snippet on tests

.gitignore

@@ -60,3 +60,4 @@ cmd/plugin/release/ingress-nginx.yaml
 cmd/plugin/release/*.tar.gz
 cmd/plugin/release/LICENSE
 tmp/
+test/junitreports/

charts/ingress-nginx/README.md

@@ -274,7 +274,7 @@ As of version `1.26.0` of this chart, by simply not providing any clusterIP valu
 | controller.admissionWebhooks.service.servicePort | int | `443` |  |
 | controller.admissionWebhooks.service.type | string | `"ClusterIP"` |  |
 | controller.affinity | object | `{}` | Affinity and anti-affinity rules for server scheduling to nodes # Ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#affinity-and-anti-affinity # |
-| controller.allowSnippetAnnotations | bool | `true` | This configuration defines if Ingress Controller should allow users to set their own *-snippet annotations, otherwise this is forbidden / dropped when users add those annotations. Global snippets in ConfigMap are still respected |
+| controller.allowSnippetAnnotations | bool | `false` | This configuration defines if Ingress Controller should allow users to set their own *-snippet annotations, otherwise this is forbidden / dropped when users add those annotations. Global snippets in ConfigMap are still respected |
 | controller.annotations | object | `{}` | Annotations to be added to the controller Deployment or DaemonSet # |
 | controller.autoscaling.annotations | object | `{}` |  |
 | controller.autoscaling.behavior | object | `{}` |  |

charts/ingress-nginx/values.yaml

@@ -81,7 +81,7 @@ controller:
   # their own *-snippet annotations, otherwise this is forbidden / dropped
   # when users add those annotations.
   # Global snippets in ConfigMap are still respected
-  allowSnippetAnnotations: true
+  allowSnippetAnnotations: false
   # -- Required for use with CNI based kubernetes installations (such as ones set up by kubeadm),
   # since CNI and hostport don't mix yet. Can be deprecated once https://github.com/kubernetes/kubernetes/issues/23920
   # is merged

cmd/plugin/util/util.go

@@ -29,8 +29,8 @@ import (
 
 // The default deployment and service names for ingress-nginx
 const (
-	DefaultIngressDeploymentName = "ingress-nginx-controller"
-	DefaultIngressServiceName    = "ingress-nginx-controller"
+	DefaultIngressDeploymentName = "ingress-nginx-controller" //#nosec G101
+	DefaultIngressServiceName    = "ingress-nginx-controller" //#nosec G101
 	DefaultIngressContainerName  = "controller"
 )
 

docs/user-guide/nginx-configuration/configmap.md

@@ -30,7 +30,7 @@ The following table shows a configuration option's name, type, and the default v
 |[add-headers](#add-headers)|string|""||
 |[allow-backend-server-header](#allow-backend-server-header)|bool|"false"||
 |[allow-cross-namespace-resources](#allow-cross-namespace-resources)|bool|"true"||
-|[allow-snippet-annotations](#allow-snippet-annotations)|bool|true||
+|[allow-snippet-annotations](#allow-snippet-annotations)|bool|false||
 |[annotations-risk-level](#annotations-risk-level)|string|Critical||
 |[annotation-value-word-blocklist](#annotation-value-word-blocklist)|string array|""||
 |[hide-headers](#hide-headers)|string array|empty||
@@ -257,7 +257,7 @@ Enables users to consume cross namespace resource on annotations, when was previ
 
 ## allow-snippet-annotations
 
-Enables Ingress to parse and add *-snippet annotations/directives created by the user. _**default:**_ `true`
+Enables Ingress to parse and add *-snippet annotations/directives created by the user. _**default:**_ `false`
 
 Warning: We recommend enabling this option only if you TRUST users with permission to create Ingress objects, as this
 may allow a user to add restricted configurations to the final nginx.conf file

internal/ingress/annotations/fastcgi/main.go

@@ -32,7 +32,7 @@ import (
 
 const (
 	fastCGIIndexAnnotation  = "fastcgi-index"
-	fastCGIParamsAnnotation = "fastcgi-params-configmap"
+	fastCGIParamsAnnotation = "fastcgi-params-configmap" //#nosec G101
 )
 
 // fast-cgi valid parameters is just a single file name (like index.php)

internal/ingress/annotations/modsecurity/main_test.go

@@ -67,7 +67,7 @@ func TestParse(t *testing.T) {
 		Spec: networking.IngressSpec{},
 	}
 
-	for _, testCase := range testCases {
+	for i, testCase := range testCases {
 		ing.SetAnnotations(testCase.annotations)
 		result, err := ap.Parse(ing)
 		if err != nil {
@@ -77,7 +77,7 @@ func TestParse(t *testing.T) {
 		if !ok {
 			t.Errorf("unexpected type: %T", result)
 		}
-		if !config.Equal(&testCase.expected) {
+		if !config.Equal(&testCases[i].expected) {
 			t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations)
 		}
 	}

internal/ingress/annotations/proxy/main.go

@@ -42,7 +42,7 @@ const (
 	proxyRedirectToAnnotation          = "proxy-redirect-to"
 	proxyBufferingAnnotation           = "proxy-buffering"
 	proxyHTTPVersionAnnotation         = "proxy-http-version"
-	proxyMaxTempFileSizeAnnotation     = "proxy-max-temp-file-size"
+	proxyMaxTempFileSizeAnnotation     = "proxy-max-temp-file-size" //#nosec G101
 )
 
 var validUpstreamAnnotation = regexp.MustCompile(`^((error|timeout|invalid_header|http_500|http_502|http_503|http_504|http_403|http_404|http_429|non_idempotent|off)\s?)+$`)

internal/ingress/annotations/sslcipher/main_test.go

@@ -64,13 +64,13 @@ func TestParse(t *testing.T) {
 		Spec: networking.IngressSpec{},
 	}
 
-	for _, testCase := range testCases {
+	for i, testCase := range testCases {
 		ing.SetAnnotations(testCase.annotations)
 		result, err := ap.Parse(ing)
 		if (err != nil) != testCase.expectErr {
 			t.Fatalf("expected error: %t got error: %t err value: %s. %+v", testCase.expectErr, err != nil, err, testCase.annotations)
 		}
-		if !reflect.DeepEqual(result, &testCase.expected) {
+		if !reflect.DeepEqual(result, &testCases[i].expected) {
 			t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations)
 		}
 	}

internal/ingress/controller/config/config.go

@@ -865,7 +865,7 @@ func NewDefault() Configuration {
 	defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}, false}
 
 	cfg := Configuration{
-		AllowSnippetAnnotations:          true,
+		AllowSnippetAnnotations:          false,
 		AllowCrossNamespaceResources:     true,
 		AllowBackendServerHeader:         false,
 		AnnotationValueWordBlocklist:     "",

internal/ingress/controller/controller.go

@@ -1057,7 +1057,7 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B
 				continue
 			}
 
-			for _, path := range rule.HTTP.Paths {
+			for i, path := range rule.HTTP.Paths {
 				if path.Backend.Service == nil {
 					// skip non-service backends
 					klog.V(3).Infof("Ingress %q and path %q does not contain a service backend, using default backend", ingKey, path.Path)
@@ -1087,7 +1087,7 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B
 
 				// add the service ClusterIP as a single Endpoint instead of individual Endpoints
 				if anns.ServiceUpstream {
-					endpoint, err := n.getServiceClusterEndpoint(svcKey, &path.Backend)
+					endpoint, err := n.getServiceClusterEndpoint(svcKey, &rule.HTTP.Paths[i].Backend)
 					if err != nil {
 						klog.Errorf("Failed to determine a suitable ClusterIP Endpoint for Service %q: %v", svcKey, err)
 					} else {
@@ -1844,7 +1844,7 @@ func ingressForHostPath(hostname, path string, servers []*ingress.Server) []*net
 			continue
 		}
 
-		for _, location := range server.Locations {
+		for i, location := range server.Locations {
 			if location.Path != path {
 				continue
 			}
@@ -1853,7 +1853,7 @@ func ingressForHostPath(hostname, path string, servers []*ingress.Server) []*net
 				continue
 			}
 
-			ingresses = append(ingresses, &location.Ingress.Ingress)
+			ingresses = append(ingresses, &server.Locations[i].Ingress.Ingress)
 		}
 	}
 

internal/ingress/metric/collectors/main.go

@@ -17,4 +17,4 @@ limitations under the License.
 package collectors
 
 // PrometheusNamespace default metric namespace
-var PrometheusNamespace = "nginx_ingress_controller"
+var PrometheusNamespace = "nginx_ingress_controller" //#nosec G101

internal/net/ssl/ssl.go

@@ -55,7 +55,7 @@ var FakeSSLCertificateUID = "00000000-0000-0000-0000-000000000000"
 var oidExtensionSubjectAltName = asn1.ObjectIdentifier{2, 5, 29, 17}
 
 const (
-	fakeCertificateName = "default-fake-certificate"
+	fakeCertificateName = "default-fake-certificate" //#nosec G101
 )
 
 // getPemFileName returns absolute file path and file name of pem cert related to given fullSecretName