From 9aff0a076f70c7bf32a6e275762e6097f8f3cb2d Mon Sep 17 00:00:00 2001 From: vinamra28 Date: Thu, 18 Jun 2020 22:01:19 +0530 Subject: [PATCH] Fix git-init in case `master` is not the default branch This fix will patch the `git-init` such that if user doesn't provides revision then instead of taking `master` as the default branch we are doing the `symbolic-link` to the HEAD branch. Also added e2e tests for it. Signed-off-by: vinamra28 --- docs/resources.md | 2 +- .../resource/v1alpha1/git/git_resource.go | 10 +-- .../v1alpha1/git/git_resource_test.go | 74 ++++++++++++++----- pkg/git/git.go | 9 ++- .../taskrun/resources/input_resource_test.go | 16 ++-- pkg/reconciler/taskrun/taskrun_test.go | 10 +-- test/git_checkout_test.go | 57 ++++++++++++++ test/v1alpha1/git_checkout_test.go | 57 ++++++++++++++ 8 files changed, 193 insertions(+), 42 deletions(-) diff --git a/docs/resources.md b/docs/resources.md index 05da3adedbf..aa3e3043822 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -330,7 +330,7 @@ Params that can be added are the following: is used. [git checkout][git-checkout] is used to switch to the revision, and will result in a detached HEAD in most cases. Use refspec along with revision if you want to checkout a particular branch without a - detached HEAD. _If no revision is specified, the resource will default to `master`._ + detached HEAD. _If no revision is specified, the resource inspects remote repository to determine the correct default branch._ 1. `refspec`: (Optional) specify a git [refspec][git-refspec] to pass to git-fetch. Note that if this field is specified, it must specify all refs, branches, tags, or commits required to checkout the specified `revision`. An additional fetch diff --git a/pkg/apis/resource/v1alpha1/git/git_resource.go b/pkg/apis/resource/v1alpha1/git/git_resource.go index 4aa7b3b5b8a..a50d34b9533 100644 --- a/pkg/apis/resource/v1alpha1/git/git_resource.go +++ b/pkg/apis/resource/v1alpha1/git/git_resource.go @@ -87,10 +87,7 @@ func NewResource(name, gitImage string, r *resource.PipelineResource) (*Resource gitResource.NOProxy = param.Value } } - // default revision to master if nothing is provided - if gitResource.Revision == "" { - gitResource.Revision = "master" - } + return &gitResource, nil } @@ -149,10 +146,13 @@ func (s *Resource) Replacements() map[string]string { func (s *Resource) GetInputTaskModifier(_ *v1beta1.TaskSpec, path string) (v1beta1.TaskModifier, error) { args := []string{ "-url", s.URL, - "-revision", s.Revision, "-path", path, } + if s.Revision != "" { + args = append(args, "-revision", s.Revision) + } + if s.Refspec != "" { args = append(args, "-refspec", s.Refspec) } diff --git a/pkg/apis/resource/v1alpha1/git/git_resource_test.go b/pkg/apis/resource/v1alpha1/git/git_resource_test.go index a36ce703ed2..125ae924586 100644 --- a/pkg/apis/resource/v1alpha1/git/git_resource_test.go +++ b/pkg/apis/resource/v1alpha1/git/git_resource_test.go @@ -74,7 +74,7 @@ func TestNewGitResource_Valid(t *testing.T) { Name: "test-resource", Type: resourcev1alpha1.PipelineResourceTypeGit, URL: "git@github.com:test/test.git", - Revision: "master", + Revision: "", Refspec: "", GitImage: "override-with-git:latest", Submodules: true, @@ -96,7 +96,7 @@ func TestNewGitResource_Valid(t *testing.T) { Name: "test-resource", Type: resourcev1alpha1.PipelineResourceTypeGit, URL: "git@github.com:test/test.git", - Revision: "master", + Revision: "", Refspec: "refs/changes/22/222134", GitImage: "override-with-git:latest", Submodules: true, @@ -117,7 +117,7 @@ func TestNewGitResource_Valid(t *testing.T) { Name: "test-resource", Type: resourcev1alpha1.PipelineResourceTypeGit, URL: "git@github.com:test/test.git", - Revision: "master", + Revision: "", Refspec: "", GitImage: "override-with-git:latest", Submodules: true, @@ -394,10 +394,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", }, WorkingDir: "/workspace", Env: []corev1.EnvVar{ @@ -431,10 +431,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", "-submodules=false", }, WorkingDir: "/workspace", @@ -469,10 +469,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", "-depth", "8", }, @@ -508,10 +508,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", "-submodules=false", "-sslVerify=false", }, @@ -546,10 +546,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", "-submodules=false", "-sslVerify=false", }, @@ -583,10 +583,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", "-submodules=false", "-sslVerify=false", }, @@ -620,10 +620,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", "-submodules=false", "-sslVerify=false", }, @@ -658,10 +658,10 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { Args: []string{ "-url", "git@github.com:test/test.git", - "-revision", - "master", "-path", "/test/test", + "-revision", + "master", "-refspec", "refs/tags/v1.0:refs/tags/v1.0 refs/heads/master:refs/heads/master", "-submodules=false", @@ -675,6 +675,42 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { {Name: "NO_PROXY", Value: "no-proxy.git.com"}, }, }, + }, { + desc: "Without Refspec and without revision", + gitResource: &git.Resource{ + Name: "git-resource", + Type: resourcev1alpha1.PipelineResourceTypeGit, + URL: "git@github.com:test/test.git", + Revision: "", + Refspec: "", + GitImage: "override-with-git:latest", + Submodules: false, + Depth: 1, + SSLVerify: true, + HTTPProxy: "http-proxy.git.com", + HTTPSProxy: "https-proxy.git.com", + NOProxy: "no-proxy.git.com", + }, + want: corev1.Container{ + Name: "git-source-git-resource-twkr2", + Image: "override-with-git:latest", + Command: []string{"/ko-app/git-init"}, + Args: []string{ + "-url", + "git@github.com:test/test.git", + "-path", + "/test/test", + "-submodules=false", + }, + WorkingDir: "/workspace", + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, + {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, + {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, + {Name: "NO_PROXY", Value: "no-proxy.git.com"}, + }, + }, }} { t.Run(tc.desc, func(t *testing.T) { ts := v1beta1.TaskSpec{} diff --git a/pkg/git/git.go b/pkg/git/git.go index 7b5cc047f4d..50fe8fe194f 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -64,9 +64,6 @@ func Fetch(logger *zap.SugaredLogger, spec FetchSpec) error { return err } - if spec.Revision == "" { - spec.Revision = "master" - } if spec.Path != "" { if _, err := run(logger, "", "init", spec.Path); err != nil { return err @@ -85,6 +82,12 @@ func Fetch(logger *zap.SugaredLogger, spec FetchSpec) error { logger.Warnf("Failed to set http.sslVerify in git config: %s", err) return err } + if spec.Revision == "" { + spec.Revision = "HEAD" + if _, err := run(logger, "", "symbolic-ref", spec.Revision, "refs/remotes/origin/HEAD"); err != nil { + return err + } + } fetchArgs := []string{"fetch"} if spec.Submodules { diff --git a/pkg/reconciler/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index 81ce89b9fed..051e525b534 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -369,7 +369,7 @@ func TestAddInputResourceToTask(t *testing.T) { Name: "git-source-the-git-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}, @@ -410,7 +410,7 @@ func TestAddInputResourceToTask(t *testing.T) { Name: "git-source-the-git-with-branch-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/gitspace", "-revision", "branch"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, @@ -458,7 +458,7 @@ func TestAddInputResourceToTask(t *testing.T) { Name: "git-source-the-git-with-branch-mz4c7", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/gitspace", "-revision", "branch"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, @@ -468,7 +468,7 @@ func TestAddInputResourceToTask(t *testing.T) { Name: "git-source-the-git-with-branch-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/git-duplicate-space"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/git-duplicate-space", "-revision", "branch"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, @@ -509,7 +509,7 @@ func TestAddInputResourceToTask(t *testing.T) { Name: "git-source-the-git-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}, @@ -550,7 +550,7 @@ func TestAddInputResourceToTask(t *testing.T) { Name: "git-source-the-git-with-branch-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/gitspace", "-revision", "branch"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, @@ -639,7 +639,7 @@ func TestAddInputResourceToTask(t *testing.T) { Name: "git-source-the-git-with-sslVerify-false-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace", "-sslVerify=false"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/gitspace", "-revision", "branch", "-sslVerify=false"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-sslVerify-false"}, @@ -964,7 +964,7 @@ gsutil cp gs://fake-bucket/rules.zip /workspace/gcs-dir Name: "git-source-the-git-with-branch-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, - Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, + Args: []string{"-url", "https://github.com/grafeas/kritis", "-path", "/workspace/gitspace", "-revision", "branch"}, WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 224c250c6e1..108c614f006 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -940,7 +940,7 @@ func TestReconcile(t *testing.T) { tb.Command(entrypointLocation), tb.Args("-wait_file", "/tekton/tools/0", "-post_file", "/tekton/tools/1", "-termination_path", "/tekton/termination", "-entrypoint", "/ko-app/git-init", "--", "-url", "https://foo.git", - "-revision", "master", "-path", "/workspace/workspace"), + "-path", "/workspace/workspace"), tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/tekton/home"), tb.EnvVar("TEKTON_RESOURCE_NAME", "workspace"), @@ -1023,8 +1023,6 @@ func TestReconcile(t *testing.T) { "--", "-url", "https://foo.git", - "-revision", - "master", "-path", "/workspace/workspace", ), @@ -1132,10 +1130,10 @@ func TestReconcile(t *testing.T) { "--", "-url", "github.com/foo/bar.git", - "-revision", - "rel-can", "-path", - "/workspace/workspace"), + "/workspace/workspace", + "-revision", + "rel-can"), tb.WorkingDir(workspaceDir), // Note: the duplication of HOME env var here is intentional: our pod builder // adds it first and the git pipelineresource adds its own to ensure that HOME diff --git a/test/git_checkout_test.go b/test/git_checkout_test.go index f16fefe112b..814b6f4995d 100644 --- a/test/git_checkout_test.go +++ b/test/git_checkout_test.go @@ -299,6 +299,63 @@ func TestGitPipelineRunFail_HTTPS_PROXY(t *testing.T) { } } +// TestGitPipelineRunWithNonMasterBranch is an integration test that will verify the source code is either fetched or pulled +// successfully under different revision inputs (default branch, branch) +// This test will run on spring-petclinic repository which does not contain a master branch as the default branch +func TestGitPipelineRunWithNonMasterBranch(t *testing.T) { + t.Parallel() + + revisions := []string{"", "main"} + + for _, revision := range revisions { + + t.Run(revision, func(t *testing.T) { + c, namespace := setup(t) + knativetest.CleanupOnInterrupt(func() { tearDown(t, c, namespace) }, t.Logf) + defer tearDown(t, c, namespace) + + t.Logf("Creating Git PipelineResource %s", gitSourceResourceName) + if _, err := c.PipelineResourceClient.Create(getGitPipelineResourceSpringPetClinic(revision, "", "true", "", "", "")); err != nil { + t.Fatalf("Failed to create Pipeline Resource `%s`: %s", gitSourceResourceName, err) + } + + t.Logf("Creating Task %s", gitTestTaskName) + if _, err := c.TaskClient.Create(getGitCheckTask(namespace)); err != nil { + t.Fatalf("Failed to create Task `%s`: %s", gitTestTaskName, err) + } + + t.Logf("Creating Pipeline %s", gitTestPipelineName) + if _, err := c.PipelineClient.Create(getGitCheckPipeline(namespace)); err != nil { + t.Fatalf("Failed to create Pipeline `%s`: %s", gitTestPipelineName, err) + } + + t.Logf("Creating PipelineRun %s", gitTestPipelineRunName) + if _, err := c.PipelineRunClient.Create(getGitCheckPipelineRun(namespace)); err != nil { + t.Fatalf("Failed to create Pipeline `%s`: %s", gitTestPipelineRunName, err) + } + + if err := WaitForPipelineRunState(c, gitTestPipelineRunName, timeout, PipelineRunSucceed(gitTestPipelineRunName), "PipelineRunCompleted"); err != nil { + t.Errorf("Error waiting for PipelineRun %s to finish: %s", gitTestPipelineRunName, err) + t.Fatalf("PipelineRun execution failed") + } + }) + } +} + +// getGitPipelineResourceSpringPetClinic will help to clone the spring-petclinic repository which does not contains master branch +func getGitPipelineResourceSpringPetClinic(revision, refspec, sslverify, httpproxy, httpsproxy, noproxy string) *v1alpha1.PipelineResource { + return tb.PipelineResource(gitSourceResourceName, tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeGit, + tb.PipelineResourceSpecParam("Url", "https://github.com/spring-projects/spring-petclinic"), + tb.PipelineResourceSpecParam("Revision", revision), + tb.PipelineResourceSpecParam("Refspec", refspec), + tb.PipelineResourceSpecParam("sslVerify", sslverify), + tb.PipelineResourceSpecParam("httpProxy", httpproxy), + tb.PipelineResourceSpecParam("httpsProxy", httpsproxy), + tb.PipelineResourceSpecParam("noProxy", noproxy), + )) +} + func getGitPipelineResource(revision, refspec, sslverify, httpproxy, httpsproxy, noproxy string) *v1alpha1.PipelineResource { return tb.PipelineResource(gitSourceResourceName, tb.PipelineResourceSpec( v1alpha1.PipelineResourceTypeGit, diff --git a/test/v1alpha1/git_checkout_test.go b/test/v1alpha1/git_checkout_test.go index efa40c9eee1..b28f81471d9 100644 --- a/test/v1alpha1/git_checkout_test.go +++ b/test/v1alpha1/git_checkout_test.go @@ -297,6 +297,63 @@ func TestGitPipelineRunFail_HTTPS_PROXY(t *testing.T) { } } +// TestGitPipelineRunWithNonMasterBranch is an integration test that will verify the source code is either fetched or pulled +// successfully under different revision inputs (default branch, branch) +// This test will run on spring-petclinic repository which does not contain a master branch as the default branch +func TestGitPipelineRunWithNonMasterBranch(t *testing.T) { + t.Parallel() + + revisions := []string{"", "main"} + + for _, revision := range revisions { + + t.Run(revision, func(t *testing.T) { + c, namespace := setup(t) + knativetest.CleanupOnInterrupt(func() { tearDown(t, c, namespace) }, t.Logf) + defer tearDown(t, c, namespace) + + t.Logf("Creating Git PipelineResource %s", gitSourceResourceName) + if _, err := c.PipelineResourceClient.Create(getGitPipelineResourceSpringPetClinic(revision, "", "true", "", "", "")); err != nil { + t.Fatalf("Failed to create Pipeline Resource `%s`: %s", gitSourceResourceName, err) + } + + t.Logf("Creating Task %s", gitTestTaskName) + if _, err := c.TaskClient.Create(getGitCheckTask()); err != nil { + t.Fatalf("Failed to create Task `%s`: %s", gitTestTaskName, err) + } + + t.Logf("Creating Pipeline %s", gitTestPipelineName) + if _, err := c.PipelineClient.Create(getGitCheckPipeline()); err != nil { + t.Fatalf("Failed to create Pipeline `%s`: %s", gitTestPipelineName, err) + } + + t.Logf("Creating PipelineRun %s", gitTestPipelineRunName) + if _, err := c.PipelineRunClient.Create(getGitCheckPipelineRun()); err != nil { + t.Fatalf("Failed to create Pipeline `%s`: %s", gitTestPipelineRunName, err) + } + + if err := WaitForPipelineRunState(c, gitTestPipelineRunName, timeout, PipelineRunSucceed(gitTestPipelineRunName), "PipelineRunCompleted"); err != nil { + t.Errorf("Error waiting for PipelineRun %s to finish: %s", gitTestPipelineRunName, err) + t.Fatalf("PipelineRun execution failed") + } + }) + } +} + +// getGitPipelineResourceSpringPetClinic will help to clone the spring-petclinic repository which does not contains master branch +func getGitPipelineResourceSpringPetClinic(revision, refspec, sslverify, httpproxy, httpsproxy, noproxy string) *v1alpha1.PipelineResource { + return tb.PipelineResource(gitSourceResourceName, tb.PipelineResourceSpec( + v1alpha1.PipelineResourceTypeGit, + tb.PipelineResourceSpecParam("Url", "https://github.com/spring-projects/spring-petclinic"), + tb.PipelineResourceSpecParam("Revision", revision), + tb.PipelineResourceSpecParam("Refspec", refspec), + tb.PipelineResourceSpecParam("sslVerify", sslverify), + tb.PipelineResourceSpecParam("httpProxy", httpproxy), + tb.PipelineResourceSpecParam("httpsProxy", httpsproxy), + tb.PipelineResourceSpecParam("noProxy", noproxy), + )) +} + func getGitPipelineResource(revision, refspec, sslverify, httpproxy, httpsproxy, noproxy string) *v1alpha1.PipelineResource { return tb.PipelineResource(gitSourceResourceName, tb.PipelineResourceSpec( v1alpha1.PipelineResourceTypeGit,