Alerting: Update provisioning to validate user-defined UID on create (#73793)

* add ValidateUID to util
* provisioning to validate UID on rule creation

---------

Co-authored-by: brendamuir <100768211+brendamuir@users.noreply.github.com>
Co-authored-by: Alexander Weaver <weaver.alex.d@gmail.com>

conf/provisioning/alerting/sample.yaml

@@ -13,7 +13,7 @@ apiVersion: 1
 #     interval: 60s
 #     # <list, required> list of rules that are part of the rule group
 #     rules:
-#       # <string, required> unique identifier for the rule
+#       # <string, required> unique identifier for the rule. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
 #     - uid: my_id_1
 #       # <string, required> title of the rule, will be displayed in the UI
 #       title: my_first_rule
@@ -82,7 +82,7 @@ apiVersion: 1
 #     # <string, required> name of the contact point
 #     name: cp_1
 #     receivers:
-#       # <string, required> unique identifier for the receiver
+#       # <string, required> unique identifier for the receiver. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
 #     - uid: first_uid
 #       # <string, required> type of the receiver
 #       type: prometheus-alertmanager

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

@@ -65,7 +65,7 @@ groups:
     interval: 60s
     # <list, required> list of rules that are part of the rule group
     rules:
-      # <string, required> unique identifier for the rule
+      # <string, required> unique identifier for the rule. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
       - uid: my_id_1
         # <string, required> title of the rule that will be displayed in the UI
         title: my_first_rule
@@ -158,7 +158,7 @@ contactPoints:
     # <string, required> name of the contact point
     name: cp_1
     receivers:
-      # <string, required> unique identifier for the receiver
+      # <string, required> unique identifier for the receiver. Should not exceed 40 symbols. Only letters, numbers, - (hyphen), and _ (underscore) allowed.
       - uid: first_uid
         # <string, required> type of the receiver
         type: prometheus-alertmanager

pkg/services/ngalert/api/api_provisioning_test.go

@@ -33,6 +33,7 @@ import (
 	secrets_fakes "github.com/grafana/grafana/pkg/services/secrets/fakes"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
 )
 
@@ -283,7 +284,7 @@ func TestProvisioningApi(t *testing.T) {
 
 			t.Run("PUT sets expected fields with no provenance", func(t *testing.T) {
 				sut := createProvisioningSrvSut(t)
-				uid := t.Name()
+				uid := util.GenerateShortUID()
 				rule := createTestAlertRule("rule", 1)
 				rule.UID = uid
 				insertRuleInOrg(t, sut, rule, 3)

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

@@ -2663,6 +2663,9 @@
      "type": "string"
     },
     "uid": {
+     "maxLength": 40,
+     "minLength": 1,
+     "pattern": "^[a-zA-Z0-9-_]+$",
      "type": "string"
     },
     "updated": {

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

@@ -108,7 +108,11 @@ type AlertRuleHeaders struct {
 type ProvisionedAlertRules []ProvisionedAlertRule
 
 type ProvisionedAlertRule struct {
-	ID  int64  `json:"id"`
+	ID int64 `json:"id"`
+	// required: false
+	// minLength: 1
+	// maxLength: 40
+	// pattern: ^[a-zA-Z0-9-_]+$
 	UID string `json:"uid"`
 	// required: true
 	OrgID int64 `json:"orgID"`

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

@@ -81,6 +81,10 @@ type ContactPoints []EmbeddedContactPoint
 type EmbeddedContactPoint struct {
 	// UID is the unique identifier of the contact point. The UID can be
 	// set by the user.
+	// required: false
+	// minLength: 1
+	// maxLength: 40
+	// pattern: ^[a-zA-Z0-9\-\_]+$
 	// example: my_external_reference
 	UID string `json:"uid"`
 	// Name is used as grouping key in the UI. Contact points with the

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

@@ -764,6 +764,9 @@
     "uid": {
      "description": "UID is the unique identifier of the contact point. The UID can be\nset by the user.",
      "example": "my_external_reference",
+     "maxLength": 40,
+     "minLength": 1,
+     "pattern": "^[a-zA-Z0-9\\-\\_]+$",
      "type": "string"
     }
    },
@@ -2663,6 +2666,9 @@
      "type": "string"
     },
     "uid": {
+     "maxLength": 40,
+     "minLength": 1,
+     "pattern": "^[a-zA-Z0-9-_]+$",
      "type": "string"
     },
     "updated": {

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

@@ -3625,6 +3625,9 @@
         "uid": {
           "description": "UID is the unique identifier of the contact point. The UID can be\nset by the user.",
           "type": "string",
+          "maxLength": 40,
+          "minLength": 1,
+          "pattern": "^[a-zA-Z0-9\\-\\_]+$",
           "example": "my_external_reference"
         }
       }
@@ -5534,7 +5537,10 @@
           "example": "Always firing"
         },
         "uid": {
-          "type": "string"
+          "type": "string",
+          "maxLength": 40,
+          "minLength": 1,
+          "pattern": "^[a-zA-Z0-9-_]+$"
         },
         "updated": {
           "type": "string",

pkg/services/ngalert/provisioning/alert_rules.go

@@ -112,6 +112,8 @@ func (service *AlertRuleService) GetAlertRuleWithFolderTitle(ctx context.Context
 func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule models.AlertRule, provenance models.Provenance, userID int64) (models.AlertRule, error) {
 	if rule.UID == "" {
 		rule.UID = util.GenerateShortUID()
+	} else if err := util.ValidateUID(rule.UID); err != nil {
+		return models.AlertRule{}, errors.Join(models.ErrAlertRuleFailedValidation, fmt.Errorf("cannot create rule with UID '%s': %w", rule.UID, err))
 	}
 	interval, err := service.ruleStore.GetRuleGroupInterval(ctx, rule.OrgID, rule.NamespaceUID, rule.RuleGroup)
 	// if the alert group does not exists we just use the default interval

pkg/services/ngalert/provisioning/alert_rules_test.go

@@ -4,12 +4,15 @@ import (
 	"context"
 	"encoding/json"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
 
-	"github.com/grafana/grafana/pkg/expr"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/grafana/pkg/expr"
+	"github.com/grafana/grafana/pkg/util"
+
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
@@ -21,21 +24,6 @@ func TestAlertRuleService(t *testing.T) {
 	ruleService := createAlertRuleService(t)
 	var orgID int64 = 1
 
-	t.Run("alert rule creation should return the created id", func(t *testing.T) {
-		rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0)
-		require.NoError(t, err)
-		require.NotEqual(t, 0, rule.ID, "expected to get the created id and not the zero value")
-	})
-
-	t.Run("alert rule creation should set the right provenance", func(t *testing.T) {
-		rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#2", orgID), models.ProvenanceAPI, 0)
-		require.NoError(t, err)
-
-		_, provenance, err := ruleService.GetAlertRule(context.Background(), orgID, rule.UID)
-		require.NoError(t, err)
-		require.Equal(t, models.ProvenanceAPI, provenance)
-	})
-
 	t.Run("group creation should set the right provenance", func(t *testing.T) {
 		group := createDummyGroup("group-test-1", orgID)
 		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
@@ -531,6 +519,45 @@ func TestAlertRuleService(t *testing.T) {
 	})
 }
 
+func TestCreateAlertRule(t *testing.T) {
+	ruleService := createAlertRuleService(t)
+	var orgID int64 = 1
+
+	t.Run("should return the created id", func(t *testing.T) {
+		rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0)
+		require.NoError(t, err)
+		require.NotEqual(t, 0, rule.ID, "expected to get the created id and not the zero value")
+	})
+
+	t.Run("should set the right provenance", func(t *testing.T) {
+		rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#2", orgID), models.ProvenanceAPI, 0)
+		require.NoError(t, err)
+
+		_, provenance, err := ruleService.GetAlertRule(context.Background(), orgID, rule.UID)
+		require.NoError(t, err)
+		require.Equal(t, models.ProvenanceAPI, provenance)
+	})
+
+	t.Run("when UID is specified", func(t *testing.T) {
+		t.Run("return error if it is not valid UID", func(t *testing.T) {
+			rule := dummyRule("test#3", orgID)
+			rule.UID = strings.Repeat("1", util.MaxUIDLength+1)
+			rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+			require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
+		})
+		t.Run("should create a new rule with this UID", func(t *testing.T) {
+			rule := dummyRule("test#3", orgID)
+			uid := util.GenerateShortUID()
+			rule.UID = uid
+			created, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)
+			require.NoError(t, err)
+			require.Equal(t, uid, created.UID)
+			_, _, err = ruleService.GetAlertRule(context.Background(), orgID, uid)
+			require.NoError(t, err)
+		})
+	})
+}
+
 func createAlertRuleService(t *testing.T) AlertRuleService {
 	t.Helper()
 	sqlStore := db.InitTestDB(t)

pkg/services/ngalert/provisioning/contactpoints.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -190,6 +191,8 @@ func (ecp *ContactPointService) CreateContactPoint(ctx context.Context, orgID in
 
 	if contactPoint.UID == "" {
 		contactPoint.UID = util.GenerateShortUID()
+	} else if err := util.ValidateUID(contactPoint.UID); err != nil {
+		return apimodels.EmbeddedContactPoint{}, errors.Join(ErrValidation, fmt.Errorf("cannot create contact point with UID '%s': %w", contactPoint.UID, err))
 	}
 
 	jsonData, err := contactPoint.Settings.MarshalJSON()

pkg/services/ngalert/provisioning/contactpoints_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/prometheus/alertmanager/config"
@@ -23,6 +24,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/secrets/manager"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util"
 )
 
 func TestContactPointService(t *testing.T) {
@@ -84,6 +86,16 @@ func TestContactPointService(t *testing.T) {
 		require.Equal(t, customUID, cps[1].UID)
 	})
 
+	t.Run("it's not possible to use invalid UID", func(t *testing.T) {
+		customUID := strings.Repeat("1", util.MaxUIDLength+1)
+		sut := createContactPointServiceSut(t, secretsService)
+		newCp := createTestContactPoint()
+		newCp.UID = customUID
+
+		_, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI)
+		require.ErrorIs(t, err, ErrValidation)
+	})
+
 	t.Run("it's not possible to use the same uid twice", func(t *testing.T) {
 		customUID := "1337"
 		sut := createContactPointServiceSut(t, secretsService)

pkg/util/shortid_generator.go

@@ -1,6 +1,8 @@
 package util
 
 import (
+	"errors"
+	"fmt"
 	"math/rand"
 	"regexp"
 	"sync"
@@ -9,10 +11,18 @@ import (
 	"github.com/google/uuid"
 )
 
+const MaxUIDLength = 40
+
 var uidrand = rand.New(rand.NewSource(time.Now().UnixNano()))
 var alphaRunes = []rune("abcdefghijklmnopqrstuvwxyz")
 var hexLetters = []rune("abcdef")
 
+var (
+	ErrUIDTooLong       = fmt.Errorf("UID is longer than %d symbols", MaxUIDLength)
+	ErrUIDFormatInvalid = errors.New("invalid format of UID. Only letters, numbers, '-' and '_' are allowed")
+	ErrUIDEmpty         = fmt.Errorf("UID is empty")
+)
+
 // We want to protect our number generator as they are not thread safe. Not using
 // the mutex could result in panics in certain cases where UIDs would be generated
 // at the same time.
@@ -29,7 +39,7 @@ func IsValidShortUID(uid string) bool {
 
 // IsShortUIDTooLong checks if short unique identifier is too long
 func IsShortUIDTooLong(uid string) bool {
-	return len(uid) > 40
+	return len(uid) > MaxUIDLength
 }
 
 // GenerateShortUID will generate a UUID that can also be a k8s name
@@ -51,3 +61,17 @@ func GenerateShortUID() string {
 	}
 	return uuid
 }
+
+// ValidateUID checks the format and length of the string and returns error if it does not pass the condition
+func ValidateUID(uid string) error {
+	if len(uid) == 0 {
+		return ErrUIDEmpty
+	}
+	if IsShortUIDTooLong(uid) {
+		return ErrUIDTooLong
+	}
+	if !IsValidShortUID(uid) {
+		return ErrUIDFormatInvalid
+	}
+	return nil
+}

pkg/util/shortid_generator_test.go

@@ -4,6 +4,7 @@ import (
 	"sync"
 	"testing"
 
+	"cuelang.org/go/pkg/strings"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -84,3 +85,47 @@ func TestIsShortUIDTooLong(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateUID(t *testing.T) {
+	var tests = []struct {
+		name     string
+		uid      string
+		expected error
+	}{
+		{
+			name:     "no error when string is of correct length",
+			uid:      "f8cc010c-ee72-4681-89d2-d46e1bd47d33",
+			expected: nil,
+		},
+		{
+			name:     "error when string is empty",
+			uid:      "",
+			expected: ErrUIDEmpty,
+		},
+		{
+			name:     "error when string is too long",
+			uid:      strings.Repeat("1", MaxUIDLength+1),
+			expected: ErrUIDTooLong,
+		},
+		{
+			name:     "error when string has invalid characters",
+			uid:      "f8cc010c.ee72.4681;89d2+d46e1bd47d33",
+			expected: ErrUIDFormatInvalid,
+		},
+		{
+			name:     "error when string has only whitespaces",
+			uid:      " ",
+			expected: ErrUIDFormatInvalid,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateUID(tt.uid)
+			if tt.expected == nil {
+				require.NoError(t, err)
+			} else {
+				require.ErrorIs(t, err, tt.expected)
+			}
+		})
+	}
+}