fix(repohub): GetFiles should fetch Git updates from origin only for specified branch (#159)

lucaspin · web-flow · commit e6be51bd6394 · 2025-03-25T09:31:49.000-03:00
Right now, when fetching the YAML files for workflow editor, we fetch
Git remote updates from all branches in the repository. That can take a
while to run for big repositories, which can lead to 500s in workflow
editor page. Here, we limit the `git fetch origin` command to fetch
changes only for the branch we are interested.
diff --git a/front/lib/front/models/repohub.ex b/front/lib/front/models/repohub.ex
@@ -3,12 +3,16 @@ defmodule Front.Models.Repohub do
 
   alias InternalApi.Repository.RepositoryService.Stub
 
-  def fetch_sempahore_files(repository_id, initial_yaml, commit_sha \\ "") do
+  def fetch_semaphore_files(repository_id, initial_yaml, commit_sha \\ "", reference \\ "") do
     Watchman.benchmark("external.repohub.find_all_yaml_files.duration", fn ->
       request =
         InternalApi.Repository.GetFilesRequest.new(
           repository_id: repository_id,
-          revision: InternalApi.Repository.Revision.new(commit_sha: commit_sha),
+          revision:
+            InternalApi.Repository.Revision.new(
+              commit_sha: commit_sha,
+              reference: reference
+            ),
           selectors: extract_selectors(initial_yaml),
           include_content: true
         )
diff --git a/front/lib/front_web/controllers/project_controller.ex b/front/lib/front_web/controllers/project_controller.ex
@@ -198,7 +198,7 @@ defmodule FrontWeb.ProjectController do
 
     fetch_yaml_files =
       Async.run(
-        fn -> Repohub.fetch_sempahore_files(project.repo_id, project.initial_pipeline_file) end,
+        fn -> Repohub.fetch_semaphore_files(project.repo_id, project.initial_pipeline_file) end,
         metric: "workflow.edit.load_all_yaml_files"
       )
 
diff --git a/front/lib/front_web/controllers/workflow_controller.ex b/front/lib/front_web/controllers/workflow_controller.ex
@@ -117,10 +117,38 @@ defmodule FrontWeb.WorkflowController do
   end
 
   defp fetch_yaml_files(repo_id, hook, initial_yaml) do
-    if hook.type == "pr" do
-      Repohub.fetch_sempahore_files(repo_id, initial_yaml, hook.pr_sha)
-    else
-      Repohub.fetch_sempahore_files(repo_id, initial_yaml, hook.head_commit_sha)
+    cond do
+      hook.type == "pr" and hook.forked_pr ->
+        Repohub.fetch_semaphore_files(
+          repo_id,
+          initial_yaml,
+          hook.pr_sha,
+          "refs/pulls/#{hook.pr_number}/head"
+        )
+
+      hook.type == "pr" and !hook.forked_pr ->
+        Repohub.fetch_semaphore_files(
+          repo_id,
+          initial_yaml,
+          hook.pr_sha,
+          "refs/heads/#{hook.pr_branch_name}"
+        )
+
+      hook.type == "branch" ->
+        Repohub.fetch_semaphore_files(
+          repo_id,
+          initial_yaml,
+          hook.pr_sha,
+          "refs/heads/#{hook.branch_name}"
+        )
+
+      hook.type == "tag" ->
+        Repohub.fetch_semaphore_files(
+          repo_id,
+          initial_yaml,
+          hook.pr_sha,
+          "refs/tags/#{hook.tag_name}"
+        )
     end
   end
 
diff --git a/repohub/Dockerfile b/repohub/Dockerfile
@@ -1,4 +1,4 @@
-ARG GO_VERSION=1.21
+ARG GO_VERSION=1.22
 ARG UBUNTU_VERSION=22.04
 ARG BUILDER_IMAGE="golang:${GO_VERSION}"
 ARG RUNNER_IMAGE="ubuntu:${UBUNTU_VERSION}"
diff --git a/repohub/go.mod b/repohub/go.mod
@@ -1,10 +1,10 @@
 module github.com/semaphoreio/semaphore/repohub
 
-go 1.21
+go 1.22
 
 require (
 	github.com/bmatcuk/doublestar v1.1.5
-	github.com/golang-jwt/jwt/v5 v5.2.1
+	github.com/golang-jwt/jwt/v5 v5.2.2
 	github.com/golang/protobuf v1.5.4
 	github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.2.0
 	github.com/jinzhu/gorm v1.9.16
diff --git a/repohub/go.sum b/repohub/go.sum
@@ -10,8 +10,8 @@ github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5 h1:Yzb9+7DP
 github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5/go.mod h1:a2zkGnVExMxdzMo3M0Hi/3sEU+cWnZpSni0O6/Yb/P0=
 github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs=
 github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
-github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=
-github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
+github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8=
+github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
 github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe h1:lXe2qZdvpiX5WZkZR4hgp4KJVfY3nMkvmwbVkpv1rVY=
 github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0=
 github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
diff --git a/repohub/pkg/fetcher/fetcher.go b/repohub/pkg/fetcher/fetcher.go
@@ -97,7 +97,7 @@ func (f *Fetcher) Sync(index int, totalRepoCount int, r *models.Repository) bool
 		Password: token,
 	}
 
-	op := gitrekt.NewUpdateOrCloneOperation(repo)
+	op := gitrekt.NewUpdateOrCloneOperation(repo, "")
 
 	log.Printf("Syncing repo %d/%d START: Repo %s from %s",
 		index,
diff --git a/repohub/pkg/gitrekt/commit.go b/repohub/pkg/gitrekt/commit.go
@@ -100,7 +100,7 @@ func (op *CommitOperation) Run() (*Revision, error) {
 	err := op.Validate()
 	check(err, "Invalid parameters")
 
-	repoPath, err := UpdateOrClone(op.Repository)
+	repoPath, err := UpdateOrClone(op.Repository, nil)
 	check(err, "Failed to clone repository")
 
 	r, err := git.OpenRepository(repoPath)
diff --git a/repohub/pkg/gitrekt/list_changed_files.go b/repohub/pkg/gitrekt/list_changed_files.go
@@ -22,7 +22,7 @@ func ListChangedFiles(repo *Repository, base Revision, head Revision, comparison
 
 	log.Printf("ListChangedFiles Started. Repo: %s", repo.HttpURL)
 
-	repoPath, err := UpdateOrClone(repo)
+	repoPath, err := UpdateOrClone(repo, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/repohub/pkg/gitrekt/search.go b/repohub/pkg/gitrekt/search.go
@@ -31,7 +31,7 @@ func Search(repo *Repository, rev Revision, options *SearchOptions) ([]*File, er
 		rev,
 	)
 
-	repoPath, err := UpdateOrClone(repo)
+	repoPath, err := UpdateOrClone(repo, &rev)
 	if err != nil {
 		return nil, err
 	}
diff --git a/repohub/pkg/gitrekt/update_or_clone.go b/repohub/pkg/gitrekt/update_or_clone.go
@@ -12,10 +12,12 @@ import (
 	"github.com/renderedtext/go-watchman"
 )
 
-func UpdateOrClone(repo *Repository) (string, error) {
+func UpdateOrClone(repo *Repository, revision *Revision) (string, error) {
 	defer watchman.BenchmarkWithTags(time.Now(), "gitrekt.UpdateOrClone", []string{repo.HttpURL})
 
-	op := NewUpdateOrCloneOperation(repo)
+	reference := extractReference(revision)
+
+	op := NewUpdateOrCloneOperation(repo, reference)
 	err := op.Run()
 
 	log.Printf("UpdateOrClone took %f seconds", op.Duration())
@@ -29,12 +31,13 @@ func UpdateOrClone(repo *Repository) (string, error) {
 
 type UpdateOrCloneOperation struct {
 	Repository *Repository
+	Reference  string
 	Started    time.Time
 	Finished   time.Time
 }
 
-func NewUpdateOrCloneOperation(repo *Repository) *UpdateOrCloneOperation {
-	return &UpdateOrCloneOperation{Repository: repo}
+func NewUpdateOrCloneOperation(repo *Repository, reference string) *UpdateOrCloneOperation {
+	return &UpdateOrCloneOperation{Repository: repo, Reference: reference}
 }
 
 func (o *UpdateOrCloneOperation) Duration() float64 {
@@ -63,8 +66,16 @@ func (o *UpdateOrCloneOperation) Update() error {
 	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Minute)
 	defer cancel()
 
-	log.Printf("fetching from remotes %s", o.Repository.Path())
-	cmd := exec.CommandContext(ctx, "git", "fetch", "origin")
+	var cmd *exec.Cmd
+	if o.Reference != "" {
+		log.Printf("fetching from remotes %s with revision %v", o.Repository.Path(), o.Reference)
+
+		// #nosec G204
+		cmd = exec.CommandContext(ctx, "git", "fetch", "origin", o.Reference)
+	} else {
+		log.Printf("fetching from remotes %s without revision", o.Repository.Path())
+		cmd = exec.CommandContext(ctx, "git", "fetch", "origin")
+	}
 
 	cmd.Dir = o.Repository.Path()
 	cmd.Env = append(cmd.Env, "GIT_ASKPASS=/app/git-ask-pass.sh")
@@ -231,7 +242,7 @@ func (o *UpdateOrCloneOperation) parseError(output []byte, err error) error {
 		}
 	}
 
-	return err
+	return fmt.Errorf("err: %s: %v", output, err)
 }
 
 func cleanupDirectory(o *UpdateOrCloneOperation) {
@@ -240,3 +251,17 @@ func cleanupDirectory(o *UpdateOrCloneOperation) {
 		log.Printf("(err) Failed to remove directory %s, err: %s", o.Repository.Path(), err.Error())
 	}
 }
+
+func extractReference(r *Revision) string {
+	if r == nil {
+		return ""
+	}
+
+	ref := r.Reference
+
+	if strings.HasPrefix(r.Reference, "refs/remotes/origin/") {
+		ref = "refs/heads/" + strings.TrimPrefix(r.Reference, "refs/remotes/origin/")
+	}
+
+	return ref
+}
diff --git a/repohub/pkg/hub/repo_service.go b/repohub/pkg/hub/repo_service.go
@@ -125,6 +125,11 @@ func (s *RepoService) GetFiles(ctx context.Context, request *ia_repository.GetFi
 		options,
 	)
 
+	if err != nil {
+		log.Printf("Error getting files for %s: %v", id, err)
+		return nil, status.Error(codes.Unknown, err.Error())
+	}
+
 	response := &ia_repository.GetFilesResponse{
 		Files: s.serializeFiles(files),
 	}
diff --git a/repohub/test/hub/repo_service_test.go b/repohub/test/hub/repo_service_test.go
@@ -344,3 +344,28 @@ func Test__GetFiles__ContentRegex(t *testing.T) {
 	assert.Equal(t, res.Files[0].Path, "scripts/deploy/a.sh")
 	assert.Equal(t, res.Files[0].Content, "AAA !\n\nA is the best!\n")
 }
+
+func Test__GetFiles__WithReferenceThatDoesExist(t *testing.T) {
+	support.PurgeDB()
+
+	repo := support.CreateRepository()
+	client := ia_repository.NewRepositoryServiceClient(testConn)
+
+	req := ia_repository.GetFilesRequest{
+		RepositoryId: repo.ID.String(),
+
+		Revision: &ia_repository.Revision{
+			CommitSha: "5f4138948b165a491b7034d3e95ce023a1a098c5",
+			Reference: "refs/heads/does-not-exist",
+		},
+
+		Selectors: []*ia_repository.GetFilesRequest_Selector{
+			&ia_repository.GetFilesRequest_Selector{
+				Glob: "*.txt",
+			},
+		},
+	}
+
+	_, err := client.GetFiles(context.Background(), &req)
+	assert.ErrorContains(t, err, "couldn't find remote ref refs/heads/does-not-exist")
+}

Original file line number	Diff line number	Diff line change
`@@ -198,7 +198,7 @@ defmodule FrontWeb.ProjectController do`
`198`	`198`
`199`	`199`	`fetch_yaml_files =`
`200`	`200`	`Async.run(`
`201`		`- fn -> Repohub.fetch_sempahore_files(project.repo_id, project.initial_pipeline_file) end,`
	`201`	`+ fn -> Repohub.fetch_semaphore_files(project.repo_id, project.initial_pipeline_file) end,`
`202`	`202`	`metric: "workflow.edit.load_all_yaml_files"`
`203`	`203`	`)`
`204`	`204`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-ARG GO_VERSION=1.21`
	`1`	`+ARG GO_VERSION=1.22`
`2`	`2`	`ARG UBUNTU_VERSION=22.04`
`3`	`3`	`ARG BUILDER_IMAGE="golang:${GO_VERSION}"`
`4`	`4`	`ARG RUNNER_IMAGE="ubuntu:${UBUNTU_VERSION}"`
Original file line number	Diff line number	Diff line change
`@@ -97,7 +97,7 @@ func (f Fetcher) Sync(index int, totalRepoCount int, r models.Repository) bool`
`97`	`97`	`Password: token,`
`98`	`98`	`}`
`99`	`99`
`100`		`- op := gitrekt.NewUpdateOrCloneOperation(repo)`
	`100`	`+ op := gitrekt.NewUpdateOrCloneOperation(repo, "")`
`101`	`101`
`102`	`102`	`log.Printf("Syncing repo %d/%d START: Repo %s from %s",`
`103`	`103`	`index,`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ func ListChangedFiles(repo *Repository, base Revision, head Revision, comparison`
`22`	`22`
`23`	`23`	`log.Printf("ListChangedFiles Started. Repo: %s", repo.HttpURL)`
`24`	`24`
`25`		`- repoPath, err := UpdateOrClone(repo)`
	`25`	`+ repoPath, err := UpdateOrClone(repo, nil)`
`26`	`26`	`if err != nil {`
`27`	`27`	`return nil, err`
`28`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ func Search(repo Repository, rev Revision, options SearchOptions) ([]*File, er`
`31`	`31`	`rev,`
`32`	`32`	`)`
`33`	`33`
`34`		`- repoPath, err := UpdateOrClone(repo)`
	`34`	`+ repoPath, err := UpdateOrClone(repo, &rev)`
`35`	`35`	`if err != nil {`
`36`	`36`	`return nil, err`
`37`	`37`	`}`