feat: filter tenant limits endpoint by allowlist - #18521
Conversation
|
💻 Deploy preview deleted. |
Add sensible default list of commonly needed, non-sensitive tenant limit fields that are returned by the tenant limits endpoint. The default includes fields like ingestion_rate_mb, max_query_series, retention_period, and other essential configuration values while excluding sensitive fields like S3 encryption keys. Updated validation tests to expect the new default allowlist values in all test cases to maintain test consistency. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
|
||
| writeYAMLResponse(w, limit) | ||
| // Apply allowlist filtering if configured | ||
| allowlist := t.Cfg.LimitsConfig.TenantLimitsAllowPublish |
There was a problem hiding this comment.
This is not using the tenant-scoped limit. So if we override the per-tenant TenantLimitsAllowPublish it will take no effect.
Should we use limit.TenantLimitsAllowPublish instead? limit comes fromt.TenantLimits.TenantLimits(user) or t.Overrides.DefaultLimits() if that's empty.
There was a problem hiding this comment.
While it's in the LimitsConfig, I actually wasn't thinking this should be a per-tenant limit. I thought this is something that would be configured once by the operator. What's the case an operator would want some tenants to see limits others couldn't. I think having it just be a regular config is simpler.
There was a problem hiding this comment.
Problem is, as it is currently implemented, that can be configured per tenant, or at least it looks like it can from the docs.
Even if we hide the docs, we can see it on the code and think aha that's a hidden per-tenant limit and be misused.
If we don't want it per tenant (fine with that), IMO this should live somewhere else.
2 participants
What this PR does / why we need it:
The limits endpoint was added for self-serve config, but this information is useful to Logs Drilldown. In an effort to expose this information publicly, both in cloud and OSS, this PR adds an allowlist, so only properties in the allow list are exposed. This list is configurable, giving operators control over which limits they expose.
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR