Skip to content

Commit 99fd7b8

Browse files
yuri-tceretianbrendamuiralexweav
authored
Alerting: Update provisioning to validate user-defined UID on create (grafana#73793)
* add ValidateUID to util * provisioning to validate UID on rule creation --------- Co-authored-by: brendamuir <[email protected]> Co-authored-by: Alexander Weaver <[email protected]>
1 parent 9c50296 commit 99fd7b8

File tree

14 files changed

+161
-24
lines changed

14 files changed

+161
-24
lines changed

‎conf/provisioning/alerting/sample.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ apiVersion: 1
1313
# interval: 60s
1414
# # <list, required> list of rules that are part of the rule group
1515
# rules:
16-
# # <string, required> unique identifier for the rule
16+
# # <string, required> unique identifier for the rule. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
1717
# - uid: my_id_1
1818
# # <string, required> title of the rule, will be displayed in the UI
1919
# title: my_first_rule
@@ -82,7 +82,7 @@ apiVersion: 1
8282
# # <string, required> name of the contact point
8383
# name: cp_1
8484
# receivers:
85-
# # <string, required> unique identifier for the receiver
85+
# # <string, required> unique identifier for the receiver. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
8686
# - uid: first_uid
8787
# # <string, required> type of the receiver
8888
# type: prometheus-alertmanager

‎docs/sources/alerting/set-up/provision-alerting-resources/file-provisioning/index.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ groups:
6565
interval: 60s
6666
# <list, required> list of rules that are part of the rule group
6767
rules:
68-
# <string, required> unique identifier for the rule
68+
# <string, required> unique identifier for the rule. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
6969
- uid: my_id_1
7070
# <string, required> title of the rule that will be displayed in the UI
7171
title: my_first_rule
@@ -158,7 +158,7 @@ contactPoints:
158158
# <string, required> name of the contact point
159159
name: cp_1
160160
receivers:
161-
# <string, required> unique identifier for the receiver
161+
# <string, required> unique identifier for the receiver. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
162162
- uid: first_uid
163163
# <string, required> type of the receiver
164164
type: prometheus-alertmanager

‎pkg/services/ngalert/api/api_provisioning_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
secrets_fakes "github.com/grafana/grafana/pkg/services/secrets/fakes"
3434
"github.com/grafana/grafana/pkg/services/user"
3535
"github.com/grafana/grafana/pkg/setting"
36+
"github.com/grafana/grafana/pkg/util"
3637
"github.com/grafana/grafana/pkg/web"
3738
)
3839

@@ -283,7 +284,7 @@ func TestProvisioningApi(t *testing.T) {
283284

284285
t.Run("PUT sets expected fields with no provenance", func(t *testing.T) {
285286
sut := createProvisioningSrvSut(t)
286-
uid := t.Name()
287+
uid := util.GenerateShortUID()
287288
rule := createTestAlertRule("rule", 1)
288289
rule.UID = uid
289290
insertRuleInOrg(t, sut, rule, 3)

‎pkg/services/ngalert/api/tooling/api.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2663,6 +2663,9 @@
26632663
"type": "string"
26642664
},
26652665
"uid": {
2666+
"maxLength": 40,
2667+
"minLength": 1,
2668+
"pattern": "^[a-zA-Z0-9-_]+$",
26662669
"type": "string"
26672670
},
26682671
"updated": {

‎pkg/services/ngalert/api/tooling/definitions/provisioning_alert_rules.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,11 @@ type AlertRuleHeaders struct {
108108
type ProvisionedAlertRules []ProvisionedAlertRule
109109

110110
type ProvisionedAlertRule struct {
111-
ID int64 `json:"id"`
111+
ID int64 `json:"id"`
112+
// required: false
113+
// minLength: 1
114+
// maxLength: 40
115+
// pattern: ^[a-zA-Z0-9-_]+$
112116
UID string `json:"uid"`
113117
// required: true
114118
OrgID int64 `json:"orgID"`

‎pkg/services/ngalert/api/tooling/definitions/provisioning_contactpoints.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ type ContactPoints []EmbeddedContactPoint
8181
type EmbeddedContactPoint struct {
8282
// UID is the unique identifier of the contact point. The UID can be
8383
// set by the user.
84+
// required: false
85+
// minLength: 1
86+
// maxLength: 40
87+
// pattern: ^[a-zA-Z0-9\-\_]+$
8488
// example: my_external_reference
8589
UID string `json:"uid"`
8690
// Name is used as grouping key in the UI. Contact points with the

‎pkg/services/ngalert/api/tooling/post.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,9 @@
764764
"uid": {
765765
"description": "UID is the unique identifier of the contact point. The UID can be\nset by the user.",
766766
"example": "my_external_reference",
767+
"maxLength": 40,
768+
"minLength": 1,
769+
"pattern": "^[a-zA-Z0-9\\-\\_]+$",
767770
"type": "string"
768771
}
769772
},
@@ -2663,6 +2666,9 @@
26632666
"type": "string"
26642667
},
26652668
"uid": {
2669+
"maxLength": 40,
2670+
"minLength": 1,
2671+
"pattern": "^[a-zA-Z0-9-_]+$",
26662672
"type": "string"
26672673
},
26682674
"updated": {

‎pkg/services/ngalert/api/tooling/spec.json‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3625,6 +3625,9 @@
36253625
"uid": {
36263626
"description": "UID is the unique identifier of the contact point. The UID can be\nset by the user.",
36273627
"type": "string",
3628+
"maxLength": 40,
3629+
"minLength": 1,
3630+
"pattern": "^[a-zA-Z0-9\\-\\_]+$",
36283631
"example": "my_external_reference"
36293632
}
36303633
}
@@ -5534,7 +5537,10 @@
55345537
"example": "Always firing"
55355538
},
55365539
"uid": {
5537-
"type": "string"
5540+
"type": "string",
5541+
"maxLength": 40,
5542+
"minLength": 1,
5543+
"pattern": "^[a-zA-Z0-9-_]+$"
55385544
},
55395545
"updated": {
55405546
"type": "string",

‎pkg/services/ngalert/provisioning/alert_rules.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ func (service *AlertRuleService) GetAlertRuleWithFolderTitle(ctx context.Context
112112
func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule models.AlertRule, provenance models.Provenance, userID int64) (models.AlertRule, error) {
113113
if rule.UID == "" {
114114
rule.UID = util.GenerateShortUID()
115+
} else if err := util.ValidateUID(rule.UID); err != nil {
116+
return models.AlertRule{}, errors.Join(models.ErrAlertRuleFailedValidation, fmt.Errorf("cannot create rule with UID '%s': %w", rule.UID, err))
115117
}
116118
interval, err := service.ruleStore.GetRuleGroupInterval(ctx, rule.OrgID, rule.NamespaceUID, rule.RuleGroup)
117119
// if the alert group does not exists we just use the default interval

‎pkg/services/ngalert/provisioning/alert_rules_test.go‎

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@ import (
44
"context"
55
"encoding/json"
66
"strconv"
7+
"strings"
78
"testing"
89
"time"
910

10-
"github.com/grafana/grafana/pkg/expr"
1111
"github.com/stretchr/testify/require"
1212

13+
"github.com/grafana/grafana/pkg/expr"
14+
"github.com/grafana/grafana/pkg/util"
15+
1316
"github.com/grafana/grafana/pkg/infra/db"
1417
"github.com/grafana/grafana/pkg/infra/log"
1518
"github.com/grafana/grafana/pkg/services/ngalert/models"
@@ -21,21 +24,6 @@ func TestAlertRuleService(t *testing.T) {
2124
ruleService := createAlertRuleService(t)
2225
var orgID int64 = 1
2326

24-
t.Run("alert rule creation should return the created id", func(t *testing.T) {
25-
rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0)
26-
require.NoError(t, err)
27-
require.NotEqual(t, 0, rule.ID, "expected to get the created id and not the zero value")
28-
})
29-
30-
t.Run("alert rule creation should set the right provenance", func(t *testing.T) {
31-
rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#2", orgID), models.ProvenanceAPI, 0)
32-
require.NoError(t, err)
33-
34-
_, provenance, err := ruleService.GetAlertRule(context.Background(), orgID, rule.UID)
35-
require.NoError(t, err)
36-
require.Equal(t, models.ProvenanceAPI, provenance)
37-
})
38-
3927
t.Run("group creation should set the right provenance", func(t *testing.T) {
4028
group := createDummyGroup("group-test-1", orgID)
4129
err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
@@ -531,6 +519,45 @@ func TestAlertRuleService(t *testing.T) {
531519
})
532520
}
533521

522+
func TestCreateAlertRule(t *testing.T) {
523+
ruleService := createAlertRuleService(t)
524+
var orgID int64 = 1
525+
526+
t.Run("should return the created id", func(t *testing.T) {
527+
rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0)
528+
require.NoError(t, err)
529+
require.NotEqual(t, 0, rule.ID, "expected to get the created id and not the zero value")
530+
})
531+
532+
t.Run("should set the right provenance", func(t *testing.T) {
533+
rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#2", orgID), models.ProvenanceAPI, 0)
534+
require.NoError(t, err)
535+
536+
_, provenance, err := ruleService.GetAlertRule(context.Background(), orgID, rule.UID)
537+
require.NoError(t, err)
538+
require.Equal(t, models.ProvenanceAPI, provenance)
539+
})
540+
541+
t.Run("when UID is specified", func(t *testing.T) {
542+
t.Run("return error if it is not valid UID", func(t *testing.T) {
543+
rule := dummyRule("test#3", orgID)
544+
rule.UID = strings.Repeat("1", util.MaxUIDLength+1)
545+
rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
546+
require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
547+
})
548+
t.Run("should create a new rule with this UID", func(t *testing.T) {
549+
rule := dummyRule("test#3", orgID)
550+
uid := util.GenerateShortUID()
551+
rule.UID = uid
552+
created, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
553+
require.NoError(t, err)
554+
require.Equal(t, uid, created.UID)
555+
_, _, err = ruleService.GetAlertRule(context.Background(), orgID, uid)
556+
require.NoError(t, err)
557+
})
558+
})
559+
}
560+
534561
func createAlertRuleService(t *testing.T) AlertRuleService {
535562
t.Helper()
536563
sqlStore := db.InitTestDB(t)

0 commit comments

Comments
 (0)