From 03129bdab0c02b99dfc55ac60cfe71601f1ecfce Mon Sep 17 00:00:00 2001 From: MoofMonkey <11695747+MoofMonkey@users.noreply.github.com> Date: Sat, 1 Oct 2022 17:45:53 +0300 Subject: [PATCH] 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 --- config/configuration.go | 10 +++++----- config/configuration_test.go | 10 +++++----- ingress/ingress.go | 6 +++--- ingress/ingress_test.go | 14 +++++++------- ingress/rule.go | 4 ++-- ingress/rule_test.go | 8 ++++---- proxy/proxy.go | 16 ++++++---------- proxy/proxy_test.go | 27 +++++++++++++++++++-------- 8 files changed, 51 insertions(+), 44 deletions(-) diff --git a/config/configuration.go b/config/configuration.go index aa40759b..adf77983 100644 --- 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 diff --git a/config/configuration_test.go b/config/configuration_test.go index 6d72c18d..bf2d16d3 100644 --- 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 diff --git a/ingress/ingress.go b/ingress/ingress.go index a55bd226..65f2b089 100644 --- 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, } diff --git a/ingress/ingress_test.go b/ingress/ingress_test.go index 977c6e60..691917e9 100644 --- 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, diff --git a/ingress/rule.go b/ingress/rule.go index 2ac92ef5..e963ec4d 100644 --- 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 diff --git a/ingress/rule_test.go b/ingress/rule_test.go index cf42d578..8e355a86 100644 --- 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, }, } diff --git a/proxy/proxy.go b/proxy/proxy.go index fb6a54a5..86cf5a02 100644 --- 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") diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index acee55ad..fe0f5797 100644 --- 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,