From 520416adf0723321101235780f86245c3a714c3c Mon Sep 17 00:00:00 2001
From: Matt Farina <matt@mattfarina.com>
Date: Mon, 21 Sep 2020 10:42:47 -0400
Subject: [PATCH] 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 baf5b76a957dc52a2fca84fa1328628cc78cc307)
---
 cmd/helm/repo_add.go                   | 22 +++++++++++++----
 cmd/helm/repo_add_test.go              | 34 ++++++++++++++++++++++----
 cmd/helm/testdata/output/repo-add2.txt |  1 +
 3 files changed, 47 insertions(+), 10 deletions(-)
 create mode 100644 cmd/helm/testdata/output/repo-add2.txt

diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go
index 4df4a42a9..a8f71a88e 100644
--- a/cmd/helm/repo_add.go
+++ b/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
diff --git a/cmd/helm/repo_add_test.go b/cmd/helm/repo_add_test.go
index e1d7efb8f..72612c118 100644
--- a/cmd/helm/repo_add_test.go
+++ b/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{{
-		name:   "add a repository",
-		cmd:    fmt.Sprintf("repo add test-name %s --repository-config %s --repository-cache %s", srv.URL(), repoFile, tmpdir),
-		golden: "output/repo-add.txt",
-	}}
+	tests := []cmdTestCase{
+		{
+			name:   "add a repository",
+			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)
 }
diff --git a/cmd/helm/testdata/output/repo-add2.txt b/cmd/helm/testdata/output/repo-add2.txt
new file mode 100644
index 000000000..263ffa9e4
--- /dev/null
+++ b/cmd/helm/testdata/output/repo-add2.txt
@@ -0,0 +1 @@
+"test-name" already exists with the same configuration, skipping