Util: Fix panic when generating UIDs concurrently (#69476)

pkg/util/shortid_generator.go

@@ -3,6 +3,7 @@ package util
 import (
 	"math/rand"
 	"regexp"
+	"sync"
 	"time"
 
 	"github.com/google/uuid"
@@ -12,6 +13,11 @@ var uidrand = rand.New(rand.NewSource(time.Now().UnixNano()))
 var alphaRunes = []rune("abcdefghijklmnopqrstuvwxyz")
 var hexLetters = []rune("abcdef")
 
+// 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.
+var mtx sync.Mutex
+
 // Legacy UID pattern
 var validUIDPattern = regexp.MustCompile(`^[a-zA-Z0-9\-\_]*$`).MatchString
 
@@ -30,6 +36,8 @@ func IsShortUIDTooLong(uid string) bool {
 // it is guaranteed to have a character as the first letter
 // This UID will be a valid k8s name
 func GenerateShortUID() string {
+	mtx.Lock()
+	defer mtx.Unlock()
 	uid, err := uuid.NewRandom()
 	if err != nil {
 		// This should never happen... but this seems better than a panic

pkg/util/shortid_generator_test.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"sync"
 	"testing"
 
 	"github.com/google/uuid"
@@ -16,6 +17,25 @@ func TestAllowedCharMatchesUidPattern(t *testing.T) {
 	}
 }
 
+// Run with "go test -race -run ^TestThreadSafe$ github.com/grafana/grafana/pkg/util"
+func TestThreadSafe(t *testing.T) {
+	// This test was used to showcase the bug, unfortunately there is
+	// no way to enable the -race flag programmatically.
+	t.Skip()
+	// Use 1000 go routines to create 100 UIDs each at roughly the same time.
+	var wg sync.WaitGroup
+	for i := 0; i < 1000; i++ {
+		go func() {
+			for ii := 0; ii < 100; ii++ {
+				_ = GenerateShortUID()
+			}
+			wg.Done()
+		}()
+		wg.Add(1)
+	}
+	wg.Wait()
+}
+
 func TestRandomUIDs(t *testing.T) {
 	for i := 0; i < 100; i++ {
 		v := GenerateShortUID()