Skip to content

Commit 4cf456d

Browse files
sjbermansjberman
committed
Support NGINX Plus usage reporting
Problem: As part of the Flexible Consumption Plan, NGINX Plus users are required to report usage to NGINX Instance Manager. Solution: Provide configuration options when deploying NGF to acquire credentials and send basic usage data (clusterUID, podCount, nodeCount) to the NGINX Instance Manager k8s API endpoint. Doc included to inform users how to do this.
1 parent 8d5f023 commit 4cf456d

34 files changed

+1739
-69
lines changed

‎.goreleaser.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,4 @@ release:
6666
extra_files:
6767
- glob: ./build/out/crds.yaml
6868
- glob: ./deploy/manifests/nginx-gateway.yaml
69+
- glob: ./deploy/manifests/nginx-plus-gateway.yaml

‎cmd/gateway/commands.go‎

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ func createStaticModeCommand() *cobra.Command {
5757
leaderElectionLockNameFlag = "leader-election-lock-name"
5858
plusFlag = "nginx-plus"
5959
gwAPIExperimentalFlag = "gateway-api-experimental-features"
60+
usageReportSecretFlag = "usage-report-secret"
61+
usageReportServerURLFlag = "usage-report-server-url"
62+
usageReportSkipVerifyFlag = "usage-report-skip-verify"
63+
usageReportClusterNameFlag = "usage-report-cluster-name"
6064
)
6165

6266
// flag values
@@ -95,9 +99,17 @@ func createStaticModeCommand() *cobra.Command {
9599
value: "nginx-gateway-leader-election-lock",
96100
}
97101

98-
plus bool
99-
100102
gwExperimentalFeatures bool
103+
104+
plus bool
105+
usageReportSkipVerify bool
106+
usageReportClusterName = stringValidatingValue{
107+
validator: validateQualifiedName,
108+
}
109+
usageReportSecretName = namespacedNameValue{}
110+
usageReportServerURL = stringValidatingValue{
111+
validator: validateURL,
112+
}
101113
)
102114

103115
cmd := &cobra.Command{
@@ -144,6 +156,20 @@ func createStaticModeCommand() *cobra.Command {
144156
gwNsName = &gateway.value
145157
}
146158

159+
var usageReportConfig *config.UsageReportConfig
160+
if cmd.Flags().Changed(usageReportSecretFlag) {
161+
if !plus {
162+
return errors.New("usage-report arguments are only valid if using nginx-plus")
163+
}
164+
165+
usageReportConfig = &config.UsageReportConfig{
166+
SecretNsName: usageReportSecretName.value,
167+
ServerURL: usageReportServerURL.value,
168+
ClusterDisplayName: usageReportClusterName.value,
169+
InsecureSkipVerify: usageReportSkipVerify,
170+
}
171+
}
172+
147173
conf := config.Config{
148174
GatewayCtlrName: gatewayCtlrName.value,
149175
ConfigName: configName.String(),
@@ -167,11 +193,12 @@ func createStaticModeCommand() *cobra.Command {
167193
Port: metricsListenPort.value,
168194
Secure: metricsSecure,
169195
},
170-
LeaderElection: config.LeaderElection{
196+
LeaderElection: config.LeaderElectionConfig{
171197
Enabled: !disableLeaderElection,
172198
LockName: leaderElectionLockName.String(),
173199
Identity: podName,
174200
},
201+
UsageReportConfig: usageReportConfig,
175202
Plus: plus,
176203
TelemetryReportPeriod: period,
177204
Version: version,
@@ -297,6 +324,33 @@ func createStaticModeCommand() *cobra.Command {
297324
"Requires the Gateway APIs installed from the experimental channel.",
298325
)
299326

327+
cmd.Flags().Var(
328+
&usageReportSecretName,
329+
usageReportSecretFlag,
330+
"The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting.",
331+
)
332+
333+
cmd.Flags().Var(
334+
&usageReportServerURL,
335+
usageReportServerURLFlag,
336+
"The base server URL of the NGINX Plus usage reporting server.",
337+
)
338+
339+
cmd.MarkFlagsRequiredTogether(usageReportSecretFlag, usageReportServerURLFlag)
340+
341+
cmd.Flags().Var(
342+
&usageReportClusterName,
343+
usageReportClusterNameFlag,
344+
"The display name of the Kubernetes cluster in the NGINX Plus usage reporting server.",
345+
)
346+
347+
cmd.Flags().BoolVar(
348+
&usageReportSkipVerify,
349+
usageReportSkipVerifyFlag,
350+
false,
351+
"Disable client verification of the NGINX Plus usage reporting server certificate.",
352+
)
353+
300354
return cmd
301355
}
302356

‎cmd/gateway/commands_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,9 @@ func TestStaticModeCmdFlagValidation(t *testing.T) {
155155
"--health-disable",
156156
"--leader-election-lock-name=my-lock",
157157
"--leader-election-disable=false",
158+
"--usage-report-secret=default/my-secret",
159+
"--usage-report-server-url=https://my-api.com",
160+
"--usage-report-cluster-name=my-cluster",
158161
},
159162
wantErr: false,
160163
},
@@ -310,6 +313,66 @@ func TestStaticModeCmdFlagValidation(t *testing.T) {
310313
wantErr: true,
311314
expectedErrPrefix: `invalid argument "" for "--leader-election-disable" flag: strconv.ParseBool`,
312315
},
316+
{
317+
name: "usage-report-secret is set to empty string",
318+
args: []string{
319+
"--usage-report-secret=",
320+
},
321+
wantErr: true,
322+
expectedErrPrefix: `invalid argument "" for "--usage-report-secret" flag: must be set`,
323+
},
324+
{
325+
name: "usage-report-secret is invalid",
326+
args: []string{
327+
"--usage-report-secret=my-secret", // no namespace
328+
},
329+
wantErr: true,
330+
expectedErrPrefix: `invalid argument "my-secret" for "--usage-report-secret" flag: invalid format; ` +
331+
"must be NAMESPACE/NAME",
332+
},
333+
{
334+
name: "usage-report-server-url is set to empty string",
335+
args: []string{
336+
"--usage-report-server-url=",
337+
},
338+
wantErr: true,
339+
expectedErrPrefix: `invalid argument "" for "--usage-report-server-url" flag: must be set`,
340+
},
341+
{
342+
name: "usage-report-server-url is an invalid url",
343+
args: []string{
344+
"--usage-report-server-url=invalid",
345+
},
346+
wantErr: true,
347+
expectedErrPrefix: `invalid argument "invalid" for "--usage-report-server-url" flag: "invalid" must be a valid URL`,
348+
},
349+
{
350+
name: "usage secret and server url not specified together",
351+
args: []string{
352+
"--gateway-ctlr-name=gateway.nginx.org/nginx-gateway",
353+
"--gatewayclass=nginx",
354+
"--usage-report-server-url=http://example.com",
355+
},
356+
wantErr: true,
357+
expectedErrPrefix: "if any flags in the group [usage-report-secret usage-report-server-url] " +
358+
"are set they must all be set",
359+
},
360+
{
361+
name: "usage-report-cluster-name is set to empty string",
362+
args: []string{
363+
"--usage-report-cluster-name=",
364+
},
365+
wantErr: true,
366+
expectedErrPrefix: `invalid argument "" for "--usage-report-cluster-name" flag: must be set`,
367+
},
368+
{
369+
name: "usage-report-cluster-name is invalid",
370+
args: []string{
371+
"--usage-report-cluster-name=$invalid*(#)",
372+
},
373+
wantErr: true,
374+
expectedErrPrefix: `invalid argument "$invalid*(#)" for "--usage-report-cluster-name" flag: invalid format`,
375+
},
313376
}
314377

315378
// common flags validation is tested separately

‎cmd/gateway/validation.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"net"
7+
"net/url"
78
"regexp"
89
"strings"
910

@@ -89,6 +90,38 @@ func parseNamespacedResourceName(value string) (types.NamespacedName, error) {
8990
}, nil
9091
}
9192

93+
func validateQualifiedName(name string) error {
94+
if len(name) == 0 {
95+
return errors.New("must be set")
96+
}
97+
98+
messages := validation.IsQualifiedName(name)
99+
if len(messages) > 0 {
100+
msg := strings.Join(messages, "; ")
101+
return fmt.Errorf("invalid format: %s", msg)
102+
}
103+
104+
return nil
105+
}
106+
107+
func validateURL(value string) error {
108+
if len(value) == 0 {
109+
return errors.New("must be set")
110+
}
111+
val, err := url.Parse(value)
112+
if err != nil {
113+
return fmt.Errorf("%q must be a valid URL: %w", value, err)
114+
}
115+
if val.Scheme == "" {
116+
return fmt.Errorf("%q must be a valid URL: bad scheme", value)
117+
}
118+
if val.Host == "" {
119+
return fmt.Errorf("%q must be a valid URL: bad host", value)
120+
}
121+
122+
return nil
123+
}
124+
92125
func validateIP(ip string) error {
93126
if ip == "" {
94127
return errors.New("IP address must be set")

‎cmd/gateway/validation_test.go‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,130 @@ func TestParseNamespacedResourceName(t *testing.T) {
255255
}
256256
}
257257

258+
func TestValidateQualifiedName(t *testing.T) {
259+
tests := []struct {
260+
name string
261+
value string
262+
expErr bool
263+
}{
264+
{
265+
name: "valid",
266+
value: "myName",
267+
expErr: false,
268+
},
269+
{
270+
name: "valid with hyphen",
271+
value: "my-name",
272+
expErr: false,
273+
},
274+
{
275+
name: "valid with numbers",
276+
value: "myName123",
277+
expErr: false,
278+
},
279+
{
280+
name: "valid with '/'",
281+
value: "my/name",
282+
expErr: false,
283+
},
284+
{
285+
name: "valid with '.'",
286+
value: "my.name",
287+
expErr: false,
288+
},
289+
{
290+
name: "empty",
291+
value: "",
292+
expErr: true,
293+
},
294+
{
295+
name: "invalid character '$'",
296+
value: "myName$",
297+
expErr: true,
298+
},
299+
{
300+
name: "invalid character '^'",
301+
value: "my^Name",
302+
expErr: true,
303+
},
304+
}
305+
306+
for _, test := range tests {
307+
t.Run(test.name, func(t *testing.T) {
308+
g := NewWithT(t)
309+
310+
err := validateQualifiedName(test.value)
311+
if test.expErr {
312+
g.Expect(err).To(HaveOccurred())
313+
} else {
314+
g.Expect(err).ToNot(HaveOccurred())
315+
}
316+
})
317+
}
318+
}
319+
320+
func TestValidateURL(t *testing.T) {
321+
tests := []struct {
322+
name string
323+
url string
324+
expErr bool
325+
}{
326+
{
327+
name: "valid",
328+
url: "http://server.com",
329+
expErr: false,
330+
},
331+
{
332+
name: "valid https",
333+
url: "https://server.com",
334+
expErr: false,
335+
},
336+
{
337+
name: "valid with port",
338+
url: "http://server.com:8080",
339+
expErr: false,
340+
},
341+
{
342+
name: "valid with ip address",
343+
url: "http://10.0.0.1",
344+
expErr: false,
345+
},
346+
{
347+
name: "valid with ip address and port",
348+
url: "http://10.0.0.1:8080",
349+
expErr: false,
350+
},
351+
{
352+
name: "invalid scheme",
353+
url: "http//server.com",
354+
expErr: true,
355+
},
356+
{
357+
name: "no scheme",
358+
url: "server.com",
359+
expErr: true,
360+
},
361+
{
362+
name: "no domain",
363+
url: "http://",
364+
expErr: true,
365+
},
366+
}
367+
368+
for _, tc := range tests {
369+
t.Run(tc.name, func(t *testing.T) {
370+
g := NewWithT(t)
371+
372+
err := validateURL(tc.url)
373+
if !tc.expErr {
374+
g.Expect(err).ToNot(HaveOccurred())
375+
} else {
376+
g.Expect(err).To(HaveOccurred())
377+
}
378+
})
379+
}
380+
}
381+
258382
func TestValidateIP(t *testing.T) {
259383
tests := []struct {
260384
name string

‎deploy/helm-chart/README.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,10 @@ The following tables lists the configurable parameters of the NGINX Gateway Fabr
297297
| `nginx.image.tag` | The tag for the NGINX image. | edge |
298298
| `nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always |
299299
| `nginx.plus` | Is NGINX Plus image being used | false |
300+
| `nginx.usage.secretName` | The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting. | |
301+
| `nginx.usage.serverURL` | The base server URL of the NGINX Plus usage reporting server. | |
302+
| `nginx.usage.clusterName` | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. | |
303+
| `nginx.usage.insecureSkipVerify` | Disable client verification of the NGINX Plus usage reporting server certificate. | false |
300304
| `nginx.lifecycle` | The `lifecycle` of the nginx container. | {} |
301305
| `nginx.extraVolumeMounts` | Extra `volumeMounts` for the nginx container. | {} |
302306
| `terminationGracePeriodSeconds` | The termination grace period of the NGINX Gateway Fabric pod. | 30 |

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,18 @@ spec:
5555
{{- if .Values.nginxGateway.gwAPIExperimentalFeatures.enable }}
5656
- --gateway-api-experimental-features
5757
{{- end }}
58+
{{- if .Values.nginx.usage.secretName }}
59+
- --usage-report-secret={{ .Values.nginx.usage.secretName }}
60+
{{- end }}
61+
{{- if .Values.nginx.usage.serverURL }}
62+
- --usage-report-server-url={{ .Values.nginx.usage.serverURL }}
63+
{{- end }}
64+
{{- if .Values.nginx.usage.clusterName }}
65+
- --usage-report-cluster-name={{ .Values.nginx.usage.clusterName }}
66+
{{- end }}
67+
{{- if .Values.nginx.usage.insecureSkipVerify }}
68+
- --usage-report-skip-verify
69+
{{- end }}
5870
env:
5971
- name: POD_IP
6072
valueFrom:

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ rules:
3636
- configmaps
3737
{{- end }}
3838
verbs:
39+
- get
3940
- list
4041
- watch
4142
# FIXME(bjee19): make nodes, pods, replicasets permission dependent on telemetry being enabled.

0 commit comments

Comments
 (0)