From e8f7a30395effb7d312027769fe2f972f8710a0f Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 26 Apr 2023 18:35:31 -0400 Subject: [PATCH 1/5] Support multiple Gateways with dedicated Services --- deploy/manifests/nginx-gateway.yaml | 6 ++ internal/events/handler.go | 6 +- internal/manager/manager.go | 1 + internal/nginx/config/generator.go | 6 +- internal/nginx/config/http/config.go | 1 + internal/nginx/config/servers.go | 50 +++++++----- internal/nginx/config/servers_template.go | 9 ++- internal/nginx/config/split_clients.go | 14 +++- internal/nginx/config/upstreams.go | 24 +++--- internal/state/change_processor.go | 93 +++++++++++++++++++++-- internal/state/dataplane/configuration.go | 38 +++++---- internal/state/graph/gateway.go | 21 +++++ internal/state/graph/graph.go | 18 ++++- internal/state/graph/httproute.go | 24 +++--- internal/state/statuses.go | 15 ++-- internal/status/updater.go | 6 +- 16 files changed, 253 insertions(+), 79 deletions(-) diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 2a026ceb4f..f5afe8d276 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -9,6 +9,12 @@ apiVersion: rbac.authorization.k8s.io/v1 metadata: name: nginx-gateway rules: +- apiGroups: + - "" + resources: + - services + verbs: + - create - apiGroups: - "" resources: diff --git a/internal/events/handler.go b/internal/events/handler.go index ee7ab749a3..896eca62ae 100644 --- a/internal/events/handler.go +++ b/internal/events/handler.go @@ -87,10 +87,14 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc h.cfg.Logger.Info("NGINX configuration was successfully updated") } + // provision or update Services + + // update Gateway-related statuses to include information about Services (IPs) + h.cfg.StatusUpdater.Update(ctx, statuses) } -func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Configuration) error { +func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf []dataplane.Configuration) error { // Write all secrets (nuke and pave). // This will remove all secrets in the secrets directory before writing the requested secrets. // FIXME(kate-osborn): We may want to rethink this approach in the future and write and remove secrets individually. diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 79cd8c11e2..416722b38d 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -127,6 +127,7 @@ func Start(cfg config.Config) error { }, EventRecorder: recorder, Scheme: scheme, + Client: mgr.GetClient(), }) configGenerator := ngxcfg.NewGeneratorImpl() diff --git a/internal/nginx/config/generator.go b/internal/nginx/config/generator.go index 0e756d36dc..61b96f87cb 100644 --- a/internal/nginx/config/generator.go +++ b/internal/nginx/config/generator.go @@ -10,7 +10,7 @@ import ( // This interface is used for testing purposes only. type Generator interface { // Generate generates NGINX configuration from internal representation. - Generate(configuration dataplane.Configuration) []byte + Generate(configuration []dataplane.Configuration) []byte } // GeneratorImpl is an implementation of Generator. @@ -22,13 +22,13 @@ func NewGeneratorImpl() GeneratorImpl { } // executeFunc is a function that generates NGINX configuration from internal representation. -type executeFunc func(configuration dataplane.Configuration) []byte +type executeFunc func(configuration []dataplane.Configuration) []byte // Generate generates NGINX configuration from internal representation. // It is the responsibility of the caller to validate the configuration before calling this function. // In case of invalid configuration, NGINX will fail to reload or could be configured with malicious configuration. // To validate, use the validators from the validation package. -func (g GeneratorImpl) Generate(conf dataplane.Configuration) []byte { +func (g GeneratorImpl) Generate(conf []dataplane.Configuration) []byte { var generated []byte for _, execute := range getExecuteFuncs() { generated = append(generated, execute(conf)...) diff --git a/internal/nginx/config/http/config.go b/internal/nginx/config/http/config.go index f799bfb345..68e76b5066 100644 --- a/internal/nginx/config/http/config.go +++ b/internal/nginx/config/http/config.go @@ -2,6 +2,7 @@ package http // Server holds all configuration for an HTTP server. type Server struct { + Port int32 SSL *SSL ServerName string Locations []Location diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 2c0b1c52c5..f285f01033 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -10,6 +10,7 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" ) var serversTemplate = gotemplate.Must(gotemplate.New("servers").Parse(serversTemplateText)) @@ -20,53 +21,61 @@ const ( rootPath = "/" ) -func executeServers(conf dataplane.Configuration) []byte { - servers := createServers(conf.HTTPServers, conf.SSLServers) +func executeServers(confs []dataplane.Configuration) []byte { + var servers []http.Server + + for _, conf := range confs { + servers = append(servers, createServers(conf.Key, conf.Ports, conf.HTTPServers, conf.SSLServers)...) + } return execute(serversTemplate, servers) } -func createServers(httpServers, sslServers []dataplane.VirtualServer) []http.Server { +func createServers(key string, ports graph.GatewayPorts, httpServers, sslServers []dataplane.VirtualServer) []http.Server { + // user port information to create servers + servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) for _, s := range httpServers { - servers = append(servers, createServer(s)) + servers = append(servers, createServer(key, ports.HTTP, s)) } for _, s := range sslServers { - servers = append(servers, createSSLServer(s)) + servers = append(servers, createSSLServer(key, ports.HTTPS, s)) } return servers } -func createSSLServer(virtualServer dataplane.VirtualServer) http.Server { +func createSSLServer(key string, httpsPort int32, virtualServer dataplane.VirtualServer) http.Server { if virtualServer.IsDefault { - return createDefaultSSLServer() + return createDefaultSSLServer(httpsPort) } return http.Server{ + Port: httpsPort, ServerName: virtualServer.Hostname, SSL: &http.SSL{ Certificate: virtualServer.SSL.CertificatePath, CertificateKey: virtualServer.SSL.CertificatePath, }, - Locations: createLocations(virtualServer.PathRules, 443), + Locations: createLocations(key, virtualServer.PathRules, httpsPort), } } -func createServer(virtualServer dataplane.VirtualServer) http.Server { +func createServer(key string, httpPort int32, virtualServer dataplane.VirtualServer) http.Server { if virtualServer.IsDefault { - return createDefaultHTTPServer() + return createDefaultHTTPServer(httpPort) } return http.Server{ + Port: httpPort, ServerName: virtualServer.Hostname, - Locations: createLocations(virtualServer.PathRules, 80), + Locations: createLocations(key, virtualServer.PathRules, httpPort), } } -func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Location { +func createLocations(key string, pathRules []dataplane.PathRule, listenerPort int32) []http.Location { lenPathRules := len(pathRules) if lenPathRules == 0 { @@ -126,12 +135,13 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo // RequestRedirect and proxying are mutually exclusive. if r.Filters.RequestRedirect != nil { - loc.Return = createReturnValForRedirectFilter(r.Filters.RequestRedirect, listenerPort) + loc.Return = createReturnValForRedirectFilter(r.Filters.RequestRedirect, int(listenerPort)) locs = append(locs, loc) continue } backendName := backendGroupName(r.BackendGroup) + backendName = fmt.Sprintf("%s__%s", key, backendName) if backendGroupNeedsSplit(r.BackendGroup) { loc.ProxyPass = createProxyPassForVar(backendName) @@ -165,12 +175,18 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo return locs } -func createDefaultSSLServer() http.Server { - return http.Server{IsDefaultSSL: true} +func createDefaultSSLServer(port int32) http.Server { + return http.Server{ + Port: port, + IsDefaultSSL: true, + } } -func createDefaultHTTPServer() http.Server { - return http.Server{IsDefaultHTTP: true} +func createDefaultHTTPServer(port int32) http.Server { + return http.Server{ + Port: port, + IsDefaultHTTP: true, + } } func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int) *http.Return { diff --git a/internal/nginx/config/servers_template.go b/internal/nginx/config/servers_template.go index 0f8a617129..075ef761a3 100644 --- a/internal/nginx/config/servers_template.go +++ b/internal/nginx/config/servers_template.go @@ -4,13 +4,13 @@ var serversTemplateText = ` {{ range $s := . }} {{ if $s.IsDefaultSSL }} server { - listen 443 ssl default_server; + listen {{ $s.Port }} ssl default_server; ssl_reject_handshake on; } {{ else if $s.IsDefaultHTTP }} server { - listen 80 default_server; + listen {{ $s.Port }} default_server; default_type text/html; return 404; @@ -18,14 +18,17 @@ server { {{ else }} server { {{ if $s.SSL }} - listen 443 ssl; + listen {{ $s.Port }} ssl; ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; if ($ssl_server_name != $host) { return 421; } + {{ else }} + listen {{ $s.Port }}; {{ end }} + server_name {{ $s.ServerName }}; diff --git a/internal/nginx/config/split_clients.go b/internal/nginx/config/split_clients.go index 28d39030d3..0875b746f5 100644 --- a/internal/nginx/config/split_clients.go +++ b/internal/nginx/config/split_clients.go @@ -12,13 +12,17 @@ import ( var splitClientsTemplate = gotemplate.Must(gotemplate.New("split_clients").Parse(splitClientsTemplateText)) -func executeSplitClients(conf dataplane.Configuration) []byte { - splitClients := createSplitClients(conf.BackendGroups) +func executeSplitClients(confs []dataplane.Configuration) []byte { + var splitClients []http.SplitClient + + for _, conf := range confs { + splitClients = append(splitClients, createSplitClients(conf.Key, conf.BackendGroups)...) + } return execute(splitClientsTemplate, splitClients) } -func createSplitClients(backendGroups []graph.BackendGroup) []http.SplitClient { +func createSplitClients(key string, backendGroups []graph.BackendGroup) []http.SplitClient { numSplits := 0 for _, group := range backendGroups { if backendGroupNeedsSplit(group) { @@ -39,8 +43,10 @@ func createSplitClients(backendGroups []graph.BackendGroup) []http.SplitClient { continue } + name := fmt.Sprintf("%s__%s", key, group.GroupName()) + splitClients = append(splitClients, http.SplitClient{ - VariableName: convertStringToSafeVariableName(group.GroupName()), + VariableName: convertStringToSafeVariableName(name), Distributions: distributions, }) diff --git a/internal/nginx/config/upstreams.go b/internal/nginx/config/upstreams.go index 927745ed5a..50e2e6a24b 100644 --- a/internal/nginx/config/upstreams.go +++ b/internal/nginx/config/upstreams.go @@ -19,29 +19,33 @@ const ( invalidBackendRef = "invalid-backend-ref" ) -func executeUpstreams(conf dataplane.Configuration) []byte { - upstreams := createUpstreams(conf.Upstreams) +func executeUpstreams(confs []dataplane.Configuration) []byte { + var upstreams []http.Upstream + + for _, conf := range confs { + upstreams = append(upstreams, createUpstreams(conf.Key, conf.Upstreams)...) + } return execute(upstreamsTemplate, upstreams) } -func createUpstreams(upstreams []dataplane.Upstream) []http.Upstream { +func createUpstreams(key string, upstreams []dataplane.Upstream) []http.Upstream { // capacity is the number of upstreams + 1 for the invalid backend ref upstream ups := make([]http.Upstream, 0, len(upstreams)+1) for _, u := range upstreams { - ups = append(ups, createUpstream(u)) + ups = append(ups, createUpstream(key, u)) } - ups = append(ups, createInvalidBackendRefUpstream()) + ups = append(ups, createInvalidBackendRefUpstream(key)) return ups } -func createUpstream(up dataplane.Upstream) http.Upstream { +func createUpstream(key string, up dataplane.Upstream) http.Upstream { if len(up.Endpoints) == 0 { return http.Upstream{ - Name: up.Name, + Name: fmt.Sprintf("%s__%s", key, up.Name), Servers: []http.UpstreamServer{ { Address: nginx502Server, @@ -58,14 +62,14 @@ func createUpstream(up dataplane.Upstream) http.Upstream { } return http.Upstream{ - Name: up.Name, + Name: fmt.Sprintf("%s__%s", key, up.Name), Servers: upstreamServers, } } -func createInvalidBackendRefUpstream() http.Upstream { +func createInvalidBackendRefUpstream(key string) http.Upstream { return http.Upstream{ - Name: invalidBackendRef, + Name: fmt.Sprintf("%s__%s", key, invalidBackendRef), Servers: []http.UpstreamServer{ { Address: nginx500Server, diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index 911d8061fa..5411cebb18 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -8,9 +8,11 @@ import ( "github.com/go-logr/logr" apiv1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" @@ -50,7 +52,7 @@ type ChangeProcessor interface { // 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(ctx context.Context) (changed bool, conf dataplane.Configuration, statuses Statuses) + Process(ctx context.Context) (changed bool, confs []dataplane.Configuration, statuses Statuses) } // ChangeProcessorConfig holds configuration parameters for ChangeProcessorImpl. @@ -73,6 +75,7 @@ type ChangeProcessorConfig struct { GatewayCtlrName string // GatewayClassName is the name of the GatewayClass resource. GatewayClassName string + Client client.Client } // ChangeProcessorImpl is an implementation of ChangeProcessor. @@ -86,6 +89,8 @@ type ChangeProcessorImpl struct { cfg ChangeProcessorConfig + portAllocator *gatewayPortAllocator + lock sync.Mutex } @@ -169,6 +174,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { getAndResetClusterStateChanged: trackingUpdater.getAndResetChangedStatus, updater: updater, clusterState: clusterStore, + portAllocator: newGatewayPortAllocator(), } } @@ -200,12 +206,12 @@ func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType client.Object, ns func (c *ChangeProcessorImpl) Process( ctx context.Context, -) (changed bool, conf dataplane.Configuration, statuses Statuses) { +) (changed bool, confs []dataplane.Configuration, statuses Statuses) { c.lock.Lock() defer c.lock.Unlock() if !c.getAndResetClusterStateChanged() { - return false, conf, statuses + return false, confs, statuses } g := graph.BuildGraph( @@ -216,8 +222,85 @@ func (c *ChangeProcessorImpl) Process( c.cfg.Validators, ) - conf = dataplane.BuildConfiguration(ctx, g, c.cfg.ServiceResolver) + // provision Services + for _, gw := range g.Gateways { + gw.Ports = c.portAllocator.getPorts(client.ObjectKeyFromObject(gw.Source)) + + if gw.Service != nil { + continue + } + + svc := &apiv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: gw.Source.Name, + Namespace: "nginx-gateway", + }, + Spec: apiv1.ServiceSpec{ + Ports: []apiv1.ServicePort{ + { + Name: "http", + Protocol: "TCP", + Port: 80, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: gw.Ports.HTTP, + }, + }, + { + Name: "https", + Protocol: "TCP", + Port: 443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: gw.Ports.HTTPS, + }, + }, + }, + Selector: map[string]string{ + "app": "nginx-gateway", + }, + }, + } + + err := c.cfg.Client.Create(ctx, svc) + if err != nil { + panic(fmt.Errorf("failed to create service: %w", err)) + } + + gw.Service = svc + } + + // pass ports here + confs = dataplane.BuildConfiguration(ctx, g, c.cfg.ServiceResolver) statuses = buildStatuses(g) - return true, conf, statuses + return true, confs, statuses +} + +type gatewayPortAllocator struct { + allocations map[types.NamespacedName]graph.GatewayPorts + nextAvailablePort int32 +} + +func newGatewayPortAllocator() *gatewayPortAllocator { + return &gatewayPortAllocator{ + allocations: make(map[types.NamespacedName]graph.GatewayPorts), + nextAvailablePort: 5000, + } +} + +func (p *gatewayPortAllocator) getPorts(gw types.NamespacedName) graph.GatewayPorts { + if ports, ok := p.allocations[gw]; ok { + return ports + } + + ports := graph.GatewayPorts{ + HTTP: p.nextAvailablePort, + HTTPS: p.nextAvailablePort + 1, + } + + p.nextAvailablePort += 2 + p.allocations[gw] = ports + + return ports } diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 30650df628..d36d88a186 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -15,6 +15,8 @@ const wildcardHostname = "~^" // Configuration is an intermediate representation of dataplane configuration. type Configuration struct { + Key string + Ports graph.GatewayPorts // HTTPServers holds all HTTPServers. // FIXME(pleshakov) We assume that all servers are HTTP and listen on port 80. HTTPServers []VirtualServer @@ -97,27 +99,37 @@ func (r *MatchRule) GetMatch() v1beta1.HTTPRouteMatch { // BuildConfiguration builds the Configuration from the Graph. // FIXME(pleshakov) For now we only handle paths with prefix matches. Handle exact and regex matches -func BuildConfiguration(ctx context.Context, g *graph.Graph, resolver resolver.ServiceResolver) Configuration { +func BuildConfiguration(ctx context.Context, g *graph.Graph, resolver resolver.ServiceResolver) []Configuration { if g.GatewayClass == nil || !g.GatewayClass.Valid { - return Configuration{} + return nil } - if g.Gateway == nil { - return Configuration{} + if len(g.Gateways) == 0 { + return nil } - upstreamsMap := buildUpstreamsMap(ctx, g.Gateway.Listeners, resolver) - httpServers, sslServers := buildServers(g.Gateway.Listeners) - backendGroups := buildBackendGroups(g.Gateway.Listeners) + confs := make([]Configuration, 0, len(g.Gateways)) + + // for each Gateway, build its servers + + for _, gw := range g.Gateways { + upstreamsMap := buildUpstreamsMap(ctx, gw.Listeners, resolver) + httpServers, sslServers := buildServers(gw.Listeners) + backendGroups := buildBackendGroups(gw.Listeners) + + config := Configuration{ + Key: fmt.Sprintf("gateway__%s__%s", gw.Source.Namespace, gw.Source.Name), + Ports: gw.Ports, + HTTPServers: httpServers, + SSLServers: sslServers, + Upstreams: upstreamsMapToSlice(upstreamsMap), + BackendGroups: backendGroups, + } - config := Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - Upstreams: upstreamsMapToSlice(upstreamsMap), - BackendGroups: backendGroups, + confs = append(confs, config) } - return config + return confs } func upstreamsMapToSlice(upstreamsMap map[string]Upstream) []Upstream { diff --git a/internal/state/graph/gateway.go b/internal/state/graph/gateway.go index 90a47fb3e7..08f6b43d05 100644 --- a/internal/state/graph/gateway.go +++ b/internal/state/graph/gateway.go @@ -3,6 +3,7 @@ package graph import ( "sort" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -17,6 +18,12 @@ type Gateway struct { Source *v1beta1.Gateway // Listeners include the listeners of the Gateway. Listeners map[string]*Listener + Service *v1.Service + Ports GatewayPorts +} + +type GatewayPorts struct { + HTTP, HTTPS int32 } // processedGateways holds the resources that belong to NKG. @@ -49,6 +56,20 @@ func (gws processedGateways) GetAllNsNames() []types.NamespacedName { return allNsNames } +func getGateways(gws map[types.NamespacedName]*v1beta1.Gateway, gcName string) []*v1beta1.Gateway { + referencedGws := make([]*v1beta1.Gateway, 0, len(gws)) + + for _, gw := range gws { + if string(gw.Spec.GatewayClassName) != gcName { + continue + } + + referencedGws = append(referencedGws, gw) + } + + return referencedGws +} + // processGateways determines which Gateway resource belong to NKG (determined by the Gateway GatewayClassName field). func processGateways( gws map[types.NamespacedName]*v1beta1.Gateway, diff --git a/internal/state/graph/graph.go b/internal/state/graph/graph.go index 300916b32c..42d199ff52 100644 --- a/internal/state/graph/graph.go +++ b/internal/state/graph/graph.go @@ -22,7 +22,8 @@ type Graph struct { // GatewayClass holds the GatewayClass resource. GatewayClass *GatewayClass // Gateway holds the winning Gateway resource. - Gateway *Gateway + Gateways []*Gateway + // IgnoredGateways holds the ignored Gateway resources, which belong to the NGINX Gateway (based on the // GatewayClassName field of the resource) but ignored. It doesn't hold the Gateway resources that do not belong to // the NGINX Gateway. @@ -48,16 +49,25 @@ func BuildGraph( gc := buildGatewayClass(gatewayClass) processedGws := processGateways(state.Gateways, gcName) + gws := getGateways(state.Gateways, gcName) + + graphGws := make([]*Gateway, 0, len(gws)) - gw := buildGateway(processedGws.Winner, secretMemoryMgr, gc) + for _, gw := range gws { + graphGw := buildGateway(gw, secretMemoryMgr, gc) + // assume the same name + graphGw.Service = state.Services[types.NamespacedName{Namespace: "nginx-gateway", Name: gw.Name}] + + graphGws = append(graphGws, graphGw) + } routes := buildRoutesForGateways(validators.HTTPFieldsValidator, state.HTTPRoutes, processedGws.GetAllNsNames()) - bindRoutesToListeners(routes, gw) + bindRoutesToListeners(routes, graphGws) addBackendGroupsToRoutes(routes, state.Services) g := &Graph{ GatewayClass: gc, - Gateway: gw, + Gateways: graphGws, Routes: routes, IgnoredGateways: processedGws.Ignored, } diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index b7f0b1dbd9..00dc638e4e 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -272,17 +272,17 @@ func buildRoute( return r } -func bindRoutesToListeners(routes map[types.NamespacedName]*Route, gw *Gateway) { - if gw == nil { +func bindRoutesToListeners(routes map[types.NamespacedName]*Route, gws []*Gateway) { + if len(gws) == 0 { return } for _, r := range routes { - bindRouteToListeners(r, gw) + bindRouteToListeners(r, gws) } } -func bindRouteToListeners(r *Route, gw *Gateway) { +func bindRouteToListeners(r *Route, gws []*Gateway) { if !r.Valid { return } @@ -310,16 +310,20 @@ func bindRouteToListeners(r *Route, gw *Gateway) { continue } - // Case 2: the parentRef references an ignored Gateway resource. + // Find a Gateway - referencesWinningGw := ref.Gateway.Namespace == gw.Source.Namespace && ref.Gateway.Name == gw.Source.Name + var gw *Gateway - if !referencesWinningGw { - attachment.FailedCondition = conditions.NewTODO("Gateway is ignored") - continue + for i := range gws { + if gws[i].Source.Namespace == ref.Gateway.Namespace && gws[i].Source.Name == ref.Gateway.Name { + gw = gws[i] + break + } } - // Case 3 - winning Gateway + if gw == nil { + panic("Gateway not found") + } // Find a listener diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 50c2a7be1a..8ddc0b3947 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -18,7 +18,7 @@ type HTTPRouteStatuses map[types.NamespacedName]HTTPRouteStatus // Statuses holds the status-related information about Gateway API resources. type Statuses struct { GatewayClassStatus *GatewayClassStatus - GatewayStatus *GatewayStatus + GatewayStatuses []GatewayStatus IgnoredGatewayStatuses IgnoredGatewayStatuses HTTPRouteStatuses HTTPRouteStatuses } @@ -96,12 +96,13 @@ func buildStatuses(graph *graph.Graph) Statuses { } } - if graph.Gateway != nil { + for _, gw := range graph.Gateways { + listenerStatuses := make(map[string]ListenerStatus) defaultConds := conditions.NewDefaultListenerConditions() - for name, l := range graph.Gateway.Listeners { + for name, l := range gw.Listeners { conds := make([]conditions.Condition, 0, len(l.Conditions)+len(defaultConds)) // We add default conds first, so that any additional conditions will override them, which is @@ -115,11 +116,13 @@ func buildStatuses(graph *graph.Graph) Statuses { } } - statuses.GatewayStatus = &GatewayStatus{ - NsName: client.ObjectKeyFromObject(graph.Gateway.Source), + status := GatewayStatus{ + NsName: client.ObjectKeyFromObject(gw.Source), ListenerStatuses: listenerStatuses, - ObservedGeneration: graph.Gateway.Source.Generation, + ObservedGeneration: gw.Source.Generation, } + + statuses.GatewayStatuses = append(statuses.GatewayStatuses, status) } for nsname, gw := range graph.IgnoredGateways { diff --git a/internal/status/updater.go b/internal/status/updater.go index 5abc9a339a..5bc085e8e7 100644 --- a/internal/status/updater.go +++ b/internal/status/updater.go @@ -98,10 +98,10 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses state.Statuses) { ) } - if statuses.GatewayStatus != nil { - upd.update(ctx, statuses.GatewayStatus.NsName, &v1beta1.Gateway{}, func(object client.Object) { + for _, gs := range statuses.GatewayStatuses { + upd.update(ctx, gs.NsName, &v1beta1.Gateway{}, func(object client.Object) { gw := object.(*v1beta1.Gateway) - gw.Status = prepareGatewayStatus(*statuses.GatewayStatus, upd.cfg.Clock.Now()) + gw.Status = prepareGatewayStatus(gs, upd.cfg.Clock.Now()) }) } From 602e9280ef6c76dffa80f40367a33293cb9d3b5e Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 27 Apr 2023 13:18:40 -0400 Subject: [PATCH 2/5] Delete Services for removed Gateways --- deploy/manifests/nginx-gateway.yaml | 1 + internal/state/change_processor.go | 33 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index f5afe8d276..348c20a081 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -15,6 +15,7 @@ rules: - services verbs: - create + - delete - apiGroups: - "" resources: diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index 5411cebb18..3ee4f1d34f 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -9,6 +9,7 @@ import ( apiv1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -234,6 +235,9 @@ func (c *ChangeProcessorImpl) Process( ObjectMeta: metav1.ObjectMeta{ Name: gw.Source.Name, Namespace: "nginx-gateway", + Labels: map[string]string{ + "app": "nginx-gateway", + }, }, Spec: apiv1.ServiceSpec{ Ports: []apiv1.ServicePort{ @@ -270,6 +274,35 @@ func (c *ChangeProcessorImpl) Process( gw.Service = svc } + // delete orphaned Services + + var services apiv1.ServiceList + + err := c.cfg.Client.List(ctx, &services, &client.ListOptions{ + Namespace: "nginx-gateway", + LabelSelector: labels.SelectorFromSet(map[string]string{"app": "nginx-gateway"}), + }) + if err != nil { + panic(fmt.Errorf("failed to list services: %w", err)) + } + + for _, svc := range services.Items { + found := false + for _, gw := range g.Gateways { + if gw.Service != nil && gw.Service.Name == svc.Name { + found = true + break + } + } + + if !found { + err := c.cfg.Client.Delete(ctx, &svc) + if err != nil { + panic(fmt.Errorf("failed to delete service: %w", err)) + } + } + } + // pass ports here confs = dataplane.BuildConfiguration(ctx, g, c.cfg.ServiceResolver) statuses = buildStatuses(g) From be5df6d26b9ae2eaaa60513921a67a3b4aaedb64 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 27 Apr 2023 13:35:45 -0400 Subject: [PATCH 3/5] Support Accepted and Programmed Conditions in Gateway status --- internal/state/conditions/conditions.go | 17 +++++++++++++++++ internal/state/graph/graph.go | 8 ++++---- internal/state/statuses.go | 3 ++- internal/status/gateway.go | 2 +- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index e686386490..b77783bd30 100644 --- a/internal/state/conditions/conditions.go +++ b/internal/state/conditions/conditions.go @@ -112,6 +112,23 @@ func NewTODO(msg string) Condition { } } +func NewDefaultGatewayConditions() []Condition { + return []Condition{ + { + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(v1beta1.GatewayReasonAccepted), + Message: "Gateway is accepted", + }, + { + Type: string(v1beta1.GatewayConditionProgrammed), + Status: metav1.ConditionTrue, + Reason: string(v1beta1.GatewayReasonProgrammed), + Message: "Gateway is programmed", + }, + } +} + // NewRouteInvalidListener returns a Condition that indicates that the HTTPRoute is not accepted because of an // invalid listener. func NewRouteInvalidListener() Condition { diff --git a/internal/state/graph/graph.go b/internal/state/graph/graph.go index 42d199ff52..5ef2f079d8 100644 --- a/internal/state/graph/graph.go +++ b/internal/state/graph/graph.go @@ -66,10 +66,10 @@ func BuildGraph( addBackendGroupsToRoutes(routes, state.Services) g := &Graph{ - GatewayClass: gc, - Gateways: graphGws, - Routes: routes, - IgnoredGateways: processedGws.Ignored, + GatewayClass: gc, + Gateways: graphGws, + Routes: routes, + // IgnoredGateways: processedGws.Ignored, } return g diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 8ddc0b3947..64cd128674 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -25,6 +25,7 @@ type Statuses struct { // GatewayStatus holds the status of the winning Gateway resource. type GatewayStatus struct { + Conditions []conditions.Condition // ListenerStatuses holds the statuses of listeners defined on the Gateway. ListenerStatuses ListenerStatuses // NsName is the namespaced name of the winning Gateway resource. @@ -97,7 +98,6 @@ func buildStatuses(graph *graph.Graph) Statuses { } for _, gw := range graph.Gateways { - listenerStatuses := make(map[string]ListenerStatus) defaultConds := conditions.NewDefaultListenerConditions() @@ -117,6 +117,7 @@ func buildStatuses(graph *graph.Graph) Statuses { } status := GatewayStatus{ + Conditions: conditions.NewDefaultGatewayConditions(), NsName: client.ObjectKeyFromObject(gw.Source), ListenerStatuses: listenerStatuses, ObservedGeneration: gw.Source.Generation, diff --git a/internal/status/gateway.go b/internal/status/gateway.go index 06e12e268d..45098a8af7 100644 --- a/internal/status/gateway.go +++ b/internal/status/gateway.go @@ -50,7 +50,7 @@ func prepareGatewayStatus(gatewayStatus state.GatewayStatus, transitionTime meta return v1beta1.GatewayStatus{ Listeners: listenerStatuses, - Conditions: nil, // FIXME(pleshakov) Create conditions for the Gateway resource. + Conditions: convertConditions(gatewayStatus.Conditions, gatewayStatus.ObservedGeneration, transitionTime), } } From e07710e46def5696360ca5f04e2472a711ebfab0 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 27 Apr 2023 13:43:57 -0400 Subject: [PATCH 4/5] Report IP address in Gateway Status --- internal/state/statuses.go | 2 ++ internal/status/gateway.go | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 64cd128674..4b3f823163 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -25,6 +25,7 @@ type Statuses struct { // GatewayStatus holds the status of the winning Gateway resource. type GatewayStatus struct { + IPAddress string Conditions []conditions.Condition // ListenerStatuses holds the statuses of listeners defined on the Gateway. ListenerStatuses ListenerStatuses @@ -117,6 +118,7 @@ func buildStatuses(graph *graph.Graph) Statuses { } status := GatewayStatus{ + IPAddress: gw.Service.Spec.ClusterIP, Conditions: conditions.NewDefaultGatewayConditions(), NsName: client.ObjectKeyFromObject(gw.Source), ListenerStatuses: listenerStatuses, diff --git a/internal/status/gateway.go b/internal/status/gateway.go index 45098a8af7..aadd074968 100644 --- a/internal/status/gateway.go +++ b/internal/status/gateway.go @@ -49,6 +49,11 @@ func prepareGatewayStatus(gatewayStatus state.GatewayStatus, transitionTime meta } return v1beta1.GatewayStatus{ + Addresses: []v1beta1.GatewayAddress{ + { + Value: gatewayStatus.IPAddress, + }, + }, Listeners: listenerStatuses, Conditions: convertConditions(gatewayStatus.Conditions, gatewayStatus.ObservedGeneration, transitionTime), } From fa56b48670c98283ddc2801d81041401ac1ecaa5 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 27 Apr 2023 16:51:39 -0400 Subject: [PATCH 5/5] Add conformance tests --- go.mod | 2 ++ go.sum | 5 ++++ tests/Dockerfile | 8 +++++ tests/Makefile | 31 +++++++++++++++++++ tests/conformance_test.go | 63 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+) create mode 100644 tests/Dockerfile create mode 100644 tests/Makefile create mode 100644 tests/conformance_test.go diff --git a/go.mod b/go.mod index 4b8d270851..d3687e05d0 100644 --- a/go.mod +++ b/go.mod @@ -47,10 +47,12 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/nxadm/tail v1.4.8 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.14.0 // indirect github.com/prometheus/client_model v0.3.0 // indirect github.com/prometheus/common v0.37.0 // indirect github.com/prometheus/procfs v0.8.0 // indirect + github.com/stretchr/testify v1.8.1 // indirect go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.7.0 // indirect go.uber.org/zap v1.24.0 // indirect diff --git a/go.sum b/go.sum index f0a5941278..5fac3e41a3 100644 --- a/go.sum +++ b/go.sum @@ -278,13 +278,18 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/tests/Dockerfile b/tests/Dockerfile new file mode 100644 index 0000000000..d8a9861f23 --- /dev/null +++ b/tests/Dockerfile @@ -0,0 +1,8 @@ +FROM golang:1.20 + +WORKDIR /go/src/github.com/nginxinc/nginx-kubernetes-gateway/tests + +COPY go.mod go.sum /go/src/github.com/nginxinc/nginx-kubernetes-gateway +RUN go mod download + +COPY tests /go/src/github.com/nginxinc/nginx-kubernetes-gateway/tests \ No newline at end of file diff --git a/tests/Makefile b/tests/Makefile new file mode 100644 index 0000000000..d3e61ff803 --- /dev/null +++ b/tests/Makefile @@ -0,0 +1,31 @@ +KIND_KUBE_CONFIG_FOLDER = $${HOME}/.kube/kind +TAG = latest +PREFIX = test-runner + +build: + docker build -t $(PREFIX):$(TAG) -f Dockerfile .. + +create-kind-cluster: + kind create cluster --image kindest/node:v1.27.1 + kind export kubeconfig --kubeconfig $(KIND_KUBE_CONFIG_FOLDER)/config + +delete-kind-cluster: + kind delete cluster + +load-image-to-kind: + kind load docker-image nginx-kubernetes-gateway:edge + +install-nkg: + kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v0.6.2/standard-install.yaml + sleep 30s # to allow webhook to get ready + kubectl apply -f ../deploy/manifests/namespace.yaml + kubectl create configmap njs-modules --from-file=../internal/nginx/modules/src/httpmatches.js -n nginx-gateway + kubectl apply -f ../deploy/manifests/gatewayclass.yaml + cat ../deploy/manifests/nginx-gateway.yaml | sed "s|image: ghcr.io/nginxinc/nginx-kubernetes-gateway.*|image: nginx-kubernetes-gateway:edge|" | sed "s|imagePullPolicy: Always|imagePullPolicy: Never|" | kubectl apply -f - + +run-tests-in-kind: update-test-kind-config ## Run tests in Kind + docker run --network=kind --rm -v $(KIND_KUBE_CONFIG_FOLDER):/root/.kube $(PREFIX):$(TAG) \ + go test -v . -args --gateway-class=nginx + +update-test-kind-config: + sed -ir "s|server:.*|server: https://kind-control-plane:6443|" $(KIND_KUBE_CONFIG_FOLDER)/config \ No newline at end of file diff --git a/tests/conformance_test.go b/tests/conformance_test.go new file mode 100644 index 0000000000..e491fd0c61 --- /dev/null +++ b/tests/conformance_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package tests + +import ( + "testing" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/config" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + "sigs.k8s.io/gateway-api/apis/v1beta1" + "sigs.k8s.io/gateway-api/conformance/tests" + "sigs.k8s.io/gateway-api/conformance/utils/flags" + "sigs.k8s.io/gateway-api/conformance/utils/suite" +) + +func TestConformance(t *testing.T) { + cfg, err := config.GetConfig() + if err != nil { + t.Fatalf("Error loading Kubernetes config: %v", err) + } + client, err := client.New(cfg, client.Options{}) + if err != nil { + t.Fatalf("Error initializing Kubernetes client: %v", err) + } + v1alpha2.AddToScheme(client.Scheme()) + v1beta1.AddToScheme(client.Scheme()) + + var skipTests []string + for _, test := range tests.ConformanceTests { + if test.ShortName != "GatewayObservedGenerationBump" { + skipTests = append(skipTests, test.ShortName) + } + } + + t.Logf("Running conformance tests with %s GatewayClass\n cleanup: %t\n debug: %t\n enable all features: %t \n supported features: [%v]\n exempt features: [%v]", + *flags.GatewayClassName, *flags.CleanupBaseResources, *flags.ShowDebug, *flags.EnableAllSupportedFeatures, *flags.SupportedFeatures, *flags.ExemptFeatures) + + cSuite := suite.New(suite.Options{ + Client: client, + GatewayClassName: *flags.GatewayClassName, + Debug: *flags.ShowDebug, + CleanupBaseResources: *flags.CleanupBaseResources, + SupportedFeatures: nil, + EnableAllSupportedFeatures: *flags.EnableAllSupportedFeatures, + SkipTests: skipTests, + }) + cSuite.Setup(t) + cSuite.Run(t, tests.ConformanceTests) +}