Skip to content

Commit

Permalink
Support NGINX Plus usage reporting (#1544)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sjberman authored Feb 20, 2024
1 parent 8d5f023 commit dec2f1b
Show file tree
Hide file tree
Showing 34 changed files with 1,840 additions and 72 deletions.
3 changes: 3 additions & 0 deletions .goreleaser.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,6 @@ release:
extra_files:
- glob: ./build/out/crds.yaml
- glob: ./deploy/manifests/nginx-gateway.yaml
- glob: ./deploy/manifests/nginx-plus-gateway.yaml
- glob: ./deploy/manifests/nginx-gateway-experimental.yaml
- glob: ./deploy/manifests/nginx-plus-gateway-experimental.yaml
60 changes: 57 additions & 3 deletions cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ func createStaticModeCommand() *cobra.Command {
leaderElectionLockNameFlag = "leader-election-lock-name"
plusFlag = "nginx-plus"
gwAPIExperimentalFlag = "gateway-api-experimental-features"
usageReportSecretFlag = "usage-report-secret"
usageReportServerURLFlag = "usage-report-server-url"
usageReportSkipVerifyFlag = "usage-report-skip-verify"
usageReportClusterNameFlag = "usage-report-cluster-name"
)

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

plus bool

gwExperimentalFeatures bool

plus bool
usageReportSkipVerify bool
usageReportClusterName = stringValidatingValue{
validator: validateQualifiedName,
}
usageReportSecretName = namespacedNameValue{}
usageReportServerURL = stringValidatingValue{
validator: validateURL,
}
)

cmd := &cobra.Command{
Expand Down Expand Up @@ -144,6 +156,20 @@ func createStaticModeCommand() *cobra.Command {
gwNsName = &gateway.value
}

var usageReportConfig *config.UsageReportConfig
if cmd.Flags().Changed(usageReportSecretFlag) {
if !plus {
return errors.New("usage-report arguments are only valid if using nginx-plus")
}

usageReportConfig = &config.UsageReportConfig{
SecretNsName: usageReportSecretName.value,
ServerURL: usageReportServerURL.value,
ClusterDisplayName: usageReportClusterName.value,
InsecureSkipVerify: usageReportSkipVerify,
}
}

conf := config.Config{
GatewayCtlrName: gatewayCtlrName.value,
ConfigName: configName.String(),
Expand All @@ -167,11 +193,12 @@ func createStaticModeCommand() *cobra.Command {
Port: metricsListenPort.value,
Secure: metricsSecure,
},
LeaderElection: config.LeaderElection{
LeaderElection: config.LeaderElectionConfig{
Enabled: !disableLeaderElection,
LockName: leaderElectionLockName.String(),
Identity: podName,
},
UsageReportConfig: usageReportConfig,
Plus: plus,
TelemetryReportPeriod: period,
Version: version,
Expand Down Expand Up @@ -297,6 +324,33 @@ func createStaticModeCommand() *cobra.Command {
"Requires the Gateway APIs installed from the experimental channel.",
)

cmd.Flags().Var(
&usageReportSecretName,
usageReportSecretFlag,
"The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting.",
)

cmd.Flags().Var(
&usageReportServerURL,
usageReportServerURLFlag,
"The base server URL of the NGINX Plus usage reporting server.",
)

cmd.MarkFlagsRequiredTogether(usageReportSecretFlag, usageReportServerURLFlag)

cmd.Flags().Var(
&usageReportClusterName,
usageReportClusterNameFlag,
"The display name of the Kubernetes cluster in the NGINX Plus usage reporting server.",
)

cmd.Flags().BoolVar(
&usageReportSkipVerify,
usageReportSkipVerifyFlag,
false,
"Disable client verification of the NGINX Plus usage reporting server certificate.",
)

return cmd
}

Expand Down
63 changes: 63 additions & 0 deletions cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ func TestStaticModeCmdFlagValidation(t *testing.T) {
"--health-disable",
"--leader-election-lock-name=my-lock",
"--leader-election-disable=false",
"--usage-report-secret=default/my-secret",
"--usage-report-server-url=https://my-api.com",
"--usage-report-cluster-name=my-cluster",
},
wantErr: false,
},
Expand Down Expand Up @@ -310,6 +313,66 @@ func TestStaticModeCmdFlagValidation(t *testing.T) {
wantErr: true,
expectedErrPrefix: `invalid argument "" for "--leader-election-disable" flag: strconv.ParseBool`,
},
{
name: "usage-report-secret is set to empty string",
args: []string{
"--usage-report-secret=",
},
wantErr: true,
expectedErrPrefix: `invalid argument "" for "--usage-report-secret" flag: must be set`,
},
{
name: "usage-report-secret is invalid",
args: []string{
"--usage-report-secret=my-secret", // no namespace
},
wantErr: true,
expectedErrPrefix: `invalid argument "my-secret" for "--usage-report-secret" flag: invalid format; ` +
"must be NAMESPACE/NAME",
},
{
name: "usage-report-server-url is set to empty string",
args: []string{
"--usage-report-server-url=",
},
wantErr: true,
expectedErrPrefix: `invalid argument "" for "--usage-report-server-url" flag: must be set`,
},
{
name: "usage-report-server-url is an invalid url",
args: []string{
"--usage-report-server-url=invalid",
},
wantErr: true,
expectedErrPrefix: `invalid argument "invalid" for "--usage-report-server-url" flag: "invalid" must be a valid URL`,
},
{
name: "usage secret and server url not specified together",
args: []string{
"--gateway-ctlr-name=gateway.nginx.org/nginx-gateway",
"--gatewayclass=nginx",
"--usage-report-server-url=http://example.com",
},
wantErr: true,
expectedErrPrefix: "if any flags in the group [usage-report-secret usage-report-server-url] " +
"are set they must all be set",
},
{
name: "usage-report-cluster-name is set to empty string",
args: []string{
"--usage-report-cluster-name=",
},
wantErr: true,
expectedErrPrefix: `invalid argument "" for "--usage-report-cluster-name" flag: must be set`,
},
{
name: "usage-report-cluster-name is invalid",
args: []string{
"--usage-report-cluster-name=$invalid*(#)",
},
wantErr: true,
expectedErrPrefix: `invalid argument "$invalid*(#)" for "--usage-report-cluster-name" flag: invalid format`,
},
}

// common flags validation is tested separately
Expand Down
33 changes: 33 additions & 0 deletions cmd/gateway/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"net"
"net/url"
"regexp"
"strings"

Expand Down Expand Up @@ -89,6 +90,38 @@ func parseNamespacedResourceName(value string) (types.NamespacedName, error) {
}, nil
}

func validateQualifiedName(name string) error {
if len(name) == 0 {
return errors.New("must be set")
}

messages := validation.IsQualifiedName(name)
if len(messages) > 0 {
msg := strings.Join(messages, "; ")
return fmt.Errorf("invalid format: %s", msg)
}

return nil
}

func validateURL(value string) error {
if len(value) == 0 {
return errors.New("must be set")
}
val, err := url.Parse(value)
if err != nil {
return fmt.Errorf("%q must be a valid URL: %w", value, err)
}
if val.Scheme == "" {
return fmt.Errorf("%q must be a valid URL: bad scheme", value)
}
if val.Host == "" {
return fmt.Errorf("%q must be a valid URL: bad host", value)
}

return nil
}

func validateIP(ip string) error {
if ip == "" {
return errors.New("IP address must be set")
Expand Down
124 changes: 124 additions & 0 deletions cmd/gateway/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,130 @@ func TestParseNamespacedResourceName(t *testing.T) {
}
}

func TestValidateQualifiedName(t *testing.T) {
tests := []struct {
name string
value string
expErr bool
}{
{
name: "valid",
value: "myName",
expErr: false,
},
{
name: "valid with hyphen",
value: "my-name",
expErr: false,
},
{
name: "valid with numbers",
value: "myName123",
expErr: false,
},
{
name: "valid with '/'",
value: "my/name",
expErr: false,
},
{
name: "valid with '.'",
value: "my.name",
expErr: false,
},
{
name: "empty",
value: "",
expErr: true,
},
{
name: "invalid character '$'",
value: "myName$",
expErr: true,
},
{
name: "invalid character '^'",
value: "my^Name",
expErr: true,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)

err := validateQualifiedName(test.value)
if test.expErr {
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(err).ToNot(HaveOccurred())
}
})
}
}

func TestValidateURL(t *testing.T) {
tests := []struct {
name string
url string
expErr bool
}{
{
name: "valid",
url: "http://server.com",
expErr: false,
},
{
name: "valid https",
url: "https://server.com",
expErr: false,
},
{
name: "valid with port",
url: "http://server.com:8080",
expErr: false,
},
{
name: "valid with ip address",
url: "http://10.0.0.1",
expErr: false,
},
{
name: "valid with ip address and port",
url: "http://10.0.0.1:8080",
expErr: false,
},
{
name: "invalid scheme",
url: "http//server.com",
expErr: true,
},
{
name: "no scheme",
url: "server.com",
expErr: true,
},
{
name: "no domain",
url: "http://",
expErr: true,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

err := validateURL(tc.url)
if !tc.expErr {
g.Expect(err).ToNot(HaveOccurred())
} else {
g.Expect(err).To(HaveOccurred())
}
})
}
}

func TestValidateIP(t *testing.T) {
tests := []struct {
name string
Expand Down
4 changes: 4 additions & 0 deletions deploy/helm-chart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,10 @@ 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.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. | |
| `nginx.usage.insecureSkipVerify` | Disable client verification of the NGINX Plus usage reporting server certificate. | false |
| `nginx.lifecycle` | The `lifecycle` of the nginx container. | {} |
| `nginx.extraVolumeMounts` | Extra `volumeMounts` for the nginx container. | {} |
| `terminationGracePeriodSeconds` | The termination grace period of the NGINX Gateway Fabric pod. | 30 |
Expand Down
12 changes: 12 additions & 0 deletions deploy/helm-chart/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ spec:
{{- if .Values.nginxGateway.gwAPIExperimentalFeatures.enable }}
- --gateway-api-experimental-features
{{- end }}
{{- if .Values.nginx.usage.secretName }}
- --usage-report-secret={{ .Values.nginx.usage.secretName }}
{{- end }}
{{- if .Values.nginx.usage.serverURL }}
- --usage-report-server-url={{ .Values.nginx.usage.serverURL }}
{{- end }}
{{- if .Values.nginx.usage.clusterName }}
- --usage-report-cluster-name={{ .Values.nginx.usage.clusterName }}
{{- end }}
{{- if .Values.nginx.usage.insecureSkipVerify }}
- --usage-report-skip-verify
{{- end }}
env:
- name: POD_IP
valueFrom:
Expand Down
Loading

0 comments on commit dec2f1b

Please sign in to comment.