Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [ENHANCEMENT] Distributor: Add a label references validation for remote write v2 request. #7074
* [ENHANCEMENT] Distributor: Add count, spans, and buckets validations for native histogram. #7072
* [BUGFIX] Ring: Change DynamoDB KV to retry indefinitely for WatchKey. #7088
* [BUGFIX] Ruler: Add XFunctions validation support. #7111

## 1.20.0 2025-11-10

Expand Down
46 changes: 35 additions & 11 deletions pkg/ruler/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"net/http"
"os"
"strings"
"sync"
"time"

Expand All @@ -24,6 +25,7 @@ import (
"github.com/weaveworks/common/user"
"golang.org/x/net/context/ctxhttp"

"github.com/cortexproject/cortex/pkg/parser"
"github.com/cortexproject/cortex/pkg/ring/client"
"github.com/cortexproject/cortex/pkg/ruler/rulespb"
)
Expand Down Expand Up @@ -457,19 +459,41 @@ func (*DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []error
}

for i, r := range g.Rules {
// Use Cortex parser for expression validation (supports XFunctions)
if r.Expr != "" {
if _, err := parser.ParseExpr(r.Expr); err != nil {
var ruleName string
if r.Alert != "" {
ruleName = r.Alert
} else {
ruleName = r.Record
}
errs = append(errs, &rulefmt.Error{
Group: g.Name,
Rule: i,
RuleName: ruleName,
Err: rulefmt.WrappedError{},
})
}
}

// Validate other rule fields using Prometheus validation
for _, err := range r.Validate(rulefmt.RuleNode{}) {
var ruleName string
if r.Alert != "" {
ruleName = r.Alert
} else {
ruleName = r.Record
// Skip expression validation errors since we handle them above
if !strings.Contains(err.Error(), "could not parse expression") {
var ruleName string
if r.Alert != "" {
ruleName = r.Alert
} else {
ruleName = r.Record
}
errs = append(errs, &rulefmt.Error{
Group: g.Name,
Rule: i,
RuleName: ruleName,
Err: err,
})
}
errs = append(errs, &rulefmt.Error{
Group: g.Name,
Rule: i,
RuleName: ruleName,
Err: err,
})
}
}

Expand Down
91 changes: 91 additions & 0 deletions pkg/ruler/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/go-kit/log"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/model/rulefmt"
"github.com/prometheus/prometheus/notifier"
promRules "github.com/prometheus/prometheus/rules"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -361,3 +362,93 @@ func (m *mockRulesManager) Stop() {
m.running.Store(false)
close(m.done)
}

func TestValidateRuleGroup_AcceptsXFunctions(t *testing.T) {
manager := &DefaultMultiTenantManager{}

// Test rule with XFunction
ruleGroupWithXFunc := rulefmt.RuleGroup{
Name: "test_group",
Rules: []rulefmt.Rule{
{
Alert: "TestAlert",
Expr: "xrate(cpu_usage[5m]) > 0.8", // XFunction
},
},
}

errs := manager.ValidateRuleGroup(ruleGroupWithXFunc)

// Should not have validation errors
if len(errs) != 0 {
t.Fatalf("Expected no validation errors for XFunction after fix, got: %v", errs)
}
}

func TestValidateRuleGroup_AcceptsStandardFunctions(t *testing.T) {
manager := &DefaultMultiTenantManager{}

// Test rule with standard function (should pass)
ruleGroupStandard := rulefmt.RuleGroup{
Name: "test_group",
Rules: []rulefmt.Rule{
{
Alert: "TestAlert",
Expr: "rate(cpu_usage[5m]) > 0.8", // Standard function
},
},
}

errs := manager.ValidateRuleGroup(ruleGroupStandard)

// Should have no validation errors
if len(errs) != 0 {
t.Fatalf("Expected no validation errors for standard function, got: %v", errs)
}
}

func TestValidateRuleGroup_RejectsInvalidRules(t *testing.T) {
manager := &DefaultMultiTenantManager{}

// Test rule with invalid expression syntax
ruleGroupInvalid := rulefmt.RuleGroup{
Name: "test_group",
Rules: []rulefmt.Rule{
{
Alert: "TestAlert",
Expr: "invalid_syntax_here >", // Invalid expression
},
},
}

errs := manager.ValidateRuleGroup(ruleGroupInvalid)

// Should have validation errors and they should be properly propagated
require.NotEmpty(t, errs, "Expected validation errors for invalid expression")
// Verify the error is a rulefmt.Error with proper group information
ruleErr, ok := errs[0].(*rulefmt.Error)
require.True(t, ok, "Error should be of type *rulefmt.Error")
require.Equal(t, "test_group", ruleErr.Group, "Error should contain correct group name")
require.Equal(t, "TestAlert", ruleErr.RuleName, "Error should contain correct rule name")
}

func TestValidateRuleGroup_RejectsEmptyGroupName(t *testing.T) {
manager := &DefaultMultiTenantManager{}

// Test rule group with empty name
ruleGroupEmptyName := rulefmt.RuleGroup{
Name: "", // Empty name
Rules: []rulefmt.Rule{
{
Alert: "TestAlert",
Expr: "rate(cpu_usage[5m]) > 0.8",
},
},
}

errs := manager.ValidateRuleGroup(ruleGroupEmptyName)

// Should have validation errors
require.NotEmpty(t, errs, "Expected validation errors for empty group name")
require.Contains(t, errs[0].Error(), "rule group name must not be empty", "Error should mention empty group name")
}
Loading