Skip to content

Commit

Permalink
fix: missing Helm params (#9565) (#9566)
Browse files Browse the repository at this point in the history
* fix: missing Helm params

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* use absolute paths, fix tests

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* fix race in test

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
  • Loading branch information
crenshaw-dev committed Jun 15, 2022
1 parent 5e6b788 commit 3fab7de
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 14 deletions.
30 changes: 30 additions & 0 deletions reposerver/repository/repository_test.go
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
2 changes: 2 additions & 0 deletions reposerver/repository/testdata/values-files/Chart.yaml
@@ -0,0 +1,2 @@
name: my-chart
version: 1.1.0
Empty file.
1 change: 1 addition & 0 deletions reposerver/repository/testdata/values-files/exclude.yaml
@@ -0,0 +1 @@
exclude: yaml
@@ -0,0 +1,4 @@
has:
the:
word:
values: yaml
1 change: 1 addition & 0 deletions reposerver/repository/testdata/values-files/values.yaml
@@ -0,0 +1 @@
values: yaml
Empty file.
6 changes: 2 additions & 4 deletions util/helm/helm.go
Expand Up @@ -6,7 +6,6 @@ import (
"net/url"
"os"
"os/exec"
"path"
"strings"

"github.com/ghodss/yaml"
Expand Down Expand Up @@ -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)
Expand Down
43 changes: 33 additions & 10 deletions 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"
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions util/io/path/resolved.go
Expand Up @@ -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
Expand Down

0 comments on commit 3fab7de

Please sign in to comment.