From 4ee42e9e73f99a9d58baf83afa5b55fb3cdbd0ed Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 14 Dec 2023 01:04:46 +0200 Subject: [PATCH] app/vmauth: allow specifying an empty retry_status_codes and and zero drop_src_path_prefix_parts in order to override user-level setting Previously `retry_status_codes: []` and `drop_src_path_prefix_parts: 0` at `url_map` were equivalent to missing values. This was resulting in using the user-level values instead. --- app/vmauth/auth_config.go | 21 +++++++++++++++++---- app/vmauth/auth_config_test.go | 6 +++++- app/vmauth/main.go | 4 ++-- app/vmauth/target_url.go | 8 ++++---- app/vmauth/target_url_test.go | 27 +++++++++++++-------------- 5 files changed, 41 insertions(+), 25 deletions(-) diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go index 89d86c0faa..fa05a0aaf7 100644 --- a/app/vmauth/auth_config.go +++ b/app/vmauth/auth_config.go @@ -56,7 +56,7 @@ type UserInfo struct { DefaultURL *URLPrefix `yaml:"default_url,omitempty"` RetryStatusCodes []int `yaml:"retry_status_codes,omitempty"` LoadBalancingPolicy string `yaml:"load_balancing_policy,omitempty"` - DropSrcPathPrefixParts int `yaml:"drop_src_path_prefix_parts,omitempty"` + DropSrcPathPrefixParts *int `yaml:"drop_src_path_prefix_parts,omitempty"` TLSInsecureSkipVerify *bool `yaml:"tls_insecure_skip_verify,omitempty"` TLSCAFile string `yaml:"tls_ca_file,omitempty"` @@ -145,7 +145,7 @@ type URLMap struct { LoadBalancingPolicy string `yaml:"load_balancing_policy,omitempty"` // DropSrcPathPrefixParts is the number of `/`-delimited request path prefix parts to drop before proxying the request to backend. - DropSrcPathPrefixParts int `yaml:"drop_src_path_prefix_parts,omitempty"` + DropSrcPathPrefixParts *int `yaml:"drop_src_path_prefix_parts,omitempty"` } // Regex represents a regex @@ -166,6 +166,9 @@ type URLPrefix struct { // load balancing policy used loadBalancingPolicy string + + // how many request path prefix parts to drop before routing the request to backendURL. + dropSrcPathPrefixParts int } func (up *URLPrefix) setLoadBalancingPolicy(loadBalancingPolicy string) error { @@ -606,17 +609,22 @@ func parseAuthConfigUsers(ac *AuthConfig) (map[string]*UserInfo, error) { func (ui *UserInfo) initURLs() error { retryStatusCodes := defaultRetryStatusCodes.Values() loadBalancingPolicy := *defaultLoadBalancingPolicy + dropSrcPathPrefixParts := 0 if ui.URLPrefix != nil { if err := ui.URLPrefix.sanitize(); err != nil { return err } - if len(ui.RetryStatusCodes) > 0 { + if ui.RetryStatusCodes != nil { retryStatusCodes = ui.RetryStatusCodes } if ui.LoadBalancingPolicy != "" { loadBalancingPolicy = ui.LoadBalancingPolicy } + if ui.DropSrcPathPrefixParts != nil { + dropSrcPathPrefixParts = *ui.DropSrcPathPrefixParts + } ui.URLPrefix.retryStatusCodes = retryStatusCodes + ui.URLPrefix.dropSrcPathPrefixParts = dropSrcPathPrefixParts if err := ui.URLPrefix.setLoadBalancingPolicy(loadBalancingPolicy); err != nil { return err } @@ -638,16 +646,21 @@ func (ui *UserInfo) initURLs() error { } rscs := retryStatusCodes lbp := loadBalancingPolicy - if len(e.RetryStatusCodes) > 0 { + dsp := dropSrcPathPrefixParts + if e.RetryStatusCodes != nil { rscs = e.RetryStatusCodes } if e.LoadBalancingPolicy != "" { lbp = e.LoadBalancingPolicy } + if e.DropSrcPathPrefixParts != nil { + dsp = *e.DropSrcPathPrefixParts + } e.URLPrefix.retryStatusCodes = rscs if err := e.URLPrefix.setLoadBalancingPolicy(lbp); err != nil { return err } + e.URLPrefix.dropSrcPathPrefixParts = dsp } if len(ui.URLMaps) == 0 && ui.URLPrefix == nil { return fmt.Errorf("missing `url_prefix`") diff --git a/app/vmauth/auth_config_test.go b/app/vmauth/auth_config_test.go index fee8e9ac31..df96d878d4 100644 --- a/app/vmauth/auth_config_test.go +++ b/app/vmauth/auth_config_test.go @@ -292,7 +292,7 @@ users: TLSInsecureSkipVerify: &insecureSkipVerifyFalse, RetryStatusCodes: []int{500, 501}, LoadBalancingPolicy: "first_available", - DropSrcPathPrefixParts: 1, + DropSrcPathPrefixParts: intp(1), }, }) @@ -584,3 +584,7 @@ func mustParseURLs(us []string) *URLPrefix { bus: bus, } } + +func intp(n int) *int { + return &n +} diff --git a/app/vmauth/main.go b/app/vmauth/main.go index 23e960ff57..b206619774 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -164,7 +164,7 @@ func processUserRequest(w http.ResponseWriter, r *http.Request, ui *UserInfo) { func processRequest(w http.ResponseWriter, r *http.Request, ui *UserInfo) { u := normalizeURL(r.URL) - up, hc, dropSrcPathPrefixParts := ui.getURLPrefixAndHeaders(u) + up, hc := ui.getURLPrefixAndHeaders(u) isDefault := false if up == nil { if ui.DefaultURL == nil { @@ -198,7 +198,7 @@ func processRequest(w http.ResponseWriter, r *http.Request, ui *UserInfo) { query.Set("request_path", u.String()) targetURL.RawQuery = query.Encode() } else { // Update path for regular routes. - targetURL = mergeURLs(targetURL, u, dropSrcPathPrefixParts) + targetURL = mergeURLs(targetURL, u, up.dropSrcPathPrefixParts) } ok := tryProcessingRequest(w, r, targetURL, hc, up.retryStatusCodes, ui.httpTransport) bu.put() diff --git a/app/vmauth/target_url.go b/app/vmauth/target_url.go index 1178318fd3..9fa229be4e 100644 --- a/app/vmauth/target_url.go +++ b/app/vmauth/target_url.go @@ -49,16 +49,16 @@ func dropPrefixParts(path string, parts int) string { return path } -func (ui *UserInfo) getURLPrefixAndHeaders(u *url.URL) (*URLPrefix, HeadersConf, int) { +func (ui *UserInfo) getURLPrefixAndHeaders(u *url.URL) (*URLPrefix, HeadersConf) { for _, e := range ui.URLMaps { if matchAnyRegex(e.SrcHosts, u.Host) && matchAnyRegex(e.SrcPaths, u.Path) { - return e.URLPrefix, e.HeadersConf, e.DropSrcPathPrefixParts + return e.URLPrefix, e.HeadersConf } } if ui.URLPrefix != nil { - return ui.URLPrefix, ui.HeadersConf, ui.DropSrcPathPrefixParts + return ui.URLPrefix, ui.HeadersConf } - return nil, HeadersConf{}, 0 + return nil, HeadersConf{} } func matchAnyRegex(rs []*Regex, s string) bool { diff --git a/app/vmauth/target_url_test.go b/app/vmauth/target_url_test.go index 2f9a273131..2f2f271c23 100644 --- a/app/vmauth/target_url_test.go +++ b/app/vmauth/target_url_test.go @@ -89,12 +89,12 @@ func TestCreateTargetURLSuccess(t *testing.T) { t.Fatalf("cannot parse %q: %s", requestURI, err) } u = normalizeURL(u) - up, hc, dropSrcPathPrefixParts := ui.getURLPrefixAndHeaders(u) + up, hc := ui.getURLPrefixAndHeaders(u) if up == nil { t.Fatalf("cannot determie backend: %s", err) } bu := up.getLeastLoadedBackendURL() - target := mergeURLs(bu.url, u, dropSrcPathPrefixParts) + target := mergeURLs(bu.url, u, up.dropSrcPathPrefixParts) bu.put() if target.String() != expectedTarget { t.Fatalf("unexpected target; got %q; want %q", target, expectedTarget) @@ -109,8 +109,8 @@ func TestCreateTargetURLSuccess(t *testing.T) { if up.loadBalancingPolicy != expectedLoadBalancingPolicy { t.Fatalf("unexpected loadBalancingPolicy; got %q; want %q", up.loadBalancingPolicy, expectedLoadBalancingPolicy) } - if dropSrcPathPrefixParts != expectedDropSrcPathPrefixParts { - t.Fatalf("unexpected dropSrcPathPrefixParts; got %d; want %d", dropSrcPathPrefixParts, expectedDropSrcPathPrefixParts) + if up.dropSrcPathPrefixParts != expectedDropSrcPathPrefixParts { + t.Fatalf("unexpected dropSrcPathPrefixParts; got %d; want %d", up.dropSrcPathPrefixParts, expectedDropSrcPathPrefixParts) } } // Simple routing with `url_prefix` @@ -127,7 +127,7 @@ func TestCreateTargetURLSuccess(t *testing.T) { }, RetryStatusCodes: []int{503, 501}, LoadBalancingPolicy: "first_available", - DropSrcPathPrefixParts: 2, + DropSrcPathPrefixParts: intp(2), }, "/a/b/c", "http://foo.bar/c", `[{"bb" "aaa"}]`, `[]`, []int{503, 501}, "first_available", 2) f(&UserInfo{ URLPrefix: mustParseURL("http://foo.bar/federate"), @@ -172,11 +172,13 @@ func TestCreateTargetURLSuccess(t *testing.T) { }, RetryStatusCodes: []int{503, 500, 501}, LoadBalancingPolicy: "first_available", - DropSrcPathPrefixParts: 1, + DropSrcPathPrefixParts: intp(1), }, { - SrcPaths: getRegexs([]string{"/api/v1/write"}), - URLPrefix: mustParseURL("http://vminsert/0/prometheus"), + SrcPaths: getRegexs([]string{"/api/v1/write"}), + URLPrefix: mustParseURL("http://vminsert/0/prometheus"), + RetryStatusCodes: []int{}, + DropSrcPathPrefixParts: intp(0), }, }, URLPrefix: mustParseURL("http://default-server"), @@ -191,13 +193,13 @@ func TestCreateTargetURLSuccess(t *testing.T) { }}, }, RetryStatusCodes: []int{502}, - DropSrcPathPrefixParts: 2, + DropSrcPathPrefixParts: intp(2), } f(ui, "http://host42/vmsingle/api/v1/query?query=up", "http://vmselect/0/prometheus/api/v1/query?query=up", `[{"xx" "aa"} {"yy" "asdf"}]`, `[{"qwe" "rty"}]`, []int{503, 500, 501}, "first_available", 1) f(ui, "http://host123/vmsingle/api/v1/query?query=up", "http://default-server/v1/query?query=up", `[{"bb" "aaa"}]`, `[{"x" "y"}]`, []int{502}, "least_loaded", 2) - f(ui, "https://foo-host/api/v1/write", "http://vminsert/0/prometheus/api/v1/write", "[]", "[]", []int{502}, "least_loaded", 0) + f(ui, "https://foo-host/api/v1/write", "http://vminsert/0/prometheus/api/v1/write", "[]", "[]", []int{}, "least_loaded", 0) f(ui, "https://foo-host/foo/bar/api/v1/query_range", "http://default-server/api/v1/query_range", `[{"bb" "aaa"}]`, `[{"x" "y"}]`, []int{502}, "least_loaded", 2) // Complex routing regexp paths in `url_map` @@ -241,7 +243,7 @@ func TestCreateTargetURLFailure(t *testing.T) { t.Fatalf("cannot parse %q: %s", requestURI, err) } u = normalizeURL(u) - up, hc, dropSrcPathPrefixParts := ui.getURLPrefixAndHeaders(u) + up, hc := ui.getURLPrefixAndHeaders(u) if up != nil { t.Fatalf("unexpected non-empty up=%#v", up) } @@ -251,9 +253,6 @@ func TestCreateTargetURLFailure(t *testing.T) { if hc.ResponseHeaders != nil { t.Fatalf("unexpected non-empty response headers=%q", hc.ResponseHeaders) } - if dropSrcPathPrefixParts != 0 { - t.Fatalf("unexpected non-zero dropSrcPathPrefixParts=%d", dropSrcPathPrefixParts) - } } f(&UserInfo{}, "/foo/bar") f(&UserInfo{