diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index e845751c1..23dca194a 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -63,9 +63,15 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command { func (o *repoUpdateOptions) run(out io.Writer) error { f, err := repo.LoadFile(o.repoFile) - if isNotExist(err) || len(f.Repositories) == 0 { + switch { + case isNotExist(err): + return errNoRepositories + case err != nil: + return errors.Wrapf(err, "failed loading file: %s", o.repoFile) + case len(f.Repositories) == 0: return errNoRepositories } + var repos []*repo.ChartRepository for _, cfg := range f.Repositories { r, err := repo.NewChartRepository(cfg, getter.All(settings)) diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index f4936b367..83ef24349 100644 --- a/cmd/helm/repo_update_test.go +++ b/cmd/helm/repo_update_test.go @@ -19,6 +19,7 @@ import ( "bytes" "fmt" "io" + "io/ioutil" "os" "path/filepath" "strings" @@ -53,20 +54,27 @@ func TestUpdateCmd(t *testing.T) { } func TestUpdateCustomCacheCmd(t *testing.T) { - var out bytes.Buffer rootDir := ensure.TempDir(t) cachePath := filepath.Join(rootDir, "updcustomcache") - _ = os.Mkdir(cachePath, os.ModePerm) + os.Mkdir(cachePath, os.ModePerm) defer os.RemoveAll(cachePath) + + ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") + if err != nil { + t.Fatal(err) + } + defer ts.Stop() + o := &repoUpdateOptions{ update: updateCharts, - repoFile: "testdata/repositories.yaml", + repoFile: filepath.Join(ts.Root(), "repositories.yaml"), repoCache: cachePath, } - if err := o.run(&out); err != nil { + b := ioutil.Discard + if err := o.run(b); err != nil { t.Fatal(err) } - if _, err := os.Stat(filepath.Join(cachePath, "charts-index.yaml")); err != nil { + if _, err := os.Stat(filepath.Join(cachePath, "test-index.yaml")); err != nil { t.Fatalf("error finding created index file in custom cache: %v", err) } } diff --git a/cmd/helm/search_repo.go b/cmd/helm/search_repo.go index ba26ee5e9..ba692a2e7 100644 --- a/cmd/helm/search_repo.go +++ b/cmd/helm/search_repo.go @@ -143,7 +143,7 @@ func (o *searchRepoOptions) setupSearchedVersion() { } func (o *searchRepoOptions) applyConstraint(res []*search.Result) ([]*search.Result, error) { - if len(o.version) == 0 { + if o.version == "" { return res, nil } @@ -154,26 +154,19 @@ func (o *searchRepoOptions) applyConstraint(res []*search.Result) ([]*search.Res data := res[:0] foundNames := map[string]bool{} - appendSearchResults := func(res *search.Result) { - data = append(data, res) - if !o.versions { - foundNames[res.Name] = true // If user hasn't requested all versions, only show the latest that matches - } - } for _, r := range res { - if _, found := foundNames[r.Name]; found { + // if not returning all versions and already have found a result, + // you're done! + if !o.versions && foundNames[r.Name] { continue } v, err := semver.NewVersion(r.Chart.Version) - if err != nil { - // If the current version number check appears ErrSegmentStartsZero or ErrInvalidPrerelease error and not devel mode, ignore - if (err == semver.ErrSegmentStartsZero || err == semver.ErrInvalidPrerelease) && !o.devel { - continue - } - appendSearchResults(r) - } else if constraint.Check(v) { - appendSearchResults(r) + continue + } + if constraint.Check(v) { + data = append(data, r) + foundNames[r.Name] = true } } @@ -194,6 +187,7 @@ func (o *searchRepoOptions) buildIndex() (*search.Index, error) { ind, err := repo.LoadIndexFile(f) if err != nil { warning("Repo %q is corrupt or missing. Try 'helm repo update'.", n) + warning("%s", err) continue } diff --git a/cmd/helm/search_repo_test.go b/cmd/helm/search_repo_test.go index 86519cd42..39c9c53f5 100644 --- a/cmd/helm/search_repo_test.go +++ b/cmd/helm/search_repo_test.go @@ -68,14 +68,6 @@ func TestSearchRepositoriesCmd(t *testing.T) { name: "search for 'maria', expect valid json output", cmd: "search repo maria --output json", golden: "output/search-output-json.txt", - }, { - name: "search for 'maria', expect one match with semver begin with zero development version", - cmd: "search repo maria --devel", - golden: "output/search-semver-pre-zero-devel-release.txt", - }, { - name: "search for 'nginx-ingress', expect one match with invalid development pre version", - cmd: "search repo nginx-ingress --devel", - golden: "output/search-semver-pre-invalid-release.txt", }, { name: "search for 'alpine', expect valid yaml output", cmd: "search repo alpine --output yaml", diff --git a/cmd/helm/testdata/helmhome/helm/repository/testing-index.yaml b/cmd/helm/testdata/helmhome/helm/repository/testing-index.yaml index d76501e57..889d7d87a 100644 --- a/cmd/helm/testdata/helmhome/helm/repository/testing-index.yaml +++ b/cmd/helm/testdata/helmhome/helm/repository/testing-index.yaml @@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine url: https://charts.helm.sh/stable/alpine-0.2.0.tgz checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d @@ -25,6 +26,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine url: https://charts.helm.sh/stable/alpine-0.3.0-rc.1.tgz checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d @@ -37,6 +39,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 mariadb: - name: mariadb url: https://charts.helm.sh/stable/mariadb-0.3.0.tgz @@ -55,33 +58,4 @@ entries: - name: Bitnami email: containers@bitnami.com icon: "" - - name: mariadb - url: https://charts.helm.sh/stable/mariadb-0.3.0-0565674.tgz - checksum: 65229f6de44a2be9f215d11dbff311673fc8ba56 - home: https://mariadb.org - sources: - - https://github.com/bitnami/bitnami-docker-mariadb - version: 0.3.0-0565674 - description: Chart for MariaDB - keywords: - - mariadb - - mysql - - database - - sql - maintainers: - - name: Bitnami - email: containers@bitnami.com - icon: "" - nginx-ingress: - - name: nginx-ingress - url: https://github.com/kubernetes/ingress-nginx/ingress-a.b.c.sdfsdf.tgz - checksum: 25229f6de44a2be9f215d11dbff31167ddc8ba56 - home: https://github.com/kubernetes/ingress-nginx - sources: - - https://github.com/kubernetes/ingress-nginx - version: a.b.c.sdfsdf - description: Chart for nginx-ingress - keywords: - - ingress - - nginx - icon: "" + apiVersion: v2 diff --git a/cmd/helm/testdata/output/search-semver-pre-invalid-release.txt b/cmd/helm/testdata/output/search-semver-pre-invalid-release.txt deleted file mode 100644 index ea39e2978..000000000 --- a/cmd/helm/testdata/output/search-semver-pre-invalid-release.txt +++ /dev/null @@ -1,2 +0,0 @@ -NAME CHART VERSION APP VERSION DESCRIPTION -testing/nginx-ingress a.b.c.sdfsdf Chart for nginx-ingress diff --git a/cmd/helm/testdata/output/search-semver-pre-zero-devel-release.txt b/cmd/helm/testdata/output/search-semver-pre-zero-devel-release.txt deleted file mode 100644 index 971c6523e..000000000 --- a/cmd/helm/testdata/output/search-semver-pre-zero-devel-release.txt +++ /dev/null @@ -1,2 +0,0 @@ -NAME CHART VERSION APP VERSION DESCRIPTION -testing/mariadb 0.3.0-0565674 Chart for MariaDB diff --git a/internal/resolver/testdata/repository/kubernetes-charts-index.yaml b/internal/resolver/testdata/repository/kubernetes-charts-index.yaml index 493a162f5..c6b7962a1 100644 --- a/internal/resolver/testdata/repository/kubernetes-charts-index.yaml +++ b/internal/resolver/testdata/repository/kubernetes-charts-index.yaml @@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine urls: - https://charts.helm.sh/stable/alpine-0.2.0.tgz @@ -25,6 +26,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 mariadb: - name: mariadb urls: @@ -44,3 +46,4 @@ entries: - name: Bitnami email: containers@bitnami.com icon: "" + apiVersion: v2 diff --git a/pkg/chart/dependency.go b/pkg/chart/dependency.go index 9ec4544c2..b2819f373 100644 --- a/pkg/chart/dependency.go +++ b/pkg/chart/dependency.go @@ -49,6 +49,23 @@ type Dependency struct { Alias string `json:"alias,omitempty"` } +// Validate 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 (d *Dependency) Validate() error { + d.Name = sanitizeString(d.Name) + d.Version = sanitizeString(d.Version) + d.Repository = sanitizeString(d.Repository) + d.Condition = sanitizeString(d.Condition) + for i := range d.Tags { + d.Tags[i] = sanitizeString(d.Tags[i]) + } + if d.Alias != "" && !aliasNameFormat.MatchString(d.Alias) { + return ValidationErrorf("dependency %q has disallowed characters in the alias", d.Name) + } + return nil +} + // Lock is a lock file for dependencies. // // It represents the state that the dependencies should be in. diff --git a/pkg/chart/dependency_test.go b/pkg/chart/dependency_test.go new file mode 100644 index 000000000..99c45b4b5 --- /dev/null +++ b/pkg/chart/dependency_test.go @@ -0,0 +1,44 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package chart + +import ( + "testing" +) + +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 := dep.Validate() + 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) + } + } +} diff --git a/pkg/chart/metadata.go b/pkg/chart/metadata.go index 1848eb280..1925e45ac 100644 --- a/pkg/chart/metadata.go +++ b/pkg/chart/metadata.go @@ -15,6 +15,13 @@ limitations under the License. package chart +import ( + "strings" + "unicode" + + "github.com/Masterminds/semver/v3" +) + // Maintainer describes a Chart maintainer. type Maintainer struct { // Name is a user name or organization name @@ -25,15 +32,23 @@ type Maintainer struct { URL string `json:"url,omitempty"` } +// Validate checks valid data and sanitizes string characters. +func (m *Maintainer) Validate() error { + m.Name = sanitizeString(m.Name) + m.Email = sanitizeString(m.Email) + m.URL = sanitizeString(m.URL) + return nil +} + // Metadata for a Chart file. This models the structure of a Chart.yaml file. type Metadata struct { - // The name of the chart + // The name of the chart. Required. Name string `json:"name,omitempty"` // The URL to a relevant project page, git repo, or contact person Home string `json:"home,omitempty"` // Source is the URL to the source code of this chart Sources []string `json:"sources,omitempty"` - // A SemVer 2 conformant version string of the chart + // A SemVer 2 conformant version string of the chart. Required. Version string `json:"version,omitempty"` // A one-sentence description of the chart Description string `json:"description,omitempty"` @@ -43,7 +58,7 @@ type Metadata struct { Maintainers []*Maintainer `json:"maintainers,omitempty"` // The URL to an icon file. Icon string `json:"icon,omitempty"` - // The API Version of this chart. + // The API Version of this chart. Required. APIVersion string `json:"apiVersion,omitempty"` // The condition to check to enable chart Condition string `json:"condition,omitempty"` @@ -64,11 +79,28 @@ type Metadata struct { Type string `json:"type,omitempty"` } -// Validate checks the metadata for known issues, returning an error if metadata is not correct +// Validate checks the metadata for known issues and sanitizes string +// characters. func (md *Metadata) Validate() error { if md == nil { return ValidationError("chart.metadata is required") } + + md.Name = sanitizeString(md.Name) + md.Description = sanitizeString(md.Description) + md.Home = sanitizeString(md.Home) + md.Icon = sanitizeString(md.Icon) + md.Condition = sanitizeString(md.Condition) + md.Tags = sanitizeString(md.Tags) + md.AppVersion = sanitizeString(md.AppVersion) + md.KubeVersion = sanitizeString(md.KubeVersion) + for i := range md.Sources { + md.Sources[i] = sanitizeString(md.Sources[i]) + } + for i := range md.Keywords { + md.Keywords[i] = sanitizeString(md.Keywords[i]) + } + if md.APIVersion == "" { return ValidationError("chart.metadata.apiVersion is required") } @@ -78,19 +110,26 @@ func (md *Metadata) Validate() error { if md.Version == "" { return ValidationError("chart.metadata.version is required") } + if !isValidSemver(md.Version) { + return ValidationErrorf("chart.metadata.version %q is invalid", md.Version) + } if !isValidChartType(md.Type) { return ValidationError("chart.metadata.type must be application or library") } + for _, m := range md.Maintainers { + if err := m.Validate(); err != nil { + return err + } + } + // 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 { + if err := dependency.Validate(); err != nil { return err } } - - // TODO validate valid semver here? return nil } @@ -102,12 +141,20 @@ 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 +func isValidSemver(v string) bool { + _, err := semver.NewVersion(v) + return err == nil +} + +// sanitizeString normalize spaces and removes non-printable characters. +func sanitizeString(str string) string { + return strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return ' ' + } + if unicode.IsPrint(r) { + return r + } + return -1 + }, str) } diff --git a/pkg/chart/metadata_test.go b/pkg/chart/metadata_test.go index 0c7b173dd..9f881a4e1 100644 --- a/pkg/chart/metadata_test.go +++ b/pkg/chart/metadata_test.go @@ -72,6 +72,10 @@ func TestValidate(t *testing.T) { }, ValidationError("dependency \"bad\" has disallowed characters in the alias"), }, + { + &Metadata{APIVersion: "v2", Name: "test", Version: "1.2.3.4"}, + ValidationError("chart.metadata.version \"1.2.3.4\" is invalid"), + }, } for _, tt := range tests { @@ -82,26 +86,15 @@ func TestValidate(t *testing.T) { } } -func TestValidateDependency(t *testing.T) { - dep := &Dependency{ - Name: "example", +func TestValidate_sanitize(t *testing.T) { + md := &Metadata{APIVersion: "v2", Name: "test", Version: "1.0", Description: "\adescr\u0081iption\rtest", Maintainers: []*Maintainer{{Name: "\r"}}} + if err := md.Validate(); err != nil { + t.Fatalf("unexpected error: %s", err) } - 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) - } + if md.Description != "description test" { + t.Fatalf("description was not sanitized: %q", md.Description) + } + if md.Maintainers[0].Name != " " { + t.Fatal("maintainer name was not sanitized") } } diff --git a/pkg/chartutil/save_test.go b/pkg/chartutil/save_test.go index 3a45b2992..23c3f9467 100644 --- a/pkg/chartutil/save_test.go +++ b/pkg/chartutil/save_test.go @@ -139,7 +139,7 @@ func TestSavePreservesTimestamps(t *testing.T) { Metadata: &chart.Metadata{ APIVersion: chart.APIVersionV1, Name: "ahab", - Version: "1.2.3.4", + Version: "1.2.3", }, Values: map[string]interface{}{ "imageName": "testimage", diff --git a/pkg/downloader/testdata/repository/kubernetes-charts-index.yaml b/pkg/downloader/testdata/repository/kubernetes-charts-index.yaml index b9e3dc69e..52dcf930b 100644 --- a/pkg/downloader/testdata/repository/kubernetes-charts-index.yaml +++ b/pkg/downloader/testdata/repository/kubernetes-charts-index.yaml @@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine urls: - https://charts.helm.sh/stable/alpine-0.2.0.tgz @@ -25,6 +26,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 mariadb: - name: mariadb urls: @@ -44,3 +46,4 @@ entries: - name: Bitnami email: containers@bitnami.com icon: "" + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/malformed-index.yaml b/pkg/downloader/testdata/repository/malformed-index.yaml index 887e129e9..fa319abdd 100644 --- a/pkg/downloader/testdata/repository/malformed-index.yaml +++ b/pkg/downloader/testdata/repository/malformed-index.yaml @@ -13,3 +13,4 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/testing-basicauth-index.yaml b/pkg/downloader/testdata/repository/testing-basicauth-index.yaml index da3ed5108..ed092ef41 100644 --- a/pkg/downloader/testdata/repository/testing-basicauth-index.yaml +++ b/pkg/downloader/testdata/repository/testing-basicauth-index.yaml @@ -12,3 +12,4 @@ entries: - http://username:password@example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/testing-ca-file-index.yaml b/pkg/downloader/testdata/repository/testing-ca-file-index.yaml index 17cdde1c6..81901efc7 100644 --- a/pkg/downloader/testdata/repository/testing-ca-file-index.yaml +++ b/pkg/downloader/testdata/repository/testing-ca-file-index.yaml @@ -12,3 +12,4 @@ entries: - https://example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/testing-https-index.yaml b/pkg/downloader/testdata/repository/testing-https-index.yaml index 17cdde1c6..81901efc7 100644 --- a/pkg/downloader/testdata/repository/testing-https-index.yaml +++ b/pkg/downloader/testdata/repository/testing-https-index.yaml @@ -12,3 +12,4 @@ entries: - https://example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/testing-index.yaml b/pkg/downloader/testdata/repository/testing-index.yaml index c238b8f8d..f588bf1fb 100644 --- a/pkg/downloader/testdata/repository/testing-index.yaml +++ b/pkg/downloader/testdata/repository/testing-index.yaml @@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine urls: - http://example.com/alpine-0.2.0.tgz @@ -26,6 +27,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 foo: - name: foo description: Foo Chart @@ -38,3 +40,4 @@ entries: - http://example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/testing-querystring-index.yaml b/pkg/downloader/testdata/repository/testing-querystring-index.yaml index 887e129e9..fa319abdd 100644 --- a/pkg/downloader/testdata/repository/testing-querystring-index.yaml +++ b/pkg/downloader/testdata/repository/testing-querystring-index.yaml @@ -13,3 +13,4 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/testing-relative-index.yaml b/pkg/downloader/testdata/repository/testing-relative-index.yaml index 62197b698..ba27ed257 100644 --- a/pkg/downloader/testdata/repository/testing-relative-index.yaml +++ b/pkg/downloader/testdata/repository/testing-relative-index.yaml @@ -1,26 +1,28 @@ -apiVersion: v1 -entries: - foo: - - name: foo - description: Foo Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - charts/foo-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d - bar: - - name: bar - description: Bar Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - bar-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d +apiVersion: v1 +entries: + foo: + - name: foo + description: Foo Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - charts/foo-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 + bar: + - name: bar + description: Bar Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - bar-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 diff --git a/pkg/downloader/testdata/repository/testing-relative-trailing-slash-index.yaml b/pkg/downloader/testdata/repository/testing-relative-trailing-slash-index.yaml index 62197b698..ba27ed257 100644 --- a/pkg/downloader/testdata/repository/testing-relative-trailing-slash-index.yaml +++ b/pkg/downloader/testdata/repository/testing-relative-trailing-slash-index.yaml @@ -1,26 +1,28 @@ -apiVersion: v1 -entries: - foo: - - name: foo - description: Foo Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - charts/foo-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d - bar: - - name: bar - description: Bar Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - bar-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d +apiVersion: v1 +entries: + foo: + - name: foo + description: Foo Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - charts/foo-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 + bar: + - name: bar + description: Bar Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - bar-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 93b5527a1..9ead561b2 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -23,6 +23,7 @@ import ( "regexp" "runtime" "strings" + "unicode" "github.com/pkg/errors" "sigs.k8s.io/yaml" @@ -175,10 +176,25 @@ func validatePluginData(plug *Plugin, filepath string) error { if !validPluginName.MatchString(plug.Metadata.Name) { return fmt.Errorf("invalid plugin name at %q", filepath) } + plug.Metadata.Usage = sanitizeString(plug.Metadata.Usage) + // We could also validate SemVer, executable, and other fields should we so choose. return nil } +// sanitizeString normalize spaces and removes non-printable characters. +func sanitizeString(str string) string { + return strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return ' ' + } + if unicode.IsPrint(r) { + return r + } + return -1 + }, str) +} + func detectDuplicates(plugs []*Plugin) error { names := map[string]string{} diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 92892bb85..09b94fd42 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -82,6 +82,8 @@ func NewChartRepository(cfg *Entry, getters getter.Providers) (*ChartRepository, // Load loads a directory of charts as if it were a repository. // // It requires the presence of an index.yaml file in the directory. +// +// Deprecated: remove in Helm 4. func (r *ChartRepository) Load() error { dirInfo, err := os.Stat(r.Config.Name) if err != nil { @@ -99,7 +101,7 @@ func (r *ChartRepository) Load() error { if strings.Contains(f.Name(), "-index.yaml") { i, err := LoadIndexFile(path) if err != nil { - return nil + return err } r.IndexFile = i } else if strings.HasSuffix(f.Name(), ".tgz") { @@ -137,7 +139,7 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) { return "", err } - indexFile, err := loadIndex(index) + indexFile, err := loadIndex(index, r.Config.URL) if err != nil { return "", err } @@ -187,7 +189,9 @@ func (r *ChartRepository) generateIndex() error { } if !r.IndexFile.Has(ch.Name(), ch.Metadata.Version) { - r.IndexFile.Add(ch.Metadata, path, r.Config.URL, digest) + if err := r.IndexFile.MustAdd(ch.Metadata, path, r.Config.URL, digest); err != nil { + return errors.Wrapf(err, "failed adding to %s to index", path) + } } // TODO: If a chart exists, but has a different Digest, should we error? } diff --git a/pkg/repo/index.go b/pkg/repo/index.go index 43f1e1c87..5adaf5530 100644 --- a/pkg/repo/index.go +++ b/pkg/repo/index.go @@ -19,6 +19,7 @@ package repo import ( "bytes" "io/ioutil" + "log" "os" "path" "path/filepath" @@ -105,16 +106,27 @@ func LoadIndexFile(path string) (*IndexFile, error) { if err != nil { return nil, err } - return loadIndex(b) + i, err := loadIndex(b, path) + if err != nil { + return nil, errors.Wrapf(err, "error loading %s", path) + } + return i, nil } -// Add adds a file to the index +// MustAdd adds a file to the index // This can leave the index in an unsorted state -func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) { +func (i IndexFile) MustAdd(md *chart.Metadata, filename, baseURL, digest string) error { + if md.APIVersion == "" { + md.APIVersion = chart.APIVersionV1 + } + if err := md.Validate(); err != nil { + return errors.Wrapf(err, "validate failed for %s", filename) + } + u := filename if baseURL != "" { - var err error _, file := filepath.Split(filename) + var err error u, err = urlutil.URLJoin(baseURL, file) if err != nil { u = path.Join(baseURL, file) @@ -126,10 +138,17 @@ func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) { Digest: digest, Created: time.Now(), } - if ee, ok := i.Entries[md.Name]; !ok { - i.Entries[md.Name] = ChartVersions{cr} - } else { - i.Entries[md.Name] = append(ee, cr) + ee := i.Entries[md.Name] + i.Entries[md.Name] = append(ee, cr) + return nil +} + +// Add adds a file to the index and logs an error. +// +// Deprecated: Use index.MustAdd instead. +func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) { + if err := i.MustAdd(md, filename, baseURL, digest); err != nil { + log.Printf("skipping loading invalid entry for chart %q %q from %s: %s", md.Name, md.Version, filename, err) } } @@ -294,19 +313,34 @@ func IndexDirectory(dir, baseURL string) (*IndexFile, error) { if err != nil { return index, err } - index.Add(c.Metadata, fname, parentURL, hash) + if err := index.MustAdd(c.Metadata, fname, parentURL, hash); err != nil { + return index, errors.Wrapf(err, "failed adding to %s to index", fname) + } } return index, nil } // loadIndex loads an index file and does minimal validity checking. // +// The source parameter is only used for logging. // This will fail if API Version is not set (ErrNoAPIVersion) or if the unmarshal fails. -func loadIndex(data []byte) (*IndexFile, error) { +func loadIndex(data []byte, source string) (*IndexFile, error) { i := &IndexFile{} if err := yaml.UnmarshalStrict(data, i); err != nil { return i, err } + + for name, cvs := range i.Entries { + for idx := len(cvs) - 1; idx >= 0; idx-- { + if cvs[idx].APIVersion == "" { + cvs[idx].APIVersion = chart.APIVersionV1 + } + if err := cvs[idx].Validate(); err != nil { + log.Printf("skipping loading invalid entry for chart %q %q from %s: %s", name, cvs[idx].Version, source, err) + cvs = append(cvs[:idx], cvs[idx+1:]...) + } + } + } i.SortEntries() if i.APIVersion == "" { return i, ErrNoAPIVersion diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index b3d91402b..47f3c6b2e 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -27,11 +27,10 @@ import ( "strings" "testing" + "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/helmpath" - - "helm.sh/helm/v3/pkg/chart" ) const ( @@ -65,12 +64,23 @@ entries: func TestIndexFile(t *testing.T) { i := NewIndexFile() - i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") - i.Add(&chart.Metadata{Name: "cutter", Version: "0.1.1"}, "cutter-0.1.1.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "cutter", Version: "0.1.0"}, "cutter-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "cutter", Version: "0.2.0"}, "cutter-0.2.0.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "setter", Version: "0.1.9+alpha"}, "setter-0.1.9+alpha.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "setter", Version: "0.1.9+beta"}, "setter-0.1.9+beta.tgz", "http://example.com/charts", "sha256:1234567890abc") + for _, x := range []struct { + md *chart.Metadata + filename string + baseURL string + digest string + }{ + {&chart.Metadata{APIVersion: "v2", Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"}, + {&chart.Metadata{APIVersion: "v2", Name: "cutter", Version: "0.1.1"}, "cutter-0.1.1.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "cutter", Version: "0.1.0"}, "cutter-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "cutter", Version: "0.2.0"}, "cutter-0.2.0.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "setter", Version: "0.1.9+alpha"}, "setter-0.1.9+alpha.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "setter", Version: "0.1.9+beta"}, "setter-0.1.9+beta.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + } { + if err := i.MustAdd(x.md, x.filename, x.baseURL, x.digest); err != nil { + t.Errorf("unexpected error adding to index: %s", err) + } + } i.SortEntries() @@ -126,11 +136,7 @@ func TestLoadIndex(t *testing.T) { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() - b, err := ioutil.ReadFile(tc.Filename) - if err != nil { - t.Fatal(err) - } - i, err := loadIndex(b) + i, err := LoadIndexFile(tc.Filename) if err != nil { t.Fatal(err) } @@ -141,19 +147,11 @@ func TestLoadIndex(t *testing.T) { // TestLoadIndex_Duplicates is a regression to make sure that we don't non-deterministically allow duplicate packages. func TestLoadIndex_Duplicates(t *testing.T) { - if _, err := loadIndex([]byte(indexWithDuplicates)); err == nil { + if _, err := loadIndex([]byte(indexWithDuplicates), "indexWithDuplicates"); err == nil { t.Errorf("Expected an error when duplicate entries are present") } } -func TestLoadIndexFile(t *testing.T) { - i, err := LoadIndexFile(testfile) - if err != nil { - t.Fatal(err) - } - verifyLocalIndex(t, i) -} - func TestLoadIndexFileAnnotations(t *testing.T) { i, err := LoadIndexFile(annotationstestfile) if err != nil { @@ -170,11 +168,7 @@ func TestLoadIndexFileAnnotations(t *testing.T) { } func TestLoadUnorderedIndex(t *testing.T) { - b, err := ioutil.ReadFile(unorderedTestfile) - if err != nil { - t.Fatal(err) - } - i, err := loadIndex(b) + i, err := LoadIndexFile(unorderedTestfile) if err != nil { t.Fatal(err) } @@ -183,20 +177,26 @@ func TestLoadUnorderedIndex(t *testing.T) { func TestMerge(t *testing.T) { ind1 := NewIndexFile() - ind1.Add(&chart.Metadata{ - Name: "dreadnought", - Version: "0.1.0", - }, "dreadnought-0.1.0.tgz", "http://example.com", "aaaa") + + if err := ind1.MustAdd(&chart.Metadata{APIVersion: "v2", Name: "dreadnought", Version: "0.1.0"}, "dreadnought-0.1.0.tgz", "http://example.com", "aaaa"); err != nil { + t.Fatalf("unexpected error: %s", err) + } ind2 := NewIndexFile() - ind2.Add(&chart.Metadata{ - Name: "dreadnought", - Version: "0.2.0", - }, "dreadnought-0.2.0.tgz", "http://example.com", "aaaabbbb") - ind2.Add(&chart.Metadata{ - Name: "doughnut", - Version: "0.2.0", - }, "doughnut-0.2.0.tgz", "http://example.com", "ccccbbbb") + + for _, x := range []struct { + md *chart.Metadata + filename string + baseURL string + digest string + }{ + {&chart.Metadata{APIVersion: "v2", Name: "dreadnought", Version: "0.2.0"}, "dreadnought-0.2.0.tgz", "http://example.com", "aaaabbbb"}, + {&chart.Metadata{APIVersion: "v2", Name: "doughnut", Version: "0.2.0"}, "doughnut-0.2.0.tgz", "http://example.com", "ccccbbbb"}, + } { + if err := ind2.MustAdd(x.md, x.filename, x.baseURL, x.digest); err != nil { + t.Errorf("unexpected error: %s", err) + } + } ind1.Merge(ind2) @@ -239,12 +239,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - b, err := ioutil.ReadFile(idx) - if err != nil { - t.Fatalf("error reading index file: %#v", err) - } - - i, err := loadIndex(b) + i, err := LoadIndexFile(idx) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) } @@ -256,7 +251,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created charts file: %#v", err) } - b, err = ioutil.ReadFile(idx) + b, err := ioutil.ReadFile(idx) if err != nil { t.Fatalf("error reading charts file: %#v", err) } @@ -297,12 +292,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - b, err := ioutil.ReadFile(idx) - if err != nil { - t.Fatalf("error reading index file: %#v", err) - } - - i, err := loadIndex(b) + i, err := LoadIndexFile(idx) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) } @@ -314,7 +304,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created charts file: %#v", err) } - b, err = ioutil.ReadFile(idx) + b, err := ioutil.ReadFile(idx) if err != nil { t.Fatalf("error reading charts file: %#v", err) } @@ -345,6 +335,7 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) { expects := []*ChartVersion{ { Metadata: &chart.Metadata{ + APIVersion: "v2", Name: "alpine", Description: "string", Version: "1.0.0", @@ -359,6 +350,7 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) { }, { Metadata: &chart.Metadata{ + APIVersion: "v2", Name: "nginx", Description: "string", Version: "0.2.0", @@ -372,6 +364,7 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) { }, { Metadata: &chart.Metadata{ + APIVersion: "v2", Name: "nginx", Description: "string", Version: "0.1.0", @@ -476,28 +469,44 @@ func TestIndexDirectory(t *testing.T) { func TestIndexAdd(t *testing.T) { i := NewIndexFile() - i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") + + for _, x := range []struct { + md *chart.Metadata + filename string + baseURL string + digest string + }{ + + {&chart.Metadata{APIVersion: "v2", Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"}, + {&chart.Metadata{APIVersion: "v2", Name: "alpine", Version: "0.1.0"}, "/home/charts/alpine-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"}, + {&chart.Metadata{APIVersion: "v2", Name: "deis", Version: "0.1.0"}, "/home/charts/deis-0.1.0.tgz", "http://example.com/charts/", "sha256:1234567890"}, + } { + if err := i.MustAdd(x.md, x.filename, x.baseURL, x.digest); err != nil { + t.Errorf("unexpected error adding to index: %s", err) + } + } if i.Entries["clipper"][0].URLs[0] != "http://example.com/charts/clipper-0.1.0.tgz" { t.Errorf("Expected http://example.com/charts/clipper-0.1.0.tgz, got %s", i.Entries["clipper"][0].URLs[0]) } - - i.Add(&chart.Metadata{Name: "alpine", Version: "0.1.0"}, "/home/charts/alpine-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") - if i.Entries["alpine"][0].URLs[0] != "http://example.com/charts/alpine-0.1.0.tgz" { t.Errorf("Expected http://example.com/charts/alpine-0.1.0.tgz, got %s", i.Entries["alpine"][0].URLs[0]) } - - i.Add(&chart.Metadata{Name: "deis", Version: "0.1.0"}, "/home/charts/deis-0.1.0.tgz", "http://example.com/charts/", "sha256:1234567890") - if i.Entries["deis"][0].URLs[0] != "http://example.com/charts/deis-0.1.0.tgz" { t.Errorf("Expected http://example.com/charts/deis-0.1.0.tgz, got %s", i.Entries["deis"][0].URLs[0]) } + + // test error condition + if err := i.MustAdd(&chart.Metadata{}, "error-0.1.0.tgz", "", ""); err == nil { + t.Fatal("expected error adding to index") + } } func TestIndexWrite(t *testing.T) { i := NewIndexFile() - i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") + if err := i.MustAdd(&chart.Metadata{APIVersion: "v2", Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"); err != nil { + t.Fatalf("unexpected error: %s", err) + } dir, err := ioutil.TempDir("", "helm-tmp") if err != nil { t.Fatal(err) diff --git a/pkg/repo/testdata/chartmuseum-index.yaml b/pkg/repo/testdata/chartmuseum-index.yaml index 364e42c54..349a529aa 100644 --- a/pkg/repo/testdata/chartmuseum-index.yaml +++ b/pkg/repo/testdata/chartmuseum-index.yaml @@ -14,6 +14,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.1.0.tgz name: nginx @@ -25,6 +26,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -39,6 +41,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -48,3 +51,4 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 diff --git a/pkg/repo/testdata/local-index-annotations.yaml b/pkg/repo/testdata/local-index-annotations.yaml index d429aa11c..833ab854b 100644 --- a/pkg/repo/testdata/local-index-annotations.yaml +++ b/pkg/repo/testdata/local-index-annotations.yaml @@ -12,6 +12,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.1.0.tgz name: nginx @@ -23,6 +24,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -37,6 +39,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -46,5 +49,6 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 annotations: helm.sh/test: foo bar diff --git a/pkg/repo/testdata/local-index-unordered.yaml b/pkg/repo/testdata/local-index-unordered.yaml index 3af72dfd1..cdfaa7f24 100644 --- a/pkg/repo/testdata/local-index-unordered.yaml +++ b/pkg/repo/testdata/local-index-unordered.yaml @@ -12,6 +12,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.2.0.tgz name: nginx @@ -23,6 +24,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -37,6 +39,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -46,3 +49,4 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 diff --git a/pkg/repo/testdata/local-index.yaml b/pkg/repo/testdata/local-index.yaml index f8fa32bb2..d61f40dda 100644 --- a/pkg/repo/testdata/local-index.yaml +++ b/pkg/repo/testdata/local-index.yaml @@ -12,6 +12,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.1.0.tgz name: nginx @@ -23,6 +24,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -37,6 +39,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -46,3 +49,4 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2