app/vmauth: parse url_prefix only once during config load

app/vmauth/auth_config.go

@@ -33,7 +33,7 @@ type UserInfo struct {
 	BearerToken string   `yaml:"bearer_token"`
 	Username    string   `yaml:"username"`
 	Password    string   `yaml:"password"`
-	URLPrefix   string   `yaml:"url_prefix"`
+	URLPrefix   *yamlURL `yaml:"url_prefix"`
 	URLMap      []URLMap `yaml:"url_map"`
 
 	requests *metrics.Counter
@@ -42,7 +42,7 @@ type UserInfo struct {
 // URLMap is a mapping from source paths to target urls.
 type URLMap struct {
 	SrcPaths  []*SrcPath `yaml:"src_paths"`
-	URLPrefix string     `yaml:"url_prefix"`
+	URLPrefix *yamlURL   `yaml:"url_prefix"`
 }
 
 // SrcPath represents an src path
@@ -51,6 +51,27 @@ type SrcPath struct {
 	re        *regexp.Regexp
 }
 
+type yamlURL struct {
+	u *url.URL
+}
+
+func (yu *yamlURL) UnmarshalYAML(f func(interface{}) error) error {
+	var s string
+	if err := f(&s); err != nil {
+		return err
+	}
+	u, err := url.Parse(s)
+	if err != nil {
+		return fmt.Errorf("cannot unmarshal %q into url: %w", s, err)
+	}
+	yu.u = u
+	return nil
+}
+
+func (yu *yamlURL) MarshalYAML() (interface{}, error) {
+	return yu.u.String(), nil
+}
+
 func (sp *SrcPath) match(s string) bool {
 	prefix, ok := sp.re.LiteralPrefix()
 	if ok {
@@ -173,24 +194,27 @@ func parseAuthConfig(data []byte) (map[string]*UserInfo, error) {
 		if byAuthToken[authToken] != nil {
 			return nil, fmt.Errorf("duplicate auth token found for bearer_token=%q, username=%q: %q", authToken, ui.BearerToken, ui.Username)
 		}
-		if len(ui.URLPrefix) > 0 {
+		if ui.URLPrefix != nil {
-			urlPrefix, err := sanitizeURLPrefix(ui.URLPrefix)
+			urlPrefix, err := sanitizeURLPrefix(ui.URLPrefix.u)
 			if err != nil {
 				return nil, err
 			}
-			ui.URLPrefix = urlPrefix
+			ui.URLPrefix.u = urlPrefix
 		}
 		for _, e := range ui.URLMap {
 			if len(e.SrcPaths) == 0 {
-				return nil, fmt.Errorf("missing `src_paths`")
+				return nil, fmt.Errorf("missing `src_paths` in `url_map`")
 			}
-			urlPrefix, err := sanitizeURLPrefix(e.URLPrefix)
+			if e.URLPrefix == nil {
+				return nil, fmt.Errorf("missing `url_prefix` in `url_map`")
+			}
+			urlPrefix, err := sanitizeURLPrefix(e.URLPrefix.u)
 			if err != nil {
 				return nil, err
 			}
-			e.URLPrefix = urlPrefix
+			e.URLPrefix.u = urlPrefix
 		}
-		if len(ui.URLMap) == 0 && len(ui.URLPrefix) == 0 {
+		if len(ui.URLMap) == 0 && ui.URLPrefix == nil {
 			return nil, fmt.Errorf("missing `url_prefix`")
 		}
 		if ui.BearerToken != "" {
@@ -218,21 +242,17 @@ func getAuthToken(bearerToken, username, password string) string {
 	return "Basic " + token64
 }
 
-func sanitizeURLPrefix(urlPrefix string) (string, error) {
+func sanitizeURLPrefix(urlPrefix *url.URL) (*url.URL, error) {
 	// Remove trailing '/' from urlPrefix
-	for strings.HasSuffix(urlPrefix, "/") {
+	for strings.HasSuffix(urlPrefix.Path, "/") {
-		urlPrefix = urlPrefix[:len(urlPrefix)-1]
+		urlPrefix.Path = urlPrefix.Path[:len(urlPrefix.Path)-1]
 	}
 	// Validate urlPrefix
-	target, err := url.Parse(urlPrefix)
+	if urlPrefix.Scheme != "http" && urlPrefix.Scheme != "https" {
-	if err != nil {
+		return nil, fmt.Errorf("unsupported scheme for `url_prefix: %q`: %q; must be `http` or `https`", urlPrefix, urlPrefix.Scheme)
-		return "", fmt.Errorf("invalid `url_prefix: %q`: %w", urlPrefix, err)
 	}
-	if target.Scheme != "http" && target.Scheme != "https" {
+	if urlPrefix.Host == "" {
-		return "", fmt.Errorf("unsupported scheme for `url_prefix: %q`: %q; must be `http` or `https`", urlPrefix, target.Scheme)
+		return nil, fmt.Errorf("missing hostname in `url_prefix %q`", urlPrefix.Host)
-	}
-	if target.Host == "" {
-		return "", fmt.Errorf("missing hostname in `url_prefix %q`", urlPrefix)
 	}
 	return urlPrefix, nil
 }

app/vmauth/auth_config_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"bytes"
 	"fmt"
+	"net/url"
 	"regexp"
 	"testing"
 
@@ -55,6 +56,11 @@ users:
 - username: foo
   url_prefix: http:///bar
 `)
+	f(`
+users:
+- username: foo
+  url_prefix: [bar]
+`)
 
 	// Username and bearer_token in a single config
 	f(`
@@ -102,6 +108,15 @@ users:
   - src_paths: ["/foo/bar"]
 `)
 
+	// Invalid url_prefix in url_map
+	f(`
+users:
+- username: a
+  url_map:
+  - src_paths: ["/foo/bar"]
+    url_prefix: foo.bar
+`)
+
 	// Missing src_paths in url_map
 	f(`
 users:
@@ -143,7 +158,7 @@ users:
 		getAuthToken("", "foo", "bar"): {
 			Username:  "foo",
 			Password:  "bar",
-			URLPrefix: "http://aaa:343/bbb",
+			URLPrefix: mustParseURL("http://aaa:343/bbb"),
 		},
 	})
 
@@ -157,11 +172,11 @@ users:
 `, map[string]*UserInfo{
 		getAuthToken("", "foo", ""): {
 			Username:  "foo",
-			URLPrefix: "http://foo",
+			URLPrefix: mustParseURL("http://foo"),
 		},
 		getAuthToken("", "bar", ""): {
 			Username:  "bar",
-			URLPrefix: "https://bar/x",
+			URLPrefix: mustParseURL("https://bar/x"),
 		},
 	})
 
@@ -180,11 +195,11 @@ users:
 			URLMap: []URLMap{
 				{
 					SrcPaths:  getSrcPaths([]string{"/api/v1/query", "/api/v1/query_range", "/api/v1/label/[^./]+/.+"}),
-					URLPrefix: "http://vmselect/select/0/prometheus",
+					URLPrefix: mustParseURL("http://vmselect/select/0/prometheus"),
 				},
 				{
 					SrcPaths:  getSrcPaths([]string{"/api/v1/write"}),
-					URLPrefix: "http://vminsert/insert/0/prometheus",
+					URLPrefix: mustParseURL("http://vminsert/insert/0/prometheus"),
 				},
 			},
 		},
@@ -222,3 +237,13 @@ func areEqualConfigs(a, b map[string]*UserInfo) error {
 	}
 	return nil
 }
+
+func mustParseURL(u string) *yamlURL {
+	pu, err := url.Parse(u)
+	if err != nil {
+		panic(fmt.Errorf("BUG: cannot parse %q: %w", u, err))
+	}
+	return &yamlURL{
+		u: pu,
+	}
+}

app/vmauth/main.go

@@ -65,11 +65,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
 		httpserver.Errorf(w, r, "cannot determine targetURL: %s", err)
 		return true
 	}
-	if _, err := url.Parse(targetURL); err != nil {
+	r.Header.Set("vm-target-url", targetURL.String())
-		httpserver.Errorf(w, r, "invalid targetURL=%q: %s", targetURL, err)
-		return true
-	}
-	r.Header.Set("vm-target-url", targetURL)
 	reverseProxy.ServeHTTP(w, r)
 	return true
 }

app/vmauth/target_url.go

@@ -7,37 +7,31 @@ import (
 	"strings"
 )
 
-func mergeURLs(uiURL string, requestURI *url.URL) (string, error) {
+func mergeURLs(uiURL, requestURI *url.URL) *url.URL {
-	prefixURL, err := url.Parse(uiURL)
+	targetURL := *uiURL
-	if err != nil {
+	targetURL.Path += requestURI.Path
-		return "", fmt.Errorf("BUG - cannot parse userInfo url: %q, err: %w", uiURL, err)
-	}
-	prefixURL.Path += requestURI.Path
 	requestParams := requestURI.Query()
 	// fast path
 	if len(requestParams) == 0 {
-		return prefixURL.String(), nil
+		return &targetURL
 	}
 	// merge query parameters from requests.
-	userInfoParams := prefixURL.Query()
+	uiParams := targetURL.Query()
 	for k, v := range requestParams {
 		// skip clashed query params from original request
-		if exist := userInfoParams.Get(k); len(exist) > 0 {
+		if exist := uiParams.Get(k); len(exist) > 0 {
 			continue
 		}
 		for i := range v {
-			userInfoParams.Add(k, v[i])
+			uiParams.Add(k, v[i])
 		}
 	}
-	prefixURL.RawQuery = userInfoParams.Encode()
+	targetURL.RawQuery = uiParams.Encode()
-	return prefixURL.String(), nil
+	return &targetURL
 }
 
-func createTargetURL(ui *UserInfo, uOrig *url.URL) (string, error) {
+func createTargetURL(ui *UserInfo, uOrig *url.URL) (*url.URL, error) {
-	u, err := url.Parse(uOrig.String())
+	u := *uOrig
-	if err != nil {
-		return "", fmt.Errorf("cannot make a copy of %q: %w", u, err)
-	}
 	// Prevent from attacks with using `..` in r.URL.Path
 	u.Path = path.Clean(u.Path)
 	if !strings.HasPrefix(u.Path, "/") {
@@ -46,12 +40,12 @@ func createTargetURL(ui *UserInfo, uOrig *url.URL) (string, error) {
 	for _, e := range ui.URLMap {
 		for _, sp := range e.SrcPaths {
 			if sp.match(u.Path) {
-				return mergeURLs(e.URLPrefix, u)
+				return mergeURLs(e.URLPrefix.u, &u), nil
 			}
 		}
 	}
-	if len(ui.URLPrefix) > 0 {
+	if ui.URLPrefix != nil {
-		return mergeURLs(ui.URLPrefix, u)
+		return mergeURLs(ui.URLPrefix.u, &u), nil
 	}
-	return "", fmt.Errorf("missing route for %q", u)
+	return nil, fmt.Errorf("missing route for %q", u.String())
 }

app/vmauth/target_url_test.go

@@ -16,28 +16,28 @@ func TestCreateTargetURLSuccess(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unexpected error: %s", err)
 		}
-		if target != expectedTarget {
+		if target.String() != expectedTarget {
 			t.Fatalf("unexpected target; got %q; want %q", target, expectedTarget)
 		}
 	}
 	// Simple routing with `url_prefix`
 	f(&UserInfo{
-		URLPrefix: "http://foo.bar",
+		URLPrefix: mustParseURL("http://foo.bar"),
 	}, "", "http://foo.bar/.")
 	f(&UserInfo{
-		URLPrefix: "http://foo.bar",
+		URLPrefix: mustParseURL("http://foo.bar"),
 	}, "/", "http://foo.bar/")
 	f(&UserInfo{
-		URLPrefix: "http://foo.bar",
+		URLPrefix: mustParseURL("http://foo.bar"),
 	}, "a/b?c=d", "http://foo.bar/a/b?c=d")
 	f(&UserInfo{
-		URLPrefix: "https://sss:3894/x/y",
+		URLPrefix: mustParseURL("https://sss:3894/x/y"),
 	}, "/z", "https://sss:3894/x/y/z")
 	f(&UserInfo{
-		URLPrefix: "https://sss:3894/x/y",
+		URLPrefix: mustParseURL("https://sss:3894/x/y"),
 	}, "/../../aaa", "https://sss:3894/x/y/aaa")
 	f(&UserInfo{
-		URLPrefix: "https://sss:3894/x/y",
+		URLPrefix: mustParseURL("https://sss:3894/x/y"),
 	}, "/./asd/../../aaa?a=d&s=s/../d", "https://sss:3894/x/y/aaa?a=d&s=s%2F..%2Fd")
 
 	// Complex routing with `url_map`
@@ -45,14 +45,14 @@ func TestCreateTargetURLSuccess(t *testing.T) {
 		URLMap: []URLMap{
 			{
 				SrcPaths:  getSrcPaths([]string{"/api/v1/query"}),
-				URLPrefix: "http://vmselect/0/prometheus",
+				URLPrefix: mustParseURL("http://vmselect/0/prometheus"),
 			},
 			{
 				SrcPaths:  getSrcPaths([]string{"/api/v1/write"}),
-				URLPrefix: "http://vminsert/0/prometheus",
+				URLPrefix: mustParseURL("http://vminsert/0/prometheus"),
 			},
 		},
-		URLPrefix: "http://default-server",
+		URLPrefix: mustParseURL("http://default-server"),
 	}
 	f(ui, "/api/v1/query?query=up", "http://vmselect/0/prometheus/api/v1/query?query=up")
 	f(ui, "/api/v1/write", "http://vminsert/0/prometheus/api/v1/write")
@@ -63,14 +63,14 @@ func TestCreateTargetURLSuccess(t *testing.T) {
 		URLMap: []URLMap{
 			{
 				SrcPaths:  getSrcPaths([]string{"/api/v1/query(_range)?", "/api/v1/label/[^/]+/values"}),
-				URLPrefix: "http://vmselect/0/prometheus",
+				URLPrefix: mustParseURL("http://vmselect/0/prometheus"),
 			},
 			{
 				SrcPaths:  getSrcPaths([]string{"/api/v1/write"}),
-				URLPrefix: "http://vminsert/0/prometheus",
+				URLPrefix: mustParseURL("http://vminsert/0/prometheus"),
 			},
 		},
-		URLPrefix: "http://default-server",
+		URLPrefix: mustParseURL("http://default-server"),
 	}
 	f(ui, "/api/v1/query?query=up", "http://vmselect/0/prometheus/api/v1/query?query=up")
 	f(ui, "/api/v1/query_range?query=up", "http://vmselect/0/prometheus/api/v1/query_range?query=up")
@@ -78,10 +78,10 @@ func TestCreateTargetURLSuccess(t *testing.T) {
 	f(ui, "/api/v1/write", "http://vminsert/0/prometheus/api/v1/write")
 	f(ui, "/api/v1/foo/bar", "http://default-server/api/v1/foo/bar")
 	f(&UserInfo{
-		URLPrefix: "http://foo.bar?extra_label=team=dev",
+		URLPrefix: mustParseURL("http://foo.bar?extra_label=team=dev"),
 	}, "/api/v1/query", "http://foo.bar/api/v1/query?extra_label=team=dev")
 	f(&UserInfo{
-		URLPrefix: "http://foo.bar?extra_label=team=mobile",
+		URLPrefix: mustParseURL("http://foo.bar?extra_label=team=mobile"),
 	}, "/api/v1/query?extra_label=team=dev", "http://foo.bar/api/v1/query?extra_label=team%3Dmobile")
 
 }
@@ -97,7 +97,7 @@ func TestCreateTargetURLFailure(t *testing.T) {
 		if err == nil {
 			t.Fatalf("expecting non-nil error")
 		}
-		if target != "" {
+		if target != nil {
 			t.Fatalf("unexpected target=%q; want empty string", target)
 		}
 	}
@@ -106,7 +106,7 @@ func TestCreateTargetURLFailure(t *testing.T) {
 		URLMap: []URLMap{
 			{
 				SrcPaths:  getSrcPaths([]string{"/api/v1/query"}),
-				URLPrefix: "http://foobar/baz",
+				URLPrefix: mustParseURL("http://foobar/baz"),
 			},
 		},
 	}, "/api/v1/write")