From f4506f549a5e7c8db4b915a9614af1b41e8b8321 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Thu, 3 Aug 2023 19:56:06 +0200 Subject: [PATCH 01/23] Make query frontend forward tenant info downstream Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 13 +++++++++++-- pkg/queryfrontend/config.go | 2 ++ pkg/queryfrontend/roundtrip.go | 30 +++++++++++++++++++++++------- pkg/tenancy/tenancy.go | 15 ++++++++++++++- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index f3d2b1a9f7..34ed52b471 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -4,6 +4,7 @@ package main import ( + "github.com/thanos-io/thanos/pkg/tenancy" "net" "net/http" "time" @@ -37,10 +38,10 @@ import ( ) type queryFrontendConfig struct { + queryfrontend.Config http httpConfig webDisableCORS bool - queryfrontend.Config - orgIdHeaders []string + orgIdHeaders []string } func registerQueryFrontend(app *extkingpin.App) { @@ -146,6 +147,9 @@ func registerQueryFrontend(app *extkingpin.App) { cmd.Flag("query-frontend.forward-header", "List of headers forwarded by the query-frontend to downstream queriers, default is empty").PlaceHolder("").StringsVar(&cfg.ForwardHeaders) + cmd.Flag("query-frontend.tenant-header", "HTTP header to determine tenant").Default(tenancy.DefaultTenantHeader).Hidden().StringVar(&cfg.TenantHeader) + cmd.Flag("query-frontend.default-tenant", "Default tenant to use if tenant header is not present").Default(tenancy.DefaultTenant).Hidden().StringVar(&cfg.DefaultTenant) + cmd.Flag("query-frontend.vertical-shards", "Number of shards to use when distributing shardable PromQL queries. For more details, you can refer to the Vertical query sharding proposal: https://thanos.io/tip/proposals-accepted/202205-vertical-query-sharding.md").IntVar(&cfg.NumShards) cmd.Flag("log.request.decision", "Deprecation Warning - This flag would be soon deprecated, and replaced with `request.logging-config`. Request Logging for logging the start and end of requests. By default this flag is disabled. LogFinishCall : Logs the finish call of the requests. LogStartAndFinishCall : Logs the start and finish call of the requests. NoLogCall : Disable request logging.").Default("").EnumVar(&cfg.RequestLoggingDecision, "NoLogCall", "LogFinishCall", "LogStartAndFinishCall", "") @@ -217,6 +221,11 @@ func runQueryFrontend( cfg *queryFrontendConfig, comp component.Component, ) error { + if cfg.TenantHeader != "" { + cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) + cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader, tenancy.DefaultTenantHeader) + } + queryRangeCacheConfContentYaml, err := cfg.QueryRangeConfig.CachePathOrContent.Content() if err != nil { return err diff --git a/pkg/queryfrontend/config.go b/pkg/queryfrontend/config.go index a56551995c..83c3779dcb 100644 --- a/pkg/queryfrontend/config.go +++ b/pkg/queryfrontend/config.go @@ -203,6 +203,8 @@ type Config struct { DownstreamURL string ForwardHeaders []string NumShards int + TenantHeader string + DefaultTenant string } // QueryRangeConfig holds the config for query range tripperware. diff --git a/pkg/queryfrontend/roundtrip.go b/pkg/queryfrontend/roundtrip.go index ef557e0a35..d1f3f5fbde 100644 --- a/pkg/queryfrontend/roundtrip.go +++ b/pkg/queryfrontend/roundtrip.go @@ -4,6 +4,7 @@ package queryfrontend import ( + "github.com/thanos-io/thanos/pkg/tenancy" "net/http" "regexp" "strings" @@ -78,22 +79,36 @@ func NewTripperware(config Config, reg prometheus.Registerer, logger log.Logger) config.ForwardHeaders, ) return func(next http.RoundTripper) http.RoundTripper { - return newRoundTripper(next, queryRangeTripperware(next), labelsTripperware(next), queryInstantTripperware(next), reg) + // TODO: Create a tripperware to rewrite the tenant header for internal communications and wrap the ones below + // with it. + return newRoundTripper( + next, + queryRangeTripperware(next), + labelsTripperware(next), + queryInstantTripperware(next), + config.TenantHeader, + config.DefaultTenant, + reg, + ) }, nil } type roundTripper struct { next, queryInstant, queryRange, labels http.RoundTripper - queriesCount *prometheus.CounterVec + queriesCount *prometheus.CounterVec + tenantHeader string + defaultTenant string } -func newRoundTripper(next, queryRange, metadata, queryInstant http.RoundTripper, reg prometheus.Registerer) roundTripper { +func newRoundTripper(next, queryRange, metadata, queryInstant http.RoundTripper, tenantHeader, defaultTenant string, reg prometheus.Registerer) roundTripper { r := roundTripper{ - next: next, - queryInstant: queryInstant, - queryRange: queryRange, - labels: metadata, + next: next, + queryInstant: queryInstant, + queryRange: queryRange, + labels: metadata, + tenantHeader: tenantHeader, + defaultTenant: defaultTenant, queriesCount: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ Name: "thanos_query_frontend_queries_total", Help: "Total queries passing through query frontend", @@ -109,6 +124,7 @@ func newRoundTripper(next, queryRange, metadata, queryInstant http.RoundTripper, } func (r roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + tenancy.ForwardTenantInternalRequest(req, r.tenantHeader) switch op := getOperation(req); op { case instantQueryOp: r.queriesCount.WithLabelValues(instantQueryOp).Inc() diff --git a/pkg/tenancy/tenancy.go b/pkg/tenancy/tenancy.go index 13775cf6e1..77e5359bd4 100644 --- a/pkg/tenancy/tenancy.go +++ b/pkg/tenancy/tenancy.go @@ -45,7 +45,10 @@ func GetTenantFromHTTP(r *http.Request, tenantHeader string, defaultTenantID str var err error tenant := r.Header.Get(tenantHeader) if tenant == "" { - tenant = defaultTenantID + tenant = r.Header.Get(DefaultTenantHeader) + if tenant == "" { + tenant = defaultTenantID + } } if certTenantField != "" { @@ -63,6 +66,16 @@ func GetTenantFromHTTP(r *http.Request, tenantHeader string, defaultTenantID str return tenant, nil } +// ForwardTenantInternalRequest rewrites the configurable tenancy header in the request into the hardcoded tenancy +// header that is used for internal communication in Thanos components. +func ForwardTenantInternalRequest(r *http.Request, customTenantHeader string) *http.Request { + if tenant := r.Header.Get(customTenantHeader); tenant != "" { + r.Header.Set(DefaultTenantHeader, tenant) + r.Header.Del(customTenantHeader) + } + return r +} + // getTenantFromCertificate extracts the tenant value from a client's presented certificate. The x509 field to use as // value can be configured with Options.TenantField. An error is returned when the extraction has not succeeded. func getTenantFromCertificate(r *http.Request, certTenantField string) (string, error) { From 4b2ca14c2adb37fa59ed53a445cd36b1bbb48271 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Thu, 3 Aug 2023 20:04:51 +0200 Subject: [PATCH 02/23] Add TODO for client certificate Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- pkg/tenancy/tenancy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/tenancy/tenancy.go b/pkg/tenancy/tenancy.go index 77e5359bd4..4710b3ee1a 100644 --- a/pkg/tenancy/tenancy.go +++ b/pkg/tenancy/tenancy.go @@ -68,6 +68,7 @@ func GetTenantFromHTTP(r *http.Request, tenantHeader string, defaultTenantID str // ForwardTenantInternalRequest rewrites the configurable tenancy header in the request into the hardcoded tenancy // header that is used for internal communication in Thanos components. +// TODO: Add support for forwarding tenant information from client certificates. func ForwardTenantInternalRequest(r *http.Request, customTenantHeader string) *http.Request { if tenant := r.Header.Get(customTenantHeader); tenant != "" { r.Header.Set(DefaultTenantHeader, tenant) From aa33d04bbcba19c9d82aa8d8a020caaa24cecba4 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Mon, 7 Aug 2023 18:00:28 +0200 Subject: [PATCH 03/23] Finish qfe tenant forward features * Add tenant forwarding based on tenant cert. * Fix bugs in the header management. Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 12 +++-- pkg/queryfrontend/config.go | 1 + pkg/queryfrontend/roundtrip.go | 6 +-- pkg/tenancy/tenancy.go | 27 +++++++---- test/e2e/query_frontend_test.go | 80 ++++++++++++++++++++++++++++++++- 5 files changed, 109 insertions(+), 17 deletions(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index 34ed52b471..c6b2cf3663 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -4,11 +4,12 @@ package main import ( - "github.com/thanos-io/thanos/pkg/tenancy" "net" "net/http" "time" + "github.com/thanos-io/thanos/pkg/tenancy" + extflag "github.com/efficientgo/tools/extkingpin" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -149,6 +150,7 @@ func registerQueryFrontend(app *extkingpin.App) { cmd.Flag("query-frontend.tenant-header", "HTTP header to determine tenant").Default(tenancy.DefaultTenantHeader).Hidden().StringVar(&cfg.TenantHeader) cmd.Flag("query-frontend.default-tenant", "Default tenant to use if tenant header is not present").Default(tenancy.DefaultTenant).Hidden().StringVar(&cfg.DefaultTenant) + cmd.Flag("query-frontend.tenant-certificate-field", "Use TLS client's certificate field to determine tenant for requests. Must be one of "+tenancy.CertificateFieldOrganization+", "+tenancy.CertificateFieldOrganizationalUnit+" or "+tenancy.CertificateFieldCommonName+". This setting will cause the query-frontend.tenant-header flag value to be ignored.").Default("").EnumVar(&cfg.TenantCertField, "", tenancy.CertificateFieldOrganization, tenancy.CertificateFieldOrganizationalUnit, tenancy.CertificateFieldCommonName) cmd.Flag("query-frontend.vertical-shards", "Number of shards to use when distributing shardable PromQL queries. For more details, you can refer to the Vertical query sharding proposal: https://thanos.io/tip/proposals-accepted/202205-vertical-query-sharding.md").IntVar(&cfg.NumShards) @@ -221,9 +223,11 @@ func runQueryFrontend( cfg *queryFrontendConfig, comp component.Component, ) error { - if cfg.TenantHeader != "" { - cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) - cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader, tenancy.DefaultTenantHeader) + cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) + cfg.orgIdHeaders = append(cfg.orgIdHeaders, tenancy.DefaultTenantHeader) + if cfg.TenantHeader != "" && cfg.TenantHeader != tenancy.DefaultTenantHeader { + cfg.ForwardHeaders = append(cfg.ForwardHeaders, cfg.TenantHeader) + cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader) } queryRangeCacheConfContentYaml, err := cfg.QueryRangeConfig.CachePathOrContent.Content() diff --git a/pkg/queryfrontend/config.go b/pkg/queryfrontend/config.go index 83c3779dcb..a84243b16c 100644 --- a/pkg/queryfrontend/config.go +++ b/pkg/queryfrontend/config.go @@ -205,6 +205,7 @@ type Config struct { NumShards int TenantHeader string DefaultTenant string + TenantCertField string } // QueryRangeConfig holds the config for query range tripperware. diff --git a/pkg/queryfrontend/roundtrip.go b/pkg/queryfrontend/roundtrip.go index d1f3f5fbde..81f8351b54 100644 --- a/pkg/queryfrontend/roundtrip.go +++ b/pkg/queryfrontend/roundtrip.go @@ -79,9 +79,7 @@ func NewTripperware(config Config, reg prometheus.Registerer, logger log.Logger) config.ForwardHeaders, ) return func(next http.RoundTripper) http.RoundTripper { - // TODO: Create a tripperware to rewrite the tenant header for internal communications and wrap the ones below - // with it. - return newRoundTripper( + tripper := newRoundTripper( next, queryRangeTripperware(next), labelsTripperware(next), @@ -90,6 +88,7 @@ func NewTripperware(config Config, reg prometheus.Registerer, logger log.Logger) config.DefaultTenant, reg, ) + return tenancy.InternalTenancyConversionTripper(config.TenantHeader, config.TenantCertField, tripper) }, nil } @@ -124,7 +123,6 @@ func newRoundTripper(next, queryRange, metadata, queryInstant http.RoundTripper, } func (r roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - tenancy.ForwardTenantInternalRequest(req, r.tenantHeader) switch op := getOperation(req); op { case instantQueryOp: r.queriesCount.WithLabelValues(instantQueryOp).Inc() diff --git a/pkg/tenancy/tenancy.go b/pkg/tenancy/tenancy.go index 4710b3ee1a..4c6ee2912b 100644 --- a/pkg/tenancy/tenancy.go +++ b/pkg/tenancy/tenancy.go @@ -66,15 +66,26 @@ func GetTenantFromHTTP(r *http.Request, tenantHeader string, defaultTenantID str return tenant, nil } -// ForwardTenantInternalRequest rewrites the configurable tenancy header in the request into the hardcoded tenancy -// header that is used for internal communication in Thanos components. -// TODO: Add support for forwarding tenant information from client certificates. -func ForwardTenantInternalRequest(r *http.Request, customTenantHeader string) *http.Request { - if tenant := r.Header.Get(customTenantHeader); tenant != "" { +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (r roundTripperFunc) RoundTrip(request *http.Request) (*http.Response, error) { + return r(request) +} + +// InternalTenancyConversionTripper is a tripperware that rewrites the configurable tenancy header in the request into +// the hardcoded tenancy header that is used for internal communication in Thanos components. If any custom tenant +// header is configured and present in the request, it will be stripped out. +func InternalTenancyConversionTripper(customTenantHeader, certTenantField string, next http.RoundTripper) http.RoundTripper { + return roundTripperFunc(func(r *http.Request) (*http.Response, error) { + tenant, _ := GetTenantFromHTTP(r, customTenantHeader, DefaultTenant, certTenantField) r.Header.Set(DefaultTenantHeader, tenant) - r.Header.Del(customTenantHeader) - } - return r + // If the custom tenant header is not the same as the default internal header, we want to exclude the custom + // one from the request to keep things simple. + if customTenantHeader != DefaultTenantHeader { + r.Header.Del(customTenantHeader) + } + return next.RoundTrip(r) + }) } // getTenantFromCertificate extracts the tenant value from a client's presented certificate. The x509 field to use as diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index 762eeecea0..eff75dd3a4 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -6,6 +6,10 @@ package e2e_test import ( "context" "fmt" + "github.com/prometheus/prometheus/model/labels" + "github.com/thanos-io/thanos/pkg/tenancy" + "net/http" + "net/http/httptest" "os" "reflect" "sort" @@ -16,11 +20,12 @@ import ( e2emon "github.com/efficientgo/e2e/monitoring" "github.com/efficientgo/e2e/monitoring/matchers" "github.com/pkg/errors" + "github.com/prometheus/client_golang/api" "github.com/prometheus/common/model" - "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/timestamp" "github.com/efficientgo/core/testutil" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/thanos-io/thanos/pkg/block/metadata" "github.com/thanos-io/thanos/pkg/cacheutil" "github.com/thanos-io/thanos/pkg/promclient" @@ -921,3 +926,76 @@ func TestInstantQueryShardingWithRandomData(t *testing.T) { }) } } + +func TestQueryFrontendTenantForward(t *testing.T) { + t.Parallel() + + e, err := e2e.New(e2e.WithName("query-frontend")) + testutil.Ok(t, err) + t.Cleanup(e2ethanos.CleanScenario(t, e)) + + testTenant := "test-tenant" + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + testutil.Equals(t, testTenant, r.Header.Get("THANOS-TENANT")) + testutil.Equals(t, testTenant, r.Header.Get("X-Scope-OrgID")) + })) + t.Cleanup(ts.Close) + tsPort := urlParse(t, ts.URL).Port() + + inMemoryCacheConfig := queryfrontend.CacheProviderConfig{ + Type: queryfrontend.INMEMORY, + Config: queryfrontend.InMemoryResponseCacheConfig{ + MaxSizeItems: 1000, + Validity: time.Hour, + }, + } + queryFrontendConfig := queryfrontend.Config{} + queryFrontend := e2ethanos.NewQueryFrontend( + e, + "1", + fmt.Sprintf("http://%s:%s", e.HostAddr(), tsPort), + queryFrontendConfig, + inMemoryCacheConfig, + ) + testutil.Ok(t, e2e.StartAndWaitReady(queryFrontend)) + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + t.Cleanup(cancel) + + promClient, err := api.NewClient(api.Config{ + Address: "http://" + queryFrontend.Endpoint("http"), + RoundTripper: tenantRoundTripper{ + tenant: testTenant, + rt: http.DefaultTransport, + }, + }) + testutil.Ok(t, err) + v1api := v1.NewAPI(promClient) + + r := v1.Range{ + Start: time.Now().Add(-time.Hour), + End: time.Now(), + Step: time.Minute, + } + + _, _, _ = v1api.QueryRange(ctx, "rate(prometheus_tsdb_head_samples_appended_total[5m])", r) +} + +type tenantRoundTripper struct { + tenant string + tenantHeader string + rt http.RoundTripper +} + +var _ http.RoundTripper = tenantRoundTripper{} + +// RoundTrip implements the http.RoundTripper interface. +func (u tenantRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { + if u.tenantHeader == "" { + u.tenantHeader = tenancy.DefaultTenantHeader + } + r.Header.Set(u.tenantHeader, u.tenant) + return u.rt.RoundTrip(r) +} From 6e8ed124a6a9bbc4929c8dae3eb212b4bbd50520 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Mon, 7 Aug 2023 18:48:47 +0200 Subject: [PATCH 04/23] Test more scenarios Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- test/e2e/e2ethanos/services.go | 7 ++ test/e2e/query_frontend_test.go | 132 +++++++++++++++++++++----------- 2 files changed, 93 insertions(+), 46 deletions(-) diff --git a/test/e2e/e2ethanos/services.go b/test/e2e/e2ethanos/services.go index 65d20a53ab..763552dad6 100644 --- a/test/e2e/e2ethanos/services.go +++ b/test/e2e/e2ethanos/services.go @@ -978,6 +978,13 @@ func NewQueryFrontend(e e2e.Environment, name, downstreamURL string, config quer flags["--query-range.split-interval"] = "0" } + if config.TenantHeader != "" { + flags["--query-frontend.tenant-header"] = config.TenantHeader + } + if config.DefaultTenant != "" { + flags["--query-frontend.default-tenant"] = config.DefaultTenant + } + return e2eobs.AsObservable(e.Runnable(fmt.Sprintf("query-frontend-%s", name)). WithPorts(map[string]int{"http": 8080}). Init(e2e.StartOptions{ diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index eff75dd3a4..9a075b7553 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -5,6 +5,7 @@ package e2e_test import ( "context" + "crypto/sha256" "fmt" "github.com/prometheus/prometheus/model/labels" "github.com/thanos-io/thanos/pkg/tenancy" @@ -930,57 +931,96 @@ func TestInstantQueryShardingWithRandomData(t *testing.T) { func TestQueryFrontendTenantForward(t *testing.T) { t.Parallel() - e, err := e2e.New(e2e.WithName("query-frontend")) - testutil.Ok(t, err) - t.Cleanup(e2ethanos.CleanScenario(t, e)) - - testTenant := "test-tenant" - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusNoContent) - testutil.Equals(t, testTenant, r.Header.Get("THANOS-TENANT")) - testutil.Equals(t, testTenant, r.Header.Get("X-Scope-OrgID")) - })) - t.Cleanup(ts.Close) - tsPort := urlParse(t, ts.URL).Port() - - inMemoryCacheConfig := queryfrontend.CacheProviderConfig{ - Type: queryfrontend.INMEMORY, - Config: queryfrontend.InMemoryResponseCacheConfig{ - MaxSizeItems: 1000, - Validity: time.Hour, + tests := []struct { + name string + customTenantHeaderName string + tenantName string + }{ + { + name: "default tenant header name with a tenant name", + customTenantHeaderName: tenancy.DefaultTenantHeader, + tenantName: "test-tenant", + }, + { + name: "default tenant header name without a tenant name", + customTenantHeaderName: tenancy.DefaultTenantHeader, + }, + { + name: "custom tenant header name with a tenant name", + customTenantHeaderName: "X-Foobar-Tenant", + tenantName: "test-tenant", + }, + { + name: "custom tenant header name without a tenant name", + customTenantHeaderName: "X-Foobar-Tenant", }, } - queryFrontendConfig := queryfrontend.Config{} - queryFrontend := e2ethanos.NewQueryFrontend( - e, - "1", - fmt.Sprintf("http://%s:%s", e.HostAddr(), tsPort), - queryFrontendConfig, - inMemoryCacheConfig, - ) - testutil.Ok(t, e2e.StartAndWaitReady(queryFrontend)) - - ctx, cancel := context.WithTimeout(context.Background(), time.Minute) - t.Cleanup(cancel) + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if tc.tenantName == "" { + tc.tenantName = tenancy.DefaultTenant + } + // Use a shorthash of tc.name as e2e env name because the random name generator is having a collision for + // some reason. + e2ename := fmt.Sprintf("%x", sha256.Sum256([]byte(tc.name)))[:8] + e, err := e2e.New(e2e.WithName(e2ename)) + testutil.Ok(t, err) + t.Cleanup(e2ethanos.CleanScenario(t, e)) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + testutil.Equals(t, tc.tenantName, r.Header.Get(tenancy.DefaultTenantHeader)) + if tc.customTenantHeaderName != tenancy.DefaultTenantHeader { + testutil.Equals(t, "", r.Header.Get(tc.customTenantHeaderName)) + } + testutil.Equals(t, tc.tenantName, r.Header.Get("X-Scope-OrgID")) + })) + t.Cleanup(ts.Close) + tsPort := urlParse(t, ts.URL).Port() + + inMemoryCacheConfig := queryfrontend.CacheProviderConfig{ + Type: queryfrontend.INMEMORY, + Config: queryfrontend.InMemoryResponseCacheConfig{ + MaxSizeItems: 1000, + Validity: time.Hour, + }, + } + queryFrontendConfig := queryfrontend.Config{ + TenantHeader: tc.customTenantHeaderName, + } + queryFrontend := e2ethanos.NewQueryFrontend( + e, + "qfe", + fmt.Sprintf("http://%s:%s", e.HostAddr(), tsPort), + queryFrontendConfig, + inMemoryCacheConfig, + ) + testutil.Ok(t, e2e.StartAndWaitReady(queryFrontend)) + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + t.Cleanup(cancel) + + promClient, err := api.NewClient(api.Config{ + Address: "http://" + queryFrontend.Endpoint("http"), + RoundTripper: tenantRoundTripper{ + tenant: tc.tenantName, + rt: http.DefaultTransport, + }, + }) + testutil.Ok(t, err) + v1api := v1.NewAPI(promClient) - promClient, err := api.NewClient(api.Config{ - Address: "http://" + queryFrontend.Endpoint("http"), - RoundTripper: tenantRoundTripper{ - tenant: testTenant, - rt: http.DefaultTransport, - }, - }) - testutil.Ok(t, err) - v1api := v1.NewAPI(promClient) + r := v1.Range{ + Start: time.Now().Add(-time.Hour), + End: time.Now(), + Step: time.Minute, + } - r := v1.Range{ - Start: time.Now().Add(-time.Hour), - End: time.Now(), - Step: time.Minute, + _, _, _ = v1api.QueryRange(ctx, "rate(prometheus_tsdb_head_samples_appended_total[5m])", r) + }) } - - _, _, _ = v1api.QueryRange(ctx, "rate(prometheus_tsdb_head_samples_appended_total[5m])", r) } type tenantRoundTripper struct { From ffcbed6670415f1cb58c04900195865c85e1c9b2 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Tue, 8 Aug 2023 13:12:41 +0200 Subject: [PATCH 05/23] Goimports files Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- pkg/queryfrontend/roundtrip.go | 3 ++- test/e2e/query_frontend_test.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/queryfrontend/roundtrip.go b/pkg/queryfrontend/roundtrip.go index 81f8351b54..1cdfb81671 100644 --- a/pkg/queryfrontend/roundtrip.go +++ b/pkg/queryfrontend/roundtrip.go @@ -4,12 +4,13 @@ package queryfrontend import ( - "github.com/thanos-io/thanos/pkg/tenancy" "net/http" "regexp" "strings" "time" + "github.com/thanos-io/thanos/pkg/tenancy" + "github.com/thanos-io/thanos/pkg/querysharding" "github.com/go-kit/log" diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index 9a075b7553..92002cc5ba 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -7,8 +7,6 @@ import ( "context" "crypto/sha256" "fmt" - "github.com/prometheus/prometheus/model/labels" - "github.com/thanos-io/thanos/pkg/tenancy" "net/http" "net/http/httptest" "os" @@ -17,6 +15,9 @@ import ( "testing" "time" + "github.com/prometheus/prometheus/model/labels" + "github.com/thanos-io/thanos/pkg/tenancy" + "github.com/efficientgo/e2e" e2emon "github.com/efficientgo/e2e/monitoring" "github.com/efficientgo/e2e/monitoring/matchers" From e4ea65726f57c959b6ed71cbb0fb4030569b14a0 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Tue, 8 Aug 2023 18:19:20 +0200 Subject: [PATCH 06/23] Rerun CI Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> From 85506abcb013e5a336bb0f94a75babd7fa63d634 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 9 Aug 2023 12:26:39 +0200 Subject: [PATCH 07/23] Rerun goimports without any line breaks Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 8 +++----- test/e2e/query_frontend_test.go | 10 ++++------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index c6b2cf3663..e80b2a9286 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -8,8 +8,6 @@ import ( "net/http" "time" - "github.com/thanos-io/thanos/pkg/tenancy" - extflag "github.com/efficientgo/tools/extkingpin" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -18,9 +16,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - "github.com/weaveworks/common/user" - "gopkg.in/yaml.v2" - cortexfrontend "github.com/thanos-io/thanos/internal/cortex/frontend" "github.com/thanos-io/thanos/internal/cortex/frontend/transport" "github.com/thanos-io/thanos/internal/cortex/querier/queryrange" @@ -35,7 +30,10 @@ import ( "github.com/thanos-io/thanos/pkg/queryfrontend" httpserver "github.com/thanos-io/thanos/pkg/server/http" "github.com/thanos-io/thanos/pkg/server/http/middleware" + "github.com/thanos-io/thanos/pkg/tenancy" "github.com/thanos-io/thanos/pkg/tracing" + "github.com/weaveworks/common/user" + "gopkg.in/yaml.v2" ) type queryFrontendConfig struct { diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index 92002cc5ba..1590ba3e98 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -15,24 +15,22 @@ import ( "testing" "time" - "github.com/prometheus/prometheus/model/labels" - "github.com/thanos-io/thanos/pkg/tenancy" - + "github.com/efficientgo/core/testutil" "github.com/efficientgo/e2e" e2emon "github.com/efficientgo/e2e/monitoring" "github.com/efficientgo/e2e/monitoring/matchers" "github.com/pkg/errors" "github.com/prometheus/client_golang/api" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/timestamp" - - "github.com/efficientgo/core/testutil" - v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/thanos-io/thanos/pkg/block/metadata" "github.com/thanos-io/thanos/pkg/cacheutil" "github.com/thanos-io/thanos/pkg/promclient" "github.com/thanos-io/thanos/pkg/queryfrontend" "github.com/thanos-io/thanos/pkg/runutil" + "github.com/thanos-io/thanos/pkg/tenancy" "github.com/thanos-io/thanos/pkg/testutil/e2eutil" "github.com/thanos-io/thanos/test/e2e/e2ethanos" ) From 399a3656000682a671b12b21e1271ca69a1168c8 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 9 Aug 2023 12:33:33 +0200 Subject: [PATCH 08/23] Resort imports in groups (std, 3rd-party, local) Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 5 +++-- test/e2e/query_frontend_test.go | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index e80b2a9286..81c84fb60d 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -16,6 +16,9 @@ import ( "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" + "github.com/weaveworks/common/user" + "gopkg.in/yaml.v2" + cortexfrontend "github.com/thanos-io/thanos/internal/cortex/frontend" "github.com/thanos-io/thanos/internal/cortex/frontend/transport" "github.com/thanos-io/thanos/internal/cortex/querier/queryrange" @@ -32,8 +35,6 @@ import ( "github.com/thanos-io/thanos/pkg/server/http/middleware" "github.com/thanos-io/thanos/pkg/tenancy" "github.com/thanos-io/thanos/pkg/tracing" - "github.com/weaveworks/common/user" - "gopkg.in/yaml.v2" ) type queryFrontendConfig struct { diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index 1590ba3e98..37017d1d3f 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -25,6 +25,7 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/timestamp" + "github.com/thanos-io/thanos/pkg/block/metadata" "github.com/thanos-io/thanos/pkg/cacheutil" "github.com/thanos-io/thanos/pkg/promclient" From 6e9ffeb3074b29057b72bc47929f1724f9981fb3 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 9 Aug 2023 12:35:59 +0200 Subject: [PATCH 09/23] No need to forward a custom tenant header downstream We should always and only forward the default (internal) one. Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index 81c84fb60d..787c72dd33 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -225,7 +225,6 @@ func runQueryFrontend( cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) cfg.orgIdHeaders = append(cfg.orgIdHeaders, tenancy.DefaultTenantHeader) if cfg.TenantHeader != "" && cfg.TenantHeader != tenancy.DefaultTenantHeader { - cfg.ForwardHeaders = append(cfg.ForwardHeaders, cfg.TenantHeader) cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader) } From aa9f3d307165ae8333660885368413ee0eb9d92a Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 12:45:59 +0200 Subject: [PATCH 10/23] Make query frontend tenant cert config hidden Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index 787c72dd33..c999e85455 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -149,7 +149,7 @@ func registerQueryFrontend(app *extkingpin.App) { cmd.Flag("query-frontend.tenant-header", "HTTP header to determine tenant").Default(tenancy.DefaultTenantHeader).Hidden().StringVar(&cfg.TenantHeader) cmd.Flag("query-frontend.default-tenant", "Default tenant to use if tenant header is not present").Default(tenancy.DefaultTenant).Hidden().StringVar(&cfg.DefaultTenant) - cmd.Flag("query-frontend.tenant-certificate-field", "Use TLS client's certificate field to determine tenant for requests. Must be one of "+tenancy.CertificateFieldOrganization+", "+tenancy.CertificateFieldOrganizationalUnit+" or "+tenancy.CertificateFieldCommonName+". This setting will cause the query-frontend.tenant-header flag value to be ignored.").Default("").EnumVar(&cfg.TenantCertField, "", tenancy.CertificateFieldOrganization, tenancy.CertificateFieldOrganizationalUnit, tenancy.CertificateFieldCommonName) + cmd.Flag("query-frontend.tenant-certificate-field", "Use TLS client's certificate field to determine tenant for requests. Must be one of "+tenancy.CertificateFieldOrganization+", "+tenancy.CertificateFieldOrganizationalUnit+" or "+tenancy.CertificateFieldCommonName+". This setting will cause the query-frontend.tenant-header flag value to be ignored.").Hidden().Default("").EnumVar(&cfg.TenantCertField, "", tenancy.CertificateFieldOrganization, tenancy.CertificateFieldOrganizationalUnit, tenancy.CertificateFieldCommonName) cmd.Flag("query-frontend.vertical-shards", "Number of shards to use when distributing shardable PromQL queries. For more details, you can refer to the Vertical query sharding proposal: https://thanos.io/tip/proposals-accepted/202205-vertical-query-sharding.md").IntVar(&cfg.NumShards) From f955f71583084eb4305801a651718bdd07eee50b Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 15:46:16 +0200 Subject: [PATCH 11/23] Error out when org ig and tenant header are provided at the same time Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index c999e85455..ada1aba716 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -140,7 +140,9 @@ func registerQueryFrontend(app *extkingpin.App) { cmd.Flag("query-frontend.log-queries-longer-than", "Log queries that are slower than the specified duration. "+ "Set to 0 to disable. Set to < 0 to enable on all queries.").Default("0").DurationVar(&cfg.CortexHandlerConfig.LogQueriesLongerThan) - cmd.Flag("query-frontend.org-id-header", "Request header names used to identify the source of slow queries (repeated flag). "+ + cmd.Flag("query-frontend.org-id-header", "Deprecation Warning - This flag will be soon deprecated in favour of query-frontend.tenant-header"+ + " and both flags cannot be used at the same time. "+ + "Request header names used to identify the source of slow queries (repeated flag). "+ "The values of the header will be added to the org id field in the slow query log. "+ "If multiple headers match the request, the first matching arg specified will take precedence. "+ "If no headers match 'anonymous' will be used.").PlaceHolder("").StringsVar(&cfg.orgIdHeaders) @@ -222,6 +224,13 @@ func runQueryFrontend( cfg *queryFrontendConfig, comp component.Component, ) error { + if len(cfg.orgIdHeaders) != 0 && cfg.TenantHeader != "" { + return errors.New("query-frontend.org-id-header and query-frontend.tenant-header cannot be used together") + } + + // Temporarily manually adding the default tenant header into the list of headers to forward and org id headers. + // This facilitates the transition from org id to tenant id with minimal amount of changes. This should be removed + // once the org id header is deprecated. cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) cfg.orgIdHeaders = append(cfg.orgIdHeaders, tenancy.DefaultTenantHeader) if cfg.TenantHeader != "" && cfg.TenantHeader != tenancy.DefaultTenantHeader { From 08dd0c403d5bbfe34d1b974865d737f9a1e69fe1 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 15:54:25 +0200 Subject: [PATCH 12/23] Fix typo Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index ada1aba716..17582039d6 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -140,7 +140,7 @@ func registerQueryFrontend(app *extkingpin.App) { cmd.Flag("query-frontend.log-queries-longer-than", "Log queries that are slower than the specified duration. "+ "Set to 0 to disable. Set to < 0 to enable on all queries.").Default("0").DurationVar(&cfg.CortexHandlerConfig.LogQueriesLongerThan) - cmd.Flag("query-frontend.org-id-header", "Deprecation Warning - This flag will be soon deprecated in favour of query-frontend.tenant-header"+ + cmd.Flag("query-frontend.org-id-header", "Deprecation Warning - This flag will be soon deprecated in favor of query-frontend.tenant-header"+ " and both flags cannot be used at the same time. "+ "Request header names used to identify the source of slow queries (repeated flag). "+ "The values of the header will be added to the org id field in the slow query log. "+ From dd619e35b3b55cd0c8a1707bf65cb9f015b6c04a Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 16:06:23 +0200 Subject: [PATCH 13/23] Update docs Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- docs/components/query-frontend.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/components/query-frontend.md b/docs/components/query-frontend.md index 20e197d7db..f58debe1a3 100644 --- a/docs/components/query-frontend.md +++ b/docs/components/query-frontend.md @@ -269,13 +269,17 @@ Flags: duration. Set to 0 to disable. Set to < 0 to enable on all queries. --query-frontend.org-id-header= ... - Request header names used to identify the - source of slow queries (repeated flag). - The values of the header will be added to - the org id field in the slow query log. If - multiple headers match the request, the first - matching arg specified will take precedence. - If no headers match 'anonymous' will be used. + Deprecation Warning - This flag + will be soon deprecated in favor of + query-frontend.tenant-header and both flags + cannot be used at the same time. Request header + names used to identify the source of slow + queries (repeated flag). The values of the + header will be added to the org id field in + the slow query log. If multiple headers match + the request, the first matching arg specified + will take precedence. If no headers match + 'anonymous' will be used. --query-frontend.vertical-shards=QUERY-FRONTEND.VERTICAL-SHARDS Number of shards to use when distributing shardable PromQL queries. From 531ded79b35427a71184d68d5e4c221878e268bd Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 18:07:59 +0200 Subject: [PATCH 14/23] Improve comments around forward and org id headers for tenancy Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index 17582039d6..98c789fc2f 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -229,10 +229,14 @@ func runQueryFrontend( } // Temporarily manually adding the default tenant header into the list of headers to forward and org id headers. - // This facilitates the transition from org id to tenant id with minimal amount of changes. This should be removed - // once the org id header is deprecated. + // This facilitates the transition from org id to tenant id with minimal amount of changes. + // TODO: This should be removed once the org id header is deprecated in Thanos. cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) cfg.orgIdHeaders = append(cfg.orgIdHeaders, tenancy.DefaultTenantHeader) + + // If tenant header is set and different from the default tenant header, add it to the list of org id headers. + // In this case we don't need to add the tenant header to `cfg.ForwardHeaders` because tripperware will modify + // the request, renaming the tenant header to the default tenant header, before the header propagation logic runs. if cfg.TenantHeader != "" && cfg.TenantHeader != tenancy.DefaultTenantHeader { cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader) } From 9d62032ccb92bf97ef370c199cf92c26fc370827 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 18:12:40 +0200 Subject: [PATCH 15/23] Add some test comments Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- test/e2e/query_frontend_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index 37017d1d3f..907f2fd3d9 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -971,10 +971,16 @@ func TestQueryFrontendTenantForward(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) + // The tenant header present in the outgoing request should be the default tenant header. testutil.Equals(t, tc.tenantName, r.Header.Get(tenancy.DefaultTenantHeader)) + + // In case the query frontend is configured with a custom tenant header name, verify such header + // is not present in the outgoing request. if tc.customTenantHeaderName != tenancy.DefaultTenantHeader { testutil.Equals(t, "", r.Header.Get(tc.customTenantHeaderName)) } + + // Verify the outgoing request will keep the X-Scope-OrgID header for compatibility with Cortex. testutil.Equals(t, tc.tenantName, r.Header.Get("X-Scope-OrgID")) })) t.Cleanup(ts.Close) From 01f80714b359a26ff5945c2b98f95b0cee6090b1 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 18:37:36 +0200 Subject: [PATCH 16/23] Improve logic for detecting tenant and org id headers were provided Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index 98c789fc2f..6154527e65 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -224,8 +224,17 @@ func runQueryFrontend( cfg *queryFrontendConfig, comp component.Component, ) error { - if len(cfg.orgIdHeaders) != 0 && cfg.TenantHeader != "" { - return errors.New("query-frontend.org-id-header and query-frontend.tenant-header cannot be used together") + tenantHeaderProvided := cfg.TenantHeader != "" && cfg.TenantHeader != tenancy.DefaultTenantHeader + // If tenant header is set and different from the default tenant header, add it to the list of org id headers. + // In this case we don't need to add the tenant header to `cfg.ForwardHeaders` because tripperware will modify + // the request, renaming the tenant header to the default tenant header, before the header propagation logic runs. + if tenantHeaderProvided { + cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader) + + // If tenant header is provided together with the org id header, error out. + if len(cfg.orgIdHeaders) != 0 { + return errors.New("query-frontend.org-id-header and query-frontend.tenant-header cannot be used together") + } } // Temporarily manually adding the default tenant header into the list of headers to forward and org id headers. @@ -234,13 +243,6 @@ func runQueryFrontend( cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) cfg.orgIdHeaders = append(cfg.orgIdHeaders, tenancy.DefaultTenantHeader) - // If tenant header is set and different from the default tenant header, add it to the list of org id headers. - // In this case we don't need to add the tenant header to `cfg.ForwardHeaders` because tripperware will modify - // the request, renaming the tenant header to the default tenant header, before the header propagation logic runs. - if cfg.TenantHeader != "" && cfg.TenantHeader != tenancy.DefaultTenantHeader { - cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader) - } - queryRangeCacheConfContentYaml, err := cfg.QueryRangeConfig.CachePathOrContent.Content() if err != nil { return err From 60e093ec7327e0f66efa8fb4898a0cf2b30fd83c Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 21:06:57 +0200 Subject: [PATCH 17/23] Fix order of check for org id headers Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index 6154527e65..d289a5e82f 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -229,12 +229,12 @@ func runQueryFrontend( // In this case we don't need to add the tenant header to `cfg.ForwardHeaders` because tripperware will modify // the request, renaming the tenant header to the default tenant header, before the header propagation logic runs. if tenantHeaderProvided { - cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader) - // If tenant header is provided together with the org id header, error out. if len(cfg.orgIdHeaders) != 0 { return errors.New("query-frontend.org-id-header and query-frontend.tenant-header cannot be used together") } + + cfg.orgIdHeaders = append(cfg.orgIdHeaders, cfg.TenantHeader) } // Temporarily manually adding the default tenant header into the list of headers to forward and org id headers. From 4e8ef609284a916734d9e8355a6397f050e36b64 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 16 Aug 2023 21:51:56 +0200 Subject: [PATCH 18/23] Add some TODO comments for the future Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index d289a5e82f..e2670c1416 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -228,6 +228,7 @@ func runQueryFrontend( // If tenant header is set and different from the default tenant header, add it to the list of org id headers. // In this case we don't need to add the tenant header to `cfg.ForwardHeaders` because tripperware will modify // the request, renaming the tenant header to the default tenant header, before the header propagation logic runs. + // TODO: This should be removed once the org id header is fully removed in Thanos. if tenantHeaderProvided { // If tenant header is provided together with the org id header, error out. if len(cfg.orgIdHeaders) != 0 { @@ -239,8 +240,8 @@ func runQueryFrontend( // Temporarily manually adding the default tenant header into the list of headers to forward and org id headers. // This facilitates the transition from org id to tenant id with minimal amount of changes. - // TODO: This should be removed once the org id header is deprecated in Thanos. cfg.ForwardHeaders = append(cfg.ForwardHeaders, tenancy.DefaultTenantHeader) + // TODO: This should be removed once the org id header is fully removed in Thanos. cfg.orgIdHeaders = append(cfg.orgIdHeaders, tenancy.DefaultTenantHeader) queryRangeCacheConfContentYaml, err := cfg.QueryRangeConfig.CachePathOrContent.Content() From 8cf2ba0b212d3814024af228fb251645571ef074 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Thu, 17 Aug 2023 10:29:52 +0200 Subject: [PATCH 19/23] Rerun CI Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> From f5c5cba021ea3eed860f6b4224eebe1dbf0f26ca Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 13 Sep 2023 16:15:00 +0200 Subject: [PATCH 20/23] Make default tenant flag in qfe match receive's Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- cmd/thanos/query_frontend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index e2670c1416..094637cb88 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -150,7 +150,7 @@ func registerQueryFrontend(app *extkingpin.App) { cmd.Flag("query-frontend.forward-header", "List of headers forwarded by the query-frontend to downstream queriers, default is empty").PlaceHolder("").StringsVar(&cfg.ForwardHeaders) cmd.Flag("query-frontend.tenant-header", "HTTP header to determine tenant").Default(tenancy.DefaultTenantHeader).Hidden().StringVar(&cfg.TenantHeader) - cmd.Flag("query-frontend.default-tenant", "Default tenant to use if tenant header is not present").Default(tenancy.DefaultTenant).Hidden().StringVar(&cfg.DefaultTenant) + cmd.Flag("query-frontend.default-tenant-id", "Default tenant ID to use if tenant header is not present").Default(tenancy.DefaultTenant).Hidden().StringVar(&cfg.DefaultTenant) cmd.Flag("query-frontend.tenant-certificate-field", "Use TLS client's certificate field to determine tenant for requests. Must be one of "+tenancy.CertificateFieldOrganization+", "+tenancy.CertificateFieldOrganizationalUnit+" or "+tenancy.CertificateFieldCommonName+". This setting will cause the query-frontend.tenant-header flag value to be ignored.").Hidden().Default("").EnumVar(&cfg.TenantCertField, "", tenancy.CertificateFieldOrganization, tenancy.CertificateFieldOrganizationalUnit, tenancy.CertificateFieldCommonName) cmd.Flag("query-frontend.vertical-shards", "Number of shards to use when distributing shardable PromQL queries. For more details, you can refer to the Vertical query sharding proposal: https://thanos.io/tip/proposals-accepted/202205-vertical-query-sharding.md").IntVar(&cfg.NumShards) From 1033baf1d78c12ae9ac2353c55daeb7cfdb3eda8 Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 27 Sep 2023 19:01:24 +0200 Subject: [PATCH 21/23] Remove unnecessary struct fields Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- pkg/queryfrontend/roundtrip.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/queryfrontend/roundtrip.go b/pkg/queryfrontend/roundtrip.go index 1cdfb81671..b901950ba8 100644 --- a/pkg/queryfrontend/roundtrip.go +++ b/pkg/queryfrontend/roundtrip.go @@ -85,8 +85,6 @@ func NewTripperware(config Config, reg prometheus.Registerer, logger log.Logger) queryRangeTripperware(next), labelsTripperware(next), queryInstantTripperware(next), - config.TenantHeader, - config.DefaultTenant, reg, ) return tenancy.InternalTenancyConversionTripper(config.TenantHeader, config.TenantCertField, tripper) @@ -96,19 +94,15 @@ func NewTripperware(config Config, reg prometheus.Registerer, logger log.Logger) type roundTripper struct { next, queryInstant, queryRange, labels http.RoundTripper - queriesCount *prometheus.CounterVec - tenantHeader string - defaultTenant string + queriesCount *prometheus.CounterVec } -func newRoundTripper(next, queryRange, metadata, queryInstant http.RoundTripper, tenantHeader, defaultTenant string, reg prometheus.Registerer) roundTripper { +func newRoundTripper(next, queryRange, metadata, queryInstant http.RoundTripper, reg prometheus.Registerer) roundTripper { r := roundTripper{ - next: next, - queryInstant: queryInstant, - queryRange: queryRange, - labels: metadata, - tenantHeader: tenantHeader, - defaultTenant: defaultTenant, + next: next, + queryInstant: queryInstant, + queryRange: queryRange, + labels: metadata, queriesCount: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ Name: "thanos_query_frontend_queries_total", Help: "Total queries passing through query frontend", From 41c3f6edf9e6072fcc1b2ae3c57819f922f73eee Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Wed, 27 Sep 2023 19:01:42 +0200 Subject: [PATCH 22/23] Add instant query to tenant forward tests Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --- test/e2e/query_frontend_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go index 6dbcafd3bb..d3bbdc7971 100644 --- a/test/e2e/query_frontend_test.go +++ b/test/e2e/query_frontend_test.go @@ -1026,6 +1026,7 @@ func TestQueryFrontendTenantForward(t *testing.T) { } _, _, _ = v1api.QueryRange(ctx, "rate(prometheus_tsdb_head_samples_appended_total[5m])", r) + _, _, _ = v1api.Query(ctx, "rate(prometheus_tsdb_head_samples_appended_total[5m])", time.Now()) }) } } From 6daddf735e258b9dbefae11b71eef32954be935a Mon Sep 17 00:00:00 2001 From: Douglas Camata <159076+douglascamata@users.noreply.github.com> Date: Tue, 24 Oct 2023 11:45:29 +0200 Subject: [PATCH 23/23] Rerun build Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>