From e73530da09eb9cac69dbdb605c861e7e908b4519 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Thu, 12 Jun 2025 10:15:40 +0200 Subject: [PATCH] Advisor: Add a check for pinned instances (#106059) --- .../pkg/app/checkregistry/checkregistry.go | 2 + .../pkg/app/checks/instancechecks/check.go | 72 ++++++++ .../app/checks/instancechecks/check_test.go | 63 +++++++ .../instancechecks/out_of_support_step.go | 122 ++++++++++++++ .../out_of_support_step_test.go | 157 ++++++++++++++++++ .../instancechecks/pinned_version_step.go | 62 +++++++ .../pinned_version_step_test.go | 79 +++++++++ 7 files changed, 557 insertions(+) create mode 100644 apps/advisor/pkg/app/checks/instancechecks/check.go create mode 100644 apps/advisor/pkg/app/checks/instancechecks/check_test.go create mode 100644 apps/advisor/pkg/app/checks/instancechecks/out_of_support_step.go create mode 100644 apps/advisor/pkg/app/checks/instancechecks/out_of_support_step_test.go create mode 100644 apps/advisor/pkg/app/checks/instancechecks/pinned_version_step.go create mode 100644 apps/advisor/pkg/app/checks/instancechecks/pinned_version_step_test.go diff --git a/apps/advisor/pkg/app/checkregistry/checkregistry.go b/apps/advisor/pkg/app/checkregistry/checkregistry.go index 9fbf0ab66ef..1ec5f075fc7 100644 --- 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), } } diff --git a/apps/advisor/pkg/app/checks/instancechecks/check.go b/apps/advisor/pkg/app/checks/instancechecks/check.go new file mode 100644 index 00000000000..34b606d1c37 --- /dev/null +++ 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, + }, + } +} diff --git a/apps/advisor/pkg/app/checks/instancechecks/check_test.go b/apps/advisor/pkg/app/checks/instancechecks/check_test.go new file mode 100644 index 00000000000..00c38c4b0a5 --- /dev/null +++ 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]) + }) + } +} diff --git a/apps/advisor/pkg/app/checks/instancechecks/out_of_support_step.go b/apps/advisor/pkg/app/checks/instancechecks/out_of_support_step.go new file mode 100644 index 00000000000..5a15d775eb0 --- /dev/null +++ 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. " + + "" + + "Learn more about version support." +} + +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) +} diff --git 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 new file mode 100644 index 00000000000..9785fbfd196 --- /dev/null +++ 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 +} diff --git a/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step.go b/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step.go new file mode 100644 index 00000000000..a07b772625e --- /dev/null +++ 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 " + + "support ticket 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 +} diff --git a/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step_test.go b/apps/advisor/pkg/app/checks/instancechecks/pinned_version_step_test.go new file mode 100644 index 00000000000..f6dbda63a30 --- /dev/null +++ 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) + } + }) + } +}