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 <martin.hickey@ie.ibm.com>

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)
 }

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
-}

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
+}

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,

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
 	}