Advisor: Documentation and config check (#105929)

apps/advisor/pkg/app/checkregistry/checkregistry.go

@@ -3,6 +3,7 @@ package checkregistry
 import (
 	"github.com/grafana/grafana/apps/advisor/pkg/app/checks"
 	"github.com/grafana/grafana/apps/advisor/pkg/app/checks/authchecks"
+	"github.com/grafana/grafana/apps/advisor/pkg/app/checks/configchecks"
 	"github.com/grafana/grafana/apps/advisor/pkg/app/checks/datasourcecheck"
 	"github.com/grafana/grafana/apps/advisor/pkg/app/checks/plugincheck"
 	"github.com/grafana/grafana/pkg/plugins"
@@ -33,13 +34,14 @@ type Service struct {
 	provisionedPlugins    provisionedplugins.Manager
 	ssoSettingsSvc        ssosettings.Service
 	GrafanaVersion        string
+	cfg                   *setting.Cfg
 }
 
 func ProvideService(datasourceSvc datasources.DataSourceService, pluginStore pluginstore.Store,
 	pluginContextProvider *plugincontext.Provider, pluginClient plugins.Client,
 	updateChecker pluginchecker.PluginUpdateChecker,
 	pluginRepo repo.Service, pluginPreinstall pluginchecker.Preinstall, managedPlugins managedplugins.Manager,
-	provisionedPlugins provisionedplugins.Manager, ssoSettingsSvc ssosettings.Service, settings *setting.Cfg,
+	provisionedPlugins provisionedplugins.Manager, ssoSettingsSvc ssosettings.Service, cfg *setting.Cfg,
 ) *Service {
 	return &Service{
 		datasourceSvc:         datasourceSvc,
@@ -52,7 +54,8 @@ func ProvideService(datasourceSvc datasources.DataSourceService, pluginStore plu
 		managedPlugins:        managedPlugins,
 		provisionedPlugins:    provisionedPlugins,
 		ssoSettingsSvc:        ssoSettingsSvc,
-		GrafanaVersion:        settings.BuildVersion,
+		GrafanaVersion:        cfg.BuildVersion,
+		cfg:                   cfg,
 	}
 }
 
@@ -73,6 +76,7 @@ func (s *Service) Checks() []checks.Check {
 			s.GrafanaVersion,
 		),
 		authchecks.New(s.ssoSettingsSvc),
+		configchecks.New(s.cfg),
 	}
 }
 

apps/advisor/pkg/app/checks/configchecks/check.go

@@ -0,0 +1,48 @@
+package configchecks
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/apps/advisor/pkg/app/checks"
+	"github.com/grafana/grafana/pkg/setting"
+)
+
+var _ checks.Check = (*check)(nil)
+
+type check struct {
+	cfg *setting.Cfg
+}
+
+func New(cfg *setting.Cfg) checks.Check {
+	return &check{
+		cfg: cfg,
+	}
+}
+
+func (c *check) ID() string {
+	return "config"
+}
+
+func (c *check) Name() string {
+	return "config setting"
+}
+
+func (c *check) Items(ctx context.Context) ([]any, error) {
+	return []any{"security.secret_key"}, nil
+}
+
+func (c *check) Item(ctx context.Context, id string) (any, error) {
+	return id, nil
+}
+
+func (c *check) Init(ctx context.Context) error {
+	return nil
+}
+
+func (c *check) Steps() []checks.Step {
+	return []checks.Step{
+		&securityConfigStep{
+			securitySection: c.cfg.SectionWithEnvOverrides("security"),
+		},
+	}
+}

apps/advisor/pkg/app/checks/configchecks/security_config_step.go

@@ -0,0 +1,73 @@
+package configchecks
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"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/setting"
+)
+
+const (
+	// nolint:gosec // Defined in defaults.ini originally
+	defaultSecretKey = "SW2YcwTIb9zpOOhoPsMm"
+)
+
+type securityConfigStep struct {
+	securitySection *setting.DynamicSection
+}
+
+func (s *securityConfigStep) Title() string {
+	return "Security config check"
+}
+
+func (s *securityConfigStep) Description() string {
+	return "Check if the Grafana security config is set correctly."
+}
+
+func (s *securityConfigStep) Resolution() string {
+	return "Follow the documentation for each element."
+}
+
+func (s *securityConfigStep) ID() string {
+	return "security_config"
+}
+
+func (s *securityConfigStep) Run(ctx context.Context, log logging.Logger, _ *advisor.CheckSpec, it any) ([]advisor.CheckReportFailure, error) {
+	itemPath, ok := it.(string)
+	if !ok {
+		return nil, fmt.Errorf("invalid item type %T", it)
+	}
+	items := strings.Split(itemPath, ".")
+	if len(items) != 2 {
+		// Not interested in this item
+		return nil, nil
+	}
+	section, key := items[0], items[1]
+	if section != "security" {
+		// Only interested in security section
+		return nil, nil
+	}
+	if key == "secret_key" {
+		secretKey := s.securitySection.Key("secret_key").Value()
+		if secretKey == defaultSecretKey {
+			return []advisor.CheckReportFailure{checks.NewCheckReportFailure(
+				advisor.CheckReportFailureSeverityHigh,
+				s.ID(),
+				"secret_key",
+				itemPath,
+				[]advisor.CheckErrorLink{
+					{
+						Message: "Avoid default value",
+						Url:     "https://grafana.com/docs/grafana/latest/setup-grafana/configure-security/configure-database-encryption/",
+					},
+				},
+			)}, nil
+		}
+	}
+
+	return nil, nil
+}

apps/advisor/pkg/app/checks/configchecks/security_config_step_test.go

@@ -0,0 +1,33 @@
+package configchecks
+
+import (
+	"context"
+	"testing"
+
+	"github.com/grafana/grafana-app-sdk/logging"
+	"github.com/grafana/grafana/pkg/setting"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSecurityConfigStepSuccess(t *testing.T) {
+	cfg := setting.NewCfg()
+	step := &securityConfigStep{
+		securitySection: cfg.SectionWithEnvOverrides("security"),
+	}
+
+	errs, err := step.Run(context.Background(), logging.DefaultLogger, nil, "security.secret_key")
+	require.NoError(t, err)
+	require.Len(t, errs, 0)
+}
+
+func TestSecurityConfigStepFailure(t *testing.T) {
+	cfg := setting.NewCfg()
+	cfg.SectionWithEnvOverrides("security").Key("secret_key").SetValue(defaultSecretKey)
+	step := &securityConfigStep{
+		securitySection: cfg.SectionWithEnvOverrides("security"),
+	}
+
+	errs, err := step.Run(context.Background(), logging.DefaultLogger, nil, "security.secret_key")
+	require.NoError(t, err)
+	require.Len(t, errs, 1)
+}