Chore: Replace short UID generation with more standard UUIDs (#62731)

This commit is contained in:
Ryan McKinley 2023-02-06 17:44:37 -08:00 committed by GitHub
parent ee2e294b4e
commit b1e58eb47e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 64 additions and 84 deletions

4
go.sum
View File

@ -1248,8 +1248,6 @@ github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad
github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/grafana/alerting v0.0.0-20230125210216-facc6b27b9e0 h1:BzkQNnj+eevX30EMqJiUS1w3CPoGc8kp7pDf/ari/4Y=
github.com/grafana/alerting v0.0.0-20230125210216-facc6b27b9e0/go.mod h1:NoSLbfmUwE+omWFReFrLtbtOItmvTbuQERJ6XFYp9ME=
github.com/grafana/alerting v0.0.0-20230203015918-0e4e2675d7aa h1:ue2fctL9LHJWYw9V+R1O/uWLNTfAr/KU1EUFRsqWlK4=
github.com/grafana/alerting v0.0.0-20230203015918-0e4e2675d7aa/go.mod h1:NoSLbfmUwE+omWFReFrLtbtOItmvTbuQERJ6XFYp9ME=
github.com/grafana/codejen v0.0.3 h1:tAWxoTUuhgmEqxJPOLtJoxlPBbMULFwKFOcRsPRPXDw=
@ -1268,8 +1266,6 @@ github.com/grafana/grafana-google-sdk-go v0.0.0-20211104130251-b190293eaf58 h1:2
github.com/grafana/grafana-google-sdk-go v0.0.0-20211104130251-b190293eaf58/go.mod h1:Vo2TKWfDVmNTELBUM+3lkrZvFtBws0qSZdXhQxRdJrE=
github.com/grafana/grafana-plugin-sdk-go v0.94.0/go.mod h1:3VXz4nCv6wH5SfgB3mlW39s+c+LetqSCjFj7xxPC5+M=
github.com/grafana/grafana-plugin-sdk-go v0.114.0/go.mod h1:D7x3ah+1d4phNXpbnOaxa/osSaZlwh9/ZUnGGzegRbk=
github.com/grafana/grafana-plugin-sdk-go v0.147.0 h1:VavvJOa/Ubs+wzalzWIl+FQmdaD4vEK8KVYU0a8rf+E=
github.com/grafana/grafana-plugin-sdk-go v0.147.0/go.mod h1:NMgO3t2gR5wyLx8bWZ9CTmpDk5Txp4wYFccFLHdYn3Q=
github.com/grafana/grafana-plugin-sdk-go v0.148.0 h1:M8v6L9agAFMlZMnak1yInII+aVF5FjZ1Qv4Q+GANyk4=
github.com/grafana/grafana-plugin-sdk-go v0.148.0/go.mod h1:NMgO3t2gR5wyLx8bWZ9CTmpDk5Txp4wYFccFLHdYn3Q=
github.com/grafana/phlare/api v0.1.2 h1:1jrwd3KnsXMzj/tJih9likx5EvbY3pbvLbDqAAYem30=

View File

@ -682,7 +682,6 @@ func TestDashboardAPIEndpoint(t *testing.T) {
{SaveError: dashboards.ErrDashboardTitleEmpty, ExpectedStatusCode: 400},
{SaveError: dashboards.ErrDashboardFolderCannotHaveParent, ExpectedStatusCode: 400},
{SaveError: alerting.ValidationError{Reason: "Mu"}, ExpectedStatusCode: 422},
{SaveError: dashboards.ErrDashboardFailedGenerateUniqueUid, ExpectedStatusCode: 500},
{SaveError: dashboards.ErrDashboardTypeMismatch, ExpectedStatusCode: 400},
{SaveError: dashboards.ErrDashboardFolderWithSameNameAsDashboard, ExpectedStatusCode: 400},
{SaveError: dashboards.ErrDashboardWithSameNameAsFolder, ExpectedStatusCode: 400},

View File

@ -71,7 +71,6 @@ func TestFoldersAPIEndpoint(t *testing.T) {
{Error: dashboards.ErrFolderAccessDenied, ExpectedStatusCode: 403},
{Error: dashboards.ErrFolderNotFound, ExpectedStatusCode: 404},
{Error: dashboards.ErrFolderVersionMismatch, ExpectedStatusCode: 412},
{Error: dashboards.ErrFolderFailedGenerateUniqueUid, ExpectedStatusCode: 500},
}
cmd := folder.CreateFolderCommand{
@ -124,7 +123,6 @@ func TestFoldersAPIEndpoint(t *testing.T) {
{Error: dashboards.ErrFolderAccessDenied, ExpectedStatusCode: 403},
{Error: dashboards.ErrFolderNotFound, ExpectedStatusCode: 404},
{Error: dashboards.ErrFolderVersionMismatch, ExpectedStatusCode: 412},
{Error: dashboards.ErrFolderFailedGenerateUniqueUid, ExpectedStatusCode: 500},
}
title := "Folder upd"

View File

@ -453,11 +453,7 @@ func saveDashboard(sess *db.Session, cmd *dashboards.SaveDashboardCommand, emitE
}
if dash.UID == "" {
uid, err := generateNewDashboardUid(sess, dash.OrgID)
if err != nil {
return nil, err
}
dash.SetUID(uid)
dash.SetUID(util.GenerateShortUID())
}
parentVersion := dash.Version
@ -536,23 +532,6 @@ func saveDashboard(sess *db.Session, cmd *dashboards.SaveDashboardCommand, emitE
return dash, nil
}
func generateNewDashboardUid(sess *db.Session, orgId int64) (string, error) {
for i := 0; i < 3; i++ {
uid := util.GenerateShortUID()
exists, err := sess.Where("org_id=? AND uid=?", orgId, uid).Get(&dashboards.Dashboard{})
if err != nil {
return "", err
}
if !exists {
return uid, nil
}
}
return "", dashboards.ErrDashboardFailedGenerateUniqueUid
}
func saveProvisionedData(sess *db.Session, provisioning *dashboards.DashboardProvisioning, dashboard *dashboards.Dashboard) error {
result := &dashboards.DashboardProvisioning{}

View File

@ -54,10 +54,6 @@ var (
Reason: "Multiple dashboards with the same slug exists",
StatusCode: 412,
}
ErrDashboardFailedGenerateUniqueUid = DashboardErr{
Reason: "Failed to generate unique dashboard id",
StatusCode: 500,
}
ErrDashboardTypeMismatch = DashboardErr{
Reason: "Dashboard cannot be changed to a folder",
StatusCode: 400,
@ -126,15 +122,14 @@ var (
Status: "not-found",
}
ErrFolderNotFound = errors.New("folder not found")
ErrFolderVersionMismatch = errors.New("the folder has been changed by someone else")
ErrFolderTitleEmpty = errors.New("folder title cannot be empty")
ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists")
ErrFolderInvalidUID = errors.New("invalid uid for folder provided")
ErrFolderSameNameExists = errors.New("a folder or dashboard in the general folder with the same name already exists")
ErrFolderFailedGenerateUniqueUid = errors.New("failed to generate unique folder ID")
ErrFolderAccessDenied = errors.New("access denied to folder")
ErrFolderContainsAlertRules = errors.New("folder contains alert rules")
ErrFolderNotFound = errors.New("folder not found")
ErrFolderVersionMismatch = errors.New("the folder has been changed by someone else")
ErrFolderTitleEmpty = errors.New("folder title cannot be empty")
ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists")
ErrFolderInvalidUID = errors.New("invalid uid for folder provided")
ErrFolderSameNameExists = errors.New("a folder or dashboard in the general folder with the same name already exists")
ErrFolderAccessDenied = errors.New("access denied to folder")
ErrFolderContainsAlertRules = errors.New("folder contains alert rules")
)
// DashboardErr represents a dashboard error.

View File

@ -805,9 +805,5 @@ func toFolderError(err error) error {
return dashboards.ErrFolderNotFound
}
if errors.Is(err, dashboards.ErrDashboardFailedGenerateUniqueUid) {
err = dashboards.ErrFolderFailedGenerateUniqueUid
}
return err
}

View File

@ -300,7 +300,6 @@ func TestIntegrationFolderService(t *testing.T) {
{ActualError: dashboards.ErrDashboardWithSameUIDExists, ExpectedError: dashboards.ErrFolderWithSameUIDExists},
{ActualError: dashboards.ErrDashboardVersionMismatch, ExpectedError: dashboards.ErrFolderVersionMismatch},
{ActualError: dashboards.ErrDashboardNotFound, ExpectedError: dashboards.ErrFolderNotFound},
{ActualError: dashboards.ErrDashboardFailedGenerateUniqueUid, ExpectedError: dashboards.ErrFolderFailedGenerateUniqueUid},
{ActualError: dashboards.ErrDashboardInvalidUid, ExpectedError: dashboards.ErrDashboardInvalidUid},
}

View File

@ -9,7 +9,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/shorturls"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
"github.com/teris-io/shortid"
)
var getTime = time.Now
@ -44,10 +44,15 @@ func (s ShortURLService) CreateShortURL(ctx context.Context, user *user.SignedIn
return nil, shorturls.ErrShortURLInvalidPath.Errorf("path cannot contain '../': %s", relPath)
}
uid, err := shortid.Generate()
if err != nil {
return nil, shorturls.ErrShortURLInternal.Errorf("failed to generate uid: %w", err)
}
now := time.Now().Unix()
shortURL := shorturls.ShortUrl{
OrgId: user.OrgID,
Uid: util.GenerateShortUID(),
Uid: uid,
Path: relPath,
CreatedBy: user.UserID,
CreatedAt: now,

View File

@ -101,12 +101,7 @@ func (m *folderHelper) createFolder(orgID int64, title string) (*dashboard, erro
}),
}
dash := cmd.getDashboardModel()
uid, err := m.generateNewDashboardUid(dash.OrgId)
if err != nil {
return nil, err
}
dash.setUid(uid)
dash.setUid(util.GenerateShortUID())
parentVersion := dash.Version
dash.setVersion(1)
@ -116,7 +111,7 @@ func (m *folderHelper) createFolder(orgID int64, title string) (*dashboard, erro
dash.UpdatedBy = FOLDER_CREATED_BY
metrics.MApiDashboardInsert.Inc()
if _, err = m.sess.Insert(dash); err != nil {
if _, err := m.sess.Insert(dash); err != nil {
return nil, err
}
@ -138,23 +133,6 @@ func (m *folderHelper) createFolder(orgID int64, title string) (*dashboard, erro
return dash, nil
}
func (m *folderHelper) generateNewDashboardUid(orgId int64) (string, error) {
for i := 0; i < 3; i++ {
uid := util.GenerateShortUID()
exists, err := m.sess.Where("org_id=? AND uid=?", orgId, uid).Get(&dashboards.Dashboard{})
if err != nil {
return "", err
}
if !exists {
return uid, nil
}
}
return "", dashboards.ErrDashboardFailedGenerateUniqueUid
}
// based on SQLStore.UpdateDashboardACL()
// it should be called from inside a transaction
func (m *folderHelper) setACL(orgID int64, dashboardID int64, items []*dashboardACL) error {

View File

@ -1,21 +1,22 @@
package util
import (
"math/rand"
"regexp"
"time"
"github.com/teris-io/shortid"
"github.com/google/uuid"
)
var allowedChars = shortid.DefaultABC
var uidrand = rand.New(rand.NewSource(time.Now().UnixNano()))
var alphaRunes = []rune("abcdefghijklmnopqrstuvwxyz")
var hexLetters = []rune("abcdef")
// Legacy UID pattern
var validUIDPattern = regexp.MustCompile(`^[a-zA-Z0-9\-\_]*$`).MatchString
func init() {
gen, _ := shortid.New(1, allowedChars, 1)
shortid.SetDefault(gen)
}
// IsValidShortUID checks if short unique identifier contains valid characters
// NOTE: future Grafana UIDs will need conform to https://github.com/kubernetes/apimachinery/blob/master/pkg/util/validation/validation.go#L43
func IsValidShortUID(uid string) bool {
return validUIDPattern(uid)
}
@ -25,7 +26,20 @@ func IsShortUIDTooLong(uid string) bool {
return len(uid) > 40
}
// GenerateShortUID generates a short unique identifier.
// GenerateShortUID will generate a UUID that can also be a k8s name
// it is guaranteed to have a character as the first letter
// This UID will be a valid k8s name
func GenerateShortUID() string {
return shortid.MustGenerate()
uid, err := uuid.NewRandom()
if err != nil {
// This should never happen... but this seems better than a panic
for i := range uid {
uid[i] = byte(uidrand.Intn(255))
}
}
uuid := uid.String()
if rune(uuid[0]) < rune('a') {
return string(hexLetters[uidrand.Intn(len(hexLetters))]) + uuid[1:]
}
return uuid
}

View File

@ -3,17 +3,38 @@ package util
import (
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/util/validation"
)
func TestAllowedCharMatchesUidPattern(t *testing.T) {
for _, c := range allowedChars {
for _, c := range alphaRunes {
if !IsValidShortUID(string(c)) {
t.Fatalf("charset for creating new shortids contains chars not present in uid pattern")
}
}
}
func TestRandomUIDs(t *testing.T) {
for i := 0; i < 100; i++ {
v := GenerateShortUID()
if !IsValidShortUID(v) {
t.Fatalf("charset for creating new shortids contains chars not present in uid pattern")
}
validation := validation.IsQualifiedName(v)
if validation != nil {
t.Fatalf("created invalid name: %v", validation)
}
_, err := uuid.Parse(v)
require.NoError(t, err)
//fmt.Println(v)
}
// t.FailNow()
}
func TestIsShortUIDTooLong(t *testing.T) {
var tests = []struct {
name string
@ -22,7 +43,7 @@ func TestIsShortUIDTooLong(t *testing.T) {
}{
{
name: "when the length of uid is longer than 40 chars then IsShortUIDTooLong should return true",
uid: allowedChars,
uid: string(alphaRunes) + string(alphaRunes),
expected: true,
},
{