From 0b809dd07882501af7d4eaf2506aa8105f2e30df Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Wed, 1 May 2019 14:56:32 +0100 Subject: [PATCH] Validate library chart files after chart loaded Validate the library chart files after the chart and its depoendencies have been loaded. This is an update following review comment: - https://github.com/helm/helm/pull/5441#pullrequestreview-231339425 Signed-off-by: Martin Hickey --- pkg/chart/loader/archive.go | 43 +++------------------------- pkg/chart/loader/load.go | 57 ++----------------------------------- pkg/chartutil/chartfile.go | 16 +++++++++-- pkg/engine/engine.go | 4 +++ pkg/registry/cache.go | 2 +- 5 files changed, 25 insertions(+), 97 deletions(-) diff --git a/pkg/chart/loader/archive.go b/pkg/chart/loader/archive.go index 71a111235..a8dc3f17b 100644 --- a/pkg/chart/loader/archive.go +++ b/pkg/chart/loader/archive.go @@ -51,30 +51,11 @@ func LoadFile(name string) (*chart.Chart, error) { } defer raw.Close() - isLibChart, err := IsArchiveLibraryChart(raw) - if err != nil { - return nil, err - } - _, _ = raw.Seek(0, 0) - return LoadArchive(raw, isLibChart) -} - -// IsArchiveLibraryChart return true if it is a library chart -func IsArchiveLibraryChart(in io.Reader) (bool, error) { - var isLibChart = false - _, err := traverse(in, false, &isLibChart) - if err != nil { - return false, err - } - return isLibChart, nil + return LoadArchive(raw) } // LoadArchive loads from a reader containing a compressed tar archive. -func LoadArchive(in io.Reader, isLibChart bool) (*chart.Chart, error) { - return traverse(in, true, &isLibChart) -} - -func traverse(in io.Reader, loadFiles bool, isLibChart *bool) (*chart.Chart, error) { +func LoadArchive(in io.Reader) (*chart.Chart, error) { unzipped, err := gzip.NewReader(in) if err != nil { return &chart.Chart{}, err @@ -119,29 +100,13 @@ func traverse(in io.Reader, loadFiles bool, isLibChart *bool) (*chart.Chart, err return &chart.Chart{}, err } - if !loadFiles && n == "Chart.yaml" { - var err error - *isLibChart, err = IsLibraryChart(b.Bytes()) - if err != nil { - return nil, errors.Wrapf(err, "cannot load Chart.yaml") - } - return nil, nil - } - - if IsFileValid(n, *isLibChart) { - files = append(files, &BufferedFile{Name: n, Data: b.Bytes()}) - } - + files = append(files, &BufferedFile{Name: n, Data: b.Bytes()}) b.Reset() } - if loadFiles && len(files) == 0 { + if len(files) == 0 { return nil, errors.New("no files in chart archive") } - if !loadFiles { - return nil, errors.New("cannot find the chart type") - } - return LoadFiles(files) } diff --git a/pkg/chart/loader/load.go b/pkg/chart/loader/load.go index 6c78b7964..cd886d8c7 100644 --- a/pkg/chart/loader/load.go +++ b/pkg/chart/loader/load.go @@ -124,30 +124,8 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { return c, errors.Errorf("error unpacking tar in %s: expected %s, got %s", c.Name(), n, file.Name) } // Untar the chart and add to c.Dependencies - var isLibChart bool - isLibChart, err = IsArchiveLibraryChart(bytes.NewBuffer(file.Data)) - if err == nil { - sc, err = LoadArchive(bytes.NewBuffer(file.Data), isLibChart) - } + sc, err = LoadArchive(bytes.NewBuffer(file.Data)) default: - // Need to ascertain if the subchart is a library chart as will parse files - // differently from a standard application chart - var isLibChart = false - for _, f := range files { - parts := strings.SplitN(f.Name, "/", 2) - if len(parts) < 2 { - continue - } - name := parts[1] - if name == "Chart.yaml" { - isLibChart, err = IsLibraryChart(f.Data) - if err != nil { - return c, errors.Wrapf(err, "cannot load Chart.yaml for %s in %s", n, c.Name()) - } - break - } - } - // We have to trim the prefix off of every file, and ignore any file // that is in charts/, but isn't actually a chart. buff := make([]*BufferedFile, 0, len(files)) @@ -157,12 +135,7 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { continue } f.Name = parts[1] - - // If the subchart is a library chart then will only want - // to render library and value files, and not any included template files - if IsFileValid(f.Name, isLibChart) { - buff = append(buff, f) - } + buff = append(buff, f) } sc, err = LoadFiles(buff) } @@ -170,34 +143,8 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { if err != nil { return c, errors.Wrapf(err, "error unpacking %s in %s", n, c.Name()) } - c.AddDependency(sc) } return c, nil } - -// IsLibraryChart return true if it is a library chart -func IsLibraryChart(data []byte) (bool, error) { - var isLibChart = false - metadata := new(chart.Metadata) - if err := yaml.Unmarshal(data, metadata); err != nil { - return false, errors.Wrapf(err, "cannot load data") - } - if strings.EqualFold(metadata.Type, "library") { - isLibChart = true - } - return isLibChart, nil -} - -// IsFileValid return true if this file is valid for library or -// application chart -func IsFileValid(filename string, isLibChart bool) bool { - if isLibChart { - if strings.HasPrefix(filepath.Base(filename), "_") || !strings.HasPrefix(filepath.Dir(filename), "template") { - return true - } - return false - } - return true -} diff --git a/pkg/chartutil/chartfile.go b/pkg/chartutil/chartfile.go index 17d6e2bb2..9caa3f87e 100644 --- a/pkg/chartutil/chartfile.go +++ b/pkg/chartutil/chartfile.go @@ -88,8 +88,7 @@ func IsChartDir(dirName string) (bool, error) { // // Application chart type is only installable func IsChartInstallable(chart *chart.Chart) (bool, error) { - chartType := chart.Metadata.Type - if strings.EqualFold(chartType, "library") { + if IsLibraryChart(chart) { return false, errors.New("Library charts are not installable") } validChartType, _ := IsValidChartType(chart) @@ -110,3 +109,16 @@ func IsValidChartType(chart *chart.Chart) (bool, error) { } return true, nil } + +// IsLibraryChart returns true if the chart is a library chart +func IsLibraryChart(c *chart.Chart) bool { + return strings.EqualFold(c.Metadata.Type, "library") +} + +// IsTemplateValid returns true if the template is valid for the chart type +func IsTemplateValid(templateName string, isLibChart bool) bool { + if isLibChart { + return strings.HasPrefix(filepath.Base(templateName), "_") + } + return true +} diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index b1bc93040..4d3e418bc 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -268,8 +268,12 @@ func recAllTpls(c *chart.Chart, templates map[string]renderable, vals chartutil. recAllTpls(child, templates, next) } + isLibChart := chartutil.IsLibraryChart(c) newParentID := c.ChartFullPath() for _, t := range c.Templates { + if !chartutil.IsTemplateValid(t.Name, isLibChart) { + continue + } templates[path.Join(newParentID, t.Name)] = renderable{ tpl: string(t.Data), vals: next, diff --git a/pkg/registry/cache.go b/pkg/registry/cache.go index cd5d0a203..39dec1467 100644 --- a/pkg/registry/cache.go +++ b/pkg/registry/cache.go @@ -85,7 +85,7 @@ func (cache *filesystemCache) LayersToChart(layers []ocispec.Descriptor) (*chart } // Construct chart object and attach metadata - ch, err := loader.LoadArchive(bytes.NewBuffer(contentRaw), false) + ch, err := loader.LoadArchive(bytes.NewBuffer(contentRaw)) if err != nil { return nil, err }