From 2afb068f0f5498eb7917eb6316485702086395f3 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
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{