ref(pkg/chartutil): simplify chart dependency unit tests

- simplify unit tests
- refactor typed errors
- unexport internal functions

Signed-off-by: Adam Reese <adam@reese.io>

cmd/helm/template.go

@@ -171,10 +171,7 @@ func (o *templateOptions) run(out io.Writer) error {
 	if err := yaml.Unmarshal(config, &m); err != nil {
 		return err
 	}
-	if err := chartutil.ProcessDependencyEnabled(c, m); err != nil {
-		return err
-	}
-	if err := chartutil.ProcessDependencyImportValues(c); err != nil {
+	if err := chartutil.ProcessDependencies(c, m); err != nil {
 		return err
 	}
 

pkg/chartutil/dependencies.go

@@ -25,8 +25,15 @@ import (
 	"k8s.io/helm/pkg/version"
 )
 
-// ProcessDependencyConditions disables charts based on condition path value in values
-func ProcessDependencyConditions(reqs []*chart.Dependency, cvals Values) {
+func ProcessDependencies(c *chart.Chart, v Values) error {
+	if err := processDependencyEnabled(c, v); err != nil {
+		return err
+	}
+	return processDependencyImportValues(c)
+}
+
+// processDependencyConditions disables charts based on condition path value in values
+func processDependencyConditions(reqs []*chart.Dependency, cvals Values) {
 	if reqs == nil {
 		return
 	}
@@ -66,8 +73,8 @@ func ProcessDependencyConditions(reqs []*chart.Dependency, cvals Values) {
 	}
 }
 
-// ProcessDependencyTags disables charts based on tags in values
-func ProcessDependencyTags(reqs []*chart.Dependency, cvals Values) {
+// processDependencyTags disables charts based on tags in values
+func processDependencyTags(reqs []*chart.Dependency, cvals Values) {
 	if reqs == nil {
 		return
 	}
@@ -99,34 +106,32 @@ func ProcessDependencyTags(reqs []*chart.Dependency, cvals Values) {
 	}
 }
 
-func getAliasDependency(charts []*chart.Chart, aliasChart *chart.Dependency) *chart.Chart {
-	var chartFound chart.Chart
-	for _, existingChart := range charts {
-		if existingChart == nil {
+func getAliasDependency(charts []*chart.Chart, dep *chart.Dependency) *chart.Chart {
+	for _, c := range charts {
+		if c == nil {
 			continue
 		}
-		if existingChart.Metadata == nil {
+		if c.Name() != dep.Name {
 			continue
 		}
-		if existingChart.Metadata.Name != aliasChart.Name {
+		if !version.IsCompatibleRange(dep.Version, c.Metadata.Version) {
 			continue
 		}
-		if !version.IsCompatibleRange(aliasChart.Version, existingChart.Metadata.Version) {
-			continue
-		}
-		chartFound = *existingChart
-		newMetadata := *existingChart.Metadata
-		if aliasChart.Alias != "" {
-			newMetadata.Name = aliasChart.Alias
+
+		out := *c
+		md := *c.Metadata
+		out.Metadata = &md
+
+		if dep.Alias != "" {
+			md.Name = dep.Alias
 		}
-		chartFound.Metadata = &newMetadata
-		return &chartFound
+		return &out
 	}
 	return nil
 }
 
-// ProcessDependencyEnabled removes disabled charts from dependencies
-func ProcessDependencyEnabled(c *chart.Chart, v map[string]interface{}) error {
+// processDependencyEnabled removes disabled charts from dependencies
+func processDependencyEnabled(c *chart.Chart, v map[string]interface{}) error {
 	if c.Metadata.Dependencies == nil {
 		return nil
 	}
@@ -137,17 +142,14 @@ func ProcessDependencyEnabled(c *chart.Chart, v map[string]interface{}) error {
 	// However, if the dependency is already specified in Chart.yaml
 	// we should not add it, as it would be anyways processed from Chart.yaml
 
-	for _, existingDependency := range c.Dependencies() {
-		var dependencyFound bool
+Loop:
+	for _, existing := range c.Dependencies() {
 		for _, req := range c.Metadata.Dependencies {
-			if existingDependency.Metadata.Name == req.Name && version.IsCompatibleRange(req.Version, existingDependency.Metadata.Version) {
-				dependencyFound = true
-				break
+			if existing.Name() == req.Name && version.IsCompatibleRange(req.Version, existing.Metadata.Version) {
+				continue Loop
 			}
 		}
-		if !dependencyFound {
-			chartDependencies = append(chartDependencies, existingDependency)
-		}
+		chartDependencies = append(chartDependencies, existing)
 	}
 
 	for _, req := range c.Metadata.Dependencies {
@@ -170,8 +172,8 @@ func ProcessDependencyEnabled(c *chart.Chart, v map[string]interface{}) error {
 		return err
 	}
 	// flag dependencies as enabled/disabled
-	ProcessDependencyTags(c.Metadata.Dependencies, cvals)
-	ProcessDependencyConditions(c.Metadata.Dependencies, cvals)
+	processDependencyTags(c.Metadata.Dependencies, cvals)
+	processDependencyConditions(c.Metadata.Dependencies, cvals)
 	// make a map of charts to remove
 	rm := map[string]struct{}{}
 	for _, r := range c.Metadata.Dependencies {
@@ -191,7 +193,7 @@ func ProcessDependencyEnabled(c *chart.Chart, v map[string]interface{}) error {
 
 	// recursively call self to process sub dependencies
 	for _, t := range cd {
-		if err := ProcessDependencyEnabled(t, cvals); err != nil {
+		if err := processDependencyEnabled(t, cvals); err != nil {
 			return err
 		}
 	}
@@ -276,11 +278,11 @@ func processImportValues(c *chart.Chart) error {
 	return nil
 }
 
-// ProcessDependencyImportValues imports specified chart values from child to parent.
-func ProcessDependencyImportValues(c *chart.Chart) error {
+// processDependencyImportValues imports specified chart values from child to parent.
+func processDependencyImportValues(c *chart.Chart) error {
 	for _, d := range c.Dependencies() {
 		// recurse
-		if err := ProcessDependencyImportValues(d); err != nil {
+		if err := processDependencyImportValues(d); err != nil {
 			return err
 		}
 	}

pkg/chartutil/dependencies_test.go

@@ -16,141 +16,134 @@ package chartutil
 
 import (
 	"sort"
-	"testing"
-
 	"strconv"
-
-	"github.com/ghodss/yaml"
+	"testing"
 
 	"k8s.io/helm/pkg/chart"
 	"k8s.io/helm/pkg/chart/loader"
 	"k8s.io/helm/pkg/version"
 )
 
-func TestLoadDependency(t *testing.T) {
-	c, err := loader.Load("testdata/frobnitz")
+func loadChart(t *testing.T, path string) *chart.Chart {
+	c, err := loader.Load(path)
 	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
+		t.Fatalf("failed to load testdata: %s", err)
 	}
-	verifyDependency(t, c)
+	return c
 }
 
-func TestLoadChartLock(t *testing.T) {
-	c, err := loader.Load("testdata/frobnitz")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
+func TestLoadDependency(t *testing.T) {
+	tests := []*chart.Dependency{
+		{Name: "alpine", Version: "0.1.0", Repository: "https://example.com/charts"},
+		{Name: "mariner", Version: "4.3.2", Repository: "https://example.com/charts"},
 	}
-	verifyChartLock(t, c)
+
+	check := func(deps []*chart.Dependency) {
+		if len(deps) != 2 {
+			t.Errorf("expected 2 dependencies, got %d", len(deps))
+		}
+		for i, tt := range tests {
+			if deps[i].Name != tt.Name {
+				t.Errorf("expected dependency named %q, got %q", tt.Name, deps[i].Name)
+			}
+			if deps[i].Version != tt.Version {
+				t.Errorf("expected dependency named %q to have version %q, got %q", tt.Name, tt.Version, deps[i].Version)
+			}
+			if deps[i].Repository != tt.Repository {
+				t.Errorf("expected dependency named %q to have repository %q, got %q", tt.Name, tt.Repository, deps[i].Repository)
+			}
+		}
+	}
+	c := loadChart(t, "testdata/frobnitz")
+	check(c.Metadata.Dependencies)
+	check(c.Lock.Dependencies)
 }
 
 func TestDependencyEnabled(t *testing.T) {
+	type M = map[string]interface{}
 	tests := []struct {
 		name string
-		v    []byte
+		v    M
 		e    []string // expected charts including duplicates in alphanumeric order
 	}{{
 		"tags with no effect",
-		[]byte("tags:\n  nothinguseful: false\n\n"),
-		[]string{"parentchart", "subchart1", "subcharta", "subchartb"},
-	}, {
-		"tags with no effect",
-		[]byte("tags:\n  nothinguseful: false\n\n"),
-		[]string{"parentchart", "subchart1", "subcharta", "subchartb"},
+		M{"tags": M{"nothinguseful": false}},
+		[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb"},
 	}, {
 		"tags disabling a group",
-		[]byte("tags:\n  front-end: false\n\n"),
+		M{"tags": M{"front-end": false}},
 		[]string{"parentchart"},
 	}, {
 		"tags disabling a group and enabling a different group",
-		[]byte("tags:\n  front-end: false\n\n  back-end: true\n"),
-		[]string{"parentchart", "subchart2", "subchartb", "subchartc"},
+		M{"tags": M{"front-end": false, "back-end": true}},
+		[]string{"parentchart", "parentchart.subchart2", "parentchart.subchart2.subchartb", "parentchart.subchart2.subchartc"},
 	}, {
 		"tags disabling only children, children still enabled since tag front-end=true in values.yaml",
-		[]byte("tags:\n  subcharta: false\n\n  subchartb: false\n"),
-		[]string{"parentchart", "subchart1", "subcharta", "subchartb"},
+		M{"tags": M{"subcharta": false, "subchartb": false}},
+		[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb"},
 	}, {
 		"tags disabling all parents/children with additional tag re-enabling a parent",
-		[]byte("tags:\n  front-end: false\n\n  subchart1: true\n\n  back-end: false\n"),
-		[]string{"parentchart", "subchart1"},
-	}, {
-		"tags with no effect",
-		[]byte("subchart1:\n  nothinguseful: false\n\n"),
-		[]string{"parentchart", "subchart1", "subcharta", "subchartb"},
+		M{"tags": M{"front-end": false, "subchart1": true, "back-end": false}},
+		[]string{"parentchart", "parentchart.subchart1"},
 	}, {
 		"conditions enabling the parent charts, but back-end (b, c) is still disabled via values.yaml",
-		[]byte("subchart1:\n  enabled: true\nsubchart2:\n  enabled: true\n"),
-		[]string{"parentchart", "subchart1", "subchart2", "subcharta", "subchartb"},
+		M{"subchart1": M{"enabled": true}, "subchart2": M{"enabled": true}},
+		[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2"},
 	}, {
 		"conditions disabling the parent charts, effectively disabling children",
-		[]byte("subchart1:\n  enabled: false\nsubchart2:\n  enabled: false\n"),
+		M{"subchart1": M{"enabled": false}, "subchart2": M{"enabled": false}},
 		[]string{"parentchart"},
 	}, {
 		"conditions a child using the second condition path of child's condition",
-		[]byte("subchart1:\n  subcharta:\n    enabled: false\n"),
-		[]string{"parentchart", "subchart1", "subchartb"},
+		M{"subchart1": M{"subcharta": M{"enabled": false}}},
+		[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subchartb"},
 	}, {
 		"tags enabling a parent/child group with condition disabling one child",
-		[]byte("subchartc:\n  enabled: false\ntags:\n  back-end: true\n"),
-		[]string{"parentchart", "subchart1", "subchart2", "subcharta", "subchartb", "subchartb"},
+		M{"subchartc": M{"enabled": false}, "tags": M{"back-end": true}},
+		[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2", "parentchart.subchart2.subchartb"},
 	}, {
 		"tags will not enable a child if parent is explicitly disabled with condition",
-		[]byte("subchart1:\n  enabled: false\ntags:\n  front-end: true\n"),
+		M{"subchart1": M{"enabled": false}, "tags": M{"front-end": true}},
 		[]string{"parentchart"},
 	}}
 
 	for _, tc := range tests {
-		c, err := loader.Load("testdata/subpop")
-		if err != nil {
-			t.Fatalf("Failed to load testdata: %s", err)
-		}
+		c := loadChart(t, "testdata/subpop")
 		t.Run(tc.name, func(t *testing.T) {
-			verifyDependencyEnabled(t, c, tc.v, tc.e)
-		})
-	}
-}
-
-func verifyDependencyEnabled(t *testing.T, c *chart.Chart, v []byte, e []string) {
-	var m map[string]interface{}
-	yaml.Unmarshal(v, &m)
-	if err := ProcessDependencyEnabled(c, m); err != nil {
-		t.Errorf("Error processing enabled dependencies %v", err)
-	}
+			if err := processDependencyEnabled(c, tc.v); err != nil {
+				t.Fatalf("error processing enabled dependencies %v", err)
+			}
 
-	out := extractCharts(c, nil)
-	// build list of chart names
-	var p []string
-	for _, r := range out {
-		p = append(p, r.Name())
-	}
-	//sort alphanumeric and compare to expectations
-	sort.Strings(p)
-	if len(p) != len(e) {
-		t.Errorf("Error slice lengths do not match got %v, expected %v", len(p), len(e))
-		return
-	}
-	for i := range p {
-		if p[i] != e[i] {
-			t.Errorf("Error slice values do not match got %v, expected %v", p[i], e[i])
-		}
+			names := extractChartNames(c)
+			if len(names) != len(tc.e) {
+				t.Fatalf("slice lengths do not match got %v, expected %v", len(names), len(tc.e))
+			}
+			for i := range names {
+				if names[i] != tc.e[i] {
+					t.Fatalf("slice values do not match got %v, expected %v", names, tc.e)
+				}
+			}
+		})
 	}
 }
 
 // extractCharts recursively searches chart dependencies returning all charts found
-func extractCharts(c *chart.Chart, out []*chart.Chart) []*chart.Chart {
-	if len(c.Name()) > 0 {
-		out = append(out, c)
-	}
-	for _, d := range c.Dependencies() {
-		out = extractCharts(d, out)
+func extractChartNames(c *chart.Chart) []string {
+	var out []string
+	var fn func(c *chart.Chart)
+	fn = func(c *chart.Chart) {
+		out = append(out, c.ChartPath())
+		for _, d := range c.Dependencies() {
+			fn(d)
+		}
 	}
+	fn(c)
+	sort.Strings(out)
 	return out
 }
 
 func TestProcessDependencyImportValues(t *testing.T) {
-	c, err := loader.Load("testdata/subpop")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
+	c := loadChart(t, "testdata/subpop")
 
 	e := make(map[string]string)
 
@@ -213,71 +206,57 @@ func TestProcessDependencyImportValues(t *testing.T) {
 	e["SCBexported2A"] = "blaster"
 	e["global.SC1exported2.all.SC1exported3"] = "SC1expstr"
 
-	verifyDependencyImportValues(t, c, e)
-}
-
-func verifyDependencyImportValues(t *testing.T, c *chart.Chart, e map[string]string) {
-	if err := ProcessDependencyImportValues(c); err != nil {
-		t.Fatalf("Error processing import values dependencies %v", err)
+	if err := processDependencyImportValues(c); err != nil {
+		t.Fatalf("processing import values dependencies %v", err)
 	}
 	cc := Values(c.Values)
 	for kk, vv := range e {
 		pv, err := cc.PathValue(kk)
 		if err != nil {
-			t.Fatalf("Error retrieving import values table %v %v", kk, err)
-			return
+			t.Fatalf("retrieving import values table %v %v", kk, err)
 		}
 
 		switch pv.(type) {
 		case float64:
-			s := strconv.FormatFloat(pv.(float64), 'f', -1, 64)
-			if s != vv {
-				t.Errorf("Failed to match imported float value %v with expected %v", s, vv)
-				return
+			if s := strconv.FormatFloat(pv.(float64), 'f', -1, 64); s != vv {
+				t.Errorf("failed to match imported float value %v with expected %v", s, vv)
 			}
 		case bool:
-			b := strconv.FormatBool(pv.(bool))
-			if b != vv {
-				t.Errorf("Failed to match imported bool value %v with expected %v", b, vv)
-				return
+			if b := strconv.FormatBool(pv.(bool)); b != vv {
+				t.Errorf("failed to match imported bool value %v with expected %v", b, vv)
 			}
 		default:
 			if pv.(string) != vv {
-				t.Errorf("Failed to match imported string value %q with expected %q", pv, vv)
-				return
+				t.Errorf("failed to match imported string value %q with expected %q", pv, vv)
 			}
 		}
 	}
 }
 
 func TestGetAliasDependency(t *testing.T) {
-	c, err := loader.Load("testdata/frobnitz")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
-
+	c := loadChart(t, "testdata/frobnitz")
 	req := c.Metadata.Dependencies
 
 	if len(req) == 0 {
-		t.Fatalf("There are no dependencies to test")
+		t.Fatalf("there are no dependencies to test")
 	}
 
 	// Success case
 	aliasChart := getAliasDependency(c.Dependencies(), req[0])
 	if aliasChart == nil {
-		t.Fatalf("Failed to get dependency chart for alias %s", req[0].Name)
+		t.Fatalf("failed to get dependency chart for alias %s", req[0].Name)
 	}
 	if req[0].Alias != "" {
 		if aliasChart.Name() != req[0].Alias {
-			t.Fatalf("Dependency chart name should be %s but got %s", req[0].Alias, aliasChart.Name())
+			t.Fatalf("dependency chart name should be %s but got %s", req[0].Alias, aliasChart.Name())
 		}
 	} else if aliasChart.Name() != req[0].Name {
-		t.Fatalf("Dependency chart name should be %s but got %s", req[0].Name, aliasChart.Name())
+		t.Fatalf("dependency chart name should be %s but got %s", req[0].Name, aliasChart.Name())
 	}
 
 	if req[0].Version != "" {
 		if !version.IsCompatibleRange(req[0].Version, aliasChart.Metadata.Version) {
-			t.Fatalf("Dependency chart version is not in the compatible range")
+			t.Fatalf("dependency chart version is not in the compatible range")
 		}
 	}
 
@@ -289,161 +268,99 @@ func TestGetAliasDependency(t *testing.T) {
 
 	req[0].Version = "something else which is not in the compatible range"
 	if version.IsCompatibleRange(req[0].Version, aliasChart.Metadata.Version) {
-		t.Fatalf("Dependency chart version which is not in the compatible range should cause a failure other than a success ")
+		t.Fatalf("dependency chart version which is not in the compatible range should cause a failure other than a success ")
 	}
 }
 
 func TestDependentChartAliases(t *testing.T) {
-	c, err := loader.Load("testdata/dependent-chart-alias")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
+	c := loadChart(t, "testdata/dependent-chart-alias")
 
-	if len(c.Dependencies()) == 0 {
-		t.Fatal("There are no dependencies to run this test")
+	if len(c.Dependencies()) != 2 {
+		t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies()))
 	}
 
-	origLength := len(c.Dependencies())
-	if err := ProcessDependencyEnabled(c, c.Values); err != nil {
-		t.Fatalf("Expected no errors but got %q", err)
+	if err := processDependencyEnabled(c, c.Values); err != nil {
+		t.Fatalf("expected no errors but got %q", err)
 	}
 
-	if len(c.Dependencies()) == origLength {
-		t.Fatal("Expected alias dependencies to be added, but did not got that")
+	if len(c.Dependencies()) != 3 {
+		t.Fatal("expected alias dependencies to be added")
 	}
 
 	if len(c.Dependencies()) != len(c.Metadata.Dependencies) {
-		t.Fatalf("Expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies()))
+		t.Fatalf("expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies()))
 	}
+	// FIXME test for correct aliases
 }
 
 func TestDependentChartWithSubChartsAbsentInDependency(t *testing.T) {
-	c, err := loader.Load("testdata/dependent-chart-no-requirements-yaml")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
+	c := loadChart(t, "testdata/dependent-chart-no-requirements-yaml")
 
 	if len(c.Dependencies()) != 2 {
-		t.Fatalf("Expected 2 dependencies for this chart, but got %d", len(c.Dependencies()))
+		t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies()))
 	}
 
-	origLength := len(c.Dependencies())
-	if err := ProcessDependencyEnabled(c, c.Values); err != nil {
-		t.Fatalf("Expected no errors but got %q", err)
+	if err := processDependencyEnabled(c, c.Values); err != nil {
+		t.Fatalf("expected no errors but got %q", err)
 	}
 
-	if len(c.Dependencies()) != origLength {
-		t.Fatal("Expected no changes in dependencies to be, but did something got changed")
+	if len(c.Dependencies()) != 2 {
+		t.Fatal("expected no changes in dependencies")
 	}
 }
 
 func TestDependentChartWithSubChartsHelmignore(t *testing.T) {
-	if _, err := loader.Load("testdata/dependent-chart-helmignore"); err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
+	// FIXME what does this test?
+	loadChart(t, "testdata/dependent-chart-helmignore")
 }
 
 func TestDependentChartsWithSubChartsSymlink(t *testing.T) {
-	c, err := loader.Load("testdata/joonix")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
+	c := loadChart(t, "testdata/joonix")
+
 	if c.Name() != "joonix" {
-		t.Fatalf("Unexpected chart name: %s", c.Name())
+		t.Fatalf("unexpected chart name: %s", c.Name())
 	}
 	if n := len(c.Dependencies()); n != 1 {
-		t.Fatalf("Expected 1 dependency for this chart, but got %d", n)
+		t.Fatalf("expected 1 dependency for this chart, but got %d", n)
 	}
 }
 
 func TestDependentChartsWithSubchartsAllSpecifiedInDependency(t *testing.T) {
-	c, err := loader.Load("testdata/dependent-chart-with-all-in-requirements-yaml")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
+	c := loadChart(t, "testdata/dependent-chart-with-all-in-requirements-yaml")
 
-	if len(c.Dependencies()) == 0 {
-		t.Fatal("There are no dependencies to run this test")
+	if len(c.Dependencies()) != 2 {
+		t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies()))
 	}
 
-	origLength := len(c.Dependencies())
-	if err := ProcessDependencyEnabled(c, c.Values); err != nil {
-		t.Fatalf("Expected no errors but got %q", err)
+	if err := processDependencyEnabled(c, c.Values); err != nil {
+		t.Fatalf("expected no errors but got %q", err)
 	}
 
-	if len(c.Dependencies()) != origLength {
-		t.Fatal("Expected no changes in dependencies to be, but did something got changed")
+	if len(c.Dependencies()) != 2 {
+		t.Fatal("expected no changes in dependencies")
 	}
 
 	if len(c.Dependencies()) != len(c.Metadata.Dependencies) {
-		t.Fatalf("Expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies()))
+		t.Fatalf("expected number of chart dependencies %d, but got %d", len(c.Metadata.Dependencies), len(c.Dependencies()))
 	}
 }
 
 func TestDependentChartsWithSomeSubchartsSpecifiedInDependency(t *testing.T) {
-	c, err := loader.Load("testdata/dependent-chart-with-mixed-requirements-yaml")
-	if err != nil {
-		t.Fatalf("Failed to load testdata: %s", err)
-	}
-
-	if len(c.Dependencies()) == 0 {
-		t.Fatal("There are no dependencies to run this test")
-	}
-
-	origLength := len(c.Dependencies())
-	if err := ProcessDependencyEnabled(c, c.Values); err != nil {
-		t.Fatalf("Expected no errors but got %q", err)
-	}
+	c := loadChart(t, "testdata/dependent-chart-with-mixed-requirements-yaml")
 
-	if len(c.Dependencies()) != origLength {
-		t.Fatal("Expected no changes in dependencies to be, but did something got changed")
+	if len(c.Dependencies()) != 2 {
+		t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies()))
 	}
 
-	if len(c.Dependencies()) <= len(c.Metadata.Dependencies) {
-		t.Fatalf("Expected more dependencies than specified in Chart.yaml(%d), but got %d", len(c.Metadata.Dependencies), len(c.Dependencies()))
+	if err := processDependencyEnabled(c, c.Values); err != nil {
+		t.Fatalf("expected no errors but got %q", err)
 	}
-}
 
-func verifyDependency(t *testing.T, c *chart.Chart) {
-	if len(c.Metadata.Dependencies) != 2 {
-		t.Errorf("Expected 2 dependencies, got %d", len(c.Metadata.Dependencies))
-	}
-	tests := []*chart.Dependency{
-		{Name: "alpine", Version: "0.1.0", Repository: "https://example.com/charts"},
-		{Name: "mariner", Version: "4.3.2", Repository: "https://example.com/charts"},
-	}
-	for i, tt := range tests {
-		d := c.Metadata.Dependencies[i]
-		if d.Name != tt.Name {
-			t.Errorf("Expected dependency named %q, got %q", tt.Name, d.Name)
-		}
-		if d.Version != tt.Version {
-			t.Errorf("Expected dependency named %q to have version %q, got %q", tt.Name, tt.Version, d.Version)
-		}
-		if d.Repository != tt.Repository {
-			t.Errorf("Expected dependency named %q to have repository %q, got %q", tt.Name, tt.Repository, d.Repository)
-		}
+	if len(c.Dependencies()) != 2 {
+		t.Fatal("expected no changes in dependencies")
 	}
-}
 
-func verifyChartLock(t *testing.T, c *chart.Chart) {
-	if len(c.Metadata.Dependencies) != 2 {
-		t.Errorf("Expected 2 dependencies, got %d", len(c.Metadata.Dependencies))
-	}
-	tests := []*chart.Dependency{
-		{Name: "alpine", Version: "0.1.0", Repository: "https://example.com/charts"},
-		{Name: "mariner", Version: "4.3.2", Repository: "https://example.com/charts"},
-	}
-	for i, tt := range tests {
-		d := c.Metadata.Dependencies[i]
-		if d.Name != tt.Name {
-			t.Errorf("Expected dependency named %q, got %q", tt.Name, d.Name)
-		}
-		if d.Version != tt.Version {
-			t.Errorf("Expected dependency named %q to have version %q, got %q", tt.Name, tt.Version, d.Version)
-		}
-		if d.Repository != tt.Repository {
-			t.Errorf("Expected dependency named %q to have repository %q, got %q", tt.Name, tt.Repository, d.Repository)
-		}
+	if len(c.Metadata.Dependencies) != 1 {
+		t.Fatalf("expected 1 dependency specified in Chart.yaml, got %d", len(c.Metadata.Dependencies))
 	}
 }

pkg/chartutil/testdata/subpop/Chart.yaml

@@ -3,33 +3,33 @@ description: A Helm chart for Kubernetes
 name: parentchart
 version: 0.1.0
 dependencies:
-      - name: subchart1
-        repository: http://localhost:10191
-        version: 0.1.0
-        condition: subchart1.enabled
-        tags:
-          - front-end
-          - subchart1
-        import-values:
-          - child: SC1data
-            parent: imported-chart1
-          - child: SC1data
-            parent: overridden-chart1
-          - child: imported-chartA
-            parent: imported-chartA
-          - child: imported-chartA-B
-            parent: imported-chartA-B
-          - child: overridden-chartA-B
-            parent: overridden-chartA-B
-          - child: SCBexported1A
-            parent: .
-          - SCBexported2
-          - SC1exported1
+  - name: subchart1
+    repository: http://localhost:10191
+    version: 0.1.0
+    condition: subchart1.enabled
+    tags:
+      - front-end
+      - subchart1
+    import-values:
+      - child: SC1data
+        parent: imported-chart1
+      - child: SC1data
+        parent: overridden-chart1
+      - child: imported-chartA
+        parent: imported-chartA
+      - child: imported-chartA-B
+        parent: imported-chartA-B
+      - child: overridden-chartA-B
+        parent: overridden-chartA-B
+      - child: SCBexported1A
+        parent: .
+      - SCBexported2
+      - SC1exported1
 
-      - name: subchart2
-        repository: http://localhost:10191
-        version: 0.1.0
-        condition: subchart2.enabled
-        tags:
-          - back-end
-          - subchart2
+  - name: subchart2
+    repository: http://localhost:10191
+    version: 0.1.0
+    condition: subchart2.enabled
+    tags:
+      - back-end
+      - subchart2

pkg/chartutil/testdata/subpop/charts/subchart1/Chart.yaml

@@ -3,34 +3,34 @@ description: A Helm chart for Kubernetes
 name: subchart1
 version: 0.1.0
 dependencies:
-      - name: subcharta
-        repository: http://localhost:10191
-        version: 0.1.0
-        condition: subcharta.enabled,subchart1.subcharta.enabled
-        tags:
-          - front-end
-          - subcharta
-        import-values:
-          - child: SCAdata
-            parent: imported-chartA
-          - child: SCAdata
-            parent: overridden-chartA
-          - child: SCAdata
-            parent: imported-chartA-B
+  - name: subcharta
+    repository: http://localhost:10191
+    version: 0.1.0
+    condition: subcharta.enabled,subchart1.subcharta.enabled
+    tags:
+      - front-end
+      - subcharta
+    import-values:
+      - child: SCAdata
+        parent: imported-chartA
+      - child: SCAdata
+        parent: overridden-chartA
+      - child: SCAdata
+        parent: imported-chartA-B
 
-      - name: subchartb
-        repository: http://localhost:10191
-        version: 0.1.0
-        condition: subchartb.enabled
-        import-values:
-          - child: SCBdata
-            parent: imported-chartB
-          - child: SCBdata
-            parent: imported-chartA-B
-          - child: exports.SCBexported2
-            parent: exports.SCBexported2
-          - SCBexported1
+  - name: subchartb
+    repository: http://localhost:10191
+    version: 0.1.0
+    condition: subchartb.enabled
+    import-values:
+      - child: SCBdata
+        parent: imported-chartB
+      - child: SCBdata
+        parent: imported-chartA-B
+      - child: exports.SCBexported2
+        parent: exports.SCBexported2
+      - SCBexported1
 
-        tags:
-          - front-end
-          - subchartb
+    tags:
+      - front-end
+      - subchartb

pkg/chartutil/testdata/subpop/charts/subchart2/Chart.yaml

@@ -3,17 +3,17 @@ description: A Helm chart for Kubernetes
 name: subchart2
 version: 0.1.0
 dependencies:
-      - name: subchartb
-        repository: http://localhost:10191
-        version: 0.1.0
-        condition: subchartb.enabled,subchart2.subchartb.enabled
-        tags:
-          - back-end
-          - subchartb
-      - name: subchartc
-        repository: http://localhost:10191
-        version: 0.1.0
-        condition: subchartc.enabled
-        tags:
-          - back-end
-          - subchartc
+  - name: subchartb
+    repository: http://localhost:10191
+    version: 0.1.0
+    condition: subchartb.enabled,subchart2.subchartb.enabled
+    tags:
+      - back-end
+      - subchartb
+  - name: subchartc
+    repository: http://localhost:10191
+    version: 0.1.0
+    condition: subchartc.enabled
+    tags:
+      - back-end
+      - subchartc

pkg/chartutil/testdata/subpop/values.yaml

@@ -38,4 +38,3 @@ overridden-chartA-B:
 tags:
   front-end: true
   back-end: false
-

pkg/chartutil/values.go

@@ -17,6 +17,7 @@ limitations under the License.
 package chartutil
 
 import (
+	"fmt"
 	"io"
 	"io/ioutil"
 	"log"
@@ -29,10 +30,14 @@ import (
 )
 
 // ErrNoTable indicates that a chart does not have a matching table.
-type ErrNoTable error
+type ErrNoTable string
+
+func (e ErrNoTable) Error() string { return fmt.Sprintf("%q is not a table", e) }
 
 // ErrNoValue indicates that Values does not contain a key with a value
-type ErrNoValue error
+type ErrNoValue string
+
+func (e ErrNoValue) Error() string { return fmt.Sprintf("%q is not a value", e) }
 
 // GlobalKey is the name of the Values key that is used for storing global vars.
 const GlobalKey = "global"
@@ -63,9 +68,8 @@ func (v Values) Table(name string) (Values, error) {
 	var err error
 
 	for _, n := range parsePath(name) {
-		table, err = tableLookup(table, n)
-		if err != nil {
-			return table, err
+		if table, err = tableLookup(table, n); err != nil {
+			break
 		}
 	}
 	return table, err
@@ -95,7 +99,7 @@ func (v Values) Encode(w io.Writer) error {
 func tableLookup(v Values, simple string) (Values, error) {
 	v2, ok := v[simple]
 	if !ok {
-		return v, ErrNoTable(errors.Errorf("no table named %q (%v)", simple, v))
+		return v, ErrNoTable(simple)
 	}
 	if vv, ok := v2.(map[string]interface{}); ok {
 		return vv, nil
@@ -108,8 +112,7 @@ func tableLookup(v Values, simple string) (Values, error) {
 		return vv, nil
 	}
 
-	var e ErrNoTable = errors.Errorf("no table named %q", simple)
-	return Values{}, e
+	return Values{}, ErrNoTable(simple)
 }
 
 // ReadValues will parse YAML byte data into a Values.
@@ -150,19 +153,18 @@ func CoalesceValues(chrt *chart.Chart, vals []byte) (Values, error) {
 			return cvals, err
 		}
 	}
-
-	coalesce(chrt, cvals)
-
+	if _, err := coalesce(chrt, cvals); err != nil {
+		return cvals, err
+	}
 	return coalesceDeps(chrt, cvals)
 }
 
 // coalesce coalesces the dest values and the chart values, giving priority to the dest values.
 //
 // This is a helper function for CoalesceValues.
-func coalesce(ch *chart.Chart, dest map[string]interface{}) map[string]interface{} {
+func coalesce(ch *chart.Chart, dest map[string]interface{}) (map[string]interface{}, error) {
 	coalesceValues(ch, dest)
-	coalesceDeps(ch, dest)
-	return dest
+	return coalesceDeps(ch, dest)
 }
 
 // coalesceDeps coalesces the dependencies of the given chart.
@@ -181,7 +183,11 @@ func coalesceDeps(chrt *chart.Chart, dest map[string]interface{}) (map[string]in
 			coalesceGlobals(dvmap, dest)
 
 			// Now coalesce the rest of the values.
-			dest[subchart.Name()] = coalesce(subchart, dvmap)
+			var err error
+			dest[subchart.Name()], err = coalesce(subchart, dvmap)
+			if err != nil {
+				return dest, err
+			}
 		}
 	}
 	return dest, nil
@@ -363,20 +369,20 @@ func (v Values) pathValue(path []string) (interface{}, error) {
 		if _, ok := v[path[0]]; ok && !istable(v[path[0]]) {
 			return v[path[0]], nil
 		}
-		return nil, ErrNoValue(errors.Errorf("%v is not a value", path[0]))
+		return nil, ErrNoValue(path[0])
 	}
 
 	key, path := path[len(path)-1], path[:len(path)-1]
 	// get our table for table path
 	t, err := v.Table(joinPath(path...))
 	if err != nil {
-		return nil, ErrNoValue(errors.Errorf("%v is not a value", key))
+		return nil, ErrNoValue(key)
 	}
 	// check table for key and ensure value is not a table
 	if k, ok := t[key]; ok && !istable(k) {
 		return k, nil
 	}
-	return nil, ErrNoValue(errors.Errorf("key not found: %s", key))
+	return nil, ErrNoValue(key)
 }
 
 func parsePath(key string) []string { return strings.Split(key, ".") }

pkg/chartutil/values_test.go

@@ -26,7 +26,6 @@ import (
 	kversion "k8s.io/apimachinery/pkg/version"
 
 	"k8s.io/helm/pkg/chart"
-	"k8s.io/helm/pkg/chart/loader"
 	"k8s.io/helm/pkg/version"
 )
 
@@ -290,11 +289,7 @@ pequod:
 `
 
 func TestCoalesceValues(t *testing.T) {
-	tchart := "testdata/moby"
-	c, err := loader.LoadDir(tchart)
-	if err != nil {
-		t.Fatal(err)
-	}
+	c := loadChart(t, "testdata/moby")
 
 	v, err := CoalesceValues(c, []byte(testCoalesceValuesYaml))
 	if err != nil {

pkg/helm/client.go

@@ -97,15 +97,10 @@ func (c *Client) InstallReleaseFromChart(chart *chart.Chart, ns string, opts ...
 	}
 	var m map[string]interface{}
 	yaml.Unmarshal(req.Values, &m)
-	err := chartutil.ProcessDependencyEnabled(req.Chart, m)
+	err := chartutil.ProcessDependencies(req.Chart, m)
 	if err != nil {
 		return nil, err
 	}
-	err = chartutil.ProcessDependencyImportValues(req.Chart)
-	if err != nil {
-		return nil, err
-	}
-
 	return c.tiller.InstallRelease(req)
 }
 
@@ -171,13 +166,9 @@ func (c *Client) UpdateReleaseFromChart(rlsName string, chart *chart.Chart, opts
 	if err := yaml.Unmarshal(req.Values, &m); err != nil {
 		return nil, err
 	}
-	if err := chartutil.ProcessDependencyEnabled(req.Chart, m); err != nil {
+	if err := chartutil.ProcessDependencies(req.Chart, m); err != nil {
 		return nil, err
 	}
-	if err := chartutil.ProcessDependencyImportValues(req.Chart); err != nil {
-		return nil, err
-	}
-
 	return c.tiller.UpdateRelease(req)
 }