Some of the requested changes

- Rename pathReplacement to rewrite - Don't clone request for path rewriting - Move out path rewriting from proxyHTTPRequest to ProxyHTTP for better readability - Add proxy test for two back-reference replacements
2022-10-01 17:45:53 +03:00 · 2022-10-01 17:45:53 +03:00 · 03129bdab0
parent e5c248ff6f
commit 03129bdab0
8 changed files with 51 additions and 44 deletions
--- a/config/configuration.go
+++ b/config/configuration.go
@ -177,11 +177,11 @@ func ValidateUrl(c *cli.Context, allowURLFromArgs bool) (*url.URL, error) {
 }

 type UnvalidatedIngressRule struct {
-	Hostname        string              `json:"hostname,omitempty"`
-	Path            string              `json:"path,omitempty"`
-	PathReplacement string              `yaml:"pathReplacement" json:"pathReplacement,omitempty"`
-	Service         string              `json:"service,omitempty"`
-	OriginRequest   OriginRequestConfig `yaml:"originRequest" json:"originRequest"`
+	Hostname      string              `json:"hostname,omitempty"`
+	Path          string              `json:"path,omitempty"`
+	Rewrite       string              `json:"rewrite,omitempty"`
+	Service       string              `json:"service,omitempty"`
+	OriginRequest OriginRequestConfig `yaml:"originRequest" json:"originRequest"`
 }

 // OriginRequestConfig is a set of optional fields that users may set to
--- a/config/configuration_test.go
+++ b/config/configuration_test.go
@ -13,10 +13,10 @@ import (
 func TestConfigFileSettings(t *testing.T) {
 	var (
 		firstIngress = UnvalidatedIngressRule{
-			Hostname:        "tunnel1.example.com",
-			Path:            "/id",
-			PathReplacement: "/id",
-			Service:         "https://localhost:8000",
+			Hostname: "tunnel1.example.com",
+			Path:     "/id",
+			Rewrite:  "/id",
+			Service:  "https://localhost:8000",
 		}
 		secondIngress = UnvalidatedIngressRule{
 			Hostname: "*",
@ -46,7 +46,7 @@ originRequest:
 ingress:
 - hostname: tunnel1.example.com
   path: /id
-   pathReplacement: /id
+   rewrite: /id
   service: https://localhost:8000
 - hostname: "*"
   service: https://localhost:8001
--- a/ingress/ingress.go
+++ b/ingress/ingress.go
@ -294,8 +294,8 @@ func validateIngress(ingress []config.UnvalidatedIngressRule, defaults OriginReq
 				return Ingress{}, errors.Wrapf(err, "Rule #%d has an invalid regex", i+1)
 			}
 			pathRegexp = &Regexp{Regexp: regex}
-		} else if r.PathReplacement != "" {
-			return Ingress{}, fmt.Errorf("rule #%d has a path replacement without a path specified", i+1)
+		} else if r.Rewrite != "" {
+			return Ingress{}, fmt.Errorf("rule #%d has a rewrite without a path specified", i+1)
 		}

 		rules[i] = Rule{
@ -303,7 +303,7 @@ func validateIngress(ingress []config.UnvalidatedIngressRule, defaults OriginReq
 			punycodeHostname: punycodeHostname,
 			Service:          service,
 			Path:             pathRegexp,
-			PathReplacement:  r.PathReplacement,
+			Rewrite:          r.Rewrite,
 			Handlers:         handlers,
 			Config:           cfg,
 		}
--- a/ingress/ingress_test.go
+++ b/ingress/ingress_test.go
@ -452,16 +452,16 @@ ingress:
 - hostname: test.example.com
  service: https://localhost:8000
  path: ^/api/(.*)$
-  pathReplacement: /$1
+  rewrite: /$1
 - service: http_status:404
 `},
 			want: []Rule{
 				{
-					Hostname:        "test.example.com",
-					Service:         &httpService{url: localhost8000},
-					Path:            &Regexp{Regexp: regexp.MustCompile("^/api/(.*)$")},
-					PathReplacement: "/$1",
-					Config:          defaultConfig,
+					Hostname: "test.example.com",
+					Service:  &httpService{url: localhost8000},
+					Path:     &Regexp{Regexp: regexp.MustCompile("^/api/(.*)$")},
+					Rewrite:  "/$1",
+					Config:   defaultConfig,
 				},
 				{
 					Service: &fourOhFour,
@ -475,7 +475,7 @@ ingress:
 ingress:
 - hostname: test.example.com
  service: https://localhost:8000
-  pathReplacement: /$1
+  rewrite: /$1
 - service: http_status:404
 `},
 			wantErr: true,
--- a/ingress/rule.go
+++ b/ingress/rule.go
@ -20,9 +20,9 @@ type Rule struct {
 	// Path is an optional regex that can specify path-driven ingress rules.
 	Path *Regexp `json:"path"`

-	// PathReplacement is an optional regexp replacement string for Path,
+	// Rewrite is an optional regexp replacement string for Path,
 	// that gets sent to the Service
-	PathReplacement string `json:"pathReplacement"`
+	Rewrite string `json:"rewrite"`

 	// A (probably local) address. Requests for a hostname which matches this
 	// rule's hostname pattern will be proxied to the service running on this
--- a/ingress/rule_test.go
+++ b/ingress/rule_test.go
@ -194,25 +194,25 @@ func TestMarshalJSON(t *testing.T) {
 		{
 			name:     "Nil",
 			path:     nil,
-			expected: `{"hostname":"example.com","path":null,"pathReplacement":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
+			expected: `{"hostname":"example.com","path":null,"rewrite":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
 			want:     true,
 		},
 		{
 			name:     "Nil regex",
 			path:     &Regexp{Regexp: nil},
-			expected: `{"hostname":"example.com","path":null,"pathReplacement":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
+			expected: `{"hostname":"example.com","path":null,"rewrite":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
 			want:     true,
 		},
 		{
 			name:     "Empty",
 			path:     &Regexp{Regexp: regexp.MustCompile("")},
-			expected: `{"hostname":"example.com","path":"","pathReplacement":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
+			expected: `{"hostname":"example.com","path":"","rewrite":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
 			want:     true,
 		},
 		{
 			name:     "Basic",
 			path:     &Regexp{Regexp: regexp.MustCompile("/echo")},
-			expected: `{"hostname":"example.com","path":"/echo","pathReplacement":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
+			expected: `{"hostname":"example.com","path":"/echo","rewrite":"","service":"https://localhost:8000","Handlers":null,"originRequest":{"connectTimeout":30,"tlsTimeout":10,"tcpKeepAlive":30,"noHappyEyeballs":false,"keepAliveTimeout":90,"keepAliveConnections":100,"httpHostHeader":"","originServerName":"","caPool":"","noTLSVerify":false,"disableChunkedEncoding":false,"bastionMode":false,"proxyAddress":"127.0.0.1","proxyPort":0,"proxyType":"","ipRules":null,"http2Origin":false,"access":{"teamName":"","audTag":null}}}`,
 			want:     true,
 		},
 	}
--- a/proxy/proxy.go
+++ b/proxy/proxy.go
@ -101,6 +101,11 @@ func (p *Proxy) ProxyHTTP(
 	p.logRequest(req, logFields)
 	ruleSpan.SetAttributes(attribute.Int("rule-num", ruleNum))
 	ruleSpan.End()
+
+	if rule.Rewrite != "" {
+		req.URL.Path = rule.Path.ReplaceAllString(req.URL.Path, rule.Rewrite)
+	}
+
 	if err, applied := p.applyIngressMiddleware(rule, req, w); err != nil {
 		if applied {
 			p.log.Error().Msg(err.Error())
@ -117,8 +122,6 @@ func (p *Proxy) ProxyHTTP(
 			originProxy,
 			isWebsocket,
 			rule.Config.DisableChunkedEncoding,
-			rule.Path,
-			rule.PathReplacement,
 			logFields,
 		); err != nil {
 			rule, srv := ruleField(p.ingressRules, ruleNum)
@ -191,18 +194,11 @@ func (p *Proxy) proxyHTTPRequest(
 	httpService ingress.HTTPOriginProxy,
 	isWebsocket bool,
 	disableChunkedEncoding bool,
-	pathRegexp *ingress.Regexp,
-	pathReplacement string,
 	fields logFields,
 ) error {
 	roundTripReq := tr.Request
-	if isWebsocket || pathReplacement != "" {
-		roundTripReq = tr.Clone(tr.Request.Context())
-	}
-	if pathReplacement != "" {
-		roundTripReq.URL.Path = pathRegexp.ReplaceAllString(roundTripReq.URL.Path, pathReplacement)
-	}
 	if isWebsocket {
+		roundTripReq = tr.Clone(tr.Request.Context())
 		roundTripReq.Header.Set("Connection", "Upgrade")
 		roundTripReq.Header.Set("Upgrade", "websocket")
 		roundTripReq.Header.Set("Sec-Websocket-Version", "13")
--- a/proxy/proxy_test.go
+++ b/proxy/proxy_test.go
@ -296,16 +296,22 @@ func TestProxyMultipleOrigins(t *testing.T) {

 	unvalidatedIngress := []config.UnvalidatedIngressRule{
 		{
-			Hostname:        "api.example.com",
-			Service:         api.URL,
-			Path:            "^/service1/(.*)$",
-			PathReplacement: "/$1",
+			Hostname: "api.example.com",
+			Service:  api.URL,
+			Path:     "^/service1/(.*)$",
+			Rewrite:  "/$1",
 		},
 		{
-			Hostname:        "api.example.com",
-			Service:         api.URL,
-			Path:            "^/service2/(.*)$",
-			PathReplacement: "/route2/$1",
+			Hostname: "api.example.com",
+			Service:  api.URL,
+			Path:     "^/service2/(.*)$",
+			Rewrite:  "/route2/$1",
+		},
+		{
+			Hostname: "api.example.com",
+			Service:  api.URL,
+			Path:     `^/service3/(.*)/(\w+)$`,
+			Rewrite:  "/$2/$1",
 		},
 		{
 			Hostname: "api.example.com",
@ -337,6 +343,11 @@ func TestProxyMultipleOrigins(t *testing.T) {
 			expectedStatus: http.StatusCreated,
 			expectedBody:   []byte(fmt.Sprintf("/route2%s", hello.HealthRoute)),
 		},
+		{
+			url:            fmt.Sprintf("http://api.example.com/service3/func%s", hello.HealthRoute),
+			expectedStatus: http.StatusCreated,
+			expectedBody:   []byte(fmt.Sprintf("%s/func", hello.HealthRoute)),
+		},
 		{
 			url:            "http://api.example.com/created",
 			expectedStatus: http.StatusCreated,