Unistore: ignore name when validating collection keys (#112086)

Signed-off-by: Maicon Costa <maiconscosta@gmail.com>

pkg/storage/unified/resource/bulk.go

@@ -170,9 +170,9 @@ func (s *server) BulkProcess(stream resourcepb.BulkStore_BulkProcessServer) erro
 		})
 	}
 
-	// Verify all request keys are valid
+	// Verify all collection request keys are valid
 	for _, k := range settings.Collection {
-		if r := verifyRequestKey(k); r != nil {
+		if r := verifyRequestKeyCollection(k); r != nil {
 			return sendAndClose(&resourcepb.BulkResponse{
 				Error: &resourcepb.ErrorResult{
 					Message: fmt.Sprintf("invalid request key: %s", r.Message),

pkg/storage/unified/resource/keys.go

@@ -8,7 +8,24 @@ import (
 	"github.com/grafana/grafana/pkg/storage/unified/resourcepb"
 )
 
+// verifyRequestKey verifies that the key is valid for a request (all fields set and valid, including name)
 func verifyRequestKey(key *resourcepb.ResourceKey) *resourcepb.ErrorResult {
+	if err := verifyRequestKeyNamespaceGroupResource(key); err != nil {
+		return NewBadRequestError(err.Message)
+	}
+	if err := validation.IsValidGrafanaName(key.Name); err != nil {
+		return NewBadRequestError(err[0])
+	}
+	return nil
+}
+
+// verifyRequestKeyCollection verifies that the key is valid for a collection (namespace/group/resource set and valid)
+func verifyRequestKeyCollection(key *resourcepb.ResourceKey) *resourcepb.ErrorResult {
+	return verifyRequestKeyNamespaceGroupResource(key)
+}
+
+// verifyRequestKeyNamespaceGroupResource verifies that the key has namespace/group/resource set and valid
+func verifyRequestKeyNamespaceGroupResource(key *resourcepb.ResourceKey) *resourcepb.ErrorResult {
 	if key == nil {
 		return NewBadRequestError("missing resource key")
 	}
@@ -27,9 +44,6 @@ func verifyRequestKey(key *resourcepb.ResourceKey) *resourcepb.ErrorResult {
 	if err := validation.IsValidateResource(key.Resource); err != nil {
 		return NewBadRequestError(err[0])
 	}
-	if err := validation.IsValidGrafanaName(key.Name); err != nil {
-		return NewBadRequestError(err[0])
-	}
 	return nil
 }
 

pkg/storage/unified/resource/keys_test.go

@@ -181,3 +181,88 @@ func TestVerifyRequestKey(t *testing.T) {
 		})
 	}
 }
+
+func TestVerifyRequestKeyCollection(t *testing.T) {
+	validGroup := "group.grafana.app"
+	validResource := "resource"
+	validNamespace := "default"
+	invalidName := "    " // only spaces
+
+	invalidGroup := "group.~~~~~grafana.app"
+	invalidResource := "##resource"
+	invalidNamespace := "(((((default"
+
+	namespaceTooLong := strings.Repeat("a", 61)
+
+	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: "invalid namespace returns error",
+			input: &resourcepb.ResourceKey{
+				Namespace: invalidNamespace,
+				Group:     validGroup,
+				Resource:  validResource,
+			},
+			expectedCode: http.StatusBadRequest,
+		},
+		{
+			name: "invalid group returns error",
+			input: &resourcepb.ResourceKey{
+				Namespace: validNamespace,
+				Group:     invalidGroup,
+				Resource:  validResource,
+			},
+			expectedCode: http.StatusBadRequest,
+		},
+		{
+			name: "invalid resource returns error",
+			input: &resourcepb.ResourceKey{
+				Namespace: validNamespace,
+				Group:     validGroup,
+				Resource:  invalidResource,
+			},
+			expectedCode: http.StatusBadRequest,
+		},
+		{
+			name: "invalid name returns no error",
+			input: &resourcepb.ResourceKey{
+				Namespace: validNamespace,
+				Group:     validGroup,
+				Resource:  validResource,
+				Name:      invalidName,
+			},
+		},
+		{
+			name: "namespace too long returns error",
+			input: &resourcepb.ResourceKey{
+				Namespace: namespaceTooLong,
+				Group:     validGroup,
+				Resource:  validResource,
+			},
+			expectedCode: http.StatusBadRequest,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := verifyRequestKeyCollection(test.input)
+			if test.expectedCode == 0 {
+				require.Nil(t, err)
+				return
+			}
+
+			require.Equal(t, test.expectedCode, err.Code)
+		})
+	}
+}