Cache all versions by source (#2779)

Author: pkwarren

internal/cmd/fetcher/main.go

@@ -13,7 +13,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"slices"
 	"strings"
 	"time"
 
@@ -94,7 +93,8 @@ func (f *pluginFilter) includes(org, name string) bool {
 
 // Fetcher is an interface for fetching plugin versions from external sources.
 type Fetcher interface {
-	Fetch(ctx context.Context, config *source.Config) (string, error)
+	// FetchVersions return all non-prerelease versions for the specified source.
+	FetchVersions(ctx context.Context, src *source.Source) ([]string, error)
 }
 
 func main() {
@@ -646,7 +646,7 @@ func fetchPendingCreations(
 	versionTime func(ctx context.Context, path string) (time.Time, error),
 ) (map[string]*pluginToCreate, error) {
 	filter := newPluginFilter(includes)
-	latestVersions := make(map[string]string, len(configs))
+	versionsByCacheKey := make(map[string][]string, len(configs))
 	pendingCreations := make(map[string]*pluginToCreate)
 
 	for _, config := range configs {
@@ -670,35 +670,29 @@ func fetchPendingCreations(
 				continue
 			}
 		}
-		newVersion := latestVersions[config.CacheKey()]
-		if newVersion == "" {
+		versions, ok := versionsByCacheKey[config.CacheKey()]
+		if !ok {
 			var err error
-			newVersion, err = fetcher.Fetch(ctx, config)
+			versions, err = fetcher.FetchVersions(ctx, &config.Source)
 			if err != nil {
-				if errors.Is(err, fetchclient.ErrSemverPrerelease) {
-					logger.InfoContext(ctx, "skipping source", slog.String("filename", config.Filename), slog.Any("error", err))
-					continue
-				}
-				return nil, err
+				return nil, fmt.Errorf("%s: %w", config.Filename, err)
 			}
-			latestVersions[config.CacheKey()] = newVersion
+			versionsByCacheKey[config.CacheKey()] = versions
 		}
-		// Some plugins share the same source but specify different ignore versions.
-		// Ensure we continue to only fetch the latest version once but still respect ignores.
-		if slices.Contains(config.Source.IgnoreVersions, newVersion) {
-			logger.InfoContext(ctx, "skipping source", slog.String("filename", config.Filename), slog.String("version", newVersion))
-			continue
+		newVersion, err := config.Source.LatestVersion(versions)
+		if err != nil {
+			return nil, fmt.Errorf("%s: %w", config.Filename, err)
 		}
 		// Convert to absolute path to match plugin.Walk behavior (which converts paths via filepath.Abs)
 		pluginDir, err := filepath.Abs(filepath.Dir(config.Filename))
 		if err != nil {
 			return nil, err
 		}
-		ok, err := checkDirExists(filepath.Join(pluginDir, newVersion))
+		exists, err := checkDirExists(filepath.Join(pluginDir, newVersion))
 		if err != nil {
 			return nil, err
 		}
-		if ok {
+		if exists {
 			continue
 		}
 		previousVersion, err := getLatestVersionFromDir(pluginDir)

internal/cmd/fetcher/main_test.go

@@ -203,9 +203,9 @@ func TestRunDependencyOrdering(t *testing.T) {
 	// Mock fetcher that returns new versions for our test plugins
 	// Cache keys are formatted as "github-owner-repository"
 	fetcher := &mockFetcher{
-		versions: map[string]string{
-			"github-test-base-plugin":     "v2.0.0",
-			"github-test-consumer-plugin": "v2.0.0",
+		versions: map[string][]string{
+			"github-test-base-plugin":     {"v1.0.0", "v2.0.0"},
+			"github-test-consumer-plugin": {"v1.0.0", "v2.0.0"},
 		},
 	}
 
@@ -244,9 +244,9 @@ func TestRunUpdateFrequency(t *testing.T) {
     repository: base-plugin
 `
 	fetcher := &mockFetcher{
-		versions: map[string]string{
-			"github-test-base-plugin":     "v2.0.0",
-			"github-test-consumer-plugin": "v2.0.0",
+		versions: map[string][]string{
+			"github-test-base-plugin":     {"v1.0.0", "v2.0.0"},
+			"github-test-consumer-plugin": {"v1.0.0", "v2.0.0"},
 		},
 	}
 
@@ -294,16 +294,14 @@ func TestRunUpdateFrequency(t *testing.T) {
 
 // mockFetcher returns predetermined versions for testing.
 type mockFetcher struct {
-	versions map[string]string // maps cache key (e.g., "github-owner-repo") -> version to return
+	versions map[string][]string // maps cache key (e.g., "github-owner-repo") -> versions to return
 }
 
-func (m *mockFetcher) Fetch(_ context.Context, config *source.Config) (string, error) {
-	key := config.CacheKey()
-	if version, ok := m.versions[key]; ok {
-		return version, nil
+func (m *mockFetcher) FetchVersions(_ context.Context, src *source.Source) ([]string, error) {
+	if versions, ok := m.versions[src.CacheKey()]; ok {
+		return versions, nil
 	}
-	// Return a default version if not in map
-	return "v1.0.0", nil
+	return []string{"v1.0.0"}, nil
 }
 
 // setupTestRepository creates a complete test repository structure with:
@@ -614,3 +612,45 @@ func newTestContainer(t *testing.T, root string) appext.Container {
 	logger := slog.New(slog.NewTextHandler(testWriter{t}, &slog.HandlerOptions{Level: slog.LevelDebug}))
 	return appext.NewContainer(nameContainer, logger, appext.LogLevelDebug, appext.LogFormatText)
 }
+
+func TestFetchPendingCreationsMaxVersionSharedSource(t *testing.T) {
+	t.Parallel()
+	pluginsDir := filepath.Join(t.TempDir(), "plugins")
+	writePlugin := func(name string, sourceYAML string) {
+		pluginDir := filepath.Join(pluginsDir, "test", name)
+		require.NoError(t, os.MkdirAll(filepath.Join(pluginDir, "v1.0.0"), 0755))
+		require.NoError(t, os.WriteFile(filepath.Join(pluginDir, "source.yaml"), []byte(sourceYAML), 0644))
+	}
+	writePlugin("unbounded", `source:
+  github:
+    owner: test
+    repository: shared
+`)
+	writePlugin("bounded", `source:
+  github:
+    owner: test
+    repository: shared
+  max_version: 2.0.0
+`)
+	configs, err := source.GatherConfigs(pluginsDir)
+	require.NoError(t, err)
+	fetcher := &mockFetcher{
+		versions: map[string][]string{
+			"github-test-shared": {"v1.0.0", "v1.1.0", "v2.0.0"},
+		},
+	}
+	logger := slog.New(slog.DiscardHandler)
+	pending, err := fetchPendingCreations(t.Context(), logger, fetcher, configs, nil, nil)
+	require.NoError(t, err)
+	require.Len(t, pending, 2)
+	for _, config := range configs {
+		pluginDir, err := filepath.Abs(filepath.Dir(config.Filename))
+		require.NoError(t, err)
+		require.Contains(t, pending, pluginDir)
+		want := "v2.0.0"
+		if config.Source.MaxVersion != "" {
+			want = "v1.1.0"
+		}
+		assert.Equal(t, want, pending[pluginDir].newVersion, pluginDir)
+	}
+}