diff --git a/README.md b/README.md index 8ecb0f4aa9..c951ea2e9b 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ NGINX Kubernetes Gateway is an open-source project that provides an implementation of the [Gateway API](https://gateway-api.sigs.k8s.io/) using [NGINX](https://nginx.org/) as the data plane. The goal of this project is to implement the core Gateway APIs -- `Gateway`, `GatewayClass`, `HTTPRoute`, `TCPRoute`, `TLSRoute`, and `UDPRoute` -- to configure an HTTP or TCP/UDP load balancer, reverse-proxy, or API gateway for applications running on Kubernetes. NGINX Kubernetes Gateway is currently under development and supports a subset of the Gateway API. -For a list of supported Gateway API resources and features, see the [Gateway API Compatibility](docs/gateway-api-compatibility.md.md) doc. +For a list of supported Gateway API resources and features, see the [Gateway API Compatibility](docs/gateway-api-compatibility.md) doc. > Warning: This project is actively in development (beta feature state) and should not be deployed in a production environment. > All APIs, SDKs, designs, and packages are subject to change. diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index a944dd77d8..98b591b2e8 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -82,6 +82,8 @@ spec: volumes: - name: nginx-config emptyDir: { } + - name: var-lib-nginx + emptyDir: { } - name: njs-modules configMap: name: njs-modules @@ -118,5 +120,7 @@ spec: volumeMounts: - name: nginx-config mountPath: /etc/nginx + - name: var-lib-nginx + mountPath: /var/lib/nginx - name: njs-modules mountPath: /usr/lib/nginx/modules/njs diff --git a/docs/gateway-api-compatibility.md.md b/docs/gateway-api-compatibility.md similarity index 97% rename from docs/gateway-api-compatibility.md.md rename to docs/gateway-api-compatibility.md index d0ce025dbf..fdae4a808c 100644 --- a/docs/gateway-api-compatibility.md.md +++ b/docs/gateway-api-compatibility.md @@ -91,7 +91,7 @@ Fields: * `type` - supported. * `requestRedirect` - supported except for the experimental `path` field. If multiple filters with `requestRedirect` are configured, NGINX Kubernetes Gateway will choose the first one and ignore the rest. * `requestHeaderModifier`, `requestMirror`, `urlRewrite`, `extensionRef` - not supported. - * `backendRefs` - partially supported. Only a single backend ref without support for `weight`. Backend ref `filters` are not supported. + * `backendRefs` - partially supported. Backend ref `filters` are not supported. * `status` * `parents` * `parentRef` - supported. diff --git a/examples/traffic-splitting/README.md b/examples/traffic-splitting/README.md new file mode 100644 index 0000000000..205554b9fe --- /dev/null +++ b/examples/traffic-splitting/README.md @@ -0,0 +1,101 @@ +# Example + +In this example we will deploy NGINX Kubernetes Gateway and configure traffic splitting for a simple cafe application. +We will use `HTTPRoute` resources to split traffic between two versions of the application -- `coffee-v1` and `coffee-v2`. + +## Running the Example + +## 1. Deploy NGINX Kubernetes Gateway + +1. Follow the [installation instructions](/docs/installation.md) to deploy NGINX Gateway. + +1. Save the public IP address of NGINX Kubernetes Gateway into a shell variable: + + ``` + GW_IP=XXX.YYY.ZZZ.III + ``` + +1. Save the port of NGINX Kubernetes Gateway: + + ``` + GW_PORT= + ``` + +## 2. Deploy the Coffee Application + +1. Create the Cafe Deployments and Services: + + ``` + kubectl apply -f cafe.yaml + ``` + +1. Check that the Pods are running in the `default` namespace: + + ``` + kubectl -n default get pods + NAME READY STATUS RESTARTS AGE + coffee-v1-7c57c576b-rfjsh 1/1 Running 0 21m + coffee-v2-698f66dc46-vcb6r 1/1 Running 0 21m + ``` + +## 3. Configure Routing + +1. Create the `Gateway`: + + ``` + kubectl apply -f gateway.yaml + ``` + +1. Create the `HTTPRoute` resources: + + ``` + kubectl apply -f cafe-route.yaml + ``` + +This `HTTPRoute` resource defines a route for the path `/coffee` that sends 80% of the requests to `coffee-v1` and 20% to `coffee-v2`. +In this example, we use 80 and 20; however, the weights are calculated proportionally and do not need to sum to 100. +For example, the weights of 8 and 2, 16 and 4, or 32 and 8 all evaluate to the same relative proportions. + +## 4. Test the Application + +To access the application, we will use `curl` to send requests to `/coffee`: + +``` +curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/coffee +``` + +80% of the responses will come from `coffee-v1`: + +``` +Server address: 10.12.0.18:80 +Server name: coffee-v1-7c57c576b-rfjsh +``` + +20% of the responses will come from `coffee-v2`: + +``` +Server address: 10.12.0.19:80 +Server name: coffee-v2-698f66dc46-vcb6r +``` + +### 5. Modify the Traffic Split Configuration + +Let's shift more of the traffic to `coffee-v2`. To do this we will update the `HTTPRoute` resource and change the weight +of the `coffee-v2` backend to 80. Backends with equal weights will receive an equal share of traffic. + +1. Apply the updated `HTTPRoute` resource: + + ``` + kubectl apply -f cafe-route-equal-weight.yaml + ``` + +2. Test the application again: + + ``` + curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/coffee + ``` + +The responses will now be split evenly between `coffee-v1` and `coffee-v2`. + +We can continue modifying the weights of the backends to shift more and more traffic to `coffee-v2`. If there's an issue +with `coffee-v2`, we can quickly shift traffic back to `coffee-v1`. diff --git a/examples/traffic-splitting/cafe-route-equal-weight.yaml b/examples/traffic-splitting/cafe-route-equal-weight.yaml new file mode 100644 index 0000000000..f979034e4c --- /dev/null +++ b/examples/traffic-splitting/cafe-route-equal-weight.yaml @@ -0,0 +1,22 @@ +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute +metadata: + name: cafe-route +spec: + parentRefs: + - name: gateway + sectionName: http + hostnames: + - "cafe.example.com" + rules: + - matches: + - path: + type: PathPrefix + value: /coffee + backendRefs: + - name: coffee-v1 + port: 80 + weight: 80 + - name: coffee-v2 + port: 80 + weight: 80 diff --git a/examples/traffic-splitting/cafe-route.yaml b/examples/traffic-splitting/cafe-route.yaml new file mode 100644 index 0000000000..b25cf71d9b --- /dev/null +++ b/examples/traffic-splitting/cafe-route.yaml @@ -0,0 +1,22 @@ +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute +metadata: + name: cafe-route +spec: + parentRefs: + - name: gateway + sectionName: http + hostnames: + - "cafe.example.com" + rules: + - matches: + - path: + type: PathPrefix + value: /coffee + backendRefs: + - name: coffee-v1 + port: 80 + weight: 80 + - name: coffee-v2 + port: 80 + weight: 20 diff --git a/examples/traffic-splitting/cafe.yaml b/examples/traffic-splitting/cafe.yaml new file mode 100644 index 0000000000..82dc493850 --- /dev/null +++ b/examples/traffic-splitting/cafe.yaml @@ -0,0 +1,65 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: coffee-v1 +spec: + replicas: 1 + selector: + matchLabels: + app: coffee-v1 + template: + metadata: + labels: + app: coffee-v1 + spec: + containers: + - name: coffee-v1 + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 +--- +apiVersion: v1 +kind: Service +metadata: + name: coffee-v1 +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: coffee-v1 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: coffee-v2 +spec: + replicas: 1 + selector: + matchLabels: + app: coffee-v2 + template: + metadata: + labels: + app: coffee-v2 + spec: + containers: + - name: coffee-v2 + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 +--- +apiVersion: v1 +kind: Service +metadata: + name: coffee-v2 +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: coffee-v2 diff --git a/examples/traffic-splitting/gateway.yaml b/examples/traffic-splitting/gateway.yaml new file mode 100644 index 0000000000..9fb0ebd1af --- /dev/null +++ b/examples/traffic-splitting/gateway.yaml @@ -0,0 +1,12 @@ +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: Gateway +metadata: + name: gateway + labels: + domain: k8s-gateway.nginx.org +spec: + gatewayClassName: nginx + listeners: + - name: http + port: 80 + protocol: HTTP diff --git a/internal/nginx/config/execute.go b/internal/nginx/config/execute.go new file mode 100644 index 0000000000..4a65a1d424 --- /dev/null +++ b/internal/nginx/config/execute.go @@ -0,0 +1,18 @@ +package config + +import ( + "bytes" + "text/template" +) + +// executes the template with the given data. +func execute(template *template.Template, data interface{}) []byte { + var buf bytes.Buffer + + err := template.Execute(&buf, data) + if err != nil { + panic(err) + } + + return buf.Bytes() +} diff --git a/internal/nginx/config/execute_test.go b/internal/nginx/config/execute_test.go new file mode 100644 index 0000000000..618fa438e0 --- /dev/null +++ b/internal/nginx/config/execute_test.go @@ -0,0 +1,30 @@ +package config + +import ( + "testing" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" +) + +func TestExecute(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Errorf("execute() panicked with %v", r) + } + }() + + bytes := execute(serversTemplate, []http.Server{}) + if len(bytes) == 0 { + t.Error("template.execute() did not generate anything") + } +} + +func TestExecutePanics(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("template.execute() did not panic") + } + }() + + _ = execute(serversTemplate, "not-correct-data") +} diff --git a/internal/nginx/config/generator.go b/internal/nginx/config/generator.go index 1bb1e21b3f..8b9de2670a 100644 --- a/internal/nginx/config/generator.go +++ b/internal/nginx/config/generator.go @@ -1,336 +1,42 @@ package config import ( - "encoding/json" - "fmt" - "strings" - - "sigs.k8s.io/gateway-api/apis/v1beta1" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) -// 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. +// This interface is used for testing purposes only. type Generator interface { // Generate generates NGINX configuration from internal representation. Generate(configuration state.Configuration) []byte } -// GeneratorImpl is an implementation of Generator -type GeneratorImpl struct { - executor *templateExecutor -} +// GeneratorImpl is an implementation of Generator. +type GeneratorImpl struct{} // NewGeneratorImpl creates a new GeneratorImpl. -func NewGeneratorImpl() *GeneratorImpl { - return &GeneratorImpl{ - executor: newTemplateExecutor(), - } +func NewGeneratorImpl() GeneratorImpl { + return GeneratorImpl{} } -func (g *GeneratorImpl) Generate(conf state.Configuration) []byte { - httpServers := generateHTTPServers(conf) - - httpUpstreams := generateHTTPUpstreams(conf.Upstreams) - - retVal := append(g.executor.ExecuteForHTTPUpstreams(httpUpstreams), g.executor.ExecuteForHTTPServers(httpServers)...) +// executeFunc is a function that generates NGINX configuration from internal representation. +type executeFunc func(configuration state.Configuration) []byte - return retVal -} - -func generateHTTPServers(conf state.Configuration) httpServers { - confServers := append(conf.HTTPServers, conf.SSLServers...) - - servers := make([]server, 0, len(confServers)+2) - - if len(conf.HTTPServers) > 0 { - defaultHTTPServer := generateDefaultHTTPServer() - - servers = append(servers, defaultHTTPServer) +func (g GeneratorImpl) Generate(conf state.Configuration) []byte { + var generated []byte + for _, execute := range getExecuteFuncs() { + generated = append(generated, execute(conf)...) } - if len(conf.SSLServers) > 0 { - defaultSSLServer := generateDefaultSSLServer() - - servers = append(servers, defaultSSLServer) - } - - for _, s := range confServers { - servers = append(servers, generateServer(s)) - } - - return httpServers{Servers: servers} -} - -func generateDefaultSSLServer() server { - return server{IsDefaultSSL: true} + return generated } -func generateDefaultHTTPServer() server { - return server{IsDefaultHTTP: true} -} - -func generateServer(virtualServer state.VirtualServer) server { - s := server{ServerName: virtualServer.Hostname} - - listenerPort := 80 - - if virtualServer.SSL != nil { - s.SSL = &ssl{ - Certificate: virtualServer.SSL.CertificatePath, - CertificateKey: virtualServer.SSL.CertificatePath, - } - - listenerPort = 443 +func getExecuteFuncs() []executeFunc { + return []executeFunc{ + executeUpstreams, + executeSplitClients, + executeServers, } - - if len(virtualServer.PathRules) == 0 { - // generate default "/" 404 location - s.Locations = []location{{Path: "/", Return: &returnVal{Code: statusNotFound}}} - return s - } - - locs := make([]location, 0, len(virtualServer.PathRules)) // FIXME(pleshakov): expand with rule.Routes - for _, rule := range virtualServer.PathRules { - matches := make([]httpMatch, 0, len(rule.MatchRules)) - - for matchRuleIdx, r := range rule.MatchRules { - m := r.GetMatch() - - var loc location - - // handle case where the only route is a path-only match - // generate a standard location block without http_matches. - if len(rule.MatchRules) == 1 && isPathOnlyMatch(m) { - loc = location{ - Path: rule.Path, - } - } else { - path := createPathForMatch(rule.Path, matchRuleIdx) - loc = generateMatchLocation(path) - matches = append(matches, createHTTPMatch(m, path)) - } - - // FIXME(pleshakov): There could be a case when the filter has the type set but not the corresponding field. - // For example, type is v1beta1.HTTPRouteFilterRequestRedirect, but RequestRedirect field is nil. - // The validation webhook catches that. - // If it doesn't work as expected, such situation is silently handled below in findFirstFilters. - // Consider reporting an error. But that should be done in a separate validation layer. - - // RequestRedirect and proxying are mutually exclusive. - if r.Filters.RequestRedirect != nil { - loc.Return = generateReturnValForRedirectFilter(r.Filters.RequestRedirect, listenerPort) - } else { - loc.ProxyPass = generateProxyPass(r.UpstreamName) - } - - locs = append(locs, loc) - } - - if len(matches) > 0 { - b, err := json.Marshal(matches) - if err != nil { - // panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly. - panic(fmt.Errorf("could not marshal http match: %w", err)) - } - - pathLoc := location{ - Path: rule.Path, - HTTPMatchVar: string(b), - } - - locs = append(locs, pathLoc) - } - } - - s.Locations = locs - - return s -} - -func generateReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int) *returnVal { - if filter == nil { - return nil - } - - hostname := "$host" - if filter.Hostname != nil { - hostname = string(*filter.Hostname) - } - - // FIXME(pleshakov): Unknown values here must result in the implementation setting the Attached Condition for - // the Route to `status: False`, with a Reason of `UnsupportedValue`. In that case, all routes of the Route will be - // ignored. NGINX will return 500. This should be implemented in the validation layer. - code := statusFound - if filter.StatusCode != nil { - code = statusCode(*filter.StatusCode) - } - - port := listenerPort - if filter.Port != nil { - port = int(*filter.Port) - } - - // FIXME(pleshakov): Same as the FIXME about StatusCode above. - scheme := "$scheme" - if filter.Scheme != nil { - scheme = *filter.Scheme - } - - return &returnVal{ - Code: code, - URL: fmt.Sprintf("%s://%s:%d$request_uri", scheme, hostname, port), - } -} - -func generateHTTPUpstreams(upstreams []state.Upstream) httpUpstreams { - // capacity is the number of upstreams + 1 for the invalid backend ref upstream - ups := make([]upstream, 0, len(upstreams)+1) - for _, u := range upstreams { - ups = append(ups, generateUpstream(u)) - } - - ups = append(ups, generateInvalidBackendRefUpstream()) - - return httpUpstreams{ - Upstreams: ups, - } -} - -func generateUpstream(up state.Upstream) upstream { - if len(up.Endpoints) == 0 { - return upstream{ - Name: up.Name, - Servers: []upstreamServer{ - { - Address: nginx502Server, - }, - }, - } - } - - upstreamServers := make([]upstreamServer, len(up.Endpoints)) - for idx, ep := range up.Endpoints { - upstreamServers[idx] = upstreamServer{ - Address: fmt.Sprintf("%s:%d", ep.Address, ep.Port), - } - } - - return upstream{ - Name: up.Name, - Servers: upstreamServers, - } -} - -func generateInvalidBackendRefUpstream() upstream { - return upstream{ - Name: state.InvalidBackendRef, - Servers: []upstreamServer{ - { - Address: nginx502Server, - }, - }, - } -} - -func generateProxyPass(address string) string { - return "http://" + address -} - -func generateMatchLocation(path string) location { - return location{ - Path: path, - Internal: true, - } -} - -func createPathForMatch(path string, routeIdx int) string { - return fmt.Sprintf("%s_route%d", path, routeIdx) -} - -// httpMatch is an internal representation of an HTTPRouteMatch. -// This struct is marshaled into a string and stored as a variable in the nginx location block for the route's path. -// The NJS httpmatches module will lookup this variable on the request object and compare the request against the Method, Headers, and QueryParams contained in httpMatch. -// If the request satisfies the httpMatch, the request will be internally redirected to the location RedirectPath by NGINX. -type httpMatch struct { - // Any represents a match with no match conditions. - Any bool `json:"any,omitempty"` - // Method is the HTTPMethod of the HTTPRouteMatch. - Method v1beta1.HTTPMethod `json:"method,omitempty"` - // Headers is a list of HTTPHeaders name value pairs with the format "{name}:{value}". - Headers []string `json:"headers,omitempty"` - // QueryParams is a list of HTTPQueryParams name value pairs with the format "{name}={value}". - QueryParams []string `json:"params,omitempty"` - // RedirectPath is the path to redirect the request to if the request satisfies the match conditions. - RedirectPath string `json:"redirectPath,omitempty"` -} - -func createHTTPMatch(match v1beta1.HTTPRouteMatch, redirectPath string) httpMatch { - hm := httpMatch{ - RedirectPath: redirectPath, - } - - if isPathOnlyMatch(match) { - hm.Any = true - return hm - } - - if match.Method != nil { - hm.Method = *match.Method - } - - if match.Headers != nil { - headers := make([]string, 0, len(match.Headers)) - headerNames := make(map[string]struct{}) - - // FIXME(kate-osborn): For now we only support type "Exact". - for _, h := range match.Headers { - if *h.Type == v1beta1.HeaderMatchExact { - // duplicate header names are not permitted by the spec - // only configure the first entry for every header name (case-insensitive) - lowerName := strings.ToLower(string(h.Name)) - if _, ok := headerNames[lowerName]; !ok { - headers = append(headers, createHeaderKeyValString(h)) - headerNames[lowerName] = struct{}{} - } - } - } - hm.Headers = headers - } - - if match.QueryParams != nil { - params := make([]string, 0, len(match.QueryParams)) - - // FIXME(kate-osborn): For now we only support type "Exact". - for _, p := range match.QueryParams { - if *p.Type == v1beta1.QueryParamMatchExact { - params = append(params, createQueryParamKeyValString(p)) - } - } - hm.QueryParams = params - } - - return hm -} - -// The name and values are delimited by "=". A name and value can always be recovered using strings.SplitN(arg,"=", 2). -// Query Parameters are case-sensitive so case is preserved. -func createQueryParamKeyValString(p v1beta1.HTTPQueryParamMatch) string { - return p.Name + "=" + p.Value -} - -// The name and values are delimited by ":". A name and value can always be recovered using strings.Split(arg, ":"). -// Header names are case-insensitive while header values are case-sensitive (e.g. foo:bar == FOO:bar, but foo:bar != foo:BAR). -// We preserve the case of the name here because NGINX allows us to lookup the header names in a case-insensitive manner. -func createHeaderKeyValString(h v1beta1.HTTPHeaderMatch) string { - return string(h.Name) + ":" + h.Value -} - -func isPathOnlyMatch(match v1beta1.HTTPRouteMatch) bool { - return match.Method == nil && match.Headers == nil && match.QueryParams == nil } diff --git a/internal/nginx/config/generator_test.go b/internal/nginx/config/generator_test.go index 6f7614e884..593d112f69 100644 --- a/internal/nginx/config/generator_test.go +++ b/internal/nginx/config/generator_test.go @@ -1,24 +1,26 @@ -package config +package config_test import ( - "encoding/json" - "fmt" "strings" "testing" - "github.com/google/go-cmp/cmp" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/gateway-api/apis/v1beta1" + "k8s.io/apimachinery/pkg/types" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" ) +// Note: this test only verifies that Generate() returns a byte array with upstream, server, and split_client blocks. +// It does not test the correctness of those blocks. That functionality is covered by other tests in this package. func TestGenerate(t *testing.T) { - // Note: this test only verifies that Generate() returns a byte array with http upstreams and server blocks. - // It does not test the correctness of those blocks. That functionality is covered by other tests in this file. - generator := NewGeneratorImpl() + bg := state.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 0, + Backends: []state.BackendRef{ + {Name: "test", Valid: true, Weight: 1}, + {Name: "test2", Valid: true, Weight: 1}, + }, + } conf := state.Configuration{ HTTPServers: []state.VirtualServer{ @@ -37,8 +39,9 @@ func TestGenerate(t *testing.T) { Endpoints: nil, }, }, + BackendGroups: []state.BackendGroup{bg}, } - + generator := config.NewGeneratorImpl() cfg := string(generator.Generate(conf)) if !strings.Contains(cfg, "listen 80") { @@ -52,902 +55,8 @@ func TestGenerate(t *testing.T) { if !strings.Contains(cfg, "upstream") { t.Errorf("Generate() did not generate a config with an upstream block; config: %s", cfg) } -} - -func TestGenerateForDefaultServers(t *testing.T) { - generator := NewGeneratorImpl() - - testcases := []struct { - conf state.Configuration - httpDefault bool - sslDefault bool - msg string - }{ - { - conf: state.Configuration{}, - httpDefault: false, - sslDefault: false, - msg: "no servers", - }, - { - conf: state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "example.com", - }, - }, - }, - httpDefault: true, - sslDefault: false, - msg: "only HTTP servers", - }, - { - conf: state.Configuration{ - SSLServers: []state.VirtualServer{ - { - Hostname: "example.com", - }, - }, - }, - httpDefault: false, - sslDefault: true, - msg: "only HTTPS servers", - }, - { - conf: state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "example.com", - }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "example.com", - }, - }, - }, - httpDefault: true, - sslDefault: true, - msg: "both HTTP and HTTPS servers", - }, - } - - for _, tc := range testcases { - cfg := generator.Generate(tc.conf) - - defaultSSLExists := strings.Contains(string(cfg), "listen 443 ssl default_server") - defaultHTTPExists := strings.Contains(string(cfg), "listen 80 default_server") - - if tc.sslDefault && !defaultSSLExists { - t.Errorf("Generate() did not generate a config with a default TLS termination server for test: %q", tc.msg) - } - - if !tc.sslDefault && defaultSSLExists { - t.Errorf("Generate() generated a config with a default TLS termination server for test: %q", tc.msg) - } - - if tc.httpDefault && !defaultHTTPExists { - t.Errorf("Generate() did not generate a config with a default http server for test: %q", tc.msg) - } - - if !tc.httpDefault && defaultHTTPExists { - t.Errorf("Generate() generated a config with a default http server for test: %q", tc.msg) - } - - if len(cfg) == 0 { - t.Errorf("Generate() generated empty config for test: %q", tc.msg) - } - } -} - -func TestGenerateHTTPServers(t *testing.T) { - const ( - certPath = "/etc/nginx/secrets/cert" - ) - - hr := &v1beta1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "route1", - }, - Spec: v1beta1.HTTPRouteSpec{ - Hostnames: []v1beta1.Hostname{ - "cafe.example.com", - }, - Rules: []v1beta1.HTTPRouteRule{ - { - // matches with path and methods - Matches: []v1beta1.HTTPRouteMatch{ - { - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/"), - }, - Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPost), - }, - { - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/"), - }, - Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPatch), - }, - { - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/"), // should generate an "any" httpmatch since other matches exists for / - }, - }, - }, - }, - { - // A match with all possible fields set - Matches: []v1beta1.HTTPRouteMatch{ - { - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/test"), - }, - Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodGet), - Headers: []v1beta1.HTTPHeaderMatch{ - { - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), - Name: "Version", - Value: "V1", - }, - { - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), - Name: "test", - Value: "foo", - }, - { - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), - Name: "my-header", - Value: "my-value", - }, - }, - QueryParams: []v1beta1.HTTPQueryParamMatch{ - { - Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), - Name: "GrEat", // query names and values should not be normalized to lowercase - Value: "EXAMPLE", - }, - { - Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), - Name: "test", - Value: "foo=bar", - }, - }, - }, - }, - }, - { - // A match with just path - Matches: []v1beta1.HTTPRouteMatch{ - { - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/path-only"), - }, - }, - }, - }, - { - // A match with a redirect with implicit port - Matches: []v1beta1.HTTPRouteMatch{ - { - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/redirect-implicit-port"), - }, - }, - }, - // redirect is set in the corresponding state.MatchRule - }, - { - // A match with a redirect with explicit port - Matches: []v1beta1.HTTPRouteMatch{ - { - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/redirect-explicit-port"), - }, - }, - }, - // redirect is set in the corresponding state.MatchRule - }, - }, - }, - } - - cafePathRules := []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - UpstreamName: "test_foo_80", - Source: hr, - }, - { - MatchIdx: 1, - RuleIdx: 0, - UpstreamName: "test_foo_80", - Source: hr, - }, - { - MatchIdx: 2, - RuleIdx: 0, - UpstreamName: "test_foo_80", - Source: hr, - }, - }, - }, - { - Path: "/test", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 1, - UpstreamName: "test_bar_80", - Source: hr, - }, - }, - }, - { - Path: "/path-only", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 2, - UpstreamName: "test_baz_80", - Source: hr, - }, - }, - }, - { - Path: "/redirect-implicit-port", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 3, - Source: hr, - Filters: state.Filters{ - RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ - Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("foo.example.com")), - }, - }, - }, - }, - }, - { - Path: "/redirect-explicit-port", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 4, - Source: hr, - Filters: state.Filters{ - RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ - Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("bar.example.com")), - Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(8080)), - }, - }, - }, - }, - }, - } - - httpServers := []state.VirtualServer{ - { - Hostname: "cafe.example.com", - PathRules: cafePathRules, - }, - } - - sslServers := []state.VirtualServer{ - { - Hostname: "cafe.example.com", - SSL: &state.SSL{CertificatePath: certPath}, - PathRules: cafePathRules, - }, - } - - expectedMatchString := func(m []httpMatch) string { - b, err := json.Marshal(m) - if err != nil { - t.Errorf("error marshaling test match: %v", err) - } - return string(b) - } - - slashMatches := []httpMatch{ - {Method: v1beta1.HTTPMethodPost, RedirectPath: "/_route0"}, - {Method: v1beta1.HTTPMethodPatch, RedirectPath: "/_route1"}, - {Any: true, RedirectPath: "/_route2"}, - } - testMatches := []httpMatch{ - { - Method: v1beta1.HTTPMethodGet, - Headers: []string{"Version:V1", "test:foo", "my-header:my-value"}, - QueryParams: []string{"GrEat=EXAMPLE", "test=foo=bar"}, - RedirectPath: "/test_route0", - }, - } - - getExpectedLocations := func(isHTTPS bool) []location { - port := 80 - if isHTTPS { - port = 443 - } - - return []location{ - { - Path: "/_route0", - Internal: true, - ProxyPass: "http://test_foo_80", - }, - { - Path: "/_route1", - Internal: true, - ProxyPass: "http://test_foo_80", - }, - { - Path: "/_route2", - Internal: true, - ProxyPass: "http://test_foo_80", - }, - { - Path: "/", - HTTPMatchVar: expectedMatchString(slashMatches), - }, - { - Path: "/test_route0", - Internal: true, - ProxyPass: "http://test_bar_80", - }, - { - Path: "/test", - HTTPMatchVar: expectedMatchString(testMatches), - }, - { - Path: "/path-only", - ProxyPass: "http://test_baz_80", - }, - { - Path: "/redirect-implicit-port", - Return: &returnVal{ - Code: 302, - URL: fmt.Sprintf("$scheme://foo.example.com:%d$request_uri", port), - }, - }, - { - Path: "/redirect-explicit-port", - Return: &returnVal{ - Code: 302, - URL: "$scheme://bar.example.com:8080$request_uri", - }, - }, - } - } - - expectedServers := []server{ - { - IsDefaultHTTP: true, - }, - { - IsDefaultSSL: true, - }, - { - ServerName: "cafe.example.com", - Locations: getExpectedLocations(false), - }, - { - ServerName: "cafe.example.com", - SSL: &ssl{Certificate: certPath, CertificateKey: certPath}, - Locations: getExpectedLocations(true), - }, - } - - conf := state.Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - } - - result := generateHTTPServers(conf) - - if diff := cmp.Diff(expectedServers, result.Servers); diff != "" { - t.Errorf("generate() mismatch on servers (-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) - } -} - -func TestGenerateHTTPUpstreams(t *testing.T) { - stateUpstreams := []state.Upstream{ - { - Name: "up1", - Endpoints: []resolver.Endpoint{ - { - Address: "10.0.0.0", - Port: 80, - }, - { - Address: "10.0.0.1", - Port: 80, - }, - { - Address: "10.0.0.2", - Port: 80, - }, - }, - }, - { - Name: "up2", - Endpoints: []resolver.Endpoint{ - { - Address: "11.0.0.0", - Port: 80, - }, - }, - }, - { - Name: "up3", - Endpoints: []resolver.Endpoint{}, - }, - } - - expUpstreams := httpUpstreams{ - Upstreams: []upstream{ - { - Name: "up1", - Servers: []upstreamServer{ - { - Address: "10.0.0.0:80", - }, - { - Address: "10.0.0.1:80", - }, - { - Address: "10.0.0.2:80", - }, - }, - }, - { - Name: "up2", - Servers: []upstreamServer{ - { - Address: "11.0.0.0:80", - }, - }, - }, - { - Name: "up3", - Servers: []upstreamServer{ - { - Address: nginx502Server, - }, - }, - }, - { - Name: state.InvalidBackendRef, - Servers: []upstreamServer{ - { - Address: nginx502Server, - }, - }, - }, - }, - } - result := generateHTTPUpstreams(stateUpstreams) - if diff := cmp.Diff(expUpstreams, result); diff != "" { - t.Errorf("generateHTTPUpstreams() mismatch (-want +got):\n%s", diff) - } -} - -func TestGenerateUpstream(t *testing.T) { - tests := []struct { - stateUpstream state.Upstream - expectedUpstream upstream - msg string - }{ - { - stateUpstream: state.Upstream{ - Name: "nil-endpoints", - Endpoints: nil, - }, - expectedUpstream: upstream{Name: "nil-endpoints", Servers: []upstreamServer{{Address: nginx502Server}}}, - msg: "nil endpoints", - }, - { - stateUpstream: state.Upstream{ - Name: "no-endpoints", - Endpoints: []resolver.Endpoint{}, - }, - expectedUpstream: upstream{Name: "no-endpoints", Servers: []upstreamServer{{Address: nginx502Server}}}, - msg: "no endpoints", - }, - { - stateUpstream: state.Upstream{ - Name: "multiple-endpoints", - Endpoints: []resolver.Endpoint{ - { - Address: "10.0.0.1", - Port: 80, - }, - { - Address: "10.0.0.2", - Port: 80, - }, - { - Address: "10.0.0.3", - Port: 80, - }, - }, - }, - expectedUpstream: upstream{Name: "multiple-endpoints", Servers: []upstreamServer{{Address: "10.0.0.1:80"}, {Address: "10.0.0.2:80"}, {Address: "10.0.0.3:80"}}}, - msg: "multiple endpoints", - }, - } - - for _, test := range tests { - result := generateUpstream(test.stateUpstream) - if diff := cmp.Diff(test.expectedUpstream, result); diff != "" { - t.Errorf("generateUpstream() %q mismatch (-want +got):\n%s", test.msg, diff) - } - } -} - -func TestGenerateReturnValForRedirectFilter(t *testing.T) { - const listenerPort = 123 - - tests := []struct { - filter *v1beta1.HTTPRequestRedirectFilter - expected *returnVal - msg string - }{ - { - filter: nil, - expected: nil, - msg: "filter is nil", - }, - { - filter: &v1beta1.HTTPRequestRedirectFilter{}, - expected: &returnVal{ - Code: statusFound, - URL: "$scheme://$host:123$request_uri", - }, - msg: "all fields are empty", - }, - { - filter: &v1beta1.HTTPRequestRedirectFilter{ - Scheme: helpers.GetStringPointer("https"), - Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("foo.example.com")), - Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(2022)), - StatusCode: helpers.GetIntPointer(101), - }, - expected: &returnVal{ - Code: 101, - URL: "https://foo.example.com:2022$request_uri", - }, - msg: "all fields are set", - }, - } - - for _, test := range tests { - result := generateReturnValForRedirectFilter(test.filter, listenerPort) - if diff := cmp.Diff(test.expected, result); diff != "" { - t.Errorf("generateReturnValForRedirectFilter() mismatch '%s' (-want +got):\n%s", test.msg, diff) - } - } -} - -func TestGenerateMatchLocation(t *testing.T) { - expected := location{ - Path: "/path", - Internal: true, - } - - result := generateMatchLocation("/path") - if result != expected { - t.Errorf("generateMatchLocation() returned %v but expected %v", result, expected) - } -} - -func TestCreatePathForMatch(t *testing.T) { - expected := "/path_route1" - - result := createPathForMatch("/path", 1) - if result != expected { - t.Errorf("createPathForMatch() returned %q but expected %q", result, expected) - } -} - -func TestCreateArgKeyValString(t *testing.T) { - expected := "key=value" - - result := createQueryParamKeyValString( - v1beta1.HTTPQueryParamMatch{ - Name: "key", - Value: "value", - }, - ) - if result != expected { - t.Errorf("createQueryParamKeyValString() returned %q but expected %q", result, expected) - } - - expected = "KeY=vaLUe==" - - result = createQueryParamKeyValString( - v1beta1.HTTPQueryParamMatch{ - Name: "KeY", - Value: "vaLUe==", - }, - ) - if result != expected { - t.Errorf("createQueryParamKeyValString() returned %q but expected %q", result, expected) - } -} - -func TestCreateHeaderKeyValString(t *testing.T) { - expected := "kEy:vALUe" - - result := createHeaderKeyValString( - v1beta1.HTTPHeaderMatch{ - Name: "kEy", - Value: "vALUe", - }, - ) - - if result != expected { - t.Errorf("createHeaderKeyValString() returned %q but expected %q", result, expected) - } -} - -func TestMatchLocationNeeded(t *testing.T) { - tests := []struct { - match v1beta1.HTTPRouteMatch - expected bool - msg string - }{ - { - match: v1beta1.HTTPRouteMatch{ - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/path"), - }, - }, - expected: true, - msg: "path only match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/path"), - }, - Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodGet), - }, - expected: false, - msg: "method defined in match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/path"), - }, - Headers: []v1beta1.HTTPHeaderMatch{ - { - Name: "header", - Value: "val", - }, - }, - }, - expected: false, - msg: "headers defined in match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/path"), - }, - QueryParams: []v1beta1.HTTPQueryParamMatch{ - { - Name: "arg", - Value: "val", - }, - }, - }, - expected: false, - msg: "query params defined in match", - }, - } - - for _, tc := range tests { - result := isPathOnlyMatch(tc.match) - - if result != tc.expected { - t.Errorf("isPathOnlyMatch() returned %t but expected %t for test case %q", result, tc.expected, tc.msg) - } - } -} - -func TestCreateHTTPMatch(t *testing.T) { - testPath := "/internal_loc" - - testPathMatch := v1beta1.HTTPPathMatch{Value: helpers.GetStringPointer("/")} - testMethodMatch := helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPut) - testHeaderMatches := []v1beta1.HTTPHeaderMatch{ - { - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), - Name: "header-1", - Value: "val-1", - }, - { - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), - Name: "header-2", - Value: "val-2", - }, - { - // regex type is not supported. This should not be added to the httpMatch headers. - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchRegularExpression), - Name: "ignore-this-header", - Value: "val", - }, - { - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), - Name: "header-3", - Value: "val-3", - }, - } - - testDuplicateHeaders := make([]v1beta1.HTTPHeaderMatch, 0, 5) - duplicateHeaderMatch := v1beta1.HTTPHeaderMatch{ - Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), - Name: "HEADER-2", // header names are case-insensitive - Value: "val-2", - } - testDuplicateHeaders = append(testDuplicateHeaders, testHeaderMatches...) - testDuplicateHeaders = append(testDuplicateHeaders, duplicateHeaderMatch) - - testQueryParamMatches := []v1beta1.HTTPQueryParamMatch{ - { - Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), - Name: "arg1", - Value: "val1", - }, - { - Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), - Name: "arg2", - Value: "val2=another-val", - }, - { - // regex type is not supported. This should not be added to the httpMatch args - Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchRegularExpression), - Name: "ignore-this-arg", - Value: "val", - }, - { - Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), - Name: "arg3", - Value: "==val3", - }, - } - - expectedHeaders := []string{"header-1:val-1", "header-2:val-2", "header-3:val-3"} - expectedArgs := []string{"arg1=val1", "arg2=val2=another-val", "arg3===val3"} - - tests := []struct { - match v1beta1.HTTPRouteMatch - expected httpMatch - msg string - }{ - { - match: v1beta1.HTTPRouteMatch{ - Path: &testPathMatch, - }, - expected: httpMatch{ - Any: true, - RedirectPath: testPath, - }, - msg: "path only match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Path: &testPathMatch, // A path match with a method should not set the Any field to true - Method: testMethodMatch, - }, - expected: httpMatch{ - Method: "PUT", - RedirectPath: testPath, - }, - msg: "method only match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Headers: testHeaderMatches, - }, - expected: httpMatch{ - RedirectPath: testPath, - Headers: expectedHeaders, - }, - msg: "headers only match", - }, - { - match: v1beta1.HTTPRouteMatch{ - QueryParams: testQueryParamMatches, - }, - expected: httpMatch{ - QueryParams: expectedArgs, - RedirectPath: testPath, - }, - msg: "query params only match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Method: testMethodMatch, - QueryParams: testQueryParamMatches, - }, - expected: httpMatch{ - Method: "PUT", - QueryParams: expectedArgs, - RedirectPath: testPath, - }, - msg: "method and query params match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Method: testMethodMatch, - Headers: testHeaderMatches, - }, - expected: httpMatch{ - Method: "PUT", - Headers: expectedHeaders, - RedirectPath: testPath, - }, - msg: "method and headers match", - }, - { - match: v1beta1.HTTPRouteMatch{ - QueryParams: testQueryParamMatches, - Headers: testHeaderMatches, - }, - expected: httpMatch{ - QueryParams: expectedArgs, - Headers: expectedHeaders, - RedirectPath: testPath, - }, - msg: "query params and headers match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Headers: testHeaderMatches, - QueryParams: testQueryParamMatches, - Method: testMethodMatch, - }, - expected: httpMatch{ - Method: "PUT", - Headers: expectedHeaders, - QueryParams: expectedArgs, - RedirectPath: testPath, - }, - msg: "method, headers, and query params match", - }, - { - match: v1beta1.HTTPRouteMatch{ - Headers: testDuplicateHeaders, - }, - expected: httpMatch{ - Headers: expectedHeaders, - RedirectPath: testPath, - }, - msg: "duplicate header names", - }, - } - for _, tc := range tests { - result := createHTTPMatch(tc.match, testPath) - if diff := helpers.Diff(result, tc.expected); diff != "" { - t.Errorf("createHTTPMatch() returned incorrect httpMatch for test case: %q, diff: %+v", tc.msg, diff) - } + if !strings.Contains(cfg, "split_clients") { + t.Errorf("Generate() did not generate a config with an split_clients block; config: %s", cfg) } } diff --git a/internal/nginx/config/http.go b/internal/nginx/config/http.go deleted file mode 100644 index b4a3dc1cc2..0000000000 --- a/internal/nginx/config/http.go +++ /dev/null @@ -1,51 +0,0 @@ -package config - -type httpServers struct { - Servers []server -} - -type httpUpstreams struct { - Upstreams []upstream -} - -type server struct { - SSL *ssl - ServerName string - Locations []location - IsDefaultHTTP bool - IsDefaultSSL bool -} - -type location struct { - Return *returnVal - Path string - ProxyPass string - HTTPMatchVar string - Internal bool -} - -type returnVal struct { - Code statusCode - URL string -} - -type ssl struct { - Certificate string - CertificateKey string -} - -type statusCode int - -const ( - statusFound statusCode = 302 - statusNotFound statusCode = 404 -) - -type upstream struct { - Name string - Servers []upstreamServer -} - -type upstreamServer struct { - Address string -} diff --git a/internal/nginx/config/http/config.go b/internal/nginx/config/http/config.go new file mode 100644 index 0000000000..bdb55a872e --- /dev/null +++ b/internal/nginx/config/http/config.go @@ -0,0 +1,64 @@ +package http + +// Server holds all configuration for an HTTP server. +type Server struct { + SSL *SSL + ServerName string + Locations []Location + IsDefaultHTTP bool + IsDefaultSSL bool +} + +// Location holds all configuration for an HTTP location. +type Location struct { + Return *Return + Path string + ProxyPass string + HTTPMatchVar string + Internal bool +} + +// Return represents an HTTP return. +type Return struct { + Code StatusCode + URL string +} + +// SSL holds all SSL related configuration. +type SSL struct { + Certificate string + CertificateKey string +} + +// StatusCode is an HTTP status code. +type StatusCode int + +const ( + // StatusFound is the HTTP 302 status code. + StatusFound StatusCode = 302 + // StatusNotFound is the HTTP 404 status code. + StatusNotFound StatusCode = 404 +) + +// Upstream holds all configuration for an HTTP upstream. +type Upstream struct { + Name string + Servers []UpstreamServer +} + +// UpstreamServer holds all configuration for an HTTP upstream server. +type UpstreamServer struct { + Address string +} + +// SplitClient holds all configuration for an HTTP split client. +type SplitClient struct { + VariableName string + Distributions []SplitClientDistribution +} + +// SplitClientDistribution maps Percentage to Value in a SplitClient. +type SplitClientDistribution struct { + Percent string + Value string +} diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go new file mode 100644 index 0000000000..2068fafd8d --- /dev/null +++ b/internal/nginx/config/servers.go @@ -0,0 +1,278 @@ +package config + +import ( + "encoding/json" + "fmt" + "strings" + gotemplate "text/template" + + "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" +) + +var serversTemplate = gotemplate.Must(gotemplate.New("servers").Parse(serversTemplateText)) + +func executeServers(conf state.Configuration) []byte { + servers := createServers(conf.HTTPServers, conf.SSLServers) + + return execute(serversTemplate, servers) +} + +func createServers(httpServers, sslServers []state.VirtualServer) []http.Server { + confServers := append(httpServers, sslServers...) + + servers := make([]http.Server, 0, len(confServers)+2) + + if len(httpServers) > 0 { + defaultHTTPServer := createDefaultHTTPServer() + + servers = append(servers, defaultHTTPServer) + } + + if len(sslServers) > 0 { + defaultSSLServer := createDefaultSSLServer() + + servers = append(servers, defaultSSLServer) + } + + for _, s := range confServers { + servers = append(servers, createServer(s)) + } + + return servers +} + +func createServer(virtualServer state.VirtualServer) http.Server { + s := http.Server{ + ServerName: virtualServer.Hostname, + } + + listenerPort := 80 + + if virtualServer.SSL != nil { + s.SSL = &http.SSL{ + Certificate: virtualServer.SSL.CertificatePath, + CertificateKey: virtualServer.SSL.CertificatePath, + } + + listenerPort = 443 + } + + if len(virtualServer.PathRules) == 0 { + // generate default "/" 404 location + s.Locations = []http.Location{{Path: "/", Return: &http.Return{Code: http.StatusNotFound}}} + return s + } + + locs := make([]http.Location, 0, len(virtualServer.PathRules)) // FIXME(pleshakov): expand with rule.Routes + + for _, rule := range virtualServer.PathRules { + matches := make([]httpMatch, 0, len(rule.MatchRules)) + + for matchRuleIdx, r := range rule.MatchRules { + m := r.GetMatch() + + var loc http.Location + + // handle case where the only route is a path-only match + // generate a standard location block without http_matches. + if len(rule.MatchRules) == 1 && isPathOnlyMatch(m) { + loc = http.Location{ + Path: rule.Path, + } + } else { + path := createPathForMatch(rule.Path, matchRuleIdx) + loc = createMatchLocation(path) + matches = append(matches, createHTTPMatch(m, path)) + } + + // FIXME(pleshakov): There could be a case when the filter has the type set but not the corresponding field. + // For example, type is v1beta1.HTTPRouteFilterRequestRedirect, but RequestRedirect field is nil. + // The validation webhook catches that. + // If it doesn't work as expected, such situation is silently handled below in findFirstFilters. + // Consider reporting an error. But that should be done in a separate validation layer. + + // RequestRedirect and proxying are mutually exclusive. + if r.Filters.RequestRedirect != nil { + loc.Return = createReturnValForRedirectFilter(r.Filters.RequestRedirect, listenerPort) + + locs = append(locs, loc) + continue + } + + backendName := backendGroupName(r.BackendGroup) + + if backendGroupNeedsSplit(r.BackendGroup) { + loc.ProxyPass = createProxyPassForVar(backendName) + } else { + loc.ProxyPass = createProxyPass(backendName) + } + + locs = append(locs, loc) + } + + if len(matches) > 0 { + b, err := json.Marshal(matches) + if err != nil { + // panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly. + panic(fmt.Errorf("could not marshal http match: %w", err)) + } + + pathLoc := http.Location{ + Path: rule.Path, + HTTPMatchVar: string(b), + } + + locs = append(locs, pathLoc) + } + } + + s.Locations = locs + return s +} + +func createDefaultSSLServer() http.Server { + return http.Server{IsDefaultSSL: true} +} + +func createDefaultHTTPServer() http.Server { + return http.Server{IsDefaultHTTP: true} +} + +func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int) *http.Return { + if filter == nil { + return nil + } + + hostname := "$host" + if filter.Hostname != nil { + hostname = string(*filter.Hostname) + } + + // FIXME(pleshakov): Unknown values here must result in the implementation setting the Attached Condition for + // the Route to `status: False`, with a Reason of `UnsupportedValue`. In that case, all routes of the Route will be + // ignored. NGINX will return 500. This should be implemented in the validation layer. + code := http.StatusFound + if filter.StatusCode != nil { + code = http.StatusCode(*filter.StatusCode) + } + + port := listenerPort + if filter.Port != nil { + port = int(*filter.Port) + } + + // FIXME(pleshakov): Same as the FIXME about StatusCode above. + scheme := "$scheme" + if filter.Scheme != nil { + scheme = *filter.Scheme + } + + return &http.Return{ + Code: code, + URL: fmt.Sprintf("%s://%s:%d$request_uri", scheme, hostname, port), + } +} + +// httpMatch is an internal representation of an HTTPRouteMatch. +// This struct is marshaled into a string and stored as a variable in the nginx location block for the route's path. +// The NJS httpmatches module will lookup this variable on the request object and compare the request against the Method, Headers, and QueryParams contained in httpMatch. +// If the request satisfies the httpMatch, the request will be internally redirected to the location RedirectPath by NGINX. +type httpMatch struct { + // Any represents a match with no match conditions. + Any bool `json:"any,omitempty"` + // Method is the HTTPMethod of the HTTPRouteMatch. + Method v1beta1.HTTPMethod `json:"method,omitempty"` + // Headers is a list of HTTPHeaders name value pairs with the format "{name}:{value}". + Headers []string `json:"headers,omitempty"` + // QueryParams is a list of HTTPQueryParams name value pairs with the format "{name}={value}". + QueryParams []string `json:"params,omitempty"` + // RedirectPath is the path to redirect the request to if the request satisfies the match conditions. + RedirectPath string `json:"redirectPath,omitempty"` +} + +func createHTTPMatch(match v1beta1.HTTPRouteMatch, redirectPath string) httpMatch { + hm := httpMatch{ + RedirectPath: redirectPath, + } + + if isPathOnlyMatch(match) { + hm.Any = true + return hm + } + + if match.Method != nil { + hm.Method = *match.Method + } + + if match.Headers != nil { + headers := make([]string, 0, len(match.Headers)) + headerNames := make(map[string]struct{}) + + // FIXME(kate-osborn): For now we only support type "Exact". + for _, h := range match.Headers { + if *h.Type == v1beta1.HeaderMatchExact { + // duplicate header names are not permitted by the spec + // only configure the first entry for every header name (case-insensitive) + lowerName := strings.ToLower(string(h.Name)) + if _, ok := headerNames[lowerName]; !ok { + headers = append(headers, createHeaderKeyValString(h)) + headerNames[lowerName] = struct{}{} + } + } + } + hm.Headers = headers + } + + if match.QueryParams != nil { + params := make([]string, 0, len(match.QueryParams)) + + // FIXME(kate-osborn): For now we only support type "Exact". + for _, p := range match.QueryParams { + if *p.Type == v1beta1.QueryParamMatchExact { + params = append(params, createQueryParamKeyValString(p)) + } + } + hm.QueryParams = params + } + + return hm +} + +// The name and values are delimited by "=". A name and value can always be recovered using strings.SplitN(arg,"=", 2). +// Query Parameters are case-sensitive so case is preserved. +func createQueryParamKeyValString(p v1beta1.HTTPQueryParamMatch) string { + return p.Name + "=" + p.Value +} + +// The name and values are delimited by ":". A name and value can always be recovered using strings.Split(arg, ":"). +// Header names are case-insensitive while header values are case-sensitive (e.g. foo:bar == FOO:bar, but foo:bar != foo:BAR). +// We preserve the case of the name here because NGINX allows us to lookup the header names in a case-insensitive manner. +func createHeaderKeyValString(h v1beta1.HTTPHeaderMatch) string { + return string(h.Name) + ":" + h.Value +} + +func isPathOnlyMatch(match v1beta1.HTTPRouteMatch) bool { + return match.Method == nil && match.Headers == nil && match.QueryParams == nil +} + +func createProxyPass(address string) string { + return "http://" + address +} + +func createProxyPassForVar(variable string) string { + return "http://$" + convertStringToSafeVariableName(variable) +} + +func createMatchLocation(path string) http.Location { + return http.Location{ + Path: path, + Internal: true, + } +} + +func createPathForMatch(path string, routeIdx int) string { + return fmt.Sprintf("%s_route%d", path, routeIdx) +} diff --git a/internal/nginx/config/servers_template.go b/internal/nginx/config/servers_template.go index 3cb0d6680b..c96da61371 100644 --- a/internal/nginx/config/servers_template.go +++ b/internal/nginx/config/servers_template.go @@ -1,7 +1,7 @@ package config -var httpServersTemplate = ` -{{ range $s := .Servers }} +var serversTemplateText = ` +{{ range $s := . }} {{ if $s.IsDefaultSSL }} server { listen 443 ssl default_server; @@ -53,4 +53,17 @@ server { } {{ end }} {{ end }} +server { + listen unix:/var/lib/nginx/nginx-502-server.sock; + access_log off; + + return 502; +} + +server { + listen unix:/var/lib/nginx/nginx-500-server.sock; + access_log off; + + return 500; +} ` diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go new file mode 100644 index 0000000000..2b6871bd85 --- /dev/null +++ b/internal/nginx/config/servers_test.go @@ -0,0 +1,884 @@ +package config + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" +) + +func TestExecuteServers(t *testing.T) { + conf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "example.com", + }, + { + Hostname: "cafe.example.com", + }, + }, + SSLServers: []state.VirtualServer{ + { + Hostname: "example.com", + SSL: &state.SSL{ + CertificatePath: "cert-path", + }, + }, + { + Hostname: "cafe.example.com", + SSL: &state.SSL{ + CertificatePath: "cert-path", + }, + }, + }, + } + + expSubStrings := map[string]int{ + "listen 80 default_server;": 1, + "listen 443 ssl;": 2, + "listen 443 ssl default_server;": 1, + "server_name example.com;": 2, + "server_name cafe.example.com;": 2, + "ssl_certificate cert-path;": 2, + "ssl_certificate_key cert-path;": 2, + } + + servers := string(executeServers(conf)) + for expSubStr, expCount := range expSubStrings { + if expCount != strings.Count(servers, expSubStr) { + t.Errorf( + "executeServers() did not generate servers with substring %q %d times. Servers: %v", + expSubStr, + expCount, + servers, + ) + } + } +} + +func TestExecuteForDefaultServers(t *testing.T) { + testcases := []struct { + conf state.Configuration + httpDefault bool + sslDefault bool + msg string + }{ + { + conf: state.Configuration{}, + httpDefault: false, + sslDefault: false, + msg: "no servers", + }, + { + conf: state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "example.com", + }, + }, + }, + httpDefault: true, + sslDefault: false, + msg: "only HTTP servers", + }, + { + conf: state.Configuration{ + SSLServers: []state.VirtualServer{ + { + Hostname: "example.com", + }, + }, + }, + httpDefault: false, + sslDefault: true, + msg: "only HTTPS servers", + }, + { + conf: state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "example.com", + }, + }, + SSLServers: []state.VirtualServer{ + { + Hostname: "example.com", + }, + }, + }, + httpDefault: true, + sslDefault: true, + msg: "both HTTP and HTTPS servers", + }, + } + + for _, tc := range testcases { + cfg := string(executeServers(tc.conf)) + + defaultSSLExists := strings.Contains(cfg, "listen 443 ssl default_server") + defaultHTTPExists := strings.Contains(cfg, "listen 80 default_server") + + if tc.sslDefault && !defaultSSLExists { + t.Errorf( + "executeServers() did not generate a config with a default TLS termination server for test: %q", + tc.msg, + ) + } + + if !tc.sslDefault && defaultSSLExists { + t.Errorf("executeServers() generated a config with a default TLS termination server for test: %q", tc.msg) + } + + if tc.httpDefault && !defaultHTTPExists { + t.Errorf("executeServers() did not generate a config with a default http server for test: %q", tc.msg) + } + + if !tc.httpDefault && defaultHTTPExists { + t.Errorf("executeServers() generated a config with a default http server for test: %q", tc.msg) + } + + if len(cfg) == 0 { + t.Errorf("executeServers() generated empty config for test: %q", tc.msg) + } + } +} + +func TestCreateServers(t *testing.T) { + const ( + certPath = "/etc/nginx/secrets/cert" + ) + + hr := &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "route1", + }, + Spec: v1beta1.HTTPRouteSpec{ + Hostnames: []v1beta1.Hostname{ + "cafe.example.com", + }, + Rules: []v1beta1.HTTPRouteRule{ + { + // matches with path and methods + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/"), + }, + Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPost), + }, + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/"), + }, + Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPatch), + }, + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/"), // should generate an "any" httpmatch since other matches exists for / + }, + }, + }, + }, + { + // A match with all possible fields set + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/test"), + }, + Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodGet), + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "Version", + Value: "V1", + }, + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "test", + Value: "foo", + }, + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "my-header", + Value: "my-value", + }, + }, + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), + Name: "GrEat", // query names and values should not be normalized to lowercase + Value: "EXAMPLE", + }, + { + Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), + Name: "test", + Value: "foo=bar", + }, + }, + }, + }, + }, + { + // A match with just path + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path-only"), + }, + }, + }, + }, + { + // A match with a redirect with implicit port + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/redirect-implicit-port"), + }, + }, + }, + // redirect is set in the corresponding state.MatchRule + }, + { + // A match with a redirect with explicit port + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/redirect-explicit-port"), + }, + }, + }, + // redirect is set in the corresponding state.MatchRule + }, + }, + }, + } + + hrNsName := types.NamespacedName{Namespace: hr.Namespace, Name: hr.Name} + + fooGroup := state.BackendGroup{ + Source: hrNsName, + RuleIdx: 0, + Backends: []state.BackendRef{ + { + Name: "test_foo_80", + Valid: true, + Weight: 1, + }, + }, + } + + // barGroup has two backends, which should generate a proxy pass with a variable. + barGroup := state.BackendGroup{ + Source: hrNsName, + RuleIdx: 1, + Backends: []state.BackendRef{ + { + Name: "test_bar_80", + Valid: true, + Weight: 50, + }, + { + Name: "test_bar2_80", + Valid: true, + Weight: 50, + }, + }, + } + + // baz group has an invalid backend, which should generate a proxy pass to the invalid ref backend. + bazGroup := state.BackendGroup{ + Source: hrNsName, + RuleIdx: 2, + Backends: []state.BackendRef{ + { + Name: "test_baz_80", + Valid: false, + Weight: 1, + }, + }, + } + + filterGroup1 := state.BackendGroup{Source: hrNsName, RuleIdx: 3} + + filterGroup2 := state.BackendGroup{Source: hrNsName, RuleIdx: 4} + + cafePathRules := []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: fooGroup, + Source: hr, + }, + { + MatchIdx: 1, + RuleIdx: 0, + BackendGroup: fooGroup, + Source: hr, + }, + { + MatchIdx: 2, + RuleIdx: 0, + BackendGroup: fooGroup, + Source: hr, + }, + }, + }, + { + Path: "/test", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + BackendGroup: barGroup, + Source: hr, + }, + }, + }, + { + Path: "/path-only", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 2, + BackendGroup: bazGroup, + Source: hr, + }, + }, + }, + { + Path: "/redirect-implicit-port", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 3, + Source: hr, + Filters: state.Filters{ + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("foo.example.com")), + }, + }, + BackendGroup: filterGroup1, + }, + }, + }, + { + Path: "/redirect-explicit-port", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 4, + Source: hr, + Filters: state.Filters{ + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("bar.example.com")), + Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(8080)), + }, + }, + BackendGroup: filterGroup2, + }, + }, + }, + } + + httpServers := []state.VirtualServer{ + { + Hostname: "cafe.example.com", + PathRules: cafePathRules, + }, + } + + sslServers := []state.VirtualServer{ + { + Hostname: "cafe.example.com", + SSL: &state.SSL{CertificatePath: certPath}, + PathRules: cafePathRules, + }, + } + + expectedMatchString := func(m []httpMatch) string { + b, err := json.Marshal(m) + if err != nil { + t.Errorf("error marshaling test match: %v", err) + } + return string(b) + } + + slashMatches := []httpMatch{ + {Method: v1beta1.HTTPMethodPost, RedirectPath: "/_route0"}, + {Method: v1beta1.HTTPMethodPatch, RedirectPath: "/_route1"}, + {Any: true, RedirectPath: "/_route2"}, + } + testMatches := []httpMatch{ + { + Method: v1beta1.HTTPMethodGet, + Headers: []string{"Version:V1", "test:foo", "my-header:my-value"}, + QueryParams: []string{"GrEat=EXAMPLE", "test=foo=bar"}, + RedirectPath: "/test_route0", + }, + } + + getExpectedLocations := func(isHTTPS bool) []http.Location { + port := 80 + if isHTTPS { + port = 443 + } + + return []http.Location{ + { + Path: "/_route0", + Internal: true, + ProxyPass: "http://test_foo_80", + }, + { + Path: "/_route1", + Internal: true, + ProxyPass: "http://test_foo_80", + }, + { + Path: "/_route2", + Internal: true, + ProxyPass: "http://test_foo_80", + }, + { + Path: "/", + HTTPMatchVar: expectedMatchString(slashMatches), + }, + { + Path: "/test_route0", + Internal: true, + ProxyPass: "http://$test__route1_rule1", + }, + { + Path: "/test", + HTTPMatchVar: expectedMatchString(testMatches), + }, + { + Path: "/path-only", + ProxyPass: "http://invalid-backend-ref", + }, + { + Path: "/redirect-implicit-port", + Return: &http.Return{ + Code: 302, + URL: fmt.Sprintf("$scheme://foo.example.com:%d$request_uri", port), + }, + }, + { + Path: "/redirect-explicit-port", + Return: &http.Return{ + Code: 302, + URL: "$scheme://bar.example.com:8080$request_uri", + }, + }, + } + } + + expectedServers := []http.Server{ + { + IsDefaultHTTP: true, + }, + { + IsDefaultSSL: true, + }, + { + ServerName: "cafe.example.com", + Locations: getExpectedLocations(false), + }, + { + ServerName: "cafe.example.com", + SSL: &http.SSL{Certificate: certPath, CertificateKey: certPath}, + Locations: getExpectedLocations(true), + }, + } + + result := createServers(httpServers, sslServers) + + if diff := cmp.Diff(expectedServers, result); diff != "" { + t.Errorf("createServers() mismatch (-want +got):\n%s", diff) + } +} + +func TestCreateReturnValForRedirectFilter(t *testing.T) { + const listenerPort = 123 + + tests := []struct { + filter *v1beta1.HTTPRequestRedirectFilter + expected *http.Return + msg string + }{ + { + filter: nil, + expected: nil, + msg: "filter is nil", + }, + { + filter: &v1beta1.HTTPRequestRedirectFilter{}, + expected: &http.Return{ + Code: http.StatusFound, + URL: "$scheme://$host:123$request_uri", + }, + msg: "all fields are empty", + }, + { + filter: &v1beta1.HTTPRequestRedirectFilter{ + Scheme: helpers.GetStringPointer("https"), + Hostname: (*v1beta1.PreciseHostname)(helpers.GetStringPointer("foo.example.com")), + Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(2022)), + StatusCode: helpers.GetIntPointer(101), + }, + expected: &http.Return{ + Code: 101, + URL: "https://foo.example.com:2022$request_uri", + }, + msg: "all fields are set", + }, + } + + for _, test := range tests { + result := createReturnValForRedirectFilter(test.filter, listenerPort) + if diff := cmp.Diff(test.expected, result); diff != "" { + t.Errorf("createReturnValForRedirectFilter() mismatch %q (-want +got):\n%s", test.msg, diff) + } + } +} + +func TestCreateHTTPMatch(t *testing.T) { + testPath := "/internal_loc" + + testPathMatch := v1beta1.HTTPPathMatch{Value: helpers.GetStringPointer("/")} + testMethodMatch := helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPut) + testHeaderMatches := []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "header-1", + Value: "val-1", + }, + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "header-2", + Value: "val-2", + }, + { + // regex type is not supported. This should not be added to the httpMatch headers. + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchRegularExpression), + Name: "ignore-this-header", + Value: "val", + }, + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "header-3", + Value: "val-3", + }, + } + + testDuplicateHeaders := make([]v1beta1.HTTPHeaderMatch, 0, 5) + duplicateHeaderMatch := v1beta1.HTTPHeaderMatch{ + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "HEADER-2", // header names are case-insensitive + Value: "val-2", + } + testDuplicateHeaders = append(testDuplicateHeaders, testHeaderMatches...) + testDuplicateHeaders = append(testDuplicateHeaders, duplicateHeaderMatch) + + testQueryParamMatches := []v1beta1.HTTPQueryParamMatch{ + { + Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), + Name: "arg1", + Value: "val1", + }, + { + Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), + Name: "arg2", + Value: "val2=another-val", + }, + { + // regex type is not supported. This should not be added to the httpMatch args + Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchRegularExpression), + Name: "ignore-this-arg", + Value: "val", + }, + { + Type: helpers.GetQueryParamMatchTypePointer(v1beta1.QueryParamMatchExact), + Name: "arg3", + Value: "==val3", + }, + } + + expectedHeaders := []string{"header-1:val-1", "header-2:val-2", "header-3:val-3"} + expectedArgs := []string{"arg1=val1", "arg2=val2=another-val", "arg3===val3"} + + tests := []struct { + match v1beta1.HTTPRouteMatch + expected httpMatch + msg string + }{ + { + match: v1beta1.HTTPRouteMatch{ + Path: &testPathMatch, + }, + expected: httpMatch{ + Any: true, + RedirectPath: testPath, + }, + msg: "path only match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Path: &testPathMatch, // A path match with a method should not set the Any field to true + Method: testMethodMatch, + }, + expected: httpMatch{ + Method: "PUT", + RedirectPath: testPath, + }, + msg: "method only match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Headers: testHeaderMatches, + }, + expected: httpMatch{ + RedirectPath: testPath, + Headers: expectedHeaders, + }, + msg: "headers only match", + }, + { + match: v1beta1.HTTPRouteMatch{ + QueryParams: testQueryParamMatches, + }, + expected: httpMatch{ + QueryParams: expectedArgs, + RedirectPath: testPath, + }, + msg: "query params only match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Method: testMethodMatch, + QueryParams: testQueryParamMatches, + }, + expected: httpMatch{ + Method: "PUT", + QueryParams: expectedArgs, + RedirectPath: testPath, + }, + msg: "method and query params match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Method: testMethodMatch, + Headers: testHeaderMatches, + }, + expected: httpMatch{ + Method: "PUT", + Headers: expectedHeaders, + RedirectPath: testPath, + }, + msg: "method and headers match", + }, + { + match: v1beta1.HTTPRouteMatch{ + QueryParams: testQueryParamMatches, + Headers: testHeaderMatches, + }, + expected: httpMatch{ + QueryParams: expectedArgs, + Headers: expectedHeaders, + RedirectPath: testPath, + }, + msg: "query params and headers match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Headers: testHeaderMatches, + QueryParams: testQueryParamMatches, + Method: testMethodMatch, + }, + expected: httpMatch{ + Method: "PUT", + Headers: expectedHeaders, + QueryParams: expectedArgs, + RedirectPath: testPath, + }, + msg: "method, headers, and query params match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Headers: testDuplicateHeaders, + }, + expected: httpMatch{ + Headers: expectedHeaders, + RedirectPath: testPath, + }, + msg: "duplicate header names", + }, + } + for _, tc := range tests { + result := createHTTPMatch(tc.match, testPath) + if diff := helpers.Diff(result, tc.expected); diff != "" { + t.Errorf("createHTTPMatch() returned incorrect httpMatch for test case: %q, diff: %+v", tc.msg, diff) + } + } +} + +func TestCreateQueryParamKeyValString(t *testing.T) { + expected := "key=value" + + result := createQueryParamKeyValString( + v1beta1.HTTPQueryParamMatch{ + Name: "key", + Value: "value", + }, + ) + if result != expected { + t.Errorf("createQueryParamKeyValString() returned %q but expected %q", result, expected) + } + + expected = "KeY=vaLUe==" + + result = createQueryParamKeyValString( + v1beta1.HTTPQueryParamMatch{ + Name: "KeY", + Value: "vaLUe==", + }, + ) + if result != expected { + t.Errorf("createQueryParamKeyValString() returned %q but expected %q", result, expected) + } +} + +func TestCreateHeaderKeyValString(t *testing.T) { + expected := "kEy:vALUe" + + result := createHeaderKeyValString( + v1beta1.HTTPHeaderMatch{ + Name: "kEy", + Value: "vALUe", + }, + ) + + if result != expected { + t.Errorf("createHeaderKeyValString() returned %q but expected %q", result, expected) + } +} + +func TestIsPathOnlyMatch(t *testing.T) { + tests := []struct { + match v1beta1.HTTPRouteMatch + expected bool + msg string + }{ + { + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path"), + }, + }, + expected: true, + msg: "path only match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path"), + }, + Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodGet), + }, + expected: false, + msg: "method defined in match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path"), + }, + Headers: []v1beta1.HTTPHeaderMatch{ + { + Name: "header", + Value: "val", + }, + }, + }, + expected: false, + msg: "headers defined in match", + }, + { + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/path"), + }, + QueryParams: []v1beta1.HTTPQueryParamMatch{ + { + Name: "arg", + Value: "val", + }, + }, + }, + expected: false, + msg: "query params defined in match", + }, + } + + for _, tc := range tests { + result := isPathOnlyMatch(tc.match) + + if result != tc.expected { + t.Errorf("isPathOnlyMatch() returned %t but expected %t for test case %q", result, tc.expected, tc.msg) + } + } +} + +func TestCreateProxyPass(t *testing.T) { + expected := "http://10.0.0.1:80" + + result := createProxyPass("10.0.0.1:80") + if result != expected { + t.Errorf("createProxyPass() returned %s but expected %s", result, expected) + } +} + +func TestCreateProxyPassForVar(t *testing.T) { + expected := "http://$my_variable" + + result := createProxyPassForVar("my-variable") + if result != expected { + t.Errorf("createProxyPassForVar() returned %s but expected %s", result, expected) + } +} + +func TestCreateMatchLocation(t *testing.T) { + expected := http.Location{ + Path: "/path", + Internal: true, + } + + result := createMatchLocation("/path") + if result != expected { + t.Errorf("createMatchLocation() returned %v but expected %v", result, expected) + } +} + +func TestCreatePathForMatch(t *testing.T) { + expected := "/path_route1" + + result := createPathForMatch("/path", 1) + if result != expected { + t.Errorf("createPathForMatch() returned %q but expected %q", result, expected) + } +} diff --git a/internal/nginx/config/split_clients.go b/internal/nginx/config/split_clients.go new file mode 100644 index 0000000000..93a826cd28 --- /dev/null +++ b/internal/nginx/config/split_clients.go @@ -0,0 +1,141 @@ +package config + +import ( + "fmt" + "math" + gotemplate "text/template" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" +) + +var splitClientsTemplate = gotemplate.Must(gotemplate.New("split_clients").Parse(splitClientsTemplateText)) + +func executeSplitClients(conf state.Configuration) []byte { + splitClients := createSplitClients(conf.BackendGroups) + + return execute(splitClientsTemplate, splitClients) +} + +func createSplitClients(backendGroups []state.BackendGroup) []http.SplitClient { + numSplits := 0 + for _, group := range backendGroups { + if backendGroupNeedsSplit(group) { + numSplits++ + } + } + + if numSplits == 0 { + return nil + } + + splitClients := make([]http.SplitClient, 0, numSplits) + + for _, group := range backendGroups { + + distributions := createSplitClientDistributions(group) + if distributions == nil { + continue + } + + splitClients = append(splitClients, http.SplitClient{ + VariableName: convertStringToSafeVariableName(group.GroupName()), + Distributions: distributions, + }) + + } + + return splitClients +} + +func createSplitClientDistributions(group state.BackendGroup) []http.SplitClientDistribution { + if !backendGroupNeedsSplit(group) { + return nil + } + + backends := group.Backends + + totalWeight := int32(0) + for _, b := range backends { + totalWeight += b.Weight + } + + if totalWeight == 0 { + return []http.SplitClientDistribution{ + { + Percent: "100", + Value: invalidBackendRef, + }, + } + } + + distributions := make([]http.SplitClientDistribution, 0, len(backends)) + + // The percentage of all backends cannot exceed 100. + availablePercentage := float64(100) + + // Iterate over all backends except the last one. + // The last backend will get the remaining percentage. + for i := 0; i < len(backends)-1; i++ { + b := backends[i] + + percentage := percentOf(b.Weight, totalWeight) + availablePercentage -= percentage + + distributions = append(distributions, http.SplitClientDistribution{ + Percent: fmt.Sprintf("%.2f", percentage), + Value: getSplitClientValue(b), + }) + } + + // The last backend gets the remaining percentage. + // This is done to guarantee that the sum of all percentages is 100. + lastBackend := backends[len(backends)-1] + + distributions = append(distributions, http.SplitClientDistribution{ + Percent: fmt.Sprintf("%.2f", availablePercentage), + Value: getSplitClientValue(lastBackend), + }) + + return distributions +} + +func getSplitClientValue(b state.BackendRef) string { + if b.Valid { + return b.Name + } + return invalidBackendRef +} + +// percentOf returns the percentage of a weight out of a totalWeight. +// The percentage is rounded to 2 decimal places using the Floor method. +// Floor is used here in order to guarantee that the sum of all percentages does not exceed 100. +// Ex. percentOf(2, 3) = 66.66 +// Ex. percentOf(800, 2000) = 40.00 +func percentOf(weight, totalWeight int32) float64 { + p := (float64(weight) * 100) / float64(totalWeight) + return math.Floor(p*100) / 100 +} + +func backendGroupNeedsSplit(group state.BackendGroup) bool { + return len(group.Backends) > 1 +} + +// backendGroupName returns the name of the backend group. +// If the group needs to be split, the name returned is the group name. +// If the group doesn't need to be split, the name returned is the name of the backend if it is valid. +// If the name cannot be determined, it returns the name of the invalid backend upstream. +func backendGroupName(group state.BackendGroup) string { + switch len(group.Backends) { + case 0: + return invalidBackendRef + case 1: + b := group.Backends[0] + if b.Weight <= 0 || !b.Valid { + return invalidBackendRef + } + return b.Name + default: + return group.GroupName() + } +} diff --git a/internal/nginx/config/split_clients_template.go b/internal/nginx/config/split_clients_template.go new file mode 100644 index 0000000000..2562a6ec05 --- /dev/null +++ b/internal/nginx/config/split_clients_template.go @@ -0,0 +1,14 @@ +package config + +var splitClientsTemplateText = ` +{{ range $sc := . }} +split_clients $request_id ${{ $sc.VariableName }} { + {{ range $d := $sc.Distributions }} + {{ if eq $d.Percent "0.00" }} + # {{ $d.Percent }}% {{ $d.Value }}; + {{ else }} + {{ $d.Percent }}% {{ $d.Value }}; + {{ end }} + {{ end }} +} +{{ end }}` diff --git a/internal/nginx/config/split_clients_test.go b/internal/nginx/config/split_clients_test.go new file mode 100644 index 0000000000..10afa9fdee --- /dev/null +++ b/internal/nginx/config/split_clients_test.go @@ -0,0 +1,692 @@ +package config + +import ( + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "k8s.io/apimachinery/pkg/types" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" +) + +func TestExecuteSplitClients(t *testing.T) { + bg1 := state.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 0, + Backends: []state.BackendRef{ + {Name: "test1", Valid: true, Weight: 1}, + {Name: "test2", Valid: true, Weight: 1}, + }, + } + + bg2 := state.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "no-split"}, + RuleIdx: 1, + Backends: []state.BackendRef{ + {Name: "no-split", Valid: true, Weight: 1}, + }, + } + + bg3 := state.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 1, + Backends: []state.BackendRef{ + {Name: "test3", Valid: true, Weight: 1}, + {Name: "test4", Valid: true, Weight: 1}, + }, + } + + tests := []struct { + msg string + backendGroups []state.BackendGroup + expStrings []string + notExpStrings []string + }{ + { + msg: "non-zero weights", + backendGroups: []state.BackendGroup{ + bg1, + bg2, + bg3, + }, + expStrings: []string{ + "split_clients $request_id $test__hr_rule0", + "split_clients $request_id $test__hr_rule1", + "50.00% test1;", + "50.00% test2;", + "50.00% test3;", + "50.00% test4;", + }, + notExpStrings: []string{"no-split", "#"}, + }, + { + msg: "zero weight", + backendGroups: []state.BackendGroup{ + { + Source: types.NamespacedName{Namespace: "test", Name: "zero-percent"}, + RuleIdx: 0, + Backends: []state.BackendRef{ + {Name: "non-zero", Valid: true, Weight: 1}, + {Name: "zero", Valid: true, Weight: 0}, + }, + }, + }, + expStrings: []string{ + "split_clients $request_id $test__zero_percent_rule0", + "100.00% non-zero;", + "# 0.00% zero;", + }, + notExpStrings: nil, + }, + { + msg: "no split clients", + backendGroups: []state.BackendGroup{ + { + Source: types.NamespacedName{Namespace: "test", Name: "single-backend-route"}, + RuleIdx: 0, + Backends: []state.BackendRef{ + {Name: "single-backend", Valid: true, Weight: 1}, + }, + }, + }, + expStrings: []string{}, + notExpStrings: []string{"split_clients"}, + }, + } + + for _, test := range tests { + sc := string(executeSplitClients(state.Configuration{BackendGroups: test.backendGroups})) + + for _, expSubString := range test.expStrings { + if !strings.Contains(sc, expSubString) { + t.Errorf( + "executeSplitClients() did not generate split clients with substring %q for test %q. Got: %v", + expSubString, + test.msg, + sc, + ) + } + } + + for _, notExpString := range test.notExpStrings { + if strings.Contains(sc, notExpString) { + t.Errorf( + "executeSplitClients() generated split clients with unexpected substring %q for test %q. Got: %v", + notExpString, + test.msg, + sc, + ) + } + } + } +} + +func TestCreateSplitClients(t *testing.T) { + hrNoSplit := types.NamespacedName{Namespace: "test", Name: "hr-no-split"} + hrOneSplit := types.NamespacedName{Namespace: "test", Name: "hr-one-split"} + hrTwoSplits := types.NamespacedName{Namespace: "test", Name: "hr-two-splits"} + + createBackendGroup := func( + sourceNsName types.NamespacedName, + ruleIdx int, + backends ...state.BackendRef, + ) state.BackendGroup { + return state.BackendGroup{ + Source: sourceNsName, + RuleIdx: ruleIdx, + Backends: backends, + } + } + // the following backends do not need splits + noBackends := createBackendGroup(hrNoSplit, 0) + + oneBackend := createBackendGroup( + hrNoSplit, + 0, + state.BackendRef{Name: "one-backend", Valid: true, Weight: 1}, + ) + + invalidBackend := createBackendGroup( + hrNoSplit, + 0, + state.BackendRef{Name: "invalid-backend", Valid: false, Weight: 1}, + ) + + // the following backends need splits + oneSplit := createBackendGroup( + hrOneSplit, + 0, + state.BackendRef{Name: "one-split-1", Valid: true, Weight: 50}, + state.BackendRef{Name: "one-split-2", Valid: true, Weight: 50}, + ) + + twoSplitGroup0 := createBackendGroup( + hrTwoSplits, + 0, + state.BackendRef{Name: "two-split-1", Valid: true, Weight: 50}, + state.BackendRef{Name: "two-split-2", Valid: true, Weight: 50}, + ) + + twoSplitGroup1 := createBackendGroup( + hrTwoSplits, + 1, + state.BackendRef{Name: "two-split-3", Valid: true, Weight: 50}, + state.BackendRef{Name: "two-split-4", Valid: true, Weight: 50}, + state.BackendRef{Name: "two-split-5", Valid: true, Weight: 50}, + ) + + tests := []struct { + msg string + backendGroups []state.BackendGroup + expSplitClients []http.SplitClient + }{ + { + msg: "normal case", + backendGroups: []state.BackendGroup{ + noBackends, + oneBackend, + invalidBackend, + oneSplit, + twoSplitGroup0, + twoSplitGroup1, + }, + expSplitClients: []http.SplitClient{ + { + VariableName: "test__hr_one_split_rule0", + Distributions: []http.SplitClientDistribution{ + { + Percent: "50.00", + Value: "one-split-1", + }, + { + Percent: "50.00", + Value: "one-split-2", + }, + }, + }, + { + VariableName: "test__hr_two_splits_rule0", + Distributions: []http.SplitClientDistribution{ + { + Percent: "50.00", + Value: "two-split-1", + }, + { + Percent: "50.00", + Value: "two-split-2", + }, + }, + }, + { + VariableName: "test__hr_two_splits_rule1", + Distributions: []http.SplitClientDistribution{ + { + Percent: "33.33", + Value: "two-split-3", + }, + { + Percent: "33.33", + Value: "two-split-4", + }, + { + Percent: "33.34", + Value: "two-split-5", + }, + }, + }, + }, + }, + { + msg: "no split clients are needed", + backendGroups: []state.BackendGroup{ + noBackends, + oneBackend, + }, + expSplitClients: nil, + }, + } + + for _, test := range tests { + result := createSplitClients(test.backendGroups) + if diff := cmp.Diff(test.expSplitClients, result); diff != "" { + t.Errorf("createSplitClients() mismatch for %q (-want +got):\n%s", test.msg, diff) + } + } +} + +func TestCreateSplitClientDistributions(t *testing.T) { + tests := []struct { + msg string + backends []state.BackendRef + expDistributions []http.SplitClientDistribution + }{ + { + msg: "no backends", + backends: nil, + expDistributions: nil, + }, + { + msg: "one backend", + backends: []state.BackendRef{ + { + Name: "one", + Valid: true, + Weight: 1, + }, + }, + expDistributions: nil, + }, + { + msg: "total weight 0", + backends: []state.BackendRef{ + { + Name: "one", + Valid: true, + Weight: 0, + }, + { + Name: "two", + Valid: true, + Weight: 0, + }, + }, + expDistributions: []http.SplitClientDistribution{ + { + Percent: "100", + Value: invalidBackendRef, + }, + }, + }, + { + msg: "two backends; equal weights that sum to 100", + backends: []state.BackendRef{ + { + Name: "one", + Valid: true, + Weight: 1, + }, + { + Name: "two", + Valid: true, + Weight: 1, + }, + }, + expDistributions: []http.SplitClientDistribution{ + { + Percent: "50.00", + Value: "one", + }, + { + Percent: "50.00", + Value: "two", + }, + }, + }, + { + msg: "three backends; whole percentages that sum to 100", + backends: []state.BackendRef{ + { + Name: "one", + Valid: true, + Weight: 20, + }, + { + Name: "two", + Valid: true, + Weight: 30, + }, + { + Name: "three", + Valid: true, + Weight: 50, + }, + }, + expDistributions: []http.SplitClientDistribution{ + { + Percent: "20.00", + Value: "one", + }, + { + Percent: "30.00", + Value: "two", + }, + { + Percent: "50.00", + Value: "three", + }, + }, + }, + { + msg: "three backends; whole percentages that sum to less than 100", + backends: []state.BackendRef{ + { + Name: "one", + Valid: true, + Weight: 3, + }, + { + Name: "two", + Valid: true, + Weight: 3, + }, + { + Name: "three", + Valid: true, + Weight: 3, + }, + }, + expDistributions: []http.SplitClientDistribution{ + { + Percent: "33.33", + Value: "one", + }, + { + Percent: "33.33", + Value: "two", + }, + { + Percent: "33.34", // the last backend gets the remainder. + Value: "three", + }, + }, + }, + } + + for _, test := range tests { + result := createSplitClientDistributions(state.BackendGroup{Backends: test.backends}) + if diff := cmp.Diff(test.expDistributions, result); diff != "" { + t.Errorf("createSplitClientDistributions() mismatch for %q (-want +got):\n%s", test.msg, diff) + } + } +} + +func TestGetSplitClientValue(t *testing.T) { + tests := []struct { + msg string + backend state.BackendRef + expValue string + }{ + { + msg: "valid backend", + backend: state.BackendRef{ + Name: "valid", + Valid: true, + }, + expValue: "valid", + }, + { + msg: "invalid backend", + backend: state.BackendRef{ + Name: "invalid", + Valid: false, + }, + expValue: invalidBackendRef, + }, + } + + for _, test := range tests { + result := getSplitClientValue(test.backend) + if result != test.expValue { + t.Errorf( + "getSplitClientValue() mismatch for %q; expected %s, got %s", + test.msg, test.expValue, result, + ) + } + } +} + +func TestPercentOf(t *testing.T) { + tests := []struct { + msg string + weight int32 + totalWeight int32 + expPercent float64 + }{ + { + msg: "50/100", + weight: 50, + totalWeight: 100, + expPercent: 50, + }, + { + msg: "2000/4000", + weight: 2000, + totalWeight: 4000, + expPercent: 50, + }, + { + msg: "100/100", + weight: 100, + totalWeight: 100, + expPercent: 100, + }, + { + msg: "5/5", + weight: 5, + totalWeight: 5, + expPercent: 100, + }, + { + msg: "0/8000", + weight: 0, + totalWeight: 8000, + expPercent: 0, + }, + { + msg: "2/3", + weight: 2, + totalWeight: 3, + expPercent: 66.66, + }, + { + msg: "4/15", + weight: 4, + totalWeight: 15, + expPercent: 26.66, + }, + { + msg: "800/2000", + weight: 800, + totalWeight: 2000, + expPercent: 40, + }, + { + msg: "300/2400", + weight: 300, + totalWeight: 2400, + expPercent: 12.5, + }, + } + + for _, test := range tests { + percent := percentOf(test.weight, test.totalWeight) + if percent != test.expPercent { + t.Errorf( + "percentOf() mismatch for test %q; expected %f, got %f", + test.msg, test.expPercent, percent, + ) + } + } +} + +func TestBackendGroupNeedsSplit(t *testing.T) { + tests := []struct { + msg string + backends []state.BackendRef + expSplit bool + }{ + { + msg: "empty backends", + backends: []state.BackendRef{}, + expSplit: false, + }, + { + msg: "nil backends", + backends: nil, + expSplit: false, + }, + { + msg: "one valid backend", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: true, + Weight: 1, + }, + }, + expSplit: false, + }, + { + msg: "one invalid backend", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: false, + Weight: 1, + }, + }, + expSplit: false, + }, + { + msg: "multiple valid backends", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: true, + Weight: 1, + }, + { + Name: "backend2", + Valid: true, + Weight: 1, + }, + }, + expSplit: true, + }, + { + msg: "multiple backends - one invalid", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: true, + Weight: 1, + }, + { + Name: "backend2", + Valid: false, + Weight: 1, + }, + }, + expSplit: true, + }, + } + + for _, test := range tests { + bg := state.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + Backends: test.backends, + } + result := backendGroupNeedsSplit(bg) + if result != test.expSplit { + t.Errorf("backendGroupNeedsSplit() mismatch for %q; expected %t", test.msg, result) + } + } +} + +func TestBackendGroupName(t *testing.T) { + tests := []struct { + msg string + backends []state.BackendRef + expName string + }{ + { + msg: "empty backends", + backends: []state.BackendRef{}, + expName: invalidBackendRef, + }, + { + msg: "nil backends", + backends: nil, + expName: invalidBackendRef, + }, + { + msg: "one valid backend with non-zero weight", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: true, + Weight: 1, + }, + }, + expName: "backend1", + }, + { + msg: "one valid backend with zero weight", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: true, + Weight: 0, + }, + }, + expName: invalidBackendRef, + }, + { + msg: "one invalid backend", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: false, + Weight: 1, + }, + }, + expName: invalidBackendRef, + }, + { + msg: "multiple valid backends", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: true, + Weight: 1, + }, + { + Name: "backend2", + Valid: true, + Weight: 1, + }, + }, + expName: "test__hr_rule0", + }, + { + msg: "multiple invalid backends", + backends: []state.BackendRef{ + { + Name: "backend1", + Valid: false, + Weight: 1, + }, + { + Name: "backend2", + Valid: false, + Weight: 1, + }, + }, + expName: "test__hr_rule0", + }, + } + + for _, test := range tests { + bg := state.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 0, + Backends: test.backends, + } + result := backendGroupName(bg) + if result != test.expName { + t.Errorf("backendGroupName() mismatch for %q; expected %s, got %s", test.msg, test.expName, result) + } + } +} diff --git a/internal/nginx/config/template.go b/internal/nginx/config/template.go deleted file mode 100644 index e335f33cd0..0000000000 --- a/internal/nginx/config/template.go +++ /dev/null @@ -1,52 +0,0 @@ -package config - -import ( - "bytes" - "fmt" - "text/template" -) - -// 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 http servers and upstreams, but in the future we will also need to generate -// the main NGINX configuration file and stream servers. -type templateExecutor struct { - httpServersTemplate *template.Template - httpUpstreamsTemplate *template.Template -} - -func newTemplateExecutor() *templateExecutor { - serverTemplate, err := template.New("http-servers").Parse(httpServersTemplate) - if err != nil { - panic(fmt.Errorf("failed to parse http servers template: %w", err)) - } - - upstreamTemplate, err := template.New("http-upstreams").Parse(httpUpstreamsTemplate) - if err != nil { - panic(fmt.Errorf("failed to parse http upstreams template: %w", err)) - } - - return &templateExecutor{httpServersTemplate: serverTemplate, httpUpstreamsTemplate: upstreamTemplate} -} - -func (e *templateExecutor) ExecuteForHTTPServers(servers httpServers) []byte { - var buf bytes.Buffer - - err := e.httpServersTemplate.Execute(&buf, servers) - if err != nil { - panic(fmt.Errorf("failed to execute http servers template: %w", err)) - } - - return buf.Bytes() -} - -func (e *templateExecutor) ExecuteForHTTPUpstreams(upstreams httpUpstreams) []byte { - var buf bytes.Buffer - - err := e.httpUpstreamsTemplate.Execute(&buf, upstreams) - if err != nil { - panic(fmt.Errorf("failed to execute http upstream template: %w", err)) - } - - return buf.Bytes() -} diff --git a/internal/nginx/config/template_test.go b/internal/nginx/config/template_test.go deleted file mode 100644 index 83595dd610..0000000000 --- a/internal/nginx/config/template_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package config - -import ( - "testing" - "text/template" -) - -func TestExecuteForHTTPServers(t *testing.T) { - executor := newTemplateExecutor() - - servers := httpServers{ - Servers: []server{ - { - ServerName: "example.com", - Locations: []location{ - { - Path: "/", - ProxyPass: "http://example-upstream", - }, - }, - }, - }, - } - - 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 { - t.Error("ExecuteForHTTPServers() returned 0-length config") - } -} - -func TestExecuteForHTTPUpstreams(t *testing.T) { - executor := newTemplateExecutor() - - upstreams := httpUpstreams{ - Upstreams: []upstream{ - { - Name: "example-upstream", - Servers: []upstreamServer{ - { - Address: "http://10.0.0.1:80", - }, - }, - }, - }, - } - - cfg := executor.ExecuteForHTTPUpstreams(upstreams) - // we only do a sanity check here. - // the config generation logic is tested in the Generator tests. - if len(cfg) == 0 { - t.Error("ExecuteForHTTPUpstreams() returned 0-length config") - } -} - -func TestNewTemplateExecutorPanics(t *testing.T) { - defer func() { - r := recover() - if r == nil { - t.Error("newTemplateExecutor() didn't panic") - } - }() - - httpServersTemplate = "{{ end }}" // invalid template - newTemplateExecutor() -} - -func TestExecuteForHTTPServersPanics(t *testing.T) { - defer func() { - r := recover() - if r == nil { - t.Error("ExecuteForHTTPServers() didn't panic") - } - }() - - tmpl, err := template.New("test").Parse("{{ .NonExistingField }}") - if err != nil { - t.Fatalf("Failed to parse template: %v", err) - } - - executor := &templateExecutor{httpServersTemplate: tmpl} - - _ = executor.ExecuteForHTTPServers(httpServers{}) -} - -func TestExecuteForHTTPUpstreamsPanics(t *testing.T) { - defer func() { - r := recover() - if r == nil { - t.Error("ExecuteForHTTPUpstreams() didn't panic") - } - }() - - tmpl, err := template.New("test").Parse("{{ .NonExistingField }}") - if err != nil { - t.Fatalf("Failed to parse template: %v", err) - } - - executor := &templateExecutor{httpUpstreamsTemplate: tmpl} - - _ = executor.ExecuteForHTTPUpstreams(httpUpstreams{}) -} diff --git a/internal/nginx/config/upstreams.go b/internal/nginx/config/upstreams.go new file mode 100644 index 0000000000..c7fdbbf841 --- /dev/null +++ b/internal/nginx/config/upstreams.go @@ -0,0 +1,75 @@ +package config + +import ( + "fmt" + gotemplate "text/template" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" +) + +var upstreamsTemplate = gotemplate.Must(gotemplate.New("upstreams").Parse(upstreamsTemplateText)) + +const ( + // nginx502Server is used as a backend for services that cannot be resolved (have no IP address). + nginx502Server = "unix:/var/lib/nginx/nginx-502-server.sock" + // nginx500Server is used as a server for the invalid backend ref upstream. + nginx500Server = "unix:/var/lib/nginx/nginx-500-server.sock" + // invalidBackendRef is used as an upstream name for invalid backend references. + invalidBackendRef = "invalid-backend-ref" +) + +func executeUpstreams(conf state.Configuration) []byte { + upstreams := createUpstreams(conf.Upstreams) + + return execute(upstreamsTemplate, upstreams) +} + +func createUpstreams(upstreams []state.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, createInvalidBackendRefUpstream()) + + return ups +} + +func createUpstream(up state.Upstream) http.Upstream { + if len(up.Endpoints) == 0 { + return http.Upstream{ + Name: up.Name, + Servers: []http.UpstreamServer{ + { + Address: nginx502Server, + }, + }, + } + } + + upstreamServers := make([]http.UpstreamServer, len(up.Endpoints)) + for idx, ep := range up.Endpoints { + upstreamServers[idx] = http.UpstreamServer{ + Address: fmt.Sprintf("%s:%d", ep.Address, ep.Port), + } + } + + return http.Upstream{ + Name: up.Name, + Servers: upstreamServers, + } +} + +func createInvalidBackendRefUpstream() http.Upstream { + return http.Upstream{ + Name: invalidBackendRef, + Servers: []http.UpstreamServer{ + { + Address: nginx500Server, + }, + }, + } +} diff --git a/internal/nginx/config/upstreams_template.go b/internal/nginx/config/upstreams_template.go index 3c2f6cc3b7..4365b51b98 100644 --- a/internal/nginx/config/upstreams_template.go +++ b/internal/nginx/config/upstreams_template.go @@ -1,7 +1,8 @@ package config // FIXME(kate-osborn): Add upstream zone size for each upstream. This should be dynamically calculated based on the number of upstreams. -var httpUpstreamsTemplate = `{{ range $u := .Upstreams }} +var upstreamsTemplateText = ` +{{ range $u := . }} upstream {{ $u.Name }} { random two least_conn; {{ range $server := $u.Servers }} diff --git a/internal/nginx/config/upstreams_test.go b/internal/nginx/config/upstreams_test.go new file mode 100644 index 0000000000..8d2f526289 --- /dev/null +++ b/internal/nginx/config/upstreams_test.go @@ -0,0 +1,221 @@ +package config + +import ( + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/http" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" +) + +func TestExecuteUpstreams(t *testing.T) { + stateUpstreams := []state.Upstream{ + { + Name: "up1", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.0", + Port: 80, + }, + }, + }, + { + Name: "up2", + Endpoints: []resolver.Endpoint{ + { + Address: "11.0.0.0", + Port: 80, + }, + }, + }, + { + Name: "up3", + Endpoints: []resolver.Endpoint{}, + }, + } + + expectedSubStrings := []string{ + "upstream up1", + "upstream up2", + "upstream up3", + "upstream invalid-backend-ref", + "server 10.0.0.0:80;", + "server 11.0.0.0:80;", + "server unix:/var/lib/nginx/nginx-502-server.sock;", + } + + upstreams := string(executeUpstreams(state.Configuration{Upstreams: stateUpstreams})) + for _, expSubString := range expectedSubStrings { + if !strings.Contains(upstreams, expSubString) { + t.Errorf( + "executeUpstreams() did not generate upstreams with expected substring %q, got %q", + expSubString, + upstreams, + ) + } + } +} + +func TestCreateUpstreams(t *testing.T) { + stateUpstreams := []state.Upstream{ + { + Name: "up1", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.0", + Port: 80, + }, + { + Address: "10.0.0.1", + Port: 80, + }, + { + Address: "10.0.0.2", + Port: 80, + }, + }, + }, + { + Name: "up2", + Endpoints: []resolver.Endpoint{ + { + Address: "11.0.0.0", + Port: 80, + }, + }, + }, + { + Name: "up3", + Endpoints: []resolver.Endpoint{}, + }, + } + + expUpstreams := []http.Upstream{ + { + Name: "up1", + Servers: []http.UpstreamServer{ + { + Address: "10.0.0.0:80", + }, + { + Address: "10.0.0.1:80", + }, + { + Address: "10.0.0.2:80", + }, + }, + }, + { + Name: "up2", + Servers: []http.UpstreamServer{ + { + Address: "11.0.0.0:80", + }, + }, + }, + { + Name: "up3", + Servers: []http.UpstreamServer{ + { + Address: nginx502Server, + }, + }, + }, + { + Name: invalidBackendRef, + Servers: []http.UpstreamServer{ + { + Address: nginx500Server, + }, + }, + }, + } + + result := createUpstreams(stateUpstreams) + if diff := cmp.Diff(expUpstreams, result); diff != "" { + t.Errorf("createUpstreams() mismatch (-want +got):\n%s", diff) + } +} + +func TestCreateUpstream(t *testing.T) { + tests := []struct { + stateUpstream state.Upstream + expectedUpstream http.Upstream + msg string + }{ + { + stateUpstream: state.Upstream{ + Name: "nil-endpoints", + Endpoints: nil, + }, + expectedUpstream: http.Upstream{ + Name: "nil-endpoints", + Servers: []http.UpstreamServer{ + { + Address: nginx502Server, + }, + }, + }, + msg: "nil endpoints", + }, + { + stateUpstream: state.Upstream{ + Name: "no-endpoints", + Endpoints: []resolver.Endpoint{}, + }, + expectedUpstream: http.Upstream{ + Name: "no-endpoints", + Servers: []http.UpstreamServer{ + { + Address: nginx502Server, + }, + }, + }, + msg: "no endpoints", + }, + { + stateUpstream: state.Upstream{ + Name: "multiple-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.1", + Port: 80, + }, + { + Address: "10.0.0.2", + Port: 80, + }, + { + Address: "10.0.0.3", + Port: 80, + }, + }, + }, + expectedUpstream: http.Upstream{ + Name: "multiple-endpoints", + Servers: []http.UpstreamServer{ + { + Address: "10.0.0.1:80", + }, + { + Address: "10.0.0.2:80", + }, + { + Address: "10.0.0.3:80", + }, + }, + }, + msg: "multiple endpoints", + }, + } + + for _, test := range tests { + result := createUpstream(test.stateUpstream) + if diff := cmp.Diff(test.expectedUpstream, result); diff != "" { + t.Errorf("createUpstream() %q mismatch (-want +got):\n%s", test.msg, diff) + } + } +} diff --git a/internal/nginx/config/variable_names.go b/internal/nginx/config/variable_names.go new file mode 100644 index 0000000000..5ea718e79e --- /dev/null +++ b/internal/nginx/config/variable_names.go @@ -0,0 +1,11 @@ +package config + +import ( + "strings" +) + +// NGINX Variable names cannot have hyphens. +// This function converts a hyphenated string to an underscored string. +func convertStringToSafeVariableName(s string) string { + return strings.ReplaceAll(s, "-", "_") +} diff --git a/internal/nginx/config/variable_names_test.go b/internal/nginx/config/variable_names_test.go new file mode 100644 index 0000000000..8e45a9fcb2 --- /dev/null +++ b/internal/nginx/config/variable_names_test.go @@ -0,0 +1,32 @@ +package config + +import "testing" + +func TestConvertStringToSafeVariableName(t *testing.T) { + tests := []struct { + msg string + s string + expected string + }{ + { + msg: "no hyphens", + s: "foo", + expected: "foo", + }, + { + msg: "hyphens", + s: "foo-bar-baz", + expected: "foo_bar_baz", + }, + } + for _, test := range tests { + if result := convertStringToSafeVariableName(test.s); result != test.expected { + t.Errorf( + "convertStringToSafeVariableName() mismatch for test %q; expected %s, got %s", + test.msg, + test.expected, + result, + ) + } + } +} diff --git a/internal/state/backend_group.go b/internal/state/backend_group.go new file mode 100644 index 0000000000..9c8b5be3fe --- /dev/null +++ b/internal/state/backend_group.go @@ -0,0 +1,34 @@ +package state + +import ( + "fmt" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" +) + +// BackendGroup represents a group of backends for a rule in an HTTPRoute. +type BackendGroup struct { + Errors []string + Source types.NamespacedName + RuleIdx int + Backends []BackendRef +} + +// BackendRef is an internal representation of a backendRef in an HTTPRoute. +type BackendRef struct { + Name string + Svc *v1.Service + Port int32 + Valid bool + Weight int32 +} + +// GroupName returns the name of the backend group. +// This name must be unique across all HTTPRoutes and all rules within the same HTTPRoute. +// The RuleIdx is used to make the name unique across all rules within the same HTTPRoute. +// The RuleIdx may change for a given rule if an update is made to the HTTPRoute, but it will always match the index +// of the rule in the stored HTTPRoute. +func (bg *BackendGroup) GroupName() string { + return fmt.Sprintf("%s__%s_rule%d", bg.Source.Namespace, bg.Source.Name, bg.RuleIdx) +} diff --git a/internal/state/backend_group_test.go b/internal/state/backend_group_test.go new file mode 100644 index 0000000000..0b8e216ca9 --- /dev/null +++ b/internal/state/backend_group_test.go @@ -0,0 +1,21 @@ +package state_test + +import ( + "testing" + + "k8s.io/apimachinery/pkg/types" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" +) + +func TestBackendGroup_GroupName(t *testing.T) { + bg := state.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 20, + } + expected := "test__hr_rule20" + result := bg.GroupName() + if result != expected { + t.Errorf("BackendGroup.GroupName() mismatch; expected %s, got %s", expected, result) + } +} diff --git a/internal/state/backend_refs.go b/internal/state/backend_refs.go new file mode 100644 index 0000000000..abbb80d657 --- /dev/null +++ b/internal/state/backend_refs.go @@ -0,0 +1,108 @@ +package state + +import ( + "errors" + "fmt" + + 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" +) + +// addBackendGroupsToRoutes iterates over the routes and adds BackendGroups to the routes. +// The routes are modified in place. +// If a backend ref is invalid it will store an error message in the BackendGroup.Errors field. +// A backend ref is invalid if: +// - the Kind is not Service +// - the Namespace is not the same as the HTTPRoute namespace +// - the Port is nil +func addBackendGroupsToRoutes( + routes map[types.NamespacedName]*route, + services map[types.NamespacedName]*v1.Service, +) { + for _, r := range routes { + r.BackendGroups = make([]BackendGroup, len(r.Source.Spec.Rules)) + + for idx, rule := range r.Source.Spec.Rules { + + group := BackendGroup{ + Source: client.ObjectKeyFromObject(r.Source), + RuleIdx: idx, + } + + if len(rule.BackendRefs) == 0 { + + r.BackendGroups[idx] = group + continue + } + + group.Errors = make([]string, 0, len(rule.BackendRefs)) + group.Backends = make([]BackendRef, 0, len(rule.BackendRefs)) + + for _, ref := range rule.BackendRefs { + + weight := int32(1) + if ref.Weight != nil { + weight = *ref.Weight + } + + svc, port, err := getServiceAndPortFromRef(ref.BackendRef, r.Source.Namespace, services) + if err != nil { + group.Backends = append(group.Backends, BackendRef{Weight: weight}) + + group.Errors = append(group.Errors, err.Error()) + + continue + } + + group.Backends = append(group.Backends, BackendRef{ + Name: fmt.Sprintf("%s_%s_%d", svc.Namespace, svc.Name, port), + Svc: svc, + Port: port, + Valid: true, + Weight: weight, + }) + } + + r.BackendGroups[idx] = group + } + } +} + +func getServiceAndPortFromRef( + ref v1beta1.BackendRef, + routeNamespace string, + services map[types.NamespacedName]*v1.Service, +) (*v1.Service, int32, error) { + err := validateBackendRef(ref, routeNamespace) + if err != nil { + return nil, 0, err + } + + svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: routeNamespace} + + svc, ok := services[svcNsName] + if !ok { + return nil, 0, fmt.Errorf("the Service %s does not exist", svcNsName) + } + + // safe to dereference port here because we already validated that the port is not nil. + return svc, int32(*ref.Port), nil +} + +func validateBackendRef(ref v1beta1.BackendRef, routeNs string) error { + if ref.Kind != nil && *ref.Kind != "Service" { + return fmt.Errorf("the Kind must be Service; got %s", *ref.Kind) + } + + if ref.Namespace != nil && string(*ref.Namespace) != routeNs { + return fmt.Errorf("cross-namespace routing is not permitted; namespace %s does not match the HTTPRoute namespace %s", *ref.Namespace, routeNs) + } + + if ref.Port == nil { + return errors.New("port is missing") + } + + return nil +} diff --git a/internal/state/backend_refs_test.go b/internal/state/backend_refs_test.go new file mode 100644 index 0000000000..432b62c11b --- /dev/null +++ b/internal/state/backend_refs_test.go @@ -0,0 +1,397 @@ +package state + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" +) + +func getNormalRef() v1beta1.BackendRef { + return v1beta1.BackendRef{ + BackendObjectReference: v1beta1.BackendObjectReference{ + Kind: (*v1beta1.Kind)(helpers.GetStringPointer("Service")), + Name: "service1", + Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), + Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), + }, + Weight: helpers.GetInt32Pointer(1), + } +} + +func getModifiedRef(mod func(ref v1beta1.BackendRef) v1beta1.BackendRef) v1beta1.BackendRef { + return mod(getNormalRef()) +} + +func TestValidateBackendRef(t *testing.T) { + tests := []struct { + msg string + ref v1beta1.BackendRef + expErr bool + }{ + { + msg: "normal case", + ref: getNormalRef(), + expErr: false, + }, + { + msg: "normal case with implicit namespace", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Namespace = nil + return backend + }), + expErr: false, + }, + { + msg: "normal case with implicit kind Service", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Kind = nil + return backend + }), + expErr: false, + }, + { + msg: "not a service kind", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Kind = (*v1beta1.Kind)(helpers.GetStringPointer("NotService")) + return backend + }), + expErr: true, + }, + { + msg: "invalid namespace", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Namespace = (*v1beta1.Namespace)(helpers.GetStringPointer("invalid")) + return backend + }), + expErr: true, + }, + { + msg: "missing port", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Port = nil + return backend + }), + expErr: true, + }, + } + + for _, test := range tests { + err := validateBackendRef(test.ref, "test") + errOccurred := err != nil + if errOccurred != test.expErr { + t.Errorf("validateBackendRef() returned incorrect error for %q; error: %v", test.msg, err) + } + } +} + +func TestGetServiceAndPortFromRef(t *testing.T) { + svc1 := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service1", + Namespace: "test", + }, + } + + svc2 := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service2", + Namespace: "test", + }, + } + + tests := []struct { + msg string + ref v1beta1.BackendRef + expService *v1.Service + expPort int32 + expErr bool + }{ + { + msg: "normal case", + ref: getNormalRef(), + expService: svc1, + expPort: 80, + }, + { + msg: "invalid backend ref", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Port = nil + return backend + }), + expErr: true, + }, + { + msg: "service does not exist", + ref: getModifiedRef(func(backend v1beta1.BackendRef) v1beta1.BackendRef { + backend.Name = "dne" + return backend + }), + expErr: true, + }, + } + + services := map[types.NamespacedName]*v1.Service{ + {Namespace: "test", Name: "service1"}: svc1, + {Namespace: "test", Name: "service2"}: svc2, + } + + for _, test := range tests { + svc, port, err := getServiceAndPortFromRef(test.ref, "test", services) + + errOccurred := err != nil + if errOccurred != test.expErr { + t.Errorf("getServiceAndPortFromRef() returned incorrect error for %q; error: %v", test.msg, err) + } + + if svc != test.expService { + t.Errorf("getServiceAndPortFromRef() returned incorrect service for %q; expected: %v, got: %v", + test.msg, test.expService, svc) + } + + if port != test.expPort { + t.Errorf("getServiceAndPortFromRef() returned incorrect port for %q; expected: %d, got: %d", + test.msg, test.expPort, port) + } + } +} + +func TestResolveBackendRefs(t *testing.T) { + createRoute := func(name string, kind string, serviceNames ...string) *v1beta1.HTTPRoute { + hr := &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + }, + } + + hr.Spec.Rules = make([]v1beta1.HTTPRouteRule, len(serviceNames)) + + for idx, svcName := range serviceNames { + hr.Spec.Rules[idx] = v1beta1.HTTPRouteRule{ + BackendRefs: []v1beta1.HTTPBackendRef{ + { + BackendRef: v1beta1.BackendRef{ + BackendObjectReference: v1beta1.BackendObjectReference{ + Kind: (*v1beta1.Kind)(helpers.GetStringPointer(kind)), + Name: v1beta1.ObjectName(svcName), + Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), + Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), + }, + Weight: nil, // should use default weight of 1 + }, + }, + { + BackendRef: v1beta1.BackendRef{ + BackendObjectReference: v1beta1.BackendObjectReference{ + Kind: (*v1beta1.Kind)(helpers.GetStringPointer(kind)), + Name: v1beta1.ObjectName(svcName), + Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), + Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(81)), + }, + Weight: helpers.GetInt32Pointer(5), + }, + }, + }, + } + } + return hr + } + + removeRefs := func(route *v1beta1.HTTPRoute) *v1beta1.HTTPRoute { + route.Spec.Rules[0].BackendRefs = nil + return route + } + + hr1 := createRoute("hr1", "Service", "svc1", "svc2", "svc3") + hr2 := createRoute("hr2", "Service", "svc1", "svc4") + hr3 := createRoute("hr3", "NotService", "not-svc") + hr4 := removeRefs(createRoute("hr4", "Service", "no-backend-refs")) + + routes := map[types.NamespacedName]*route{ + {Namespace: "test", Name: "hr1"}: { + Source: hr1, + }, + {Namespace: "test", Name: "hr2"}: { + Source: hr2, + }, + {Namespace: "test", Name: "hr3"}: { + Source: hr3, + }, + {Namespace: "test", Name: "hr4"}: { + Source: hr4, + }, + } + + svc1 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc1"}} + svc2 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc2"}} + svc3 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc3"}} + svc4 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "svc4"}} + + services := map[types.NamespacedName]*v1.Service{ + {Namespace: "test", Name: "svc1"}: svc1, + {Namespace: "test", Name: "svc2"}: svc2, + {Namespace: "test", Name: "svc3"}: svc3, + {Namespace: "test", Name: "svc4"}: svc4, + } + + expRoutes := map[types.NamespacedName]*route{ + {Namespace: "test", Name: "hr1"}: { + Source: hr1, + BackendGroups: []BackendGroup{ + { + Source: client.ObjectKeyFromObject(hr1), + RuleIdx: 0, + Errors: []string{}, + Backends: []BackendRef{ + { + Name: "test_svc1_80", + Svc: svc1, + Port: 80, + Valid: true, + Weight: 1, + }, + { + Name: "test_svc1_81", + Svc: svc1, + Port: 81, + Valid: true, + Weight: 5, + }, + }, + }, + { + Source: client.ObjectKeyFromObject(hr1), + RuleIdx: 1, + Errors: []string{}, + Backends: []BackendRef{ + { + Name: "test_svc2_80", + Svc: svc2, + Port: 80, + Valid: true, + Weight: 1, + }, + { + Name: "test_svc2_81", + Svc: svc2, + Port: 81, + Valid: true, + Weight: 5, + }, + }, + }, + { + Source: client.ObjectKeyFromObject(hr1), + RuleIdx: 2, + Errors: []string{}, + Backends: []BackendRef{ + { + Name: "test_svc3_80", + Svc: svc3, + Port: 80, + Valid: true, + Weight: 1, + }, + { + Name: "test_svc3_81", + Svc: svc3, + Port: 81, + Valid: true, + Weight: 5, + }, + }, + }, + }, + }, + {Namespace: "test", Name: "hr2"}: { + Source: hr2, + BackendGroups: []BackendGroup{ + { + Source: client.ObjectKeyFromObject(hr2), + RuleIdx: 0, + Errors: []string{}, + Backends: []BackendRef{ + { + Name: "test_svc1_80", + Svc: svc1, + Port: 80, + Valid: true, + Weight: 1, + }, + { + Name: "test_svc1_81", + Svc: svc1, + Port: 81, + Valid: true, + Weight: 5, + }, + }, + }, + { + Source: client.ObjectKeyFromObject(hr2), + RuleIdx: 1, + Errors: []string{}, + Backends: []BackendRef{ + { + Name: "test_svc4_80", + Svc: svc4, + Port: 80, + Valid: true, + Weight: 1, + }, + { + Name: "test_svc4_81", + Svc: svc4, + Port: 81, + Valid: true, + Weight: 5, + }, + }, + }, + }, + }, + {Namespace: "test", Name: "hr3"}: { + Source: hr3, + BackendGroups: []BackendGroup{ + { + Errors: []string{ + "the Kind must be Service; got NotService", + "the Kind must be Service; got NotService", + }, + Source: client.ObjectKeyFromObject(hr3), + RuleIdx: 0, + Backends: []BackendRef{ + { + Weight: 1, + }, + { + Weight: 5, + }, + }, + }, + }, + }, + {Namespace: "test", Name: "hr4"}: { + Source: hr4, + BackendGroups: []BackendGroup{ + { + Source: client.ObjectKeyFromObject(hr4), + RuleIdx: 0, + }, + }, + }, + } + + addBackendGroupsToRoutes(routes, services) + + if diff := cmp.Diff(expRoutes, routes); diff != "" { + t.Errorf("resolveBackendRefs() mismatch on routes (-want +got):\n%s", diff) + } +} diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index 65861b2713..c026c12751 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -145,15 +145,16 @@ func (c *ChangeProcessorImpl) Process(ctx context.Context) (changed bool, conf C c.store.changed = false c.changed = false - graph, warnings := buildGraph( - ctx, + graph := buildGraph( c.store, c.cfg.GatewayCtlrName, c.cfg.GatewayClassName, c.cfg.SecretMemoryManager, - c.cfg.ServiceResolver, ) + var warnings Warnings + conf, warnings = buildConfiguration(ctx, graph, c.cfg.ServiceResolver) + for obj, objWarnings := range warnings { for _, w := range objWarnings { // FIXME(pleshakov): report warnings via Object status @@ -165,7 +166,6 @@ func (c *ChangeProcessorImpl) Process(ctx context.Context) (changed bool, conf C } } - conf = buildConfiguration(graph) statuses = buildStatuses(graph) return true, conf, statuses diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index bdba79cd9f..3cdb4649e7 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -181,6 +181,7 @@ var _ = Describe("ChangeProcessor", func() { var ( gcUpdated *v1beta1.GatewayClass hr1, hr1Updated, hr2 *v1beta1.HTTPRoute + hr1Group, hr2Group state.BackendGroup gw1, gw1Updated, gw2 *v1beta1.Gateway ) BeforeAll(func() { @@ -189,11 +190,21 @@ var _ = Describe("ChangeProcessor", func() { hr1 = createRoute("hr-1", "gateway-1", "foo.example.com") + hr1Group = state.BackendGroup{ + Source: types.NamespacedName{Namespace: hr1.Namespace, Name: hr1.Name}, + RuleIdx: 0, + } + hr1Updated = hr1.DeepCopy() hr1Updated.Generation++ hr2 = createRoute("hr-2", "gateway-2", "bar.example.com") + hr2Group = state.BackendGroup{ + Source: types.NamespacedName{Namespace: hr2.Namespace, Name: hr2.Name}, + RuleIdx: 0, + } + gw1 = createGatewayWithTLSListener("gateway-1") gw1Updated = gw1.DeepCopy() @@ -278,7 +289,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1, }, }, @@ -297,7 +308,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1, }, }, @@ -309,7 +320,9 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, + BackendGroups: []state.BackendGroup{ + hr1Group, + }, } expectedStatuses := state.Statuses{ @@ -373,7 +386,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -392,7 +405,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -404,7 +417,9 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, + BackendGroups: []state.BackendGroup{ + hr1Group, + }, } expectedStatuses := state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ @@ -467,7 +482,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -486,7 +501,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -498,7 +513,9 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, + BackendGroups: []state.BackendGroup{ + hr1Group, + }, } expectedStatuses := state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ @@ -561,7 +578,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -580,7 +597,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -592,7 +609,9 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, + BackendGroups: []state.BackendGroup{ + hr1Group, + }, } expectedStatuses := state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ @@ -652,7 +671,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -670,7 +689,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -685,7 +704,9 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, + BackendGroups: []state.BackendGroup{ + hr1Group, + }, } expectedStatuses := state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ @@ -741,7 +762,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -760,7 +781,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr1Group, Source: hr1Updated, }, }, @@ -772,7 +793,9 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, + BackendGroups: []state.BackendGroup{ + hr1Group, + }, } expectedStatuses := state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ @@ -835,7 +858,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr2Group, Source: hr2, }, }, @@ -854,7 +877,7 @@ var _ = Describe("ChangeProcessor", func() { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: state.InvalidBackendRef, + BackendGroup: hr2Group, Source: hr2, }, }, @@ -866,7 +889,9 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, + BackendGroups: []state.BackendGroup{ + hr2Group, + }, } expectedStatuses := state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ @@ -915,7 +940,6 @@ var _ = Describe("ChangeProcessor", func() { SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - Upstreams: []state.Upstream{}, } expectedStatuses := state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ diff --git a/internal/state/configuration.go b/internal/state/configuration.go index 3733ac0f3b..28e9b4e1a4 100644 --- a/internal/state/configuration.go +++ b/internal/state/configuration.go @@ -1,6 +1,7 @@ package state import ( + "context" "fmt" "sort" @@ -21,8 +22,10 @@ type Configuration struct { // SSLServers holds all SSLServers. // FIXME(kate-osborn) We assume that all SSL servers listen on port 443. SSLServers []VirtualServer - // Upstreams holds all Upstreams. + // Upstreams holds all unique Upstreams. Upstreams []Upstream + // BackendGroups holds all unique BackendGroups. + BackendGroups []BackendGroup } // VirtualServer is a virtual server. @@ -38,6 +41,8 @@ type VirtualServer struct { type Upstream struct { // Name is the name of the Upstream. Will be unique for each service/port combination. Name string + // ErrorMsg contains the error message if the Upstream is invalid. + ErrorMsg string // Endpoints are the endpoints of the Upstream. Endpoints []resolver.Endpoint } @@ -68,14 +73,14 @@ type MatchRule struct { MatchIdx int // RuleIdx is the index of the corresponding rule in the HTTPRoute. RuleIdx int - // UpstreamName is the name of the upstream for this routing rule. - UpstreamName string + // Filters holds the filters for the MatchRule. + Filters Filters + // BackendGroup is the group of Backends that the rule routes to. + BackendGroup BackendGroup // Source is the corresponding HTTPRoute resource. - // FIXME(pleshakov): Consider referencing only the parts neeeded for the config generation rather than + // FIXME(pleshakov): Consider referencing only the parts needed for the config generation rather than // the entire resource. Source *v1beta1.HTTPRoute - // Filters holds the filters for the MatchRule. - Filters Filters } // GetMatch returns the HTTPRouteMatch of the Route . @@ -85,25 +90,134 @@ 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(graph *graph) Configuration { +func buildConfiguration( + ctx context.Context, + graph *graph, + resolver resolver.ServiceResolver, +) (Configuration, Warnings) { if graph.GatewayClass == nil || !graph.GatewayClass.Valid { - return Configuration{} + return Configuration{}, nil } if graph.Gateway == nil { - return Configuration{} + return Configuration{}, nil } - upstreams := buildUpstreams(graph.Backends) + upstreamsMap := buildUpstreamsMap(ctx, graph.Gateway.Listeners, resolver) httpServers, sslServers := buildServers(graph.Gateway.Listeners) + backendGroups := buildBackendGroups(graph.Gateway.Listeners) + + warnings := buildWarnings(graph, upstreamsMap) config := Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - Upstreams: upstreams, + HTTPServers: httpServers, + SSLServers: sslServers, + Upstreams: upstreamsMapToSlice(upstreamsMap), + BackendGroups: backendGroups, + } + + return config, warnings +} + +func upstreamsMapToSlice(upstreamsMap map[string]Upstream) []Upstream { + if len(upstreamsMap) == 0 { + return nil + } + + upstreams := make([]Upstream, 0, len(upstreamsMap)) + for _, upstream := range upstreamsMap { + upstreams = append(upstreams, upstream) + } + + return upstreams +} + +func buildWarnings(graph *graph, upstreams map[string]Upstream) Warnings { + warnings := newWarnings() + + for _, l := range graph.Gateway.Listeners { + for _, r := range l.Routes { + if !l.Valid { + warnings.AddWarningf( + r.Source, + "cannot configure routes for listener %s; listener is invalid", + l.Source.Name, + ) + + continue + } + + for _, group := range r.BackendGroups { + + for _, errMsg := range group.Errors { + warnings.AddWarningf(r.Source, "invalid backend ref: %s", errMsg) + } + + for _, backend := range group.Backends { + if backend.Name != "" { + upstream, ok := upstreams[backend.Name] + + // this should never happen; but we check it just in case + if !ok { + warnings.AddWarningf( + r.Source, + "cannot resolve backend ref; internal error: upstream %s not found in map", + backend.Name, + ) + } + + if upstream.ErrorMsg != "" { + warnings.AddWarningf( + r.Source, + "cannot resolve backend ref: %s", + upstream.ErrorMsg, + ) + } + } + } + } + } + } + + if len(warnings) == 0 { + return nil + } + + return warnings +} + +func buildBackendGroups(listeners map[string]*listener) []BackendGroup { + // There can be duplicate backend groups if a route is attached to multiple listeners. + // We use a map to deduplicate them. + uniqueGroups := make(map[string]BackendGroup) + + for _, l := range listeners { + + if !l.Valid { + continue + } + + for _, r := range l.Routes { + for _, group := range r.BackendGroups { + if _, ok := uniqueGroups[group.GroupName()]; !ok { + uniqueGroups[group.GroupName()] = group + } + } + } + + } + + numGroups := len(uniqueGroups) + if len(uniqueGroups) == 0 { + return nil } - return config + groups := make([]BackendGroup, 0, numGroups) + for _, group := range uniqueGroups { + groups = append(groups, group) + } + + return groups } func buildServers(listeners map[string]*listener) (http, ssl []VirtualServer) { @@ -176,8 +290,8 @@ func (hpr *hostPathRules) upsertListener(l *listener) { rule.MatchRules = append(rule.MatchRules, MatchRule{ MatchIdx: j, RuleIdx: i, - UpstreamName: generateUpstreamName(r.BackendServices[ruleIndex(i)]), Source: r.Source, + BackendGroup: r.BackendGroups[i], Filters: filters, }) @@ -212,7 +326,7 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { s.PathRules = append(s.PathRules, r) } - // sort rules for predictable order + // We sort the path rules so the order is preserved after reconfiguration. sort.Slice(s.PathRules, func(i, j int) bool { return s.PathRules[i].Path < s.PathRules[j].Path }) @@ -237,6 +351,7 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { } } + // We sort the servers so the order is preserved after reconfiguration. sort.Slice(servers, func(i, j int) bool { return servers[i].Hostname < servers[j].Hostname }) @@ -244,6 +359,52 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { return servers } +func buildUpstreamsMap( + ctx context.Context, + listeners map[string]*listener, + resolver resolver.ServiceResolver, +) map[string]Upstream { + // There can be duplicate upstreams if multiple routes reference the same upstream. + // We use a map to deduplicate them. + uniqueUpstreams := make(map[string]Upstream) + + for _, l := range listeners { + + if !l.Valid { + continue + } + + for _, route := range l.Routes { + for _, group := range route.BackendGroups { + for _, backend := range group.Backends { + if name := backend.Name; name != "" { + _, exist := uniqueUpstreams[name] + + if exist { + continue + } + + var errMsg string + + eps, err := resolver.Resolve(ctx, backend.Svc, backend.Port) + if err != nil { + errMsg = err.Error() + } + + uniqueUpstreams[name] = Upstream{ + Name: name, + Endpoints: eps, + ErrorMsg: errMsg, + } + } + } + } + } + } + + return uniqueUpstreams +} + func getListenerHostname(h *v1beta1.Hostname) string { name := getHostname(h) if name == "" { diff --git a/internal/state/configuration_test.go b/internal/state/configuration_test.go index 05a3ef5559..e22b779e4d 100644 --- a/internal/state/configuration_test.go +++ b/internal/state/configuration_test.go @@ -1,15 +1,21 @@ package state import ( + "context" + "errors" + "fmt" + "sort" "testing" "github.com/google/go-cmp/cmp" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver/resolverfakes" ) func TestBuildConfiguration(t *testing.T) { @@ -56,149 +62,103 @@ func TestBuildConfiguration(t *testing.T) { return hr } - fooBackendSvc := backendService{Name: "foo", Namespace: "test", Port: 80} - - fooBackend := backend{ - Endpoints: []resolver.Endpoint{ - { - Address: "10.0.0.0", - Port: 8080, - }, - }, - } - fooUpstreamName := "test_foo_80" - fooUpstream := Upstream{ - Name: fooUpstreamName, - Endpoints: []resolver.Endpoint{ - { - Address: "10.0.0.0", - Port: 8080, - }, - }, - } - - hr1 := createRoute("hr-1", "foo.example.com", "listener-80-1", "/") - - routeHR1 := &route{ - Source: hr1, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, + fooEndpoints := []resolver.Endpoint{ + { + Address: "10.0.0.0", + Port: 8080, }, } - hr2 := createRoute("hr-2", "bar.example.com", "listener-80-1", "/") - - routeHR2 := &route{ - Source: hr2, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, - }, + fooUpstream := Upstream{ + Name: fooUpstreamName, + Endpoints: fooEndpoints, } - httpsHR1 := createRoute("https-hr-1", "foo.example.com", "listener-443-1", "/") + fooSvc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test"}} - httpsRouteHR1 := &route{ - Source: httpsHR1, - ValidSectionNameRefs: map[string]struct{}{ - "listener-443-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, - }, - } + fakeResolver := &resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveReturns(fooEndpoints, nil) - httpsHR2 := createRoute("https-hr-2", "bar.example.com", "listener-443-1", "/") - - httpsRouteHR2 := &route{ - Source: httpsHR2, - ValidSectionNameRefs: map[string]struct{}{ - "listener-443-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, - }, + createBackendGroup := func(nsname types.NamespacedName, idx int) BackendGroup { + return BackendGroup{ + Source: nsname, + RuleIdx: idx, + Backends: []BackendRef{ + { + Name: fooUpstreamName, + Svc: fooSvc, + Port: 80, + Valid: true, + Weight: 1, + }, + }, + } } - hr3 := createRoute("hr-3", "foo.example.com", "listener-80-1", "/", "/third") - - routeHR3 := &route{ - Source: hr3, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, - ruleIndex(1): fooBackendSvc, - }, + createInternalRoute := func(source *v1beta1.HTTPRoute, validSectionName string, groups ...BackendGroup) *route { + r := &route{ + Source: source, + InvalidSectionNameRefs: make(map[string]struct{}), + ValidSectionNameRefs: map[string]struct{}{validSectionName: {}}, + BackendGroups: groups, + } + return r } - httpsHR3 := createRoute("https-hr-3", "foo.example.com", "listener-443-1", "/", "/third") + createTestResources := func(name, hostname, listenerName string, paths ...string) ( + *v1beta1.HTTPRoute, []BackendGroup, *route, + ) { + hr := createRoute(name, hostname, listenerName, paths...) + groups := make([]BackendGroup, 0, len(paths)) + for idx := range paths { + groups = append(groups, createBackendGroup(types.NamespacedName{Namespace: "test", Name: name}, idx)) + } - httpsRouteHR3 := &route{ - Source: httpsHR3, - ValidSectionNameRefs: map[string]struct{}{ - "listener-443-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, - ruleIndex(1): fooBackendSvc, - }, + route := createInternalRoute(hr, listenerName, groups...) + return hr, groups, route } - hr4 := createRoute("hr-4", "foo.example.com", "listener-80-1", "/fourth", "/") + hr1, hr1Groups, routeHR1 := createTestResources("hr-1", "foo.example.com", "listener-80-1", "/") + hr2, hr2Groups, routeHR2 := createTestResources("hr-2", "bar.example.com", "listener-80-1", "/") + hr3, hr3Groups, routeHR3 := createTestResources("hr-3", "foo.example.com", "listener-80-1", "/", "/third") + hr4, hr4Groups, routeHR4 := createTestResources("hr-4", "foo.example.com", "listener-80-1", "/fourth", "/") - routeHR4 := &route{ - Source: hr4, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, - ruleIndex(1): fooBackendSvc, - }, - } + httpsHR1, httpsHR1Groups, httpsRouteHR1 := createTestResources( + "https-hr-1", + "foo.example.com", + "listener-443-1", + "/", + ) - httpsHR4 := createRoute("https-hr-4", "foo.example.com", "listener-443-1", "/fourth", "/") + httpsHR2, httpsHR2Groups, httpsRouteHR2 := createTestResources( + "https-hr-2", + "bar.example.com", + "listener-443-1", + "/", + ) - httpsRouteHR4 := &route{ - Source: httpsHR4, - ValidSectionNameRefs: map[string]struct{}{ - "listener-443-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): fooBackendSvc, - ruleIndex(1): fooBackendSvc, - }, - } + httpsHR3, httpsHR3Groups, httpsRouteHR3 := createTestResources( + "https-hr-3", + "foo.example.com", + "listener-443-1", + "/", "/third", + ) - httpsHR5 := createRoute("https-hr-5", "example.com", "listener-443-with-hostname", "/") + httpsHR4, httpsHR4Groups, httpsRouteHR4 := createTestResources( + "https-hr-4", + "foo.example.com", + "listener-443-1", + "/fourth", "/", + ) - httpsRouteHR5 := &route{ - Source: httpsHR5, - ValidSectionNameRefs: map[string]struct{}{ - "listener-443-with-hostname": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): {}, // invalid upstream - }, - } + httpsHR5, httpsHR5Groups, httpsRouteHR5 := createTestResources( + "https-hr-5", + "example.com", + "listener-443-with-hostname", + "/", + ) redirect := v1beta1.HTTPRouteFilter{ Type: v1beta1.HTTPRouteFilterRequestRedirect, @@ -207,17 +167,21 @@ func TestBuildConfiguration(t *testing.T) { }, } - hr6 := addFilters( - createRoute("hr-6", "foo.example.com", "listener-80-1", "/"), + hr5 := addFilters( + createRoute("hr-5", "foo.example.com", "listener-80-1", "/"), []v1beta1.HTTPRouteFilter{redirect}, ) - routeHR6 := &route{ - Source: hr6, - ValidSectionNameRefs: map[string]struct{}{ - "listener-80-1": {}, - }, - InvalidSectionNameRefs: map[string]struct{}{}, + hr5BackendGroup := BackendGroup{ + Source: types.NamespacedName{Namespace: hr5.Namespace, Name: hr5.Name}, + RuleIdx: 0, + } + + routeHR5 := &route{ + Source: hr5, + InvalidSectionNameRefs: make(map[string]struct{}), + ValidSectionNameRefs: map[string]struct{}{"listener-80-1": {}}, + BackendGroups: []BackendGroup{hr5BackendGroup}, } listener80 := v1beta1.Listener{ @@ -275,7 +239,8 @@ func TestBuildConfiguration(t *testing.T) { tests := []struct { graph *graph - expected Configuration + expConf Configuration + expWarns Warnings msg string }{ { @@ -290,10 +255,9 @@ func TestBuildConfiguration(t *testing.T) { }, Routes: map[types.NamespacedName]*route{}, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{}, SSLServers: []VirtualServer{}, - Upstreams: []Upstream{}, }, msg: "no listeners and routes", }, @@ -316,10 +280,9 @@ func TestBuildConfiguration(t *testing.T) { }, Routes: map[types.NamespacedName]*route{}, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{}, SSLServers: []VirtualServer{}, - Upstreams: []Upstream{}, }, msg: "http listener with no routes", }, @@ -350,7 +313,7 @@ func TestBuildConfiguration(t *testing.T) { }, Routes: map[types.NamespacedName]*route{}, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{}, SSLServers: []VirtualServer{ { @@ -362,7 +325,6 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{CertificatePath: secretPath}, }, }, - Upstreams: []Upstream{}, }, msg: "https listeners with no routes", }, @@ -395,10 +357,13 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "https-hr-2"}: httpsRouteHR2, }, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{}, SSLServers: []VirtualServer{}, - Upstreams: []Upstream{}, + }, + expWarns: Warnings{ + httpsHR1: []string{"cannot configure routes for listener invalid-listener; listener is invalid"}, + httpsHR2: []string{"cannot configure routes for listener invalid-listener; listener is invalid"}, }, msg: "invalid listener", }, @@ -429,11 +394,8 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "hr-1"}: routeHR1, {Namespace: "test", Name: "hr-2"}: routeHR2, }, - Backends: map[backendService]backend{ - fooBackendSvc: fooBackend, - }, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{ { Hostname: "bar.example.com", @@ -444,7 +406,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: hr2Groups[0], Source: hr2, }, }, @@ -460,7 +422,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: hr1Groups[0], Source: hr1, }, }, @@ -468,8 +430,9 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, - SSLServers: []VirtualServer{}, - Upstreams: []Upstream{fooUpstream}, + SSLServers: []VirtualServer{}, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []BackendGroup{hr1Groups[0], hr2Groups[0]}, }, msg: "one http listener with two routes for different hostnames", }, @@ -513,11 +476,8 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "https-hr-2"}: httpsRouteHR2, {Namespace: "test", Name: "https-hr-5"}: httpsRouteHR5, }, - Backends: map[backendService]backend{ - fooBackendSvc: fooBackend, - }, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{}, SSLServers: []VirtualServer{ { @@ -529,7 +489,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: httpsHR2Groups[0], Source: httpsHR2, }, }, @@ -548,7 +508,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: InvalidBackendRef, + BackendGroup: httpsHR5Groups[0], Source: httpsHR5, }, }, @@ -567,7 +527,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: httpsHR1Groups[0], Source: httpsHR1, }, }, @@ -582,7 +542,8 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{CertificatePath: secretPath}, }, }, - Upstreams: []Upstream{fooUpstream}, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []BackendGroup{httpsHR1Groups[0], httpsHR2Groups[0], httpsHR5Groups[0]}, }, msg: "two https listeners each with routes for different hostnames", }, @@ -626,11 +587,8 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "https-hr-3"}: httpsRouteHR3, {Namespace: "test", Name: "https-hr-4"}: httpsRouteHR4, }, - Backends: map[backendService]backend{ - fooBackendSvc: fooBackend, - }, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{ { Hostname: "foo.example.com", @@ -641,13 +599,13 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: hr3Groups[0], Source: hr3, }, { MatchIdx: 0, RuleIdx: 1, - UpstreamName: fooUpstreamName, + BackendGroup: hr4Groups[1], Source: hr4, }, }, @@ -658,7 +616,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: hr4Groups[0], Source: hr4, }, }, @@ -669,7 +627,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 1, - UpstreamName: fooUpstreamName, + BackendGroup: hr3Groups[1], Source: hr3, }, }, @@ -690,13 +648,13 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: httpsHR3Groups[0], Source: httpsHR3, }, { MatchIdx: 0, RuleIdx: 1, - UpstreamName: fooUpstreamName, + BackendGroup: httpsHR4Groups[1], Source: httpsHR4, }, }, @@ -707,7 +665,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - UpstreamName: fooUpstreamName, + BackendGroup: httpsHR4Groups[0], Source: httpsHR4, }, }, @@ -718,7 +676,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 1, - UpstreamName: fooUpstreamName, + BackendGroup: httpsHR3Groups[1], Source: httpsHR3, }, }, @@ -730,7 +688,8 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{CertificatePath: secretPath}, }, }, - Upstreams: []Upstream{fooUpstream}, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []BackendGroup{hr3Groups[0], hr3Groups[1], hr4Groups[0], hr4Groups[1], httpsHR3Groups[0], httpsHR3Groups[1], httpsHR4Groups[0], httpsHR4Groups[1]}, }, msg: "one http and one https listener with two routes with the same hostname with and without collisions", }, @@ -760,8 +719,8 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "hr-1"}: routeHR1, }, }, - expected: Configuration{}, - msg: "invalid gatewayclass", + expConf: Configuration{}, + msg: "invalid gatewayclass", }, { graph: &graph{ @@ -784,12 +743,9 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*route{ {Namespace: "test", Name: "hr-1"}: routeHR1, }, - Backends: map[backendService]backend{ - fooBackendSvc: fooBackend, - }, }, - expected: Configuration{}, - msg: "missing gatewayclass", + expConf: Configuration{}, + msg: "missing gatewayclass", }, { graph: &graph{ @@ -800,8 +756,8 @@ func TestBuildConfiguration(t *testing.T) { Gateway: nil, Routes: map[types.NamespacedName]*route{}, }, - expected: Configuration{}, - msg: "missing gateway", + expConf: Configuration{}, + msg: "missing gateway", }, { graph: &graph{ @@ -816,7 +772,7 @@ func TestBuildConfiguration(t *testing.T) { Source: listener80, Valid: true, Routes: map[types.NamespacedName]*route{ - {Namespace: "test", Name: "hr-6"}: routeHR6, + {Namespace: "test", Name: "hr-5"}: routeHR5, }, AcceptedHostnames: map[string]struct{}{ "foo.example.com": {}, @@ -825,10 +781,10 @@ func TestBuildConfiguration(t *testing.T) { }, }, Routes: map[types.NamespacedName]*route{ - {Namespace: "test", Name: "hr-6"}: routeHR6, + {Namespace: "test", Name: "hr-5"}: routeHR5, }, }, - expected: Configuration{ + expConf: Configuration{ HTTPServers: []VirtualServer{ { Hostname: "foo.example.com", @@ -839,8 +795,8 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - Source: hr6, - UpstreamName: "invalid_backend_ref", + Source: hr5, + BackendGroup: hr5BackendGroup, Filters: Filters{ RequestRedirect: redirect.RequestRedirect, }, @@ -850,17 +806,30 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, - SSLServers: []VirtualServer{}, - Upstreams: []Upstream{}, + SSLServers: []VirtualServer{}, + BackendGroups: []BackendGroup{hr5BackendGroup}, }, msg: "one http listener with one route with filters", }, } for _, test := range tests { - result := buildConfiguration(test.graph) - if diff := cmp.Diff(test.expected, result); diff != "" { - t.Errorf("buildConfiguration() %q mismatch (-want +got):\n%s", test.msg, diff) + result, warns := buildConfiguration(context.TODO(), test.graph, fakeResolver) + + sort.Slice(result.BackendGroups, func(i, j int) bool { + return result.BackendGroups[i].GroupName() < result.BackendGroups[j].GroupName() + }) + + sort.Slice(result.Upstreams, func(i, j int) bool { + return result.Upstreams[i].Name < result.Upstreams[j].Name + }) + + if diff := cmp.Diff(test.expConf, result); diff != "" { + t.Errorf("buildConfiguration() %q mismatch for configuration (-want +got):\n%s", test.msg, diff) + } + + if diff := cmp.Diff(test.expWarns, warns); diff != "" { + t.Errorf("buildConfiguration() %q mismatch for warnings (-want +got):\n%s", test.msg, diff) } } } @@ -1089,35 +1058,419 @@ func TestBuildUpstreams(t *testing.T) { }, } - backends := map[backendService]backend{ - {Name: "foo", Namespace: "test", Port: 80}: {Endpoints: fooEndpoints}, - {Name: "bar", Namespace: "test", Port: 8080}: {Endpoints: barEndpoints}, - {Name: "nil-endpoints", Namespace: "test", Port: 443}: {Endpoints: nil}, - {Name: "empty-endpoints", Namespace: "test", Port: 443}: {Endpoints: []resolver.Endpoint{}}, + bazEndpoints := []resolver.Endpoint{ + { + Address: "12.0.0.0", + Port: 80, + }, + } + + baz2Endpoints := []resolver.Endpoint{ + { + Address: "13.0.0.0", + Port: 80, + }, } - expUpstreams := []Upstream{ - {Name: "test_bar_8080", Endpoints: barEndpoints}, - {Name: "test_empty-endpoints_443", Endpoints: []resolver.Endpoint{}}, - {Name: "test_foo_80", Endpoints: fooEndpoints}, - {Name: "test_nil-endpoints_443", Endpoints: nil}, + createBackendGroup := func(serviceNames ...string) BackendGroup { + var backends []BackendRef + for _, name := range serviceNames { + backends = append(backends, BackendRef{ + Name: name, + Svc: &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: name}}, + }) + } + return BackendGroup{ + Backends: backends, + } } - upstreams := buildUpstreams(backends) + hr1Group0 := createBackendGroup("foo", "bar") + + hr1Group1 := createBackendGroup("baz", "", "") // empty service names should be ignored + + hr2Group0 := createBackendGroup("foo", "baz") // shouldn't duplicate foo and baz upstream + + hr2Group1 := createBackendGroup("nil-endpoints") - if diff := helpers.Diff(expUpstreams, upstreams); diff != "" { - t.Errorf("buildUpstreams() returned incorrect Upstreams, diff: %+v", diff) + hr3Group0 := createBackendGroup("baz") // shouldn't duplicate baz upstream + + hr4Group0 := createBackendGroup("empty-endpoints", "") + + hr4Group1 := createBackendGroup("baz2") + + invalidGroup := createBackendGroup("invalid") + + routes := map[types.NamespacedName]*route{ + {Name: "hr1", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr1Group0, hr1Group1}, + }, + {Name: "hr2", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr2Group0, hr2Group1}, + }, + {Name: "hr3", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr3Group0}, + }, + } + + routes2 := map[types.NamespacedName]*route{ + {Name: "hr4", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr4Group0, hr4Group1}, + }, + } + + invalidRoutes := map[types.NamespacedName]*route{ + {Name: "invalid", Namespace: "test"}: { + BackendGroups: []BackendGroup{invalidGroup}, + }, + } + + listeners := map[string]*listener{ + "invalid-listener": { + Valid: false, + Routes: invalidRoutes, // shouldn't be included since listener is invalid + }, + "listener-1": { + Valid: true, + Routes: routes, + }, + "listener-2": { + Valid: true, + Routes: routes2, + }, + } + + emptyEndpointsErrMsg := "empty endpoints error" + nilEndpointsErrMsg := "nil endpoints error" + + expUpstreams := map[string]Upstream{ + "bar": { + Name: "bar", + Endpoints: barEndpoints, + }, + "baz": { + Name: "baz", + Endpoints: bazEndpoints, + }, + "baz2": { + Name: "baz2", + Endpoints: baz2Endpoints, + }, + "empty-endpoints": { + Name: "empty-endpoints", + Endpoints: []resolver.Endpoint{}, + ErrorMsg: emptyEndpointsErrMsg, + }, + "foo": { + Name: "foo", + Endpoints: fooEndpoints, + }, + "nil-endpoints": { + Name: "nil-endpoints", + Endpoints: nil, + ErrorMsg: nilEndpointsErrMsg, + }, + } + fakeResolver := &resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveCalls(func(ctx context.Context, svc *v1.Service, port int32) ([]resolver.Endpoint, error) { + switch svc.Name { + case "bar": + return barEndpoints, nil + case "baz": + return bazEndpoints, nil + case "baz2": + return baz2Endpoints, nil + case "empty-endpoints": + return []resolver.Endpoint{}, errors.New(emptyEndpointsErrMsg) + case "foo": + return fooEndpoints, nil + case "nil-endpoints": + return nil, errors.New(nilEndpointsErrMsg) + default: + return nil, fmt.Errorf("unexpected service %s", svc.Name) + } + }) + + upstreams := buildUpstreamsMap(context.TODO(), listeners, fakeResolver) + + if diff := cmp.Diff(expUpstreams, upstreams); diff != "" { + t.Errorf("buildUpstreamsMap() mismatch (-want +got):\n%s", diff) } } -func TestGenerateUpstreamName(t *testing.T) { - // empty backend service - if name := generateUpstreamName(backendService{}); name != InvalidBackendRef { - t.Errorf("generateUpstreamName() returned unexepected name: %s, expected: %s", name, InvalidBackendRef) +func TestBuildBackendGroups(t *testing.T) { + createBackendGroup := func(name string, ruleIdx int, backendNames ...string) BackendGroup { + backends := make([]BackendRef, len(backendNames)) + for i, name := range backendNames { + backends[i] = BackendRef{Name: name} + } + + return BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: name}, + RuleIdx: ruleIdx, + Backends: backends, + } + } + + hr1Rule0 := createBackendGroup("hr1", 0, "foo", "bar") + + hr1Rule1 := createBackendGroup("hr1", 1, "foo") + + hr2Rule0 := createBackendGroup("hr2", 0, "foo", "bar") + + hr2Rule1 := createBackendGroup("hr2", 1, "foo") + + hr3Rule0 := createBackendGroup("hr3", 0, "foo", "bar") + + hr3Rule1 := createBackendGroup("hr3", 1, "foo") + + hrInvalid := createBackendGroup("hr-invalid", 0, "invalid") + + invalidRoutes := map[types.NamespacedName]*route{ + {Name: "invalid", Namespace: "test"}: { + BackendGroups: []BackendGroup{hrInvalid}, + }, + } + + routes := map[types.NamespacedName]*route{ + {Name: "hr1", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr1Rule0, hr1Rule1}, + }, + {Name: "hr2", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr2Rule0, hr2Rule1}, + }, } - expName := "test_foo_9090" - if name := generateUpstreamName(backendService{Name: "foo", Namespace: "test", Port: 9090}); name != expName { - t.Errorf("generateUpstreamName() returned unexepected name: %s, expected: %s", name, expName) + routes2 := map[types.NamespacedName]*route{ + // this backend group is a dupe and should be ignored. + {Name: "hr1", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr1Rule0, hr1Rule1}, + }, + {Name: "hr3", Namespace: "test"}: { + BackendGroups: []BackendGroup{hr3Rule0, hr3Rule1}, + }, + } + + listeners := map[string]*listener{ + "invalid-listener": { + Valid: false, + Routes: invalidRoutes, // routes on invalid listener should be ignored. + }, + "listener-1": { + Valid: true, + Routes: routes, + }, + "listener-2": { + Valid: true, + Routes: routes2, + }, + } + + expGroups := []BackendGroup{ + hr1Rule0, + hr1Rule1, + hr2Rule0, + hr2Rule1, + hr3Rule0, + hr3Rule1, + } + + result := buildBackendGroups(listeners) + + sort.Slice(result, func(i, j int) bool { + return result[i].GroupName() < result[j].GroupName() + }) + + if diff := helpers.Diff(expGroups, result); diff != "" { + t.Errorf("buildBackendGroups() mismatch: %+v", diff) + } +} + +func TestBuildWarnings(t *testing.T) { + createBackendRefs := func(names ...string) []BackendRef { + backends := make([]BackendRef, len(names)) + for idx, name := range names { + backends[idx] = BackendRef{Name: name} + } + + return backends + } + + createBackendGroup := func(sourceName string, backends []BackendRef, errMsgs ...string) BackendGroup { + return BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: sourceName}, + Backends: backends, + Errors: errMsgs, + } + } + + hr1BackendGroup0 := createBackendGroup( + "hr1", + createBackendRefs("foo"), + "error1-1", "error1-2", "error1-3", + ) + + hr1BackendGroup1 := createBackendGroup( + "hr1", + createBackendRefs("bar"), + ) + + hr2BackendGroup0 := createBackendGroup( + "hr2", + createBackendRefs("foo", "bar"), + ) + + hr2BackendGroup1 := createBackendGroup( + "hr2", + createBackendRefs("resolve-error"), + "error2", + ) + + hr3BackendGroup0 := createBackendGroup( + "hr3", + createBackendRefs(""), // empty backend name should be skipped + "error3", + ) + + hr3BackendGroup1 := createBackendGroup( + "hr3", + createBackendRefs("dne"), + ) + + hrInvalidGroup := createBackendGroup( + "hr-invalid", + createBackendRefs("invalid"), + "invalid", + ) + + hr1 := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr1", Namespace: "test"}} + hr2 := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr2", Namespace: "test"}} + hr3 := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr3", Namespace: "test"}} + hrInvalid := &v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "hr-invalid", Namespace: "test"}} + + invalidRoutes := map[types.NamespacedName]*route{ + {Name: "invalid", Namespace: "test"}: { + Source: hrInvalid, + BackendGroups: []BackendGroup{hrInvalidGroup}, + }, + } + + routes := map[types.NamespacedName]*route{ + {Name: "hr1", Namespace: "test"}: { + Source: hr1, + BackendGroups: []BackendGroup{hr1BackendGroup0, hr1BackendGroup1}, + }, + {Name: "hr2", Namespace: "test"}: { + Source: hr2, + BackendGroups: []BackendGroup{hr2BackendGroup0, hr2BackendGroup1}, + }, + } + + routes2 := map[types.NamespacedName]*route{ + {Name: "hr3", Namespace: "test"}: { + Source: hr3, + BackendGroups: []BackendGroup{hr3BackendGroup0, hr3BackendGroup1}, + }, + } + + upstreamMap := map[string]Upstream{ + "foo": {}, + "bar": {}, + "resolve-error": {ErrorMsg: "resolve error"}, + } + + graph := &graph{ + Gateway: &gateway{ + Listeners: map[string]*listener{ + "invalid-listener": { + Source: v1beta1.Listener{ + Name: "invalid", + }, + Valid: false, + Routes: invalidRoutes, + }, + "listener": { + Source: v1beta1.Listener{ + Name: "valid", + }, + Valid: true, + Routes: routes, + }, + "listener2": { + Source: v1beta1.Listener{ + Name: "valid2", + }, + Valid: true, + Routes: routes2, + }, + }, + }, + } + + expWarns := Warnings{ + hr1: []string{ + "invalid backend ref: error1-1", + "invalid backend ref: error1-2", + "invalid backend ref: error1-3", + }, + hr2: []string{ + "invalid backend ref: error2", + "cannot resolve backend ref: resolve error", + }, + hr3: []string{ + "invalid backend ref: error3", + "cannot resolve backend ref; internal error: upstream dne not found in map", + }, + hrInvalid: []string{"cannot configure routes for listener invalid; listener is invalid"}, + } + + warns := buildWarnings(graph, upstreamMap) + if diff := cmp.Diff(expWarns, warns); diff != "" { + t.Errorf("buildWarnings() mismatch (-want +got):\n%s", diff) + } +} + +func TestUpstreamsMapToSlice(t *testing.T) { + fooUpstream := Upstream{ + Name: "foo", + Endpoints: []resolver.Endpoint{ + {Address: "10.0.0.0", Port: 80}, + {Address: "10.0.0.0", Port: 81}, + }, + } + + barUpstream := Upstream{ + Name: "bar", + ErrorMsg: "error", + Endpoints: nil, + } + + bazUpstream := Upstream{ + Name: "baz", + Endpoints: []resolver.Endpoint{ + {Address: "11.0.0.0", Port: 80}, + }, + } + + upstreamMap := map[string]Upstream{ + "foo": fooUpstream, + "bar": barUpstream, + "baz": bazUpstream, + } + + expUpstreams := []Upstream{ + barUpstream, + bazUpstream, + fooUpstream, + } + + upstreams := upstreamsMapToSlice(upstreamMap) + + sort.Slice(upstreams, func(i, j int) bool { + return upstreams[i].Name < upstreams[j].Name + }) + + if diff := cmp.Diff(expUpstreams, upstreams); diff != "" { + t.Errorf("upstreamMapToSlice() mismatch (-want +got):\n%s", diff) } } diff --git a/internal/state/graph.go b/internal/state/graph.go index 569725dc3a..86c2267b5c 100644 --- a/internal/state/graph.go +++ b/internal/state/graph.go @@ -1,17 +1,12 @@ package state import ( - "context" - "errors" "fmt" "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" - - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" ) // gateway represents the winning Gateway resource. @@ -26,7 +21,8 @@ type gateway struct { type route struct { // Source is the source resource of the route. // FIXME(pleshakov) - // For now, we assume that the source is only HTTPRoute. Later we can support more types - TLSRoute, TCPRoute and UDPRoute. + // For now, we assume that the source is only HTTPRoute. + // Later we can support more types - TLSRoute, TCPRoute and UDPRoute. Source *v1beta1.HTTPRoute // ValidSectionNameRefs includes the sectionNames from the parentRefs of the HTTPRoute that are valid -- i.e. @@ -34,8 +30,11 @@ type route struct { ValidSectionNameRefs map[string]struct{} // ValidSectionNameRefs includes the sectionNames from the parentRefs of the HTTPRoute that are invalid. InvalidSectionNameRefs map[string]struct{} - // BackendServices maps HTTPRouteRules to their backend Service. - BackendServices map[ruleIndex]backendService + // BackendGroups includes the backend groups of the HTTPRoute. + // There's one BackendGroup per rule in the HTTPRoute. + // The BackendGroups are stored in order of the rules. + // Ex: Source.Spec.Rules[0] -> BackendGroups[0]. + BackendGroups []BackendGroup } // gatewayClass represents the GatewayClass resource. @@ -48,26 +47,6 @@ type gatewayClass struct { ErrorMsg string } -// ruleIndex is the index of the HTTPRouteRule. -type ruleIndex int - -type backendService struct { - // Name is the name of the Kubernetes Service. - Name string - // Namespace is the namespace of the Kubernetes Service. - Namespace string - // Port is the desired port of the Kubernetes Service. - Port int32 -} - -type backend struct { - // Endpoints are the endpoints for the backend. - Endpoints []resolver.Endpoint - - // ErrorMsg explains the error when the backend is not valid - ErrorMsg string -} - // graph is a graph-like representation of Gateway API resources. type graph struct { // GatewayClass holds the GatewayClass resource. @@ -80,19 +59,15 @@ type graph struct { IgnoredGateways map[types.NamespacedName]*v1beta1.Gateway // Routes holds route resources. Routes map[types.NamespacedName]*route - // Backends holds all the backend services referenced by http routes. - Backends map[backendService]backend } // buildGraph builds a graph from a store assuming that the Gateway resource has the gwNsName namespace and name. func buildGraph( - ctx context.Context, store *store, controllerName string, gcName string, secretMemoryMgr SecretDiskMemoryManager, - serviceResolver resolver.ServiceResolver, -) (*graph, Warnings) { +) *graph { gc := buildGatewayClass(store.gc, controllerName) gw, ignoredGws := processGateways(store.gateways, gcName) @@ -107,13 +82,12 @@ func buildGraph( } } - backends, warnings := resolveBackends(ctx, routes, store.services, serviceResolver) + addBackendGroupsToRoutes(routes, store.services) g := &graph{ GatewayClass: gc, Routes: routes, IgnoredGateways: ignoredGws, - Backends: backends, } if gw != nil { @@ -123,93 +97,7 @@ func buildGraph( } } - return g, warnings -} - -func resolveBackends( - ctx context.Context, - routes map[types.NamespacedName]*route, - services map[types.NamespacedName]*v1.Service, - serviceResolver resolver.ServiceResolver, -) (map[backendService]backend, Warnings) { - warnings := newWarnings() - backends := make(map[backendService]backend) - - for _, r := range routes { - - backendServicesForRoute := make(map[ruleIndex]backendService) - - for i, rule := range r.Source.Spec.Rules { - - backendSvc, err := getBackendServiceFromRouteRule(rule, r.Source.Namespace) - backendServicesForRoute[ruleIndex(i)] = backendSvc - - if err != nil { - warnings.AddWarningf(r.Source, "invalid backend ref for rule %d: %s", i, err.Error()) - - continue - } - - if b, exists := backends[backendSvc]; exists { - if b.ErrorMsg != "" { - warnings.AddWarningf(r.Source, "cannot resolve backend ref for rule %d: %s", i, b.ErrorMsg) - } - - continue - } - - b := backend{} - - svcName := types.NamespacedName{Namespace: backendSvc.Namespace, Name: backendSvc.Name} - svc, exists := services[svcName] - - var svcErr error - - if exists { - b.Endpoints, svcErr = serviceResolver.Resolve(ctx, svc, backendSvc.Port) - } else { - svcErr = fmt.Errorf("the Service %s does not exist", svcName) - } - - if svcErr != nil { - b.ErrorMsg = svcErr.Error() - - warnings.AddWarningf(r.Source, "cannot resolve backend ref for rule %d: %s", i, b.ErrorMsg) - } - - backends[backendSvc] = b - } - - r.BackendServices = backendServicesForRoute - } - - return backends, warnings -} - -func getBackendServiceFromRouteRule(rule v1beta1.HTTPRouteRule, routeNs string) (backendService, error) { - if len(rule.BackendRefs) == 0 { - return backendService{}, errors.New("no backend refs provided") - } - - // FIXME(kate-osborn): Need to handle multiple backend refs per rule once we support the weight field. - ref := rule.BackendRefs[0] - if ref.Kind != nil && *ref.Kind != "Service" { - return backendService{}, fmt.Errorf("backend ref Kind must be Service; got %s", *ref.Kind) - } - - if ref.Namespace != nil && string(*ref.Namespace) != routeNs { - return backendService{}, fmt.Errorf("cross-namespace routing is not permitted; namespace %s does not match the HTTPRoute namespace %s", *ref.Namespace, routeNs) - } - - if ref.Port == nil { - return backendService{}, errors.New("port is missing") - } - - return backendService{ - Name: string(ref.Name), - Namespace: routeNs, - Port: int32(*ref.Port), - }, nil + return g } // processGateways determines which Gateway resource the NGINX Gateway will use (the winner) and which Gateway(s) will diff --git a/internal/state/graph_test.go b/internal/state/graph_test.go index e9d610b728..30f97ee138 100644 --- a/internal/state/graph_test.go +++ b/internal/state/graph_test.go @@ -2,9 +2,6 @@ package state import ( - "context" - "errors" - "strings" "testing" "github.com/google/go-cmp/cmp" @@ -14,8 +11,6 @@ import ( "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver/resolverfakes" ) var testSecret = &v1.Secret{ @@ -136,6 +131,38 @@ func TestBuildGraph(t *testing.T) { hr2 := createRoute("hr-2", "wrong-gateway", "listener-80-1") hr3 := createRoute("hr-3", "gateway-1", "listener-443-1") // https listener; should not conflict with hr1 + fooSvc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test"}} + + hr1Group := BackendGroup{ + Errors: []string{}, + Source: types.NamespacedName{Namespace: hr1.Namespace, Name: hr1.Name}, + RuleIdx: 0, + Backends: []BackendRef{ + { + Name: "test_foo_80", + Svc: fooSvc, + Port: 80, + Valid: true, + Weight: 1, + }, + }, + } + + hr3Group := BackendGroup{ + Errors: []string{}, + Source: types.NamespacedName{Namespace: hr3.Namespace, Name: hr3.Name}, + RuleIdx: 0, + Backends: []BackendRef{ + { + Name: "test_foo_80", + Svc: fooSvc, + Port: 80, + Valid: true, + Weight: 1, + }, + }, + } + createGateway := func(name string) *v1beta1.Gateway { return &v1beta1.Gateway{ ObjectMeta: metav1.ObjectMeta{ @@ -204,13 +231,7 @@ func TestBuildGraph(t *testing.T) { "listener-80-1": {}, }, InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): { - Name: "foo", - Namespace: "test", - Port: 80, - }, - }, + BackendGroups: []BackendGroup{hr1Group}, } routeHR3 := &route{ @@ -219,13 +240,7 @@ func TestBuildGraph(t *testing.T) { "listener-443-1": {}, }, InvalidSectionNameRefs: map[string]struct{}{}, - BackendServices: map[ruleIndex]backendService{ - ruleIndex(0): { - Name: "foo", - Namespace: "test", - Port: 80, - }, - }, + BackendGroups: []BackendGroup{hr3Group}, } // add test secret to store @@ -233,16 +248,6 @@ func TestBuildGraph(t *testing.T) { secretStore.Upsert(testSecret) secretMemoryMgr := NewSecretDiskMemoryManager(secretsDirectory, secretStore) - expFooEndpoints := []resolver.Endpoint{ - { - Address: "10.0.0.0", - Port: 8080, - }, - } - - fakeServiceResolver := &resolverfakes.FakeServiceResolver{} - fakeServiceResolver.ResolveReturns(expFooEndpoints, nil) - expected := &graph{ GatewayClass: &gatewayClass{ Source: store.gc, @@ -281,26 +286,12 @@ func TestBuildGraph(t *testing.T) { {Namespace: "test", Name: "hr-1"}: routeHR1, {Namespace: "test", Name: "hr-3"}: routeHR3, }, - Backends: map[backendService]backend{ - { - Name: "foo", - Namespace: "test", - Port: 80, - }: { - Endpoints: expFooEndpoints, - }, - }, } - expWarnings := Warnings{} - - result, warnings := buildGraph(context.TODO(), store, controllerName, gcName, secretMemoryMgr, fakeServiceResolver) + result := buildGraph(store, controllerName, gcName, secretMemoryMgr) if diff := cmp.Diff(expected, result); diff != "" { t.Errorf("buildGraph() mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff(expWarnings, warnings); diff != "" { - t.Errorf("buildGraph() mismatch on warnings (-want +got):\n%s", diff) - } } func TestProcessGateways(t *testing.T) { @@ -1134,261 +1125,3 @@ func TestValidateGatewayClass(t *testing.T) { t.Errorf("validateGatewayClass() didn't return an error") } } - -func TestGetBackendServiceFromRouteRule(t *testing.T) { - getNormalRefs := func() []v1beta1.HTTPBackendRef { - return []v1beta1.HTTPBackendRef{ - { - BackendRef: v1beta1.BackendRef{ - BackendObjectReference: v1beta1.BackendObjectReference{ - Kind: (*v1beta1.Kind)(helpers.GetStringPointer("Service")), - Name: "service1", - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), - }, - }, - }, - } - } - - getModifiedRefs := func(mod func([]v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - return mod(getNormalRefs()) - } - - testcases := []struct { - rule v1beta1.HTTPRouteRule - expSvc backendService - expErr bool - msg string - }{ - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: getNormalRefs(), - }, - expSvc: backendService{ - Name: "service1", - Namespace: "test", - Port: 80, - }, - expErr: false, - msg: "normal case", - }, - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: getModifiedRefs( - func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - refs[0].BackendRef.Namespace = nil - return refs - }, - ), - }, - expSvc: backendService{ - Name: "service1", - Namespace: "test", - Port: 80, - }, - expErr: false, - msg: "normal case with implicit namespace", - }, - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: getModifiedRefs( - func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - refs[0].BackendRef.Kind = nil - return refs - }, - ), - }, - expSvc: backendService{ - Name: "service1", - Namespace: "test", - Port: 80, - }, - expErr: false, - msg: "normal case with implicit service", - }, - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: getModifiedRefs( - func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - secondRef := refs[0].DeepCopy() - secondRef.Name = "service2" - return append(refs, *secondRef) - }, - ), - }, - expSvc: backendService{ - Name: "service1", - Namespace: "test", - Port: 80, - }, - expErr: false, - msg: "first backend ref is used", - }, - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: nil, - }, - expSvc: backendService{}, - expErr: true, - msg: "no backend refs", - }, - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: getModifiedRefs( - func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - refs[0].BackendRef.Kind = (*v1beta1.Kind)(helpers.GetStringPointer("NotService")) - return refs - }, - ), - }, - expSvc: backendService{}, - expErr: true, - msg: "not a service kind", - }, - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: getModifiedRefs( - func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - refs[0].BackendRef.Namespace = (*v1beta1.Namespace)(helpers.GetStringPointer("not-test")) - return refs - }, - ), - }, - expSvc: backendService{}, - expErr: true, - msg: "invalid namespace", - }, - { - rule: v1beta1.HTTPRouteRule{ - BackendRefs: getModifiedRefs( - func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - refs[0].BackendRef.Port = nil - return refs - }, - ), - }, - expSvc: backendService{}, - expErr: true, - msg: "missing port", - }, - } - - for _, tc := range testcases { - svc, err := getBackendServiceFromRouteRule(tc.rule, "test") - - if err != nil && !tc.expErr { - t.Errorf("getBackendServiceFromRouteRule() returned unexpected error: %v for test case: %q", err, tc.msg) - } - - if err == nil && tc.expErr { - t.Errorf("getBackendServiceFromRouteRule() did not return error for test case: %q", tc.msg) - } - - if diff := helpers.Diff(tc.expSvc, svc); diff != "" { - t.Errorf("getBackendServiceFromRouteRule() returned incorrect backend service for test case: %q, diff: %v", tc.msg, diff) - } - } -} - -func TestResolveBackends(t *testing.T) { - fakeK8sResolver := &resolverfakes.FakeServiceResolver{} - - fakeK8sResolver.ResolveCalls(func(ctx context.Context, svc *v1.Service, port int32) ([]resolver.Endpoint, error) { - if strings.Contains(svc.Name, "error") { - return nil, errors.New("resolve error") - } - - return []resolver.Endpoint{{Address: "10.0.0.0", Port: 80}}, nil - }) - - createRoute := func(name string, kind string, serviceNames ...string) *v1beta1.HTTPRoute { - hr := &v1beta1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: name, - }, - } - - hr.Spec.Rules = make([]v1beta1.HTTPRouteRule, len(serviceNames)) - - for idx, svcName := range serviceNames { - hr.Spec.Rules[idx] = v1beta1.HTTPRouteRule{ - BackendRefs: []v1beta1.HTTPBackendRef{ - { - BackendRef: v1beta1.BackendRef{ - BackendObjectReference: v1beta1.BackendObjectReference{ - Kind: (*v1beta1.Kind)(helpers.GetStringPointer(kind)), - Name: v1beta1.ObjectName(svcName), - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), - }, - }, - }, - }, - } - } - return hr - } - - hr1 := createRoute("hr1", "Service", "svc1", "svc2", "svc3") - hr2 := createRoute("hr2", "Service", "svc1", "error-svc4") - hr3 := createRoute("hr3", "Service", "dne") - hr4 := createRoute("hr4", "NotService", "not-svc") - hr5 := createRoute("hr5", "Service", "error-svc4") - - routes := map[types.NamespacedName]*route{ - {Namespace: "test", Name: "hr1"}: {Source: hr1}, - {Namespace: "test", Name: "hr2"}: {Source: hr2}, - {Namespace: "test", Name: "hr3"}: {Source: hr3}, - {Namespace: "test", Name: "hr4"}: {Source: hr4}, - {Namespace: "test", Name: "hr5"}: {Source: hr5}, - } - - svc1 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1"}} - svc2 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc2"}} - svc3 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc3"}} - svc4 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "error-svc4"}} - - services := map[types.NamespacedName]*v1.Service{ - {Namespace: "test", Name: "svc1"}: svc1, - {Namespace: "test", Name: "svc2"}: svc2, - {Namespace: "test", Name: "svc3"}: svc3, - {Namespace: "test", Name: "error-svc4"}: svc4, - } - - backendSvc1 := backendService{Name: "svc1", Namespace: "test", Port: 80} - backendSvc2 := backendService{Name: "svc2", Namespace: "test", Port: 80} - backendSvc3 := backendService{Name: "svc3", Namespace: "test", Port: 80} - backendSvc4 := backendService{Name: "error-svc4", Namespace: "test", Port: 80} - backendSvcDne := backendService{Name: "dne", Namespace: "test", Port: 80} - - expBackends := map[backendService]backend{ - backendSvc1: {Endpoints: []resolver.Endpoint{{Address: "10.0.0.0", Port: 80}}}, - backendSvc2: {Endpoints: []resolver.Endpoint{{Address: "10.0.0.0", Port: 80}}}, - backendSvc3: {Endpoints: []resolver.Endpoint{{Address: "10.0.0.0", Port: 80}}}, - backendSvc4: {Endpoints: nil, ErrorMsg: "resolve error"}, - backendSvcDne: {Endpoints: nil, ErrorMsg: "the Service test/dne does not exist"}, - } - - expWarnings := Warnings{ - hr2: []string{"cannot resolve backend ref for rule 1: resolve error"}, - hr3: []string{"cannot resolve backend ref for rule 0: the Service test/dne does not exist"}, - hr4: []string{"invalid backend ref for rule 0: backend ref Kind must be Service; got NotService"}, - hr5: []string{"cannot resolve backend ref for rule 0: resolve error"}, - } - - backends, warnings := resolveBackends(context.TODO(), routes, services, fakeK8sResolver) - - if fakeK8sResolver.ResolveCallCount() != 4 { - t.Errorf("resolveBackends() mismatch on resolve call count; expected 5, got %d", fakeK8sResolver.ResolveCallCount()) - } - - if diff := cmp.Diff(expBackends, backends); diff != "" { - t.Errorf("resolveBackends() mismatch on backends (-want +got):\n%s", diff) - } - - if diff := cmp.Diff(expWarnings, warnings); diff != "" { - t.Errorf("resolveBackends() mismatch on warnings (-want +got):\n%s", diff) - } -} diff --git a/internal/state/relationship/capturer.go b/internal/state/relationship/capturer.go index f643ae4848..31449ff880 100644 --- a/internal/state/relationship/capturer.go +++ b/internal/state/relationship/capturer.go @@ -135,26 +135,22 @@ func (c *CapturerImpl) decrementRefCount(svcName types.NamespacedName) { } } -// FIXME(pleshakov): for now, we only support a single backend reference func getBackendServiceNamesFromRoute(hr *v1beta1.HTTPRoute) map[types.NamespacedName]struct{} { svcNames := make(map[types.NamespacedName]struct{}) for _, rule := range hr.Spec.Rules { - if len(rule.BackendRefs) == 0 { - continue - } - ref := rule.BackendRefs[0].BackendRef + for _, ref := range rule.BackendRefs { + if ref.Kind != nil && *ref.Kind != "Service" { + continue + } - if ref.Kind != nil && *ref.Kind != "Service" { - continue - } + ns := hr.Namespace + if ref.Namespace != nil { + ns = string(*ref.Namespace) + } - ns := hr.Namespace - if ref.Namespace != nil { - ns = string(*ref.Namespace) + svcNames[types.NamespacedName{Namespace: ns, Name: string(ref.Name)}] = struct{}{} } - - svcNames[types.NamespacedName{Namespace: ns, Name: string(ref.Name)}] = struct{}{} } return svcNames diff --git a/internal/state/relationship/relationships_test.go b/internal/state/relationship/relationships_test.go index 079c903788..e378bb2ee9 100644 --- a/internal/state/relationship/relationships_test.go +++ b/internal/state/relationship/relationships_test.go @@ -71,6 +71,22 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { { BackendRefs: getNormalRefs("svc2"), }, + { + BackendRefs: getModifiedRefs( + "multiple-refs", + func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { + return append(refs, v1beta1.HTTPBackendRef{ + BackendRef: v1beta1.BackendRef{ + BackendObjectReference: v1beta1.BackendObjectReference{ + Kind: (*v1beta1.Kind)(helpers.GetStringPointer("Service")), + Name: "multiple-refs2", + Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), + Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), + }, + }, + }) + }), + }, }, }, } @@ -80,6 +96,8 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { {Namespace: "test", Name: "nil-namespace"}: {}, {Namespace: "not-test", Name: "diff-namespace"}: {}, {Namespace: "test", Name: "svc2"}: {}, + {Namespace: "test", Name: "multiple-refs"}: {}, + {Namespace: "test", Name: "multiple-refs2"}: {}, } names := getBackendServiceNamesFromRoute(hr) if diff := cmp.Diff(expNames, names); diff != "" { diff --git a/internal/state/upstreams.go b/internal/state/upstreams.go deleted file mode 100644 index 6d96d29491..0000000000 --- a/internal/state/upstreams.go +++ /dev/null @@ -1,35 +0,0 @@ -package state - -import ( - "fmt" - "sort" -) - -// InvalidBackendRef is the upstream name for a backend ref that is invalid. -// Invalid in this case means that a Kubernetes Service cannot be extracted from it. -const InvalidBackendRef = "invalid_backend_ref" - -func generateUpstreamName(service backendService) string { - if service.Name == "" { - return InvalidBackendRef - } - return fmt.Sprintf("%s_%s_%d", service.Namespace, service.Name, service.Port) -} - -func buildUpstreams(backends map[backendService]backend) []Upstream { - upstreams := make([]Upstream, 0, len(backends)) - - for svc, b := range backends { - upstreams = append(upstreams, Upstream{ - Name: generateUpstreamName(svc), - Endpoints: b.Endpoints, - }) - } - - // sort upstreams for test-ability - sort.Slice(upstreams, func(i, j int) bool { - return upstreams[i].Name < upstreams[j].Name - }) - - return upstreams -}