pull/9611/merge
rcousineau-xandr 3 years ago committed by GitHub
commit b9807bf2e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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
showValidationErrors 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.showValidationErrors, "show-repo-validation-errors", 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.showValidationErrors)
}
func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error {
func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool, showValidationErrors 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 showValidationErrors {
_, 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 {

@ -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, showValidationErrors 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, showValidationErrors 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, showValidationErrors 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,50 @@ 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
}
var expectedErr = "Failed to update the following repositories"
var receivedErr = err.Error()
if !strings.Contains(receivedErr, expectedErr) {
t.Errorf("Expected error (%s) but got (%s) instead", expectedErr, receivedErr)
}
if !strings.Contains(receivedErr, invalidURL) {
t.Errorf("Expected invalid URL (%s) in error message but got (%s) instead", invalidURL, receivedErr)
}
got := b.String()
if !strings.Contains(got, "Unable to get an update") {
t.Errorf("Repo should have failed update but instead got: %q", got)
}
if strings.Contains(got, "Update Complete.") {
t.Error("Update was not successful and should return error message because 'fail-on-repo-update-fail' flag set")
}
}
func TestUpdateChartsShowValidationErrors(t *testing.T) {
defer resetEnv()()
defer ensure.HelmHome(t)()
ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*")
if err != nil {
t.Fatal(err)
}
defer ts.Stop()
var invalidURL = ts.URL() + "55"
r, err := repo.NewChartRepository(&repo.Entry{
Name: "charts",
URL: invalidURL,
}, getter.All(settings))
if err != nil {
t.Error(err)
}
b := bytes.NewBuffer(nil)
err = updateCharts([]*repo.ChartRepository{r}, b, true, true)
if err == nil {
t.Error("Repo update should return error because update of repository fails and 'fail-on-repo-update-fail' flag set")
return

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

@ -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)
}
@ -330,7 +330,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, consolidateValidationErrors bool) (*IndexFile, error) {
i := &IndexFile{}
if len(data) == 0 {
@ -341,6 +341,7 @@ 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] == nil {
@ -351,11 +352,17 @@ 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)
invalidCharts++
if !consolidateValidationErrors {
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 && consolidateValidationErrors {
log.Printf("skipped loading %d invalid chart entries from %s\nrun 'helm repo update --show-repo-validation-errors' for full list of invalid entries", invalidCharts, source)
}
i.SortEntries()
if i.APIVersion == "" {
return i, ErrNoAPIVersion

@ -156,7 +156,7 @@ 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")
}
}
@ -168,7 +168,7 @@ func TestLoadIndex_EmptyEntry(t *testing.T) {
}
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.")
}
}

Loading…
Cancel
Save