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

Provisioning: Avoid using listers.RepositoryLister outside a controller (#110948)

Browse Source 
* use raw storage

* avoid informer cached lister
This commit is contained in:
Ryan McKinley
2025-09-12 11:23:02 +02:00
committed by GitHub
parent b747ec8f24
commit 1004b26a4a
3 changed files with 37 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/provisioning/register.go
+32 -15
View File
@@ -10,14 +10,15 @@ import (
"github.com/prometheus/client_golang/prometheus"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
"k8s.io/kube-openapi/pkg/common"
@@ -29,21 +30,20 @@ import (
dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
appcontroller "github.com/grafana/grafana/apps/provisioning/pkg/controller"
clientset "github.com/grafana/grafana/apps/provisioning/pkg/generated/clientset/versioned"
client "github.com/grafana/grafana/apps/provisioning/pkg/generated/clientset/versioned/typed/provisioning/v0alpha1"
informers "github.com/grafana/grafana/apps/provisioning/pkg/generated/informers/externalversions"
listers "github.com/grafana/grafana/apps/provisioning/pkg/generated/listers/provisioning/v0alpha1"
"github.com/grafana/grafana/apps/provisioning/pkg/loki"
"github.com/grafana/grafana/apps/provisioning/pkg/repository"
"github.com/grafana/grafana/pkg/apimachinery/identity"
apiutils "github.com/grafana/grafana/pkg/apimachinery/utils"
grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/infra/usagestats"
"github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/controller"
appcontroller "github.com/grafana/grafana/apps/provisioning/pkg/controller"
"github.com/grafana/grafana/apps/provisioning/pkg/loki"
"github.com/grafana/grafana/apps/provisioning/pkg/repository"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs"
deletepkg "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/delete"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs/export"
@@ -87,7 +87,7 @@ type APIBuilder struct {
usageStats usagestats.Service
tracer tracing.Tracer
getter rest.Getter
store grafanarest.Storage
parsers resources.ParserFactory
repositoryResources resources.RepositoryResourcesFactory
clients resources.ClientFactory
@@ -98,7 +98,6 @@ type APIBuilder struct {
jobHistoryConfig *JobHistoryConfig
jobHistoryLoki *jobs.LokiJobHistory
resourceLister resources.ResourceLister
repositoryLister listers.RepositoryLister
legacyMigrator legacy.LegacyMigrator
storageStatus dualwrite.Service
unified resource.ResourceClient
@@ -403,7 +402,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI
return fmt.Errorf("failed to create repository storage: %w", err)
}
repositoryStatusStorage := grafanaregistry.NewRegistryStatusStore(opts.Scheme, repositoryStorage)
b.getter = repositoryStorage
b.store = repositoryStorage
jobStore, err := grafanaregistry.NewCompleteRegistryStore(opts.Scheme, provisioning.JobResourceInfo, opts.OptsGetter)
if err != nil {
@@ -564,7 +563,7 @@ func (b *APIBuilder) Validate(ctx context.Context, a admission.Attributes, o adm
}
// Exit early if we have already found errors
targetError := b.verifyAgaintsExistingRepositories(cfg)
targetError := b.verifyAgainstExistingRepositories(cfg)
if targetError != nil {
return invalidRepositoryError(a.GetName(), field.ErrorList{targetError})
}
@@ -578,9 +577,28 @@ func invalidRepositoryError(name string, list field.ErrorList) error {
name, list)
}
func (b *APIBuilder) getRepositoriesInNamespace(ctx context.Context) ([]provisioning.Repository, error) {
obj, err := b.store.List(ctx, &internalversion.ListOptions{
Limit: 100,
})
if err != nil {
return nil, err
}
all, ok := obj.(*provisioning.RepositoryList)
if !ok {
return nil, fmt.Errorf("expected repository list")
}
return all.Items, nil
}
// TODO: move this to a more appropriate place. Probably controller/validation.go
func (b *APIBuilder) verifyAgaintsExistingRepositories(cfg *provisioning.Repository) *field.Error {
all, err := b.repositoryLister.Repositories(cfg.Namespace).List(labels.Everything())
func (b *APIBuilder) verifyAgainstExistingRepositories(cfg *provisioning.Repository) *field.Error {
ctx, _, err := identity.WithProvisioningIdentity(context.Background(), cfg.Namespace)
if err != nil {
return &field.Error{Type: field.ErrorTypeInternal, Detail: err.Error()}
}
all, err := b.getRepositoriesInNamespace(request.WithNamespace(ctx, cfg.Namespace))
if err != nil {
return field.Forbidden(field.NewPath("spec"),
"Unable to verify root target: "+err.Error())
@@ -633,7 +651,6 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH
jobInformer := sharedInformerFactory.Provisioning().V0alpha1().Jobs()
b.client = c.ProvisioningV0alpha1()
b.repositoryLister = repoInformer.Lister()
// Initialize the API client-based job store
b.jobs, err = jobs.NewJobStore(b.client, 30*time.Second)
@@ -659,7 +676,7 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH
}
// Create the repository resources factory
usageMetricCollector := usage.MetricCollector(b.tracer, b.repositoryLister, b.unified)
usageMetricCollector := usage.MetricCollector(b.tracer, b.getRepositoriesInNamespace, b.unified)
b.usageStats.RegisterMetricsFunc(usageMetricCollector)
stageIfPossible := repository.WrapWithStageAndPushIfPossible
@@ -1231,7 +1248,7 @@ func (b *APIBuilder) tryRunningOnlyUnifiedStorage() error {
// TODO: where should the helpers live?
func (b *APIBuilder) GetRepository(ctx context.Context, name string) (repository.Repository, error) {
obj, err := b.getter.Get(ctx, name, &metav1.GetOptions{})
obj, err := b.store.Get(ctx, name, &metav1.GetOptions{})
if err != nil {
return nil, err
}
pkg/registry/apis/provisioning/routes.go
+2 -2
View File
@@ -6,8 +6,8 @@ import (
"net/http"
"time"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/kube-openapi/pkg/spec3"
"k8s.io/kube-openapi/pkg/validation/spec"
@@ -150,7 +150,7 @@ func (b *APIBuilder) handleSettings(w http.ResponseWriter, r *http.Request) {
}
// TODO: check if lister could list too many repositories or resources
all, err := b.repositoryLister.Repositories(u.GetNamespace()).List(labels.Everything())
all, err := b.getRepositoriesInNamespace(request.WithNamespace(r.Context(), u.GetNamespace()))
if err != nil {
errhttp.Write(r.Context(), err, w)
return
pkg/registry/apis/provisioning/usage/usage.go
+3 -4
View File
@@ -6,10 +6,9 @@ import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apiserver/pkg/endpoints/request"
listers "github.com/grafana/grafana/apps/provisioning/pkg/generated/listers/provisioning/v0alpha1"
provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/infra/usagestats"
@@ -17,7 +16,7 @@ import (
"github.com/grafana/grafana/pkg/storage/unified/resourcepb"
)
func MetricCollector(tracer tracing.Tracer, repositoryLister listers.RepositoryLister, unified resource.ResourceClient) usagestats.MetricsFunc {
func MetricCollector(tracer tracing.Tracer, repositoryLister func(ctx context.Context) ([]provisioning.Repository, error), unified resource.ResourceClient) usagestats.MetricsFunc {
return func(ctx context.Context) (metrics map[string]any, err error) {
ctx, span := tracer.Start(ctx, "Provisioning.Usage.collectProvisioningStats")
defer func() {
@@ -65,7 +64,7 @@ func MetricCollector(tracer tracing.Tracer, repositoryLister listers.RepositoryL
}
// Inspect all configs
repos, err := repositoryLister.List(labels.Everything())
repos, err := repositoryLister(ctx)
if err != nil {
return m, fmt.Errorf("list repositories: %w", err)
}