From 2607f70f6e4d076c4d25c26c2463f7860c07824d Mon Sep 17 00:00:00 2001 From: Saloni Date: Fri, 22 Mar 2024 00:54:06 -0600 Subject: [PATCH] fix: reload erros due to large num of matches --- .../mode/static/nginx/config/http/config.go | 2 +- internal/mode/static/nginx/config/servers.go | 45 ++++++++++++++++--- .../static/nginx/config/servers_template.go | 6 +-- .../mode/static/nginx/config/servers_test.go | 5 ++- .../static/nginx/modules/src/httpmatches.js | 28 +++++++----- 5 files changed, 62 insertions(+), 24 deletions(-) diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 8486b7d464..7606d9aeaa 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -16,7 +16,7 @@ type Location struct { ProxySSLVerify *ProxySSLVerify Path string ProxyPass string - HTTPMatchVar string + HTTPMatchVar []string Rewrites []string ProxySetHeaders []Header } diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index a80de123f7..d704f8be91 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -1,6 +1,7 @@ package config import ( + "bytes" "encoding/json" "fmt" "strings" @@ -16,6 +17,7 @@ const ( // HeaderMatchSeparator is the separator for constructing header-based match for NJS. HeaderMatchSeparator = ":" rootPath = "/" + matchMaxLength = 2000 // TODO: account for character escaping when mapped to config ) // baseHeaders contains the constant headers set in each server block @@ -558,17 +560,48 @@ func convertSetHeaders(headers []dataplane.HTTPHeader) []http.Header { return locHeaders } -func convertMatchesToString(matches []httpMatch) string { +func convertMatchesToString(matches []httpMatch) []string { // FIXME(sberman): De-dupe matches and associated locations // so we don't need nginx/njs to perform unnecessary matching. // https://github.com/nginxinc/nginx-gateway-fabric/issues/662 - b, err := json.Marshal(matches) - if err != nil { - // panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly. - panic(fmt.Errorf("could not marshal http match: %w", err)) + + var matchList []string + + var buffer bytes.Buffer + for _, match := range matches { + matchBytes, err := json.Marshal(match) + if err != nil { + // panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly. + panic(fmt.Errorf("could not marshal http match: %w", err)) + } + matchLen := len(matchBytes) + if buffer.Len()+matchLen+4 > matchMaxLength { + // Flush buffer if adding current match exceeds the maximum length + buffer.WriteString("]") + matchList = append(matchList, buffer.String()) + buffer.Reset() + buffer.WriteString("[") + } else { + if buffer.Len() > 1 { + // Add comma if not first match in the buffer + buffer.WriteString(",") + } else { + // Add opening braces if the first match in the buffer + buffer.WriteString("[") + } + } + buffer.Write(matchBytes) + } + + if buffer.Len() > 0 { + buffer.WriteString("]") + matchList = append(matchList, buffer.String()) } - return string(b) + for _, m := range matchList { + fmt.Println("match --> ", m) + } + return matchList } func exactPath(path string) string { diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index dbf37575ae..af11cdb264 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -41,10 +41,10 @@ server { return {{ $l.Return.Code }} "{{ $l.Return.Body }}"; {{- end }} - {{- if $l.HTTPMatchVar }} - set $http_matches {{ $l.HTTPMatchVar | printf "%q" }}; - js_content httpmatches.redirect; + {{ range $i, $h := $l.HTTPMatchVar }} + set $http_matches_{{ $i }} {{ $h | printf "%q" }}; {{- end }} + js_content httpmatches.redirect; {{- if $l.ProxyPass -}} {{ range $h := $l.ProxySetHeaders }} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 218e59d9d3..f6a8dacef4 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -508,11 +508,12 @@ func TestCreateServers(t *testing.T) { }, } - expectedMatchString := func(m []httpMatch) string { + expectedMatchString := func(m []httpMatch) []string { g := NewWithT(t) b, err := json.Marshal(m) g.Expect(err).ToNot(HaveOccurred()) - return string(b) + str := string(b) + return []string{str} } slashMatches := []httpMatch{ diff --git a/internal/mode/static/nginx/modules/src/httpmatches.js b/internal/mode/static/nginx/modules/src/httpmatches.js index 0fcf5ef210..29c353a3f6 100644 --- a/internal/mode/static/nginx/modules/src/httpmatches.js +++ b/internal/mode/static/nginx/modules/src/httpmatches.js @@ -50,22 +50,26 @@ function redirect(r) { } function extractMatchesFromRequest(r) { - if (!r.variables[MATCHES_VARIABLE]) { - throw Error( - `cannot redirect the request; the variable ${MATCHES_VARIABLE} is not defined on the request object`, - ); - } - + let index = 0; let matches; - try { - matches = JSON.parse(r.variables[MATCHES_VARIABLE]); - } catch (e) { - throw Error( - `cannot redirect the request; error parsing ${r.variables[MATCHES_VARIABLE]} into a JSON object: ${e}`, - ); + while (true) { + const MATCHES_VARIABLE = `http_matches_${index}`; + if (!r.variables[MATCHES_VARIABLE]) { + break; + } + + try { + matches = JSON.parse(r.variables[MATCHES_VARIABLE]); + } catch (e) { + throw Error( + `cannot redirect the request; error parsing ${r.variables[MATCHES_VARIABLE]} into a JSON object: ${e}`, + ); + } } + console.log('matches in js', matches); + if (!Array.isArray(matches)) { throw Error(`cannot redirect the request; expected a list of matches, got ${matches}`); }