From 1e1adafeecdb54847c1310fee0b825eda1276d71 Mon Sep 17 00:00:00 2001 From: Moustafa Baiou Date: Fri, 7 Nov 2025 12:01:16 -0500 Subject: [PATCH] Alerting: Add admission hooks for rules app (#113429) This adds validating admission hooks to enforce the requirements on AlertRules and RecordingRules that are currently enforced through the provisioning service and storage mechanisms in preparation of a consistent validation in both legacy storage and unified storage. It also adds a mutating admission hook to the app to ensure that folder annotations and folder labels are kept in sync so we can perform label-selector lists. --- .../rules/definitions/alerting-manifest.yaml | 22 ++- apps/alerting/rules/go.mod | 2 +- apps/alerting/rules/kinds/alertRule.cue | 12 ++ apps/alerting/rules/kinds/recordingRule.cue | 12 ++ .../apis/alerting/v0alpha1/alertrule_ext.go | 77 +++++++- .../rules/pkg/apis/alerting/v0alpha1/ext.go | 29 +++ .../alerting/v0alpha1/recordingrule_ext.go | 46 ++++- .../rules/pkg/apis/alerting_manifest.go | 32 +++- .../rules/pkg/app/alertrule/mutator.go | 45 +++++ .../rules/pkg/app/alertrule/validator.go | 123 ++++++++++++ apps/alerting/rules/pkg/app/app.go | 35 +++- apps/alerting/rules/pkg/app/app_test.go | 175 ++++++++++++++++++ apps/alerting/rules/pkg/app/config/runtime.go | 22 +++ .../rules/pkg/app/recordingrule/mutator.go | 37 ++++ .../rules/pkg/app/recordingrule/validator.go | 95 ++++++++++ apps/alerting/rules/pkg/app/util/validator.go | 27 +++ .../apps/alerting/rules/alertrule/compat.go | 4 + .../alerting/rules/recordingrule/compat.go | 4 + .../rules/recordingrule/legacy_storage.go | 7 +- pkg/registry/apps/alerting/rules/register.go | 69 ++++++- .../rules/alertrule/alertrule_test.go | 147 ++++++++++++++- .../rules/recordingrule/recordingrule_test.go | 138 +++++++++++++- 22 files changed, 1144 insertions(+), 16 deletions(-) create mode 100644 apps/alerting/rules/pkg/app/alertrule/mutator.go create mode 100644 apps/alerting/rules/pkg/app/alertrule/validator.go create mode 100644 apps/alerting/rules/pkg/app/app_test.go create mode 100644 apps/alerting/rules/pkg/app/config/runtime.go create mode 100644 apps/alerting/rules/pkg/app/recordingrule/mutator.go create mode 100644 apps/alerting/rules/pkg/app/recordingrule/validator.go create mode 100644 apps/alerting/rules/pkg/app/util/validator.go diff --git a/apps/alerting/rules/definitions/alerting-manifest.yaml b/apps/alerting/rules/definitions/alerting-manifest.yaml index 836bd2c9a94..a8927dd1ea4 100644 --- a/apps/alerting/rules/definitions/alerting-manifest.yaml +++ b/apps/alerting/rules/definitions/alerting-manifest.yaml @@ -8,7 +8,16 @@ spec: preferredVersion: v0alpha1 versions: - kinds: - - conversion: false + - admission: + mutation: + operations: + - CREATE + - UPDATE + validation: + operations: + - CREATE + - UPDATE + conversion: false kind: AlertRule plural: AlertRules schemas: @@ -214,7 +223,16 @@ spec: - spec.panelRef.dashboardUID - spec.panelRef.panelID - spec.notificationSettings.receiver - - conversion: false + - admission: + mutation: + operations: + - CREATE + - UPDATE + validation: + operations: + - CREATE + - UPDATE + conversion: false kind: RecordingRule plural: RecordingRules schemas: diff --git a/apps/alerting/rules/go.mod b/apps/alerting/rules/go.mod index f10d66be03f..f1ccc575706 100644 --- a/apps/alerting/rules/go.mod +++ b/apps/alerting/rules/go.mod @@ -5,6 +5,7 @@ go 1.25.3 require ( github.com/grafana/grafana-app-sdk v0.48.1 github.com/grafana/grafana-app-sdk/logging v0.48.1 + github.com/prometheus/common v0.67.1 k8s.io/apimachinery v0.34.1 k8s.io/kube-openapi v0.0.0-20250910181357-589584f1c912 ) @@ -49,7 +50,6 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.23.2 // indirect github.com/prometheus/client_model v0.6.2 // indirect - github.com/prometheus/common v0.67.1 // indirect github.com/prometheus/procfs v0.16.1 // indirect github.com/puzpuzpuz/xsync/v2 v2.5.1 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect diff --git a/apps/alerting/rules/kinds/alertRule.cue b/apps/alerting/rules/kinds/alertRule.cue index bbc374c244c..f3c7293cf76 100644 --- a/apps/alerting/rules/kinds/alertRule.cue +++ b/apps/alerting/rules/kinds/alertRule.cue @@ -13,6 +13,18 @@ alertRulev0alpha1: alertRuleKind & { schema: { spec: v0alpha1.AlertRuleSpec } + validation: { + operations: [ + "CREATE", + "UPDATE", + ] + } + mutation: { + operations: [ + "CREATE", + "UPDATE", + ] + } selectableFields: [ "spec.title", "spec.paused", diff --git a/apps/alerting/rules/kinds/recordingRule.cue b/apps/alerting/rules/kinds/recordingRule.cue index 3a6e14907ba..3a126afc0f2 100644 --- a/apps/alerting/rules/kinds/recordingRule.cue +++ b/apps/alerting/rules/kinds/recordingRule.cue @@ -13,6 +13,18 @@ recordingRulev0alpha1: recordingRuleKind & { schema: { spec: v0alpha1.RecordingRuleSpec } + validation: { + operations: [ + "CREATE", + "UPDATE", + ] + } + mutation: { + operations: [ + "CREATE", + "UPDATE", + ] + } selectableFields: [ "spec.title", "spec.paused", diff --git a/apps/alerting/rules/pkg/apis/alerting/v0alpha1/alertrule_ext.go b/apps/alerting/rules/pkg/apis/alerting/v0alpha1/alertrule_ext.go index da4b07b3df8..a705925fd5f 100644 --- a/apps/alerting/rules/pkg/apis/alerting/v0alpha1/alertrule_ext.go +++ b/apps/alerting/rules/pkg/apis/alerting/v0alpha1/alertrule_ext.go @@ -3,6 +3,7 @@ package v0alpha1 import ( "fmt" "slices" + "time" ) func (o *AlertRule) GetProvenanceStatus() string { @@ -48,4 +49,78 @@ func (s *AlertRuleSpec) ExecErrStateOrDefault() string { return s.ExecErrState } -// TODO: add duration clamping for the field types AlertRulePromDuration, AlertRulePromDurationWMillis, and the For and KeepFiringFor string pointers +func (d *AlertRulePromDuration) ToDuration() (time.Duration, error) { + return ToDuration(string(*d)) +} + +func (d *AlertRulePromDurationWMillis) ToDuration() (time.Duration, error) { + return ToDuration(string(*d)) +} + +func (d *AlertRulePromDuration) Clamp() error { + clampedDuration, err := ClampDuration(string(*d)) + if err != nil { + return err + } + *d = AlertRulePromDuration(clampedDuration) + return nil +} + +func (d *AlertRulePromDurationWMillis) Clamp() error { + clampedDuration, err := ClampDuration(string(*d)) + if err != nil { + return err + } + *d = AlertRulePromDurationWMillis(clampedDuration) + return nil +} + +func (spec *AlertRuleSpec) ClampDurations() error { + // clamp all duration fields + if err := spec.Trigger.Interval.Clamp(); err != nil { + return err + } + if spec.For != nil { + clamped, err := ClampDuration(*spec.For) + if err != nil { + return err + } + spec.For = &clamped + } + if spec.KeepFiringFor != nil { + clamped, err := ClampDuration(*spec.KeepFiringFor) + if err != nil { + return err + } + spec.KeepFiringFor = &clamped + } + if spec.NotificationSettings != nil { + if spec.NotificationSettings.GroupWait != nil { + if err := spec.NotificationSettings.GroupWait.Clamp(); err != nil { + return err + } + } + if spec.NotificationSettings.GroupInterval != nil { + if err := spec.NotificationSettings.GroupInterval.Clamp(); err != nil { + return err + } + } + if spec.NotificationSettings.RepeatInterval != nil { + if err := spec.NotificationSettings.RepeatInterval.Clamp(); err != nil { + return err + } + } + } + for k, expr := range spec.Expressions { + if expr.RelativeTimeRange != nil { + if err := expr.RelativeTimeRange.From.Clamp(); err != nil { + return err + } + if err := expr.RelativeTimeRange.To.Clamp(); err != nil { + return err + } + spec.Expressions[k] = expr + } + } + return nil +} diff --git a/apps/alerting/rules/pkg/apis/alerting/v0alpha1/ext.go b/apps/alerting/rules/pkg/apis/alerting/v0alpha1/ext.go index 7b87f8ee8e9..73606736a3f 100644 --- a/apps/alerting/rules/pkg/apis/alerting/v0alpha1/ext.go +++ b/apps/alerting/rules/pkg/apis/alerting/v0alpha1/ext.go @@ -1,10 +1,22 @@ package v0alpha1 +import ( + "fmt" + "time" + + prom_model "github.com/prometheus/common/model" +) + const ( InternalPrefix = "grafana.com/" GroupLabelKey = InternalPrefix + "group" GroupIndexLabelKey = GroupLabelKey + "-index" ProvenanceStatusAnnotationKey = InternalPrefix + "provenance" + // Copy of the max title length used in legacy validation path + AlertRuleMaxTitleLength = 190 + // Annotation key used to store the folder UID on resources + FolderAnnotationKey = "grafana.app/folder" + FolderLabelKey = FolderAnnotationKey ) const ( @@ -15,3 +27,20 @@ const ( var ( AcceptedProvenanceStatuses = []string{ProvenanceStatusNone, ProvenanceStatusAPI} ) + +func ToDuration(s string) (time.Duration, error) { + promDuration, err := prom_model.ParseDuration(s) + if err != nil { + return 0, fmt.Errorf("invalid duration format: %w", err) + } + return time.Duration(promDuration), nil +} + +// Convert the string duration to the longest valid Prometheus duration format (e.g., "60s" -> "1m") +func ClampDuration(s string) (string, error) { + promDuration, err := prom_model.ParseDuration(s) + if err != nil { + return "", fmt.Errorf("invalid duration format: %w", err) + } + return promDuration.String(), nil +} diff --git a/apps/alerting/rules/pkg/apis/alerting/v0alpha1/recordingrule_ext.go b/apps/alerting/rules/pkg/apis/alerting/v0alpha1/recordingrule_ext.go index 0d40fc4c7b6..50bd606ef39 100644 --- a/apps/alerting/rules/pkg/apis/alerting/v0alpha1/recordingrule_ext.go +++ b/apps/alerting/rules/pkg/apis/alerting/v0alpha1/recordingrule_ext.go @@ -3,6 +3,7 @@ package v0alpha1 import ( "fmt" "slices" + "time" ) func (o *RecordingRule) GetProvenanceStatus() string { @@ -27,4 +28,47 @@ func (o *RecordingRule) SetProvenanceStatus(status string) (err error) { return } -// TODO: add duration clamping for the field types RecordingRulePromDurationWMillis and RecordingRulePromDuration +func (d *RecordingRulePromDuration) ToDuration() (time.Duration, error) { + return ToDuration(string(*d)) +} + +func (d *RecordingRulePromDurationWMillis) ToDuration() (time.Duration, error) { + return ToDuration(string(*d)) +} + +func (d *RecordingRulePromDuration) Clamp() error { + clampedDuration, err := ClampDuration(string(*d)) + if err != nil { + return err + } + *d = RecordingRulePromDuration(clampedDuration) + return nil +} + +func (d *RecordingRulePromDurationWMillis) Clamp() error { + clampedDuration, err := ClampDuration(string(*d)) + if err != nil { + return err + } + *d = RecordingRulePromDurationWMillis(clampedDuration) + return nil +} + +func (spec *RecordingRuleSpec) ClampDurations() error { + // clamp all duration fields + if err := spec.Trigger.Interval.Clamp(); err != nil { + return err + } + for k, expr := range spec.Expressions { + if expr.RelativeTimeRange != nil { + if err := expr.RelativeTimeRange.From.Clamp(); err != nil { + return err + } + if err := expr.RelativeTimeRange.To.Clamp(); err != nil { + return err + } + spec.Expressions[k] = expr + } + } + return nil +} diff --git a/apps/alerting/rules/pkg/apis/alerting_manifest.go b/apps/alerting/rules/pkg/apis/alerting_manifest.go index 5057a58c6e6..f81fb7d43fc 100644 --- a/apps/alerting/rules/pkg/apis/alerting_manifest.go +++ b/apps/alerting/rules/pkg/apis/alerting_manifest.go @@ -42,7 +42,21 @@ var appManifestData = app.ManifestData{ Plural: "AlertRules", Scope: "Namespaced", Conversion: false, - Schema: &versionSchemaAlertRulev0alpha1, + Admission: &app.AdmissionCapabilities{ + Validation: &app.ValidationCapability{ + Operations: []app.AdmissionOperation{ + app.AdmissionOperationCreate, + app.AdmissionOperationUpdate, + }, + }, + Mutation: &app.MutationCapability{ + Operations: []app.AdmissionOperation{ + app.AdmissionOperationCreate, + app.AdmissionOperationUpdate, + }, + }, + }, + Schema: &versionSchemaAlertRulev0alpha1, SelectableFields: []string{ "spec.title", "spec.paused", @@ -57,7 +71,21 @@ var appManifestData = app.ManifestData{ Plural: "RecordingRules", Scope: "Namespaced", Conversion: false, - Schema: &versionSchemaRecordingRulev0alpha1, + Admission: &app.AdmissionCapabilities{ + Validation: &app.ValidationCapability{ + Operations: []app.AdmissionOperation{ + app.AdmissionOperationCreate, + app.AdmissionOperationUpdate, + }, + }, + Mutation: &app.MutationCapability{ + Operations: []app.AdmissionOperation{ + app.AdmissionOperationCreate, + app.AdmissionOperationUpdate, + }, + }, + }, + Schema: &versionSchemaRecordingRulev0alpha1, SelectableFields: []string{ "spec.title", "spec.paused", diff --git a/apps/alerting/rules/pkg/app/alertrule/mutator.go b/apps/alerting/rules/pkg/app/alertrule/mutator.go new file mode 100644 index 00000000000..a127e0699c9 --- /dev/null +++ b/apps/alerting/rules/pkg/app/alertrule/mutator.go @@ -0,0 +1,45 @@ +package alertrule + +import ( + "context" + + "github.com/grafana/grafana-app-sdk/app" + "github.com/grafana/grafana-app-sdk/simple" + v1 "github.com/grafana/grafana/apps/alerting/rules/pkg/apis/alerting/v0alpha1" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/config" +) + +func NewMutator(cfg config.RuntimeConfig) *simple.Mutator { + return &simple.Mutator{ + MutateFunc: func(ctx context.Context, req *app.AdmissionRequest) (*app.MutatingResponse, error) { + // Mutate folder label to match folder UID from annotation + r, ok := req.Object.(*v1.AlertRule) + if !ok || r == nil { + // Nothing to do or wrong type; no mutation + return nil, nil + } + + // Read folder UID from annotation + folderUID := "" + if r.Annotations != nil { + folderUID = r.Annotations[v1.FolderAnnotationKey] + } + + // Ensure labels map exists and set the folder label if folderUID is present + if folderUID != "" { + if r.Labels == nil { + r.Labels = make(map[string]string) + } + // Maintain folder metadata label for downstream systems (alertmanager grouping etc.) + r.Labels[v1.FolderLabelKey] = folderUID + } + + // clamp all duration fields + if err := r.Spec.ClampDurations(); err != nil { + return nil, err + } + + return &app.MutatingResponse{UpdatedObject: r}, nil + }, + } +} diff --git a/apps/alerting/rules/pkg/app/alertrule/validator.go b/apps/alerting/rules/pkg/app/alertrule/validator.go new file mode 100644 index 00000000000..e86d17cab48 --- /dev/null +++ b/apps/alerting/rules/pkg/app/alertrule/validator.go @@ -0,0 +1,123 @@ +package alertrule + +import ( + "context" + "fmt" + "slices" + "strconv" + "time" + + "github.com/grafana/grafana-app-sdk/app" + "github.com/grafana/grafana-app-sdk/resource" + "github.com/grafana/grafana-app-sdk/simple" + model "github.com/grafana/grafana/apps/alerting/rules/pkg/apis/alerting/v0alpha1" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/config" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/util" + prom_model "github.com/prometheus/common/model" +) + +func NewValidator(cfg config.RuntimeConfig) *simple.Validator { + return &simple.Validator{ + ValidateFunc: func(ctx context.Context, req *app.AdmissionRequest) error { + // Cast to specific type + r, ok := req.Object.(*model.AlertRule) + if !ok { + return fmt.Errorf("object is not of type *v0alpha1.AlertRule") + } + + // 1) Validate provenance status annotation + sourceProv := r.GetProvenanceStatus() + if !slices.Contains(model.AcceptedProvenanceStatuses, sourceProv) { + return fmt.Errorf("invalid provenance status: %s", sourceProv) + } + + // 2) Validate group labels rules + group := r.Labels[model.GroupLabelKey] + groupIndexStr := r.Labels[model.GroupIndexLabelKey] + if req.Action == resource.AdmissionActionCreate { + if group != "" || groupIndexStr != "" { + return fmt.Errorf("cannot set group when creating alert rule") + } + } + if group != "" { // if group is set, group-index must be set and numeric + if groupIndexStr == "" { + return fmt.Errorf("%s must be set when %s is set", model.GroupIndexLabelKey, model.GroupLabelKey) + } + if _, err := strconv.Atoi(groupIndexStr); err != nil { + return fmt.Errorf("invalid %s: %w", model.GroupIndexLabelKey, err) + } + } + + // 3) Validate folder is set and exists + // Read folder UID directly from annotations + folderUID := "" + if r.Annotations != nil { + folderUID = r.Annotations[model.FolderAnnotationKey] + } + if folderUID == "" { + return fmt.Errorf("folder is required") + } + if cfg.FolderValidator != nil { + ok, verr := cfg.FolderValidator(ctx, folderUID) + if verr != nil { + return fmt.Errorf("failed to validate folder: %w", verr) + } + if !ok { + return fmt.Errorf("folder does not exist: %s", folderUID) + } + } + + // 4) Validate notification settings receiver if provided + if r.Spec.NotificationSettings != nil && r.Spec.NotificationSettings.Receiver != "" && cfg.NotificationSettingsValidator != nil { + ok, nerr := cfg.NotificationSettingsValidator(ctx, r.Spec.NotificationSettings.Receiver) + if nerr != nil { + return fmt.Errorf("failed to validate notification settings: %w", nerr) + } + if !ok { + return fmt.Errorf("invalid notification receiver: %s", r.Spec.NotificationSettings.Receiver) + } + } + + // 5) Enforce max title length + if len(r.Spec.Title) > model.AlertRuleMaxTitleLength { + return fmt.Errorf("alert rule title is too long. Max length is %d", model.AlertRuleMaxTitleLength) + } + + // 6) Validate evaluation interval against base interval + if err := util.ValidateInterval(cfg.BaseEvaluationInterval, &r.Spec.Trigger.Interval); err != nil { + return err + } + + // 7) Disallow reserved/spec system label keys + if r.Spec.Labels != nil { + for key := range r.Spec.Labels { + if _, bad := cfg.ReservedLabelKeys[key]; bad { + return fmt.Errorf("label key is reserved and cannot be specified: %s", key) + } + } + } + + // 8) For and KeepFiringFor must be >= 0 if set + if r.Spec.For != nil { + d, err := prom_model.ParseDuration(*r.Spec.For) + if err != nil { + return fmt.Errorf("invalid 'for' duration: %w", err) + } + if time.Duration(d) < 0 { + return fmt.Errorf("'for' cannot be less than 0") + } + } + if r.Spec.KeepFiringFor != nil { + d, err := prom_model.ParseDuration(*r.Spec.KeepFiringFor) + if err != nil { + return fmt.Errorf("invalid 'keepFiringFor' duration: %w", err) + } + if time.Duration(d) < 0 { + return fmt.Errorf("'keepFiringFor' cannot be less than 0") + } + } + + return nil + }, + } +} diff --git a/apps/alerting/rules/pkg/app/app.go b/apps/alerting/rules/pkg/app/app.go index ef0ab1883cf..3c546239c81 100644 --- a/apps/alerting/rules/pkg/app/app.go +++ b/apps/alerting/rules/pkg/app/app.go @@ -6,16 +6,29 @@ import ( "github.com/grafana/grafana-app-sdk/app" "github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana-app-sdk/operator" + "github.com/grafana/grafana-app-sdk/resource" "github.com/grafana/grafana-app-sdk/simple" "github.com/grafana/grafana/apps/alerting/rules/pkg/apis" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/alertrule" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/config" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/recordingrule" ) func New(cfg app.Config) (app.App, error) { managedKinds := make([]simple.AppManagedKind, 0) + runtimeCfg, ok := cfg.SpecificConfig.(config.RuntimeConfig) + if !ok { + return nil, config.ErrInvalidRuntimeConfig + } for _, kinds := range apis.GetKinds() { for _, kind := range kinds { - managedKinds = append(managedKinds, simple.AppManagedKind{Kind: kind}) + managedKind := simple.AppManagedKind{ + Kind: kind, + Validator: buildKindValidator(kind, runtimeCfg), + Mutator: buildKindMutator(kind, runtimeCfg), + } + managedKinds = append(managedKinds, managedKind) } } @@ -44,3 +57,23 @@ func New(cfg app.Config) (app.App, error) { return a, nil } + +func buildKindValidator(kind resource.Kind, cfg config.RuntimeConfig) *simple.Validator { + switch kind.Kind() { + case "AlertRule": + return alertrule.NewValidator(cfg) + case "RecordingRule": + return recordingrule.NewValidator(cfg) + } + return nil +} + +func buildKindMutator(kind resource.Kind, cfg config.RuntimeConfig) *simple.Mutator { + switch kind.Kind() { + case "AlertRule": + return alertrule.NewMutator(cfg) + case "RecordingRule": + return recordingrule.NewMutator(cfg) + } + return nil +} diff --git a/apps/alerting/rules/pkg/app/app_test.go b/apps/alerting/rules/pkg/app/app_test.go new file mode 100644 index 00000000000..405e397f8cd --- /dev/null +++ b/apps/alerting/rules/pkg/app/app_test.go @@ -0,0 +1,175 @@ +package app_test + +import ( + "context" + "testing" + "time" + + appsdk "github.com/grafana/grafana-app-sdk/app" + "github.com/grafana/grafana-app-sdk/resource" + + v1 "github.com/grafana/grafana/apps/alerting/rules/pkg/apis/alerting/v0alpha1" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/alertrule" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/config" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/recordingrule" +) + +func makeDefaultRuntimeConfig() config.RuntimeConfig { + return config.RuntimeConfig{ + FolderValidator: func(ctx context.Context, folderUID string) (bool, error) { return folderUID == "f1", nil }, + BaseEvaluationInterval: 60 * time.Second, // seconds + ReservedLabelKeys: map[string]struct{}{"__reserved__": {}, "grafana_folder": {}}, + NotificationSettingsValidator: func(ctx context.Context, receiver string) (bool, error) { return receiver == "notif-ok", nil }, + } +} + +func TestAlertRuleValidation_Success(t *testing.T) { + r := &v1.AlertRule{} + r.SetGroupVersionKind(v1.AlertRuleKind().GroupVersionKind()) + r.Name = "uid-1" + r.Namespace = "ns1" + r.Annotations = map[string]string{v1.FolderAnnotationKey: "f1"} + r.Labels = map[string]string{} + r.Spec = v1.AlertRuleSpec{ + Title: "ok", + Trigger: v1.AlertRuleIntervalTrigger{Interval: v1.AlertRulePromDuration("60s")}, + Expressions: v1.AlertRuleExpressionMap{"A": v1.AlertRuleExpression{Model: map[string]any{"expr": "1"}, Source: boolPtr(true)}}, + NoDataState: v1.DefaultNoDataState, + ExecErrState: v1.DefaultExecErrState, + NotificationSettings: &v1.AlertRuleV0alpha1SpecNotificationSettings{Receiver: "notif-ok"}, + } + + req := &appsdk.AdmissionRequest{Action: resource.AdmissionActionCreate, Object: r} + validator := alertrule.NewValidator(makeDefaultRuntimeConfig()) + if err := validator.Validate(context.Background(), req); err != nil { + t.Fatalf("expected success, got error: %v", err) + } +} + +func TestAlertRuleValidation_Errors(t *testing.T) { + mk := func(mut func(r *v1.AlertRule)) error { + r := baseAlertRule() + mut(r) + return alertrule.NewValidator(makeDefaultRuntimeConfig()).Validate(context.Background(), &appsdk.AdmissionRequest{Action: resource.AdmissionActionCreate, Object: r}) + } + + if err := mk(func(r *v1.AlertRule) { r.Annotations = nil }); err == nil { + t.Errorf("want folder required error") + } + if err := mk(func(r *v1.AlertRule) { r.Annotations[v1.FolderAnnotationKey] = "bad" }); err == nil { + t.Errorf("want folder not exist error") + } + if err := mk(func(r *v1.AlertRule) { r.Spec.Trigger.Interval = v1.AlertRulePromDuration("30s") }); err == nil { + t.Errorf("want base interval multiple error") + } + if err := mk(func(r *v1.AlertRule) { + r.Spec.NotificationSettings = &v1.AlertRuleV0alpha1SpecNotificationSettings{Receiver: "bad"} + }); err == nil { + t.Errorf("want invalid receiver error") + } + if err := mk(func(r *v1.AlertRule) { r.Labels[v1.GroupLabelKey] = "grp" }); err == nil { + t.Errorf("want group set on create error") + } + if err := mk(func(r *v1.AlertRule) { r.Spec.For = strPtr("-10s") }); err == nil { + t.Errorf("want for>=0 error") + } + if err := mk(func(r *v1.AlertRule) { + if r.Spec.Labels == nil { + r.Spec.Labels = map[string]v1.AlertRuleTemplateString{} + } + r.Spec.Labels["__reserved__"] = v1.AlertRuleTemplateString("x") + }); err == nil { + t.Errorf("want reserved label key error") + } +} + +func baseAlertRule() *v1.AlertRule { + r := &v1.AlertRule{} + r.SetGroupVersionKind(v1.AlertRuleKind().GroupVersionKind()) + r.Name = "uid-1" + r.Namespace = "ns1" + r.Annotations = map[string]string{v1.FolderAnnotationKey: "f1"} + r.Labels = map[string]string{} + r.Spec = v1.AlertRuleSpec{ + Title: "ok", + Trigger: v1.AlertRuleIntervalTrigger{Interval: v1.AlertRulePromDuration("60s")}, + Expressions: v1.AlertRuleExpressionMap{"A": v1.AlertRuleExpression{Model: map[string]any{"expr": "1"}, Source: boolPtr(true)}}, + NoDataState: v1.DefaultNoDataState, + ExecErrState: v1.DefaultExecErrState, + } + return r +} + +func TestRecordingRuleValidation_Success(t *testing.T) { + r := &v1.RecordingRule{} + r.SetGroupVersionKind(v1.RecordingRuleKind().GroupVersionKind()) + r.Name = "uid-2" + r.Namespace = "ns1" + r.Annotations = map[string]string{v1.FolderAnnotationKey: "f1"} + r.Labels = map[string]string{} + r.Spec = v1.RecordingRuleSpec{ + Title: "ok", + Trigger: v1.RecordingRuleIntervalTrigger{Interval: v1.RecordingRulePromDuration("60s")}, + Expressions: v1.RecordingRuleExpressionMap{"A": v1.RecordingRuleExpression{Model: map[string]any{"expr": "1"}, Source: boolPtr(true)}}, + Metric: "test_metric", + TargetDatasourceUID: "ds1", + } + + req := &appsdk.AdmissionRequest{Action: resource.AdmissionActionCreate, Object: r} + validator := recordingrule.NewValidator(makeDefaultRuntimeConfig()) + if err := validator.Validate(context.Background(), req); err != nil { + t.Fatalf("expected success, got error: %v", err) + } +} + +func TestRecordingRuleValidation_Errors(t *testing.T) { + mk := func(mut func(r *v1.RecordingRule)) error { + r := baseRecordingRule() + mut(r) + return recordingrule.NewValidator(makeDefaultRuntimeConfig()).Validate(context.Background(), &appsdk.AdmissionRequest{Action: resource.AdmissionActionCreate, Object: r}) + } + + if err := mk(func(r *v1.RecordingRule) { r.Annotations = nil }); err == nil { + t.Errorf("want folder required error") + } + if err := mk(func(r *v1.RecordingRule) { r.Annotations[v1.FolderAnnotationKey] = "bad" }); err == nil { + t.Errorf("want folder not exist error") + } + if err := mk(func(r *v1.RecordingRule) { r.Spec.Trigger.Interval = v1.RecordingRulePromDuration("30s") }); err == nil { + t.Errorf("want base interval multiple error") + } + if err := mk(func(r *v1.RecordingRule) { r.Labels[v1.GroupLabelKey] = "grp" }); err == nil { + t.Errorf("want group set on create error") + } + if err := mk(func(r *v1.RecordingRule) { r.Spec.Metric = "" }); err == nil { + t.Errorf("want metric required error") + } + if err := mk(func(r *v1.RecordingRule) { + if r.Spec.Labels == nil { + r.Spec.Labels = map[string]v1.RecordingRuleTemplateString{} + } + r.Spec.Labels["__reserved__"] = v1.RecordingRuleTemplateString("x") + }); err == nil { + t.Errorf("want reserved label key error") + } +} + +func baseRecordingRule() *v1.RecordingRule { + r := &v1.RecordingRule{} + r.SetGroupVersionKind(v1.RecordingRuleKind().GroupVersionKind()) + r.Name = "uid-1" + r.Namespace = "ns1" + r.Annotations = map[string]string{v1.FolderAnnotationKey: "f1"} + r.Labels = map[string]string{} + r.Spec = v1.RecordingRuleSpec{ + Title: "ok", + Trigger: v1.RecordingRuleIntervalTrigger{Interval: v1.RecordingRulePromDuration("60s")}, + Expressions: v1.RecordingRuleExpressionMap{"A": v1.RecordingRuleExpression{Model: map[string]any{"expr": "1"}, Source: boolPtr(true)}}, + Metric: "test_metric", + TargetDatasourceUID: "ds1", + } + return r +} + +func boolPtr(b bool) *bool { return &b } +func strPtr(s string) *string { return &s } diff --git a/apps/alerting/rules/pkg/app/config/runtime.go b/apps/alerting/rules/pkg/app/config/runtime.go new file mode 100644 index 00000000000..60936925a15 --- /dev/null +++ b/apps/alerting/rules/pkg/app/config/runtime.go @@ -0,0 +1,22 @@ +package config + +import ( + "context" + "errors" + "time" +) + +var ( + ErrInvalidRuntimeConfig = errors.New("invalid runtime config provided to alerting/rules app") +) + +// RuntimeConfig holds configuration values needed at runtime by the alerting/rules app from the running Grafana instance. +type RuntimeConfig struct { + // function to check folder existence given its uid + FolderValidator func(ctx context.Context, folderUID string) (bool, error) + // base evaluation interval + BaseEvaluationInterval time.Duration + // set of strings which are illegal for label keys on rules + ReservedLabelKeys map[string]struct{} + NotificationSettingsValidator func(ctx context.Context, receiver string) (bool, error) +} diff --git a/apps/alerting/rules/pkg/app/recordingrule/mutator.go b/apps/alerting/rules/pkg/app/recordingrule/mutator.go new file mode 100644 index 00000000000..31378c553f0 --- /dev/null +++ b/apps/alerting/rules/pkg/app/recordingrule/mutator.go @@ -0,0 +1,37 @@ +package recordingrule + +import ( + "context" + + "github.com/grafana/grafana-app-sdk/app" + "github.com/grafana/grafana-app-sdk/simple" + v1 "github.com/grafana/grafana/apps/alerting/rules/pkg/apis/alerting/v0alpha1" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/config" +) + +func NewMutator(cfg config.RuntimeConfig) *simple.Mutator { + return &simple.Mutator{ + MutateFunc: func(ctx context.Context, req *app.AdmissionRequest) (*app.MutatingResponse, error) { + r, ok := req.Object.(*v1.RecordingRule) + if !ok || r == nil { + return nil, nil + } + + folderUID := "" + if r.Annotations != nil { + folderUID = r.Annotations[v1.FolderAnnotationKey] + } + + if folderUID != "" { + if r.Labels == nil { + r.Labels = make(map[string]string) + } + r.Labels[v1.FolderLabelKey] = folderUID + } + if err := r.Spec.ClampDurations(); err != nil { + return nil, err + } + return &app.MutatingResponse{UpdatedObject: r}, nil + }, + } +} diff --git a/apps/alerting/rules/pkg/app/recordingrule/validator.go b/apps/alerting/rules/pkg/app/recordingrule/validator.go new file mode 100644 index 00000000000..8354b9d7501 --- /dev/null +++ b/apps/alerting/rules/pkg/app/recordingrule/validator.go @@ -0,0 +1,95 @@ +package recordingrule + +import ( + "context" + "fmt" + "slices" + "strconv" + + "github.com/grafana/grafana-app-sdk/app" + "github.com/grafana/grafana-app-sdk/resource" + "github.com/grafana/grafana-app-sdk/simple" + model "github.com/grafana/grafana/apps/alerting/rules/pkg/apis/alerting/v0alpha1" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/config" + "github.com/grafana/grafana/apps/alerting/rules/pkg/app/util" + prom_model "github.com/prometheus/common/model" +) + +func NewValidator(cfg config.RuntimeConfig) *simple.Validator { + return &simple.Validator{ + ValidateFunc: func(ctx context.Context, req *app.AdmissionRequest) error { + // Cast to specific type + r, ok := req.Object.(*model.RecordingRule) + if !ok { + return fmt.Errorf("object is not of type *v0alpha1.RecordingRule") + } + + sourceProv := r.GetProvenanceStatus() + if !slices.Contains(model.AcceptedProvenanceStatuses, sourceProv) { + return fmt.Errorf("invalid provenance status: %s", sourceProv) + } + + group := r.Labels[model.GroupLabelKey] + groupIndexStr := r.Labels[model.GroupIndexLabelKey] + if req.Action == resource.AdmissionActionCreate { + if group != "" || groupIndexStr != "" { + return fmt.Errorf("cannot set group when creating recording rule") + } + } + if group != "" { + if groupIndexStr == "" { + return fmt.Errorf("%s must be set when %s is set", model.GroupIndexLabelKey, model.GroupLabelKey) + } + if _, err := strconv.Atoi(groupIndexStr); err != nil { + return fmt.Errorf("invalid %s: %w", model.GroupIndexLabelKey, err) + } + } + + folderUID := "" + if r.Annotations != nil { + folderUID = r.Annotations[model.FolderAnnotationKey] + } + if folderUID == "" { + return fmt.Errorf("folder is required") + } + if cfg.FolderValidator != nil { + ok, verr := cfg.FolderValidator(ctx, folderUID) + if verr != nil { + return fmt.Errorf("failed to validate folder: %w", verr) + } + if !ok { + return fmt.Errorf("folder does not exist: %s", folderUID) + } + } + + if len(r.Spec.Title) > model.AlertRuleMaxTitleLength { + return fmt.Errorf("recording rule title is too long. Max length is %d", model.AlertRuleMaxTitleLength) + } + + if err := util.ValidateInterval(cfg.BaseEvaluationInterval, &r.Spec.Trigger.Interval); err != nil { + return err + } + + if r.Spec.Labels != nil { + for key := range r.Spec.Labels { + if _, bad := cfg.ReservedLabelKeys[key]; bad { + return fmt.Errorf("label key is reserved and cannot be specified: %s", key) + } + } + } + + if r.Spec.Metric == "" { + return fmt.Errorf("metric must be specified") + } + metric := prom_model.LabelValue(r.Spec.Metric) + if !metric.IsValid() { + return fmt.Errorf("metric contains invalid characters") + } + if !prom_model.IsValidMetricName(metric) { // nolint:staticcheck + return fmt.Errorf("invalid metric name") + } + + return nil + }, + } +} diff --git a/apps/alerting/rules/pkg/app/util/validator.go b/apps/alerting/rules/pkg/app/util/validator.go new file mode 100644 index 00000000000..5f09c61e1cc --- /dev/null +++ b/apps/alerting/rules/pkg/app/util/validator.go @@ -0,0 +1,27 @@ +package util + +import ( + "fmt" + "time" +) + +type DurationLike interface { + ToDuration() (time.Duration, error) +} + +func ValidateInterval(baseInterval time.Duration, d DurationLike) error { + interval, err := d.ToDuration() + if err != nil { + return fmt.Errorf("invalid trigger interval: %w", err) + } + // Ensure interval is positive and an integer multiple of BaseEvaluationInterval (if provided) + if interval <= 0 { + return fmt.Errorf("trigger interval must be greater than 0") + } + if baseInterval > 0 { + if (interval % baseInterval) != 0 { + return fmt.Errorf("trigger interval must be a multiple of base evaluation interval (%s)", baseInterval.String()) + } + } + return nil +} diff --git a/pkg/registry/apps/alerting/rules/alertrule/compat.go b/pkg/registry/apps/alerting/rules/alertrule/compat.go index 0ea9507c6e0..3b780f0c1d7 100644 --- a/pkg/registry/apps/alerting/rules/alertrule/compat.go +++ b/pkg/registry/apps/alerting/rules/alertrule/compat.go @@ -128,6 +128,10 @@ func convertToK8sResource( return nil, fmt.Errorf("failed to get metadata: %w", err) } meta.SetFolder(rule.NamespaceUID) + // Keep metadata label in sync with folder annotation for downstream consumers + if rule.NamespaceUID != "" { + k8sRule.Labels[model.FolderLabelKey] = rule.NamespaceUID + } if rule.UpdatedBy != nil { meta.SetUpdatedBy(string(*rule.UpdatedBy)) k8sRule.SetUpdatedBy(string(*rule.UpdatedBy)) diff --git a/pkg/registry/apps/alerting/rules/recordingrule/compat.go b/pkg/registry/apps/alerting/rules/recordingrule/compat.go index e9ef0c4e295..8626a0b7e91 100644 --- a/pkg/registry/apps/alerting/rules/recordingrule/compat.go +++ b/pkg/registry/apps/alerting/rules/recordingrule/compat.go @@ -76,6 +76,10 @@ func convertToK8sResource( return nil, fmt.Errorf("failed to get metadata: %w", err) } meta.SetFolder(rule.NamespaceUID) + // Keep metadata label in sync with folder annotation for downstream consumers + if rule.NamespaceUID != "" { + k8sRule.Labels[model.FolderLabelKey] = rule.NamespaceUID + } if rule.UpdatedBy != nil { meta.SetUpdatedBy(string(*rule.UpdatedBy)) k8sRule.SetUpdatedBy(string(*rule.UpdatedBy)) diff --git a/pkg/registry/apps/alerting/rules/recordingrule/legacy_storage.go b/pkg/registry/apps/alerting/rules/recordingrule/legacy_storage.go index 395b955e0d0..df1b57db523 100644 --- a/pkg/registry/apps/alerting/rules/recordingrule/legacy_storage.go +++ b/pkg/registry/apps/alerting/rules/recordingrule/legacy_storage.go @@ -104,7 +104,7 @@ func (s *legacyStorage) Get(ctx context.Context, name string, _ *metav1.GetOptio return obj, err } -func (s *legacyStorage) Create(ctx context.Context, obj runtime.Object, _ rest.ValidateObjectFunc, _ *metav1.CreateOptions) (runtime.Object, error) { +func (s *legacyStorage) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, _ *metav1.CreateOptions) (runtime.Object, error) { info, err := request.NamespaceInfoFrom(ctx, true) if err != nil { return nil, err @@ -114,6 +114,11 @@ func (s *legacyStorage) Create(ctx context.Context, obj runtime.Object, _ rest.V if err != nil { return nil, err } + if createValidation != nil { + if err := createValidation(ctx, obj); err != nil { + return nil, err + } + } p, ok := obj.(*model.RecordingRule) if !ok { diff --git a/pkg/registry/apps/alerting/rules/register.go b/pkg/registry/apps/alerting/rules/register.go index 85b602ea832..d406495ac0e 100644 --- a/pkg/registry/apps/alerting/rules/register.go +++ b/pkg/registry/apps/alerting/rules/register.go @@ -14,13 +14,17 @@ import ( "github.com/grafana/grafana/apps/alerting/rules/pkg/apis" rulesApp "github.com/grafana/grafana/apps/alerting/rules/pkg/app" + rulesAppConfig "github.com/grafana/grafana/apps/alerting/rules/pkg/app/config" + "github.com/grafana/grafana/pkg/apimachinery/identity" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/registry/apps/alerting/rules/alertrule" "github.com/grafana/grafana/pkg/registry/apps/alerting/rules/recordingrule" "github.com/grafana/grafana/pkg/services/apiserver/appinstaller" - "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" + reqns "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/ngalert" + ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/notifier" "github.com/grafana/grafana/pkg/setting" ) @@ -50,11 +54,66 @@ func RegisterAppInstaller( ng: ng, } - provider := simple.NewAppProvider(apis.LocalManifest(), nil, rulesApp.New) + appSpecificConfig := rulesAppConfig.RuntimeConfig{ + // Validate folder existence using the folder service + FolderValidator: func(ctx context.Context, folderUID string) (bool, error) { + if folderUID == "" { + return false, nil + } + orgID, err := reqns.OrgIDForList(ctx) + user, _ := identity.GetRequester(ctx) + if (err != nil || orgID < 1) && user != nil { + orgID = user.GetOrgID() + } + if user == nil || orgID < 1 { + // If we can't resolve identity/org in this context, don't block creation based on existence + return true, nil + } + // Use the RuleStore to check namespace (folder) visibility + _, err = ng.Api.RuleStore.GetNamespaceByUID(ctx, folderUID, orgID, user) + if err != nil { + return false, nil + } + return true, nil + }, + BaseEvaluationInterval: ng.Cfg.UnifiedAlerting.BaseInterval, + ReservedLabelKeys: ngmodels.LabelsUserCannotSpecify, + // Validate that the configured notification receiver exists in the Alertmanager config + NotificationSettingsValidator: func(ctx context.Context, receiver string) (bool, error) { + if receiver == "" { + return false, nil + } + orgID, err := reqns.OrgIDForList(ctx) + if err != nil || orgID < 1 { + if user, _ := identity.GetRequester(ctx); user != nil { + orgID = user.GetOrgID() + } + } + if orgID < 1 { + // Without org context, skip validation rather than block + return true, nil + } + provider := notifier.NewCachedNotificationSettingsValidationService(ng.Api.AlertingStore) + vd, err := provider.Validator(ctx, orgID) + if err != nil { + log.New("alerting.rules.app").Error("failed to create notification settings validator", "error", err) + // If we cannot build a validator, don't block admission + return true, nil + } + // Only validate receiver presence; construct minimal settings + if err := vd.Validate(ngmodels.NotificationSettings{Receiver: receiver}); err != nil { + return false, nil + } + return true, nil + }, + } + + provider := simple.NewAppProvider(apis.LocalManifest(), appSpecificConfig, rulesApp.New) appConfig := app.Config{ - KubeConfig: restclient.Config{}, // this will be overridden by the installer's InitializeApp method - ManifestData: *apis.LocalManifest().ManifestData, + KubeConfig: restclient.Config{}, // this will be overridden by the installer's InitializeApp method + ManifestData: *apis.LocalManifest().ManifestData, + SpecificConfig: appSpecificConfig, } i, err := appsdkapiserver.NewDefaultAppInstaller(provider, appConfig, &apis.GoTypeAssociator{}) @@ -81,7 +140,7 @@ func (a *AlertingRulesAppInstaller) GetAuthorizer() authorizer.Authorizer { } func (a *AlertingRulesAppInstaller) GetLegacyStorage(gvr schema.GroupVersionResource) grafanarest.Storage { - namespacer := request.GetNamespaceMapper(a.cfg) + namespacer := reqns.GetNamespaceMapper(a.cfg) switch gvr { case recordingrule.ResourceInfo.GroupVersionResource(): return recordingrule.NewStorage(*a.ng.Api.AlertRules, namespacer) diff --git a/pkg/tests/apis/alerting/rules/alertrule/alertrule_test.go b/pkg/tests/apis/alerting/rules/alertrule/alertrule_test.go index ef44d030360..20b19346a3d 100644 --- a/pkg/tests/apis/alerting/rules/alertrule/alertrule_test.go +++ b/pkg/tests/apis/alerting/rules/alertrule/alertrule_test.go @@ -461,7 +461,7 @@ func TestIntegrationCRUD(t *testing.T) { } created, err := adminClient.Create(ctx, alertRule, v1.CreateOptions{}) - require.ErrorContains(t, err, "invalid alert rule") + require.ErrorContains(t, err, "trigger interval must be a multiple of base evaluation interval") require.Nil(t, created) }) } @@ -564,3 +564,148 @@ func TestIntegrationBasicAPI(t *testing.T) { t.Logf("Got error: %s", err) }) } + +func TestIntegrationFolderLabelSyncAndValidation(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + ctx := context.Background() + helper := common.GetTestHelper(t) + client := common.NewAlertRuleClient(t, helper.Org1.Admin) + + // Prepare two folders for label sync update scenario + common.CreateTestFolder(t, helper, "test-folder-a") + common.CreateTestFolder(t, helper, "test-folder-b") + + baseGen := ngmodels.RuleGen.With( + ngmodels.RuleMuts.WithUniqueUID(), + ngmodels.RuleMuts.WithUniqueTitle(), + ngmodels.RuleMuts.WithNamespaceUID("test-folder-a"), + ngmodels.RuleMuts.WithGroupName("test-group"), + ngmodels.RuleMuts.WithIntervalMatching(time.Duration(10)*time.Second), + ) + + t.Run("should keep folder label in sync with folder annotation on create and update", func(t *testing.T) { + rule := baseGen.Generate() + + alertRule := &v0alpha1.AlertRule{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Annotations: map[string]string{ + v0alpha1.FolderAnnotationKey: "test-folder-a", + }, + }, + Spec: v0alpha1.AlertRuleSpec{ + Title: rule.Title, + Expressions: v0alpha1.AlertRuleExpressionMap{ + "A": { + QueryType: util.Pointer(rule.Data[0].QueryType), + DatasourceUID: util.Pointer(v0alpha1.AlertRuleDatasourceUID(rule.Data[0].DatasourceUID)), + Model: rule.Data[0].Model, + Source: util.Pointer(true), + RelativeTimeRange: &v0alpha1.AlertRuleRelativeTimeRange{ + From: v0alpha1.AlertRulePromDurationWMillis("5m"), + To: v0alpha1.AlertRulePromDurationWMillis("0s"), + }, + }, + }, + Trigger: v0alpha1.AlertRuleIntervalTrigger{ + Interval: v0alpha1.AlertRulePromDuration(fmt.Sprintf("%ds", rule.IntervalSeconds)), + }, + NoDataState: string(rule.NoDataState), + ExecErrState: string(rule.ExecErrState), + }, + } + + created, err := client.Create(ctx, alertRule, v1.CreateOptions{}) + require.NoError(t, err) + defer func() { _ = client.Delete(ctx, created.Name, v1.DeleteOptions{}) }() + + // On create, metadata.labels[v0alpha1.FolderLabelKey] should mirror annotation + require.Equal(t, "test-folder-a", created.Labels[v0alpha1.FolderLabelKey]) + + // Update annotation to point to a different folder and ensure label follows + updated := created.Copy().(*v0alpha1.AlertRule) + if updated.Annotations == nil { + updated.Annotations = map[string]string{} + } + updated.Annotations[v0alpha1.FolderAnnotationKey] = "test-folder-b" + + after, err := client.Update(ctx, updated, v1.UpdateOptions{}) + require.NoError(t, err) + require.Equal(t, "test-folder-b", after.Annotations[v0alpha1.FolderAnnotationKey]) + require.Equal(t, "test-folder-b", after.Labels[v0alpha1.FolderLabelKey]) + }) + + t.Run("should fail to create rule without folder annotation", func(t *testing.T) { + rule := baseGen.Generate() + + alertRule := &v0alpha1.AlertRule{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Annotations: map[string]string{}, // missing grafana.app/folder + }, + Spec: v0alpha1.AlertRuleSpec{ + Title: rule.Title, + Expressions: v0alpha1.AlertRuleExpressionMap{ + "A": { + QueryType: util.Pointer(rule.Data[0].QueryType), + DatasourceUID: util.Pointer(v0alpha1.AlertRuleDatasourceUID(rule.Data[0].DatasourceUID)), + Model: rule.Data[0].Model, + Source: util.Pointer(true), + RelativeTimeRange: &v0alpha1.AlertRuleRelativeTimeRange{ + From: v0alpha1.AlertRulePromDurationWMillis("5m"), + To: v0alpha1.AlertRulePromDurationWMillis("0s"), + }, + }, + }, + Trigger: v0alpha1.AlertRuleIntervalTrigger{ + Interval: v0alpha1.AlertRulePromDuration("10s"), + }, + NoDataState: "NoData", + ExecErrState: "Error", + }, + } + + created, err := client.Create(ctx, alertRule, v1.CreateOptions{}) + require.Error(t, err) + require.Nil(t, created) + }) + + t.Run("should fail to create rule with group labels preset", func(t *testing.T) { + rule := baseGen.Generate() + alertRule := &v0alpha1.AlertRule{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Annotations: map[string]string{ + v0alpha1.FolderAnnotationKey: "test-folder-a", + }, + Labels: map[string]string{ + v0alpha1.GroupLabelKey: "some-group", + v0alpha1.GroupIndexLabelKey: "0", + }, + }, + Spec: v0alpha1.AlertRuleSpec{ + Title: rule.Title, + Expressions: v0alpha1.AlertRuleExpressionMap{ + "A": { + QueryType: util.Pointer(rule.Data[0].QueryType), + DatasourceUID: util.Pointer(v0alpha1.AlertRuleDatasourceUID(rule.Data[0].DatasourceUID)), + Model: rule.Data[0].Model, + Source: util.Pointer(true), + RelativeTimeRange: &v0alpha1.AlertRuleRelativeTimeRange{ + From: v0alpha1.AlertRulePromDurationWMillis("5m"), + To: v0alpha1.AlertRulePromDurationWMillis("0s"), + }, + }, + }, + Trigger: v0alpha1.AlertRuleIntervalTrigger{Interval: v0alpha1.AlertRulePromDuration("10s")}, + NoDataState: "NoData", + ExecErrState: "Error", + }, + } + + created, err := client.Create(ctx, alertRule, v1.CreateOptions{}) + require.Error(t, err) + require.Nil(t, created) + }) +} diff --git a/pkg/tests/apis/alerting/rules/recordingrule/recordingrule_test.go b/pkg/tests/apis/alerting/rules/recordingrule/recordingrule_test.go index 783a90f777d..e1c586f9bb9 100644 --- a/pkg/tests/apis/alerting/rules/recordingrule/recordingrule_test.go +++ b/pkg/tests/apis/alerting/rules/recordingrule/recordingrule_test.go @@ -454,7 +454,7 @@ func TestIntegrationCRUD(t *testing.T) { } created, err := adminClient.Create(ctx, recordingRule, v1.CreateOptions{}) - require.ErrorContains(t, err, "invalid alert rule") + require.ErrorContains(t, err, "trigger interval must be a multiple of base evaluation interval") require.Nil(t, created) }) } @@ -557,3 +557,139 @@ func TestIntegrationBasicAPI(t *testing.T) { t.Logf("Got error: %s", err) }) } + +func TestIntegrationFolderLabelSyncAndValidation(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + ctx := context.Background() + helper := common.GetTestHelper(t) + client := common.NewRecordingRuleClient(t, helper.Org1.Admin) + + // Prepare two folders for label sync update scenario + common.CreateTestFolder(t, helper, "test-folder-a") + common.CreateTestFolder(t, helper, "test-folder-b") + + baseGen := ngmodels.RuleGen.With( + ngmodels.RuleMuts.WithUniqueUID(), + ngmodels.RuleMuts.WithUniqueTitle(), + ngmodels.RuleMuts.WithNamespaceUID("test-folder-a"), + ngmodels.RuleMuts.WithGroupName("test-group"), + ngmodels.RuleMuts.WithAllRecordingRules(), + ngmodels.RuleMuts.WithIntervalMatching(time.Duration(10)*time.Second), + ) + + t.Run("should keep folder label in sync with folder annotation on create and update", func(t *testing.T) { + rule := baseGen.Generate() + recordingRule := &v0alpha1.RecordingRule{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Annotations: map[string]string{ + v0alpha1.FolderAnnotationKey: "test-folder-a", + }, + }, + Spec: v0alpha1.RecordingRuleSpec{ + Title: rule.Title, + Metric: rule.Record.Metric, + Expressions: v0alpha1.RecordingRuleExpressionMap{ + "A": { + QueryType: util.Pointer(rule.Data[0].QueryType), + DatasourceUID: util.Pointer(v0alpha1.RecordingRuleDatasourceUID(rule.Data[0].DatasourceUID)), + Model: rule.Data[0].Model, + Source: util.Pointer(true), + RelativeTimeRange: &v0alpha1.RecordingRuleRelativeTimeRange{ + From: v0alpha1.RecordingRulePromDurationWMillis("5m"), + To: v0alpha1.RecordingRulePromDurationWMillis("0s"), + }, + }, + }, + Trigger: v0alpha1.RecordingRuleIntervalTrigger{Interval: v0alpha1.RecordingRulePromDuration("10s")}, + }, + } + + created, err := client.Create(ctx, recordingRule, v1.CreateOptions{}) + require.NoError(t, err) + defer func() { _ = client.Delete(ctx, created.Name, v1.DeleteOptions{}) }() + + // On create, metadata.labels[v0alpha1.FolderLabelKey] should mirror annotation + require.Equal(t, "test-folder-a", created.Labels[v0alpha1.FolderLabelKey]) + + updated := created.Copy().(*v0alpha1.RecordingRule) + if updated.Annotations == nil { + updated.Annotations = map[string]string{} + } + updated.Annotations[v0alpha1.FolderAnnotationKey] = "test-folder-b" + + after, err := client.Update(ctx, updated, v1.UpdateOptions{}) + require.NoError(t, err) + require.Equal(t, "test-folder-b", after.Annotations[v0alpha1.FolderAnnotationKey]) + require.Equal(t, "test-folder-b", after.Labels[v0alpha1.FolderLabelKey]) + }) + + t.Run("should fail to create recording rule without folder annotation", func(t *testing.T) { + rule := baseGen.Generate() + recordingRule := &v0alpha1.RecordingRule{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: v0alpha1.RecordingRuleSpec{ + Title: rule.Title, + Metric: rule.Record.Metric, + Expressions: v0alpha1.RecordingRuleExpressionMap{ + "A": { + QueryType: util.Pointer(rule.Data[0].QueryType), + DatasourceUID: util.Pointer(v0alpha1.RecordingRuleDatasourceUID(rule.Data[0].DatasourceUID)), + Model: rule.Data[0].Model, + Source: util.Pointer(true), + RelativeTimeRange: &v0alpha1.RecordingRuleRelativeTimeRange{ + From: v0alpha1.RecordingRulePromDurationWMillis("5m"), + To: v0alpha1.RecordingRulePromDurationWMillis("0s"), + }, + }, + }, + Trigger: v0alpha1.RecordingRuleIntervalTrigger{Interval: v0alpha1.RecordingRulePromDuration("10s")}, + }, + } + + created, err := client.Create(ctx, recordingRule, v1.CreateOptions{}) + require.Error(t, err) + require.Nil(t, created) + }) + + t.Run("should fail to create rule with group labels preset", func(t *testing.T) { + rule := baseGen.Generate() + recordingRule := &v0alpha1.RecordingRule{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + Annotations: map[string]string{ + v0alpha1.FolderAnnotationKey: "test-folder-a", + }, + Labels: map[string]string{ + v0alpha1.GroupLabelKey: "some-group", + v0alpha1.GroupIndexLabelKey: "0", + }, + }, + Spec: v0alpha1.RecordingRuleSpec{ + Title: rule.Title, + Metric: rule.Record.Metric, + Expressions: v0alpha1.RecordingRuleExpressionMap{ + "A": { + QueryType: util.Pointer(rule.Data[0].QueryType), + DatasourceUID: util.Pointer(v0alpha1.RecordingRuleDatasourceUID(rule.Data[0].DatasourceUID)), + Model: rule.Data[0].Model, + Source: util.Pointer(true), + RelativeTimeRange: &v0alpha1.RecordingRuleRelativeTimeRange{ + From: v0alpha1.RecordingRulePromDurationWMillis("5m"), + To: v0alpha1.RecordingRulePromDurationWMillis("0s"), + }, + }, + }, + Trigger: v0alpha1.RecordingRuleIntervalTrigger{Interval: v0alpha1.RecordingRulePromDuration("10s")}, + }, + } + + created, err := client.Create(ctx, recordingRule, v1.CreateOptions{}) + require.Error(t, err) + require.Nil(t, created) + }) +}