Fixing issue with idempotent repo add

A security issue fixed in 3.3.2 caught repos with the same name
being added a second time and produced an error. This caused an
issue for tools, such as helmfile, that will add the same name
with the same configuration multiple times.

This fix checks that the configuration on the existing and new
repo are the same. If there is no change it notes it and exists
with a 0 exit code. If there is a change the existing error is
returned (for reverse compat). If --force-update is given the
user opts in to changing the config for the name.

Closes #8771

Signed-off-by: Matt Farina <matt@mattfarina.com>
(cherry picked from commit baf5b76a95)

cmd/helm/repo_add.go

@@ -114,11 +114,6 @@ func (o *repoAddOptions) run(out io.Writer) error {
 		return err
 	}
 
-	// If the repo exists and --force-update was not specified, error out.
-	if !o.forceUpdate && f.Has(o.name) {
-		return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name)
-	}
-
 	c := repo.Entry{
 		Name:                  o.name,
 		URL:                   o.url,
@@ -130,6 +125,23 @@ func (o *repoAddOptions) run(out io.Writer) error {
 		InsecureSkipTLSverify: o.insecureSkipTLSverify,
 	}
 
+	// If the repo exists do one of two things:
+	// 1. If the configuration for the name is the same continue without error
+	// 2. When the config is different require --force-update
+	if !o.forceUpdate && f.Has(o.name) {
+		existing := f.Get(o.name)
+		if c != *existing {
+
+			// The input coming in for the name is different from what is already
+			// configured. Return an error.
+			return errors.Errorf("repository name (%s) already exists, please specify a different name", o.name)
+		}
+
+		// The add is idempotent so do nothing
+		fmt.Fprintf(out, "%q already exists with the same configuration, skipping\n", o.name)
+		return nil
+	}
+
 	r, err := repo.NewChartRepository(&c, getter.All(settings))
 	if err != nil {
 		return err

cmd/helm/repo_add_test.go

@@ -40,14 +40,38 @@ func TestRepoAddCmd(t *testing.T) {
 	}
 	defer srv.Stop()
 
+	// A second test server is setup to verify URL changing
+	srv2, err := repotest.NewTempServer("testdata/testserver/*.*")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer srv2.Stop()
+
 	tmpdir := ensure.TempDir(t)
 	repoFile := filepath.Join(tmpdir, "repositories.yaml")
 
-	tests := []cmdTestCase{{
+	tests := []cmdTestCase{
-		name:   "add a repository",
+		{
-		cmd:    fmt.Sprintf("repo add test-name %s --repository-config %s --repository-cache %s", srv.URL(), repoFile, tmpdir),
+			name:   "add a repository",
-		golden: "output/repo-add.txt",
+			cmd:    fmt.Sprintf("repo add test-name %s --repository-config %s --repository-cache %s", srv.URL(), repoFile, tmpdir),
-	}}
+			golden: "output/repo-add.txt",
+		},
+		{
+			name:   "add repository second time",
+			cmd:    fmt.Sprintf("repo add test-name %s --repository-config %s --repository-cache %s", srv.URL(), repoFile, tmpdir),
+			golden: "output/repo-add2.txt",
+		},
+		{
+			name:      "add repository different url",
+			cmd:       fmt.Sprintf("repo add test-name %s --repository-config %s --repository-cache %s", srv2.URL(), repoFile, tmpdir),
+			wantError: true,
+		},
+		{
+			name:   "add repository second time",
+			cmd:    fmt.Sprintf("repo add test-name %s --repository-config %s --repository-cache %s --force-update", srv2.URL(), repoFile, tmpdir),
+			golden: "output/repo-add.txt",
+		},
+	}
 
 	runTestCmd(t, tests)
 }

cmd/helm/testdata/output/repo-add2.txt

@@ -0,0 +1 @@
+"test-name" already exists with the same configuration, skipping