Merge pull request #6492 from bacongobbler/port-5678

fix(cmd): lock repository file during `helm repo add`

Gopkg.lock

@@ -538,6 +538,14 @@
   revision = "5ccd90ef52e1e632236f7326478d4faa74f99438"
   version = "v0.2.3"
 
+[[projects]]
+  digest = "1:d7b2d6ad2a9768bb5c9e3bfa4d9ceaa635ca2737cca288507986f116fa4b8da2"
+  name = "github.com/gofrs/flock"
+  packages = ["."]
+  pruneopts = "T"
+  revision = "392e7fae8f1b0bdbd67dad7237d23f618feb6dbb"
+  version = "v0.7.1"
+
 [[projects]]
   digest = "1:f5ccd717b5f093cbabc51ee2e7a5979b92f17d217f9031d6d64f337101c408e4"
   name = "github.com/gogo/protobuf"
@@ -1330,7 +1338,11 @@
   branch = "release-1.15"
   digest = "1:dffbde7aabb4d8c613f9dd53317fd5b3aa0b2722cd4d7159772be68637116793"
   name = "k8s.io/apiextensions-apiserver"
-  packages = ["pkg/features"]
+  packages = [
+    "pkg/apis/apiextensions",
+    "pkg/apis/apiextensions/v1beta1",
+    "pkg/features",
+  ]
   pruneopts = "T"
   revision = "23f08c7096c0273b53178de488b95473d5cd3808"
 
@@ -1831,6 +1843,7 @@
     "github.com/docker/go-units",
     "github.com/evanphx/json-patch",
     "github.com/gobwas/glob",
+    "github.com/gofrs/flock",
     "github.com/gosuri/uitable",
     "github.com/mattn/go-shellwords",
     "github.com/opencontainers/go-digest",
@@ -1858,6 +1871,7 @@
     "k8s.io/api/batch/v1",
     "k8s.io/api/core/v1",
     "k8s.io/api/extensions/v1beta1",
+    "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1",
     "k8s.io/apimachinery/pkg/api/errors",
     "k8s.io/apimachinery/pkg/api/meta",
     "k8s.io/apimachinery/pkg/apis/meta/v1",

Gopkg.toml

@@ -74,6 +74,10 @@
   name = "sigs.k8s.io/yaml"
   version = "1.1.0"
 
+[[constraint]]
+  name = "github.com/gofrs/flock"
+  version = "0.7.1"
+
 [[override]]
   name = "sigs.k8s.io/kustomize"
   version = "2.0.3"

cmd/helm/repo_add.go

@@ -17,11 +17,14 @@ limitations under the License.
 package main
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
+	"time"
 
+	"github.com/gofrs/flock"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"gopkg.in/yaml.v2"
@@ -75,6 +78,18 @@ func newRepoAddCmd(out io.Writer) *cobra.Command {
 }
 
 func (o *repoAddOptions) run(out io.Writer) error {
+	// Lock the repository file for concurrent goroutines or processes synchronization
+	fileLock := flock.New(o.repoFile)
+	lockCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	locked, err := fileLock.TryLockContext(lockCtx, time.Second)
+	if err == nil && locked {
+		defer fileLock.Unlock()
+	}
+	if err != nil {
+		return err
+	}
+
 	b, err := ioutil.ReadFile(o.repoFile)
 	if err != nil && !os.IsNotExist(err) {
 		return err

cmd/helm/repo_add_test.go

@@ -20,8 +20,11 @@ import (
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
+	"sync"
 	"testing"
 
+	"gopkg.in/yaml.v2"
+
 	"helm.sh/helm/internal/test/ensure"
 	"helm.sh/helm/pkg/repo"
 	"helm.sh/helm/pkg/repo/repotest"
@@ -86,3 +89,51 @@ func TestRepoAdd(t *testing.T) {
 		t.Errorf("Duplicate repository name was added")
 	}
 }
+
+func TestRepoAddConcurrentGoRoutines(t *testing.T) {
+	const testName = "test-name"
+
+	ts, err := repotest.NewTempServer("testdata/testserver/*.*")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ts.Stop()
+
+	repoFile := filepath.Join(ensure.TempDir(t), "repositories.yaml")
+
+	var wg sync.WaitGroup
+	wg.Add(3)
+	for i := 0; i < 3; i++ {
+		go func(name string) {
+			defer wg.Done()
+			o := &repoAddOptions{
+				name:     name,
+				url:      ts.URL(),
+				noUpdate: true,
+				repoFile: repoFile,
+			}
+			if err := o.run(ioutil.Discard); err != nil {
+				t.Error(err)
+			}
+		}(fmt.Sprintf("%s-%d", testName, i))
+	}
+	wg.Wait()
+
+	b, err := ioutil.ReadFile(repoFile)
+	if err != nil {
+		t.Error(err)
+	}
+
+	var f repo.File
+	if err := yaml.Unmarshal(b, &f); err != nil {
+		t.Error(err)
+	}
+
+	var name string
+	for i := 0; i < 3; i++ {
+		name = fmt.Sprintf("%s-%d", testName, i)
+		if !f.Has(name) {
+			t.Errorf("%s was not successfully inserted into %s: %s", name, repoFile, f.Repositories[0])
+		}
+	}
+}