diff --git a/pkg/chart/metadata.go b/pkg/chart/metadata.go index ae572abb7..97bfc2c0c 100644 --- a/pkg/chart/metadata.go +++ b/pkg/chart/metadata.go @@ -128,10 +128,19 @@ func (md *Metadata) Validate() error { // Aliases need to be validated here to make sure that the alias name does // not contain any illegal characters. + dependencies := map[string]*Dependency{} for _, dependency := range md.Dependencies { if err := dependency.Validate(); err != nil { return err } + key := dependency.Name + if dependency.Alias != "" { + key = dependency.Alias + } + if dependencies[key] != nil { + return ValidationErrorf("more than one dependency with name or alias %q", key) + } + dependencies[key] = dependency } return nil } diff --git a/pkg/chart/metadata_test.go b/pkg/chart/metadata_test.go index cc04f095b..c8b89ae55 100644 --- a/pkg/chart/metadata_test.go +++ b/pkg/chart/metadata_test.go @@ -21,34 +21,42 @@ import ( func TestValidate(t *testing.T) { tests := []struct { - md *Metadata - err error + name string + md *Metadata + err error }{ { + "chart without metadata", nil, ValidationError("chart.metadata is required"), }, { + "chart without apiVersion", &Metadata{Name: "test", Version: "1.0"}, ValidationError("chart.metadata.apiVersion is required"), }, { + "chart without name", &Metadata{APIVersion: "v2", Version: "1.0"}, ValidationError("chart.metadata.name is required"), }, { + "chart without version", &Metadata{Name: "test", APIVersion: "v2"}, ValidationError("chart.metadata.version is required"), }, { + "chart with bad type", &Metadata{Name: "test", APIVersion: "v2", Version: "1.0", Type: "test"}, ValidationError("chart.metadata.type must be application or library"), }, { + "chart without dependency", &Metadata{Name: "test", APIVersion: "v2", Version: "1.0", Type: "application"}, nil, }, { + "dependency with valid alias", &Metadata{ Name: "test", APIVersion: "v2", @@ -61,6 +69,7 @@ func TestValidate(t *testing.T) { nil, }, { + "dependency with bad characters in alias", &Metadata{ Name: "test", APIVersion: "v2", @@ -73,6 +82,67 @@ func TestValidate(t *testing.T) { ValidationError("dependency \"bad\" has disallowed characters in the alias"), }, { + "same dependency twice", + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + Dependencies: []*Dependency{ + {Name: "foo", Alias: ""}, + {Name: "foo", Alias: ""}, + }, + }, + ValidationError("more than one dependency with name or alias \"foo\""), + }, + { + "two dependencies with alias from second dependency shadowing first one", + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + Dependencies: []*Dependency{ + {Name: "foo", Alias: ""}, + {Name: "bar", Alias: "foo"}, + }, + }, + ValidationError("more than one dependency with name or alias \"foo\""), + }, + { + // this case would make sense and could work in future versions of Helm, currently template rendering would + // result in undefined behaviour + "same dependency twice with different version", + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + Dependencies: []*Dependency{ + {Name: "foo", Alias: "", Version: "1.2.3"}, + {Name: "foo", Alias: "", Version: "1.0.0"}, + }, + }, + ValidationError("more than one dependency with name or alias \"foo\""), + }, + { + // this case would make sense and could work in future versions of Helm, currently template rendering would + // result in undefined behaviour + "two dependencies with same name but different repos", + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + Dependencies: []*Dependency{ + {Name: "foo", Repository: "repo-0"}, + {Name: "foo", Repository: "repo-1"}, + }, + }, + ValidationError("more than one dependency with name or alias \"foo\""), + }, + { + "dependencies has nil", &Metadata{ Name: "test", APIVersion: "v2", @@ -85,6 +155,7 @@ func TestValidate(t *testing.T) { ValidationError("dependencies must not contain empty or null nodes"), }, { + "maintainer not empty", &Metadata{ Name: "test", APIVersion: "v2", @@ -97,6 +168,7 @@ func TestValidate(t *testing.T) { ValidationError("maintainers must not contain empty or null nodes"), }, { + "version invalid", &Metadata{APIVersion: "v2", Name: "test", Version: "1.2.3.4"}, ValidationError("chart.metadata.version \"1.2.3.4\" is invalid"), }, @@ -105,7 +177,7 @@ func TestValidate(t *testing.T) { for _, tt := range tests { result := tt.md.Validate() if result != tt.err { - t.Errorf("expected '%s', got '%s'", tt.err, result) + t.Errorf("expected %q, got %q in test %q", tt.err, result, tt.name) } } } diff --git a/pkg/lint/rules/dependencies.go b/pkg/lint/rules/dependencies.go index abecd1feb..f1ab1dcad 100644 --- a/pkg/lint/rules/dependencies.go +++ b/pkg/lint/rules/dependencies.go @@ -37,6 +37,7 @@ func Dependencies(linter *support.Linter) { } linter.RunLinterRule(support.ErrorSev, linter.ChartDir, validateDependencyInMetadata(c)) + linter.RunLinterRule(support.ErrorSev, linter.ChartDir, validateDependenciesUnique(c)) linter.RunLinterRule(support.WarningSev, linter.ChartDir, validateDependencyInChartsDir(c)) } @@ -80,3 +81,23 @@ func validateDependencyInMetadata(c *chart.Chart) (err error) { } return err } + +func validateDependenciesUnique(c *chart.Chart) (err error) { + dependencies := map[string]*chart.Dependency{} + shadowing := []string{} + + for _, dep := range c.Metadata.Dependencies { + key := dep.Name + if dep.Alias != "" { + key = dep.Alias + } + if dependencies[key] != nil { + shadowing = append(shadowing, key) + } + dependencies[key] = dep + } + if len(shadowing) > 0 { + err = fmt.Errorf("multiple dependencies with name or alias: %s", strings.Join(shadowing, ",")) + } + return err +} diff --git a/pkg/lint/rules/dependencies_test.go b/pkg/lint/rules/dependencies_test.go index 24c5faf7f..c0afff133 100644 --- a/pkg/lint/rules/dependencies_test.go +++ b/pkg/lint/rules/dependencies_test.go @@ -76,6 +76,67 @@ func TestValidateDependencyInMetadata(t *testing.T) { } } +func TestValidateDependenciesUnique(t *testing.T) { + tests := []struct { + chart chart.Chart + }{ + {chart.Chart{ + Metadata: &chart.Metadata{ + Name: "badchart", + Version: "0.1.0", + APIVersion: "v2", + Dependencies: []*chart.Dependency{ + { + Name: "foo", + }, + { + Name: "foo", + }, + }, + }, + }}, + {chart.Chart{ + Metadata: &chart.Metadata{ + Name: "badchart", + Version: "0.1.0", + APIVersion: "v2", + Dependencies: []*chart.Dependency{ + { + Name: "foo", + Alias: "bar", + }, + { + Name: "bar", + }, + }, + }, + }}, + {chart.Chart{ + Metadata: &chart.Metadata{ + Name: "badchart", + Version: "0.1.0", + APIVersion: "v2", + Dependencies: []*chart.Dependency{ + { + Name: "foo", + Alias: "baz", + }, + { + Name: "bar", + Alias: "baz", + }, + }, + }, + }}, + } + + for _, tt := range tests { + if err := validateDependenciesUnique(&tt.chart); err == nil { + t.Errorf("chart should have been flagged for dependency shadowing") + } + } +} + func TestDependencies(t *testing.T) { tmp := t.TempDir()