From f625902e4b2b82063522e1fa3b03d4bc4bde28da Mon Sep 17 00:00:00 2001 From: Roberto Jimenez Sanchez Date: Fri, 9 Jan 2026 13:18:59 +0100 Subject: [PATCH] Provisioning: Add fieldSelector for Repository by spec.connection.name This change adds the ability to filter repositories by their connection name using Kubernetes field selectors, enabling queries like: kubectl get repositories --field-selector spec.connection.name=my-connection Implementation: - Add RepositoryGetAttrs and RepositoryToSelectableFields functions - Register field label conversion for spec.connection.name in InstallSchema - Extend generic storage to support custom selectable fields via NewRegistryStoreWithSelectableFields - Add unit tests for repository field functions - Add integration tests for field selector functionality --- pkg/apiserver/registry/generic/storage.go | 36 +++- pkg/registry/apis/provisioning/register.go | 27 ++- .../apis/provisioning/repository_fields.go | 44 +++++ .../provisioning/repository_fields_test.go | 184 ++++++++++++++++++ .../apis/provisioning/connection_test.go | 172 ++++++++++++++++ 5 files changed, 460 insertions(+), 3 deletions(-) create mode 100644 pkg/registry/apis/provisioning/repository_fields.go create mode 100644 pkg/registry/apis/provisioning/repository_fields_test.go diff --git a/pkg/apiserver/registry/generic/storage.go b/pkg/apiserver/registry/generic/storage.go index 98e2f1fe9df..91a7f94d5f7 100644 --- a/pkg/apiserver/registry/generic/storage.go +++ b/pkg/apiserver/registry/generic/storage.go @@ -1,26 +1,58 @@ package generic import ( + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/generic/registry" + "k8s.io/apiserver/pkg/storage" "github.com/grafana/grafana/pkg/apimachinery/utils" ) +// SelectableFieldsOptions allows customizing field selector behavior for a resource. +type SelectableFieldsOptions struct { + // GetAttrs returns labels and fields for the object. + // If nil, the default GetAttrs is used which only exposes metadata.name. + GetAttrs func(obj runtime.Object) (labels.Set, fields.Set, error) +} + func NewRegistryStore(scheme *runtime.Scheme, resourceInfo utils.ResourceInfo, optsGetter generic.RESTOptionsGetter) (*registry.Store, error) { + return NewRegistryStoreWithSelectableFields(scheme, resourceInfo, optsGetter, SelectableFieldsOptions{}) +} + +// NewRegistryStoreWithSelectableFields creates a registry store with custom selectable fields support. +// Use this when you need to filter resources by custom fields like spec.connection.name. +func NewRegistryStoreWithSelectableFields(scheme *runtime.Scheme, resourceInfo utils.ResourceInfo, optsGetter generic.RESTOptionsGetter, fieldOpts SelectableFieldsOptions) (*registry.Store, error) { gv := resourceInfo.GroupVersion() gv.Version = runtime.APIVersionInternal strategy := NewStrategy(scheme, gv) if resourceInfo.IsClusterScoped() { strategy = strategy.WithClusterScope() } + + // Use custom GetAttrs if provided, otherwise use default + attrFunc := GetAttrs + predicateFunc := Matcher + if fieldOpts.GetAttrs != nil { + attrFunc = fieldOpts.GetAttrs + // Create a matcher that uses the custom GetAttrs + predicateFunc = func(label labels.Selector, field fields.Selector) storage.SelectionPredicate { + return storage.SelectionPredicate{ + Label: label, + Field: field, + GetAttrs: attrFunc, + } + } + } + store := ®istry.Store{ NewFunc: resourceInfo.NewFunc, NewListFunc: resourceInfo.NewListFunc, KeyRootFunc: KeyRootFunc(resourceInfo.GroupResource()), KeyFunc: NamespaceKeyFunc(resourceInfo.GroupResource()), - PredicateFunc: Matcher, + PredicateFunc: predicateFunc, DefaultQualifiedResource: resourceInfo.GroupResource(), SingularQualifiedResource: resourceInfo.SingularGroupResource(), TableConvertor: resourceInfo.TableConverter(), @@ -28,7 +60,7 @@ func NewRegistryStore(scheme *runtime.Scheme, resourceInfo utils.ResourceInfo, o UpdateStrategy: strategy, DeleteStrategy: strategy, } - options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: GetAttrs} + options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: attrFunc} if err := store.CompleteWithOptions(options); err != nil { return nil, err } diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 026797eb474..e54a8c2fc28 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -559,6 +559,22 @@ func (b *APIBuilder) InstallSchema(scheme *runtime.Scheme) error { return err } + // Register custom field label conversion for Repository to enable field selectors like spec.connection.name + err = scheme.AddFieldLabelConversionFunc( + provisioning.SchemeGroupVersion.WithKind("Repository"), + func(label, value string) (string, string, error) { + switch label { + case "metadata.name", "metadata.namespace", "spec.connection.name": + return label, value, nil + default: + return "", "", fmt.Errorf("field label not supported for Repository: %s", label) + } + }, + ) + if err != nil { + return err + } + metav1.AddToGroupVersion(scheme, provisioning.SchemeGroupVersion) // Only 1 version (for now?) return scheme.SetVersionPriority(provisioning.SchemeGroupVersion) @@ -569,10 +585,19 @@ func (b *APIBuilder) AllowedV0Alpha1Resources() []string { } func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupInfo, opts builder.APIGroupOptions) error { - repositoryStorage, err := grafanaregistry.NewRegistryStore(opts.Scheme, provisioning.RepositoryResourceInfo, opts.OptsGetter) + // Create repository storage with custom field selectors (e.g., spec.connection.name) + repositoryStorage, err := grafanaregistry.NewRegistryStoreWithSelectableFields( + opts.Scheme, + provisioning.RepositoryResourceInfo, + opts.OptsGetter, + grafanaregistry.SelectableFieldsOptions{ + GetAttrs: RepositoryGetAttrs, + }, + ) if err != nil { return fmt.Errorf("failed to create repository storage: %w", err) } + repositoryStatusStorage := grafanaregistry.NewRegistryStatusStore(opts.Scheme, repositoryStorage) b.store = repositoryStorage diff --git a/pkg/registry/apis/provisioning/repository_fields.go b/pkg/registry/apis/provisioning/repository_fields.go new file mode 100644 index 00000000000..0849c558e2f --- /dev/null +++ b/pkg/registry/apis/provisioning/repository_fields.go @@ -0,0 +1,44 @@ +package provisioning + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/registry/generic" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" +) + +// RepositoryToSelectableFields returns a field set that can be used for field selectors. +// This includes standard metadata fields plus custom fields like spec.connection.name. +func RepositoryToSelectableFields(obj *provisioning.Repository) fields.Set { + objectMetaFields := generic.ObjectMetaFieldsSet(&obj.ObjectMeta, true) + + // Add custom selectable fields + specificFields := fields.Set{ + "spec.connection.name": getConnectionName(obj), + } + + return generic.MergeFieldsSets(objectMetaFields, specificFields) +} + +// getConnectionName safely extracts the connection name from a Repository. +// Returns empty string if no connection is configured. +func getConnectionName(obj *provisioning.Repository) string { + if obj == nil || obj.Spec.Connection == nil { + return "" + } + return obj.Spec.Connection.Name +} + +// RepositoryGetAttrs returns labels and fields of a Repository object. +// This is used by the storage layer for filtering. +func RepositoryGetAttrs(obj runtime.Object) (labels.Set, fields.Set, error) { + repo, ok := obj.(*provisioning.Repository) + if !ok { + return nil, nil, fmt.Errorf("given object is not a Repository") + } + return labels.Set(repo.Labels), RepositoryToSelectableFields(repo), nil +} diff --git a/pkg/registry/apis/provisioning/repository_fields_test.go b/pkg/registry/apis/provisioning/repository_fields_test.go new file mode 100644 index 00000000000..89a2271477c --- /dev/null +++ b/pkg/registry/apis/provisioning/repository_fields_test.go @@ -0,0 +1,184 @@ +package provisioning + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" +) + +func TestGetConnectionName(t *testing.T) { + tests := []struct { + name string + repo *provisioning.Repository + expected string + }{ + { + name: "nil repository returns empty string", + repo: nil, + expected: "", + }, + { + name: "repository without connection returns empty string", + repo: &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Title: "test-repo", + }, + }, + expected: "", + }, + { + name: "repository with connection returns connection name", + repo: &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Title: "test-repo", + Connection: &provisioning.ConnectionInfo{ + Name: "my-connection", + }, + }, + }, + expected: "my-connection", + }, + { + name: "repository with empty connection name returns empty string", + repo: &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Title: "test-repo", + Connection: &provisioning.ConnectionInfo{ + Name: "", + }, + }, + }, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getConnectionName(tt.repo) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestRepositoryToSelectableFields(t *testing.T) { + tests := []struct { + name string + repo *provisioning.Repository + expectedFields map[string]string + }{ + { + name: "includes metadata.name and metadata.namespace", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repository", + }, + }, + expectedFields: map[string]string{ + "metadata.name": "test-repo", + "metadata.namespace": "default", + "spec.connection.name": "", + }, + }, + { + name: "includes spec.connection.name when set", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "repo-with-connection", + Namespace: "org-1", + }, + Spec: provisioning.RepositorySpec{ + Title: "Repo With Connection", + Connection: &provisioning.ConnectionInfo{ + Name: "github-connection", + }, + }, + }, + expectedFields: map[string]string{ + "metadata.name": "repo-with-connection", + "metadata.namespace": "org-1", + "spec.connection.name": "github-connection", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fields := RepositoryToSelectableFields(tt.repo) + + for key, expectedValue := range tt.expectedFields { + actualValue, exists := fields[key] + assert.True(t, exists, "field %s should exist", key) + assert.Equal(t, expectedValue, actualValue, "field %s should have correct value", key) + } + }) + } +} + +func TestRepositoryGetAttrs(t *testing.T) { + t.Run("returns error for non-Repository object", func(t *testing.T) { + // Pass a different runtime.Object type instead of a Repository + connection := &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "not-a-repository", + }, + } + _, _, err := RepositoryGetAttrs(connection) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a Repository") + }) + + t.Run("returns labels and fields for valid Repository", func(t *testing.T) { + repo := &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + Labels: map[string]string{ + "app": "grafana", + "env": "test", + }, + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repository", + Connection: &provisioning.ConnectionInfo{ + Name: "my-connection", + }, + }, + } + + labels, fields, err := RepositoryGetAttrs(repo) + require.NoError(t, err) + + // Check labels + assert.Equal(t, "grafana", labels["app"]) + assert.Equal(t, "test", labels["env"]) + + // Check fields + assert.Equal(t, "test-repo", fields["metadata.name"]) + assert.Equal(t, "default", fields["metadata.namespace"]) + assert.Equal(t, "my-connection", fields["spec.connection.name"]) + }) + + t.Run("returns empty connection name when not set", func(t *testing.T) { + repo := &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repository", + }, + } + + _, fields, err := RepositoryGetAttrs(repo) + require.NoError(t, err) + assert.Equal(t, "", fields["spec.connection.name"]) + }) +} diff --git a/pkg/tests/apis/provisioning/connection_test.go b/pkg/tests/apis/provisioning/connection_test.go index 02c5436badb..99f32dffa93 100644 --- a/pkg/tests/apis/provisioning/connection_test.go +++ b/pkg/tests/apis/provisioning/connection_test.go @@ -559,3 +559,175 @@ func TestIntegrationConnectionController_HealthCheckUpdates(t *testing.T) { assert.True(t, final.Status.Health.Healthy, "connection should remain healthy") }) } + +func TestIntegrationProvisioning_RepositoryFieldSelectorByConnection(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + helper := runGrafana(t) + ctx := context.Background() + createOptions := metav1.CreateOptions{FieldValidation: "Strict"} + + // Create a connection first + connection := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "provisioning.grafana.app/v0alpha1", + "kind": "Connection", + "metadata": map[string]any{ + "name": "test-conn-for-field-selector", + "namespace": "default", + }, + "spec": map[string]any{ + "type": "github", + "github": map[string]any{ + "appID": "123456", + "installationID": "789012", + }, + }, + "secure": map[string]any{ + "privateKey": map[string]any{ + "create": "test-private-key", + }, + }, + }} + + _, err := helper.Connections.Resource.Create(ctx, connection, createOptions) + require.NoError(t, err, "failed to create connection") + + t.Cleanup(func() { + // Clean up repositories first + _ = helper.Repositories.Resource.Delete(ctx, "repo-with-connection", metav1.DeleteOptions{}) + _ = helper.Repositories.Resource.Delete(ctx, "repo-without-connection", metav1.DeleteOptions{}) + _ = helper.Repositories.Resource.Delete(ctx, "repo-with-different-connection", metav1.DeleteOptions{}) + // Then clean up the connection + _ = helper.Connections.Resource.Delete(ctx, "test-conn-for-field-selector", metav1.DeleteOptions{}) + }) + + // Create a repository WITH the connection + repoWithConnection := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "provisioning.grafana.app/v0alpha1", + "kind": "Repository", + "metadata": map[string]any{ + "name": "repo-with-connection", + "namespace": "default", + }, + "spec": map[string]any{ + "title": "Repo With Connection", + "type": "local", + "sync": map[string]any{ + "enabled": false, + "target": "folder", + }, + "local": map[string]any{ + "path": helper.ProvisioningPath, + }, + "connection": map[string]any{ + "name": "test-conn-for-field-selector", + }, + }, + }} + + _, err = helper.Repositories.Resource.Create(ctx, repoWithConnection, createOptions) + require.NoError(t, err, "failed to create repository with connection") + + // Create a repository WITHOUT the connection + repoWithoutConnection := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "provisioning.grafana.app/v0alpha1", + "kind": "Repository", + "metadata": map[string]any{ + "name": "repo-without-connection", + "namespace": "default", + }, + "spec": map[string]any{ + "title": "Repo Without Connection", + "type": "local", + "sync": map[string]any{ + "enabled": false, + "target": "folder", + }, + "local": map[string]any{ + "path": helper.ProvisioningPath, + }, + }, + }} + + _, err = helper.Repositories.Resource.Create(ctx, repoWithoutConnection, createOptions) + require.NoError(t, err, "failed to create repository without connection") + + // Create a repository with a DIFFERENT connection name (non-existent) + repoWithDifferentConnection := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "provisioning.grafana.app/v0alpha1", + "kind": "Repository", + "metadata": map[string]any{ + "name": "repo-with-different-connection", + "namespace": "default", + }, + "spec": map[string]any{ + "title": "Repo With Different Connection", + "type": "local", + "sync": map[string]any{ + "enabled": false, + "target": "folder", + }, + "local": map[string]any{ + "path": helper.ProvisioningPath, + }, + "connection": map[string]any{ + "name": "some-other-connection", + }, + }, + }} + + _, err = helper.Repositories.Resource.Create(ctx, repoWithDifferentConnection, createOptions) + require.NoError(t, err, "failed to create repository with different connection") + + t.Run("filter repositories by spec.connection.name", func(t *testing.T) { + // List repositories with field selector for the specific connection + list, err := helper.Repositories.Resource.List(ctx, metav1.ListOptions{ + FieldSelector: "spec.connection.name=test-conn-for-field-selector", + }) + require.NoError(t, err, "failed to list repositories with field selector") + + // Should only return the repository with the matching connection + assert.Len(t, list.Items, 1, "should return exactly one repository") + assert.Equal(t, "repo-with-connection", list.Items[0].GetName(), "should return the correct repository") + }) + + t.Run("filter repositories by non-existent connection returns empty", func(t *testing.T) { + // List repositories with field selector for a non-existent connection + list, err := helper.Repositories.Resource.List(ctx, metav1.ListOptions{ + FieldSelector: "spec.connection.name=non-existent-connection", + }) + require.NoError(t, err, "failed to list repositories with field selector") + + // Should return empty list + assert.Len(t, list.Items, 0, "should return no repositories for non-existent connection") + }) + + t.Run("filter repositories by empty connection name", func(t *testing.T) { + // List repositories with field selector for empty connection (repos without connection) + list, err := helper.Repositories.Resource.List(ctx, metav1.ListOptions{ + FieldSelector: "spec.connection.name=", + }) + require.NoError(t, err, "failed to list repositories with empty connection field selector") + + // Should return the repository without a connection + assert.Len(t, list.Items, 1, "should return exactly one repository without connection") + assert.Equal(t, "repo-without-connection", list.Items[0].GetName(), "should return the repository without connection") + }) + + t.Run("list all repositories without field selector", func(t *testing.T) { + // List all repositories without field selector + list, err := helper.Repositories.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err, "failed to list all repositories") + + // Should return all three repositories + assert.Len(t, list.Items, 3, "should return all three repositories") + + names := make([]string, len(list.Items)) + for i, item := range list.Items { + names[i] = item.GetName() + } + assert.Contains(t, names, "repo-with-connection") + assert.Contains(t, names, "repo-without-connection") + assert.Contains(t, names, "repo-with-different-connection") + }) +}