Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
* [FEATURE] Ruler: Add XFunctions validation support. #7111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move change log?
The changelog order is:

* `[CHANGE]`
* `[FEATURE]`
* `[ENHANCEMENT]`
* `[BUGFIX]`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call it a bugfix instead of a feature. Ruler xfunction support was added in last release but it doesn't work because of this failure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it seems a bug rather than feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have updated to bugfix.


## 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