Skip to content

Commit

Permalink
chore: remove dead Helm version code (argoproj#19449)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Crenshaw <[email protected]>
  • Loading branch information
crenshaw-dev authored Aug 9, 2024
1 parent 992716b commit 5ccea0d
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 120 deletions.
10 changes: 1 addition & 9 deletions reposerver/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -1215,10 +1215,6 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
}

defer h.Dispose()
err = h.Init()
if err != nil {
return nil, fmt.Errorf("error initializing helm app: %w", err)
}

out, err := h.Template(templateOpts)
if err != nil {
Expand Down Expand Up @@ -2082,10 +2078,6 @@ func populateHelmAppDetails(res *apiclient.RepoAppDetailsResponse, appPath strin
return err
}
defer h.Dispose()
err = h.Init()
if err != nil {
return err
}

if resolvedValuesPath, _, err := pathutil.ResolveValueFilePathOrUrl(appPath, repoRoot, "values.yaml", []string{}); err == nil {
if err := loadFileIntoIfExists(resolvedValuesPath, &res.Helm.Values); err != nil {
Expand Down Expand Up @@ -2323,7 +2315,7 @@ func (s *Service) GetRevisionChartDetails(ctx context.Context, q *apiclient.Repo
return nil, fmt.Errorf("error extracting chart: %w", err)
}
defer io.Close(closer)
helmCmd, err := helm.NewCmdWithVersion(chartPath, helm.HelmV3, q.Repo.EnableOCI, q.Repo.Proxy)
helmCmd, err := helm.NewCmdWithVersion(chartPath, q.Repo.EnableOCI, q.Repo.Proxy)
if err != nil {
return nil, fmt.Errorf("error creating helm cmd: %w", err)
}
Expand Down
9 changes: 2 additions & 7 deletions util/helm/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,12 @@ func untarChart(tempDir string, cachedChartPath string, manifestMaxExtractedSize

func (c *nativeHelmChart) ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) {
// always use Helm V3 since we don't have chart content to determine correct Helm version
helmCmd, err := NewCmdWithVersion("", HelmV3, c.enableOci, c.proxy)
helmCmd, err := NewCmdWithVersion("", c.enableOci, c.proxy)
if err != nil {
return "", nil, err
}
defer helmCmd.Close()

_, err = helmCmd.Init()
if err != nil {
return "", nil, err
}

// throw away temp directory that stores extracted chart and should be deleted as soon as no longer needed by returned closer
tempDir, err := files.CreateTempDir(os.TempDir())
if err != nil {
Expand Down Expand Up @@ -274,7 +269,7 @@ func (c *nativeHelmChart) TestHelmOCI() (bool, error) {
}
defer func() { _ = os.RemoveAll(tmpDir) }()

helmCmd, err := NewCmdWithVersion(tmpDir, HelmV3, c.enableOci, c.proxy)
helmCmd, err := NewCmdWithVersion(tmpDir, c.enableOci, c.proxy)
if err != nil {
return false, err
}
Expand Down
68 changes: 39 additions & 29 deletions util/helm/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

// A thin wrapper around the "helm" command, adding logging and error translation.
type Cmd struct {
HelmVer
helmHome string
WorkDir string
IsLocal bool
Expand All @@ -33,25 +32,25 @@ func NewCmd(workDir string, version string, proxy string) (*Cmd, error) {
switch version {
// If v3 is specified (or by default, if no value is specified) then use v3
case "", "v3":
return NewCmdWithVersion(workDir, HelmV3, false, proxy)
return NewCmdWithVersion(workDir, false, proxy)
}
return nil, fmt.Errorf("helm chart version '%s' is not supported", version)
}

func NewCmdWithVersion(workDir string, version HelmVer, isHelmOci bool, proxy string) (*Cmd, error) {
func NewCmdWithVersion(workDir string, isHelmOci bool, proxy string) (*Cmd, error) {
tmpDir, err := os.MkdirTemp("", "helm")
if err != nil {
return nil, err
}
return &Cmd{WorkDir: workDir, helmHome: tmpDir, HelmVer: version, IsHelmOci: isHelmOci, proxy: proxy}, err
return &Cmd{WorkDir: workDir, helmHome: tmpDir, IsHelmOci: isHelmOci, proxy: proxy}, err
}

var redactor = func(text string) string {
return regexp.MustCompile("(--username|--password) [^ ]*").ReplaceAllString(text, "$1 ******")
}

func (c Cmd) run(args ...string) (string, error) {
cmd := exec.Command(c.binaryName, args...)
cmd := exec.Command("helm", args...)
cmd.Dir = c.WorkDir
cmd.Env = os.Environ()
if !c.IsLocal {
Expand All @@ -71,13 +70,6 @@ func (c Cmd) run(args ...string) (string, error) {
return executil.RunWithRedactor(cmd, redactor)
}

func (c *Cmd) Init() (string, error) {
if c.initSupported {
return c.run("init", "--client-only", "--skip-refresh")
}
return "", nil
}

func (c *Cmd) RegistryLogin(repo string, creds Creds) (string, error) {
args := []string{"registry", "login"}
args = append(args, repo)
Expand Down Expand Up @@ -146,7 +138,7 @@ func (c *Cmd) RepoAdd(name string, url string, opts Creds, passCredentials bool)
args = append(args, "--ca-file", opts.CAPath)
}

if opts.InsecureSkipVerify && c.insecureSkipVerifySupported {
if opts.InsecureSkipVerify {
args = append(args, "--insecure-skip-tls-verify")
}

Expand Down Expand Up @@ -176,7 +168,7 @@ func (c *Cmd) RepoAdd(name string, url string, opts Creds, passCredentials bool)
args = append(args, "--key-file", keyFile.Name())
}

if c.helmPassCredentialsSupported && passCredentials {
if passCredentials {
args = append(args, "--pass-credentials")
}

Expand Down Expand Up @@ -209,7 +201,7 @@ func writeToTmp(data []byte) (string, argoio.Closer, error) {
}

func (c *Cmd) Fetch(repo, chartName, version, destination string, creds Creds, passCredentials bool) (string, error) {
args := []string{c.pullCommand, "--destination", destination}
args := []string{"pull", "--destination", destination}
if version != "" {
args = append(args, "--version", version)
}
Expand All @@ -219,7 +211,7 @@ func (c *Cmd) Fetch(repo, chartName, version, destination string, creds Creds, p
if creds.Password != "" {
args = append(args, "--password", creds.Password)
}
if creds.InsecureSkipVerify && c.insecureSkipVerifySupported {
if creds.InsecureSkipVerify {
args = append(args, "--insecure-skip-tls-verify")
}

Expand All @@ -244,7 +236,7 @@ func (c *Cmd) Fetch(repo, chartName, version, destination string, creds Creds, p
defer argoio.Close(closer)
args = append(args, "--key-file", filePath)
}
if passCredentials && c.helmPassCredentialsSupported {
if passCredentials {
args = append(args, "--pass-credentials")
}

Expand Down Expand Up @@ -280,7 +272,7 @@ func (c *Cmd) PullOCI(repo string, chart string, version string, destination str
args = append(args, "--key-file", filePath)
}

if creds.InsecureSkipVerify && c.insecureSkipVerifySupported {
if creds.InsecureSkipVerify {
args = append(args, "--insecure-skip-tls-verify")
}
return c.run(args...)
Expand All @@ -291,11 +283,11 @@ func (c *Cmd) dependencyBuild() (string, error) {
}

func (c *Cmd) inspectValues(values string) (string, error) {
return c.run(c.showCommand, "values", values)
return c.run("show", "values", values)
}

func (c *Cmd) InspectChart() (string, error) {
return c.run(c.showCommand, "chart", ".")
return c.run("show", "chart", ".")
}

type TemplateOpts struct {
Expand Down Expand Up @@ -324,20 +316,18 @@ func cleanSetParameters(val string) string {
}

func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, error) {
if c.HelmVer.getPostTemplateCallback != nil {
if callback, err := c.HelmVer.getPostTemplateCallback(filepath.Clean(path.Join(c.WorkDir, chartPath))); err == nil {
defer callback()
} else {
return "", err
}
if callback, err := cleanupChartLockFile(filepath.Clean(path.Join(c.WorkDir, chartPath))); err == nil {
defer callback()
} else {
return "", err
}

args := []string{"template", chartPath, c.templateNameArg, opts.Name}
args := []string{"template", chartPath, "--name-template", opts.Name}

if opts.Namespace != "" {
args = append(args, "--namespace", opts.Namespace)
}
if opts.KubeVersion != "" && c.kubeVersionSupported {
if opts.KubeVersion != "" {
args = append(args, "--kube-version", opts.KubeVersion)
}
for key, val := range opts.Set {
Expand All @@ -355,7 +345,7 @@ func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, error) {
for _, v := range opts.APIVersions {
args = append(args, "--api-versions", v)
}
if c.HelmVer.includeCrds && !opts.SkipCrds {
if !opts.SkipCrds {
args = append(args, "--include-crds")
}

Expand All @@ -371,6 +361,26 @@ func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, error) {
return out, nil
}

// Workaround for Helm3 behavior (see https://github.com/helm/helm/issues/6870).
// The `helm template` command generates Chart.lock after which `helm dependency build` does not work
// As workaround removing lock file unless it exists before running helm template
func cleanupChartLockFile(chartPath string) (func(), error) {
exists := true
lockPath := path.Join(chartPath, "Chart.lock")
if _, err := os.Stat(lockPath); err != nil {
if os.IsNotExist(err) {
exists = false
} else {
return nil, err
}
}
return func() {
if !exists {
_ = os.Remove(lockPath)
}
}, nil
}

func (c *Cmd) Freestyle(args ...string) (string, error) {
return c.run(args...)
}
Expand Down
16 changes: 2 additions & 14 deletions util/helm/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func Test_cmd_redactor(t *testing.T) {
}

func TestCmd_template_kubeVersion(t *testing.T) {
cmd, err := NewCmdWithVersion(".", HelmV3, false, "")
cmd, err := NewCmdWithVersion(".", false, "")
require.NoError(t, err)
s, err := cmd.template("testdata/redis", &TemplateOpts{
KubeVersion: "1.14",
Expand All @@ -25,7 +25,7 @@ func TestCmd_template_kubeVersion(t *testing.T) {
}

func TestCmd_template_noApiVersionsInError(t *testing.T) {
cmd, err := NewCmdWithVersion(".", HelmV3, false, "")
cmd, err := NewCmdWithVersion(".", false, "")
require.NoError(t, err)
_, err = cmd.template("testdata/chart-does-not-exist", &TemplateOpts{
KubeVersion: "1.14",
Expand All @@ -36,18 +36,6 @@ func TestCmd_template_noApiVersionsInError(t *testing.T) {
assert.ErrorContains(t, err, "<api versions removed> ")
}

func TestNewCmd_helmV3(t *testing.T) {
cmd, err := NewCmd(".", "v3", "")
require.NoError(t, err)
assert.Equal(t, "helm", cmd.HelmVer.binaryName)
}

func TestNewCmd_helmDefaultVersion(t *testing.T) {
cmd, err := NewCmd(".", "", "")
require.NoError(t, err)
assert.Equal(t, "helm", cmd.HelmVer.binaryName)
}

func TestNewCmd_helmInvalidVersion(t *testing.T) {
_, err := NewCmd(".", "abcd", "")
log.Println(err)
Expand Down
7 changes: 0 additions & 7 deletions util/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ type Helm interface {
GetParameters(valuesFiles []pathutil.ResolvedFilePath, appPath, repoRoot string) (map[string]string, error)
// DependencyBuild runs `helm dependency build` to download a chart's dependencies
DependencyBuild() error
// Init runs `helm init --client-only`
Init() error
// Dispose deletes temp resources
Dispose()
}
Expand Down Expand Up @@ -109,11 +107,6 @@ func (h *helm) DependencyBuild() error {
return err
}

func (h *helm) Init() error {
_, err := h.cmd.Init()
return err
}

func (h *helm) Dispose() {
h.cmd.Close()
}
Expand Down
54 changes: 0 additions & 54 deletions util/helm/helmver.go

This file was deleted.

0 comments on commit 5ccea0d

Please sign in to comment.