Implement support for multiple args for repo remove (#7791)

* Implement support for multiple args for repo remove

Signed-off-by: Andreas Lindhé <andreas@lindhe.io>
Signed-off-by: Matheus Hunsche <matheus.hunsche@ifood.com.br>

cmd/helm/repo_list.go

@@ -97,13 +97,38 @@ func (r *repoListWriter) encodeByFormat(out io.Writer, format output.Format) err
 	return nil
 }
 
+// Returns all repos from repos, except those with names matching ignoredRepoNames
+// Inspired by https://stackoverflow.com/a/28701031/893211
+func filterRepos(repos []*repo.Entry, ignoredRepoNames []string) []*repo.Entry {
+	// if ignoredRepoNames is nil, just return repo
+	if ignoredRepoNames == nil {
+		return repos
+	}
+
+	filteredRepos := make([]*repo.Entry, 0)
+
+	ignored := make(map[string]bool, len(ignoredRepoNames))
+	for _, repoName := range ignoredRepoNames {
+		ignored[repoName] = true
+	}
+
+	for _, repo := range repos {
+		if _, removed := ignored[repo.Name]; !removed {
+			filteredRepos = append(filteredRepos, repo)
+		}
+	}
+
+	return filteredRepos
+}
+
 // Provide dynamic auto-completion for repo names
-func compListRepos(prefix string) []string {
+func compListRepos(prefix string, ignoredRepoNames []string) []string {
 	var rNames []string
 
 	f, err := repo.LoadFile(settings.RepositoryConfig)
 	if err == nil && len(f.Repositories) > 0 {
-		for _, repo := range f.Repositories {
+		filteredRepos := filterRepos(f.Repositories, ignoredRepoNames)
+		for _, repo := range filteredRepos {
 			if strings.HasPrefix(repo.Name, prefix) {
 				rNames = append(rNames, repo.Name)
 			}

cmd/helm/repo_remove.go

@@ -32,7 +32,7 @@ import (
 )
 
 type repoRemoveOptions struct {
-	name      string
+	names     []string
 	repoFile  string
 	repoCache string
 }
@@ -41,24 +41,21 @@ func newRepoRemoveCmd(out io.Writer) *cobra.Command {
 	o := &repoRemoveOptions{}
 
 	cmd := &cobra.Command{
-		Use:     "remove [NAME]",
+		Use:     "remove [REPO1 [REPO2 ...]]",
 		Aliases: []string{"rm"},
-		Short:   "remove a chart repository",
+		Short:   "remove one or more chart repositories",
-		Args:    require.ExactArgs(1),
+		Args:    require.MinimumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			o.repoFile = settings.RepositoryConfig
 			o.repoCache = settings.RepositoryCache
-			o.name = args[0]
+			o.names = args
 			return o.run(out)
 		},
 	}
 
 	// Function providing dynamic auto-completion
 	completion.RegisterValidArgsFunc(cmd, func(cmd *cobra.Command, args []string, toComplete string) ([]string, completion.BashCompDirective) {
-		if len(args) != 0 {
+		return compListRepos(toComplete, args), completion.BashCompDirectiveNoFileComp
-			return nil, completion.BashCompDirectiveNoFileComp
-		}
-		return compListRepos(toComplete), completion.BashCompDirectiveNoFileComp
 	})
 
 	return cmd
@@ -70,18 +67,20 @@ func (o *repoRemoveOptions) run(out io.Writer) error {
 		return errors.New("no repositories configured")
 	}
 
-	if !r.Remove(o.name) {
+	for _, name := range o.names {
-		return errors.Errorf("no repo named %q found", o.name)
+		if !r.Remove(name) {
-	}
+			return errors.Errorf("no repo named %q found", name)
-	if err := r.WriteFile(o.repoFile, 0644); err != nil {
+		}
-		return err
+		if err := r.WriteFile(o.repoFile, 0644); err != nil {
-	}
+			return err
+		}
 
-	if err := removeRepoCache(o.repoCache, o.name); err != nil {
+		if err := removeRepoCache(o.repoCache, name); err != nil {
-		return err
+			return err
+		}
+		fmt.Fprintf(out, "%q has been removed from your repositories\n", name)
 	}
 
-	fmt.Fprintf(out, "%q has been removed from your repositories\n", o.name)
 	return nil
 }
 

cmd/helm/repo_remove_test.go

@@ -44,7 +44,7 @@ func TestRepoRemove(t *testing.T) {
 	b := bytes.NewBuffer(nil)
 
 	rmOpts := repoRemoveOptions{
-		name:      testRepoName,
+		names:     []string{testRepoName},
 		repoFile:  repoFile,
 		repoCache: rootDir,
 	}
@@ -62,14 +62,9 @@ func TestRepoRemove(t *testing.T) {
 		t.Error(err)
 	}
 
-	idx := filepath.Join(rootDir, helmpath.CacheIndexFile(testRepoName))
+	cacheIndexFile, cacheChartsFile := createCacheFiles(rootDir, testRepoName)
-	mf, _ := os.Create(idx)
-	mf.Close()
-
-	idx2 := filepath.Join(rootDir, helmpath.CacheChartsFile(testRepoName))
-	mf, _ = os.Create(idx2)
-	mf.Close()
 
+	// Reset the buffer before running repo remove
 	b.Reset()
 
 	if err := rmOpts.run(b); err != nil {
@@ -79,13 +74,7 @@ func TestRepoRemove(t *testing.T) {
 		t.Errorf("Unexpected output: %s", b.String())
 	}
 
-	if _, err := os.Stat(idx); err == nil {
+	testCacheFiles(t, cacheIndexFile, cacheChartsFile, testRepoName)
-		t.Errorf("Error cache index file was not removed for repository %s", testRepoName)
-	}
-
-	if _, err := os.Stat(idx2); err == nil {
-		t.Errorf("Error cache chart file was not removed for repository %s", testRepoName)
-	}
 
 	f, err := repo.LoadFile(repoFile)
 	if err != nil {
@@ -95,4 +84,79 @@ func TestRepoRemove(t *testing.T) {
 	if f.Has(testRepoName) {
 		t.Errorf("%s was not successfully removed from repositories list", testRepoName)
 	}
+
+	// Test removal of multiple repos in one go
+	var testRepoNames = []string{"foo", "bar", "baz"}
+	cacheFiles := make(map[string][]string, len(testRepoNames))
+
+	// Add test repos
+	for _, repoName := range testRepoNames {
+		o := &repoAddOptions{
+			name:     repoName,
+			url:      ts.URL(),
+			repoFile: repoFile,
+		}
+
+		if err := o.run(os.Stderr); err != nil {
+			t.Error(err)
+		}
+
+		cacheIndex, cacheChart := createCacheFiles(rootDir, repoName)
+		cacheFiles[repoName] = []string{cacheIndex, cacheChart}
+
+	}
+
+	// Create repo remove command
+	multiRmOpts := repoRemoveOptions{
+		names:     testRepoNames,
+		repoFile:  repoFile,
+		repoCache: rootDir,
+	}
+
+	// Reset the buffer before running repo remove
+	b.Reset()
+
+	// Run repo remove command
+	if err := multiRmOpts.run(b); err != nil {
+		t.Errorf("Error removing list of repos from repositories: %q", testRepoNames)
+	}
+
+	// Check that stuff were removed
+	if !strings.Contains(b.String(), "has been removed") {
+		t.Errorf("Unexpected output: %s", b.String())
+	}
+
+	for _, repoName := range testRepoNames {
+		f, err := repo.LoadFile(repoFile)
+		if err != nil {
+			t.Error(err)
+		}
+		if f.Has(repoName) {
+			t.Errorf("%s was not successfully removed from repositories list", repoName)
+		}
+		cacheIndex := cacheFiles[repoName][0]
+		cacheChart := cacheFiles[repoName][1]
+		testCacheFiles(t, cacheIndex, cacheChart, repoName)
+	}
+}
+
+func createCacheFiles(rootDir string, repoName string) (cacheIndexFile string, cacheChartsFile string) {
+	cacheIndexFile = filepath.Join(rootDir, helmpath.CacheIndexFile(repoName))
+	mf, _ := os.Create(cacheIndexFile)
+	mf.Close()
+
+	cacheChartsFile = filepath.Join(rootDir, helmpath.CacheChartsFile(repoName))
+	mf, _ = os.Create(cacheChartsFile)
+	mf.Close()
+
+	return cacheIndexFile, cacheChartsFile
+}
+
+func testCacheFiles(t *testing.T, cacheIndexFile string, cacheChartsFile string, repoName string) {
+	if _, err := os.Stat(cacheIndexFile); err == nil {
+		t.Errorf("Error cache index file was not removed for repository %s", repoName)
+	}
+	if _, err := os.Stat(cacheChartsFile); err == nil {
+		t.Errorf("Error cache chart file was not removed for repository %s", repoName)
+	}
 }

cmd/helm/search_repo.go

@@ -298,7 +298,7 @@ func compListCharts(toComplete string, includeFiles bool) ([]string, completion.
 	var completions []string
 
 	// First check completions for repos
-	repos := compListRepos("")
+	repos := compListRepos("", nil)
 	for _, repo := range repos {
 		repoWithSlash := fmt.Sprintf("%s/", repo)
 		if strings.HasPrefix(toComplete, repoWithSlash) {