Adding parser for Xfunctions in Rule validation (#7111)

Author: PaurushGarg

CHANGELOG.md

@@ -8,6 +8,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
 

pkg/ruler/manager.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"strings"
 	"sync"
 	"time"
 
@@ -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"
 )
@@ -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,
-			})
 		}
 	}
 

pkg/ruler/manager_test.go

@@ -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"
@@ -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")
+}