Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support NGINX Plus usage reporting #1544

Merged
merged 6 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
sjberman marked this conversation as resolved.
Show resolved Hide resolved
- 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"
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
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
sjberman marked this conversation as resolved.
Show resolved Hide resolved
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")
pleshakov marked this conversation as resolved.
Show resolved Hide resolved
}

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(
sjberman marked this conversation as resolved.
Show resolved Hide resolved
&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.",
pleshakov marked this conversation as resolved.
Show resolved Hide resolved
)

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 {
pleshakov marked this conversation as resolved.
Show resolved Hide resolved
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
Loading