From 3fa07d572fea612791c8a8f3a93a16cb3a9ca631 Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Tue, 24 Sep 2019 11:22:48 -0700 Subject: [PATCH] fix(cmd): lock repository file during `helm repo add` Signed-off-by: Matthew Fisher --- Gopkg.lock | 16 +++++++++++- Gopkg.toml | 4 +++ cmd/helm/repo_add.go | 15 ++++++++++++ cmd/helm/repo_add_test.go | 51 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/Gopkg.lock b/Gopkg.lock index efec4475f..56189fff4 100644 --- a/Gopkg.lock +++ b/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", diff --git a/Gopkg.toml b/Gopkg.toml index 4620a9150..117f11949 100644 --- a/Gopkg.toml +++ b/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" diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index 53432264f..1bbffe202 100644 --- a/cmd/helm/repo_add.go +++ b/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 diff --git a/cmd/helm/repo_add_test.go b/cmd/helm/repo_add_test.go index 217a8124c..c0268eef6 100644 --- a/cmd/helm/repo_add_test.go +++ b/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]) + } + } +}