From a590688b159a2ccd83fd944bf6bbb885769ad169 Mon Sep 17 00:00:00 2001 From: Russell Cousineau Date: Wed, 14 Apr 2021 14:21:10 -0700 Subject: [PATCH 1/2] feat(helm): optionally hide validation warnings when doing the following commands: * `helm repo add` * `helm repo update` * `helm search repo` the new flag `--hide-validation-warnings` will suppress the message "skipping loading invalid entry for chart" Closes #9407 Signed-off-by: Russell Cousineau --- cmd/helm/dependency_build_test.go | 2 +- cmd/helm/dependency_update_test.go | 2 +- cmd/helm/flags.go | 2 +- cmd/helm/repo_add.go | 16 +++++++++------- cmd/helm/repo_index.go | 2 +- cmd/helm/repo_index_test.go | 6 +++--- cmd/helm/repo_update.go | 23 +++++++++++++---------- cmd/helm/repo_update_test.go | 8 ++++---- cmd/helm/search_repo.go | 22 ++++++++++++---------- internal/resolver/resolver.go | 2 +- pkg/downloader/chart_downloader.go | 4 ++-- pkg/downloader/manager.go | 4 ++-- pkg/repo/chartrepo.go | 10 +++++----- pkg/repo/chartrepo_test.go | 6 +++--- pkg/repo/index.go | 10 ++++++---- pkg/repo/index_test.go | 16 ++++++++-------- 16 files changed, 72 insertions(+), 63 deletions(-) diff --git a/cmd/helm/dependency_build_test.go b/cmd/helm/dependency_build_test.go index 33198a9dd..4e6bb12a3 100644 --- a/cmd/helm/dependency_build_test.go +++ b/cmd/helm/dependency_build_test.go @@ -109,7 +109,7 @@ func TestDependencyBuildCmd(t *testing.T) { t.Fatal(err) } - i, err := repo.LoadIndexFile(filepath.Join(rootDir, "index.yaml")) + i, err := repo.LoadIndexFile(filepath.Join(rootDir, "index.yaml"), false) if err != nil { t.Fatal(err) } diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index 9f7b0f303..75f49ce4d 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -96,7 +96,7 @@ func TestDependencyUpdateCmd(t *testing.T) { t.Fatal(err) } - i, err := repo.LoadIndexFile(dir(helmpath.CacheIndexFile("test"))) + i, err := repo.LoadIndexFile(dir(helmpath.CacheIndexFile("test")), false) if err != nil { t.Fatal(err) } diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index fe653625d..7a75063f1 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -150,7 +150,7 @@ func compVersionFlag(chartRef string, toComplete string) ([]string, cobra.ShellC path := filepath.Join(settings.RepositoryCache, helmpath.CacheIndexFile(repoName)) var versions []string - if indexFile, err := repo.LoadIndexFile(path); err == nil { + if indexFile, err := repo.LoadIndexFile(path, false); err == nil { for _, details := range indexFile.Entries[chartName] { version := details.Metadata.Version if strings.HasPrefix(version, toComplete) { diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 52cd020f5..7d64b3e1e 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -44,12 +44,13 @@ var deprecatedRepos = map[string]string{ } type repoAddOptions struct { - name string - url string - username string - password string - forceUpdate bool - allowDeprecatedRepos bool + name string + url string + username string + password string + forceUpdate bool + allowDeprecatedRepos bool + hideValidationWarnings bool certFile string keyFile string @@ -91,6 +92,7 @@ func newRepoAddCmd(out io.Writer) *cobra.Command { f.StringVar(&o.caFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") f.BoolVar(&o.insecureSkipTLSverify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the repository") f.BoolVar(&o.allowDeprecatedRepos, "allow-deprecated-repos", false, "by default, this command will not allow adding official repos that have been permanently deleted. This disables that behavior") + f.BoolVar(&o.hideValidationWarnings, "hide-validation-warnings", false, "hide validation warnings while indexing repository") return cmd } @@ -180,7 +182,7 @@ func (o *repoAddOptions) run(out io.Writer) error { if o.repoCache != "" { r.CachePath = o.repoCache } - if _, err := r.DownloadIndexFile(); err != nil { + if _, err := r.DownloadIndexFile(o.hideValidationWarnings); err != nil { return errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", o.url) } diff --git a/cmd/helm/repo_index.go b/cmd/helm/repo_index.go index 917acd442..346b7b554 100644 --- a/cmd/helm/repo_index.go +++ b/cmd/helm/repo_index.go @@ -97,7 +97,7 @@ func index(dir, url, mergeTo string) error { i2 = repo.NewIndexFile() i2.WriteFile(mergeTo, 0644) } else { - i2, err = repo.LoadIndexFile(mergeTo) + i2, err = repo.LoadIndexFile(mergeTo, false) if err != nil { return errors.Wrap(err, "merge failed") } diff --git a/cmd/helm/repo_index_test.go b/cmd/helm/repo_index_test.go index ae3390154..3ab71ba4a 100644 --- a/cmd/helm/repo_index_test.go +++ b/cmd/helm/repo_index_test.go @@ -49,7 +49,7 @@ func TestRepoIndexCmd(t *testing.T) { destIndex := filepath.Join(dir, "index.yaml") - index, err := repo.LoadIndexFile(destIndex) + index, err := repo.LoadIndexFile(destIndex, false) if err != nil { t.Fatal(err) } @@ -90,7 +90,7 @@ func TestRepoIndexCmd(t *testing.T) { t.Error(err) } - index, err = repo.LoadIndexFile(destIndex) + index, err = repo.LoadIndexFile(destIndex, false) if err != nil { t.Fatal(err) } @@ -119,7 +119,7 @@ func TestRepoIndexCmd(t *testing.T) { t.Error(err) } - index, err = repo.LoadIndexFile(destIndex) + index, err = repo.LoadIndexFile(destIndex, false) if err != nil { t.Fatal(err) } diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index 23dca194a..f1cd365b6 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -24,7 +24,6 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" - "helm.sh/helm/v3/cmd/helm/require" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" ) @@ -37,9 +36,10 @@ Information is cached locally, where it is used by commands like 'helm search'. var errNoRepositories = errors.New("no repositories found. You must add one before updating") type repoUpdateOptions struct { - update func([]*repo.ChartRepository, io.Writer) - repoFile string - repoCache string + hideValidationWarnings bool + update func([]*repo.ChartRepository, io.Writer, bool) + repoFile string + repoCache string } func newRepoUpdateCmd(out io.Writer) *cobra.Command { @@ -50,18 +50,21 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command { Aliases: []string{"up"}, Short: "update information of available charts locally from chart repositories", Long: updateDesc, - Args: require.NoArgs, ValidArgsFunction: noCompletions, RunE: func(cmd *cobra.Command, args []string) error { o.repoFile = settings.RepositoryConfig o.repoCache = settings.RepositoryCache - return o.run(out) + return o.run(out, args) }, } + + f := cmd.Flags() + f.BoolVar(&o.hideValidationWarnings, "hide-validation-warnings", false, "hide validation warnings 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): @@ -84,18 +87,18 @@ func (o *repoUpdateOptions) run(out io.Writer) error { repos = append(repos, r) } - o.update(repos, out) + o.update(repos, out, o.hideValidationWarnings) return nil } -func updateCharts(repos []*repo.ChartRepository, out io.Writer) { +func updateCharts(repos []*repo.ChartRepository, out io.Writer, hideValidationWarnings bool) { fmt.Fprintln(out, "Hang tight while we grab the latest from your chart repositories...") var wg sync.WaitGroup for _, re := range repos { wg.Add(1) go func(re *repo.ChartRepository) { defer wg.Done() - if _, err := re.DownloadIndexFile(); err != nil { + if _, err := re.DownloadIndexFile(hideValidationWarnings); 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) } else { fmt.Fprintf(out, "...Successfully got an update from the %q chart repository\n", re.Config.Name) diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index 83ef24349..5f22f392c 100644 --- a/cmd/helm/repo_update_test.go +++ b/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) { + updater := func(repos []*repo.ChartRepository, out io.Writer, hideValidationWarnings bool) { for _, re := range repos { fmt.Fprintln(out, re.Config.Name) } @@ -44,7 +44,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) } @@ -71,7 +71,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 { @@ -98,7 +98,7 @@ func TestUpdateCharts(t *testing.T) { } b := bytes.NewBuffer(nil) - updateCharts([]*repo.ChartRepository{r}, b) + updateCharts([]*repo.ChartRepository{r}, b, false) got := b.String() if strings.Contains(got, "Unable to get an update") { diff --git a/cmd/helm/search_repo.go b/cmd/helm/search_repo.go index ba692a2e7..3cea56003 100644 --- a/cmd/helm/search_repo.go +++ b/cmd/helm/search_repo.go @@ -63,14 +63,15 @@ Repositories are managed with 'helm repo' commands. const searchMaxScore = 25 type searchRepoOptions struct { - versions bool - regexp bool - devel bool - version string - maxColWidth uint - repoFile string - repoCacheDir string - outputFormat output.Format + versions bool + regexp bool + devel bool + hideValidationWarnings bool + version string + maxColWidth uint + repoFile string + repoCacheDir string + outputFormat output.Format } func newSearchRepoCmd(out io.Writer) *cobra.Command { @@ -91,6 +92,7 @@ func newSearchRepoCmd(out io.Writer) *cobra.Command { f.BoolVarP(&o.regexp, "regexp", "r", false, "use regular expressions for searching repositories you have added") f.BoolVarP(&o.versions, "versions", "l", false, "show the long listing, with each version of each chart on its own line, for repositories you have added") f.BoolVar(&o.devel, "devel", false, "use development versions (alpha, beta, and release candidate releases), too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored") + f.BoolVar(&o.hideValidationWarnings, "hide-validation-warnings", false, "hide validation warnings while indexing repository") f.StringVar(&o.version, "version", "", "search using semantic versioning constraints on repositories you have added") f.UintVar(&o.maxColWidth, "max-col-width", 50, "maximum column width for output table") bindOutputFlag(cmd, &o.outputFormat) @@ -184,7 +186,7 @@ func (o *searchRepoOptions) buildIndex() (*search.Index, error) { for _, re := range rf.Repositories { n := re.Name f := filepath.Join(o.repoCacheDir, helmpath.CacheIndexFile(n)) - ind, err := repo.LoadIndexFile(f) + ind, err := repo.LoadIndexFile(f, o.hideValidationWarnings) if err != nil { warning("Repo %q is corrupt or missing. Try 'helm repo update'.", n) warning("%s", err) @@ -276,7 +278,7 @@ func compListChartsOfRepo(repoName string, prefix string) []string { // installed but before the user does a 'helm repo update' to generate the // first cached charts file. path = filepath.Join(settings.RepositoryCache, helmpath.CacheIndexFile(repoName)) - if indexFile, err := repo.LoadIndexFile(path); err == nil { + if indexFile, err := repo.LoadIndexFile(path, false); err == nil { for name := range indexFile.Entries { fullName := fmt.Sprintf("%s/%s", repoName, name) if strings.HasPrefix(fullName, prefix) { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index de0634093..4a9cdfef3 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -113,7 +113,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string var ok bool found := true if !strings.HasPrefix(d.Repository, "oci://") { - repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName))) + repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)), false) if err != nil { return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName) } diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 6c600bebb..463915bea 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -230,7 +230,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er // Next, we need to load the index, and actually look up the chart. idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name)) - i, err := repo.LoadIndexFile(idxFile) + i, err := repo.LoadIndexFile(idxFile, false) if err != nil { return u, errors.Wrap(err, "no cached repo found. (try 'helm repo update')") } @@ -347,7 +347,7 @@ func (c *ChartDownloader) scanReposForURL(u string, rf *repo.File) (*repo.Entry, } idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name)) - i, err := repo.LoadIndexFile(idxFile) + i, err := repo.LoadIndexFile(idxFile, false) if err != nil { return nil, errors.Wrap(err, "no cached repo found. (try 'helm repo update')") } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index e89ac7c02..9b4692704 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -652,7 +652,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { } wg.Add(1) go func(r *repo.ChartRepository) { - if _, err := r.DownloadIndexFile(); err != nil { + if _, err := r.DownloadIndexFile(false); err != nil { // For those dependencies that are not known to helm and using a // generated key name we display the repo url. if strings.HasPrefix(r.Config.Name, managerKeyPrefix) { @@ -795,7 +795,7 @@ func (m *Manager) loadChartRepositories() (map[string]*repo.ChartRepository, err for _, re := range rf.Repositories { lname := re.Name idxFile := filepath.Join(m.RepositoryCache, helmpath.CacheIndexFile(lname)) - index, err := repo.LoadIndexFile(idxFile) + index, err := repo.LoadIndexFile(idxFile, false) if err != nil { return indices, err } diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 09b94fd42..55c551552 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -99,7 +99,7 @@ func (r *ChartRepository) Load() error { filepath.Walk(r.Config.Name, func(path string, f os.FileInfo, err error) error { if !f.IsDir() { if strings.Contains(f.Name(), "-index.yaml") { - i, err := LoadIndexFile(path) + i, err := LoadIndexFile(path, false) if err != nil { return err } @@ -114,7 +114,7 @@ func (r *ChartRepository) Load() error { } // DownloadIndexFile fetches the index from a repository. -func (r *ChartRepository) DownloadIndexFile() (string, error) { +func (r *ChartRepository) DownloadIndexFile(hideValidationWarnings bool) (string, error) { parsedURL, err := url.Parse(r.Config.URL) if err != nil { return "", err @@ -139,7 +139,7 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) { return "", err } - indexFile, err := loadIndex(index, r.Config.URL) + indexFile, err := loadIndex(index, r.Config.URL, hideValidationWarnings) if err != nil { return "", err } @@ -237,13 +237,13 @@ func FindChartInAuthAndTLSRepoURL(repoURL, username, password, chartName, chartV if err != nil { return "", err } - idx, err := r.DownloadIndexFile() + idx, err := r.DownloadIndexFile(false) if err != nil { return "", errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", repoURL) } // Read the index file for the repository to get chart information and return chart URL - repoIndex, err := LoadIndexFile(idx) + repoIndex, err := LoadIndexFile(idx, false) if err != nil { return "", err } diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index 7bd563460..ad1ad0557 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -93,7 +93,7 @@ func TestIndex(t *testing.T) { } tempIndexPath := filepath.Join(testRepository, indexPath) - actual, err := LoadIndexFile(tempIndexPath) + actual, err := LoadIndexFile(tempIndexPath, false) defer os.Remove(tempIndexPath) // clean up if err != nil { t.Errorf("Error loading index file %v", err) @@ -105,7 +105,7 @@ func TestIndex(t *testing.T) { if err != nil { t.Errorf("Error performing re-index: %s\n", err) } - second, err := LoadIndexFile(tempIndexPath) + second, err := LoadIndexFile(tempIndexPath, false) if err != nil { t.Errorf("Error re-loading index file %v", err) } @@ -156,7 +156,7 @@ func TestIndexCustomSchemeDownload(t *testing.T) { } defer os.Remove(tempIndexFile.Name()) - idx, err := repo.DownloadIndexFile() + idx, err := repo.DownloadIndexFile(false) if err != nil { t.Fatalf("Failed to download index file to %s: %v", idx, err) } diff --git a/pkg/repo/index.go b/pkg/repo/index.go index e86b17349..d58b3f28e 100644 --- a/pkg/repo/index.go +++ b/pkg/repo/index.go @@ -101,12 +101,12 @@ func NewIndexFile() *IndexFile { } // LoadIndexFile takes a file at the given path and returns an IndexFile object -func LoadIndexFile(path string) (*IndexFile, error) { +func LoadIndexFile(path string, hideValidationWarnings bool) (*IndexFile, error) { b, err := ioutil.ReadFile(path) if err != nil { return nil, err } - i, err := loadIndex(b, path) + i, err := loadIndex(b, path, hideValidationWarnings) if err != nil { return nil, errors.Wrapf(err, "error loading %s", path) } @@ -324,7 +324,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, hideValidationWarnings bool) (*IndexFile, error) { i := &IndexFile{} if err := yaml.UnmarshalStrict(data, i); err != nil { return i, err @@ -336,7 +336,9 @@ func loadIndex(data []byte, source string) (*IndexFile, error) { 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) + if !hideValidationWarnings { + 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:]...) } } diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index 47f3c6b2e..d98b3190e 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -136,7 +136,7 @@ func TestLoadIndex(t *testing.T) { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() - i, err := LoadIndexFile(tc.Filename) + i, err := LoadIndexFile(tc.Filename, false) if err != nil { t.Fatal(err) } @@ -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 TestLoadIndexFileAnnotations(t *testing.T) { - i, err := LoadIndexFile(annotationstestfile) + i, err := LoadIndexFile(annotationstestfile, false) if err != nil { t.Fatal(err) } @@ -168,7 +168,7 @@ func TestLoadIndexFileAnnotations(t *testing.T) { } func TestLoadUnorderedIndex(t *testing.T) { - i, err := LoadIndexFile(unorderedTestfile) + i, err := LoadIndexFile(unorderedTestfile, false) if err != nil { t.Fatal(err) } @@ -230,7 +230,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, err := r.DownloadIndexFile() + idx, err := r.DownloadIndexFile(false) if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } @@ -239,7 +239,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - i, err := LoadIndexFile(idx) + i, err := LoadIndexFile(idx, false) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) } @@ -283,7 +283,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, err := r.DownloadIndexFile() + idx, err := r.DownloadIndexFile(false) if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } @@ -292,7 +292,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - i, err := LoadIndexFile(idx) + i, err := LoadIndexFile(idx, false) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) } From 6019257a050f540c73bb60bf14ab06676984ee60 Mon Sep 17 00:00:00 2001 From: Russell Cousineau Date: Wed, 14 Apr 2021 17:26:57 -0700 Subject: [PATCH 2/2] preserve backwards compatibility create new functions instead of modifying existing ones Signed-off-by: Russell Cousineau --- cmd/helm/dependency_build_test.go | 2 +- cmd/helm/dependency_update_test.go | 2 +- cmd/helm/flags.go | 2 +- cmd/helm/repo_add.go | 7 +++++- cmd/helm/repo_index.go | 2 +- cmd/helm/repo_index_test.go | 6 +++--- cmd/helm/repo_update.go | 8 ++++++- cmd/helm/search_repo.go | 9 ++++++-- internal/resolver/resolver.go | 2 +- pkg/downloader/chart_downloader.go | 4 ++-- pkg/downloader/manager.go | 4 ++-- pkg/repo/chartrepo.go | 34 ++++++++++++++++++------------ pkg/repo/chartrepo_test.go | 6 +++--- pkg/repo/index.go | 17 +++++++++++++-- pkg/repo/index_test.go | 14 ++++++------ 15 files changed, 78 insertions(+), 41 deletions(-) diff --git a/cmd/helm/dependency_build_test.go b/cmd/helm/dependency_build_test.go index 4e6bb12a3..33198a9dd 100644 --- a/cmd/helm/dependency_build_test.go +++ b/cmd/helm/dependency_build_test.go @@ -109,7 +109,7 @@ func TestDependencyBuildCmd(t *testing.T) { t.Fatal(err) } - i, err := repo.LoadIndexFile(filepath.Join(rootDir, "index.yaml"), false) + i, err := repo.LoadIndexFile(filepath.Join(rootDir, "index.yaml")) if err != nil { t.Fatal(err) } diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index 75f49ce4d..9f7b0f303 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -96,7 +96,7 @@ func TestDependencyUpdateCmd(t *testing.T) { t.Fatal(err) } - i, err := repo.LoadIndexFile(dir(helmpath.CacheIndexFile("test")), false) + i, err := repo.LoadIndexFile(dir(helmpath.CacheIndexFile("test"))) if err != nil { t.Fatal(err) } diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 7a75063f1..fe653625d 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -150,7 +150,7 @@ func compVersionFlag(chartRef string, toComplete string) ([]string, cobra.ShellC path := filepath.Join(settings.RepositoryCache, helmpath.CacheIndexFile(repoName)) var versions []string - if indexFile, err := repo.LoadIndexFile(path, false); err == nil { + if indexFile, err := repo.LoadIndexFile(path); err == nil { for _, details := range indexFile.Entries[chartName] { version := details.Metadata.Version if strings.HasPrefix(version, toComplete) { diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 7d64b3e1e..aaeaa2a38 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -182,7 +182,12 @@ func (o *repoAddOptions) run(out io.Writer) error { if o.repoCache != "" { r.CachePath = o.repoCache } - if _, err := r.DownloadIndexFile(o.hideValidationWarnings); err != nil { + if o.hideValidationWarnings { + _, err = r.DownloadIndexFileHideValidationWarnings() + } else { + _, err = r.DownloadIndexFile() + } + if err != nil { return errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", o.url) } diff --git a/cmd/helm/repo_index.go b/cmd/helm/repo_index.go index 346b7b554..917acd442 100644 --- a/cmd/helm/repo_index.go +++ b/cmd/helm/repo_index.go @@ -97,7 +97,7 @@ func index(dir, url, mergeTo string) error { i2 = repo.NewIndexFile() i2.WriteFile(mergeTo, 0644) } else { - i2, err = repo.LoadIndexFile(mergeTo, false) + i2, err = repo.LoadIndexFile(mergeTo) if err != nil { return errors.Wrap(err, "merge failed") } diff --git a/cmd/helm/repo_index_test.go b/cmd/helm/repo_index_test.go index 3ab71ba4a..ae3390154 100644 --- a/cmd/helm/repo_index_test.go +++ b/cmd/helm/repo_index_test.go @@ -49,7 +49,7 @@ func TestRepoIndexCmd(t *testing.T) { destIndex := filepath.Join(dir, "index.yaml") - index, err := repo.LoadIndexFile(destIndex, false) + index, err := repo.LoadIndexFile(destIndex) if err != nil { t.Fatal(err) } @@ -90,7 +90,7 @@ func TestRepoIndexCmd(t *testing.T) { t.Error(err) } - index, err = repo.LoadIndexFile(destIndex, false) + index, err = repo.LoadIndexFile(destIndex) if err != nil { t.Fatal(err) } @@ -119,7 +119,7 @@ func TestRepoIndexCmd(t *testing.T) { t.Error(err) } - index, err = repo.LoadIndexFile(destIndex, false) + index, err = repo.LoadIndexFile(destIndex) if err != nil { t.Fatal(err) } diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index f1cd365b6..1e67b66ee 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -98,7 +98,13 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer, hideValidationWa wg.Add(1) go func(re *repo.ChartRepository) { defer wg.Done() - if _, err := re.DownloadIndexFile(hideValidationWarnings); err != nil { + var err error + if hideValidationWarnings { + _, err = re.DownloadIndexFileHideValidationWarnings() + } 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) } else { fmt.Fprintf(out, "...Successfully got an update from the %q chart repository\n", re.Config.Name) diff --git a/cmd/helm/search_repo.go b/cmd/helm/search_repo.go index 3cea56003..9831d9d7f 100644 --- a/cmd/helm/search_repo.go +++ b/cmd/helm/search_repo.go @@ -186,7 +186,12 @@ func (o *searchRepoOptions) buildIndex() (*search.Index, error) { for _, re := range rf.Repositories { n := re.Name f := filepath.Join(o.repoCacheDir, helmpath.CacheIndexFile(n)) - ind, err := repo.LoadIndexFile(f, o.hideValidationWarnings) + var ind *repo.IndexFile + if o.hideValidationWarnings { + ind, err = repo.LoadIndexFileHideValidationWarnings(f) + } else { + ind, err = repo.LoadIndexFile(f) + } if err != nil { warning("Repo %q is corrupt or missing. Try 'helm repo update'.", n) warning("%s", err) @@ -278,7 +283,7 @@ func compListChartsOfRepo(repoName string, prefix string) []string { // installed but before the user does a 'helm repo update' to generate the // first cached charts file. path = filepath.Join(settings.RepositoryCache, helmpath.CacheIndexFile(repoName)) - if indexFile, err := repo.LoadIndexFile(path, false); err == nil { + if indexFile, err := repo.LoadIndexFile(path); err == nil { for name := range indexFile.Entries { fullName := fmt.Sprintf("%s/%s", repoName, name) if strings.HasPrefix(fullName, prefix) { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 4a9cdfef3..de0634093 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -113,7 +113,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string var ok bool found := true if !strings.HasPrefix(d.Repository, "oci://") { - repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)), false) + repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName))) if err != nil { return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName) } diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 463915bea..6c600bebb 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -230,7 +230,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er // Next, we need to load the index, and actually look up the chart. idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name)) - i, err := repo.LoadIndexFile(idxFile, false) + i, err := repo.LoadIndexFile(idxFile) if err != nil { return u, errors.Wrap(err, "no cached repo found. (try 'helm repo update')") } @@ -347,7 +347,7 @@ func (c *ChartDownloader) scanReposForURL(u string, rf *repo.File) (*repo.Entry, } idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name)) - i, err := repo.LoadIndexFile(idxFile, false) + i, err := repo.LoadIndexFile(idxFile) if err != nil { return nil, errors.Wrap(err, "no cached repo found. (try 'helm repo update')") } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 9b4692704..e89ac7c02 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -652,7 +652,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { } wg.Add(1) go func(r *repo.ChartRepository) { - if _, err := r.DownloadIndexFile(false); err != nil { + if _, err := r.DownloadIndexFile(); err != nil { // For those dependencies that are not known to helm and using a // generated key name we display the repo url. if strings.HasPrefix(r.Config.Name, managerKeyPrefix) { @@ -795,7 +795,7 @@ func (m *Manager) loadChartRepositories() (map[string]*repo.ChartRepository, err for _, re := range rf.Repositories { lname := re.Name idxFile := filepath.Join(m.RepositoryCache, helmpath.CacheIndexFile(lname)) - index, err := repo.LoadIndexFile(idxFile, false) + index, err := repo.LoadIndexFile(idxFile) if err != nil { return indices, err } diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 55c551552..938b13b4b 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -99,7 +99,7 @@ func (r *ChartRepository) Load() error { filepath.Walk(r.Config.Name, func(path string, f os.FileInfo, err error) error { if !f.IsDir() { if strings.Contains(f.Name(), "-index.yaml") { - i, err := LoadIndexFile(path, false) + i, err := LoadIndexFile(path) if err != nil { return err } @@ -114,7 +114,24 @@ func (r *ChartRepository) Load() error { } // DownloadIndexFile fetches the index from a repository. -func (r *ChartRepository) DownloadIndexFile(hideValidationWarnings bool) (string, error) { +func (r *ChartRepository) DownloadIndexFile() (string, error) { + return r.downloadIndexFile(false) +} + +func (r *ChartRepository) DownloadIndexFileHideValidationWarnings() (string, error) { + return r.downloadIndexFile(true) +} + +// 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) downloadIndexFile(hideValidationWarnings bool) (string, error) { parsedURL, err := url.Parse(r.Config.URL) if err != nil { return "", err @@ -159,15 +176,6 @@ func (r *ChartRepository) DownloadIndexFile(hideValidationWarnings bool) (string 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 { @@ -237,13 +245,13 @@ func FindChartInAuthAndTLSRepoURL(repoURL, username, password, chartName, chartV if err != nil { return "", err } - idx, err := r.DownloadIndexFile(false) + idx, err := r.DownloadIndexFile() if err != nil { return "", errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", repoURL) } // Read the index file for the repository to get chart information and return chart URL - repoIndex, err := LoadIndexFile(idx, false) + repoIndex, err := LoadIndexFile(idx) if err != nil { return "", err } diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index ad1ad0557..7bd563460 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -93,7 +93,7 @@ func TestIndex(t *testing.T) { } tempIndexPath := filepath.Join(testRepository, indexPath) - actual, err := LoadIndexFile(tempIndexPath, false) + actual, err := LoadIndexFile(tempIndexPath) defer os.Remove(tempIndexPath) // clean up if err != nil { t.Errorf("Error loading index file %v", err) @@ -105,7 +105,7 @@ func TestIndex(t *testing.T) { if err != nil { t.Errorf("Error performing re-index: %s\n", err) } - second, err := LoadIndexFile(tempIndexPath, false) + second, err := LoadIndexFile(tempIndexPath) if err != nil { t.Errorf("Error re-loading index file %v", err) } @@ -156,7 +156,7 @@ func TestIndexCustomSchemeDownload(t *testing.T) { } defer os.Remove(tempIndexFile.Name()) - idx, err := repo.DownloadIndexFile(false) + idx, err := repo.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %v", idx, err) } diff --git a/pkg/repo/index.go b/pkg/repo/index.go index d58b3f28e..06cc66e03 100644 --- a/pkg/repo/index.go +++ b/pkg/repo/index.go @@ -101,12 +101,25 @@ func NewIndexFile() *IndexFile { } // LoadIndexFile takes a file at the given path and returns an IndexFile object -func LoadIndexFile(path string, hideValidationWarnings bool) (*IndexFile, error) { +func LoadIndexFile(path string) (*IndexFile, error) { b, err := ioutil.ReadFile(path) if err != nil { return nil, err } - i, err := loadIndex(b, path, hideValidationWarnings) + i, err := loadIndex(b, path, false) + if err != nil { + return nil, errors.Wrapf(err, "error loading %s", path) + } + return i, nil +} + +// LoadIndexFile takes a file at the given path and returns an IndexFile object +func LoadIndexFileHideValidationWarnings(path string) (*IndexFile, error) { + b, err := ioutil.ReadFile(path) + if err != nil { + return nil, err + } + i, err := loadIndex(b, path, true) if err != nil { return nil, errors.Wrapf(err, "error loading %s", path) } diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index d98b3190e..1e8e1a9cd 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -136,7 +136,7 @@ func TestLoadIndex(t *testing.T) { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() - i, err := LoadIndexFile(tc.Filename, false) + i, err := LoadIndexFile(tc.Filename) if err != nil { t.Fatal(err) } @@ -153,7 +153,7 @@ func TestLoadIndex_Duplicates(t *testing.T) { } func TestLoadIndexFileAnnotations(t *testing.T) { - i, err := LoadIndexFile(annotationstestfile, false) + i, err := LoadIndexFile(annotationstestfile) if err != nil { t.Fatal(err) } @@ -168,7 +168,7 @@ func TestLoadIndexFileAnnotations(t *testing.T) { } func TestLoadUnorderedIndex(t *testing.T) { - i, err := LoadIndexFile(unorderedTestfile, false) + i, err := LoadIndexFile(unorderedTestfile) if err != nil { t.Fatal(err) } @@ -230,7 +230,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, err := r.DownloadIndexFile(false) + idx, err := r.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } @@ -239,7 +239,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - i, err := LoadIndexFile(idx, false) + i, err := LoadIndexFile(idx) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) } @@ -283,7 +283,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, err := r.DownloadIndexFile(false) + idx, err := r.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } @@ -292,7 +292,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - i, err := LoadIndexFile(idx, false) + i, err := LoadIndexFile(idx) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) }