From 7a1c05b82e7878840ce6864cb1df10c6c63f5efc Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 18 May 2023 16:34:23 -0400 Subject: [PATCH 1/6] Support multiple commands in gateway binary This commit: - Changes the cmd package to support multiple commands in gateway binary. - Adds the root command, which simply prints help. - Adds control-plane command, which starts the control plane -- the previously available functionally of the gateway binary. - Adds provisioner command, currently not implemented. Needed by https://github.com/nginxinc/nginx-kubernetes-gateway/issues/634 --- cmd/gateway/commands.go | 135 ++++++++++++++ cmd/gateway/commands_test.go | 100 +++++++++++ cmd/gateway/main.go | 73 +------- cmd/gateway/main_test.go | 47 ----- cmd/gateway/setup.go | 122 ------------- cmd/gateway/setup_test.go | 266 ---------------------------- cmd/gateway/validation.go | 66 +++++++ cmd/gateway/validation_test.go | 163 +++++++++++++++++ deploy/manifests/nginx-gateway.yaml | 1 + docs/cli-args.md | 8 - docs/cli-help.md | 20 +++ docs/gateway-api-compatibility.md | 3 +- go.mod | 4 +- go.sum | 6 + 14 files changed, 502 insertions(+), 512 deletions(-) create mode 100644 cmd/gateway/commands.go create mode 100644 cmd/gateway/commands_test.go delete mode 100644 cmd/gateway/main_test.go delete mode 100644 cmd/gateway/setup.go delete mode 100644 cmd/gateway/setup_test.go create mode 100644 cmd/gateway/validation.go create mode 100644 cmd/gateway/validation_test.go delete mode 100644 docs/cli-args.md create mode 100644 docs/cli-help.md diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go new file mode 100644 index 0000000000..00d0244bdc --- /dev/null +++ b/cmd/gateway/commands.go @@ -0,0 +1,135 @@ +package main + +import ( + "errors" + "fmt" + "os" + + "github.com/spf13/cobra" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/config" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager" +) + +const ( + domain = "k8s-gateway.nginx.org" + gatewayClassFlag = "gatewayclass" + gatewayClassNameUsage = `The name of the GatewayClass resource. ` + + `Every NGINX Gateway must have a unique corresponding GatewayClass resource.` + gatewayCtrlNameFlag = "gateway-ctlr-name" + gatewayCtrlNameUsageFmt = `The name of the Gateway controller. ` + + `The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'` +) + +var ( + // Backing values for common cli flags shared among all subcommands + // The values are managed by the Root command. + gatewayCtlrName = stringValidatingValue{ + validator: validateGatewayControllerName, + } + + gatewayClassName = stringValidatingValue{ + validator: validateResourceName, + } +) + +// stringValidatingValue is a string flag value with custom validation logic. +// stringValidatingValue 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" +} + +func createRootCommand() *cobra.Command { + rootCmd := &cobra.Command{ + Use: "gateway", + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() + }, + } + + rootCmd.PersistentFlags().Var( + &gatewayCtlrName, + gatewayCtrlNameFlag, + fmt.Sprintf(gatewayCtrlNameUsageFmt, domain), + ) + utilruntime.Must(rootCmd.MarkPersistentFlagRequired(gatewayCtrlNameFlag)) + + rootCmd.PersistentFlags().Var( + &gatewayClassName, + gatewayClassFlag, + gatewayClassNameUsage, + ) + utilruntime.Must(rootCmd.MarkPersistentFlagRequired(gatewayClassFlag)) + + return rootCmd +} + +func createControlPlaneCommand() *cobra.Command { + return &cobra.Command{ + Use: "control-plane", + Short: "Start the control plane", + RunE: func(cmd *cobra.Command, args []string) error { + logger := zap.New() + logger.Info("Starting NGINX Kubernetes Gateway Control Plane", + "version", version, + "commit", commit, + "date", date, + ) + + podIP := os.Getenv("POD_IP") + if err := validateIP(podIP); err != nil { + return fmt.Errorf("error validating POD_IP environment variable: %w", err) + } + + conf := config.Config{ + GatewayCtlrName: gatewayCtlrName.value, + Logger: logger, + GatewayClassName: gatewayClassName.value, + PodIP: podIP, + } + + if err := manager.Start(conf); err != nil { + return fmt.Errorf("failed to start control loop: %w", err) + } + + return nil + }, + } +} + +func createProvisionerCommand() *cobra.Command { + return &cobra.Command{ + Use: "provisioner", + Short: "Start the provisioner", + Hidden: true, + RunE: func(cmd *cobra.Command, args []string) error { + logger := zap.New() + logger.Info("Starting NGINX Kubernetes Gateway Provisioner", + "version", version, + "commit", commit, + "date", date, + ) + + return errors.New("not implemented yet") + }, + } +} diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go new file mode 100644 index 0000000000..9af6ad0b42 --- /dev/null +++ b/cmd/gateway/commands_test.go @@ -0,0 +1,100 @@ +package main + +import ( + "io" + "testing" + + . "github.com/onsi/gomega" +) + +func TestRootCmdFlagValidation(t *testing.T) { + tests := []struct { + name string + expectedErrPrefix string + args []string + wantErr bool + }{ + { + name: "valid flags", + args: []string{ + "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + "--gatewayclass=nginx", + }, + wantErr: false, + }, + { + name: "gateway-ctlr-name is not set", + args: []string{ + "--gatewayclass=nginx", + }, + wantErr: true, + expectedErrPrefix: `required flag(s) "gateway-ctlr-name" not set`, + }, + { + name: "gateway-ctrl-name is set to empty string", + args: []string{ + "--gateway-ctlr-name=", + "--gatewayclass=nginx", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "" for "--gateway-ctlr-name" flag: must be set`, + }, + { + name: "gateway-ctlr-name is invalid", + args: []string{ + "--gateway-ctlr-name=nginx-gateway", + "--gatewayclass=nginx", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "nginx-gateway" for "--gateway-ctlr-name" flag: invalid format; ` + + "must be DOMAIN/PATH", + }, + { + name: "gatewayclass is not set", + args: []string{ + "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + }, + wantErr: true, + expectedErrPrefix: `required flag(s) "gatewayclass" not set`, + }, + { + name: "gatewayclass is set to empty string", + args: []string{ + "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + "--gatewayclass=", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "" for "--gatewayclass" flag: must be set`, + }, + { + name: "gatewayclass is invalid", + args: []string{ + "--gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway", + "--gatewayclass=@", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "@" for "--gatewayclass" flag: invalid format`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + rootCmd := createRootCommand() + // discard any output generated by cobra + rootCmd.SetOut(io.Discard) + rootCmd.SetErr(io.Discard) + + rootCmd.SetArgs(test.args) + err := rootCmd.Execute() + + if test.wantErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix)) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index 48cc0caa1c..051078b379 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -1,24 +1,8 @@ package main import ( - "errors" "fmt" - "net" "os" - - flag "github.com/spf13/pflag" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - - "github.com/nginxinc/nginx-kubernetes-gateway/internal/config" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager" -) - -const ( - domain = "k8s-gateway.nginx.org" - gatewayClassNameUsage = `The name of the GatewayClass resource. ` + - `Every NGINX Gateway must have a unique corresponding GatewayClass resource.` - gatewayCtrlNameUsageFmt = `The name of the Gateway controller. ` + - `The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'` ) var ( @@ -26,62 +10,17 @@ var ( version string commit string date string - - // Command-line flags - gatewayCtlrName = flag.String( - "gateway-ctlr-name", - "", - fmt.Sprintf(gatewayCtrlNameUsageFmt, domain), - ) - - gatewayClassName = flag.String("gatewayclass", "", gatewayClassNameUsage) - - // Environment variables - podIP = os.Getenv("POD_IP") ) -func validateIP(ip string) error { - if ip == "" { - return errors.New("IP address must be set") - } - if net.ParseIP(ip) == nil { - return fmt.Errorf("%q must be a valid IP address", ip) - } - - return nil -} - func main() { - flag.Parse() - - MustValidateArguments( - flag.CommandLine, - GatewayControllerParam(domain), - GatewayClassParam(), - ) - - if err := validateIP(podIP); err != nil { - fmt.Printf("error validating POD_IP environment variable: %v\n", err) - os.Exit(1) - } - - logger := zap.New() - conf := config.Config{ - GatewayCtlrName: *gatewayCtlrName, - Logger: logger, - GatewayClassName: *gatewayClassName, - PodIP: podIP, - } + rootCmd := createRootCommand() - logger.Info("Starting NGINX Kubernetes Gateway", - "version", version, - "commit", commit, - "date", date, - ) + rootCmd.AddCommand(createControlPlaneCommand()) + p := createProvisionerCommand() + rootCmd.AddCommand(p) - err := manager.Start(conf) - if err != nil { - logger.Error(err, "Failed to start control loop") + if err := rootCmd.Execute(); err != nil { + _, _ = fmt.Fprintln(os.Stderr, err) os.Exit(1) } } diff --git a/cmd/gateway/main_test.go b/cmd/gateway/main_test.go deleted file mode 100644 index d69d77c3b2..0000000000 --- a/cmd/gateway/main_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package main - -import ( - "testing" - - . "github.com/onsi/gomega" -) - -func TestValidateIP(t *testing.T) { - tests := []struct { - name string - expSubMsg string - ip string - expErr bool - }{ - { - name: "var not set", - ip: "", - expErr: true, - expSubMsg: "must be set", - }, - { - name: "invalid ip address", - ip: "invalid", - expErr: true, - expSubMsg: "must be a valid", - }, - { - name: "valid ip address", - ip: "1.2.3.4", - expErr: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - g := NewGomegaWithT(t) - - err := validateIP(tc.ip) - if !tc.expErr { - g.Expect(err).ToNot(HaveOccurred()) - } else { - g.Expect(err.Error()).To(ContainSubstring(tc.expSubMsg)) - } - }) - } -} diff --git a/cmd/gateway/setup.go b/cmd/gateway/setup.go deleted file mode 100644 index 4507c4f9d4..0000000000 --- a/cmd/gateway/setup.go +++ /dev/null @@ -1,122 +0,0 @@ -package main - -import ( - "errors" - "fmt" - "os" - "regexp" - "strings" - - flag "github.com/spf13/pflag" - "k8s.io/apimachinery/pkg/util/validation" - - // Adding a dummy import here to remind us to check the controllerNameRegex when we update the Gateway API version. - _ "sigs.k8s.io/gateway-api/apis/v1beta1" -) - -const ( - errTmpl = "failed validation - flag: '--%s' reason: '%s'\n" - // nolint:lll - // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/v0.7.0/apis/v1beta1/shared_types.go#L495 - controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` //nolint:lll -) - -type ( - Validator func(*flag.FlagSet) error - ValidatorContext struct { - V Validator - Key string - } -) - -func GatewayControllerParam(domain string) ValidatorContext { - name := "gateway-ctlr-name" - return ValidatorContext{ - Key: name, - V: func(flagset *flag.FlagSet) error { - param, err := flagset.GetString(name) - if err != nil { - return err - } - - if len(param) == 0 { - return errors.New("flag must be set") - } - - fields := strings.Split(param, "/") - l := len(fields) - if l < 2 { - return errors.New("invalid format; must be DOMAIN/PATH") - } - - if fields[0] != domain { - return fmt.Errorf("invalid domain: %s; domain must be: %s", fields[0], domain) - } - - return validateControllerName(param) - }, - } -} - -func validateControllerName(name string) error { - re := regexp.MustCompile(controllerNameRegex) - if !re.MatchString(name) { - return fmt.Errorf("invalid gateway controller name: %s; expected format is DOMAIN/PATH", name) - } - return nil -} - -func GatewayClassParam() ValidatorContext { - name := "gatewayclass" - return ValidatorContext{ - Key: name, - V: func(flagset *flag.FlagSet) error { - param, err := flagset.GetString(name) - if err != nil { - return err - } - - if len(param) == 0 { - return errors.New("flag must be set") - } - - // used by Kubernetes to validate resource names - messages := validation.IsDNS1123Subdomain(param) - if len(messages) > 0 { - msg := strings.Join(messages, "; ") - return fmt.Errorf("invalid format: %s", msg) - } - - return nil - }, - } -} - -func ValidateArguments(flagset *flag.FlagSet, validators ...ValidatorContext) []string { - var msgs []string - for _, v := range validators { - if flagset.Lookup(v.Key) != nil { - err := v.V(flagset) - if err != nil { - msgs = append(msgs, fmt.Sprintf(errTmpl, v.Key, err.Error())) - } - } - } - - return msgs -} - -func MustValidateArguments(flagset *flag.FlagSet, validators ...ValidatorContext) { - msgs := ValidateArguments(flagset, validators...) - if msgs != nil { - for i := range msgs { - fmt.Fprintf(os.Stderr, "%s", msgs[i]) - } - fmt.Fprintln(os.Stderr, "") - - fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) - flag.PrintDefaults() - - os.Exit(1) - } -} diff --git a/cmd/gateway/setup_test.go b/cmd/gateway/setup_test.go deleted file mode 100644 index 78a65eec89..0000000000 --- a/cmd/gateway/setup_test.go +++ /dev/null @@ -1,266 +0,0 @@ -package main_test - -import ( - "errors" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - flag "github.com/spf13/pflag" - - . "github.com/nginxinc/nginx-kubernetes-gateway/cmd/gateway" -) - -func MockValidator(name string, called *int, succeed bool) ValidatorContext { - return ValidatorContext{ - Key: name, - V: func(_ *flag.FlagSet) error { - *called++ - - if !succeed { - return errors.New("mock error") - } - return nil - }, - } -} - -var _ = Describe("Main Setup", func() { - Describe("Generic Validator", func() { - var mockFlags *flag.FlagSet - BeforeEach(func() { - mockFlags = flag.NewFlagSet("mock", flag.PanicOnError) - _ = mockFlags.String("validator-1", "", "validator-1") - _ = mockFlags.String("validator-2", "", "validator-2") - _ = mockFlags.String("validator-3", "", "validator-3") - err := mockFlags.Parse([]string{}) - Expect(err).ToNot(HaveOccurred()) - }) - AfterEach(func() { - mockFlags = nil - }) - It("should call all validators", func() { - var called int - table := []struct { - Contexts []ValidatorContext - ExpectedCalls int - Success bool - }{ - { - Contexts: []ValidatorContext{}, - ExpectedCalls: 0, - Success: true, - }, - { - Contexts: []ValidatorContext{ - MockValidator("no-flag-set", &called, true), - }, - ExpectedCalls: 0, - Success: true, - }, - { - Contexts: []ValidatorContext{ - MockValidator("validator-1", &called, true), - }, - ExpectedCalls: 1, - Success: true, - }, - { - Contexts: []ValidatorContext{ - MockValidator("no-flag-set", &called, true), - MockValidator("validator-1", &called, true), - }, - ExpectedCalls: 1, - Success: true, - }, - { - Contexts: []ValidatorContext{ - MockValidator("validator-1", &called, true), - MockValidator("validator-2", &called, true), - }, - ExpectedCalls: 2, - Success: true, - }, - { - Contexts: []ValidatorContext{ - MockValidator("validator-1", &called, true), - MockValidator("validator-2", &called, true), - MockValidator("validator-3", &called, true), - }, - ExpectedCalls: 3, - Success: true, - }, - { - Contexts: []ValidatorContext{ - MockValidator("validator-1", &called, false), - MockValidator("validator-2", &called, true), - MockValidator("validator-3", &called, true), - }, - ExpectedCalls: 3, - Success: false, - }, - { - Contexts: []ValidatorContext{ - MockValidator("validator-1", &called, true), - MockValidator("validator-2", &called, true), - MockValidator("validator-3", &called, false), - }, - ExpectedCalls: 3, - Success: false, - }, - } - - for i := range table { - called = 0 - msgs := ValidateArguments(mockFlags, table[i].Contexts...) - Expect(msgs == nil).To(Equal(table[i].Success)) - Expect(called).To(Equal(table[i].ExpectedCalls)) - } - }) // should call all validators - }) // Generic Validator - - Describe("CLI argument validation", func() { - type testCase struct { - ValidatorContext ValidatorContext - Flag string - Value string - ExpError bool - } - - const ( - expectError = true - expectSuccess = false - ) - - var mockFlags *flag.FlagSet - - tester := func(t testCase) { - err := mockFlags.Set(t.Flag, t.Value) - Expect(err).ToNot(HaveOccurred()) - - err = t.ValidatorContext.V(mockFlags) - - if t.ExpError { - Expect(err).To(HaveOccurred()) - } else { - Expect(err).ToNot(HaveOccurred()) - } - } - runner := func(table []testCase) { - for i := range table { - tester(table[i]) - } - } - - Describe("gateway-ctlr-name validation", func() { - prepareTestCase := func(value string, expError bool) testCase { - return testCase{ - Flag: "gateway-ctlr-name", - Value: value, - ValidatorContext: GatewayControllerParam("k8s-gateway.nginx.org"), - ExpError: expError, - } - } - - BeforeEach(func() { - mockFlags = flag.NewFlagSet("mock", flag.PanicOnError) - _ = mockFlags.String("gateway-ctlr-name", "", "mock gateway-ctlr-name") - err := mockFlags.Parse([]string{}) - Expect(err).ToNot(HaveOccurred()) - }) - AfterEach(func() { - mockFlags = nil - }) - - It("should parse valid gateway-ctlr-name", func() { - table := []testCase{ - prepareTestCase( - "k8s-gateway.nginx.org/my-gateway", - expectSuccess, - ), - prepareTestCase( - "k8s-gateway.nginx.org/nginx-gateway/my-gateway", - expectSuccess, - ), - prepareTestCase( - "k8s-gateway.nginx.org/nginx-gateway/my-gateway/v1", - expectSuccess, - ), - } - runner(table) - }) // should parse valid gateway-ctlr-name - - It("should fail with too few path elements", func() { - table := []testCase{ - prepareTestCase( - "k8s-gateway.nginx.org", - expectError, - ), - prepareTestCase( - "k8s-gateway.nginx.org/", - expectError, - ), - } - - runner(table) - }) // should fail with too few path elements - - It("should verify constraints", func() { - table := []testCase{ - prepareTestCase( - // bad domain - "invalid-domain/my-gateway", - expectError, - ), - prepareTestCase( - // bad domain - "/default/my-gateway", - expectError, - ), - prepareTestCase( - // bad name - "k8s-gateway.nginx.org/", - expectError, - ), - } - - runner(table) - }) // should verify constraints - }) // gateway-ctlr-name validation - - Describe("gatewayclass validation", func() { - prepareTestCase := func(value string, expError bool) testCase { - return testCase{ - Flag: "gatewayclass", - Value: value, - ValidatorContext: GatewayClassParam(), - ExpError: expError, - } - } - - BeforeEach(func() { - mockFlags = flag.NewFlagSet("mock", flag.PanicOnError) - _ = mockFlags.String("gatewayclass", "", "mock gatewayclass") - err := mockFlags.Parse([]string{}) - Expect(err).ToNot(HaveOccurred()) - }) - AfterEach(func() { - mockFlags = nil - }) - - It("should succeed on valid name", func() { - t := prepareTestCase( - "nginx", - expectSuccess, - ) - tester(t) - }) // should succeed on valid name - - It("should fail with invalid name", func() { - t := prepareTestCase( - "$nginx", - expectError) - tester(t) - }) // should fail with invalid name - }) // gatewayclass validation - }) // CLI argument validation -}) // end Main diff --git a/cmd/gateway/validation.go b/cmd/gateway/validation.go new file mode 100644 index 0000000000..2a0ff4014e --- /dev/null +++ b/cmd/gateway/validation.go @@ -0,0 +1,66 @@ +package main + +import ( + "errors" + "fmt" + "net" + "regexp" + "strings" + + "k8s.io/apimachinery/pkg/util/validation" +) + +const ( + // nolint:lll + // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/v0.6.2/apis/v1beta1/shared_types.go#L495 + controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` //nolint:lll +) + +func validateGatewayControllerName(value string) error { + if len(value) == 0 { + return errors.New("must be set") + } + + fields := strings.Split(value, "/") + l := len(fields) + if l < 2 { + return errors.New("invalid format; must be DOMAIN/PATH") + } + + if fields[0] != domain { + return fmt.Errorf("invalid domain: %s; domain must be: %s", fields[0], domain) + } + + re := regexp.MustCompile(controllerNameRegex) + if !re.MatchString(value) { + return fmt.Errorf("invalid gateway controller name: %s; expected format is DOMAIN/PATH", value) + } + + return nil +} + +func validateResourceName(value string) error { + if len(value) == 0 { + return errors.New("must be set") + } + + // used by Kubernetes to validate resource names + messages := validation.IsDNS1123Subdomain(value) + if len(messages) > 0 { + msg := strings.Join(messages, "; ") + return fmt.Errorf("invalid format: %s", msg) + } + + return nil +} + +func validateIP(ip string) error { + if ip == "" { + return errors.New("IP address must be set") + } + if net.ParseIP(ip) == nil { + return fmt.Errorf("%q must be a valid IP address", ip) + } + + return nil +} diff --git a/cmd/gateway/validation_test.go b/cmd/gateway/validation_test.go new file mode 100644 index 0000000000..e8d5e8648c --- /dev/null +++ b/cmd/gateway/validation_test.go @@ -0,0 +1,163 @@ +package main + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestValidateGatewayControllerName(t *testing.T) { + tests := []struct { + name string + value string + expErr bool + }{ + { + name: "valid", + value: "k8s-gateway.nginx.org/nginx-gateway", + expErr: false, + }, + { + name: "valid - with subpath", + value: "k8s-gateway.nginx.org/nginx-gateway/my-gateway", + expErr: false, + }, + { + name: "valid - with complex subpath", + value: "k8s-gateway.nginx.org/nginx-gateway/my-gateway/v1", + expErr: false, + }, + { + name: "invalid - empty", + value: "", + expErr: true, + }, + { + name: "invalid - lacks path", + value: "k8s-gateway.nginx.org", + expErr: true, + }, + { + name: "invalid - lacks path, only slash is present", + value: "k8s-gateway.nginx.org/", + expErr: true, + }, + { + name: "invalid - invalid domain", + value: "invalid-domain/my-gateway", + expErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + err := validateGatewayControllerName(test.value) + + if test.expErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} + +func TestValidateResourceName(t *testing.T) { + tests := []struct { + name string + value string + expErr bool + }{ + { + name: "valid", + value: "mygateway", + expErr: false, + }, + { + name: "valid - with dash", + value: "my-gateway", + expErr: false, + }, + { + name: "valid - with dash and numbers", + value: "my-gateway-123", + expErr: false, + }, + { + name: "invalid - empty", + value: "", + expErr: true, + }, + { + name: "invalid - invalid character '/'", + value: "my/gateway", + expErr: true, + }, + { + name: "invalid - invalid character '_'", + value: "my_gateway", + expErr: true, + }, + { + name: "invalid - invalid character '@'", + value: "my@gateway", + expErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + err := validateResourceName(test.value) + + if test.expErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} + +func TestValidateIP(t *testing.T) { + tests := []struct { + name string + expSubMsg string + ip string + expErr bool + }{ + { + name: "var not set", + ip: "", + expErr: true, + expSubMsg: "must be set", + }, + { + name: "invalid ip address", + ip: "invalid", + expErr: true, + expSubMsg: "must be a valid", + }, + { + name: "valid ip address", + ip: "1.2.3.4", + expErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + err := validateIP(tc.ip) + if !tc.expErr { + g.Expect(err).ToNot(HaveOccurred()) + } else { + g.Expect(err.Error()).To(ContainSubstring(tc.expSubMsg)) + } + }) + } +} diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 9fb6a815ac..09e6cecd5d 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -119,6 +119,7 @@ spec: fieldRef: fieldPath: status.podIP args: + - control-plane - --gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - image: nginx:1.23 diff --git a/docs/cli-args.md b/docs/cli-args.md deleted file mode 100644 index a0d7675c5e..0000000000 --- a/docs/cli-args.md +++ /dev/null @@ -1,8 +0,0 @@ -# Command-line Arguments - -The table below describes the command-line arguments of the `gateway` binary from the `nginx-kubernetes-gateway` container. - -| Name | Type | Description | -|-|-|-| -|`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 `k8s-gateway.nginx.org`. | -|`gatewayclass`| `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. | diff --git a/docs/cli-help.md b/docs/cli-help.md new file mode 100644 index 0000000000..065dd18da2 --- /dev/null +++ b/docs/cli-help.md @@ -0,0 +1,20 @@ +# Command-line Help + +This document describes the commands available in the `gateway` binary of `nginx-kubernetes-gateway` container. + +## Control Plane + +This command starts the control plane. + +Usage: + +``` + gateway control-plane [flags] +``` + +Flags: + +| Name | Type | Description | +|-|-|-| +| `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 `k8s-gateway.nginx.org`. | +| `gatewayclass` | `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. | diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index d7b510020d..0c71c27239 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -34,7 +34,8 @@ For a description of each field, visit the [Gateway API documentation](https://g > Status: Partially supported. -NGINX Kubernetes Gateway supports only a single GatewayClass resource configured via `--gatewayclass` [cli argument](./cli-args.md). +NGINX Kubernetes Gateway supports only a single GatewayClass resource configured via `--gatewayclass` flag +of the [control-plane](./cli-help.md#control-plane) command. Fields: * `spec` diff --git a/go.mod b/go.mod index 8f10777e45..c0e6359648 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/maxbrunsfeld/counterfeiter/v6 v6.6.1 github.com/onsi/ginkgo/v2 v2.9.5 github.com/onsi/gomega v1.27.7 - github.com/spf13/pflag v1.0.5 + github.com/spf13/cobra v1.7.0 k8s.io/api v0.26.3 k8s.io/apimachinery v0.26.3 k8s.io/client-go v0.26.3 @@ -37,6 +37,7 @@ require ( github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 // indirect github.com/google/uuid v1.1.2 // indirect github.com/imdario/mergo v0.3.12 // indirect + github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/mailru/easyjson v0.7.6 // indirect @@ -49,6 +50,7 @@ require ( github.com/prometheus/client_model v0.3.0 // indirect github.com/prometheus/common v0.37.0 // indirect github.com/prometheus/procfs v0.8.0 // indirect + github.com/spf13/pflag v1.0.5 // indirect go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.7.0 // indirect go.uber.org/zap v1.24.0 // indirect diff --git a/go.sum b/go.sum index 91e15e9cbc..61f1c255bf 100644 --- a/go.sum +++ b/go.sum @@ -53,6 +53,7 @@ github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5P github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= +github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -173,6 +174,8 @@ github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1: github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU= github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA= +github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= +github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= @@ -256,10 +259,13 @@ github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1 github.com/prometheus/procfs v0.8.0 h1:ODq8ZFEaYeCaZOJlZZdJA2AbQR98dSHSM1KW/You5mo= github.com/prometheus/procfs v0.8.0/go.mod h1:z7EfXMXOkbkqb9IINtpCn86r/to3BnA0uaxHdg830/4= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= +github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sclevine/spec v1.4.0 h1:z/Q9idDcay5m5irkZ28M7PtQM4aOISzOpj4bUPkDee8= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= +github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= +github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= From acc274ba94dc28b744cfedb6a49de091d8a5ef2e Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 19 May 2023 13:30:04 -0400 Subject: [PATCH 2/6] Improve rootCmd init --- cmd/gateway/main.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index 051078b379..79c4f5d214 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -15,9 +15,10 @@ var ( func main() { rootCmd := createRootCommand() - rootCmd.AddCommand(createControlPlaneCommand()) - p := createProvisionerCommand() - rootCmd.AddCommand(p) + rootCmd.AddCommand( + createControlPlaneCommand(), + createProvisionerCommand(), + ) if err := rootCmd.Execute(); err != nil { _, _ = fmt.Fprintln(os.Stderr, err) From a883b58a7b58ced6dea56052b15741a76f5547ce Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 19 May 2023 13:31:13 -0400 Subject: [PATCH 3/6] Fix the link in the comment --- cmd/gateway/validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gateway/validation.go b/cmd/gateway/validation.go index 2a0ff4014e..157ce69944 100644 --- a/cmd/gateway/validation.go +++ b/cmd/gateway/validation.go @@ -12,7 +12,7 @@ import ( const ( // nolint:lll - // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/v0.6.2/apis/v1beta1/shared_types.go#L495 + // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/v0.7.0/apis/v1beta1/shared_types.go#L495 controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` //nolint:lll ) From c4b3a7ebb73c7c5472b39c138889e84b6de78370 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 19 May 2023 13:49:41 -0400 Subject: [PATCH 4/6] Fix a type --- cmd/gateway/commands_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 9af6ad0b42..a03a2107c2 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -31,7 +31,7 @@ func TestRootCmdFlagValidation(t *testing.T) { expectedErrPrefix: `required flag(s) "gateway-ctlr-name" not set`, }, { - name: "gateway-ctrl-name is set to empty string", + name: "gateway-ctlr-name is set to empty string", args: []string{ "--gateway-ctlr-name=", "--gatewayclass=nginx", From 4ac1df24859f4bb4ee127bc243f26fb3864ce287 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 19 May 2023 15:40:26 -0400 Subject: [PATCH 5/6] Rename commands --- cmd/gateway/commands.go | 14 +++++++------- cmd/gateway/main.go | 4 ++-- deploy/manifests/nginx-gateway.yaml | 2 +- docs/cli-help.md | 10 +++++++--- docs/gateway-api-compatibility.md | 6 ++++-- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 00d0244bdc..7c2ffe9c0f 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -83,13 +83,13 @@ func createRootCommand() *cobra.Command { return rootCmd } -func createControlPlaneCommand() *cobra.Command { +func createStaticModeCommand() *cobra.Command { return &cobra.Command{ - Use: "control-plane", - Short: "Start the control plane", + Use: "static-mode", + Short: "Configure NGINX in the scope of a single Gateway resource", RunE: func(cmd *cobra.Command, args []string) error { logger := zap.New() - logger.Info("Starting NGINX Kubernetes Gateway Control Plane", + logger.Info("Starting NGINX Kubernetes Gateway in static mode", "version", version, "commit", commit, "date", date, @@ -116,10 +116,10 @@ func createControlPlaneCommand() *cobra.Command { } } -func createProvisionerCommand() *cobra.Command { +func createProvisionerModeCommand() *cobra.Command { return &cobra.Command{ - Use: "provisioner", - Short: "Start the provisioner", + Use: "provisioner-mode", + Short: "Provision an NGINX Gateway Deployment per Gateway resource configured in the static mode", Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { logger := zap.New() diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index 79c4f5d214..12b2a89517 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -16,8 +16,8 @@ func main() { rootCmd := createRootCommand() rootCmd.AddCommand( - createControlPlaneCommand(), - createProvisionerCommand(), + createStaticModeCommand(), + createProvisionerModeCommand(), ) if err := rootCmd.Execute(); err != nil { diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 09e6cecd5d..b7e3be400d 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -119,7 +119,7 @@ spec: fieldRef: fieldPath: status.podIP args: - - control-plane + - static-mode - --gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - image: nginx:1.23 diff --git a/docs/cli-help.md b/docs/cli-help.md index 065dd18da2..31eb08cf2e 100644 --- a/docs/cli-help.md +++ b/docs/cli-help.md @@ -2,14 +2,18 @@ This document describes the commands available in the `gateway` binary of `nginx-kubernetes-gateway` container. -## Control Plane +## Static Mode -This command starts the control plane. +This command configures NGINX in the scope of a single Gateway resource. In case of multiple Gateway resources created +in the cluster, NGINX Kubernetes Gateway will use a deterministic conflict resolution strategy: it will choose the +oldest resource by creation timestamp. If the timestamps are equal, NGINX Kubernetes Gateway will choose the resource +that appears first in alphabetical order by “{namespace}/{name}”. We might support multiple Gateway resources. Please +share your use case with us if you're interested in that support. Usage: ``` - gateway control-plane [flags] + gateway static-mode [flags] ``` Flags: diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 0c71c27239..4d2ea97f47 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -35,7 +35,7 @@ For a description of each field, visit the [Gateway API documentation](https://g > Status: Partially supported. NGINX Kubernetes Gateway supports only a single GatewayClass resource configured via `--gatewayclass` flag -of the [control-plane](./cli-help.md#control-plane) command. +of the [static-mode](./cli-help.md#static-mode) command. Fields: * `spec` @@ -49,7 +49,9 @@ Fields: > Status: Partially supported. -NGINX Kubernetes Gateway supports only a single Gateway resource. The Gateway resource must reference NGINX Kubernetes Gateway's corresponding GatewayClass. In case of multiple Gateway resources created in the cluster, NGINX Kubernetes Gateway will use a deterministic conflict resolution strategy: it will choose the oldest resource by creation timestamp. If the timestamps are equal, NGINX Kubernetes Gateway will choose the resource that appears first in alphabetical order by “{namespace}/{name}”. We might support multiple Gateway resources. Please share your use case with us if you're interested in that support. +NGINX Kubernetes Gateway supports only a single Gateway resource. The Gateway resource must reference NGINX Kubernetes Gateway's corresponding GatewayClass. +In case of multiple Gateway resources created in the cluster, NGINX Kubernetes Gateway will use a deterministic conflict resolution strategy. +See [static-mode](./cli-help.md#static-mode) command for more info. Fields: * `spec` From dc306a8e1aee9f412b2e64640fe1a347d46abfba Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 19 May 2023 16:23:28 -0400 Subject: [PATCH 6/6] Update doc for provisioner --- cmd/gateway/commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 7c2ffe9c0f..b1b7459477 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -119,7 +119,7 @@ func createStaticModeCommand() *cobra.Command { func createProvisionerModeCommand() *cobra.Command { return &cobra.Command{ Use: "provisioner-mode", - Short: "Provision an NGINX Gateway Deployment per Gateway resource configured in the static mode", + Short: "Provision a static-mode NGINX Gateway Deployment per Gateway resource", Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { logger := zap.New()