Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameterize git clone depth and fix disabling submodules #1729

Merged
merged 3 commits into from
Dec 12, 2019
Merged

Parameterize git clone depth and fix disabling submodules #1729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ddf9b0
Fix git resource submodule test descriptions
Dec 10, 2019
12df70d
Respect submodules: false in git resource
Dec 10, 2019
e810e89
Parameterize Git clone depth
Dec 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions cmd/git-init/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
imjasonh marked this conversation as resolved.
Show resolved Hide resolved
terminationMessagePath string
)

func init() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: any reason for this to be in init() and not on top of main() ; the end result should be the same :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this more intuitive than sticking it in main. Most uses of the flag package I’ve seen don’t use the …Var forms and just live inside a top-level var block. Those calls run before main and stand out visually from the code in main – to me, this felt like a more consistent and minimal refactor than adding them to main.

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)
}
Expand All @@ -57,5 +65,5 @@ func main() {
},
}

termination.WriteMessage(logger, *terminationMessagePath, output)
termination.WriteMessage(logger, terminationMessagePath, output)
}
15 changes: 10 additions & 5 deletions docs/resources.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
26 changes: 24 additions & 2 deletions pkg/apis/pipeline/v1alpha1/git_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"fmt"
"strconv"
"strings"

"github.com/tektoncd/pipeline/pkg/names"
Expand All @@ -42,6 +43,8 @@ type GitResource struct {
Revision string `json:"revision"`
Submodules bool `json:"submodules"`

Depth uint `json:"depth"`

GitImage string `json:"-"`
}

Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -106,16 +120,24 @@ 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")
}
if s.Depth != 1 {
args = append(args, "-depth", strconv.FormatUint(uint64(s.Depth), 10))
}

step := Step{
Container: corev1.Container{
Expand Down
170 changes: 139 additions & 31 deletions pkg/apis/pipeline/v1alpha1/git_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -67,9 +68,27 @@ func Test_Valid_NewGitResource(t *testing.T) {
Revision: "master",
GitImage: "override-with-git:latest",
Submodules: true,
Depth: 1,
},
}, {
desc: "With Submodules",
pipelineResource: tb.PipelineResource("git-resource", "default",
tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeGit,
tb.PipelineResourceSpecParam("URL", "git@github.com:test/test.git"),
tb.PipelineResourceSpecParam("Revision", "test"),
),
),
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: 1,
},
}, {
desc: "Without Submodules",
pipelineResource: tb.PipelineResource("git-resource", "default",
tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeGit,
tb.PipelineResourceSpecParam("URL", "git@github.com:test/test.git"),
Expand All @@ -84,13 +103,15 @@ func Test_Valid_NewGitResource(t *testing.T) {
Revision: "test",
GitImage: "override-with-git:latest",
Submodules: false,
Depth: 1,
},
}, {
desc: "Without Submodules",
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{
Expand All @@ -100,6 +121,25 @@ func Test_Valid_NewGitResource(t *testing.T) {
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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What actually happens when you clone with --depth=0? Is this a case we should catch and fail in validation? Should we reject negative values?

(It's probably fine to just pass through whatever and let git fail as it wants, but it can be useful to avoid running a container that we know will fail -- this validation also doesn't have to happen in this change)

},
}} {
t.Run(tc.desc, func(t *testing.T) {
Expand All @@ -121,13 +161,15 @@ 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{
"name": "git-resource",
"type": string(v1alpha1.PipelineResourceTypeGit),
"url": "git@github.com:test/test.git",
"revision": "master",
"depth": "16",
}

got := r.Replacements()
Expand All @@ -140,37 +182,103 @@ 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,
Depth: 1,
},
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"}},
},
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,
Depth: 1,
},
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"}},
},
}, {
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{}
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)
}
})
}
}
Loading