From 67fd5fca9a4dc95613deac9d9c25d37d16861755 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Thu, 20 Jun 2024 14:08:49 -0600 Subject: [PATCH 01/47] Update nginx template for TLS passthrough Problem: nginx configuration templates didn't support TLS passthrough Solution: I added a template setup fro stream servers --- .../templates/deployment.yaml | 6 + internal/mode/static/nginx/conf/nginx.conf | 4 + .../nginx/config/base_http_config_template.go | 16 +++ .../mode/static/nginx/config/generator.go | 9 ++ internal/mode/static/nginx/config/maps.go | 43 ++++++++ .../mode/static/nginx/config/maps_template.go | 16 --- .../mode/static/nginx/config/maps_test.go | 82 ++++++++++++++ .../mode/static/nginx/config/stream/config.go | 9 ++ .../static/nginx/config/stream_servers.go | 56 ++++++++++ .../nginx/config/stream_servers_template.go | 19 ++++ .../nginx/config/stream_servers_test.go | 103 ++++++++++++++++++ .../mode/static/nginx/config/upstreams.go | 18 +++ .../static/state/dataplane/configuration.go | 8 ++ internal/mode/static/state/dataplane/types.go | 12 ++ 14 files changed, 385 insertions(+), 16 deletions(-) create mode 100644 internal/mode/static/nginx/config/stream/config.go create mode 100644 internal/mode/static/nginx/config/stream_servers.go create mode 100644 internal/mode/static/nginx/config/stream_servers_template.go create mode 100644 internal/mode/static/nginx/config/stream_servers_test.go diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index 9bd5b40223..3ccbde4368 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -118,6 +118,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -153,6 +155,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -187,6 +191,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index b70e857e0d..3729b0844a 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -36,3 +36,7 @@ http { } } } + +stream { + include /etc/nginx/stream-conf.d/*.conf; +} diff --git a/internal/mode/static/nginx/config/base_http_config_template.go b/internal/mode/static/nginx/config/base_http_config_template.go index a909001ab6..e5f20e48e2 100644 --- a/internal/mode/static/nginx/config/base_http_config_template.go +++ b/internal/mode/static/nginx/config/base_http_config_template.go @@ -2,4 +2,20 @@ package config const baseHTTPTemplateText = ` {{- if .HTTP2 }}http2 on;{{ end }} + +# Set $gw_api_compliant_host variable to the value of $http_host unless $http_host is empty, then set it to the value +# of $host. We prefer $http_host because it contains the original value of the host header, which is required by the +# Gateway API. However, in an HTTP/1.0 request, it's possible that $http_host can be empty. In this case, we will use +# the value of $host. See http://nginx.org/en/docs/http/ngx_http_core_module.html#var_host. +map $http_host $gw_api_compliant_host { + '' $host; + default $http_host; +} + +# Set $connection_header variable to upgrade when the $http_upgrade header is set, otherwise, set it to close. This +# allows support for websocket connections. See https://nginx.org/en/docs/http/websocket.html. +map $http_upgrade $connection_upgrade { + default upgrade; + '' close; +} ` diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index a0509194e0..8ec3dc1ebd 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -17,6 +17,9 @@ const ( // httpFolder is the folder where NGINX HTTP configuration files are stored. httpFolder = configFolder + "/conf.d" + // streamFolder is the folder where NGINX HTTP configuration files are stored. + streamFolder = configFolder + "/stream-conf.d" + // modulesIncludesFolder is the folder where the included "load_module" file is stored. modulesIncludesFolder = configFolder + "/module-includes" @@ -29,6 +32,9 @@ const ( // httpConfigFile is the path to the configuration file with HTTP configuration. httpConfigFile = httpFolder + "/http.conf" + // streamConfigFile is the path to the configuration file with Stream configuration. + streamConfigFile = streamFolder + "/stream.conf" + // configVersionFile is the path to the config version configuration file. configVersionFile = httpFolder + "/config-version.conf" @@ -157,6 +163,9 @@ func (g GeneratorImpl) getExecuteFuncs() []executeFunc { executeSplitClients, executeMaps, executeTelemetry, + executeStreamServers, + g.executeStreamUpstreams, + executeStreamMaps, } } diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index a784390170..0ea2d42b99 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "strings" gotemplate "text/template" @@ -21,6 +22,48 @@ func executeMaps(conf dataplane.Configuration) []executeResult { return []executeResult{result} } +func executeStreamMaps(conf dataplane.Configuration) []executeResult { + maps := createStreamMaps(conf) + + result := executeResult{ + dest: streamConfigFile, + data: helpers.MustExecuteTemplate(mapsTemplate, maps), + } + + return []executeResult{result} +} + +func createStreamMaps(conf dataplane.Configuration) []*http.Map { + var maps []*http.Map + portsToMap := make(map[int32]*http.Map) + + for _, t := range conf.TLSServers { + streamMap, ok := portsToMap[t.Port] + + if !ok { + m := http.Map{ + Source: "$ssl_preread_server_name", + Variable: "$dest" + fmt.Sprint(t.Port), + Parameters: []http.MapParameter{ + { + Value: t.Hostname, + Result: "unix:/var/lib/nginx/" + t.Hostname + fmt.Sprint(t.Port) + ".sock", + }, + }, + } + maps = append(maps, &m) + portsToMap[t.Port] = &m + } else { + streamMap.Parameters = append(streamMap.Parameters, http.MapParameter{ + Value: t.Hostname, + Result: "unix:/var/lib/nginx/" + t.Hostname + fmt.Sprint(t.Port) + ".sock", + }) + } + } + + return maps +} + func buildAddHeaderMaps(servers []dataplane.VirtualServer) []http.Map { addHeaderNames := make(map[string]struct{}) diff --git a/internal/mode/static/nginx/config/maps_template.go b/internal/mode/static/nginx/config/maps_template.go index 4b4407a1a4..31a62281e8 100644 --- a/internal/mode/static/nginx/config/maps_template.go +++ b/internal/mode/static/nginx/config/maps_template.go @@ -8,20 +8,4 @@ map {{ $m.Source }} {{ $m.Variable }} { {{ end }} } {{- end }} - -# Set $gw_api_compliant_host variable to the value of $http_host unless $http_host is empty, then set it to the value -# of $host. We prefer $http_host because it contains the original value of the host header, which is required by the -# Gateway API. However, in an HTTP/1.0 request, it's possible that $http_host can be empty. In this case, we will use -# the value of $host. See http://nginx.org/en/docs/http/ngx_http_core_module.html#var_host. -map $http_host $gw_api_compliant_host { - '' $host; - default $http_host; -} - -# Set $connection_header variable to upgrade when the $http_upgrade header is set, otherwise, set it to close. This -# allows support for websocket connections. See https://nginx.org/en/docs/http/websocket.html. -map $http_upgrade $connection_upgrade { - default upgrade; - '' close; -} ` diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index e903c17151..48922fa934 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -189,3 +189,85 @@ func TestBuildAddHeaderMaps(t *testing.T) { g.Expect(maps).To(ConsistOf(expectedMap)) } + +func TestExecuteStreamMaps(t *testing.T) { + g := NewWithT(t) + conf := dataplane.Configuration{ + TLSServers: []dataplane.Layer4Server{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + UpstreamName: "backend2", + }, + }, + } + + expSubStrings := map[string]int{ + "example.com unix:/var/lib/nginx/example.com8081.sock;": 1, + "example.com unix:/var/lib/nginx/example.com8080.sock;": 1, + "cafe.example.com unix:/var/lib/nginx/cafe.example.com8080.sock;": 1, + } + + type assertion func(g *WithT, data string) + + expectedResults := map[string]assertion{ + streamConfigFile: func(g *WithT, data string) { + for expSubStr, expCount := range expSubStrings { + g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) + } + }, + } + + results := executeStreamMaps(conf) + g.Expect(results).To(HaveLen(len(expectedResults))) + + for _, res := range results { + g.Expect(expectedResults).To(HaveKey(res.dest), "executeStreamServers returned unexpected result destination") + + assertData := expectedResults[res.dest] + assertData(g, string(res.data)) + } +} + +func TestCreateStreamMaps(t *testing.T) { + g := NewWithT(t) + conf := dataplane.Configuration{ + TLSServers: []dataplane.Layer4Server{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + UpstreamName: "backend2", + }, + }, + } + + maps := createStreamMaps(conf) + g.Expect(maps).To(HaveLen(2)) + + g.Expect(maps[0].Parameters).To(HaveLen(1)) + g.Expect(maps[1].Parameters).To(HaveLen(2)) + + g.Expect(maps[0].Parameters[0].Result).To(Equal("unix:/var/lib/nginx/example.com8081.sock")) + g.Expect(maps[1].Parameters[0].Result).To(Equal("unix:/var/lib/nginx/example.com8080.sock")) + g.Expect(maps[1].Parameters[1].Result).To(Equal("unix:/var/lib/nginx/cafe.example.com8080.sock")) +} diff --git a/internal/mode/static/nginx/config/stream/config.go b/internal/mode/static/nginx/config/stream/config.go new file mode 100644 index 0000000000..59721f9055 --- /dev/null +++ b/internal/mode/static/nginx/config/stream/config.go @@ -0,0 +1,9 @@ +package stream + +// Server holds all configuration for a stream server. +type Server struct { + Listen string + Destination string + SSLPreread bool + ProxyPass bool +} diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go new file mode 100644 index 0000000000..f5498230bf --- /dev/null +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -0,0 +1,56 @@ +package config + +import ( + "fmt" + gotemplate "text/template" + + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +var streamServersTemplate = gotemplate.Must(gotemplate.New("servers").Parse(streamServersTemplateText)) + +func executeStreamServers(conf dataplane.Configuration) []executeResult { + streamServers := createStreamServers(conf) + + streamServerResult := executeResult{ + dest: streamConfigFile, + data: helpers.MustExecuteTemplate(streamServersTemplate, streamServers), + } + + result := []executeResult{ + streamServerResult, + } + + return result +} + +func createStreamServers(conf dataplane.Configuration) []stream.Server { + streamServers := make([]stream.Server, 0) + for _, t := range conf.TLSServers { + streamServers = append(streamServers, stream.Server{ + Listen: "unix:/var/lib/nginx/" + t.Hostname + fmt.Sprint(t.Port) + ".sock", + Destination: t.UpstreamName, + ProxyPass: true, + SSLPreread: false, + }) + } + + ports := make(map[int32]bool) + + for _, t := range conf.TLSServers { + if ports[t.Port] { + continue + } + ports[t.Port] = true + streamServers = append(streamServers, stream.Server{ + Listen: fmt.Sprint(t.Port), + Destination: "$dest" + fmt.Sprint(t.Port), + ProxyPass: false, + SSLPreread: true, + }) + } + + return streamServers +} diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go new file mode 100644 index 0000000000..7d80f5f1fd --- /dev/null +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -0,0 +1,19 @@ +package config + +const streamServersTemplateText = ` +{{- range $s := . }} +server { + listen {{ $s.Listen }}; + + {{- if $s.ProxyPass }} + proxy_pass {{ $s.Destination }}; + {{- else }} + pass {{ $s.Destination }}; + {{- end }} + + {{- if $s.SSLPreread }} + ssl_preread on; + {{- end }} +} +{{- end }} +` diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go new file mode 100644 index 0000000000..8e69cd5a1d --- /dev/null +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -0,0 +1,103 @@ +package config + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +func TestExecuteStreamServers(t *testing.T) { + conf := dataplane.Configuration{ + TLSServers: []dataplane.Layer4Server{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + UpstreamName: "backend2", + }, + }, + } + + expSubStrings := map[string]int{ + "pass $dest8081;": 1, + "pass $dest8080;": 1, + "ssl_preread on;": 2, + "proxy_pass": 3, + } + + type assertion func(g *WithT, data string) + + expectedResults := map[string]assertion{ + streamConfigFile: func(g *WithT, data string) { + for expSubStr, expCount := range expSubStrings { + g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) + } + }, + } + g := NewWithT(t) + + results := executeStreamServers(conf) + g.Expect(results).To(HaveLen(len(expectedResults))) + + for _, res := range results { + g.Expect(expectedResults).To(HaveKey(res.dest), "executeStreamServers returned unexpected result destination") + + assertData := expectedResults[res.dest] + assertData(g, string(res.data)) + } +} + +func TestCreateStreamServers(t *testing.T) { + conf := dataplane.Configuration{ + TLSServers: []dataplane.Layer4Server{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + UpstreamName: "backend2", + }, + }, + } + + streamServers := createStreamServers(conf) + + g := NewWithT(t) + + g.Expect(streamServers).To(HaveLen(5)) + + SSLPrereadCount := 0 + ProxyPassCount := 0 + + for _, streamServer := range streamServers { + if streamServer.SSLPreread { + SSLPrereadCount++ + } + if streamServer.ProxyPass { + ProxyPassCount++ + } + } + + g.Expect(SSLPrereadCount).To(Equal(2)) + g.Expect(ProxyPassCount).To(Equal(3)) +} diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index 2e1906234e..f982e3fee4 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -35,6 +35,24 @@ func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeR return []executeResult{result} } +func (g GeneratorImpl) executeStreamUpstreams(_ dataplane.Configuration) []executeResult { + upstreams := []http.Upstream{ + { + Name: "backend1", + Servers: []http.UpstreamServer{{ + Address: "10.244.0.7:8443", + }}, + }, + } + + result := executeResult{ + dest: streamConfigFile, + data: helpers.MustExecuteTemplate(upstreamsTemplate, upstreams), + } + + return []executeResult{result} +} + func (g GeneratorImpl) createUpstreams(upstreams []dataplane.Upstream) []http.Upstream { // capacity is the number of upstreams + 1 for the invalid backend ref upstream ups := make([]http.Upstream, 0, len(upstreams)+1) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 56569aa432..965a2dc8f8 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -48,10 +48,18 @@ func BuildConfiguration( certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) baseHTTPConfig := buildBaseHTTPConfig(g) + tlsServers := []Layer4Server{ + { + Hostname: "app.example.com", + Port: 443, + UpstreamName: "backend1", + }, + } config := Configuration{ HTTPServers: httpServers, SSLServers: sslServers, + TLSServers: tlsServers, Upstreams: upstreams, BackendGroups: backendGroups, SSLKeyPairs: keyPairs, diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 77cd66c008..b60520b481 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -29,6 +29,8 @@ type Configuration struct { HTTPServers []VirtualServer // SSLServers holds all SSLServers. SSLServers []VirtualServer + // TLSServers hold all TLSServers + TLSServers []Layer4Server // Upstreams holds all unique Upstreams. Upstreams []Upstream // BackendGroups holds all unique BackendGroups. @@ -76,6 +78,16 @@ type VirtualServer struct { IsDefault bool } +// Layer4Server is a virtual server for Layer 4 traffic. +type Layer4Server struct { + // Hostname is the hostname of the server. + Hostname string + // UpstreamName refers to the name of the upstream that is used. + UpstreamName string + // Port is the port of the server. + Port int32 +} + // Addition holds additional configuration. type Addition struct { // Identifier is a unique identifier for the addition. From 0ff40acd95a04555963e9b8e1fb436e9c8baff38 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 21 Jun 2024 12:06:46 -0600 Subject: [PATCH 02/47] add https server support and manifests made --- config/tests/static-deployment.yaml | 6 ++++ .../manifests/nginx-gateway-experimental.yaml | 6 ++++ deploy/manifests/nginx-gateway.yaml | 6 ++++ .../nginx-plus-gateway-experimental.yaml | 6 ++++ deploy/manifests/nginx-plus-gateway.yaml | 6 ++++ .../mode/static/nginx/config/http/config.go | 2 +- internal/mode/static/nginx/config/maps.go | 17 +++++++++ .../mode/static/nginx/config/maps_test.go | 9 ++++- internal/mode/static/nginx/config/servers.go | 35 ++++++++++++++----- .../static/nginx/config/servers_template.go | 8 ++--- .../mode/static/nginx/config/servers_test.go | 16 ++++----- 11 files changed, 95 insertions(+), 22 deletions(-) diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index 4544caf4ea..eff64185a6 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -70,6 +70,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -98,6 +100,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -117,6 +121,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/deploy/manifests/nginx-gateway-experimental.yaml b/deploy/manifests/nginx-gateway-experimental.yaml index 68571eff79..da66bf0ea1 100644 --- a/deploy/manifests/nginx-gateway-experimental.yaml +++ b/deploy/manifests/nginx-gateway-experimental.yaml @@ -226,6 +226,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -254,6 +256,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -273,6 +277,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 689355cbbe..0c2d21c2c6 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -222,6 +222,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -250,6 +252,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -269,6 +273,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/deploy/manifests/nginx-plus-gateway-experimental.yaml b/deploy/manifests/nginx-plus-gateway-experimental.yaml index 28e125362b..0bdb4d4764 100644 --- a/deploy/manifests/nginx-plus-gateway-experimental.yaml +++ b/deploy/manifests/nginx-plus-gateway-experimental.yaml @@ -233,6 +233,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -261,6 +263,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -280,6 +284,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/deploy/manifests/nginx-plus-gateway.yaml b/deploy/manifests/nginx-plus-gateway.yaml index 762eb5d324..5dc94e4632 100644 --- a/deploy/manifests/nginx-plus-gateway.yaml +++ b/deploy/manifests/nginx-plus-gateway.yaml @@ -229,6 +229,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -257,6 +259,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -276,6 +280,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 19e7db223d..0410b79b16 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -4,9 +4,9 @@ package http type Server struct { SSL *SSL ServerName string + Listen string Locations []Location Includes []string - Port int32 IsDefaultHTTP bool IsDefaultSSL bool GRPC bool diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 0ea2d42b99..a12225f67f 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -61,6 +61,23 @@ func createStreamMaps(conf dataplane.Configuration) []*http.Map { } } + for _, s := range conf.SSLServers { + streamMap, ok := portsToMap[s.Port] + + hostname := s.Hostname + + if s.IsDefault { + hostname = "default" + } + + if ok { + streamMap.Parameters = append(streamMap.Parameters, http.MapParameter{ + Value: hostname, + Result: "unix:/var/lib/nginx/" + s.Hostname + fmt.Sprint(s.Port) + ".sock", + }) + } + } + return maps } diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 48922fa934..f99d0edb30 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -259,15 +259,22 @@ func TestCreateStreamMaps(t *testing.T) { UpstreamName: "backend2", }, }, + SSLServers: []dataplane.VirtualServer{ + { + Hostname: "app.example.com", + Port: 8080, + }, + }, } maps := createStreamMaps(conf) g.Expect(maps).To(HaveLen(2)) g.Expect(maps[0].Parameters).To(HaveLen(1)) - g.Expect(maps[1].Parameters).To(HaveLen(2)) + g.Expect(maps[1].Parameters).To(HaveLen(3)) g.Expect(maps[0].Parameters[0].Result).To(Equal("unix:/var/lib/nginx/example.com8081.sock")) g.Expect(maps[1].Parameters[0].Result).To(Equal("unix:/var/lib/nginx/example.com8080.sock")) g.Expect(maps[1].Parameters[1].Result).To(Equal("unix:/var/lib/nginx/cafe.example.com8080.sock")) + g.Expect(maps[1].Parameters[2].Result).To(Equal("unix:/var/lib/nginx/app.example.com8080.sock")) } diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 54bbe709ed..71917469b5 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -58,7 +58,7 @@ var grpcBaseHeaders = []http.Header{ } func executeServers(conf dataplane.Configuration) []executeResult { - servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) + servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, conf.TLSServers) serverResult := executeResult{ dest: httpConfigFile, @@ -141,10 +141,19 @@ func createIncludes(additions []dataplane.Addition) []string { return includes } -func createServers(httpServers, sslServers []dataplane.VirtualServer) ([]http.Server, httpMatchPairs) { +func createServers( + httpServers, sslServers []dataplane.VirtualServer, + tlsServers []dataplane.Layer4Server, +) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) + ports := map[int32]bool{} + + for _, tlsServer := range tlsServers { + ports[tlsServer.Port] = true + } + for serverID, s := range httpServers { httpServer, matchPairs := createServer(s, serverID) servers = append(servers, httpServer) @@ -152,7 +161,7 @@ func createServers(httpServers, sslServers []dataplane.VirtualServer) ([]http.Se } for serverID, s := range sslServers { - sslServer, matchPair := createSSLServer(s, serverID) + sslServer, matchPair := createSSLServer(s, serverID, ports[s.Port]) servers = append(servers, sslServer) maps.Copy(finalMatchPairs, matchPair) } @@ -160,11 +169,19 @@ func createServers(httpServers, sslServers []dataplane.VirtualServer) ([]http.Se return servers, finalMatchPairs } -func createSSLServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) { +func createSSLServer( + virtualServer dataplane.VirtualServer, + serverID int, + useSocket bool, +) (http.Server, httpMatchPairs) { + listen := fmt.Sprint(virtualServer.Port) + if useSocket { + listen = "unix:/var/lib/nginx/" + virtualServer.Hostname + fmt.Sprint(virtualServer.Port) + ".sock" + } if virtualServer.IsDefault { return http.Server{ IsDefaultSSL: true, - Port: virtualServer.Port, + Listen: listen, }, nil } @@ -177,17 +194,19 @@ func createSSLServer(virtualServer dataplane.VirtualServer, serverID int) (http. CertificateKey: generatePEMFileName(virtualServer.SSL.KeyPairID), }, Locations: locs, - Port: virtualServer.Port, GRPC: grpc, Includes: createIncludes(virtualServer.Additions), + Listen: listen, }, matchPairs } func createServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) { + listen := fmt.Sprint(virtualServer.Port) + if virtualServer.IsDefault { return http.Server{ IsDefaultHTTP: true, - Port: virtualServer.Port, + Listen: listen, }, nil } @@ -196,7 +215,7 @@ func createServer(virtualServer dataplane.VirtualServer, serverID int) (http.Ser return http.Server{ ServerName: virtualServer.Hostname, Locations: locs, - Port: virtualServer.Port, + Listen: listen, GRPC: grpc, Includes: createIncludes(virtualServer.Additions), }, matchPairs diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 465bd6fc81..a559725866 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -5,13 +5,13 @@ js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := . -}} {{ if $s.IsDefaultSSL -}} server { - listen {{ $s.Port }} ssl default_server; + listen {{ $s.Listen }} ssl default_server; ssl_reject_handshake on; } {{- else if $s.IsDefaultHTTP }} server { - listen {{ $s.Port }} default_server; + listen {{ $s.Listen }} default_server; default_type text/html; return 404; @@ -19,7 +19,7 @@ server { {{- else }} server { {{- if $s.SSL }} - listen {{ $s.Port }} ssl; + listen {{ $s.Listen }} ssl; ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -27,7 +27,7 @@ server { return 421; } {{- else }} - listen {{ $s.Port }}; + listen {{ $s.Listen }}; {{- end }} server_name {{ $s.ServerName }}; diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index ae458e8f03..eff016e0aa 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -1024,12 +1024,12 @@ func TestCreateServers(t *testing.T) { expectedServers := []http.Server{ { IsDefaultHTTP: true, - Port: 8080, + Listen: "8080", }, { ServerName: "cafe.example.com", Locations: getExpectedLocations(false), - Port: 8080, + Listen: "8080", GRPC: true, Includes: []string{ includesFolder + "/server-addition-1.conf", @@ -1038,7 +1038,7 @@ func TestCreateServers(t *testing.T) { }, { IsDefaultSSL: true, - Port: 8443, + Listen: "8443", }, { ServerName: "cafe.example.com", @@ -1047,7 +1047,7 @@ func TestCreateServers(t *testing.T) { CertificateKey: expectedPEMPath, }, Locations: getExpectedLocations(true), - Port: 8443, + Listen: "8443", GRPC: true, Includes: []string{ includesFolder + "/server-addition-1.conf", @@ -1058,7 +1058,7 @@ func TestCreateServers(t *testing.T) { g := NewWithT(t) - result, httpMatchPair := createServers(httpServers, sslServers) + result, httpMatchPair := createServers(httpServers, sslServers, []dataplane.Layer4Server{}) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) @@ -1256,18 +1256,18 @@ func TestCreateServersConflicts(t *testing.T) { expectedServers := []http.Server{ { IsDefaultHTTP: true, - Port: 8080, + Listen: "8080", }, { ServerName: "cafe.example.com", Locations: test.expLocs, - Port: 8080, + Listen: "8080", }, } g := NewWithT(t) - result, _ := createServers(httpServers, []dataplane.VirtualServer{}) + result, _ := createServers(httpServers, []dataplane.VirtualServer{}, []dataplane.Layer4Server{}) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) }) } From 7fd449e1ae9a41d1e4fae1402eca504c0e47ff89 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 21 Jun 2024 13:22:21 -0600 Subject: [PATCH 03/47] add hash directives to stream block --- internal/mode/static/nginx/conf/nginx.conf | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index 3729b0844a..b6436561c8 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -39,4 +39,10 @@ http { stream { include /etc/nginx/stream-conf.d/*.conf; + + variables_hash_bucket_size 512; + variables_hash_max_size 1024; + + map_hash_max_size 2048; + map_hash_bucket_size 256; } From ffce60fb932c8239d33836050047202c3b623849 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal <68310924+sarthyparty@users.noreply.github.com> Date: Fri, 21 Jun 2024 13:27:10 -0600 Subject: [PATCH 04/47] Update comment in generator.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/nginx/config/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 8ec3dc1ebd..4148b93506 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -17,7 +17,7 @@ const ( // httpFolder is the folder where NGINX HTTP configuration files are stored. httpFolder = configFolder + "/conf.d" - // streamFolder is the folder where NGINX HTTP configuration files are stored. + // streamFolder is the folder where NGINX Stream configuration files are stored. streamFolder = configFolder + "/stream-conf.d" // modulesIncludesFolder is the folder where the included "load_module" file is stored. From a491af29b2236b465ebeb546ee73a82f915fb254 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 21 Jun 2024 13:54:02 -0600 Subject: [PATCH 05/47] remove pointer for map slice and fix tests --- .../mode/static/nginx/config/generator_test.go | 2 +- internal/mode/static/nginx/config/maps.go | 18 +++++++++--------- internal/mode/static/nginx/config/maps_test.go | 2 -- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index d4e6a5f841..e8e73d1a2f 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -81,7 +81,7 @@ func TestGenerate(t *testing.T) { files := generator.Generate(conf) - g.Expect(files).To(HaveLen(6)) + g.Expect(files).To(HaveLen(7)) arrange := func(i, j int) bool { return files[i].Path < files[j].Path } diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index a12225f67f..ea428cfc15 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -33,12 +33,12 @@ func executeStreamMaps(conf dataplane.Configuration) []executeResult { return []executeResult{result} } -func createStreamMaps(conf dataplane.Configuration) []*http.Map { - var maps []*http.Map - portsToMap := make(map[int32]*http.Map) +func createStreamMaps(conf dataplane.Configuration) []http.Map { + var maps []http.Map + portsToMap := make(map[int32]int) for _, t := range conf.TLSServers { - streamMap, ok := portsToMap[t.Port] + mapInd, ok := portsToMap[t.Port] if !ok { m := http.Map{ @@ -51,10 +51,10 @@ func createStreamMaps(conf dataplane.Configuration) []*http.Map { }, }, } - maps = append(maps, &m) - portsToMap[t.Port] = &m + maps = append(maps, m) + portsToMap[t.Port] = len(maps) - 1 } else { - streamMap.Parameters = append(streamMap.Parameters, http.MapParameter{ + maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ Value: t.Hostname, Result: "unix:/var/lib/nginx/" + t.Hostname + fmt.Sprint(t.Port) + ".sock", }) @@ -62,7 +62,7 @@ func createStreamMaps(conf dataplane.Configuration) []*http.Map { } for _, s := range conf.SSLServers { - streamMap, ok := portsToMap[s.Port] + mapInd, ok := portsToMap[s.Port] hostname := s.Hostname @@ -71,7 +71,7 @@ func createStreamMaps(conf dataplane.Configuration) []*http.Map { } if ok { - streamMap.Parameters = append(streamMap.Parameters, http.MapParameter{ + maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ Value: hostname, Result: "unix:/var/lib/nginx/" + s.Hostname + fmt.Sprint(s.Port) + ".sock", }) diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index f99d0edb30..1ad11a3464 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -84,8 +84,6 @@ func TestExecuteMaps(t *testing.T) { "map ${http_my_second_add_header} $my_second_add_header_header_var {": 1, "~.* ${http_my_second_add_header},;": 1, "map ${http_my_set_header} $my_set_header_header_var {": 0, - "map $http_host $gw_api_compliant_host {": 1, - "map $http_upgrade $connection_upgrade {": 1, } mapResult := executeMaps(conf) From 621b55274e7805342c6897c3de424da2c2729175 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal <68310924+sarthyparty@users.noreply.github.com> Date: Fri, 21 Jun 2024 14:00:15 -0600 Subject: [PATCH 06/47] rewrite string logic for creating Map Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/nginx/config/maps.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index ea428cfc15..1ed6e8d43f 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -43,7 +43,7 @@ func createStreamMaps(conf dataplane.Configuration) []http.Map { if !ok { m := http.Map{ Source: "$ssl_preread_server_name", - Variable: "$dest" + fmt.Sprint(t.Port), + Variable: fmt.Sprintf("dest_%d", t.Port), Parameters: []http.MapParameter{ { Value: t.Hostname, From 46b88389f9a0ec3bc513d944ac3b22a4bb5f4df7 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 21 Jun 2024 14:07:49 -0600 Subject: [PATCH 07/47] refactor loop variables to be named server --- internal/mode/static/nginx/config/maps.go | 26 +++++++++---------- .../static/nginx/config/stream_servers.go | 16 ++++++------ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 1ed6e8d43f..ca824cf4e6 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -37,43 +37,43 @@ func createStreamMaps(conf dataplane.Configuration) []http.Map { var maps []http.Map portsToMap := make(map[int32]int) - for _, t := range conf.TLSServers { - mapInd, ok := portsToMap[t.Port] + for _, server := range conf.TLSServers { + mapInd, ok := portsToMap[server.Port] if !ok { m := http.Map{ Source: "$ssl_preread_server_name", - Variable: fmt.Sprintf("dest_%d", t.Port), + Variable: fmt.Sprintf("dest_%d", server.Port), Parameters: []http.MapParameter{ { - Value: t.Hostname, - Result: "unix:/var/lib/nginx/" + t.Hostname + fmt.Sprint(t.Port) + ".sock", + Value: server.Hostname, + Result: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", }, }, } maps = append(maps, m) - portsToMap[t.Port] = len(maps) - 1 + portsToMap[server.Port] = len(maps) - 1 } else { maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ - Value: t.Hostname, - Result: "unix:/var/lib/nginx/" + t.Hostname + fmt.Sprint(t.Port) + ".sock", + Value: server.Hostname, + Result: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", }) } } - for _, s := range conf.SSLServers { - mapInd, ok := portsToMap[s.Port] + for _, server := range conf.SSLServers { + mapInd, ok := portsToMap[server.Port] - hostname := s.Hostname + hostname := server.Hostname - if s.IsDefault { + if server.IsDefault { hostname = "default" } if ok { maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ Value: hostname, - Result: "unix:/var/lib/nginx/" + s.Hostname + fmt.Sprint(s.Port) + ".sock", + Result: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", }) } } diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index f5498230bf..0665d1f751 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -28,10 +28,10 @@ func executeStreamServers(conf dataplane.Configuration) []executeResult { func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers := make([]stream.Server, 0) - for _, t := range conf.TLSServers { + for _, server := range conf.TLSServers { streamServers = append(streamServers, stream.Server{ - Listen: "unix:/var/lib/nginx/" + t.Hostname + fmt.Sprint(t.Port) + ".sock", - Destination: t.UpstreamName, + Listen: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", + Destination: server.UpstreamName, ProxyPass: true, SSLPreread: false, }) @@ -39,14 +39,14 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { ports := make(map[int32]bool) - for _, t := range conf.TLSServers { - if ports[t.Port] { + for _, server := range conf.TLSServers { + if ports[server.Port] { continue } - ports[t.Port] = true + ports[server.Port] = true streamServers = append(streamServers, stream.Server{ - Listen: fmt.Sprint(t.Port), - Destination: "$dest" + fmt.Sprint(t.Port), + Listen: fmt.Sprint(server.Port), + Destination: "$dest" + fmt.Sprint(server.Port), ProxyPass: false, SSLPreread: true, }) From 0cdcd78ab337cbe993f80b0cf9bd3ab418a1fac5 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 21 Jun 2024 14:57:38 -0600 Subject: [PATCH 08/47] add helper function to create unix socket name --- internal/mode/static/nginx/config/maps.go | 6 +++--- internal/mode/static/nginx/config/maps_test.go | 14 +++++++------- internal/mode/static/nginx/config/servers.go | 2 +- internal/mode/static/nginx/config/sockets.go | 9 +++++++++ 4 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 internal/mode/static/nginx/config/sockets.go diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index ca824cf4e6..78a584b183 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -47,7 +47,7 @@ func createStreamMaps(conf dataplane.Configuration) []http.Map { Parameters: []http.MapParameter{ { Value: server.Hostname, - Result: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", + Result: getSocketName(server.Port, server.Hostname), }, }, } @@ -56,7 +56,7 @@ func createStreamMaps(conf dataplane.Configuration) []http.Map { } else { maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ Value: server.Hostname, - Result: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", + Result: getSocketName(server.Port, server.Hostname), }) } } @@ -73,7 +73,7 @@ func createStreamMaps(conf dataplane.Configuration) []http.Map { if ok { maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ Value: hostname, - Result: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", + Result: getSocketName(server.Port, hostname), }) } } diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 1ad11a3464..0f3105da7d 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -211,9 +211,9 @@ func TestExecuteStreamMaps(t *testing.T) { } expSubStrings := map[string]int{ - "example.com unix:/var/lib/nginx/example.com8081.sock;": 1, - "example.com unix:/var/lib/nginx/example.com8080.sock;": 1, - "cafe.example.com unix:/var/lib/nginx/cafe.example.com8080.sock;": 1, + "example.com unix:/var/run/nginx/example.com8081.sock;": 1, + "example.com unix:/var/run/nginx/example.com8080.sock;": 1, + "cafe.example.com unix:/var/run/nginx/cafe.example.com8080.sock;": 1, } type assertion func(g *WithT, data string) @@ -271,8 +271,8 @@ func TestCreateStreamMaps(t *testing.T) { g.Expect(maps[0].Parameters).To(HaveLen(1)) g.Expect(maps[1].Parameters).To(HaveLen(3)) - g.Expect(maps[0].Parameters[0].Result).To(Equal("unix:/var/lib/nginx/example.com8081.sock")) - g.Expect(maps[1].Parameters[0].Result).To(Equal("unix:/var/lib/nginx/example.com8080.sock")) - g.Expect(maps[1].Parameters[1].Result).To(Equal("unix:/var/lib/nginx/cafe.example.com8080.sock")) - g.Expect(maps[1].Parameters[2].Result).To(Equal("unix:/var/lib/nginx/app.example.com8080.sock")) + g.Expect(maps[0].Parameters[0].Result).To(Equal("unix:/var/run/nginx/example.com8081.sock")) + g.Expect(maps[1].Parameters[0].Result).To(Equal("unix:/var/run/nginx/example.com8080.sock")) + g.Expect(maps[1].Parameters[1].Result).To(Equal("unix:/var/run/nginx/cafe.example.com8080.sock")) + g.Expect(maps[1].Parameters[2].Result).To(Equal("unix:/var/run/nginx/app.example.com8080.sock")) } diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 71917469b5..4d808d76dd 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -176,7 +176,7 @@ func createSSLServer( ) (http.Server, httpMatchPairs) { listen := fmt.Sprint(virtualServer.Port) if useSocket { - listen = "unix:/var/lib/nginx/" + virtualServer.Hostname + fmt.Sprint(virtualServer.Port) + ".sock" + listen = getSocketName(virtualServer.Port, virtualServer.Hostname) } if virtualServer.IsDefault { return http.Server{ diff --git a/internal/mode/static/nginx/config/sockets.go b/internal/mode/static/nginx/config/sockets.go new file mode 100644 index 0000000000..4172338258 --- /dev/null +++ b/internal/mode/static/nginx/config/sockets.go @@ -0,0 +1,9 @@ +package config + +import ( + "fmt" +) + +func getSocketName(port int32, hostname string) string { + return fmt.Sprintf("unix:/var/run/nginx/%s%d.sock", hostname, port) +} From d11c72f15c31264f733f73b88a6cb37162423f54 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 21 Jun 2024 15:44:13 -0600 Subject: [PATCH 09/47] add preallocated space for slices --- internal/mode/static/nginx/config/maps.go | 4 ++-- internal/mode/static/nginx/config/stream_servers.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 78a584b183..b9a17f21d8 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -34,8 +34,8 @@ func executeStreamMaps(conf dataplane.Configuration) []executeResult { } func createStreamMaps(conf dataplane.Configuration) []http.Map { - var maps []http.Map - portsToMap := make(map[int32]int) + maps := make([]http.Map, 0, len(conf.HTTPServers)) + portsToMap := make(map[int32]int, len(conf.HTTPServers)) for _, server := range conf.TLSServers { mapInd, ok := portsToMap[server.Port] diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 0665d1f751..43bf2ae700 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -27,7 +27,7 @@ func executeStreamServers(conf dataplane.Configuration) []executeResult { } func createStreamServers(conf dataplane.Configuration) []stream.Server { - streamServers := make([]stream.Server, 0) + streamServers := make([]stream.Server, 0, len(conf.TLSServers)*2) for _, server := range conf.TLSServers { streamServers = append(streamServers, stream.Server{ Listen: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", @@ -37,7 +37,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { }) } - ports := make(map[int32]bool) + ports := make(map[int32]bool, len(streamServers)) for _, server := range conf.TLSServers { if ports[server.Port] { From 24ad258fd998469923e4d7fb480c34c323c8bb47 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 24 Jun 2024 11:20:51 -0600 Subject: [PATCH 10/47] remove local testing setup --- internal/mode/static/nginx/config/upstreams.go | 9 +-------- internal/mode/static/state/dataplane/configuration.go | 8 +------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index f982e3fee4..3f3a316be4 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -36,14 +36,7 @@ func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeR } func (g GeneratorImpl) executeStreamUpstreams(_ dataplane.Configuration) []executeResult { - upstreams := []http.Upstream{ - { - Name: "backend1", - Servers: []http.UpstreamServer{{ - Address: "10.244.0.7:8443", - }}, - }, - } + var upstreams []http.Upstream result := executeResult{ dest: streamConfigFile, diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 965a2dc8f8..971711fa4b 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -48,13 +48,7 @@ func BuildConfiguration( certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) baseHTTPConfig := buildBaseHTTPConfig(g) - tlsServers := []Layer4Server{ - { - Hostname: "app.example.com", - Port: 443, - UpstreamName: "backend1", - }, - } + var tlsServers []Layer4Server config := Configuration{ HTTPServers: httpServers, From 31ae435578e4f40cc7db68ab969551adab362463 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 24 Jun 2024 12:30:35 -0600 Subject: [PATCH 11/47] switch map from bool to struct --- internal/mode/static/nginx/config/servers.go | 18 ++++++++++-------- .../mode/static/nginx/config/stream_servers.go | 7 ++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 4d808d76dd..bdb6995126 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -148,10 +148,10 @@ func createServers( servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) - ports := map[int32]bool{} + sharedPorts := make(map[int32]struct{}) for _, tlsServer := range tlsServers { - ports[tlsServer.Port] = true + sharedPorts[tlsServer.Port] = struct{}{} } for serverID, s := range httpServers { @@ -161,7 +161,13 @@ func createServers( } for serverID, s := range sslServers { - sslServer, matchPair := createSSLServer(s, serverID, ports[s.Port]) + listen := fmt.Sprint(s.Port) + + _, portInUse := sharedPorts[s.Port] + if portInUse { + listen = getSocketName(s.Port, s.Hostname) + } + sslServer, matchPair := createSSLServer(s, serverID, listen) servers = append(servers, sslServer) maps.Copy(finalMatchPairs, matchPair) } @@ -172,12 +178,8 @@ func createServers( func createSSLServer( virtualServer dataplane.VirtualServer, serverID int, - useSocket bool, + listen string, ) (http.Server, httpMatchPairs) { - listen := fmt.Sprint(virtualServer.Port) - if useSocket { - listen = getSocketName(virtualServer.Port, virtualServer.Hostname) - } if virtualServer.IsDefault { return http.Server{ IsDefaultSSL: true, diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 43bf2ae700..97d60b8460 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -37,13 +37,14 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { }) } - ports := make(map[int32]bool, len(streamServers)) + portSet := make(map[int32]struct{}, len(streamServers)) for _, server := range conf.TLSServers { - if ports[server.Port] { + _, inPortSet := portSet[server.Port] + if inPortSet { continue } - ports[server.Port] = true + portSet[server.Port] = struct{}{} streamServers = append(streamServers, stream.Server{ Listen: fmt.Sprint(server.Port), Destination: "$dest" + fmt.Sprint(server.Port), From 50857905f276b1899d8453742a76862b45807f32 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 24 Jun 2024 13:46:06 -0600 Subject: [PATCH 12/47] refactor tests --- internal/mode/static/nginx/config/maps.go | 3 +- .../mode/static/nginx/config/maps_test.go | 48 ++++++------- internal/mode/static/nginx/config/sockets.go | 4 ++ .../static/nginx/config/stream_servers.go | 4 +- .../nginx/config/stream_servers_test.go | 69 +++++++++++-------- 5 files changed, 72 insertions(+), 56 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index b9a17f21d8..d9816d6d94 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -1,7 +1,6 @@ package config import ( - "fmt" "strings" gotemplate "text/template" @@ -43,7 +42,7 @@ func createStreamMaps(conf dataplane.Configuration) []http.Map { if !ok { m := http.Map{ Source: "$ssl_preread_server_name", - Variable: fmt.Sprintf("dest_%d", server.Port), + Variable: getVariableName(server.Port), Parameters: []http.MapParameter{ { Value: server.Hostname, diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 0f3105da7d..f9c336ff8c 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -216,24 +216,13 @@ func TestExecuteStreamMaps(t *testing.T) { "cafe.example.com unix:/var/run/nginx/cafe.example.com8080.sock;": 1, } - type assertion func(g *WithT, data string) - - expectedResults := map[string]assertion{ - streamConfigFile: func(g *WithT, data string) { - for expSubStr, expCount := range expSubStrings { - g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) - } - }, - } - results := executeStreamMaps(conf) - g.Expect(results).To(HaveLen(len(expectedResults))) + g.Expect(results).To(HaveLen(1)) + result := results[0] - for _, res := range results { - g.Expect(expectedResults).To(HaveKey(res.dest), "executeStreamServers returned unexpected result destination") - - assertData := expectedResults[res.dest] - assertData(g, string(res.data)) + g.Expect(result.dest).To(Equal(streamConfigFile)) + for expSubStr, expCount := range expSubStrings { + g.Expect(strings.Count(string(result.data), expSubStr)).To(Equal(expCount)) } } @@ -266,13 +255,24 @@ func TestCreateStreamMaps(t *testing.T) { } maps := createStreamMaps(conf) - g.Expect(maps).To(HaveLen(2)) - g.Expect(maps[0].Parameters).To(HaveLen(1)) - g.Expect(maps[1].Parameters).To(HaveLen(3)) - - g.Expect(maps[0].Parameters[0].Result).To(Equal("unix:/var/run/nginx/example.com8081.sock")) - g.Expect(maps[1].Parameters[0].Result).To(Equal("unix:/var/run/nginx/example.com8080.sock")) - g.Expect(maps[1].Parameters[1].Result).To(Equal("unix:/var/run/nginx/cafe.example.com8080.sock")) - g.Expect(maps[1].Parameters[2].Result).To(Equal("unix:/var/run/nginx/app.example.com8080.sock")) + expectedMaps := []http.Map{ + { + Source: "$ssl_preread_server_name", + Variable: getVariableName(8081), + Parameters: []http.MapParameter{ + {Value: "example.com", Result: "unix:/var/run/nginx/example.com8081.sock"}, + }, + }, + { + Source: "$ssl_preread_server_name", + Variable: getVariableName(8080), + Parameters: []http.MapParameter{ + {Value: "example.com", Result: "unix:/var/run/nginx/example.com8080.sock"}, + {Value: "cafe.example.com", Result: "unix:/var/run/nginx/cafe.example.com8080.sock"}, + {Value: "app.example.com", Result: "unix:/var/run/nginx/app.example.com8080.sock"}, + }, + }, + } + g.Expect(maps).To(Equal(expectedMaps)) } diff --git a/internal/mode/static/nginx/config/sockets.go b/internal/mode/static/nginx/config/sockets.go index 4172338258..d62883265e 100644 --- a/internal/mode/static/nginx/config/sockets.go +++ b/internal/mode/static/nginx/config/sockets.go @@ -7,3 +7,7 @@ import ( func getSocketName(port int32, hostname string) string { return fmt.Sprintf("unix:/var/run/nginx/%s%d.sock", hostname, port) } + +func getVariableName(port int32) string { + return fmt.Sprintf("dest%d", port) +} diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 97d60b8460..179adabb71 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -30,7 +30,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers := make([]stream.Server, 0, len(conf.TLSServers)*2) for _, server := range conf.TLSServers { streamServers = append(streamServers, stream.Server{ - Listen: "unix:/var/lib/nginx/" + server.Hostname + fmt.Sprint(server.Port) + ".sock", + Listen: getSocketName(server.Port, server.Hostname), Destination: server.UpstreamName, ProxyPass: true, SSLPreread: false, @@ -47,7 +47,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { portSet[server.Port] = struct{}{} streamServers = append(streamServers, stream.Server{ Listen: fmt.Sprint(server.Port), - Destination: "$dest" + fmt.Sprint(server.Port), + Destination: getVariableName(server.Port), ProxyPass: false, SSLPreread: true, }) diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 8e69cd5a1d..93aa375050 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -1,11 +1,13 @@ package config import ( + "fmt" "strings" "testing" . "github.com/onsi/gomega" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -36,26 +38,15 @@ func TestExecuteStreamServers(t *testing.T) { "ssl_preread on;": 2, "proxy_pass": 3, } - - type assertion func(g *WithT, data string) - - expectedResults := map[string]assertion{ - streamConfigFile: func(g *WithT, data string) { - for expSubStr, expCount := range expSubStrings { - g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) - } - }, - } g := NewWithT(t) results := executeStreamServers(conf) - g.Expect(results).To(HaveLen(len(expectedResults))) - - for _, res := range results { - g.Expect(expectedResults).To(HaveKey(res.dest), "executeStreamServers returned unexpected result destination") + g.Expect(results).To(HaveLen(1)) + result := results[0] - assertData := expectedResults[res.dest] - assertData(g, string(res.data)) + g.Expect(result.dest).To(Equal(streamConfigFile)) + for expSubStr, expCount := range expSubStrings { + g.Expect(strings.Count(string(result.data), expSubStr)).To(Equal(expCount)) } } @@ -86,18 +77,40 @@ func TestCreateStreamServers(t *testing.T) { g.Expect(streamServers).To(HaveLen(5)) - SSLPrereadCount := 0 - ProxyPassCount := 0 - - for _, streamServer := range streamServers { - if streamServer.SSLPreread { - SSLPrereadCount++ - } - if streamServer.ProxyPass { - ProxyPassCount++ - } + expectedStreamServers := []stream.Server{ + { + Listen: getSocketName(conf.TLSServers[0].Port, conf.TLSServers[0].Hostname), + Destination: conf.TLSServers[0].UpstreamName, + ProxyPass: true, + SSLPreread: false, + }, + { + Listen: getSocketName(conf.TLSServers[1].Port, conf.TLSServers[1].Hostname), + Destination: conf.TLSServers[1].UpstreamName, + ProxyPass: true, + SSLPreread: false, + }, + { + Listen: getSocketName(conf.TLSServers[2].Port, conf.TLSServers[2].Hostname), + Destination: conf.TLSServers[2].UpstreamName, + ProxyPass: true, + SSLPreread: false, + }, + { + Listen: fmt.Sprint(8081), + Destination: getVariableName(8081), + ProxyPass: false, + SSLPreread: true, + }, + { + Listen: fmt.Sprint(8080), + Destination: getVariableName(8080), + ProxyPass: false, + SSLPreread: true, + }, } - g.Expect(SSLPrereadCount).To(Equal(2)) - g.Expect(ProxyPassCount).To(Equal(3)) + for i := range streamServers { + g.Expect(streamServers[i]).To(Equal(expectedStreamServers[i])) + } } From a221eddfbcff7dd22e7e35c7980359d985ae2555 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal <68310924+sarthyparty@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:51:23 -0600 Subject: [PATCH 13/47] Update gotemplate name Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/nginx/config/stream_servers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 179adabb71..01027db0cc 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -9,7 +9,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) -var streamServersTemplate = gotemplate.Must(gotemplate.New("servers").Parse(streamServersTemplateText)) +var streamServersTemplate = gotemplate.Must(gotemplate.New("streamServers").Parse(streamServersTemplateText)) func executeStreamServers(conf dataplane.Configuration) []executeResult { streamServers := createStreamServers(conf) From fcf25885457aafe9ec24597cc011990dc0015c8b Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 24 Jun 2024 14:26:01 -0600 Subject: [PATCH 14/47] swap star character for :s --- internal/mode/static/nginx/config/sockets.go | 14 +++++++++++++- internal/mode/static/nginx/config/sockets_test.go | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 internal/mode/static/nginx/config/sockets_test.go diff --git a/internal/mode/static/nginx/config/sockets.go b/internal/mode/static/nginx/config/sockets.go index d62883265e..07b4656ef0 100644 --- a/internal/mode/static/nginx/config/sockets.go +++ b/internal/mode/static/nginx/config/sockets.go @@ -2,10 +2,22 @@ package config import ( "fmt" + "strings" ) +var forbiddenChars = map[string]string{":": "::", "*": ":s"} + +// Swap forbidden characters treating ":" as an escape character +func swapCharacters(name string) string { + for old, replace := range forbiddenChars { + name = strings.Replace(name, old, replace, -1) + } + return name +} + func getSocketName(port int32, hostname string) string { - return fmt.Sprintf("unix:/var/run/nginx/%s%d.sock", hostname, port) + newName := swapCharacters(hostname) + return fmt.Sprintf("unix:/var/run/nginx/%s%d.sock", newName, port) } func getVariableName(port int32) string { diff --git a/internal/mode/static/nginx/config/sockets_test.go b/internal/mode/static/nginx/config/sockets_test.go new file mode 100644 index 0000000000..4167672667 --- /dev/null +++ b/internal/mode/static/nginx/config/sockets_test.go @@ -0,0 +1,14 @@ +package config + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestGetSocketName(t *testing.T) { + res := getSocketName(800, "*.cafe:example.com") + + g := NewGomegaWithT(t) + g.Expect(res).To(Equal("unix:/var/run/nginx/:s.cafe::example.com800.sock")) +} From efcdc5d439cc6cd0ea462d87efe0aba79590b761 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 24 Jun 2024 15:58:29 -0600 Subject: [PATCH 15/47] refactor Server type definition --- internal/mode/static/nginx/config/sockets.go | 5 ++- .../mode/static/nginx/config/stream/config.go | 8 ++-- .../static/nginx/config/stream_servers.go | 14 +++---- .../nginx/config/stream_servers_template.go | 8 ++-- .../nginx/config/stream_servers_test.go | 37 +++++++++---------- 5 files changed, 35 insertions(+), 37 deletions(-) diff --git a/internal/mode/static/nginx/config/sockets.go b/internal/mode/static/nginx/config/sockets.go index 07b4656ef0..a2604c4f69 100644 --- a/internal/mode/static/nginx/config/sockets.go +++ b/internal/mode/static/nginx/config/sockets.go @@ -5,10 +5,11 @@ import ( "strings" ) -var forbiddenChars = map[string]string{":": "::", "*": ":s"} +var forbiddenChars = map[string]string{"*": ":s"} // Swap forbidden characters treating ":" as an escape character func swapCharacters(name string) string { + name = strings.Replace(name, ":", "::", -1) for old, replace := range forbiddenChars { name = strings.Replace(name, old, replace, -1) } @@ -21,5 +22,5 @@ func getSocketName(port int32, hostname string) string { } func getVariableName(port int32) string { - return fmt.Sprintf("dest%d", port) + return fmt.Sprintf("$dest%d", port) } diff --git a/internal/mode/static/nginx/config/stream/config.go b/internal/mode/static/nginx/config/stream/config.go index 59721f9055..f01adae269 100644 --- a/internal/mode/static/nginx/config/stream/config.go +++ b/internal/mode/static/nginx/config/stream/config.go @@ -2,8 +2,8 @@ package stream // Server holds all configuration for a stream server. type Server struct { - Listen string - Destination string - SSLPreread bool - ProxyPass bool + Listen string + ProxyPass string + Pass string + SSLPreread bool } diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 01027db0cc..f6175bb620 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -30,10 +30,9 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers := make([]stream.Server, 0, len(conf.TLSServers)*2) for _, server := range conf.TLSServers { streamServers = append(streamServers, stream.Server{ - Listen: getSocketName(server.Port, server.Hostname), - Destination: server.UpstreamName, - ProxyPass: true, - SSLPreread: false, + Listen: getSocketName(server.Port, server.Hostname), + ProxyPass: server.UpstreamName, + SSLPreread: false, }) } @@ -46,10 +45,9 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { } portSet[server.Port] = struct{}{} streamServers = append(streamServers, stream.Server{ - Listen: fmt.Sprint(server.Port), - Destination: getVariableName(server.Port), - ProxyPass: false, - SSLPreread: true, + Listen: fmt.Sprint(server.Port), + Pass: getVariableName(server.Port), + SSLPreread: true, }) } diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index 7d80f5f1fd..e0e1c00ba8 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -6,9 +6,11 @@ server { listen {{ $s.Listen }}; {{- if $s.ProxyPass }} - proxy_pass {{ $s.Destination }}; - {{- else }} - pass {{ $s.Destination }}; + proxy_pass {{ $s.ProxyPass }}; + {{- end }} + + {{- if $s.Pass }} + pass {{ $s.Pass }}; {{- end }} {{- if $s.SSLPreread }} diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 93aa375050..a262a7a09b 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -44,6 +44,8 @@ func TestExecuteStreamServers(t *testing.T) { g.Expect(results).To(HaveLen(1)) result := results[0] + fmt.Println(string(result.data)) + g.Expect(result.dest).To(Equal(streamConfigFile)) for expSubStr, expCount := range expSubStrings { g.Expect(strings.Count(string(result.data), expSubStr)).To(Equal(expCount)) @@ -79,34 +81,29 @@ func TestCreateStreamServers(t *testing.T) { expectedStreamServers := []stream.Server{ { - Listen: getSocketName(conf.TLSServers[0].Port, conf.TLSServers[0].Hostname), - Destination: conf.TLSServers[0].UpstreamName, - ProxyPass: true, - SSLPreread: false, + Listen: getSocketName(conf.TLSServers[0].Port, conf.TLSServers[0].Hostname), + ProxyPass: conf.TLSServers[0].UpstreamName, + SSLPreread: false, }, { - Listen: getSocketName(conf.TLSServers[1].Port, conf.TLSServers[1].Hostname), - Destination: conf.TLSServers[1].UpstreamName, - ProxyPass: true, - SSLPreread: false, + Listen: getSocketName(conf.TLSServers[1].Port, conf.TLSServers[1].Hostname), + ProxyPass: conf.TLSServers[1].UpstreamName, + SSLPreread: false, }, { - Listen: getSocketName(conf.TLSServers[2].Port, conf.TLSServers[2].Hostname), - Destination: conf.TLSServers[2].UpstreamName, - ProxyPass: true, - SSLPreread: false, + Listen: getSocketName(conf.TLSServers[2].Port, conf.TLSServers[2].Hostname), + ProxyPass: conf.TLSServers[2].UpstreamName, + SSLPreread: false, }, { - Listen: fmt.Sprint(8081), - Destination: getVariableName(8081), - ProxyPass: false, - SSLPreread: true, + Listen: fmt.Sprint(8081), + Pass: getVariableName(8081), + SSLPreread: true, }, { - Listen: fmt.Sprint(8080), - Destination: getVariableName(8080), - ProxyPass: false, - SSLPreread: true, + Listen: fmt.Sprint(8080), + Pass: getVariableName(8080), + SSLPreread: true, }, } From a3fe0c4e6ed4f4fbdb846d962d37c6096078731f Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 24 Jun 2024 16:27:50 -0600 Subject: [PATCH 16/47] rename types --- internal/mode/static/nginx/config/maps.go | 6 ++--- .../mode/static/nginx/config/maps_test.go | 4 ++-- internal/mode/static/nginx/config/servers.go | 4 ++-- .../mode/static/nginx/config/servers_test.go | 4 ++-- .../mode/static/nginx/config/stream/config.go | 12 ++++++++++ .../static/nginx/config/stream_servers.go | 6 ++--- .../nginx/config/stream_servers_test.go | 16 +++++++------- .../mode/static/nginx/config/upstreams.go | 17 ++++++++++++-- .../static/state/dataplane/configuration.go | 22 +++++++++---------- internal/mode/static/state/dataplane/types.go | 8 +++---- 10 files changed, 62 insertions(+), 37 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index d9816d6d94..04477c0b88 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -33,10 +33,10 @@ func executeStreamMaps(conf dataplane.Configuration) []executeResult { } func createStreamMaps(conf dataplane.Configuration) []http.Map { - maps := make([]http.Map, 0, len(conf.HTTPServers)) - portsToMap := make(map[int32]int, len(conf.HTTPServers)) + maps := make([]http.Map, 0, len(conf.TLSPassthroughServers)) + portsToMap := make(map[int32]int, len(conf.TLSPassthroughServers)) - for _, server := range conf.TLSServers { + for _, server := range conf.TLSPassthroughServers { mapInd, ok := portsToMap[server.Port] if !ok { diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index f9c336ff8c..a41a4da0cb 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -191,7 +191,7 @@ func TestBuildAddHeaderMaps(t *testing.T) { func TestExecuteStreamMaps(t *testing.T) { g := NewWithT(t) conf := dataplane.Configuration{ - TLSServers: []dataplane.Layer4Server{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ { Hostname: "example.com", Port: 8081, @@ -229,7 +229,7 @@ func TestExecuteStreamMaps(t *testing.T) { func TestCreateStreamMaps(t *testing.T) { g := NewWithT(t) conf := dataplane.Configuration{ - TLSServers: []dataplane.Layer4Server{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ { Hostname: "example.com", Port: 8081, diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index bdb6995126..c20c96292c 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -58,7 +58,7 @@ var grpcBaseHeaders = []http.Header{ } func executeServers(conf dataplane.Configuration) []executeResult { - servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, conf.TLSServers) + servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, conf.TLSPassthroughServers) serverResult := executeResult{ dest: httpConfigFile, @@ -143,7 +143,7 @@ func createIncludes(additions []dataplane.Addition) []string { func createServers( httpServers, sslServers []dataplane.VirtualServer, - tlsServers []dataplane.Layer4Server, + tlsServers []dataplane.Layer4VirtualServer, ) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index eff016e0aa..d218be6161 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -1058,7 +1058,7 @@ func TestCreateServers(t *testing.T) { g := NewWithT(t) - result, httpMatchPair := createServers(httpServers, sslServers, []dataplane.Layer4Server{}) + result, httpMatchPair := createServers(httpServers, sslServers, []dataplane.Layer4VirtualServer{}) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) @@ -1267,7 +1267,7 @@ func TestCreateServersConflicts(t *testing.T) { g := NewWithT(t) - result, _ := createServers(httpServers, []dataplane.VirtualServer{}, []dataplane.Layer4Server{}) + result, _ := createServers(httpServers, []dataplane.VirtualServer{}, []dataplane.Layer4VirtualServer{}) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) }) } diff --git a/internal/mode/static/nginx/config/stream/config.go b/internal/mode/static/nginx/config/stream/config.go index f01adae269..93f16b22cc 100644 --- a/internal/mode/static/nginx/config/stream/config.go +++ b/internal/mode/static/nginx/config/stream/config.go @@ -7,3 +7,15 @@ type Server struct { Pass string SSLPreread bool } + +// Upstream holds all configuration for a stream upstream. +type Upstream struct { + Name string + ZoneSize string // format: 512k, 1m + Servers []UpstreamServer +} + +// UpstreamServer holds all configuration for a stream upstream server. +type UpstreamServer struct { + Address string +} diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index f6175bb620..a99bedb4fd 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -27,8 +27,8 @@ func executeStreamServers(conf dataplane.Configuration) []executeResult { } func createStreamServers(conf dataplane.Configuration) []stream.Server { - streamServers := make([]stream.Server, 0, len(conf.TLSServers)*2) - for _, server := range conf.TLSServers { + streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) + for _, server := range conf.TLSPassthroughServers { streamServers = append(streamServers, stream.Server{ Listen: getSocketName(server.Port, server.Hostname), ProxyPass: server.UpstreamName, @@ -38,7 +38,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { portSet := make(map[int32]struct{}, len(streamServers)) - for _, server := range conf.TLSServers { + for _, server := range conf.TLSPassthroughServers { _, inPortSet := portSet[server.Port] if inPortSet { continue diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index a262a7a09b..c687f66983 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -13,7 +13,7 @@ import ( func TestExecuteStreamServers(t *testing.T) { conf := dataplane.Configuration{ - TLSServers: []dataplane.Layer4Server{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ { Hostname: "example.com", Port: 8081, @@ -54,7 +54,7 @@ func TestExecuteStreamServers(t *testing.T) { func TestCreateStreamServers(t *testing.T) { conf := dataplane.Configuration{ - TLSServers: []dataplane.Layer4Server{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ { Hostname: "example.com", Port: 8081, @@ -81,18 +81,18 @@ func TestCreateStreamServers(t *testing.T) { expectedStreamServers := []stream.Server{ { - Listen: getSocketName(conf.TLSServers[0].Port, conf.TLSServers[0].Hostname), - ProxyPass: conf.TLSServers[0].UpstreamName, + Listen: getSocketName(conf.TLSPassthroughServers[0].Port, conf.TLSPassthroughServers[0].Hostname), + ProxyPass: conf.TLSPassthroughServers[0].UpstreamName, SSLPreread: false, }, { - Listen: getSocketName(conf.TLSServers[1].Port, conf.TLSServers[1].Hostname), - ProxyPass: conf.TLSServers[1].UpstreamName, + Listen: getSocketName(conf.TLSPassthroughServers[1].Port, conf.TLSPassthroughServers[1].Hostname), + ProxyPass: conf.TLSPassthroughServers[1].UpstreamName, SSLPreread: false, }, { - Listen: getSocketName(conf.TLSServers[2].Port, conf.TLSServers[2].Hostname), - ProxyPass: conf.TLSServers[2].UpstreamName, + Listen: getSocketName(conf.TLSPassthroughServers[2].Port, conf.TLSPassthroughServers[2].Hostname), + ProxyPass: conf.TLSPassthroughServers[2].UpstreamName, SSLPreread: false, }, { diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index 3f3a316be4..857207e1cb 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -35,8 +35,8 @@ func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeR return []executeResult{result} } -func (g GeneratorImpl) executeStreamUpstreams(_ dataplane.Configuration) []executeResult { - var upstreams []http.Upstream +func (g GeneratorImpl) executeStreamUpstreams(conf dataplane.Configuration) []executeResult { + upstreams := g.createStreamUpstreams(conf.Upstreams) result := executeResult{ dest: streamConfigFile, @@ -46,6 +46,19 @@ func (g GeneratorImpl) executeStreamUpstreams(_ dataplane.Configuration) []execu return []executeResult{result} } +func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []http.Upstream { + // capacity is the number of upstreams + 1 for the invalid backend ref upstream + ups := make([]http.Upstream, 0, len(upstreams)+1) + + for _, u := range upstreams { + ups = append(ups, g.createUpstream(u)) + } + + ups = append(ups, createInvalidBackendRefUpstream()) + + return ups +} + func (g GeneratorImpl) createUpstreams(upstreams []dataplane.Upstream) []http.Upstream { // capacity is the number of upstreams + 1 for the invalid backend ref upstream ups := make([]http.Upstream, 0, len(upstreams)+1) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 971711fa4b..44135af65e 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -48,19 +48,19 @@ func BuildConfiguration( certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) baseHTTPConfig := buildBaseHTTPConfig(g) - var tlsServers []Layer4Server + var tlsServers []Layer4VirtualServer config := Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - TLSServers: tlsServers, - Upstreams: upstreams, - BackendGroups: backendGroups, - SSLKeyPairs: keyPairs, - Version: configVersion, - CertBundles: certBundles, - Telemetry: telemetry, - BaseHTTPConfig: baseHTTPConfig, + HTTPServers: httpServers, + SSLServers: sslServers, + TLSPassthroughServers: tlsServers, + Upstreams: upstreams, + BackendGroups: backendGroups, + SSLKeyPairs: keyPairs, + Version: configVersion, + CertBundles: certBundles, + Telemetry: telemetry, + BaseHTTPConfig: baseHTTPConfig, } return config diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index b60520b481..2100ca7efd 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -29,8 +29,8 @@ type Configuration struct { HTTPServers []VirtualServer // SSLServers holds all SSLServers. SSLServers []VirtualServer - // TLSServers hold all TLSServers - TLSServers []Layer4Server + // TLSPassthroughServers hold all TLSPassthroughServers + TLSPassthroughServers []Layer4VirtualServer // Upstreams holds all unique Upstreams. Upstreams []Upstream // BackendGroups holds all unique BackendGroups. @@ -78,8 +78,8 @@ type VirtualServer struct { IsDefault bool } -// Layer4Server is a virtual server for Layer 4 traffic. -type Layer4Server struct { +// Layer4VirtualServer is a virtual server for Layer 4 traffic. +type Layer4VirtualServer struct { // Hostname is the hostname of the server. Hostname string // UpstreamName refers to the name of the upstream that is used. From d2ee81cbd84bea7e64445f79c0c341c72750e8cb Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 25 Jun 2024 12:31:40 -0600 Subject: [PATCH 17/47] add stream upstream functions --- internal/mode/static/handler.go | 2 +- .../mode/static/nginx/config/http/config.go | 13 ------ internal/mode/static/nginx/config/maps.go | 39 +++++++++-------- .../mode/static/nginx/config/maps_test.go | 15 ++++--- internal/mode/static/nginx/config/servers.go | 2 +- .../static/nginx/config/servers_template.go | 1 + .../mode/static/nginx/config/shared/config.go | 14 ++++++ internal/mode/static/nginx/config/sockets.go | 17 ++------ .../mode/static/nginx/config/sockets_test.go | 4 +- .../static/nginx/config/stream_servers.go | 2 +- .../nginx/config/stream_servers_test.go | 8 ++-- .../mode/static/nginx/config/upstreams.go | 43 ++++++++++++++++--- internal/mode/static/state/dataplane/types.go | 4 +- 13 files changed, 97 insertions(+), 67 deletions(-) create mode 100644 internal/mode/static/nginx/config/shared/config.go diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index d90edc337e..3890ddfe54 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -169,7 +169,7 @@ func newEventHandlerImpl(cfg eventHandlerConfig) *eventHandlerImpl { func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Logger, batch events.EventBatch) { start := time.Now() - logger.V(1).Info("Started processing event batch") + logger.V(1).Info("Started processing event batch hello", "name", "sarthak") defer func() { duration := time.Since(start) diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 0410b79b16..a72f0148b1 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -88,19 +88,6 @@ type SplitClientDistribution struct { Value string } -// Map defines an NGINX map. -type Map struct { - Source string - Variable string - Parameters []MapParameter -} - -// Parameter defines a Value and Result pair in a Map. -type MapParameter struct { - Value string - Result string -} - // ProxySSLVerify holds the proxied HTTPS server verification configuration. type ProxySSLVerify struct { TrustedCertificate string diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 04477c0b88..60cd9863f8 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -4,8 +4,9 @@ import ( "strings" gotemplate "text/template" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -32,36 +33,39 @@ func executeStreamMaps(conf dataplane.Configuration) []executeResult { return []executeResult{result} } -func createStreamMaps(conf dataplane.Configuration) []http.Map { - maps := make([]http.Map, 0, len(conf.TLSPassthroughServers)) - portsToMap := make(map[int32]int, len(conf.TLSPassthroughServers)) +func createStreamMaps(conf dataplane.Configuration) []shared.Map { + maps := make([]shared.Map, 0) + portsToMap := make(map[int32]int) for _, server := range conf.TLSPassthroughServers { mapInd, ok := portsToMap[server.Port] if !ok { - m := http.Map{ + m := shared.Map{ Source: "$ssl_preread_server_name", Variable: getVariableName(server.Port), - Parameters: []http.MapParameter{ + Parameters: []shared.MapParameter{ { Value: server.Hostname, - Result: getSocketName(server.Port, server.Hostname), + Result: getSocketNameTLS(server.Port, server.Hostname), }, }, } maps = append(maps, m) portsToMap[server.Port] = len(maps) - 1 } else { - maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ + maps[mapInd].Parameters = append(maps[mapInd].Parameters, shared.MapParameter{ Value: server.Hostname, - Result: getSocketName(server.Port, server.Hostname), + Result: getSocketNameTLS(server.Port, server.Hostname), }) } } + coveredPorts := make(map[int32]struct{}) + for _, server := range conf.SSLServers { mapInd, ok := portsToMap[server.Port] + _, covered := coveredPorts[server.Port] hostname := server.Hostname @@ -69,18 +73,19 @@ func createStreamMaps(conf dataplane.Configuration) []http.Map { hostname = "default" } - if ok { - maps[mapInd].Parameters = append(maps[mapInd].Parameters, http.MapParameter{ + if ok && !covered { + maps[mapInd].Parameters = append(maps[mapInd].Parameters, shared.MapParameter{ Value: hostname, - Result: getSocketName(server.Port, hostname), + Result: getSocketNameHTTPS(server.Port), }) + coveredPorts[server.Port] = struct{}{} } } return maps } -func buildAddHeaderMaps(servers []dataplane.VirtualServer) []http.Map { +func buildAddHeaderMaps(servers []dataplane.VirtualServer) []shared.Map { addHeaderNames := make(map[string]struct{}) for _, s := range servers { @@ -98,7 +103,7 @@ func buildAddHeaderMaps(servers []dataplane.VirtualServer) []http.Map { } } - maps := make([]http.Map, 0, len(addHeaderNames)) + maps := make([]shared.Map, 0, len(addHeaderNames)) for m := range addHeaderNames { maps = append(maps, createAddHeadersMap(m)) } @@ -111,11 +116,11 @@ const ( anyStringFmt = `~.*` ) -func createAddHeadersMap(name string) http.Map { +func createAddHeadersMap(name string) shared.Map { underscoreName := convertStringToSafeVariableName(name) httpVarSource := "${http_" + underscoreName + "}" mapVarName := generateAddHeaderMapVariableName(name) - params := []http.MapParameter{ + params := []shared.MapParameter{ { Value: "default", Result: "''", @@ -125,7 +130,7 @@ func createAddHeadersMap(name string) http.Map { Result: httpVarSource + ",", }, } - return http.Map{ + return shared.Map{ Source: httpVarSource, Variable: "$" + mapVarName, Parameters: params, diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index a41a4da0cb..bd6ff36979 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -4,9 +4,10 @@ import ( "strings" "testing" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" + . "github.com/onsi/gomega" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -159,11 +160,11 @@ func TestBuildAddHeaderMaps(t *testing.T) { IsDefault: true, }, } - expectedMap := []http.Map{ + expectedMap := []shared.Map{ { Source: "${http_my_add_header}", Variable: "$my_add_header_header_var", - Parameters: []http.MapParameter{ + Parameters: []shared.MapParameter{ {Value: "default", Result: "''"}, { Value: "~.*", @@ -174,7 +175,7 @@ func TestBuildAddHeaderMaps(t *testing.T) { { Source: "${http_my_second_add_header}", Variable: "$my_second_add_header_header_var", - Parameters: []http.MapParameter{ + Parameters: []shared.MapParameter{ {Value: "default", Result: "''"}, { Value: "~.*", @@ -256,18 +257,18 @@ func TestCreateStreamMaps(t *testing.T) { maps := createStreamMaps(conf) - expectedMaps := []http.Map{ + expectedMaps := []shared.Map{ { Source: "$ssl_preread_server_name", Variable: getVariableName(8081), - Parameters: []http.MapParameter{ + Parameters: []shared.MapParameter{ {Value: "example.com", Result: "unix:/var/run/nginx/example.com8081.sock"}, }, }, { Source: "$ssl_preread_server_name", Variable: getVariableName(8080), - Parameters: []http.MapParameter{ + Parameters: []shared.MapParameter{ {Value: "example.com", Result: "unix:/var/run/nginx/example.com8080.sock"}, {Value: "cafe.example.com", Result: "unix:/var/run/nginx/cafe.example.com8080.sock"}, {Value: "app.example.com", Result: "unix:/var/run/nginx/app.example.com8080.sock"}, diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index c20c96292c..a8c1f8a681 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -165,7 +165,7 @@ func createServers( _, portInUse := sharedPorts[s.Port] if portInUse { - listen = getSocketName(s.Port, s.Hostname) + listen = getSocketNameTLS(s.Port, s.Hostname) } sslServer, matchPair := createSSLServer(s, serverID, listen) servers = append(servers, sslServer) diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index a559725866..b357d68c63 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -4,6 +4,7 @@ const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := . -}} {{ if $s.IsDefaultSSL -}} +# this is a comment server { listen {{ $s.Listen }} ssl default_server; diff --git a/internal/mode/static/nginx/config/shared/config.go b/internal/mode/static/nginx/config/shared/config.go new file mode 100644 index 0000000000..659d890464 --- /dev/null +++ b/internal/mode/static/nginx/config/shared/config.go @@ -0,0 +1,14 @@ +package shared + +// Map defines an NGINX map. +type Map struct { + Source string + Variable string + Parameters []MapParameter +} + +// MapParameter Parameter defines a Value and Result pair in a Map. +type MapParameter struct { + Value string + Result string +} diff --git a/internal/mode/static/nginx/config/sockets.go b/internal/mode/static/nginx/config/sockets.go index a2604c4f69..5382c1f8d6 100644 --- a/internal/mode/static/nginx/config/sockets.go +++ b/internal/mode/static/nginx/config/sockets.go @@ -2,23 +2,14 @@ package config import ( "fmt" - "strings" ) -var forbiddenChars = map[string]string{"*": ":s"} - -// Swap forbidden characters treating ":" as an escape character -func swapCharacters(name string) string { - name = strings.Replace(name, ":", "::", -1) - for old, replace := range forbiddenChars { - name = strings.Replace(name, old, replace, -1) - } - return name +func getSocketNameTLS(port int32, hostname string) string { + return fmt.Sprintf("unix:/var/run/nginx/%s%d.sock", hostname, port) } -func getSocketName(port int32, hostname string) string { - newName := swapCharacters(hostname) - return fmt.Sprintf("unix:/var/run/nginx/%s%d.sock", newName, port) +func getSocketNameHTTPS(port int32) string { + return fmt.Sprintf("unix:/var/run/nginx/https%d.sock", port) } func getVariableName(port int32) string { diff --git a/internal/mode/static/nginx/config/sockets_test.go b/internal/mode/static/nginx/config/sockets_test.go index 4167672667..bc2ff5643e 100644 --- a/internal/mode/static/nginx/config/sockets_test.go +++ b/internal/mode/static/nginx/config/sockets_test.go @@ -7,8 +7,8 @@ import ( ) func TestGetSocketName(t *testing.T) { - res := getSocketName(800, "*.cafe:example.com") + res := getSocketNameTLS(800, "*.cafe.example.com") g := NewGomegaWithT(t) - g.Expect(res).To(Equal("unix:/var/run/nginx/:s.cafe::example.com800.sock")) + g.Expect(res).To(Equal("unix:/var/run/nginx/*.cafe.example.com800.sock")) } diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index a99bedb4fd..07416ebced 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -30,7 +30,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) for _, server := range conf.TLSPassthroughServers { streamServers = append(streamServers, stream.Server{ - Listen: getSocketName(server.Port, server.Hostname), + Listen: getSocketNameTLS(server.Port, server.Hostname), ProxyPass: server.UpstreamName, SSLPreread: false, }) diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index c687f66983..8a8dfeee5c 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -44,8 +44,6 @@ func TestExecuteStreamServers(t *testing.T) { g.Expect(results).To(HaveLen(1)) result := results[0] - fmt.Println(string(result.data)) - g.Expect(result.dest).To(Equal(streamConfigFile)) for expSubStr, expCount := range expSubStrings { g.Expect(strings.Count(string(result.data), expSubStr)).To(Equal(expCount)) @@ -81,17 +79,17 @@ func TestCreateStreamServers(t *testing.T) { expectedStreamServers := []stream.Server{ { - Listen: getSocketName(conf.TLSPassthroughServers[0].Port, conf.TLSPassthroughServers[0].Hostname), + Listen: getSocketNameTLS(conf.TLSPassthroughServers[0].Port, conf.TLSPassthroughServers[0].Hostname), ProxyPass: conf.TLSPassthroughServers[0].UpstreamName, SSLPreread: false, }, { - Listen: getSocketName(conf.TLSPassthroughServers[1].Port, conf.TLSPassthroughServers[1].Hostname), + Listen: getSocketNameTLS(conf.TLSPassthroughServers[1].Port, conf.TLSPassthroughServers[1].Hostname), ProxyPass: conf.TLSPassthroughServers[1].UpstreamName, SSLPreread: false, }, { - Listen: getSocketName(conf.TLSPassthroughServers[2].Port, conf.TLSPassthroughServers[2].Hostname), + Listen: getSocketNameTLS(conf.TLSPassthroughServers[2].Port, conf.TLSPassthroughServers[2].Hostname), ProxyPass: conf.TLSPassthroughServers[2].UpstreamName, SSLPreread: false, }, diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index 857207e1cb..a1bcbd842c 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -6,6 +6,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -36,7 +37,7 @@ func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeR } func (g GeneratorImpl) executeStreamUpstreams(conf dataplane.Configuration) []executeResult { - upstreams := g.createStreamUpstreams(conf.Upstreams) + upstreams := g.createStreamUpstreams(conf.StreamUpstreams) result := executeResult{ dest: streamConfigFile, @@ -46,19 +47,49 @@ func (g GeneratorImpl) executeStreamUpstreams(conf dataplane.Configuration) []ex return []executeResult{result} } -func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []http.Upstream { +func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []stream.Upstream { // capacity is the number of upstreams + 1 for the invalid backend ref upstream - ups := make([]http.Upstream, 0, len(upstreams)+1) + ups := make([]stream.Upstream, 0, len(upstreams)+1) for _, u := range upstreams { - ups = append(ups, g.createUpstream(u)) + ups = append(ups, g.createStreamUpstream(u)) } - ups = append(ups, createInvalidBackendRefUpstream()) - return ups } +func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstream { + zoneSize := ossZoneSize + if g.plus { + zoneSize = plusZoneSize + } + + if len(up.Endpoints) == 0 { + return stream.Upstream{ + Name: up.Name, + ZoneSize: zoneSize, + Servers: []stream.UpstreamServer{ + { + Address: nginx502Server, + }, + }, + } + } + + upstreamServers := make([]stream.UpstreamServer, len(up.Endpoints)) + for idx, ep := range up.Endpoints { + upstreamServers[idx] = stream.UpstreamServer{ + Address: fmt.Sprintf("%s:%d", ep.Address, ep.Port), + } + } + + return stream.Upstream{ + Name: up.Name, + ZoneSize: zoneSize, + Servers: upstreamServers, + } +} + func (g GeneratorImpl) createUpstreams(upstreams []dataplane.Upstream) []http.Upstream { // capacity is the number of upstreams + 1 for the invalid backend ref upstream ups := make([]http.Upstream, 0, len(upstreams)+1) diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 2100ca7efd..3176477c2a 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -31,8 +31,10 @@ type Configuration struct { SSLServers []VirtualServer // TLSPassthroughServers hold all TLSPassthroughServers TLSPassthroughServers []Layer4VirtualServer - // Upstreams holds all unique Upstreams. + // Upstreams holds all unique http Upstreams. Upstreams []Upstream + // StreamUpstreams holds all unique stream Upstreams + StreamUpstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup // Telemetry holds the Otel configuration. From 8238431b598b9af86461dcf540694be1f472a609 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 26 Jun 2024 12:34:02 -0600 Subject: [PATCH 18/47] add https socket --- internal/mode/static/nginx/conf/nginx.conf | 4 +-- internal/mode/static/nginx/config/maps.go | 7 ++---- .../mode/static/nginx/config/maps_template.go | 5 ++++ internal/mode/static/nginx/config/servers.go | 2 +- .../mode/static/nginx/config/shared/config.go | 7 +++--- .../static/nginx/config/stream_servers.go | 7 +++--- .../static/state/dataplane/configuration.go | 25 ++++++++++++++++--- 7 files changed, 40 insertions(+), 17 deletions(-) diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index b6436561c8..3584222e23 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -43,6 +43,6 @@ stream { variables_hash_bucket_size 512; variables_hash_max_size 1024; - map_hash_max_size 2048; - map_hash_bucket_size 256; +# map_hash_max_size 2048; +# map_hash_bucket_size 256; } diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 60cd9863f8..e0e1f84779 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -50,6 +50,7 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { Result: getSocketNameTLS(server.Port, server.Hostname), }, }, + UseHostnames: true, } maps = append(maps, m) portsToMap[server.Port] = len(maps) - 1 @@ -61,11 +62,8 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { } } - coveredPorts := make(map[int32]struct{}) - for _, server := range conf.SSLServers { mapInd, ok := portsToMap[server.Port] - _, covered := coveredPorts[server.Port] hostname := server.Hostname @@ -73,12 +71,11 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { hostname = "default" } - if ok && !covered { + if ok { maps[mapInd].Parameters = append(maps[mapInd].Parameters, shared.MapParameter{ Value: hostname, Result: getSocketNameHTTPS(server.Port), }) - coveredPorts[server.Port] = struct{}{} } } diff --git a/internal/mode/static/nginx/config/maps_template.go b/internal/mode/static/nginx/config/maps_template.go index 31a62281e8..5ea0fd5353 100644 --- a/internal/mode/static/nginx/config/maps_template.go +++ b/internal/mode/static/nginx/config/maps_template.go @@ -3,6 +3,11 @@ package config const mapsTemplateText = ` {{ range $m := . }} map {{ $m.Source }} {{ $m.Variable }} { + + {{- if $m.UseHostnames -}} + hostnames; + {{ end }} + {{ range $p := $m.Parameters }} {{ $p.Value }} {{ $p.Result }}; {{ end }} diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index a8c1f8a681..bbf82d1706 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -165,7 +165,7 @@ func createServers( _, portInUse := sharedPorts[s.Port] if portInUse { - listen = getSocketNameTLS(s.Port, s.Hostname) + listen = getSocketNameHTTPS(s.Port) } sslServer, matchPair := createSSLServer(s, serverID, listen) servers = append(servers, sslServer) diff --git a/internal/mode/static/nginx/config/shared/config.go b/internal/mode/static/nginx/config/shared/config.go index 659d890464..d0091dcc1f 100644 --- a/internal/mode/static/nginx/config/shared/config.go +++ b/internal/mode/static/nginx/config/shared/config.go @@ -2,9 +2,10 @@ package shared // Map defines an NGINX map. type Map struct { - Source string - Variable string - Parameters []MapParameter + Source string + Variable string + Parameters []MapParameter + UseHostnames bool } // MapParameter Parameter defines a Value and Result pair in a Map. diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 07416ebced..ffc94acc93 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -29,11 +29,12 @@ func executeStreamServers(conf dataplane.Configuration) []executeResult { func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) for _, server := range conf.TLSPassthroughServers { + listen := getSocketNameTLS(server.Port, server.Hostname) streamServers = append(streamServers, stream.Server{ - Listen: getSocketNameTLS(server.Port, server.Hostname), - ProxyPass: server.UpstreamName, - SSLPreread: false, + Listen: listen, + ProxyPass: server.UpstreamName, }) + } portSet := make(map[int32]struct{}, len(streamServers)) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 44135af65e..20ad2ec8c1 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -29,7 +29,7 @@ const ( func BuildConfiguration( ctx context.Context, g *graph.Graph, - resolver resolver.ServiceResolver, + serviceResolver resolver.ServiceResolver, generator policies.ConfigGenerator, configVersion int, ) Configuration { @@ -41,20 +41,39 @@ func BuildConfiguration( return Configuration{Version: configVersion} } - upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver) + upstreams := buildUpstreams(ctx, g.Gateway.Listeners, serviceResolver) httpServers, sslServers := buildServers(g, generator) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners) certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) baseHTTPConfig := buildBaseHTTPConfig(g) - var tlsServers []Layer4VirtualServer + tlsServers := []Layer4VirtualServer{ + { + Hostname: "app.example.com", + UpstreamName: "backend1", + Port: 443, + }, + } + streamUpstreams := []Upstream{ + { + Name: "backend1", + ErrorMsg: "error", + Endpoints: []resolver.Endpoint{ + { + Address: "10.244.0.7", + Port: 8443, + }, + }, + }, + } config := Configuration{ HTTPServers: httpServers, SSLServers: sslServers, TLSPassthroughServers: tlsServers, Upstreams: upstreams, + StreamUpstreams: streamUpstreams, BackendGroups: backendGroups, SSLKeyPairs: keyPairs, Version: configVersion, From ec359e58323d059e6b11203d924324c40b2a7b60 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 26 Jun 2024 14:26:57 -0600 Subject: [PATCH 19/47] fix tests --- internal/mode/static/nginx/conf/nginx.conf | 8 ++++---- internal/mode/static/nginx/config/maps_test.go | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index 3584222e23..3ebeeeacba 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -38,11 +38,11 @@ http { } stream { - include /etc/nginx/stream-conf.d/*.conf; - variables_hash_bucket_size 512; variables_hash_max_size 1024; -# map_hash_max_size 2048; -# map_hash_bucket_size 256; + map_hash_max_size 2048; + map_hash_bucket_size 256; + + include /etc/nginx/stream-conf.d/*.conf; } diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index bd6ff36979..7a318260f5 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -264,6 +264,7 @@ func TestCreateStreamMaps(t *testing.T) { Parameters: []shared.MapParameter{ {Value: "example.com", Result: "unix:/var/run/nginx/example.com8081.sock"}, }, + UseHostnames: true, }, { Source: "$ssl_preread_server_name", @@ -271,8 +272,9 @@ func TestCreateStreamMaps(t *testing.T) { Parameters: []shared.MapParameter{ {Value: "example.com", Result: "unix:/var/run/nginx/example.com8080.sock"}, {Value: "cafe.example.com", Result: "unix:/var/run/nginx/cafe.example.com8080.sock"}, - {Value: "app.example.com", Result: "unix:/var/run/nginx/app.example.com8080.sock"}, + {Value: "app.example.com", Result: "unix:/var/run/nginx/https8080.sock"}, }, + UseHostnames: true, }, } g.Expect(maps).To(Equal(expectedMaps)) From b42508c0cb486d7599be0b03dbbfc1f6a4ae6c05 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 26 Jun 2024 14:32:04 -0600 Subject: [PATCH 20/47] add socket name test --- internal/mode/static/nginx/config/sockets_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/sockets_test.go b/internal/mode/static/nginx/config/sockets_test.go index bc2ff5643e..ac09c836e6 100644 --- a/internal/mode/static/nginx/config/sockets_test.go +++ b/internal/mode/static/nginx/config/sockets_test.go @@ -6,9 +6,16 @@ import ( . "github.com/onsi/gomega" ) -func TestGetSocketName(t *testing.T) { +func TestGetSocketNameTLS(t *testing.T) { res := getSocketNameTLS(800, "*.cafe.example.com") g := NewGomegaWithT(t) g.Expect(res).To(Equal("unix:/var/run/nginx/*.cafe.example.com800.sock")) } + +func TestGetSocketNameHTTPS(t *testing.T) { + res := getSocketNameHTTPS(800) + + g := NewGomegaWithT(t) + g.Expect(res).To(Equal("unix:/var/run/nginx/https800.sock")) +} From ef0100520b2842c860e4f4d68a63fb751c276200 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 26 Jun 2024 14:35:07 -0600 Subject: [PATCH 21/47] remove local testing setup --- .../static/state/dataplane/configuration.go | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 20ad2ec8c1..b82603a1d7 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -48,25 +48,8 @@ func BuildConfiguration( certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) baseHTTPConfig := buildBaseHTTPConfig(g) - tlsServers := []Layer4VirtualServer{ - { - Hostname: "app.example.com", - UpstreamName: "backend1", - Port: 443, - }, - } - streamUpstreams := []Upstream{ - { - Name: "backend1", - ErrorMsg: "error", - Endpoints: []resolver.Endpoint{ - { - Address: "10.244.0.7", - Port: 8443, - }, - }, - }, - } + var tlsServers []Layer4VirtualServer + var streamUpstreams []Upstream config := Configuration{ HTTPServers: httpServers, From 9c1babe2026d347dff19c04de68da63894fb95c8 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 26 Jun 2024 14:56:44 -0600 Subject: [PATCH 22/47] update generator test --- internal/mode/static/nginx/config/generator_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index e8e73d1a2f..93dbeadc5a 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -127,4 +127,7 @@ func TestGenerate(t *testing.T) { Path: "/etc/nginx/secrets/test-keypair.pem", Content: []byte("test-cert\ntest-key"), })) + + g.Expect(files[6].Path).To(Equal("/etc/nginx/stream-conf.d/stream.conf")) + g.Expect(files[6].Type).To(Equal(file.TypeRegular)) } From 4a0b72b08e0c039ae44bec136c8d4352a774ddd2 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 26 Jun 2024 15:33:46 -0600 Subject: [PATCH 23/47] reset log to original --- internal/mode/static/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 3890ddfe54..d90edc337e 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -169,7 +169,7 @@ func newEventHandlerImpl(cfg eventHandlerConfig) *eventHandlerImpl { func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Logger, batch events.EventBatch) { start := time.Now() - logger.V(1).Info("Started processing event batch hello", "name", "sarthak") + logger.V(1).Info("Started processing event batch") defer func() { duration := time.Since(start) From de308a53c91ee923d233e74fc7560686b5172f16 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Thu, 27 Jun 2024 11:40:17 -0600 Subject: [PATCH 24/47] move args to separate line --- internal/mode/static/nginx/config/servers.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index bbf82d1706..7b7baccea0 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -142,7 +142,8 @@ func createIncludes(additions []dataplane.Addition) []string { } func createServers( - httpServers, sslServers []dataplane.VirtualServer, + httpServers, + sslServers []dataplane.VirtualServer, tlsServers []dataplane.Layer4VirtualServer, ) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) From f1098720b9c43496e5f98872318875079ca84776 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Thu, 27 Jun 2024 15:23:30 -0600 Subject: [PATCH 25/47] add test for stream conf content --- .../static/nginx/config/generator_test.go | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index 93dbeadc5a..7262871605 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -47,12 +47,25 @@ func TestGenerate(t *testing.T) { Port: 443, }, }, + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ + { + Hostname: "app.example.com", + Port: 443, + UpstreamName: "stream_up", + }, + }, Upstreams: []dataplane.Upstream{ { Name: "up", Endpoints: nil, }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "stream_up", + Endpoints: nil, + }, + }, BackendGroups: []dataplane.BackendGroup{bg}, SSLKeyPairs: map[dataplane.SSLKeyPairID]dataplane.SSLKeyPair{ "test-keypair": { @@ -98,7 +111,7 @@ func TestGenerate(t *testing.T) { // Note: this 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. g.Expect(httpCfg).To(ContainSubstring("listen 80")) - g.Expect(httpCfg).To(ContainSubstring("listen 443")) + g.Expect(httpCfg).To(ContainSubstring("listen unix:/var/run/nginx/https443.sock")) g.Expect(httpCfg).To(ContainSubstring("upstream")) g.Expect(httpCfg).To(ContainSubstring("split_clients")) @@ -130,4 +143,9 @@ func TestGenerate(t *testing.T) { g.Expect(files[6].Path).To(Equal("/etc/nginx/stream-conf.d/stream.conf")) g.Expect(files[6].Type).To(Equal(file.TypeRegular)) + streamCfg := string(files[6].Content) + g.Expect(streamCfg).To(ContainSubstring("listen unix:/var/run/nginx/app.example.com443.sock")) + g.Expect(streamCfg).To(ContainSubstring("listen 443")) + g.Expect(streamCfg).To(ContainSubstring("app.example.com unix:/var/run/nginx/app.example.com443.sock")) + g.Expect(streamCfg).To(ContainSubstring("example.com unix:/var/run/nginx/https443.sock")) } From 73f9b089ba2d2be124b2efe7b8c27db09a01c3b0 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Thu, 27 Jun 2024 15:46:57 -0600 Subject: [PATCH 26/47] refactor create stream maps to use map instead of slice --- internal/mode/static/nginx/config/maps.go | 38 +++++++++++++---------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index e0e1f84779..d8e15c0332 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -34,36 +34,34 @@ func executeStreamMaps(conf dataplane.Configuration) []executeResult { } func createStreamMaps(conf dataplane.Configuration) []shared.Map { - maps := make([]shared.Map, 0) - portsToMap := make(map[int32]int) + portsToMap := make(map[int32]shared.Map) for _, server := range conf.TLSPassthroughServers { - mapInd, ok := portsToMap[server.Port] + streamMap, portInUse := portsToMap[server.Port] - if !ok { + mapParam := shared.MapParameter{ + Value: server.Hostname, + Result: getSocketNameTLS(server.Port, server.Hostname), + } + + if !portInUse { m := shared.Map{ Source: "$ssl_preread_server_name", Variable: getVariableName(server.Port), Parameters: []shared.MapParameter{ - { - Value: server.Hostname, - Result: getSocketNameTLS(server.Port, server.Hostname), - }, + mapParam, }, UseHostnames: true, } - maps = append(maps, m) - portsToMap[server.Port] = len(maps) - 1 + portsToMap[server.Port] = m } else { - maps[mapInd].Parameters = append(maps[mapInd].Parameters, shared.MapParameter{ - Value: server.Hostname, - Result: getSocketNameTLS(server.Port, server.Hostname), - }) + streamMap.Parameters = append(streamMap.Parameters, mapParam) + portsToMap[server.Port] = streamMap } } for _, server := range conf.SSLServers { - mapInd, ok := portsToMap[server.Port] + streamMap, portInUse := portsToMap[server.Port] hostname := server.Hostname @@ -71,14 +69,20 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { hostname = "default" } - if ok { - maps[mapInd].Parameters = append(maps[mapInd].Parameters, shared.MapParameter{ + if portInUse { + streamMap.Parameters = append(streamMap.Parameters, shared.MapParameter{ Value: hostname, Result: getSocketNameHTTPS(server.Port), }) } } + maps := make([]shared.Map, 0, len(portsToMap)) + + for _, m := range portsToMap { + maps = append(maps, m) + } + return maps } From b331e6fd07a0d1a115fc70ae5f0205b374b9e66c Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Thu, 27 Jun 2024 15:50:34 -0600 Subject: [PATCH 27/47] rename variable name gen func --- internal/mode/static/nginx/config/maps.go | 3 ++- internal/mode/static/nginx/config/maps_test.go | 4 ++-- internal/mode/static/nginx/config/sockets.go | 2 +- internal/mode/static/nginx/config/stream_servers.go | 2 +- internal/mode/static/nginx/config/stream_servers_test.go | 4 ++-- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index d8e15c0332..d6544df45b 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -47,7 +47,7 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { if !portInUse { m := shared.Map{ Source: "$ssl_preread_server_name", - Variable: getVariableName(server.Port), + Variable: getTLSPassthroughVarName(server.Port), Parameters: []shared.MapParameter{ mapParam, }, @@ -74,6 +74,7 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { Value: hostname, Result: getSocketNameHTTPS(server.Port), }) + portsToMap[server.Port] = streamMap } } diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 7a318260f5..c120062533 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -260,7 +260,7 @@ func TestCreateStreamMaps(t *testing.T) { expectedMaps := []shared.Map{ { Source: "$ssl_preread_server_name", - Variable: getVariableName(8081), + Variable: getTLSPassthroughVarName(8081), Parameters: []shared.MapParameter{ {Value: "example.com", Result: "unix:/var/run/nginx/example.com8081.sock"}, }, @@ -268,7 +268,7 @@ func TestCreateStreamMaps(t *testing.T) { }, { Source: "$ssl_preread_server_name", - Variable: getVariableName(8080), + Variable: getTLSPassthroughVarName(8080), Parameters: []shared.MapParameter{ {Value: "example.com", Result: "unix:/var/run/nginx/example.com8080.sock"}, {Value: "cafe.example.com", Result: "unix:/var/run/nginx/cafe.example.com8080.sock"}, diff --git a/internal/mode/static/nginx/config/sockets.go b/internal/mode/static/nginx/config/sockets.go index 5382c1f8d6..f507d7d751 100644 --- a/internal/mode/static/nginx/config/sockets.go +++ b/internal/mode/static/nginx/config/sockets.go @@ -12,6 +12,6 @@ func getSocketNameHTTPS(port int32) string { return fmt.Sprintf("unix:/var/run/nginx/https%d.sock", port) } -func getVariableName(port int32) string { +func getTLSPassthroughVarName(port int32) string { return fmt.Sprintf("$dest%d", port) } diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index ffc94acc93..32e6d8230f 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -47,7 +47,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { portSet[server.Port] = struct{}{} streamServers = append(streamServers, stream.Server{ Listen: fmt.Sprint(server.Port), - Pass: getVariableName(server.Port), + Pass: getTLSPassthroughVarName(server.Port), SSLPreread: true, }) } diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 8a8dfeee5c..16158aa0fc 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -95,12 +95,12 @@ func TestCreateStreamServers(t *testing.T) { }, { Listen: fmt.Sprint(8081), - Pass: getVariableName(8081), + Pass: getTLSPassthroughVarName(8081), SSLPreread: true, }, { Listen: fmt.Sprint(8080), - Pass: getVariableName(8080), + Pass: getTLSPassthroughVarName(8080), SSLPreread: true, }, } From c092f6a7247eab119cf0d4df733c7f2a73bfdb67 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Thu, 27 Jun 2024 15:56:05 -0600 Subject: [PATCH 28/47] add short circuit in createStreamMaps --- internal/mode/static/nginx/config/maps.go | 4 ++++ internal/mode/static/nginx/config/maps_test.go | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index d6544df45b..6d714e29f0 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -34,6 +34,10 @@ func executeStreamMaps(conf dataplane.Configuration) []executeResult { } func createStreamMaps(conf dataplane.Configuration) []shared.Map { + if len(conf.TLSPassthroughServers) == 0 { + return []shared.Map{} + } + portsToMap := make(map[int32]shared.Map) for _, server := range conf.TLSPassthroughServers { diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index c120062533..a0363dcb91 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -4,10 +4,9 @@ import ( "strings" "testing" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" - . "github.com/onsi/gomega" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) From b8f92d92b499363e7beac6023464f10ccd5e3bfd Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Thu, 27 Jun 2024 16:17:19 -0600 Subject: [PATCH 29/47] update map and base tests and pipeline --- .../static/nginx/config/base_http_config_test.go | 4 ++++ internal/mode/static/nginx/config/maps_test.go | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/internal/mode/static/nginx/config/base_http_config_test.go b/internal/mode/static/nginx/config/base_http_config_test.go index 4eb202ba1b..e307f2289e 100644 --- a/internal/mode/static/nginx/config/base_http_config_test.go +++ b/internal/mode/static/nginx/config/base_http_config_test.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "strings" "testing" @@ -46,6 +47,9 @@ func TestExecuteBaseHttp(t *testing.T) { res := executeBaseHTTPConfig(test.conf) g.Expect(res).To(HaveLen(1)) + fmt.Println(string(res[0].data)) g.Expect(test.expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) + g.Expect(strings.Count(string(res[0].data), "map $http_host $gw_api_compliant_host {")).To(Equal(1)) + g.Expect(strings.Count(string(res[0].data), "map $http_upgrade $connection_upgrade {")).To(Equal(1)) } } diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index a0363dcb91..985e06c682 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -208,12 +208,20 @@ func TestExecuteStreamMaps(t *testing.T) { UpstreamName: "backend2", }, }, + SSLServers: []dataplane.VirtualServer{ + { + Hostname: "app.example.com", + Port: 8080, + }, + }, } expSubStrings := map[string]int{ "example.com unix:/var/run/nginx/example.com8081.sock;": 1, "example.com unix:/var/run/nginx/example.com8080.sock;": 1, "cafe.example.com unix:/var/run/nginx/cafe.example.com8080.sock;": 1, + "app.example.com unix:/var/run/nginx/https8080.sock;": 1, + "hostnames": 2, } results := executeStreamMaps(conf) @@ -251,6 +259,10 @@ func TestCreateStreamMaps(t *testing.T) { Hostname: "app.example.com", Port: 8080, }, + { + Port: 8080, + IsDefault: true, + }, }, } @@ -272,6 +284,7 @@ func TestCreateStreamMaps(t *testing.T) { {Value: "example.com", Result: "unix:/var/run/nginx/example.com8080.sock"}, {Value: "cafe.example.com", Result: "unix:/var/run/nginx/cafe.example.com8080.sock"}, {Value: "app.example.com", Result: "unix:/var/run/nginx/https8080.sock"}, + {Value: "default", Result: "unix:/var/run/nginx/https8080.sock"}, }, UseHostnames: true, }, From 17bc73adc779e9139a9d6fc5f4818dc8cc2092ca Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 28 Jun 2024 10:27:35 -0600 Subject: [PATCH 30/47] fix a bunch of nits --- tests/go.mod | 2 +- tests/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/go.mod b/tests/go.mod index 94badd2f5a..ca217e7ed5 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -14,7 +14,7 @@ require ( k8s.io/apimachinery v0.30.2 k8s.io/client-go v0.30.2 sigs.k8s.io/controller-runtime v0.18.4 - sigs.k8s.io/gateway-api v1.1.0 + sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7 sigs.k8s.io/yaml v1.4.0 ) diff --git a/tests/go.sum b/tests/go.sum index 2162f2b53b..4e3b43e164 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -210,8 +210,8 @@ pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw= pgregory.net/rapid v1.1.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04= sigs.k8s.io/controller-runtime v0.18.4 h1:87+guW1zhvuPLh1PHybKdYFLU0YJp4FhJRmiHvm5BZw= sigs.k8s.io/controller-runtime v0.18.4/go.mod h1:TVoGrfdpbA9VRFaRnKgk9P5/atA0pMwq+f+msb9M8Sg= -sigs.k8s.io/gateway-api v1.1.0 h1:DsLDXCi6jR+Xz8/xd0Z1PYl2Pn0TyaFMOPPZIj4inDM= -sigs.k8s.io/gateway-api v1.1.0/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= +sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7 h1:udSMkvcUfzNYdPIbvUUR1F084KEvAIhcsmADAwAab6c= +sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0= sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4= From 0fe630ab199c24d2909675fccaa3ceccce043be2 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 1 Jul 2024 11:46:37 -0600 Subject: [PATCH 31/47] add connection closing server --- config/tests/static-deployment.yaml | 9 ++-- .../nginx/config/base_http_config_test.go | 2 - internal/mode/static/nginx/config/servers.go | 7 ++- .../static/nginx/config/servers_template.go | 1 - .../mode/static/nginx/config/servers_test.go | 14 ++++-- .../mode/static/nginx/config/shared/config.go | 2 +- .../mode/static/nginx/config/sockets_test.go | 7 +++ .../static/nginx/config/stream_servers.go | 28 +++++++---- .../nginx/config/stream_servers_template.go | 5 ++ .../nginx/config/stream_servers_test.go | 10 ++-- .../mode/static/nginx/config/upstreams.go | 20 +++++++- .../static/state/dataplane/configuration.go | 46 +++++++++++++------ 12 files changed, 111 insertions(+), 40 deletions(-) diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index eff64185a6..f6c0b5cbee 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -32,6 +32,7 @@ spec: - --health-port=8081 - --leader-election-lock-name=nginx-gateway-leader-election - --product-telemetry-disable + - --gateway-api-experimental-features env: - name: POD_IP valueFrom: @@ -45,8 +46,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge - imagePullPolicy: Always + image: nginx-gateway-fabric:edge + imagePullPolicy: Never name: nginx-gateway ports: - name: health @@ -80,8 +81,8 @@ spec: mountPath: /var/run/nginx - name: nginx-includes mountPath: /etc/nginx/includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge - imagePullPolicy: Always + - image: nginx-gateway-fabric/nginx:edge + imagePullPolicy: Never name: nginx ports: - containerPort: 80 diff --git a/internal/mode/static/nginx/config/base_http_config_test.go b/internal/mode/static/nginx/config/base_http_config_test.go index e307f2289e..151d3283b3 100644 --- a/internal/mode/static/nginx/config/base_http_config_test.go +++ b/internal/mode/static/nginx/config/base_http_config_test.go @@ -1,7 +1,6 @@ package config import ( - "fmt" "strings" "testing" @@ -47,7 +46,6 @@ func TestExecuteBaseHttp(t *testing.T) { res := executeBaseHTTPConfig(test.conf) g.Expect(res).To(HaveLen(1)) - fmt.Println(string(res[0].data)) g.Expect(test.expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) g.Expect(strings.Count(string(res[0].data), "map $http_host $gw_api_compliant_host {")).To(Equal(1)) g.Expect(strings.Count(string(res[0].data), "map $http_upgrade $connection_upgrade {")).To(Equal(1)) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 7b7baccea0..812dfc207b 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -149,10 +149,10 @@ func createServers( servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) - sharedPorts := make(map[int32]struct{}) + sharedTLSPorts := make(map[int32]struct{}) for _, tlsServer := range tlsServers { - sharedPorts[tlsServer.Port] = struct{}{} + sharedTLSPorts[tlsServer.Port] = struct{}{} } for serverID, s := range httpServers { @@ -164,8 +164,7 @@ func createServers( for serverID, s := range sslServers { listen := fmt.Sprint(s.Port) - _, portInUse := sharedPorts[s.Port] - if portInUse { + if _, portInUse := sharedTLSPorts[s.Port]; portInUse { listen = getSocketNameHTTPS(s.Port) } sslServer, matchPair := createSSLServer(s, serverID, listen) diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index b357d68c63..a559725866 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -4,7 +4,6 @@ const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := . -}} {{ if $s.IsDefaultSSL -}} -# this is a comment server { listen {{ $s.Listen }} ssl default_server; diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index d218be6161..9f519334d3 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -664,6 +664,14 @@ func TestCreateServers(t *testing.T) { }, } + tlsServers := []dataplane.Layer4VirtualServer{ + { + Hostname: "app.example.com", + Port: 8443, + UpstreamName: "sup", + }, + } + expMatchPairs := httpMatchPairs{ "1_0": { {Method: "POST", RedirectPath: "@rule0-route0"}, @@ -1038,7 +1046,7 @@ func TestCreateServers(t *testing.T) { }, { IsDefaultSSL: true, - Listen: "8443", + Listen: getSocketNameHTTPS(8443), }, { ServerName: "cafe.example.com", @@ -1047,7 +1055,7 @@ func TestCreateServers(t *testing.T) { CertificateKey: expectedPEMPath, }, Locations: getExpectedLocations(true), - Listen: "8443", + Listen: getSocketNameHTTPS(8443), GRPC: true, Includes: []string{ includesFolder + "/server-addition-1.conf", @@ -1058,7 +1066,7 @@ func TestCreateServers(t *testing.T) { g := NewWithT(t) - result, httpMatchPair := createServers(httpServers, sslServers, []dataplane.Layer4VirtualServer{}) + result, httpMatchPair := createServers(httpServers, sslServers, tlsServers) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) diff --git a/internal/mode/static/nginx/config/shared/config.go b/internal/mode/static/nginx/config/shared/config.go index d0091dcc1f..baab86c73a 100644 --- a/internal/mode/static/nginx/config/shared/config.go +++ b/internal/mode/static/nginx/config/shared/config.go @@ -8,7 +8,7 @@ type Map struct { UseHostnames bool } -// MapParameter Parameter defines a Value and Result pair in a Map. +// MapParameter defines a Value and Result pair in a Map. type MapParameter struct { Value string Result string diff --git a/internal/mode/static/nginx/config/sockets_test.go b/internal/mode/static/nginx/config/sockets_test.go index ac09c836e6..fab3a02430 100644 --- a/internal/mode/static/nginx/config/sockets_test.go +++ b/internal/mode/static/nginx/config/sockets_test.go @@ -19,3 +19,10 @@ func TestGetSocketNameHTTPS(t *testing.T) { g := NewGomegaWithT(t) g.Expect(res).To(Equal("unix:/var/run/nginx/https800.sock")) } + +func TestGetTLSPassthroughVarName(t *testing.T) { + res := getTLSPassthroughVarName(800) + + g := NewGomegaWithT(t) + g.Expect(res).To(Equal("$dest800")) +} diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 32e6d8230f..a3582a5181 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -27,23 +27,35 @@ func executeStreamServers(conf dataplane.Configuration) []executeResult { } func createStreamServers(conf dataplane.Configuration) []stream.Server { + if len(conf.TLSPassthroughServers) == 0 { + return nil + } + streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) - for _, server := range conf.TLSPassthroughServers { - listen := getSocketNameTLS(server.Port, server.Hostname) - streamServers = append(streamServers, stream.Server{ - Listen: listen, - ProxyPass: server.UpstreamName, - }) + portSet := make(map[int32]struct{}) + upstreamNameSet := make(map[string]struct{}) + for _, u := range conf.StreamUpstreams { + upstreamNameSet[u.Name] = struct{}{} } - portSet := make(map[int32]struct{}, len(streamServers)) - for _, server := range conf.TLSPassthroughServers { + proxyPass := server.UpstreamName + + if _, nameExists := upstreamNameSet[proxyPass]; !nameExists { + proxyPass = invalidBackendRefStream + } + streamServers = append(streamServers, stream.Server{ + Listen: getSocketNameTLS(server.Port, server.Hostname), + ProxyPass: proxyPass, + }) + _, inPortSet := portSet[server.Port] + if inPortSet { continue } + portSet[server.Port] = struct{}{} streamServers = append(streamServers, stream.Server{ Listen: fmt.Sprint(server.Port), diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index e0e1c00ba8..cd6c0f78f1 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -18,4 +18,9 @@ server { {{- end }} } {{- end }} + +server { + listen unix:/var/run/nginx/connection-closed-server.sock; + return ""; +} ` diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 16158aa0fc..f45661e281 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "sort" "strings" "testing" @@ -105,7 +106,10 @@ func TestCreateStreamServers(t *testing.T) { }, } - for i := range streamServers { - g.Expect(streamServers[i]).To(Equal(expectedStreamServers[i])) - } + sort.Slice(expectedStreamServers, func(i, j int) bool { + return expectedStreamServers[i].Listen < expectedStreamServers[j].Listen + }) + sort.Slice(streamServers, func(i, j int) bool { return streamServers[i].Listen < streamServers[j].Listen }) + + g.Expect(streamServers).To(Equal(expectedStreamServers)) } diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index a1bcbd842c..97fe11fca5 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -19,6 +19,10 @@ const ( nginx500Server = "unix:/var/run/nginx/nginx-500-server.sock" // invalidBackendRef is used as an upstream name for invalid backend references. invalidBackendRef = "invalid-backend-ref" + // nginxConnectionClosedServer is used as a stream backend that will close the connection + nginxConnectionClosedServer = "unix:/var/run/nginx/connection-closed-server.sock" + // invalidBackendRef is used as an upstream name for invalid backend references in stream. + invalidBackendRefStream = "invalid-backend-ref-stream" // ossZoneSize is the upstream zone size for nginx open source. ossZoneSize = "512k" // plusZoneSize is the upstream zone size for nginx plus. @@ -55,6 +59,8 @@ func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []s ups = append(ups, g.createStreamUpstream(u)) } + ups = append(ups, createInvalidStreamBackendRefUpstream()) + return ups } @@ -70,7 +76,7 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre ZoneSize: zoneSize, Servers: []stream.UpstreamServer{ { - Address: nginx502Server, + Address: nginxConnectionClosedServer, }, }, } @@ -135,6 +141,18 @@ func (g GeneratorImpl) createUpstream(up dataplane.Upstream) http.Upstream { } } +func createInvalidStreamBackendRefUpstream() stream.Upstream { + // ZoneSize is omitted since we will only ever proxy to one destination/backend. + return stream.Upstream{ + Name: invalidBackendRefStream, + Servers: []stream.UpstreamServer{ + { + Address: nginxConnectionClosedServer, + }, + }, + } +} + func createInvalidBackendRefUpstream() http.Upstream { // ZoneSize is omitted since we will only ever proxy to one destination/backend. return http.Upstream{ diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index b82603a1d7..416b7c7ee1 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -48,21 +48,41 @@ func BuildConfiguration( certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) baseHTTPConfig := buildBaseHTTPConfig(g) - var tlsServers []Layer4VirtualServer - var streamUpstreams []Upstream config := Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - TLSPassthroughServers: tlsServers, - Upstreams: upstreams, - StreamUpstreams: streamUpstreams, - BackendGroups: backendGroups, - SSLKeyPairs: keyPairs, - Version: configVersion, - CertBundles: certBundles, - Telemetry: telemetry, - BaseHTTPConfig: baseHTTPConfig, + HTTPServers: httpServers, + SSLServers: sslServers, + TLSPassthroughServers: []Layer4VirtualServer{ + { + Hostname: "app.example.com", + Port: 443, + UpstreamName: "backend", + }, + { + Hostname: "cafe.example.com", + Port: 443, + UpstreamName: "rfj", + }, + }, + + StreamUpstreams: []Upstream{ + { + Name: "backend", + Endpoints: []resolver.Endpoint{ + { + Address: "10.244.0.12", + Port: 8443, + }, + }, + }, + }, + Upstreams: upstreams, + BackendGroups: backendGroups, + SSLKeyPairs: keyPairs, + Version: configVersion, + CertBundles: certBundles, + Telemetry: telemetry, + BaseHTTPConfig: baseHTTPConfig, } return config From 63efd48a289f5750ac731f6cd86fa2afd5d03805 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 1 Jul 2024 14:48:01 -0600 Subject: [PATCH 32/47] update invalid upstream name to remove socket --- internal/mode/static/nginx/conf/nginx.conf | 5 ++++ internal/mode/static/nginx/config/maps.go | 14 +++++++++- .../static/nginx/config/stream_servers.go | 13 ++++----- .../nginx/config/stream_servers_template.go | 5 ---- .../mode/static/nginx/config/upstreams.go | 16 ----------- .../static/state/dataplane/configuration.go | 28 ++----------------- 6 files changed, 25 insertions(+), 56 deletions(-) diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index 3ebeeeacba..ea069a9c62 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -44,5 +44,10 @@ stream { map_hash_max_size 2048; map_hash_bucket_size 256; + log_format stream-main '$remote_addr [$time_local] ' + '$protocol $status $bytes_sent $bytes_received ' + '$session_time "$ssl_preread_server_name"'; + access_log /dev/stdout stream-main; + include /etc/nginx/stream-conf.d/*.conf; } diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 6d714e29f0..6f8bec2347 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -38,14 +38,26 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { return []shared.Map{} } + upstreamNameSet := make(map[string]struct{}) + + for _, u := range conf.StreamUpstreams { + upstreamNameSet[u.Name] = struct{}{} + } + portsToMap := make(map[int32]shared.Map) for _, server := range conf.TLSPassthroughServers { streamMap, portInUse := portsToMap[server.Port] + socket := "\"\"" + + if _, nameExists := upstreamNameSet[server.UpstreamName]; nameExists { + socket = getSocketNameTLS(server.Port, server.Hostname) + } + mapParam := shared.MapParameter{ Value: server.Hostname, - Result: getSocketNameTLS(server.Port, server.Hostname), + Result: socket, } if !portInUse { diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index a3582a5181..58c4d60ed6 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -40,15 +40,12 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { } for _, server := range conf.TLSPassthroughServers { - proxyPass := server.UpstreamName - - if _, nameExists := upstreamNameSet[proxyPass]; !nameExists { - proxyPass = invalidBackendRefStream + if _, nameExists := upstreamNameSet[server.UpstreamName]; nameExists { + streamServers = append(streamServers, stream.Server{ + Listen: getSocketNameTLS(server.Port, server.Hostname), + ProxyPass: server.UpstreamName, + }) } - streamServers = append(streamServers, stream.Server{ - Listen: getSocketNameTLS(server.Port, server.Hostname), - ProxyPass: proxyPass, - }) _, inPortSet := portSet[server.Port] diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index cd6c0f78f1..e0e1c00ba8 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -18,9 +18,4 @@ server { {{- end }} } {{- end }} - -server { - listen unix:/var/run/nginx/connection-closed-server.sock; - return ""; -} ` diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index 97fe11fca5..d3c56657f2 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -21,8 +21,6 @@ const ( invalidBackendRef = "invalid-backend-ref" // nginxConnectionClosedServer is used as a stream backend that will close the connection nginxConnectionClosedServer = "unix:/var/run/nginx/connection-closed-server.sock" - // invalidBackendRef is used as an upstream name for invalid backend references in stream. - invalidBackendRefStream = "invalid-backend-ref-stream" // ossZoneSize is the upstream zone size for nginx open source. ossZoneSize = "512k" // plusZoneSize is the upstream zone size for nginx plus. @@ -59,8 +57,6 @@ func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []s ups = append(ups, g.createStreamUpstream(u)) } - ups = append(ups, createInvalidStreamBackendRefUpstream()) - return ups } @@ -141,18 +137,6 @@ func (g GeneratorImpl) createUpstream(up dataplane.Upstream) http.Upstream { } } -func createInvalidStreamBackendRefUpstream() stream.Upstream { - // ZoneSize is omitted since we will only ever proxy to one destination/backend. - return stream.Upstream{ - Name: invalidBackendRefStream, - Servers: []stream.UpstreamServer{ - { - Address: nginxConnectionClosedServer, - }, - }, - } -} - func createInvalidBackendRefUpstream() http.Upstream { // ZoneSize is omitted since we will only ever proxy to one destination/backend. return http.Upstream{ diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 416b7c7ee1..197e77fc40 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -50,32 +50,8 @@ func BuildConfiguration( baseHTTPConfig := buildBaseHTTPConfig(g) config := Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - TLSPassthroughServers: []Layer4VirtualServer{ - { - Hostname: "app.example.com", - Port: 443, - UpstreamName: "backend", - }, - { - Hostname: "cafe.example.com", - Port: 443, - UpstreamName: "rfj", - }, - }, - - StreamUpstreams: []Upstream{ - { - Name: "backend", - Endpoints: []resolver.Endpoint{ - { - Address: "10.244.0.12", - Port: 8443, - }, - }, - }, - }, + HTTPServers: httpServers, + SSLServers: sslServers, Upstreams: upstreams, BackendGroups: backendGroups, SSLKeyPairs: keyPairs, From 64a0cca228f435f849edd9148b6c5ba146c5f813 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 1 Jul 2024 14:51:28 -0600 Subject: [PATCH 33/47] readd close connection server --- internal/mode/static/nginx/config/stream_servers_template.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index e0e1c00ba8..cd6c0f78f1 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -18,4 +18,9 @@ server { {{- end }} } {{- end }} + +server { + listen unix:/var/run/nginx/connection-closed-server.sock; + return ""; +} ` From bca9a5b7c0257a468d973b6445c187d27a4bc38f Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 08:57:17 -0600 Subject: [PATCH 34/47] fix lint checks and manifests --- config/tests/static-deployment.yaml | 9 ++++----- internal/mode/static/nginx/config/upstreams.go | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index f6c0b5cbee..eff64185a6 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -32,7 +32,6 @@ spec: - --health-port=8081 - --leader-election-lock-name=nginx-gateway-leader-election - --product-telemetry-disable - - --gateway-api-experimental-features env: - name: POD_IP valueFrom: @@ -46,8 +45,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: nginx-gateway-fabric:edge - imagePullPolicy: Never + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + imagePullPolicy: Always name: nginx-gateway ports: - name: health @@ -81,8 +80,8 @@ spec: mountPath: /var/run/nginx - name: nginx-includes mountPath: /etc/nginx/includes - - image: nginx-gateway-fabric/nginx:edge - imagePullPolicy: Never + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + imagePullPolicy: Always name: nginx ports: - containerPort: 80 diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index d3c56657f2..db8ed87efb 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -19,7 +19,7 @@ const ( nginx500Server = "unix:/var/run/nginx/nginx-500-server.sock" // invalidBackendRef is used as an upstream name for invalid backend references. invalidBackendRef = "invalid-backend-ref" - // nginxConnectionClosedServer is used as a stream backend that will close the connection + // nginxConnectionClosedServer is used as a stream backend that will close the connection. nginxConnectionClosedServer = "unix:/var/run/nginx/connection-closed-server.sock" // ossZoneSize is the upstream zone size for nginx open source. ossZoneSize = "512k" From 0315e774fdfa06643ef5b42d4696fb8bbd468f5b Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 09:44:46 -0600 Subject: [PATCH 35/47] fix tests and update plus config --- internal/mode/static/nginx/conf/nginx-plus.conf | 15 +++++++++++++++ internal/mode/static/nginx/config/maps_test.go | 16 ++++++++++++++++ .../static/nginx/config/stream_servers_test.go | 16 ++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/internal/mode/static/nginx/conf/nginx-plus.conf b/internal/mode/static/nginx/conf/nginx-plus.conf index b8b9cc77d8..32d3ba2c93 100644 --- a/internal/mode/static/nginx/conf/nginx-plus.conf +++ b/internal/mode/static/nginx/conf/nginx-plus.conf @@ -52,6 +52,21 @@ http { } } +stream { + variables_hash_bucket_size 512; + variables_hash_max_size 1024; + + map_hash_max_size 2048; + map_hash_bucket_size 256; + + log_format stream-main '$remote_addr [$time_local] ' + '$protocol $status $bytes_sent $bytes_received ' + '$session_time "$ssl_preread_server_name"'; + access_log /dev/stdout stream-main; + + include /etc/nginx/stream-conf.d/*.conf; +} + mgmt { usage_report interval=0s; } diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 985e06c682..0b9ec46920 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -214,6 +214,14 @@ func TestExecuteStreamMaps(t *testing.T) { Port: 8080, }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + }, + { + Name: "backend2", + }, + }, } expSubStrings := map[string]int{ @@ -264,6 +272,14 @@ func TestCreateStreamMaps(t *testing.T) { IsDefault: true, }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + }, + { + Name: "backend2", + }, + }, } maps := createStreamMaps(conf) diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index f45661e281..e0ab688ebb 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -31,6 +31,14 @@ func TestExecuteStreamServers(t *testing.T) { UpstreamName: "backend2", }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + }, + { + Name: "backend2", + }, + }, } expSubStrings := map[string]int{ @@ -70,6 +78,14 @@ func TestCreateStreamServers(t *testing.T) { UpstreamName: "backend2", }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + }, + { + Name: "backend2", + }, + }, } streamServers := createStreamServers(conf) From 449c59cb08dead9e49ce36133176cc373a1d83e7 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 11:36:36 -0600 Subject: [PATCH 36/47] revert gateway api tests version --- tests/go.mod | 2 +- tests/go.sum | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/go.mod b/tests/go.mod index ca217e7ed5..94badd2f5a 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -14,7 +14,7 @@ require ( k8s.io/apimachinery v0.30.2 k8s.io/client-go v0.30.2 sigs.k8s.io/controller-runtime v0.18.4 - sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7 + sigs.k8s.io/gateway-api v1.1.0 sigs.k8s.io/yaml v1.4.0 ) diff --git a/tests/go.sum b/tests/go.sum index 4e3b43e164..fe0149111a 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -210,6 +210,7 @@ pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw= pgregory.net/rapid v1.1.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04= sigs.k8s.io/controller-runtime v0.18.4 h1:87+guW1zhvuPLh1PHybKdYFLU0YJp4FhJRmiHvm5BZw= sigs.k8s.io/controller-runtime v0.18.4/go.mod h1:TVoGrfdpbA9VRFaRnKgk9P5/atA0pMwq+f+msb9M8Sg= +sigs.k8s.io/gateway-api v1.1.0/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7 h1:udSMkvcUfzNYdPIbvUUR1F084KEvAIhcsmADAwAab6c= sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= From 0eb7092b4b604254ddc029361944c158be4c240e Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 11:38:42 -0600 Subject: [PATCH 37/47] update go sum --- tests/go.sum | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/go.sum b/tests/go.sum index fe0149111a..2162f2b53b 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -210,9 +210,8 @@ pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw= pgregory.net/rapid v1.1.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04= sigs.k8s.io/controller-runtime v0.18.4 h1:87+guW1zhvuPLh1PHybKdYFLU0YJp4FhJRmiHvm5BZw= sigs.k8s.io/controller-runtime v0.18.4/go.mod h1:TVoGrfdpbA9VRFaRnKgk9P5/atA0pMwq+f+msb9M8Sg= +sigs.k8s.io/gateway-api v1.1.0 h1:DsLDXCi6jR+Xz8/xd0Z1PYl2Pn0TyaFMOPPZIj4inDM= sigs.k8s.io/gateway-api v1.1.0/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= -sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7 h1:udSMkvcUfzNYdPIbvUUR1F084KEvAIhcsmADAwAab6c= -sigs.k8s.io/gateway-api v1.1.1-0.20240625203242-cb5bf1541fa7/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0= sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4= From c28a56608839476616c90b0ea64423b9481c97ec Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 14:28:13 -0600 Subject: [PATCH 38/47] update tests for stream maps and servers --- internal/mode/static/nginx/config/maps.go | 19 ++++++--------- .../mode/static/nginx/config/maps_test.go | 23 +++++++++++++++++++ .../static/nginx/config/stream_servers.go | 11 ++------- .../nginx/config/stream_servers_template.go | 5 ---- .../nginx/config/stream_servers_test.go | 21 +++++++++++++++-- 5 files changed, 51 insertions(+), 28 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 6f8bec2347..61d2fe8fa0 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -4,14 +4,16 @@ import ( "strings" gotemplate "text/template" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) var mapsTemplate = gotemplate.Must(gotemplate.New("maps").Parse(mapsTemplateText)) +// emptyStringSocket is used when we want NGINX to show a 500 error in logs. +const emptyStringSocket = "\"\"" + func executeMaps(conf dataplane.Configuration) []executeResult { maps := buildAddHeaderMaps(append(conf.HTTPServers, conf.SSLServers...)) result := executeResult{ @@ -35,23 +37,16 @@ func executeStreamMaps(conf dataplane.Configuration) []executeResult { func createStreamMaps(conf dataplane.Configuration) []shared.Map { if len(conf.TLSPassthroughServers) == 0 { - return []shared.Map{} + return nil } - - upstreamNameSet := make(map[string]struct{}) - - for _, u := range conf.StreamUpstreams { - upstreamNameSet[u.Name] = struct{}{} - } - portsToMap := make(map[int32]shared.Map) for _, server := range conf.TLSPassthroughServers { streamMap, portInUse := portsToMap[server.Port] - socket := "\"\"" + socket := emptyStringSocket - if _, nameExists := upstreamNameSet[server.UpstreamName]; nameExists { + if server.UpstreamName != "" { socket = getSocketNameTLS(server.Port, server.Hostname) } diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 0b9ec46920..f466cfa6d6 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -1,6 +1,7 @@ package config import ( + "sort" "strings" "testing" @@ -261,6 +262,11 @@ func TestCreateStreamMaps(t *testing.T) { Port: 8080, UpstreamName: "backend2", }, + { + Hostname: "wrong.example.com", + Port: 8080, + UpstreamName: "", + }, }, SSLServers: []dataplane.VirtualServer{ { @@ -299,11 +305,28 @@ func TestCreateStreamMaps(t *testing.T) { Parameters: []shared.MapParameter{ {Value: "example.com", Result: "unix:/var/run/nginx/example.com8080.sock"}, {Value: "cafe.example.com", Result: "unix:/var/run/nginx/cafe.example.com8080.sock"}, + {Value: "wrong.example.com", Result: "\"\""}, {Value: "app.example.com", Result: "unix:/var/run/nginx/https8080.sock"}, {Value: "default", Result: "unix:/var/run/nginx/https8080.sock"}, }, UseHostnames: true, }, } + + sort.Slice(expectedMaps, func(i, j int) bool { return expectedMaps[i].Source < expectedMaps[j].Source }) + sort.Slice(maps, func(i, j int) bool { return maps[i].Source < maps[j].Source }) + g.Expect(maps).To(Equal(expectedMaps)) +} + +func TestCreateStreamMapsWithEmpty(t *testing.T) { + g := NewWithT(t) + conf := dataplane.Configuration{ + TLSPassthroughServers: nil, + } + + maps := createStreamMaps(conf) + + var expectedMaps []shared.Map + g.Expect(maps).To(Equal(expectedMaps)) } diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 58c4d60ed6..b4d7cc9b70 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -33,23 +33,16 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) portSet := make(map[int32]struct{}) - upstreamNameSet := make(map[string]struct{}) - - for _, u := range conf.StreamUpstreams { - upstreamNameSet[u.Name] = struct{}{} - } for _, server := range conf.TLSPassthroughServers { - if _, nameExists := upstreamNameSet[server.UpstreamName]; nameExists { + if server.UpstreamName != "" { streamServers = append(streamServers, stream.Server{ Listen: getSocketNameTLS(server.Port, server.Hostname), ProxyPass: server.UpstreamName, }) } - _, inPortSet := portSet[server.Port] - - if inPortSet { + if _, inPortSet := portSet[server.Port]; inPortSet { continue } diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index cd6c0f78f1..e0e1c00ba8 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -18,9 +18,4 @@ server { {{- end }} } {{- end }} - -server { - listen unix:/var/run/nginx/connection-closed-server.sock; - return ""; -} ` diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index e0ab688ebb..80d7a11427 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -77,6 +77,11 @@ func TestCreateStreamServers(t *testing.T) { Port: 8080, UpstreamName: "backend2", }, + { + Hostname: "wrong.example.com", + Port: 8081, + UpstreamName: "", + }, }, StreamUpstreams: []dataplane.Upstream{ { @@ -92,8 +97,6 @@ func TestCreateStreamServers(t *testing.T) { g := NewWithT(t) - g.Expect(streamServers).To(HaveLen(5)) - expectedStreamServers := []stream.Server{ { Listen: getSocketNameTLS(conf.TLSPassthroughServers[0].Port, conf.TLSPassthroughServers[0].Hostname), @@ -129,3 +132,17 @@ func TestCreateStreamServers(t *testing.T) { g.Expect(streamServers).To(Equal(expectedStreamServers)) } + +func TestCreateStreamServersWithNone(t *testing.T) { + conf := dataplane.Configuration{ + TLSPassthroughServers: nil, + } + + streamServers := createStreamServers(conf) + + g := NewWithT(t) + + var expectedStreamServers []stream.Server + + g.Expect(streamServers).To(Equal(expectedStreamServers)) +} From bb7e505e0af02dd615e5f2aaff71f209936bc0c2 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 14:37:19 -0600 Subject: [PATCH 39/47] add stream upstream tests, remove invalid upstream ref --- .../mode/static/nginx/config/upstreams.go | 18 +- .../static/nginx/config/upstreams_test.go | 194 ++++++++++++++++++ 2 files changed, 197 insertions(+), 15 deletions(-) diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index db8ed87efb..55a5105b1a 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -19,8 +19,6 @@ const ( nginx500Server = "unix:/var/run/nginx/nginx-500-server.sock" // invalidBackendRef is used as an upstream name for invalid backend references. invalidBackendRef = "invalid-backend-ref" - // nginxConnectionClosedServer is used as a stream backend that will close the connection. - nginxConnectionClosedServer = "unix:/var/run/nginx/connection-closed-server.sock" // ossZoneSize is the upstream zone size for nginx open source. ossZoneSize = "512k" // plusZoneSize is the upstream zone size for nginx plus. @@ -54,7 +52,9 @@ func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []s ups := make([]stream.Upstream, 0, len(upstreams)+1) for _, u := range upstreams { - ups = append(ups, g.createStreamUpstream(u)) + if len(u.Endpoints) != 0 { + ups = append(ups, g.createStreamUpstream(u)) + } } return ups @@ -66,18 +66,6 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre zoneSize = plusZoneSize } - if len(up.Endpoints) == 0 { - return stream.Upstream{ - Name: up.Name, - ZoneSize: zoneSize, - Servers: []stream.UpstreamServer{ - { - Address: nginxConnectionClosedServer, - }, - }, - } - } - upstreamServers := make([]stream.UpstreamServer, len(up.Endpoints)) for idx, ep := range up.Endpoints { upstreamServers[idx] = stream.UpstreamServer{ diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index b0a631858d..a225177f68 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/gomega" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) @@ -254,3 +255,196 @@ func TestCreateUpstreamPlus(t *testing.T) { g := NewWithT(t) g.Expect(result).To(Equal(expectedUpstream)) } + +func TestExecuteStreamUpstreams(t *testing.T) { + gen := GeneratorImpl{} + stateUpstreams := []dataplane.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", + "server 10.0.0.0:80;", + "server 11.0.0.0:80;", + } + + upstreamResults := gen.executeStreamUpstreams(dataplane.Configuration{StreamUpstreams: stateUpstreams}) + g := NewWithT(t) + g.Expect(upstreamResults).To(HaveLen(1)) + upstreams := string(upstreamResults[0].data) + + g.Expect(upstreamResults[0].dest).To(Equal(streamConfigFile)) + for _, expSubString := range expectedSubStrings { + g.Expect(upstreams).To(ContainSubstring(expSubString)) + } +} + +func TestCreateStreamUpstreams(t *testing.T) { + gen := GeneratorImpl{} + stateUpstreams := []dataplane.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 := []stream.Upstream{ + { + Name: "up1", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "10.0.0.0:80", + }, + { + Address: "10.0.0.1:80", + }, + { + Address: "10.0.0.2:80", + }, + }, + }, + { + Name: "up2", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "11.0.0.0:80", + }, + }, + }, + } + + g := NewWithT(t) + result := gen.createStreamUpstreams(stateUpstreams) + g.Expect(result).To(Equal(expUpstreams)) +} + +func TestCreatStreamUpstream(t *testing.T) { + gen := GeneratorImpl{} + tests := []struct { + msg string + stateUpstream dataplane.Upstream + expectedUpstream stream.Upstream + }{ + { + stateUpstream: dataplane.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: stream.Upstream{ + Name: "multiple-endpoints", + ZoneSize: ossZoneSize, + Servers: []stream.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 { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + result := gen.createStreamUpstream(test.stateUpstream) + g.Expect(result).To(Equal(test.expectedUpstream)) + }) + } +} + +func TestCreateStreamUpstreamPlus(t *testing.T) { + gen := GeneratorImpl{plus: true} + + stateUpstream := dataplane.Upstream{ + Name: "multiple-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.1", + Port: 80, + }, + }, + } + expectedUpstream := stream.Upstream{ + Name: "multiple-endpoints", + ZoneSize: plusZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "10.0.0.1:80", + }, + }, + } + + result := gen.createStreamUpstream(stateUpstream) + + g := NewWithT(t) + g.Expect(result).To(Equal(expectedUpstream)) +} From e90646f252b5bde18c0436bf1edd3d535c2ba587 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 14:39:22 -0600 Subject: [PATCH 40/47] fix a code nit --- internal/mode/static/nginx/config/stream_servers.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index b4d7cc9b70..29f0991cf0 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -19,11 +19,9 @@ func executeStreamServers(conf dataplane.Configuration) []executeResult { data: helpers.MustExecuteTemplate(streamServersTemplate, streamServers), } - result := []executeResult{ + return []executeResult{ streamServerResult, } - - return result } func createStreamServers(conf dataplane.Configuration) []stream.Server { From 83c0565eac4e58e1cc9ce5909d745633b7193550 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Tue, 2 Jul 2024 16:49:43 -0600 Subject: [PATCH 41/47] update server and map tests --- internal/mode/static/nginx/config/maps.go | 3 +- .../mode/static/nginx/config/maps_test.go | 25 ++-------------- .../nginx/config/stream_servers_test.go | 29 ++----------------- .../mode/static/nginx/config/upstreams.go | 3 +- .../static/nginx/config/upstreams_test.go | 2 +- 5 files changed, 8 insertions(+), 54 deletions(-) diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 61d2fe8fa0..5f841d99de 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -11,7 +11,8 @@ import ( var mapsTemplate = gotemplate.Must(gotemplate.New("maps").Parse(mapsTemplateText)) -// emptyStringSocket is used when we want NGINX to show a 500 error in logs. +// emptyStringSocket is used when the stream server has an invalid upstream. In this case, we route the request +// to a socket that doesn't exist so that NGINX will return an error status code (500). const emptyStringSocket = "\"\"" func executeMaps(conf dataplane.Configuration) []executeResult { diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index f466cfa6d6..83026b90a2 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -1,7 +1,6 @@ package config import ( - "sort" "strings" "testing" @@ -215,14 +214,6 @@ func TestExecuteStreamMaps(t *testing.T) { Port: 8080, }, }, - StreamUpstreams: []dataplane.Upstream{ - { - Name: "backend1", - }, - { - Name: "backend2", - }, - }, } expSubStrings := map[string]int{ @@ -278,14 +269,6 @@ func TestCreateStreamMaps(t *testing.T) { IsDefault: true, }, }, - StreamUpstreams: []dataplane.Upstream{ - { - Name: "backend1", - }, - { - Name: "backend2", - }, - }, } maps := createStreamMaps(conf) @@ -313,9 +296,7 @@ func TestCreateStreamMaps(t *testing.T) { }, } - sort.Slice(expectedMaps, func(i, j int) bool { return expectedMaps[i].Source < expectedMaps[j].Source }) - sort.Slice(maps, func(i, j int) bool { return maps[i].Source < maps[j].Source }) - g.Expect(maps).To(Equal(expectedMaps)) + g.Expect(maps).To(ConsistOf(expectedMaps)) } func TestCreateStreamMapsWithEmpty(t *testing.T) { @@ -326,7 +307,5 @@ func TestCreateStreamMapsWithEmpty(t *testing.T) { maps := createStreamMaps(conf) - var expectedMaps []shared.Map - - g.Expect(maps).To(Equal(expectedMaps)) + g.Expect(maps).To(BeNil()) } diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 80d7a11427..1f6a94d9b7 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -2,7 +2,6 @@ package config import ( "fmt" - "sort" "strings" "testing" @@ -31,14 +30,6 @@ func TestExecuteStreamServers(t *testing.T) { UpstreamName: "backend2", }, }, - StreamUpstreams: []dataplane.Upstream{ - { - Name: "backend1", - }, - { - Name: "backend2", - }, - }, } expSubStrings := map[string]int{ @@ -83,14 +74,6 @@ func TestCreateStreamServers(t *testing.T) { UpstreamName: "", }, }, - StreamUpstreams: []dataplane.Upstream{ - { - Name: "backend1", - }, - { - Name: "backend2", - }, - }, } streamServers := createStreamServers(conf) @@ -124,13 +107,7 @@ func TestCreateStreamServers(t *testing.T) { SSLPreread: true, }, } - - sort.Slice(expectedStreamServers, func(i, j int) bool { - return expectedStreamServers[i].Listen < expectedStreamServers[j].Listen - }) - sort.Slice(streamServers, func(i, j int) bool { return streamServers[i].Listen < streamServers[j].Listen }) - - g.Expect(streamServers).To(Equal(expectedStreamServers)) + g.Expect(streamServers).To(ConsistOf(expectedStreamServers)) } func TestCreateStreamServersWithNone(t *testing.T) { @@ -142,7 +119,5 @@ func TestCreateStreamServersWithNone(t *testing.T) { g := NewWithT(t) - var expectedStreamServers []stream.Server - - g.Expect(streamServers).To(Equal(expectedStreamServers)) + g.Expect(streamServers).To(BeNil()) } diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index 55a5105b1a..fcbca1128d 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -48,8 +48,7 @@ func (g GeneratorImpl) executeStreamUpstreams(conf dataplane.Configuration) []ex } func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []stream.Upstream { - // capacity is the number of upstreams + 1 for the invalid backend ref upstream - ups := make([]stream.Upstream, 0, len(upstreams)+1) + ups := make([]stream.Upstream, 0, len(upstreams)) for _, u := range upstreams { if len(u.Endpoints) != 0 { diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index a225177f68..6514ee2d92 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -368,7 +368,7 @@ func TestCreateStreamUpstreams(t *testing.T) { g.Expect(result).To(Equal(expUpstreams)) } -func TestCreatStreamUpstream(t *testing.T) { +func TestCreateStreamUpstream(t *testing.T) { gen := GeneratorImpl{} tests := []struct { msg string From 2135010bc2f71e324e4195544641c0fad7ce3448 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 3 Jul 2024 08:58:04 -0600 Subject: [PATCH 42/47] rename tlsservers --- internal/mode/static/nginx/config/servers.go | 6 +++--- internal/mode/static/nginx/config/servers_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 812dfc207b..a56a35b2d5 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -144,15 +144,15 @@ func createIncludes(additions []dataplane.Addition) []string { func createServers( httpServers, sslServers []dataplane.VirtualServer, - tlsServers []dataplane.Layer4VirtualServer, + tlsPassthroughServers []dataplane.Layer4VirtualServer, ) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) sharedTLSPorts := make(map[int32]struct{}) - for _, tlsServer := range tlsServers { - sharedTLSPorts[tlsServer.Port] = struct{}{} + for _, passthroughServer := range tlsPassthroughServers { + sharedTLSPorts[passthroughServer.Port] = struct{}{} } for serverID, s := range httpServers { diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 9f519334d3..a30b23454c 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -664,7 +664,7 @@ func TestCreateServers(t *testing.T) { }, } - tlsServers := []dataplane.Layer4VirtualServer{ + tlsPassthroughServers := []dataplane.Layer4VirtualServer{ { Hostname: "app.example.com", Port: 8443, @@ -1066,7 +1066,7 @@ func TestCreateServers(t *testing.T) { g := NewWithT(t) - result, httpMatchPair := createServers(httpServers, sslServers, tlsServers) + result, httpMatchPair := createServers(httpServers, sslServers, tlsPassthroughServers) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) From 82bf8c422cf17a22bc8fb91fa4d839b26d191d19 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Wed, 3 Jul 2024 08:59:00 -0600 Subject: [PATCH 43/47] remove extra spaces in conf files --- internal/mode/static/nginx/conf/nginx-plus.conf | 4 ++-- internal/mode/static/nginx/conf/nginx.conf | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/mode/static/nginx/conf/nginx-plus.conf b/internal/mode/static/nginx/conf/nginx-plus.conf index 32d3ba2c93..5eea380335 100644 --- a/internal/mode/static/nginx/conf/nginx-plus.conf +++ b/internal/mode/static/nginx/conf/nginx-plus.conf @@ -59,10 +59,10 @@ stream { map_hash_max_size 2048; map_hash_bucket_size 256; - log_format stream-main '$remote_addr [$time_local] ' + log_format stream-main '$remote_addr [$time_local] ' '$protocol $status $bytes_sent $bytes_received ' '$session_time "$ssl_preread_server_name"'; - access_log /dev/stdout stream-main; + access_log /dev/stdout stream-main; include /etc/nginx/stream-conf.d/*.conf; } diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index ea069a9c62..51f8d440c6 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -44,10 +44,10 @@ stream { map_hash_max_size 2048; map_hash_bucket_size 256; - log_format stream-main '$remote_addr [$time_local] ' + log_format stream-main '$remote_addr [$time_local] ' '$protocol $status $bytes_sent $bytes_received ' '$session_time "$ssl_preread_server_name"'; - access_log /dev/stdout stream-main; + access_log /dev/stdout stream-main; include /etc/nginx/stream-conf.d/*.conf; } From d023f288c66afb6d2ef670904640992e175aed75 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 5 Jul 2024 11:26:20 -0600 Subject: [PATCH 44/47] remove escape slash and add stream folder to slice --- internal/mode/static/nginx/config/generator.go | 2 +- internal/mode/static/nginx/config/maps.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 4148b93506..688ebe8353 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -46,7 +46,7 @@ const ( ) // ConfigFolders is a list of folders where NGINX configuration files are stored. -var ConfigFolders = []string{httpFolder, secretsFolder, includesFolder, modulesIncludesFolder} +var ConfigFolders = []string{httpFolder, secretsFolder, includesFolder, modulesIncludesFolder, streamFolder} // Generator generates NGINX configuration files. // This interface is used for testing purposes only. diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 5f841d99de..64d77659a0 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -13,7 +13,7 @@ var mapsTemplate = gotemplate.Must(gotemplate.New("maps").Parse(mapsTemplateText // emptyStringSocket is used when the stream server has an invalid upstream. In this case, we route the request // to a socket that doesn't exist so that NGINX will return an error status code (500). -const emptyStringSocket = "\"\"" +const emptyStringSocket = `""` func executeMaps(conf dataplane.Configuration) []executeResult { maps := buildAddHeaderMaps(append(conf.HTTPServers, conf.SSLServers...)) From fb857b0b1e55fae769a41feeffc59da71dc02d76 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 5 Jul 2024 12:32:54 -0600 Subject: [PATCH 45/47] update socket name creation and map template --- .../static/nginx/config/generator_test.go | 4 ++-- internal/mode/static/nginx/config/maps.go | 7 +++++-- .../mode/static/nginx/config/maps_template.go | 7 +++---- .../mode/static/nginx/config/maps_test.go | 20 +++++++++---------- internal/mode/static/nginx/config/sockets.go | 2 +- .../mode/static/nginx/config/sockets_test.go | 2 +- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index 7262871605..67311f4223 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -144,8 +144,8 @@ func TestGenerate(t *testing.T) { g.Expect(files[6].Path).To(Equal("/etc/nginx/stream-conf.d/stream.conf")) g.Expect(files[6].Type).To(Equal(file.TypeRegular)) streamCfg := string(files[6].Content) - g.Expect(streamCfg).To(ContainSubstring("listen unix:/var/run/nginx/app.example.com443.sock")) + g.Expect(streamCfg).To(ContainSubstring("listen unix:/var/run/nginx/app.example.com-443.sock")) g.Expect(streamCfg).To(ContainSubstring("listen 443")) - g.Expect(streamCfg).To(ContainSubstring("app.example.com unix:/var/run/nginx/app.example.com443.sock")) + g.Expect(streamCfg).To(ContainSubstring("app.example.com unix:/var/run/nginx/app.example.com-443.sock")) g.Expect(streamCfg).To(ContainSubstring("example.com unix:/var/run/nginx/https443.sock")) } diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 64d77659a0..703b2b7b04 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -11,8 +11,11 @@ import ( var mapsTemplate = gotemplate.Must(gotemplate.New("maps").Parse(mapsTemplateText)) -// emptyStringSocket is used when the stream server has an invalid upstream. In this case, we route the request -// to a socket that doesn't exist so that NGINX will return an error status code (500). +// emptyStringSocket is used when the stream server has an invalid upstream. In this case, we pass the connection +// to the empty socket so that NGINX will close the connection with an error in the error log -- +// no host in pass "" -- and set $status variable to 500 (logged by stream access log), +// which will indicate the problem to the user. +// https://nginx.org/en/docs/stream/ngx_stream_core_module.html#var_status const emptyStringSocket = `""` func executeMaps(conf dataplane.Configuration) []executeResult { diff --git a/internal/mode/static/nginx/config/maps_template.go b/internal/mode/static/nginx/config/maps_template.go index 5ea0fd5353..00604410c1 100644 --- a/internal/mode/static/nginx/config/maps_template.go +++ b/internal/mode/static/nginx/config/maps_template.go @@ -3,14 +3,13 @@ package config const mapsTemplateText = ` {{ range $m := . }} map {{ $m.Source }} {{ $m.Variable }} { - {{- if $m.UseHostnames -}} hostnames; {{ end }} - {{ range $p := $m.Parameters }} - {{ $p.Value }} {{ $p.Result }}; - {{ end }} + {{ range $p := $m.Parameters }} + {{ $p.Value }} {{ $p.Result }}; + {{ end }} } {{- end }} ` diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 83026b90a2..96a3c8381a 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -217,10 +217,10 @@ func TestExecuteStreamMaps(t *testing.T) { } expSubStrings := map[string]int{ - "example.com unix:/var/run/nginx/example.com8081.sock;": 1, - "example.com unix:/var/run/nginx/example.com8080.sock;": 1, - "cafe.example.com unix:/var/run/nginx/cafe.example.com8080.sock;": 1, - "app.example.com unix:/var/run/nginx/https8080.sock;": 1, + "example.com unix:/var/run/nginx/example.com-8081.sock;": 1, + "example.com unix:/var/run/nginx/example.com-8080.sock;": 1, + "cafe.example.com unix:/var/run/nginx/cafe.example.com-8080.sock;": 1, + "app.example.com unix:/var/run/nginx/https8080.sock;": 1, "hostnames": 2, } @@ -278,7 +278,7 @@ func TestCreateStreamMaps(t *testing.T) { Source: "$ssl_preread_server_name", Variable: getTLSPassthroughVarName(8081), Parameters: []shared.MapParameter{ - {Value: "example.com", Result: "unix:/var/run/nginx/example.com8081.sock"}, + {Value: "example.com", Result: getSocketNameTLS(8081, "example.com")}, }, UseHostnames: true, }, @@ -286,11 +286,11 @@ func TestCreateStreamMaps(t *testing.T) { Source: "$ssl_preread_server_name", Variable: getTLSPassthroughVarName(8080), Parameters: []shared.MapParameter{ - {Value: "example.com", Result: "unix:/var/run/nginx/example.com8080.sock"}, - {Value: "cafe.example.com", Result: "unix:/var/run/nginx/cafe.example.com8080.sock"}, - {Value: "wrong.example.com", Result: "\"\""}, - {Value: "app.example.com", Result: "unix:/var/run/nginx/https8080.sock"}, - {Value: "default", Result: "unix:/var/run/nginx/https8080.sock"}, + {Value: "example.com", Result: getSocketNameTLS(8080, "example.com")}, + {Value: "cafe.example.com", Result: getSocketNameTLS(8080, "cafe.example.com")}, + {Value: "wrong.example.com", Result: `""`}, + {Value: "app.example.com", Result: getSocketNameHTTPS(8080)}, + {Value: "default", Result: getSocketNameHTTPS(8080)}, }, UseHostnames: true, }, diff --git a/internal/mode/static/nginx/config/sockets.go b/internal/mode/static/nginx/config/sockets.go index f507d7d751..9707ef01a8 100644 --- a/internal/mode/static/nginx/config/sockets.go +++ b/internal/mode/static/nginx/config/sockets.go @@ -5,7 +5,7 @@ import ( ) func getSocketNameTLS(port int32, hostname string) string { - return fmt.Sprintf("unix:/var/run/nginx/%s%d.sock", hostname, port) + return fmt.Sprintf("unix:/var/run/nginx/%s-%d.sock", hostname, port) } func getSocketNameHTTPS(port int32) string { diff --git a/internal/mode/static/nginx/config/sockets_test.go b/internal/mode/static/nginx/config/sockets_test.go index fab3a02430..cbab84aea3 100644 --- a/internal/mode/static/nginx/config/sockets_test.go +++ b/internal/mode/static/nginx/config/sockets_test.go @@ -10,7 +10,7 @@ func TestGetSocketNameTLS(t *testing.T) { res := getSocketNameTLS(800, "*.cafe.example.com") g := NewGomegaWithT(t) - g.Expect(res).To(Equal("unix:/var/run/nginx/*.cafe.example.com800.sock")) + g.Expect(res).To(Equal("unix:/var/run/nginx/*.cafe.example.com-800.sock")) } func TestGetSocketNameHTTPS(t *testing.T) { From b98951e5c50b437af8e595223f1ee2a86ad8193b Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Fri, 5 Jul 2024 12:49:57 -0600 Subject: [PATCH 46/47] update upstream zone size and test --- .../mode/static/nginx/config/upstreams.go | 8 +- .../static/nginx/config/upstreams_test.go | 74 ++++++++----------- 2 files changed, 37 insertions(+), 45 deletions(-) diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index fcbca1128d..20f8df8a2d 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -23,6 +23,10 @@ const ( ossZoneSize = "512k" // plusZoneSize is the upstream zone size for nginx plus. plusZoneSize = "1m" + // ossZoneSize is the upstream zone size for nginx open source. + ossZoneSizeStream = "512k" + // plusZoneSize is the upstream zone size for nginx plus. + plusZoneSizeStream = "1m" ) func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeResult { @@ -60,9 +64,9 @@ func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []s } func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstream { - zoneSize := ossZoneSize + zoneSize := ossZoneSizeStream if g.plus { - zoneSize = plusZoneSize + zoneSize = plusZoneSizeStream } upstreamServers := make([]stream.UpstreamServer, len(up.Endpoints)) diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index 6514ee2d92..c764f9ef65 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -370,55 +370,43 @@ func TestCreateStreamUpstreams(t *testing.T) { func TestCreateStreamUpstream(t *testing.T) { gen := GeneratorImpl{} - tests := []struct { - msg string - stateUpstream dataplane.Upstream - expectedUpstream stream.Upstream - }{ - { - stateUpstream: dataplane.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, - }, - }, + up := dataplane.Upstream{ + Name: "multiple-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.1", + Port: 80, }, - expectedUpstream: stream.Upstream{ - Name: "multiple-endpoints", - ZoneSize: ossZoneSize, - Servers: []stream.UpstreamServer{ - { - Address: "10.0.0.1:80", - }, - { - Address: "10.0.0.2:80", - }, - { - Address: "10.0.0.3:80", - }, - }, + { + Address: "10.0.0.2", + Port: 80, + }, + { + Address: "10.0.0.3", + Port: 80, }, - msg: "multiple endpoints", }, } - for _, test := range tests { - t.Run(test.msg, func(t *testing.T) { - g := NewWithT(t) - result := gen.createStreamUpstream(test.stateUpstream) - g.Expect(result).To(Equal(test.expectedUpstream)) - }) + expectedUpstream := stream.Upstream{ + Name: "multiple-endpoints", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "10.0.0.1:80", + }, + { + Address: "10.0.0.2:80", + }, + { + Address: "10.0.0.3:80", + }, + }, } + + g := NewWithT(t) + result := gen.createStreamUpstream(up) + g.Expect(result).To(Equal(expectedUpstream)) } func TestCreateStreamUpstreamPlus(t *testing.T) { From 3f5ee8bb473341634b23559618332b8f83274181 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 8 Jul 2024 14:16:38 -0600 Subject: [PATCH 47/47] update comment for zone size upstream --- internal/mode/static/nginx/config/upstreams_template.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/config/upstreams_template.go b/internal/mode/static/nginx/config/upstreams_template.go index fd5130dc2b..a04915bec8 100644 --- a/internal/mode/static/nginx/config/upstreams_template.go +++ b/internal/mode/static/nginx/config/upstreams_template.go @@ -1,8 +1,9 @@ package config // FIXME(kate-osborn): Dynamically calculate upstream zone size based on the number of upstreams. -// 512k will support up to 648 upstream servers for OSS. -// NGINX Plus needs 1m to support roughly the same amount of servers (556 upstream servers). +// 512k will support up to 648 http upstream servers for OSS. +// NGINX Plus needs 1m to support roughly the same amount of http servers (556 upstream servers). +// For stream upstream servers, 512k will support 576 in OSS and 1m will support 991 in NGINX Plus // https://github.com/nginxinc/nginx-gateway-fabric/issues/483 const upstreamsTemplateText = ` {{ range $u := . }}