From e10aa4c164bdb4b926c3befd61f50ac97a67cf5d Mon Sep 17 00:00:00 2001 From: Jacob Baungard Hansen Date: Mon, 8 Jan 2024 11:05:21 +0100 Subject: [PATCH] Address review comments Minor changes to CLI docs, code-comments and changelog. Signed-off-by: Jacob Baungard Hansen --- CHANGELOG.md | 3 +-- cmd/thanos/query.go | 4 ++-- docs/components/query.md | 12 ++++++------ pkg/tenancy/tenancy.go | 2 +- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63ec8c6e1b..a0bccbbaf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re ### Added -- [#6756](https://github.com/thanos-io/thanos/pull/6756) Query: Add the following options to allow enforcement of tenancy on the query path: `query.enable-tenancy`, `query.tenant-label-name`. +- [#6756](https://github.com/thanos-io/thanos/pull/6756) Query: Add `query.enable-tenancy` & `query.tenant-label-name` options to allow enforcement of tenancy on the query path, by injecting labels into queries (uses prom-label-proxy internally). - [#6944](https://github.com/thanos-io/thanos/pull/6944) Receive: Added a new flag for maximum retention bytes. - [#6891](https://github.com/thanos-io/thanos/pull/6891) Objstore: Bump `objstore` which adds support for Azure Workload Identity. - [#6453](https://github.com/thanos-io/thanos/pull/6453) Sidecar: Added `--reloader.method` to support configuration reloads via SIHUP signal. @@ -27,7 +27,6 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re - [#6943](https://github.com/thanos-io/thanos/pull/6943) Ruler: Added `keep_firing_for` field in alerting rule. - [#6972](https://github.com/thanos-io/thanos/pull/6972) Store Gateway: Apply series limit when streaming series for series actually matched if lazy postings is enabled. - [#6984](https://github.com/thanos-io/thanos/pull/6984) Store Gateway: Added `--store.index-header-lazy-download-strategy` to specify how to lazily download index headers when lazy mmap is enabled. - - [#6887](https://github.com/thanos-io/thanos/pull/6887) Query Frontend: *breaking :warning:* Add tenant label to relevant exported metrics. Note that this change may cause some pre-existing custom dashboard queries to be incorrect due to the added label. - [#7028](https://github.com/thanos-io/thanos/pull/7028) Query|Query Frontend: Add new `--query-frontend.enable-x-functions` flag to enable experimental extended functions. diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index 5d9bd1bf5e..4d831ab6d1 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -220,8 +220,8 @@ func registerQuery(app *extkingpin.App) { tenantHeader := cmd.Flag("query.tenant-header", "HTTP header to determine tenant.").Default(tenancy.DefaultTenantHeader).String() defaultTenant := cmd.Flag("query.default-tenant-id", "Default tenant ID to use if tenant header is not present").Default(tenancy.DefaultTenant).String() tenantCertField := cmd.Flag("query.tenant-certificate-field", "Use TLS client's certificate field to determine tenant for write requests. Must be one of "+tenancy.CertificateFieldOrganization+", "+tenancy.CertificateFieldOrganizationalUnit+" or "+tenancy.CertificateFieldCommonName+". This setting will cause the query.tenant-header flag value to be ignored.").Default("").Enum("", tenancy.CertificateFieldOrganization, tenancy.CertificateFieldOrganizationalUnit, tenancy.CertificateFieldCommonName) - enforceTenancy := cmd.Flag("query.enforce-tenancy", "Enforce tenancy on Query APIs. Only responses where the value of the configured tenant-label-name and value of the tenant header matches are returned.").Default("false").Bool() - tenantLabel := cmd.Flag("query.tenant-label-name", "Label name to use when enforce tenancy when -querier.tenancy is enabled").Default(tenancy.DefaultTenantLabel).String() + enforceTenancy := cmd.Flag("query.enforce-tenancy", "Enforce tenancy on Query APIs. Responses are returned only if the label value of the configured tenant-label-name and the value of the tenant header matches.").Default("false").Bool() + tenantLabel := cmd.Flag("query.tenant-label-name", "Label name to use when enforcing tenancy (if --query.enforce-tenancy is enabled).").Default(tenancy.DefaultTenantLabel).String() var storeRateLimits store.SeriesSelectLimits storeRateLimits.RegisterFlags(cmd) diff --git a/docs/components/query.md b/docs/components/query.md index 2d08b7d39a..c70b749ef1 100644 --- a/docs/components/query.md +++ b/docs/components/query.md @@ -367,10 +367,10 @@ Flags: Whether to enable extended rate functions (xrate, xincrease and xdelta). Only has effect when used with Thanos engine. - --query.enforce-tenancy Enforce tenancy on Query APIs. Only - responses where the value of the configured - tenant-label-name and value of the tenant - header matches are returned. + --query.enforce-tenancy Enforce tenancy on Query APIs. Responses + are returned only if the label value of the + configured tenant-label-name and the value of + the tenant header matches. --query.lookback-delta=QUERY.LOOKBACK-DELTA The maximum lookback duration for retrieving metrics during expression evaluations. @@ -424,8 +424,8 @@ Flags: --query.tenant-header="THANOS-TENANT" HTTP header to determine tenant. --query.tenant-label-name="tenant_id" - Label name to use when enforce tenancy when - -querier.tenancy is enabled + Label name to use when enforcing tenancy (if + --query.enforce-tenancy is enabled). --query.timeout=2m Maximum time to process query by query node. --request.logging-config= Alternative to 'request.logging-config-file' diff --git a/pkg/tenancy/tenancy.go b/pkg/tenancy/tenancy.go index 13592833a8..aec0bad86a 100644 --- a/pkg/tenancy/tenancy.go +++ b/pkg/tenancy/tenancy.go @@ -199,7 +199,7 @@ func getLabelMatchers(formMatchers []string, tenant string, enforceTenancy bool, // This function will: // - Get tenant from HTTP header and add it to context. -// - if tenancy is enforce, add a tenant matcher. +// - if tenancy is enforced, add a tenant matcher to the promQL expression. func RewritePromQL(ctx context.Context, r *http.Request, tenantHeader string, defaultTenantID string, certTenantField string, enforceTenancy bool, tenantLabel string, queryStr string) (string, string, context.Context, error) { tenant, err := GetTenantFromHTTP(r, tenantHeader, defaultTenantID, certTenantField) if err != nil {