From 8ddf9b0ac43313d7c410b59b71184a0c8e17f01f Mon Sep 17 00:00:00 2001 From: Lyra Naeseth Date: Tue, 10 Dec 2019 10:52:35 -0800 Subject: [PATCH 1/3] Fix git resource submodule test descriptions --- pkg/apis/pipeline/v1alpha1/git_resource_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/apis/pipeline/v1alpha1/git_resource_test.go b/pkg/apis/pipeline/v1alpha1/git_resource_test.go index 50942588464..022d5d629f8 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource_test.go @@ -74,7 +74,6 @@ func Test_Valid_NewGitResource(t *testing.T) { tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeGit, tb.PipelineResourceSpecParam("URL", "git@github.com:test/test.git"), tb.PipelineResourceSpecParam("Revision", "test"), - tb.PipelineResourceSpecParam("Submodules", "false"), ), ), want: &v1alpha1.GitResource{ @@ -83,7 +82,7 @@ func Test_Valid_NewGitResource(t *testing.T) { URL: "git@github.com:test/test.git", Revision: "test", GitImage: "override-with-git:latest", - Submodules: false, + Submodules: true, }, }, { desc: "Without Submodules", @@ -91,6 +90,7 @@ func Test_Valid_NewGitResource(t *testing.T) { tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeGit, tb.PipelineResourceSpecParam("URL", "git@github.com:test/test.git"), tb.PipelineResourceSpecParam("Revision", "test"), + tb.PipelineResourceSpecParam("Submodules", "false"), ), ), want: &v1alpha1.GitResource{ @@ -99,7 +99,7 @@ func Test_Valid_NewGitResource(t *testing.T) { URL: "git@github.com:test/test.git", Revision: "test", GitImage: "override-with-git:latest", - Submodules: true, + Submodules: false, }, }} { t.Run(tc.desc, func(t *testing.T) { From 12df70d40490eeea586dd9e6cf3b8a12ade11c4f Mon Sep 17 00:00:00 2001 From: Lyra Naeseth Date: Tue, 10 Dec 2019 11:26:50 -0800 Subject: [PATCH 2/3] Respect submodules: false in git resource Fixes #1727. In #1531, the submodules parameter was added, but it has no influence on the generated git-init invocation. --- pkg/apis/pipeline/v1alpha1/git_resource.go | 9 +- .../pipeline/v1alpha1/git_resource_test.go | 96 +++++++++++++------ 2 files changed, 73 insertions(+), 32 deletions(-) diff --git a/pkg/apis/pipeline/v1alpha1/git_resource.go b/pkg/apis/pipeline/v1alpha1/git_resource.go index 2f47dd2076b..ecc08fc1365 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource.go @@ -106,16 +106,21 @@ func (s *GitResource) Replacements() map[string]string { "type": string(s.Type), "url": s.URL, "revision": s.Revision, + "depth": strconv.FormatUint(uint64(s.Depth), 10), } } // GetInputTaskModifier returns the TaskModifier to be used when this resource is an input. func (s *GitResource) GetInputTaskModifier(_ *TaskSpec, path string) (TaskModifier, error) { - args := []string{"-url", s.URL, + args := []string{ + "-url", s.URL, "-revision", s.Revision, + "-path", path, } - args = append(args, []string{"-path", path}...) + if !s.Submodules { + args = append(args, "-submodules", "false") + } step := Step{ Container: corev1.Container{ diff --git a/pkg/apis/pipeline/v1alpha1/git_resource_test.go b/pkg/apis/pipeline/v1alpha1/git_resource_test.go index 022d5d629f8..be9c21b941c 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource_test.go @@ -140,37 +140,73 @@ func Test_GitResource_Replacements(t *testing.T) { func Test_GitResource_GetDownloadTaskModifier(t *testing.T) { names.TestingSeed() - r := &v1alpha1.GitResource{ - Name: "git-resource", - Type: v1alpha1.PipelineResourceTypeGit, - URL: "git@github.com:test/test.git", - Revision: "master", - GitImage: "override-with-git:latest", - } - - ts := v1alpha1.TaskSpec{} - modifier, err := r.GetInputTaskModifier(&ts, "/test/test") - if err != nil { - t.Fatalf("Unexpected error getting GetDownloadTaskModifier: %s", err) - } - - want := []v1alpha1.Step{{Container: corev1.Container{ - Name: "git-source-git-resource-9l9zj", - Image: "override-with-git:latest", - Command: []string{"/ko-app/git-init"}, - Args: []string{ - "-url", - "git@github.com:test/test.git", - "-revision", - "master", - "-path", - "/test/test", + for _, tc := range []struct { + desc string + gitResource *v1alpha1.GitResource + want corev1.Container + }{{ + desc: "With basic values", + gitResource: &v1alpha1.GitResource{ + Name: "git-resource", + Type: v1alpha1.PipelineResourceTypeGit, + URL: "git@github.com:test/test.git", + Revision: "master", + GitImage: "override-with-git:latest", + Submodules: true, }, - WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}}, - }}} + want: corev1.Container{ + Name: "git-source-git-resource-9l9zj", + Image: "override-with-git:latest", + Command: []string{"/ko-app/git-init"}, + Args: []string{ + "-url", + "git@github.com:test/test.git", + "-revision", + "master", + "-path", + "/test/test", + }, + WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}}, + }, + }, { + desc: "Without submodules", + gitResource: &v1alpha1.GitResource{ + Name: "git-resource", + Type: v1alpha1.PipelineResourceTypeGit, + URL: "git@github.com:test/test.git", + Revision: "master", + GitImage: "override-with-git:latest", + Submodules: false, + }, + want: corev1.Container{ + Name: "git-source-git-resource-mz4c7", + Image: "override-with-git:latest", + Command: []string{"/ko-app/git-init"}, + Args: []string{ + "-url", + "git@github.com:test/test.git", + "-revision", + "master", + "-path", + "/test/test", + "-submodules", + "false", + }, + WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}}, + }, + }} { + t.Run(tc.desc, func(t *testing.T) { + ts := v1alpha1.TaskSpec{} + modifier, err := tc.gitResource.GetInputTaskModifier(&ts, "/test/test") + if err != nil { + t.Fatalf("Unexpected error getting GetDownloadTaskModifier: %s", err) + } - if diff := cmp.Diff(want, modifier.GetStepsToPrepend()); diff != "" { - t.Errorf("Mismatch of GitResource DownloadContainerSpec: %s", diff) + if diff := cmp.Diff([]v1alpha1.Step{{Container: tc.want}}, modifier.GetStepsToPrepend()); diff != "" { + t.Errorf("Mismatch of GitResource DownloadContainerSpec: %s", diff) + } + }) } } From e810e89b7c404cd6bcaaf4583a5a70b005f35625 Mon Sep 17 00:00:00 2001 From: Lyra Naeseth Date: Tue, 10 Dec 2019 13:29:45 -0800 Subject: [PATCH 3/3] Parameterize Git clone depth This lets users control the shallow clone depth, or disable shallow clones entirely by setting depth to 0. Some build processes expect to be able to access Git history. Increasing depth also helps users re-run builds of commits that may no longer be at the head of a branch without falling back to the full `git pull` code path. Closes #1450. --- cmd/git-init/main.go | 28 +++++--- docs/resources.md | 15 ++-- pkg/apis/pipeline/v1alpha1/git_resource.go | 17 +++++ .../pipeline/v1alpha1/git_resource_test.go | 72 +++++++++++++++++++ pkg/git/git.go | 37 +++++++--- 5 files changed, 143 insertions(+), 26 deletions(-) diff --git a/cmd/git-init/main.go b/cmd/git-init/main.go index 0408d0de14f..5957a9c98ae 100644 --- a/cmd/git-init/main.go +++ b/cmd/git-init/main.go @@ -25,28 +25,36 @@ import ( ) var ( - url = flag.String("url", "", "The url of the Git repository to initialize.") - revision = flag.String("revision", "", "The Git revision to make the repository HEAD") - path = flag.String("path", "", "Path of directory under which git repository will be copied") - terminationMessagePath = flag.String("terminationMessagePath", "/dev/termination-log", "Location of file containing termination message") - submodules = flag.Bool("submodules", true, "Initialize and fetch git submodules") + fetchSpec git.FetchSpec + submodules bool + terminationMessagePath string ) +func init() { + flag.StringVar(&fetchSpec.URL, "url", "", "Git origin URL to fetch") + flag.StringVar(&fetchSpec.Revision, "revision", "", "The Git revision to make the repository HEAD") + flag.StringVar(&fetchSpec.Path, "path", "", "Path of directory under which Git repository will be copied") + flag.BoolVar(&submodules, "submodules", true, "Initialize and fetch Git submodules") + flag.UintVar(&fetchSpec.Depth, "depth", 1, "Perform a shallow clone to this depth") + flag.StringVar(&terminationMessagePath, "terminationMessagePath", "/dev/termination-log", "Location of file containing termination message") +} + func main() { flag.Parse() + logger, _ := logging.NewLogger("", "git-init") defer logger.Sync() - if err := git.Fetch(logger, *revision, *path, *url); err != nil { + if err := git.Fetch(logger, fetchSpec); err != nil { logger.Fatalf("Error fetching git repository: %s", err) } - if *submodules { - if err := git.SubmoduleFetch(logger, *path); err != nil { + if submodules { + if err := git.SubmoduleFetch(logger, fetchSpec.Path); err != nil { logger.Fatalf("Error initalizing or fetching the git submodules") } } - commit, err := git.Commit(logger, *revision, *path) + commit, err := git.Commit(logger, fetchSpec.Revision, fetchSpec.Path) if err != nil { logger.Fatalf("Error parsing commit of git repository: %s", err) } @@ -57,5 +65,5 @@ func main() { }, } - termination.WriteMessage(logger, *terminationMessagePath, output) + termination.WriteMessage(logger, terminationMessagePath, output) } diff --git a/docs/resources.md b/docs/resources.md index 64146409935..077d5541611 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -248,14 +248,19 @@ Params that can be added are the following: 1. `url`: represents the location of the git repository, you can use this to change the repo, e.g. [to use a fork](#using-a-fork) -1. `revision`: Git - [revision](https://git-scm.com/docs/gitrevisions#_specifying_revisions) - (branch, tag, commit SHA or ref) to clone. You can use this to control what - commit [or branch](#using-a-branch) is used. _If no revision is specified, - the resource will default to `latest` from `master`._ +1. `revision`: Git [revision][git-rev] (branch, tag, commit SHA or ref) to + clone. You can use this to control what commit [or branch](#using-a-branch) + is used. _If no revision is specified, the resource will default to `latest` + from `master`._ 1. `submodules`: defines if the resource should initialize and fetch the submodules, value is either `true` or `false`. _If not specified, this will default to true_ +1. `depth`: performs a [shallow clone][git-depth] where only the most recent + commit(s) will be fetched. If set to `'0'`, all commits will be fetched. + _If not specified, the default depth is 1._ + +[git-rev]: https://git-scm.com/docs/gitrevisions#_specifying_revisions +[git-depth]: https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---depthltdepthgt When used as an input, the Git resource includes the exact commit fetched in the `resourceResults` section of the `taskRun`'s status object: diff --git a/pkg/apis/pipeline/v1alpha1/git_resource.go b/pkg/apis/pipeline/v1alpha1/git_resource.go index ecc08fc1365..0a5bf4bd526 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "fmt" + "strconv" "strings" "github.com/tektoncd/pipeline/pkg/names" @@ -42,6 +43,8 @@ type GitResource struct { Revision string `json:"revision"` Submodules bool `json:"submodules"` + Depth uint `json:"depth"` + GitImage string `json:"-"` } @@ -55,6 +58,7 @@ func NewGitResource(gitImage string, r *PipelineResource) (*GitResource, error) Type: r.Spec.Type, GitImage: gitImage, Submodules: true, + Depth: 1, } for _, param := range r.Spec.Params { switch { @@ -64,6 +68,8 @@ func NewGitResource(gitImage string, r *PipelineResource) (*GitResource, error) gitResource.Revision = param.Value case strings.EqualFold(param.Name, "Submodules"): gitResource.Submodules = toBool(param.Value, true) + case strings.EqualFold(param.Name, "Depth"): + gitResource.Depth = toUint(param.Value, 1) } } // default revision to master if nothing is provided @@ -84,6 +90,14 @@ func toBool(s string, d bool) bool { } } +func toUint(s string, d uint) uint { + v, err := strconv.ParseUint(s, 10, 32) + if err != nil { + return d + } + return uint(v) +} + // GetName returns the name of the resource func (s GitResource) GetName() string { return s.Name @@ -121,6 +135,9 @@ func (s *GitResource) GetInputTaskModifier(_ *TaskSpec, path string) (TaskModifi if !s.Submodules { args = append(args, "-submodules", "false") } + if s.Depth != 1 { + args = append(args, "-depth", strconv.FormatUint(uint64(s.Depth), 10)) + } step := Step{ Container: corev1.Container{ diff --git a/pkg/apis/pipeline/v1alpha1/git_resource_test.go b/pkg/apis/pipeline/v1alpha1/git_resource_test.go index be9c21b941c..03ed36156b5 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource_test.go @@ -52,6 +52,7 @@ func Test_Valid_NewGitResource(t *testing.T) { Revision: "test", GitImage: "override-with-git:latest", Submodules: true, + Depth: 1, }, }, { desc: "Without Revision", @@ -67,6 +68,7 @@ func Test_Valid_NewGitResource(t *testing.T) { Revision: "master", GitImage: "override-with-git:latest", Submodules: true, + Depth: 1, }, }, { desc: "With Submodules", @@ -83,6 +85,7 @@ func Test_Valid_NewGitResource(t *testing.T) { Revision: "test", GitImage: "override-with-git:latest", Submodules: true, + Depth: 1, }, }, { desc: "Without Submodules", @@ -100,6 +103,43 @@ func Test_Valid_NewGitResource(t *testing.T) { Revision: "test", GitImage: "override-with-git:latest", Submodules: false, + Depth: 1, + }, + }, { + desc: "With positive depth", + pipelineResource: tb.PipelineResource("git-resource", "default", + tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeGit, + tb.PipelineResourceSpecParam("URL", "git@github.com:test/test.git"), + tb.PipelineResourceSpecParam("Revision", "test"), + tb.PipelineResourceSpecParam("Depth", "8"), + ), + ), + want: &v1alpha1.GitResource{ + Name: "git-resource", + Type: v1alpha1.PipelineResourceTypeGit, + URL: "git@github.com:test/test.git", + Revision: "test", + GitImage: "override-with-git:latest", + Submodules: true, + Depth: 8, + }, + }, { + desc: "With zero depth", + pipelineResource: tb.PipelineResource("git-resource", "default", + tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeGit, + tb.PipelineResourceSpecParam("URL", "git@github.com:test/test.git"), + tb.PipelineResourceSpecParam("Revision", "test"), + tb.PipelineResourceSpecParam("Depth", "0"), + ), + ), + want: &v1alpha1.GitResource{ + Name: "git-resource", + Type: v1alpha1.PipelineResourceTypeGit, + URL: "git@github.com:test/test.git", + Revision: "test", + GitImage: "override-with-git:latest", + Submodules: true, + Depth: 0, }, }} { t.Run(tc.desc, func(t *testing.T) { @@ -121,6 +161,7 @@ func Test_GitResource_Replacements(t *testing.T) { Type: v1alpha1.PipelineResourceTypeGit, URL: "git@github.com:test/test.git", Revision: "master", + Depth: 16, } want := map[string]string{ @@ -128,6 +169,7 @@ func Test_GitResource_Replacements(t *testing.T) { "type": string(v1alpha1.PipelineResourceTypeGit), "url": "git@github.com:test/test.git", "revision": "master", + "depth": "16", } got := r.Replacements() @@ -153,6 +195,7 @@ func Test_GitResource_GetDownloadTaskModifier(t *testing.T) { Revision: "master", GitImage: "override-with-git:latest", Submodules: true, + Depth: 1, }, want: corev1.Container{ Name: "git-source-git-resource-9l9zj", @@ -178,6 +221,7 @@ func Test_GitResource_GetDownloadTaskModifier(t *testing.T) { Revision: "master", GitImage: "override-with-git:latest", Submodules: false, + Depth: 1, }, want: corev1.Container{ Name: "git-source-git-resource-mz4c7", @@ -196,6 +240,34 @@ func Test_GitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}}, }, + }, { + desc: "With more depth", + gitResource: &v1alpha1.GitResource{ + Name: "git-resource", + Type: v1alpha1.PipelineResourceTypeGit, + URL: "git@github.com:test/test.git", + Revision: "master", + GitImage: "override-with-git:latest", + Submodules: true, + Depth: 8, + }, + want: corev1.Container{ + Name: "git-source-git-resource-mssqb", + Image: "override-with-git:latest", + Command: []string{"/ko-app/git-init"}, + Args: []string{ + "-url", + "git@github.com:test/test.git", + "-revision", + "master", + "-path", + "/test/test", + "-depth", + "8", + }, + WorkingDir: "/workspace", + Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}}, + }, }} { t.Run(tc.desc, func(t *testing.T) { ts := v1alpha1.TaskSpec{} diff --git a/pkg/git/git.go b/pkg/git/git.go index 90029fbcd6f..c8af78670d0 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -43,42 +43,57 @@ func run(logger *zap.SugaredLogger, dir string, args ...string) (string, error) return output.String(), nil } +// FetchSpec describes how to initialize and fetch from a Git repository. +type FetchSpec struct { + URL string + Revision string + Path string + Depth uint +} + // Fetch fetches the specified git repository at the revision into path. -func Fetch(logger *zap.SugaredLogger, revision, path, url string) error { +func Fetch(logger *zap.SugaredLogger, spec FetchSpec) error { if err := ensureHomeEnv(logger); err != nil { return err } - if revision == "" { - revision = "master" + if spec.Revision == "" { + spec.Revision = "master" } - if path != "" { - if _, err := run(logger, "", "init", path); err != nil { + if spec.Path != "" { + if _, err := run(logger, "", "init", spec.Path); err != nil { return err } - if err := os.Chdir(path); err != nil { - return fmt.Errorf("failed to change directory with path %s; err: %w", path, err) + if err := os.Chdir(spec.Path); err != nil { + return fmt.Errorf("failed to change directory with path %s; err: %w", spec.Path, err) } } else if _, err := run(logger, "", "init"); err != nil { return err } - trimmedURL := strings.TrimSpace(url) + trimmedURL := strings.TrimSpace(spec.URL) if _, err := run(logger, "", "remote", "add", "origin", trimmedURL); err != nil { return err } - if _, err := run(logger, "", "fetch", "--depth=1", "--recurse-submodules=yes", "origin", revision); err != nil { + + fetchArgs := []string{"fetch", "--recurse-submodules=yes"} + if spec.Depth > 0 { + fetchArgs = append(fetchArgs, fmt.Sprintf("--depth=%d", spec.Depth)) + } + fetchArgs = append(fetchArgs, "origin", spec.Revision) + + if _, err := run(logger, "", fetchArgs...); err != nil { // Fetch can fail if an old commitid was used so try git pull, performing regardless of error // as no guarantee that the same error is returned by all git servers gitlab, github etc... if _, err := run(logger, "", "pull", "--recurse-submodules=yes", "origin"); err != nil { logger.Warnf("Failed to pull origin : %s", err) } - if _, err := run(logger, "", "checkout", revision); err != nil { + if _, err := run(logger, "", "checkout", spec.Revision); err != nil { return err } } else if _, err := run(logger, "", "reset", "--hard", "FETCH_HEAD"); err != nil { return err } - logger.Infof("Successfully cloned %s @ %s in path %s", trimmedURL, revision, path) + logger.Infof("Successfully cloned %s @ %s in path %s", trimmedURL, spec.Revision, spec.Path) return nil }