Skip to content

Commit 8bc424d

Browse files
anish-dangi-wdctekton-robot
anish-dangi-wdc
authored and
tekton-robot
committed
Introduce optional "refspec" to Git PipelineResource, Refactor
The git pipeline resource does not allow for any use case beyond a basic ref fetch, and is especially problematic for servers that don't support direct SHA fetches, which means that the user cannot check out specific commit hashes not pointed to by a named ref. The user is also not able to check out other refs alongside (example, the tag history). Current implementation also has a few corner cases, such as checking out/reset --hard on the master branch after git-init, running git reset --hard when checkout fails and so on. This leads to inconsistent behavior (see #2282). The end result is that several users might end up creating their own custom containers for fetching and checking out what they need. This change makes the git resource much more extensible, via an optional refspec parameter. This change should be fully backward compatible w.r.t to revision parameter EXCEPT the ability to fetch git commit short hashes (according to the e2e tests) - Adds ability to checkout older commit hashes on a particular ref chain (when fetch via commit SHA is not enabled on the server) - Adds ability to fetch several other refs (for example, refs/tags) alongside the main revision - Provides resolution for #2282 - If the user needs to checkout a branch, they must specify the appropriate refspec and revision as the ref of the branch. There was no way to do this in the original implementation - Fixes bug reported in #2282 by ensuring the revision is always checked out on a detached HEAD when specified w/o refspec - Fixes bug - depth parameter is not passed to git submodule update, leading to unnecessary data transfer - Fixes bug --recurse-submodules is appended to the fetch command even when the user specified submodule: "false", leading to unnecessary data transfer - Fixes bug #1843 - git init ssh symlink is failing Addresses #2282, #1843
1 parent c30d3aa commit 8bc424d

File tree

8 files changed

+344
-61
lines changed

8 files changed

+344
-61
lines changed

‎cmd/git-init/main.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@ import (
2828

2929
var (
3030
fetchSpec git.FetchSpec
31-
submodules bool
3231
terminationMessagePath string
3332
)
3433

3534
func init() {
3635
flag.StringVar(&fetchSpec.URL, "url", "", "Git origin URL to fetch")
3736
flag.StringVar(&fetchSpec.Revision, "revision", "", "The Git revision to make the repository HEAD")
37+
flag.StringVar(&fetchSpec.Refspec, "refspec", "", "The Git refspec to fetch the revision from (optional)")
3838
flag.StringVar(&fetchSpec.Path, "path", "", "Path of directory under which Git repository will be copied")
3939
flag.BoolVar(&fetchSpec.SSLVerify, "sslVerify", true, "Enable/Disable SSL verification in the git config")
40-
flag.BoolVar(&submodules, "submodules", true, "Initialize and fetch Git submodules")
40+
flag.BoolVar(&fetchSpec.Submodules, "submodules", true, "Initialize and fetch Git submodules")
4141
flag.UintVar(&fetchSpec.Depth, "depth", 1, "Perform a shallow clone to this depth")
4242
flag.StringVar(&terminationMessagePath, "terminationMessagePath", "/tekton/termination", "Location of file containing termination message")
4343
}
@@ -53,15 +53,10 @@ func main() {
5353
if err := git.Fetch(logger, fetchSpec); err != nil {
5454
logger.Fatalf("Error fetching git repository: %s", err)
5555
}
56-
if submodules {
57-
if err := git.SubmoduleFetch(logger, fetchSpec.Path); err != nil {
58-
logger.Fatalf("Error initializing or fetching the git submodules")
59-
}
60-
}
6156

62-
commit, err := git.Commit(logger, fetchSpec.Revision, fetchSpec.Path)
57+
commit, err := git.ShowCommit(logger, "HEAD", fetchSpec.Path)
6358
if err != nil {
64-
logger.Fatalf("Error parsing commit of git repository: %s", err)
59+
logger.Fatalf("Error parsing revision %s of git repository: %s", fetchSpec.Revision, err)
6560
}
6661
resourceName := os.Getenv("TEKTON_RESOURCE_NAME")
6762
output := []v1alpha1.PipelineResourceResult{

‎docs/resources.md‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,19 +313,53 @@ Params that can be added are the following:
313313
change the repo, e.g. [to use a fork](#using-a-fork)
314314
1. `revision`: Git [revision][git-rev] (branch, tag, commit SHA or ref) to
315315
clone. You can use this to control what commit [or branch](#using-a-branch)
316-
is used. _If no revision is specified, the resource will default to `latest`
317-
from `master`._
316+
is used. [git checkout][git-checkout] is used to switch to the
317+
revision, and will result in a detached HEAD in most cases. Use refspec
318+
along with revision if you want to checkout a particular branch without a
319+
detached HEAD. _If no revision is specified, the resource will default to `master`._
320+
1. `refspec`: (Optional) specify a git [refspec][git-refspec] to pass to git-fetch.
321+
Note that if this field is specified, it must specify all refs, branches, tags,
322+
or commits required to checkout the specified `revision`. An additional fetch
323+
will not be run to obtain the contents of the revision field. If no refspec
324+
is specified, the value of the `revision` field will be fetched directly.
325+
The refspec is useful in manipulating the repository in several cases:
326+
* when the server does not support fetches via the commit SHA (i.e. does
327+
not have `uploadpack.allowReachableSHA1InWant` enabled) and you want
328+
to fetch and checkout a specific commit hash from a ref chain.
329+
* when you want to fetch several other refs alongside your revision
330+
(for instance, tags)
331+
* when you want to checkout a specific branch, the revision and refspec
332+
fields can work together to be able to set the destination of the incoming
333+
branch and switch to the branch.
334+
335+
Examples:
336+
- Check out a specified revision commit SHA1 after fetching ref (detached) <br>
337+
&nbsp;&nbsp;`revision`: cb17eba165fe7973ef9afec20e7c6971565bd72f <br>
338+
&nbsp;&nbsp;`refspec`: refs/smoke/myref <br>
339+
- Fetch all tags alongside refs/heads/master and switch to the master branch
340+
(not detached) <br>
341+
&nbsp;&nbsp;`revision`: master <br>
342+
&nbsp;&nbsp;`refspec`: "refs/tags/\*:refs/tags/\* +refs/heads/master:refs/heads/master"<br>
343+
- Fetch the develop branch and switch to it (not detached) <br>
344+
&nbsp;&nbsp;`revision`: develop <br>
345+
&nbsp;&nbsp;`refspec`: refs/heads/develop:refs/heads/develop <br>
346+
- Fetch refs/pull/1009/head into the master branch and switch to it (not detached) <br>
347+
&nbsp;&nbsp;`revision`: master <br>
348+
&nbsp;&nbsp;`refspec`: refs/pull/1009/head:refs/heads/master <br>
349+
318350
1. `submodules`: defines if the resource should initialize and fetch the
319351
submodules, value is either `true` or `false`. _If not specified, this will
320352
default to true_
321353
1. `depth`: performs a [shallow clone][git-depth] where only the most recent
322-
commit(s) will be fetched. If set to `'0'`, all commits will be fetched. _If
323-
not specified, the default depth is 1._
354+
commit(s) will be fetched. This setting also applies to submodules. If set to
355+
`'0'`, all commits will be fetched. _If not specified, the default depth is 1._
324356
1. `sslVerify`: defines if [http.sslVerify][git-http.sslVerify] should be set
325357
to `true` or `false` in the global git config. _Defaults to `true` if
326358
omitted._
327359

328360
[git-rev]: https://git-scm.com/docs/gitrevisions#_specifying_revisions
361+
[git-checkout]: https://git-scm.com/docs/git-checkout
362+
[git-refspec]: https://git-scm.com/book/en/v2/Git-Internals-The-Refspec
329363
[git-depth]: https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---depthltdepthgt
330364
[git-http.sslVerify]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-httpsslVerify
331365

‎pkg/apis/pipeline/v1alpha1/pipeline_resource_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ var (
3030
)
3131

3232
const (
33-
// PipelineResourceTypeGit indicates that this source is a GitHub repo.
33+
// PipelineResourceTypeGit indicates that this source is a Git repo.
3434
PipelineResourceTypeGit PipelineResourceType = resource.PipelineResourceTypeGit
3535

3636
// PipelineResourceTypeStorage indicates that this source is a storage blob resource.

‎pkg/apis/resource/v1alpha1/git/git_resource.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ type Resource struct {
3838
Name string `json:"name"`
3939
Type resource.PipelineResourceType `json:"type"`
4040
URL string `json:"url"`
41-
// Git revision (branch, tag, commit SHA or ref) to clone. See
42-
// https://git-scm.com/docs/gitrevisions#_specifying_revisions for more
43-
// information.
41+
// Git revision (branch, tag, commit SHA) to clone, and optionally the refspec to fetch from.
42+
//See https://git-scm.com/docs/gitrevisions#_specifying_revisions for more information.
4443
Revision string `json:"revision"`
44+
Refspec string `json:"refspec"`
4545
Submodules bool `json:"submodules"`
4646

4747
Depth uint `json:"depth"`
@@ -71,6 +71,8 @@ func NewResource(gitImage string, r *resource.PipelineResource) (*Resource, erro
7171
gitResource.URL = param.Value
7272
case strings.EqualFold(param.Name, "Revision"):
7373
gitResource.Revision = param.Value
74+
case strings.EqualFold(param.Name, "Refspec"):
75+
gitResource.Refspec = param.Value
7476
case strings.EqualFold(param.Name, "Submodules"):
7577
gitResource.Submodules = toBool(param.Value, true)
7678
case strings.EqualFold(param.Name, "Depth"):
@@ -133,6 +135,8 @@ func (s *Resource) Replacements() map[string]string {
133135
"type": s.Type,
134136
"url": s.URL,
135137
"revision": s.Revision,
138+
"refspec": s.Refspec,
139+
"submodules": strconv.FormatBool(s.Submodules),
136140
"depth": strconv.FormatUint(uint64(s.Depth), 10),
137141
"sslVerify": strconv.FormatBool(s.SSLVerify),
138142
"httpProxy": s.HTTPProxy,
@@ -149,6 +153,9 @@ func (s *Resource) GetInputTaskModifier(_ *v1alpha1.TaskSpec, path string) (v1al
149153
"-path", path,
150154
}
151155

156+
if s.Refspec != "" {
157+
args = append(args, "-refspec", s.Refspec)
158+
}
152159
if !s.Submodules {
153160
args = append(args, "-submodules=false")
154161
}

‎pkg/apis/resource/v1alpha1/git/git_resource_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func TestNewGitResource_Valid(t *testing.T) {
5252
Type: resourcev1alpha1.PipelineResourceTypeGit,
5353
URL: "[email protected]:test/test.git",
5454
Revision: "test",
55+
Refspec: "",
5556
GitImage: "override-with-git:latest",
5657
Submodules: true,
5758
Depth: 1,
@@ -72,6 +73,50 @@ func TestNewGitResource_Valid(t *testing.T) {
7273
Type: resourcev1alpha1.PipelineResourceTypeGit,
7374
URL: "[email protected]:test/test.git",
7475
Revision: "master",
76+
Refspec: "",
77+
GitImage: "override-with-git:latest",
78+
Submodules: true,
79+
Depth: 1,
80+
SSLVerify: true,
81+
HTTPProxy: "",
82+
HTTPSProxy: "",
83+
NOProxy: "",
84+
},
85+
}, {
86+
desc: "With Refspec",
87+
pipelineResource: tb.PipelineResource("git-resource", "default",
88+
tb.PipelineResourceSpec(resourcev1alpha1.PipelineResourceTypeGit,
89+
tb.PipelineResourceSpecParam("URL", "[email protected]:test/test.git"),
90+
tb.PipelineResourceSpecParam("Refspec", "refs/changes/22/222134"),
91+
),
92+
),
93+
want: &git.Resource{
94+
Name: "git-resource",
95+
Type: resourcev1alpha1.PipelineResourceTypeGit,
96+
URL: "[email protected]:test/test.git",
97+
Revision: "master",
98+
Refspec: "refs/changes/22/222134",
99+
GitImage: "override-with-git:latest",
100+
Submodules: true,
101+
Depth: 1,
102+
SSLVerify: true,
103+
HTTPProxy: "",
104+
HTTPSProxy: "",
105+
NOProxy: "",
106+
},
107+
}, {
108+
desc: "Without Refspec",
109+
pipelineResource: tb.PipelineResource("git-resource", "default",
110+
tb.PipelineResourceSpec(resourcev1alpha1.PipelineResourceTypeGit,
111+
tb.PipelineResourceSpecParam("URL", "[email protected]:test/test.git"),
112+
),
113+
),
114+
want: &git.Resource{
115+
Name: "git-resource",
116+
Type: resourcev1alpha1.PipelineResourceTypeGit,
117+
URL: "[email protected]:test/test.git",
118+
Revision: "master",
119+
Refspec: "",
75120
GitImage: "override-with-git:latest",
76121
Submodules: true,
77122
Depth: 1,
@@ -93,6 +138,7 @@ func TestNewGitResource_Valid(t *testing.T) {
93138
Type: resourcev1alpha1.PipelineResourceTypeGit,
94139
URL: "[email protected]:test/test.git",
95140
Revision: "test",
141+
Refspec: "",
96142
GitImage: "override-with-git:latest",
97143
Submodules: true,
98144
Depth: 1,
@@ -115,6 +161,7 @@ func TestNewGitResource_Valid(t *testing.T) {
115161
Type: resourcev1alpha1.PipelineResourceTypeGit,
116162
URL: "[email protected]:test/test.git",
117163
Revision: "test",
164+
Refspec: "",
118165
GitImage: "override-with-git:latest",
119166
Submodules: false,
120167
Depth: 1,
@@ -137,6 +184,7 @@ func TestNewGitResource_Valid(t *testing.T) {
137184
Type: resourcev1alpha1.PipelineResourceTypeGit,
138185
URL: "[email protected]:test/test.git",
139186
Revision: "test",
187+
Refspec: "",
140188
GitImage: "override-with-git:latest",
141189
Submodules: true,
142190
Depth: 8,
@@ -159,6 +207,7 @@ func TestNewGitResource_Valid(t *testing.T) {
159207
Type: resourcev1alpha1.PipelineResourceTypeGit,
160208
URL: "[email protected]:test/test.git",
161209
Revision: "test",
210+
Refspec: "",
162211
GitImage: "override-with-git:latest",
163212
Submodules: true,
164213
Depth: 0,
@@ -182,6 +231,7 @@ func TestNewGitResource_Valid(t *testing.T) {
182231
Type: resourcev1alpha1.PipelineResourceTypeGit,
183232
URL: "[email protected]:test/test.git",
184233
Revision: "test",
234+
Refspec: "",
185235
GitImage: "override-with-git:latest",
186236
Submodules: true,
187237
Depth: 0,
@@ -205,6 +255,7 @@ func TestNewGitResource_Valid(t *testing.T) {
205255
Type: resourcev1alpha1.PipelineResourceTypeGit,
206256
URL: "[email protected]:test/test.git",
207257
Revision: "test",
258+
Refspec: "",
208259
GitImage: "override-with-git:latest",
209260
Submodules: true,
210261
Depth: 0,
@@ -228,6 +279,7 @@ func TestNewGitResource_Valid(t *testing.T) {
228279
Type: resourcev1alpha1.PipelineResourceTypeGit,
229280
URL: "[email protected]:test/test.git",
230281
Revision: "test",
282+
Refspec: "",
231283
GitImage: "override-with-git:latest",
232284
Submodules: true,
233285
Depth: 0,
@@ -251,6 +303,7 @@ func TestNewGitResource_Valid(t *testing.T) {
251303
Type: resourcev1alpha1.PipelineResourceTypeGit,
252304
URL: "[email protected]:test/test.git",
253305
Revision: "test",
306+
Refspec: "",
254307
GitImage: "override-with-git:latest",
255308
Submodules: true,
256309
Depth: 0,
@@ -279,6 +332,8 @@ func TestGitResource_Replacements(t *testing.T) {
279332
Type: resourcev1alpha1.PipelineResourceTypeGit,
280333
URL: "[email protected]:test/test.git",
281334
Revision: "master",
335+
Refspec: "",
336+
Submodules: false,
282337
Depth: 16,
283338
SSLVerify: false,
284339
HTTPProxy: "http-proxy.git.com",
@@ -291,6 +346,8 @@ func TestGitResource_Replacements(t *testing.T) {
291346
"type": string(resourcev1alpha1.PipelineResourceTypeGit),
292347
"url": "[email protected]:test/test.git",
293348
"revision": "master",
349+
"refspec": "",
350+
"submodules": "false",
294351
"depth": "16",
295352
"sslVerify": "false",
296353
"httpProxy": "http-proxy.git.com",
@@ -319,6 +376,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
319376
Type: resourcev1alpha1.PipelineResourceTypeGit,
320377
URL: "[email protected]:test/test.git",
321378
Revision: "master",
379+
Refspec: "",
322380
GitImage: "override-with-git:latest",
323381
Submodules: true,
324382
Depth: 1,
@@ -354,6 +412,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
354412
Type: resourcev1alpha1.PipelineResourceTypeGit,
355413
URL: "[email protected]:test/test.git",
356414
Revision: "master",
415+
Refspec: "",
357416
GitImage: "override-with-git:latest",
358417
Submodules: false,
359418
Depth: 1,
@@ -390,6 +449,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
390449
Type: resourcev1alpha1.PipelineResourceTypeGit,
391450
URL: "[email protected]:test/test.git",
392451
Revision: "master",
452+
Refspec: "",
393453
GitImage: "override-with-git:latest",
394454
Submodules: true,
395455
Depth: 8,
@@ -427,6 +487,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
427487
Type: resourcev1alpha1.PipelineResourceTypeGit,
428488
URL: "[email protected]:test/test.git",
429489
Revision: "master",
490+
Refspec: "",
430491
GitImage: "override-with-git:latest",
431492
Submodules: false,
432493
Depth: 1,
@@ -464,6 +525,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
464525
Type: resourcev1alpha1.PipelineResourceTypeGit,
465526
URL: "[email protected]:test/test.git",
466527
Revision: "master",
528+
Refspec: "",
467529
GitImage: "override-with-git:latest",
468530
Submodules: false,
469531
Depth: 1,
@@ -499,6 +561,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
499561
Type: resourcev1alpha1.PipelineResourceTypeGit,
500562
URL: "[email protected]:test/test.git",
501563
Revision: "master",
564+
Refspec: "",
502565
GitImage: "override-with-git:latest",
503566
Submodules: false,
504567
Depth: 1,
@@ -534,6 +597,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
534597
Type: resourcev1alpha1.PipelineResourceTypeGit,
535598
URL: "[email protected]:test/test.git",
536599
Revision: "master",
600+
Refspec: "",
537601
GitImage: "override-with-git:latest",
538602
Submodules: false,
539603
Depth: 1,
@@ -562,6 +626,45 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) {
562626
{Name: "HTTPS_PROXY", Value: "https-proxy.git.com"},
563627
},
564628
},
629+
}, {
630+
desc: "With Refspec",
631+
gitResource: &git.Resource{
632+
Name: "git-resource",
633+
Type: resourcev1alpha1.PipelineResourceTypeGit,
634+
URL: "[email protected]:test/test.git",
635+
Revision: "master",
636+
Refspec: "refs/tags/v1.0:refs/tags/v1.0 refs/heads/master:refs/heads/master",
637+
GitImage: "override-with-git:latest",
638+
Submodules: false,
639+
Depth: 1,
640+
SSLVerify: true,
641+
HTTPProxy: "http-proxy.git.com",
642+
HTTPSProxy: "https-proxy.git.com",
643+
NOProxy: "no-proxy.git.com",
644+
},
645+
want: corev1.Container{
646+
Name: "git-source-git-resource-l22wn",
647+
Image: "override-with-git:latest",
648+
Command: []string{"/ko-app/git-init"},
649+
Args: []string{
650+
"-url",
651+
"[email protected]:test/test.git",
652+
"-revision",
653+
"master",
654+
"-path",
655+
"/test/test",
656+
"-refspec",
657+
"refs/tags/v1.0:refs/tags/v1.0 refs/heads/master:refs/heads/master",
658+
"-submodules=false",
659+
},
660+
WorkingDir: "/workspace",
661+
Env: []corev1.EnvVar{
662+
{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"},
663+
{Name: "HTTP_PROXY", Value: "http-proxy.git.com"},
664+
{Name: "HTTPS_PROXY", Value: "https-proxy.git.com"},
665+
{Name: "NO_PROXY", Value: "no-proxy.git.com"},
666+
},
667+
},
565668
}} {
566669
t.Run(tc.desc, func(t *testing.T) {
567670
ts := pipelinev1alpha1.TaskSpec{}

0 commit comments

Comments
 (0)