diff --git a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt index e77aa387f..5f9952a92 100644 --- a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt +++ b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt @@ -2,7 +2,7 @@ [INFO] Chart.yaml: icon is recommended [WARNING] templates/: directory not found [ERROR] : unable to load chart - error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required + error unpacking bad-subchart in chart-with-bad-subcharts (from ./testdata/testcharts/chart-with-bad-subcharts/): validation: chart.metadata.name is required ==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart [ERROR] Chart.yaml: name is required diff --git a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt index 265e555f7..c2d89edf2 100644 --- a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt +++ b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt @@ -2,6 +2,6 @@ [INFO] Chart.yaml: icon is recommended [WARNING] templates/: directory not found [ERROR] : unable to load chart - error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required + error unpacking bad-subchart in chart-with-bad-subcharts (from ./testdata/testcharts/chart-with-bad-subcharts/): validation: chart.metadata.name is required Error: 1 chart(s) linted, 1 chart(s) failed diff --git a/cmd/helm/testdata/output/upgrade-with-bad-dependencies.txt b/cmd/helm/testdata/output/upgrade-with-bad-dependencies.txt index 6dddc7344..fd490521a 100644 --- a/cmd/helm/testdata/output/upgrade-with-bad-dependencies.txt +++ b/cmd/helm/testdata/output/upgrade-with-bad-dependencies.txt @@ -1 +1 @@ -Error: cannot load Chart.yaml: error converting YAML to JSON: yaml: line 6: did not find expected '-' indicator +Error: cannot load Chart.yaml (from ./testdata/testcharts/chart-bad-requirements/): error converting YAML to JSON: yaml: line 6: did not find expected '-' indicator diff --git a/internal/experimental/registry/cache.go b/internal/experimental/registry/cache.go index c89898a51..a549f8923 100644 --- a/internal/experimental/registry/cache.go +++ b/internal/experimental/registry/cache.go @@ -142,7 +142,8 @@ func (cache *Cache) FetchReference(ref *Reference) (*CacheRefSummary, error) { if err != nil { return &r, err } - ch, err := loader.LoadArchive(bytes.NewBuffer(contentBytes)) + ch, err := loader.LoadArchive(bytes.NewBuffer(contentBytes), + fmt.Sprintf("[Chart(Name: %s, Repo: %s, Tag: %s)]", r.Name, r.Repo, r.Tag)) if err != nil { return &r, err } diff --git a/internal/test/test.go b/internal/test/test.go index 646037606..737daea14 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -20,7 +20,9 @@ import ( "bytes" "flag" "io/ioutil" + "os" "path/filepath" + "strings" "github.com/pkg/errors" ) @@ -53,7 +55,7 @@ func AssertGoldenBytes(t TestingT, actual []byte, filename string) { func AssertGoldenString(t TestingT, actual, filename string) { t.Helper() - if err := compare([]byte(actual), path(filename)); err != nil { + if err := compare(relativize([]byte(actual)), path(filename)); err != nil { t.Fatalf("%v", err) } } @@ -77,7 +79,7 @@ func path(filename string) string { } func compare(actual []byte, filename string) error { - actual = normalize(actual) + actual = relativize(normalize(actual)) if err := update(filename, actual); err != nil { return err } @@ -103,3 +105,15 @@ func update(filename string, in []byte) error { func normalize(in []byte) []byte { return bytes.Replace(in, []byte("\r\n"), []byte("\n"), -1) } + +func relativize(relative []byte) []byte { + relativeString := string(relative) + if !strings.Contains(relativeString, "(from ") { + return relative + } + base, err := os.Getwd() + if err != nil { + return relative + } + return []byte(strings.ReplaceAll(relativeString, base, ".")) +} diff --git a/pkg/chart/loader/archive.go b/pkg/chart/loader/archive.go index 8b38cb89f..d6c26f4f4 100644 --- a/pkg/chart/loader/archive.go +++ b/pkg/chart/loader/archive.go @@ -62,7 +62,7 @@ func LoadFile(name string) (*chart.Chart, error) { return nil, err } - c, err := LoadArchive(raw) + c, err := LoadArchive(raw, name) if err != nil { if err == gzip.ErrHeader { return nil, fmt.Errorf("file '%s' does not appear to be a valid chart file (details: %s)", name, err) @@ -101,7 +101,7 @@ func ensureArchive(name string, raw *os.File) error { // LoadArchiveFiles reads in files out of an archive into memory. This function // performs important path security checks and should always be used before // expanding a tarball -func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { +func LoadArchiveFiles(in io.Reader, Path string) ([]*BufferedFile, error) { unzipped, err := gzip.NewReader(in) if err != nil { return nil, err @@ -139,6 +139,11 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { } parts := strings.Split(hd.Name, delimiter) + + if parts[0] == "Chart.yaml" { + return nil, errors.New("chart yaml not in base directory") + } + n := strings.Join(parts[1:], delimiter) // Normalize the path to the / delimiter @@ -165,10 +170,6 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { return nil, errors.New("chart contains illegally named files") } - if parts[0] == "Chart.yaml" { - return nil, errors.New("chart yaml not in base directory") - } - if _, err := io.Copy(b, tr); err != nil { return nil, err } @@ -186,11 +187,11 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { } // LoadArchive loads from a reader containing a compressed tar archive. -func LoadArchive(in io.Reader) (*chart.Chart, error) { - files, err := LoadArchiveFiles(in) +func LoadArchive(in io.Reader, Path string) (*chart.Chart, error) { + files, err := LoadArchiveFiles(in, Path) if err != nil { return nil, err } - return LoadFiles(files) + return LoadFiles(files, Path) } diff --git a/pkg/chart/loader/archive_test.go b/pkg/chart/loader/archive_test.go index 41b0af1aa..71293febb 100644 --- a/pkg/chart/loader/archive_test.go +++ b/pkg/chart/loader/archive_test.go @@ -83,7 +83,7 @@ func TestLoadArchiveFiles(t *testing.T) { _ = tw.Close() _ = gzw.Close() - files, err := LoadArchiveFiles(buf) + files, err := LoadArchiveFiles(buf, tc.name) tc.check(t, files, err) }) } diff --git a/pkg/chart/loader/directory.go b/pkg/chart/loader/directory.go index bbe543870..7a1af1f81 100644 --- a/pkg/chart/loader/directory.go +++ b/pkg/chart/loader/directory.go @@ -116,5 +116,5 @@ func LoadDir(dir string) (*chart.Chart, error) { return c, err } - return LoadFiles(files) + return LoadFiles(files, topdir) } diff --git a/pkg/chart/loader/load.go b/pkg/chart/loader/load.go index 7cc8878a8..ff64961d8 100644 --- a/pkg/chart/loader/load.go +++ b/pkg/chart/loader/load.go @@ -69,7 +69,7 @@ type BufferedFile struct { } // LoadFiles loads from in-memory files. -func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { +func LoadFiles(files []*BufferedFile, Path string) (*chart.Chart, error) { c := new(chart.Chart) subcharts := make(map[string][]*BufferedFile) @@ -82,7 +82,7 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { c.Metadata = new(chart.Metadata) } if err := yaml.Unmarshal(f.Data, c.Metadata); err != nil { - return c, errors.Wrap(err, "cannot load Chart.yaml") + return c, errors.Wrapf(err, "cannot load Chart.yaml (from %s)", Path) } // NOTE(bacongobbler): while the chart specification says that APIVersion must be set, // Helm 2 accepted charts that did not provide an APIVersion in their chart metadata. @@ -100,12 +100,12 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { case f.Name == "Chart.lock": c.Lock = new(chart.Lock) if err := yaml.Unmarshal(f.Data, &c.Lock); err != nil { - return c, errors.Wrap(err, "cannot load Chart.lock") + return c, errors.Wrapf(err, "cannot load Chart.lock (from %s)", Path) } case f.Name == "values.yaml": c.Values = make(map[string]interface{}) if err := yaml.Unmarshal(f.Data, &c.Values); err != nil { - return c, errors.Wrap(err, "cannot load values.yaml") + return c, errors.Wrapf(err, "cannot load values.yaml (from %s)", Path) } case f.Name == "values.schema.json": c.Schema = f.Data @@ -117,10 +117,10 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { c.Metadata = new(chart.Metadata) } if c.Metadata.APIVersion != chart.APIVersionV1 { - log.Printf("Warning: Dependencies are handled in Chart.yaml since apiVersion \"v2\". We recommend migrating dependencies to Chart.yaml.") + log.Printf("Warning: Dependencies are handled in Chart.yaml (from %s) since apiVersion \"v2\". We recommend migrating dependencies to Chart.yaml.", Path) } if err := yaml.Unmarshal(f.Data, c.Metadata); err != nil { - return c, errors.Wrap(err, "cannot load requirements.yaml") + return c, errors.Wrapf(err, "cannot load requirements.yaml (from %s)", Path) } if c.Metadata.APIVersion == chart.APIVersionV1 { c.Files = append(c.Files, &chart.File{Name: f.Name, Data: f.Data}) @@ -129,7 +129,7 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { case f.Name == "requirements.lock": c.Lock = new(chart.Lock) if err := yaml.Unmarshal(f.Data, &c.Lock); err != nil { - return c, errors.Wrap(err, "cannot load requirements.lock") + return c, errors.Wrapf(err, "cannot load requirements.lock (from %s)", Path) } if c.Metadata == nil { c.Metadata = new(chart.Metadata) @@ -155,7 +155,7 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { } if c.Metadata == nil { - return c, errors.New("Chart.yaml file is missing") + return c, errors.Errorf("Chart.yaml file is missing (from %s)", Path) } if err := c.Validate(); err != nil { @@ -171,10 +171,10 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { case filepath.Ext(n) == ".tgz": file := files[0] if file.Name != n { - return c, errors.Errorf("error unpacking tar in %s: expected %s, got %s", c.Name(), n, file.Name) + return c, errors.Errorf("error unpacking tar in %s (from %s): expected %s, got %s", c.Name(), Path, n, file.Name) } // Untar the chart and add to c.Dependencies - sc, err = LoadArchive(bytes.NewBuffer(file.Data)) + sc, err = LoadArchive(bytes.NewBuffer(file.Data), Path) default: // We have to trim the prefix off of every file, and ignore any file // that is in charts/, but isn't actually a chart. @@ -187,11 +187,11 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { f.Name = parts[1] buff = append(buff, f) } - sc, err = LoadFiles(buff) + sc, err = LoadFiles(buff, Path) } if err != nil { - return c, errors.Wrapf(err, "error unpacking %s in %s", n, c.Name()) + return c, errors.Wrapf(err, "error unpacking %s in %s (from %s)", n, c.Name(), Path) } c.AddDependency(sc) } diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 9605b3152..dbca4dbf8 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -223,7 +223,7 @@ func TestLoadFiles_BadCases(t *testing.T) { }, expectError: "validation: chart.metadata.apiVersion is required"}, } { - _, err := LoadFiles(tt.bufferedFiles) + _, err := LoadFiles(tt.bufferedFiles, tt.name) if err == nil { t.Fatal("expected error when load illegal files") } @@ -274,7 +274,7 @@ icon: https://example.com/64x64.png }, } - c, err := LoadFiles(goodFiles) + c, err := LoadFiles(goodFiles, t.Name()) if err != nil { t.Errorf("Expected good files to be loaded, got %v", err) } @@ -299,10 +299,10 @@ icon: https://example.com/64x64.png t.Errorf("Expected number of templates == 2, got %d", len(c.Templates)) } - if _, err = LoadFiles([]*BufferedFile{}); err == nil { + if _, err = LoadFiles([]*BufferedFile{}, t.Name()); err == nil { t.Fatal("Expected err to be non-nil") } - if err.Error() != "Chart.yaml file is missing" { + if err.Error() != "Chart.yaml file is missing (from TestLoadFiles)" { t.Errorf("Expected chart metadata missing error, got '%s'", err.Error()) } } @@ -363,7 +363,7 @@ icon: https://example.com/64x64.png log.SetOutput(stderr) }() - _, err = LoadFiles(goodFiles) + _, err = LoadFiles(goodFiles, t.Name()) if err != nil { t.Errorf("Expected good files to be loaded, got %v", err) } @@ -480,7 +480,7 @@ func TestLoadInvalidArchive(t *testing.T) { illegalChart = filepath.Join(tmpdir, "abs-path2.tgz") writeTar(illegalChart, "files/whatever.yaml", []byte("hello: world")) _, err = Load(illegalChart) - if err.Error() != "Chart.yaml file is missing" { + if !strings.Contains(err.Error(), "Chart.yaml file is missing") { t.Errorf("Unexpected error message: %s", err) } @@ -488,7 +488,7 @@ func TestLoadInvalidArchive(t *testing.T) { illegalChart = filepath.Join(tmpdir, "abs-winpath.tgz") writeTar(illegalChart, "c:\\Chart.yaml", []byte("hello: world")) _, err = Load(illegalChart) - if err.Error() != "validation: chart.metadata.name is required" { + if !strings.Contains(err.Error(), "validation: chart.metadata.name is required") { t.Error(err) } } diff --git a/pkg/chartutil/expand.go b/pkg/chartutil/expand.go index 6ad09e417..649b213ab 100644 --- a/pkg/chartutil/expand.go +++ b/pkg/chartutil/expand.go @@ -32,7 +32,7 @@ import ( // Expand uncompresses and extracts a chart into the specified directory. func Expand(dir string, r io.Reader) error { - files, err := loader.LoadArchiveFiles(r) + files, err := loader.LoadArchiveFiles(r, dir) if err != nil { return err } @@ -43,13 +43,13 @@ func Expand(dir string, r io.Reader) error { if file.Name == "Chart.yaml" { ch := &chart.Metadata{} if err := yaml.Unmarshal(file.Data, ch); err != nil { - return errors.Wrap(err, "cannot load Chart.yaml") + return errors.Wrapf(err, "cannot load Chart.yaml (from %s)", dir) } chartName = ch.Name } } if chartName == "" { - return errors.New("chart name not specified") + return errors.Errorf("chart name not specified (in %s)", dir) } // Find the base directory