Merge pull request #939 from spring-financial-group/helmfile-report-d…

…uplication

fix: issue with duplicate helmfiles being returned

pkg/cmd/helmfile/resolve/resolve.go

@@ -983,31 +983,8 @@ func (o *Options) CustomUpgrades(helmstate *state.HelmState) error {
 		}
 	}
 
-	// lets remove any unused jx3 repo
-	for i := range helmstate.Repositories {
-		repo := &helmstate.Repositories[i]
-		if repo.Name == "jx3" || repo.Name == "jenkins-x" {
-			// lets check if we have a jx3 release
-			found := false
-			prefix := repo.Name + "/"
-			for j := range helmstate.Releases {
-				release := &helmstate.Releases[j]
-				if strings.HasPrefix(release.Chart, prefix) {
-					found = true
-					break
-				}
-			}
-
-			if !found {
-				repos := helmstate.Repositories[0:i]
-				if i+1 < len(helmstate.Repositories) {
-					repos = append(repos, helmstate.Repositories[i+1:]...)
-				}
-				helmstate.Repositories = repos
-			}
-			break
-		}
-	}
+	// let's remove any unused jx3 repo
+	removeRedundantRepositories(helmstate)
 
 	// TODO lets remove the jx-labs repository if its no longer referenced...
 	if o.AddEnvironmentPipelines {
@@ -1047,6 +1024,23 @@ func (o *Options) CustomUpgrades(helmstate *state.HelmState) error {
 	return nil
 }
 
+// removeRedundantRepositories removes any repositories from a state.HelmState that are not referenced by any releases
+func removeRedundantRepositories(helmstate *state.HelmState) {
+	requiredRepositories := make(map[string]bool)
+	for i := range helmstate.Releases {
+		repoName := strings.SplitN(helmstate.Releases[i].Chart, "/", 2)[0]
+		requiredRepositories[repoName] = true
+	}
+
+	var cleanedRepositories []state.RepositorySpec
+	for i := range helmstate.Repositories {
+		if requiredRepositories[helmstate.Repositories[i].Name] {
+			cleanedRepositories = append(cleanedRepositories, helmstate.Repositories[i])
+		}
+	}
+	helmstate.Repositories = cleanedRepositories
+}
+
 func (o *Options) migrateRequirementsToV4() error {
 	path := filepath.Join(o.Dir, "jx-requirements.yml")
 	exists, err := files.FileExists(path)

pkg/cmd/helmfile/resolve/testdata/remote-chart/expected-foo-helmfile.yaml

@@ -5,7 +5,6 @@ environments:
     - jx-values.yaml
 namespace: foo
 repositories:
-- {}
 - name: bitnami
   url: https://charts.bitnami.com/bitnami
 releases:

pkg/helmfiles/helpers.go

@@ -1,13 +1,11 @@
 package helmfiles
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/helmfile/helmfile/pkg/state"
-	"github.com/jenkins-x/jx-helpers/v3/pkg/files"
 	"github.com/jenkins-x/jx-helpers/v3/pkg/yaml2s"
 	"github.com/pkg/errors"
 )
@@ -45,7 +43,6 @@ func GatherHelmfiles(helmfile, dir string) ([]Helmfile, error) {
 	helmfiles := []Helmfile{
 		{helmfile, relativePath},
 	}
-	parentHelmfileDir := filepath.Dir(helmfile)
 
 	for _, nested := range helmState.Helmfiles {
 		// lets ignore remote helmfiles
@@ -63,19 +60,6 @@ func GatherHelmfiles(helmfile, dir string) ([]Helmfile, error) {
 			return nil, errors.Wrapf(err, "failed to get nested helmnfiles %s in %s", nested.Path, dir)
 		}
 		helmfiles = append(helmfiles, nestedHelmfile...)
-
-		nestedHelmfileDepth := len(strings.Split(filepath.Dir(nested.Path), pathSeparator))
-		relativePath := strings.Repeat("../", parentHelmfileDepth+nestedHelmfileDepth)
-
-		fileLocation := filepath.Join(parentHelmfileDir, nested.Path)
-		exists, err := files.FileExists(fileLocation)
-		if err != nil {
-			return nil, errors.Wrapf(err, "failed to check for nested helmfile %s", fileLocation)
-		}
-		if !exists {
-			return nil, fmt.Errorf("failed to find nested helmfile %s", fileLocation)
-		}
-		helmfiles = append(helmfiles, Helmfile{fileLocation, relativePath})
 	}
 	return helmfiles, nil
 }

pkg/helmfiles/helpers_test.go

@@ -0,0 +1,37 @@
+package helmfiles_test
+
+import (
+	"testing"
+
+	"github.com/jenkins-x-plugins/jx-gitops/pkg/helmfiles"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGatherHelmfiles(t *testing.T) {
+	expected := []helmfiles.Helmfile{
+		{
+			Filepath:           "helpers_test_data/helmfile.yaml",
+			RelativePathToRoot: "",
+		},
+		{
+			Filepath:           "helpers_test_data/helmfiles/jx/helmfile.yaml",
+			RelativePathToRoot: "../../",
+		},
+		{
+			Filepath:           "helpers_test_data/helmfiles/jx/helmfiles/nested/helmfile.yaml",
+			RelativePathToRoot: "../../../../",
+		},
+		{
+			Filepath:           "helpers_test_data/helmfiles/nginx/helmfile.yaml",
+			RelativePathToRoot: "../../",
+		},
+		{
+			Filepath:           "helpers_test_data/helmfiles/cert-manager/helmfile.yaml",
+			RelativePathToRoot: "../../",
+		},
+	}
+
+	actual, err := helmfiles.GatherHelmfiles("helmfile.yaml", "helpers_test_data")
+	assert.NoError(t, err)
+	assert.Equal(t, expected, actual)
+}

pkg/helmfiles/helpers_test_data/helmfile.yaml

@@ -0,0 +1,7 @@
+filepath: ""
+helmfiles:
+- path: helmfiles/jx/helmfile.yaml
+- path: helmfiles/nginx/helmfile.yaml
+- path: helmfiles/cert-manager/helmfile.yaml
+templates: {}
+renderedvalues: {}

pkg/helmfiles/helpers_test_data/helmfiles/cert-manager/helmfile.yaml

@@ -0,0 +1,4 @@
+filepath: ""
+namespace: cert-manager
+templates: {}
+renderedvalues: {}

pkg/helmfiles/helpers_test_data/helmfiles/jx/helmfile.yaml

@@ -0,0 +1,6 @@
+filepath: ""
+namespace: jx
+helmfiles:
+  - path: helmfiles/nested/helmfile.yaml
+templates: {}
+renderedvalues: {}

pkg/helmfiles/helpers_test_data/helmfiles/jx/helmfiles/nested/helmfile.yaml

@@ -0,0 +1,4 @@
+filepath: ""
+namespace: nested
+templates: {}
+renderedvalues: {}

pkg/helmfiles/helpers_test_data/helmfiles/nginx/helmfile.yaml

@@ -0,0 +1,4 @@
+filepath: ""
+namespace: nginx
+templates: {}
+renderedvalues: {}