diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index bd75375a4..a3bed63a3 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -17,6 +17,7 @@ package chart import ( "path/filepath" + "regexp" "strings" ) @@ -26,6 +27,9 @@ const APIVersionV1 = "v1" // APIVersionV2 is the API version number for version 2. const APIVersionV2 = "v2" +// aliasNameFormat defines the characters that are legal in an alias name. +var aliasNameFormat = regexp.MustCompile("^[a-zA-Z0-9_-]+$") + // Chart is a helm package that contains metadata, a default config, zero or more // optionally parameterizable templates, and zero or more charts (dependencies). type Chart struct { diff --git a/pkg/chart/errors.go b/pkg/chart/errors.go index 4cb4189e6..2fad5f370 100644 --- a/pkg/chart/errors.go +++ b/pkg/chart/errors.go @@ -15,9 +15,16 @@ limitations under the License. package chart +import "fmt" + // ValidationError represents a data validation error. type ValidationError string func (v ValidationError) Error() string { return "validation: " + string(v) } + +// ValidationErrorf takes a message and formatting options and creates a ValidationError +func ValidationErrorf(msg string, args ...interface{}) ValidationError { + return ValidationError(fmt.Sprintf(msg, args...)) +} diff --git a/pkg/chart/metadata.go b/pkg/chart/metadata.go index 96a3965b9..1848eb280 100644 --- a/pkg/chart/metadata.go +++ b/pkg/chart/metadata.go @@ -81,6 +81,15 @@ func (md *Metadata) Validate() error { if !isValidChartType(md.Type) { return ValidationError("chart.metadata.type must be application or library") } + + // Aliases need to be validated here to make sure that the alias name does + // not contain any illegal characters. + for _, dependency := range md.Dependencies { + if err := validateDependency(dependency); err != nil { + return err + } + } + // TODO validate valid semver here? return nil } @@ -92,3 +101,13 @@ func isValidChartType(in string) bool { } return false } + +// validateDependency checks for common problems with the dependency datastructure in +// the chart. This check must be done at load time before the dependency's charts are +// loaded. +func validateDependency(dep *Dependency) error { + if len(dep.Alias) > 0 && !aliasNameFormat.MatchString(dep.Alias) { + return ValidationErrorf("dependency %q has disallowed characters in the alias", dep.Name) + } + return nil +} diff --git a/pkg/chart/metadata_test.go b/pkg/chart/metadata_test.go index 8b436000b..0c7b173dd 100644 --- a/pkg/chart/metadata_test.go +++ b/pkg/chart/metadata_test.go @@ -48,12 +48,60 @@ func TestValidate(t *testing.T) { &Metadata{Name: "test", APIVersion: "v2", Version: "1.0", Type: "application"}, nil, }, + { + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + Dependencies: []*Dependency{ + {Name: "dependency", Alias: "legal-alias"}, + }, + }, + nil, + }, + { + &Metadata{ + Name: "test", + APIVersion: "v2", + Version: "1.0", + Type: "application", + Dependencies: []*Dependency{ + {Name: "bad", Alias: "illegal alias"}, + }, + }, + ValidationError("dependency \"bad\" has disallowed characters in the alias"), + }, } for _, tt := range tests { result := tt.md.Validate() if result != tt.err { - t.Errorf("expected %s, got %s", tt.err, result) + t.Errorf("expected '%s', got '%s'", tt.err, result) + } + } +} + +func TestValidateDependency(t *testing.T) { + dep := &Dependency{ + Name: "example", + } + for value, shouldFail := range map[string]bool{ + "abcdefghijklmenopQRSTUVWXYZ-0123456780_": false, + "-okay": false, + "_okay": false, + "- bad": true, + " bad": true, + "bad\nvalue": true, + "bad ": true, + "bad$": true, + } { + dep.Alias = value + res := validateDependency(dep) + if res != nil && !shouldFail { + t.Errorf("Failed on case %q", dep.Alias) + } else if res == nil && shouldFail { + t.Errorf("Expected failure for %q", dep.Alias) } } }