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>
pull/8777/head
Matt Farina 4 years ago
parent 2ebe95a6ff
commit baf5b76a95

@ -116,11 +116,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)
}
if o.username != "" && o.password == "" {
fd := int(os.Stdin.Fd())
fmt.Fprint(out, "Password: ")
@ -143,6 +138,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

@ -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.NewTempServerWithCleanup(t, "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),
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)
}

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