diff --git a/internal/events/loop.go b/internal/events/loop.go index 0c4991b3ef..47a3e9e1de 100644 --- a/internal/events/loop.go +++ b/internal/events/loop.go @@ -5,10 +5,10 @@ import ( "fmt" "github.com/go-logr/logr" + "github.com/nginxinc/nginx-gateway-kubernetes/internal/nginx/config" "github.com/nginxinc/nginx-gateway-kubernetes/internal/state" "github.com/nginxinc/nginx-gateway-kubernetes/internal/status" apiv1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -16,17 +16,25 @@ import ( type EventLoop struct { conf state.Configuration serviceStore state.ServiceStore + generator config.Generator eventCh <-chan interface{} logger logr.Logger statusUpdater status.Updater } // NewEventLoop creates a new EventLoop. -func NewEventLoop(conf state.Configuration, serviceStore state.ServiceStore, eventCh <-chan interface{}, - statusUpdater status.Updater, logger logr.Logger) *EventLoop { +func NewEventLoop( + conf state.Configuration, + serviceStore state.ServiceStore, + generator config.Generator, + eventCh <-chan interface{}, + statusUpdater status.Updater, + logger logr.Logger, +) *EventLoop { return &EventLoop{ conf: conf, serviceStore: serviceStore, + generator: generator, eventCh: eventCh, statusUpdater: statusUpdater, logger: logger.WithName("eventLoop"), @@ -114,38 +122,19 @@ func (el *EventLoop) processChangesAndStatusUpdates(ctx context.Context, changes fmt.Printf("%+v\n", c) if c.Op == state.Upsert { - // The code below resolves service backend refs into their cluster IPs - // TO-DO: this code will be removed once we have the component that generates NGINX config and - // uses the ServiceStore to resolve services. - for _, g := range c.Host.PathRouteGroups { - for _, r := range g.Routes { - for _, b := range r.Source.Spec.Rules[r.RuleIdx].BackendRefs { - if b.BackendRef.Kind == nil || *b.BackendRef.Kind == "Service" { - ns := r.Source.Namespace - if b.BackendRef.Namespace != nil { - ns = string(*b.BackendRef.Namespace) - } - - address, err := el.serviceStore.Resolve(types.NamespacedName{ - Namespace: ns, - Name: string(b.BackendRef.Name), - }) - - if err != nil { - fmt.Printf("Service %s/%s error: %v\n", ns, b.BackendRef.Name, err) - continue - } - - var port int32 = 80 - if b.BackendRef.Port != nil { - port = int32(*b.BackendRef.Port) - } - - address = fmt.Sprintf("%s:%d", address, port) - - fmt.Printf("Service %s/%s: %s\n", ns, b.BackendRef.Name, address) - } - } + cfg, warnings := el.generator.GenerateForHost(c.Host) + // TO-DO: for now, we only print the generated config, without writing it on the file system + // and reloading NGINX. + fmt.Println(string(cfg)) + + for obj, objWarnings := range warnings { + for _, w := range objWarnings { + // TO-DO: 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) } } } diff --git a/internal/events/loop_test.go b/internal/events/loop_test.go index c1dc6514af..0b2b2d2129 100644 --- a/internal/events/loop_test.go +++ b/internal/events/loop_test.go @@ -4,6 +4,7 @@ import ( "context" "github.com/nginxinc/nginx-gateway-kubernetes/internal/events" + "github.com/nginxinc/nginx-gateway-kubernetes/internal/nginx/config/configfakes" "github.com/nginxinc/nginx-gateway-kubernetes/internal/state" "github.com/nginxinc/nginx-gateway-kubernetes/internal/state/statefakes" "github.com/nginxinc/nginx-gateway-kubernetes/internal/status/statusfakes" @@ -35,6 +36,7 @@ var _ = Describe("EventLoop", func() { var fakeConf *statefakes.FakeConfiguration var fakeUpdater *statusfakes.FakeUpdater var fakeServiceStore *statefakes.FakeServiceStore + var fakeGenerator *configfakes.FakeGenerator var cancel context.CancelFunc var eventCh chan interface{} var errorCh chan error @@ -44,7 +46,8 @@ var _ = Describe("EventLoop", func() { eventCh = make(chan interface{}) fakeUpdater = &statusfakes.FakeUpdater{} fakeServiceStore = &statefakes.FakeServiceStore{} - ctrl = events.NewEventLoop(fakeConf, fakeServiceStore, eventCh, fakeUpdater, zap.New()) + fakeGenerator = &configfakes.FakeGenerator{} + ctrl = events.NewEventLoop(fakeConf, fakeServiceStore, fakeGenerator, eventCh, fakeUpdater, zap.New()) var ctx context.Context @@ -66,15 +69,19 @@ var _ = Describe("EventLoop", func() { }) It("should process upsert event", func() { + fakeChanges := []state.Change{ + { + Op: state.Upsert, + Host: state.Host{}, + }, + } fakeStatusUpdates := []state.StatusUpdate{ { NamespacedName: types.NamespacedName{}, Status: nil, }, } - // for now, we pass nil, because we don't need to test how EventLoop processes changes yet. We will start - // testing once we have NGINX Configuration Manager component. - fakeConf.UpsertHTTPRouteReturns(nil, fakeStatusUpdates) + fakeConf.UpsertHTTPRouteReturns(fakeChanges, fakeStatusUpdates) hr := &v1alpha2.HTTPRoute{} @@ -87,23 +94,30 @@ var _ = Describe("EventLoop", func() { return fakeConf.UpsertHTTPRouteArgsForCall(0) }).Should(Equal(hr)) - Eventually(fakeUpdater.ProcessStatusUpdatesCallCount()).Should(Equal(1)) + Eventually(fakeUpdater.ProcessStatusUpdatesCallCount).Should(Equal(1)) Eventually(func() []state.StatusUpdate { _, updates := fakeUpdater.ProcessStatusUpdatesArgsForCall(0) return updates }).Should(Equal(fakeStatusUpdates)) + + Eventually(fakeGenerator.GenerateForHostCallCount).Should(Equal(1)) + Expect(fakeGenerator.GenerateForHostArgsForCall(0)).Should(Equal(fakeChanges[0].Host)) }) It("should process delete event", func() { + fakeChanges := []state.Change{ + { + Op: state.Delete, + Host: state.Host{}, + }, + } fakeStatusUpdates := []state.StatusUpdate{ { NamespacedName: types.NamespacedName{}, Status: nil, }, } - // for now, we pass nil, because we don't need to test how EventLoop processes changes yet. We will start - // testing once we have NGINX Configuration Manager component. - fakeConf.DeleteHTTPRouteReturns(nil, fakeStatusUpdates) + fakeConf.DeleteHTTPRouteReturns(fakeChanges, fakeStatusUpdates) nsname := types.NamespacedName{Namespace: "test", Name: "route"} @@ -117,11 +131,15 @@ var _ = Describe("EventLoop", func() { return fakeConf.DeleteHTTPRouteArgsForCall(0) }).Should(Equal(nsname)) - Eventually(fakeUpdater.ProcessStatusUpdatesCallCount()).Should(Equal(1)) + Eventually(fakeUpdater.ProcessStatusUpdatesCallCount).Should(Equal(1)) Eventually(func() []state.StatusUpdate { _, updates := fakeUpdater.ProcessStatusUpdatesArgsForCall(0) return updates }).Should(Equal(fakeStatusUpdates)) + + // TO-DO: + // once we have a component that processes host deletion, ensure that + // its corresponding method is eventually called }) }) diff --git a/internal/helpers/helpers.go b/internal/helpers/helpers.go index 3196bdd0ce..a38009f787 100644 --- a/internal/helpers/helpers.go +++ b/internal/helpers/helpers.go @@ -13,3 +13,13 @@ func Diff(x, y interface{}) string { } return r } + +// GetStringPointer takes a string and returns a pointer to it. Useful in unit tests when initializing structs. +func GetStringPointer(s string) *string { + return &s +} + +// GetInt32Pointer takes an int32 and returns a pointer to it. Useful in unit tests when initializing structs. +func GetInt32Pointer(i int32) *int32 { + return &i +} diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 67b435dc64..0c09d0e1c9 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -11,6 +11,7 @@ import ( gcfg "github.com/nginxinc/nginx-gateway-kubernetes/internal/implementations/gatewayconfig" hr "github.com/nginxinc/nginx-gateway-kubernetes/internal/implementations/httproute" svc "github.com/nginxinc/nginx-gateway-kubernetes/internal/implementations/service" + ngxcfg "github.com/nginxinc/nginx-gateway-kubernetes/internal/nginx/config" "github.com/nginxinc/nginx-gateway-kubernetes/internal/state" "github.com/nginxinc/nginx-gateway-kubernetes/internal/status" nginxgwv1alpha1 "github.com/nginxinc/nginx-gateway-kubernetes/pkg/apis/gateway/v1alpha1" @@ -76,7 +77,8 @@ func Start(cfg config.Config) error { conf := state.NewConfiguration(cfg.GatewayCtlrName, state.NewRealClock()) serviceStore := state.NewServiceStore() reporter := status.NewUpdater(mgr.GetClient(), cfg.Logger) - eventLoop := events.NewEventLoop(conf, serviceStore, eventCh, reporter, cfg.Logger) + configGenerator := ngxcfg.NewGeneratorImpl(serviceStore) + eventLoop := events.NewEventLoop(conf, serviceStore, configGenerator, eventCh, reporter, cfg.Logger) err = mgr.Add(eventLoop) if err != nil { diff --git a/internal/nginx/config/configfakes/fake_generator.go b/internal/nginx/config/configfakes/fake_generator.go new file mode 100644 index 0000000000..ff7a05b688 --- /dev/null +++ b/internal/nginx/config/configfakes/fake_generator.go @@ -0,0 +1,117 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package configfakes + +import ( + "sync" + + "github.com/nginxinc/nginx-gateway-kubernetes/internal/nginx/config" + "github.com/nginxinc/nginx-gateway-kubernetes/internal/state" +) + +type FakeGenerator struct { + GenerateForHostStub func(state.Host) ([]byte, config.Warnings) + generateForHostMutex sync.RWMutex + generateForHostArgsForCall []struct { + arg1 state.Host + } + generateForHostReturns struct { + result1 []byte + result2 config.Warnings + } + generateForHostReturnsOnCall map[int]struct { + result1 []byte + result2 config.Warnings + } + invocations map[string][][]interface{} + 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 + }{arg1}) + stub := fake.GenerateForHostStub + fakeReturns := fake.generateForHostReturns + fake.recordInvocation("GenerateForHost", []interface{}{arg1}) + fake.generateForHostMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeGenerator) GenerateForHostCallCount() int { + fake.generateForHostMutex.RLock() + defer fake.generateForHostMutex.RUnlock() + return len(fake.generateForHostArgsForCall) +} + +func (fake *FakeGenerator) GenerateForHostCalls(stub func(state.Host) ([]byte, config.Warnings)) { + fake.generateForHostMutex.Lock() + defer fake.generateForHostMutex.Unlock() + fake.GenerateForHostStub = stub +} + +func (fake *FakeGenerator) GenerateForHostArgsForCall(i int) state.Host { + fake.generateForHostMutex.RLock() + defer fake.generateForHostMutex.RUnlock() + argsForCall := fake.generateForHostArgsForCall[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 { + 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 { + result1 []byte + result2 config.Warnings + }) + } + fake.generateForHostReturnsOnCall[i] = struct { + result1 []byte + result2 config.Warnings + }{result1, result2} +} + +func (fake *FakeGenerator) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.generateForHostMutex.RLock() + defer fake.generateForHostMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeGenerator) 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 _ config.Generator = new(FakeGenerator) diff --git a/internal/nginx/config/generator.go b/internal/nginx/config/generator.go new file mode 100644 index 0000000000..c103a2ea34 --- /dev/null +++ b/internal/nginx/config/generator.go @@ -0,0 +1,104 @@ +package config + +import ( + "errors" + "fmt" + + "github.com/nginxinc/nginx-gateway-kubernetes/internal/state" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +// nginx502Server is used as a backend for services that cannot be resolved (have no IP address). +const nginx502Server = "unix:/var/lib/nginx/nginx-502-server.sock" + +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Generator + +// Generator generates NGINX configuration. +type Generator interface { + // GenerateForHost generates configuration for a host. + GenerateForHost(host state.Host) ([]byte, Warnings) +} + +// GeneratorImpl is an implementation of Generator +type GeneratorImpl struct { + executor *templateExecutor + serviceStore state.ServiceStore +} + +// NewGeneratorImpl creates a new GeneratorImpl. +func NewGeneratorImpl(serviceStore state.ServiceStore) *GeneratorImpl { + return &GeneratorImpl{ + executor: newTemplateExecutor(), + serviceStore: serviceStore, + } +} + +func (g *GeneratorImpl) GenerateForHost(host state.Host) ([]byte, Warnings) { + server, warnings := generate(host, g.serviceStore) + return g.executor.ExecuteForServer(server), warnings +} + +func generate(host state.Host, serviceStore state.ServiceStore) (server, Warnings) { + warnings := newWarnings() + + locs := make([]location, 0, len(host.PathRouteGroups)) // TO-DO: expand with g.Routes + + for _, g := range host.PathRouteGroups { + // 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] // TO-DO: 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()) + } + + loc := location{ + Path: g.Path, + ProxyPass: generateProxyPass(address), + } + locs = append(locs, loc) + } + + return server{ + ServerName: host.Value, + Locations: locs, + }, warnings +} + +func generateProxyPass(address string) string { + if address == "" { + return "http://" + nginx502Server + } + return "http://" + address +} + +func getBackendAddress(refs []v1alpha2.HTTPBackendRef, parentNS string, serviceStore state.ServiceStore) (string, error) { + // TO-DO: make sure the warnings are generated and reported to the user fot the edge cases + if len(refs) == 0 { + return "", errors.New("empty backend refs") + } + + // TO-DO: for now, we only support a single backend reference + ref := refs[0].BackendRef + + if ref.Kind != nil && *ref.Kind != "Service" { + return "", fmt.Errorf("unsupported kind %s", *ref.Kind) + } + + ns := parentNS + if ref.Namespace != nil { + ns = string(*ref.Namespace) + } + + address, err := serviceStore.Resolve(types.NamespacedName{Namespace: ns, Name: string(ref.Name)}) + if err != nil { + return "", fmt.Errorf("service %s/%s cannot be resolved: %w", ns, ref.Name, err) + } + + if ref.Port == nil { + return "", errors.New("port is nil") + } + + return fmt.Sprintf("%s:%d", address, *ref.Port), nil +} diff --git a/internal/nginx/config/generator_test.go b/internal/nginx/config/generator_test.go new file mode 100644 index 0000000000..ca959660fc --- /dev/null +++ b/internal/nginx/config/generator_test.go @@ -0,0 +1,320 @@ +package config + +import ( + "errors" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/nginxinc/nginx-gateway-kubernetes/internal/helpers" + "github.com/nginxinc/nginx-gateway-kubernetes/internal/state" + "github.com/nginxinc/nginx-gateway-kubernetes/internal/state/statefakes" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +func TestGenerateForHost(t *testing.T) { + generator := NewGeneratorImpl(&statefakes.FakeServiceStore{}) + + host := state.Host{Value: "example.com"} + + cfg, warnings := generator.GenerateForHost(host) + + if len(cfg) == 0 { + t.Errorf("GenerateForHost() generated empty config") + } + if len(warnings) > 0 { + t.Errorf("GenerateForHost() returned unexpected warnings: %v", warnings) + } +} + +func TestGenerate(t *testing.T) { + hr := &v1alpha2.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "route1", + }, + Spec: v1alpha2.HTTPRouteSpec{ + Hostnames: []v1alpha2.Hostname{ + "cafe.example.com", + }, + Rules: []v1alpha2.HTTPRouteRule{ + { + Matches: []v1alpha2.HTTPRouteMatch{ + { + Path: &v1alpha2.HTTPPathMatch{ + Value: helpers.GetStringPointer("/"), + }, + }, + }, + BackendRefs: []v1alpha2.HTTPBackendRef{ + { + BackendRef: v1alpha2.BackendRef{ + BackendObjectReference: v1alpha2.BackendObjectReference{ + Name: "service1", + Namespace: (*v1alpha2.Namespace)(helpers.GetStringPointer("test")), + Port: (*v1alpha2.PortNumber)(helpers.GetInt32Pointer(80)), + }, + }, + }, + }, + }, + { + Matches: []v1alpha2.HTTPRouteMatch{ + { + Path: &v1alpha2.HTTPPathMatch{ + Value: helpers.GetStringPointer("/test"), + }, + }, + }, + BackendRefs: nil, // no backend refs will cause warnings + }, + }, + }, + } + + host := state.Host{ + Value: "example.com", + PathRouteGroups: []state.PathRouteGroup{ + { + Path: "/", + Routes: []state.Route{ + { + MatchIdx: 0, + RuleIdx: 0, + Source: hr, + }, + }, + }, + { + Path: "/test", + Routes: []state.Route{ + { + MatchIdx: 0, + RuleIdx: 1, + Source: hr, + }, + }, + }, + }, + } + + fakeServiceStore := &statefakes.FakeServiceStore{} + fakeServiceStore.ResolveReturns("10.0.0.1", nil) + + expected := server{ + ServerName: "example.com", + Locations: []location{ + { + Path: "/", + ProxyPass: "http://10.0.0.1:80", + }, + { + Path: "/test", + ProxyPass: "http://" + nginx502Server, + }, + }, + } + expectedWarnings := Warnings{ + hr: []string{"empty backend refs"}, + } + + result, warnings := generate(host, fakeServiceStore) + + if diff := cmp.Diff(expected, result); diff != "" { + t.Errorf("generate() mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(expectedWarnings, warnings); diff != "" { + t.Errorf("generate() mismatch on warnings (-want +got):\n%s", diff) + } +} + +func TestGenerateProxyPass(t *testing.T) { + expected := "http://10.0.0.1:80" + + result := generateProxyPass("10.0.0.1:80") + if result != expected { + t.Errorf("generateProxyPass() returned %s but expected %s", result, expected) + } + + expected = "http://" + nginx502Server + + result = generateProxyPass("") + if result != expected { + t.Errorf("generateProxyPass() returned %s but expected %s", result, expected) + } +} + +func TestGetBackendAddress(t *testing.T) { + getNormalRefs := func() []v1alpha2.HTTPBackendRef { + return []v1alpha2.HTTPBackendRef{ + { + BackendRef: v1alpha2.BackendRef{ + BackendObjectReference: v1alpha2.BackendObjectReference{ + Group: (*v1alpha2.Group)(helpers.GetStringPointer("networking.k8s.io")), + Kind: (*v1alpha2.Kind)(helpers.GetStringPointer("Service")), + Name: "service1", + Namespace: (*v1alpha2.Namespace)(helpers.GetStringPointer("test")), + Port: (*v1alpha2.PortNumber)(helpers.GetInt32Pointer(80)), + }, + }, + }, + } + } + + getModifiedRefs := func(mod func([]v1alpha2.HTTPBackendRef) []v1alpha2.HTTPBackendRef) []v1alpha2.HTTPBackendRef { + return mod(getNormalRefs()) + } + + tests := []struct { + refs []v1alpha2.HTTPBackendRef + parentNS string + storeAddress string + storeErr error + expectedResolverCallCount int + expectedNsName types.NamespacedName + expectedAddress string + expectErr bool + msg string + }{ + { + refs: getNormalRefs(), + parentNS: "test", + storeAddress: "10.0.0.1", + storeErr: nil, + expectedResolverCallCount: 1, + expectedNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, + expectedAddress: "10.0.0.1:80", + expectErr: false, + msg: "normal case", + }, + { + refs: getModifiedRefs(func(refs []v1alpha2.HTTPBackendRef) []v1alpha2.HTTPBackendRef { + refs[0].BackendRef.Namespace = nil + return refs + }), + parentNS: "test", + storeAddress: "10.0.0.1", + storeErr: nil, + expectedResolverCallCount: 1, + expectedNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, + expectedAddress: "10.0.0.1:80", + expectErr: false, + msg: "normal case with implicit namespace", + }, + { + refs: getModifiedRefs(func(refs []v1alpha2.HTTPBackendRef) []v1alpha2.HTTPBackendRef { + refs[0].BackendRef.Group = nil + refs[0].BackendRef.Kind = nil + return refs + }), + parentNS: "test", + storeAddress: "10.0.0.1", + storeErr: nil, + expectedResolverCallCount: 1, + expectedNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, + expectedAddress: "10.0.0.1:80", + expectErr: false, + msg: "normal case with implicit service", + }, + { + refs: getModifiedRefs(func(refs []v1alpha2.HTTPBackendRef) []v1alpha2.HTTPBackendRef { + secondRef := refs[0].DeepCopy() + secondRef.Name = "service2" + return append(refs, *secondRef) + }), + parentNS: "test", + storeAddress: "10.0.0.1", + storeErr: nil, + expectedResolverCallCount: 1, + expectedNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, + expectedAddress: "10.0.0.1:80", + expectErr: false, + msg: "first backend ref is used", + }, + { + refs: getModifiedRefs(func(refs []v1alpha2.HTTPBackendRef) []v1alpha2.HTTPBackendRef { + refs[0].BackendRef.Kind = (*v1alpha2.Kind)(helpers.GetStringPointer("NotService")) + return refs + }), + parentNS: "test", + storeAddress: "10.0.0.1", + storeErr: nil, + expectedResolverCallCount: 0, + expectedNsName: types.NamespacedName{}, + expectedAddress: "", + expectErr: true, + msg: "not a service Kind", + }, + { + refs: nil, + parentNS: "test", + storeAddress: "10.0.0.1", + storeErr: nil, + expectedResolverCallCount: 0, + expectedNsName: types.NamespacedName{}, + expectedAddress: "", + expectErr: true, + msg: "no refs", + }, + { + refs: getModifiedRefs(func(refs []v1alpha2.HTTPBackendRef) []v1alpha2.HTTPBackendRef { + refs[0].BackendRef.Port = nil + return refs + }), + parentNS: "test", + storeAddress: "10.0.0.1", + storeErr: nil, + expectedResolverCallCount: 1, + expectedNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, + expectedAddress: "", + expectErr: true, + msg: "no port", + }, + { + refs: getNormalRefs(), + parentNS: "test", + storeAddress: "", + storeErr: errors.New(""), + expectedResolverCallCount: 1, + expectedNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, + expectedAddress: "", + expectErr: true, + msg: "service doesn't exist", + }, + } + + for _, test := range tests { + fakeServiceStore := &statefakes.FakeServiceStore{} + fakeServiceStore.ResolveReturns(test.storeAddress, test.storeErr) + + result, err := getBackendAddress(test.refs, test.parentNS, fakeServiceStore) + if result != test.expectedAddress { + t.Errorf("getBackendAddress() returned %s but expected %s for case %q", result, test.expectedAddress, test.msg) + } + + if test.expectErr { + if err == nil { + t.Errorf("getBackendAddress() didn't return any error for case %q", test.msg) + } + } else { + if err != nil { + t.Errorf("getBackendAddress() returned unexpected error %v for case %q", err, test.msg) + } + } + + callCount := fakeServiceStore.ResolveCallCount() + if callCount != test.expectedResolverCallCount { + t.Errorf("getBackendAddress() called fakeServiceStore.Resolve %d times but expected %d for case %q", callCount, test.expectedResolverCallCount, test.msg) + } + + if test.expectedResolverCallCount == 0 { + continue + } + + nsname := fakeServiceStore.ResolveArgsForCall(0) + if nsname != test.expectedNsName { + t.Errorf("getBackendAddress() called fakeServiceStore.Resolve with %v but expected %v for case %q", nsname, test.expectedNsName, test.msg) + } + } +} diff --git a/internal/nginx/config/http.go b/internal/nginx/config/http.go new file mode 100644 index 0000000000..0e8b7c96a5 --- /dev/null +++ b/internal/nginx/config/http.go @@ -0,0 +1,11 @@ +package config + +type server struct { + ServerName string + Locations []location +} + +type location struct { + Path string + ProxyPass string +} diff --git a/internal/nginx/config/template.go b/internal/nginx/config/template.go new file mode 100644 index 0000000000..3b805b25b4 --- /dev/null +++ b/internal/nginx/config/template.go @@ -0,0 +1,47 @@ +package config + +import ( + "bytes" + "fmt" + "text/template" +) + +var serverTemplate = `server { + server_name {{ .ServerName }}; + + {{ range $l := .Locations }} + location {{ $l.Path }} { + proxy_set_header Host $host; + proxy_pass {{ $l.ProxyPass }}; + } + {{ 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. +// TO-DO: for now, we only generate configuration with NGINX server. +// We will also need to generate the main NGINX configuration file, upstreams. +type templateExecutor struct { + serverTemplate *template.Template +} + +func newTemplateExecutor() *templateExecutor { + t, err := template.New("server").Parse(serverTemplate) + if err != nil { + panic(fmt.Errorf("failed to parse server template: %w", err)) + } + + return &templateExecutor{serverTemplate: t} +} + +func (e *templateExecutor) ExecuteForServer(s server) []byte { + var buf bytes.Buffer + + err := e.serverTemplate.Execute(&buf, s) + if err != nil { + panic(fmt.Errorf("failed to execute server template: %w", err)) + } + + return buf.Bytes() +} diff --git a/internal/nginx/config/template_test.go b/internal/nginx/config/template_test.go new file mode 100644 index 0000000000..bb03edad9e --- /dev/null +++ b/internal/nginx/config/template_test.go @@ -0,0 +1,57 @@ +package config + +import ( + "testing" + "text/template" +) + +func TestExecuteForServer(t *testing.T) { + executor := newTemplateExecutor() + + server := server{ + ServerName: "example.com", + Locations: []location{ + { + Path: "/", + ProxyPass: "http://10.0.0.1", + }, + }, + } + + cfg := executor.ExecuteForServer(server) + // we only do a sanity check here. + // the config generation logic is tested in the Generator tests. + if len(cfg) == 0 { + t.Error("ExecuteForServer() returned 0-length config") + } +} + +func TestNewTemplateExecutorPanics(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Error("newTemplateExecutor() didn't panic") + } + }() + + serverTemplate = "{{ end }}" // invalid template + newTemplateExecutor() +} + +func TestExecuteForServerPanics(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Error("ExecuteForServer() didn't panic") + } + }() + + tmpl, err := template.New("test").Parse("{{ .NonExistingField }}") + if err != nil { + t.Fatalf("Failed to parse template: %v", err) + } + + executor := &templateExecutor{serverTemplate: tmpl} + + _ = executor.ExecuteForServer(server{}) +} diff --git a/internal/nginx/config/warnings.go b/internal/nginx/config/warnings.go new file mode 100644 index 0000000000..a1925cd865 --- /dev/null +++ b/internal/nginx/config/warnings.go @@ -0,0 +1,24 @@ +package config + +import ( + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Warnings stores a list of warnings for a given object. +type Warnings map[client.Object][]string + +func newWarnings() Warnings { + return make(map[client.Object][]string) +} + +// AddWarningf adds a warning for the specified object using the provided format and arguments. +func (w Warnings) AddWarningf(obj client.Object, msgFmt string, args ...interface{}) { + w[obj] = append(w[obj], fmt.Sprintf(msgFmt, args...)) +} + +// AddWarning adds a warning for the specified object. +func (w Warnings) AddWarning(obj client.Object, msg string) { + w[obj] = append(w[obj], msg) +} diff --git a/internal/nginx/config/warnings_test.go b/internal/nginx/config/warnings_test.go new file mode 100644 index 0000000000..2d20a61a73 --- /dev/null +++ b/internal/nginx/config/warnings_test.go @@ -0,0 +1,46 @@ +package config + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +func TestAddWarningf(t *testing.T) { + warnings := newWarnings() + obj := &v1alpha2.HTTPRoute{} + + expected := Warnings{ + obj: []string{ + "simple", + "advanced 1", + }, + } + + warnings.AddWarningf(obj, "simple") + warnings.AddWarningf(obj, "advanced %d", 1) + + if diff := cmp.Diff(expected, warnings); diff != "" { + t.Errorf("AddWarningf mismatch (-want +got):\n%s", diff) + } +} + +func TestAddWarning(t *testing.T) { + warnings := newWarnings() + obj := &v1alpha2.HTTPRoute{} + + expected := Warnings{ + obj: []string{ + "first", + "second", + }, + } + + warnings.AddWarning(obj, "first") + warnings.AddWarning(obj, "second") + + if diff := cmp.Diff(expected, warnings); diff != "" { + t.Errorf("AddWarning mismatch (-want +got):\n%s", diff) + } +} diff --git a/internal/state/configuration_test.go b/internal/state/configuration_test.go index b37ac40596..0c58240cdc 100644 --- a/internal/state/configuration_test.go +++ b/internal/state/configuration_test.go @@ -46,7 +46,7 @@ var _ = Describe("Configuration", func() { Matches: []v1alpha2.HTTPRouteMatch{ { Path: &v1alpha2.HTTPPathMatch{ - Value: getStringPointer("/coffee"), + Value: helpers.GetStringPointer("/coffee"), }, }, }, @@ -56,7 +56,7 @@ var _ = Describe("Configuration", func() { } updatedHRWithSameGen = hr.DeepCopy() - updatedHRWithSameGen.Spec.Rules[1].Matches[0].Path.Value = getStringPointer("/tea") + updatedHRWithSameGen.Spec.Rules[1].Matches[0].Path.Value = helpers.GetStringPointer("/tea") updatedHRWithIncrementedGen = updatedHRWithSameGen.DeepCopy() updatedHRWithIncrementedGen.Generation++ @@ -275,7 +275,7 @@ var _ = Describe("Configuration", func() { Matches: []v1alpha2.HTTPRouteMatch{ { Path: &v1alpha2.HTTPPathMatch{ - Value: getStringPointer("/coffee"), + Value: helpers.GetStringPointer("/coffee"), }, }, }, @@ -285,7 +285,7 @@ var _ = Describe("Configuration", func() { } hr2Updated = hr2.DeepCopy() - hr2Updated.Spec.Rules[0].Matches[0].Path.Value = getStringPointer("/tea") + hr2Updated.Spec.Rules[0].Matches[0].Path.Value = helpers.GetStringPointer("/tea") hr2Updated.Generation++ }) @@ -643,7 +643,7 @@ var _ = Describe("Configuration", func() { Matches: []v1alpha2.HTTPRouteMatch{ { Path: &v1alpha2.HTTPPathMatch{ - Value: getStringPointer("/"), + Value: helpers.GetStringPointer("/"), }, }, }, @@ -881,7 +881,3 @@ var _ = Describe("Configuration", func() { }) }) }) - -func getStringPointer(s string) *string { - return &s -}