From 63c8fe287f8197b90eb4d578c6b7378226ca5aa3 Mon Sep 17 00:00:00 2001 From: Andrew Hackmann <5140848+bossinc@users.noreply.github.com> Date: Mon, 20 Oct 2025 10:35:30 -0500 Subject: [PATCH] Grafana Advisor: Prometheus Type Migration check (#110853) * add check for prom dep auth check in grafana advisor * remove non prom DS * clean up and add grafana docs links * lint * tests * Apply suggestions from code review Co-authored-by: Andres Martinez Gotor * Thank you for your great feedback @andresmgot * caching now resets on refresh. also check if plugin is installed * remove unused errors * add steps back sigh * make naming clearer --------- Co-authored-by: Andres Martinez Gotor --- .../pkg/app/checks/datasourcecheck/check.go | 81 ++++++++-- .../app/checks/datasourcecheck/check_test.go | 96 ++++++++++++ .../prom_dep_auth_check_step.go | 147 ++++++++++++++++++ 3 files changed, 312 insertions(+), 12 deletions(-) create mode 100644 apps/advisor/pkg/app/checks/datasourcecheck/prom_dep_auth_check_step.go diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check.go b/apps/advisor/pkg/app/checks/datasourcecheck/check.go index 38846740814..ed8aa35349b 100644 --- a/apps/advisor/pkg/app/checks/datasourcecheck/check.go +++ b/apps/advisor/pkg/app/checks/datasourcecheck/check.go @@ -3,6 +3,8 @@ package datasourcecheck import ( "context" "errors" + sysruntime "runtime" + "sync" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/apps/advisor/pkg/app/checks" @@ -18,15 +20,18 @@ const ( HealthCheckStepID = "health-check" UIDValidationStepID = "uid-validation" MissingPluginStepID = "missing-plugin" + PromDepAuthStepID = "prom-dep-auth" ) type check struct { - DatasourceSvc datasources.DataSourceService - PluginStore pluginstore.Store - PluginContextProvider pluginContextProvider - PluginClient plugins.Client - PluginRepo repo.Service - GrafanaVersion string + DatasourceSvc datasources.DataSourceService + PluginStore pluginstore.Store + PluginContextProvider pluginContextProvider + PluginClient plugins.Client + PluginRepo repo.Service + GrafanaVersion string + pluginCanBeInstalledCache map[string]bool + pluginExistsCacheMu sync.RWMutex } func New( @@ -38,12 +43,13 @@ func New( grafanaVersion string, ) checks.Check { return &check{ - DatasourceSvc: datasourceSvc, - PluginStore: pluginStore, - PluginContextProvider: pluginContextProvider, - PluginClient: pluginClient, - PluginRepo: pluginRepo, - GrafanaVersion: grafanaVersion, + DatasourceSvc: datasourceSvc, + PluginStore: pluginStore, + PluginContextProvider: pluginContextProvider, + PluginClient: pluginClient, + PluginRepo: pluginRepo, + GrafanaVersion: grafanaVersion, + pluginCanBeInstalledCache: make(map[string]bool), } } @@ -87,6 +93,7 @@ func (c *check) Name() string { } func (c *check) Init(ctx context.Context) error { + c.pluginCanBeInstalledCache = make(map[string]bool) return nil } @@ -102,9 +109,59 @@ func (c *check) Steps() []checks.Step { PluginRepo: c.PluginRepo, GrafanaVersion: c.GrafanaVersion, }, + &promDepAuthStep{ + canBeInstalled: c.canBeInstalled, + }, } } +// canBeInstalled checks if a plugin is already installed or if it's available in the plugin repository. +// Returns true if: +// - The plugin is NOT installed AND it IS available in the repository (can be installed) +// Returns false if: +// - The plugin is already installed, OR +// - The plugin is NOT available in the repository (nothing to install) +func (c *check) canBeInstalled(ctx context.Context, pluginType string) (bool, error) { + // Check cache first with read lock for performance + c.pluginExistsCacheMu.RLock() + if canBeInstalled, found := c.pluginCanBeInstalledCache[pluginType]; found { + c.pluginExistsCacheMu.RUnlock() + return canBeInstalled, nil + } + c.pluginExistsCacheMu.RUnlock() + + // Cache miss - acquire write lock and check again (double-checked locking pattern) + c.pluginExistsCacheMu.Lock() + defer c.pluginExistsCacheMu.Unlock() + + // Another goroutine may have populated the cache while we waited for the lock + if canBeInstalled, found := c.pluginCanBeInstalledCache[pluginType]; found { + return canBeInstalled, nil + } + + // Check if plugin is already installed + if _, isInstalled := c.PluginStore.Plugin(ctx, pluginType); isInstalled { + c.pluginCanBeInstalledCache[pluginType] = false + return false, nil + } + + // Plugin is not installed - check if it's available in the repository + availablePlugins, err := c.PluginRepo.GetPluginsInfo(ctx, repo.GetPluginsInfoOptions{ + IncludeDeprecated: true, + Plugins: []string{pluginType}, + }, repo.NewCompatOpts(c.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH)) + if err != nil { + // On error, assume plugin is installed/unavailable to avoid showing incorrect install links + return false, err + } + + // Plugin is not installed but IS available - return false to show install link + // Plugin is not installed and NOT available in repo - return true (nothing to install) + isAvailableInRepo := len(availablePlugins) > 0 + c.pluginCanBeInstalledCache[pluginType] = !isAvailableInRepo + return isAvailableInRepo, nil +} + type pluginContextProvider interface { GetWithDataSource(ctx context.Context, pluginID string, user identity.Requester, ds *datasources.DataSource) (backend.PluginContext, error) } diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go index fb02a8892ad..cd1e608af5b 100644 --- a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go +++ b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/repo" "github.com/grafana/grafana/pkg/services/datasources" @@ -230,6 +231,101 @@ func TestCheck_Run(t *testing.T) { assert.Equal(t, MissingPluginStepID, failures[0].StepID) assert.Len(t, failures[0].Links, 1) }) + + t.Run("should return failure when prometheus datasource uses SigV4 auth", func(t *testing.T) { + jsonData := simplejson.New() + jsonData.Set("sigV4Auth", true) + datasources := []*datasources.DataSource{ + {UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus", JsonData: jsonData}, + } + mockDatasourceSvc := &MockDatasourceSvc{dss: datasources} + mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}} + mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusOk}} + mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{ + {ID: 1, Slug: "prometheus", Status: "active"}, + {ID: 2, Slug: "grafana-amazonprometheus-datasource", Status: "active"}, + }} + mockPluginStore := &MockPluginStore{exists: true} + + check := &check{ + DatasourceSvc: mockDatasourceSvc, + PluginContextProvider: mockPluginContextProvider, + PluginClient: mockPluginClient, + PluginRepo: mockPluginRepo, + PluginStore: mockPluginStore, + GrafanaVersion: "11.0.0", + } + + failures, err := runChecks(check) + assert.NoError(t, err) + assert.Len(t, failures, 1) + assert.Equal(t, PromDepAuthStepID, failures[0].StepID) + assert.Contains(t, failures[0].Links, advisor.CheckErrorLink{ + Message: "View SigV4 docs", + Url: "https://grafana.com/docs/grafana-cloud/connect-externally-hosted/data-sources/prometheus/configure/aws-authentication/", + }) + }) + + t.Run("should return failure when prometheus datasource uses Azure auth", func(t *testing.T) { + jsonData := simplejson.New() + jsonData.Set("azureCredentials", map[string]interface{}{"authType": "msi"}) + datasources := []*datasources.DataSource{ + {UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus", JsonData: jsonData}, + } + mockDatasourceSvc := &MockDatasourceSvc{dss: datasources} + mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}} + mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusOk}} + mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{ + {ID: 1, Slug: "prometheus", Status: "active"}, + {ID: 2, Slug: "grafana-azureprometheus-datasource", Status: "active"}, + }} + mockPluginStore := &MockPluginStore{exists: true} + + check := &check{ + DatasourceSvc: mockDatasourceSvc, + PluginContextProvider: mockPluginContextProvider, + PluginClient: mockPluginClient, + PluginRepo: mockPluginRepo, + PluginStore: mockPluginStore, + GrafanaVersion: "11.0.0", + } + + failures, err := runChecks(check) + assert.NoError(t, err) + assert.Len(t, failures, 1) + assert.Equal(t, PromDepAuthStepID, failures[0].StepID) + assert.Contains(t, failures[0].Links, advisor.CheckErrorLink{ + Message: "View Azure auth docs", + Url: "https://grafana.com/docs/grafana-cloud/connect-externally-hosted/data-sources/prometheus/configure/azure-authentication/", + }) + }) + + t.Run("should not return failure when prometheus datasource does not use deprecated auth", func(t *testing.T) { + jsonData := simplejson.New() + datasources := []*datasources.DataSource{ + {UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus", JsonData: jsonData}, + } + mockDatasourceSvc := &MockDatasourceSvc{dss: datasources} + mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}} + mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusOk}} + mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{ + {ID: 1, Slug: "prometheus", Status: "active"}, + }} + mockPluginStore := &MockPluginStore{exists: true} + + check := &check{ + DatasourceSvc: mockDatasourceSvc, + PluginContextProvider: mockPluginContextProvider, + PluginClient: mockPluginClient, + PluginRepo: mockPluginRepo, + PluginStore: mockPluginStore, + GrafanaVersion: "11.0.0", + } + + failures, err := runChecks(check) + assert.NoError(t, err) + assert.Empty(t, failures) + }) } func TestCheck_Item(t *testing.T) { diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/prom_dep_auth_check_step.go b/apps/advisor/pkg/app/checks/datasourcecheck/prom_dep_auth_check_step.go new file mode 100644 index 00000000000..bc9dd1d0db1 --- /dev/null +++ b/apps/advisor/pkg/app/checks/datasourcecheck/prom_dep_auth_check_step.go @@ -0,0 +1,147 @@ +package datasourcecheck + +import ( + "context" + "fmt" + + "github.com/grafana/grafana-app-sdk/logging" + advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1" + "github.com/grafana/grafana/apps/advisor/pkg/app/checks" + "github.com/grafana/grafana/pkg/services/datasources" +) + +type promDepAuthStep struct { + canBeInstalled func(ctx context.Context, pluginType string) (bool, error) +} + +func (s *promDepAuthStep) Title() string { + return "Prometheus deprecated authentication check" +} + +func (s *promDepAuthStep) Description() string { + return "Check if Prometheus data sources are using deprecated authentication methods (Azure auth and SigV4)" +} + +func (s *promDepAuthStep) Resolution() string { + return fmt.Sprintf("Enable the feature toggle for 'prometheusTypeMigration'. If this feature toggle is already enabled, make sure that 'Azure Monitor Managed Service for Prometheus' and/or 'Amazon Managed Service for Prometheus' plugins are installed. If the data source is provisioned, edit data source type in the provisioning file to use '%s' or '%s'.", datasources.DS_AMAZON_PROMETHEUS, datasources.DS_AZURE_PROMETHEUS) +} + +func (s *promDepAuthStep) ID() string { + return PromDepAuthStepID +} + +func (s *promDepAuthStep) Run(ctx context.Context, log logging.Logger, obj *advisor.CheckSpec, item any) ([]advisor.CheckReportFailure, error) { + dataSource, ok := item.(*datasources.DataSource) + if !ok { + return nil, fmt.Errorf("invalid item type %T", item) + } + if dataSource.Type != datasources.DS_PROMETHEUS { + return nil, nil + } + if dataSource.JsonData == nil { + return nil, nil + } + + awsAuthLinks, err := s.checkUsingAWSAuth(ctx, dataSource) + if err != nil { + return nil, err + } + azureAuthLinks, err := s.checkUsingAzureAuth(ctx, dataSource) + if err != nil { + return nil, err + } + + errorLinks := append(awsAuthLinks, azureAuthLinks...) + + if len(errorLinks) == 0 { + return nil, nil + } + + return []advisor.CheckReportFailure{checks.NewCheckReportFailureWithMoreInfo( + advisor.CheckReportFailureSeverityHigh, + s.ID(), + dataSource.Name, + dataSource.UID, + errorLinks, + fmt.Sprintf("Datasource %s (UID: %s) is of type %s but it's using a deprecated authentication method so it should be migrated", dataSource.Name, dataSource.UID, dataSource.Type), + )}, nil +} + +func (s *promDepAuthStep) checkUsingAWSAuth(ctx context.Context, dataSource *datasources.DataSource) ([]advisor.CheckErrorLink, error) { + var errorLinks []advisor.CheckErrorLink + if sigV4Auth, found := dataSource.JsonData.CheckGet("sigV4Auth"); found { + if enabled, err := sigV4Auth.Bool(); err != nil || !enabled { + // Disabled or not a valid boolean + return nil, nil + } + readOnlyLink := checkReadOnly(dataSource) + + if readOnlyLink != nil { + errorLinks = append(errorLinks, *readOnlyLink) + } + + errorLinks = append(errorLinks, + advisor.CheckErrorLink{ + Message: "View SigV4 docs", + Url: "https://grafana.com/docs/grafana-cloud/connect-externally-hosted/data-sources/prometheus/configure/aws-authentication/", + }) + pluginLink := s.linkDataSource(ctx, datasources.DS_AMAZON_PROMETHEUS, "Amazon Managed Service for Prometheus") + if pluginLink != nil { + errorLinks = append(errorLinks, *pluginLink) + } + } + return errorLinks, nil +} + +func (s *promDepAuthStep) checkUsingAzureAuth(ctx context.Context, dataSource *datasources.DataSource) ([]advisor.CheckErrorLink, error) { + var errorLinks []advisor.CheckErrorLink + if azureAuth, found := dataSource.JsonData.CheckGet("azureCredentials"); found { + if _, err := azureAuth.Value(); err != nil { + // azureAuth does not have a value + return nil, nil + } + readOnlyLink := checkReadOnly(dataSource) + if readOnlyLink != nil { + errorLinks = append(errorLinks, *readOnlyLink) + } + errorLinks = append(errorLinks, + advisor.CheckErrorLink{ + Message: "View Azure auth docs", + Url: "https://grafana.com/docs/grafana-cloud/connect-externally-hosted/data-sources/prometheus/configure/azure-authentication/", + }) + pluginLink := s.linkDataSource(ctx, datasources.DS_AZURE_PROMETHEUS, "Azure Monitor Managed Service for Prometheus") + if pluginLink != nil { + errorLinks = append(errorLinks, *pluginLink) + } + } + return errorLinks, nil +} + +func checkReadOnly(dataSource *datasources.DataSource) *advisor.CheckErrorLink { + if readOnly, found := dataSource.JsonData.CheckGet("readonly"); found { + if enabled, err := readOnly.Bool(); err != nil || !enabled { + // Disabled or not a valid boolean + return nil + } + return &advisor.CheckErrorLink{ + Message: "Change provisioning file", + Url: "https://grafana.com/docs/grafana/latest/administration/provisioning/#data-sources", + } + } + return nil +} + +func (s *promDepAuthStep) linkDataSource(ctx context.Context, pluginType string, pluginName string) *advisor.CheckErrorLink { + canBeInstalled, err := s.canBeInstalled(ctx, pluginType) + if err != nil { + return nil + } + if canBeInstalled { + // Plugin is available in the repo + return &advisor.CheckErrorLink{ + Message: fmt.Sprintf("Install %s", pluginName), + Url: fmt.Sprintf("/plugins/%s", pluginType), + } + } + return nil +}