Unified Storage: KVStore use new validation (#110834)

* name field must match either k8s regex or grafana legacy uid regex. Adds tests.

* moves invalid test to being valid

* uses new US naming validation for kv store validation

* fix function name and update key regex

* fix comment

* use correct errs var

* updates kv key tests
This commit is contained in:
owensmallwood
2025-10-09 11:25:39 -06:00
committed by GitHub
parent 4d3c5d1550
commit 1f7f3c9a5a
10 changed files with 502 additions and 291 deletions
+1 -1
View File
@@ -90,7 +90,7 @@ func IsValidGroup(group string) []string {
// If the value is not valid, a list of error strings is returned.
// Otherwise an empty list (or nil) is returned.
func IsValidateResource(resource string) []string {
func IsValidResource(resource string) []string {
s := len(resource)
switch {
case s > maxResourceLength:
@@ -222,7 +222,7 @@ func TestValidation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for _, input := range tt.input {
output := validation.IsValidateResource(input)
output := validation.IsValidResource(input)
require.Equal(t, tt.expect, output, "input: %s", input)
}
})
+36 -65
View File
@@ -2,15 +2,16 @@ package resource
import (
"context"
"errors"
"fmt"
"io"
"iter"
"math"
"regexp"
"strconv"
"strings"
"time"
"github.com/grafana/grafana/pkg/apimachinery/validation"
gocache "github.com/patrickmn/go-cache"
)
@@ -54,12 +55,6 @@ type GroupResource struct {
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])?$`)
)
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)
}
@@ -69,42 +64,35 @@ func (k DataKey) Equals(other DataKey) bool {
}
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 == "" {
return fmt.Errorf("namespace is required")
}
if k.Name == "" {
return fmt.Errorf("name is required")
return NewValidationError("namespace", k.Namespace, ErrNamespaceRequired)
}
if k.ResourceVersion <= 0 {
return fmt.Errorf("resource version must be positive")
return NewValidationError("resourceVersion", fmt.Sprintf("%d", k.ResourceVersion), ErrResourceVersionInvalid)
}
if k.Action == "" {
return fmt.Errorf("action is required")
return NewValidationError("action", string(k.Action), ErrActionRequired)
}
// Validate naming conventions for all required fields
if !validNameRegex.MatchString(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) {
return fmt.Errorf("group '%s' is invalid", k.Group)
if err := validation.IsValidGroup(k.Group); err != nil {
return NewValidationError("group", k.Group, err[0])
}
if !validNameRegex.MatchString(k.Resource) {
return fmt.Errorf("resource '%s' is invalid", k.Resource)
if err := validation.IsValidResource(k.Resource); err != nil {
return NewValidationError("resource", k.Resource, err[0])
}
if !validNameRegex.MatchString(k.Name) {
return fmt.Errorf("name '%s' is invalid", k.Name)
if err := validation.IsValidGrafanaName(k.Name); err != nil {
return NewValidationError("name", k.Name, err[0])
}
// Validate folder field if provided (optional field)
if k.Folder != "" && !validNameRegex.MatchString(k.Folder) {
return fmt.Errorf("folder '%s' is invalid", k.Folder)
if 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
@@ -124,27 +112,21 @@ type ListRequestKey struct {
}
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 != "" {
return fmt.Errorf("name must be empty when namespace is empty")
return errors.New(ErrNameMustBeEmptyWhenNamespaceEmpty)
}
if k.Namespace != "" && !validNameRegex.MatchString(k.Namespace) {
return fmt.Errorf("namespace '%s' is invalid", k.Namespace)
if k.Namespace != "" {
if err := validation.IsValidNamespace(k.Namespace); err != nil {
return NewValidationError("namespace", k.Namespace, err[0])
}
}
if !validNameRegex.MatchString(k.Group) {
return fmt.Errorf("group '%s' is invalid", k.Group)
if err := validation.IsValidGroup(k.Group); err != nil {
return NewValidationError("group", k.Group, err[0])
}
if !validNameRegex.MatchString(k.Resource) {
return fmt.Errorf("resource '%s' is invalid", k.Resource)
}
if k.Name != "" && !validNameRegex.MatchString(k.Name) {
return fmt.Errorf("name '%s' is invalid", k.Name)
if err := validation.IsValidResource(k.Resource); err != nil {
return NewValidationError("resource", k.Resource, err[0])
}
return nil
}
@@ -168,31 +150,20 @@ type GetRequestKey struct {
// Validate validates the get request key
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 == "" {
return fmt.Errorf("namespace is required")
return errors.New(ErrNamespaceRequired)
}
if k.Name == "" {
return fmt.Errorf("name is required")
if err := validation.IsValidNamespace(k.Namespace); err != nil {
return NewValidationError("namespace", k.Namespace, err[0])
}
// Validate naming conventions
if !validNameRegex.MatchString(k.Namespace) {
return fmt.Errorf("namespace '%s' is invalid", k.Namespace)
if err := validation.IsValidGroup(k.Group); err != nil {
return NewValidationError("group", k.Group, err[0])
}
if !validNameRegex.MatchString(k.Group) {
return fmt.Errorf("group '%s' is invalid", k.Group)
if err := validation.IsValidResource(k.Resource); err != nil {
return NewValidationError("resource", k.Resource, err[0])
}
if !validNameRegex.MatchString(k.Resource) {
return fmt.Errorf("resource '%s' is invalid", k.Resource)
}
if !validNameRegex.MatchString(k.Name) {
return fmt.Errorf("name '%s' is invalid", k.Name)
if err := validation.IsValidGrafanaName(k.Name); err != nil {
return NewValidationError("name", k.Name, err[0])
}
return nil
+411 -191
View File
@@ -3,6 +3,7 @@ package resource
import (
"bytes"
"context"
"errors"
"fmt"
"io"
"testing"
@@ -86,6 +87,7 @@ func TestDataKey_Validate(t *testing.T) {
key DataKey
expectError bool
errorMsg string
errorField string
}{
{
name: "valid key with created action",
@@ -99,6 +101,18 @@ func TestDataKey_Validate(t *testing.T) {
},
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",
key: DataKey{
@@ -124,23 +138,23 @@ func TestDataKey_Validate(t *testing.T) {
expectError: false,
},
{
name: "valid key with dots and dashes",
name: "valid - name ends with dash",
key: DataKey{
Namespace: "test.namespace-with-dashes",
Group: "test.group-123",
Resource: "test-resource.v1",
Name: "test-name.with.dots",
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name-",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid key with single character names",
name: "valid key with minimum character lengths",
key: DataKey{
Namespace: "a",
Group: "b",
Resource: "c",
Namespace: "abc",
Group: "bcd",
Resource: "cde",
Name: "d",
ResourceVersion: rv,
Action: DataActionCreated,
@@ -159,6 +173,54 @@ func TestDataKey_Validate(t *testing.T) {
},
expectError: false,
},
{
name: "valid - uppercase in name",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "Test-Name",
ResourceVersion: rv,
Action: DataActionCreated,
},
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
{
name: "invalid - empty namespace",
@@ -171,7 +233,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated,
},
expectError: true,
errorMsg: "namespace is required",
errorMsg: ErrNamespaceRequired,
},
{
name: "invalid - empty group",
@@ -184,7 +246,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated,
},
expectError: true,
errorMsg: "group is required",
errorField: "group",
},
{
name: "invalid - empty resource",
@@ -197,7 +259,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated,
},
expectError: true,
errorMsg: "resource is required",
errorField: "resource",
},
{
name: "invalid - empty name",
@@ -210,7 +272,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated,
},
expectError: true,
errorMsg: "name is required",
errorField: "name",
},
{
name: "invalid - empty action",
@@ -223,7 +285,7 @@ func TestDataKey_Validate(t *testing.T) {
Action: "",
},
expectError: true,
errorMsg: "action is required",
errorMsg: ErrActionRequired,
},
{
name: "invalid - all fields empty",
@@ -236,74 +298,21 @@ func TestDataKey_Validate(t *testing.T) {
Action: "",
},
expectError: true,
errorMsg: "group is required",
},
// 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",
},
{
name: "invalid - uppercase in name",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "Test-Name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorMsg: "name 'Test-Name' is invalid",
errorField: "namespace",
},
// Invalid cases - invalid characters
{
name: "invalid - underscore in namespace",
name: "invalid - key with dots and dashes",
key: DataKey{
Namespace: "test_namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
Namespace: "test.namespace-with-dashes",
Group: "test.group-123",
Resource: "test-resource.v1",
Name: "test-name.with.dots",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorMsg: "namespace 'test_namespace' is invalid",
errorField: "namespace",
},
{
name: "invalid - space in group",
@@ -331,8 +340,154 @@ func TestDataKey_Validate(t *testing.T) {
expectError: true,
errorMsg: "resource 'test@resource' is invalid",
},
// Name validation tests - K8s qualified name format
{
name: "invalid - slash in name",
name: "valid - K8s format with underscores",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test_name_with_underscores",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - K8s format with dots",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test.name.with.dots",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - K8s format mixed case",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "TestName123",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - Legacy Grafana shortid format",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "a1B2c3D4e5F6g7H8",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - Legacy format with dashes and underscores",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name_with-mixed_chars123",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - Single character name",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "a",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - name starts with dash (legacy format)",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "-test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - name ends with dash (legacy format)",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name-",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - name starts with dot",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: ".test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - name ends with dot",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name.",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - name starts with underscore (legacy format)",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "_test-name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
{
name: "valid - name ends with underscore (legacy format)",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name_",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: false,
},
// Invalid name cases
{
name: "invalid - name with slash",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
@@ -342,7 +497,46 @@ func TestDataKey_Validate(t *testing.T) {
Action: DataActionCreated,
},
expectError: true,
errorMsg: "name 'test/name' is invalid",
errorField: "name",
},
{
name: "invalid - name with spaces",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test name",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorField: "name",
},
{
name: "invalid - name with special characters",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test@name#with$special",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorField: "name",
},
{
name: "invalid - empty name",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorField: "name",
},
// Invalid cases - start/end with invalid characters
{
@@ -384,19 +578,6 @@ func TestDataKey_Validate(t *testing.T) {
expectError: true,
errorMsg: "resource '.test-resource' is invalid",
},
{
name: "invalid - name ends with dash",
key: DataKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "test-name-",
ResourceVersion: rv,
Action: DataActionCreated,
},
expectError: true,
errorMsg: "name 'test-name-' is invalid",
},
// Invalid cases - invalid action
{
name: "invalid - unknown action",
@@ -421,6 +602,10 @@ func TestDataKey_Validate(t *testing.T) {
if 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 {
require.NoError(t, err)
}
@@ -974,7 +1159,7 @@ func TestDataStore_ValidationEnforced(t *testing.T) {
// Create an invalid key
invalidKey := DataKey{
Namespace: "Invalid-Namespace", // uppercase is invalid
Namespace: "Invalid-Namespace-$$$",
Group: "test-group",
Resource: "test-resource",
Name: "test-name",
@@ -988,21 +1173,27 @@ func TestDataStore_ValidationEnforced(t *testing.T) {
_, err := ds.Get(ctx, invalidKey)
require.Error(t, err)
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) {
err := ds.Save(ctx, invalidKey, testValue)
require.Error(t, err)
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) {
err := ds.Delete(ctx, invalidKey)
require.Error(t, err)
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
@@ -1043,6 +1234,7 @@ func TestListRequestKey_Validate(t *testing.T) {
key ListRequestKey
expectError bool
errorMsg string
errorField string
}{
{
name: "valid - all fields provided",
@@ -1054,6 +1246,45 @@ func TestListRequestKey_Validate(t *testing.T) {
},
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",
key: ListRequestKey{
@@ -1075,7 +1306,47 @@ func TestListRequestKey_Validate(t *testing.T) {
name: "invalid - all empty",
key: ListRequestKey{},
expectError: true,
errorMsg: "group is required",
errorField: "namespace",
},
{
name: "valid - legacy grafana uid 1",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "_4OV_5Nmz",
},
expectError: false,
},
{
name: "valid - legacy grafana uid 2",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "-Y-tnEDWk",
},
expectError: false,
},
{
name: "valid - legacy grafana uid 3",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "000000005",
},
expectError: false,
},
{
name: "valid - uppercase in name",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "Test-Name",
},
expectError: false,
},
// Invalid hierarchical cases
{
@@ -1084,7 +1355,7 @@ func TestListRequestKey_Validate(t *testing.T) {
Group: "test-group",
},
expectError: true,
errorMsg: "resource is required",
errorField: "resource",
},
{
name: "invalid - name without namespace",
@@ -1094,7 +1365,7 @@ func TestListRequestKey_Validate(t *testing.T) {
Group: "test-group",
},
expectError: true,
errorMsg: "name must be empty when namespace is empty",
errorMsg: ErrNameMustBeEmptyWhenNamespaceEmpty,
},
{
name: "invalid - name without group and resource",
@@ -1103,63 +1374,9 @@ func TestListRequestKey_Validate(t *testing.T) {
Name: "test-name",
},
expectError: true,
errorMsg: "group is required",
errorField: "group",
},
// 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 - uppercase in name",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group",
Resource: "test-resource",
Name: "Test-Name",
},
expectError: true,
errorMsg: "name 'Test-Name' 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",
key: ListRequestKey{
@@ -1182,6 +1399,16 @@ func TestListRequestKey_Validate(t *testing.T) {
expectError: true,
errorMsg: "group 'test-group.' is invalid",
},
{
name: "invalid - name contains invalid char",
key: ListRequestKey{
Namespace: "test-namespace",
Group: "test-group.",
Resource: "test-resource",
Name: "test$name",
},
expectError: true,
},
}
for _, tt := range tests {
@@ -2288,10 +2515,11 @@ func TestDataKey_SameResource(t *testing.T) {
func TestGetRequestKey_Validate(t *testing.T) {
tests := []struct {
name string
key GetRequestKey
expectErr bool
wantError string
name string
key GetRequestKey
expectErr bool
wantError string
errorField string
}{
{
name: "valid key",
@@ -2313,6 +2541,16 @@ func TestGetRequestKey_Validate(t *testing.T) {
},
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",
key: GetRequestKey{
@@ -2320,8 +2558,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Namespace: "default",
Name: "test-resource",
},
expectErr: true,
wantError: "group is required",
expectErr: true,
errorField: "group",
},
{
name: "missing resource",
@@ -2330,8 +2568,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Namespace: "default",
Name: "test-resource",
},
expectErr: true,
wantError: "resource is required",
expectErr: true,
errorField: "resource",
},
{
name: "missing namespace",
@@ -2340,8 +2578,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Resource: "resources",
Name: "test-resource",
},
expectErr: true,
wantError: "namespace is required",
expectErr: true,
errorField: "namespace",
},
{
name: "missing name",
@@ -2350,30 +2588,19 @@ func TestGetRequestKey_Validate(t *testing.T) {
Resource: "resources",
Namespace: "default",
},
expectErr: true,
wantError: "name is required",
expectErr: true,
errorField: "name",
},
{
name: "invalid namespace - uppercase",
name: "invalid group - underscore at start",
key: GetRequestKey{
Group: "apps",
Resource: "resources",
Namespace: "Default",
Name: "test-resource",
},
expectErr: true,
wantError: "namespace 'Default' is invalid",
},
{
name: "invalid group - underscore",
key: GetRequestKey{
Group: "apps_v1",
Group: "_apps_v1",
Resource: "resources",
Namespace: "default",
Name: "test-resource",
},
expectErr: true,
wantError: "group 'apps_v1' is invalid",
expectErr: true,
errorField: "group",
},
{
name: "invalid resource - starts with dash",
@@ -2383,19 +2610,8 @@ func TestGetRequestKey_Validate(t *testing.T) {
Namespace: "default",
Name: "test-resource",
},
expectErr: true,
wantError: "resource '-resources' is invalid",
},
{
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",
expectErr: true,
errorField: "resource",
},
}
@@ -2407,6 +2623,10 @@ func TestGetRequestKey_Validate(t *testing.T) {
if 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 {
require.NoError(t, err)
}
+23
View File
@@ -2,6 +2,7 @@ package resource
import (
"errors"
"fmt"
"net/http"
"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
}
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}
}
+22 -27
View File
@@ -3,6 +3,7 @@ package resource
import (
"context"
"encoding/json"
"errors"
"fmt"
"iter"
"strconv"
@@ -10,6 +11,7 @@ import (
"time"
"github.com/bwmarrin/snowflake"
"github.com/grafana/grafana/pkg/apimachinery/validation"
)
const (
@@ -37,46 +39,38 @@ func (k EventKey) String() string {
func (k EventKey) Validate() error {
if k.Namespace == "" {
return fmt.Errorf("namespace cannot be empty")
}
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")
return NewValidationError("namespace", k.Namespace, ErrNamespaceRequired)
}
if k.ResourceVersion < 0 {
return fmt.Errorf("resource version must be non-negative")
return errors.New(ErrResourceVersionInvalid)
}
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 !validNameRegex.MatchString(k.Namespace) {
return fmt.Errorf("namespace '%s' is invalid", k.Namespace)
if err := validation.IsValidGroup(k.Group); err != nil {
return NewValidationError("group", k.Group, err[0])
}
if !validNameRegex.MatchString(k.Group) {
return fmt.Errorf("group '%s' is invalid", k.Group)
if err := validation.IsValidResource(k.Resource); err != nil {
return NewValidationError("resource", k.Resource, err[0])
}
if !validNameRegex.MatchString(k.Resource) {
return fmt.Errorf("resource '%s' is invalid", k.Resource)
if err := validation.IsValidGrafanaName(k.Name); err != nil {
return NewValidationError("name", k.Name, err[0])
}
if !validNameRegex.MatchString(k.Name) {
return fmt.Errorf("name '%s' is invalid", k.Name)
}
if k.Folder != "" && !validNameRegex.MatchString(k.Folder) {
return fmt.Errorf("folder '%s' is invalid", k.Folder)
if k.Folder != "" {
if err := validation.IsValidGrafanaName(k.Folder); err != nil {
return NewValidationError("folder", k.Folder, err[0])
}
}
switch k.Action {
case DataActionCreated, DataActionUpdated, DataActionDeleted:
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
@@ -148,6 +142,7 @@ func (n *eventStore) Save(ctx context.Context, event Event) error {
Name: event.Name,
ResourceVersion: event.ResourceVersion,
Action: event.Action,
//TODO why isnt folder part of the key?
}
if err := eventKey.Validate(); err != nil {
+1 -1
View File
@@ -41,7 +41,7 @@ func verifyRequestKeyNamespaceGroupResource(key *resourcepb.ResourceKey) *resour
if err := validation.IsValidGroup(key.Group); err != nil {
return NewBadRequestError(err[0])
}
if err := validation.IsValidateResource(key.Resource); err != nil {
if err := validation.IsValidResource(key.Resource); err != nil {
return NewBadRequestError(err[0])
}
return nil
+2 -2
View File
@@ -259,9 +259,9 @@ func PrefixRangeEnd(prefix string) string {
var (
// validKeyRegex validates keys used in the unified storage
// Keys can contain lowercase alphanumeric characters, '-', '.', '/', and '~'
// Keys can contain alphanumeric characters (both upper and lowercase), '-', '.', '/', and '~'
// Any combination of these characters is allowed as long as the key is not empty
validKeyRegex = regexp.MustCompile(`^[a-z0-9./~-]+$`)
validKeyRegex = regexp.MustCompile(`^[a-zA-Z0-9./~_-]+$`)
)
func IsValidKey(key string) bool {
+4 -2
View File
@@ -235,17 +235,19 @@ func TestIsValidKey(t *testing.T) {
{"data key format", "ns/group/resource/name/123~created", true},
{"metadata key format", "group/resource/ns/name/123~created~folder", true},
{"metadata key format ending with a ~", "group/resource/ns/name/123~created~", true},
{"uppercase letters", "Valid", true},
{"underscores", "a_b", true},
{"key with underscores and mixed chars", "4_D6mSh4z", true},
{"complex key with underscores", "ns/group_name/resource-name/Name_123~action-type", true},
// invalid keys
{"empty key", "", false},
{"uppercase letters", "Invalid", false},
{"special characters", "a@b", false},
{"spaces", "a b", false},
{"leading space", " key", false},
{"trailing space", "key ", false},
{"tab character", "a\tb", false},
{"newline character", "a\nb", false},
{"underscores", "a_b", false},
}
for _, tt := range tests {
+1 -1
View File
@@ -554,7 +554,7 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *resour
return nil, NewBadRequestError(
fmt.Sprintf("key/name do not match (key: %s, name: %s)", key.Name, obj.GetName()))
}
if errs := validation.IsValidGrafanaName(obj.GetName()); err != nil {
if errs := validation.IsValidGrafanaName(obj.GetName()); errs != nil {
return nil, NewBadRequestError(errs[0])
}