diff --git a/build/Dockerfile.nginx b/build/Dockerfile.nginx index 2a6cae2287..ad12ffe2b3 100644 --- a/build/Dockerfile.nginx +++ b/build/Dockerfile.nginx @@ -1,5 +1,5 @@ # syntax=docker/dockerfile:1.6 -FROM nginx:1.25.5-alpine +FROM nginx:1.25.5-alpine-otel ARG NJS_DIR ARG NGINX_CONF_DIR diff --git a/build/Dockerfile.nginxplus b/build/Dockerfile.nginxplus index b27b7f70f1..dd4ba09fca 100644 --- a/build/Dockerfile.nginxplus +++ b/build/Dockerfile.nginxplus @@ -18,7 +18,7 @@ RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/apk/cert.pem,mode=0644 \ addgroup -g 1001 -S nginx \ && adduser -S -D -H -u 101 -h /var/cache/nginx -s /sbin/nologin -G nginx -g nginx nginx \ && printf "%s\n" "https://pkgs.nginx.com/plus/${NGINX_PLUS_VERSION}/alpine/v$(grep -E -o '^[0-9]+\.[0-9]+' /etc/alpine-release)/main" >> /etc/apk/repositories \ - && apk add --no-cache nginx-plus nginx-plus-module-njs libcap \ + && apk add --no-cache nginx-plus nginx-plus-module-njs nginx-plus-module-otel libcap \ && mkdir -p /var/lib/nginx /usr/lib/nginx/modules \ && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx \ && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx \ diff --git a/charts/nginx-gateway-fabric/README.md b/charts/nginx-gateway-fabric/README.md index bc6871d5ed..bc5084c842 100644 --- a/charts/nginx-gateway-fabric/README.md +++ b/charts/nginx-gateway-fabric/README.md @@ -224,7 +224,7 @@ To uninstall/delete the release `ngf`: ```shell helm uninstall ngf -n nginx-gateway kubectl delete ns nginx-gateway -kubectl delete crd nginxgateways.gateway.nginx.org +kubectl delete crd nginxgateways.gateway.nginx.org nginxproxies.gateway.nginx.org ``` These commands remove all the Kubernetes components associated with the release and deletes the release. @@ -269,6 +269,7 @@ The following tables lists the configurable parameters of the NGINX Gateway Fabr | `nginx.image.tag` | The tag for the NGINX image. | edge | | `nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always | | `nginx.plus` | Is NGINX Plus image being used | false | +| `nginx.config` | The configuration for the data plane that is contained in the NginxProxy resource. | [See nginx.config section](values.yaml) | | `nginx.usage.secretName` | The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting. | | | `nginx.usage.serverURL` | The base server URL of the NGINX Plus usage reporting server. | | | `nginx.usage.clusterName` | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. | | diff --git a/charts/nginx-gateway-fabric/templates/_helpers.tpl b/charts/nginx-gateway-fabric/templates/_helpers.tpl index f94eeb379f..13d78128ae 100644 --- a/charts/nginx-gateway-fabric/templates/_helpers.tpl +++ b/charts/nginx-gateway-fabric/templates/_helpers.tpl @@ -31,6 +31,14 @@ Create control plane config name. {{- printf "%s-config" $name | trunc 63 | trimSuffix "-" }} {{- end }} +{{/* +Create data plane config name. +*/}} +{{- define "nginx-gateway.proxy-config-name" -}} +{{- $name := default .Release.Name .Values.nameOverride }} +{{- printf "%s-proxy-config" $name | trunc 63 | trimSuffix "-" }} +{{- end }} + {{/* Create chart name and version as used by the chart label. */}} diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index dd272d91e9..83aeafde21 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -118,6 +118,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -149,6 +151,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -181,6 +185,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: module-includes + emptyDir: {} - name: nginx-secrets emptyDir: {} - name: nginx-run diff --git a/charts/nginx-gateway-fabric/templates/gatewayclass.yaml b/charts/nginx-gateway-fabric/templates/gatewayclass.yaml index 9bb6a01c17..4ecafd287b 100644 --- a/charts/nginx-gateway-fabric/templates/gatewayclass.yaml +++ b/charts/nginx-gateway-fabric/templates/gatewayclass.yaml @@ -6,3 +6,9 @@ metadata: {{- include "nginx-gateway.labels" . | nindent 4 }} spec: controllerName: {{ .Values.nginxGateway.gatewayControllerName }} + {{- if .Values.nginx.config }} + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: {{ include "nginx-gateway.proxy-config-name" . }} + {{- end }} diff --git a/charts/nginx-gateway-fabric/templates/nginxproxy.yaml b/charts/nginx-gateway-fabric/templates/nginxproxy.yaml new file mode 100644 index 0000000000..4214158b75 --- /dev/null +++ b/charts/nginx-gateway-fabric/templates/nginxproxy.yaml @@ -0,0 +1,10 @@ +{{- if .Values.nginx.config }} +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: {{ include "nginx-gateway.proxy-config-name" . }} + labels: + {{- include "nginx-gateway.labels" . | nindent 4 }} +spec: + {{- toYaml .Values.nginx.config | nindent 2 }} +{{- end }} diff --git a/charts/nginx-gateway-fabric/templates/rbac.yaml b/charts/nginx-gateway-fabric/templates/rbac.yaml index 851ddb42b7..8d29b828b9 100644 --- a/charts/nginx-gateway-fabric/templates/rbac.yaml +++ b/charts/nginx-gateway-fabric/templates/rbac.yaml @@ -10,7 +10,7 @@ metadata: {{- if or .Values.serviceAccount.imagePullSecret .Values.serviceAccount.imagePullSecrets }} imagePullSecrets: {{- if .Values.serviceAccount.imagePullSecret }} - - name: {{ .Values.serviceAccount.imagePullSecret}} + - name: {{ .Values.serviceAccount.imagePullSecret }} {{- end }} {{- if .Values.serviceAccount.imagePullSecrets }} {{- range .Values.serviceAccount.imagePullSecrets }} @@ -115,6 +115,13 @@ rules: - get - list - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + verbs: + - list + - watch - apiGroups: - gateway.nginx.org resources: diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index c0dbe5eb6f..4b7430e9ae 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -70,6 +70,17 @@ nginx: ## Is NGINX Plus image being used plus: false + ## The configuration for the data plane that is contained in the NginxProxy resource. + config: {} + # telemetry: + # exporter: + # endpoint: otel-collector.default.svc:4317 + # interval: 5s + # batchSize: 512 + # batchCount: 4 + # serviceName: "" + # spanAttributes: [] + ## Configuration for NGINX Plus usage reporting. usage: ## The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting. diff --git a/conformance/provisioner/static-deployment.yaml b/conformance/provisioner/static-deployment.yaml index 58396a2a0a..dc59c6a929 100644 --- a/conformance/provisioner/static-deployment.yaml +++ b/conformance/provisioner/static-deployment.yaml @@ -70,6 +70,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -94,6 +96,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -111,6 +115,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: module-includes + emptyDir: {} - name: nginx-secrets emptyDir: {} - name: nginx-run diff --git a/deploy/manifests/nginx-gateway-experimental.yaml b/deploy/manifests/nginx-gateway-experimental.yaml index 7cf2912885..5d6449a98f 100644 --- a/deploy/manifests/nginx-gateway-experimental.yaml +++ b/deploy/manifests/nginx-gateway-experimental.yaml @@ -97,6 +97,13 @@ rules: - get - list - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + verbs: + - list + - watch - apiGroups: - gateway.nginx.org resources: @@ -213,6 +220,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -237,6 +246,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -254,6 +265,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: module-includes + emptyDir: {} - name: nginx-secrets emptyDir: {} - name: nginx-run diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 933e66d4ec..f10dff697a 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -94,6 +94,13 @@ rules: - get - list - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + verbs: + - list + - watch - apiGroups: - gateway.nginx.org resources: @@ -209,6 +216,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -233,6 +242,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -250,6 +261,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: module-includes + emptyDir: {} - name: nginx-secrets emptyDir: {} - name: nginx-run diff --git a/deploy/manifests/nginx-plus-gateway-experimental.yaml b/deploy/manifests/nginx-plus-gateway-experimental.yaml index 88b5249c34..f43819b427 100644 --- a/deploy/manifests/nginx-plus-gateway-experimental.yaml +++ b/deploy/manifests/nginx-plus-gateway-experimental.yaml @@ -103,6 +103,13 @@ rules: - get - list - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + verbs: + - list + - watch - apiGroups: - gateway.nginx.org resources: @@ -220,6 +227,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -244,6 +253,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -261,6 +272,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: module-includes + emptyDir: {} - name: nginx-secrets emptyDir: {} - name: nginx-run diff --git a/deploy/manifests/nginx-plus-gateway.yaml b/deploy/manifests/nginx-plus-gateway.yaml index e0de54f541..c94be584ba 100644 --- a/deploy/manifests/nginx-plus-gateway.yaml +++ b/deploy/manifests/nginx-plus-gateway.yaml @@ -100,6 +100,13 @@ rules: - get - list - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + verbs: + - list + - watch - apiGroups: - gateway.nginx.org resources: @@ -216,6 +223,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -240,6 +249,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: module-includes + mountPath: /etc/nginx/module-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -257,6 +268,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: module-includes + emptyDir: {} - name: nginx-secrets emptyDir: {} - name: nginx-run diff --git a/docs/proposals/gateway-settings.md b/docs/proposals/gateway-settings.md index e910f47fb7..cd0b0e8103 100644 --- a/docs/proposals/gateway-settings.md +++ b/docs/proposals/gateway-settings.md @@ -1,7 +1,7 @@ # Enhancement Proposal-1775: Gateway Settings - Issue: https://github.com/nginxinc/nginx-gateway-fabric/issues/1775 -- Status: Implementable +- Status: Completed ## Summary @@ -93,7 +93,7 @@ type Telemetry struct { // SpanAttributes are custom key/value attributes that are added to each span. // // +optional - SpanAttributes map[string]string `json:"spanAttributes,omitempty"` + SpanAttributes []SpanAttribute `json:"spanAttributes,omitempty"` } // TelemetryExporter specifies OpenTelemetry export parameters. @@ -122,6 +122,15 @@ type TelemetryExporter struct { // The format is a subset of the syntax parsed by Golang time.ParseDuration. // Examples: 1h, 12m, 30s, 150ms. type Duration string + +// SpanAttribute is a key value pair to be added to a tracing span. +type SpanAttribute struct { + // Key is the key for a span attribute. + Key string `json:"key"` + + // Value is the value for a span attribute. + Value string `json:"value"` +} ``` ### Status diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index d810584a69..cb68557165 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -116,6 +116,7 @@ func StartManager(cfg config.Config) error { Logger: cfg.Logger.WithName("changeProcessor"), Validators: validation.Validators{ HTTPFieldsValidator: ngxvalidation.HTTPValidator{}, + GenericValidator: ngxvalidation.GenericValidator{}, }, EventRecorder: recorder, Scheme: scheme, @@ -414,6 +415,12 @@ func registerControllers( ), }, }, + { + objectType: &ngfAPI.NginxProxy{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, } if cfg.ExperimentalFeatures { @@ -592,6 +599,7 @@ func prepareFirstEventBatchPreparerArgs( &discoveryV1.EndpointSliceList{}, &gatewayv1.HTTPRouteList{}, &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, partialObjectMetadataList, } diff --git a/internal/mode/static/manager_test.go b/internal/mode/static/manager_test.go index 532807d374..e28fa3c7bb 100644 --- a/internal/mode/static/manager_test.go +++ b/internal/mode/static/manager_test.go @@ -16,6 +16,7 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ) @@ -52,6 +53,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &gatewayv1.HTTPRouteList{}, &gatewayv1.GatewayList{}, &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, partialObjectMetadataList, }, }, @@ -72,6 +74,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &discoveryV1.EndpointSliceList{}, &gatewayv1.HTTPRouteList{}, &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, partialObjectMetadataList, }, }, @@ -93,6 +96,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &discoveryV1.EndpointSliceList{}, &gatewayv1.HTTPRouteList{}, &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, partialObjectMetadataList, &gatewayv1alpha2.BackendTLSPolicyList{}, }, diff --git a/internal/mode/static/nginx/conf/nginx-plus.conf b/internal/mode/static/nginx/conf/nginx-plus.conf index f3e1da0806..b8b9cc77d8 100644 --- a/internal/mode/static/nginx/conf/nginx-plus.conf +++ b/internal/mode/static/nginx/conf/nginx-plus.conf @@ -1,4 +1,5 @@ load_module /usr/lib/nginx/modules/ngx_http_js_module.so; +include /etc/nginx/module-includes/*.conf; worker_processes auto; diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index 1abe5c879a..b70e857e0d 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -1,4 +1,5 @@ load_module /usr/lib/nginx/modules/ngx_http_js_module.so; +include /etc/nginx/module-includes/*.conf; worker_processes auto; diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 6127ffa094..6ed5afb94d 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -15,6 +15,10 @@ const ( // httpFolder is the folder where NGINX HTTP configuration files are stored. httpFolder = configFolder + "/conf.d" + + // modulesIncludesFolder is the folder where the included "load_module" file is stored. + modulesIncludesFolder = configFolder + "/module-includes" + // secretsFolder is the folder where secrets (like TLS certs/keys) are stored. secretsFolder = configFolder + "/secrets" @@ -26,10 +30,13 @@ const ( // httpMatchVarsFile is the path to the http_match pairs configuration file. httpMatchVarsFile = httpFolder + "/matches.json" + + // loadModulesFile is the path to the file containing any load_module directives. + loadModulesFile = modulesIncludesFolder + "/load-modules.conf" ) // ConfigFolders is a list of folders where NGINX configuration files are stored. -var ConfigFolders = []string{httpFolder, secretsFolder} +var ConfigFolders = []string{httpFolder, secretsFolder, modulesIncludesFolder} // Generator generates NGINX configuration files. // This interface is used for testing purposes only. @@ -82,6 +89,8 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File { files = append(files, generateCertBundle(id, bundle)) } + files = append(files, generateLoadModulesConf(conf)) + return files } @@ -142,6 +151,7 @@ func (g GeneratorImpl) getExecuteFuncs() []executeFunc { g.executeUpstreams, executeSplitClients, executeMaps, + executeTelemetry, } } @@ -155,3 +165,16 @@ func generateConfigVersion(configVersion int) file.File { Type: file.TypeRegular, } } + +func generateLoadModulesConf(conf dataplane.Configuration) file.File { + var c []byte + if conf.Telemetry.Endpoint != "" { + c = []byte("load_module modules/ngx_otel_module.so;") + } + + return file.File{ + Content: c, + Path: loadModulesFile, + Type: file.TypeRegular, + } +} diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index a4e334ea74..7f2bc0f7a4 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -63,6 +63,13 @@ func TestGenerate(t *testing.T) { CertBundles: map[dataplane.CertBundleID]dataplane.CertBundle{ "test-certbundle": []byte("test-cert"), }, + Telemetry: dataplane.Telemetry{ + Endpoint: "1.2.3.4:123", + ServiceName: "ngf:gw-ns:gw-name:my-name", + Interval: "5s", + BatchSize: 512, + BatchCount: 4, + }, } g := NewWithT(t) @@ -71,7 +78,7 @@ func TestGenerate(t *testing.T) { files := generator.Generate(conf) - g.Expect(files).To(HaveLen(5)) + g.Expect(files).To(HaveLen(6)) arrange := func(i, j int) bool { return files[i].Path < files[j].Path } @@ -92,16 +99,26 @@ func TestGenerate(t *testing.T) { g.Expect(httpCfg).To(ContainSubstring("upstream")) g.Expect(httpCfg).To(ContainSubstring("split_clients")) + g.Expect(httpCfg).To(ContainSubstring("endpoint 1.2.3.4:123;")) + g.Expect(httpCfg).To(ContainSubstring("interval 5s;")) + g.Expect(httpCfg).To(ContainSubstring("batch_size 512;")) + g.Expect(httpCfg).To(ContainSubstring("batch_count 4;")) + g.Expect(httpCfg).To(ContainSubstring("otel_service_name ngf:gw-ns:gw-name:my-name;")) + g.Expect(files[2].Path).To(Equal("/etc/nginx/conf.d/matches.json")) + g.Expect(files[2].Type).To(Equal(file.TypeRegular)) expString := "{}" g.Expect(string(files[2].Content)).To(Equal(expString)) - g.Expect(files[3].Path).To(Equal("/etc/nginx/secrets/test-certbundle.crt")) - certBundle := string(files[3].Content) + g.Expect(files[3].Path).To(Equal("/etc/nginx/module-includes/load-modules.conf")) + g.Expect(files[3].Content).To(Equal([]byte("load_module modules/ngx_otel_module.so;"))) + + g.Expect(files[4].Path).To(Equal("/etc/nginx/secrets/test-certbundle.crt")) + certBundle := string(files[4].Content) g.Expect(certBundle).To(Equal("test-cert")) - g.Expect(files[4]).To(Equal(file.File{ + g.Expect(files[5]).To(Equal(file.File{ Type: file.TypeSecret, Path: "/etc/nginx/secrets/test-keypair.pem", Content: []byte("test-cert\ntest-key"), diff --git a/internal/mode/static/nginx/config/maps_template.go b/internal/mode/static/nginx/config/maps_template.go index 9233b0e403..4b4407a1a4 100644 --- a/internal/mode/static/nginx/config/maps_template.go +++ b/internal/mode/static/nginx/config/maps_template.go @@ -1,6 +1,6 @@ package config -var mapsTemplateText = ` +const mapsTemplateText = ` {{ range $m := . }} map {{ $m.Source }} {{ $m.Variable }} { {{ range $p := $m.Parameters }} diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 8d272e05a7..c05fa7cea7 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -1,6 +1,6 @@ package config -var serversTemplateText = ` +const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := . -}} {{ if $s.IsDefaultSSL -}} diff --git a/internal/mode/static/nginx/config/split_clients_template.go b/internal/mode/static/nginx/config/split_clients_template.go index da51dc5cf1..6d2a0e208f 100644 --- a/internal/mode/static/nginx/config/split_clients_template.go +++ b/internal/mode/static/nginx/config/split_clients_template.go @@ -1,6 +1,6 @@ package config -var splitClientsTemplateText = ` +const splitClientsTemplateText = ` {{ range $sc := . }} split_clients $request_id ${{ $sc.VariableName }} { {{- range $d := $sc.Distributions }} diff --git a/internal/mode/static/nginx/config/telemetry.go b/internal/mode/static/nginx/config/telemetry.go new file mode 100644 index 0000000000..fef1e4ec27 --- /dev/null +++ b/internal/mode/static/nginx/config/telemetry.go @@ -0,0 +1,22 @@ +package config + +import ( + gotemplate "text/template" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +var otelTemplate = gotemplate.Must(gotemplate.New("otel").Parse(otelTemplateText)) + +func executeTelemetry(conf dataplane.Configuration) []executeResult { + if conf.Telemetry.Endpoint != "" { + result := executeResult{ + dest: httpConfigFile, + data: execute(otelTemplate, conf.Telemetry), + } + + return []executeResult{result} + } + + return nil +} diff --git a/internal/mode/static/nginx/config/telemetry_template.go b/internal/mode/static/nginx/config/telemetry_template.go new file mode 100644 index 0000000000..5152075835 --- /dev/null +++ b/internal/mode/static/nginx/config/telemetry_template.go @@ -0,0 +1,22 @@ +package config + +const otelTemplateText = ` +otel_exporter { + endpoint {{ .Endpoint }}; + {{- if .Interval }} + interval {{ .Interval }}; + {{- end }} + {{- if .BatchSize }} + batch_size {{ .BatchSize }}; + {{- end }} + {{- if .BatchCount }} + batch_count {{ .BatchCount }}; + {{- end }} +} + +otel_service_name {{ .ServiceName }}; + +{{- range $attr := .SpanAttributes }} +otel_span_attr "{{ $attr.Key }}" "{{ $attr.Value }}"; +{{- end }} +` diff --git a/internal/mode/static/nginx/config/telemetry_test.go b/internal/mode/static/nginx/config/telemetry_test.go new file mode 100644 index 0000000000..27226cfbaa --- /dev/null +++ b/internal/mode/static/nginx/config/telemetry_test.go @@ -0,0 +1,59 @@ +package config + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +func TestExecuteTelemetry(t *testing.T) { + conf := dataplane.Configuration{ + Telemetry: dataplane.Telemetry{ + Endpoint: "1.2.3.4:123", + ServiceName: "ngf:gw-ns:gw-name:my-name", + Interval: "5s", + BatchSize: 512, + BatchCount: 4, + SpanAttributes: []dataplane.SpanAttribute{ + { + Key: "key1", + Value: "val1", + }, + { + Key: "key2", + Value: "val2", + }, + }, + }, + } + + g := NewWithT(t) + expSubStrings := map[string]int{ + "endpoint 1.2.3.4:123;": 1, + "otel_service_name ngf:gw-ns:gw-name:my-name;": 1, + "interval 5s;": 1, + "batch_size 512;": 1, + "batch_count 4;": 1, + "otel_span_attr": 2, + } + + for expSubStr, expCount := range expSubStrings { + res := executeTelemetry(conf) + g.Expect(res).To(HaveLen(1)) + g.Expect(expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) + } +} + +func TestExecuteTelemetryNil(t *testing.T) { + conf := dataplane.Configuration{ + Telemetry: dataplane.Telemetry{}, + } + + g := NewWithT(t) + + res := executeTelemetry(conf) + g.Expect(res).To(BeEmpty()) +} diff --git a/internal/mode/static/nginx/config/upstreams_template.go b/internal/mode/static/nginx/config/upstreams_template.go index c2d9a95490..e57b98cb37 100644 --- a/internal/mode/static/nginx/config/upstreams_template.go +++ b/internal/mode/static/nginx/config/upstreams_template.go @@ -4,7 +4,7 @@ package config // 512k will support up to 648 upstream servers for OSS. // NGINX Plus needs 1m to support roughly the same amount of servers (556 upstream servers). // https://github.com/nginxinc/nginx-gateway-fabric/issues/483 -var upstreamsTemplateText = ` +const upstreamsTemplateText = ` {{ range $u := . }} upstream {{ $u.Name }} { random two least_conn; diff --git a/internal/mode/static/nginx/config/validation/generic.go b/internal/mode/static/nginx/config/validation/generic.go new file mode 100644 index 0000000000..bac2ecf827 --- /dev/null +++ b/internal/mode/static/nginx/config/validation/generic.go @@ -0,0 +1,83 @@ +package validation + +import ( + "errors" + "regexp" + + k8svalidation "k8s.io/apimachinery/pkg/util/validation" +) + +// GenericValidator validates values for generic cases in the nginx conf. +type GenericValidator struct{} + +// ValidateEscapedStringNoVarExpansion ensures that no invalid characters are included in the string value that +// could lead to unwanted nginx behavior. +func (GenericValidator) ValidateEscapedStringNoVarExpansion(value string) error { + return validateEscapedStringNoVarExpansion(value, nil) +} + +const ( + alphaNumericStringFmt = `[a-zA-Z0-9_-]+` + alphaNumericStringErrMsg = "must contain only alphanumeric characters or '-' or '_'" +) + +var alphaNumericStringFmtRegexp = regexp.MustCompile("^" + alphaNumericStringFmt + "$") + +// ValidateServiceName validates a service name that can only use alphanumeric characters. +func (GenericValidator) ValidateServiceName(name string) error { + if !alphaNumericStringFmtRegexp.MatchString(name) { + examples := []string{ + "svc1", + "svc-1", + "svc_1", + } + + return errors.New(k8svalidation.RegexError(alphaNumericStringErrMsg, alphaNumericStringFmt, examples...)) + } + + return nil +} + +const ( + durationStringFmt = `\d{1,4}(ms|s)?` + durationStringErrMsg = "must contain a number followed by 'ms' or 's'" +) + +var durationStringFmtRegexp = regexp.MustCompile("^" + durationStringFmt + "$") + +// ValidateNginxDuration validates a duration string that nginx can understand. +func (GenericValidator) ValidateNginxDuration(duration string) error { + if !durationStringFmtRegexp.MatchString(duration) { + examples := []string{ + "5ms", + "10s", + } + + return errors.New(k8svalidation.RegexError(durationStringFmt, durationStringErrMsg, examples...)) + } + + return nil +} + +const ( + //nolint:lll + endpointStringFmt = `(?:http?:\/\/)?[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(?::\d{1,5})?` + endpointStringErrMsg = "must be an alphanumeric hostname with optional http scheme and optional port" +) + +var endpointStringFmtRegexp = regexp.MustCompile("^" + endpointStringFmt + "$") + +// ValidateEndpoint validates an alphanumeric endpoint, with optional http scheme and port. +func (GenericValidator) ValidateEndpoint(endpoint string) error { + if !endpointStringFmtRegexp.MatchString(endpoint) { + examples := []string{ + "my-endpoint", + "my.endpoint:5678", + "http://my-endpoint", + } + + return errors.New(k8svalidation.RegexError(endpointStringFmt, endpointStringErrMsg, examples...)) + } + + return nil +} diff --git a/internal/mode/static/nginx/config/validation/generic_test.go b/internal/mode/static/nginx/config/validation/generic_test.go new file mode 100644 index 0000000000..b07131ccf0 --- /dev/null +++ b/internal/mode/static/nginx/config/validation/generic_test.go @@ -0,0 +1,86 @@ +package validation + +import "testing" + +func TestGenericValidator_ValidateEscapedStringNoVarExpansion(t *testing.T) { + validator := GenericValidator{} + + testValidValuesForSimpleValidator( + t, + validator.ValidateEscapedStringNoVarExpansion, + `test`, + `test test`, + `\"`, + `\\`, + ) + + testInvalidValuesForSimpleValidator( + t, + validator.ValidateEscapedStringNoVarExpansion, + `\`, + `test"test`, + `$test`, + ) +} + +func TestValidateServiceName(t *testing.T) { + validator := GenericValidator{} + + testValidValuesForSimpleValidator( + t, + validator.ValidateServiceName, + `test`, + `Test-test`, + `test_Test`, + `test123`, + ) + + testInvalidValuesForSimpleValidator( + t, + validator.ValidateServiceName, + `test#$%`, + `test test`, + `test.test`, + ) +} + +func TestValidateNginxDuration(t *testing.T) { + validator := GenericValidator{} + + testValidValuesForSimpleValidator( + t, + validator.ValidateNginxDuration, + `5ms`, + `10s`, + `123ms`, + ) + + testInvalidValuesForSimpleValidator( + t, + validator.ValidateNginxDuration, + `test`, + `12345`, + `5m`, + ) +} + +func TestValidateEndpoint(t *testing.T) { + validator := GenericValidator{} + + testValidValuesForSimpleValidator( + t, + validator.ValidateEndpoint, + `http://my-endpoint:5678`, + `my.endpoint`, + `myendpoint:123`, + `my-endpoint123:456`, + ) + + testInvalidValuesForSimpleValidator( + t, + validator.ValidateEndpoint, + `https://my-endpoint`, + `my_endpoint`, + `my$endpoint`, + ) +} diff --git a/internal/mode/static/nginx/config/version_template.go b/internal/mode/static/nginx/config/version_template.go index 3f0d3ea544..ccf46e02cc 100644 --- a/internal/mode/static/nginx/config/version_template.go +++ b/internal/mode/static/nginx/config/version_template.go @@ -1,6 +1,6 @@ package config -var versionTemplateText = ` +const versionTemplateText = ` server { listen unix:/var/run/nginx/nginx-config-version.sock; access_log off; diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go index db035746c4..e5ae41cc52 100644 --- a/internal/mode/static/state/change_processor.go +++ b/internal/mode/static/state/change_processor.go @@ -19,6 +19,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" @@ -105,6 +106,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { CRDMetadata: make(map[types.NamespacedName]*metav1.PartialObjectMetadata), BackendTLSPolicies: make(map[types.NamespacedName]*v1alpha2.BackendTLSPolicy), ConfigMaps: make(map[types.NamespacedName]*apiv1.ConfigMap), + NginxProxies: make(map[types.NamespacedName]*ngfAPI.NginxProxy), } extractGVK := func(obj client.Object) schema.GroupVersionKind { @@ -182,6 +184,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: newObjectStoreMapAdapter(clusterStore.CRDMetadata), predicate: annotationChangedPredicate{annotation: gatewayclass.BundleVersionAnnotation}, }, + { + gvk: extractGVK(&ngfAPI.NginxProxy{}), + store: newObjectStoreMapAdapter(clusterStore.NginxProxies), + predicate: funcPredicate{stateChanged: isReferenced}, + }, }, ) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index fab678497e..aeb8d9e0bc 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -17,6 +17,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" @@ -171,10 +172,9 @@ func createBackendRef( } func createAlwaysValidValidators() validation.Validators { - http := &validationfakes.FakeHTTPFieldsValidator{} - return validation.Validators{ - HTTPFieldsValidator: http, + HTTPFieldsValidator: &validationfakes.FakeHTTPFieldsValidator{}, + GenericValidator: &validationfakes.FakeGenericValidator{}, } } @@ -187,6 +187,7 @@ func createScheme() *runtime.Scheme { utilruntime.Must(apiv1.AddToScheme(scheme)) utilruntime.Must(discoveryV1.AddToScheme(scheme)) utilruntime.Must(apiext.AddToScheme(scheme)) + utilruntime.Must(ngfAPI.AddToScheme(scheme)) return scheme } @@ -1523,6 +1524,60 @@ var _ = Describe("ChangeProcessor", func() { }) }) }) + + Describe("NginxProxy resource changes", Ordered, func() { + paramGC := gc.DeepCopy() + paramGC.Spec.ParametersRef = &v1beta1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1beta1.Kind("NginxProxy"), + Name: "np", + } + + np := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "np", + }, + } + + npUpdated := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "np", + }, + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + Exporter: &ngfAPI.TelemetryExporter{ + Endpoint: "my-svc:123", + BatchSize: helpers.GetPointer(int32(512)), + BatchCount: helpers.GetPointer(int32(4)), + Interval: helpers.GetPointer(ngfAPI.Duration("5s")), + }, + }, + }, + } + It("handles upserts for an NginxProxy", func() { + processor.CaptureUpsertChange(np) + processor.CaptureUpsertChange(paramGC) + + changed, graph := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(graph.NginxProxy).To(Equal(np)) + }) + It("captures changes for an NginxProxy", func() { + processor.CaptureUpsertChange(npUpdated) + processor.CaptureUpsertChange(paramGC) + + changed, graph := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(graph.NginxProxy).To(Equal(npUpdated)) + }) + It("handles deletes for an NginxProxy", func() { + processor.CaptureDeleteChange(np, client.ObjectKeyFromObject(np)) + + changed, graph := processor.Process() + Expect(changed).To(Equal(state.ClusterStateChange)) + Expect(graph.NginxProxy).To(BeNil()) + }) + }) }) Describe("Ensuring non-changing changes don't override previously changing changes", func() { @@ -1531,18 +1586,19 @@ var _ = Describe("ChangeProcessor", func() { //nolint:lll var ( - processor *state.ChangeProcessorImpl - gcNsName, gwNsName, hrNsName, hr2NsName, rgNsName, svcNsName, sliceNsName, secretNsName, cmNsName, btlsNsName types.NamespacedName - gc, gcUpdated *v1.GatewayClass - gw1, gw1Updated, gw2 *v1.Gateway - hr1, hr1Updated, hr2 *v1.HTTPRoute - rg1, rg1Updated, rg2 *v1beta1.ReferenceGrant - svc, barSvc, unrelatedSvc *apiv1.Service - slice, barSlice, unrelatedSlice *discoveryV1.EndpointSlice - ns, unrelatedNS, testNs, barNs *apiv1.Namespace - secret, secretUpdated, unrelatedSecret, barSecret, barSecretUpdated *apiv1.Secret - cm, cmUpdated, unrelatedCM *apiv1.ConfigMap - btls, btlsUpdated *v1alpha2.BackendTLSPolicy + processor *state.ChangeProcessorImpl + gcNsName, gwNsName, hrNsName, hr2NsName, rgNsName, svcNsName, sliceNsName, secretNsName, cmNsName, btlsNsName, npNsName types.NamespacedName + gc, gcUpdated *v1.GatewayClass + gw1, gw1Updated, gw2 *v1.Gateway + hr1, hr1Updated, hr2 *v1.HTTPRoute + rg1, rg1Updated, rg2 *v1beta1.ReferenceGrant + svc, barSvc, unrelatedSvc *apiv1.Service + slice, barSlice, unrelatedSlice *discoveryV1.EndpointSlice + ns, unrelatedNS, testNs, barNs *apiv1.Namespace + secret, secretUpdated, unrelatedSecret, barSecret, barSecretUpdated *apiv1.Secret + cm, cmUpdated, unrelatedCM *apiv1.ConfigMap + btls, btlsUpdated *v1alpha2.BackendTLSPolicy + np, npUpdated *ngfAPI.NginxProxy ) BeforeEach(OncePerOrdered, func() { @@ -1834,6 +1890,19 @@ var _ = Describe("ChangeProcessor", func() { }, } btlsUpdated = btls.DeepCopy() + + npNsName = types.NamespacedName{Name: "np-1"} + np = &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: npNsName.Name, + }, + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + ServiceName: helpers.GetPointer("my-svc"), + }, + }, + } + npUpdated = np.DeepCopy() }) // Changing change - a change that makes processor.Process() report changed // Non-changing change - a change that doesn't do that @@ -1851,6 +1920,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(rg1) processor.CaptureUpsertChange(btls) processor.CaptureUpsertChange(cm) + processor.CaptureUpsertChange(np) changed, _ := processor.Process() Expect(changed).To(Equal(state.ClusterStateChange)) @@ -1864,6 +1934,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(rg1Updated) processor.CaptureUpsertChange(btlsUpdated) processor.CaptureUpsertChange(cmUpdated) + processor.CaptureUpsertChange(npUpdated) // there are non-changing changes processor.CaptureUpsertChange(gcUpdated) @@ -1872,6 +1943,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(rg1Updated) processor.CaptureUpsertChange(btlsUpdated) processor.CaptureUpsertChange(cmUpdated) + processor.CaptureUpsertChange(npUpdated) changed, _ := processor.Process() Expect(changed).To(Equal(state.ClusterStateChange)) @@ -1895,6 +1967,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureDeleteChange(&v1beta1.ReferenceGrant{}, rgNsName) processor.CaptureDeleteChange(&v1alpha2.BackendTLSPolicy{}, btlsNsName) processor.CaptureDeleteChange(&apiv1.ConfigMap{}, cmNsName) + processor.CaptureDeleteChange(&ngfAPI.NginxProxy{}, npNsName) // these are non-changing changes processor.CaptureUpsertChange(gw2) diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index 894fb3596c..9675112b40 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -31,11 +31,11 @@ const ( // RouteReasonBackendRefUnsupportedValue is used with the "ResolvedRefs" condition when one of the // Route rules has a backendRef with an unsupported value. - RouteReasonBackendRefUnsupportedValue = "UnsupportedValue" + RouteReasonBackendRefUnsupportedValue v1.RouteConditionReason = "UnsupportedValue" // RouteReasonInvalidGateway is used with the "Accepted" (false) condition when the Gateway the Route // references is invalid. - RouteReasonInvalidGateway = "InvalidGateway" + RouteReasonInvalidGateway v1.RouteConditionReason = "InvalidGateway" // RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener. RouteReasonInvalidListener v1.RouteConditionReason = "InvalidListener" @@ -66,6 +66,17 @@ const ( RouteMessageFailedNginxReload = GatewayMessageFailedNginxReload + ". NGINX may still be configured " + "for this HTTPRoute. However, future updates to this resource will not be configured until the Gateway " + "is programmed again" + + // GatewayClassResolvedRefs condition indicates whether the controller was able to resolve the + // parametersRef on the GatewayClass. + GatewayClassResolvedRefs v1.GatewayClassConditionType = "ResolvedRefs" + + // GatewayClassReasonResolvedRefs is used with the "GatewayClassResolvedRefs" condition when the condition is true. + GatewayClassReasonResolvedRefs v1.GatewayClassConditionReason = "ResolvedRefs" + + // GatewayClassReasonParamsRefNotFound is used with the "GatewayClassResolvedRefs" condition when the + // parametersRef resource does not exist. + GatewayClassReasonParamsRefNotFound v1.GatewayClassConditionReason = "ParametersRefNotFound" ) // NewTODO returns a Condition that can be used as a placeholder for a condition that is not yet implemented. @@ -203,7 +214,7 @@ func NewRouteBackendRefUnsupportedValue(msg string) conditions.Condition { return conditions.Condition{ Type: string(v1.RouteConditionResolvedRefs), Status: metav1.ConditionFalse, - Reason: RouteReasonBackendRefUnsupportedValue, + Reason: string(RouteReasonBackendRefUnsupportedValue), Message: msg, } } @@ -214,7 +225,7 @@ func NewRouteInvalidGateway() conditions.Condition { return conditions.Condition{ Type: string(v1.RouteConditionAccepted), Status: metav1.ConditionFalse, - Reason: RouteReasonInvalidGateway, + Reason: string(RouteReasonInvalidGateway), Message: "Gateway is invalid", } } @@ -402,13 +413,37 @@ func NewListenerRefNotPermitted(msg string) []conditions.Condition { } } +// NewGatewayClassResolvedRefs returns a Condition that indicates that the parametersRef +// on the GatewayClass is resolved. +func NewGatewayClassResolvedRefs() conditions.Condition { + return conditions.Condition{ + Type: string(GatewayClassResolvedRefs), + Status: metav1.ConditionTrue, + Reason: string(GatewayClassReasonResolvedRefs), + Message: "parametersRef resource is resolved", + } +} + +// NewGatewayClassRefNotFound returns a Condition that indicates that the parametersRef +// on the GatewayClass could not be resolved. +func NewGatewayClassRefNotFound() conditions.Condition { + return conditions.Condition{ + Type: string(GatewayClassResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(GatewayClassReasonParamsRefNotFound), + Message: "parametersRef resource could not be found", + } +} + // NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters. +// We are allowing Accepted to still be true to prevent nullifying the entire config tree if a parametersRef +// is updated to something invalid. func NewGatewayClassInvalidParameters(msg string) conditions.Condition { return conditions.Condition{ Type: string(v1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: string(v1.GatewayClassReasonInvalidParameters), - Message: msg, + Message: fmt.Sprintf("GatewayClass is accepted, but parametersRef is ignored due to an error: %s", msg), } } diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 25ed7e217a..d9f0e4264e 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -40,6 +40,7 @@ func BuildConfiguration( backendGroups := buildBackendGroups(append(httpServers, sslServers...)) keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners) certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) + telemetry := buildTelemetry(g) config := Configuration{ HTTPServers: httpServers, @@ -49,6 +50,7 @@ func BuildConfiguration( SSLKeyPairs: keyPairs, Version: configVersion, CertBundles: certBundles, + Telemetry: telemetry, } return config @@ -559,3 +561,44 @@ func generateSSLKeyPairID(secret types.NamespacedName) SSLKeyPairID { func generateCertBundleID(configMap types.NamespacedName) CertBundleID { return CertBundleID(fmt.Sprintf("cert_bundle_%s_%s", configMap.Namespace, configMap.Name)) } + +// buildTelemetry generates the Otel configuration. +func buildTelemetry(g *graph.Graph) Telemetry { + if g.NginxProxy == nil || g.NginxProxy.Spec.Telemetry == nil || g.NginxProxy.Spec.Telemetry.Exporter == nil { + return Telemetry{} + } + + serviceName := fmt.Sprintf("ngf:%s:%s", g.Gateway.Source.Namespace, g.Gateway.Source.Name) + telemetry := g.NginxProxy.Spec.Telemetry + if telemetry.ServiceName != nil { + serviceName = serviceName + ":" + *telemetry.ServiceName + } + + tel := Telemetry{ + Endpoint: telemetry.Exporter.Endpoint, + ServiceName: serviceName, + } + + spanAttrs := make([]SpanAttribute, 0, len(g.NginxProxy.Spec.Telemetry.SpanAttributes)) + for _, spanAttr := range g.NginxProxy.Spec.Telemetry.SpanAttributes { + sa := SpanAttribute{ + Key: spanAttr.Key, + Value: spanAttr.Value, + } + spanAttrs = append(spanAttrs, sa) + } + + tel.SpanAttributes = spanAttrs + + if telemetry.Exporter.BatchCount != nil { + tel.BatchCount = *telemetry.Exporter.BatchCount + } + if telemetry.Exporter.BatchSize != nil { + tel.BatchSize = *telemetry.Exporter.BatchSize + } + if telemetry.Exporter.Interval != nil { + tel.Interval = string(*telemetry.Exporter.Interval) + } + + return tel +} diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 4a44b5c5b7..5ffde9f604 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -14,6 +14,7 @@ import ( v1 "sigs.k8s.io/gateway-api/apis/v1" v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" @@ -539,6 +540,20 @@ func TestBuildConfiguration(t *testing.T) { }, } + nginxProxy := &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + Exporter: &ngfAPI.TelemetryExporter{ + Endpoint: "my-otel.svc:4563", + BatchSize: helpers.GetPointer(int32(512)), + BatchCount: helpers.GetPointer(int32(4)), + Interval: helpers.GetPointer(ngfAPI.Duration("5s")), + }, + ServiceName: helpers.GetPointer("my-svc"), + }, + }, + } + tests := []struct { graph *graph.Graph msg string @@ -1858,6 +1873,52 @@ func TestBuildConfiguration(t *testing.T) { }, msg: "https listener with httproute with backend that has a backend TLS policy with binaryData attached", }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + }, + }, + Listeners: []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{}, + }, + }, + }, + Routes: map[types.NamespacedName]*graph.Route{}, + NginxProxy: nginxProxy, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + Port: 80, + }, + }, + SSLServers: []VirtualServer{}, + SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, + CertBundles: map[CertBundleID]CertBundle{}, + Telemetry: Telemetry{ + Endpoint: "my-otel.svc:4563", + Interval: "5s", + BatchSize: 512, + BatchCount: 4, + ServiceName: "ngf:ns:gw:my-svc", + SpanAttributes: []SpanAttribute{}, + }, + }, + msg: "NginxProxy with tracing config", + }, } for _, test := range tests { @@ -1873,6 +1934,7 @@ func TestBuildConfiguration(t *testing.T) { g.Expect(result.SSLKeyPairs).To(Equal(test.expConf.SSLKeyPairs)) g.Expect(result.Version).To(Equal(1)) g.Expect(result.CertBundles).To(Equal(test.expConf.CertBundles)) + g.Expect(result.Telemetry).To(Equal(test.expConf.Telemetry)) }) } } @@ -2511,3 +2573,69 @@ func TestConvertBackendTLS(t *testing.T) { }) } } + +func TestBuildTelemetry(t *testing.T) { + telemetryConfigured := &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + Exporter: &ngfAPI.TelemetryExporter{ + Endpoint: "my-otel.svc:4563", + BatchSize: helpers.GetPointer(int32(512)), + BatchCount: helpers.GetPointer(int32(4)), + Interval: helpers.GetPointer(ngfAPI.Duration("5s")), + }, + ServiceName: helpers.GetPointer("my-svc"), + SpanAttributes: []ngfAPI.SpanAttribute{ + {Key: "key", Value: "value"}, + }, + }, + }, + } + + expTelemetryConfigured := Telemetry{ + Endpoint: "my-otel.svc:4563", + ServiceName: "ngf:ns:gw:my-svc", + Interval: "5s", + BatchSize: 512, + BatchCount: 4, + SpanAttributes: []SpanAttribute{ + {Key: "key", Value: "value"}, + }, + } + + tests := []struct { + g *graph.Graph + msg string + expTelemetry Telemetry + }{ + { + g: &graph.Graph{ + NginxProxy: &ngfAPI.NginxProxy{}, + }, + expTelemetry: Telemetry{}, + msg: "No telemetry configured", + }, + { + g: &graph.Graph{ + Gateway: &graph.Gateway{ + Source: &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + }, + }, + }, + NginxProxy: telemetryConfigured, + }, + expTelemetry: expTelemetryConfigured, + msg: "Telemetry configured", + }, + } + + for _, tc := range tests { + t.Run(tc.msg, func(t *testing.T) { + g := NewWithT(t) + g.Expect(buildTelemetry(tc.g)).To(Equal(tc.expTelemetry)) + }) + } +} diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index da1b00afaa..21c5749dfb 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -33,6 +33,8 @@ type Configuration struct { Upstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup + // Telemetry holds the Otel configuration. + Telemetry Telemetry // Version represents the version of the generated configuration. Version int } @@ -249,3 +251,27 @@ type VerifyTLS struct { Hostname string RootCAPath string } + +// Telemetry represents Otel configuration for the dataplane. +type Telemetry struct { + // Endpoint specifies the address of OTLP/gRPC endpoint that will accept telemetry data. + Endpoint string + // ServiceName is the “service.name” attribute of the OTel resource. + ServiceName string + // Interval specifies the export interval. + Interval string + // SpanAttributes are custom key/value attributes that are added to each span. + SpanAttributes []SpanAttribute + // BatchSize specifies the maximum number of spans to be sent in one batch per worker. + BatchSize int32 + // BatchCount specifies the number of pending batches per worker, spans exceeding the limit are dropped. + BatchCount int32 +} + +// SpanAttribute is a key value pair to be added to a tracing span. +type SpanAttribute struct { + // Key is the key for a span attribute. + Key string + // Value is the value for a span attribute. + Value string +} diff --git a/internal/mode/static/state/graph/gatewayclass.go b/internal/mode/static/state/graph/gatewayclass.go index fc3313f269..23bbb078e6 100644 --- a/internal/mode/static/state/graph/gatewayclass.go +++ b/internal/mode/static/state/graph/gatewayclass.go @@ -1,15 +1,19 @@ package graph import ( + "errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" ) // GatewayClass represents the GatewayClass resource. @@ -60,13 +64,15 @@ func processGatewayClasses( func buildGatewayClass( gc *v1.GatewayClass, + npCfg *ngfAPI.NginxProxy, crdVersions map[types.NamespacedName]*metav1.PartialObjectMetadata, + validator validation.GenericValidator, ) *GatewayClass { if gc == nil { return nil } - conds, valid := validateGatewayClass(gc, crdVersions) + conds, valid := validateGatewayClass(gc, npCfg, crdVersions, validator) return &GatewayClass{ Source: gc, @@ -77,20 +83,39 @@ func buildGatewayClass( func validateGatewayClass( gc *v1.GatewayClass, + npCfg *ngfAPI.NginxProxy, crdVersions map[types.NamespacedName]*metav1.PartialObjectMetadata, + validator validation.GenericValidator, ) ([]conditions.Condition, bool) { var conds []conditions.Condition - valid := true - if gc.Spec.ParametersRef != nil { + var err error path := field.NewPath("spec").Child("parametersRef") - err := field.Forbidden(path, "parametersRef is not supported") - conds = append(conds, staticConds.NewGatewayClassInvalidParameters(err.Error())) - valid = false + if _, ok := supportedParamKinds[string(gc.Spec.ParametersRef.Kind)]; !ok { + err = field.NotSupported(path.Child("kind"), string(gc.Spec.ParametersRef.Kind), []string{"NginxProxy"}) + } else if npCfg == nil { + err = field.NotFound(path.Child("name"), gc.Spec.ParametersRef.Name) + conds = append(conds, staticConds.NewGatewayClassRefNotFound()) + } else { + nginxProxyErrs := validateNginxProxy(validator, npCfg) + if len(nginxProxyErrs) > 0 { + err = errors.New(nginxProxyErrs.ToAggregate().Error()) + } + } + + if err != nil { + conds = append(conds, staticConds.NewGatewayClassInvalidParameters(err.Error())) + } else { + conds = append(conds, staticConds.NewGatewayClassResolvedRefs()) + } } supportedVersionConds, versionsValid := gatewayclass.ValidateCRDVersions(crdVersions) - return append(conds, supportedVersionConds...), valid && versionsValid + return append(conds, supportedVersionConds...), versionsValid +} + +var supportedParamKinds = map[string]struct{}{ + "NginxProxy": {}, } diff --git a/internal/mode/static/state/graph/gatewayclass_test.go b/internal/mode/static/state/graph/gatewayclass_test.go index 11c4feb321..a60da7eecc 100644 --- a/internal/mode/static/state/graph/gatewayclass_test.go +++ b/internal/mode/static/state/graph/gatewayclass_test.go @@ -1,6 +1,7 @@ package graph import ( + "errors" "testing" . "github.com/onsi/gomega" @@ -9,10 +10,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation/validationfakes" ) func TestProcessGatewayClasses(t *testing.T) { @@ -122,9 +126,22 @@ func TestProcessGatewayClasses(t *testing.T) { func TestBuildGatewayClass(t *testing.T) { validGC := &v1.GatewayClass{} - invalidGC := &v1.GatewayClass{ + gcWithParams := &v1.GatewayClass{ Spec: v1.GatewayClassSpec{ - ParametersRef: &v1.ParametersReference{}, + ParametersRef: &v1.ParametersReference{ + Kind: v1.Kind("NginxProxy"), + Namespace: helpers.GetPointer(v1.Namespace("test")), + Name: "nginx-proxy", + }, + }, + } + + gcWithInvalidKind := &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Kind: v1.Kind("Invalid"), + Namespace: helpers.GetPointer(v1.Namespace("test")), + }, }, } @@ -148,8 +165,26 @@ func TestBuildGatewayClass(t *testing.T) { }, } + createValidNPValidator := func() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateServiceNameReturns(nil) + v.ValidateEndpointReturns(nil) + + return v + } + + createInvalidNPValidator := func() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateServiceNameReturns(errors.New("error")) + v.ValidateEndpointReturns(errors.New("error")) + + return v + } + tests := []struct { gc *v1.GatewayClass + np *ngfAPI.NginxProxy + validator validation.GenericValidator crdMetadata map[types.NamespacedName]*metav1.PartialObjectMetadata expected *GatewayClass name string @@ -169,18 +204,84 @@ func TestBuildGatewayClass(t *testing.T) { name: "no gatewayclass", }, { - gc: invalidGC, - crdMetadata: validCRDs, + gc: gcWithParams, + np: &ngfAPI.NginxProxy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + }, + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + ServiceName: helpers.GetPointer("my-svc"), + }, + }, + }, + validator: createValidNPValidator(), + expected: &GatewayClass{ + Source: gcWithParams, + Valid: true, + Conditions: []conditions.Condition{staticConds.NewGatewayClassResolvedRefs()}, + }, + name: "valid gatewayclass with paramsRef", + }, + { + gc: gcWithInvalidKind, + np: &ngfAPI.NginxProxy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + }, + }, expected: &GatewayClass{ - Source: invalidGC, - Valid: false, + Source: gcWithInvalidKind, + Valid: true, + Conditions: []conditions.Condition{ + staticConds.NewGatewayClassInvalidParameters( + "spec.parametersRef.kind: Unsupported value: \"Invalid\": supported values: \"NginxProxy\"", + ), + }, + }, + name: "invalid gatewayclass with unsupported paramsRef Kind", + }, + { + gc: gcWithParams, + expected: &GatewayClass{ + Source: gcWithParams, + Valid: true, + Conditions: []conditions.Condition{ + staticConds.NewGatewayClassRefNotFound(), + staticConds.NewGatewayClassInvalidParameters( + "spec.parametersRef.name: Not found: \"nginx-proxy\"", + ), + }, + }, + name: "invalid gatewayclass with paramsRef resource that doesn't exist", + }, + { + gc: gcWithParams, + np: &ngfAPI.NginxProxy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + }, + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + ServiceName: helpers.GetPointer("my-svc"), + Exporter: &ngfAPI.TelemetryExporter{ + Endpoint: "my-endpoint", + }, + }, + }, + }, + validator: createInvalidNPValidator(), + expected: &GatewayClass{ + Source: gcWithParams, + Valid: true, Conditions: []conditions.Condition{ staticConds.NewGatewayClassInvalidParameters( - "spec.parametersRef: Forbidden: parametersRef is not supported", + "[spec.telemetry.serviceName: Invalid value: \"my-svc\": error" + + ", spec.telemetry.exporter.endpoint: Invalid value: \"my-endpoint\": error]", ), }, }, - name: "invalid gatewayclass; parameters ref", + name: "invalid gatewayclass with invalid paramsRef resource", }, { gc: validGC, @@ -198,7 +299,7 @@ func TestBuildGatewayClass(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - result := buildGatewayClass(test.gc, test.crdMetadata) + result := buildGatewayClass(test.gc, test.np, test.crdMetadata, test.validator) 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 f6ee5598b9..af60f41a20 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -10,6 +10,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" ) @@ -26,6 +27,7 @@ type ClusterState struct { CRDMetadata map[types.NamespacedName]*metav1.PartialObjectMetadata BackendTLSPolicies map[types.NamespacedName]*v1alpha2.BackendTLSPolicy ConfigMaps map[types.NamespacedName]*v1.ConfigMap + NginxProxies map[types.NamespacedName]*ngfAPI.NginxProxy } // Graph is a Graph-like representation of Gateway API resources. @@ -58,6 +60,8 @@ type Graph struct { ReferencedCaCertConfigMaps map[types.NamespacedName]*CaCertConfigMap // BackendTLSPolicies holds BackendTLSPolicy resources. BackendTLSPolicies map[types.NamespacedName]*BackendTLSPolicy + // NginxProxy holds the NginxProxy config for the GatewayClass. + NginxProxy *ngfAPI.NginxProxy } // ProtectedPorts are the ports that may not be configured by a listener with a descriptive name of each port. @@ -99,6 +103,9 @@ func (g *Graph) IsReferenced(resourceType client.Object, nsname types.Namespaced // Service Namespace should be the same Namespace as the EndpointSlice _, exists := g.ReferencedServices[types.NamespacedName{Namespace: nsname.Namespace, Name: svcName}] return exists + // NginxProxy reference exists if it is linked to a GatewayClass. + case *ngfAPI.NginxProxy: + return isNginxProxyReferenced(nsname, g.GatewayClass) default: return false } @@ -118,7 +125,8 @@ func BuildGraph( return &Graph{} } - gc := buildGatewayClass(processedGwClasses.Winner, state.CRDMetadata) + npCfg := getNginxProxy(state.NginxProxies, processedGwClasses.Winner) + gc := buildGatewayClass(processedGwClasses.Winner, npCfg, state.CRDMetadata, validators.GenericValidator) secretResolver := newSecretResolver(state.Secrets) configMapResolver := newConfigMapResolver(state.ConfigMaps) @@ -154,6 +162,7 @@ func BuildGraph( ReferencedServices: referencedServices, ReferencedCaCertConfigMaps: configMapResolver.getResolvedConfigMaps(), BackendTLSPolicies: processedBackendTLSPolicies, + NginxProxy: npCfg, } return g diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 8a278d673a..12bfb4f3ca 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" @@ -288,6 +289,23 @@ func TestBuildGraph(t *testing.T) { }, } + proxy := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-proxy", + }, + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + Exporter: &ngfAPI.TelemetryExporter{ + Endpoint: "1.2.3.4:123", + Interval: helpers.GetPointer(ngfAPI.Duration("5s")), + BatchSize: helpers.GetPointer(int32(512)), + BatchCount: helpers.GetPointer(int32(4)), + }, + ServiceName: helpers.GetPointer("my-svc"), + }, + }, + } + createStateWithGatewayClass := func(gc *gatewayv1.GatewayClass) ClusterState { return ClusterState{ GatewayClasses: map[types.NamespacedName]*gatewayv1.GatewayClass{ @@ -321,6 +339,9 @@ func TestBuildGraph(t *testing.T) { ConfigMaps: map[types.NamespacedName]*v1.ConfigMap{ client.ObjectKeyFromObject(cm): cm, }, + NginxProxies: map[types.NamespacedName]*ngfAPI.NginxProxy{ + client.ObjectKeyFromObject(proxy): proxy, + }, } } @@ -361,8 +382,9 @@ func TestBuildGraph(t *testing.T) { createExpectedGraphWithGatewayClass := func(gc *gatewayv1.GatewayClass) *Graph { return &Graph{ GatewayClass: &GatewayClass{ - Source: gc, - Valid: true, + Source: gc, + Valid: true, + Conditions: []conditions.Condition{staticConds.NewGatewayClassResolvedRefs()}, }, Gateway: &Gateway{ Source: gw1, @@ -419,6 +441,7 @@ func TestBuildGraph(t *testing.T) { BackendTLSPolicies: map[types.NamespacedName]*BackendTLSPolicy{ client.ObjectKeyFromObject(btp.Source): &btp, }, + NginxProxy: proxy, } } @@ -428,6 +451,11 @@ func TestBuildGraph(t *testing.T) { }, Spec: gatewayv1.GatewayClassSpec{ ControllerName: controllerName, + ParametersRef: &gatewayv1.ParametersReference{ + Group: gatewayv1.Group("gateway.nginx.org"), + Kind: gatewayv1.Kind("NginxProxy"), + Name: "nginx-proxy", + }, }, } differentControllerGC := &gatewayv1.GatewayClass{ @@ -464,7 +492,10 @@ func TestBuildGraph(t *testing.T) { test.store, controllerName, gcName, - validation.Validators{HTTPFieldsValidator: &validationfakes.FakeHTTPFieldsValidator{}}, + validation.Validators{ + HTTPFieldsValidator: &validationfakes.FakeHTTPFieldsValidator{}, + GenericValidator: &validationfakes.FakeGenericValidator{}, + }, protectedPorts, ) @@ -582,6 +613,30 @@ func TestIsReferenced(t *testing.T) { }, } + gcWithNginxProxy := &GatewayClass{ + Source: &gatewayv1.GatewayClass{ + Spec: gatewayv1.GatewayClassSpec{ + ParametersRef: &gatewayv1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: gatewayv1.Kind("NginxProxy"), + Name: "nginx-proxy-in-gc", + }, + }, + }, + } + + npNotInGatewayClass := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-proxy", + }, + } + + npInGatewayClass := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-proxy-in-gc", + }, + } + graph := &Graph{ Gateway: gw, ReferencedSecrets: map[types.NamespacedName]*Secret{ @@ -605,6 +660,7 @@ func TestIsReferenced(t *testing.T) { tests := []struct { graph *Graph + gc *GatewayClass resource client.Object name string expected bool @@ -696,7 +752,7 @@ func TestIsReferenced(t *testing.T) { expected: false, }, - // ConfigMap cases + // ConfigMap tests { name: "ConfigMap in graph's ReferencedConfigMaps is referenced", resource: baseConfigMap, @@ -716,6 +772,22 @@ func TestIsReferenced(t *testing.T) { expected: false, }, + // NginxProxy tests + { + name: "NginxProxy is referenced in GatewayClass", + resource: npInGatewayClass, + gc: gcWithNginxProxy, + graph: graph, + expected: true, + }, + { + name: "NginxProxy is not referenced in GatewayClass", + resource: npNotInGatewayClass, + gc: gcWithNginxProxy, + graph: graph, + expected: false, + }, + // Edge cases { name: "Resource is not supported by IsReferenced", @@ -727,6 +799,8 @@ func TestIsReferenced(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) + + test.graph.GatewayClass = test.gc result := test.graph.IsReferenced(test.resource, client.ObjectKeyFromObject(test.resource)) g.Expect(result).To(Equal(test.expected)) }) diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go new file mode 100644 index 0000000000..327d6cca89 --- /dev/null +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -0,0 +1,88 @@ +package graph + +import ( + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + v1 "sigs.k8s.io/gateway-api/apis/v1" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" +) + +// getNginxProxy returns the NginxProxy associated with the GatewayClass (if it exists). +func getNginxProxy( + nps map[types.NamespacedName]*ngfAPI.NginxProxy, + gc *v1.GatewayClass, +) *ngfAPI.NginxProxy { + if gcReferencesAnyNginxProxy(gc) { + return nps[types.NamespacedName{Name: gc.Spec.ParametersRef.Name}] + } + + return nil +} + +// isNginxProxyReferenced returns whether or not a specific NginxProxy is referenced in the GatewayClass. +func isNginxProxyReferenced(npNSName types.NamespacedName, gc *GatewayClass) bool { + return gc != nil && gcReferencesAnyNginxProxy(gc.Source) && gc.Source.Spec.ParametersRef.Name == npNSName.Name +} + +// gcReferencesNginxProxy returns whether a GatewayClass references any NginxProxy resource. +func gcReferencesAnyNginxProxy(gc *v1.GatewayClass) bool { + if gc != nil { + ref := gc.Spec.ParametersRef + return ref != nil && ref.Group == ngfAPI.GroupName && ref.Kind == v1.Kind("NginxProxy") + } + + return false +} + +// validateNginxProxy performs re-validation on string values in the case of CRD validation failure. +func validateNginxProxy( + validator validation.GenericValidator, + npCfg *ngfAPI.NginxProxy, +) field.ErrorList { + var allErrs field.ErrorList + spec := field.NewPath("spec") + + telemetry := npCfg.Spec.Telemetry + if telemetry != nil { + telPath := spec.Child("telemetry") + if telemetry.ServiceName != nil { + if err := validator.ValidateServiceName(*telemetry.ServiceName); err != nil { + allErrs = append(allErrs, field.Invalid(telPath.Child("serviceName"), *telemetry.ServiceName, err.Error())) + } + } + + if telemetry.Exporter != nil { + exp := telemetry.Exporter + expPath := telPath.Child("exporter") + + if exp.Endpoint != "" { + if err := validator.ValidateEndpoint(exp.Endpoint); err != nil { + allErrs = append(allErrs, field.Invalid(expPath.Child("endpoint"), exp.Endpoint, err.Error())) + } + } + + if exp.Interval != nil { + if err := validator.ValidateNginxDuration(string(*exp.Interval)); err != nil { + allErrs = append(allErrs, field.Invalid(expPath.Child("interval"), *exp.Interval, err.Error())) + } + } + } + + if telemetry.SpanAttributes != nil { + spanAttrPath := telPath.Child("spanAttributes") + for _, spanAttr := range telemetry.SpanAttributes { + if err := validator.ValidateEscapedStringNoVarExpansion(spanAttr.Key); err != nil { + allErrs = append(allErrs, field.Invalid(spanAttrPath.Child("key"), spanAttr.Key, err.Error())) + } + + if err := validator.ValidateEscapedStringNoVarExpansion(spanAttr.Value); err != nil { + allErrs = append(allErrs, field.Invalid(spanAttrPath.Child("value"), spanAttr.Value, err.Error())) + } + } + } + } + + return allErrs +} diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go new file mode 100644 index 0000000000..80e66da56a --- /dev/null +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -0,0 +1,336 @@ +package graph + +import ( + "errors" + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + v1 "sigs.k8s.io/gateway-api/apis/v1" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation/validationfakes" +) + +func TestGetNginxProxy(t *testing.T) { + tests := []struct { + nps map[types.NamespacedName]*ngfAPI.NginxProxy + gc *v1.GatewayClass + expNP *ngfAPI.NginxProxy + name string + }{ + { + nps: map[types.NamespacedName]*ngfAPI.NginxProxy{ + {Name: "np1"}: {}, + }, + gc: nil, + expNP: nil, + name: "nil gatewayclass", + }, + { + nps: map[types.NamespacedName]*ngfAPI.NginxProxy{}, + gc: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1.Kind("NginxProxy"), + Name: "np1", + }, + }, + }, + expNP: nil, + name: "no nginxproxy resources", + }, + { + nps: map[types.NamespacedName]*ngfAPI.NginxProxy{ + {Name: "np1"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "np1", + }, + }, + {Name: "np2"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "np2", + }, + }, + }, + gc: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1.Kind("NginxProxy"), + Name: "np2", + }, + }, + }, + expNP: &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "np2", + }, + }, + name: "returns correct resource", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(getNginxProxy(test.nps, test.gc)).To(Equal(test.expNP)) + }) + } +} + +func TestIsNginxProxyReferenced(t *testing.T) { + tests := []struct { + gc *GatewayClass + npName types.NamespacedName + name string + expRes bool + }{ + { + gc: &GatewayClass{ + Source: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1.Kind("NginxProxy"), + Name: "nginx-proxy", + }, + }, + }, + }, + npName: types.NamespacedName{}, + expRes: false, + name: "nil nginxproxy", + }, + { + gc: nil, + npName: types.NamespacedName{Name: "nginx-proxy"}, + expRes: false, + name: "nil gatewayclass", + }, + { + gc: &GatewayClass{ + Source: nil, + }, + npName: types.NamespacedName{Name: "nginx-proxy"}, + expRes: false, + name: "nil gatewayclass source", + }, + { + gc: &GatewayClass{ + Source: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1.Kind("NginxProxy"), + Name: "nginx-proxy", + }, + }, + }, + }, + npName: types.NamespacedName{Name: "nginx-proxy"}, + expRes: true, + name: "references the NginxProxy", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(isNginxProxyReferenced(test.npName, test.gc)).To(Equal(test.expRes)) + }) + } +} + +func TestGCReferencesAnyNginxProxy(t *testing.T) { + tests := []struct { + gc *v1.GatewayClass + name string + expRes bool + }{ + { + gc: nil, + expRes: false, + name: "nil gatewayclass", + }, + { + gc: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{}, + }, + expRes: false, + name: "nil paramsRef", + }, + { + gc: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Group: v1.Group("wrong-group"), + Kind: v1.Kind("NginxProxy"), + Name: "wrong-group", + }, + }, + }, + expRes: false, + name: "wrong group name", + }, + { + gc: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1.Kind("WrongKind"), + Name: "wrong-kind", + }, + }, + }, + expRes: false, + name: "wrong kind", + }, + { + gc: &v1.GatewayClass{ + Spec: v1.GatewayClassSpec{ + ParametersRef: &v1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1.Kind("NginxProxy"), + Name: "nginx-proxy", + }, + }, + }, + expRes: true, + name: "references an NginxProxy", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(gcReferencesAnyNginxProxy(test.gc)).To(Equal(test.expRes)) + }) + } +} + +func TestValidateNginxProxy(t *testing.T) { + createValidValidator := func() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateEscapedStringNoVarExpansionReturns(nil) + v.ValidateEndpointReturns(nil) + v.ValidateServiceNameReturns(nil) + v.ValidateNginxDurationReturns(nil) + + return v + } + + createInvalidValidator := func() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateEscapedStringNoVarExpansionReturns(errors.New("error")) + v.ValidateEndpointReturns(errors.New("error")) + v.ValidateServiceNameReturns(errors.New("error")) + v.ValidateNginxDurationReturns(errors.New("error")) + + return v + } + + tests := []struct { + np *ngfAPI.NginxProxy + validator *validationfakes.FakeGenericValidator + name string + expErrSubstring string + expectErrCount int + }{ + { + name: "valid nginxproxy", + validator: createValidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + ServiceName: helpers.GetPointer("my-svc"), + Exporter: &ngfAPI.TelemetryExporter{ + Interval: helpers.GetPointer[ngfAPI.Duration]("5ms"), + Endpoint: "my-endpoint", + }, + SpanAttributes: []ngfAPI.SpanAttribute{ + {Key: "key", Value: "value"}, + }, + }, + }, + }, + expectErrCount: 0, + }, + { + name: "invalid serviceName", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + ServiceName: helpers.GetPointer("my-svc"), // any value is invalid by the validator + }, + }, + }, + expErrSubstring: "telemetry.serviceName", + expectErrCount: 1, + }, + { + name: "invalid endpoint", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + Exporter: &ngfAPI.TelemetryExporter{ + Endpoint: "my-endpoint", // any value is invalid by the validator + }, + }, + }, + }, + expErrSubstring: "telemetry.exporter.endpoint", + expectErrCount: 1, + }, + { + name: "invalid interval", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + Exporter: &ngfAPI.TelemetryExporter{ + Interval: helpers.GetPointer[ngfAPI.Duration]("my-interval"), // any value is invalid by the validator + }, + }, + }, + }, + expErrSubstring: "telemetry.exporter.interval", + expectErrCount: 1, + }, + { + name: "invalid spanAttributes", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{ + SpanAttributes: []ngfAPI.SpanAttribute{ + {Key: "my-key", Value: "my-value"}, // any value is invalid by the validator + }, + }, + }, + }, + expErrSubstring: "telemetry.spanAttributes", + expectErrCount: 2, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + allErrs := validateNginxProxy(test.validator, test.np) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + if len(allErrs) > 0 { + g.Expect(allErrs.ToAggregate().Error()).To(ContainSubstring(test.expErrSubstring)) + } + }) + } +} diff --git a/internal/mode/static/state/validation/validationfakes/fake_generic_validator.go b/internal/mode/static/state/validation/validationfakes/fake_generic_validator.go new file mode 100644 index 0000000000..294b7dc526 --- /dev/null +++ b/internal/mode/static/state/validation/validationfakes/fake_generic_validator.go @@ -0,0 +1,333 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package validationfakes + +import ( + "sync" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" +) + +type FakeGenericValidator struct { + ValidateEndpointStub func(string) error + validateEndpointMutex sync.RWMutex + validateEndpointArgsForCall []struct { + arg1 string + } + validateEndpointReturns struct { + result1 error + } + validateEndpointReturnsOnCall map[int]struct { + result1 error + } + ValidateEscapedStringNoVarExpansionStub func(string) error + validateEscapedStringNoVarExpansionMutex sync.RWMutex + validateEscapedStringNoVarExpansionArgsForCall []struct { + arg1 string + } + validateEscapedStringNoVarExpansionReturns struct { + result1 error + } + validateEscapedStringNoVarExpansionReturnsOnCall map[int]struct { + result1 error + } + ValidateNginxDurationStub func(string) error + validateNginxDurationMutex sync.RWMutex + validateNginxDurationArgsForCall []struct { + arg1 string + } + validateNginxDurationReturns struct { + result1 error + } + validateNginxDurationReturnsOnCall map[int]struct { + result1 error + } + ValidateServiceNameStub func(string) error + validateServiceNameMutex sync.RWMutex + validateServiceNameArgsForCall []struct { + arg1 string + } + validateServiceNameReturns struct { + result1 error + } + validateServiceNameReturnsOnCall map[int]struct { + result1 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeGenericValidator) ValidateEndpoint(arg1 string) error { + fake.validateEndpointMutex.Lock() + ret, specificReturn := fake.validateEndpointReturnsOnCall[len(fake.validateEndpointArgsForCall)] + fake.validateEndpointArgsForCall = append(fake.validateEndpointArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateEndpointStub + fakeReturns := fake.validateEndpointReturns + fake.recordInvocation("ValidateEndpoint", []interface{}{arg1}) + fake.validateEndpointMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeGenericValidator) ValidateEndpointCallCount() int { + fake.validateEndpointMutex.RLock() + defer fake.validateEndpointMutex.RUnlock() + return len(fake.validateEndpointArgsForCall) +} + +func (fake *FakeGenericValidator) ValidateEndpointCalls(stub func(string) error) { + fake.validateEndpointMutex.Lock() + defer fake.validateEndpointMutex.Unlock() + fake.ValidateEndpointStub = stub +} + +func (fake *FakeGenericValidator) ValidateEndpointArgsForCall(i int) string { + fake.validateEndpointMutex.RLock() + defer fake.validateEndpointMutex.RUnlock() + argsForCall := fake.validateEndpointArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenericValidator) ValidateEndpointReturns(result1 error) { + fake.validateEndpointMutex.Lock() + defer fake.validateEndpointMutex.Unlock() + fake.ValidateEndpointStub = nil + fake.validateEndpointReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) ValidateEndpointReturnsOnCall(i int, result1 error) { + fake.validateEndpointMutex.Lock() + defer fake.validateEndpointMutex.Unlock() + fake.ValidateEndpointStub = nil + if fake.validateEndpointReturnsOnCall == nil { + fake.validateEndpointReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateEndpointReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) ValidateEscapedStringNoVarExpansion(arg1 string) error { + fake.validateEscapedStringNoVarExpansionMutex.Lock() + ret, specificReturn := fake.validateEscapedStringNoVarExpansionReturnsOnCall[len(fake.validateEscapedStringNoVarExpansionArgsForCall)] + fake.validateEscapedStringNoVarExpansionArgsForCall = append(fake.validateEscapedStringNoVarExpansionArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateEscapedStringNoVarExpansionStub + fakeReturns := fake.validateEscapedStringNoVarExpansionReturns + fake.recordInvocation("ValidateEscapedStringNoVarExpansion", []interface{}{arg1}) + fake.validateEscapedStringNoVarExpansionMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeGenericValidator) ValidateEscapedStringNoVarExpansionCallCount() int { + fake.validateEscapedStringNoVarExpansionMutex.RLock() + defer fake.validateEscapedStringNoVarExpansionMutex.RUnlock() + return len(fake.validateEscapedStringNoVarExpansionArgsForCall) +} + +func (fake *FakeGenericValidator) ValidateEscapedStringNoVarExpansionCalls(stub func(string) error) { + fake.validateEscapedStringNoVarExpansionMutex.Lock() + defer fake.validateEscapedStringNoVarExpansionMutex.Unlock() + fake.ValidateEscapedStringNoVarExpansionStub = stub +} + +func (fake *FakeGenericValidator) ValidateEscapedStringNoVarExpansionArgsForCall(i int) string { + fake.validateEscapedStringNoVarExpansionMutex.RLock() + defer fake.validateEscapedStringNoVarExpansionMutex.RUnlock() + argsForCall := fake.validateEscapedStringNoVarExpansionArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenericValidator) ValidateEscapedStringNoVarExpansionReturns(result1 error) { + fake.validateEscapedStringNoVarExpansionMutex.Lock() + defer fake.validateEscapedStringNoVarExpansionMutex.Unlock() + fake.ValidateEscapedStringNoVarExpansionStub = nil + fake.validateEscapedStringNoVarExpansionReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) ValidateEscapedStringNoVarExpansionReturnsOnCall(i int, result1 error) { + fake.validateEscapedStringNoVarExpansionMutex.Lock() + defer fake.validateEscapedStringNoVarExpansionMutex.Unlock() + fake.ValidateEscapedStringNoVarExpansionStub = nil + if fake.validateEscapedStringNoVarExpansionReturnsOnCall == nil { + fake.validateEscapedStringNoVarExpansionReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateEscapedStringNoVarExpansionReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) ValidateNginxDuration(arg1 string) error { + fake.validateNginxDurationMutex.Lock() + ret, specificReturn := fake.validateNginxDurationReturnsOnCall[len(fake.validateNginxDurationArgsForCall)] + fake.validateNginxDurationArgsForCall = append(fake.validateNginxDurationArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateNginxDurationStub + fakeReturns := fake.validateNginxDurationReturns + fake.recordInvocation("ValidateNginxDuration", []interface{}{arg1}) + fake.validateNginxDurationMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeGenericValidator) ValidateNginxDurationCallCount() int { + fake.validateNginxDurationMutex.RLock() + defer fake.validateNginxDurationMutex.RUnlock() + return len(fake.validateNginxDurationArgsForCall) +} + +func (fake *FakeGenericValidator) ValidateNginxDurationCalls(stub func(string) error) { + fake.validateNginxDurationMutex.Lock() + defer fake.validateNginxDurationMutex.Unlock() + fake.ValidateNginxDurationStub = stub +} + +func (fake *FakeGenericValidator) ValidateNginxDurationArgsForCall(i int) string { + fake.validateNginxDurationMutex.RLock() + defer fake.validateNginxDurationMutex.RUnlock() + argsForCall := fake.validateNginxDurationArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenericValidator) ValidateNginxDurationReturns(result1 error) { + fake.validateNginxDurationMutex.Lock() + defer fake.validateNginxDurationMutex.Unlock() + fake.ValidateNginxDurationStub = nil + fake.validateNginxDurationReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) ValidateNginxDurationReturnsOnCall(i int, result1 error) { + fake.validateNginxDurationMutex.Lock() + defer fake.validateNginxDurationMutex.Unlock() + fake.ValidateNginxDurationStub = nil + if fake.validateNginxDurationReturnsOnCall == nil { + fake.validateNginxDurationReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateNginxDurationReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) ValidateServiceName(arg1 string) error { + fake.validateServiceNameMutex.Lock() + ret, specificReturn := fake.validateServiceNameReturnsOnCall[len(fake.validateServiceNameArgsForCall)] + fake.validateServiceNameArgsForCall = append(fake.validateServiceNameArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateServiceNameStub + fakeReturns := fake.validateServiceNameReturns + fake.recordInvocation("ValidateServiceName", []interface{}{arg1}) + fake.validateServiceNameMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeGenericValidator) ValidateServiceNameCallCount() int { + fake.validateServiceNameMutex.RLock() + defer fake.validateServiceNameMutex.RUnlock() + return len(fake.validateServiceNameArgsForCall) +} + +func (fake *FakeGenericValidator) ValidateServiceNameCalls(stub func(string) error) { + fake.validateServiceNameMutex.Lock() + defer fake.validateServiceNameMutex.Unlock() + fake.ValidateServiceNameStub = stub +} + +func (fake *FakeGenericValidator) ValidateServiceNameArgsForCall(i int) string { + fake.validateServiceNameMutex.RLock() + defer fake.validateServiceNameMutex.RUnlock() + argsForCall := fake.validateServiceNameArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenericValidator) ValidateServiceNameReturns(result1 error) { + fake.validateServiceNameMutex.Lock() + defer fake.validateServiceNameMutex.Unlock() + fake.ValidateServiceNameStub = nil + fake.validateServiceNameReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) ValidateServiceNameReturnsOnCall(i int, result1 error) { + fake.validateServiceNameMutex.Lock() + defer fake.validateServiceNameMutex.Unlock() + fake.ValidateServiceNameStub = nil + if fake.validateServiceNameReturnsOnCall == nil { + fake.validateServiceNameReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateServiceNameReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeGenericValidator) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.validateEndpointMutex.RLock() + defer fake.validateEndpointMutex.RUnlock() + fake.validateEscapedStringNoVarExpansionMutex.RLock() + defer fake.validateEscapedStringNoVarExpansionMutex.RUnlock() + fake.validateNginxDurationMutex.RLock() + defer fake.validateNginxDurationMutex.RUnlock() + fake.validateServiceNameMutex.RLock() + defer fake.validateServiceNameMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeGenericValidator) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ validation.GenericValidator = new(FakeGenericValidator) diff --git a/internal/mode/static/state/validation/validator.go b/internal/mode/static/state/validation/validator.go index d6433ad363..52e20bb47f 100644 --- a/internal/mode/static/state/validation/validator.go +++ b/internal/mode/static/state/validation/validator.go @@ -1,11 +1,12 @@ package validation -// Validators include validators for Gateway API resources from the perspective of a data-plane. +// Validators include validators for API resources from the perspective of a data-plane. // It is used for fields that propagate into the data plane configuration. For example, the path in a routing rule. // However, not all such fields are validated: NGF will not validate a field using Validators if it is confident that // the field is valid. type Validators struct { HTTPFieldsValidator HTTPFieldsValidator + GenericValidator GenericValidator } // HTTPFieldsValidator validates the HTTP-related fields of Gateway API resources from the perspective of @@ -27,3 +28,14 @@ type HTTPFieldsValidator interface { ValidateRequestHeaderName(name string) error ValidateRequestHeaderValue(value string) error } + +// GenericValidator validates any generic values from NGF API resources from the perspective of a data-plane. +// These could be values that we want to re-validate in case of any CRD schema manipulation. +// +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . GenericValidator +type GenericValidator interface { + ValidateEscapedStringNoVarExpansion(value string) error + ValidateServiceName(name string) error + ValidateNginxDuration(duration string) error + ValidateEndpoint(endpoint string) error +} diff --git a/site/content/installation/installing-ngf/helm.md b/site/content/installation/installing-ngf/helm.md index 8c061ce3c1..3daf082fb1 100644 --- a/site/content/installation/installing-ngf/helm.md +++ b/site/content/installation/installing-ngf/helm.md @@ -256,7 +256,7 @@ Follow these steps to uninstall NGINX Gateway Fabric and Gateway API from your K ```shell kubectl delete ns nginx-gateway - kubectl delete crd nginxgateways.gateway.nginx.org + kubectl delete crd nginxgateways.gateway.nginx.org nginxproxies.gateway.nginx.org ``` 3. **Remove the Gateway API resources:** diff --git a/site/content/overview/gateway-api-compatibility.md b/site/content/overview/gateway-api-compatibility.md index 2673543e91..2b411b7b2f 100644 --- a/site/content/overview/gateway-api-compatibility.md +++ b/site/content/overview/gateway-api-compatibility.md @@ -58,7 +58,7 @@ NGINX Gateway Fabric supports a single GatewayClass resource configured with the - `spec` - `controllerName` - supported. - - `parametersRef` - not supported. + - `parametersRef` - NginxProxy resource supported. - `description` - supported. - `status` - `conditions` - supported (Condition/Status/Reason):