diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index b1b7459477..d07d1e993d 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -6,6 +6,7 @@ import ( "os" "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -36,7 +37,7 @@ var ( ) // stringValidatingValue is a string flag value with custom validation logic. -// stringValidatingValue implements the pflag.Value interface. +// it implements the pflag.Value interface. type stringValidatingValue struct { validator func(v string) error value string @@ -58,6 +59,34 @@ 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", @@ -84,7 +113,13 @@ func createRootCommand() *cobra.Command { } func createStaticModeCommand() *cobra.Command { - return &cobra.Command{ + const gatewayFlag = "gateway" + + // flag values + gateway := namespacedNameValue{} + var updateGCStatus bool + + cmd := &cobra.Command{ Use: "static-mode", Short: "Configure NGINX in the scope of a single Gateway resource", RunE: func(cmd *cobra.Command, args []string) error { @@ -100,11 +135,18 @@ func createStaticModeCommand() *cobra.Command { return fmt.Errorf("error validating POD_IP environment variable: %w", err) } + var gwNsName *types.NamespacedName + if cmd.Flags().Changed(gatewayFlag) { + gwNsName = &gateway.value + } + conf := config.Config{ - GatewayCtlrName: gatewayCtlrName.value, - Logger: logger, - GatewayClassName: gatewayClassName.value, - PodIP: podIP, + GatewayCtlrName: gatewayCtlrName.value, + Logger: logger, + GatewayClassName: gatewayClassName.value, + PodIP: podIP, + GatewayNsName: gwNsName, + UpdateGatewayClassStatus: updateGCStatus, } if err := manager.Start(conf); err != nil { @@ -114,6 +156,25 @@ func createStaticModeCommand() *cobra.Command { return nil }, } + + cmd.Flags().Var( + &gateway, + gatewayFlag, + "The namespaced name of the Gateway resource to use. "+ + "Must be of the form: NAMESPACE/NAME. "+ + "If not specified, the control plane will process all Gateways for the configured GatewayClass. "+ + "However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are "+ + "equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.", + ) + + cmd.Flags().BoolVar( + &updateGCStatus, + "update-gatewayclass-status", + true, + "Update the status of the GatewayClass resource.", + ) + + return cmd } func createProvisionerModeCommand() *cobra.Command { diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index a03a2107c2..425f66a2e1 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -5,15 +5,40 @@ import ( "testing" . "github.com/onsi/gomega" + "github.com/spf13/cobra" ) +type flagTestCase struct { + name string + expectedErrPrefix string + args []string + wantErr bool +} + +func testFlag(t *testing.T, cmd *cobra.Command, test flagTestCase) { + g := NewGomegaWithT(t) + // discard any output generated by cobra + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + // override RunE to avoid executing the command + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return nil + } + + cmd.SetArgs(test.args) + err := cmd.Execute() + + if test.wantErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } +} + func TestRootCmdFlagValidation(t *testing.T) { - tests := []struct { - name string - expectedErrPrefix string - args []string - wantErr bool - }{ + tests := []flagTestCase{ { name: "valid flags", args: []string{ @@ -79,22 +104,66 @@ func TestRootCmdFlagValidation(t *testing.T) { 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) + testFlag(t, rootCmd, test) + }) + } +} - rootCmd.SetArgs(test.args) - err := rootCmd.Execute() +func TestStaticModeCmdFlagValidation(t *testing.T) { + tests := []flagTestCase{ + { + name: "valid flags", + args: []string{ + "--gateway=nginx-gateway/nginx", + "--update-gatewayclass-status=true", + }, + wantErr: false, + }, + { + name: "valid flags, not set", + args: nil, + wantErr: false, + }, + { + name: "gateway is set to empty string", + args: []string{ + "--gateway=", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "" for "--gateway" flag: must be set`, + }, + { + name: "gateway is invalid", + args: []string{ + "--gateway=nginx-gateway", // no namespace + }, + wantErr: true, + expectedErrPrefix: `invalid argument "nginx-gateway" for "--gateway" flag: invalid format; ` + + "must be NAMESPACE/NAME", + }, + { + name: "update-gatewayclass-status is set to empty string", + args: []string{ + "--update-gatewayclass-status=", + }, + wantErr: true, + expectedErrPrefix: `invalid argument "" for "--update-gatewayclass-status" flag: strconv.ParseBool`, + }, + { + name: "update-gatewayclass-status is invalid", + args: []string{ + "--update-gatewayclass-status=invalid", // not a boolean + }, + wantErr: true, + expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`, + }, + } - if test.wantErr { - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix)) - } else { - g.Expect(err).ToNot(HaveOccurred()) - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cmd := createStaticModeCommand() + testFlag(t, cmd, test) }) } } diff --git a/cmd/gateway/validation.go b/cmd/gateway/validation.go index 157ce69944..6540252a9f 100644 --- a/cmd/gateway/validation.go +++ b/cmd/gateway/validation.go @@ -7,6 +7,7 @@ import ( "regexp" "strings" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" ) @@ -54,6 +55,40 @@ func validateResourceName(value string) error { return nil } +func validateNamespaceName(value string) error { + // used by Kubernetes to validate resource namespace names + messages := validation.IsDNS1123Label(value) + if len(messages) > 0 { + msg := strings.Join(messages, "; ") + return fmt.Errorf("invalid format: %s", msg) + } + + return nil +} + +func parseNamespacedResourceName(value string) (types.NamespacedName, error) { + if value == "" { + return types.NamespacedName{}, errors.New("must be set") + } + + parts := strings.Split(value, "/") + if len(parts) != 2 { + return types.NamespacedName{}, errors.New("invalid format; must be NAMESPACE/NAME") + } + + if err := validateNamespaceName(parts[0]); err != nil { + return types.NamespacedName{}, fmt.Errorf("invalid namespace name: %w", err) + } + if err := validateResourceName(parts[1]); err != nil { + return types.NamespacedName{}, fmt.Errorf("invalid resource name: %w", err) + } + + return types.NamespacedName{ + Namespace: parts[0], + Name: parts[1], + }, nil +} + func validateIP(ip string) error { if ip == "" { return errors.New("IP address must be set") diff --git a/cmd/gateway/validation_test.go b/cmd/gateway/validation_test.go index e8d5e8648c..8836fa5b18 100644 --- a/cmd/gateway/validation_test.go +++ b/cmd/gateway/validation_test.go @@ -4,6 +4,7 @@ import ( "testing" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" ) func TestValidateGatewayControllerName(t *testing.T) { @@ -81,8 +82,13 @@ func TestValidateResourceName(t *testing.T) { expErr: false, }, { - name: "valid - with dash and numbers", - value: "my-gateway-123", + name: "valid - with dot", + value: "my.gateway", + expErr: false, + }, + { + name: "valid - with numbers", + value: "mygateway123", expErr: false, }, { @@ -122,6 +128,133 @@ func TestValidateResourceName(t *testing.T) { } } +func TestValidateNamespaceName(t *testing.T) { + tests := []struct { + name string + value string + expErr bool + }{ + { + name: "valid", + value: "mynamespace", + expErr: false, + }, + { + name: "valid - with dash", + value: "my-namespace", + expErr: false, + }, + { + name: "valid - with numbers", + value: "mynamespace123", + expErr: false, + }, + { + name: "invalid - empty", + value: "", + expErr: true, + }, + { + name: "invalid - invalid character '.'", + value: "my.namespace", + expErr: true, + }, + { + name: "invalid - invalid character '/'", + value: "my/namespace", + expErr: true, + }, + { + name: "invalid - invalid character '_'", + value: "my_namespace", + expErr: true, + }, + { + name: "invalid - invalid character '@'", + value: "my@namespace", + expErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + err := validateNamespaceName(test.value) + + if test.expErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} + +func TestParseNamespacedResourceName(t *testing.T) { + tests := []struct { + name string + value string + expectedErrPrefix string + expectedNsName types.NamespacedName + expectErr bool + }{ + { + name: "valid", + value: "test/my-gateway", + expectedNsName: types.NamespacedName{ + Namespace: "test", + Name: "my-gateway", + }, + expectErr: false, + }, + { + name: "empty", + value: "", + expectedNsName: types.NamespacedName{}, + expectErr: true, + expectedErrPrefix: "must be set", + }, + { + name: "wrong number of parts", + value: "test", + expectedNsName: types.NamespacedName{}, + expectErr: true, + expectedErrPrefix: "invalid format; must be NAMESPACE/NAME", + }, + { + name: "invalid namespace", + value: "t@st/my-gateway", + expectedNsName: types.NamespacedName{}, + expectErr: true, + expectedErrPrefix: "invalid namespace name", + }, + { + name: "invalid name", + value: "test/my-g@teway", + expectedNsName: types.NamespacedName{}, + expectErr: true, + expectedErrPrefix: "invalid resource name", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + nsName, err := parseNamespacedResourceName(test.value) + + if test.expectErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix)) + } else { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(nsName).To(Equal(test.expectedNsName)) + } + }) + } +} + func TestValidateIP(t *testing.T) { tests := []struct { name string diff --git a/docs/cli-help.md b/docs/cli-help.md index 31eb08cf2e..41387ed114 100644 --- a/docs/cli-help.md +++ b/docs/cli-help.md @@ -4,11 +4,7 @@ This document describes the commands available in the `gateway` binary of `nginx ## Static Mode -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. +This command configures NGINX in the scope of a single Gateway resource. Usage: @@ -22,3 +18,5 @@ Flags: |-|-|-| | `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. | +| `gateway` | `string` | The namespaced name of the Gateway resource to use. Must be of the form: `NAMESPACE/NAME`. If not specified, the control plane will process all Gateways for the configured GatewayClass. However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}. | +| `update-gatewayclass-status` | `bool` | Update the status of the GatewayClass resource. (default true) | diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 4d2ea97f47..54b7e2d899 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -50,7 +50,6 @@ 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. See [static-mode](./cli-help.md#static-mode) command for more info. Fields: diff --git a/internal/config/config.go b/internal/config/config.go index 7de2f3e0e0..0bcd1c09ff 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,9 +10,11 @@ type Config struct { Logger logr.Logger // GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use. // The Gateway will ignore all other Gateway resources. - GatewayNsName types.NamespacedName + GatewayNsName *types.NamespacedName // GatewayClassName is the name of the GatewayClass resource that the Gateway will use. GatewayClassName string // PodIP is the IP address of this Pod. PodIP string + // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. + UpdateGatewayClassStatus bool } diff --git a/internal/controller/reconciler.go b/internal/controller/reconciler.go index 7274e1e3d9..b2a87997ea 100644 --- a/internal/controller/reconciler.go +++ b/internal/controller/reconciler.go @@ -16,7 +16,7 @@ import ( // NamespacedNameFilterFunc is a function that returns true if the resource should be processed by the reconciler. // If the function returns false, the reconciler will log the returned string. -type NamespacedNameFilterFunc func(nsname types.NamespacedName) (bool, string) +type NamespacedNameFilterFunc func(nsname types.NamespacedName) (shouldProcess bool, msg string) // ReconcilerConfig is the configuration for the reconciler. type ReconcilerConfig struct { @@ -67,7 +67,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco logger.Info("Reconciling the resource") if r.cfg.NamespacedNameFilter != nil { - if allow, msg := r.cfg.NamespacedNameFilter(req.NamespacedName); !allow { + if shouldProcess, msg := r.cfg.NamespacedNameFilter(req.NamespacedName); !shouldProcess { logger.Info(msg) return reconcile.Result{}, nil } diff --git a/internal/controller/register_test.go b/internal/controller/register_test.go index c60f6599c5..0a60fef046 100644 --- a/internal/controller/register_test.go +++ b/internal/controller/register_test.go @@ -8,8 +8,9 @@ import ( . "github.com/onsi/gomega" "github.com/onsi/gomega/gcustom" - "github.com/onsi/gomega/types" + gtypes "github.com/onsi/gomega/types" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -17,7 +18,6 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller/controllerfakes" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/filter" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/predicate" ) @@ -81,12 +81,15 @@ func TestRegister(t *testing.T) { } objectType := &v1beta1.HTTPRoute{} - namespacedNameFilter := filter.CreateFilterForGatewayClass("test") + nsNameFilter := func(nsname types.NamespacedName) (bool, string) { + return true, "" + } + fieldIndexes := index.CreateEndpointSliceFieldIndices() eventCh := make(chan<- interface{}) - beSameFunctionPointer := func(expected interface{}) types.GomegaMatcher { + beSameFunctionPointer := func(expected interface{}) gtypes.GomegaMatcher { return gcustom.MakeMatcher(func(f interface{}) (bool, error) { // comparing functions is not allowed in Go, so we're comparing the pointers return reflect.ValueOf(expected).Pointer() == reflect.ValueOf(f).Pointer(), nil @@ -101,7 +104,7 @@ func TestRegister(t *testing.T) { g.Expect(c.Getter).To(BeIdenticalTo(test.fakes.mgr.GetClient())) g.Expect(c.ObjectType).To(BeIdenticalTo(objectType)) g.Expect(c.EventCh).To(BeIdenticalTo(eventCh)) - g.Expect(c.NamespacedNameFilter).Should(beSameFunctionPointer(namespacedNameFilter)) + g.Expect(c.NamespacedNameFilter).Should(beSameFunctionPointer(nsNameFilter)) return controller.NewReconciler(c) } @@ -111,7 +114,7 @@ func TestRegister(t *testing.T) { objectType, test.fakes.mgr, eventCh, - controller.WithNamespacedNameFilter(namespacedNameFilter), + controller.WithNamespacedNameFilter(nsNameFilter), controller.WithK8sPredicate(predicate.ServicePortsChangedPredicate{}), controller.WithFieldIndices(fieldIndexes), controller.WithNewReconciler(newReconciler), diff --git a/internal/manager/filter/filter.go b/internal/manager/filter/filter.go new file mode 100644 index 0000000000..12002ca17a --- /dev/null +++ b/internal/manager/filter/filter.go @@ -0,0 +1,25 @@ +package filter + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/types" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" +) + +// CreateSingleResourceFilter creates a filter function that filters out all resources except the one +// with the given namespaced name. +func CreateSingleResourceFilter(targetNsName types.NamespacedName) controller.NamespacedNameFilterFunc { + return func(nsname types.NamespacedName) (shouldProcess bool, msg string) { + if nsname != targetNsName { + msg := fmt.Sprintf( + "Resource is ignored because this controller only supports a single resource %s/%s of that type", + targetNsName.Namespace, + targetNsName.Name, + ) + return false, msg + } + return true, "" + } +} diff --git a/internal/manager/filter/filter_test.go b/internal/manager/filter/filter_test.go new file mode 100644 index 0000000000..fadfd2b9ef --- /dev/null +++ b/internal/manager/filter/filter_test.go @@ -0,0 +1,62 @@ +package filter + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" +) + +func TestCreateSingleResourceFilter(t *testing.T) { + targetNsName := types.NamespacedName{Namespace: "test", Name: "resource"} + + filter := CreateSingleResourceFilter(targetNsName) + if filter == nil { + t.Fatal("TestCreateSingleResourceFilter() returned nil filter") + } + + const expectedMsg = "Resource is ignored because this controller only supports a single resource " + + "test/resource of that type" + + tests := []struct { + name string + nsname types.NamespacedName + expectedMsg string + expectedShouldProcess bool + }{ + { + name: "match", + nsname: targetNsName, + expectedShouldProcess: true, + expectedMsg: "", + }, + { + name: "wrong namespace", + nsname: types.NamespacedName{Namespace: targetNsName.Namespace, Name: "other-name"}, + expectedShouldProcess: false, + expectedMsg: expectedMsg, + }, + { + name: "wrong name", + nsname: types.NamespacedName{Namespace: "other-ns", Name: targetNsName.Name}, + expectedShouldProcess: false, + expectedMsg: expectedMsg, + }, + { + name: "wrong namespace and name", + nsname: types.NamespacedName{Namespace: "other-ns", Name: "other-name"}, + expectedShouldProcess: false, + expectedMsg: expectedMsg, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + shouldProcess, msg := filter(test.nsname) + g.Expect(shouldProcess).To(Equal(test.expectedShouldProcess)) + g.Expect(msg).To(Equal(test.expectedMsg)) + }) + } +} diff --git a/internal/manager/filter/gatewayclass.go b/internal/manager/filter/gatewayclass.go deleted file mode 100644 index 3cd21b3f49..0000000000 --- a/internal/manager/filter/gatewayclass.go +++ /dev/null @@ -1,23 +0,0 @@ -package filter - -import ( - "fmt" - - "k8s.io/apimachinery/pkg/types" - - "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" -) - -// CreateFilterForGatewayClass creates a filter function that filters out all GatewayClass resources except the one -// with the given name. -func CreateFilterForGatewayClass(gcName string) controller.NamespacedNameFilterFunc { - return func(nsname types.NamespacedName) (bool, string) { - if nsname.Name != gcName { - return false, fmt.Sprintf( - "GatewayClass is ignored because this controller only supports the GatewayClass %s", - gcName, - ) - } - return true, "" - } -} diff --git a/internal/manager/filter/gatewayclass_test.go b/internal/manager/filter/gatewayclass_test.go deleted file mode 100644 index d8c9c309ed..0000000000 --- a/internal/manager/filter/gatewayclass_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package filter - -import ( - "testing" - - "k8s.io/apimachinery/pkg/types" -) - -func TestCreateFilterForGatewayClass(t *testing.T) { - const gcName = "my-gc" - - filter := CreateFilterForGatewayClass(gcName) - if filter == nil { - t.Fatal("CreateFilterForGatewayClass() returned nil") - } - - tests := []struct { - nsname types.NamespacedName - expected bool - }{ - { - nsname: types.NamespacedName{Name: gcName}, - expected: true, - }, - { - nsname: types.NamespacedName{Name: gcName, Namespace: "doesn't matter"}, - expected: true, - }, - { - nsname: types.NamespacedName{Name: "some-gc"}, - expected: false, - }, - } - - for _, test := range tests { - result, msg := filter(test.nsname) - - if result != test.expected { - t.Errorf("filter(%#v) returned %v but expected %v", test.nsname, result, test.expected) - } - - if result && msg != "" { - t.Errorf("filter(%#v) returned a non-empty message %q", test.nsname, msg) - } - if !result && msg == "" { - t.Errorf("filter(%#v) returned an empty message", test.nsname) - } - } -} diff --git a/internal/manager/manager.go b/internal/manager/manager.go index de9d5a6f82..a2bfcda03f 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -8,6 +8,7 @@ import ( discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" ctlr "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -67,6 +68,9 @@ func Start(cfg config.Config) error { return fmt.Errorf("cannot build runtime manager: %w", err) } + // Note: for any new object type or a change to the existing one, + // make sure to also update prepareFirstEventBatchPreparerArgs() + // FIXME(pleshakov): Make the comment above redundant. controllerRegCfgs := []struct { objectType client.Object options []controller.Option @@ -74,11 +78,21 @@ func Start(cfg config.Config) error { { objectType: &gatewayv1beta1.GatewayClass{}, options: []controller.Option{ - controller.WithNamespacedNameFilter(filter.CreateFilterForGatewayClass(cfg.GatewayClassName)), + controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter( + types.NamespacedName{Name: cfg.GatewayClassName}, + )), }, }, { objectType: &gatewayv1beta1.Gateway{}, + options: func() []controller.Option { + if cfg.GatewayNsName != nil { + return []controller.Option{ + controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(*cfg.GatewayNsName)), + } + } + return nil + }(), }, { objectType: &gatewayv1beta1.HTTPRoute{}, @@ -134,12 +148,13 @@ func Start(cfg config.Config) error { nginxFileMgr := file.NewManagerImpl() nginxRuntimeMgr := ngxruntime.NewManagerImpl() statusUpdater := status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: cfg.GatewayCtlrName, - GatewayClassName: cfg.GatewayClassName, - Client: mgr.GetClient(), - PodIP: cfg.PodIP, - Logger: cfg.Logger.WithName("statusUpdater"), - Clock: status.NewRealClock(), + GatewayCtlrName: cfg.GatewayCtlrName, + GatewayClassName: cfg.GatewayClassName, + Client: mgr.GetClient(), + PodIP: cfg.PodIP, + Logger: cfg.Logger.WithName("statusUpdater"), + Clock: status.NewRealClock(), + UpdateGatewayClassStatus: cfg.UpdateGatewayClassStatus, }) eventHandler := events.NewEventHandlerImpl(events.EventHandlerConfig{ @@ -153,19 +168,8 @@ func Start(cfg config.Config) error { StatusUpdater: statusUpdater, }) - firstBatchPreparer := events.NewFirstEventBatchPreparerImpl( - mgr.GetCache(), - []client.Object{ - &gatewayv1beta1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: cfg.GatewayClassName}}, - }, - []client.ObjectList{ - &apiv1.ServiceList{}, - &apiv1.SecretList{}, - &discoveryV1.EndpointSliceList{}, - &gatewayv1beta1.GatewayList{}, - &gatewayv1beta1.HTTPRouteList{}, - }, - ) + objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg.GatewayClassName, cfg.GatewayNsName) + firstBatchPreparer := events.NewFirstEventBatchPreparerImpl(mgr.GetCache(), objects, objectLists) eventLoop := events.NewEventLoop( eventCh, @@ -181,3 +185,29 @@ func Start(cfg config.Config) error { logger.Info("Starting manager") return mgr.Start(ctx) } + +func prepareFirstEventBatchPreparerArgs( + gcName string, + gwNsName *types.NamespacedName, +) ([]client.Object, []client.ObjectList) { + objects := []client.Object{ + &gatewayv1beta1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: gcName}}, + } + objectLists := []client.ObjectList{ + &apiv1.ServiceList{}, + &apiv1.SecretList{}, + &discoveryV1.EndpointSliceList{}, + &gatewayv1beta1.HTTPRouteList{}, + } + + if gwNsName == nil { + objectLists = append(objectLists, &gatewayv1beta1.GatewayList{}) + } else { + objects = append( + objects, + &gatewayv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: gwNsName.Name, Namespace: gwNsName.Namespace}}, + ) + } + + return objects, objectLists +} diff --git a/internal/manager/manager_test.go b/internal/manager/manager_test.go new file mode 100644 index 0000000000..7f51051080 --- /dev/null +++ b/internal/manager/manager_test.go @@ -0,0 +1,67 @@ +package manager + +import ( + "testing" + + . "github.com/onsi/gomega" + apiv1 "k8s.io/api/core/v1" + discoveryV1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { + const gcName = "nginx" + + tests := []struct { + name string + gwNsName *types.NamespacedName + expectedObjects []client.Object + expectedObjectLists []client.ObjectList + }{ + { + name: "gwNsName is nil", + gwNsName: nil, + expectedObjects: []client.Object{ + &gatewayv1beta1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, + }, + expectedObjectLists: []client.ObjectList{ + &apiv1.ServiceList{}, + &apiv1.SecretList{}, + &discoveryV1.EndpointSliceList{}, + &gatewayv1beta1.HTTPRouteList{}, + &gatewayv1beta1.GatewayList{}, + }, + }, + { + name: "gwNsName is not nil", + gwNsName: &types.NamespacedName{ + Namespace: "test", + Name: "my-gateway", + }, + expectedObjects: []client.Object{ + &gatewayv1beta1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, + &gatewayv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "my-gateway", Namespace: "test"}}, + }, + expectedObjectLists: []client.ObjectList{ + &apiv1.ServiceList{}, + &apiv1.SecretList{}, + &discoveryV1.EndpointSliceList{}, + &gatewayv1beta1.HTTPRouteList{}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + objects, objectLists := prepareFirstEventBatchPreparerArgs(gcName, test.gwNsName) + + g.Expect(objects).To(ConsistOf(test.expectedObjects)) + g.Expect(objectLists).To(ConsistOf(test.expectedObjectLists)) + }) + } +} diff --git a/internal/status/status_suit_test.go b/internal/status/status_suit_test.go index de296f304d..b2dad44818 100644 --- a/internal/status/status_suit_test.go +++ b/internal/status/status_suit_test.go @@ -7,7 +7,7 @@ import ( . "github.com/onsi/gomega" ) -func TestState(t *testing.T) { +func TestStatus(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Status Suite") } diff --git a/internal/status/updater.go b/internal/status/updater.go index 0859bb4f3f..88d1ebd211 100644 --- a/internal/status/updater.go +++ b/internal/status/updater.go @@ -34,6 +34,8 @@ type UpdaterConfig struct { GatewayClassName string // PodIP is the IP address of this Pod. PodIP string + // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. + UpdateGatewayClassStatus bool } // updaterImpl updates statuses of the Gateway API resources. @@ -88,7 +90,7 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses state.Statuses) { // FIXME(pleshakov) Merge the new Conditions in the status with the existing Conditions // FIXME(pleshakov) Skip the status update (API call) if the status hasn't changed. - if statuses.GatewayClassStatus != nil { + if upd.cfg.UpdateGatewayClassStatus && statuses.GatewayClassStatus != nil { upd.update( ctx, types.NamespacedName{Name: upd.cfg.GatewayClassName}, diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index aac7df883f..c3560b1a48 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -26,9 +26,9 @@ var _ = Describe("Updater", func() { const gcName = "my-class" var ( - updater status.Updater client client.Client fakeClockTime metav1.Time + fakeClock *statusfakes.FakeClock gatewayCtrlName string ) @@ -45,19 +45,10 @@ var _ = Describe("Updater", func() { // We use it because updating the status in the FakeClient and then getting the resource back // involves encoding and decoding the resource to/from JSON, which uses RFC 3339 for metav1.Time. fakeClockTime = metav1.NewTime(time.Now()).Rfc3339Copy() - fakeClock := &statusfakes.FakeClock{} + fakeClock = &statusfakes.FakeClock{} fakeClock.NowReturns(fakeClockTime) gatewayCtrlName = "test.example.com" - - updater = status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: gatewayCtrlName, - GatewayClassName: gcName, - Client: client, - Logger: zap.New(), - Clock: fakeClock, - PodIP: "1.2.3.4", - }) }) Describe("Process status updates", Ordered, func() { @@ -67,6 +58,7 @@ var _ = Describe("Updater", func() { } var ( + updater status.Updater gc *v1beta1.GatewayClass gw, ignoredGw *v1beta1.Gateway hr *v1beta1.HTTPRoute @@ -213,6 +205,16 @@ var _ = Describe("Updater", func() { ) BeforeAll(func() { + updater = status.NewUpdater(status.UpdaterConfig{ + GatewayCtlrName: gatewayCtrlName, + GatewayClassName: gcName, + Client: client, + Logger: zap.New(), + Clock: fakeClock, + PodIP: "1.2.3.4", + UpdateGatewayClassStatus: true, + }) + gc = &v1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{ Name: gcName, @@ -393,4 +395,56 @@ var _ = Describe("Updater", func() { }) }) }) + + Describe("Skip GatewayClass updates", Ordered, func() { + var ( + updater status.Updater + gc *v1beta1.GatewayClass + ) + + BeforeAll(func() { + updater = status.NewUpdater(status.UpdaterConfig{ + GatewayCtlrName: gatewayCtrlName, + GatewayClassName: gcName, + Client: client, + Logger: zap.New(), + Clock: fakeClock, + PodIP: "1.2.3.4", + UpdateGatewayClassStatus: false, + }) + + gc = &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: gcName, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "GatewayClass", + APIVersion: "gateway.networking.k8s.io/v1beta1", + }, + } + }) + + It("should create resources in the API server", func() { + Expect(client.Create(context.Background(), gc)).Should(Succeed()) + }) + + It("should not update GatewayClass status", func() { + updater.Update( + context.Background(), + state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + ObservedGeneration: 1, + Conditions: status.CreateTestConditions("Test"), + }, + }, + ) + + latestGc := &v1beta1.GatewayClass{} + + err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) + Expect(err).Should(Not(HaveOccurred())) + + Expect(latestGc.Status).To(BeZero()) + }) + }) })