Merge pull request #1076 from ncabatoff/skip-charts-issue-1067

Fix #1067 by filtering out chart dirs from manifest loading of changed files

cluster/kubernetes/resource/load.go

@@ -18,16 +18,24 @@ import (
 func Load(base, atLeastOne string, more ...string) (map[string]resource.Resource, error) {
 	roots := append([]string{atLeastOne}, more...)
 	objs := map[string]resource.Resource{}
+	charts, err := newChartTracker(base)
+	if err != nil {
+		return nil, errors.Wrapf(err, "walking %q for chartdirs", base)
+	}
 	for _, root := range roots {
 		err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
 			if err != nil {
 				return errors.Wrapf(err, "walking %q for yamels", path)
 			}
 
-			if info.IsDir() && looksLikeChart(path) {
+			if charts.isDirChart(path) {
 				return filepath.SkipDir
 			}
 
+			if charts.isPathInChart(path) {
+				return nil
+			}
+
 			if !info.IsDir() && filepath.Ext(path) == ".yaml" || filepath.Ext(path) == ".yml" {
 				bytes, err := ioutil.ReadFile(path)
 				if err != nil {
@@ -57,6 +65,45 @@ func Load(base, atLeastOne string, more ...string) (map[string]resource.Resource
 	return objs, nil
 }
 
+type chartTracker map[string]bool
+
+func newChartTracker(root string) (chartTracker, error) {
+	var chartdirs = make(map[string]bool)
+	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return errors.Wrapf(err, "walking %q for charts", path)
+		}
+
+		if info.IsDir() && looksLikeChart(path) {
+			chartdirs[path] = true
+			return filepath.SkipDir
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return chartTracker(chartdirs), nil
+}
+
+func (c chartTracker) isDirChart(path string) bool {
+	return c[path]
+}
+
+func (c chartTracker) isPathInChart(path string) bool {
+	p := path
+	root := fmt.Sprintf("%c", filepath.Separator)
+	for p != root {
+		if c[p] {
+			return true
+		}
+		p = filepath.Dir(p)
+	}
+	return false
+}
+
 // looksLikeChart returns `true` if the path `dir` (assumed to be a
 // directory) looks like it contains a Helm chart, rather than
 // manifest files.

cluster/kubernetes/resource/load_test.go

@@ -2,6 +2,7 @@ package resource
 
 import (
 	"bytes"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -140,3 +141,51 @@ func TestLoadSome(t *testing.T) {
 		t.Errorf("expected %d objects from %d files, got result:\n%#v", len(testfiles.ServiceMap(dir)), len(testfiles.Files), objs)
 	}
 }
+
+func TestChartTracker(t *testing.T) {
+	dir, cleanup := testfiles.TempDir(t)
+	defer cleanup()
+	if err := testfiles.WriteTestFiles(dir); err != nil {
+		t.Fatal(err)
+	}
+
+	ct, err := newChartTracker(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	noncharts := []string{"garbage", "locked-service-deploy.yaml",
+		"test", "test/test-service-deploy.yaml"}
+	for _, f := range noncharts {
+		fq := filepath.Join(dir, f)
+		if ct.isDirChart(fq) {
+			t.Errorf("%q thought to be a chart", f)
+		}
+		if f == "garbage" {
+			continue
+		}
+		if m, err := Load(dir, fq); err != nil || len(m) == 0 {
+			t.Errorf("Load returned 0 objs, err=%v", err)
+		}
+	}
+	if !ct.isDirChart(filepath.Join(dir, "charts/nginx")) {
+		t.Errorf("charts/nginx not recognized as chart")
+	}
+	if !ct.isPathInChart(filepath.Join(dir, "charts/nginx/Chart.yaml")) {
+		t.Errorf("charts/nginx/Chart.yaml not recognized as in chart")
+	}
+
+	chartfiles := []string{"charts",
+		"charts/nginx",
+		"charts/nginx/Chart.yaml",
+		"charts/nginx/values.yaml",
+		"charts/nginx/templates/deployment.yaml",
+	}
+	for _, f := range chartfiles {
+		fq := filepath.Join(dir, f)
+		if m, err := Load(dir, fq); err != nil || len(m) != 0 {
+			t.Errorf("%q not ignored as a chart should be", f)
+		}
+	}
+
+}