Skip to content

fix: add a guardrail for TSDB requirement #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tremes wants to merge 1 commit into from
Closed

fix: add a guardrail for TSDB requirement #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions pkg/prometheus/guardrails.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const (
GuardrailDisallowBlanketRegex = "disallow-blanket-regex"
GuardrailMaxMetricCardinality = "max-metric-cardinality"
GuardrailMaxLabelCardinality = "max-label-cardinality"
GuardrailRequireTSDBEndpoint = "require-tsdb-endpoint"
)

// GuardrailViolation is returned when a query violates a specific guardrail rule.
Expand All @@ -43,6 +44,8 @@ type Guardrails struct {
// MaxLabelCardinality sets the maximum allowed label value count for blanket regex
// (0 = always disallow regex matcher provided DisallowBlanketRegex is true)
MaxLabelCardinality uint64
// Require TSBD endpoint
RequireTSDBEndpoint bool
}

// DefaultGuardrails returns a Guardrails instance with all safety checks enabled.
Expand All @@ -51,6 +54,7 @@ func DefaultGuardrails() *Guardrails {
DisallowExplicitNameLabel: true,
RequireLabelMatcher: true,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: false,
MaxMetricCardinality: 20000,
MaxLabelCardinality: 500,
}
Expand Down Expand Up @@ -81,10 +85,12 @@ func ParseGuardrails(value string) (*Guardrails, error) {
g.RequireLabelMatcher = true
case GuardrailDisallowBlanketRegex:
g.DisallowBlanketRegex = true
case GuardrailRequireTSDBEndpoint:
g.RequireTSDBEndpoint = true
default:
return nil, fmt.Errorf("unknown guardrail: %q (valid options: %s, %s, %s)",
return nil, fmt.Errorf("unknown guardrail: %q (valid options: %s, %s, %s, %s)",
name, GuardrailDisallowExplicitNameLabel, GuardrailRequireLabelMatcher,
GuardrailDisallowBlanketRegex)
GuardrailDisallowBlanketRegex, GuardrailRequireTSDBEndpoint)
}
}

Expand All @@ -102,7 +108,8 @@ func ParseGuardrails(value string) (*Guardrails, error) {
//
//nolint:gocyclo // complex validation logic, refactoring would reduce readability
func (g *Guardrails) IsSafeQuery(ctx context.Context, query string, client v1.API) (bool, error) {
if ((g.DisallowBlanketRegex && g.MaxLabelCardinality > 0) || (g.MaxMetricCardinality > 0)) && (client == nil || ctx == nil) {
if g.RequireTSDBEndpoint && ((g.DisallowBlanketRegex && g.MaxLabelCardinality > 0) ||
(g.MaxMetricCardinality > 0)) && (client == nil || ctx == nil) {
return false, fmt.Errorf("cannot verify cardinality without TSDB client")
}

Expand Down Expand Up @@ -158,7 +165,7 @@ func (g *Guardrails) IsSafeQuery(ctx context.Context, query string, client v1.AP
}

// Check metric cardinality
if g.MaxMetricCardinality > 0 {
if g.MaxMetricCardinality > 0 && g.RequireTSDBEndpoint {
metricNames, err := ExtractMetricNames(query)
if err != nil {
return false, fmt.Errorf("failed to extract metric names: %w", err)
Expand Down Expand Up @@ -206,27 +213,28 @@ func (g *Guardrails) IsSafeQuery(ctx context.Context, query string, client v1.AP
Message: fmt.Sprintf("query uses blanket regex on label %q, which is disallowed", blanketRegexLabels[0]),
}
}
if g.RequireTSDBEndpoint {
// Check TSDB label cardinality for blanket regex
tsdbResult, err := client.TSDB(ctx)
if err != nil {
return false, fmt.Errorf(
"cannot enforce max-label-cardinality guardrail: TSDB stats endpoint is unavailable on this backend "+
"(Thanos Querier < v0.40.0 does not implement /api/v1/status/tsdb); "+
"disable this guardrail with --guardrails require-label-matcher,disallow-blanket-regex: %w", err)
}

// Check TSDB label cardinality for blanket regex
tsdbResult, err := client.TSDB(ctx)
if err != nil {
return false, fmt.Errorf(
"cannot enforce max-label-cardinality guardrail: TSDB stats endpoint is unavailable on this backend "+
"(Thanos Querier < v0.40.0 does not implement /api/v1/status/tsdb); "+
"disable this guardrail with --guardrails require-label-matcher,disallow-blanket-regex: %w", err)
}

labelValueCountByLabel := make(map[string]uint64)
for _, stat := range tsdbResult.LabelValueCountByLabelName {
labelValueCountByLabel[stat.Name] = stat.Value
}
labelValueCountByLabel := make(map[string]uint64)
for _, stat := range tsdbResult.LabelValueCountByLabelName {
labelValueCountByLabel[stat.Name] = stat.Value
}

for _, labelName := range blanketRegexLabels {
if count, exists := labelValueCountByLabel[labelName]; exists {
if count > g.MaxLabelCardinality {
return false, &GuardrailViolation{
Guardrail: GuardrailMaxLabelCardinality,
Message: fmt.Sprintf("label %q has cardinality %d, which exceeds maximum allowed %d for blanket regex", labelName, count, g.MaxLabelCardinality),
for _, labelName := range blanketRegexLabels {
if count, exists := labelValueCountByLabel[labelName]; exists {
if count > g.MaxLabelCardinality {
return false, &GuardrailViolation{
Guardrail: GuardrailMaxLabelCardinality,
Message: fmt.Sprintf("label %q has cardinality %d, which exceeds maximum allowed %d for blanket regex", labelName, count, g.MaxLabelCardinality),
}
}
}
}
Expand Down
115 changes: 115 additions & 0 deletions pkg/prometheus/guardrails_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ func TestGuardrails_IsSafeQuery(t *testing.T) {
DisallowExplicitNameLabel: true,
RequireLabelMatcher: true,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: true,
MaxMetricCardinality: 0, // Disabled - no TSDB needed
MaxLabelCardinality: 0, // Disabled - blanket regex always rejected
}
Expand Down Expand Up @@ -260,6 +261,7 @@ func TestGuardrails_MaxLabelCardinality(t *testing.T) {
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
MaxLabelCardinality: 100, // Set threshold
RequireTSDBEndpoint: true,
}
// With MaxLabelCardinality set but no client provided, blanket regex should be rejected
// because we can't verify the cardinality
Expand All @@ -277,6 +279,7 @@ func TestGuardrails_MaxLabelCardinality(t *testing.T) {
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: true,
MaxLabelCardinality: 0, // 0 means always disallow
}
safe, err := g.IsSafeQuery(context.TODO(), `http_requests_total{pod=~".*"}`, nil)
Expand Down Expand Up @@ -432,6 +435,7 @@ func TestGuardrails_MaxLabelCardinalityWithMockedTSDB(t *testing.T) {
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: true,
MaxLabelCardinality: 100, // Threshold
}

Expand Down Expand Up @@ -483,6 +487,7 @@ func TestGuardrails_MaxLabelCardinalityWithMockedTSDB(t *testing.T) {
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: true,
MaxLabelCardinality: 100,
}

Expand Down Expand Up @@ -532,6 +537,7 @@ func TestGuardrails_MaxLabelCardinalityWithMockedTSDB(t *testing.T) {
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: true,
MaxLabelCardinality: 100,
}

Expand All @@ -558,6 +564,7 @@ func TestGuardrails_MaxLabelCardinalityWithMockedTSDB(t *testing.T) {
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: true,
MaxLabelCardinality: 100,
}

Expand Down Expand Up @@ -613,6 +620,7 @@ func TestGuardrails_MaxLabelCardinalityWithMockedTSDB(t *testing.T) {
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
RequireTSDBEndpoint: true,
MaxMetricCardinality: 10000,
MaxLabelCardinality: 100,
}
Expand All @@ -626,3 +634,110 @@ func TestGuardrails_MaxLabelCardinalityWithMockedTSDB(t *testing.T) {
}
})
}

func TestGuardrails_RequireTSDBEndpoint(t *testing.T) {
t.Run("ParseGuardrails recognizes require-tsdb-endpoint", func(t *testing.T) {
g, err := ParseGuardrails("require-tsdb-endpoint")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !g.RequireTSDBEndpoint {
t.Error("expected RequireTSDBEndpoint to be true")
}
})

t.Run("DefaultGuardrails does not enable RequireTSDBEndpoint", func(t *testing.T) {
g := DefaultGuardrails()
if g.RequireTSDBEndpoint {
t.Error("expected RequireTSDBEndpoint to be false in default guardrails")
}
})

t.Run("RequireTSDBEndpoint false skips TSDB calls for metric cardinality", func(t *testing.T) {
g := &Guardrails{
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: false,
MaxMetricCardinality: 1000,
MaxLabelCardinality: 0,
RequireTSDBEndpoint: false,
}

safe, err := g.IsSafeQuery(t.Context(), `http_requests_total{job="api"}`, nil)
if !safe {
t.Errorf("expected query to be safe when RequireTSDBEndpoint is false, got error: %v", err)
}
})

t.Run("RequireTSDBEndpoint false skips TSDB calls for label cardinality", func(t *testing.T) {
g := &Guardrails{
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
MaxMetricCardinality: 0,
MaxLabelCardinality: 100,
RequireTSDBEndpoint: false,
}

safe, err := g.IsSafeQuery(t.Context(), `http_requests_total{pod=~".*"}`, nil)
if !safe {
t.Errorf("expected blanket regex query to be safe when RequireTSDBEndpoint is false, got error: %v", err)
}
})

t.Run("RequireTSDBEndpoint true requires TSDB client for metric cardinality", func(t *testing.T) {
g := &Guardrails{
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: false,
MaxMetricCardinality: 1000,
MaxLabelCardinality: 0,
RequireTSDBEndpoint: true,
}
safe, err := g.IsSafeQuery(t.Context(), `http_requests_total{job="api"}`, nil)
if safe {
t.Error("expected query to be unsafe when RequireTSDBEndpoint is true but no client provided")
}
if err == nil {
t.Error("expected error explaining why query is unsafe")
}
})

t.Run("RequireTSDBEndpoint true requires TSDB client for label cardinality", func(t *testing.T) {
g := &Guardrails{
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
MaxMetricCardinality: 0,
MaxLabelCardinality: 100,
RequireTSDBEndpoint: true,
}

safe, err := g.IsSafeQuery(t.Context(), `http_requests_total{pod=~".*"}`, nil)
if safe {
t.Error("expected query to be unsafe when RequireTSDBEndpoint is true but no client provided")
}
if err == nil {
t.Error("expected error explaining why query is unsafe")
}
})

t.Run("RequireTSDBEndpoint false with MaxLabelCardinality 0 still disallows blanket regex", func(t *testing.T) {
g := &Guardrails{
DisallowExplicitNameLabel: false,
RequireLabelMatcher: false,
DisallowBlanketRegex: true,
MaxMetricCardinality: 0,
MaxLabelCardinality: 0,
RequireTSDBEndpoint: false,
}

safe, err := g.IsSafeQuery(t.Context(), `http_requests_total{pod=~".*"}`, nil)
if safe {
t.Error("expected blanket regex query to be unsafe when MaxLabelCardinality is 0")
}
if err == nil {
t.Error("expected error explaining why query is unsafe")
}
})
}