diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 29a5d78e6..498780db1 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -39,68 +39,20 @@ var ( } // Backing values for static subcommand cli flags. - updateGCStatus bool - disableMetrics bool - metricsListenPort int - metricsSecure bool + updateGCStatus bool + disableMetrics bool + metricsSecure bool + metricsListenPort = intValidatingValue{ + validator: validatePort, + value: 9113, + } gateway = namespacedNameValue{} configName = stringValidatingValue{ validator: validateResourceName, } ) -// stringValidatingValue is a string flag value with custom validation logic. -// it implements the pflag.Value interface. -type stringValidatingValue struct { - validator func(v string) error - value string -} - -func (v *stringValidatingValue) String() string { - return v.value -} - -func (v *stringValidatingValue) Set(param string) error { - if err := v.validator(param); err != nil { - return err - } - v.value = param - return nil -} - -func (v *stringValidatingValue) Type() string { - return "string" -} - -// namespacedNameValue is a string flag value that represents a namespaced name. -// it implements the pflag.Value interface. -type namespacedNameValue struct { - value types.NamespacedName -} - -func (v *namespacedNameValue) String() string { - if (v.value == types.NamespacedName{}) { - // if we don't do that, the default value in the help message will be printed as "/" - return "" - } - return v.value.String() -} - -func (v *namespacedNameValue) Set(param string) error { - nsname, err := parseNamespacedResourceName(param) - if err != nil { - return err - } - - v.value = nsname - return nil -} - -func (v *namespacedNameValue) Type() string { - return "string" -} - func createRootCommand() *cobra.Command { rootCmd := &cobra.Command{ Use: "gateway", @@ -158,11 +110,8 @@ func createStaticModeCommand() *cobra.Command { metricsConfig := config.MetricsConfig{} if !disableMetrics { - if err := validatePort(metricsListenPort); err != nil { - return fmt.Errorf("error validating metrics listen port: %w", err) - } metricsConfig.Enabled = true - metricsConfig.Port = metricsListenPort + metricsConfig.Port = metricsListenPort.value metricsConfig.Secure = metricsSecure } @@ -219,10 +168,9 @@ func createStaticModeCommand() *cobra.Command { "Disable exposing metrics in the Prometheus format.", ) - cmd.Flags().IntVar( + cmd.Flags().Var( &metricsListenPort, "metrics-port", - 9113, "Set the port where the metrics are exposed. Format: [1023 - 65535]", ) diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 59c9bd7bb..0c352fda7 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -118,6 +118,9 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { "--gateway=nginx-gateway/nginx", "--config=nginx-gateway-config", "--update-gatewayclass-status=true", + "--metrics-port=9114", + "--metrics-disable", + "--metrics-secure-serving", }, wantErr: false, }, @@ -175,6 +178,42 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { wantErr: true, expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`, }, + { + name: "metrics-port is invalid type", + args: []string{ + "--metrics-port=invalid", // not an int + }, + wantErr: true, + expectedErrPrefix: `invalid argument "invalid" for "--metrics-port" flag: failed to parse int value:` + + ` strconv.ParseInt: parsing "invalid": invalid syntax`, + }, + { + name: "metrics-port is outside of range", + args: []string{ + "--metrics-port=999", // outside of range + }, + wantErr: true, + expectedErrPrefix: `invalid argument "999" for "--metrics-port" flag:` + + ` port outside of valid port range [1024 - 65535]: 999`, + }, + { + name: "metrics-disable is not a bool", + args: []string{ + "--metrics-disable=999", // not a bool + }, + wantErr: true, + expectedErrPrefix: `invalid argument "999" for "--metrics-disable" flag: strconv.ParseBool:` + + ` parsing "999": invalid syntax`, + }, + { + name: "metrics-secure-serving is not a bool", + args: []string{ + "--metrics-secure-serving=999", // not a bool + }, + wantErr: true, + expectedErrPrefix: `invalid argument "999" for "--metrics-secure-serving" flag: strconv.ParseBool:` + + ` parsing "999": invalid syntax`, + }, } for _, test := range tests { diff --git a/cmd/gateway/validating_types.go b/cmd/gateway/validating_types.go new file mode 100644 index 000000000..42d24782c --- /dev/null +++ b/cmd/gateway/validating_types.go @@ -0,0 +1,86 @@ +package main + +import ( + "fmt" + "strconv" + + "k8s.io/apimachinery/pkg/types" +) + +// stringValidatingValue is a string flag value with custom validation logic. +// it implements the pflag.Value interface. +type stringValidatingValue struct { + validator func(v string) error + value string +} + +func (v *stringValidatingValue) String() string { + return v.value +} + +func (v *stringValidatingValue) Set(param string) error { + if err := v.validator(param); err != nil { + return err + } + v.value = param + return nil +} + +func (v *stringValidatingValue) Type() string { + return "string" +} + +type intValidatingValue struct { + validator func(v int) error + value int +} + +func (v *intValidatingValue) String() string { + return strconv.Itoa(v.value) +} + +func (v *intValidatingValue) Set(param string) error { + intVal, err := strconv.ParseInt(param, 10, 32) + if err != nil { + return fmt.Errorf("failed to parse int value: %w", err) + } + + if err := v.validator(int(intVal)); err != nil { + return err + } + + v.value = int(intVal) + return nil +} + +func (v *intValidatingValue) Type() string { + return "int" +} + +// namespacedNameValue is a string flag value that represents a namespaced name. +// it implements the pflag.Value interface. +type namespacedNameValue struct { + value types.NamespacedName +} + +func (v *namespacedNameValue) String() string { + if (v.value == types.NamespacedName{}) { + // if we don't do that, the default value in the help message will be printed as "/" + return "" + } + return v.value.String() +} + +func (v *namespacedNameValue) Set(param string) error { + nsname, err := parseNamespacedResourceName(param) + if err != nil { + return err + } + + v.value = nsname + return nil +} + +func (v *namespacedNameValue) Type() string { + return "string" +} diff --git a/docs/architecture.md b/docs/architecture.md index 8164d25e6..3eabd49f6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -96,7 +96,7 @@ parentheses. To enhance readability, the suffix "process" has been omitted from 1. (HTTPS) *NKG* reads the *Kubernetes API* to get the latest versions of the resources in the cluster and writes to the API to update the handled resources' statuses and emit events. -2. (HTTP) *Prometheus* fetches the `controller-runtime` and NGINX metrics via an HTTP endpoint that *NKG* exposes. +2. (HTTP, HTTPS) *Prometheus* fetches the `controller-runtime` and NGINX metrics via an HTTP endpoint that *NKG* exposes. The default is :9113/metrics. Note: Prometheus is not required by NKG, the endpoint can be turned off. 3. (File I/O) - Write: *NKG* generates NGINX *configuration* based on the cluster resources and writes them as `.conf` files to the diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 7e8768ea7..8280d3acd 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -70,6 +70,7 @@ func (m *ManagerImpl) Reload(ctx context.Context) error { return nil } +// EnsureNginxRunning ensures NGINX is running by locating the main process. func EnsureNginxRunning(ctx context.Context) error { if _, err := findMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) diff --git a/internal/mode/static/state/graph/gateway_listener_test.go b/internal/mode/static/state/graph/gateway_listener_test.go index 9b3073f4f..f2c1e3bba 100644 --- a/internal/mode/static/state/graph/gateway_listener_test.go +++ b/internal/mode/static/state/graph/gateway_listener_test.go @@ -114,7 +114,7 @@ func TestValidateHTTPSListener(t *testing.T) { l: v1beta1.Listener{ Port: 9113, TLS: &v1beta1.GatewayTLSConfig{ - Mode: helpers.GetTLSModePointer(v1beta1.TLSModeTerminate), + Mode: helpers.GetPointer(v1beta1.TLSModeTerminate), CertificateRefs: []v1beta1.SecretObjectReference{validSecretRef}, }, },