Add feature toggle for custom error pages /metrics

.github/workflows/ci.yaml

@@ -193,12 +193,18 @@ jobs:
           export TAGNGINX=$(cat images/nginx/TAG)
           make BASE_IMAGE=registry.k8s.io/ingress-nginx/nginx:${TAGNGINX} clean-image build image image-chroot
           make -C test/e2e-image image
+          cd images/custom-error-pages/rootfs && docker image build \
+              --build-arg=BASE_IMAGE=registry.k8s.io/ingress-nginx/nginx:${TAGNGINX} \
+              --build-arg GOLANG_VERSION=$(cat ../../../GOLANG_VERSION) \
+              --tag ingress-controller/custom-error-pages:1.0.0-dev . \
+              && cd ../../..
 
           echo "creating images cache..."
           docker save \
             nginx-ingress-controller:e2e \
             ingress-controller/controller:1.0.0-dev \
             ingress-controller/controller-chroot:1.0.0-dev \
+            ingress-controller/custom-error-pages:1.0.0-dev \
             | gzip > docker.tar.gz
 
       - name: cache

.github/workflows/zz-tmpl-k8s-e2e.yaml

@@ -43,6 +43,7 @@ jobs:
           SKIP_CLUSTER_CREATION: true
           SKIP_INGRESS_IMAGE_CREATION: true
           SKIP_E2E_IMAGE_CREATION: true
+          SKIP_CUSTOMERRORPAGES_IMAGE_CREATION: true
           IS_CHROOT: ${{ inputs.variation == 'CHROOT' }}
         run: |
           kind get kubeconfig > $HOME/.kube/kind-config-kind

charts/ingress-nginx/README.md

@@ -546,6 +546,7 @@ metadata:
 | defaultBackend.service.annotations | object | `{}` |  |
 | defaultBackend.service.clusterIPs | list | `[]` | Pre-defined cluster internal IP addresses of the default backend service. Take care of collisions with existing services. This value is immutable. Set once, it can not be changed without deleting and re-creating the service. Ref: https://kubernetes.io/docs/concepts/services-networking/service/#choosing-your-own-ip-address |
 | defaultBackend.service.externalIPs | list | `[]` | List of IP addresses at which the default backend service is available # Ref: https://kubernetes.io/docs/concepts/services-networking/service/#external-ips # |
+| defaultBackend.service.extraPorts | list | `[]` | Additional ports to expose for defaultBackend pods |
 | defaultBackend.service.loadBalancerSourceRanges | list | `[]` |  |
 | defaultBackend.service.servicePort | int | `80` |  |
 | defaultBackend.service.type | string | `"ClusterIP"` |  |

charts/ingress-nginx/ci/deployment-defaultbackend-values.yaml

@@ -0,0 +1,21 @@
+controller:
+  image:
+    repository: ingress-controller/controller
+    tag: 1.0.0-dev
+    digest: null
+  service:
+    type: ClusterIP
+
+defaultBackend:
+  enabled: true
+  extraEnvs:
+    - name: IS_METRICS_EXPORT
+      value: "false"
+    - name: METRICS_PORT
+      value: "8081"
+  service:
+    extraPorts:
+      - name: metrics
+        port: 8081
+        protocol: TCP
+        targetPort: 8081

charts/ingress-nginx/templates/default-backend-deployment.yaml

@@ -93,6 +93,13 @@ spec:
             - name: http
               containerPort: {{ .Values.defaultBackend.port }}
               protocol: TCP
+          {{- if .Values.defaultBackend.service.extraPorts }}
+            {{- range .Values.defaultBackend.service.extraPorts }}
+            - name: {{ .name }}
+              containerPort: {{ .targetPort }}
+              protocol: {{ .protocol }}
+            {{- end }}
+          {{- end }}
         {{- if .Values.defaultBackend.extraVolumeMounts }}
           volumeMounts: {{- toYaml .Values.defaultBackend.extraVolumeMounts | nindent 12 }}
         {{- end }}

charts/ingress-nginx/templates/default-backend-service.yaml

@@ -38,6 +38,9 @@ spec:
     {{- if semverCompare ">=1.20.0-0" .Capabilities.KubeVersion.Version }}
       appProtocol: http
     {{- end }}
+  {{- if .Values.defaultBackend.service.extraPorts }}
+    {{ toYaml .Values.defaultBackend.service.extraPorts | nindent 4 }}
+  {{- end}}
   selector:
     {{- include "ingress-nginx.selectorLabels" . | nindent 4 }}
     app.kubernetes.io/component: default-backend

charts/ingress-nginx/tests/default-backend-deployment_test.yaml

@@ -196,3 +196,18 @@ tests:
       - equal:
           path: spec.template.spec.automountServiceAccountToken
           value: false
+
+  - it: should create a Deployment with extraPorts if `defaultBackend.service.extraPorts` is set
+    set:
+      defaultBackend.enabled: true
+      defaultBackend.service.extraPorts[0]:
+        name: example
+        protocol: TCP
+        targetPort: 9999
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].ports
+          content:
+            name: example
+            protocol: TCP
+            containerPort: 9999

charts/ingress-nginx/tests/default-backend-service_test.yaml

@@ -50,3 +50,20 @@ tests:
           value:
             - 10.0.0.1
             - fd00::1
+
+  - it: should create a Service with extraPorts if `defaultBackend.service.extraPorts` is set
+    set:
+      defaultBackend.enabled: true
+      defaultBackend.service.extraPorts[0]:
+        name: example
+        port: 8888
+        protocol: TCP
+        targetPort: 65535
+    asserts:
+      - contains:
+          path: spec.ports
+          content:
+            name: example
+            port: 8888
+            protocol: TCP
+            targetPort: 65535

charts/ingress-nginx/values.yaml

@@ -1176,6 +1176,13 @@ defaultBackend:
     loadBalancerSourceRanges: []
     servicePort: 80
     type: ClusterIP
+
+    # -- Additional ports to expose for defaultBackend pods
+    extraPorts: []
+    #  - name: metrics
+    #    port: 9090
+    #    protocol: TCP
+    #    targetPort: 9090
   priorityClassName: ""
   # -- Labels to be added to the default backend resources
   labels: {}

docs/e2e-tests.md

@@ -283,6 +283,10 @@ Do not try to edit it manually.
 - [should return a self generated SSL certificate](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/defaultbackend/ssl.go#L29)
 ### [[Default Backend] change default settings](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/defaultbackend/with_hosts.go#L30)
 - [should apply the annotation to the default backend](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/defaultbackend/with_hosts.go#L38)
+### [[Default Backend] custom-error-pages](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/defaultbackend/custom_error_pages.go#L33)
+- [should export /metrics and /debug/vars by default](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/defaultbackend/custom_error_pages.go#L36)
+- [shouldn't export /metrics and /debug/vars when IS_METRICS_EXPORT is set to false](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/defaultbackend/custom_error_pages.go#L57)
+- [shouldn't export /metrics and /debug/vars when METRICS_PORT is set to a different port](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/defaultbackend/custom_error_pages.go#L80)
 ### [[Disable Leader] Routing works when leader election was disabled](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/disableleaderelection/disable_leader.go#L28)
 - [should create multiple ingress routings rules when leader election has disabled](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/disableleaderelection/disable_leader.go#L35)
 ### [[Endpointslices] long service name](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/endpointslices/longname.go#L29)

images/Makefile

@@ -42,7 +42,7 @@ export DOCKER_CLI_EXPERIMENTAL=enabled
 
 # build with buildx
 PLATFORMS?=linux/amd64,linux/arm,linux/arm64
-OUTPUT=
+OUTPUT?=
 PROGRESS=plain
 
 

images/custom-error-pages/rootfs/main.go

@@ -25,6 +25,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -59,15 +60,26 @@ const (
 	// RequestId is a unique ID that identifies the request - same as for backend service
 	RequestId = "X-Request-ID"
 
-	// ErrFilesPathVar is the name of the environment variable indicating
+	// ErrFilesPathEnvVar is the name of the environment variable indicating
 	// the location on disk of files served by the handler.
-	ErrFilesPathVar = "ERROR_FILES_PATH"
+	ErrFilesPathEnvVar = "ERROR_FILES_PATH"
 
-	// DefaultFormatVar is the name of the environment variable indicating
+	// DefaultFormatEnvVar is the name of the environment variable indicating
 	// the default error MIME type that should be returned if either the
 	// client does not specify an Accept header, or the Accept header provided
 	// cannot be mapped to a file extension.
-	DefaultFormatVar = "DEFAULT_RESPONSE_FORMAT"
+	DefaultFormatEnvVar = "DEFAULT_RESPONSE_FORMAT"
+
+	//  IsMetricsExportEnvVar is the name of the environment variable indicating
+	// whether or not to export /metrics and /debug/vars.
+	IsMetricsExportEnvVar = "IS_METRICS_EXPORT"
+
+	// MetricsPortEnvVar is the name of the environment variable indicating
+	// the port on which to export /metrics and /debug/vars.
+	MetricsPortEnvVar = "METRICS_PORT"
+
+	// CustomErrorPagesPort is the port on which to listen to serve custom error pages.
+	CustomErrorPagesPort = "8080"
 )
 
 func init() {
@@ -76,25 +88,97 @@ func init() {
 }
 
 func main() {
+	listeners := createListeners()
+	startListeners(listeners)
+}
+
+func createListeners() []Listener {
 	errFilesPath := "/www"
-	if os.Getenv(ErrFilesPathVar) != "" {
-		errFilesPath = os.Getenv(ErrFilesPathVar)
+	if os.Getenv(ErrFilesPathEnvVar) != "" {
+		errFilesPath = os.Getenv(ErrFilesPathEnvVar)
 	}
 
 	defaultFormat := "text/html"
-	if os.Getenv(DefaultFormatVar) != "" {
-		defaultFormat = os.Getenv(DefaultFormatVar)
+	if os.Getenv(DefaultFormatEnvVar) != "" {
+		defaultFormat = os.Getenv(DefaultFormatEnvVar)
+	}
+
+	isExportMetrics := true
+	if os.Getenv(IsMetricsExportEnvVar) != "" {
+		val, err := strconv.ParseBool(os.Getenv(IsMetricsExportEnvVar))
+		if err == nil {
+			isExportMetrics = val
+		}
+	}
+
+	metricsPort := "8080"
+	if os.Getenv(MetricsPortEnvVar) != "" {
+		metricsPort = os.Getenv(MetricsPortEnvVar)
+	}
+
+	var listeners []Listener
+
+	// MUST use NewServerMux when not exporting /metrics because expvar HTTP handler registers
+	// against DefaultServerMux as a consequence of importing it in client_golang/prometheus.
+	if !isExportMetrics {
+		mux := http.NewServeMux()
+		mux.HandleFunc("/", errorHandler(errFilesPath, defaultFormat))
+		mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		})
+		listeners = append(listeners, Listener{mux, CustomErrorPagesPort})
+		return listeners
 	}
 
-	http.HandleFunc("/", errorHandler(errFilesPath, defaultFormat))
+	// MUST use DefaultServerMux when exporting /metrics to the public because /debug/vars is
+	// only available with DefaultServerMux.
+	if metricsPort == CustomErrorPagesPort {
+		mux := http.DefaultServeMux
+		mux.Handle("/metrics", promhttp.Handler())
+		mux.HandleFunc("/", errorHandler(errFilesPath, defaultFormat))
+		mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		})
+		listeners = append(listeners, Listener{mux, CustomErrorPagesPort})
+		return listeners
+	}
 
-	http.Handle("/metrics", promhttp.Handler())
+	// MUST use DefaultServerMux for /metrics and NewServerMux for custom error pages when you
+	// wish to expose /metrics only to internal services, because expvar HTTP handler registers
+	// against DefaultServerMux.
+	metricsMux := http.DefaultServeMux
+	metricsMux.Handle("/metrics", promhttp.Handler())
 
-	http.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
+	errorsMux := http.NewServeMux()
+	errorsMux.HandleFunc("/", errorHandler(errFilesPath, defaultFormat))
+	errorsMux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	})
 
-	http.ListenAndServe(fmt.Sprintf(":8080"), nil)
+	listeners = append(listeners, Listener{metricsMux, metricsPort}, Listener{errorsMux, CustomErrorPagesPort})
+	return listeners
+}
+
+func startListeners(listeners []Listener) {
+	var wg sync.WaitGroup
+
+	for _, listener := range listeners {
+		wg.Add(1)
+		go func(l Listener) {
+			defer wg.Done()
+			err := http.ListenAndServe(fmt.Sprintf(":%s", l.port), l.mux)
+			if err != nil {
+				log.Fatal(err)
+			}
+		}(listener)
+	}
+
+	wg.Wait()
+}
+
+type Listener struct {
+	mux  *http.ServeMux
+	port string
 }
 
 func errorHandler(path, defaultFormat string) func(http.ResponseWriter, *http.Request) {

test/e2e/CUSTOMERRORPAGES_IMAGE

@@ -0,0 +1 @@
+ingress-controller/custom-error-pages:1.0.0-dev