Skip to content

Commit

Permalink
update backends and upstreams with a prefix
Browse files Browse the repository at this point in the history
  • Loading branch information
@salonichf5
salonichf5 committed Oct 28, 2024
1 parent e044a07 commit 1c477e4
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 85 deletions.
8 changes: 6 additions & 2 deletions internal/mode/static/nginx/config/servers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,11 +738,15 @@ func createProxyPass(
}

backendName := backendGroupName(backendGroup)
// The proxy_pass directive value starts with the namespace name of the backend group.
// We prefix it with "upstream_" to avoid potential situation where the namespace name starts
// with a number, which is not allowed in a variable name in nginx.conf
updatedBackendName := shared.UpstreamPrefix + backendName
if backendGroupNeedsSplit(backendGroup) {
return protocol + "://$" + convertStringToSafeVariableName(backendName) + requestURI
return protocol + "://$" + convertStringToSafeVariableName(updatedBackendName) + requestURI
}

return protocol + "://" + backendName + requestURI
return protocol + "://" + updatedBackendName + requestURI
}

func createMatchLocation(path string, grpc bool) http.Location {
Expand Down
74 changes: 37 additions & 37 deletions internal/mode/static/nginx/config/servers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1123,21 +1123,21 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "/_ngf-internal-rule0-route0",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.InternalLocationType,
Includes: internalIncludes,
},
{
Path: "/_ngf-internal-rule0-route1",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.InternalLocationType,
Includes: internalIncludes,
},
{
Path: "/_ngf-internal-rule0-route2",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.InternalLocationType,
Includes: internalIncludes,
Expand All @@ -1150,28 +1150,28 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "/_ngf-internal-rule1-route0",
ProxyPass: "http://$test__route1_rule1$request_uri",
ProxyPass: "http://$upstream_test__route1_rule1$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.InternalLocationType,
Includes: internalIncludes,
},
{
Path: "/path-only/",
ProxyPass: "http://invalid-backend-ref$request_uri",
ProxyPass: "http://upstream_invalid-backend-ref$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
Includes: externalIncludes,
},
{
Path: "= /path-only",
ProxyPass: "http://invalid-backend-ref$request_uri",
ProxyPass: "http://upstream_invalid-backend-ref$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
Includes: externalIncludes,
},
{
Path: "/backend-tls-policy/",
ProxyPass: "https://test_btp_80$request_uri",
ProxyPass: "https://upstream_test_btp_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
ProxySSLVerify: &http.ProxySSLVerify{
Name: "test-btp.example.com",
Expand All @@ -1182,7 +1182,7 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "= /backend-tls-policy",
ProxyPass: "https://test_btp_80$request_uri",
ProxyPass: "https://upstream_test_btp_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
ProxySSLVerify: &http.ProxySSLVerify{
Name: "test-btp.example.com",
Expand Down Expand Up @@ -1251,15 +1251,15 @@ func TestCreateServers(t *testing.T) {
{
Path: "/rewrite/",
Rewrites: []string{"^ /replacement break"},
ProxyPass: "http://test_foo_80",
ProxyPass: "http://upstream_test_foo_80",
ProxySetHeaders: rewriteProxySetHeaders,
Type: http.ExternalLocationType,
Includes: externalIncludes,
},
{
Path: "= /rewrite",
Rewrites: []string{"^ /replacement break"},
ProxyPass: "http://test_foo_80",
ProxyPass: "http://upstream_test_foo_80",
ProxySetHeaders: rewriteProxySetHeaders,
Type: http.ExternalLocationType,
Includes: externalIncludes,
Expand All @@ -1279,7 +1279,7 @@ func TestCreateServers(t *testing.T) {
{
Path: "/_ngf-internal-rule8-route0",
Rewrites: []string{"^ $request_uri", "^/rewrite-with-headers(.*)$ /prefix-replacement$1 break"},
ProxyPass: "http://test_foo_80",
ProxyPass: "http://upstream_test_foo_80",
ProxySetHeaders: rewriteProxySetHeaders,
Type: http.InternalLocationType,
Includes: internalIncludes,
Expand Down Expand Up @@ -1322,7 +1322,7 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "= /exact",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
Includes: externalIncludes,
Expand All @@ -1335,14 +1335,14 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "/_ngf-internal-rule12-route0",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.InternalLocationType,
Includes: internalIncludes,
},
{
Path: "/proxy-set-headers/",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: []http.Header{
{
Name: "my-header",
Expand Down Expand Up @@ -1396,7 +1396,7 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "= /proxy-set-headers",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: []http.Header{
{
Name: "my-header",
Expand Down Expand Up @@ -1450,15 +1450,15 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "= /grpc/method",
ProxyPass: "grpc://test_foo_80",
ProxyPass: "grpc://upstream_test_foo_80",
GRPC: true,
ProxySetHeaders: grpcBaseHeaders,
Type: http.ExternalLocationType,
Includes: externalIncludes,
},
{
Path: "= /grpc-with-backend-tls-policy/method",
ProxyPass: "grpcs://test_btp_80",
ProxyPass: "grpcs://upstream_test_btp_80",
ProxySSLVerify: &http.ProxySSLVerify{
Name: "test-btp.example.com",
TrustedCertificate: "/etc/nginx/secrets/test-btp.crt",
Expand All @@ -1470,7 +1470,7 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "= /include-path-only-match",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
Includes: externalIncludes,
Expand All @@ -1483,7 +1483,7 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "/_ngf-internal-rule17-route0",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Rewrites: []string{"^ $request_uri break"},
Type: http.InternalLocationType,
Expand Down Expand Up @@ -1625,13 +1625,13 @@ func TestCreateServersConflicts(t *testing.T) {
expLocs: []http.Location{
{
Path: "/coffee/",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "= /coffee",
ProxyPass: "http://test_bar_80$request_uri",
ProxyPass: "http://upstream_test_bar_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
Expand Down Expand Up @@ -1665,13 +1665,13 @@ func TestCreateServersConflicts(t *testing.T) {
expLocs: []http.Location{
{
Path: "= /coffee",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "/coffee/",
ProxyPass: "http://test_bar_80$request_uri",
ProxyPass: "http://upstream_test_bar_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
Expand Down Expand Up @@ -1715,13 +1715,13 @@ func TestCreateServersConflicts(t *testing.T) {
expLocs: []http.Location{
{
Path: "/coffee/",
ProxyPass: "http://test_bar_80$request_uri",
ProxyPass: "http://upstream_test_bar_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "= /coffee",
ProxyPass: "http://test_baz_80$request_uri",
ProxyPass: "http://upstream_test_baz_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
Expand Down Expand Up @@ -2163,13 +2163,13 @@ func TestCreateLocationsRootPath(t *testing.T) {
expLocations: []http.Location{
{
Path: "/path-1",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "/path-2",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
Expand All @@ -2188,19 +2188,19 @@ func TestCreateLocationsRootPath(t *testing.T) {
expLocations: []http.Location{
{
Path: "/path-1",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "/path-2",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "/grpc",
ProxyPass: "grpc://test_foo_80",
ProxyPass: "grpc://upstream_test_foo_80",
GRPC: true,
ProxySetHeaders: grpcBaseHeaders,
Type: http.ExternalLocationType,
Expand All @@ -2219,19 +2219,19 @@ func TestCreateLocationsRootPath(t *testing.T) {
expLocations: []http.Location{
{
Path: "/path-1",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "/path-2",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
{
Path: "/",
ProxyPass: "http://test_foo_80$request_uri",
ProxyPass: "http://upstream_test_foo_80$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.ExternalLocationType,
},
Expand Down Expand Up @@ -2798,7 +2798,7 @@ func TestCreateProxyPass(t *testing.T) {
GRPC bool
}{
{
expected: "http://10.0.0.1:80$request_uri",
expected: "http://upstream_10.0.0.1:80$request_uri",
grp: dataplane.BackendGroup{
Backends: []dataplane.Backend{
{
Expand All @@ -2810,7 +2810,7 @@ func TestCreateProxyPass(t *testing.T) {
},
},
{
expected: "http://$ns1__bg_rule0$request_uri",
expected: "http://$upstream_ns1__bg_rule0$request_uri",
grp: dataplane.BackendGroup{
Source: types.NamespacedName{Namespace: "ns1", Name: "bg"},
Backends: []dataplane.Backend{
Expand All @@ -2828,7 +2828,7 @@ func TestCreateProxyPass(t *testing.T) {
},
},
{
expected: "http://10.0.0.1:80",
expected: "http://upstream_10.0.0.1:80",
rewrite: &dataplane.HTTPURLRewriteFilter{
Path: &dataplane.HTTPPathModifier{},
},
Expand All @@ -2843,7 +2843,7 @@ func TestCreateProxyPass(t *testing.T) {
},
},
{
expected: "grpc://10.0.0.1:80",
expected: "grpc://upstream_10.0.0.1:80",
grp: dataplane.BackendGroup{
Backends: []dataplane.Backend{
{
Expand Down
1 change: 1 addition & 0 deletions internal/mode/static/nginx/config/shared/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type RewriteClientIPSettings struct {

const (
ProxyProtocolDirective = " proxy_protocol"
UpstreamPrefix = "upstream_"
)

// Include defines a file that's included via the include directive.
Expand Down
9 changes: 6 additions & 3 deletions internal/mode/static/nginx/config/split_clients.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,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/shared"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
)

Expand Down Expand Up @@ -43,8 +44,10 @@ func createSplitClients(backendGroups []dataplane.BackendGroup) []http.SplitClie
continue
}

// prefixing the splitClients variable name with "upstream_"
// to match the backend name in the proxy_pass directive.
splitClients = append(splitClients, http.SplitClient{
VariableName: convertStringToSafeVariableName(group.Name()),
VariableName: shared.UpstreamPrefix + convertStringToSafeVariableName(group.Name()),
Distributions: distributions,
})
}
Expand Down Expand Up @@ -88,7 +91,7 @@ func createSplitClientDistributions(group dataplane.BackendGroup) []http.SplitCl

distributions = append(distributions, http.SplitClientDistribution{
Percent: fmt.Sprintf("%.2f", percentage),
Value: getSplitClientValue(b),
Value: shared.UpstreamPrefix + getSplitClientValue(b),
})
}

Expand All @@ -98,7 +101,7 @@ func createSplitClientDistributions(group dataplane.BackendGroup) []http.SplitCl

distributions = append(distributions, http.SplitClientDistribution{
Percent: fmt.Sprintf("%.2f", availablePercentage),
Value: getSplitClientValue(lastBackend),
Value: shared.UpstreamPrefix + getSplitClientValue(lastBackend),
})

return distributions
Expand Down
Loading

0 comments on commit 1c477e4

Please sign in to comment.