fix: change what latest chart actually means

internal/cmd/local/local/locate.go

@@ -1,14 +1,45 @@
 package local
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/pterm/pterm"
+	"golang.org/x/mod/semver"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/getter"
 	"helm.sh/helm/v3/pkg/repo"
 )
 
+// chartRepo exists only for testing purposes.
+// This allows the DownloadIndexFile method to be mocked.
+type chartRepo interface {
+	DownloadIndexFile() (string, error)
+}
+
+var _ chartRepo = (*repo.ChartRepository)(nil)
+
+// newChartRepo exists only for testing purposes.
+// This allows a test implementation of the repo.NewChartRepository function to exist.
+type newChartRepo func(cfg *repo.Entry, getters getter.Providers) (chartRepo, error)
+
+// loadIndexFile exists only for testing purposes.
+// This allows a test implementation of the repo.LoadIndexFile function to exist.
+type loadIndexFile func(path string) (*repo.IndexFile, error)
+
+// defaultNewChartRepo is the default implementation of the newChartRepo function.
+// It simply wraps the repo.NewChartRepository function.
+// This variable should only be modified for testing purposes.
+var defaultNewChartRepo newChartRepo = func(cfg *repo.Entry, getters getter.Providers) (chartRepo, error) {
+	return repo.NewChartRepository(cfg, getters)
+}
+
+// defaultLoadIndexFile is the default implementation of the loadIndexFile function.
+// It simply wraps the repo.LoadIndexFile function.
+// This variable should only be modified for testing purposes.
+var defaultLoadIndexFile loadIndexFile = repo.LoadIndexFile
+
 func locateLatestAirbyteChart(chartName, chartVersion string) string {
 	pterm.Debug.Printf("getting helm chart %q with version %q\n", chartName, chartVersion)
 
@@ -32,36 +63,54 @@ func locateLatestAirbyteChart(chartName, chartVersion string) string {
 }
 
 func getLatestAirbyteChartUrlFromRepoIndex(repoName, repoUrl string) (string, error) {
-	chartRepo, err := repo.NewChartRepository(&repo.Entry{
+	chartRepository, err := defaultNewChartRepo(&repo.Entry{
 		Name: repoName,
 		URL:  repoUrl,
 	}, getter.All(cli.New()))
 	if err != nil {
 		return "", fmt.Errorf("unable to access repo index: %w", err)
 	}
 
-	idxPath, err := chartRepo.DownloadIndexFile()
+	idxPath, err := chartRepository.DownloadIndexFile()
 	if err != nil {
 		return "", fmt.Errorf("unable to download index file: %w", err)
 	}
 
-	idx, err := repo.LoadIndexFile(idxPath)
+	idx, err := defaultLoadIndexFile(idxPath)
 	if err != nil {
 		return "", fmt.Errorf("unable to load index file (%s): %w", idxPath, err)
 	}
 
-	airbyteEntry, ok := idx.Entries["airbyte"]
+	entries, ok := idx.Entries["airbyte"]
 	if !ok {
 		return "", fmt.Errorf("no entry for airbyte in repo index")
 	}
 
-	if len(airbyteEntry) == 0 {
-		return "", fmt.Errorf("no chart version found")
+	if len(entries) == 0 {
+		return "", errors.New("no chart version found")
+	}
+
+	var latest *repo.ChartVersion
+	for _, entry := range entries {
+		version := entry.Version
+		// the semver library requires a `v` prefix
+		if !strings.HasPrefix(version, "v") {
+			version = "v" + version
+		}
+
+		if semver.Prerelease(version) == "" {
+			latest = entry
+			break
+		}
+	}
+
+	if latest == nil {
+		return "", fmt.Errorf("no valid version of airbyte chart found in repo index")
 	}
 
-	latest := airbyteEntry[0]
 	if len(latest.URLs) != 1 {
-		return "", fmt.Errorf("unexpected number of URLs")
+		return "", fmt.Errorf("unexpected number of URLs - %d", len(latest.URLs))
 	}
+
 	return airbyteRepoURL + "/" + latest.URLs[0], nil
 }

internal/cmd/local/local/locate_test.go

@@ -0,0 +1,120 @@
+package local
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/getter"
+	"helm.sh/helm/v3/pkg/repo"
+)
+
+func TestLocate(t *testing.T) {
+	origNewChartRepo := defaultNewChartRepo
+	origLoadIndexFile := defaultLoadIndexFile
+	t.Cleanup(func() {
+		defaultNewChartRepo = origNewChartRepo
+		defaultLoadIndexFile = origLoadIndexFile
+	})
+
+	defaultNewChartRepo = mockNewChartRepo
+
+	tests := []struct {
+		name    string
+		entries map[string]repo.ChartVersions
+		exp     string
+	}{
+		{
+			name: "one release entry",
+			entries: map[string]repo.ChartVersions{
+				"airbyte": []*repo.ChartVersion{{
+					Metadata: &chart.Metadata{Version: "1.2.3"},
+					URLs:     []string{"example.test"},
+				}},
+			},
+			exp: airbyteRepoURL + "/example.test",
+		},
+		{
+			name: "one non-release entry",
+			entries: map[string]repo.ChartVersions{
+				"airbyte": []*repo.ChartVersion{{
+					Metadata: &chart.Metadata{Version: "1.2.3-alpha-df72e2940ca"},
+					URLs:     []string{"example.test"},
+				}},
+			},
+			exp: airbyteChartName,
+		},
+		{
+			name:    "no entries",
+			entries: map[string]repo.ChartVersions{},
+			exp:     airbyteChartName,
+		},
+		{
+			name: "one release entry with no URLs",
+			entries: map[string]repo.ChartVersions{
+				"airbyte": []*repo.ChartVersion{{
+					Metadata: &chart.Metadata{Version: "1.2.3"},
+					URLs:     []string{},
+				}},
+			},
+			exp: airbyteChartName,
+		},
+		{
+			name: "one release entry with two URLs",
+			entries: map[string]repo.ChartVersions{
+				"airbyte": []*repo.ChartVersion{{
+					Metadata: &chart.Metadata{Version: "1.2.3"},
+					URLs:     []string{"one.test", "two.test"},
+				}},
+			},
+			exp: airbyteChartName,
+		},
+		{
+			name: "one non-release entry followed by one release entry",
+			entries: map[string]repo.ChartVersions{
+				"airbyte": []*repo.ChartVersion{
+					{
+						Metadata: &chart.Metadata{Version: "1.2.3-test"},
+						URLs:     []string{"bad.test"},
+					},
+					{
+						Metadata: &chart.Metadata{Version: "0.9.8"},
+						URLs:     []string{"good.test"},
+					},
+				},
+			},
+			exp: airbyteRepoURL + "/good.test",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defaultLoadIndexFile = mockLoadIndexFile(repo.IndexFile{Entries: tt.entries})
+			act := locateLatestAirbyteChart(airbyteChartName, "")
+			if d := cmp.Diff(tt.exp, act); d != "" {
+				t.Errorf("mismatch (-want +got):\n%s", d)
+			}
+		})
+	}
+}
+
+func mockNewChartRepo(cfg *repo.Entry, getters getter.Providers) (chartRepo, error) {
+	return mockChartRepo{}, nil
+}
+
+func mockLoadIndexFile(idxFile repo.IndexFile) loadIndexFile {
+	return func(path string) (*repo.IndexFile, error) {
+		return &idxFile, nil
+	}
+}
+
+type mockChartRepo struct {
+	downloadFile func() (string, error)
+}
+
+func (m mockChartRepo) DownloadIndexFile() (string, error) {
+	if m.downloadFile != nil {
+		return m.downloadFile()
+	}
+	return "", nil
+}