diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f0f3e66ecf..3d126c9706 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,6 +11,7 @@ on: pull_request: branches: - main + - feature/listeners # TODO: remove before merging to main types: - opened - reopened diff --git a/README.md b/README.md index d79b9cefa0..1a764270c0 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,6 @@ Before you can build and run the NGINX Kubernetes Gateway, make sure you have th Set the `PREFIX` variable to the name of the registry you'd like to push the image to. By default, the image will be named `nginx-kubernetes-gateway:0.0.1`. - 1. Push the image to your container registry: ``` @@ -55,7 +54,6 @@ You can deploy NGINX Kubernetes Gateway on an existing Kubernetes 1.16+ cluster. Make sure to substitute the image name with the name of the image you built. - 1. Install the Gateway CRDs: ``` @@ -89,6 +87,11 @@ You can deploy NGINX Kubernetes Gateway on an existing Kubernetes 1.16+ cluster. NAME READY STATUS RESTARTS AGE nginx-gateway-5d4f4c7db7-xk2kq 2/2 Running 0 112s ``` +1. Create the Gateway resource: + + ``` + kubectl apply -f deploy/manifests/gateway.yaml + ``` ## Expose NGINX Kubernetes Gateway diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index 21486d88dd..5365a875d1 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -5,6 +5,7 @@ import ( "os" flag "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginxinc/nginx-kubernetes-gateway/internal/config" @@ -36,6 +37,11 @@ func main() { conf := config.Config{ GatewayCtlrName: *gatewayCtlrName, Logger: logger, + // FIXME(pleshakov) Allow the cluster operator to customize this value + GatewayNsName: types.NamespacedName{ + Namespace: "nginx-gateway", + Name: "gateway", + }, } MustValidateArguments( diff --git a/deploy/manifests/gateway.yaml b/deploy/manifests/gateway.yaml index c7618ba877..68294a3524 100644 --- a/deploy/manifests/gateway.yaml +++ b/deploy/manifests/gateway.yaml @@ -8,7 +8,6 @@ metadata: spec: gatewayClassName: nginx listeners: - - name: my-listener - hostname: example.com - port: 8080 + - name: http + port: 80 protocol: HTTP diff --git a/examples/advanced-routing/cafe-routes.yaml b/examples/advanced-routing/cafe-routes.yaml index d93a840919..99964a5a7c 100644 --- a/examples/advanced-routing/cafe-routes.yaml +++ b/examples/advanced-routing/cafe-routes.yaml @@ -3,6 +3,10 @@ kind: HTTPRoute metadata: name: cafe spec: + parentRefs: + - name: gateway + namespace: nginx-gateway + sectionName: http hostnames: - "cafe.example.com" rules: @@ -15,6 +19,10 @@ kind: HTTPRoute metadata: name: coffee spec: + parentRefs: + - name: gateway + namespace: nginx-gateway + sectionName: http hostnames: - "cafe.example.com" rules: @@ -37,6 +45,10 @@ kind: HTTPRoute metadata: name: tea spec: + parentRefs: + - name: gateway + namespace: nginx-gateway + sectionName: http hostnames: - "cafe.example.com" rules: diff --git a/examples/cafe-example/cafe-routes.yaml b/examples/cafe-example/cafe-routes.yaml index 56b5f80f47..a3566d7e20 100644 --- a/examples/cafe-example/cafe-routes.yaml +++ b/examples/cafe-example/cafe-routes.yaml @@ -3,6 +3,10 @@ kind: HTTPRoute metadata: name: cafe spec: + parentRefs: + - name: gateway + namespace: nginx-gateway + sectionName: http hostnames: - "cafe.example.com" rules: @@ -15,6 +19,10 @@ kind: HTTPRoute metadata: name: coffee spec: + parentRefs: + - name: gateway + namespace: nginx-gateway + sectionName: http hostnames: - "cafe.example.com" rules: @@ -31,6 +39,10 @@ kind: HTTPRoute metadata: name: tea spec: + parentRefs: + - name: gateway + namespace: nginx-gateway + sectionName: http hostnames: - "cafe.example.com" rules: diff --git a/internal/config/config.go b/internal/config/config.go index 8b1e89a8ef..e23d93c16c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,9 +2,13 @@ package config import ( "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" ) type Config struct { GatewayCtlrName string 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 } diff --git a/internal/events/loop.go b/internal/events/loop.go index 80ac2717c8..487a2f69d8 100644 --- a/internal/events/loop.go +++ b/internal/events/loop.go @@ -8,42 +8,39 @@ import ( apiv1 "k8s.io/api/core/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" ) // EventLoop is the main event loop of the Gateway. type EventLoop struct { - conf state.Configuration + processor newstate.ChangeProcessor serviceStore state.ServiceStore generator config.Generator eventCh <-chan interface{} logger logr.Logger - statusUpdater status.Updater nginxFileMgr file.Manager nginxRuntimeMgr runtime.Manager } // NewEventLoop creates a new EventLoop. func NewEventLoop( - conf state.Configuration, + processor newstate.ChangeProcessor, serviceStore state.ServiceStore, generator config.Generator, eventCh <-chan interface{}, - statusUpdater status.Updater, logger logr.Logger, nginxFileMgr file.Manager, nginxRuntimeMgr runtime.Manager, ) *EventLoop { return &EventLoop{ - conf: conf, + processor: processor, serviceStore: serviceStore, generator: generator, eventCh: eventCh, - statusUpdater: statusUpdater, logger: logger.WithName("eventLoop"), nginxFileMgr: nginxFileMgr, nginxRuntimeMgr: nginxRuntimeMgr, @@ -58,112 +55,94 @@ func (el *EventLoop) Start(ctx context.Context) error { for { select { case <-ctx.Done(): + // although we always return nil, Start must return it to satisfy + // "sigs.k8s.io/controller-runtime/pkg/manager".Runnable return nil case e := <-el.eventCh: - err := el.handleEvent(ctx, e) - if err != nil { - return err - } + el.handleEvent(ctx, e) } } } // FIXME(pleshakov): think about how to avoid using an interface{} here -func (el *EventLoop) handleEvent(ctx context.Context, event interface{}) error { - var changes []state.Change - var updates []state.StatusUpdate - var err error - +func (el *EventLoop) handleEvent(ctx context.Context, event interface{}) { switch e := event.(type) { case *UpsertEvent: - changes, updates, err = el.propagateUpsert(e) + el.propagateUpsert(e) case *DeleteEvent: - changes, updates, err = el.propagateDelete(e) + el.propagateDelete(e) default: - // FIXME(pleshakov): panic because it is a coding error - return fmt.Errorf("unknown event type %T", e) + panic(fmt.Errorf("unknown event type %T", e)) + } + + changed, conf, statuses := el.processor.Process() + if !changed { + return } + err := el.updateNginx(ctx, conf) + if err != nil { + el.logger.Error(err, "Failed to update NGINX configuration") + } + + // FIXME(pleshakov) Update resource statuses instead of printing to stdout + for name, s := range statuses.ListenerStatuses { + fmt.Printf("Listener %q, Statuses: %v\n", name, s) + } + for nsname, s := range statuses.HTTPRouteStatuses { + fmt.Printf("HTTPRoute %q, Statuses: %v\n", nsname, s) + } +} + +func (el *EventLoop) updateNginx(ctx context.Context, conf newstate.Configuration) error { + cfg, warnings := el.generator.Generate(conf) + + // For now, we keep all http servers in one config + // We might rethink that. For example, we can write each server to its file + // or group servers in some way. + err := el.nginxFileMgr.WriteHTTPServersConfig("http-servers", cfg) if err != nil { return err } - el.processChangesAndStatusUpdates(ctx, changes, updates) - return nil + for obj, objWarnings := range warnings { + for _, w := range objWarnings { + // FIXME(pleshakov): report warnings via Object status + el.logger.Info("got warning while generating config", + "kind", obj.GetObjectKind().GroupVersionKind().Kind, + "namespace", obj.GetNamespace(), + "name", obj.GetName(), + "warning", w) + } + } + + return el.nginxRuntimeMgr.Reload(ctx) } -func (el *EventLoop) propagateUpsert(e *UpsertEvent) ([]state.Change, []state.StatusUpdate, error) { +func (el *EventLoop) propagateUpsert(e *UpsertEvent) { switch r := e.Resource.(type) { + case *v1alpha2.Gateway: + el.processor.CaptureUpsertChange(r) case *v1alpha2.HTTPRoute: - changes, statusUpdates := el.conf.UpsertHTTPRoute(r) - return changes, statusUpdates, nil + el.processor.CaptureUpsertChange(r) case *apiv1.Service: - el.serviceStore.Upsert(r) // FIXME(pleshakov): make sure the affected hosts are updated - return nil, nil, nil + el.serviceStore.Upsert(r) + default: + panic(fmt.Errorf("unknown resource type %T", e.Resource)) } - - // FIXME(pleshakov): panic because it is a coding error - return nil, nil, fmt.Errorf("unknown resource type %T", e.Resource) } -func (el *EventLoop) propagateDelete(e *DeleteEvent) ([]state.Change, []state.StatusUpdate, error) { +func (el *EventLoop) propagateDelete(e *DeleteEvent) { switch e.Type.(type) { + case *v1alpha2.Gateway: + el.processor.CaptureDeleteChange(e.Type, e.NamespacedName) case *v1alpha2.HTTPRoute: - changes, statusUpdates := el.conf.DeleteHTTPRoute(e.NamespacedName) - return changes, statusUpdates, nil + el.processor.CaptureDeleteChange(e.Type, e.NamespacedName) case *apiv1.Service: - el.serviceStore.Delete(e.NamespacedName) // FIXME(pleshakov): make sure the affected hosts are updated - return nil, nil, nil - } - - // FIXME(pleshakov): panic because it is a coding error - return nil, nil, fmt.Errorf("unknown resource type %T", e.Type) -} - -func (el *EventLoop) processChangesAndStatusUpdates(ctx context.Context, changes []state.Change, updates []state.StatusUpdate) { - for _, c := range changes { - el.logger.Info("Processing a change", - "host", c.Host.Value) - - if c.Op == state.Upsert { - cfg, warnings := el.generator.GenerateForHost(c.Host) - - for obj, objWarnings := range warnings { - for _, w := range objWarnings { - // FIXME(pleshakov): report warnings via Object status - el.logger.Info("got warning while generating config", - "kind", obj.GetObjectKind().GroupVersionKind().Kind, - "namespace", obj.GetNamespace(), - "name", obj.GetName(), - "warning", w) - } - } - - el.logger.Info("Writing configuration", - "host", c.Host.Value) - - err := el.nginxFileMgr.WriteServerConfig(c.Host.Value, cfg) - if err != nil { - el.logger.Error(err, "Failed to write configuration", - "host", c.Host.Value) - } - } else { - err := el.nginxFileMgr.DeleteServerConfig(c.Host.Value) - if err != nil { - el.logger.Error(err, "Failed to delete configuration", - "host", c.Host.Value) - } - } - } - - if len(changes) > 0 { - err := el.nginxRuntimeMgr.Reload(ctx) - if err != nil { - el.logger.Error(err, "Failed to reload NGINX") - } + el.serviceStore.Delete(e.NamespacedName) + default: + panic(fmt.Errorf("unknown resource type %T", e.Type)) } - - el.statusUpdater.ProcessStatusUpdates(ctx, updates) } diff --git a/internal/events/loop_test.go b/internal/events/loop_test.go index f2f7545960..8d0467efa4 100644 --- a/internal/events/loop_test.go +++ b/internal/events/loop_test.go @@ -14,13 +14,13 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate/newstatefakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/configfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file/filefakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime/runtimefakes" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/statefakes" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes" ) type unsupportedResource struct { @@ -36,38 +36,40 @@ func (r *unsupportedResource) DeepCopyObject() runtime.Object { } var _ = Describe("EventLoop", func() { - var ctrl *events.EventLoop - var fakeConf *statefakes.FakeConfiguration - var fakeUpdater *statusfakes.FakeUpdater - var fakeServiceStore *statefakes.FakeServiceStore - var fakeGenerator *configfakes.FakeGenerator - var fakeNginxFimeMgr *filefakes.FakeManager - var fakeNginxRuntimeMgr *runtimefakes.FakeManager - var cancel context.CancelFunc - var eventCh chan interface{} - var errorCh chan error + var ( + fakeProcessor *newstatefakes.FakeChangeProcessor + fakeServiceStore *statefakes.FakeServiceStore + fakeGenerator *configfakes.FakeGenerator + fakeNginxFimeMgr *filefakes.FakeManager + fakeNginxRuntimeMgr *runtimefakes.FakeManager + cancel context.CancelFunc + eventCh chan interface{} + errorCh chan error + start func() + ) BeforeEach(func() { - fakeConf = &statefakes.FakeConfiguration{} + fakeProcessor = &newstatefakes.FakeChangeProcessor{} eventCh = make(chan interface{}) - fakeUpdater = &statusfakes.FakeUpdater{} fakeServiceStore = &statefakes.FakeServiceStore{} fakeGenerator = &configfakes.FakeGenerator{} fakeNginxFimeMgr = &filefakes.FakeManager{} fakeNginxRuntimeMgr = &runtimefakes.FakeManager{} - ctrl = events.NewEventLoop(fakeConf, fakeServiceStore, fakeGenerator, eventCh, fakeUpdater, zap.New(), fakeNginxFimeMgr, fakeNginxRuntimeMgr) + ctrl := events.NewEventLoop(fakeProcessor, fakeServiceStore, fakeGenerator, eventCh, zap.New(), fakeNginxFimeMgr, fakeNginxRuntimeMgr) var ctx context.Context - ctx, cancel = context.WithCancel(context.Background()) errorCh = make(chan error) - - go func() { + start = func() { errorCh <- ctrl.Start(ctx) - }() + } }) - Describe("Process HTTPRoute events", func() { + Describe("Process Gateway API resource events", func() { + BeforeEach(func() { + go start() + }) + AfterEach(func() { cancel() @@ -76,97 +78,70 @@ var _ = Describe("EventLoop", func() { Expect(err).To(BeNil()) }) - It("should process upsert event", func() { - fakeChanges := []state.Change{ - { - Op: state.Upsert, - Host: state.Host{ - Value: "example.com", - }, - }, - } - fakeStatusUpdates := []state.StatusUpdate{ - { - NamespacedName: types.NamespacedName{}, - Status: nil, - }, - } - fakeConf.UpsertHTTPRouteReturns(fakeChanges, fakeStatusUpdates) + DescribeTable("Upsert events", + func(e *events.UpsertEvent) { + fakeConf := newstate.Configuration{} + changed := true + fakeProcessor.ProcessReturns(changed, fakeConf, newstate.Statuses{}) - fakeCfg := []byte("fake") - fakeGenerator.GenerateForHostReturns(fakeCfg, config.Warnings{}) + fakeCfg := []byte("fake") + fakeGenerator.GenerateReturns(fakeCfg, config.Warnings{}) - hr := &v1alpha2.HTTPRoute{} - - eventCh <- &events.UpsertEvent{ - Resource: hr, - } - - Eventually(fakeConf.UpsertHTTPRouteCallCount).Should(Equal(1)) - Eventually(func() *v1alpha2.HTTPRoute { - return fakeConf.UpsertHTTPRouteArgsForCall(0) - }).Should(Equal(hr)) + eventCh <- e - Eventually(fakeUpdater.ProcessStatusUpdatesCallCount).Should(Equal(1)) - Eventually(func() []state.StatusUpdate { - _, updates := fakeUpdater.ProcessStatusUpdatesArgsForCall(0) - return updates - }).Should(Equal(fakeStatusUpdates)) + Eventually(fakeProcessor.CaptureUpsertChangeCallCount).Should(Equal(1)) + Expect(fakeProcessor.CaptureUpsertChangeArgsForCall(0)).Should(Equal(e.Resource)) + Eventually(fakeProcessor.ProcessCallCount).Should(Equal(1)) - Eventually(fakeGenerator.GenerateForHostCallCount).Should(Equal(1)) - Expect(fakeGenerator.GenerateForHostArgsForCall(0)).Should(Equal(fakeChanges[0].Host)) + Eventually(fakeGenerator.GenerateCallCount).Should(Equal(1)) + Expect(fakeGenerator.GenerateArgsForCall(0)).Should(Equal(fakeConf)) - Eventually(fakeNginxFimeMgr.WriteServerConfigCallCount).Should(Equal(1)) - host, cfg := fakeNginxFimeMgr.WriteServerConfigArgsForCall(0) - Expect(host).Should(Equal("example.com")) - Expect(cfg).Should(Equal(fakeCfg)) + Eventually(fakeNginxFimeMgr.WriteHTTPServersConfigCallCount).Should(Equal(1)) + name, cfg := fakeNginxFimeMgr.WriteHTTPServersConfigArgsForCall(0) + Expect(name).Should(Equal("http-servers")) + Expect(cfg).Should(Equal(fakeCfg)) - Eventually(fakeNginxRuntimeMgr.ReloadCallCount).Should(Equal(1)) - }) + Eventually(fakeNginxRuntimeMgr.ReloadCallCount).Should(Equal(1)) + }, + Entry("HTTPRoute", &events.UpsertEvent{Resource: &v1alpha2.HTTPRoute{}}), + Entry("Gateway", &events.UpsertEvent{Resource: &v1alpha2.Gateway{}}), + ) - It("should process delete event", func() { - fakeChanges := []state.Change{ - { - Op: state.Delete, - Host: state.Host{ - Value: "example.com", - }, - }, - } - fakeStatusUpdates := []state.StatusUpdate{ - { - NamespacedName: types.NamespacedName{}, - Status: nil, - }, - } - fakeConf.DeleteHTTPRouteReturns(fakeChanges, fakeStatusUpdates) + DescribeTable("Delete events", + func(e *events.DeleteEvent) { + fakeConf := newstate.Configuration{} + changed := true + fakeProcessor.ProcessReturns(changed, fakeConf, newstate.Statuses{}) - nsname := types.NamespacedName{Namespace: "test", Name: "route"} + fakeCfg := []byte("fake") + fakeGenerator.GenerateReturns(fakeCfg, config.Warnings{}) - eventCh <- &events.DeleteEvent{ - NamespacedName: nsname, - Type: &v1alpha2.HTTPRoute{}, - } + eventCh <- e - Eventually(fakeConf.DeleteHTTPRouteCallCount).Should(Equal(1)) - Eventually(func() types.NamespacedName { - return fakeConf.DeleteHTTPRouteArgsForCall(0) - }).Should(Equal(nsname)) + Eventually(fakeProcessor.CaptureDeleteChangeCallCount).Should(Equal(1)) + passedObj, passedNsName := fakeProcessor.CaptureDeleteChangeArgsForCall(0) + Expect(passedObj).Should(Equal(e.Type)) + Expect(passedNsName).Should(Equal(e.NamespacedName)) - Eventually(fakeUpdater.ProcessStatusUpdatesCallCount).Should(Equal(1)) - Eventually(func() []state.StatusUpdate { - _, updates := fakeUpdater.ProcessStatusUpdatesArgsForCall(0) - return updates - }).Should(Equal(fakeStatusUpdates)) + Eventually(fakeProcessor.ProcessCallCount).Should(Equal(1)) - Eventually(fakeNginxFimeMgr.DeleteServerConfigCallCount).Should(Equal(1)) - Expect(fakeNginxFimeMgr.DeleteServerConfigArgsForCall(0)).Should(Equal("example.com")) + Eventually(fakeNginxFimeMgr.WriteHTTPServersConfigCallCount).Should(Equal(1)) + name, cfg := fakeNginxFimeMgr.WriteHTTPServersConfigArgsForCall(0) + Expect(name).Should(Equal("http-servers")) + Expect(cfg).Should(Equal(fakeCfg)) - Eventually(fakeNginxRuntimeMgr.ReloadCallCount).Should(Equal(1)) - }) + Eventually(fakeNginxRuntimeMgr.ReloadCallCount).Should(Equal(1)) + }, + Entry("HTTPRoute", &events.DeleteEvent{Type: &v1alpha2.HTTPRoute{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "route"}}), + Entry("Gateway", &events.DeleteEvent{Type: &v1alpha2.Gateway{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "gateway"}}), + ) }) Describe("Process Service events", func() { + BeforeEach(func() { + go start() + }) + AfterEach(func() { cancel() @@ -183,9 +158,9 @@ var _ = Describe("EventLoop", func() { } Eventually(fakeServiceStore.UpsertCallCount).Should(Equal(1)) - Eventually(func() *apiv1.Service { - return fakeServiceStore.UpsertArgsForCall(0) - }).Should(Equal(svc)) + Expect(fakeServiceStore.UpsertArgsForCall(0)).Should(Equal(svc)) + + Eventually(fakeProcessor.ProcessCallCount).Should(Equal(1)) }) It("should process delete event", func() { @@ -197,61 +172,9 @@ var _ = Describe("EventLoop", func() { } Eventually(fakeServiceStore.DeleteCallCount).Should(Equal(1)) - Eventually(func() types.NamespacedName { - return fakeServiceStore.DeleteArgsForCall(0) - }).Should(Equal(nsname)) - }) - }) + Expect(fakeServiceStore.DeleteArgsForCall(0)).Should(Equal(nsname)) - Describe("Processing events common cases", func() { - AfterEach(func() { - cancel() - - var err error - Eventually(errorCh).Should(Receive(&err)) - Expect(err).To(BeNil()) - }) - - It("should reload once in case of multiple changes", func() { - fakeChanges := []state.Change{ - { - Op: state.Delete, - Host: state.Host{ - Value: "one.example.com", - }, - }, - { - Op: state.Upsert, - Host: state.Host{ - Value: "two.example.com", - }, - }, - } - fakeConf.DeleteHTTPRouteReturns(fakeChanges, nil) - - fakeCfg := []byte("fake") - fakeGenerator.GenerateForHostReturns(fakeCfg, config.Warnings{}) - - nsname := types.NamespacedName{Namespace: "test", Name: "route"} - - // the exact event doesn't matter. what matters is the generated changes return by DeleteHTTPRouteReturns - eventCh <- &events.DeleteEvent{ - NamespacedName: nsname, - Type: &v1alpha2.HTTPRoute{}, - } - - Eventually(fakeConf.DeleteHTTPRouteCallCount).Should(Equal(1)) - Expect(fakeConf.DeleteHTTPRouteArgsForCall(0)).Should(Equal(nsname)) - - Eventually(fakeNginxFimeMgr.WriteServerConfigCallCount).Should(Equal(1)) - host, cfg := fakeNginxFimeMgr.WriteServerConfigArgsForCall(0) - Expect(host).Should(Equal("two.example.com")) - Expect(cfg).Should(Equal(fakeCfg)) - - Eventually(fakeNginxFimeMgr.DeleteServerConfigCallCount).Should(Equal(1)) - Expect(fakeNginxFimeMgr.DeleteServerConfigArgsForCall(0)).Should(Equal("one.example.com")) - - Eventually(fakeNginxRuntimeMgr.ReloadCallCount).Should(Equal(1)) + Eventually(fakeProcessor.ProcessCallCount).Should(Equal(1)) }) }) @@ -262,19 +185,19 @@ var _ = Describe("EventLoop", func() { DescribeTable("Edge cases for events", func(e interface{}) { - eventCh <- e + go func() { + eventCh <- e + }() - var err error - Eventually(errorCh).Should(Receive(&err)) - Expect(err).Should(HaveOccurred()) + Expect(start).Should(Panic()) }, - Entry("should return error for an unknown event type", + Entry("should panic for an unknown event type", &struct{}{}), - Entry("should return error for an unknown type of resource in upsert event", + Entry("should panic for an unknown type of resource in upsert event", &events.UpsertEvent{ Resource: &unsupportedResource{}, }), - Entry("should return error for an unknown type of resource in delete event", + Entry("should panic for an unknown type of resource in delete event", &events.DeleteEvent{ Type: &unsupportedResource{}, }), diff --git a/internal/implementations/gateway/gateway.go b/internal/implementations/gateway/gateway.go index 9946e5e070..6d8d5e40ee 100644 --- a/internal/implementations/gateway/gateway.go +++ b/internal/implementations/gateway/gateway.go @@ -1,20 +1,26 @@ package implementation import ( + "fmt" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/nginxinc/nginx-kubernetes-gateway/internal/config" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" "github.com/nginxinc/nginx-kubernetes-gateway/pkg/sdk" ) type gatewayImplementation struct { - conf config.Config + conf config.Config + eventCh chan<- interface{} } -func NewGatewayImplementation(conf config.Config) sdk.GatewayImpl { +func NewGatewayImplementation(conf config.Config, eventCh chan<- interface{}) sdk.GatewayImpl { return &gatewayImplementation{ - conf: conf, + conf: conf, + eventCh: eventCh, } } @@ -22,21 +28,43 @@ func (impl *gatewayImplementation) Logger() logr.Logger { return impl.conf.Logger } -func (impl *gatewayImplementation) ControllerName() string { - return impl.conf.GatewayCtlrName -} - func (impl *gatewayImplementation) Upsert(gw *v1alpha2.Gateway) { - if gw.Name == impl.ControllerName() { - impl.Logger().Info("Found correct Gateway resource", + if gw.Namespace != impl.conf.GatewayNsName.Namespace || gw.Name != impl.conf.GatewayNsName.Name { + msg := fmt.Sprintf("Gateway was upserted but ignored because this controller only supports the Gateway %s", impl.conf.GatewayNsName) + impl.Logger().Info(msg, + "namespace", gw.Namespace, "name", gw.Name, ) return } + + impl.Logger().Info("Gateway was upserted", + "namespace", gw.Namespace, + "name", gw.Name, + ) + + impl.eventCh <- &events.UpsertEvent{ + Resource: gw, + } } -func (impl *gatewayImplementation) Remove(key string) { - impl.Logger().Info("Gateway resource was removed", - "name", key, +func (impl *gatewayImplementation) Remove(nsname types.NamespacedName) { + if nsname != impl.conf.GatewayNsName { + msg := fmt.Sprintf("Gateway was removed but ignored because this controller only supports the Gateway %s", impl.conf.GatewayNsName) + impl.Logger().Info(msg, + "namespace", nsname.Namespace, + "name", nsname.Name, + ) + return + } + + impl.Logger().Info("Gateway was removed", + "namespace", nsname.Namespace, + "name", nsname.Name, ) + + impl.eventCh <- &events.DeleteEvent{ + NamespacedName: nsname, + Type: &v1alpha2.Gateway{}, + } } diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 1347117864..0df1ff82d9 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -12,13 +12,14 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/config" "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" + gw "github.com/nginxinc/nginx-kubernetes-gateway/internal/implementations/gateway" hr "github.com/nginxinc/nginx-kubernetes-gateway/internal/implementations/httproute" svc "github.com/nginxinc/nginx-kubernetes-gateway/internal/implementations/service" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate" ngxcfg "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file" ngxruntime "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" "github.com/nginxinc/nginx-kubernetes-gateway/pkg/sdk" ) @@ -50,6 +51,10 @@ func Start(cfg config.Config) error { return fmt.Errorf("cannot build runtime manager: %w", err) } + err = sdk.RegisterGatewayController(mgr, gw.NewGatewayImplementation(cfg, eventCh)) + if err != nil { + return fmt.Errorf("cannot register gateway implementation: %w", err) + } err = sdk.RegisterHTTPRouteController(mgr, hr.NewHTTPRouteImplementation(cfg, eventCh)) if err != nil { return fmt.Errorf("cannot register httproute implementation: %w", err) @@ -59,13 +64,12 @@ func Start(cfg config.Config) error { return fmt.Errorf("cannot register service implementation: %w", err) } - conf := state.NewConfiguration(cfg.GatewayCtlrName, state.NewRealClock()) + processor := newstate.NewChangeProcessorImpl(cfg.GatewayNsName) serviceStore := state.NewServiceStore() - reporter := status.NewUpdater(mgr.GetClient(), cfg.Logger) configGenerator := ngxcfg.NewGeneratorImpl(serviceStore) nginxFileMgr := file.NewManagerImpl() nginxRuntimeMgr := ngxruntime.NewManagerImpl() - eventLoop := events.NewEventLoop(conf, serviceStore, configGenerator, eventCh, reporter, cfg.Logger, nginxFileMgr, nginxRuntimeMgr) + eventLoop := events.NewEventLoop(processor, serviceStore, configGenerator, eventCh, cfg.Logger, nginxFileMgr, nginxRuntimeMgr) err = mgr.Add(eventLoop) if err != nil { diff --git a/internal/newstate/change_processor.go b/internal/newstate/change_processor.go index f606f7d393..4e8daa4109 100644 --- a/internal/newstate/change_processor.go +++ b/internal/newstate/change_processor.go @@ -9,9 +9,27 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" ) +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ChangeProcessor + // ChangeProcessor processes the changes to resources producing the internal representation of the Gateway configuration. // ChangeProcessor only supports one Gateway resource. -type ChangeProcessor struct { +type ChangeProcessor interface { + // CaptureUpsertChange captures an upsert change to a resource. + // It panics if the resource is of unsupported type or if the passed Gateway is different from the one this ChangeProcessor + // was created for. + CaptureUpsertChange(obj client.Object) + // CaptureDeleteChange captures a delete change to a resource. + // The method panics if the resource is of unsupported type or if the passed Gateway is different from the one this ChangeProcessor + // was created for. + CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName) + // Process processes any captured changes and produces an internal representation of the Gateway configuration and + // the status information about the processed resources. + // If no changes were captured, the changed return argument will be false and both the configuration and statuses + // will be empty. + Process() (changed bool, conf Configuration, statuses Statuses) +} + +type ChangeProcessorImpl struct { store *store changed bool gwNsName types.NamespacedName @@ -19,18 +37,15 @@ type ChangeProcessor struct { lock sync.Mutex } -// NewChangeProcessor creates a new ChangeProcessor for the Gateway resource with the configured namespace name. -func NewChangeProcessor(gwNsName types.NamespacedName) *ChangeProcessor { - return &ChangeProcessor{ +// NewChangeProcessorImpl creates a new ChangeProcessorImpl for the Gateway resource with the configured namespace name. +func NewChangeProcessorImpl(gwNsName types.NamespacedName) *ChangeProcessorImpl { + return &ChangeProcessorImpl{ store: newStore(), gwNsName: gwNsName, } } -// CaptureUpsertChange captures an upsert change to a resource. -// It panics if the resource is of unsupported type or if the passed Gateway is different from the one this ChangeProcessor -// was created for. -func (c *ChangeProcessor) CaptureUpsertChange(obj client.Object) { +func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) { c.lock.Lock() defer c.lock.Unlock() @@ -49,10 +64,7 @@ func (c *ChangeProcessor) CaptureUpsertChange(obj client.Object) { } } -// CaptureDeleteChange captures a delete change to a resource. -// The method panics if the resource is of unsupported type or if the passed Gateway is different from the one this ChangeProcessor -// was created for. -func (c *ChangeProcessor) CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName) { +func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName) { c.lock.Lock() defer c.lock.Unlock() @@ -71,11 +83,7 @@ func (c *ChangeProcessor) CaptureDeleteChange(resourceType client.Object, nsname } } -// Process processes any captured changes and produces an internal representation of the Gateway configuration and -// the status information about the processed resources. -// If no changes were captured, the changed return argument will be false and both the configuration and statuses -// will be empty. -func (c *ChangeProcessor) Process() (changed bool, conf Configuration, statuses Statuses) { +func (c *ChangeProcessorImpl) Process() (changed bool, conf Configuration, statuses Statuses) { c.lock.Lock() defer c.lock.Unlock() diff --git a/internal/newstate/change_processor_test.go b/internal/newstate/change_processor_test.go index 4956aa04e5..d04a0d4168 100644 --- a/internal/newstate/change_processor_test.go +++ b/internal/newstate/change_processor_test.go @@ -16,7 +16,7 @@ var _ = Describe("ChangeProcessor", func() { Describe("Normal cases of processing changes", func() { var hr1, hr2 *v1alpha2.HTTPRoute var gw *v1alpha2.Gateway - var processor *newstate.ChangeProcessor + var processor newstate.ChangeProcessor BeforeEach(OncePerOrdered, func() { createRoute := func(name string, hostname string) *v1alpha2.HTTPRoute { @@ -73,7 +73,7 @@ var _ = Describe("ChangeProcessor", func() { }, } - processor = newstate.NewChangeProcessor(types.NamespacedName{Namespace: "test", Name: "gateway"}) + processor = newstate.NewChangeProcessorImpl(types.NamespacedName{Namespace: "test", Name: "gateway"}) }) Describe("Process resources", Ordered, func() { @@ -304,10 +304,10 @@ var _ = Describe("ChangeProcessor", func() { }) Describe("Edge cases with panic", func() { - var processor *newstate.ChangeProcessor + var processor newstate.ChangeProcessor BeforeEach(func() { - processor = newstate.NewChangeProcessor(types.NamespacedName{Namespace: "test", Name: "gateway"}) + processor = newstate.NewChangeProcessorImpl(types.NamespacedName{Namespace: "test", Name: "gateway"}) }) DescribeTable("CaptureUpsertChange must panic", diff --git a/internal/newstate/configuration.go b/internal/newstate/configuration.go index 215da3c91f..e66347eb58 100644 --- a/internal/newstate/configuration.go +++ b/internal/newstate/configuration.go @@ -42,6 +42,15 @@ type MatchRule struct { Source *v1alpha2.HTTPRoute } +// GetMatch returns the HTTPRouteMatch of the Route and true if it exists. +// If there is no Match defined on the Route, GetMatch returns an empty HTTPRouteMatch and false. +func (r *MatchRule) GetMatch() (v1alpha2.HTTPRouteMatch, bool) { + if r.MatchIdx == -1 { + return v1alpha2.HTTPRouteMatch{}, false + } + return r.Source.Spec.Rules[r.RuleIdx].Matches[r.MatchIdx], true +} + // buildConfiguration builds the Configuration from the graph. func buildConfiguration(graph *graph) Configuration { // FIXME(pleshakov) For now we only handle paths with prefix matches. Handle exact and regex matches diff --git a/internal/newstate/configuration_test.go b/internal/newstate/configuration_test.go index f76d7d5e99..b45b11a385 100644 --- a/internal/newstate/configuration_test.go +++ b/internal/newstate/configuration_test.go @@ -403,3 +403,88 @@ func TestGetPath(t *testing.T) { } } } + +func TestMatchRuleGetMatch(t *testing.T) { + var hr = &v1alpha2.HTTPRoute{ + Spec: v1alpha2.HTTPRouteSpec{ + Rules: []v1alpha2.HTTPRouteRule{ + { + Matches: []v1alpha2.HTTPRouteMatch{ + { + Path: &v1alpha2.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path-1"), + }, + }, + { + Path: &v1alpha2.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path-2"), + }, + }, + }, + }, + { + Matches: []v1alpha2.HTTPRouteMatch{ + { + Path: &v1alpha2.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path-3"), + }, + }, + { + Path: &v1alpha2.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path-4"), + }, + }, + }, + }, + }, + }, + } + + tests := []struct { + name, + expPath string + rule MatchRule + matchExists bool + }{ + { + name: "match does not exist", + expPath: "", + rule: MatchRule{MatchIdx: -1}, + matchExists: false, + }, + { + name: "first match in first rule", + expPath: "/path-1", + rule: MatchRule{MatchIdx: 0, RuleIdx: 0, Source: hr}, + matchExists: true, + }, + { + name: "second match in first rule", + expPath: "/path-2", + rule: MatchRule{MatchIdx: 1, RuleIdx: 0, Source: hr}, + matchExists: true, + }, + { + name: "second match in second rule", + expPath: "/path-4", + rule: MatchRule{MatchIdx: 1, RuleIdx: 1, Source: hr}, + matchExists: true, + }, + } + + for _, tc := range tests { + actual, exists := tc.rule.GetMatch() + if !tc.matchExists { + if exists { + t.Errorf("rule.GetMatch() incorrectly returned true (match exists) for test case: %q", tc.name) + } + } else { + if !exists { + t.Errorf("rule.GetMatch() incorrectly returned false (match does not exist) for test case: %q", tc.name) + } + if *actual.Path.Value != tc.expPath { + t.Errorf("rule.GetMatch() returned incorrect match with path: %s, expected path: %s for test case: %q", *actual.Path.Value, tc.expPath, tc.name) + } + } + } +} diff --git a/internal/newstate/newstatefakes/fake_change_processor.go b/internal/newstate/newstatefakes/fake_change_processor.go new file mode 100644 index 0000000000..367b30a021 --- /dev/null +++ b/internal/newstate/newstatefakes/fake_change_processor.go @@ -0,0 +1,194 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package newstatefakes + +import ( + "sync" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type FakeChangeProcessor struct { + CaptureDeleteChangeStub func(client.Object, types.NamespacedName) + captureDeleteChangeMutex sync.RWMutex + captureDeleteChangeArgsForCall []struct { + arg1 client.Object + arg2 types.NamespacedName + } + CaptureUpsertChangeStub func(client.Object) + captureUpsertChangeMutex sync.RWMutex + captureUpsertChangeArgsForCall []struct { + arg1 client.Object + } + ProcessStub func() (bool, newstate.Configuration, newstate.Statuses) + processMutex sync.RWMutex + processArgsForCall []struct { + } + processReturns struct { + result1 bool + result2 newstate.Configuration + result3 newstate.Statuses + } + processReturnsOnCall map[int]struct { + result1 bool + result2 newstate.Configuration + result3 newstate.Statuses + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeChangeProcessor) CaptureDeleteChange(arg1 client.Object, arg2 types.NamespacedName) { + fake.captureDeleteChangeMutex.Lock() + fake.captureDeleteChangeArgsForCall = append(fake.captureDeleteChangeArgsForCall, struct { + arg1 client.Object + arg2 types.NamespacedName + }{arg1, arg2}) + stub := fake.CaptureDeleteChangeStub + fake.recordInvocation("CaptureDeleteChange", []interface{}{arg1, arg2}) + fake.captureDeleteChangeMutex.Unlock() + if stub != nil { + fake.CaptureDeleteChangeStub(arg1, arg2) + } +} + +func (fake *FakeChangeProcessor) CaptureDeleteChangeCallCount() int { + fake.captureDeleteChangeMutex.RLock() + defer fake.captureDeleteChangeMutex.RUnlock() + return len(fake.captureDeleteChangeArgsForCall) +} + +func (fake *FakeChangeProcessor) CaptureDeleteChangeCalls(stub func(client.Object, types.NamespacedName)) { + fake.captureDeleteChangeMutex.Lock() + defer fake.captureDeleteChangeMutex.Unlock() + fake.CaptureDeleteChangeStub = stub +} + +func (fake *FakeChangeProcessor) CaptureDeleteChangeArgsForCall(i int) (client.Object, types.NamespacedName) { + fake.captureDeleteChangeMutex.RLock() + defer fake.captureDeleteChangeMutex.RUnlock() + argsForCall := fake.captureDeleteChangeArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeChangeProcessor) CaptureUpsertChange(arg1 client.Object) { + fake.captureUpsertChangeMutex.Lock() + fake.captureUpsertChangeArgsForCall = append(fake.captureUpsertChangeArgsForCall, struct { + arg1 client.Object + }{arg1}) + stub := fake.CaptureUpsertChangeStub + fake.recordInvocation("CaptureUpsertChange", []interface{}{arg1}) + fake.captureUpsertChangeMutex.Unlock() + if stub != nil { + fake.CaptureUpsertChangeStub(arg1) + } +} + +func (fake *FakeChangeProcessor) CaptureUpsertChangeCallCount() int { + fake.captureUpsertChangeMutex.RLock() + defer fake.captureUpsertChangeMutex.RUnlock() + return len(fake.captureUpsertChangeArgsForCall) +} + +func (fake *FakeChangeProcessor) CaptureUpsertChangeCalls(stub func(client.Object)) { + fake.captureUpsertChangeMutex.Lock() + defer fake.captureUpsertChangeMutex.Unlock() + fake.CaptureUpsertChangeStub = stub +} + +func (fake *FakeChangeProcessor) CaptureUpsertChangeArgsForCall(i int) client.Object { + fake.captureUpsertChangeMutex.RLock() + defer fake.captureUpsertChangeMutex.RUnlock() + argsForCall := fake.captureUpsertChangeArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeChangeProcessor) Process() (bool, newstate.Configuration, newstate.Statuses) { + fake.processMutex.Lock() + ret, specificReturn := fake.processReturnsOnCall[len(fake.processArgsForCall)] + fake.processArgsForCall = append(fake.processArgsForCall, struct { + }{}) + stub := fake.ProcessStub + fakeReturns := fake.processReturns + fake.recordInvocation("Process", []interface{}{}) + fake.processMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1, ret.result2, ret.result3 + } + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 +} + +func (fake *FakeChangeProcessor) ProcessCallCount() int { + fake.processMutex.RLock() + defer fake.processMutex.RUnlock() + return len(fake.processArgsForCall) +} + +func (fake *FakeChangeProcessor) ProcessCalls(stub func() (bool, newstate.Configuration, newstate.Statuses)) { + fake.processMutex.Lock() + defer fake.processMutex.Unlock() + fake.ProcessStub = stub +} + +func (fake *FakeChangeProcessor) ProcessReturns(result1 bool, result2 newstate.Configuration, result3 newstate.Statuses) { + fake.processMutex.Lock() + defer fake.processMutex.Unlock() + fake.ProcessStub = nil + fake.processReturns = struct { + result1 bool + result2 newstate.Configuration + result3 newstate.Statuses + }{result1, result2, result3} +} + +func (fake *FakeChangeProcessor) ProcessReturnsOnCall(i int, result1 bool, result2 newstate.Configuration, result3 newstate.Statuses) { + fake.processMutex.Lock() + defer fake.processMutex.Unlock() + fake.ProcessStub = nil + if fake.processReturnsOnCall == nil { + fake.processReturnsOnCall = make(map[int]struct { + result1 bool + result2 newstate.Configuration + result3 newstate.Statuses + }) + } + fake.processReturnsOnCall[i] = struct { + result1 bool + result2 newstate.Configuration + result3 newstate.Statuses + }{result1, result2, result3} +} + +func (fake *FakeChangeProcessor) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.captureDeleteChangeMutex.RLock() + defer fake.captureDeleteChangeMutex.RUnlock() + fake.captureUpsertChangeMutex.RLock() + defer fake.captureUpsertChangeMutex.RUnlock() + fake.processMutex.RLock() + defer fake.processMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeChangeProcessor) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ newstate.ChangeProcessor = new(FakeChangeProcessor) diff --git a/internal/nginx/config/configfakes/fake_generator.go b/internal/nginx/config/configfakes/fake_generator.go index c02af23d70..20fb112719 100644 --- a/internal/nginx/config/configfakes/fake_generator.go +++ b/internal/nginx/config/configfakes/fake_generator.go @@ -4,21 +4,21 @@ package configfakes import ( "sync" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) type FakeGenerator struct { - GenerateForHostStub func(state.Host) ([]byte, config.Warnings) - generateForHostMutex sync.RWMutex - generateForHostArgsForCall []struct { - arg1 state.Host + GenerateStub func(newstate.Configuration) ([]byte, config.Warnings) + generateMutex sync.RWMutex + generateArgsForCall []struct { + arg1 newstate.Configuration } - generateForHostReturns struct { + generateReturns struct { result1 []byte result2 config.Warnings } - generateForHostReturnsOnCall map[int]struct { + generateReturnsOnCall map[int]struct { result1 []byte result2 config.Warnings } @@ -26,16 +26,16 @@ type FakeGenerator struct { invocationsMutex sync.RWMutex } -func (fake *FakeGenerator) GenerateForHost(arg1 state.Host) ([]byte, config.Warnings) { - fake.generateForHostMutex.Lock() - ret, specificReturn := fake.generateForHostReturnsOnCall[len(fake.generateForHostArgsForCall)] - fake.generateForHostArgsForCall = append(fake.generateForHostArgsForCall, struct { - arg1 state.Host +func (fake *FakeGenerator) Generate(arg1 newstate.Configuration) ([]byte, config.Warnings) { + fake.generateMutex.Lock() + ret, specificReturn := fake.generateReturnsOnCall[len(fake.generateArgsForCall)] + fake.generateArgsForCall = append(fake.generateArgsForCall, struct { + arg1 newstate.Configuration }{arg1}) - stub := fake.GenerateForHostStub - fakeReturns := fake.generateForHostReturns - fake.recordInvocation("GenerateForHost", []interface{}{arg1}) - fake.generateForHostMutex.Unlock() + stub := fake.GenerateStub + fakeReturns := fake.generateReturns + fake.recordInvocation("Generate", []interface{}{arg1}) + fake.generateMutex.Unlock() if stub != nil { return stub(arg1) } @@ -45,46 +45,46 @@ func (fake *FakeGenerator) GenerateForHost(arg1 state.Host) ([]byte, config.Warn return fakeReturns.result1, fakeReturns.result2 } -func (fake *FakeGenerator) GenerateForHostCallCount() int { - fake.generateForHostMutex.RLock() - defer fake.generateForHostMutex.RUnlock() - return len(fake.generateForHostArgsForCall) +func (fake *FakeGenerator) GenerateCallCount() int { + fake.generateMutex.RLock() + defer fake.generateMutex.RUnlock() + return len(fake.generateArgsForCall) } -func (fake *FakeGenerator) GenerateForHostCalls(stub func(state.Host) ([]byte, config.Warnings)) { - fake.generateForHostMutex.Lock() - defer fake.generateForHostMutex.Unlock() - fake.GenerateForHostStub = stub +func (fake *FakeGenerator) GenerateCalls(stub func(newstate.Configuration) ([]byte, config.Warnings)) { + fake.generateMutex.Lock() + defer fake.generateMutex.Unlock() + fake.GenerateStub = stub } -func (fake *FakeGenerator) GenerateForHostArgsForCall(i int) state.Host { - fake.generateForHostMutex.RLock() - defer fake.generateForHostMutex.RUnlock() - argsForCall := fake.generateForHostArgsForCall[i] +func (fake *FakeGenerator) GenerateArgsForCall(i int) newstate.Configuration { + fake.generateMutex.RLock() + defer fake.generateMutex.RUnlock() + argsForCall := fake.generateArgsForCall[i] return argsForCall.arg1 } -func (fake *FakeGenerator) GenerateForHostReturns(result1 []byte, result2 config.Warnings) { - fake.generateForHostMutex.Lock() - defer fake.generateForHostMutex.Unlock() - fake.GenerateForHostStub = nil - fake.generateForHostReturns = struct { +func (fake *FakeGenerator) GenerateReturns(result1 []byte, result2 config.Warnings) { + fake.generateMutex.Lock() + defer fake.generateMutex.Unlock() + fake.GenerateStub = nil + fake.generateReturns = struct { result1 []byte result2 config.Warnings }{result1, result2} } -func (fake *FakeGenerator) GenerateForHostReturnsOnCall(i int, result1 []byte, result2 config.Warnings) { - fake.generateForHostMutex.Lock() - defer fake.generateForHostMutex.Unlock() - fake.GenerateForHostStub = nil - if fake.generateForHostReturnsOnCall == nil { - fake.generateForHostReturnsOnCall = make(map[int]struct { +func (fake *FakeGenerator) GenerateReturnsOnCall(i int, result1 []byte, result2 config.Warnings) { + fake.generateMutex.Lock() + defer fake.generateMutex.Unlock() + fake.GenerateStub = nil + if fake.generateReturnsOnCall == nil { + fake.generateReturnsOnCall = make(map[int]struct { result1 []byte result2 config.Warnings }) } - fake.generateForHostReturnsOnCall[i] = struct { + fake.generateReturnsOnCall[i] = struct { result1 []byte result2 config.Warnings }{result1, result2} @@ -93,8 +93,8 @@ func (fake *FakeGenerator) GenerateForHostReturnsOnCall(i int, result1 []byte, r func (fake *FakeGenerator) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.generateForHostMutex.RLock() - defer fake.generateForHostMutex.RUnlock() + fake.generateMutex.RLock() + defer fake.generateMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/nginx/config/generator.go b/internal/nginx/config/generator.go index 91c7778c64..28075bebc7 100644 --- a/internal/nginx/config/generator.go +++ b/internal/nginx/config/generator.go @@ -8,6 +8,7 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) @@ -18,8 +19,8 @@ const nginx502Server = "unix:/var/lib/nginx/nginx-502-server.sock" // Generator generates NGINX configuration. type Generator interface { - // GenerateForHost generates configuration for a host. - GenerateForHost(host state.Host) ([]byte, Warnings) + // Generate generates NGINX configuration from internal representation. + Generate(configuration newstate.Configuration) ([]byte, Warnings) } // GeneratorImpl is an implementation of Generator @@ -36,20 +37,32 @@ func NewGeneratorImpl(serviceStore state.ServiceStore) *GeneratorImpl { } } -func (g *GeneratorImpl) GenerateForHost(host state.Host) ([]byte, Warnings) { - server, warnings := generate(host, g.serviceStore) - return g.executor.ExecuteForServer(server), warnings +func (g *GeneratorImpl) Generate(conf newstate.Configuration) ([]byte, Warnings) { + warnings := newWarnings() + + servers := httpServers{ + Servers: make([]server, 0, len(conf.HTTPServers)), + } + + for _, s := range conf.HTTPServers { + cfg, warns := generate(s, g.serviceStore) + + servers.Servers = append(servers.Servers, cfg) + warnings.Add(warns) + } + + return g.executor.ExecuteForHTTPServers(servers), warnings } -func generate(host state.Host, serviceStore state.ServiceStore) (server, Warnings) { +func generate(httpServer newstate.HTTPServer, serviceStore state.ServiceStore) (server, Warnings) { warnings := newWarnings() - locs := make([]location, 0, len(host.PathRouteGroups)) // FIXME(pleshakov): expand with g.Routes + locs := make([]location, 0, len(httpServer.PathRules)) // FIXME(pleshakov): expand with rules.Routes - for _, g := range host.PathRouteGroups { + for _, rules := range httpServer.PathRules { // number of routes in a group is always at least 1 // otherwise, it is a bug in the state.Configuration code, so it is OK to panic here - r := g.Routes[0] // FIXME(pleshakov): for now, we only handle the first route in case there are multiple routes + r := rules.MatchRules[0] // FIXME(pleshakov): for now, we only handle the first route in case there are multiple routes address, err := getBackendAddress(r.Source.Spec.Rules[r.RuleIdx].BackendRefs, r.Source.Namespace, serviceStore) if err != nil { warnings.AddWarning(r.Source, err.Error()) @@ -59,7 +72,7 @@ func generate(host state.Host, serviceStore state.ServiceStore) (server, Warning if exists && matchLocationNeeded(match) { // FIXME(kate-osborn): route index is hardcoded to 0 for now. // Once we support multiple routes we will need to change this to the index of the current route. - path := createPathForMatch(g.Path, 0) + path := createPathForMatch(rules.Path, 0) // generate location block for this rule and match mLoc := generateMatchLocation(path, address) // generate the http_matches variable value @@ -70,13 +83,13 @@ func generate(host state.Host, serviceStore state.ServiceStore) (server, Warning } loc := location{ - Path: g.Path, + Path: rules.Path, HTTPMatchVar: string(b), } locs = append(locs, loc, mLoc) } else { loc := location{ - Path: g.Path, + Path: rules.Path, ProxyPass: generateProxyPass(address), } locs = append(locs, loc) @@ -84,7 +97,7 @@ func generate(host state.Host, serviceStore state.ServiceStore) (server, Warning } return server{ - ServerName: host.Value, + ServerName: httpServer.Hostname, Locations: locs, }, warnings } @@ -168,7 +181,7 @@ func createHTTPMatch(match v1alpha2.HTTPRouteMatch, redirectPath string) httpMat if match.Headers != nil { headers := make([]string, 0, len(match.Headers)) - //FIXME(kate-osborn): For now we only support type "Exact". + // FIXME(kate-osborn): For now we only support type "Exact". for _, h := range match.Headers { if *h.Type == v1alpha2.HeaderMatchExact { headers = append(headers, createHeaderKeyValString(h)) @@ -180,7 +193,7 @@ func createHTTPMatch(match v1alpha2.HTTPRouteMatch, redirectPath string) httpMat if match.QueryParams != nil { params := make([]string, 0, len(match.QueryParams)) - //FIXME(kate-osborn): For now we only support type "Exact". + // FIXME(kate-osborn): For now we only support type "Exact". for _, p := range match.QueryParams { if *p.Type == v1alpha2.QueryParamMatchExact { params = append(params, createQueryParamKeyValString(p)) diff --git a/internal/nginx/config/generator_test.go b/internal/nginx/config/generator_test.go index d21f0f1ca9..1706fe478e 100644 --- a/internal/nginx/config/generator_test.go +++ b/internal/nginx/config/generator_test.go @@ -11,22 +11,28 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/newstate" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/statefakes" ) func TestGenerateForHost(t *testing.T) { generator := NewGeneratorImpl(&statefakes.FakeServiceStore{}) - host := state.Host{Value: "example.com"} + conf := newstate.Configuration{ + HTTPServers: []newstate.HTTPServer{ + { + Hostname: "example.com", + }, + }, + } - cfg, warnings := generator.GenerateForHost(host) + cfg, warnings := generator.Generate(conf) if len(cfg) == 0 { - t.Errorf("GenerateForHost() generated empty config") + t.Errorf("Generate() generated empty config") } if len(warnings) > 0 { - t.Errorf("GenerateForHost() returned unexpected warnings: %v", warnings) + t.Errorf("Generate() returned unexpected warnings: %v", warnings) } } @@ -127,12 +133,12 @@ func TestGenerate(t *testing.T) { }, } - host := state.Host{ - Value: "example.com", - PathRouteGroups: []state.PathRouteGroup{ + host := newstate.HTTPServer{ + Hostname: "example.com", + PathRules: []newstate.PathRule{ { Path: "/", - Routes: []state.Route{ + MatchRules: []newstate.MatchRule{ { MatchIdx: 0, RuleIdx: 0, @@ -142,7 +148,7 @@ func TestGenerate(t *testing.T) { }, { Path: "/test", - Routes: []state.Route{ + MatchRules: []newstate.MatchRule{ { MatchIdx: 0, RuleIdx: 1, @@ -152,7 +158,7 @@ func TestGenerate(t *testing.T) { }, { Path: "/path-only", - Routes: []state.Route{ + MatchRules: []newstate.MatchRule{ { MatchIdx: 0, RuleIdx: 2, diff --git a/internal/nginx/config/http.go b/internal/nginx/config/http.go index 8ba8dd76f2..0326e680d9 100644 --- a/internal/nginx/config/http.go +++ b/internal/nginx/config/http.go @@ -1,5 +1,9 @@ package config +type httpServers struct { + Servers []server +} + type server struct { ServerName string Locations []location diff --git a/internal/nginx/config/template.go b/internal/nginx/config/template.go index ae740612db..7e17e81833 100644 --- a/internal/nginx/config/template.go +++ b/internal/nginx/config/template.go @@ -6,10 +6,11 @@ import ( "text/template" ) -var serverTemplate = `server { - server_name {{ .ServerName }}; +var httpServersTemplate = `{{ range $s := .Servers }} +server { + server_name {{ $s.ServerName }}; - {{ range $l := .Locations }} + {{ range $l := $s.Locations }} location {{ $l.Path }} { {{ if $l.Internal }} internal; @@ -28,31 +29,32 @@ var serverTemplate = `server { } {{ end }} } +{{ end }} ` // templateExecutor generates NGINX configuration using a template. // Template parsing or executing errors can only occur if there is a bug in the template, so they are handled with panics. -// For now, we only generate configuration with NGINX server, but in the future we will also need to generate -// the main NGINX configuration file, upstreams. +// For now, we only generate configuration with NGINX http servers, but in the future we will also need to generate +// the main NGINX configuration file, upstreams, stream servers. type templateExecutor struct { - serverTemplate *template.Template + httpServersTemplate *template.Template } func newTemplateExecutor() *templateExecutor { - t, err := template.New("server").Parse(serverTemplate) + t, err := template.New("server").Parse(httpServersTemplate) if err != nil { - panic(fmt.Errorf("failed to parse server template: %w", err)) + panic(fmt.Errorf("failed to parse http servers template: %w", err)) } - return &templateExecutor{serverTemplate: t} + return &templateExecutor{httpServersTemplate: t} } -func (e *templateExecutor) ExecuteForServer(s server) []byte { +func (e *templateExecutor) ExecuteForHTTPServers(servers httpServers) []byte { var buf bytes.Buffer - err := e.serverTemplate.Execute(&buf, s) + err := e.httpServersTemplate.Execute(&buf, servers) if err != nil { - panic(fmt.Errorf("failed to execute server template: %w", err)) + panic(fmt.Errorf("failed to execute http servers template: %w", err)) } return buf.Bytes() diff --git a/internal/nginx/config/template_test.go b/internal/nginx/config/template_test.go index bb03edad9e..2089dae4be 100644 --- a/internal/nginx/config/template_test.go +++ b/internal/nginx/config/template_test.go @@ -8,17 +8,21 @@ import ( func TestExecuteForServer(t *testing.T) { executor := newTemplateExecutor() - server := server{ - ServerName: "example.com", - Locations: []location{ + servers := httpServers{ + Servers: []server{ { - Path: "/", - ProxyPass: "http://10.0.0.1", + ServerName: "example.com", + Locations: []location{ + { + Path: "/", + ProxyPass: "http://10.0.0.1", + }, + }, }, }, } - cfg := executor.ExecuteForServer(server) + cfg := executor.ExecuteForHTTPServers(servers) // we only do a sanity check here. // the config generation logic is tested in the Generator tests. if len(cfg) == 0 { @@ -34,7 +38,7 @@ func TestNewTemplateExecutorPanics(t *testing.T) { } }() - serverTemplate = "{{ end }}" // invalid template + httpServersTemplate = "{{ end }}" // invalid template newTemplateExecutor() } @@ -51,7 +55,7 @@ func TestExecuteForServerPanics(t *testing.T) { t.Fatalf("Failed to parse template: %v", err) } - executor := &templateExecutor{serverTemplate: tmpl} + executor := &templateExecutor{httpServersTemplate: tmpl} - _ = executor.ExecuteForServer(server{}) + _ = executor.ExecuteForHTTPServers(httpServers{}) } diff --git a/internal/nginx/config/warnings.go b/internal/nginx/config/warnings.go index a1925cd865..a9d3d404da 100644 --- a/internal/nginx/config/warnings.go +++ b/internal/nginx/config/warnings.go @@ -22,3 +22,11 @@ func (w Warnings) AddWarningf(obj client.Object, msgFmt string, args ...interfac func (w Warnings) AddWarning(obj client.Object, msg string) { w[obj] = append(w[obj], msg) } + +// Add adds new Warnings to the map. +// Warnings for the same object are merged. +func (w Warnings) Add(warnings Warnings) { + for k, v := range warnings { + w[k] = append(w[k], v...) + } +} diff --git a/internal/nginx/config/warnings_test.go b/internal/nginx/config/warnings_test.go index 2d20a61a73..7cacc4fffa 100644 --- a/internal/nginx/config/warnings_test.go +++ b/internal/nginx/config/warnings_test.go @@ -44,3 +44,89 @@ func TestAddWarning(t *testing.T) { t.Errorf("AddWarning mismatch (-want +got):\n%s", diff) } } + +func TestAdd(t *testing.T) { + obj1 := &v1alpha2.HTTPRoute{} + obj2 := &v1alpha2.HTTPRoute{} + obj3 := &v1alpha2.HTTPRoute{} + + tests := []struct { + warnings Warnings + addedWarnings Warnings + expected Warnings + msg string + }{ + { + warnings: newWarnings(), + addedWarnings: newWarnings(), + expected: newWarnings(), + msg: "empty warnings", + }, + { + warnings: Warnings{ + obj1: []string{ + "first", + }, + }, + addedWarnings: newWarnings(), + expected: Warnings{ + obj1: []string{ + "first", + }, + }, + msg: "empty added warnings", + }, + { + warnings: newWarnings(), + addedWarnings: Warnings{ + obj1: []string{ + "first", + }, + }, + expected: Warnings{ + obj1: []string{ + "first", + }, + }, + msg: "empty warnings", + }, + { + warnings: Warnings{ + obj1: []string{ + "first 1", + }, + obj3: []string{ + "first 3", + }, + }, + addedWarnings: Warnings{ + obj2: []string{ + "first 2", + }, + obj3: []string{ + "second 3", + }, + }, + expected: Warnings{ + obj1: []string{ + "first 1", + }, + obj2: []string{ + "first 2", + }, + obj3: []string{ + "first 3", + "second 3", + }, + }, + msg: "adding and merging", + }, + } + + for _, test := range tests { + test.warnings.Add(test.addedWarnings) + if diff := cmp.Diff(test.expected, test.warnings); diff != "" { + t.Errorf("Add() %q mismatch (-want +got):\n%s", test.msg, diff) + } + } +} diff --git a/internal/nginx/file/filefakes/fake_manager.go b/internal/nginx/file/filefakes/fake_manager.go index c65078d846..179abdc995 100644 --- a/internal/nginx/file/filefakes/fake_manager.go +++ b/internal/nginx/file/filefakes/fake_manager.go @@ -8,110 +8,38 @@ import ( ) type FakeManager struct { - DeleteServerConfigStub func(string) error - deleteServerConfigMutex sync.RWMutex - deleteServerConfigArgsForCall []struct { - arg1 string - } - deleteServerConfigReturns struct { - result1 error - } - deleteServerConfigReturnsOnCall map[int]struct { - result1 error - } - WriteServerConfigStub func(string, []byte) error - writeServerConfigMutex sync.RWMutex - writeServerConfigArgsForCall []struct { + WriteHTTPServersConfigStub func(string, []byte) error + writeHTTPServersConfigMutex sync.RWMutex + writeHTTPServersConfigArgsForCall []struct { arg1 string arg2 []byte } - writeServerConfigReturns struct { + writeHTTPServersConfigReturns struct { result1 error } - writeServerConfigReturnsOnCall map[int]struct { + writeHTTPServersConfigReturnsOnCall map[int]struct { result1 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *FakeManager) DeleteServerConfig(arg1 string) error { - fake.deleteServerConfigMutex.Lock() - ret, specificReturn := fake.deleteServerConfigReturnsOnCall[len(fake.deleteServerConfigArgsForCall)] - fake.deleteServerConfigArgsForCall = append(fake.deleteServerConfigArgsForCall, struct { - arg1 string - }{arg1}) - stub := fake.DeleteServerConfigStub - fakeReturns := fake.deleteServerConfigReturns - fake.recordInvocation("DeleteServerConfig", []interface{}{arg1}) - fake.deleteServerConfigMutex.Unlock() - if stub != nil { - return stub(arg1) - } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 -} - -func (fake *FakeManager) DeleteServerConfigCallCount() int { - fake.deleteServerConfigMutex.RLock() - defer fake.deleteServerConfigMutex.RUnlock() - return len(fake.deleteServerConfigArgsForCall) -} - -func (fake *FakeManager) DeleteServerConfigCalls(stub func(string) error) { - fake.deleteServerConfigMutex.Lock() - defer fake.deleteServerConfigMutex.Unlock() - fake.DeleteServerConfigStub = stub -} - -func (fake *FakeManager) DeleteServerConfigArgsForCall(i int) string { - fake.deleteServerConfigMutex.RLock() - defer fake.deleteServerConfigMutex.RUnlock() - argsForCall := fake.deleteServerConfigArgsForCall[i] - return argsForCall.arg1 -} - -func (fake *FakeManager) DeleteServerConfigReturns(result1 error) { - fake.deleteServerConfigMutex.Lock() - defer fake.deleteServerConfigMutex.Unlock() - fake.DeleteServerConfigStub = nil - fake.deleteServerConfigReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeManager) DeleteServerConfigReturnsOnCall(i int, result1 error) { - fake.deleteServerConfigMutex.Lock() - defer fake.deleteServerConfigMutex.Unlock() - fake.DeleteServerConfigStub = nil - if fake.deleteServerConfigReturnsOnCall == nil { - fake.deleteServerConfigReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.deleteServerConfigReturnsOnCall[i] = struct { - result1 error - }{result1} -} - -func (fake *FakeManager) WriteServerConfig(arg1 string, arg2 []byte) error { +func (fake *FakeManager) WriteHTTPServersConfig(arg1 string, arg2 []byte) error { var arg2Copy []byte if arg2 != nil { arg2Copy = make([]byte, len(arg2)) copy(arg2Copy, arg2) } - fake.writeServerConfigMutex.Lock() - ret, specificReturn := fake.writeServerConfigReturnsOnCall[len(fake.writeServerConfigArgsForCall)] - fake.writeServerConfigArgsForCall = append(fake.writeServerConfigArgsForCall, struct { + fake.writeHTTPServersConfigMutex.Lock() + ret, specificReturn := fake.writeHTTPServersConfigReturnsOnCall[len(fake.writeHTTPServersConfigArgsForCall)] + fake.writeHTTPServersConfigArgsForCall = append(fake.writeHTTPServersConfigArgsForCall, struct { arg1 string arg2 []byte }{arg1, arg2Copy}) - stub := fake.WriteServerConfigStub - fakeReturns := fake.writeServerConfigReturns - fake.recordInvocation("WriteServerConfig", []interface{}{arg1, arg2Copy}) - fake.writeServerConfigMutex.Unlock() + stub := fake.WriteHTTPServersConfigStub + fakeReturns := fake.writeHTTPServersConfigReturns + fake.recordInvocation("WriteHTTPServersConfig", []interface{}{arg1, arg2Copy}) + fake.writeHTTPServersConfigMutex.Unlock() if stub != nil { return stub(arg1, arg2) } @@ -121,44 +49,44 @@ func (fake *FakeManager) WriteServerConfig(arg1 string, arg2 []byte) error { return fakeReturns.result1 } -func (fake *FakeManager) WriteServerConfigCallCount() int { - fake.writeServerConfigMutex.RLock() - defer fake.writeServerConfigMutex.RUnlock() - return len(fake.writeServerConfigArgsForCall) +func (fake *FakeManager) WriteHTTPServersConfigCallCount() int { + fake.writeHTTPServersConfigMutex.RLock() + defer fake.writeHTTPServersConfigMutex.RUnlock() + return len(fake.writeHTTPServersConfigArgsForCall) } -func (fake *FakeManager) WriteServerConfigCalls(stub func(string, []byte) error) { - fake.writeServerConfigMutex.Lock() - defer fake.writeServerConfigMutex.Unlock() - fake.WriteServerConfigStub = stub +func (fake *FakeManager) WriteHTTPServersConfigCalls(stub func(string, []byte) error) { + fake.writeHTTPServersConfigMutex.Lock() + defer fake.writeHTTPServersConfigMutex.Unlock() + fake.WriteHTTPServersConfigStub = stub } -func (fake *FakeManager) WriteServerConfigArgsForCall(i int) (string, []byte) { - fake.writeServerConfigMutex.RLock() - defer fake.writeServerConfigMutex.RUnlock() - argsForCall := fake.writeServerConfigArgsForCall[i] +func (fake *FakeManager) WriteHTTPServersConfigArgsForCall(i int) (string, []byte) { + fake.writeHTTPServersConfigMutex.RLock() + defer fake.writeHTTPServersConfigMutex.RUnlock() + argsForCall := fake.writeHTTPServersConfigArgsForCall[i] return argsForCall.arg1, argsForCall.arg2 } -func (fake *FakeManager) WriteServerConfigReturns(result1 error) { - fake.writeServerConfigMutex.Lock() - defer fake.writeServerConfigMutex.Unlock() - fake.WriteServerConfigStub = nil - fake.writeServerConfigReturns = struct { +func (fake *FakeManager) WriteHTTPServersConfigReturns(result1 error) { + fake.writeHTTPServersConfigMutex.Lock() + defer fake.writeHTTPServersConfigMutex.Unlock() + fake.WriteHTTPServersConfigStub = nil + fake.writeHTTPServersConfigReturns = struct { result1 error }{result1} } -func (fake *FakeManager) WriteServerConfigReturnsOnCall(i int, result1 error) { - fake.writeServerConfigMutex.Lock() - defer fake.writeServerConfigMutex.Unlock() - fake.WriteServerConfigStub = nil - if fake.writeServerConfigReturnsOnCall == nil { - fake.writeServerConfigReturnsOnCall = make(map[int]struct { +func (fake *FakeManager) WriteHTTPServersConfigReturnsOnCall(i int, result1 error) { + fake.writeHTTPServersConfigMutex.Lock() + defer fake.writeHTTPServersConfigMutex.Unlock() + fake.WriteHTTPServersConfigStub = nil + if fake.writeHTTPServersConfigReturnsOnCall == nil { + fake.writeHTTPServersConfigReturnsOnCall = make(map[int]struct { result1 error }) } - fake.writeServerConfigReturnsOnCall[i] = struct { + fake.writeHTTPServersConfigReturnsOnCall[i] = struct { result1 error }{result1} } @@ -166,10 +94,8 @@ func (fake *FakeManager) WriteServerConfigReturnsOnCall(i int, result1 error) { func (fake *FakeManager) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.deleteServerConfigMutex.RLock() - defer fake.deleteServerConfigMutex.RUnlock() - fake.writeServerConfigMutex.RLock() - defer fake.writeServerConfigMutex.RUnlock() + fake.writeHTTPServersConfigMutex.RLock() + defer fake.writeHTTPServersConfigMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/nginx/file/manager.go b/internal/nginx/file/manager.go index 947cde4ede..a4901271b1 100644 --- a/internal/nginx/file/manager.go +++ b/internal/nginx/file/manager.go @@ -12,12 +12,10 @@ const confdFolder = "/etc/nginx/conf.d" // Manager manages NGINX configuration files. type Manager interface { - // WriteServerConfig writes the server config on the file system. - // The name distinguishes this server config among all other server configs. For that, it must be unique. + // WriteHTTPServersConfig writes the http servers config on the file system. + // The name distinguishes this config among all other configs. For that, it must be unique. // Note that name is not the name of the corresponding configuration file. - WriteServerConfig(name string, cfg []byte) error - // DeleteServerConfig deletes the corresponding configuration file for the server from the file system. - DeleteServerConfig(name string) error + WriteHTTPServersConfig(name string, cfg []byte) error } // ManagerImpl is an implementation of Manager. @@ -28,7 +26,7 @@ func NewManagerImpl() *ManagerImpl { return &ManagerImpl{} } -func (m *ManagerImpl) WriteServerConfig(name string, cfg []byte) error { +func (m *ManagerImpl) WriteHTTPServersConfig(name string, cfg []byte) error { path := getPathForServerConfig(name) file, err := os.Create(path) @@ -46,17 +44,6 @@ func (m *ManagerImpl) WriteServerConfig(name string, cfg []byte) error { return nil } -func (m *ManagerImpl) DeleteServerConfig(name string) error { - path := getPathForServerConfig(name) - - err := os.Remove(path) - if err != nil { - return fmt.Errorf("failed to remove server config %s: %w", path, err) - } - - return nil -} - func getPathForServerConfig(name string) string { return filepath.Join(confdFolder, name+".conf") } diff --git a/pkg/sdk/gateway_controller.go b/pkg/sdk/gateway_controller.go index 6c931e5c37..e18ad483b3 100644 --- a/pkg/sdk/gateway_controller.go +++ b/pkg/sdk/gateway_controller.go @@ -47,7 +47,7 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req reconcile.Request } if !found { - r.impl.Remove(req.Name) + r.impl.Remove(req.NamespacedName) return reconcile.Result{}, nil } diff --git a/pkg/sdk/interfaces.go b/pkg/sdk/interfaces.go index 841d86ab08..88cf168dd9 100644 --- a/pkg/sdk/interfaces.go +++ b/pkg/sdk/interfaces.go @@ -15,7 +15,7 @@ type GatewayClassImpl interface { type GatewayImpl interface { Upsert(*v1alpha2.Gateway) - Remove(string) + Remove(types.NamespacedName) } type GatewayConfigImpl interface {