From 33a4f275b18211d1bcbcf887a7d062cee51886fe Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Tue, 12 Nov 2024 17:50:38 +0100 Subject: [PATCH] app/vmauth: properly inherit user-level options at url_map when url_prefix isnt set at the user level The following user-level options must be unconditionally inherited by url_map, since this is what most users expect: - retry_status_codes - load_balancing_policy - drop_src_path_prefix_parts - discover_backend_ips Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7519 --- app/vmauth/auth_config.go | 25 +++++++++++++------------ app/vmauth/target_url_test.go | 33 +++++++++++++++++++++++++++++++++ docs/changelog/CHANGELOG.md | 3 ++- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go index 63f177ef5..41d21b866 100644 --- a/app/vmauth/auth_config.go +++ b/app/vmauth/auth_config.go @@ -860,22 +860,23 @@ func (ui *UserInfo) initURLs() error { loadBalancingPolicy := *defaultLoadBalancingPolicy dropSrcPathPrefixParts := 0 discoverBackendIPs := *discoverBackendIPsGlobal + if ui.RetryStatusCodes != nil { + retryStatusCodes = ui.RetryStatusCodes + } + if ui.LoadBalancingPolicy != "" { + loadBalancingPolicy = ui.LoadBalancingPolicy + } + if ui.DropSrcPathPrefixParts != nil { + dropSrcPathPrefixParts = *ui.DropSrcPathPrefixParts + } + if ui.DiscoverBackendIPs != nil { + discoverBackendIPs = *ui.DiscoverBackendIPs + } + if ui.URLPrefix != nil { if err := ui.URLPrefix.sanitizeAndInitialize(); err != nil { return err } - if ui.RetryStatusCodes != nil { - retryStatusCodes = ui.RetryStatusCodes - } - if ui.LoadBalancingPolicy != "" { - loadBalancingPolicy = ui.LoadBalancingPolicy - } - if ui.DropSrcPathPrefixParts != nil { - dropSrcPathPrefixParts = *ui.DropSrcPathPrefixParts - } - if ui.DiscoverBackendIPs != nil { - discoverBackendIPs = *ui.DiscoverBackendIPs - } ui.URLPrefix.retryStatusCodes = retryStatusCodes ui.URLPrefix.dropSrcPathPrefixParts = dropSrcPathPrefixParts ui.URLPrefix.discoverBackendIPs = discoverBackendIPs diff --git a/app/vmauth/target_url_test.go b/app/vmauth/target_url_test.go index d26d005d3..98abff4de 100644 --- a/app/vmauth/target_url_test.go +++ b/app/vmauth/target_url_test.go @@ -187,6 +187,10 @@ func TestCreateTargetURLSuccess(t *testing.T) { RetryStatusCodes: []int{}, DropSrcPathPrefixParts: intp(0), }, + { + SrcPaths: getRegexs([]string{"/metrics"}), + URLPrefix: mustParseURL("http://metrics-server"), + }, }, URLPrefix: mustParseURL("http://default-server"), HeadersConf: HeadersConf{ @@ -206,6 +210,35 @@ func TestCreateTargetURLSuccess(t *testing.T) { "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{}, "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) + f(ui, "https://foo-host/metrics", "http://metrics-server", "", "", []int{502}, "least_loaded", 2) + + // Complex routing with `url_map` without global url_prefix + ui = &UserInfo{ + URLMaps: []URLMap{ + { + SrcPaths: getRegexs([]string{"/api/v1/write"}), + URLPrefix: mustParseURL("http://vminsert/0/prometheus"), + RetryStatusCodes: []int{}, + DropSrcPathPrefixParts: intp(0), + }, + { + SrcPaths: getRegexs([]string{"/metrics/a/b"}), + URLPrefix: mustParseURL("http://metrics-server"), + }, + }, + HeadersConf: HeadersConf{ + RequestHeaders: []*Header{ + mustNewHeader("'bb: aaa'"), + }, + ResponseHeaders: []*Header{ + mustNewHeader("'x: y'"), + }, + }, + RetryStatusCodes: []int{502}, + DropSrcPathPrefixParts: intp(2), + } + f(ui, "https://foo-host/api/v1/write", "http://vminsert/0/prometheus/api/v1/write", "", "", []int{}, "least_loaded", 0) + f(ui, "https://foo-host/metrics/a/b", "http://metrics-server/b", "", "", []int{502}, "least_loaded", 2) // Complex routing regexp paths in `url_map` ui = &UserInfo{ diff --git a/docs/changelog/CHANGELOG.md b/docs/changelog/CHANGELOG.md index 105f80710..315e3da9e 100644 --- a/docs/changelog/CHANGELOG.md +++ b/docs/changelog/CHANGELOG.md @@ -23,7 +23,8 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl/): drop rows that do not belong to the current series during import. The dropped rows should belong to another series whose tags are a superset of the current series. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7301) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7330). Thanks to @dpedu for reporting and cooperating with the test. * BUGFIX: [vmsingle](https://docs.victoriametrics.com/single-server-victoriametrics/), `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): keep the order of resulting time series when `limit_offset` is applied. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7068). * BUGFIX: [graphite](https://docs.victoriametrics.com/#graphite-render-api-usage): properly handle xFilesFactor=0 for `transformRemoveEmptySeries` function. See [this PR](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7337) for details. -* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth): properly check availability of all the backends before giving up when proxying requests. Previously, vmauth could return an error even if there were healthy backends available. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3061) for details. +* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): properly check availability of all the backends before giving up when proxying requests. Previously, vmauth could return an error even if there were healthy backends available. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3061) for details. +* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): properly inherit [`drop_src_path_prefix_parts`](https://docs.victoriametrics.com/vmauth/#dropping-request-path-prefix), [`load_balancing_policy`](https://docs.victoriametrics.com/vmauth/#high-availability), [`retry_status_codes`](https://docs.victoriametrics.com/vmauth/#load-balancing) and [`discover_backend_ips`](https://docs.victoriametrics.com/vmauth/#discovering-backend-ips) options by `url_map` entries if `url_prefix` option isn't set at the [user config level](https://docs.victoriametrics.com/vmauth/#auth-config). These options were inherited only when the `url_prefix` option was set. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7519). * BUGFIX: [dashboards](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/dashboards): add `file` label filter to vmalert dashboard panels. Previously, metrics from groups with the same name but different rule files could be mixed in the results. ## [v1.106.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.106.0)