From a51d1b76b435a7a33011fd3a7cc065af2d30ad91 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 25 Sep 2025 00:38:27 +0300 Subject: [PATCH] Dashboards: dependency cleanup (#111572) --- pkg/registry/apis/dashboard/authorizer.go | 3 +- pkg/registry/apis/dashboard/register.go | 41 +++++-------------- .../apis/dashboard/schema_validation.go | 3 +- pkg/registry/apis/dashboard/sub_dto.go | 7 +--- pkg/server/wire_gen.go | 4 +- .../integration/api_validation_test.go | 12 ++---- pkg/tests/testinfra/testinfra.go | 5 +++ 7 files changed, 25 insertions(+), 50 deletions(-) diff --git a/pkg/registry/apis/dashboard/authorizer.go b/pkg/registry/apis/dashboard/authorizer.go index 05dfc6a7e61..07ad6bfda19 100644 --- a/pkg/registry/apis/dashboard/authorizer.go +++ b/pkg/registry/apis/dashboard/authorizer.go @@ -8,13 +8,12 @@ import ( "github.com/grafana/authlib/types" dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/libraryelements" ) -func newLegacyAuthorizer(ac accesscontrol.AccessControl, l log.Logger) authorizer.Authorizer { +func newLegacyAuthorizer(ac accesscontrol.AccessControl) authorizer.Authorizer { return authorizer.AuthorizerFunc( func(ctx context.Context, attr authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) { // Note that we will return Allow more than expected. diff --git a/pkg/registry/apis/dashboard/register.go b/pkg/registry/apis/dashboard/register.go index 0bd80b9c4b7..1f295b29d21 100644 --- a/pkg/registry/apis/dashboard/register.go +++ b/pkg/registry/apis/dashboard/register.go @@ -20,6 +20,7 @@ import ( "k8s.io/kube-openapi/pkg/validation/spec" authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana-app-sdk/logging" internal "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard" dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" dashv1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" @@ -33,7 +34,6 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/utils" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacysearcher" @@ -48,7 +48,6 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/librarypanels" - "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/services/provisioning" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/search/sort" @@ -103,12 +102,10 @@ type DashboardsAPIBuilder struct { dashStore dashboards.Store QuotaService quota.Service ProvisioningService provisioning.ProvisioningService - cfg *setting.Cfg + minRefreshInterval string dualWriter dualwrite.Service folderClientProvider client.K8sHandlerProvider - log log.Logger - reg prometheus.Registerer isStandalone bool // skips any handling including anything to do with legacy storage } @@ -118,7 +115,6 @@ func RegisterAPIService( apiregistration builder.APIRegistrar, dashboardService dashboards.DashboardService, provisioningDashboardService dashboards.DashboardProvisioningService, - pluginStore pluginstore.Store, datasourceService datasources.DataSourceService, dashboardPermissions dashboards.PermissionsRegistrationService, dashboardPermissionsSvc accesscontrol.DashboardPermissionsService, @@ -142,10 +138,8 @@ func RegisterAPIService( legacyDashboardSearcher := legacysearcher.NewDashboardSearchClient(dashStore, sorter) folderClient := client.NewK8sHandler(dual, request.GetNamespaceMapper(cfg), folders.FolderResourceInfo.GroupVersionResource(), restConfigProvider.GetRestConfig, dashStore, userService, unified, sorter, features) - dashLog := log.New("grafana-apiserver.dashboards") builder := &DashboardsAPIBuilder{ - log: dashLog, - authorizer: newLegacyAuthorizer(accessControl, dashLog), + authorizer: newLegacyAuthorizer(accessControl), dashboardService: dashboardService, dashboardPermissions: dashboardPermissions, dashboardPermissionsSvc: dashboardPermissionsSvc, @@ -158,7 +152,7 @@ func RegisterAPIService( dashStore: dashStore, QuotaService: quotaService, ProvisioningService: provisioning, - cfg: cfg, + minRefreshInterval: cfg.MinRefreshInterval, dualWriter: dual, folderClientProvider: newSimpleFolderClientProvider(folderClient), @@ -166,7 +160,6 @@ func RegisterAPIService( Access: legacy.NewDashboardAccess(dbp, namespacer, dashStore, provisioning, libraryPanelSvc, sorter, dashboardPermissionsSvc, accessControl, features), DashboardService: dashboardService, }, - reg: reg, } migration.RegisterMetrics(reg) @@ -177,24 +170,10 @@ func RegisterAPIService( return builder } -func NewAPIService(ac authlib.AccessClient, features featuremgmt.FeatureToggles, folderClientProvider client.K8sHandlerProvider, datasourceProvider schemaversion.DataSourceInfoProvider, pluginStore *pluginstore.Service) *DashboardsAPIBuilder { - // TODO: Plugin store will soon be removed, - // as the cases for plugin fetching is not needed. Keeping it now to not break implementation - if pluginStore == nil { - panic("pluginStore is nil") - } - - logger := log.New("grafana-apiserver.dashboards") - +func NewAPIService(ac authlib.AccessClient, features featuremgmt.FeatureToggles, folderClientProvider client.K8sHandlerProvider, datasourceProvider schemaversion.DataSourceInfoProvider) *DashboardsAPIBuilder { migration.Initialize(datasourceProvider) - return &DashboardsAPIBuilder{ - log: logger, - reg: prometheus.NewRegistry(), - - cfg: &setting.Cfg{ - MinRefreshInterval: "10s", - }, + minRefreshInterval: "10s", accessClient: ac, authorizer: authsvc.NewResourceAuthorizer(ac), features: features, @@ -336,7 +315,7 @@ func (b *DashboardsAPIBuilder) validateCreate(ctx context.Context, a admission.A } // Validate refresh interval - if err := b.dashboardService.ValidateDashboardRefreshInterval(b.cfg.MinRefreshInterval, refresh); err != nil { + if err := b.dashboardService.ValidateDashboardRefreshInterval(b.minRefreshInterval, refresh); err != nil { return apierrors.NewBadRequest(err.Error()) } @@ -422,7 +401,7 @@ func (b *DashboardsAPIBuilder) validateUpdate(ctx context.Context, a admission.A } // Validate refresh interval - if err := b.dashboardService.ValidateDashboardRefreshInterval(b.cfg.MinRefreshInterval, refresh); err != nil { + if err := b.dashboardService.ValidateDashboardRefreshInterval(b.minRefreshInterval, refresh); err != nil { return apierrors.NewBadRequest(err.Error()) } @@ -592,7 +571,7 @@ func (b *DashboardsAPIBuilder) storageForVersion( return nil } - legacyStore, err := b.legacy.NewStore(dashboards, opts.Scheme, opts.OptsGetter, b.reg, b.dashboardPermissions, b.accessClient) + legacyStore, err := b.legacy.NewStore(dashboards, opts.Scheme, opts.OptsGetter, opts.MetricsRegister, b.dashboardPermissions, b.accessClient) if err != nil { return err } @@ -692,7 +671,7 @@ func (b *DashboardsAPIBuilder) verifyFolderAccessPermissions(ctx context.Context var accessInfo folders.FolderAccessInfo err = runtime.DefaultUnstructuredConverter.FromUnstructured(resp.Object, &accessInfo) if err != nil { - b.log.Error("Failed to convert folder access response", "error", err) + logging.FromContext(ctx).Error("Failed to convert folder access response", "error", err) return dashboards.ErrFolderAccessDenied } diff --git a/pkg/registry/apis/dashboard/schema_validation.go b/pkg/registry/apis/dashboard/schema_validation.go index 4263d2af388..c53ed82481a 100644 --- a/pkg/registry/apis/dashboard/schema_validation.go +++ b/pkg/registry/apis/dashboard/schema_validation.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" + "github.com/grafana/grafana-app-sdk/logging" v0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" v1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" v2alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" @@ -62,7 +63,7 @@ func (b *DashboardsAPIBuilder) ValidateDashboardSpec(ctx context.Context, obj ru } if alwaysLogSchemaValidationErrors && len(errors) > 0 { - b.log.Info("Schema validation errors during dashboard validation", "group_version", obj.GetObjectKind().GroupVersionKind().GroupVersion().String(), "name", accessor.GetName(), "errors", errors.ToAggregate().Error(), "schema_version_mismatch", schemaVersionError != nil) + logging.FromContext(ctx).Info("Schema validation errors during dashboard validation", "group_version", obj.GetObjectKind().GroupVersionKind().GroupVersion().String(), "name", accessor.GetName(), "errors", errors.ToAggregate().Error(), "schema_version_mismatch", schemaVersionError != nil) } if errorOnSchemaMismatches { diff --git a/pkg/registry/apis/dashboard/sub_dto.go b/pkg/registry/apis/dashboard/sub_dto.go index 80ec5c8f12c..d3023664f65 100644 --- a/pkg/registry/apis/dashboard/sub_dto.go +++ b/pkg/registry/apis/dashboard/sub_dto.go @@ -9,9 +9,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" - claims "github.com/grafana/authlib/types" + authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" - "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" @@ -145,14 +144,12 @@ func (r *DTOConnector) Connect(ctx context.Context, name string, opts runtime.Ob access.CanAdmin, _ = r.accessControl.Evaluate(ctx, user, adminEvaluator) deleteEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsDelete, dashScope) access.CanDelete, _ = r.accessControl.Evaluate(ctx, user, deleteEvaluator) - access.CanStar = user.IsIdentityType(claims.TypeUser) + access.CanStar = user.IsIdentityType(authlib.TypeUser) access.AnnotationsPermissions = &dashboard.AnnotationPermission{} r.getAnnotationPermissionsByScope(ctx, user, &access.AnnotationsPermissions.Dashboard, accesscontrol.ScopeAnnotationsTypeDashboard) r.getAnnotationPermissionsByScope(ctx, user, &access.AnnotationsPermissions.Organization, accesscontrol.ScopeAnnotationsTypeOrganization) - // FIXME!!!! does not get the title! - // The title property next to unstructured and not found in this model title := obj.FindTitle("") access.Slug = slugify.Slugify(title) access.Url = dashboards.GetDashboardFolderURL(false, name, access.Slug) diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 11a6de3b807..40e49ead04c 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -809,7 +809,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api identitySynchronizer := authnimpl.ProvideIdentitySynchronizer(authnimplService) ldapImpl := service12.ProvideService(cfg, featureToggles, ssosettingsimplService) apiService := api4.ProvideService(cfg, routeRegisterImpl, accessControl, userService, authinfoimplService, ossGroups, identitySynchronizer, orgService, ldapImpl, userAuthTokenService, bundleregistryService) - dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, pluginstoreService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) + dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) snapshotsAPIBuilder := dashboardsnapshot.RegisterAPIService(serviceImpl, apiserverService, cfg, featureToggles, sqlStore, registerer) featureFlagAPIBuilder := featuretoggle.RegisterAPIService(featureManager, accessControl, apiserverService, cfg, registerer) dataSourceAPIBuilder, err := datasource.RegisterAPIService(featureToggles, apiserverService, middlewareHandler, scopedPluginDatasourceProvider, plugincontextProvider, pluginstoreService, accessControl, registerer) @@ -1417,7 +1417,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac identitySynchronizer := authnimpl.ProvideIdentitySynchronizer(authnimplService) ldapImpl := service12.ProvideService(cfg, featureToggles, ssosettingsimplService) apiService := api4.ProvideService(cfg, routeRegisterImpl, accessControl, userService, authinfoimplService, ossGroups, identitySynchronizer, orgService, ldapImpl, userAuthTokenService, bundleregistryService) - dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, pluginstoreService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) + dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) snapshotsAPIBuilder := dashboardsnapshot.RegisterAPIService(serviceImpl, apiserverService, cfg, featureToggles, sqlStore, registerer) featureFlagAPIBuilder := featuretoggle.RegisterAPIService(featureManager, accessControl, apiserverService, cfg, registerer) dataSourceAPIBuilder, err := datasource.RegisterAPIService(featureToggles, apiserverService, middlewareHandler, scopedPluginDatasourceProvider, plugincontextProvider, pluginstoreService, accessControl, registerer) diff --git a/pkg/tests/apis/dashboard/integration/api_validation_test.go b/pkg/tests/apis/dashboard/integration/api_validation_test.go index 9239ce78309..6423f8edf56 100644 --- a/pkg/tests/apis/dashboard/integration/api_validation_test.go +++ b/pkg/tests/apis/dashboard/integration/api_validation_test.go @@ -647,12 +647,9 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) { }) t.Run("Dashboard refresh interval validations", func(t *testing.T) { - // Store original settings to restore after test - origCfg := ctx.Helper.GetEnv().Cfg - origMinRefreshInterval := origCfg.MinRefreshInterval - - // Set a fixed min_refresh_interval for all tests to make them predictable - ctx.Helper.GetEnv().Cfg.MinRefreshInterval = "10s" + // Test infrastructure is configured with + // [dashboards] + // min_refresh_interval = 10s testCases := []struct { name string @@ -720,9 +717,6 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) { } }) } - - // Restore original settings - ctx.Helper.GetEnv().Cfg.MinRefreshInterval = origMinRefreshInterval }) t.Run("Dashboard size limit validations", func(t *testing.T) { diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 0b026b7e185..bac5f5208f7 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -545,6 +545,11 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { require.NoError(t, err) } + dashboardsSection, err := getOrCreateSection("dashboards") + require.NoError(t, err) + _, err = dashboardsSection.NewKey("min_refresh_interval", "10s") + require.NoError(t, err) + if opts.APIServerRuntimeConfig != "" { section, err := getOrCreateSection("grafana-apiserver") require.NoError(t, err)