Code review; add validation and helm template

charts/nginx-gateway-fabric/README.md

@@ -249,7 +249,7 @@ To uninstall/delete the release `ngf`:
 ```shell
 helm uninstall ngf -n nginx-gateway
 kubectl delete ns nginx-gateway
-for crd in `kubectl get crds -oname | grep gateway.nginx.org | awk -F / '{ print $2 }'`; do kubectl delete crd $crd; done
+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.
@@ -300,6 +300,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. | |

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.
 */}}

charts/nginx-gateway-fabric/templates/deployment.yaml

@@ -119,7 +119,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run
@@ -152,7 +152,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run

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 }}

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 }}

charts/nginx-gateway-fabric/templates/rbac.yaml

@@ -111,11 +111,17 @@ rules:
   - gateway.nginx.org
   resources:
   - nginxgateways
-  - nginxproxies
   verbs:
   - get
   - list
   - watch
+- apiGroups:
+  - gateway.nginx.org
+  resources:
+  - nginxproxies
+  verbs:
+  - list
+  - watch
 - apiGroups:
   - gateway.nginx.org
   resources:

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.

conformance/provisioner/static-deployment.yaml

@@ -71,7 +71,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run
@@ -97,7 +97,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run

deploy/manifests/nginx-gateway-experimental.yaml

@@ -93,11 +93,17 @@ rules:
   - gateway.nginx.org
   resources:
   - nginxgateways
-  - nginxproxies
   verbs:
   - get
   - list
   - watch
+- apiGroups:
+  - gateway.nginx.org
+  resources:
+  - nginxproxies
+  verbs:
+  - list
+  - watch
 - apiGroups:
   - gateway.nginx.org
   resources:
@@ -215,7 +221,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run
@@ -241,7 +247,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run

deploy/manifests/nginx-gateway.yaml

@@ -90,11 +90,17 @@ rules:
   - gateway.nginx.org
   resources:
   - nginxgateways
-  - nginxproxies
   verbs:
   - get
   - list
   - watch
+- apiGroups:
+  - gateway.nginx.org
+  resources:
+  - nginxproxies
+  verbs:
+  - list
+  - watch
 - apiGroups:
   - gateway.nginx.org
   resources:
@@ -211,7 +217,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run
@@ -237,7 +243,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run

deploy/manifests/nginx-plus-gateway-experimental.yaml

@@ -99,11 +99,17 @@ rules:
   - gateway.nginx.org
   resources:
   - nginxgateways
-  - nginxproxies
   verbs:
   - get
   - list
   - watch
+- apiGroups:
+  - gateway.nginx.org
+  resources:
+  - nginxproxies
+  verbs:
+  - list
+  - watch
 - apiGroups:
   - gateway.nginx.org
   resources:
@@ -222,7 +228,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run
@@ -248,7 +254,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run

deploy/manifests/nginx-plus-gateway.yaml

@@ -96,11 +96,17 @@ rules:
   - gateway.nginx.org
   resources:
   - nginxgateways
-  - nginxproxies
   verbs:
   - get
   - list
   - watch
+- apiGroups:
+  - gateway.nginx.org
+  resources:
+  - nginxproxies
+  verbs:
+  - list
+  - watch
 - apiGroups:
   - gateway.nginx.org
   resources:
@@ -218,7 +224,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run
@@ -244,7 +250,7 @@ spec:
         - name: nginx-conf
           mountPath: /etc/nginx/conf.d
         - name: nginx-includes
-          mountPath: /etc/nginx/includes
+          mountPath: /etc/nginx/modules-includes
         - name: nginx-secrets
           mountPath: /etc/nginx/secrets
         - name: nginx-run

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,

internal/mode/static/nginx/conf/nginx-plus.conf

@@ -1,5 +1,5 @@
 load_module /usr/lib/nginx/modules/ngx_http_js_module.so;
-include /etc/nginx/includes/*.conf;
+include /etc/nginx/modules-includes/*.conf;
 
 worker_processes auto;
 

internal/mode/static/nginx/conf/nginx.conf

@@ -1,5 +1,5 @@
 load_module /usr/lib/nginx/modules/ngx_http_js_module.so;
-include /etc/nginx/includes/*.conf;
+include /etc/nginx/modules-includes/*.conf;
 
 worker_processes auto;
 

internal/mode/static/nginx/config/generator.go

@@ -16,6 +16,9 @@ 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 + "/modules-includes"
+
 	// secretsFolder is the folder where secrets (like TLS certs/keys) are stored.
 	secretsFolder = configFolder + "/secrets"
 
@@ -26,11 +29,11 @@ const (
 	configVersionFile = httpFolder + "/config-version.conf"
 
 	// loadModulesFile is the path to the file containing any load_module directives.
-	loadModulesFile = configFolder + "/includes/load_modules.conf"
+	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.

internal/mode/static/nginx/config/generator_test.go

@@ -110,6 +110,6 @@ func TestGenerate(t *testing.T) {
 	certBundle := string(files[3].Content)
 	g.Expect(certBundle).To(Equal("test-cert"))
 
-	g.Expect(files[4].Path).To(Equal("/etc/nginx/includes/load_modules.conf"))
+	g.Expect(files[4].Path).To(Equal("/etc/nginx/modules-includes/load-modules.conf"))
 	g.Expect(files[4].Content).To(Equal([]byte("load_module modules/ngx_otel_module.so;")))
 }

internal/mode/static/nginx/config/telemetry_test.go

@@ -6,7 +6,6 @@ import (
 
 	. "github.com/onsi/gomega"
 
-	ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
 )
 
@@ -18,7 +17,7 @@ func TestExecuteTelemetry(t *testing.T) {
 			Interval:    "5s",
 			BatchSize:   512,
 			BatchCount:  4,
-			SpanAttributes: []ngfAPI.SpanAttribute{
+			SpanAttributes: []dataplane.SpanAttribute{
 				{
 					Key:   "key1",
 					Value: "val1",

internal/mode/static/nginx/config/validation/common.go

@@ -84,3 +84,12 @@ func validateHeaderName(name string) error {
 	}
 	return nil
 }
+
+// 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)
+}

internal/mode/static/nginx/config/validation/common_test.go

@@ -71,3 +71,24 @@ func TestValidateValidHeaderName(t *testing.T) {
 		strings.Repeat("very-long-header", 16)+"1",
 	)
 }
+
+func TestGenericValidator_ValidateEscapedStringNoVarExpansion(t *testing.T) {
+	validator := GenericValidator{}
+
+	testValidValuesForSimpleValidator(
+		t,
+		validator.ValidateEscapedStringNoVarExpansion,
+		`test`,
+		`test test`,
+		`\"`,
+		`\\`,
+	)
+
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator.ValidateEscapedStringNoVarExpansion,
+		`\`,
+		`test"test`,
+		`$test`,
+	)
+}

internal/mode/static/nginx/config/validation/http_validator.go

@@ -12,6 +12,7 @@ type HTTPValidator struct {
 	HTTPRedirectValidator
 	HTTPURLRewriteValidator
 	HTTPRequestHeaderValidator
+	GenericValidator
 }
 
 var _ validation.HTTPFieldsValidator = HTTPValidator{}

internal/mode/static/state/change_processor.go

@@ -187,7 +187,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
 			{
 				gvk:       extractGVK(&ngfAPI.NginxProxy{}),
 				store:     newObjectStoreMapAdapter(clusterStore.NginxProxies),
-				predicate: nil,
+				predicate: funcPredicate{stateChanged: isReferenced},
 			},
 		},
 	)