root
/
grafana
mirror of https://github.com/grafana/grafana.git
1
0
Fork 0
Code Issues Actions 142 Packages Projects Releases Wiki Activity

uses new US naming validation for kv store validation
CodeQL checks / Detect whether code changed (push) Has been cancelled Details
CodeQL checks / Analyze (actions) (push) Has been cancelled Details
CodeQL checks / Analyze (go) (push) Has been cancelled Details
CodeQL checks / Analyze (javascript) (push) Has been cancelled Details

This commit is contained in:
Owen Smallwood 2025-10-06 17:03:26 -06:00
parent 1bc485c0ca
commit 8d59f964c0
4 changed files with 247 additions and 271 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
pkg/storage/unified/resource/datastore.go
View File

@ -2,15 +2,16 @@ package resource
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io" "io"
"iter" "iter"
"math" "math"
"regexp"
"strconv" "strconv"
"strings" "strings"
"time" "time"
"github.com/grafana/grafana/pkg/apimachinery/validation"
gocache "github.com/patrickmn/go-cache" gocache "github.com/patrickmn/go-cache"
) )
@ -54,21 +55,6 @@ type GroupResource struct {
Resource string Resource string
} }
var (
// validNameRegex validates that a name contains only lowercase alphanumeric characters, '-' or '.'
// and starts and ends with an alphanumeric character
validNameRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$`)
// k8sRegex validates Kubernetes qualified name format
// must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character
// all future Grafana UIDs will need to conform to this
k8sRegex = regexp.MustCompile(`^[A-Za-z0-9][-A-Za-z0-9_.]*[A-Za-z0-9]$`)
// legacyNameRegex validates legacy UIDs: letters, numbers, dashes, underscores
// this matches the shortids that legacy Grafana used to generate uids
legacyNameRegex = regexp.MustCompile(`^[a-zA-Z0-9\-_]+$`)
)
func (k DataKey) String() string { func (k DataKey) String() string {
return fmt.Sprintf("%s/%s/%s/%s/%d~%s~%s", k.Group, k.Resource, k.Namespace, k.Name, k.ResourceVersion, k.Action, k.Folder) return fmt.Sprintf("%s/%s/%s/%s/%d~%s~%s", k.Group, k.Resource, k.Namespace, k.Name, k.ResourceVersion, k.Action, k.Folder)
} }
@ -78,42 +64,35 @@ func (k DataKey) Equals(other DataKey) bool {
} }
func (k DataKey) Validate() error { func (k DataKey) Validate() error {
if k.Group == "" {
return fmt.Errorf("group is required")
}
if k.Resource == "" {
return fmt.Errorf("resource is required")
}
if k.Namespace == "" { if k.Namespace == "" {
return fmt.Errorf("namespace is required") return NewValidationError("namespace", k.Namespace, ErrNamespaceRequired)
}
if k.Name == "" {
return fmt.Errorf("name is required")
} }
if k.ResourceVersion <= 0 { if k.ResourceVersion <= 0 {
return fmt.Errorf("resource version must be positive") return NewValidationError("resourceVersion", fmt.Sprintf("%d", k.ResourceVersion), ErrResourceVersionInvalid)
} }
if k.Action == "" { if k.Action == "" {
return fmt.Errorf("action is required") return NewValidationError("action", string(k.Action), ErrActionRequired)
} }
// Validate naming conventions for all required fields // Validate naming conventions for all required fields
if !validNameRegex.MatchString(k.Namespace) { if err := validation.IsValidNamespace(k.Namespace); err != nil {
return fmt.Errorf("namespace '%s' is invalid", k.Namespace) return NewValidationError("namespace", k.Namespace, err[0])
} }
if !validNameRegex.MatchString(k.Group) { if err := validation.IsValidGroup(k.Group); err != nil {
return fmt.Errorf("group '%s' is invalid", k.Group) return NewValidationError("group", k.Group, err[0])
} }
if !validNameRegex.MatchString(k.Resource) { if err := validation.IsValidateResource(k.Resource); err != nil {
return fmt.Errorf("resource '%s' is invalid", k.Resource) return NewValidationError("resource", k.Resource, err[0])
} }
if !k8sRegex.MatchString(k.Name) && !legacyNameRegex.MatchString(k.Name) { if err := validation.IsValidGrafanaName(k.Name); err != nil {
return fmt.Errorf("name '%s' is invalid, must match k8s qualified name format or Grafana shortid format", k.Name) return NewValidationError("name", k.Name, err[0])
} }
// Validate folder field if provided (optional field) // Validate folder field if provided (optional field)
if k.Folder != "" && !validNameRegex.MatchString(k.Folder) { if k.Folder != "" {
return fmt.Errorf("folder '%s' is invalid", k.Folder) if err := validation.IsValidGrafanaName(k.Folder); err != nil {
return NewValidationError("folder", k.Folder, err[0])
}
} }
// Validate action is one of the valid values // Validate action is one of the valid values
@ -133,27 +112,21 @@ type ListRequestKey struct {
} }
func (k ListRequestKey) Validate() error { func (k ListRequestKey) Validate() error {
if k.Group == "" {
return fmt.Errorf("group is required")
}
if k.Resource == "" {
return fmt.Errorf("resource is required")
}
if k.Namespace == "" && k.Name != "" { if k.Namespace == "" && k.Name != "" {
return fmt.Errorf("name must be empty when namespace is empty") return errors.New(ErrNameMustBeEmptyWhenNamespaceEmpty)
} }
if k.Namespace != "" && !validNameRegex.MatchString(k.Namespace) { if k.Namespace != "" {
return fmt.Errorf("namespace '%s' is invalid", k.Namespace) if err := validation.IsValidNamespace(k.Namespace); err != nil {
return NewValidationError("namespace", k.Namespace, err[0])
}
} }
if !validNameRegex.MatchString(k.Group) { if err := validation.IsValidGroup(k.Group); err != nil {
return fmt.Errorf("group '%s' is invalid", k.Group) return NewValidationError("group", k.Group, err[0])
} }
if !validNameRegex.MatchString(k.Resource) { if err := validation.IsValidateResource(k.Resource); err != nil {
return fmt.Errorf("resource '%s' is invalid", k.Resource) return NewValidationError("resource", k.Resource, err[0])
}
if k.Name != "" && !k8sRegex.MatchString(k.Name) && !legacyNameRegex.MatchString(k.Name) {
return fmt.Errorf("name '%s' is invalid", k.Name)
} }
return nil return nil
} }
@ -177,31 +150,20 @@ type GetRequestKey struct {
// Validate validates the get request key // Validate validates the get request key
func (k GetRequestKey) Validate() error { func (k GetRequestKey) Validate() error {
if k.Group == "" {
return fmt.Errorf("group is required")
}
if k.Resource == "" {
return fmt.Errorf("resource is required")
}
if k.Namespace == "" { if k.Namespace == "" {
return fmt.Errorf("namespace is required") return errors.New(ErrNamespaceRequired)
} }
if k.Name == "" { if err := validation.IsValidNamespace(k.Namespace); err != nil {
return fmt.Errorf("name is required") return NewValidationError("namespace", k.Namespace, err[0])
} }
if err := validation.IsValidGroup(k.Group); err != nil {
// Validate naming conventions return NewValidationError("group", k.Group, err[0])
if !validNameRegex.MatchString(k.Namespace) {
return fmt.Errorf("namespace '%s' is invalid", k.Namespace)
} }
if !validNameRegex.MatchString(k.Group) { if err := validation.IsValidateResource(k.Resource); err != nil {
return fmt.Errorf("group '%s' is invalid", k.Group) return NewValidationError("resource", k.Resource, err[0])
} }
if !validNameRegex.MatchString(k.Resource) { if err := validation.IsValidGrafanaName(k.Name); err != nil {
return fmt.Errorf("resource '%s' is invalid", k.Resource) return NewValidationError("name", k.Name, err[0])
}
if !validNameRegex.MatchString(k.Name) {
return fmt.Errorf("name '%s' is invalid", k.Name)
} }
return nil return nil

336
pkg/storage/unified/resource/datastore_test.go
View File

@ -3,6 +3,7 @@ package resource
import ( import (
"bytes" "bytes"
"context" "context"
"errors"
"fmt" "fmt"
"io" "io"
"testing" "testing"
@ -86,6 +87,7 @@ func TestDataKey_Validate(t *testing.T) {
key DataKey key DataKey
expectError bool expectError bool
errorMsg string errorMsg string
errorField string
}{ }{
{ {
name: "valid key with created action", name: "valid key with created action",
@ -99,6 +101,18 @@ func TestDataKey_Validate(t *testing.T) {
}, },
expectError: false, expectError: false,
}, },
{
name: "valid - underscore in namespace",
key: DataKey{
Namespace: "test_namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{ {
name: "valid key with updated action", name: "valid key with updated action",
key: DataKey{ key: DataKey{
@ -123,18 +137,6 @@ func TestDataKey_Validate(t *testing.T) {
}, },
expectError: false, expectError: false,
}, },
{
name: "valid key with dots and dashes",
key: DataKey{
Namespace: "test.namespace-with-dashes",
Group: "test.group-123",
Resource: "test-resource.v1",
Name: "test-name.with.dots",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{ {
name: "valid - name ends with dash", name: "valid - name ends with dash",
key: DataKey{ key: DataKey{
@ -148,11 +150,11 @@ func TestDataKey_Validate(t *testing.T) {
expectError: false, expectError: false,
}, },
{ {
name: "valid key with single character names", name: "valid key with minimum character lengths",
key: DataKey{ key: DataKey{
Namespace: "a", Namespace: "abc",
Group: "b", Group: "bcd",
Resource: "c", Resource: "cde",
Name: "d", Name: "d",
ResourceVersion: rv, ResourceVersion: rv,
Action: DataActionCreated, Action: DataActionCreated,
@ -183,6 +185,42 @@ func TestDataKey_Validate(t *testing.T) {
}, },
expectError: false, expectError: false,
}, },
{
name: "valid - uppercase in namespace",
key: DataKey{
Namespace: "Test-Namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - uppercase in group",
key: DataKey{
Namespace: "test-namespace",
Group: "Test-Group",
Resource: "test-resource",
Name: "test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - uppercase in resource",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "Test-Resource",
Name: "test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
// Invalid cases - empty fields // Invalid cases - empty fields
{ {
name: "invalid - empty namespace", name: "invalid - empty namespace",
@ -195,7 +233,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "namespace is required", errorMsg: ErrNamespaceRequired,
}, },
{ {
name: "invalid - empty group", name: "invalid - empty group",
@ -208,7 +246,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "group is required", errorField: "group",
}, },
{ {
name: "invalid - empty resource", name: "invalid - empty resource",
@ -221,7 +259,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "resource is required", errorField: "resource",
}, },
{ {
name: "invalid - empty name", name: "invalid - empty name",
@ -234,7 +272,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "name is required", errorField: "name",
}, },
{ {
name: "invalid - empty action", name: "invalid - empty action",
@ -247,7 +285,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: "", Action: "",
}, },
expectError: true, expectError: true,
errorMsg: "action is required", errorMsg: ErrActionRequired,
}, },
{ {
name: "invalid - all fields empty", name: "invalid - all fields empty",
@ -260,61 +298,21 @@ func TestDataKey_Validate(t *testing.T) {
Action: "", Action: "",
}, },
expectError: true, expectError: true,
errorMsg: "group is required", errorField: "namespace",
},
// Invalid cases - uppercase characters
{
name: "invalid - uppercase in namespace",
key: DataKey{
Namespace: "Test-Namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorMsg: "namespace 'Test-Namespace' is invalid",
},
{
name: "invalid - uppercase in group",
key: DataKey{
Namespace: "test-namespace",
Group: "Test-Group",
Resource: "test-resource",
Name: "test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorMsg: "group 'Test-Group' is invalid",
},
{
name: "invalid - uppercase in resource",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "Test-Resource",
Name: "test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorMsg: "resource 'Test-Resource' is invalid",
}, },
// Invalid cases - invalid characters // Invalid cases - invalid characters
{ {
name: "invalid - underscore in namespace", name: "invalid - key with dots and dashes",
key: DataKey{ key: DataKey{
Namespace: "test_namespace", Namespace: "test.namespace-with-dashes",
Group: "test-group", Group: "test.group-123",
Resource: "test-resource", Resource: "test-resource.v1",
Name: "test-name", Name: "test-name.with.dots",
ResourceVersion: rv, ResourceVersion: rv,
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "namespace 'test_namespace' is invalid", errorField: "namespace",
}, },
{ {
name: "invalid - space in group", name: "invalid - space in group",
@ -415,7 +413,6 @@ func TestDataKey_Validate(t *testing.T) {
}, },
expectError: false, expectError: false,
}, },
// Invalid name cases
{ {
name: "valid - name starts with dash (legacy format)", name: "valid - name starts with dash (legacy format)",
key: DataKey{ key: DataKey{
@ -441,7 +438,7 @@ func TestDataKey_Validate(t *testing.T) {
expectError: false, expectError: false,
}, },
{ {
name: "invalid - name starts with dot", name: "valid - name starts with dot",
key: DataKey{ key: DataKey{
Namespace: "test-namespace", Namespace: "test-namespace",
Group: "test-group", Group: "test-group",
@ -450,11 +447,10 @@ func TestDataKey_Validate(t *testing.T) {
ResourceVersion: rv, ResourceVersion: rv,
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: false,
errorMsg: "name '.test-name' is invalid, must match k8s qualified name format or Grafana shortid format",
}, },
{ {
name: "invalid - name ends with dot", name: "valid - name ends with dot",
key: DataKey{ key: DataKey{
Namespace: "test-namespace", Namespace: "test-namespace",
Group: "test-group", Group: "test-group",
@ -463,8 +459,7 @@ func TestDataKey_Validate(t *testing.T) {
ResourceVersion: rv, ResourceVersion: rv,
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: false,
errorMsg: "name 'test-name.' is invalid, must match k8s qualified name format or Grafana shortid format",
}, },
{ {
name: "valid - name starts with underscore (legacy format)", name: "valid - name starts with underscore (legacy format)",
@ -490,6 +485,7 @@ func TestDataKey_Validate(t *testing.T) {
}, },
expectError: false, expectError: false,
}, },
// Invalid name cases
{ {
name: "invalid - name with slash", name: "invalid - name with slash",
key: DataKey{ key: DataKey{
@ -501,7 +497,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "name 'test/name' is invalid, must match k8s qualified name format or Grafana shortid format", errorField: "name",
}, },
{ {
name: "invalid - name with spaces", name: "invalid - name with spaces",
@ -514,7 +510,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "name 'test name' is invalid, must match k8s qualified name format or Grafana shortid format", errorField: "name",
}, },
{ {
name: "invalid - name with special characters", name: "invalid - name with special characters",
@ -527,7 +523,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "name 'test@name#with$special' is invalid, must match k8s qualified name format or Grafana shortid format", errorField: "name",
}, },
{ {
name: "invalid - empty name", name: "invalid - empty name",
@ -540,7 +536,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated, Action: DataActionCreated,
}, },
expectError: true, expectError: true,
errorMsg: "name cannot be empty", errorField: "name",
}, },
// Invalid cases - start/end with invalid characters // Invalid cases - start/end with invalid characters
{ {
@ -606,6 +602,10 @@ func TestDataKey_Validate(t *testing.T) {
if tt.errorMsg != "" { if tt.errorMsg != "" {
require.Contains(t, err.Error(), tt.errorMsg) require.Contains(t, err.Error(), tt.errorMsg)
} }
var validationErr *ValidationError
if errors.Is(err, validationErr) && tt.errorField != "" {
require.Equal(t, tt.errorField, validationErr.Field)
}
} else { } else {
require.NoError(t, err) require.NoError(t, err)
} }
@ -1159,7 +1159,7 @@ func TestDataStore_ValidationEnforced(t *testing.T) {
// Create an invalid key // Create an invalid key
invalidKey := DataKey{ invalidKey := DataKey{
Namespace: "Invalid-Namespace", // uppercase is invalid Namespace: "Invalid-Namespace-$$$",
Group: "test-group", Group: "test-group",
Resource: "test-resource", Resource: "test-resource",
Name: "test-name", Name: "test-name",
@ -1173,21 +1173,27 @@ func TestDataStore_ValidationEnforced(t *testing.T) {
_, err := ds.Get(ctx, invalidKey) _, err := ds.Get(ctx, invalidKey)
require.Error(t, err) require.Error(t, err)
require.Contains(t, err.Error(), "invalid data key") require.Contains(t, err.Error(), "invalid data key")
require.Contains(t, err.Error(), "namespace 'Invalid-Namespace' is invalid") var validationErr ValidationError
require.True(t, errors.As(err, &validationErr))
require.Equal(t, "namespace", validationErr.Field)
}) })
t.Run("Save with invalid key returns validation error", func(t *testing.T) { t.Run("Save with invalid key returns validation error", func(t *testing.T) {
err := ds.Save(ctx, invalidKey, testValue) err := ds.Save(ctx, invalidKey, testValue)
require.Error(t, err) require.Error(t, err)
require.Contains(t, err.Error(), "invalid data key") require.Contains(t, err.Error(), "invalid data key")
require.Contains(t, err.Error(), "namespace 'Invalid-Namespace' is invalid") var validationErr ValidationError
require.True(t, errors.As(err, &validationErr))
require.Equal(t, "namespace", validationErr.Field)
}) })
t.Run("Delete with invalid key returns validation error", func(t *testing.T) { t.Run("Delete with invalid key returns validation error", func(t *testing.T) {
err := ds.Delete(ctx, invalidKey) err := ds.Delete(ctx, invalidKey)
require.Error(t, err) require.Error(t, err)
require.Contains(t, err.Error(), "invalid data key") require.Contains(t, err.Error(), "invalid data key")
require.Contains(t, err.Error(), "namespace 'Invalid-Namespace' is invalid") var validationErr ValidationError
require.True(t, errors.As(err, &validationErr))
require.Equal(t, "namespace", validationErr.Field)
}) })
// Test another type of invalid key // Test another type of invalid key
@ -1228,6 +1234,7 @@ func TestListRequestKey_Validate(t *testing.T) {
key ListRequestKey key ListRequestKey
expectError bool expectError bool
errorMsg string errorMsg string
errorField string
}{ }{
{ {
name: "valid - all fields provided", name: "valid - all fields provided",
@ -1239,6 +1246,45 @@ func TestListRequestKey_Validate(t *testing.T) {
}, },
expectError: false, expectError: false,
}, },
{
name: "valid - uppercase in namespace",
key: ListRequestKey{
Namespace: "Test-Namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
},
expectError: false,
},
{
name: "valid - uppercase in group and resource",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "Test-Group",
Resource: "test-resource",
Name: "test-name",
},
expectError: false,
},
{
name: "valid - uppercase in resource",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "Test-Resource",
},
expectError: false,
},
{
name: "valid - underscore in namespace",
key: ListRequestKey{
Namespace: "test_namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
},
expectError: false,
},
{ {
name: "valid - only group and resource", name: "valid - only group and resource",
key: ListRequestKey{ key: ListRequestKey{
@ -1260,7 +1306,7 @@ func TestListRequestKey_Validate(t *testing.T) {
name: "invalid - all empty", name: "invalid - all empty",
key: ListRequestKey{}, key: ListRequestKey{},
expectError: true, expectError: true,
errorMsg: "group is required", errorField: "namespace",
}, },
{ {
name: "valid - legacy grafana uid 1", name: "valid - legacy grafana uid 1",
@ -1309,7 +1355,7 @@ func TestListRequestKey_Validate(t *testing.T) {
Group: "test-group", Group: "test-group",
}, },
expectError: true, expectError: true,
errorMsg: "resource is required", errorField: "resource",
}, },
{ {
name: "invalid - name without namespace", name: "invalid - name without namespace",
@ -1319,7 +1365,7 @@ func TestListRequestKey_Validate(t *testing.T) {
Group: "test-group", Group: "test-group",
}, },
expectError: true, expectError: true,
errorMsg: "name must be empty when namespace is empty", errorMsg: ErrNameMustBeEmptyWhenNamespaceEmpty,
}, },
{ {
name: "invalid - name without group and resource", name: "invalid - name without group and resource",
@ -1328,52 +1374,9 @@ func TestListRequestKey_Validate(t *testing.T) {
Name: "test-name", Name: "test-name",
}, },
expectError: true, expectError: true,
errorMsg: "group is required", errorField: "group",
}, },
// Invalid naming cases // Invalid naming cases
{
name: "invalid - uppercase in namespace",
key: ListRequestKey{
Namespace: "Test-Namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
},
expectError: true,
errorMsg: "namespace 'Test-Namespace' is invalid",
},
{
name: "invalid - uppercase in group and resource",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "Test-Group",
Resource: "test-resource",
Name: "test-name",
},
expectError: true,
errorMsg: "group 'Test-Group' is invalid",
},
{
name: "invalid - uppercase in resource",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "Test-Resource",
},
expectError: true,
errorMsg: "resource 'Test-Resource' is invalid",
},
{
name: "invalid - underscore in namespace",
key: ListRequestKey{
Namespace: "test_namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
},
expectError: true,
errorMsg: "namespace 'test_namespace' is invalid",
},
{ {
name: "invalid - starts with dash", name: "invalid - starts with dash",
key: ListRequestKey{ key: ListRequestKey{
@ -2512,10 +2515,11 @@ func TestDataKey_SameResource(t *testing.T) {
func TestGetRequestKey_Validate(t *testing.T) { func TestGetRequestKey_Validate(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
key GetRequestKey key GetRequestKey
expectErr bool expectErr bool
wantError string wantError string
errorField string
}{ }{
{ {
name: "valid key", name: "valid key",
@ -2537,6 +2541,16 @@ func TestGetRequestKey_Validate(t *testing.T) {
}, },
expectErr: false, expectErr: false,
}, },
{
name: "valid grafana name - ends with dot",
key: GetRequestKey{
Group: "apps",
Resource: "resources",
Namespace: "default",
Name: ".123_hello",
},
expectErr: false,
},
{ {
name: "missing group", name: "missing group",
key: GetRequestKey{ key: GetRequestKey{
@ -2544,8 +2558,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Namespace: "default", Namespace: "default",
Name: "test-resource", Name: "test-resource",
}, },
expectErr: true, expectErr: true,
wantError: "group is required", errorField: "group",
}, },
{ {
name: "missing resource", name: "missing resource",
@ -2554,8 +2568,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Namespace: "default", Namespace: "default",
Name: "test-resource", Name: "test-resource",
}, },
expectErr: true, expectErr: true,
wantError: "resource is required", errorField: "resource",
}, },
{ {
name: "missing namespace", name: "missing namespace",
@ -2564,8 +2578,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Resource: "resources", Resource: "resources",
Name: "test-resource", Name: "test-resource",
}, },
expectErr: true, expectErr: true,
wantError: "namespace is required", errorField: "namespace",
}, },
{ {
name: "missing name", name: "missing name",
@ -2574,30 +2588,19 @@ func TestGetRequestKey_Validate(t *testing.T) {
Resource: "resources", Resource: "resources",
Namespace: "default", Namespace: "default",
}, },
expectErr: true, expectErr: true,
wantError: "name is required", errorField: "name",
}, },
{ {
name: "invalid namespace - uppercase", name: "invalid group - underscore at start",
key: GetRequestKey{ key: GetRequestKey{
Group: "apps", Group: "_apps_v1",
Resource: "resources",
Namespace: "Default",
Name: "test-resource",
},
expectErr: true,
wantError: "namespace 'Default' is invalid",
},
{
name: "invalid group - underscore",
key: GetRequestKey{
Group: "apps_v1",
Resource: "resources", Resource: "resources",
Namespace: "default", Namespace: "default",
Name: "test-resource", Name: "test-resource",
}, },
expectErr: true, expectErr: true,
wantError: "group 'apps_v1' is invalid", errorField: "group",
}, },
{ {
name: "invalid resource - starts with dash", name: "invalid resource - starts with dash",
@ -2607,19 +2610,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Namespace: "default", Namespace: "default",
Name: "test-resource", Name: "test-resource",
}, },
expectErr: true, expectErr: true,
wantError: "resource '-resources' is invalid", errorField: "resource",
},
{
name: "invalid name - ends with dot",
key: GetRequestKey{
Group: "apps",
Resource: "resources",
Namespace: "default",
Name: "test-resource.",
},
expectErr: true,
wantError: "name 'test-resource.' is invalid",
}, },
} }
@ -2631,6 +2623,10 @@ func TestGetRequestKey_Validate(t *testing.T) {
if tt.wantError != "" { if tt.wantError != "" {
require.Contains(t, err.Error(), tt.wantError) require.Contains(t, err.Error(), tt.wantError)
} }
var validationErr *ValidationError
if errors.Is(err, validationErr) && tt.errorField != "" {
require.Equal(t, tt.errorField, validationErr.Field)
}
} else { } else {
require.NoError(t, err) require.NoError(t, err)
} }

23
pkg/storage/unified/resource/errors.go
View File

@ -2,6 +2,7 @@ package resource
import ( import (
"errors" "errors"
"fmt"
"net/http" "net/http"
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
@ -199,3 +200,25 @@ func HandleQueueError[T any](err error, makeResp func(*resourcepb.ErrorResult) *
} }
return makeResp(AsErrorResult(err)), nil return makeResp(AsErrorResult(err)), nil
} }
var (
ErrNamespaceRequired = "namespace is required"
ErrResourceVersionInvalid = "resource version must be positive"
ErrActionRequired = "action is required"
ErrActionInvalid = "action is invalid: must be one of 'created', 'updated', or 'deleted'"
ErrNameMustBeEmptyWhenNamespaceEmpty = "name must be empty when namespace is empty"
)
type ValidationError struct {
Field string
Value string
Msg string
}
func (e ValidationError) Error() string {
return fmt.Sprintf("%s '%s' is invalid: %s", e.Field, e.Value, e.Msg)
}
func NewValidationError(field, value, msg string) error {
return ValidationError{Field: field, Value: value, Msg: msg}
}

49
pkg/storage/unified/resource/eventstore.go
View File

@ -3,6 +3,7 @@ package resource
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"iter" "iter"
"strconv" "strconv"
@ -10,6 +11,7 @@ import (
"time" "time"
"github.com/bwmarrin/snowflake" "github.com/bwmarrin/snowflake"
"github.com/grafana/grafana/pkg/apimachinery/validation"
) )
const ( const (
@ -37,46 +39,38 @@ func (k EventKey) String() string {
func (k EventKey) Validate() error { func (k EventKey) Validate() error {
if k.Namespace == "" { if k.Namespace == "" {
return fmt.Errorf("namespace cannot be empty") return NewValidationError("namespace", k.Namespace, ErrNamespaceRequired)
}
if k.Group == "" {
return fmt.Errorf("group cannot be empty")
}
if k.Resource == "" {
return fmt.Errorf("resource cannot be empty")
}
if k.Name == "" {
return fmt.Errorf("name cannot be empty")
} }
if k.ResourceVersion < 0 { if k.ResourceVersion < 0 {
return fmt.Errorf("resource version must be non-negative") return errors.New(ErrResourceVersionInvalid)
} }
if k.Action == "" { if k.Action == "" {
return fmt.Errorf("action cannot be empty") return NewValidationError("action", string(k.Action), ErrActionRequired)
} }
if k.Folder != "" && !validNameRegex.MatchString(k.Folder) {
return fmt.Errorf("folder '%s' is invalid", k.Folder) // Validate each field against the naming rules
// Validate naming conventions for all required fields
if err := validation.IsValidNamespace(k.Namespace); err != nil {
return NewValidationError("namespace", k.Namespace, err[0])
} }
// Validate each field against the naming rules (reusing the regex from datastore.go) if err := validation.IsValidGroup(k.Group); err != nil {
if !validNameRegex.MatchString(k.Namespace) { return NewValidationError("group", k.Group, err[0])
return fmt.Errorf("namespace '%s' is invalid", k.Namespace)
} }
if !validNameRegex.MatchString(k.Group) { if err := validation.IsValidateResource(k.Resource); err != nil {
return fmt.Errorf("group '%s' is invalid", k.Group) return NewValidationError("resource", k.Resource, err[0])
} }
if !validNameRegex.MatchString(k.Resource) { if err := validation.IsValidGrafanaName(k.Name); err != nil {
return fmt.Errorf("resource '%s' is invalid", k.Resource) return NewValidationError("name", k.Name, err[0])
} }
if !validNameRegex.MatchString(k.Name) { if k.Folder != "" {
return fmt.Errorf("name '%s' is invalid", k.Name) if err := validation.IsValidGrafanaName(k.Folder); err != nil {
} return NewValidationError("folder", k.Folder, err[0])
if k.Folder != "" && !validNameRegex.MatchString(k.Folder) { }
return fmt.Errorf("folder '%s' is invalid", k.Folder)
} }
switch k.Action { switch k.Action {
case DataActionCreated, DataActionUpdated, DataActionDeleted: case DataActionCreated, DataActionUpdated, DataActionDeleted:
default: default:
return fmt.Errorf("action '%s' is invalid: must be one of 'created', 'updated', or 'deleted'", k.Action) return NewValidationError("action", string(k.Action), ErrActionInvalid)
} }
return nil return nil
@ -148,6 +142,7 @@ func (n *eventStore) Save(ctx context.Context, event Event) error {
Name: event.Name, Name: event.Name,
ResourceVersion: event.ResourceVersion, ResourceVersion: event.ResourceVersion,
Action: event.Action, Action: event.Action,
//TODO why isnt folder part of the key?
} }
if err := eventKey.Validate(); err != nil { if err := eventKey.Validate(); err != nil {