Fix regression in changed chart detection (#85)

* Fix regression in changed chart detection Please enter the commit message for your changes. Lines starting Signed-off-by: Reinhard Nägele <[email protected]> * Refactor to consider charts at root Signed-off-by: Reinhard Nägele <[email protected]> * Make sure charts are processed only once Signed-off-by: Reinhard Nägele <[email protected]> * Fix typo Signed-off-by: Reinhard Nägele <[email protected]>
helm · Jan 20, 2019 · b679b85 · b679b85
1 parent fa29453
commit b679b85
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 11 deletions.
diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go
@@ -119,11 +119,11 @@ type DirectoryLister interface {
 
 // ChartUtils is the interface that wraps chart-related methods
 //
-// IsChartdir checks if a directory is a chart directory
+// LookupChartDir looks up the chart's root directory based on some chart file that has changed
 //
 // ReadChartYaml reads the `Chart.yaml` from the specified directory
 type ChartUtils interface {
-	IsChartDir(dir string) bool
+	LookupChartDir(chartDirs []string, dir string) (string, error)
 	ReadChartYaml(dir string) (*util.ChartYaml, error)
 }
 
@@ -433,9 +433,15 @@ func (t *Testing) ComputeChangedChartDirectories() ([]string, error) {
 			continue
 		}
 		dir := filepath.Dir(file)
-		// Only add if not already in list and double-check if it is a chart directory
-		if !util.StringSliceContains(changedChartDirs, dir) && t.chartUtils.IsChartDir(dir) {
-			changedChartDirs = append(changedChartDirs, dir)
+		// Make sure directory is really a chart directory
+		chartDir, err := t.chartUtils.LookupChartDir(cfg.ChartDirs, dir)
+		if err == nil {
+			// Only add it if not already in the list
+			if !util.StringSliceContains(changedChartDirs, chartDir) {
+				changedChartDirs = append(changedChartDirs, chartDir)
+			}
+		} else {
+			fmt.Printf("Directory '%s' is no chart directory. Skipping...", chartDir)
 		}
 	}
 
@@ -451,7 +457,8 @@ func (t *Testing) ReadAllChartDirectories() ([]string, error) {
 	for _, chartParentDir := range cfg.ChartDirs {
 		dirs, err := t.directoryLister.ListChildDirs(chartParentDir,
 			func(dir string) bool {
-				return t.chartUtils.IsChartDir(dir) && !util.StringSliceContains(cfg.ExcludedCharts, path.Base(dir))
+				_, err := t.chartUtils.LookupChartDir(chartDirs, dir)
+				return err == nil && !util.StringSliceContains(cfg.ExcludedCharts, path.Base(dir))
 			})
 		if err != nil {
 			return nil, errors.Wrap(err, "Error reading chart directories")

diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go
@@ -51,7 +51,7 @@ func (g fakeGit) ListChangedFilesInDirs(commit string, dirs ...string) ([]string
 		"incubator/excluded/values.yaml",
 		"stable/blah/Chart.yaml",
 		"stable/blah/README.md",
-		"stable/this-is-no-chart-dir/foo.md",
+		"foo/this-is-no-chart-dir/foo.md",
 	}, nil
 }
 
@@ -76,8 +76,11 @@ func (l fakeDirLister) ListChildDirs(parentDir string, test func(dir string) boo
 
 type fakeChartUtils struct{}
 
-func (v fakeChartUtils) IsChartDir(dir string) bool {
-	return dir != "stable/this-is-no-chart-dir"
+func (v fakeChartUtils) LookupChartDir(chartDirs []string, dir string) (string, error) {
+	if strings.HasPrefix(dir, "foo")  {
+		return "", errors.New("no chart dir")
+	}
+	return dir, nil
 }
 
 func (v fakeChartUtils) ReadChartYaml(dir string) (*util.ChartYaml, error) {

diff --git a/pkg/util/util.go b/pkg/util/util.go
@@ -23,6 +23,7 @@ import (
 	"math/rand"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"time"
 )
@@ -121,8 +122,22 @@ func (l DirectoryLister) ListChildDirs(parentDir string, test func(dir string) b
 
 type ChartUtils struct{}
 
-func (u ChartUtils) IsChartDir(dir string) bool {
-	return FileExists(path.Join(dir, "Chart.yaml"))
+func (u ChartUtils) LookupChartDir(chartDirs []string, dir string) (string, error) {
+	for _, chartDir := range chartDirs {
+		currentDir := dir
+		for {
+			if FileExists(path.Join(currentDir, "Chart.yaml")) {
+				return currentDir, nil
+			}
+			currentDir = filepath.Dir(currentDir)
+			relativeDir, _ := filepath.Rel(chartDir, currentDir)
+			joined := filepath.Join(chartDir, relativeDir)
+			if joined == chartDir {
+				break
+			}
+		}
+	}
+	return "", errors.New("no chart directory")
 }
 
 func (u ChartUtils) ReadChartYaml(dir string) (*ChartYaml, error) {