app/vmauth: follow-up for b155b20de4

- Use exact matching by default for the query arg value provided via arg=value syntax at src_query_args.
  Regex matching can be enabled by using =~ instead of = . For example, arg=~regex.
  This ensures that the exact matching works as expected without the need to escape special regex chars.

- Add helper functions for creating QueryArg, Header and Regex structs in tests.
  This improves maintainability of the tests.

- Remove url.QueryUnescape() call on the url in TestCreateTargetURLSuccess(), since this is bogus approach.
  The url.QueryUnescape() must be applied to individual query args, and it mustn't be applied to the whole url,
  since in this case it may perform invalid unescaping in the context of the url, or make the resulting url invalid.

While at it, properly marshal all the fields inside UserInfo config to yaml in tests.
Previously Header and QueryArg structs were improperly marshaled because the custom MarshalYAML
is called only on pointers to Header and QueryArg structs. This improves test coverage.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6070
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6115
This commit is contained in:
Aliaksandr Valialkin 2024-04-17 14:03:15 +02:00
parent 161404813c
commit db85744e04
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB
6 changed files with 107 additions and 142 deletions

View file

@ -90,8 +90,8 @@ type UserInfo struct {
// HeadersConf represents config for request and response headers.
type HeadersConf struct {
RequestHeaders []Header `yaml:"headers,omitempty"`
ResponseHeaders []Header `yaml:"response_headers,omitempty"`
RequestHeaders []*Header `yaml:"headers,omitempty"`
ResponseHeaders []*Header `yaml:"response_headers,omitempty"`
}
func (ui *UserInfo) beginConcurrencyLimit() error {
@ -155,10 +155,10 @@ type URLMap struct {
SrcHosts []*Regex `yaml:"src_hosts,omitempty"`
// SrcQueryArgs is an optional list of query args, which must match request URL query args.
SrcQueryArgs []QueryArg `yaml:"src_query_args,omitempty"`
SrcQueryArgs []*QueryArg `yaml:"src_query_args,omitempty"`
// SrcHeaders is an optional list of headers, which must match request headers.
SrcHeaders []Header `yaml:"src_headers,omitempty"`
SrcHeaders []*Header `yaml:"src_headers,omitempty"`
// UrlPrefix contains backend url prefixes for the proxied request url.
URLPrefix *URLPrefix `yaml:"url_prefix,omitempty"`
@ -179,13 +179,6 @@ type URLMap struct {
DropSrcPathPrefixParts *int `yaml:"drop_src_path_prefix_parts,omitempty"`
}
// Regex represents a regex
type Regex struct {
re *regexp.Regexp
sOriginal string
}
// QueryArg represents HTTP query arg
type QueryArg struct {
Name string
@ -194,7 +187,7 @@ type QueryArg struct {
sOriginal string
}
// UnmarshalYAML unmarshals up from yaml.
// UnmarshalYAML unmarshals qa from yaml.
func (qa *QueryArg) UnmarshalYAML(f func(interface{}) error) error {
var s string
if err := f(&s); err != nil {
@ -208,16 +201,22 @@ func (qa *QueryArg) UnmarshalYAML(f func(interface{}) error) error {
}
qa.Name = s[:n]
expr := []byte(s[n+1:])
expr := s[n+1:]
if !strings.HasPrefix(expr, "~") {
expr = regexp.QuoteMeta(expr)
} else {
expr = expr[1:]
}
var re Regex
if err := yaml.Unmarshal(expr, &re); err != nil {
return fmt.Errorf("failed to unmarshal regex %q: %s", expr, err)
if err := yaml.Unmarshal([]byte(expr), &re); err != nil {
return fmt.Errorf("cannot unmarshal regex for %q query arg: %w", qa.Name, err)
}
qa.Value = &re
return nil
}
// MarshalYAML marshals up to yaml.
// MarshalYAML marshals qa to yaml.
func (qa *QueryArg) MarshalYAML() (interface{}, error) {
return qa.sOriginal, nil
}
@ -515,6 +514,13 @@ func (up *URLPrefix) MarshalYAML() (interface{}, error) {
return up.vOriginal, nil
}
// Regex represents a regex
type Regex struct {
re *regexp.Regexp
sOriginal string
}
func (r *Regex) match(s string) bool {
prefix, ok := r.re.LiteralPrefix()
if ok {

View file

@ -4,10 +4,11 @@ import (
"bytes"
"fmt"
"net/url"
"regexp"
"testing"
"gopkg.in/yaml.v2"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
)
func TestParseAuthConfigFailure(t *testing.T) {
@ -384,35 +385,21 @@ users:
{
SrcHosts: getRegexs([]string{"foo\\.bar", "baz:1234"}),
SrcPaths: getRegexs([]string{"/api/v1/write"}),
SrcQueryArgs: []QueryArg{
{
Name: "foo",
Value: &Regex{
sOriginal: "bar",
re: regexp.MustCompile("^(?:bar)$"),
},
},
SrcQueryArgs: []*QueryArg{
mustNewQueryArg("foo=b.+ar"),
mustNewQueryArg("baz=~.*x=y.+"),
},
SrcHeaders: []Header{
{
Name: "TenantID",
Value: "345",
},
SrcHeaders: []*Header{
mustNewHeader("'TenantID: 345'"),
},
URLPrefix: mustParseURLs([]string{
"http://vminsert1/insert/0/prometheus",
"http://vminsert2/insert/0/prometheus",
}),
HeadersConf: HeadersConf{
RequestHeaders: []Header{
{
Name: "foo",
Value: "bar",
},
{
Name: "xxx",
Value: "y",
},
RequestHeaders: []*Header{
mustNewHeader("'foo: bar'"),
mustNewHeader("'xxx: y'"),
},
},
},
@ -426,7 +413,7 @@ users:
url_prefix: http://vmselect/select/0/prometheus
- src_paths: ["/api/v1/write"]
src_hosts: ["foo\\.bar", "baz:1234"]
src_query_args: ['foo=bar']
src_query_args: ['foo=b.+ar', 'baz=~.*x=y.+']
src_headers: ['TenantID: 345']
url_prefix: ["http://vminsert1/insert/0/prometheus","http://vminsert2/insert/0/prometheus"]
headers:
@ -489,15 +476,9 @@ users:
"http://vminsert2/insert/0/prometheus",
}),
HeadersConf: HeadersConf{
RequestHeaders: []Header{
{
Name: "foo",
Value: "bar",
},
{
Name: "xxx",
Value: "y",
},
RequestHeaders: []*Header{
mustNewHeader("'foo: bar'"),
mustNewHeader("'xxx: y'"),
},
},
},
@ -521,15 +502,9 @@ users:
"http://vminsert2/insert/0/prometheus",
}),
HeadersConf: HeadersConf{
RequestHeaders: []Header{
{
Name: "foo",
Value: "bar",
},
{
Name: "xxx",
Value: "y",
},
RequestHeaders: []*Header{
mustNewHeader("'foo: bar'"),
mustNewHeader("'xxx: y'"),
},
},
},
@ -702,10 +677,7 @@ func isSetBool(boolP *bool, expectedValue bool) bool {
func getRegexs(paths []string) []*Regex {
var sps []*Regex
for _, path := range paths {
sps = append(sps, &Regex{
sOriginal: path,
re: regexp.MustCompile("^(?:" + path + ")$"),
})
sps = append(sps, mustNewRegex(path))
}
return sps
}
@ -762,3 +734,27 @@ func mustParseURLs(us []string) *URLPrefix {
func intp(n int) *int {
return &n
}
func mustNewRegex(s string) *Regex {
var re Regex
if err := yaml.Unmarshal([]byte(s), &re); err != nil {
logger.Panicf("cannot unmarshal regex %q: %s", s, err)
}
return &re
}
func mustNewQueryArg(s string) *QueryArg {
var qa QueryArg
if err := yaml.Unmarshal([]byte(s), &qa); err != nil {
logger.Panicf("cannot unmarshal query arg filter %q: %s", s, err)
}
return &qa
}
func mustNewHeader(s string) *Header {
var h Header
if err := yaml.Unmarshal([]byte(s), &h); err != nil {
logger.Panicf("cannot unmarshal header filter %q: %s", s, err)
}
return &h
}

View file

@ -323,7 +323,7 @@ func copyHeader(dst, src http.Header) {
}
}
func updateHeadersByConfig(headers http.Header, config []Header) {
func updateHeadersByConfig(headers http.Header, config []*Header) {
for _, h := range config {
if h.Value == "" {
headers.Del(h.Name)

View file

@ -86,7 +86,7 @@ func matchAnyRegex(rs []*Regex, s string) bool {
return false
}
func matchAnyQueryArg(qas []QueryArg, args url.Values) bool {
func matchAnyQueryArg(qas []*QueryArg, args url.Values) bool {
if len(qas) == 0 {
return true
}
@ -104,7 +104,7 @@ func matchAnyQueryArg(qas []QueryArg, args url.Values) bool {
return false
}
func matchAnyHeader(headers []Header, h http.Header) bool {
func matchAnyHeader(headers []*Header, h http.Header) bool {
if len(headers) == 0 {
return true
}

View file

@ -4,7 +4,6 @@ import (
"fmt"
"net/url"
"reflect"
"regexp"
"strings"
"testing"
)
@ -99,10 +98,7 @@ func TestCreateTargetURLSuccess(t *testing.T) {
target := mergeURLs(bu.url, u, up.dropSrcPathPrefixParts)
bu.put()
gotTarget, err := url.QueryUnescape(target.String())
if err != nil {
t.Fatalf("failed to unescape query %q: %s", target, err)
}
gotTarget := target.String()
if gotTarget != expectedTarget {
t.Fatalf("unexpected target; \ngot:\n%q;\nwant:\n%q", gotTarget, expectedTarget)
}
@ -129,17 +125,11 @@ func TestCreateTargetURLSuccess(t *testing.T) {
f(&UserInfo{
URLPrefix: mustParseURL("http://foo.bar"),
HeadersConf: HeadersConf{
RequestHeaders: []Header{
{
Name: "bb",
Value: "aaa",
},
RequestHeaders: []*Header{
mustNewHeader("'bb: aaa'"),
},
ResponseHeaders: []Header{
{
Name: "x",
Value: "y",
},
ResponseHeaders: []*Header{
mustNewHeader("'x: y'"),
},
},
RetryStatusCodes: []int{503, 501},
@ -160,7 +150,7 @@ func TestCreateTargetURLSuccess(t *testing.T) {
}, "/../../aaa", "https://sss:3894/x/y/aaa", "", "", nil, "least_loaded", 0)
f(&UserInfo{
URLPrefix: mustParseURL("https://sss:3894/x/y"),
}, "/./asd/../../aaa?a=d&s=s/../d", "https://sss:3894/x/y/aaa?a=d&s=s/../d", "", "", nil, "least_loaded", 0)
}, "/./asd/../../aaa?a=d&s=s/../d", "https://sss:3894/x/y/aaa?a=d&s=s%2F..%2Fd", "", "", nil, "least_loaded", 0)
// Complex routing with `url_map`
ui := &UserInfo{
@ -168,32 +158,17 @@ func TestCreateTargetURLSuccess(t *testing.T) {
{
SrcHosts: getRegexs([]string{"host42"}),
SrcPaths: getRegexs([]string{"/vmsingle/api/v1/query"}),
SrcQueryArgs: []QueryArg{
{
Name: "db",
Value: &Regex{
sOriginal: "foo",
re: regexp.MustCompile("^(?:foo)$"),
},
},
SrcQueryArgs: []*QueryArg{
mustNewQueryArg("db=foo"),
},
URLPrefix: mustParseURL("http://vmselect/0/prometheus"),
HeadersConf: HeadersConf{
RequestHeaders: []Header{
{
Name: "xx",
Value: "aa",
},
{
Name: "yy",
Value: "asdf",
},
RequestHeaders: []*Header{
mustNewHeader("'xx: aa'"),
mustNewHeader("'yy: asdf'"),
},
ResponseHeaders: []Header{
{
Name: "qwe",
Value: "rty",
},
ResponseHeaders: []*Header{
mustNewHeader("'qwe: rty'"),
},
},
RetryStatusCodes: []int{503, 500, 501},
@ -209,14 +184,12 @@ func TestCreateTargetURLSuccess(t *testing.T) {
},
URLPrefix: mustParseURL("http://default-server"),
HeadersConf: HeadersConf{
RequestHeaders: []Header{{
Name: "bb",
Value: "aaa",
}},
ResponseHeaders: []Header{{
Name: "x",
Value: "y",
}},
RequestHeaders: []*Header{
mustNewHeader("'bb: aaa'"),
},
ResponseHeaders: []*Header{
mustNewHeader("'x: y'"),
},
},
RetryStatusCodes: []int{502},
DropSrcPathPrefixParts: intp(2),
@ -258,43 +231,31 @@ func TestCreateTargetURLSuccess(t *testing.T) {
}, "/api/v1/query", "http://foo.bar/api/v1/query?extra_label=team=dev", "", "", nil, "least_loaded", 0)
f(&UserInfo{
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=mobile", "", "", nil, "least_loaded", 0)
}, "/api/v1/query?extra_label=team=dev", "http://foo.bar/api/v1/query?extra_label=team%3Dmobile", "", "", nil, "least_loaded", 0)
// Complex routing regexp query args in `url_map`
ui = &UserInfo{
URLMaps: []URLMap{
{
SrcPaths: getRegexs([]string{"/api/v1/query"}),
SrcQueryArgs: []QueryArg{
{
Name: "query",
Value: &Regex{
sOriginal: "foo",
re: regexp.MustCompile(`^(?:.*env="dev".*)$`),
},
},
SrcQueryArgs: []*QueryArg{
mustNewQueryArg(`query=~.*{.*env="dev".*}*.`),
},
URLPrefix: mustParseURL("http://vmselect/0/prometheus"),
},
{
SrcPaths: getRegexs([]string{"/api/v1/query"}),
SrcQueryArgs: []QueryArg{
{
Name: "query",
Value: &Regex{
sOriginal: "foo",
re: regexp.MustCompile(`^(?:.*env="prod".*)$`),
},
},
SrcQueryArgs: []*QueryArg{
mustNewQueryArg(`query=~.*{.*env="prod".*}.*`),
},
URLPrefix: mustParseURL("http://vmselect/1/prometheus"),
},
},
URLPrefix: mustParseURL("http://default-server"),
}
f(ui, `/api/v1/query?query=up{env="prod"}`, `http://vmselect/1/prometheus/api/v1/query?query=up{env="prod"}`, "", "", nil, "least_loaded", 0)
f(ui, `/api/v1/query?query=up{foo="bar", env="dev", pod!=""}`, `http://vmselect/0/prometheus/api/v1/query?query=up{foo="bar", env="dev", pod!=""}`, "", "", nil, "least_loaded", 0)
f(ui, `/api/v1/query?query=up{foo="bar"}`, `http://default-server/api/v1/query?query=up{foo="bar"}`, "", "", nil, "least_loaded", 0)
f(ui, `/api/v1/query?query=up{env="prod"}`, `http://vmselect/1/prometheus/api/v1/query?query=up%7Benv%3D%22prod%22%7D`, "", "", nil, "least_loaded", 0)
f(ui, `/api/v1/query?query=up{foo="bar",env="dev",pod!=""}`, `http://vmselect/0/prometheus/api/v1/query?query=up%7Bfoo%3D%22bar%22%2Cenv%3D%22dev%22%2Cpod%21%3D%22%22%7D`, "", "", nil, "least_loaded", 0)
f(ui, `/api/v1/query?query=up{foo="bar"}`, `http://default-server/api/v1/query?query=up%7Bfoo%3D%22bar%22%7D`, "", "", nil, "least_loaded", 0)
}
func TestCreateTargetURLFailure(t *testing.T) {
@ -310,10 +271,10 @@ func TestCreateTargetURLFailure(t *testing.T) {
t.Fatalf("unexpected non-empty up=%#v", up)
}
if hc.RequestHeaders != nil {
t.Fatalf("unexpected non-empty request headers=%q", hc.RequestHeaders)
t.Fatalf("unexpected non-empty request headers: %s", headersToString(hc.RequestHeaders))
}
if hc.ResponseHeaders != nil {
t.Fatalf("unexpected non-empty response headers=%q", hc.ResponseHeaders)
t.Fatalf("unexpected non-empty response headers: %s", headersToString(hc.ResponseHeaders))
}
}
f(&UserInfo{}, "/foo/bar")
@ -327,7 +288,7 @@ func TestCreateTargetURLFailure(t *testing.T) {
}, "/api/v1/write")
}
func headersToString(hs []Header) string {
func headersToString(hs []*Header) string {
a := make([]string, len(hs))
for i, h := range hs {
a[i] = fmt.Sprintf("%s: %s", h.Name, h.Value)

View file

@ -135,18 +135,20 @@ If `src_query_args` contains multiple entries, then it is enough to match only a
If `src_query_args` are specified together with `src_hosts`, `src_paths` or `src_headers`, then the request is routed to the given `url_prefix`
if its query args, host, path and headers match the given lists simultaneously.
`src_query_args` supports regex matching:
`src_query_args` supports [regex matching](https://github.com/google/re2/wiki/Syntax) by using `arg=~regex` syntax:
```yaml
unauthorized_user:
url_map:
- src_query_args: [ "query=.*env=\"prod\".*" ]
url_prefix: "http://prod-backend/"
- src_query_args: [ "query=.*env=\"dev\".*" ]
url_prefix: "http://dev-backend/"
- src_query_args: ['query=~.*{.*env="prod".*}.*']
url_prefix: 'http://prod-backend/'
- src_query_args: ['query=~.*{.*env="dev".*}.*']
url_prefix: 'http://dev-backend/'
```
The config above will route requests like `/api/v1/query?query=up{env="prod"}` to `http://prod-backend/`.
And queries matching `.*env=\"dev\".*` will be routed to `http://dev-backend/`.
_Please note, by default Grafana sends `query` param in request's body and vmauth won't be able to read it.
The config above routes requests like `/api/v1/query?query=up{env="prod"}` to `http://prod-backend/`, while requests to `/api/v1/query?query=up{env="dev"}` are routed to `http://dev-backend/`.
_Please note, by default Grafana sends `query` param in request's body and vmauth won't be able to read it.
You need to manually switch datasource settings in Grafana to use GET method for sending queries._
An optional `src_headers` can be used for routing requests based on HTTP request headers additionally to hostname, path and [HTTP query args](https://en.wikipedia.org/wiki/Query_string).