diff --git a/Makefile b/Makefile index eeb61356e1..52f371b676 100644 --- a/Makefile +++ b/Makefile @@ -135,7 +135,7 @@ build-nkg-debug-image: debug-build build-nkg-image ## Build NKG image with debug generate-manifests: ## Generate manifests using Helm. cp $(CHART_DIR)/crds/* $(MANIFEST_DIR)/crds/ helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) $(HELM_TEMPLATE_EXTRA_ARGS_FOR_ALL_MANIFESTS_FILE) -n nginx-gateway | cat $(strip $(MANIFEST_DIR))/namespace.yaml - > $(strip $(MANIFEST_DIR))/nginx-gateway.yaml - helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) -n nginx-gateway -s templates/deployment.yaml > conformance/provisioner/static-deployment.yaml + helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) --set metrics.disable=true -n nginx-gateway -s templates/deployment.yaml > conformance/provisioner/static-deployment.yaml helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) -n nginx-gateway -s templates/service.yaml > $(strip $(MANIFEST_DIR))/service/loadbalancer.yaml helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) --set service.annotations.'service\.beta\.kubernetes\.io\/aws-load-balancer-type'="nlb" -n nginx-gateway -s templates/service.yaml > $(strip $(MANIFEST_DIR))/service/loadbalancer-aws-nlb.yaml helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) --set service.type=NodePort --set service.externalTrafficPolicy="" -n nginx-gateway -s templates/service.yaml > $(strip $(MANIFEST_DIR))/service/nodeport.yaml diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 7627ee5fdc..498780db14 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -24,6 +24,7 @@ const ( gatewayCtrlNameFlag = "gateway-ctlr-name" gatewayCtrlNameUsageFmt = `The name of the Gateway controller. ` + `The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'` + gatewayFlag = "gateway" ) var ( @@ -36,58 +37,21 @@ var ( gatewayClassName = stringValidatingValue{ validator: validateResourceName, } -) -// stringValidatingValue is a string flag value with custom validation logic. -// it implements the pflag.Value interface. -type stringValidatingValue struct { - validator func(v string) error - value string -} + // Backing values for static subcommand cli flags. + updateGCStatus bool + disableMetrics bool + metricsSecure bool -func (v *stringValidatingValue) String() string { - return v.value -} - -func (v *stringValidatingValue) Set(param string) error { - if err := v.validator(param); err != nil { - return err + metricsListenPort = intValidatingValue{ + validator: validatePort, + value: 9113, } - v.value = param - return nil -} - -func (v *stringValidatingValue) Type() string { - return "string" -} - -// namespacedNameValue is a string flag value that represents a namespaced name. -// it implements the pflag.Value interface. -type namespacedNameValue struct { - value types.NamespacedName -} - -func (v *namespacedNameValue) String() string { - if (v.value == types.NamespacedName{}) { - // if we don't do that, the default value in the help message will be printed as "/" - return "" - } - return v.value.String() -} - -func (v *namespacedNameValue) Set(param string) error { - nsname, err := parseNamespacedResourceName(param) - if err != nil { - return err + gateway = namespacedNameValue{} + configName = stringValidatingValue{ + validator: validateResourceName, } - - v.value = nsname - return nil -} - -func (v *namespacedNameValue) Type() string { - return "string" -} +) func createRootCommand() *cobra.Command { rootCmd := &cobra.Command{ @@ -115,15 +79,6 @@ func createRootCommand() *cobra.Command { } func createStaticModeCommand() *cobra.Command { - const gatewayFlag = "gateway" - - // flag values - gateway := namespacedNameValue{} - var updateGCStatus bool - configName := stringValidatingValue{ - validator: validateResourceName, - } - cmd := &cobra.Command{ Use: "static-mode", Short: "Configure NGINX in the scope of a single Gateway resource", @@ -153,6 +108,13 @@ func createStaticModeCommand() *cobra.Command { gwNsName = &gateway.value } + metricsConfig := config.MetricsConfig{} + if !disableMetrics { + metricsConfig.Enabled = true + metricsConfig.Port = metricsListenPort.value + metricsConfig.Secure = metricsSecure + } + conf := config.Config{ GatewayCtlrName: gatewayCtlrName.value, ConfigName: configName.String(), @@ -163,6 +125,7 @@ func createStaticModeCommand() *cobra.Command { Namespace: namespace, GatewayNsName: gwNsName, UpdateGatewayClassStatus: updateGCStatus, + MetricsConfig: metricsConfig, } if err := static.StartManager(conf); err != nil { @@ -198,6 +161,27 @@ func createStaticModeCommand() *cobra.Command { "Update the status of the GatewayClass resource.", ) + cmd.Flags().BoolVar( + &disableMetrics, + "metrics-disable", + false, + "Disable exposing metrics in the Prometheus format.", + ) + + cmd.Flags().Var( + &metricsListenPort, + "metrics-port", + "Set the port where the metrics are exposed. Format: [1023 - 65535]", + ) + + cmd.Flags().BoolVar( + &metricsSecure, + "metrics-secure-serving", + false, + "Enable serving metrics via https. By default metrics are served via http."+ + " Please note that this endpoint will be secured with a self-signed certificate.", + ) + return cmd } diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 59c9bd7bbc..0c352fda79 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -118,6 +118,9 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { "--gateway=nginx-gateway/nginx", "--config=nginx-gateway-config", "--update-gatewayclass-status=true", + "--metrics-port=9114", + "--metrics-disable", + "--metrics-secure-serving", }, wantErr: false, }, @@ -175,6 +178,42 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { wantErr: true, expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`, }, + { + name: "metrics-port is invalid type", + args: []string{ + "--metrics-port=invalid", // not an int + }, + wantErr: true, + expectedErrPrefix: `invalid argument "invalid" for "--metrics-port" flag: failed to parse int value:` + + ` strconv.ParseInt: parsing "invalid": invalid syntax`, + }, + { + name: "metrics-port is outside of range", + args: []string{ + "--metrics-port=999", // outside of range + }, + wantErr: true, + expectedErrPrefix: `invalid argument "999" for "--metrics-port" flag:` + + ` port outside of valid port range [1024 - 65535]: 999`, + }, + { + name: "metrics-disable is not a bool", + args: []string{ + "--metrics-disable=999", // not a bool + }, + wantErr: true, + expectedErrPrefix: `invalid argument "999" for "--metrics-disable" flag: strconv.ParseBool:` + + ` parsing "999": invalid syntax`, + }, + { + name: "metrics-secure-serving is not a bool", + args: []string{ + "--metrics-secure-serving=999", // not a bool + }, + wantErr: true, + expectedErrPrefix: `invalid argument "999" for "--metrics-secure-serving" flag: strconv.ParseBool:` + + ` parsing "999": invalid syntax`, + }, } for _, test := range tests { diff --git a/cmd/gateway/validating_types.go b/cmd/gateway/validating_types.go new file mode 100644 index 0000000000..42d24782cb --- /dev/null +++ b/cmd/gateway/validating_types.go @@ -0,0 +1,86 @@ +package main + +import ( + "fmt" + "strconv" + + "k8s.io/apimachinery/pkg/types" +) + +// stringValidatingValue is a string flag value with custom validation logic. +// it implements the pflag.Value interface. +type stringValidatingValue struct { + validator func(v string) error + value string +} + +func (v *stringValidatingValue) String() string { + return v.value +} + +func (v *stringValidatingValue) Set(param string) error { + if err := v.validator(param); err != nil { + return err + } + v.value = param + return nil +} + +func (v *stringValidatingValue) Type() string { + return "string" +} + +type intValidatingValue struct { + validator func(v int) error + value int +} + +func (v *intValidatingValue) String() string { + return strconv.Itoa(v.value) +} + +func (v *intValidatingValue) Set(param string) error { + intVal, err := strconv.ParseInt(param, 10, 32) + if err != nil { + return fmt.Errorf("failed to parse int value: %w", err) + } + + if err := v.validator(int(intVal)); err != nil { + return err + } + + v.value = int(intVal) + return nil +} + +func (v *intValidatingValue) Type() string { + return "int" +} + +// namespacedNameValue is a string flag value that represents a namespaced name. +// it implements the pflag.Value interface. +type namespacedNameValue struct { + value types.NamespacedName +} + +func (v *namespacedNameValue) String() string { + if (v.value == types.NamespacedName{}) { + // if we don't do that, the default value in the help message will be printed as "/" + return "" + } + return v.value.String() +} + +func (v *namespacedNameValue) Set(param string) error { + nsname, err := parseNamespacedResourceName(param) + if err != nil { + return err + } + + v.value = nsname + return nil +} + +func (v *namespacedNameValue) Type() string { + return "string" +} diff --git a/cmd/gateway/validation.go b/cmd/gateway/validation.go index a92dd76018..8a40d7970f 100644 --- a/cmd/gateway/validation.go +++ b/cmd/gateway/validation.go @@ -99,3 +99,11 @@ func validateIP(ip string) error { return nil } + +// validatePort makes sure a given port is inside the valid port range for its usage +func validatePort(port int) error { + if port < 1024 || port > 65535 { + return fmt.Errorf("port outside of valid port range [1024 - 65535]: %v", port) + } + return nil +} diff --git a/cmd/gateway/validation_test.go b/cmd/gateway/validation_test.go index 47231c73ec..66a983a3d7 100644 --- a/cmd/gateway/validation_test.go +++ b/cmd/gateway/validation_test.go @@ -294,3 +294,40 @@ func TestValidateIP(t *testing.T) { }) } } + +func TestValidatePort(t *testing.T) { + tests := []struct { + name string + port int + expErr bool + }{ + { + name: "port under minimum allowed value", + port: 1023, + expErr: true, + }, + { + name: "port over maximum allowed value", + port: 65536, + expErr: true, + }, + { + name: "valid port", + port: 9113, + expErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + err := validatePort(tc.port) + if !tc.expErr { + g.Expect(err).ToNot(HaveOccurred()) + } else { + g.Expect(err).To(HaveOccurred()) + } + }) + } +} diff --git a/conformance/provisioner/static-deployment.yaml b/conformance/provisioner/static-deployment.yaml index 184fb4c996..fe2cbd20de 100644 --- a/conformance/provisioner/static-deployment.yaml +++ b/conformance/provisioner/static-deployment.yaml @@ -28,6 +28,7 @@ spec: - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - --config=nginx-gateway-config + - --metrics-disable env: - name: POD_IP valueFrom: diff --git a/deploy/helm-chart/README.md b/deploy/helm-chart/README.md index 3d4dddc08e..d826042764 100644 --- a/deploy/helm-chart/README.md +++ b/deploy/helm-chart/README.md @@ -142,3 +142,6 @@ The following tables lists the configurable parameters of the NGINX Kubernetes G | `service.externalTrafficPolicy` | The `externalTrafficPolicy` of the service. The value `Local` preserves the client source IP. | Local | | `service.annotations` | The `annotations` of the NGINX Kubernetes Gateway service. | {} | | `service.ports` | A list of ports to expose through the NGINX Kubernetes Gateway service. Update it to match the listener ports from your Gateway resource. Follows the conventional Kubernetes yaml syntax for service ports. | [ port: 80, targetPort: 80, protocol: TCP, name: http; port: 443, targetPort: 443, protocol: TCP, name: https ] | +| `metrics.disable` | Disable exposing metrics in the Prometheus format. |false | +| `metrics.port` | Set the port where the Prometheus metrics are exposed. Format: [1024 - 65535] |9113 | +| `metrics.secure` | Enable serving metrics via https. By default metrics are served via http. Please note that this endpoint will be secured with a self-signed certificate. |false | diff --git a/deploy/helm-chart/templates/deployment.yaml b/deploy/helm-chart/templates/deployment.yaml index d662174283..8bf7492e15 100644 --- a/deploy/helm-chart/templates/deployment.yaml +++ b/deploy/helm-chart/templates/deployment.yaml @@ -16,6 +16,14 @@ spec: metadata: labels: {{- include "nginx-gateway.selectorLabels" . | nindent 8 }} + {{- if not .Values.metrics.disable }} + annotations: + prometheus.io/scrape: "true" + prometheus.io/port: "{{ .Values.metrics.port }}" + {{- if .Values.metrics.secure }} + prometheus.io/scheme: "https" + {{- end }} + {{- end }} spec: containers: - args: @@ -23,6 +31,14 @@ spec: - --gateway-ctlr-name={{ .Values.nginxGateway.gatewayControllerName }} - --gatewayclass={{ .Values.nginxGateway.gatewayClassName }} - --config={{ include "nginx-gateway.config-name" . }} + {{- if not .Values.metrics.disable }} + - --metrics-port={{ .Values.metrics.port }} + {{- if .Values.metrics.secure }} + - --metrics-secure-serving + {{- end }} + {{- else }} + - --metrics-disable + {{- end }} env: - name: POD_IP valueFrom: @@ -35,6 +51,11 @@ spec: image: {{ .Values.nginxGateway.image.repository }}:{{ .Values.nginxGateway.image.tag | default .Chart.AppVersion }} imagePullPolicy: {{ .Values.nginxGateway.image.pullPolicy }} name: nginx-gateway + {{- if not .Values.metrics.disable }} + ports: + - name: metrics + containerPort: {{ .Values.metrics.port }} + {{- end }} securityContext: allowPrivilegeEscalation: false capabilities: diff --git a/deploy/helm-chart/values.yaml b/deploy/helm-chart/values.yaml index d616a630cb..fd3a77007e 100644 --- a/deploy/helm-chart/values.yaml +++ b/deploy/helm-chart/values.yaml @@ -54,3 +54,12 @@ service: targetPort: 443 protocol: TCP name: https + +metrics: + ## Disable exposing metrics in the Prometheus format. + disable: false + ## Set the port where the Prometheus metrics are exposed. Format: [1024 - 65535] + port: 9113 + ## Enable serving metrics via https. By default metrics are served via http. + ## Please note that this endpoint will be secured with a self-signed certificate. + secure: false diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 3bd5059ecd..129147c3db 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -122,6 +122,9 @@ spec: labels: app.kubernetes.io/name: nginx-gateway app.kubernetes.io/instance: nginx-gateway + annotations: + prometheus.io/scrape: "true" + prometheus.io/port: "9113" spec: containers: - args: @@ -129,6 +132,7 @@ spec: - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - --config=nginx-gateway-config + - --metrics-port=9113 env: - name: POD_IP valueFrom: @@ -141,6 +145,9 @@ spec: image: ghcr.io/nginxinc/nginx-kubernetes-gateway:edge imagePullPolicy: Always name: nginx-gateway + ports: + - name: metrics + containerPort: 9113 securityContext: allowPrivilegeEscalation: false capabilities: diff --git a/docs/architecture.md b/docs/architecture.md index 3eb383d8fe..3eabd49f65 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -96,33 +96,37 @@ parentheses. To enhance readability, the suffix "process" has been omitted from 1. (HTTPS) *NKG* reads the *Kubernetes API* to get the latest versions of the resources in the cluster and writes to the API to update the handled resources' statuses and emit events. -2. (File I/O) +2. (HTTP, HTTPS) *Prometheus* fetches the `controller-runtime` and NGINX metrics via an HTTP endpoint that *NKG* exposes. + The default is :9113/metrics. Note: Prometheus is not required by NKG, the endpoint can be turned off. +3. (File I/O) - Write: *NKG* generates NGINX *configuration* based on the cluster resources and writes them as `.conf` files to the mounted `nginx-conf` volume, located at `/etc/nginx/conf.d`. It also writes *TLS certificates* and *keys* from [TLS Secrets][secrets] referenced in the accepted Gateway resource to the `nginx-secrets` volume at the path `/etc/nginx/secrets`. - Read: *NKG* reads the PID file `nginx.pid` from the `nginx-run` volume, located at `/var/run/nginx`. *NKG* extracts the PID of the nginx process from this file in order to send reload signals to *NGINX master*. -3. (File I/O) *NKG* writes logs to its *stdout* and *stderr*, which are collected by the container runtime. -4. (Signal) To reload NGINX, *NKG* sends the [reload signal][reload] to the **NGINX master**. -5. (File I/O) +4. (File I/O) *NKG* writes logs to its *stdout* and *stderr*, which are collected by the container runtime. +5. (HTTP) *NKG* fetches the NGINX metrics via the unix:/var/lib/nginx/nginx-status.sock UNIX socket and converts it to + *Prometheus* format used in #2. +6. (Signal) To reload NGINX, *NKG* sends the [reload signal][reload] to the **NGINX master**. +7. (File I/O) - Write: The *NGINX master* writes its PID to the `nginx.pid` file stored in the `nginx-run` volume. - Read: The *NGINX master* reads *configuration files* and the *TLS cert and keys* referenced in the configuration when it starts or during a reload. These files, certificates, and keys are stored in the `nginx-conf` and `nginx-secrets` volumes that are mounted to both the `nginx-gateway` and `nginx` containers. -6. (File I/O) +8. (File I/O) - Write: The *NGINX master* writes to the auxiliary Unix sockets folder, which is located in the `/var/lib/nginx` directory. - Read: The *NGINX master* reads the `nginx.conf` file from the `/etc/nginx` directory. This [file][conf-file] contains the global and http configuration settings for NGINX. In addition, *NGINX master* reads the NJS modules referenced in the configuration when it starts or during a reload. NJS modules are stored in the `/usr/lib/nginx/modules` directory. -7. (File I/O) The *NGINX master* sends logs to its *stdout* and *stderr*, which are collected by the container runtime. -8. (File I/O) An *NGINX worker* writes logs to its *stdout* and *stderr*, which are collected by the container runtime. -9. (Signal) The *NGINX master* controls the [lifecycle of *NGINX workers*][lifecycle] it creates workers with the new +9. (File I/O) The *NGINX master* sends logs to its *stdout* and *stderr*, which are collected by the container runtime. +10. (File I/O) An *NGINX worker* writes logs to its *stdout* and *stderr*, which are collected by the container runtime. +11. (Signal) The *NGINX master* controls the [lifecycle of *NGINX workers*][lifecycle] it creates workers with the new configuration and shutdowns workers with the old configuration. -10. (HTTP,HTTPS) A *client* sends traffic to and receives traffic from any of the *NGINX workers* on ports 80 and 443. -11. (HTTP,HTTPS) An *NGINX worker* sends traffic to and receives traffic from the *backends*. +12. (HTTP,HTTPS) A *client* sends traffic to and receives traffic from any of the *NGINX workers* on ports 80 and 443. +13. (HTTP,HTTPS) An *NGINX worker* sends traffic to and receives traffic from the *backends*. [controller]: https://kubernetes.io/docs/concepts/architecture/controller/ diff --git a/docs/cli-help.md b/docs/cli-help.md index df780388d6..fb1adeb9de 100644 --- a/docs/cli-help.md +++ b/docs/cli-help.md @@ -20,4 +20,6 @@ Flags: | `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. | | `gateway` | `string` | The namespaced name of the Gateway resource to use. Must be of the form: `NAMESPACE/NAME`. If not specified, the control plane will process all Gateways for the configured GatewayClass. However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}. | | `config` | `string` | The name of the NginxGateway resource to be used for this controller's dynamic configuration. Lives in the same Namespace as the controller. | -| `update-gatewayclass-status` | `bool` | Update the status of the GatewayClass resource. (default true) | +| `metrics-disable` | `bool` | Disable exposing metrics in the Prometheus format. (default false) | +| `metrics-listen-port` | `int` | Sets the port where the Prometheus metrics are exposed. Format: `[1024 - 65535]` (default `9113`) | +| `metrics-secure-serving` | `bool` | Configures if the metrics endpoint should be secured using https. Please note that this endpoint will be secured with a self-signed certificate. (default false) | diff --git a/docs/images/nkg-pod.png b/docs/images/nkg-pod.png index 5f58045480..7af069f201 100644 Binary files a/docs/images/nkg-pod.png and b/docs/images/nkg-pod.png differ diff --git a/docs/monitoring.md b/docs/monitoring.md new file mode 100644 index 0000000000..5e5862157d --- /dev/null +++ b/docs/monitoring.md @@ -0,0 +1,94 @@ +# Monitoring + +The NGINX Kubernetes Gateway exposes a number of metrics in the [Prometheus](https://prometheus.io/) format. Those +include NGINX and the controller-runtime metrics. These are delivered using a metrics server orchestrated by the +controller-runtime package. Metrics are enabled by default, and are served via http on port `9113`. + +> **Note** +> By default metrics are served via http. Please note that if serving metrics via https is enabled, this +> endpoint will be secured with a self-signed certificate. Since the metrics server is using a self-signed certificate, +> the Prometheus Pod scrape configuration will also require the `insecure_skip_verify` flag set. See +> [the Prometheus documentation](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#tls_config). + +## Changing the default Metrics configuration + +### Using Helm + +If you're using *Helm* to install the NGINX Kubernetes Gateway, set the `metrics.*` parameters to the required values +for your environment. See the [Helm README](/deploy/helm-chart/README.md). + +### Using Manifests + +If you're using *Kubernetes manifests* to install NGINX Kubernetes Gateway, you can modify the +[manifest](/deploy/manifests/nginx-gateway.yaml) to change the default metrics configuration: + +#### Disabling metrics + +1. Set the `-metrics-disable` [command-line argument](/docs/cli-help.md) to `true` and remove the other `-metrics-*` + command line arguments. + +2. Remove the metrics port entry from the list of the ports of the NGINX Kubernetes Gateway container in the template + of the NGINX Kubernetes Gateway Pod: + + ```yaml + - name: metrics + containerPort: 9113 + ``` + +3. Remove the following annotations from the template of the NGINX Kubernetes Gateway Pod: + + ```yaml + annotations: + prometheus.io/scrape: "true" + prometheus.io/port: "9113" + ``` + +#### Changing the default port + +1. Set the `-metrics-port` [command-line argument](/docs/cli-help.md) to the required value. + +2. Change the metrics port entry in the list of the ports of the NGINX Kubernetes Gateway container in the template + of the NGINX Kubernetes Gateway Pod: + + ```yaml + - name: metrics + containerPort: + ``` + +3. Change the following annotation in the template of the NGINX Kubernetes Gateway Pod: + + ```yaml + annotations: + <...> + prometheus.io/port: "" + <...> + ``` + +#### Enable serving metrics via https + +1. Set the `-metrics-secure-serving` [command-line argument](/docs/cli-help.md) to `true`. + +2. Add the following annotation in the template of the NGINX Kubernetes Gateway Pod: + + ```yaml + annotations: + <...> + prometheus.io/scheme: "https" + <...> + ``` + +## Available Metrics + +NGINX Kubernetes Gateway exports the following metrics: + +- NGINX metrics: + - Exported by NGINX. Refer to the [NGINX Prometheus Exporter developer docs](https://github.com/nginxinc/nginx-prometheus-exporter#metrics-for-nginx-oss) + - These metrics have the namespace `nginx_kubernetes_gateway`, and include the label `class` which is set to the + Gateway class of NKG. For example, `nginx_kubernetes_gateway_connections_accepted{class="nginx"}`. + +- [controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) metrics. These include: + - Total number of reconciliation errors per controller + - Length of reconcile queue per controller + - Reconciliation latency + - Usual resource metrics such as CPU, memory usage, file descriptor usage + - Go runtime metrics such as number of Go routines, GC duration, and Go version information diff --git a/go.mod b/go.mod index 46c6bd6220..42510c6e6f 100644 --- a/go.mod +++ b/go.mod @@ -9,8 +9,10 @@ require ( github.com/go-logr/logr v1.2.4 github.com/google/go-cmp v0.5.9 github.com/maxbrunsfeld/counterfeiter/v6 v6.7.0 + github.com/nginxinc/nginx-prometheus-exporter v0.11.0 github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.10 + github.com/prometheus/client_golang v1.16.0 github.com/spf13/cobra v1.7.0 go.uber.org/zap v1.25.0 k8s.io/api v0.28.1 @@ -55,9 +57,9 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/nginxinc/nginx-plus-go-client v0.10.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/prometheus/client_golang v1.16.0 // indirect github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/common v0.44.0 // indirect github.com/prometheus/procfs v0.10.1 // indirect diff --git a/go.sum b/go.sum index ec9d7db72f..aab3b8f0ac 100644 --- a/go.sum +++ b/go.sum @@ -127,6 +127,10 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= +github.com/nginxinc/nginx-plus-go-client v0.10.0 h1:3zsMMkPvRDo8D7ZSprXtbAEW/SDmezZWzxdyS+6oAlc= +github.com/nginxinc/nginx-plus-go-client v0.10.0/go.mod h1:0v3RsQCvRn/IyrMtW+DK6CNkz+PxEsXDJPjQ3yUMBF0= +github.com/nginxinc/nginx-prometheus-exporter v0.11.0 h1:21xjnqNgxtni2jDgAQ90bl15uDnrTreO9sIlu1YsX/U= +github.com/nginxinc/nginx-prometheus-exporter v0.11.0/go.mod h1:GdyHnWAb8q8OW1Pssrrqbcqra0SH0Vn6UXICMmyWkw8= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index f29b45b1de..09bdcf1240 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -26,4 +26,16 @@ type Config struct { Namespace string // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. UpdateGatewayClassStatus bool + // MetricsConfig specifies the metrics config. + MetricsConfig MetricsConfig +} + +// MetricsConfig specifies the metrics config. +type MetricsConfig struct { + // Port is the port the metrics should be exposed on. + Port int + // Enabled is the flag for toggling metrics on or off. + Enabled bool + // Secure is the flag for toggling the metrics endpoint to https. + Secure bool } diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index f2754df8fe..01ca2da11b 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -16,6 +16,7 @@ import ( ctlr "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/metrics" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" k8spredicate "sigs.k8s.io/controller-runtime/pkg/predicate" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -28,6 +29,7 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/events" "github.com/nginxinc/nginx-kubernetes-gateway/internal/framework/status" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/config" + nkgmetrics "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/metrics" ngxcfg "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/config" ngxvalidation "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/config/validation" "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/file" @@ -56,13 +58,9 @@ func StartManager(cfg config.Config) error { logger := cfg.Logger options := manager.Options{ - Scheme: scheme, - Logger: logger, - // We disable the metrics server because we reserve all ports (1-65535) for the data plane. - // Once we add support for Prometheus, we can make this port configurable by the user. - Metrics: metricsserver.Options{ - BindAddress: "0", - }, + Scheme: scheme, + Logger: logger, + Metrics: getMetricsOptions(cfg.MetricsConfig), } eventCh := make(chan interface{}) @@ -164,6 +162,11 @@ func StartManager(cfg config.Config) error { } } + // protectedPorts is the map of ports that may not be configured by a listener, and the name of what it is used for + protectedPorts := map[int32]string{ + int32(cfg.MetricsConfig.Port): "MetricsPort", + } + processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: cfg.GatewayCtlrName, GatewayClassName: cfg.GatewayClassName, @@ -172,8 +175,9 @@ func StartManager(cfg config.Config) error { Validators: validation.Validators{ HTTPFieldsValidator: ngxvalidation.HTTPValidator{}, }, - EventRecorder: recorder, - Scheme: scheme, + EventRecorder: recorder, + Scheme: scheme, + ProtectedPorts: protectedPorts, }) configGenerator := ngxcfg.NewGeneratorImpl() @@ -227,6 +231,17 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot register event loop: %w", err) } + // Ensure NGINX is running before registering metrics & starting the manager. + if err := ngxruntime.EnsureNginxRunning(ctx); err != nil { + return fmt.Errorf("NGINX is not running: %w", err) + } + + if cfg.MetricsConfig.Enabled { + if err := configureNginxMetrics(cfg.GatewayClassName); err != nil { + return err + } + } + logger.Info("Starting manager") return mgr.Start(ctx) } @@ -278,3 +293,25 @@ func setInitialConfig( // resource is processed by the controller return updateControlPlane(&config, logger, eventRecorder, configName, logLevelSetter) } + +func configureNginxMetrics(gatewayClassName string) error { + constLabels := map[string]string{"class": gatewayClassName} + ngxCollector, err := nkgmetrics.NewNginxMetricsCollector(constLabels) + if err != nil { + return fmt.Errorf("cannot get NGINX metrics: %w", err) + } + return metrics.Registry.Register(ngxCollector) +} + +func getMetricsOptions(cfg config.MetricsConfig) metricsserver.Options { + metricsOptions := metricsserver.Options{BindAddress: "0"} + + if cfg.Enabled { + if cfg.Secure { + metricsOptions.SecureServing = true + } + metricsOptions.BindAddress = fmt.Sprintf(":%v", cfg.Port) + } + + return metricsOptions +} diff --git a/internal/mode/static/manager_test.go b/internal/mode/static/manager_test.go index 4fbe4c9f5d..07b73f1c4f 100644 --- a/internal/mode/static/manager_test.go +++ b/internal/mode/static/manager_test.go @@ -9,7 +9,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/config" ) func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { @@ -69,3 +72,51 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { }) } } + +func TestGetMetricsOptions(t *testing.T) { + tests := []struct { + name string + expectedOptions metricsserver.Options + metricsConfig config.MetricsConfig + }{ + { + name: "Metrics disabled", + metricsConfig: config.MetricsConfig{Enabled: false}, + expectedOptions: metricsserver.Options{BindAddress: "0"}, + }, + { + name: "Metrics enabled, not secure", + metricsConfig: config.MetricsConfig{ + Port: 9113, + Enabled: true, + Secure: false, + }, + expectedOptions: metricsserver.Options{ + SecureServing: false, + BindAddress: ":9113", + }, + }, + { + name: "Metrics enabled, secure", + metricsConfig: config.MetricsConfig{ + Port: 9113, + Enabled: true, + Secure: true, + }, + expectedOptions: metricsserver.Options{ + SecureServing: true, + BindAddress: ":9113", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + metricsServerOptions := getMetricsOptions(test.metricsConfig) + + g.Expect(metricsServerOptions).To(Equal(test.expectedOptions)) + }) + } +} diff --git a/internal/mode/static/metrics/nginx.go b/internal/mode/static/metrics/nginx.go new file mode 100644 index 0000000000..33946722d2 --- /dev/null +++ b/internal/mode/static/metrics/nginx.go @@ -0,0 +1,39 @@ +package metrics + +import ( + "context" + "net" + "net/http" + "time" + + prometheusClient "github.com/nginxinc/nginx-prometheus-exporter/client" + nginxCollector "github.com/nginxinc/nginx-prometheus-exporter/collector" + "github.com/prometheus/client_golang/prometheus" +) + +const ( + nginxStatusSock = "/var/run/nginx/nginx-status.sock" + nginxStatusURI = "http://config-status/stub_status" + nginxStatusSockTimeout = 10 * time.Second +) + +// NewNginxMetricsCollector creates an NginxCollector which fetches stats from NGINX over a unix socket +func NewNginxMetricsCollector(constLabels map[string]string) (prometheus.Collector, error) { + httpClient := getSocketClient(nginxStatusSock) + client, err := prometheusClient.NewNginxClient(&httpClient, nginxStatusURI) + if err != nil { + return nil, err + } + return nginxCollector.NewNginxCollector(client, "nginx_kubernetes_gateway", constLabels), nil +} + +// getSocketClient gets an http.Client with a unix socket transport. +func getSocketClient(sockPath string) http.Client { + return http.Client{ + Transport: &http.Transport{ + DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { + return net.Dial("unix", sockPath) + }, + }, + } +} diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index 02f6c9f91f..74af2ee4db 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -14,4 +14,13 @@ http { server_names_hash_max_size 1024; variables_hash_bucket_size 512; variables_hash_max_size 1024; + + server { + listen unix:/var/run/nginx/nginx-status.sock; + access_log off; + + location /stub_status { + stub_status; + } + } } diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 88fa44afbb..8280d3acd8 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -70,6 +70,14 @@ func (m *ManagerImpl) Reload(ctx context.Context) error { return nil } +// EnsureNginxRunning ensures NGINX is running by locating the main process. +func EnsureNginxRunning(ctx context.Context) error { + if _, err := findMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { + return fmt.Errorf("failed to find NGINX main process: %w", err) + } + return nil +} + func findMainProcess( ctx context.Context, checkFile checkFileFunc, diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go index bc6aa71da7..6ccf00d751 100644 --- a/internal/mode/static/state/change_processor.go +++ b/internal/mode/static/state/change_processor.go @@ -55,12 +55,14 @@ type ChangeProcessorConfig struct { RelationshipCapturer relationship.Capturer // Validators validate resources according to data-plane specific rules. Validators validation.Validators - // Logger is the logger for this Change Processor. - Logger logr.Logger // EventRecorder records events for Kubernetes resources. EventRecorder record.EventRecorder // Scheme is the Kubernetes scheme. Scheme *runtime.Scheme + // ProtectedPorts are the ports that may not be configured by a listener with a descriptive name of the ports. + ProtectedPorts graph.ProtectedPorts + // Logger is the logger for this Change Processor. + Logger logr.Logger // GatewayCtlrName is the name of the Gateway controller. GatewayCtlrName string // GatewayClassName is the name of the GatewayClass resource. @@ -230,6 +232,7 @@ func (c *ChangeProcessorImpl) Process() (bool, *graph.Graph) { c.cfg.GatewayCtlrName, c.cfg.GatewayClassName, c.cfg.Validators, + c.cfg.ProtectedPorts, ) return true, c.latestGraph diff --git a/internal/mode/static/state/graph/gateway.go b/internal/mode/static/state/graph/gateway.go index 278cc996a1..908aa7ef19 100644 --- a/internal/mode/static/state/graph/gateway.go +++ b/internal/mode/static/state/graph/gateway.go @@ -95,6 +95,7 @@ func buildGateway( secretResolver *secretResolver, gc *GatewayClass, refGrantResolver *referenceGrantResolver, + protectedPorts ProtectedPorts, ) *Gateway { if gw == nil { return nil @@ -112,7 +113,7 @@ func buildGateway( return &Gateway{ Source: gw, - Listeners: buildListeners(gw, secretResolver, refGrantResolver), + Listeners: buildListeners(gw, secretResolver, refGrantResolver, protectedPorts), Valid: true, } } diff --git a/internal/mode/static/state/graph/gateway_listener.go b/internal/mode/static/state/graph/gateway_listener.go index 71e66a0f7c..f8373d299f 100644 --- a/internal/mode/static/state/graph/gateway_listener.go +++ b/internal/mode/static/state/graph/gateway_listener.go @@ -40,10 +40,11 @@ func buildListeners( gw *v1beta1.Gateway, secretResolver *secretResolver, refGrantResolver *referenceGrantResolver, + protectedPorts ProtectedPorts, ) map[string]*Listener { listeners := make(map[string]*Listener) - listenerFactory := newListenerConfiguratorFactory(gw, secretResolver, refGrantResolver) + listenerFactory := newListenerConfiguratorFactory(gw, secretResolver, refGrantResolver, protectedPorts) for _, gl := range gw.Spec.Listeners { configurator := listenerFactory.getConfiguratorForListener(gl) @@ -72,6 +73,7 @@ func newListenerConfiguratorFactory( gw *v1beta1.Gateway, secretResolver *secretResolver, refGrantResolver *referenceGrantResolver, + protectedPorts ProtectedPorts, ) *listenerConfiguratorFactory { sharedPortConflictResolver := createPortConflictResolver() @@ -93,7 +95,7 @@ func newListenerConfiguratorFactory( validateListenerAllowedRouteKind, validateListenerLabelSelector, validateListenerHostname, - validateHTTPListener, + createHTTPListenerValidator(protectedPorts), }, conflictResolvers: []listenerConflictResolver{ sharedPortConflictResolver, @@ -104,7 +106,7 @@ func newListenerConfiguratorFactory( validateListenerAllowedRouteKind, validateListenerLabelSelector, validateListenerHostname, - createHTTPSListenerValidator(), + createHTTPSListenerValidator(protectedPorts), }, conflictResolvers: []listenerConflictResolver{ sharedPortConflictResolver, @@ -275,33 +277,41 @@ func validateListenerLabelSelector(listener v1beta1.Listener) []conditions.Condi return nil } -func validateHTTPListener(listener v1beta1.Listener) []conditions.Condition { - if err := validateListenerPort(listener.Port); err != nil { - path := field.NewPath("port") - valErr := field.Invalid(path, listener.Port, err.Error()) - return staticConds.NewListenerUnsupportedValue(valErr.Error()) - } +func createHTTPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { + return func(listener v1beta1.Listener) []conditions.Condition { + var conds []conditions.Condition - if listener.TLS != nil { - panicForBrokenWebhookAssumption(fmt.Errorf("tls is not nil for HTTP listener %q", listener.Name)) - } + if err := validateListenerPort(listener.Port, protectedPorts); err != nil { + path := field.NewPath("port") + valErr := field.Invalid(path, listener.Port, err.Error()) + conds = append(conds, staticConds.NewListenerUnsupportedValue(valErr.Error())...) + } - return nil + if listener.TLS != nil { + panicForBrokenWebhookAssumption(fmt.Errorf("tls is not nil for HTTP listener %q", listener.Name)) + } + + return conds + } } -func validateListenerPort(port v1beta1.PortNumber) error { +func validateListenerPort(port v1beta1.PortNumber, protectedPorts ProtectedPorts) error { if port < 1 || port > 65535 { return errors.New("port must be between 1-65535") } + if portName, ok := protectedPorts[int32(port)]; ok { + return fmt.Errorf("port is already in use as %v", portName) + } + return nil } -func createHTTPSListenerValidator() listenerValidator { +func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidator { return func(listener v1beta1.Listener) []conditions.Condition { var conds []conditions.Condition - if err := validateListenerPort(listener.Port); err != nil { + if err := validateListenerPort(listener.Port, protectedPorts); err != nil { path := field.NewPath("port") valErr := field.Invalid(path, listener.Port, err.Error()) conds = append(conds, staticConds.NewListenerUnsupportedValue(valErr.Error())...) diff --git a/internal/mode/static/state/graph/gateway_listener_test.go b/internal/mode/static/state/graph/gateway_listener_test.go index 7700cdd9ec..f2c1e3bba2 100644 --- a/internal/mode/static/state/graph/gateway_listener_test.go +++ b/internal/mode/static/state/graph/gateway_listener_test.go @@ -14,6 +14,8 @@ import ( ) func TestValidateHTTPListener(t *testing.T) { + protectedPorts := ProtectedPorts{9113: "MetricsPort"} + tests := []struct { l v1beta1.Listener name string @@ -33,13 +35,25 @@ func TestValidateHTTPListener(t *testing.T) { expected: staticConds.NewListenerUnsupportedValue(`port: Invalid value: 0: port must be between 1-65535`), name: "invalid port", }, + { + l: v1beta1.Listener{ + Port: 9113, + }, + expected: staticConds.NewListenerUnsupportedValue( + `port: Invalid value: 9113: port is already in use as MetricsPort`, + ), + name: "invalid protected port", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - result := validateHTTPListener(test.l) + v := createHTTPListenerValidator(protectedPorts) + + result := v(test.l) + g.Expect(result).To(Equal(test.expected)) }) } @@ -67,6 +81,8 @@ func TestValidateHTTPSListener(t *testing.T) { Namespace: (*v1beta1.Namespace)(helpers.GetPointer(secretNs)), } + protectedPorts := ProtectedPorts{9113: "MetricsPort"} + tests := []struct { l v1beta1.Listener name string @@ -94,6 +110,19 @@ func TestValidateHTTPSListener(t *testing.T) { expected: staticConds.NewListenerUnsupportedValue(`port: Invalid value: 0: port must be between 1-65535`), name: "invalid port", }, + { + l: v1beta1.Listener{ + Port: 9113, + TLS: &v1beta1.GatewayTLSConfig{ + Mode: helpers.GetPointer(v1beta1.TLSModeTerminate), + CertificateRefs: []v1beta1.SecretObjectReference{validSecretRef}, + }, + }, + expected: staticConds.NewListenerUnsupportedValue( + `port: Invalid value: 9113: port is already in use as MetricsPort`, + ), + name: "invalid protected port", + }, { l: v1beta1.Listener{ Port: 443, @@ -164,7 +193,7 @@ func TestValidateHTTPSListener(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - v := createHTTPSListenerValidator() + v := createHTTPSListenerValidator(protectedPorts) result := v(test.l) g.Expect(result).To(Equal(test.expected)) @@ -388,19 +417,20 @@ func TestValidateListenerLabelSelector(t *testing.T) { func TestValidateListenerPort(t *testing.T) { validPorts := []v1beta1.PortNumber{1, 80, 443, 1000, 50000, 65535} - invalidPorts := []v1beta1.PortNumber{-1, 0, 65536, 80000} + invalidPorts := []v1beta1.PortNumber{-1, 0, 65536, 80000, 9113} + protectedPorts := ProtectedPorts{9113: "MetricsPort"} for _, p := range validPorts { t.Run(fmt.Sprintf("valid port %d", p), func(t *testing.T) { g := NewWithT(t) - g.Expect(validateListenerPort(p)).To(Succeed()) + g.Expect(validateListenerPort(p, protectedPorts)).To(Succeed()) }) } for _, p := range invalidPorts { t.Run(fmt.Sprintf("invalid port %d", p), func(t *testing.T) { g := NewWithT(t) - g.Expect(validateListenerPort(p)).ToNot(Succeed()) + g.Expect(validateListenerPort(p, protectedPorts)).ToNot(Succeed()) }) } } diff --git a/internal/mode/static/state/graph/gateway_test.go b/internal/mode/static/state/graph/gateway_test.go index e5fdb85d44..8cfebea0e4 100644 --- a/internal/mode/static/state/graph/gateway_test.go +++ b/internal/mode/static/state/graph/gateway_test.go @@ -145,6 +145,9 @@ func TestBuildGateway(t *testing.T) { labelSet := map[string]string{ "key": "value", } + protectedPorts := ProtectedPorts{ + 9113: "MetricsPort", + } listenerAllowedRoutes := v1beta1.Listener{ Name: "listener-with-allowed-routes", Hostname: helpers.GetPointer[v1beta1.Hostname]("foo.example.com"), @@ -279,6 +282,7 @@ func TestBuildGateway(t *testing.T) { // invalid listeners invalidProtocolListener := createTCPListener("invalid-protocol", "bar.example.com", 80) invalidPortListener := createHTTPListener("invalid-port", "invalid-port", 0) + invalidProtectedPortListener := createHTTPListener("invalid-protected-port", "invalid-protected-port", 9113) invalidHostnameListener := createHTTPListener("invalid-hostname", "$example.com", 80) invalidHTTPSHostnameListener := createHTTPSListener( "invalid-https-hostname", @@ -532,7 +536,13 @@ func TestBuildGateway(t *testing.T) { }, { gateway: createGateway( - gatewayCfg{listeners: []v1beta1.Listener{invalidPortListener, invalidHTTPSPortListener}}, + gatewayCfg{ + listeners: []v1beta1.Listener{ + invalidPortListener, + invalidHTTPSPortListener, + invalidProtectedPortListener, + }, + }, ), gatewayClass: validGC, expected: &Gateway{ @@ -558,6 +568,16 @@ func TestBuildGateway(t *testing.T) { {Kind: "HTTPRoute"}, }, }, + "invalid-protected-port": { + Source: invalidProtectedPortListener, + Valid: false, + Conditions: staticConds.NewListenerUnsupportedValue( + `port: Invalid value: 9113: port is already in use as MetricsPort`, + ), + SupportedKinds: []v1beta1.RouteGroupKind{ + {Kind: "HTTPRoute"}, + }, + }, }, Valid: true, }, @@ -843,7 +863,7 @@ func TestBuildGateway(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) resolver := newReferenceGrantResolver(test.refGrants) - result := buildGateway(test.gateway, secretResolver, test.gatewayClass, resolver) + result := buildGateway(test.gateway, secretResolver, test.gatewayClass, resolver, protectedPorts) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) } diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 8581c35c59..cc9ab1da4b 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -43,6 +43,9 @@ type Graph struct { ReferencedSecrets map[types.NamespacedName]*Secret } +// ProtectedPorts are the ports that may not be configured by a listener with a descriptive name of each port. +type ProtectedPorts map[int32]string + // IsReferenced returns true if the Graph references the resource. func (g *Graph) IsReferenced(resourceType client.Object, nsname types.NamespacedName) bool { // FIMXE(pleshakov): For now, only works with Secrets. @@ -65,6 +68,7 @@ func BuildGraph( controllerName string, gcName string, validators validation.Validators, + protectedPorts ProtectedPorts, ) *Graph { processedGwClasses, gcExists := processGatewayClasses(state.GatewayClasses, gcName, controllerName) if gcExists && processedGwClasses.Winner == nil { @@ -78,7 +82,7 @@ func BuildGraph( processedGws := processGateways(state.Gateways, gcName) refGrantResolver := newReferenceGrantResolver(state.ReferenceGrants) - gw := buildGateway(processedGws.Winner, secretResolver, gc, refGrantResolver) + gw := buildGateway(processedGws.Winner, secretResolver, gc, refGrantResolver, protectedPorts) routes := buildRoutesForGateways(validators.HTTPFieldsValidator, state.HTTPRoutes, processedGws.GetAllNsNames()) bindRoutesToListeners(routes, gw, state.Namespaces) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 198ac5ea03..efe1bc5c2a 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -22,6 +22,10 @@ func TestBuildGraph(t *testing.T) { controllerName = "my.controller" ) + protectedPorts := ProtectedPorts{ + 9113: "MetricsPort", + } + createValidRuleWithBackendRefs := func(refs []BackendRef) Rule { return Rule{ ValidMatches: true, @@ -343,6 +347,7 @@ func TestBuildGraph(t *testing.T) { controllerName, gcName, validation.Validators{HTTPFieldsValidator: &validationfakes.FakeHTTPFieldsValidator{}}, + protectedPorts, ) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty())