From d0a508d1caf66c15a7ee5e1860d31ba45e757d1d Mon Sep 17 00:00:00 2001 From: Nikolay Date: Fri, 29 Nov 2024 14:04:07 +0100 Subject: [PATCH 1/3] app/vmagent: fixes multitenant token parse Previously, vmagent produced parsing error for 'multitenant' auth token value for the cases: * data ingestion with enableMultitentEndpoints * data scrapping at promscrape It's inconsistent to the other VictoriaMetrics components. Since 'multitenant' is well-known token value for multitenancy via labels. And vmagent is intended to be compatible with vminsert ingestion endpoints. This commit replaces NewToken with NewTokenPossibleMultitenant function for token parsing. It allows to use multitenant value for it. And it makes token values consistent for the all components. Related issue: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7694 --- app/vmagent/main.go | 2 +- docs/changelog/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/vmagent/main.go b/app/vmagent/main.go index c20b9f53fe..926947e339 100644 --- a/app/vmagent/main.go +++ b/app/vmagent/main.go @@ -498,7 +498,7 @@ func processMultitenantRequest(w http.ResponseWriter, r *http.Request, path stri httpserver.Errorf(w, r, `unsupported multitenant prefix: %q; expected "insert"`, p.Prefix) return true } - at, err := auth.NewToken(p.AuthToken) + at, err := auth.NewTokenPossibleMultitenant(p.AuthToken) if err != nil { httpserver.Errorf(w, r, "cannot obtain auth token: %s", err) return true diff --git a/docs/changelog/CHANGELOG.md b/docs/changelog/CHANGELOG.md index 1ad17bd610..e2a310af38 100644 --- a/docs/changelog/CHANGELOG.md +++ b/docs/changelog/CHANGELOG.md @@ -30,6 +30,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): add `dryRun` flag to validate configuration. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7505) for details. * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): add `removeXFFHTTPHeaderValue` flag to remove content of `X-Forwarded-For` HTTP Header before proxy it to the backend. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6883) for details. +* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent): properly parse `multitenant` token value for multitenant endpoints. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7694). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent): Properly return `200 OK` HTTP status code when importing data via [Pushgateway protocol](https://docs.victoriametrics.com/#how-to-import-data-in-prometheus-exposition-format) using [multitenant URL format](https://docs.victoriametrics.com/cluster-victoriametrics/#url-format). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3636) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7571). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent): Properly set `TCP` connection timeout for `Kubernetes API server` connection for metric scrapping with `kubernetes_sd_configs`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7127). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent): fix the `resource_group` filter for Azure service discovery on virtual machine scale sets. Previously, this filter did not apply to virtual machine scale sets, causing all virtual machines to be discovered. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7630). From f97489dcf9ce4a9ea3a547dd4900c99d503f44f1 Mon Sep 17 00:00:00 2001 From: f41gh7 Date: Fri, 29 Nov 2024 14:36:59 +0100 Subject: [PATCH 2/3] app/vmagent: follow-up 430163d and 680b8c2 Removes global defaultAuthToken, since it's no longer needed. It was added as fallback for 'remoteWrite.multitenantURL' feature. This feature was deprecated at v1.102 version and removed. Updates newRemoteWriteCtxs function, it shouldn't accept auth.Token no longer. This was also a part of remove feature. Signed-off-by: f41gh7 --- app/vmagent/remotewrite/remotewrite.go | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/app/vmagent/remotewrite/remotewrite.go b/app/vmagent/remotewrite/remotewrite.go index ea74c4985d..897b099031 100644 --- a/app/vmagent/remotewrite/remotewrite.go +++ b/app/vmagent/remotewrite/remotewrite.go @@ -99,9 +99,6 @@ var ( // rwctxsGlobal contains statically populated entries when -remoteWrite.url is specified. rwctxsGlobal []*remoteWriteCtx - // Data without tenant id is written to defaultAuthToken if -enableMultitenantHandlers is specified. - defaultAuthToken = &auth.Token{} - // ErrQueueFullHTTPRetry must be returned when TryPush() returns false. ErrQueueFullHTTPRetry = &httpserver.ErrorWithStatusCode{ Err: fmt.Errorf("remote storage systems cannot keep up with the data ingestion rate; retry the request later " + @@ -209,7 +206,7 @@ func Init() { initStreamAggrConfigGlobal() - rwctxsGlobal = newRemoteWriteCtxs(nil, *remoteWriteURLs) + rwctxsGlobal = newRemoteWriteCtxs(*remoteWriteURLs) disableOnDiskQueues := []bool(*disableOnDiskQueue) disableOnDiskQueueAny = slices.Contains(disableOnDiskQueues, true) @@ -294,7 +291,7 @@ var ( relabelConfigTimestamp = metrics.NewCounter(`vmagent_relabel_config_last_reload_success_timestamp_seconds`) ) -func newRemoteWriteCtxs(at *auth.Token, urls []string) []*remoteWriteCtx { +func newRemoteWriteCtxs(urls []string) []*remoteWriteCtx { if len(urls) == 0 { logger.Panicf("BUG: urls must be non-empty") } @@ -316,11 +313,6 @@ func newRemoteWriteCtxs(at *auth.Token, urls []string) []*remoteWriteCtx { logger.Fatalf("invalid -remoteWrite.url=%q: %s", remoteWriteURL, err) } sanitizedURL := fmt.Sprintf("%d:secret-url", i+1) - if at != nil { - // Construct full remote_write url for the given tenant according to https://docs.victoriametrics.com/cluster-victoriametrics/#url-format - remoteWriteURL.Path = fmt.Sprintf("%s/insert/%d:%d/prometheus/api/v1/write", remoteWriteURL.Path, at.AccountID, at.ProjectID) - sanitizedURL = fmt.Sprintf("%s:%d:%d", sanitizedURL, at.AccountID, at.ProjectID) - } if *showRemoteWriteURL { sanitizedURL = fmt.Sprintf("%d:%s", i+1, remoteWriteURL) } @@ -411,11 +403,6 @@ func TryPush(at *auth.Token, wr *prompbmarshal.WriteRequest) bool { func tryPush(at *auth.Token, wr *prompbmarshal.WriteRequest, forceDropSamplesOnFailure bool) bool { tss := wr.Timeseries - if at == nil && MultitenancyEnabled() { - // Write data to default tenant if at isn't set when multitenancy is enabled. - at = defaultAuthToken - } - var tenantRctx *relabelCtx if at != nil { // Convert at to (vm_account_id, vm_project_id) labels. From de9a2169988bc60480a210adabe50144ea4161c2 Mon Sep 17 00:00:00 2001 From: f41gh7 Date: Fri, 29 Nov 2024 14:45:18 +0100 Subject: [PATCH 3/3] app/vmctl: follow-up after vendor-update Comment broken tests for remote_read integration test. Prometheus broke library compatibility and it's required to rewrite tests. Also, test structure and format should be revisited and improved according to our test code style. Signed-off-by: f41gh7 --- app/vmctl/remoteread/remoteread.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/vmctl/remoteread/remoteread.go b/app/vmctl/remoteread/remoteread.go index 2bf62a138c..c76024a8fa 100644 --- a/app/vmctl/remoteread/remoteread.go +++ b/app/vmctl/remoteread/remoteread.go @@ -15,8 +15,10 @@ import ( "github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil" "github.com/gogo/protobuf/proto" "github.com/golang/snappy" + "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/storage/remote" + "github.com/prometheus/prometheus/tsdb/chunkenc" ) @@ -238,7 +240,7 @@ func processStreamResponse(body io.ReadCloser, callback StreamCallback) error { bb := bbPool.Get() defer func() { bbPool.Put(bb) }() - stream := remote.NewChunkedReader(body, remote.DefaultChunkedReadLimit, bb.B) + stream := remote.NewChunkedReader(body, config.DefaultChunkedReadLimit, bb.B) for { res := &prompb.ChunkedReadResponse{} err := stream.NextProto(res)