From 7d1c47ab5408a40993993b1b47954a4fd44d39f3 Mon Sep 17 00:00:00 2001 From: James Strong Date: Tue, 31 Jan 2023 20:09:06 -0500 Subject: [PATCH] Switch logic on path type validation and setting it to false (#9543) * update path type validation to be false and update e2e test scripts Signed-off-by: James Strong * update to make tests clear Signed-off-by: James Strong * update test params Signed-off-by: James Strong * Adding else per pr comments Signed-off-by: James Strong --------- Signed-off-by: James Strong Signed-off-by: James Strong --- Makefile | 4 +- charts/ingress-nginx/README.md | 2 +- .../templates/controller-configmap.yaml | 2 +- charts/ingress-nginx/values.yaml | 6 +- .../nginx-configuration/configmap.md | 394 +++++++++--------- internal/ingress/controller/config/config.go | 11 +- internal/ingress/controller/controller.go | 2 +- .../ingress/controller/controller_test.go | 6 +- internal/ingress/controller/store/store.go | 2 +- pkg/util/ingress/ingress.go | 43 +- pkg/util/ingress/ingress_test.go | 66 +-- test/e2e/admission/admission.go | 28 +- {build => test/e2e}/run-e2e-suite.sh | 15 +- test/e2e/{run.sh => run-kind-e2e.sh} | 92 ++-- 14 files changed, 354 insertions(+), 319 deletions(-) rename {build => test/e2e}/run-e2e-suite.sh (97%) rename test/e2e/{run.sh => run-kind-e2e.sh} (62%) diff --git a/Makefile b/Makefile index 0ce3e3cef..25855a224 100644 --- a/Makefile +++ b/Makefile @@ -153,11 +153,11 @@ lua-test: ## Run lua unit tests. .PHONY: e2e-test e2e-test: ## Run e2e tests (expects access to a working Kubernetes cluster). - @build/run-e2e-suite.sh + @test/e2e/run-e2e-suite.sh .PHONY: kind-e2e-test kind-e2e-test: ## Run e2e tests using kind. - @test/e2e/run.sh + @test/e2e/run-kind-e2e.sh .PHONY: kind-e2e-chart-tests kind-e2e-chart-tests: ## Run helm chart e2e tests diff --git a/charts/ingress-nginx/README.md b/charts/ingress-nginx/README.md index 174a8870d..7f3ea1866 100644 --- a/charts/ingress-nginx/README.md +++ b/charts/ingress-nginx/README.md @@ -253,6 +253,7 @@ Kubernetes: `>=1.20.0-0` | Key | Type | Default | Description | |-----|------|---------|-------------| | commonLabels | object | `{}` | | +| controller.EnablePathTypeValidation | bool | `false` | This configuration defines if Ingress Controller should validate pathType. If false, special characters will be allowed on paths of any pathType. If true, special characters are only allowed on paths with pathType = ImplementationSpecific | | controller.addHeaders | object | `{}` | Will add custom headers before sending response traffic to the client according to: https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#add-headers | | controller.admissionWebhooks.annotations | object | `{}` | | | controller.admissionWebhooks.certManager.admissionCert.duration | string | `""` | | @@ -311,7 +312,6 @@ Kubernetes: `>=1.20.0-0` | controller.containerPort | object | `{"http":80,"https":443}` | Configures the ports that the nginx-controller listens on | | controller.customTemplate.configMapKey | string | `""` | | | controller.customTemplate.configMapName | string | `""` | | -| controller.disablePathTypeValidation | bool | `false` | This configuration defines if Ingress Controller should validate pathType. If this is true, special characters will be allowed on paths of any pathType. If false, special characters are only allowed on paths with pathType = ImplementationSpecific | | controller.dnsConfig | object | `{}` | Optionally customize the pod dnsConfig. | | controller.dnsPolicy | string | `"ClusterFirst"` | Optionally change this to ClusterFirstWithHostNet in case you have 'hostNetwork: true'. By default, while using host network, name resolution uses the host's DNS. If you wish nginx-controller to keep resolving names inside the k8s network, use ClusterFirstWithHostNet. | | controller.electionID | string | `""` | Election ID to use for status update, by default it uses the controller name combined with a suffix of 'leader' | diff --git a/charts/ingress-nginx/templates/controller-configmap.yaml b/charts/ingress-nginx/templates/controller-configmap.yaml index ffd003ee8..a1fbdf549 100644 --- a/charts/ingress-nginx/templates/controller-configmap.yaml +++ b/charts/ingress-nginx/templates/controller-configmap.yaml @@ -14,7 +14,7 @@ metadata: namespace: {{ .Release.Namespace }} data: allow-snippet-annotations: "{{ .Values.controller.allowSnippetAnnotations }}" - disable-pathtype-validation: "{{ .Values.controller.disablePathTypeValidation }}" + enable-pathtype-validation: "{{ .Values.controller.EnablePathTypeValidation }}" {{- if .Values.controller.addHeaders }} add-headers: {{ .Release.Namespace }}/{{ include "ingress-nginx.fullname" . }}-custom-add-headers {{- end }} diff --git a/charts/ingress-nginx/values.yaml b/charts/ingress-nginx/values.yaml index 072493ccc..4c587ee88 100644 --- a/charts/ingress-nginx/values.yaml +++ b/charts/ingress-nginx/values.yaml @@ -88,9 +88,9 @@ controller: allowSnippetAnnotations: true # -- This configuration defines if Ingress Controller should validate pathType. - # If this is true, special characters will be allowed on paths of any pathType. If - # false, special characters are only allowed on paths with pathType = ImplementationSpecific - disablePathTypeValidation: false + # If false, special characters will be allowed on paths of any pathType. + # If true, special characters are only allowed on paths with pathType = ImplementationSpecific + EnablePathTypeValidation: 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 diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index baefa7c72..3832bba16 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -25,199 +25,199 @@ data: The following table shows a configuration option's name, type, and the default value: -|name|type|default| -|:---|:---|:------| -|[add-headers](#add-headers)|string|""| -|[allow-backend-server-header](#allow-backend-server-header)|bool|"false"| -|[allow-snippet-annotations](#allow-snippet-annotations)|bool|true| -|[annotation-value-word-blocklist](#annotation-value-word-blocklist)|string array|""| -|[hide-headers](#hide-headers)|string array|empty| -|[access-log-params](#access-log-params)|string|""| -|[access-log-path](#access-log-path)|string|"/var/log/nginx/access.log"| -|[http-access-log-path](#http-access-log-path)|string|""| -|[stream-access-log-path](#stream-access-log-path)|string|""| -|[enable-access-log-for-default-backend](#enable-access-log-for-default-backend)|bool|"false"| -|[error-log-path](#error-log-path)|string|"/var/log/nginx/error.log"| -|[enable-modsecurity](#enable-modsecurity)|bool|"false"| -|[modsecurity-snippet](#modsecurity-snippet)|string|""| -|[enable-owasp-modsecurity-crs](#enable-owasp-modsecurity-crs)|bool|"false"| -|[client-header-buffer-size](#client-header-buffer-size)|string|"1k"| -|[client-header-timeout](#client-header-timeout)|int|60| -|[client-body-buffer-size](#client-body-buffer-size)|string|"8k"| -|[client-body-timeout](#client-body-timeout)|int|60| -|[disable-access-log](#disable-access-log)|bool|false| -|[disable-ipv6](#disable-ipv6)|bool|false| -|[disable-ipv6-dns](#disable-ipv6-dns)|bool|false| -|[enable-underscores-in-headers](#enable-underscores-in-headers)|bool|false| -|[enable-ocsp](#enable-ocsp)|bool|false| -|[ignore-invalid-headers](#ignore-invalid-headers)|bool|true| -|[retry-non-idempotent](#retry-non-idempotent)|bool|"false"| -|[error-log-level](#error-log-level)|string|"notice"| -|[http2-max-field-size](#http2-max-field-size)|string|"4k"| -|[http2-max-header-size](#http2-max-header-size)|string|"16k"| -|[http2-max-requests](#http2-max-requests)|int|1000| -|[http2-max-concurrent-streams](#http2-max-concurrent-streams)|int|128| -|[hsts](#hsts)|bool|"true"| -|[hsts-include-subdomains](#hsts-include-subdomains)|bool|"true"| -|[hsts-max-age](#hsts-max-age)|string|"15724800"| -|[hsts-preload](#hsts-preload)|bool|"false"| -|[keep-alive](#keep-alive)|int|75| -|[keep-alive-requests](#keep-alive-requests)|int|1000| -|[large-client-header-buffers](#large-client-header-buffers)|string|"4 8k"| -|[log-format-escape-none](#log-format-escape-none)|bool|"false"| -|[log-format-escape-json](#log-format-escape-json)|bool|"false"| -|[log-format-upstream](#log-format-upstream)|string|`$remote_addr - $remote_user [$time_local] "$request" $status $body_bytes_sent "$http_referer" "$http_user_agent" $request_length $request_time [$proxy_upstream_name] [$proxy_alternative_upstream_name] $upstream_addr $upstream_response_length $upstream_response_time $upstream_status $req_id`| -|[log-format-stream](#log-format-stream)|string|`[$remote_addr] [$time_local] $protocol $status $bytes_sent $bytes_received $session_time`| -|[enable-multi-accept](#enable-multi-accept)|bool|"true"| -|[max-worker-connections](#max-worker-connections)|int|16384| -|[max-worker-open-files](#max-worker-open-files)|int|0| -|[map-hash-bucket-size](#max-hash-bucket-size)|int|64| -|[nginx-status-ipv4-whitelist](#nginx-status-ipv4-whitelist)|[]string|"127.0.0.1"| -|[nginx-status-ipv6-whitelist](#nginx-status-ipv6-whitelist)|[]string|"::1"| -|[proxy-real-ip-cidr](#proxy-real-ip-cidr)|[]string|"0.0.0.0/0"| -|[proxy-set-headers](#proxy-set-headers)|string|""| -|[server-name-hash-max-size](#server-name-hash-max-size)|int|1024| -|[server-name-hash-bucket-size](#server-name-hash-bucket-size)|int|`` -|[proxy-headers-hash-max-size](#proxy-headers-hash-max-size)|int|512| -|[proxy-headers-hash-bucket-size](#proxy-headers-hash-bucket-size)|int|64| -|[plugins](#plugins)|[]string| | -|[reuse-port](#reuse-port)|bool|"true"| -|[server-tokens](#server-tokens)|bool|"false"| -|[ssl-ciphers](#ssl-ciphers)|string|"ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384"| -|[ssl-ecdh-curve](#ssl-ecdh-curve)|string|"auto"| -|[ssl-dh-param](#ssl-dh-param)|string|""| -|[ssl-protocols](#ssl-protocols)|string|"TLSv1.2 TLSv1.3"| -|[ssl-session-cache](#ssl-session-cache)|bool|"true"| -|[ssl-session-cache-size](#ssl-session-cache-size)|string|"10m"| -|[ssl-session-tickets](#ssl-session-tickets)|bool|"false"| -|[ssl-session-ticket-key](#ssl-session-ticket-key)|string|`` -|[ssl-session-timeout](#ssl-session-timeout)|string|"10m"| -|[ssl-buffer-size](#ssl-buffer-size)|string|"4k"| -|[use-proxy-protocol](#use-proxy-protocol)|bool|"false"| -|[proxy-protocol-header-timeout](#proxy-protocol-header-timeout)|string|"5s"| -|[use-gzip](#use-gzip)|bool|"false"| -|[use-geoip](#use-geoip)|bool|"true"| -|[use-geoip2](#use-geoip2)|bool|"false"| -|[enable-brotli](#enable-brotli)|bool|"false"| -|[brotli-level](#brotli-level)|int|4| -|[brotli-min-length](#brotli-min-length)|int|20| -|[brotli-types](#brotli-types)|string|"application/xml+rss application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/javascript text/plain text/x-component"| -|[use-http2](#use-http2)|bool|"true"| -|[gzip-disable](#gzip-disable)|string|""| -|[gzip-level](#gzip-level)|int|1| -|[gzip-min-length](#gzip-min-length)|int|256| -|[gzip-types](#gzip-types)|string|"application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/javascript text/plain text/x-component"| -|[worker-processes](#worker-processes)|string|``| -|[worker-cpu-affinity](#worker-cpu-affinity)|string|""| -|[worker-shutdown-timeout](#worker-shutdown-timeout)|string|"240s"| -|[load-balance](#load-balance)|string|"round_robin"| -|[variables-hash-bucket-size](#variables-hash-bucket-size)|int|128| -|[variables-hash-max-size](#variables-hash-max-size)|int|2048| -|[upstream-keepalive-connections](#upstream-keepalive-connections)|int|320| -|[upstream-keepalive-time](#upstream-keepalive-time)|string|"1h"| -|[upstream-keepalive-timeout](#upstream-keepalive-timeout)|int|60| -|[upstream-keepalive-requests](#upstream-keepalive-requests)|int|10000| -|[limit-conn-zone-variable](#limit-conn-zone-variable)|string|"$binary_remote_addr"| -|[proxy-stream-timeout](#proxy-stream-timeout)|string|"600s"| -|[proxy-stream-next-upstream](#proxy-stream-next-upstream)|bool|"true"| -|[proxy-stream-next-upstream-timeout](#proxy-stream-next-upstream-timeout)|string|"600s"| -|[proxy-stream-next-upstream-tries](#proxy-stream-next-upstream-tries)|int|3| -|[proxy-stream-responses](#proxy-stream-responses)|int|1| -|[bind-address](#bind-address)|[]string|""| -|[use-forwarded-headers](#use-forwarded-headers)|bool|"false"| -|[enable-real-ip](#enable-real-ip)|bool|"false"| -|[forwarded-for-header](#forwarded-for-header)|string|"X-Forwarded-For"| -|[compute-full-forwarded-for](#compute-full-forwarded-for)|bool|"false"| -|[proxy-add-original-uri-header](#proxy-add-original-uri-header)|bool|"false"| -|[generate-request-id](#generate-request-id)|bool|"true"| -|[enable-opentracing](#enable-opentracing)|bool|"false"| -|[opentracing-operation-name](#opentracing-operation-name)|string|""| -|[opentracing-location-operation-name](#opentracing-location-operation-name)|string|""| -|[zipkin-collector-host](#zipkin-collector-host)|string|""| -|[zipkin-collector-port](#zipkin-collector-port)|int|9411| -|[zipkin-service-name](#zipkin-service-name)|string|"nginx"| -|[zipkin-sample-rate](#zipkin-sample-rate)|float|1.0| -|[jaeger-collector-host](#jaeger-collector-host)|string|""| -|[jaeger-collector-port](#jaeger-collector-port)|int|6831| -|[jaeger-endpoint](#jaeger-endpoint)|string|""| -|[jaeger-service-name](#jaeger-service-name)|string|"nginx"| -|[jaeger-propagation-format](#jaeger-propagation-format)|string|"jaeger"| -|[jaeger-sampler-type](#jaeger-sampler-type)|string|"const"| -|[jaeger-sampler-param](#jaeger-sampler-param)|string|"1"| -|[jaeger-sampler-host](#jaeger-sampler-host)|string|"http://127.0.0.1"| -|[jaeger-sampler-port](#jaeger-sampler-port)|int|5778| -|[jaeger-trace-context-header-name](#jaeger-trace-context-header-name)|string|uber-trace-id| -|[jaeger-debug-header](#jaeger-debug-header)|string|uber-debug-id| -|[jaeger-baggage-header](#jaeger-baggage-header)|string|jaeger-baggage| -|[jaeger-trace-baggage-header-prefix](#jaeger-trace-baggage-header-prefix)|string|uberctx-| -|[datadog-collector-host](#datadog-collector-host)|string|""| -|[datadog-collector-port](#datadog-collector-port)|int|8126| -|[datadog-service-name](#datadog-service-name)|string|"nginx"| -|[datadog-environment](#datadog-environment)|string|"prod"| -|[datadog-operation-name-override](#datadog-operation-name-override)|string|"nginx.handle"| -|[datadog-priority-sampling](#datadog-priority-sampling)|bool|"true"| -|[datadog-sample-rate](#datadog-sample-rate)|float|1.0| -|[main-snippet](#main-snippet)|string|""| -|[http-snippet](#http-snippet)|string|""| -|[server-snippet](#server-snippet)|string|""| -|[stream-snippet](#stream-snippet)|string|""| -|[location-snippet](#location-snippet)|string|""| -|[custom-http-errors](#custom-http-errors)|[]int|[]int{}| -|[proxy-body-size](#proxy-body-size)|string|"1m"| -|[proxy-connect-timeout](#proxy-connect-timeout)|int|5| -|[proxy-read-timeout](#proxy-read-timeout)|int|60| -|[proxy-send-timeout](#proxy-send-timeout)|int|60| -|[proxy-buffers-number](#proxy-buffers-number)|int|4| -|[proxy-buffer-size](#proxy-buffer-size)|string|"4k"| -|[proxy-cookie-path](#proxy-cookie-path)|string|"off"| -|[proxy-cookie-domain](#proxy-cookie-domain)|string|"off"| -|[proxy-next-upstream](#proxy-next-upstream)|string|"error timeout"| -|[proxy-next-upstream-timeout](#proxy-next-upstream-timeout)|int|0| -|[proxy-next-upstream-tries](#proxy-next-upstream-tries)|int|3| -|[proxy-redirect-from](#proxy-redirect-from)|string|"off"| -|[proxy-request-buffering](#proxy-request-buffering)|string|"on"| -|[ssl-redirect](#ssl-redirect)|bool|"true"| -|[force-ssl-redirect](#force-ssl-redirect)|bool|"false"| -|[denylist-source-range](#denylist-source-range)|[]string|[]string{}| -|[whitelist-source-range](#whitelist-source-range)|[]string|[]string{}| -|[skip-access-log-urls](#skip-access-log-urls)|[]string|[]string{}| -|[limit-rate](#limit-rate)|int|0| -|[limit-rate-after](#limit-rate-after)|int|0| -|[lua-shared-dicts](#lua-shared-dicts)|string|""| -|[http-redirect-code](#http-redirect-code)|int|308| -|[proxy-buffering](#proxy-buffering)|string|"off"| -|[limit-req-status-code](#limit-req-status-code)|int|503| -|[limit-conn-status-code](#limit-conn-status-code)|int|503| -|[enable-syslog](#enable-syslog)|bool|false| -|[syslog-host](#syslog-host)|string|""| -|[syslog-port](#syslog-port)|int|514| -|[no-tls-redirect-locations](#no-tls-redirect-locations)|string|"/.well-known/acme-challenge"| -|[global-auth-url](#global-auth-url)|string|""| -|[global-auth-method](#global-auth-method)|string|""| -|[global-auth-signin](#global-auth-signin)|string|""| -|[global-auth-signin-redirect-param](#global-auth-signin-redirect-param)|string|"rd"| -|[global-auth-response-headers](#global-auth-response-headers)|string|""| -|[global-auth-request-redirect](#global-auth-request-redirect)|string|""| -|[global-auth-snippet](#global-auth-snippet)|string|""| -|[global-auth-cache-key](#global-auth-cache-key)|string|""| -|[global-auth-cache-duration](#global-auth-cache-duration)|string|"200 202 401 5m"| -|[no-auth-locations](#no-auth-locations)|string|"/.well-known/acme-challenge"| -|[block-cidrs](#block-cidrs)|[]string|""| -|[block-user-agents](#block-user-agents)|[]string|""| -|[block-referers](#block-referers)|[]string|""| -|[proxy-ssl-location-only](#proxy-ssl-location-only)|bool|"false"| -|[default-type](#default-type)|string|"text/html"| -|[global-rate-limit-memcached-host](#global-rate-limit)|string|""| -|[global-rate-limit-memcached-port](#global-rate-limit)|int|11211| -|[global-rate-limit-memcached-connect-timeout](#global-rate-limit)|int|50| -|[global-rate-limit-memcached-max-idle-timeout](#global-rate-limit)|int|10000| -|[global-rate-limit-memcached-pool-size](#global-rate-limit)|int|50| -|[global-rate-limit-status-code](#global-rate-limit)|int|429| -|[service-upstream](#service-upstream)|bool|"false"| -|[ssl-reject-handshake](#ssl-reject-handshake)|bool|"false"| -|[debug-connections](#debug-connections)|[]string|"127.0.0.1,1.1.1.1/24"| -|[disable-pathtype-validation](#disable-pathtype-validation)|bool|"false"| -|[path-additional-allowed-chars](#path-additional-allowed-chars)|string|"^%$[](){}*+?"| +| name | type | default | +|:--------------------------------------------------------------------------------|:-------------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| [add-headers](#add-headers) | string | "" | +| [allow-backend-server-header](#allow-backend-server-header) | bool | "false" | +| [allow-snippet-annotations](#allow-snippet-annotations) | bool | true | +| [annotation-value-word-blocklist](#annotation-value-word-blocklist) | string array | "" | +| [hide-headers](#hide-headers) | string array | empty | +| [access-log-params](#access-log-params) | string | "" | +| [access-log-path](#access-log-path) | string | "/var/log/nginx/access.log" | +| [http-access-log-path](#http-access-log-path) | string | "" | +| [stream-access-log-path](#stream-access-log-path) | string | "" | +| [enable-access-log-for-default-backend](#enable-access-log-for-default-backend) | bool | "false" | +| [error-log-path](#error-log-path) | string | "/var/log/nginx/error.log" | +| [enable-modsecurity](#enable-modsecurity) | bool | "false" | +| [modsecurity-snippet](#modsecurity-snippet) | string | "" | +| [enable-owasp-modsecurity-crs](#enable-owasp-modsecurity-crs) | bool | "false" | +| [client-header-buffer-size](#client-header-buffer-size) | string | "1k" | +| [client-header-timeout](#client-header-timeout) | int | 60 | +| [client-body-buffer-size](#client-body-buffer-size) | string | "8k" | +| [client-body-timeout](#client-body-timeout) | int | 60 | +| [disable-access-log](#disable-access-log) | bool | false | +| [disable-ipv6](#disable-ipv6) | bool | false | +| [disable-ipv6-dns](#disable-ipv6-dns) | bool | false | +| [enable-underscores-in-headers](#enable-underscores-in-headers) | bool | false | +| [enable-ocsp](#enable-ocsp) | bool | false | +| [ignore-invalid-headers](#ignore-invalid-headers) | bool | true | +| [retry-non-idempotent](#retry-non-idempotent) | bool | "false" | +| [error-log-level](#error-log-level) | string | "notice" | +| [http2-max-field-size](#http2-max-field-size) | string | "4k" | +| [http2-max-header-size](#http2-max-header-size) | string | "16k" | +| [http2-max-requests](#http2-max-requests) | int | 1000 | +| [http2-max-concurrent-streams](#http2-max-concurrent-streams) | int | 128 | +| [hsts](#hsts) | bool | "true" | +| [hsts-include-subdomains](#hsts-include-subdomains) | bool | "true" | +| [hsts-max-age](#hsts-max-age) | string | "15724800" | +| [hsts-preload](#hsts-preload) | bool | "false" | +| [keep-alive](#keep-alive) | int | 75 | +| [keep-alive-requests](#keep-alive-requests) | int | 1000 | +| [large-client-header-buffers](#large-client-header-buffers) | string | "4 8k" | +| [log-format-escape-none](#log-format-escape-none) | bool | "false" | +| [log-format-escape-json](#log-format-escape-json) | bool | "false" | +| [log-format-upstream](#log-format-upstream) | string | `$remote_addr - $remote_user [$time_local] "$request" $status $body_bytes_sent "$http_referer" "$http_user_agent" $request_length $request_time [$proxy_upstream_name] [$proxy_alternative_upstream_name] $upstream_addr $upstream_response_length $upstream_response_time $upstream_status $req_id` | +| [log-format-stream](#log-format-stream) | string | `[$remote_addr] [$time_local] $protocol $status $bytes_sent $bytes_received $session_time` | +| [enable-multi-accept](#enable-multi-accept) | bool | "true" | +| [max-worker-connections](#max-worker-connections) | int | 16384 | +| [max-worker-open-files](#max-worker-open-files) | int | 0 | +| [map-hash-bucket-size](#max-hash-bucket-size) | int | 64 | +| [nginx-status-ipv4-whitelist](#nginx-status-ipv4-whitelist) | []string | "127.0.0.1" | +| [nginx-status-ipv6-whitelist](#nginx-status-ipv6-whitelist) | []string | "::1" | +| [proxy-real-ip-cidr](#proxy-real-ip-cidr) | []string | "0.0.0.0/0" | +| [proxy-set-headers](#proxy-set-headers) | string | "" | +| [server-name-hash-max-size](#server-name-hash-max-size) | int | 1024 | +| [server-name-hash-bucket-size](#server-name-hash-bucket-size) | int | `` | +| [proxy-headers-hash-max-size](#proxy-headers-hash-max-size) | int | 512 | +| [proxy-headers-hash-bucket-size](#proxy-headers-hash-bucket-size) | int | 64 | +| [plugins](#plugins) | []string | | +| [reuse-port](#reuse-port) | bool | "true" | +| [server-tokens](#server-tokens) | bool | "false" | +| [ssl-ciphers](#ssl-ciphers) | string | "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384" | +| [ssl-ecdh-curve](#ssl-ecdh-curve) | string | "auto" | +| [ssl-dh-param](#ssl-dh-param) | string | "" | +| [ssl-protocols](#ssl-protocols) | string | "TLSv1.2 TLSv1.3" | +| [ssl-session-cache](#ssl-session-cache) | bool | "true" | +| [ssl-session-cache-size](#ssl-session-cache-size) | string | "10m" | +| [ssl-session-tickets](#ssl-session-tickets) | bool | "false" | +| [ssl-session-ticket-key](#ssl-session-ticket-key) | string | `` | +| [ssl-session-timeout](#ssl-session-timeout) | string | "10m" | +| [ssl-buffer-size](#ssl-buffer-size) | string | "4k" | +| [use-proxy-protocol](#use-proxy-protocol) | bool | "false" | +| [proxy-protocol-header-timeout](#proxy-protocol-header-timeout) | string | "5s" | +| [use-gzip](#use-gzip) | bool | "false" | +| [use-geoip](#use-geoip) | bool | "true" | +| [use-geoip2](#use-geoip2) | bool | "false" | +| [enable-brotli](#enable-brotli) | bool | "false" | +| [brotli-level](#brotli-level) | int | 4 | +| [brotli-min-length](#brotli-min-length) | int | 20 | +| [brotli-types](#brotli-types) | string | "application/xml+rss application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/javascript text/plain text/x-component" | +| [use-http2](#use-http2) | bool | "true" | +| [gzip-disable](#gzip-disable) | string | "" | +| [gzip-level](#gzip-level) | int | 1 | +| [gzip-min-length](#gzip-min-length) | int | 256 | +| [gzip-types](#gzip-types) | string | "application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/javascript text/plain text/x-component" | +| [worker-processes](#worker-processes) | string | `` | +| [worker-cpu-affinity](#worker-cpu-affinity) | string | "" | +| [worker-shutdown-timeout](#worker-shutdown-timeout) | string | "240s" | +| [load-balance](#load-balance) | string | "round_robin" | +| [variables-hash-bucket-size](#variables-hash-bucket-size) | int | 128 | +| [variables-hash-max-size](#variables-hash-max-size) | int | 2048 | +| [upstream-keepalive-connections](#upstream-keepalive-connections) | int | 320 | +| [upstream-keepalive-time](#upstream-keepalive-time) | string | "1h" | +| [upstream-keepalive-timeout](#upstream-keepalive-timeout) | int | 60 | +| [upstream-keepalive-requests](#upstream-keepalive-requests) | int | 10000 | +| [limit-conn-zone-variable](#limit-conn-zone-variable) | string | "$binary_remote_addr" | +| [proxy-stream-timeout](#proxy-stream-timeout) | string | "600s" | +| [proxy-stream-next-upstream](#proxy-stream-next-upstream) | bool | "true" | +| [proxy-stream-next-upstream-timeout](#proxy-stream-next-upstream-timeout) | string | "600s" | +| [proxy-stream-next-upstream-tries](#proxy-stream-next-upstream-tries) | int | 3 | +| [proxy-stream-responses](#proxy-stream-responses) | int | 1 | +| [bind-address](#bind-address) | []string | "" | +| [use-forwarded-headers](#use-forwarded-headers) | bool | "false" | +| [enable-real-ip](#enable-real-ip) | bool | "false" | +| [forwarded-for-header](#forwarded-for-header) | string | "X-Forwarded-For" | +| [compute-full-forwarded-for](#compute-full-forwarded-for) | bool | "false" | +| [proxy-add-original-uri-header](#proxy-add-original-uri-header) | bool | "false" | +| [generate-request-id](#generate-request-id) | bool | "true" | +| [enable-opentracing](#enable-opentracing) | bool | "false" | +| [opentracing-operation-name](#opentracing-operation-name) | string | "" | +| [opentracing-location-operation-name](#opentracing-location-operation-name) | string | "" | +| [zipkin-collector-host](#zipkin-collector-host) | string | "" | +| [zipkin-collector-port](#zipkin-collector-port) | int | 9411 | +| [zipkin-service-name](#zipkin-service-name) | string | "nginx" | +| [zipkin-sample-rate](#zipkin-sample-rate) | float | 1.0 | +| [jaeger-collector-host](#jaeger-collector-host) | string | "" | +| [jaeger-collector-port](#jaeger-collector-port) | int | 6831 | +| [jaeger-endpoint](#jaeger-endpoint) | string | "" | +| [jaeger-service-name](#jaeger-service-name) | string | "nginx" | +| [jaeger-propagation-format](#jaeger-propagation-format) | string | "jaeger" | +| [jaeger-sampler-type](#jaeger-sampler-type) | string | "const" | +| [jaeger-sampler-param](#jaeger-sampler-param) | string | "1" | +| [jaeger-sampler-host](#jaeger-sampler-host) | string | "http://127.0.0.1" | +| [jaeger-sampler-port](#jaeger-sampler-port) | int | 5778 | +| [jaeger-trace-context-header-name](#jaeger-trace-context-header-name) | string | uber-trace-id | +| [jaeger-debug-header](#jaeger-debug-header) | string | uber-debug-id | +| [jaeger-baggage-header](#jaeger-baggage-header) | string | jaeger-baggage | +| [jaeger-trace-baggage-header-prefix](#jaeger-trace-baggage-header-prefix) | string | uberctx- | +| [datadog-collector-host](#datadog-collector-host) | string | "" | +| [datadog-collector-port](#datadog-collector-port) | int | 8126 | +| [datadog-service-name](#datadog-service-name) | string | "nginx" | +| [datadog-environment](#datadog-environment) | string | "prod" | +| [datadog-operation-name-override](#datadog-operation-name-override) | string | "nginx.handle" | +| [datadog-priority-sampling](#datadog-priority-sampling) | bool | "true" | +| [datadog-sample-rate](#datadog-sample-rate) | float | 1.0 | +| [main-snippet](#main-snippet) | string | "" | +| [http-snippet](#http-snippet) | string | "" | +| [server-snippet](#server-snippet) | string | "" | +| [stream-snippet](#stream-snippet) | string | "" | +| [location-snippet](#location-snippet) | string | "" | +| [custom-http-errors](#custom-http-errors) | []int | []int{} | +| [proxy-body-size](#proxy-body-size) | string | "1m" | +| [proxy-connect-timeout](#proxy-connect-timeout) | int | 5 | +| [proxy-read-timeout](#proxy-read-timeout) | int | 60 | +| [proxy-send-timeout](#proxy-send-timeout) | int | 60 | +| [proxy-buffers-number](#proxy-buffers-number) | int | 4 | +| [proxy-buffer-size](#proxy-buffer-size) | string | "4k" | +| [proxy-cookie-path](#proxy-cookie-path) | string | "off" | +| [proxy-cookie-domain](#proxy-cookie-domain) | string | "off" | +| [proxy-next-upstream](#proxy-next-upstream) | string | "error timeout" | +| [proxy-next-upstream-timeout](#proxy-next-upstream-timeout) | int | 0 | +| [proxy-next-upstream-tries](#proxy-next-upstream-tries) | int | 3 | +| [proxy-redirect-from](#proxy-redirect-from) | string | "off" | +| [proxy-request-buffering](#proxy-request-buffering) | string | "on" | +| [ssl-redirect](#ssl-redirect) | bool | "true" | +| [force-ssl-redirect](#force-ssl-redirect) | bool | "false" | +| [denylist-source-range](#denylist-source-range) | []string | []string{} | +| [whitelist-source-range](#whitelist-source-range) | []string | []string{} | +| [skip-access-log-urls](#skip-access-log-urls) | []string | []string{} | +| [limit-rate](#limit-rate) | int | 0 | +| [limit-rate-after](#limit-rate-after) | int | 0 | +| [lua-shared-dicts](#lua-shared-dicts) | string | "" | +| [http-redirect-code](#http-redirect-code) | int | 308 | +| [proxy-buffering](#proxy-buffering) | string | "off" | +| [limit-req-status-code](#limit-req-status-code) | int | 503 | +| [limit-conn-status-code](#limit-conn-status-code) | int | 503 | +| [enable-syslog](#enable-syslog) | bool | false | +| [syslog-host](#syslog-host) | string | "" | +| [syslog-port](#syslog-port) | int | 514 | +| [no-tls-redirect-locations](#no-tls-redirect-locations) | string | "/.well-known/acme-challenge" | +| [global-auth-url](#global-auth-url) | string | "" | +| [global-auth-method](#global-auth-method) | string | "" | +| [global-auth-signin](#global-auth-signin) | string | "" | +| [global-auth-signin-redirect-param](#global-auth-signin-redirect-param) | string | "rd" | +| [global-auth-response-headers](#global-auth-response-headers) | string | "" | +| [global-auth-request-redirect](#global-auth-request-redirect) | string | "" | +| [global-auth-snippet](#global-auth-snippet) | string | "" | +| [global-auth-cache-key](#global-auth-cache-key) | string | "" | +| [global-auth-cache-duration](#global-auth-cache-duration) | string | "200 202 401 5m" | +| [no-auth-locations](#no-auth-locations) | string | "/.well-known/acme-challenge" | +| [block-cidrs](#block-cidrs) | []string | "" | +| [block-user-agents](#block-user-agents) | []string | "" | +| [block-referers](#block-referers) | []string | "" | +| [proxy-ssl-location-only](#proxy-ssl-location-only) | bool | "false" | +| [default-type](#default-type) | string | "text/html" | +| [global-rate-limit-memcached-host](#global-rate-limit) | string | "" | +| [global-rate-limit-memcached-port](#global-rate-limit) | int | 11211 | +| [global-rate-limit-memcached-connect-timeout](#global-rate-limit) | int | 50 | +| [global-rate-limit-memcached-max-idle-timeout](#global-rate-limit) | int | 10000 | +| [global-rate-limit-memcached-pool-size](#global-rate-limit) | int | 50 | +| [global-rate-limit-status-code](#global-rate-limit) | int | 429 | +| [service-upstream](#service-upstream) | bool | "false" | +| [ssl-reject-handshake](#ssl-reject-handshake) | bool | "false" | +| [debug-connections](#debug-connections) | []string | "127.0.0.1,1.1.1.1/24" | +| [enable-pathtype-validation](#enable-pathtype-validation) | bool | "false" | +| [path-additional-allowed-chars](#path-additional-allowed-chars) | string | "^%$[](){}*+?" | ## add-headers @@ -1329,7 +1329,7 @@ _**default:**_ "" _References:_ [http://nginx.org/en/docs/ngx_core_module.html#debug_connection](http://nginx.org/en/docs/ngx_core_module.html#debug_connection) -## disable-pathtype-validation +## enable-pathtype-validation Ingress Controller validates the pathType, and only allows special characters on "path" if pathType is ImplementationSpecific. @@ -1339,13 +1339,11 @@ will be 0-9, a-z, A-Z, "-", ".", "_", "~", "/". If the validation is disabled, the [#path-additional-allowed-chars](#path-additional-allowed-chars) will be allowed on any pathType. -This behavior can be disabled, so special characters are accepted regardless of pathType +This behavior is disabled by default, so special characters are accepted regardless of pathType _**default:**_ "false" ## path-additional-allowed-chars -When validating path on Ingress resources, defines the additional set of special characters that +When [enable-pathtype-validation](enable-pathtype-validation) is set to true [#path-additional-allowed-chars](#path-additional-allowed-chars) defines the additional set of special characters that will be allowed. -See also [#disable-pathtype-validation](#disable-pathtype-validation). - _**default:**_ "^%$[](){}*+?|" diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 8be92167d..b02e5998e 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -783,14 +783,15 @@ type Configuration struct { // Default: "" DebugConnections []string `json:"debug-connections"` - // DisablePathTypeValidation allows the admin to disable the pathType validation. - // If PathTypeValidation is enabled, the Controller will only allow alphanumeric + // EnablePathTypeValidation allows the admin to enable the pathType validation. + // If EnablePathTypeValidation is enabled, the Controller will only allow alphanumeric // characters on path (0-9, a-z, A-Z, "-", ".", "_", "~", "/") - DisablePathTypeValidation bool `json:"disable-pathtype-validation"` + // to control what characters are allowed set them with PathAdditionalAllowedChars + EnablePathTypeValidation bool `json:"enable-pathtype-validation"` // PathAdditionalAllowedChars allows the admin to specify what are the additional // characters allowed in case of pathType=ImplementationSpecific. - // Case disable-pathtype-validation=true, this characters will be allowed on any path. + // Case enable-pathtype-validation=true, this characters will be only allowed on ImplementationSpecific. // Defaults to: "^%$[](){}*+?" PathAdditionalAllowedChars string `json:"path-additional-allowed-chars"` } @@ -828,7 +829,7 @@ func NewDefault() Configuration { ClientHeaderTimeout: 60, ClientBodyBufferSize: "8k", ClientBodyTimeout: 60, - DisablePathTypeValidation: false, + EnablePathTypeValidation: false, PathAdditionalAllowedChars: "^%$[](){}*+?|", EnableUnderscoresInHeaders: false, ErrorLogLevel: errorLevel, diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 5b22fd17a..9812bd97e 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -325,7 +325,7 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error { k8s.SetDefaultNGINXPathType(ing) - if err := utilingress.ValidateIngressPath(ing, cfg.DisablePathTypeValidation, cfg.PathAdditionalAllowedChars); err != nil { + if err := utilingress.ValidateIngressPath(ing, cfg.EnablePathTypeValidation, cfg.PathAdditionalAllowedChars); err != nil { return fmt.Errorf("ingress contains invalid characters: %s", err) } diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index d91760552..dab1e6e37 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -203,11 +203,11 @@ func TestCheckIngress(t *testing.T) { } t.Run("when validating pathType", func(t *testing.T) { - t.Run("When ingress contains invalid path and pathType validation is not disabled", func(t *testing.T) { + t.Run("When ingress contains invalid path and pathType validation is enabled", func(t *testing.T) { nginx.store = fakeIngressStore{ ingresses: []*ingress.Ingress{}, configuration: ngx_config.Configuration{ - DisablePathTypeValidation: false, + EnablePathTypeValidation: true, }, } nginx.command = testNginxTestCommand{ @@ -253,7 +253,7 @@ func TestCheckIngress(t *testing.T) { nginx.store = fakeIngressStore{ ingresses: []*ingress.Ingress{}, configuration: ngx_config.Configuration{ - DisablePathTypeValidation: true, + EnablePathTypeValidation: false, PathAdditionalAllowedChars: "^%$[](){}*+?|", }, } diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 1006858e8..e0f6cfb54 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -846,7 +846,7 @@ func (s *k8sStore) syncIngress(ing *networkingv1.Ingress) { copyIng := &networkingv1.Ingress{} ing.ObjectMeta.DeepCopyInto(©Ing.ObjectMeta) - if err := ingressutils.ValidateIngressPath(ing, s.backendConfig.DisablePathTypeValidation, s.backendConfig.PathAdditionalAllowedChars); err != nil { + if err := ingressutils.ValidateIngressPath(ing, s.backendConfig.EnablePathTypeValidation, s.backendConfig.PathAdditionalAllowedChars); err != nil { klog.Errorf("ingress %s contains invalid path and will be skipped: %s", key, err) return } diff --git a/pkg/util/ingress/ingress.go b/pkg/util/ingress/ingress.go index e16518251..acadad432 100644 --- a/pkg/util/ingress/ingress.go +++ b/pkg/util/ingress/ingress.go @@ -246,44 +246,65 @@ func BuildRedirects(servers []*ingress.Server) []*redirect { return redirectServers } -func ValidateIngressPath(copyIng *networkingv1.Ingress, disablePathTypeValidation bool, additionalChars string) error { +func ValidateIngressPath(copyIng *networkingv1.Ingress, enablePathTypeValidation bool, pathAdditionalAllowedChars string) error { if copyIng == nil { return nil } - escapedAdditionalChars := regexp.QuoteMeta(additionalChars) - regexPath, err := regexp.Compile("^[" + alphaNumericChars + escapedAdditionalChars + "]*$") + escapedPathAdditionalAllowedChars := regexp.QuoteMeta(pathAdditionalAllowedChars) + regexPath, err := regexp.Compile("^[" + alphaNumericChars + escapedPathAdditionalAllowedChars + "]*$") if err != nil { - return fmt.Errorf("ingress has misconfigured validation regex on configmap: %s - %w", additionalChars, err) + return fmt.Errorf("ingress has misconfigured validation regex on configmap: %s - %w", pathAdditionalAllowedChars, err) } for _, rule := range copyIng.Spec.Rules { + if rule.HTTP == nil { continue } - if err := checkPath(rule.HTTP.Paths, disablePathTypeValidation, regexPath); err != nil { + + if err := checkPath(rule.HTTP.Paths, enablePathTypeValidation, regexPath); err != nil { return fmt.Errorf("error validating ingressPath: %w", err) } } return nil } -func checkPath(paths []networkingv1.HTTPIngressPath, disablePathTypeValidation bool, regexSpecificChars *regexp.Regexp) error { +func checkPath(paths []networkingv1.HTTPIngressPath, enablePathTypeValidation bool, regexSpecificChars *regexp.Regexp) error { + for _, path := range paths { if path.PathType == nil { path.PathType = &defaultPathType } - if disablePathTypeValidation || *path.PathType == networkingv1.PathTypeImplementationSpecific { + klog.V(9).InfoS("PathType Validation", "enablePathTypeValidation", enablePathTypeValidation, "regexSpecificChars", regexSpecificChars.String(), "Path", path.Path) + + switch pathType := *path.PathType; pathType { + case networkingv1.PathTypeImplementationSpecific: + //only match on regex chars per Ingress spec when path is implementation specific if !regexSpecificChars.MatchString(path.Path) { return fmt.Errorf("path %s of type %s contains invalid characters", path.Path, *path.PathType) } - continue - } - if !pathAlphaNumericRegex(path.Path) { - return fmt.Errorf("path %s of type %s contains invalid characters", path.Path, *path.PathType) + case networkingv1.PathTypeExact, networkingv1.PathTypePrefix: + //enforce path type validation + if enablePathTypeValidation { + //only allow alphanumeric chars, no regex chars + if !pathAlphaNumericRegex(path.Path) { + return fmt.Errorf("path %s of type %s contains invalid characters", path.Path, *path.PathType) + } + continue + } else { + //path validation is disabled, so we check what regex chars are allowed by user + if !regexSpecificChars.MatchString(path.Path) { + return fmt.Errorf("path %s of type %s contains invalid characters", path.Path, *path.PathType) + } + continue + } + + default: + return fmt.Errorf("unknown path type %v on path %v", *path.PathType, path.Path) } } return nil diff --git a/pkg/util/ingress/ingress_test.go b/pkg/util/ingress/ingress_test.go index a79da6b6f..3bd7f1230 100644 --- a/pkg/util/ingress/ingress_test.go +++ b/pkg/util/ingress/ingress_test.go @@ -212,11 +212,11 @@ const ( func TestValidateIngressPath(t *testing.T) { tests := []struct { - name string - copyIng *networkingv1.Ingress - disablePathTypeValidation bool - additionalChars string - wantErr bool + name string + copyIng *networkingv1.Ingress + EnablePathTypeValidation bool + additionalChars string + wantErr bool }{ { name: "should return nil when ingress = nil", @@ -251,49 +251,53 @@ func TestValidateIngressPath(t *testing.T) { { name: "should deny path with bad characters and pathType not implementationSpecific", wantErr: true, - additionalChars: defaultAdditionalChars, + additionalChars: "()", copyIng: generateDumbIngressforPathTest(&pathTypeExact, "/foo/bar/(.+)"), }, { - name: "should accept path with regex characters and pathType implementationSpecific", - wantErr: false, - additionalChars: defaultAdditionalChars, - copyIng: generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.+)"), + name: "should accept path with regex characters and pathType implementationSpecific", + wantErr: false, + additionalChars: defaultAdditionalChars, + EnablePathTypeValidation: false, + copyIng: generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.+)"), }, { - name: "should accept path with regex characters and pathType exact, but pathType validation disabled", - wantErr: false, - additionalChars: defaultAdditionalChars, - disablePathTypeValidation: true, - copyIng: generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.+)"), + name: "should accept path with regex characters and pathType exact, but pathType validation disabled", + wantErr: false, + additionalChars: defaultAdditionalChars, + EnablePathTypeValidation: false, + copyIng: generateDumbIngressforPathTest(&pathTypeExact, "/foo/bar/(.+)"), }, { - name: "should reject path when the allowed additional set does not match", - wantErr: true, - additionalChars: "().?", - disablePathTypeValidation: false, - copyIng: generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.+)"), + name: "should reject path when the allowed additional set does not match", + wantErr: true, + additionalChars: "().?", + EnablePathTypeValidation: false, + copyIng: generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.+)"), }, { - name: "should accept path when the allowed additional set does match", - wantErr: false, - additionalChars: "().?", - copyIng: generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.?)"), + name: "should accept path when the allowed additional set does match", + wantErr: false, + additionalChars: "().?", + EnablePathTypeValidation: false, + copyIng: generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.?)"), }, { - name: "should block if at least one path is bad", - wantErr: true, - copyIng: generateComplexIngress(generateDumbIngressforPathTest(&pathTypeExact, "/foo/bar/(.?)")), + name: "should block if at least one path is bad", + wantErr: true, + EnablePathTypeValidation: false, + copyIng: generateComplexIngress(generateDumbIngressforPathTest(&pathTypeExact, "/foo/bar/(.?)")), }, { - name: "should block if at least one path is bad", - wantErr: true, - copyIng: generateComplexIngress(generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.?)")), + name: "should block if at least one path is bad", + wantErr: true, + EnablePathTypeValidation: false, + copyIng: generateComplexIngress(generateDumbIngressforPathTest(&pathTypeImplSpecific, "/foo/bar/(.?)")), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := ValidateIngressPath(tt.copyIng, tt.disablePathTypeValidation, tt.additionalChars); (err != nil) != tt.wantErr { + if err := ValidateIngressPath(tt.copyIng, tt.EnablePathTypeValidation, tt.additionalChars); (err != nil) != tt.wantErr { t.Errorf("ValidateIngressPath() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/test/e2e/admission/admission.go b/test/e2e/admission/admission.go index 5c037a0c5..e0f55df4e 100644 --- a/test/e2e/admission/admission.go +++ b/test/e2e/admission/admission.go @@ -160,9 +160,22 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() { assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid annotation value should return an error") }) - ginkgo.It("should reject ingress with bad characters and pathType != ImplementationSpecific", func() { + ginkgo.It("ADMISSION should not validate characters on ingress when validation of pathType is disabled", func() { host := "admission-test" + f.UpdateNginxConfigMapData("enable-pathtype-validation", "false") + + firstIngress := framework.NewSingleIngress("first-ingress", "/xpto*", host, f.Namespace, framework.EchoService, 80, nil) + firstIngress.Spec.Rules[0].IngressRuleValue.HTTP.Paths[0].PathType = &pathPrefix + _, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{}) + assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress with regex chars on path and pathType validation disabled should be accepted") + }) + + ginkgo.It("ADMISSION should reject ingress with bad characters and pathType != ImplementationSpecific", func() { + host := "admission-test" + + f.UpdateNginxConfigMapData("enable-pathtype-validation", "true") + firstIngress := framework.NewSingleIngress("first-ingress", "/xpto*", host, f.Namespace, framework.EchoService, 80, nil) firstIngress.Spec.Rules[0].IngressRuleValue.HTTP.Paths[0].PathType = &pathPrefix _, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{}) @@ -175,18 +188,7 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() { }) - ginkgo.It("should not validate characters on ingress when validation of pathType is disabled", func() { - host := "admission-test" - - f.UpdateNginxConfigMapData("disable-pathtype-validation", "true") - - firstIngress := framework.NewSingleIngress("first-ingress", "/xpto*", host, f.Namespace, framework.EchoService, 80, nil) - firstIngress.Spec.Rules[0].IngressRuleValue.HTTP.Paths[0].PathType = &pathPrefix - _, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{}) - assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress with regex chars on path and pathType validation disabled should be accepted") - }) - - ginkgo.It("should return an error if there is a forbidden value in some annotation", func() { + ginkgo.It("ADMISSION should return an error if there is a forbidden value in some annotation", func() { host := "admission-test" annotations := map[string]string{ diff --git a/build/run-e2e-suite.sh b/test/e2e/run-e2e-suite.sh similarity index 97% rename from build/run-e2e-suite.sh rename to test/e2e/run-e2e-suite.sh index ae38b5fcc..7920c0523 100755 --- a/build/run-e2e-suite.sh +++ b/test/e2e/run-e2e-suite.sh @@ -14,10 +14,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -if ! [ -z "$DEBUG" ]; then +if [ -n "$DEBUG" ]; then set -x +else + trap cleanup EXIT fi +function cleanup { + kubectl delete pod e2e 2>/dev/null || true +} + set -o errexit set -o nounset set -o pipefail @@ -43,16 +49,11 @@ if [ "$missing" = true ]; then exit 1 fi -function cleanup { - kubectl delete pod e2e 2>/dev/null || true -} -trap cleanup EXIT - E2E_CHECK_LEAKS=${E2E_CHECK_LEAKS:-} FOCUS=${FOCUS:-.*} BASEDIR=$(dirname "$0") -NGINX_BASE_IMAGE=$(cat $BASEDIR/../NGINX_BASE) +NGINX_BASE_IMAGE=$(cat $BASEDIR/../../NGINX_BASE) export E2E_CHECK_LEAKS export FOCUS diff --git a/test/e2e/run.sh b/test/e2e/run-kind-e2e.sh similarity index 62% rename from test/e2e/run.sh rename to test/e2e/run-kind-e2e.sh index 17dad6c39..bb61706b4 100755 --- a/test/e2e/run.sh +++ b/test/e2e/run-kind-e2e.sh @@ -14,13 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -KIND_LOG_LEVEL="1" - -if ! [ -z $DEBUG ]; then - set -x - KIND_LOG_LEVEL="6" -fi - set -o errexit set -o nounset set -o pipefail @@ -31,45 +24,56 @@ cleanup() { fi kind delete cluster \ - --verbosity=${KIND_LOG_LEVEL} \ - --name ${KIND_CLUSTER_NAME} + --verbosity="${KIND_LOG_LEVEL}" \ + --name "${KIND_CLUSTER_NAME}" } -trap cleanup EXIT +DEBUG=${DEBUG:=false} +if [ "${DEBUG}" = "true" ]; then + set -x + KIND_LOG_LEVEL="6" +else + trap cleanup EXIT +fi + +KIND_LOG_LEVEL="1" +IS_CHROOT="${IS_CHROOT:-false}" export KIND_CLUSTER_NAME=${KIND_CLUSTER_NAME:-ingress-nginx-dev} +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +# Use 1.0.0-dev to make sure we use the latest configuration in the helm template +export TAG=1.0.0-dev +export ARCH=${ARCH:-amd64} +export REGISTRY=ingress-controller +NGINX_BASE_IMAGE=$(cat "$DIR"/../../NGINX_BASE) +export NGINX_BASE_IMAGE=$NGINX_BASE_IMAGE +export DOCKER_CLI_EXPERIMENTAL=enabled +export KUBECONFIG="${KUBECONFIG:-$HOME/.kube/kind-config-$KIND_CLUSTER_NAME}" +SKIP_INGRESS_IMAGE_CREATION="${SKIP_INGRESS_IMAGE_CREATION:-false}" +SKIP_E2E_IMAGE_CREATION="${SKIP_E2E_IMAGE_CREATION:=false}" +SKIP_CLUSTER_CREATION="${SKIP_CLUSTER_CREATION:-false}" if ! command -v kind --version &> /dev/null; then echo "kind is not installed. Use the package manager or visit the official site https://kind.sigs.k8s.io/" exit 1 fi -DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" - -# Use 1.0.0-dev to make sure we use the latest configuration in the helm template -export TAG=1.0.0-dev -export ARCH=${ARCH:-amd64} -export REGISTRY=ingress-controller - -NGINX_BASE_IMAGE=$(cat $DIR/../../NGINX_BASE) - echo "Running e2e with nginx base image ${NGINX_BASE_IMAGE}" -export NGINX_BASE_IMAGE=$NGINX_BASE_IMAGE - -export DOCKER_CLI_EXPERIMENTAL=enabled - -export KUBECONFIG="${KUBECONFIG:-$HOME/.kube/kind-config-$KIND_CLUSTER_NAME}" - -if [ "${SKIP_CLUSTER_CREATION:-false}" = "false" ]; then +if [ "${SKIP_CLUSTER_CREATION}" = "false" ]; then echo "[dev-env] creating Kubernetes cluster with kind" export K8S_VERSION=${K8S_VERSION:-v1.25.2@sha256:9be91e9e9cdf116809841fc77ebdb8845443c4c72fe5218f3ae9eb57fdb4bace} + # delete the cluster if it exists + if kind get clusters | grep "${KIND_CLUSTER_NAME}"; then + kind delete cluster --name "${KIND_CLUSTER_NAME}" + fi + kind create cluster \ - --verbosity=${KIND_LOG_LEVEL} \ - --name ${KIND_CLUSTER_NAME} \ - --config ${DIR}/kind.yaml \ + --verbosity="${KIND_LOG_LEVEL}" \ + --name "${KIND_CLUSTER_NAME}" \ + --config "${DIR}"/kind.yaml \ --retain \ --image "kindest/node:${K8S_VERSION}" @@ -77,16 +81,26 @@ if [ "${SKIP_CLUSTER_CREATION:-false}" = "false" ]; then kubectl get nodes -o wide fi -if [ "${SKIP_IMAGE_CREATION:-false}" = "false" ]; then +if [ "${SKIP_INGRESS_IMAGE_CREATION}" = "false" ]; then + echo "[dev-env] building image" + if [ "${IS_CHROOT}" = "true" ]; then + make -C "${DIR}"/../../ clean-image build image-chroot + docker tag ${REGISTRY}/controller-chroot:${TAG} ${REGISTRY}/controller:${TAG} + else + make -C "${DIR}"/../../ clean-image build image + fi + + echo "[dev-env] .. done building controller images" +fi + +if [ "${SKIP_E2E_IMAGE_CREATION}" = "false" ]; then if ! command -v ginkgo &> /dev/null; then go get github.com/onsi/ginkgo/v2/ginkgo@v2.6.1 fi - echo "[dev-env] building image" - make -C ${DIR}/../../ clean-image build image image-chroot echo "[dev-env] .. done building controller images" echo "[dev-env] now building e2e-image.." - make -C ${DIR}/../e2e-image image + make -C "${DIR}"/../e2e-image image echo "[dev-env] ..done building e2e-image" fi @@ -95,13 +109,7 @@ KIND_WORKERS=$(kind get nodes --name="${KIND_CLUSTER_NAME}" | grep worker | awk echo "[dev-env] copying docker images to cluster..." -kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes=${KIND_WORKERS} nginx-ingress-controller:e2e - -if [ "${IS_CHROOT:-false}" = "true" ]; then - docker tag ${REGISTRY}/controller-chroot:${TAG} ${REGISTRY}/controller:${TAG} -fi - -kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes=${KIND_WORKERS} ${REGISTRY}/controller:${TAG} - +kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes="${KIND_WORKERS}" nginx-ingress-controller:e2e +kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes="${KIND_WORKERS}" "${REGISTRY}"/controller:"${TAG}" echo "[dev-env] running e2e tests..." -make -C ${DIR}/../../ e2e-test +make -C "${DIR}"/../../ e2e-test \ No newline at end of file