From 57cfbb82b8c2e2a38d4441d0d451b871522df7d6 Mon Sep 17 00:00:00 2001 From: Max Goltzsche Date: Sun, 30 Oct 2022 02:47:23 +0200 Subject: [PATCH] fix: wire up git getter within cli Also, disable git getter by default --- README.md | 7 +++++-- cmd/khelm/root.go | 30 +++++++++++++++++++++++++----- cmd/khelm/template_test.go | 14 ++++++++++++++ e2e/cli-tests.bats | 17 +++++++++++++---- pkg/getter/git/gitgetter.go | 5 ++++- pkg/helm/helm.go | 6 ++---- pkg/helm/load.go | 4 ++-- pkg/helm/package.go | 26 ++++++++++++++++++++++++-- pkg/helm/providers.go | 31 ------------------------------- pkg/helm/render_test.go | 2 -- 10 files changed, 89 insertions(+), 53 deletions(-) delete mode 100644 pkg/helm/providers.go diff --git a/README.md b/README.md index 630a9ad..9e386ff 100644 --- a/README.md +++ b/README.md @@ -265,8 +265,9 @@ It exposes a `Helm` struct that provides a `Render()` function that returns the | `outputPathMapping[].selectors[].kind` | | Selects resources by kind. | | `outputPathMapping[].selectors[].namespace` | | Selects resources by namespace. | | `outputPathMapping[].selectors[].name` | | Selects resources by name. | -| | `--output-replace` | If enabled replace the output directory or file (CLI-only). | -| | `--trust-any-repo` | If enabled repositories that are not registered within `repositories.yaml` can be used as well (env var `KHELM_TRUST_ANY_REPO`). Within the kpt function this behaviour can be disabled by mounting `/helm/repository/repositories.yaml` or disabling network access. | +| | `--output-replace` | If enabled, replace the output directory or file (CLI-only). | +| | `--trust-any-repo` | If enabled, repositories that are not registered within `repositories.yaml` can be used as well (env var `KHELM_TRUST_ANY_REPO`). Within the kpt function this behaviour can be disabled by mounting `/helm/repository/repositories.yaml` or disabling network access. | +| | `--enable-git-getter` | If enabled, support helm repository URLs with the git+https scheme (env var `KHELM_ENABLE_GIT_GETTER`). | | `debug` | `--debug` | Enables debug log and provides a stack trace on error. | ### Repository configuration @@ -288,6 +289,8 @@ The following example points to an old version of cert-manager using a git URL: git+https://github.com/cert-manager/cert-manager@deploy/charts?ref=v0.6.2 ``` +To enable this feature, set the `--enable-git-getter` option or the corresponding environment variable: `KHELM_ENABLE_GIT_GETTER=true`. + This feature is meant to be compatible with Helm's [helm-git](https://github.com/aslafy-z/helm-git#usage) plugin (but is reimplemented in Go). However currently khelm does not support `sparse` git checkouts (due to [lack of support in go-git](https://github.com/go-git/go-git/issues/90)). diff --git a/cmd/khelm/root.go b/cmd/khelm/root.go index 6a1a68c..05df8f7 100644 --- a/cmd/khelm/root.go +++ b/cmd/khelm/root.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "fmt" "io" "log" @@ -10,13 +11,16 @@ import ( "strconv" "strings" + "github.com/mgoltzsche/khelm/v2/pkg/getter/git" "github.com/mgoltzsche/khelm/v2/pkg/helm" "github.com/spf13/cobra" + helmgetter "helm.sh/helm/v3/pkg/getter" ) const ( envKustomizePluginConfig = "KUSTOMIZE_PLUGIN_CONFIG_STRING" envKustomizePluginConfigRoot = "KUSTOMIZE_PLUGIN_CONFIG_ROOT" + envEnableGitGetter = "KHELM_ENABLE_GIT_GETTER" envTrustAnyRepo = "KHELM_TRUST_ANY_REPO" envDebug = "KHELM_DEBUG" envHelmDebug = "HELM_DEBUG" @@ -36,11 +40,15 @@ func Execute(reader io.Reader, writer io.Writer) error { helmDebug, _ := strconv.ParseBool(os.Getenv(envHelmDebug)) h := helm.NewHelm() debug = debug || helmDebug + enableGitGetter := false h.Settings.Debug = debug if trustAnyRepo, ok := os.LookupEnv(envTrustAnyRepo); ok { trust, _ := strconv.ParseBool(trustAnyRepo) h.TrustAnyRepository = &trust } + if gitSupportStr, ok := os.LookupEnv(envEnableGitGetter); ok { + enableGitGetter, _ = strconv.ParseBool(gitSupportStr) + } // Run as kustomize plugin (if kustomize-specific env var provided) if kustomizeGenCfgYAML, isKustomizePlugin := os.LookupEnv(envKustomizePluginConfig); isKustomizePlugin { @@ -50,12 +58,16 @@ func Execute(reader io.Reader, writer io.Writer) error { return err } - logVersionPreRun := func(_ *cobra.Command, _ []string) { + preRun := func(_ *cobra.Command, _ []string) { logVersion() + if enableGitGetter { + addGitGetterSupport(h) + } } rootCmd := &cobra.Command{ - PreRun: logVersionPreRun, + PersistentPreRun: preRun, } + rootCmd.PersistentFlags().BoolVar(&enableGitGetter, "enable-git-getter", enableGitGetter, fmt.Sprintf("enable git+https helm repository URL scheme support (%s)", envEnableGitGetter)) errBuf := bytes.Buffer{} if filepath.Base(os.Args[0]) == "khelmfn" { @@ -65,8 +77,7 @@ func Execute(reader io.Reader, writer io.Writer) error { rootCmd.SetOut(writer) rootCmd.SetErr(&errBuf) rootCmd.PersistentFlags().BoolVar(&debug, "debug", debug, fmt.Sprintf("enable debug log (%s)", envDebug)) - rootCmd.PreRun = func(_ *cobra.Command, _ []string) { - logVersion() + rootCmd.PreRun = func(cmd *cobra.Command, args []string) { fmt.Printf("# Reading kpt function input from stdin (use `%s template` to run without kpt)\n", os.Args[0]) } } @@ -88,7 +99,7 @@ In addition to helm's templating capabilities khelm allows to: templateCmd := templateCommand(h, writer) templateCmd.SetOut(writer) templateCmd.SetErr(&errBuf) - templateCmd.PreRun = logVersionPreRun + templateCmd.PreRun = preRun rootCmd.AddCommand(templateCmd) // Run command @@ -110,3 +121,12 @@ func logVersion() { func versionInfo() string { return fmt.Sprintf("%s (helm %s)", khelmVersion, helmVersion) } + +func addGitGetterSupport(h *helm.Helm) { + h.Getters = append(h.Getters, helmgetter.Provider{ + Schemes: git.Schemes, + New: git.New(&h.Settings, h.Repositories, func(ctx context.Context, chartDir, repoDir string) (string, error) { + return h.Package(ctx, chartDir, repoDir, chartDir) + }), + }) +} diff --git a/cmd/khelm/template_test.go b/cmd/khelm/template_test.go index d7d3b57..0683796 100644 --- a/cmd/khelm/template_test.go +++ b/cmd/khelm/template_test.go @@ -93,6 +93,16 @@ func TestTemplateCommand(t *testing.T) { []string{filepath.Join(exampleDir, "chart-hooks"), "--no-hooks"}, 1, "myvalue", }, + { + "git-dependency", + []string{filepath.Join(exampleDir, "git-dependency"), "--enable-git-getter", "--trust-any-repo"}, + 24, "ca-sync", + }, + { + "local-chart-with-transitive-remote-and-git-dependencies", + []string{filepath.Join(exampleDir, "localrefref-with-git"), "--enable-git-getter", "--trust-any-repo"}, + 33, "admission.certmanager.k8s.io", + }, } { t.Run(c.name, func(t *testing.T) { var out bytes.Buffer @@ -128,6 +138,10 @@ func TestTemplateCommandError(t *testing.T) { "reject cluster scoped resources", []string{"cert-manager", "--repo=https://charts.jetstack.io", "--namespaced-only"}, }, + { + "reject git urls by default", + []string{"git-dependency", "--enable-git-getter=false"}, + }, } { t.Run(c.name, func(t *testing.T) { os.Args = append([]string{"testee", "template"}, c.args...) diff --git a/e2e/cli-tests.bats b/e2e/cli-tests.bats index f492991..d7c90c3 100755 --- a/e2e/cli-tests.bats +++ b/e2e/cli-tests.bats @@ -60,7 +60,9 @@ teardown() { } @test "CLI should accept git url as helm repository" { - docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" "$IMAGE" template cert-manager \ + docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" \ + -e KHELM_ENABLE_GIT_GETTER=true \ + "$IMAGE" template cert-manager \ --repo git+https://github.com/cert-manager/cert-manager@deploy/charts?ref=v0.6.2 \ --output /out/manifest.yaml \ --debug @@ -70,14 +72,18 @@ teardown() { @test "CLI should cache git repository" { mkdir $OUT_DIR/cache - docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" -v "$OUT_DIR/cache:/helm/cache" "$IMAGE" template cert-manager \ + docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" -v "$OUT_DIR/cache:/helm/cache" \ + -e KHELM_ENABLE_GIT_GETTER=true \ + "$IMAGE" template cert-manager \ --repo git+https://github.com/cert-manager/cert-manager@deploy/charts?ref=v0.6.2 \ --output /out/manifest.yaml \ --debug [ -f "$OUT_DIR/manifest.yaml" ] grep -q ca-sync "$OUT_DIR/manifest.yaml" rm -f "$OUT_DIR/manifest.yaml" - docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" -v "$OUT_DIR/cache:/helm/cache" --network=none "$IMAGE" template cert-manager \ + docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" -v "$OUT_DIR/cache:/helm/cache" \ + -e KHELM_ENABLE_GIT_GETTER=true \ + --network=none "$IMAGE" template cert-manager \ --repo git+https://github.com/cert-manager/cert-manager@deploy/charts?ref=v0.6.2 \ --output /out/manifest.yaml \ --debug @@ -86,7 +92,10 @@ teardown() { } @test "CLI should reject git repository when not in repositories.yaml and trust-any disabled" { - run -1 docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" -e KHELM_TRUST_ANY_REPO=false "$IMAGE" template cert-manager \ + run -1 docker run --rm -u $(id -u):$(id -g) -v "$OUT_DIR:/out" \ + -e KHELM_ENABLE_GIT_GETTER=true \ + -e KHELM_TRUST_ANY_REPO=false \ + "$IMAGE" template cert-manager \ --repo git+https://github.com/cert-manager/cert-manager@deploy/charts?ref=v0.6.2 \ --output /out/manifest.yaml \ --debug diff --git a/pkg/getter/git/gitgetter.go b/pkg/getter/git/gitgetter.go index 4f3029b..58d648e 100644 --- a/pkg/getter/git/gitgetter.go +++ b/pkg/getter/git/gitgetter.go @@ -20,7 +20,10 @@ import ( helmyaml "sigs.k8s.io/yaml" ) -var gitCheckout = gitCheckoutImpl +var ( + Schemes = []string{"git+https", "git+ssh"} + gitCheckout = gitCheckoutImpl +) type HelmPackageFunc func(ctx context.Context, path, repoDir string) (string, error) diff --git a/pkg/helm/helm.go b/pkg/helm/helm.go index df90cf5..d5bd273 100644 --- a/pkg/helm/helm.go +++ b/pkg/helm/helm.go @@ -25,12 +25,10 @@ func NewHelm() *Helm { // Fallback for old helm env var settings.RepositoryConfig = filepath.Join(helmHome, "repository", "repositories.yaml") } - h := &Helm{Settings: *settings} - h.Getters = getters(settings, h.repositories) - return h + return &Helm{Settings: *settings, Getters: getter.All(settings)} } -func (h *Helm) repositories() (repositories.Interface, error) { +func (h *Helm) Repositories() (repositories.Interface, error) { if h.repos != nil { return h.repos, nil } diff --git a/pkg/helm/load.go b/pkg/helm/load.go index 1f350b9..27e160b 100644 --- a/pkg/helm/load.go +++ b/pkg/helm/load.go @@ -29,7 +29,7 @@ func (h *Helm) loadChart(ctx context.Context, cfg *config.ChartConfig) (*chart.C fileExists := err == nil if cfg.Repository == "" { if fileExists { - repos, err := h.repositories() + repos, err := h.Repositories() if err != nil { return nil, err } @@ -46,7 +46,7 @@ func (h *Helm) loadChart(ctx context.Context, cfg *config.ChartConfig) (*chart.C func (h *Helm) loadRemoteChart(ctx context.Context, cfg *config.ChartConfig) (*chart.Chart, error) { repoURLs := map[string]struct{}{cfg.Repository: {}} - repos, err := h.repositories() + repos, err := h.Repositories() if err != nil { return nil, err } diff --git a/pkg/helm/package.go b/pkg/helm/package.go index 485b62b..fe2b784 100644 --- a/pkg/helm/package.go +++ b/pkg/helm/package.go @@ -10,7 +10,29 @@ import ( "helm.sh/helm/v3/pkg/getter" ) -func packageHelmChart(ctx context.Context, cfg *config.ChartConfig, destDir string, repos repositories.Interface, settings cli.EnvSettings, getters getter.Providers) (string, error) { +type PackageOptions struct { + ChartDir string + BaseDir string + DestDir string +} + +// Package builds and packages a local Helm chart. +// Returns the tgz file path. +func (h *Helm) Package(ctx context.Context, chartDir, baseDir, destDir string) (string, error) { + repos, err := h.Repositories() + if err != nil { + return "", err + } + cfg := config.ChartConfig{ + LoaderConfig: config.LoaderConfig{ + Chart: chartDir, + }, + BaseDir: baseDir, + } + return packageHelmChart(ctx, &cfg, repos, h.Settings, h.Getters) +} + +func packageHelmChart(ctx context.Context, cfg *config.ChartConfig, repos repositories.Interface, settings cli.EnvSettings, getters getter.Providers) (string, error) { // TODO: add unit test (there is an e2e/cli test for this though) _, err := buildAndLoadLocalChart(ctx, cfg, repos, settings, getters) if err != nil { @@ -18,7 +40,7 @@ func packageHelmChart(ctx context.Context, cfg *config.ChartConfig, destDir stri } // See https://github.com/helm/helm/blob/v3.10.0/cmd/helm/package.go#L104 client := action.NewPackage() - client.Destination = destDir + client.Destination = cfg.BaseDir chartPath := absPath(cfg.Chart, cfg.BaseDir) tgzFile, err := client.Run(chartPath, map[string]interface{}{}) if err != nil { diff --git a/pkg/helm/providers.go b/pkg/helm/providers.go deleted file mode 100644 index 49a157c..0000000 --- a/pkg/helm/providers.go +++ /dev/null @@ -1,31 +0,0 @@ -package helm - -import ( - "context" - - "github.com/mgoltzsche/khelm/v2/pkg/config" - "github.com/mgoltzsche/khelm/v2/pkg/getter/git" - "github.com/mgoltzsche/khelm/v2/pkg/repositories" - "helm.sh/helm/v3/pkg/cli" - helmgetter "helm.sh/helm/v3/pkg/getter" -) - -func getters(settings *cli.EnvSettings, reposFn func() (repositories.Interface, error)) helmgetter.Providers { - g := helmgetter.All(settings) - g = append(g, helmgetter.Provider{ - Schemes: []string{"git+https", "git+ssh"}, - New: git.New(settings, reposFn, func(ctx context.Context, chartDir, repoDir string) (string, error) { - repos, err := reposFn() - if err != nil { - return "", err - } - return packageHelmChart(ctx, &config.ChartConfig{ - LoaderConfig: config.LoaderConfig{ - Chart: chartDir, - }, - BaseDir: repoDir, - }, chartDir, repos, *settings, g) - }), - }) - return g -} diff --git a/pkg/helm/render_test.go b/pkg/helm/render_test.go index f4636b6..22f6a42 100644 --- a/pkg/helm/render_test.go +++ b/pkg/helm/render_test.go @@ -72,8 +72,6 @@ func TestRender(t *testing.T) { "chart-hooks-test", }}, {"chart-hooks-disabled", "example/chart-hooks-disabled/generator.yaml", []string{"default"}, " key: myvalue", []string{"chart-hooks-disabled-myconfig"}}, - {"git-dependency", "example/git-dependency/generator.yaml", []string{"cert-manager", "kube-system"}, "ca-sync", nil}, - {"local-chart-with-transitive-remote-and-git-dependencies", "example/localrefref-with-git/generator.yaml", []string{"kube-system", "myotherns"}, "admission.certmanager.k8s.io", nil}, } { t.Run(c.name, func(t *testing.T) { for _, cached := range []string{"", "cached "} {