Skip to content

Commit 5abcc8a

Browse files
salonichf5salonichf5
committed
fix: fix reload errors due long matching conditions
1 parent f3e9b9c commit 5abcc8a

File tree

8 files changed

+302
-233
lines changed

8 files changed

+302
-233
lines changed

‎internal/mode/static/nginx/config/generator.go

+29-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package config
22

33
import (
4+
"encoding/json"
5+
"fmt"
46
"path/filepath"
57

68
"github.com./nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
@@ -23,6 +25,9 @@ const (
2325

2426
// configVersionFile is the path to the config version configuration file.
2527
configVersionFile = httpFolder + "/config-version.conf"
28+
29+
// httpMatchVarsFile is the path to the http_match pairs configuration file.
30+
httpMatchVarsFile = httpFolder + "/match.json"
2631
)
2732

2833
// ConfigFolders is a list of folders where NGINX configuration files are stored.
@@ -66,7 +71,8 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File {
6671
files = append(files, generatePEM(id, pair.Cert, pair.Key))
6772
}
6873

69-
files = append(files, g.generateHTTPConfig(conf))
74+
httpFiles := g.generateHTTPConfig(conf)
75+
files = append(files, httpFiles...)
7076

7177
files = append(files, generateConfigVersion(conf.Version))
7278

@@ -106,24 +112,43 @@ func generateCertBundleFileName(id dataplane.CertBundleID) string {
106112
return filepath.Join(secretsFolder, string(id)+".crt")
107113
}
108114

109-
func (g GeneratorImpl) generateHTTPConfig(conf dataplane.Configuration) file.File {
115+
func (g GeneratorImpl) generateHTTPConfig(conf dataplane.Configuration) []file.File {
110116
var c []byte
111117
for _, execute := range g.getExecuteFuncs() {
112118
c = append(c, execute(conf)...)
113119
}
114120

115-
return file.File{
121+
servers, httpMatchPairs := executeServers(conf)
122+
123+
// create server conf
124+
serverConf := execute(serversTemplate, servers)
125+
c = append(c, serverConf...)
126+
127+
httpConf := file.File{
116128
Content: c,
117129
Path: httpConfigFile,
118130
Type: file.TypeRegular,
119131
}
132+
133+
// create httpMatchPair conf
134+
b, err := json.Marshal(httpMatchPairs)
135+
if err != nil {
136+
// panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly.
137+
panic(fmt.Errorf("could not marshal http match pairs: %w", err))
138+
}
139+
matchConf := file.File{
140+
Content: b,
141+
Path: httpMatchVarsFile,
142+
Type: file.TypeRegular,
143+
}
144+
145+
return []file.File{httpConf, matchConf}
120146
}
121147

122148
func (g GeneratorImpl) getExecuteFuncs() []executeFunc {
123149
return []executeFunc{
124150
g.executeUpstreams,
125151
executeSplitClients,
126-
executeServers,
127152
executeMaps,
128153
}
129154
}

‎internal/mode/static/nginx/config/generator_test.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func TestGenerate(t *testing.T) {
7070

7171
files := generator.Generate(conf)
7272

73-
g.Expect(files).To(HaveLen(4))
73+
g.Expect(files).To(HaveLen(5))
7474

7575
g.Expect(files[0]).To(Equal(file.File{
7676
Type: file.TypeSecret,
@@ -88,12 +88,15 @@ func TestGenerate(t *testing.T) {
8888
g.Expect(httpCfg).To(ContainSubstring("upstream"))
8989
g.Expect(httpCfg).To(ContainSubstring("split_clients"))
9090

91+
g.Expect(files[2].Path).To(Equal("/etc/nginx/conf.d/match.json"))
9192
g.Expect(files[2].Type).To(Equal(file.TypeRegular))
92-
g.Expect(files[2].Path).To(Equal("/etc/nginx/conf.d/config-version.conf"))
93-
configVersion := string(files[2].Content)
93+
94+
g.Expect(files[3].Type).To(Equal(file.TypeRegular))
95+
g.Expect(files[3].Path).To(Equal("/etc/nginx/conf.d/config-version.conf"))
96+
configVersion := string(files[3].Content)
9497
g.Expect(configVersion).To(ContainSubstring(fmt.Sprintf("return 200 %d", conf.Version)))
9598

96-
g.Expect(files[3].Path).To(Equal("/etc/nginx/secrets/test-certbundle.crt"))
97-
certBundle := string(files[3].Content)
99+
g.Expect(files[4].Path).To(Equal("/etc/nginx/secrets/test-certbundle.crt"))
100+
certBundle := string(files[4].Content)
98101
g.Expect(certBundle).To(Equal("test-cert"))
99102
}

‎internal/mode/static/nginx/config/http/config.go

+22-4
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ type Server struct {
1212

1313
// Location holds all configuration for an HTTP location.
1414
type Location struct {
15-
Return *Return
16-
ProxySSLVerify *ProxySSLVerify
1715
Path string
1816
ProxyPass string
19-
HTTPMatchVar string
20-
Rewrites []string
17+
HTTPMatchKey string
2118
ProxySetHeaders []Header
19+
ProxySSLVerify *ProxySSLVerify
20+
Return *Return
21+
Rewrites []string
2222
}
2323

2424
// Header defines a HTTP header to be passed to the proxied server.
@@ -93,3 +93,21 @@ type ProxySSLVerify struct {
9393
TrustedCertificate string
9494
Name string
9595
}
96+
97+
// httpMatch is an internal representation of an HTTPRouteMatch.
98+
// This struct is marshaled into a string and stored as a variable in the nginx location block for the route's path.
99+
// The NJS httpmatches module will look up this variable on the request object and compare the request against the
100+
// Method, Headers, and QueryParams contained in httpMatch.
101+
// If the request satisfies the httpMatch, NGINX will redirect the request to the location RedirectPath.
102+
type RouteMatch struct {
103+
// Method is the HTTPMethod of the HTTPRouteMatch.
104+
Method string `json:"method,omitempty"`
105+
// RedirectPath is the path to redirect the request to if the request satisfies the match conditions.
106+
RedirectPath string `json:"redirectPath,omitempty"`
107+
// Headers is a list of HTTPHeaders name value pairs with the format "{name}:{value}".
108+
Headers []string `json:"headers,omitempty"`
109+
// QueryParams is a list of HTTPQueryParams name value pairs with the format "{name}={value}".
110+
QueryParams []string `json:"params,omitempty"`
111+
// Any represents a match with no match conditions.
112+
Any bool `json:"any,omitempty"`
113+
}

‎internal/mode/static/nginx/config/servers.go

+67-59
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
package config
22

33
import (
4-
"encoding/json"
54
"fmt"
5+
"regexp"
6+
"strconv"
67
"strings"
78
gotemplate "text/template"
89

@@ -38,58 +39,82 @@ var baseHeaders = []http.Header{
3839
},
3940
}
4041

41-
func executeServers(conf dataplane.Configuration) []byte {
42-
servers := createServers(conf.HTTPServers, conf.SSLServers)
42+
func executeServers(conf dataplane.Configuration) ([]http.Server, map[string][]http.RouteMatch) {
43+
servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers)
4344

44-
return execute(serversTemplate, servers)
45+
return servers, httpMatchPairs
4546
}
4647

47-
func createServers(httpServers, sslServers []dataplane.VirtualServer) []http.Server {
48+
func createServers(httpServers, sslServers []dataplane.VirtualServer) (
49+
[]http.Server,
50+
map[string][]http.RouteMatch,
51+
) {
4852
servers := make([]http.Server, 0, len(httpServers)+len(sslServers))
53+
finalMatchPairs := make(map[string][]http.RouteMatch)
4954

5055
for _, s := range httpServers {
51-
servers = append(servers, createServer(s))
56+
httpServer, matchPair := createServer(s)
57+
servers = append(servers, httpServer)
58+
59+
for key, val := range matchPair {
60+
finalMatchPairs[key] = val
61+
}
5262
}
5363

5464
for _, s := range sslServers {
55-
servers = append(servers, createSSLServer(s))
65+
sslServer, matchPair := createSSLServer(s)
66+
servers = append(servers, sslServer)
67+
68+
for key, val := range matchPair {
69+
finalMatchPairs[key] = val
70+
}
5671
}
5772

58-
return servers
73+
return servers, finalMatchPairs
5974
}
6075

61-
func createSSLServer(virtualServer dataplane.VirtualServer) http.Server {
76+
func createSSLServer(virtualServer dataplane.VirtualServer) (
77+
http.Server,
78+
map[string][]http.RouteMatch,
79+
) {
6280
if virtualServer.IsDefault {
6381
return http.Server{
6482
IsDefaultSSL: true,
6583
Port: virtualServer.Port,
66-
}
84+
}, nil
6785
}
6886

87+
locs, matchPairs := createLocations(virtualServer)
88+
6989
return http.Server{
7090
ServerName: virtualServer.Hostname,
7191
SSL: &http.SSL{
7292
Certificate: generatePEMFileName(virtualServer.SSL.KeyPairID),
7393
CertificateKey: generatePEMFileName(virtualServer.SSL.KeyPairID),
7494
},
75-
Locations: createLocations(virtualServer.PathRules, virtualServer.Port),
95+
Locations: locs,
7696
Port: virtualServer.Port,
77-
}
97+
}, matchPairs
7898
}
7999

80-
func createServer(virtualServer dataplane.VirtualServer) http.Server {
100+
func createServer(virtualServer dataplane.VirtualServer) (
101+
http.Server,
102+
map[string][]http.RouteMatch,
103+
) {
81104
if virtualServer.IsDefault {
82105
return http.Server{
83106
IsDefaultHTTP: true,
84107
Port: virtualServer.Port,
85-
}
108+
}, nil
86109
}
87110

111+
locs, matchPairs := createLocations(virtualServer)
112+
88113
return http.Server{
89114
ServerName: virtualServer.Hostname,
90-
Locations: createLocations(virtualServer.PathRules, virtualServer.Port),
115+
Locations: locs,
91116
Port: virtualServer.Port,
92-
}
117+
}, matchPairs
93118
}
94119

95120
// rewriteConfig contains the configuration for a location to rewrite paths,
@@ -99,13 +124,19 @@ type rewriteConfig struct {
99124
Rewrite string
100125
}
101126

102-
func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.Location {
103-
maxLocs, pathsAndTypes := getMaxLocationCountAndPathMap(pathRules)
127+
type httpMatchPairs map[string][]http.RouteMatch
128+
129+
func createLocations(server dataplane.VirtualServer) (
130+
[]http.Location,
131+
map[string][]http.RouteMatch,
132+
) {
133+
maxLocs, pathsAndTypes := getMaxLocationCountAndPathMap(server.PathRules)
104134
locs := make([]http.Location, 0, maxLocs)
135+
matchPairs := make(httpMatchPairs)
105136
var rootPathExists bool
106137

107-
for pathRuleIdx, rule := range pathRules {
108-
matches := make([]httpMatch, 0, len(rule.MatchRules))
138+
for pathRuleIdx, rule := range server.PathRules {
139+
matches := make([]http.RouteMatch, 0, len(rule.MatchRules))
109140

110141
if rule.Path == rootPath {
111142
rootPathExists = true
@@ -121,14 +152,15 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.
121152
matches = append(matches, match)
122153
}
123154

124-
buildLocations = updateLocationsForFilters(r.Filters, buildLocations, r, listenerPort, rule.Path)
155+
buildLocations = updateLocationsForFilters(r.Filters, buildLocations, r, server.Port, rule.Path)
125156
locs = append(locs, buildLocations...)
126157
}
127158

128159
if len(matches) > 0 {
129-
matchesStr := convertMatchesToString(matches)
130160
for i := range extLocations {
131-
extLocations[i].HTTPMatchVar = matchesStr
161+
key := server.Hostname + extLocations[i].Path + strconv.Itoa(int(server.Port))
162+
extLocations[i].HTTPMatchKey = sanitizeKey(key)
163+
matchPairs[extLocations[i].HTTPMatchKey] = matches
132164
}
133165
locs = append(locs, extLocations...)
134166
}
@@ -138,7 +170,14 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.
138170
locs = append(locs, createDefaultRootLocation())
139171
}
140172

141-
return locs
173+
return locs, matchPairs
174+
}
175+
176+
// removeSpecialCharacters removes '/', '.' from key and replaces '= ' with 'EXACT',
177+
// to avoid compilation issues with NJS and NGINX Conf.
178+
func sanitizeKey(input string) string {
179+
s := regexp.MustCompile("[./]").ReplaceAllString(input, "")
180+
return regexp.MustCompile("= ").ReplaceAllString(s, "EXACT")
142181
}
143182

144183
// pathAndTypeMap contains a map of paths and any path types defined for that path
@@ -217,9 +256,9 @@ func initializeInternalLocation(
217256
pathruleIdx,
218257
matchRuleIdx int,
219258
match dataplane.Match,
220-
) (http.Location, httpMatch) {
259+
) (http.Location, http.RouteMatch) {
221260
path := fmt.Sprintf("@rule%d-route%d", pathruleIdx, matchRuleIdx)
222-
return createMatchLocation(path), createHTTPMatch(match, path)
261+
return createMatchLocation(path), createRouteMatch(match, path)
223262
}
224263

225264
// updateLocationsForFilters updates the existing locations with any relevant filters.
@@ -392,26 +431,8 @@ func createRewritesValForRewriteFilter(filter *dataplane.HTTPURLRewriteFilter, p
392431
return rewrites
393432
}
394433

395-
// httpMatch is an internal representation of an HTTPRouteMatch.
396-
// This struct is marshaled into a string and stored as a variable in the nginx location block for the route's path.
397-
// The NJS httpmatches module will look up this variable on the request object and compare the request against the
398-
// Method, Headers, and QueryParams contained in httpMatch.
399-
// If the request satisfies the httpMatch, NGINX will redirect the request to the location RedirectPath.
400-
type httpMatch struct {
401-
// Method is the HTTPMethod of the HTTPRouteMatch.
402-
Method string `json:"method,omitempty"`
403-
// RedirectPath is the path to redirect the request to if the request satisfies the match conditions.
404-
RedirectPath string `json:"redirectPath,omitempty"`
405-
// Headers is a list of HTTPHeaders name value pairs with the format "{name}:{value}".
406-
Headers []string `json:"headers,omitempty"`
407-
// QueryParams is a list of HTTPQueryParams name value pairs with the format "{name}={value}".
408-
QueryParams []string `json:"params,omitempty"`
409-
// Any represents a match with no match conditions.
410-
Any bool `json:"any,omitempty"`
411-
}
412-
413-
func createHTTPMatch(match dataplane.Match, redirectPath string) httpMatch {
414-
hm := httpMatch{
434+
func createRouteMatch(match dataplane.Match, redirectPath string) http.RouteMatch {
435+
hm := http.RouteMatch{
415436
RedirectPath: redirectPath,
416437
}
417438

@@ -558,19 +579,6 @@ func convertSetHeaders(headers []dataplane.HTTPHeader) []http.Header {
558579
return locHeaders
559580
}
560581

561-
func convertMatchesToString(matches []httpMatch) string {
562-
// FIXME(sberman): De-dupe matches and associated locations
563-
// so we don't need nginx/njs to perform unnecessary matching.
564-
// https://github.com./nginxinc/nginx-gateway-fabric/issues/662
565-
b, err := json.Marshal(matches)
566-
if err != nil {
567-
// panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly.
568-
panic(fmt.Errorf("could not marshal http match: %w", err))
569-
}
570-
571-
return string(b)
572-
}
573-
574582
func exactPath(path string) string {
575583
return fmt.Sprintf("= %s", path)
576584
}

‎internal/mode/static/nginx/config/servers_template.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ server {
1717
}
1818
{{- else }}
1919
server {
20+
js_preload_object matches from /etc/nginx/conf.d/match.json;
2021
{{- if $s.SSL }}
2122
listen {{ $s.Port }} ssl;
2223
ssl_certificate {{ $s.SSL.Certificate }};
@@ -41,8 +42,8 @@ server {
4142
return {{ $l.Return.Code }} "{{ $l.Return.Body }}";
4243
{{- end }}
4344
44-
{{- if $l.HTTPMatchVar }}
45-
set $http_matches {{ $l.HTTPMatchVar | printf "%q" }};
45+
{{- if $l.HTTPMatchKey }}
46+
set $match_key {{ $l.HTTPMatchKey }};
4647
js_content httpmatches.redirect;
4748
{{- end }}
4849

0 commit comments

Comments
 (0)