From 6533296e7127b0cfaf5eb2e5cef13acb7a0c93db Mon Sep 17 00:00:00 2001 From: Jim Ryan Date: Sun, 26 Nov 2023 14:11:36 +0000 Subject: [PATCH] Refactor virtual server listens with makeHTTP(s)Listener functions (#4693) * makeHTTP(s)Listener and tests * fix test * reduce cyclomatic complexity * gofumpt and unexport enums * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * reduce code paths * fix lint * fix formatting * fix tests with spacing * merge main --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../version2/nginx-plus.virtualserver.tmpl | 26 +--- .../configs/version2/nginx.virtualserver.tmpl | 27 +--- internal/configs/version2/template_helper.go | 101 +++++++++++++++ .../configs/version2/template_helper_test.go | 121 ++++++++++++++++++ 4 files changed, 230 insertions(+), 45 deletions(-) diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index c17d27d84a..5a7f512c92 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -65,15 +65,7 @@ proxy_cache_path /var/cache/nginx/jwks_uri_{{$s.VSName}} levels=1 keys_zone=jwks server { {{- if $s.Gunzip }}gunzip on;{{end}} - {{- if not $s.CustomListeners }} - listen 80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{ if not $s.DisableIPV6 }}listen [::]:80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- else }} - {{- if (gt $s.HTTPPort 0) }} - listen {{ $s.HTTPPort }}{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{ if not $s.DisableIPV6 }}listen [::]:{{ $s.HTTPPort }}{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- end }} - {{- end }} + {{ makeHTTPListener $s | printf }} server_name {{ $s.ServerName }}; status_zone {{ $s.StatusZone }}; @@ -105,21 +97,11 @@ server { set_real_ip_from unix:; real_ip_header proxy_protocol; {{- else }} - {{- if not $s.CustomListeners }} - listen 443 ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{- if not $s.DisableIPV6 }}listen [::]:443 ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- else }} - {{- if (gt $s.HTTPSPort 0) }} - listen {{ $s.HTTPSPort }} ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{- if not $s.DisableIPV6 }}listen [::]:{{ $s.HTTPSPort }} ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- end }} + {{ makeHTTPSListener $s | printf }} {{- end }} - {{- end }} - - {{- if $ssl.HTTP2 }} + {{- if $ssl.HTTP2 }} http2 on; - {{- end }} - + {{- end }} {{- if $ssl.RejectHandshake }} ssl_reject_handshake on; diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 8a0db0d376..4d5811e725 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -42,15 +42,7 @@ limit_req_zone {{ $z.Key }} zone={{ $z.ZoneName }}:{{ $z.ZoneSize }} rate={{ $z. {{- $s := .Server }} server { {{- if $s.Gunzip }}gunzip on;{{end}} - {{- if not $s.CustomListeners }} - listen 80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{ if not $s.DisableIPV6 }}listen [::]:80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- else }} - {{- if (gt $s.HTTPPort 0) }} - listen {{ $s.HTTPPort }}{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{ if not $s.DisableIPV6 }}listen [::]:{{ $s.HTTPPort }}{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- end }} - {{- end }} + {{ makeHTTPListener $s | printf }} server_name {{ $s.ServerName }}; @@ -58,28 +50,17 @@ server { set $resource_name "{{$s.VSName}}"; set $resource_namespace "{{$s.VSNamespace}}"; - {{- with $ssl := $s.SSL }} {{- if $s.TLSPassthrough }} listen unix:/var/lib/nginx/passthrough-https.sock proxy_protocol; set_real_ip_from unix:; real_ip_header proxy_protocol; {{- else }} - {{- if not $s.CustomListeners }} - listen 443 ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{- if not $s.DisableIPV6 }}listen [::]:443 ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- else }} - {{- if (gt $s.HTTPSPort 0) }} - listen {{ $s.HTTPSPort }} ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; - {{- if not $s.DisableIPV6 }}listen [::]:{{ $s.HTTPSPort }} ssl{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }} - {{- end }} + {{ makeHTTPSListener $s | printf }} {{- end }} - {{- end }} - - {{- if $ssl.HTTP2 }} + {{- if $ssl.HTTP2 }} http2 on; - {{- end }} - + {{- end }} {{- if $ssl.RejectHandshake }} ssl_reject_handshake on; diff --git a/internal/configs/version2/template_helper.go b/internal/configs/version2/template_helper.go index 22289e3e6a..c09e26a9ce 100644 --- a/internal/configs/version2/template_helper.go +++ b/internal/configs/version2/template_helper.go @@ -1,10 +1,27 @@ package version2 import ( + "strconv" "strings" "text/template" ) +type protocol int + +const ( + http protocol = iota + https +) + +type listenerType int + +const ( + ipv4 listenerType = iota + ipv6 +) + +const spacing = " " + func headerListToCIMap(headers []Header) map[string]string { ret := make(map[string]string) @@ -20,6 +37,88 @@ func hasCIKey(key string, d map[string]string) bool { return ok } +func makeListener(listenerType protocol, s Server) string { + var directives string + + if !s.CustomListeners { + directives += buildDefaultListenerDirectives(listenerType, s) + } else { + directives += buildCustomListenerDirectives(listenerType, s) + } + + return directives +} + +func buildDefaultListenerDirectives(listenerType protocol, s Server) string { + var directives string + port := getDefaultPort(listenerType) + + directives += buildListenDirective(port, s.ProxyProtocol, ipv4) + + if !s.DisableIPV6 { + directives += spacing + directives += buildListenDirective(port, s.ProxyProtocol, ipv6) + } + + return directives +} + +func buildCustomListenerDirectives(listenerType protocol, s Server) string { + var directives string + + if (listenerType == http && s.HTTPPort > 0) || (listenerType == https && s.HTTPSPort > 0) { + port := getCustomPort(listenerType, s) + directives += buildListenDirective(port, s.ProxyProtocol, ipv4) + + if !s.DisableIPV6 { + directives += spacing + directives += buildListenDirective(port, s.ProxyProtocol, ipv6) + } + } + + return directives +} + +func getDefaultPort(listenerType protocol) string { + if listenerType == http { + return "80" + } + return "443 ssl" +} + +func getCustomPort(listenerType protocol, s Server) string { + if listenerType == http { + return strconv.Itoa(s.HTTPPort) + } + return strconv.Itoa(s.HTTPSPort) + " ssl" +} + +func buildListenDirective(port string, proxyProtocol bool, listenType listenerType) string { + base := "listen" + var directive string + + if listenType == ipv6 { + directive = base + " [::]:" + port + } else { + directive = base + " " + port + } + + if proxyProtocol { + directive += " proxy_protocol" + } + + directive += ";\n" + return directive +} + +func makeHTTPListener(s Server) string { + return makeListener(http, s) +} + +func makeHTTPSListener(s Server) string { + return makeListener(https, s) +} + var helperFunctions = template.FuncMap{ "headerListToCIMap": headerListToCIMap, "hasCIKey": hasCIKey, @@ -28,4 +127,6 @@ var helperFunctions = template.FuncMap{ "hasSuffix": strings.HasSuffix, "toLower": strings.ToLower, "toUpper": strings.ToUpper, + "makeHTTPListener": makeHTTPListener, + "makeHTTPSListener": makeHTTPSListener, } diff --git a/internal/configs/version2/template_helper_test.go b/internal/configs/version2/template_helper_test.go index dfba45a8b3..a96855e792 100644 --- a/internal/configs/version2/template_helper_test.go +++ b/internal/configs/version2/template_helper_test.go @@ -145,6 +145,127 @@ func TestToUpperInputString(t *testing.T) { } } +func TestMakeHTTPListener(t *testing.T) { + t.Parallel() + + testCases := []struct { + server Server + expected string + }{ + {server: Server{ + CustomListeners: false, + DisableIPV6: true, + ProxyProtocol: false, + }, expected: "listen 80;\n"}, + {server: Server{ + CustomListeners: false, + DisableIPV6: false, + ProxyProtocol: false, + }, expected: "listen 80;\n listen [::]:80;\n"}, + {server: Server{ + CustomListeners: false, + DisableIPV6: true, + ProxyProtocol: true, + }, expected: "listen 80 proxy_protocol;\n"}, + {server: Server{ + CustomListeners: false, + DisableIPV6: false, + ProxyProtocol: true, + }, expected: "listen 80 proxy_protocol;\n listen [::]:80 proxy_protocol;\n"}, + {server: Server{ + CustomListeners: true, + HTTPPort: 81, + DisableIPV6: true, + ProxyProtocol: false, + }, expected: "listen 81;\n"}, + {server: Server{ + CustomListeners: true, + HTTPPort: 81, + DisableIPV6: false, + ProxyProtocol: false, + }, expected: "listen 81;\n listen [::]:81;\n"}, + {server: Server{ + CustomListeners: true, + HTTPPort: 81, + DisableIPV6: true, + ProxyProtocol: true, + }, expected: "listen 81 proxy_protocol;\n"}, + {server: Server{ + CustomListeners: true, + HTTPPort: 81, + DisableIPV6: false, + ProxyProtocol: true, + }, expected: "listen 81 proxy_protocol;\n listen [::]:81 proxy_protocol;\n"}, + } + + for _, tc := range testCases { + got := makeHTTPListener(tc.server) + if got != tc.expected { + t.Errorf("Function generated wrong config, got %v but expected %v.", got, tc.expected) + } + } +} + +func TestMakeHTTPSListener(t *testing.T) { + t.Parallel() + + testCases := []struct { + server Server + expected string + }{ + {server: Server{ + CustomListeners: false, + DisableIPV6: true, + ProxyProtocol: false, + }, expected: "listen 443 ssl;\n"}, + {server: Server{ + CustomListeners: false, + DisableIPV6: false, + ProxyProtocol: false, + }, expected: "listen 443 ssl;\n listen [::]:443 ssl;\n"}, + {server: Server{ + CustomListeners: false, + DisableIPV6: true, + ProxyProtocol: true, + }, expected: "listen 443 ssl proxy_protocol;\n"}, + {server: Server{ + CustomListeners: false, + DisableIPV6: false, + ProxyProtocol: true, + }, expected: "listen 443 ssl proxy_protocol;\n listen [::]:443 ssl proxy_protocol;\n"}, + {server: Server{ + CustomListeners: true, + HTTPSPort: 444, + DisableIPV6: true, + ProxyProtocol: false, + }, expected: "listen 444 ssl;\n"}, + {server: Server{ + CustomListeners: true, + HTTPSPort: 444, + DisableIPV6: false, + ProxyProtocol: false, + }, expected: "listen 444 ssl;\n listen [::]:444 ssl;\n"}, + {server: Server{ + CustomListeners: true, + HTTPSPort: 444, + DisableIPV6: true, + ProxyProtocol: true, + }, expected: "listen 444 ssl proxy_protocol;\n"}, + {server: Server{ + CustomListeners: true, + HTTPSPort: 444, + DisableIPV6: false, + ProxyProtocol: true, + }, expected: "listen 444 ssl proxy_protocol;\n listen [::]:444 ssl proxy_protocol;\n"}, + } + for _, tc := range testCases { + got := makeHTTPSListener(tc.server) + if got != tc.expected { + t.Errorf("Function generated wrong config, got %v but expected %v.", got, tc.expected) + } + } +} + func newContainsTemplate(t *testing.T) *template.Template { t.Helper() tmpl, err := template.New("testTemplate").Funcs(helperFunctions).Parse(`{{contains .InputString .Substring}}`)