From db53730a2f27609e171caf99d6349b4871ddb68c Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Wed, 9 Oct 2024 09:43:56 -0600 Subject: [PATCH 1/2] Fix main includes Problem: The main includes were defined twice, causing some issues in reloading nginx. Solution: Deduplicate the code. --- .../mode/static/nginx/config/generator.go | 1 - .../mode/static/nginx/config/main_config.go | 8 +-- .../nginx/config/main_config_template.go | 4 +- .../static/nginx/config/main_config_test.go | 18 +++++ .../mode/static/nginx/config/main_includes.go | 19 ----- .../nginx/config/main_includes_template.go | 7 -- .../static/nginx/config/main_includes_test.go | 72 ------------------- 7 files changed, 25 insertions(+), 104 deletions(-) delete mode 100644 internal/mode/static/nginx/config/main_includes.go delete mode 100644 internal/mode/static/nginx/config/main_includes_template.go delete mode 100644 internal/mode/static/nginx/config/main_includes_test.go diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 78ff921ed3..31c9d60ee2 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -146,7 +146,6 @@ func (g GeneratorImpl) getExecuteFuncs(generator policies.Generator) []executeFu g.executeStreamUpstreams, executeStreamMaps, executeVersion, - executeMainIncludesConfig, } } diff --git a/internal/mode/static/nginx/config/main_config.go b/internal/mode/static/nginx/config/main_config.go index a857266179..e9a089fbc8 100644 --- a/internal/mode/static/nginx/config/main_config.go +++ b/internal/mode/static/nginx/config/main_config.go @@ -11,16 +11,16 @@ import ( var mainConfigTemplate = gotemplate.Must(gotemplate.New("main").Parse(mainConfigTemplateText)) type mainConfig struct { - Includes []shared.Include - TelemetryEnabled bool + Includes []shared.Include + Conf dataplane.Configuration } func executeMainConfig(conf dataplane.Configuration) []executeResult { includes := createIncludesFromSnippets(conf.MainSnippets) mc := mainConfig{ - TelemetryEnabled: conf.Telemetry.Endpoint != "", - Includes: includes, + Conf: conf, + Includes: includes, } results := make([]executeResult, 0, len(includes)+1) diff --git a/internal/mode/static/nginx/config/main_config_template.go b/internal/mode/static/nginx/config/main_config_template.go index dbd3dca2d6..f25e0b8d5e 100644 --- a/internal/mode/static/nginx/config/main_config_template.go +++ b/internal/mode/static/nginx/config/main_config_template.go @@ -1,10 +1,12 @@ package config const mainConfigTemplateText = ` -{{ if .TelemetryEnabled -}} +{{ if .Conf.Telemetry.Endpoint -}} load_module modules/ngx_otel_module.so; {{ end -}} +error_log stderr {{ .Conf.Logging.ErrorLevel }}; + {{ range $i := .Includes -}} include {{ $i.Name }}; {{ end -}} diff --git a/internal/mode/static/nginx/config/main_config_test.go b/internal/mode/static/nginx/config/main_config_test.go index 93c210b265..3f9a50caca 100644 --- a/internal/mode/static/nginx/config/main_config_test.go +++ b/internal/mode/static/nginx/config/main_config_test.go @@ -56,6 +56,24 @@ func TestExecuteMainConfig_Telemetry(t *testing.T) { } } +func TestExecuteMainConfig_Logging(t *testing.T) { + t.Parallel() + + conf := dataplane.Configuration{ + Logging: dataplane.Logging{ + ErrorLevel: "info", + }, + } + + g := NewWithT(t) + + res := executeMainConfig(conf) + g.Expect(res).To(HaveLen(1)) + g.Expect(res[0].dest).To(Equal(mainIncludesConfigFile)) + + g.Expect(string(res[0].data)).To(ContainSubstring("error_log stderr info")) +} + func TestExecuteMainConfig_Snippets(t *testing.T) { t.Parallel() diff --git a/internal/mode/static/nginx/config/main_includes.go b/internal/mode/static/nginx/config/main_includes.go deleted file mode 100644 index 04e3675dfc..0000000000 --- a/internal/mode/static/nginx/config/main_includes.go +++ /dev/null @@ -1,19 +0,0 @@ -package config - -import ( - gotemplate "text/template" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" -) - -var mainIncludesTemplate = gotemplate.Must(gotemplate.New("mainIncludes").Parse(mainIncludesTemplateText)) - -func executeMainIncludesConfig(conf dataplane.Configuration) []executeResult { - result := executeResult{ - dest: mainIncludesConfigFile, - data: helpers.MustExecuteTemplate(mainIncludesTemplate, conf), - } - - return []executeResult{result} -} diff --git a/internal/mode/static/nginx/config/main_includes_template.go b/internal/mode/static/nginx/config/main_includes_template.go deleted file mode 100644 index c444b1efe2..0000000000 --- a/internal/mode/static/nginx/config/main_includes_template.go +++ /dev/null @@ -1,7 +0,0 @@ -package config - -const mainIncludesTemplateText = ` -{{- if .Telemetry.Endpoint }}load_module modules/ngx_otel_module.so;{{ end -}} - -error_log stderr {{ .Logging.ErrorLevel }}; -` diff --git a/internal/mode/static/nginx/config/main_includes_test.go b/internal/mode/static/nginx/config/main_includes_test.go deleted file mode 100644 index b5f26f91fe..0000000000 --- a/internal/mode/static/nginx/config/main_includes_test.go +++ /dev/null @@ -1,72 +0,0 @@ -package config - -import ( - "strings" - "testing" - - . "github.com/onsi/gomega" - - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" -) - -func TestExecuteMainIncludesConfig(t *testing.T) { - t.Parallel() - - completeConfiguration := 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, - }, - Logging: dataplane.Logging{ - ErrorLevel: "info", - }, - } - - missingTelemetryEndpoint := dataplane.Configuration{ - Logging: dataplane.Logging{ - ErrorLevel: "info", - }, - } - - // Configuration.Logging will always be set, so no need to test if it is missing - tests := []struct { - name string - conf dataplane.Configuration - expTelemetryEndpointCount int - }{ - { - name: "complete configuration", - conf: completeConfiguration, - expTelemetryEndpointCount: 1, - }, - { - name: "missing telemetry endpoint", - conf: missingTelemetryEndpoint, - expTelemetryEndpointCount: 0, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - t.Parallel() - g := NewWithT(t) - - res := executeMainIncludesConfig(test.conf) - - g.Expect(res).To(HaveLen(1)) - - g.Expect(strings.Count( - string(res[0].data), - "load_module modules/ngx_otel_module.so;"), - ).To(Equal(test.expTelemetryEndpointCount)) - - g.Expect(strings.Count( - string(res[0].data), - "error_log stderr "+test.conf.Logging.ErrorLevel+";", - )).To(Equal(1)) - }) - } -} From 56b95056d5fc806ddf538bb2911b5338a27294a2 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Wed, 9 Oct 2024 09:50:34 -0600 Subject: [PATCH 2/2] Add generated files --- .../nginx-gateway-fabric/values.schema.json | 14 ++++++ .../snippets-filters-nginx-plus/deploy.yaml | 43 +++++++++++++++++++ deploy/snippets-filters/deploy.yaml | 43 +++++++++++++++++++ 3 files changed, 100 insertions(+) diff --git a/charts/nginx-gateway-fabric/values.schema.json b/charts/nginx-gateway-fabric/values.schema.json index 5ff5178961..94211e9322 100644 --- a/charts/nginx-gateway-fabric/values.schema.json +++ b/charts/nginx-gateway-fabric/values.schema.json @@ -519,6 +519,20 @@ "required": [], "title": "securityContext", "type": "object" + }, + "snippetsFilters": { + "properties": { + "enable": { + "default": false, + "description": "Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX\nconfig for HTTPRoute and GRPCRoute resources.", + "required": [], + "title": "enable", + "type": "boolean" + } + }, + "required": [], + "title": "snippetsFilters", + "type": "object" } }, "required": [ diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml index dfb0bcc72e..7cc456b8ee 100644 --- a/deploy/snippets-filters-nginx-plus/deploy.yaml +++ b/deploy/snippets-filters-nginx-plus/deploy.yaml @@ -153,6 +153,19 @@ subjects: namespace: nginx-gateway --- apiVersion: v1 +data: + main.conf: | + error_log stderr info; +kind: ConfigMap +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-includes + namespace: nginx-gateway +--- +apiVersion: v1 kind: Service metadata: labels: @@ -299,6 +312,33 @@ spec: name: nginx-cache - mountPath: /etc/nginx/includes name: nginx-includes + initContainers: + - command: + - /usr/bin/gateway + - copy + - --source + - /includes/main.conf + - --destination + - /etc/nginx/main-includes/main.conf + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + imagePullPolicy: Always + name: copy-nginx-config + securityContext: + capabilities: + add: + - KILL + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 102 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /includes + name: nginx-includes-configmap + - mountPath: /etc/nginx/main-includes + name: nginx-main-includes securityContext: fsGroup: 1001 runAsNonRoot: true @@ -320,6 +360,9 @@ spec: name: nginx-cache - emptyDir: {} name: nginx-includes + - configMap: + name: nginx-includes + name: nginx-includes-configmap --- apiVersion: gateway.networking.k8s.io/v1 kind: GatewayClass diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml index d48e4b3f3c..27d566f983 100644 --- a/deploy/snippets-filters/deploy.yaml +++ b/deploy/snippets-filters/deploy.yaml @@ -145,6 +145,19 @@ subjects: namespace: nginx-gateway --- apiVersion: v1 +data: + main.conf: | + error_log stderr info; +kind: ConfigMap +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-includes + namespace: nginx-gateway +--- +apiVersion: v1 kind: Service metadata: labels: @@ -290,6 +303,33 @@ spec: name: nginx-cache - mountPath: /etc/nginx/includes name: nginx-includes + initContainers: + - command: + - /usr/bin/gateway + - copy + - --source + - /includes/main.conf + - --destination + - /etc/nginx/main-includes/main.conf + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + imagePullPolicy: Always + name: copy-nginx-config + securityContext: + capabilities: + add: + - KILL + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 102 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /includes + name: nginx-includes-configmap + - mountPath: /etc/nginx/main-includes + name: nginx-main-includes securityContext: fsGroup: 1001 runAsNonRoot: true @@ -311,6 +351,9 @@ spec: name: nginx-cache - emptyDir: {} name: nginx-includes + - configMap: + name: nginx-includes + name: nginx-includes-configmap --- apiVersion: gateway.networking.k8s.io/v1 kind: GatewayClass