Skip to content

Commit 99719fe

Browse files
sjbermansjberman
committed
Code review; add validation and helm template
1 parent 0081b2f commit 99719fe

35 files changed

+758
-108
lines changed

‎charts/nginx-gateway-fabric/README.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ To uninstall/delete the release `ngf`:
249249
```shell
250250
helm uninstall ngf -n nginx-gateway
251251
kubectl delete ns nginx-gateway
252-
for crd in `kubectl get crds -oname | grep gateway.nginx.org | awk -F / '{ print $2 }'`; do kubectl delete crd $crd; done
252+
kubectl delete crd nginxgateways.gateway.nginx.org nginxproxies.gateway.nginx.org
253253
```
254254

255255
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
300300
| `nginx.image.tag` | The tag for the NGINX image. | edge |
301301
| `nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always |
302302
| `nginx.plus` | Is NGINX Plus image being used | false |
303+
| `nginx.config` | The configuration for the data plane that is contained in the NginxProxy resource. | [See nginx.config section](values.yaml) |
303304
| `nginx.usage.secretName` | The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting. | |
304305
| `nginx.usage.serverURL` | The base server URL of the NGINX Plus usage reporting server. | |
305306
| `nginx.usage.clusterName` | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. | |

‎charts/nginx-gateway-fabric/templates/_helpers.tpl

+8
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ Create control plane config name.
3131
{{- printf "%s-config" $name | trunc 63 | trimSuffix "-" }}
3232
{{- end }}
3333

34+
{{/*
35+
Create data plane config name.
36+
*/}}
37+
{{- define "nginx-gateway.proxy-config-name" -}}
38+
{{- $name := default .Release.Name .Values.nameOverride }}
39+
{{- printf "%s-proxy-config" $name | trunc 63 | trimSuffix "-" }}
40+
{{- end }}
41+
3442
{{/*
3543
Create chart name and version as used by the chart label.
3644
*/}}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ spec:
119119
- name: nginx-conf
120120
mountPath: /etc/nginx/conf.d
121121
- name: nginx-includes
122-
mountPath: /etc/nginx/includes
122+
mountPath: /etc/nginx/modules-includes
123123
- name: nginx-secrets
124124
mountPath: /etc/nginx/secrets
125125
- name: nginx-run
@@ -152,7 +152,7 @@ spec:
152152
- name: nginx-conf
153153
mountPath: /etc/nginx/conf.d
154154
- name: nginx-includes
155-
mountPath: /etc/nginx/includes
155+
mountPath: /etc/nginx/modules-includes
156156
- name: nginx-secrets
157157
mountPath: /etc/nginx/secrets
158158
- name: nginx-run

‎charts/nginx-gateway-fabric/templates/gatewayclass.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,9 @@ metadata:
66
{{- include "nginx-gateway.labels" . | nindent 4 }}
77
spec:
88
controllerName: {{ .Values.nginxGateway.gatewayControllerName }}
9+
{{- if .Values.nginx.config }}
10+
parametersRef:
11+
group: gateway.nginx.org
12+
kind: NginxProxy
13+
name: {{ include "nginx-gateway.proxy-config-name" . }}
14+
{{- end }}

‎charts/nginx-gateway-fabric/templates/nginxproxy.yaml

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{{- if .Values.nginx.config }}
2+
apiVersion: gateway.nginx.org/v1alpha1
3+
kind: NginxProxy
4+
metadata:
5+
name: {{ include "nginx-gateway.proxy-config-name" . }}
6+
labels:
7+
{{- include "nginx-gateway.labels" . | nindent 4 }}
8+
spec:
9+
{{- toYaml .Values.nginx.config | nindent 2 }}
10+
{{- end }}

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

+7-1
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,17 @@ rules:
111111
- gateway.nginx.org
112112
resources:
113113
- nginxgateways
114-
- nginxproxies
115114
verbs:
116115
- get
117116
- list
118117
- watch
118+
- apiGroups:
119+
- gateway.nginx.org
120+
resources:
121+
- nginxproxies
122+
verbs:
123+
- list
124+
- watch
119125
- apiGroups:
120126
- gateway.nginx.org
121127
resources:

‎charts/nginx-gateway-fabric/values.yaml

+11
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,17 @@ nginx:
7070
## Is NGINX Plus image being used
7171
plus: false
7272

73+
## The configuration for the data plane that is contained in the NginxProxy resource.
74+
config: {}
75+
# telemetry:
76+
# exporter:
77+
# endpoint: otel-collector.default.svc:4317
78+
# interval: 5s
79+
# batchSize: 512
80+
# batchCount: 4
81+
# serviceName: ""
82+
# spanAttributes: []
83+
7384
## Configuration for NGINX Plus usage reporting.
7485
usage:
7586
## The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting.

‎conformance/provisioner/static-deployment.yaml

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ spec:
7171
- name: nginx-conf
7272
mountPath: /etc/nginx/conf.d
7373
- name: nginx-includes
74-
mountPath: /etc/nginx/includes
74+
mountPath: /etc/nginx/modules-includes
7575
- name: nginx-secrets
7676
mountPath: /etc/nginx/secrets
7777
- name: nginx-run
@@ -97,7 +97,7 @@ spec:
9797
- name: nginx-conf
9898
mountPath: /etc/nginx/conf.d
9999
- name: nginx-includes
100-
mountPath: /etc/nginx/includes
100+
mountPath: /etc/nginx/modules-includes
101101
- name: nginx-secrets
102102
mountPath: /etc/nginx/secrets
103103
- name: nginx-run

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

+9-3
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,17 @@ rules:
9393
- gateway.nginx.org
9494
resources:
9595
- nginxgateways
96-
- nginxproxies
9796
verbs:
9897
- get
9998
- list
10099
- watch
100+
- apiGroups:
101+
- gateway.nginx.org
102+
resources:
103+
- nginxproxies
104+
verbs:
105+
- list
106+
- watch
101107
- apiGroups:
102108
- gateway.nginx.org
103109
resources:
@@ -215,7 +221,7 @@ spec:
215221
- name: nginx-conf
216222
mountPath: /etc/nginx/conf.d
217223
- name: nginx-includes
218-
mountPath: /etc/nginx/includes
224+
mountPath: /etc/nginx/modules-includes
219225
- name: nginx-secrets
220226
mountPath: /etc/nginx/secrets
221227
- name: nginx-run
@@ -241,7 +247,7 @@ spec:
241247
- name: nginx-conf
242248
mountPath: /etc/nginx/conf.d
243249
- name: nginx-includes
244-
mountPath: /etc/nginx/includes
250+
mountPath: /etc/nginx/modules-includes
245251
- name: nginx-secrets
246252
mountPath: /etc/nginx/secrets
247253
- name: nginx-run

‎deploy/manifests/nginx-gateway.yaml

+9-3
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,17 @@ rules:
9090
- gateway.nginx.org
9191
resources:
9292
- nginxgateways
93-
- nginxproxies
9493
verbs:
9594
- get
9695
- list
9796
- watch
97+
- apiGroups:
98+
- gateway.nginx.org
99+
resources:
100+
- nginxproxies
101+
verbs:
102+
- list
103+
- watch
98104
- apiGroups:
99105
- gateway.nginx.org
100106
resources:
@@ -211,7 +217,7 @@ spec:
211217
- name: nginx-conf
212218
mountPath: /etc/nginx/conf.d
213219
- name: nginx-includes
214-
mountPath: /etc/nginx/includes
220+
mountPath: /etc/nginx/modules-includes
215221
- name: nginx-secrets
216222
mountPath: /etc/nginx/secrets
217223
- name: nginx-run
@@ -237,7 +243,7 @@ spec:
237243
- name: nginx-conf
238244
mountPath: /etc/nginx/conf.d
239245
- name: nginx-includes
240-
mountPath: /etc/nginx/includes
246+
mountPath: /etc/nginx/modules-includes
241247
- name: nginx-secrets
242248
mountPath: /etc/nginx/secrets
243249
- name: nginx-run

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

+9-3
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,17 @@ rules:
9999
- gateway.nginx.org
100100
resources:
101101
- nginxgateways
102-
- nginxproxies
103102
verbs:
104103
- get
105104
- list
106105
- watch
106+
- apiGroups:
107+
- gateway.nginx.org
108+
resources:
109+
- nginxproxies
110+
verbs:
111+
- list
112+
- watch
107113
- apiGroups:
108114
- gateway.nginx.org
109115
resources:
@@ -222,7 +228,7 @@ spec:
222228
- name: nginx-conf
223229
mountPath: /etc/nginx/conf.d
224230
- name: nginx-includes
225-
mountPath: /etc/nginx/includes
231+
mountPath: /etc/nginx/modules-includes
226232
- name: nginx-secrets
227233
mountPath: /etc/nginx/secrets
228234
- name: nginx-run
@@ -248,7 +254,7 @@ spec:
248254
- name: nginx-conf
249255
mountPath: /etc/nginx/conf.d
250256
- name: nginx-includes
251-
mountPath: /etc/nginx/includes
257+
mountPath: /etc/nginx/modules-includes
252258
- name: nginx-secrets
253259
mountPath: /etc/nginx/secrets
254260
- name: nginx-run

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

+9-3
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,17 @@ rules:
9696
- gateway.nginx.org
9797
resources:
9898
- nginxgateways
99-
- nginxproxies
10099
verbs:
101100
- get
102101
- list
103102
- watch
103+
- apiGroups:
104+
- gateway.nginx.org
105+
resources:
106+
- nginxproxies
107+
verbs:
108+
- list
109+
- watch
104110
- apiGroups:
105111
- gateway.nginx.org
106112
resources:
@@ -218,7 +224,7 @@ spec:
218224
- name: nginx-conf
219225
mountPath: /etc/nginx/conf.d
220226
- name: nginx-includes
221-
mountPath: /etc/nginx/includes
227+
mountPath: /etc/nginx/modules-includes
222228
- name: nginx-secrets
223229
mountPath: /etc/nginx/secrets
224230
- name: nginx-run
@@ -244,7 +250,7 @@ spec:
244250
- name: nginx-conf
245251
mountPath: /etc/nginx/conf.d
246252
- name: nginx-includes
247-
mountPath: /etc/nginx/includes
253+
mountPath: /etc/nginx/modules-includes
248254
- name: nginx-secrets
249255
mountPath: /etc/nginx/secrets
250256
- name: nginx-run

‎internal/mode/static/manager.go

+1
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ func StartManager(cfg config.Config) error {
116116
Logger: cfg.Logger.WithName("changeProcessor"),
117117
Validators: validation.Validators{
118118
HTTPFieldsValidator: ngxvalidation.HTTPValidator{},
119+
GenericValidator: ngxvalidation.GenericValidator{},
119120
},
120121
EventRecorder: recorder,
121122
Scheme: scheme,

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
load_module /usr/lib/nginx/modules/ngx_http_js_module.so;
2-
include /etc/nginx/includes/*.conf;
2+
include /etc/nginx/modules-includes/*.conf;
33

44
worker_processes auto;
55

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
load_module /usr/lib/nginx/modules/ngx_http_js_module.so;
2-
include /etc/nginx/includes/*.conf;
2+
include /etc/nginx/modules-includes/*.conf;
33

44
worker_processes auto;
55

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ const (
1616
// httpFolder is the folder where NGINX HTTP configuration files are stored.
1717
httpFolder = configFolder + "/conf.d"
1818

19+
// modulesIncludesFolder is the folder where the included "load_module" file is stored.
20+
modulesIncludesFolder = configFolder + "/modules-includes"
21+
1922
// secretsFolder is the folder where secrets (like TLS certs/keys) are stored.
2023
secretsFolder = configFolder + "/secrets"
2124

@@ -26,11 +29,11 @@ const (
2629
configVersionFile = httpFolder + "/config-version.conf"
2730

2831
// loadModulesFile is the path to the file containing any load_module directives.
29-
loadModulesFile = configFolder + "/includes/load_modules.conf"
32+
loadModulesFile = modulesIncludesFolder + "/load-modules.conf"
3033
)
3134

3235
// ConfigFolders is a list of folders where NGINX configuration files are stored.
33-
var ConfigFolders = []string{httpFolder, secretsFolder}
36+
var ConfigFolders = []string{httpFolder, secretsFolder, modulesIncludesFolder}
3437

3538
// Generator generates NGINX configuration files.
3639
// This interface is used for testing purposes only.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,6 @@ func TestGenerate(t *testing.T) {
110110
certBundle := string(files[3].Content)
111111
g.Expect(certBundle).To(Equal("test-cert"))
112112

113-
g.Expect(files[4].Path).To(Equal("/etc/nginx/includes/load_modules.conf"))
113+
g.Expect(files[4].Path).To(Equal("/etc/nginx/modules-includes/load-modules.conf"))
114114
g.Expect(files[4].Content).To(Equal([]byte("load_module modules/ngx_otel_module.so;")))
115115
}

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66

77
. "github.com/onsi/gomega"
88

9-
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
109
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
1110
)
1211

@@ -18,7 +17,7 @@ func TestExecuteTelemetry(t *testing.T) {
1817
Interval: "5s",
1918
BatchSize: 512,
2019
BatchCount: 4,
21-
SpanAttributes: []ngfAPI.SpanAttribute{
20+
SpanAttributes: []dataplane.SpanAttribute{
2221
{
2322
Key: "key1",
2423
Value: "val1",

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

+9
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,12 @@ func validateHeaderName(name string) error {
8484
}
8585
return nil
8686
}
87+
88+
// GenericValidator validates values for generic cases in the nginx conf.
89+
type GenericValidator struct{}
90+
91+
// ValidateEscapedStringNoVarExpansion ensures that no invalid characters are included in the string value that
92+
// could lead to unwanted nginx behavior.
93+
func (GenericValidator) ValidateEscapedStringNoVarExpansion(value string) error {
94+
return validateEscapedStringNoVarExpansion(value, nil)
95+
}

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

+21
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,24 @@ func TestValidateValidHeaderName(t *testing.T) {
7171
strings.Repeat("very-long-header", 16)+"1",
7272
)
7373
}
74+
75+
func TestGenericValidator_ValidateEscapedStringNoVarExpansion(t *testing.T) {
76+
validator := GenericValidator{}
77+
78+
testValidValuesForSimpleValidator(
79+
t,
80+
validator.ValidateEscapedStringNoVarExpansion,
81+
`test`,
82+
`test test`,
83+
`\"`,
84+
`\\`,
85+
)
86+
87+
testInvalidValuesForSimpleValidator(
88+
t,
89+
validator.ValidateEscapedStringNoVarExpansion,
90+
`\`,
91+
`test"test`,
92+
`$test`,
93+
)
94+
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type HTTPValidator struct {
1212
HTTPRedirectValidator
1313
HTTPURLRewriteValidator
1414
HTTPRequestHeaderValidator
15+
GenericValidator
1516
}
1617

1718
var _ validation.HTTPFieldsValidator = HTTPValidator{}

‎internal/mode/static/state/change_processor.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
187187
{
188188
gvk: extractGVK(&ngfAPI.NginxProxy{}),
189189
store: newObjectStoreMapAdapter(clusterStore.NginxProxies),
190-
predicate: nil,
190+
predicate: funcPredicate{stateChanged: isReferenced},
191191
},
192192
},
193193
)

0 commit comments

Comments
 (0)