public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Unistore: Add validation for resource names" (#111408)

Browse Source 
Revert "Unistore: Add validation for resource names (#110990)"

This reverts commit b63e3fd3ae.
This commit is contained in:
maicon
2025-09-19 18:03:23 +00:00
committed by GitHub
parent 316b5fe538
commit b3ed600bca
7 changed files with 4 additions and 364 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/apiserver/storage/testing/watcher_tests.go
+4 -4
View File
@@ -684,7 +684,7 @@ func RunTestClusterScopedWatch(ctx context.Context, t *testing.T, store storage.
currentObjs := map[string]*example.Pod{}
for _, watchTest := range tt.watchTests {
out := &example.Pod{}
key := "pods/ns-1/" + watchTest.obj.Name
key := "pods/" + watchTest.obj.Name
err := store.GuaranteedUpdate(ctx, key, out, true, nil, storage.SimpleUpdate(
func(runtime.Object) (runtime.Object, error) {
obj := watchTest.obj.DeepCopy()
@@ -1607,15 +1607,15 @@ func clusterScopedNodeNameAttrFunc(obj runtime.Object) (labels.Set, fields.Set,
}
func basePod(podName string) *example.Pod {
return baseNamespacedPod(podName, "ns-1")
return baseNamespacedPod(podName, "")
}
func basePodUpdated(podName string) *example.Pod {
return baseNamespacedPodUpdated(podName, "ns-1")
return baseNamespacedPodUpdated(podName, "")
}
func basePodAssigned(podName, nodeName string) *example.Pod {
return baseNamespacedPodAssigned(podName, "ns-1", nodeName)
return baseNamespacedPodAssigned(podName, "", nodeName)
}
func baseNamespacedPod(podName, namespace string) *example.Pod {
pkg/storage/unified/resource/bulk.go
-12
View File
@@ -170,18 +170,6 @@ func (s *server) BulkProcess(stream resourcepb.BulkStore_BulkProcessServer) erro
})
}
// Verify all request keys are valid
for _, k := range settings.Collection {
if r := verifyRequestKey(k); r != nil {
return sendAndClose(&resourcepb.BulkResponse{
Error: &resourcepb.ErrorResult{
Message: fmt.Sprintf("invalid request key: %s", r.Message),
Code: http.StatusBadRequest,
},
})
}
}
if settings.RebuildCollection {
for _, k := range settings.Collection {
// Can we delete the whole collection
pkg/storage/unified/resource/keys.go
-12
View File
@@ -17,18 +17,6 @@ func verifyRequestKey(key *resourcepb.ResourceKey) *resourcepb.ErrorResult {
if key.Resource == "" {
return NewBadRequestError("request key is missing resource")
}
if err := validateName(key.Name); err != nil {
return NewBadRequestError(fmt.Sprintf("name '%s' is invalid: '%s'", key.Name, err))
}
if err := validateQualifiedName(key.Namespace); err != nil {
return NewBadRequestError(fmt.Sprintf("namespace '%s' is invalid: '%s'", key.Namespace, err))
}
if err := validateQualifiedName(key.Group); err != nil {
return NewBadRequestError(fmt.Sprintf("group '%s' is invalid: '%s'", key.Group, err))
}
if err := validateQualifiedName(key.Resource); err != nil {
return NewBadRequestError(fmt.Sprintf("resource '%s' is invalid: '%s'", key.Resource, err))
}
return nil
}
pkg/storage/unified/resource/keys_test.go
-115
View File
@@ -1,8 +1,6 @@
package resource
import (
"net/http"
"strings"
"testing"
"github.com/stretchr/testify/require"
@@ -68,116 +66,3 @@ func TestSearchIDKeys(t *testing.T) {
}
}
}
func TestVerifyRequestKey(t *testing.T) {
validGroup := "group.grafana.app"
validResource := "resource"
validNamespace := "default"
validName := "fdgsv37qslr0ga"
validLegacyUID := "f8cc010c-ee72-4681-89d2-d46e1bd47d33"
invalidGroup := "group.~~~~~grafana.app"
invalidResource := "##resource"
invalidNamespace := "(((((default"
invalidName := " " // only spaces
namespaceTooLong := strings.Repeat("a", MaxQualifiedNameLength+1)
nameTooLong := strings.Repeat("a", 300)
tests := []struct {
name string
input *resourcepb.ResourceKey
expectedCode int32
}{
{
name: "no error when all fields are set and valid",
input: &resourcepb.ResourceKey{
Namespace: validNamespace,
Group: validGroup,
Resource: validResource,
Name: validName,
},
},
{
name: "invalid namespace returns error",
input: &resourcepb.ResourceKey{
Namespace: invalidNamespace,
Group: validGroup,
Resource: validResource,
Name: validName,
},
expectedCode: http.StatusBadRequest,
},
{
name: "invalid group returns error",
input: &resourcepb.ResourceKey{
Namespace: validNamespace,
Group: invalidGroup,
Resource: validResource,
Name: validName,
},
expectedCode: http.StatusBadRequest,
},
{
name: "invalid resource returns error",
input: &resourcepb.ResourceKey{
Namespace: validNamespace,
Group: validGroup,
Resource: invalidResource,
Name: validName,
},
expectedCode: http.StatusBadRequest,
},
{
name: "invalid name returns error",
input: &resourcepb.ResourceKey{
Namespace: validNamespace,
Group: validGroup,
Resource: validResource,
Name: invalidName,
},
expectedCode: http.StatusBadRequest,
},
{
name: "valid legacy UID returns no error",
input: &resourcepb.ResourceKey{
Namespace: validNamespace,
Group: validGroup,
Resource: validResource,
Name: validLegacyUID,
},
},
{
name: "namespace too long returns error",
input: &resourcepb.ResourceKey{
Namespace: namespaceTooLong,
Group: validGroup,
Resource: validResource,
Name: validName,
},
expectedCode: http.StatusBadRequest,
},
{
name: "name too long returns error",
input: &resourcepb.ResourceKey{
Namespace: namespaceTooLong,
Group: validGroup,
Resource: validResource,
Name: nameTooLong,
},
expectedCode: http.StatusBadRequest,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := verifyRequestKey(test.input)
if test.expectedCode == 0 {
require.Nil(t, err)
return
}
require.Equal(t, test.expectedCode, err.Code)
})
}
}
pkg/storage/unified/resource/server.go
-4
View File
@@ -626,10 +626,6 @@ func (s *server) Create(ctx context.Context, req *resourcepb.CreateRequest) (*re
ctx, span := s.tracer.Start(ctx, "storage_server.Create")
defer span.End()
if r := verifyRequestKey(req.Key); r != nil {
return nil, fmt.Errorf("invalid request key: %s", r.Message)
}
rsp := &resourcepb.CreateResponse{}
user, ok := claims.AuthInfoFrom(ctx)
if !ok || user == nil {
pkg/storage/unified/resource/server_test.go
-197
View File
@@ -7,7 +7,6 @@ import (
"log/slog"
"net/http"
"os"
"strings"
"sync"
"testing"
"time"
@@ -196,202 +195,6 @@ func TestSimpleServer(t *testing.T) {
require.Len(t, all.Items, 0) // empty
})
t.Run("playlist FAIL CRUD paths due to invalid key", func(t *testing.T) {
raw := []byte(`{
"apiVersion": "playlist.grafana.app/v0alpha1",
"kind": "Playlist",
"metadata": {
"name": "fdgsv37#qslr0ga",
"uid": "xyz",
"namespace": "default",
"annotations": {
"grafana.app/repoName": "elsewhere",
"grafana.app/repoPath": "path/to/item",
"grafana.app/repoTimestamp": "2024-02-02T00:00:00Z"
}
},
"spec": {
"title": "hello",
"interval": "5m",
"items": [
{
"type": "dashboard_by_uid",
"value": "vmie2cmWz"
}
]
}
}`)
// invalid group
key := &resourcepb.ResourceKey{
Group: "playlist.grafana.app###",
Resource: "rrrr", // can be anything :(
Namespace: "default",
Name: "fdgsv37qslr0ga",
}
created, err := server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.Error(t, err)
require.Nil(t, created)
// invalid resource
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: "rrrr###", // can be anything :(
Namespace: "default",
Name: "fdgsv37qslr0ga",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.Error(t, err)
require.Nil(t, created)
// invalid namespace
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: "rrrr", // can be anything :(
Namespace: "default###",
Name: "fdgsv37qslr0ga",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.Error(t, err)
require.Nil(t, created)
// invalid name
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: "rrrr", // can be anything :(
Namespace: "default",
Name: "fdgsv37qslr0g###",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.Error(t, err)
require.Nil(t, created)
// legacy name - valid
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: "rrrr", // can be anything :(
Namespace: "default",
Name: "2c7e5361-7360-4d2a-ae45-5e79bba458d6",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.NoError(t, err)
require.NotNil(t, created)
// legacy name - also valid
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: "rrrr", // can be anything :(
Namespace: "default",
Name: "IvIsO_YGz",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.NoError(t, err)
require.NotNil(t, created)
// legacy name - also valid
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: "rrrr", // can be anything :(
Namespace: "default",
Name: "_IvIsOYGz",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.NoError(t, err)
require.NotNil(t, created)
invalidQualifiedNames := []string{
"", // empty
strings.Repeat("1", MaxQualifiedNameLength+1), // too long
" ", // only spaces
"f8cc010c.ee72.4681;89d2+d46e1bd47d33", // invalid chars
}
// group
for _, invalidGroup := range invalidQualifiedNames {
key = &resourcepb.ResourceKey{
Group: invalidGroup,
Resource: "rrrr", // can be anything :(
Namespace: "default",
Name: "_IvIsOYGz",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.Error(t, err)
require.Nil(t, created)
}
// resource
for _, invalidResource := range invalidQualifiedNames {
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: invalidResource,
Namespace: "default",
Name: "_IvIsOYGz",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.Error(t, err)
require.Nil(t, created)
}
// namespace
for _, invalidNamespace := range invalidQualifiedNames {
key = &resourcepb.ResourceKey{
Group: "playlist.grafana.app",
Resource: "rrrr", // can be anything :(
Namespace: invalidNamespace,
Name: "_IvIsOYGz",
}
created, err = server.Create(ctx, &resourcepb.CreateRequest{
Value: raw,
Key: key,
})
require.Error(t, err)
require.Nil(t, created)
}
})
t.Run("playlist update optimistic concurrency check", func(t *testing.T) {
raw := []byte(`{
"apiVersion": "playlist.grafana.app/v0alpha1",
pkg/storage/unified/resource/validation.go
-20
View File
@@ -1,15 +1,11 @@
package resource
import (
"fmt"
"regexp"
"github.com/grafana/grafana/pkg/storage/unified/resourcepb"
"k8s.io/apimachinery/pkg/util/validation"
)
const MaxQualifiedNameLength = 40
var validNameCharPattern = `a-zA-Z0-9:\-\_\.`
var validNamePattern = regexp.MustCompile(`^[` + validNameCharPattern + `]*$`).MatchString
@@ -28,19 +24,3 @@ func validateName(name string) *resourcepb.ErrorResult {
// so we will be slightly more lenient than standard k8s
return nil
}
func validateQualifiedName(value string) *resourcepb.ErrorResult {
if len(value) == 0 {
return NewBadRequestError("value is too short")
}
if len(value) > MaxQualifiedNameLength {
return NewBadRequestError("value is too long")
}
err := validation.IsQualifiedName(value)
if len(err) > 0 {
return NewBadRequestError(fmt.Sprintf("name is not a valid qualified name: %+v", err))
}
return nil
}