From 1d2e445fdbfbbf9584d5cbab3d13ae8649e7e94f Mon Sep 17 00:00:00 2001 From: Alex Buchanan Date: Tue, 3 Sep 2024 14:34:56 -0700 Subject: [PATCH] fix: avoid helm conflict with local directory (#99) --- internal/cmd/local/local/cmd.go | 46 ++++++++++++------- internal/cmd/local/local/cmd_test.go | 18 ++++++-- internal/cmd/local/local/locate.go | 67 ++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 19 deletions(-) create mode 100644 internal/cmd/local/local/locate.go diff --git a/internal/cmd/local/local/cmd.go b/internal/cmd/local/local/cmd.go index b06d1de..3ec22f7 100644 --- a/internal/cmd/local/local/cmd.go +++ b/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, diff --git a/internal/cmd/local/local/cmd_test.go b/internal/cmd/local/local/cmd_test.go index 153163c..64f0a52 100644 --- a/internal/cmd/local/local/cmd_test.go +++ b/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 { diff --git a/internal/cmd/local/local/locate.go b/internal/cmd/local/local/locate.go new file mode 100644 index 0000000..8550db9 --- /dev/null +++ b/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 +}