feat(helm): consolidate validation warnings

when doing the following commands:
* `helm repo add`
* `helm repo update`
* `helm search repo`

during validation of repository, invalid charts are counted and a
single warning is printed:

    skipped loading 2964 invalid chart entries from www.helm.repo

also, new `helm repo validate` command that works identical to
`helm repo update` but shows a warning for each invalid chart

Closes #9407

Signed-off-by: Russell Cousineau <russell.cousineau@xandr.com>

cmd/helm/repo_update.go

@@ -41,7 +41,8 @@ To update all the repositories, use 'helm repo update'.
 var errNoRepositories = errors.New("no repositories found. You must add one before updating")
 
 type repoUpdateOptions struct {
-	update               func([]*repo.ChartRepository, io.Writer, bool) error
+	showAllWarnings      bool
+	update               func([]*repo.ChartRepository, io.Writer, bool, bool) error
 	repoFile             string
 	repoCache            string
 	names                []string
@@ -64,7 +65,7 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command {
 			o.repoFile = settings.RepositoryConfig
 			o.repoCache = settings.RepositoryCache
 			o.names = args
-			return o.run(out)
+			return o.run(out, args)
 		},
 	}
 
@@ -74,10 +75,12 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command {
 	// This should be deprecated in Helm 4 by update to the behaviour of `helm repo update` command.
 	f.BoolVar(&o.failOnRepoUpdateFail, "fail-on-repo-update-fail", false, "update fails if any of the repository updates fail")
 
+	f.BoolVar(&o.showAllWarnings, "show-all-warnings", false, "show every invalid chart while indexing repository")
+
 	return cmd
 }
 
-func (o *repoUpdateOptions) run(out io.Writer) error {
+func (o *repoUpdateOptions) run(out io.Writer, args []string) error {
 	f, err := repo.LoadFile(o.repoFile)
 	switch {
 	case isNotExist(err):
@@ -111,10 +114,10 @@ func (o *repoUpdateOptions) run(out io.Writer) error {
 		}
 	}
 
-	return o.update(repos, out, o.failOnRepoUpdateFail)
+	return o.update(repos, out, o.failOnRepoUpdateFail, o.showAllWarnings)
 }
 
-func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error {
+func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool, showAllWarnings bool) error {
 	fmt.Fprintln(out, "Hang tight while we grab the latest from your chart repositories...")
 	var wg sync.WaitGroup
 	var repoFailList []string
@@ -122,7 +125,13 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdate
 		wg.Add(1)
 		go func(re *repo.ChartRepository) {
 			defer wg.Done()
-			if _, err := re.DownloadIndexFile(); err != nil {
+			var err error
+			if showAllWarnings {
+				_, err = re.ValidateAndDownloadIndexFile()
+			} else {
+				_, err = re.DownloadIndexFile()
+			}
+			if err != nil {
 				fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", re.Config.Name, re.Config.URL, err)
 				repoFailList = append(repoFailList, re.Config.URL)
 			} else {

cmd/helm/repo_update_test.go

@@ -35,7 +35,7 @@ func TestUpdateCmd(t *testing.T) {
 	var out bytes.Buffer
 	// Instead of using the HTTP updater, we provide our own for this test.
 	// The TestUpdateCharts test verifies the HTTP behavior independently.
-	updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error {
+	updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool, showAllWarnings bool) error {
 		for _, re := range repos {
 			fmt.Fprintln(out, re.Config.Name)
 		}
@@ -45,7 +45,7 @@ func TestUpdateCmd(t *testing.T) {
 		update:   updater,
 		repoFile: "testdata/repositories.yaml",
 	}
-	if err := o.run(&out); err != nil {
+	if err := o.run(&out, []string{}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -60,7 +60,7 @@ func TestUpdateCmdMultiple(t *testing.T) {
 	var out bytes.Buffer
 	// Instead of using the HTTP updater, we provide our own for this test.
 	// The TestUpdateCharts test verifies the HTTP behavior independently.
-	updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error {
+	updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool, showAllWarnings bool) error {
 		for _, re := range repos {
 			fmt.Fprintln(out, re.Config.Name)
 		}
@@ -71,7 +71,7 @@ func TestUpdateCmdMultiple(t *testing.T) {
 		repoFile: "testdata/repositories.yaml",
 		names:    []string{"firstexample", "charts"},
 	}
-	if err := o.run(&out); err != nil {
+	if err := o.run(&out, []string{}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -86,7 +86,7 @@ func TestUpdateCmdInvalid(t *testing.T) {
 	var out bytes.Buffer
 	// Instead of using the HTTP updater, we provide our own for this test.
 	// The TestUpdateCharts test verifies the HTTP behavior independently.
-	updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error {
+	updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool, showAllWarnings bool) error {
 		for _, re := range repos {
 			fmt.Fprintln(out, re.Config.Name)
 		}
@@ -97,7 +97,7 @@ func TestUpdateCmdInvalid(t *testing.T) {
 		repoFile: "testdata/repositories.yaml",
 		names:    []string{"firstexample", "invalid"},
 	}
-	if err := o.run(&out); err == nil {
+	if err := o.run(&out, []string{}); err == nil {
 		t.Fatal("expected error but did not get one")
 	}
 }
@@ -120,7 +120,7 @@ func TestUpdateCustomCacheCmd(t *testing.T) {
 		repoCache: cachePath,
 	}
 	b := ioutil.Discard
-	if err := o.run(b); err != nil {
+	if err := o.run(b, []string{}); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := os.Stat(filepath.Join(cachePath, "test-index.yaml")); err != nil {
@@ -147,7 +147,7 @@ func TestUpdateCharts(t *testing.T) {
 	}
 
 	b := bytes.NewBuffer(nil)
-	updateCharts([]*repo.ChartRepository{r}, b, false)
+	updateCharts([]*repo.ChartRepository{r}, b, false, false)
 
 	got := b.String()
 	if strings.Contains(got, "Unable to get an update") {
@@ -183,7 +183,7 @@ func TestUpdateChartsFail(t *testing.T) {
 	}
 
 	b := bytes.NewBuffer(nil)
-	if err := updateCharts([]*repo.ChartRepository{r}, b, false); err != nil {
+	if err := updateCharts([]*repo.ChartRepository{r}, b, false, false); err != nil {
 		t.Error("Repo update should not return error if update of repository fails")
 	}
 
@@ -216,7 +216,7 @@ func TestUpdateChartsFailWithError(t *testing.T) {
 	}
 
 	b := bytes.NewBuffer(nil)
-	err = updateCharts([]*repo.ChartRepository{r}, b, true)
+	err = updateCharts([]*repo.ChartRepository{r}, b, true, false)
 	if err == nil {
 		t.Error("Repo update should return error because update of repository fails and 'fail-on-repo-update-fail' flag set")
 		return

pkg/repo/chartrepo.go

@@ -116,6 +116,32 @@ func (r *ChartRepository) Load() error {
 
 // DownloadIndexFile fetches the index from a repository.
 func (r *ChartRepository) DownloadIndexFile() (string, error) {
+	return r.downloadIndexFile(true)
+}
+
+// ValidateAndDownloadIndexFile fetches the index without consolidating validation warnings.
+func (r *ChartRepository) ValidateAndDownloadIndexFile() (string, error) {
+	return r.downloadIndexFile(false)
+}
+
+// Index generates an index for the chart repository and writes an index.yaml file.
+func (r *ChartRepository) Index() error {
+	err := r.generateIndex()
+	if err != nil {
+		return err
+	}
+	return r.saveIndexFile()
+}
+
+func (r *ChartRepository) saveIndexFile() error {
+	index, err := yaml.Marshal(r.IndexFile)
+	if err != nil {
+		return err
+	}
+	return ioutil.WriteFile(filepath.Join(r.Config.Name, indexPath), index, 0644)
+}
+
+func (r *ChartRepository) downloadIndexFile(consolidateWarnings bool) (string, error) {
 	parsedURL, err := url.Parse(r.Config.URL)
 	if err != nil {
 		return "", err
@@ -141,7 +167,7 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) {
 		return "", err
 	}
 
-	indexFile, err := loadIndex(index, r.Config.URL)
+	indexFile, err := loadIndex(index, r.Config.URL, consolidateWarnings)
 	if err != nil {
 		return "", err
 	}
@@ -161,23 +187,6 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) {
 	return fname, ioutil.WriteFile(fname, index, 0644)
 }
 
-// Index generates an index for the chart repository and writes an index.yaml file.
-func (r *ChartRepository) Index() error {
-	err := r.generateIndex()
-	if err != nil {
-		return err
-	}
-	return r.saveIndexFile()
-}
-
-func (r *ChartRepository) saveIndexFile() error {
-	index, err := yaml.Marshal(r.IndexFile)
-	if err != nil {
-		return err
-	}
-	return ioutil.WriteFile(filepath.Join(r.Config.Name, indexPath), index, 0644)
-}
-
 func (r *ChartRepository) generateIndex() error {
 	for _, path := range r.ChartPaths {
 		ch, err := loader.Load(path)

pkg/repo/index.go

@@ -108,7 +108,7 @@ func LoadIndexFile(path string) (*IndexFile, error) {
 	if err != nil {
 		return nil, err
 	}
-	i, err := loadIndex(b, path)
+	i, err := loadIndex(b, path, true)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error loading %s", path)
 	}
@@ -326,7 +326,7 @@ func IndexDirectory(dir, baseURL string) (*IndexFile, error) {
 //
 // 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, source string) (*IndexFile, error) {
+func loadIndex(data []byte, source string, consolidateWarnings bool) (*IndexFile, error) {
 	i := &IndexFile{}
 
 	if len(data) == 0 {
@@ -337,17 +337,24 @@ func loadIndex(data []byte, source string) (*IndexFile, error) {
 		return i, err
 	}
 
+	invalidCharts := 0
 	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 {
+				invalidCharts++
+				if !consolidateWarnings {
 					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:]...)
 			}
 		}
 	}
+	if invalidCharts > 0 && consolidateWarnings {
+		log.Printf("skipped loading %d invalid chart entries from %s\nrun 'helm repo update --show-all-warnings' for full list of invalid entries", invalidCharts, source)
+	}
 	i.SortEntries()
 	if i.APIVersion == "" {
 		return i, ErrNoAPIVersion

pkg/repo/index_test.go

@@ -147,13 +147,13 @@ 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), "indexWithDuplicates"); err == nil {
+	if _, err := loadIndex([]byte(indexWithDuplicates), "indexWithDuplicates", false); err == nil {
 		t.Errorf("Expected an error when duplicate entries are present")
 	}
 }
 
 func TestLoadIndex_Empty(t *testing.T) {
-	if _, err := loadIndex([]byte(""), "indexWithEmpty"); err == nil {
+	if _, err := loadIndex([]byte(""), "indexWithEmpty", false); err == nil {
 		t.Errorf("Expected an error when index.yaml is empty.")
 	}
 }