fix: avoid helm conflict with local directory (#99)

Author: abuchanan-airbyte

internal/cmd/local/local/cmd.go

@@ -62,18 +62,22 @@ type HTTPClient interface {
 // BrowserLauncher primarily for testing purposes.
 type BrowserLauncher func(url string) error
 
+// ChartLocator primarily for testing purposes.
+type ChartLocator func(repoName, repoUrl string) string
+
 // Command is the local command, responsible for installing, uninstalling, or other local actions.
 type Command struct {
-	provider k8s.Provider
-	cluster  k8s.Cluster
-	http     HTTPClient
-	helm     helm.Client
-	k8s      k8s.Client
-	portHTTP int
-	spinner  *pterm.SpinnerPrinter
-	tel      telemetry.Client
-	launcher BrowserLauncher
-	userHome string
+	provider    k8s.Provider
+	cluster     k8s.Cluster
+	http        HTTPClient
+	helm        helm.Client
+	k8s         k8s.Client
+	portHTTP    int
+	spinner     *pterm.SpinnerPrinter
+	tel         telemetry.Client
+	launcher    BrowserLauncher
+	locateChart ChartLocator
+	userHome    string
 }
 
 // Option for configuring the Command, primarily exists for testing
@@ -114,6 +118,12 @@ func WithBrowserLauncher(launcher BrowserLauncher) Option {
 	}
 }
 
+func WithChartLocator(locator ChartLocator) Option {
+	return func(c *Command) {
+		c.locateChart = locator
+	}
+}
+
 // WithUserHome define the user's home directory.
 func WithUserHome(home string) Option {
 	return func(c *Command) {
@@ -140,6 +150,10 @@ func New(provider k8s.Provider, opts ...Option) (*Command, error) {
 		opt(c)
 	}
 
+	if c.locateChart == nil {
+		c.locateChart = locateLatestAirbyteChart
+	}
+
 	// determine userhome if not defined
 	if c.userHome == "" {
 		c.userHome = paths.UserHome
@@ -698,11 +712,13 @@ func (c *Command) handleChart(
 		return fmt.Errorf("unable to add %s chart repo: %w", req.name, err)
 	}
 
-	c.spinner.UpdateText(fmt.Sprintf("Fetching %s Helm Chart", req.chartName))
-	helmChart, _, err := c.helm.GetChart(req.chartName, &action.ChartPathOptions{Version: req.chartVersion})
+	c.spinner.UpdateText(fmt.Sprintf("Fetching %s Helm Chart with version", req.chartName))
+
+	chartLoc := c.locateChart(req.chartName, req.chartVersion)
+
+	helmChart, _, err := c.helm.GetChart(chartLoc, &action.ChartPathOptions{Version: req.chartVersion})
 	if err != nil {
-		pterm.Error.Printfln("Unable to fetch %s Helm Chart", req.chartName)
-		return fmt.Errorf("unable to fetch chart %s: %w", req.chartName, err)
+		return fmt.Errorf("unable to fetch helm chart %q: %w", req.chartName, err)
 	}
 
 	c.tel.Attr(fmt.Sprintf("helm_%s_chart_version", req.name), helmChart.Metadata.Version)
@@ -741,7 +757,7 @@ func (c *Command) handleChart(
 	))
 	helmRelease, err := c.helm.InstallOrUpgradeChart(ctx, &helmclient.ChartSpec{
 		ReleaseName:     req.chartRelease,
-		ChartName:       req.chartName,
+		ChartName:       chartLoc,
 		CreateNamespace: true,
 		Namespace:       req.namespace,
 		Wait:            true,

internal/cmd/local/local/cmd_test.go

@@ -26,6 +26,14 @@ import (
 )
 
 const portTest = 9999
+const testAirbyteChartLoc = "https://airbytehq.github.io/helm-charts/airbyte-1.2.3.tgz"
+
+func testChartLocator(chartName, chartVersion string) string {
+	if chartName == airbyteChartName && chartVersion == "" {
+		return testAirbyteChartLoc
+	}
+	return chartName
+}
 
 func TestCommand_Install(t *testing.T) {
 	expChartRepoCnt := 0
@@ -48,7 +56,7 @@ func TestCommand_Install(t *testing.T) {
 		{
 			chart: helmclient.ChartSpec{
 				ReleaseName:     airbyteChartRelease,
-				ChartName:       airbyteChartName,
+				ChartName:       testAirbyteChartLoc,
 				Namespace:       airbyteNamespace,
 				CreateNamespace: true,
 				Wait:            true,
@@ -106,7 +114,7 @@ func TestCommand_Install(t *testing.T) {
 
 		getChart: func(name string, _ *action.ChartPathOptions) (*chart.Chart, string, error) {
 			switch {
-			case name == airbyteChartName:
+			case name == testAirbyteChartLoc:
 				return &chart.Chart{Metadata: &chart.Metadata{Version: "test.airbyte.version"}}, "", nil
 			case name == nginxChartName:
 				return &chart.Chart{Metadata: &chart.Metadata{Version: "test.nginx.version"}}, "", nil
@@ -183,6 +191,7 @@ func TestCommand_Install(t *testing.T) {
 		WithBrowserLauncher(func(url string) error {
 			return nil
 		}),
+		WithChartLocator(testChartLocator),
 	)
 
 	if err != nil {
@@ -215,7 +224,7 @@ func TestCommand_Install_ValuesFile(t *testing.T) {
 		{
 			chart: helmclient.ChartSpec{
 				ReleaseName:     airbyteChartRelease,
-				ChartName:       airbyteChartName,
+				ChartName:       testAirbyteChartLoc,
 				Namespace:       airbyteNamespace,
 				CreateNamespace: true,
 				Wait:            true,
@@ -274,7 +283,7 @@ func TestCommand_Install_ValuesFile(t *testing.T) {
 
 		getChart: func(name string, _ *action.ChartPathOptions) (*chart.Chart, string, error) {
 			switch {
-			case name == airbyteChartName:
+			case name == testAirbyteChartLoc:
 				return &chart.Chart{Metadata: &chart.Metadata{Version: "test.airbyte.version"}}, "", nil
 			case name == nginxChartName:
 				return &chart.Chart{Metadata: &chart.Metadata{Version: "test.nginx.version"}}, "", nil
@@ -351,6 +360,7 @@ func TestCommand_Install_ValuesFile(t *testing.T) {
 		WithBrowserLauncher(func(url string) error {
 			return nil
 		}),
+		WithChartLocator(testChartLocator),
 	)
 
 	if err != nil {

internal/cmd/local/local/locate.go

@@ -0,0 +1,67 @@
+package local
+
+import (
+	"fmt"
+
+	"github.com/pterm/pterm"
+	"helm.sh/helm/v3/pkg/cli"
+	"helm.sh/helm/v3/pkg/getter"
+	"helm.sh/helm/v3/pkg/repo"
+)
+
+func locateLatestAirbyteChart(chartName, chartVersion string) string {
+	pterm.Debug.Printf("getting helm chart %q with version %q\n", chartName, chartVersion)
+
+	// Helm will consider a local directory path named "airbyte/airbyte" to be a chart repo,
+	// but it might not be, which causes errors like "Chart.yaml file is missing".
+	// This trips up plenty of people, see: https://github.com/helm/helm/issues/7862
+	//
+	// Here we avoid that problem by figuring out the full URL of the airbyte chart,
+	// which forces Helm to resolve the chart over HTTP and ignore local directories.
+	// If the locator fails, fall back to the original helm behavior.
+	if chartName == airbyteChartName && chartVersion == "" {
+		if url, err := getLatestAirbyteChartUrlFromRepoIndex(airbyteRepoName, airbyteRepoURL); err == nil {
+			pterm.Debug.Printf("determined latest airbyte chart url: %s\n", url)
+			return url
+		} else {
+			pterm.Debug.Printf("error determining latest airbyte chart, falling back to default behavior: %s\n", err)
+		}
+	}
+
+	return chartName
+}
+
+func getLatestAirbyteChartUrlFromRepoIndex(repoName, repoUrl string) (string, error) {
+	chartRepo, err := repo.NewChartRepository(&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()
+	if err != nil {
+		return "", fmt.Errorf("unable to download index file: %w", err)
+	}
+
+	idx, err := repo.LoadIndexFile(idxPath)
+	if err != nil {
+		return "", fmt.Errorf("unable to load index file (%s): %w", idxPath, err)
+	}
+
+	airbyteEntry, 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")
+	}
+
+	latest := airbyteEntry[0]
+	if len(latest.URLs) != 1 {
+		return "", fmt.Errorf("unexpected number of URLs")
+	}
+	return airbyteRepoURL + "/" + latest.URLs[0], nil
+}