From fc38e9be4f7e7438146dc9dc59dc134318a69e82 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Mon, 3 Oct 2022 21:07:14 -0300 Subject: [PATCH 1/3] [content-service] Revert #11895 --- .../content-service/pkg/initializer/git.go | 39 +++++++------------ 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/components/content-service/pkg/initializer/git.go b/components/content-service/pkg/initializer/git.go index a07d0432963116..a0a09f052b1384 100644 --- a/components/content-service/pkg/initializer/git.go +++ b/components/content-service/pkg/initializer/git.go @@ -174,29 +174,18 @@ func (ws *GitInitializer) realizeCloneTarget(ctx context.Context) (err error) { // checkout branch switch ws.TargetMode { case RemoteBranch: - // we already cloned the git repository but we need to check CloneTarget exists, - // except when the name is main or master because either value could be wrong - // and we are going to use the incorrect value (default is main). - if ws.CloneTarget == "main" || ws.CloneTarget == "master" { - // confirm the value of the default branch name using rev-parse - gitout, _ := ws.GitWithOutput(ctx, nil, "rev-parse", "--abbrev-ref", "origin/HEAD") - defaultBranch := strings.TrimSpace(strings.Replace(string(gitout), "origin/", "", -1)) - if defaultBranch != ws.CloneTarget { - // the default branch name we cloned is not the one specified by the user - // check if the branch exits in the repository - gitout, err := ws.GitWithOutput(ctx, nil, "ls-remote", "--exit-code", "origin", ws.CloneTarget) - if err != nil || len(gitout) == 0 { - log.WithField("remoteURI", ws.RemoteURI).WithField("branch", ws.CloneTarget).Warnf("Invalid default branch name. Changing to %v", defaultBranch) - ws.CloneTarget = defaultBranch - } - } - } else { - // check remote branch exists before git checkout when the branch is not the default - gitout, err := ws.GitWithOutput(ctx, nil, "ls-remote", "--exit-code", "origin", ws.CloneTarget) - if err != nil || len(gitout) == 0 { - log.WithError(err).WithField("remoteURI", ws.RemoteURI).WithField("branch", ws.CloneTarget).Error("Remote branch doesn't exist.") - return xerrors.Errorf("Remote branch %v does not exist in %v", ws.CloneTarget, ws.RemoteURI) - } + // confirm the value of the default branch name using rev-parse + gitout, _ := ws.GitWithOutput(ctx, nil, "rev-parse", "--abbrev-ref", "origin/HEAD") + defaultBranch := strings.TrimSpace(strings.Replace(string(gitout), "origin/", "", -1)) + + branchName := ws.CloneTarget + + // we already cloned the git repository but we need to check CloneTarget exists + // to avoid calling fetch from a non-existing branch + gitout, err := ws.GitWithOutput(ctx, nil, "ls-remote", "--exit-code", "origin", ws.CloneTarget) + if err != nil || len(gitout) == 0 { + log.WithField("remoteURI", ws.RemoteURI).WithField("branch", ws.CloneTarget).Warnf("Invalid default branch name. Changing to %v", defaultBranch) + ws.CloneTarget = defaultBranch } if err := ws.Git(ctx, "fetch", "--depth=1", "origin", ws.CloneTarget); err != nil { @@ -204,8 +193,8 @@ func (ws *GitInitializer) realizeCloneTarget(ctx context.Context) (err error) { return err } - if err := ws.Git(ctx, "checkout", ws.CloneTarget); err != nil { - log.WithError(err).WithField("remoteURI", ws.RemoteURI).WithField("branch", ws.CloneTarget).Error("Cannot fetch remote branch") + if err := ws.Git(ctx, "checkout", "-B", branchName, "origin/"+ws.CloneTarget); err != nil { + log.WithError(err).WithField("remoteURI", ws.RemoteURI).WithField("branch", branchName).Error("Cannot fetch remote branch") return err } case LocalBranch: From e4c5cf489f2b63ea115ef82fcba8b52ac416d139 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Mon, 3 Oct 2022 21:07:25 -0300 Subject: [PATCH 2/3] Add integration tests for additionalRepositories --- .../additional_repositories_test.go | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 test/tests/components/ws-manager/additional_repositories_test.go diff --git a/test/tests/components/ws-manager/additional_repositories_test.go b/test/tests/components/ws-manager/additional_repositories_test.go new file mode 100644 index 00000000000000..44c2655251c154 --- /dev/null +++ b/test/tests/components/ws-manager/additional_repositories_test.go @@ -0,0 +1,94 @@ +// Copyright (c) 2022 Gitpod GmbH. All rights reserved. +// Licensed under the GNU Affero General Public License (AGPL). +// See License-AGPL.txt in the project root for license information. + +package wsmanager + +import ( + "context" + "testing" + "time" + + "sigs.k8s.io/e2e-framework/pkg/envconf" + "sigs.k8s.io/e2e-framework/pkg/features" + + csapi "github.com/gitpod-io/gitpod/content-service/api" + "github.com/gitpod-io/gitpod/test/pkg/integration" + wsmanapi "github.com/gitpod-io/gitpod/ws-manager/api" +) + +func TestAdditionalRepositories(t *testing.T) { + f := features.New("additional-repositories"). + WithLabel("component", "ws-manager"). + Assess("can open a workspace using the additionalRepositories property", func(_ context.Context, t *testing.T, cfg *envconf.Config) context.Context { + tests := []struct { + Name string + ContextURL string + CloneTaget string + }{ + { + Name: "workspace with additionalRepositories using a branch", + ContextURL: "https://github.com/gitpod-io/gitpod-test-repo", + CloneTaget: "aledbf/test-additional-repositories", + }, + { + Name: "workspace with additionalRepositories also using a branch in one of the additionalRepositories", + ContextURL: "https://github.com/gitpod-io/gitpod-test-repo", + CloneTaget: "aledbf/test-additional-repositories-with-branches", + }, + } + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(5*len(tests))*time.Minute) + defer cancel() + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + api := integration.NewComponentAPI(ctx, cfg.Namespace(), kubeconfig, cfg.Client()) + t.Cleanup(func() { + api.Done(t) + }) + + ws, stopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { + testRepoName := "gitpod-test-repo" + req.Spec.WorkspaceLocation = testRepoName + req.Spec.Initializer = &csapi.WorkspaceInitializer{ + Spec: &csapi.WorkspaceInitializer_Git{ + Git: &csapi.GitInitializer{ + RemoteUri: test.ContextURL, + CloneTaget: test.CloneTaget, + Config: &csapi.GitConfig{}, + TargetMode: csapi.CloneTargetMode_REMOTE_BRANCH, + }, + }, + } + + return nil + })) + if err != nil { + t.Fatal(err) + } + + defer func() { + // stop workspace in defer function to prevent we forget to stop the workspace + if err := stopWorkspace(t, cfg, stopWs); err != nil { + t.Errorf("cannot stop workspace: %q", err) + } + }() + + rsa, closer, err := integration.Instrument(integration.ComponentWorkspace, "workspace", cfg.Namespace(), kubeconfig, cfg.Client(), + integration.WithInstanceID(ws.Req.Id), + ) + if err != nil { + t.Fatal(err) + } + + integration.DeferCloser(t, closer) + defer rsa.Close() + }) + } + + return ctx + }). + Feature() + + testEnv.Test(t, f) +} From 179f90ede5a688d749e177628762e34ca36ad606 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Tue, 4 Oct 2022 09:52:13 -0300 Subject: [PATCH 3/3] Check the branch is the expected --- .../additional_repositories_test.go | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/test/tests/components/ws-manager/additional_repositories_test.go b/test/tests/components/ws-manager/additional_repositories_test.go index 44c2655251c154..f9d88d4a86157c 100644 --- a/test/tests/components/ws-manager/additional_repositories_test.go +++ b/test/tests/components/ws-manager/additional_repositories_test.go @@ -6,6 +6,8 @@ package wsmanager import ( "context" + "fmt" + "strings" "testing" "time" @@ -13,6 +15,7 @@ import ( "sigs.k8s.io/e2e-framework/pkg/features" csapi "github.com/gitpod-io/gitpod/content-service/api" + agent "github.com/gitpod-io/gitpod/test/pkg/agent/workspace/api" "github.com/gitpod-io/gitpod/test/pkg/integration" wsmanapi "github.com/gitpod-io/gitpod/ws-manager/api" ) @@ -37,6 +40,7 @@ func TestAdditionalRepositories(t *testing.T) { CloneTaget: "aledbf/test-additional-repositories-with-branches", }, } + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(5*len(tests))*time.Minute) defer cancel() @@ -47,16 +51,17 @@ func TestAdditionalRepositories(t *testing.T) { api.Done(t) }) + testRepoName := "gitpod-test-repo" ws, stopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { - testRepoName := "gitpod-test-repo" req.Spec.WorkspaceLocation = testRepoName req.Spec.Initializer = &csapi.WorkspaceInitializer{ Spec: &csapi.WorkspaceInitializer_Git{ Git: &csapi.GitInitializer{ - RemoteUri: test.ContextURL, - CloneTaget: test.CloneTaget, - Config: &csapi.GitConfig{}, - TargetMode: csapi.CloneTargetMode_REMOTE_BRANCH, + RemoteUri: test.ContextURL, + CloneTaget: test.CloneTaget, + Config: &csapi.GitConfig{}, + TargetMode: csapi.CloneTargetMode_REMOTE_BRANCH, + CheckoutLocation: testRepoName, }, }, } @@ -83,6 +88,24 @@ func TestAdditionalRepositories(t *testing.T) { integration.DeferCloser(t, closer) defer rsa.Close() + + var gitOut agent.ExecResponse + err = rsa.Call("WorkspaceAgent.Exec", &agent.ExecRequest{ + Dir: fmt.Sprintf("/workspace/%v", testRepoName), + Command: "bash", + Args: []string{ + "-c", + "git symbolic-ref --short HEAD", + }, + }, &gitOut) + if err != nil { + t.Fatal(err) + } + + out := strings.TrimSpace(gitOut.Stdout) + if test.CloneTaget != out { + t.Errorf("returned branch %v is not %v", out, test.CloneTaget) + } }) }