Advisor: Add a check for pinned instances (#106059)

2025-06-12 10:15:40 +02:00 · 2025-06-12 10:15:40 +02:00 · e73530da09
parent 44e7be134d
commit e73530da09
7 changed files with 557 additions and 0 deletions
--- a/apps/advisor/pkg/app/checkregistry/checkregistry.go
+++ b/apps/advisor/pkg/app/checkregistry/checkregistry.go
@ -5,6 +5,7 @@ import (
 	"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/instancechecks"
 	"github.com/grafana/grafana/apps/advisor/pkg/app/checks/plugincheck"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/repo"
@ -81,6 +82,7 @@ func (s *Service) Checks() []checks.Check {
 		),
 		authchecks.New(s.ssoSettingsSvc),
 		configchecks.New(s.cfg),
+		instancechecks.New(s.cfg),
 	}
 }

--- a/apps/advisor/pkg/app/checks/instancechecks/check.go
+++ b/apps/advisor/pkg/app/checks/instancechecks/check.go
@ -0,0 +1,72 @@
+package instancechecks
+
+import (
+	"context"
+	"time"
+
+	"github.com/google/go-github/v70/github"
+	"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
+	isCloudInstance bool
+}
+
+func New(cfg *setting.Cfg) checks.Check {
+	return &check{
+		cfg:             cfg,
+		isCloudInstance: cfg.StackID != "",
+	}
+}
+
+func (c *check) ID() string {
+	return "instance"
+}
+
+func (c *check) Name() string {
+	return "instance attribute"
+}
+
+func (c *check) Items(ctx context.Context) ([]any, error) {
+	if c.isCloudInstance {
+		return []any{
+			pinnedVersion, // pinned version check
+		}, nil
+	}
+
+	return []any{
+		outOfSupportVersion, // out of support version check
+	}, 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 {
+	// If running in cloud, we need to check if the version is pinned
+	if c.isCloudInstance {
+		return []checks.Step{
+			&pinnedVersionStep{
+				BuildBranch: c.cfg.BuildBranch,
+			},
+		}
+	}
+
+	// If running in self-managed, we need to check if the version is out of support
+	return []checks.Step{
+		&outOfSupportVersionStep{
+			GrafanaVersion: c.cfg.BuildVersion,
+			BuildDate:      time.Unix(c.cfg.BuildStamp, 0).UTC(),
+			ghClient:       github.NewClient(nil).Repositories,
+		},
+	}
+}
--- a/apps/advisor/pkg/app/checks/instancechecks/check_test.go
+++ b/apps/advisor/pkg/app/checks/instancechecks/check_test.go
@ -0,0 +1,63 @@
+package instancechecks
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/grafana/grafana/pkg/setting"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCheck_IsCloudInstance_Logic(t *testing.T) {
+	tests := []struct {
+		name           string
+		stackID        string
+		expectedCloud  bool
+		expectedStepID string
+		expectedItemID string
+	}{
+		{
+			name:           "empty stackID should run out of support check",
+			stackID:        "",
+			expectedCloud:  false,
+			expectedStepID: outOfSupportVersion,
+			expectedItemID: outOfSupportVersion,
+		},
+		{
+			name:           "non-empty stackID should run pinned version check",
+			stackID:        "12345",
+			expectedCloud:  true,
+			expectedStepID: pinnedVersion,
+			expectedItemID: pinnedVersion,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &setting.Cfg{
+				StackID:      tt.stackID,
+				BuildBranch:  "v10.0.0",
+				BuildVersion: "10.0.0",
+				BuildStamp:   time.Now().Unix(),
+			}
+
+			check := New(cfg).(*check)
+
+			// Verify isCloudInstance field is set correctly
+			assert.Equal(t, tt.expectedCloud, check.isCloudInstance)
+
+			// Verify Steps() returns the correct step type
+			steps := check.Steps()
+			require.Len(t, steps, 1)
+			assert.Equal(t, tt.expectedStepID, steps[0].ID())
+
+			// Verify Items() returns the correct item type
+			items, err := check.Items(context.Background())
+			require.NoError(t, err)
+			require.Len(t, items, 1)
+			assert.Equal(t, tt.expectedItemID, items[0])
+		})
+	}
+}
--- a/apps/advisor/pkg/app/checks/instancechecks/out_of_support_step.go
+++ b/apps/advisor/pkg/app/checks/instancechecks/out_of_support_step.go
@ -0,0 +1,122 @@
+package instancechecks
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/Masterminds/semver/v3"
+	"github.com/google/go-github/v70/github"
+	"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"
+)
+
+var _ checks.Step = &outOfSupportVersionStep{}
+
+const (
+	outOfSupportVersion = "out_of_support_version"
+)
+
+type outOfSupportVersionStep struct {
+	GrafanaVersion string
+	BuildDate      time.Time
+	ghClient       gitHubClient
+}
+
+func (s *outOfSupportVersionStep) Title() string {
+	return "Grafana version check"
+}
+
+func (s *outOfSupportVersionStep) Description() string {
+	return "Check if the current Grafana version is out of support."
+}
+
+func (s *outOfSupportVersionStep) Resolution() string {
+	return "Out of support versions will not receive security updates or bug fixes. " +
+		"Upgrade to a more recent version. " +
+		"<a href='https://grafana.com/docs/grafana/latest/upgrade-guide/when-to-upgrade/#what-to-know-about-version-support' target='_blank'>" +
+		"Learn more about version support</a>."
+}
+
+func (s *outOfSupportVersionStep) ID() string {
+	return outOfSupportVersion
+}
+
+func (s *outOfSupportVersionStep) Run(ctx context.Context, log logging.Logger, _ *advisor.CheckSpec, it any) ([]advisor.CheckReportFailure, error) {
+	item, ok := it.(string)
+	if !ok {
+		return nil, fmt.Errorf("invalid item type %T", it)
+	}
+	if item != outOfSupportVersion {
+		// Not interested in this item
+		return nil, nil
+	}
+
+	// If the build date is less than 9 months old, it's supported
+	if s.BuildDate.After(time.Now().AddDate(0, -9, 0)) {
+		return nil, nil
+	}
+
+	// If the build date is older than 15 months, it's not supported
+	isOutOfSupport := false
+	if s.BuildDate.Before(time.Now().AddDate(0, -15, 0)) {
+		isOutOfSupport = true
+	} else {
+		// In other cases, we need to check if the version is out of support.
+		// Minor versions are generally supported for 9 months but the last
+		// minor version for a major version is supported for 15 months.
+		// This is the case when we keep doing releases for old minor versions (e.g. 11.x when 12.x is out).
+		// https://grafana.com/docs/grafana/latest/upgrade-guide/when-to-upgrade/#what-to-know-about-version-support
+
+		// Parse the current version using semver
+		version, err := semver.NewVersion(s.GrafanaVersion)
+		if err != nil {
+			// Unable to parse the version so unable to check if it's out of support
+			log.Error("Unable to parse the version", "version", s.GrafanaVersion, "error", err)
+			return nil, nil
+		}
+
+		// To verify if the current version is the latest minor version for this major version,
+		// we try to get the version vX.Y+1.0 from GitHub
+		nextMinorVersion := fmt.Sprintf("v%d.%d.0", version.Major(), version.Minor()+1)
+		_, res, err := s.ghClient.GetReleaseByTag(ctx, "grafana", "grafana", nextMinorVersion)
+		if err != nil && res.StatusCode != 404 {
+			// Unable to get the release info so unable to check if it's out of support
+			log.Error("Unable to get the release info", "version", s.GrafanaVersion, "error", err.Error())
+			return nil, nil
+		}
+		latestMinor := false
+		if res.StatusCode == 404 {
+			// No next minor version found, so the current version is the latest minor version
+			latestMinor = true
+		}
+
+		// Calculate support end date
+		supportMonths := 9
+		if latestMinor {
+			supportMonths = 15 // Extended support for last minor version
+		}
+
+		supportEndDate := s.BuildDate.AddDate(0, supportMonths, 0)
+		if time.Now().After(supportEndDate) {
+			isOutOfSupport = true
+		}
+	}
+
+	if isOutOfSupport {
+		return []advisor.CheckReportFailure{checks.NewCheckReportFailure(
+			advisor.CheckReportFailureSeverityHigh,
+			s.ID(),
+			fmt.Sprintf("Grafana version %s is out of support", s.GrafanaVersion),
+			outOfSupportVersion,
+			[]advisor.CheckErrorLink{},
+		)}, nil
+	}
+	return nil, nil
+}
+
+// gitHubClient is a mockable interface for the GitHub client
+type gitHubClient interface {
+	GetReleaseByTag(ctx context.Context, owner, repo, tag string) (*github.RepositoryRelease, *github.Response, error)
+}
--- a/apps/advisor/pkg/app/checks/instancechecks/out_of_support_step_test.go
+++ b/apps/advisor/pkg/app/checks/instancechecks/out_of_support_step_test.go
@ -0,0 +1,157 @@
+package instancechecks
+
+import (
+	"context"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/google/go-github/v70/github"
+	"github.com/grafana/grafana-app-sdk/logging"
+	advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestOutOfSupportVersionStep(t *testing.T) {
+	now := time.Now()
+	oldDate := now.AddDate(0, -16, 0)                          // 16 months ago
+	recentDate := now.AddDate(0, -8, 0)                        // 8 months ago
+	oldDateButSupportedIfLatestMinor := now.AddDate(0, -10, 0) // 10 months ago
+
+	tests := []struct {
+		name        string
+		step        *outOfSupportVersionStep
+		input       any
+		wantFailure bool
+		wantErr     bool
+	}{
+		{
+			name: "should return error for invalid input type",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "9.5.0",
+				BuildDate:      now,
+				ghClient:       &mockGitHubClient{},
+			},
+			input:   123, // invalid type
+			wantErr: true,
+		},
+		{
+			name: "should return nil for non-out-of-support item",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "9.5.0",
+				BuildDate:      now,
+				ghClient:       &mockGitHubClient{},
+			},
+			input:       "other_item",
+			wantFailure: false,
+			wantErr:     false,
+		},
+		{
+			name: "should return nil for recent build date",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "11.5.0",
+				BuildDate:      recentDate,
+				ghClient:       &mockGitHubClient{},
+			},
+			input:       outOfSupportVersion,
+			wantFailure: false,
+			wantErr:     false,
+		},
+		{
+			name: "should return failure for old build date",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "9.5.0",
+				BuildDate:      oldDate,
+				ghClient:       &mockGitHubClient{},
+			},
+			input:       outOfSupportVersion,
+			wantFailure: true,
+			wantErr:     false,
+		},
+		{
+			name: "should return nil for invalid version format",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "invalid-version",
+				BuildDate:      oldDateButSupportedIfLatestMinor,
+				ghClient:       &mockGitHubClient{},
+			},
+			input:       outOfSupportVersion,
+			wantFailure: false,
+			wantErr:     false,
+		},
+		{
+			name: "should return nil for GitHub API error",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "9.5.0",
+				BuildDate:      oldDateButSupportedIfLatestMinor,
+				ghClient: &mockGitHubClient{
+					errCode: http.StatusTooManyRequests,
+				},
+			},
+			input:       outOfSupportVersion,
+			wantFailure: false,
+			wantErr:     false,
+		},
+		{
+			name: "should return failure for non-latest minor version",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "9.5.0",
+				BuildDate:      oldDateButSupportedIfLatestMinor,
+				ghClient: &mockGitHubClient{
+					release: &github.RepositoryRelease{},
+				},
+			},
+			input:       outOfSupportVersion,
+			wantFailure: true,
+			wantErr:     false,
+		},
+		{
+			name: "should return nil for latest minor version",
+			step: &outOfSupportVersionStep{
+				GrafanaVersion: "9.5.0",
+				BuildDate:      oldDateButSupportedIfLatestMinor,
+				ghClient: &mockGitHubClient{
+					errCode: http.StatusNotFound,
+				},
+			},
+			input:       outOfSupportVersion,
+			wantFailure: false,
+			wantErr:     false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			failures, err := tt.step.Run(context.Background(), logging.DefaultLogger, &advisor.CheckSpec{}, tt.input)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+
+			if tt.wantFailure {
+				require.Len(t, failures, 1)
+				assert.Equal(t, advisor.CheckReportFailureSeverityHigh, failures[0].Severity)
+				assert.Equal(t, outOfSupportVersion, failures[0].ItemID)
+				assert.Equal(t, "Grafana version "+tt.step.GrafanaVersion+" is out of support", failures[0].Item)
+			} else {
+				assert.Empty(t, failures)
+			}
+		})
+	}
+}
+
+type mockGitHubClient struct {
+	release *github.RepositoryRelease
+	errCode int
+}
+
+func (m *mockGitHubClient) GetReleaseByTag(ctx context.Context, owner, repo, tag string) (*github.RepositoryRelease, *github.Response, error) {
+	if m.errCode != 0 {
+		return nil, &github.Response{Response: &http.Response{StatusCode: m.errCode}}, assert.AnError
+	}
+	return m.release, &github.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil
+}
--- a/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step.go
+++ b/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step.go
@ -0,0 +1,62 @@
+package instancechecks
+
+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"
+)
+
+var _ checks.Step = &pinnedVersionStep{}
+
+const (
+	pinnedVersion = "pinned_version"
+)
+
+type pinnedVersionStep struct {
+	BuildBranch string
+}
+
+func (s *pinnedVersionStep) Title() string {
+	return "Grafana Cloud version check"
+}
+
+func (s *pinnedVersionStep) Description() string {
+	return "Check if the Grafana version is pinned."
+}
+
+func (s *pinnedVersionStep) Resolution() string {
+	return "You may miss out on security updates and bug fixes if you use a pinned version. " +
+		"Contact your Grafana administrator and open a " +
+		"<a href='https://grafana.com/profile/org#support' target=_blank>support ticket</a> to help you get unpinned."
+}
+
+func (s *pinnedVersionStep) ID() string {
+	return pinnedVersion
+}
+
+func (s *pinnedVersionStep) Run(ctx context.Context, log logging.Logger, _ *advisor.CheckSpec, it any) ([]advisor.CheckReportFailure, error) {
+	item, ok := it.(string)
+	if !ok {
+		return nil, fmt.Errorf("invalid item type %T", it)
+	}
+	if item != pinnedVersion {
+		// Not interested in this item
+		return nil, nil
+	}
+
+	if s.BuildBranch == "HEAD" {
+		// Not a pinned version
+		return nil, nil
+	}
+
+	return []advisor.CheckReportFailure{checks.NewCheckReportFailure(
+		advisor.CheckReportFailureSeverityLow,
+		s.ID(),
+		"Grafana version is pinned",
+		"pinned_version",
+		[]advisor.CheckErrorLink{},
+	)}, nil
+}
--- a/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step_test.go
+++ b/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step_test.go
@ -0,0 +1,79 @@
+package instancechecks
+
+import (
+	"context"
+	"testing"
+
+	"github.com/grafana/grafana-app-sdk/logging"
+	advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPinnedVersionStep(t *testing.T) {
+	tests := []struct {
+		name        string
+		step        *pinnedVersionStep
+		input       any
+		wantFailure bool
+		wantErr     bool
+	}{
+		{
+			name: "should return error for invalid input type",
+			step: &pinnedVersionStep{
+				BuildBranch: "test-branch",
+			},
+			input:   123, // invalid type
+			wantErr: true,
+		},
+		{
+			name: "should return nil for non-pinned version item",
+			step: &pinnedVersionStep{
+				BuildBranch: "test-branch",
+			},
+			input:       "other_item",
+			wantFailure: false,
+			wantErr:     false,
+		},
+		{
+			name: "should return nil for HEAD build branch",
+			step: &pinnedVersionStep{
+				BuildBranch: "HEAD",
+			},
+			input:       pinnedVersion,
+			wantFailure: false,
+			wantErr:     false,
+		},
+		{
+			name: "should return failure for pinned version",
+			step: &pinnedVersionStep{
+				BuildBranch: "v9.5.0",
+			},
+			input:       pinnedVersion,
+			wantFailure: true,
+			wantErr:     false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			failures, err := tt.step.Run(context.Background(), logging.DefaultLogger, &advisor.CheckSpec{}, tt.input)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+
+			if tt.wantFailure {
+				require.Len(t, failures, 1)
+				assert.Equal(t, advisor.CheckReportFailureSeverityLow, failures[0].Severity)
+				assert.Equal(t, "pinned_version", failures[0].ItemID)
+				assert.Equal(t, "Grafana version is pinned", failures[0].Item)
+			} else {
+				assert.Empty(t, failures)
+			}
+		})
+	}
+}