Skip to content

Commit 041fea9

Browse files
sjbermansjberman
committed
Address all of the code reviews
1 parent 42707be commit 041fea9

File tree

27 files changed

+278
-149
lines changed

27 files changed

+278
-149
lines changed

‎apis/v1alpha1/nginxgateway_types.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ const (
6363
// NginxGatewayStatus defines the state of the NginxGateway.
6464
type NginxGatewayStatus struct {
6565
// +optional
66+
// +listType=map
67+
// +listMapKey=type
68+
// +kubebuilder:validation:MaxItems=8
6669
Conditions []metav1.Condition `json:"conditions,omitempty"`
6770
}
6871

@@ -75,12 +78,13 @@ type NginxGatewayConditionType string
7578
type NginxGatewayConditionReason string
7679

7780
const (
78-
// This condition is true when the NginxGateway configuration is syntactically and semantically valid.
81+
// NginxGatewayConditionValid is a condition that is true when the NginxGateway
82+
// configuration is syntactically and semantically valid.
7983
NginxGatewayConditionValid NginxGatewayConditionType = "Valid"
8084

81-
// This reason is used with the "Valid" condition when the condition is True.
85+
// NginxGatewayReasonValid is a reason that is used with the "Valid" condition when the condition is True.
8286
NginxGatewayReasonValid NginxGatewayConditionReason = "Valid"
8387

84-
// This reason is used with the "Valid" condition when the condition is False.
88+
// NginxGatewayReasonInvalid is a reason that is used with the "Valid" condition when the condition is False.
8589
NginxGatewayReasonInvalid NginxGatewayConditionReason = "Invalid"
8690
)

‎cmd/gateway/commands.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,9 @@ func createStaticModeCommand() *cobra.Command {
120120
// flag values
121121
gateway := namespacedNameValue{}
122122
var updateGCStatus bool
123-
var configCRDName string
123+
configName := stringValidatingValue{
124+
validator: validateResourceName,
125+
}
124126

125127
cmd := &cobra.Command{
126128
Use: "static-mode",
@@ -153,7 +155,7 @@ func createStaticModeCommand() *cobra.Command {
153155

154156
conf := config.Config{
155157
GatewayCtlrName: gatewayCtlrName.value,
156-
ConfigCRDName: configCRDName,
158+
ConfigName: configName.String(),
157159
Logger: logger,
158160
AtomicLevel: atom,
159161
GatewayClassName: gatewayClassName.value,
@@ -181,11 +183,12 @@ func createStaticModeCommand() *cobra.Command {
181183
"equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.",
182184
)
183185

184-
cmd.Flags().StringVar(
185-
&configCRDName,
186-
"nginx-gateway-config-name",
187-
"nginx-gateway-config",
188-
`The name of the NginxGateway CRD to be used for this controller's dynamic configuration.`,
186+
cmd.Flags().VarP(
187+
&configName,
188+
"config",
189+
"c",
190+
`The name of the NginxGateway resource to be used for this controller's dynamic configuration.`+
191+
` Lives in the same Namespace as the controller.`,
189192
)
190193

191194
cmd.Flags().BoolVar(

‎conformance/provisioner/static-deployment.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ spec:
2727
- static-mode
2828
- --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller
2929
- --gatewayclass=nginx
30-
- --nginx-gateway-config-name=nginx-gateway-config
30+
- --config=nginx-gateway-config
3131
env:
3232
- name: POD_IP
3333
valueFrom:

‎deploy/helm-chart/README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,9 @@ The following tables lists the configurable parameters of the NGINX Kubernetes G
129129
| `nginxGateway.image.tag` | The tag for the NGINX Kubernetes Gateway image. | edge |
130130
| `nginxGateway.image.pullPolicy` | The `imagePullPolicy` for the NGINX Kubernetes Gateway image. | Always |
131131
| `nginxGateway.gatewayClassName` | The name of the GatewayClass for the NGINX Kubernetes Gateway deployment. | nginx |
132-
| `nginxGateway.gatewayControllerName` | The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is k8s-gateway.nginx.org. | k8s-gateway.nginx.org/nginx-gateway-controller |
132+
| `nginxGateway.gatewayControllerName` | The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is gateway.nginx.org. | gateway.nginx.org/nginx-gateway-controller |
133133
| `nginxGateway.kind` | The kind of the NGINX Kubernetes Gateway installation - currently, only Deployment is supported. | deployment |
134+
|`nginxGateway.config` | The dynamic configuration for the control plane that is contained in the NginxGateway resource | [See nginxGateway.config section](values.yaml) |
134135
| `nginx.image.repository` | The repository for the NGINX image. | ghcr.io/nginxinc/nginx-kubernetes-gateway/nginx |
135136
| `nginx.image.tag` | The tag for the NGINX image. | edge |
136137
| `nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always |

‎deploy/helm-chart/crds/gateway.nginx.org_nginxgateways.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,11 @@ spec:
119119
- status
120120
- type
121121
type: object
122+
maxItems: 8
122123
type: array
124+
x-kubernetes-list-map-keys:
125+
- type
126+
x-kubernetes-list-type: map
123127
type: object
124128
required:
125129
- spec

‎deploy/helm-chart/templates/_helpers.tpl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ If release name contains chart name it will be used as a full name.
2323
{{- end }}
2424
{{- end }}
2525

26+
{{/*
27+
Create control plane config name.
28+
*/}}
29+
{{- define "nginx-gateway.config-name" -}}
30+
{{- $name := default .Release.Name .Values.nameOverride }}
31+
{{- printf "%s-config" $name | trunc 63 | trimSuffix "-" }}
32+
{{- end }}
33+
2634
{{/*
2735
Create chart name and version as used by the chart label.
2836
*/}}

‎deploy/helm-chart/templates/control-config.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
apiVersion: gateway.nginx.org/v1alpha1
22
kind: NginxGateway
33
metadata:
4-
name: {{ .Values.nginxGateway.crdConfig.name }}
4+
name: {{ include "nginx-gateway.config-name" . }}
55
namespace: {{ .Release.Namespace }}
66
labels:
77
{{- include "nginx-gateway.labels" . | nindent 4 }}
88
spec:
9-
{{- toYaml .Values.nginxGateway.crdConfig.spec | nindent 2 }}
9+
{{- toYaml .Values.nginxGateway.config | nindent 2 }}

‎deploy/helm-chart/templates/deployment.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ spec:
2222
- static-mode
2323
- --gateway-ctlr-name={{ .Values.nginxGateway.gatewayControllerName }}
2424
- --gatewayclass={{ .Values.nginxGateway.gatewayClassName }}
25-
- --nginx-gateway-config-name={{ .Values.nginxGateway.crdConfig.name }}
25+
- --config={{ include "nginx-gateway.config-name" . }}
2626
env:
2727
- name: POD_IP
2828
valueFrom:

‎deploy/helm-chart/templates/rbac.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ rules:
6868
resources:
6969
- nginxgateways
7070
verbs:
71+
- get
7172
- list
7273
- watch
7374
- apiGroups:

‎deploy/helm-chart/values.yaml

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,11 @@ nginxGateway:
88
## The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain
99
## is gateway.nginx.org.
1010
gatewayControllerName: gateway.nginx.org/nginx-gateway-controller
11-
## The dynamic configuration for the control plane that is contained in the NginxGateway CRD.
12-
crdConfig:
13-
name: nginx-gateway-config
14-
spec:
15-
logging:
16-
## Log level. Supported values "info", "debug", "error".
17-
level: info
11+
## The dynamic configuration for the control plane that is contained in the NginxGateway resource.
12+
config:
13+
logging:
14+
## Log level. Supported values "info", "debug", "error".
15+
level: info
1816

1917
image:
2018
## The NGINX Kubernetes Gateway image to use

‎deploy/manifests/nginx-gateway.yaml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,11 @@ spec:
125125
- status
126126
- type
127127
type: object
128+
maxItems: 8
128129
type: array
130+
x-kubernetes-list-map-keys:
131+
- type
132+
x-kubernetes-list-type: map
129133
type: object
130134
required:
131135
- spec
@@ -212,6 +216,7 @@ rules:
212216
resources:
213217
- nginxgateways
214218
verbs:
219+
- get
215220
- list
216221
- watch
217222
- apiGroups:
@@ -267,7 +272,7 @@ spec:
267272
- static-mode
268273
- --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller
269274
- --gatewayclass=nginx
270-
- --nginx-gateway-config-name=nginx-gateway-config
275+
- --config=nginx-gateway-config
271276
env:
272277
- name: POD_IP
273278
valueFrom:

‎docs/cli-help.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ Flags:
1616

1717
| Name | Type | Description |
1818
|------------------------------|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
19-
| `gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/PATH`. The controller's domain is `gateway.nginx.org`. |
20-
| `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. |
19+
| `gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/PATH`. The controller's domain is `gateway.nginx.org`. |
20+
| `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. |
2121
| `gateway` | `string` | The namespaced name of the Gateway resource to use. Must be of the form: `NAMESPACE/NAME`. If not specified, the control plane will process all Gateways for the configured GatewayClass. However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}. |
22-
| `update-gatewayclass-status` | `bool` | Update the status of the GatewayClass resource. (default true) |
22+
| `config` | `string` | The name of the NginxGateway resource to be used for this controller's dynamic configuration. Lives in the same Namespace as the controller. |
23+
| `update-gatewayclass-status` | `bool` | Update the status of the GatewayClass resource. (default true) |

‎docs/control-plane-configuration.md

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,35 @@ This document describes how to dynamically update the NGINX Kubernetes Gateway c
55
## Overview
66

77
NGINX Kubernetes Gateway offers a way to update the control plane configuration dynamically without the need for a
8-
restart. These configuration options include:
8+
restart. The control plane configuration is stored in the NginxGateway custom resource. This resource is created
9+
during the installation of NGINX Kubernetes Gateway.
910

10-
| Option | Available values | Default value |
11-
|---------------|--------------------|---------------|
12-
| Logging Level | info, debug, error | info |
11+
If using manifests, the default name of the resource is `nginx-gateway-config`. If using Helm, the default name
12+
of the resource is `<release-name>-config`. It is deployed in the same Namespace as the controller
13+
(default `nginx-gateway`).
1314

15+
The control plane only watches this single instance of the custom resource. If the resource is invalid per the OpenAPI
16+
schema, the Kubernetes API server will reject the changes. If the resource is deleted or deemed invalid by NGINX
17+
Kubernetes Gateway, an error Event is created in the `nginx-gateway` Namespace, and the default values will be used by
18+
the control plane for its configuration.
1419

15-
The control plane configuration is stored in the NginxGateway custom resource. This resource is created during the
16-
installation of NGINX Kubernetes Gateway. The default name of the resource is `nginx-gateway-config` and is deployed
17-
in the same Namespace as the controller (`nginx-gateway`).
20+
### Spec
1821

19-
The control plane only watches this single instance of the custom resource. If the resource is deleted or invalid, an
20-
error is emitted and the default values will be used by the control plane for its configuration.
22+
| name | description | type | required |
23+
|---------|-----------------------------------------------------------------|--------------------------|----------|
24+
| logging | Logging defines logging related settings for the control plane. | [logging](#speclogging) | no |
25+
26+
### Spec.Logging
27+
28+
| name | description | type | required |
29+
|-------|------------------------------------------------------------------------|--------|----------|
30+
| level | Level defines the logging level. Supported values: info, debug, error. | string | no |
2131

2232
## Viewing and Updating the Configuration
2333

34+
> For the following examples, the name `nginx-gateway-config` should be updated to the name of the resource that
35+
> was created by your installation.
36+
2437
To view the current configuration:
2538

2639
```shell
@@ -35,3 +48,9 @@ kubectl -n nginx-gateway edit nginxgateways nginx-gateway-config
3548

3649
This will open the configuration in your default editor. You can then update and save the configuration, which is
3750
applied automatically to the control plane.
51+
52+
To view the status of the configuration:
53+
54+
```shell
55+
kubectl -n nginx-gateway describe nginxgateways nginx-gateway-config
56+
```

‎examples/advanced-routing/gateway.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1
22
kind: Gateway
33
metadata:
44
name: gateway
5-
labels:
6-
domain: gateway.nginx.org
75
spec:
86
gatewayClassName: nginx
97
listeners:

‎examples/cafe-example/gateway.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1
22
kind: Gateway
33
metadata:
44
name: gateway
5-
labels:
6-
domain: gateway.nginx.org
75
spec:
86
gatewayClassName: nginx
97
listeners:

‎examples/cross-namespace-routing/gateway.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1
22
kind: Gateway
33
metadata:
44
name: gateway
5-
labels:
6-
domain: gateway.nginx.org
75
spec:
86
gatewayClassName: nginx
97
listeners:

‎examples/http-header-filter/gateway.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1
22
kind: Gateway
33
metadata:
44
name: gateway
5-
labels:
6-
domain: gateway.nginx.org
75
spec:
86
gatewayClassName: nginx
97
listeners:

‎examples/https-termination/gateway.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1
22
kind: Gateway
33
metadata:
44
name: gateway
5-
labels:
6-
domain: gateway.nginx.org
75
spec:
86
gatewayClassName: nginx
97
listeners:

‎examples/traffic-splitting/gateway.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ apiVersion: gateway.networking.k8s.io/v1beta1
22
kind: Gateway
33
metadata:
44
name: gateway
5-
labels:
6-
domain: gateway.nginx.org
75
spec:
86
gatewayClassName: nginx
97
listeners:

‎internal/framework/status/statuses.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ type Statuses struct {
2424
GatewayClassStatuses GatewayClassStatuses
2525
GatewayStatuses GatewayStatuses
2626
HTTPRouteStatuses HTTPRouteStatuses
27-
NginxGatewayStatus NginxGatewayStatus
27+
NginxGatewayStatus *NginxGatewayStatus
2828
}
2929

3030
// GatewayStatus holds the status of the winning Gateway resource.
@@ -75,8 +75,8 @@ type GatewayClassStatus struct {
7575

7676
// NginxGatewayStatus holds status-related information about the NginxGateway resource.
7777
type NginxGatewayStatus struct {
78-
// NSName is the NamespacedName of the NginxGateway resource.
79-
NSName types.NamespacedName
78+
// NsName is the NamespacedName of the NginxGateway resource.
79+
NsName types.NamespacedName
8080
// Conditions is the list of conditions for this NginxGateway.
8181
Conditions []conditions.Condition
8282
// ObservedGeneration is the generation of the resource that was processed.

‎internal/framework/status/updater.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses Statuses) {
121121
}
122122

123123
ngStatus := statuses.NginxGatewayStatus
124-
if len(ngStatus.Conditions) > 0 {
125-
upd.update(ctx, ngStatus.NSName, &nkgAPI.NginxGateway{}, func(object client.Object) {
124+
if ngStatus != nil {
125+
upd.update(ctx, ngStatus.NsName, &nkgAPI.NginxGateway{}, func(object client.Object) {
126126
ng := object.(*nkgAPI.NginxGateway)
127127
ng.Status = nkgAPI.NginxGatewayStatus{
128128
Conditions: convertConditions(

‎internal/framework/status/updater_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,12 @@ var _ = Describe("Updater", func() {
109109
},
110110
},
111111
},
112-
NginxGatewayStatus: status.NginxGatewayStatus{
113-
NSName: types.NamespacedName{
112+
NginxGatewayStatus: &status.NginxGatewayStatus{
113+
NsName: types.NamespacedName{
114114
Namespace: "nginx-gateway",
115115
Name: "nginx-gateway-config",
116116
},
117-
ObservedGeneration: 0,
117+
ObservedGeneration: 3,
118118
Conditions: status.CreateTestConditions("Test"),
119119
},
120120
}
@@ -364,7 +364,7 @@ var _ = Describe("Updater", func() {
364364
APIVersion: "gateway.nginx.org/v1alpha1",
365365
},
366366
Status: nkgAPI.NginxGatewayStatus{
367-
Conditions: status.CreateExpectedAPIConditions("Test", 0, fakeClockTime),
367+
Conditions: status.CreateExpectedAPIConditions("Test", 3, fakeClockTime),
368368
},
369369
}
370370

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ import (
99
type Config struct {
1010
// GatewayCtlrName is the name of this controller.
1111
GatewayCtlrName string
12-
// ConfigCRDName is the name of the NginxControlConfig CRD for this controller.
13-
ConfigCRDName string
14-
Logger logr.Logger
15-
AtomicLevel zap.AtomicLevel
12+
// ConfigName is the name of the NginxGateway resource for this controller.
13+
ConfigName string
14+
// Logger is the Zap Logger used by all components.
15+
Logger logr.Logger
16+
// AtomicLevel is an atomically changeable, dynamic logging level.
17+
AtomicLevel zap.AtomicLevel
1618
// GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use.
1719
// The Gateway will ignore all other Gateway resources.
1820
GatewayNsName *types.NamespacedName

0 commit comments

Comments
 (0)