From 3fab7def3eba5bcca14b6693e7092a18656d7f35 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Sat, 4 Jun 2022 17:16:54 -0700 Subject: [PATCH] fix: missing Helm params (#9565) (#9566) * fix: missing Helm params Signed-off-by: Michael Crenshaw * use absolute paths, fix tests Signed-off-by: Michael Crenshaw * fix race in test Signed-off-by: Michael Crenshaw --- reposerver/repository/repository_test.go | 30 +++++++++++++ .../testdata/values-files/Chart.yaml | 2 + .../values-files/caps-extn-values.YAML | 0 .../testdata/values-files/exclude.yaml | 1 + .../values-files/has-the-word-values.yaml | 4 ++ .../testdata/values-files/values.yaml | 1 + .../testdata/values-files/values.yml | 0 util/helm/helm.go | 6 +-- util/helm/helm_test.go | 43 ++++++++++++++----- util/io/path/resolved.go | 1 + 10 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 reposerver/repository/testdata/values-files/Chart.yaml create mode 100644 reposerver/repository/testdata/values-files/caps-extn-values.YAML create mode 100644 reposerver/repository/testdata/values-files/exclude.yaml create mode 100644 reposerver/repository/testdata/values-files/has-the-word-values.yaml create mode 100644 reposerver/repository/testdata/values-files/values.yaml create mode 100644 reposerver/repository/testdata/values-files/values.yml diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 3bce57c54686..4576849982b4 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -1120,6 +1120,7 @@ func TestListApps(t *testing.T) { "kustomization_yml": "Kustomize", "my-chart": "Helm", "my-chart-2": "Helm", + "values-files": "Helm", } assert.Equal(t, expectedApps, res.Apps) } @@ -1847,3 +1848,32 @@ func Test_getHelmDependencyRepos(t *testing.T) { assert.Equal(t, repos[0].Repo, repo1) assert.Equal(t, repos[1].Repo, repo2) } + +func Test_findHelmValueFilesInPath(t *testing.T) { + t.Run("does not exist", func(t *testing.T) { + files, err := findHelmValueFilesInPath("/obviously/does/not/exist") + assert.Error(t, err) + assert.Empty(t, files) + }) + t.Run("values files", func(t *testing.T) { + files, err := findHelmValueFilesInPath("./testdata/values-files") + assert.NoError(t, err) + assert.Len(t, files, 4) + }) +} + +func Test_populateHelmAppDetails(t *testing.T) { + res := apiclient.RepoAppDetailsResponse{} + q := apiclient.RepoServerAppDetailsQuery{ + Repo: &argoappv1.Repository{}, + Source: &argoappv1.ApplicationSource{ + Helm: &argoappv1.ApplicationSourceHelm{ValueFiles: []string{"exclude.yaml", "has-the-word-values.yaml"}}, + }, + } + appPath, err := filepath.Abs("./testdata/values-files/") + require.NoError(t, err) + err = populateHelmAppDetails(&res, appPath, appPath, &q) + require.NoError(t, err) + assert.Len(t, res.Helm.Parameters, 3) + assert.Len(t, res.Helm.ValueFiles, 4) +} diff --git a/reposerver/repository/testdata/values-files/Chart.yaml b/reposerver/repository/testdata/values-files/Chart.yaml new file mode 100644 index 000000000000..0c9526bab8cc --- /dev/null +++ b/reposerver/repository/testdata/values-files/Chart.yaml @@ -0,0 +1,2 @@ +name: my-chart +version: 1.1.0 \ No newline at end of file diff --git a/reposerver/repository/testdata/values-files/caps-extn-values.YAML b/reposerver/repository/testdata/values-files/caps-extn-values.YAML new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/reposerver/repository/testdata/values-files/exclude.yaml b/reposerver/repository/testdata/values-files/exclude.yaml new file mode 100644 index 000000000000..c9bb22cd260f --- /dev/null +++ b/reposerver/repository/testdata/values-files/exclude.yaml @@ -0,0 +1 @@ +exclude: yaml diff --git a/reposerver/repository/testdata/values-files/has-the-word-values.yaml b/reposerver/repository/testdata/values-files/has-the-word-values.yaml new file mode 100644 index 000000000000..911b485e5ef9 --- /dev/null +++ b/reposerver/repository/testdata/values-files/has-the-word-values.yaml @@ -0,0 +1,4 @@ +has: + the: + word: + values: yaml diff --git a/reposerver/repository/testdata/values-files/values.yaml b/reposerver/repository/testdata/values-files/values.yaml new file mode 100644 index 000000000000..55262d50ff71 --- /dev/null +++ b/reposerver/repository/testdata/values-files/values.yaml @@ -0,0 +1 @@ +values: yaml diff --git a/reposerver/repository/testdata/values-files/values.yml b/reposerver/repository/testdata/values-files/values.yml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/util/helm/helm.go b/util/helm/helm.go index fe9c96b0ae83..004059560776 100644 --- a/util/helm/helm.go +++ b/util/helm/helm.go @@ -6,7 +6,6 @@ import ( "net/url" "os" "os/exec" - "path" "strings" "github.com/ghodss/yaml" @@ -142,11 +141,10 @@ func (h *helm) GetParameters(valuesFiles []pathutil.ResolvedFilePath) (map[strin if err == nil && (parsedURL.Scheme == "http" || parsedURL.Scheme == "https") { fileValues, err = config.ReadRemoteFile(file) } else { - filePath := path.Join(h.cmd.WorkDir, file) - if _, err := os.Stat(filePath); os.IsNotExist(err) { + if _, err := os.Stat(file); os.IsNotExist(err) { continue } - fileValues, err = ioutil.ReadFile(filePath) + fileValues, err = ioutil.ReadFile(file) } if err != nil { return nil, fmt.Errorf("failed to read value file %s: %s", file, err) diff --git a/util/helm/helm_test.go b/util/helm/helm_test.go index 2fdb5506ad1a..65e32cd20b7a 100644 --- a/util/helm/helm_test.go +++ b/util/helm/helm_test.go @@ -1,8 +1,11 @@ package helm import ( + "path/filepath" "testing" + "github.com/stretchr/testify/require" + "github.com/argoproj/argo-cd/v2/util/io/path" "github.com/argoproj/gitops-engine/pkg/utils/kube" @@ -51,11 +54,16 @@ func TestHelmTemplateParams(t *testing.T) { } func TestHelmTemplateValues(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", []HelmRepository{}, false, "", "") + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, []HelmRepository{}, false, "", "") assert.NoError(t, err) + valuesPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil) + require.NoError(t, err) opts := TemplateOpts{ Name: "test", - Values: []path.ResolvedFilePath{"values-production.yaml"}, + Values: []path.ResolvedFilePath{valuesPath}, } objs, err := template(h, &opts) assert.Nil(t, err) @@ -72,33 +80,48 @@ func TestHelmTemplateValues(t *testing.T) { } func TestHelmGetParams(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", nil, false, "", "") + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, nil, false, "", "") assert.NoError(t, err) params, err := h.GetParameters(nil) assert.Nil(t, err) slaveCountParam := params["cluster.slaveCount"] - assert.Equal(t, slaveCountParam, "1") + assert.Equal(t, "1", slaveCountParam) } func TestHelmGetParamsValueFiles(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", nil, false, "", "") + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, nil, false, "", "") assert.NoError(t, err) - params, err := h.GetParameters([]path.ResolvedFilePath{"values-production.yaml"}) + valuesPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil) + require.NoError(t, err) + params, err := h.GetParameters([]path.ResolvedFilePath{valuesPath}) assert.Nil(t, err) slaveCountParam := params["cluster.slaveCount"] - assert.Equal(t, slaveCountParam, "3") + assert.Equal(t, "3", slaveCountParam) } func TestHelmGetParamsValueFilesThatExist(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", nil, false, "", "") + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, nil, false, "", "") assert.NoError(t, err) - params, err := h.GetParameters([]path.ResolvedFilePath{"values-missing.yaml", "values-production.yaml"}) + valuesMissingPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-missing.yaml", nil) + require.NoError(t, err) + valuesProductionPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil) + require.NoError(t, err) + params, err := h.GetParameters([]path.ResolvedFilePath{valuesMissingPath, valuesProductionPath}) assert.Nil(t, err) slaveCountParam := params["cluster.slaveCount"] - assert.Equal(t, slaveCountParam, "3") + assert.Equal(t, "3", slaveCountParam) } func TestHelmTemplateReleaseNameOverwrite(t *testing.T) { diff --git a/util/io/path/resolved.go b/util/io/path/resolved.go index 0343a379f049..abc3ff1de975 100644 --- a/util/io/path/resolved.go +++ b/util/io/path/resolved.go @@ -11,6 +11,7 @@ import ( ) // ResolvedFilePath represents a resolved file path and intended to prevent unintentional use of not verified file path. +// It is always either a URL or an absolute path. type ResolvedFilePath string // resolveSymbolicLinkRecursive resolves the symlink path recursively to its