From 6bbc8295932081d50f26756649f99f1dc2738de8 Mon Sep 17 00:00:00 2001 From: jregan Date: Tue, 8 Dec 2020 12:58:35 -0800 Subject: [PATCH] Add a loader test. --- api/internal/git/cloner.go | 83 +++++------------------------------ api/internal/git/gitrunner.go | 65 +++++++++++++++++++++++++++ api/krusty/remoteload_test.go | 40 +++++++++++++++++ api/loader/getter.go | 26 ++++++++++- api/types/errtimeout.go | 36 +++++++++++++++ 5 files changed, 177 insertions(+), 73 deletions(-) create mode 100644 api/internal/git/gitrunner.go create mode 100644 api/krusty/remoteload_test.go create mode 100644 api/types/errtimeout.go diff --git a/api/internal/git/cloner.go b/api/internal/git/cloner.go index 7323f20bc..e3daccd88 100644 --- a/api/internal/git/cloner.go +++ b/api/internal/git/cloner.go @@ -4,10 +4,6 @@ package git import ( - "log" - "os/exec" - - "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/filesys" ) @@ -18,84 +14,29 @@ type Cloner func(repoSpec *RepoSpec) error // to say, some remote API, to obtain a local clone of // a remote repo. func ClonerUsingGitExec(repoSpec *RepoSpec) error { - gitProgram, err := exec.LookPath("git") - if err != nil { - return errors.Wrap(err, "no 'git' program on path") - } - repoSpec.Dir, err = filesys.NewTmpConfirmedDir() + r, err := newCmdRunner() if err != nil { return err } - - cmd := exec.Command( - gitProgram, - "init", - repoSpec.Dir.String()) - out, err := cmd.CombinedOutput() - if err != nil { - log.Printf("Error initializing git repo: %s", out) - return errors.Wrapf( - err, - "trouble initializing git repo in %s", - repoSpec.Dir.String()) + repoSpec.Dir = r.dir + if err = r.run("init"); err != nil { + return err } - - cmd = exec.Command( - gitProgram, - "remote", - "add", - "origin", - repoSpec.CloneSpec()) - cmd.Dir = repoSpec.Dir.String() - out, err = cmd.CombinedOutput() - if err != nil { - log.Printf("Error adding remote: %s", out) - return errors.Wrapf(err, "trouble adding remote %s", repoSpec.CloneSpec()) + if err = r.run( + "remote", "add", "origin", repoSpec.CloneSpec()); err != nil { + return err } - ref := "HEAD" if repoSpec.Ref != "" { ref = repoSpec.Ref } - - cmd = exec.Command( - gitProgram, - "fetch", - "--depth=1", - "origin", - ref) - cmd.Dir = repoSpec.Dir.String() - out, err = cmd.CombinedOutput() - if err != nil { - log.Printf("Error fetching ref: %s", out) - return errors.Wrapf(err, "trouble fetching %s", ref) + if err = r.run("fetch", "--depth=1", "origin", ref); err != nil { + return err } - - cmd = exec.Command( - gitProgram, - "checkout", - "FETCH_HEAD") - cmd.Dir = repoSpec.Dir.String() - out, err = cmd.CombinedOutput() - if err != nil { - log.Printf("Error checking out ref: %s", out) - return errors.Wrapf(err, "trouble checking out %s", ref) + if err = r.run("checkout", "FETCH_HEAD"); err != nil { + return err } - - cmd = exec.Command( - gitProgram, - "submodule", - "update", - "--init", - "--recursive") - cmd.Dir = repoSpec.Dir.String() - out, err = cmd.CombinedOutput() - if err != nil { - log.Printf("Error fetching submodules: %s", out) - return errors.Wrapf(err, "trouble fetching submodules for %s", repoSpec.CloneSpec()) - } - - return nil + return r.run("submodule", "update", "--init", "--recursive") } // DoNothingCloner returns a cloner that only sets diff --git a/api/internal/git/gitrunner.go b/api/internal/git/gitrunner.go new file mode 100644 index 000000000..edf3dfa0c --- /dev/null +++ b/api/internal/git/gitrunner.go @@ -0,0 +1,65 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package git + +import ( + "os/exec" + "time" + + "github.com/pkg/errors" + "sigs.k8s.io/kustomize/api/filesys" + "sigs.k8s.io/kustomize/api/types" +) + +// Arbitrary, but non-infinite, timeout for running commands. +const defaultDuration = 27 * time.Second + +// gitRunner runs the external git binary. +type gitRunner struct { + gitProgram string + duration time.Duration + dir filesys.ConfirmedDir +} + +// newCmdRunner returns a gitRunner if it can find the binary. +// It also creats a temp directory for cloning repos. +func newCmdRunner() (*gitRunner, error) { + gitProgram, err := exec.LookPath("git") + if err != nil { + return nil, errors.Wrap(err, "no 'git' program on path") + } + dir, err := filesys.NewTmpConfirmedDir() + if err != nil { + return nil, err + } + return &gitRunner{ + gitProgram: gitProgram, + duration: defaultDuration, + dir: dir, + }, nil +} + +// run a command with a timeout. +func (r gitRunner) run(args ...string) (err error) { + ch := make(chan bool, 1) + defer close(ch) + //nolint: gosec + cmd := exec.Command(r.gitProgram, args...) + cmd.Dir = r.dir.String() + timer := time.NewTimer(r.duration) + defer timer.Stop() + go func() { + _, err = cmd.CombinedOutput() + ch <- true + }() + select { + case <-ch: + if err != nil { + return errors.Wrapf(err, "git cmd = '%s'", cmd.String()) + } + return nil + case <-timer.C: + return types.NewErrTimeOut(r.duration, cmd.String()) + } +} diff --git a/api/krusty/remoteload_test.go b/api/krusty/remoteload_test.go new file mode 100644 index 000000000..37caae9a7 --- /dev/null +++ b/api/krusty/remoteload_test.go @@ -0,0 +1,40 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package krusty_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/api/filesys" + "sigs.k8s.io/kustomize/api/krusty" + "sigs.k8s.io/kustomize/api/types" +) + +func TestRemoteLoad(t *testing.T) { + fSys := filesys.MakeFsOnDisk() + b := krusty.MakeKustomizer(fSys, krusty.MakeDefaultOptions()) + m, err := b.Run( + "github.com/kubernetes-sigs/kustomize/examples/multibases/dev/?ref=v1.0.6") + if types.IsErrTimeout(err) { + // Don't fail on timeouts. + t.SkipNow() + } + if !assert.NoError(t, err) { + t.FailNow() + } + yml, err := m.AsYaml() + assert.NoError(t, err) + assert.Equal(t, `apiVersion: v1 +kind: Pod +metadata: + labels: + app: myapp + name: dev-myapp-pod +spec: + containers: + - image: nginx:1.7.9 + name: nginx +`, string(yml)) +} diff --git a/api/loader/getter.go b/api/loader/getter.go index 566ada052..fd45b512d 100644 --- a/api/loader/getter.go +++ b/api/loader/getter.go @@ -7,11 +7,13 @@ import ( "context" "log" "os" + "time" "github.com/yujunz/go-getter" "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/git" + "sigs.k8s.io/kustomize/api/types" ) type remoteTargetSpec struct { @@ -28,7 +30,12 @@ type remoteTargetSpec struct { // Getter is a function that can gets resource type remoteTargetGetter func(rs *remoteTargetSpec) error -func newLoaderAtGetter(raw string, fSys filesys.FileSystem, referrer *fileLoader, cloner git.Cloner, getter remoteTargetGetter) (ifc.Loader, error) { +func newLoaderAtGetter( + raw string, + fSys filesys.FileSystem, + referrer *fileLoader, + cloner git.Cloner, + getter remoteTargetGetter) (ifc.Loader, error) { rs := &remoteTargetSpec{ Raw: raw, } @@ -86,7 +93,22 @@ func getRemoteTarget(rs *remoteTargetSpec) error { }, Options: opts, } - return client.Get() + + ch := make(chan bool, 1) + defer close(ch) + d := 21 * time.Second // arbitrary + timer := time.NewTimer(d) + defer timer.Stop() + go func() { + err = client.Get() + ch <- true + }() + select { + case <-ch: + case <-timer.C: + err = types.NewErrTimeOut(d, "go-getter client.Get") + } + return err } func getNothing(rs *remoteTargetSpec) error { diff --git a/api/types/errtimeout.go b/api/types/errtimeout.go new file mode 100644 index 000000000..29fe31834 --- /dev/null +++ b/api/types/errtimeout.go @@ -0,0 +1,36 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "fmt" + "time" + + "github.com/pkg/errors" +) + +type errTimeOut struct { + duration time.Duration + cmd string +} + +func NewErrTimeOut(d time.Duration, c string) errTimeOut { + return errTimeOut{duration: d, cmd: c} +} + +func (e errTimeOut) Error() string { + return fmt.Sprintf("hit %s timeout running '%s'", e.duration, e.cmd) +} + +func IsErrTimeout(err error) bool { + if err == nil { + return false + } + _, ok := err.(errTimeOut) + if ok { + return true + } + _, ok = errors.Cause(err).(errTimeOut) + return ok +}