diff --git a/pkg/apimachinery/utils/meta.go b/pkg/apimachinery/utils/meta.go index cc764ec3a4d..6cf75d4f899 100644 --- a/pkg/apimachinery/utils/meta.go +++ b/pkg/apimachinery/utils/meta.go @@ -58,6 +58,9 @@ const AnnoKeySourcePath = "grafana.app/sourcePath" const AnnoKeySourceChecksum = "grafana.app/sourceChecksum" const AnnoKeySourceTimestamp = "grafana.app/sourceTimestamp" +// Only used in modes 0-2 (legacy db) for returning the folder fullpath + +const LabelGetFullpath = "grafana.app/fullpath" const AnnoKeyFullpath = "grafana.app/fullpath" const AnnoKeyFullpathUIDs = "grafana.app/fullpathUIDs" diff --git a/pkg/registry/apis/folders/legacy_storage.go b/pkg/registry/apis/folders/legacy_storage.go index 86c27b49a94..473a1273140 100644 --- a/pkg/registry/apis/folders/legacy_storage.go +++ b/pkg/registry/apis/folders/legacy_storage.go @@ -108,7 +108,7 @@ func (s *legacyStorage) List(ctx context.Context, options *internalversion.ListO } // only admins can add this to the query, otherwise we may return parent folder names that are not visible to the user - if user.GetOrgRole() == org.RoleAdmin && options.LabelSelector != nil && options.LabelSelector.Matches(labels.Set{utils.AnnoKeyFullpath: "true"}) { + if user.GetOrgRole() == org.RoleAdmin && options.LabelSelector != nil && options.LabelSelector.Matches(labels.Set{utils.LabelGetFullpath: "true"}) { query.WithFullpath = true query.WithFullpathUIDs = true } diff --git a/pkg/registry/apis/folders/legacy_storage_test.go b/pkg/registry/apis/folders/legacy_storage_test.go index 267dcd51b51..0da1619c1f7 100644 --- a/pkg/registry/apis/folders/legacy_storage_test.go +++ b/pkg/registry/apis/folders/legacy_storage_test.go @@ -158,7 +158,7 @@ func TestLegacyStorage_List_LabelSelector(t *testing.T) { }) t.Run("should set fullpath query parameters when label selector matches", func(t *testing.T) { - selector, err := labels.Parse(utils.AnnoKeyFullpath + "=true") + selector, err := labels.Parse(utils.LabelGetFullpath + "=true") require.NoError(t, err) options := &metainternalversion.ListOptions{ LabelSelector: selector, diff --git a/pkg/services/folder/folderimpl/unifiedstore.go b/pkg/services/folder/folderimpl/unifiedstore.go index 57aa43cabb3..1e80c2afb57 100644 --- a/pkg/services/folder/folderimpl/unifiedstore.go +++ b/pkg/services/folder/folderimpl/unifiedstore.go @@ -312,7 +312,7 @@ func (ss *FolderUnifiedStoreImpl) GetFolders(ctx context.Context, q folder.GetFo if q.WithFullpath || q.WithFullpathUIDs { // only supported in modes 0-2, to keep the alerting queries from causing tons of get folder requests // to retrieve the parent for all folders in grafana - opts.LabelSelector = utils.AnnoKeyFullpath + "=true" + opts.LabelSelector = utils.LabelGetFullpath + "=true" } out, err := ss.k8sclient.List(ctx, q.OrgID, opts) diff --git a/pkg/storage/unified/apistore/util.go b/pkg/storage/unified/apistore/util.go index 44ed68d0b68..017af091581 100644 --- a/pkg/storage/unified/apistore/util.go +++ b/pkg/storage/unified/apistore/util.go @@ -60,11 +60,18 @@ func toListRequest(k *resource.ResourceKey, opts storage.ListOptions) (*resource v := r.Key() // Parse the history request from labels - if v == utils.LabelKeyGetHistory || v == utils.LabelKeyGetTrash { + // TODO: for LabelGetFullpath, we just skip this for unistore. We need a better solution for + // getting the full path for folders in unistore, without making a request for each parent folder. + // In modes 0-2 we added this label to indicate that the sql query should return that data as + // an annotation on the folder. However, this annotation cannot be saved to unified storage, otherwise + // we will have to recompute annotations for all descendants of a folder during a folder move. + // While we look for a better solution, unified storage will continue to return all folders & the folder + // service will get the full path by retrieving each parent folder. + if v == utils.LabelKeyGetHistory || v == utils.LabelKeyGetTrash || v == utils.LabelGetFullpath { if len(requirements) != 1 { return nil, predicate, apierrors.NewBadRequest("single label supported with: " + v) } - if !opts.Predicate.Field.Empty() { + if opts.Predicate.Field != nil && !opts.Predicate.Field.Empty() { return nil, predicate, apierrors.NewBadRequest("field selector not supported with: " + v) } if r.Operator() != selection.Equals { @@ -81,7 +88,7 @@ func toListRequest(k *resource.ResourceKey, opts storage.ListOptions) (*resource if vals[0] != "true" { return nil, predicate, apierrors.NewBadRequest("expecting true for: " + v) } - } else { + } else if v == utils.LabelKeyGetHistory { req.Source = resource.ListRequest_HISTORY req.Options.Key.Name = vals[0] } diff --git a/pkg/storage/unified/apistore/util_test.go b/pkg/storage/unified/apistore/util_test.go new file mode 100644 index 00000000000..7c0b7f5b036 --- /dev/null +++ b/pkg/storage/unified/apistore/util_test.go @@ -0,0 +1,218 @@ +package apistore + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/apiserver/pkg/storage" + + "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/storage/unified/resource" +) + +func TestToListRequest(t *testing.T) { + tests := []struct { + name string + key *resource.ResourceKey + opts storage.ListOptions + want *resource.ListRequest + wantPredicate storage.SelectionPredicate + wantErr error + }{ + { + name: "basic list request", + key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + opts: storage.ListOptions{}, + want: &resource.ListRequest{ + VersionMatchV2: 1, + Options: &resource.ListOptions{ + Key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + }, + }, + wantPredicate: storage.SelectionPredicate{}, + wantErr: nil, + }, + { + name: "with resource version", + key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + opts: storage.ListOptions{ + ResourceVersion: "123", + }, + want: &resource.ListRequest{ + VersionMatchV2: 1, + ResourceVersion: 123, + Options: &resource.ListOptions{ + Key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + }, + }, + wantPredicate: storage.SelectionPredicate{}, + wantErr: nil, + }, + { + name: "invalid resource version", + key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + opts: storage.ListOptions{ + ResourceVersion: "invalid", + }, + want: nil, + wantPredicate: storage.SelectionPredicate{}, + wantErr: apierrors.NewBadRequest("invalid resource version: invalid"), + }, + { + name: "with label selector", + key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + opts: storage.ListOptions{ + Predicate: storage.SelectionPredicate{ + Label: labels.SelectorFromSet(labels.Set{"key": "value"}), + }, + }, + want: &resource.ListRequest{ + VersionMatchV2: 1, + Options: &resource.ListOptions{ + Key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + Labels: []*resource.Requirement{ + { + Key: "key", + Operator: string(selection.Equals), + Values: []string{"value"}, + }, + }, + }, + }, + wantPredicate: storage.SelectionPredicate{ + Label: labels.SelectorFromSet(labels.Set{"key": "value"}), + }, + wantErr: nil, + }, + { + name: "with trash label", + key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + opts: storage.ListOptions{ + Predicate: storage.SelectionPredicate{ + Label: labels.SelectorFromSet(labels.Set{utils.LabelKeyGetTrash: "true"}), + }, + }, + want: &resource.ListRequest{ + VersionMatchV2: 1, + Source: resource.ListRequest_TRASH, + Options: &resource.ListOptions{ + Labels: nil, + Fields: nil, + Key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + }, + }, + wantPredicate: storage.Everything, + wantErr: nil, + }, + { + name: "with history label", + key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + opts: storage.ListOptions{ + Predicate: storage.SelectionPredicate{ + Label: labels.SelectorFromSet(labels.Set{utils.LabelKeyGetHistory: "test-name"}), + }, + }, + want: &resource.ListRequest{ + VersionMatchV2: 1, + Source: resource.ListRequest_HISTORY, + Options: &resource.ListOptions{ + Labels: nil, + Fields: nil, + Key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + Name: "test-name", + }, + }, + }, + wantPredicate: storage.Everything, + wantErr: nil, + }, + { + name: "with fullpath label", + key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + opts: storage.ListOptions{ + Predicate: storage.SelectionPredicate{ + Label: labels.SelectorFromSet(labels.Set{utils.LabelGetFullpath: "true"}), + }, + }, + want: &resource.ListRequest{ + VersionMatchV2: 1, + Options: &resource.ListOptions{ + Labels: nil, + Fields: nil, + Key: &resource.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "default", + }, + }, + }, + wantPredicate: storage.Everything, + wantErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, gotPredicate, err := toListRequest(tt.key, tt.opts) + if tt.wantErr != nil { + assert.Error(t, err) + assert.Equal(t, tt.wantErr.Error(), err.Error()) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + assert.Equal(t, tt.wantPredicate, gotPredicate) + }) + } +} diff --git a/pkg/storage/unified/resource/server.go b/pkg/storage/unified/resource/server.go index aa1b2e044a6..c0992e8e1ec 100644 --- a/pkg/storage/unified/resource/server.go +++ b/pkg/storage/unified/resource/server.go @@ -371,7 +371,7 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *Resour // Make sure the command labels are not saved for k := range obj.GetLabels() { - if k == utils.LabelKeyGetHistory || k == utils.LabelKeyGetTrash { + if k == utils.LabelKeyGetHistory || k == utils.LabelKeyGetTrash || k == utils.LabelGetFullpath { return nil, NewBadRequestError("can not save label: " + k) } }