workflows: allow customizing git fetch (#6470)

enterprise/server/cmd/ci_runner/main.go

@@ -151,6 +151,8 @@ var (
 	patchURIs          = flag.Slice("patch_uri", []string{}, "URIs of patches to apply to the repo after checkout. Can be specified multiple times to apply multiple patches.")
 	recordRunMetadata  = flag.Bool("record_run_metadata", false, "Instead of running a target, extract metadata about it and report it in the build event stream.")
 	gitCleanExclude    = flag.Slice("git_clean_exclude", []string{}, "Directories to exclude from `git clean` while setting up the repo.")
+	gitFetchFilters    = flag.Slice("git_fetch_filters", []string{}, "Filters to apply to `git fetch` commands.")
+	gitFetchDepth      = flag.Int("git_fetch_depth", 0, "Depth to use for `git fetch` commands.")
 
 	shutdownAndExit = flag.Bool("shutdown_and_exit", false, "If set, runs bazel shutdown with the configured bazel_command, and exits. No other commands are run.")
 
@@ -970,7 +972,7 @@ func (ar *actionRunner) Run(ctx context.Context, ws *workspace) error {
 		artifactsDir := artifactsPathForCommand(ws, i)
 		namedSetID := filepath.Base(artifactsDir)
 
-		runErr := runCommand(ctx, *bazelCommand, expandEnv(args), action.Env, action.BazelWorkspaceDir, ar.reporter)
+		runErr := runCommand(ctx, *bazelCommand, expandEnv(args), nil, action.BazelWorkspaceDir, ar.reporter)
 		exitCode := getExitCode(runErr)
 		ar.reporter.Publish(&bespb.BuildEvent{
 			Id: &bespb.BuildEventId{Id: &bespb.BuildEventId_TargetCompleted{
@@ -1533,7 +1535,7 @@ func (ws *workspace) setup(ctx context.Context) error {
 	if err := os.Chdir(repoDirName); err != nil {
 		return status.WrapErrorf(err, "cd %q", repoDirName)
 	}
-	if err := ws.clone(ctx, *targetRepoURL); err != nil {
+	if err := ws.init(ctx); err != nil {
 		return err
 	}
 	if err := ws.config(ctx); err != nil {
@@ -1687,6 +1689,10 @@ func (ws *workspace) config(ctx context.Context) error {
 		{"user.email", "ci-runner@buildbuddy.io"},
 		{"user.name", "BuildBuddy"},
 		{"advice.detachedHead", "false"},
+		// With the version of git that we have installed in the CI runner
+		// image, --filter=blob:none requires the partialClone extension to be
+		// enabled.
+		{"extensions.partialClone", "true"},
 	}
 	writeCommandSummary(ws.log, "Configuring repository...")
 	for _, kv := range cfg {
@@ -1713,16 +1719,9 @@ func (ws *workspace) config(ctx context.Context) error {
 	return nil
 }
 
-func (ws *workspace) clone(ctx context.Context, remoteURL string) error {
-	authURL, err := gitutil.AuthRepoURL(remoteURL, os.Getenv(repoUserEnvVarName), os.Getenv(repoTokenEnvVarName))
-	if err != nil {
-		return err
-	}
-	writeCommandSummary(ws.log, "Cloning target repo...")
-	// Don't show command since the URL may contain the repo access token.
-	args := []string{"clone", "--config=credential.helper=", "--filter=blob:none", "--no-checkout", authURL, "."}
-	if err := runCommand(ctx, "git", args, map[string]string{}, "" /*=dir*/, ws.log); err != nil {
-		return status.UnknownError("Command `git clone --filter=blob:none --no-checkout <url>` failed.")
+func (ws *workspace) init(ctx context.Context) error {
+	if _, err := git(ctx, ws.log, "init"); err != nil {
+		return status.UnknownError("git init failed")
 	}
 	return nil
 }
@@ -1751,7 +1750,15 @@ func (ws *workspace) fetch(ctx context.Context, remoteURL string, branches []str
 			return status.UnknownErrorf("Command `git remote add %q <url>` failed.", remoteName)
 		}
 	}
-	fetchArgs := append([]string{"-c", "credential.helper=", "fetch", "--filter=blob:none", "--force", remoteName}, branches...)
+	fetchArgs := []string{"-c", "credential.helper=", "fetch", "--force"}
+	for _, filter := range *gitFetchFilters {
+		fetchArgs = append(fetchArgs, "--filter="+filter)
+	}
+	if *gitFetchDepth > 0 {
+		fetchArgs = append(fetchArgs, fmt.Sprintf("--depth=%d", *gitFetchDepth))
+	}
+	fetchArgs = append(fetchArgs, remoteName)
+	fetchArgs = append(fetchArgs, branches...)
 	if _, err := git(ctx, ws.log, fetchArgs...); err != nil {
 		return err
 	}

enterprise/server/test/integration/ci_runner/ci_runner_test.go

@@ -193,6 +193,30 @@ actions:
 `,
 	}
 
+	workspaceContentsWithGitFetchDepth = map[string]string{
+		"WORKSPACE": `workspace(name = "test")`,
+		"BUILD":     `sh_binary(name = "fetch_depth_test", srcs = ["fetch_depth_test.sh"])`,
+		"fetch_depth_test.sh": `
+			cd "$BUILD_WORKSPACE_DIRECTORY"
+			# We should not have fetched the merge-base commit between the
+			# current branch and master, since the branches should have diverged
+			# and we're fetching with --depth=1.
+			MERGE_BASE=$(git merge-base origin/master origin/pr-branch)
+			echo "merge-base: '$MERGE_BASE' (should be empty)"
+			test -z "$MERGE_BASE" || exit 1
+`,
+		"buildbuddy.yaml": `
+actions:
+  - name: "Test"
+    triggers:
+      pull_request: { branches: [ master ], merge_with_base: false }
+      push: { branches: [ master ] }
+    git_fetch_depth: 1
+    bazel_commands:
+      - run :fetch_depth_test
+`,
+	}
+
 	invocationIDPattern = regexp.MustCompile(`Invocation URL:\s+.*?/invocation/([a-f0-9-]+)`)
 )
 
@@ -1015,6 +1039,29 @@ func TestFailedGitSetup_StillPublishesBuildMetadata(t *testing.T) {
 		"should publish workflow invocation metadata to BES despite failed repo setup")
 }
 
+func TestFetchFilters(t *testing.T) {
+	wsPath := testfs.MakeTempDir(t)
+	repoPath, headCommitSHA := makeGitRepo(t, workspaceContentsWithBazelVersionAction)
+	app := buildbuddy.Run(t)
+
+	runnerFlags := []string{
+		"--workflow_id=test-workflow",
+		"--action_name=Show bazel version",
+		"--trigger_event=push",
+		"--pushed_repo_url=file://" + repoPath,
+		"--pushed_branch=master",
+		"--commit_sha=" + headCommitSHA,
+		"--target_repo_url=file://" + repoPath,
+		"--target_branch=master",
+		"--git_fetch_filters=blob:none",
+	}
+	runnerFlags = append(runnerFlags, app.BESBazelFlags()...)
+
+	result := invokeRunner(t, runnerFlags, []string{}, wsPath)
+
+	checkRunnerResult(t, result)
+}
+
 func TestDisableBaseBranchMerging(t *testing.T) {
 	wsPath := testfs.MakeTempDir(t)
 	repoPath, headCommitSHA := makeGitRepo(t, workspaceContentsWithExitScriptAndMergeDisabled)
@@ -1048,6 +1095,42 @@ func TestDisableBaseBranchMerging(t *testing.T) {
 	checkRunnerResult(t, result)
 }
 
+func TestFetchDepth1(t *testing.T) {
+	wsPath := testfs.MakeTempDir(t)
+	repoPath, headCommitSHA := makeGitRepo(t, workspaceContentsWithGitFetchDepth)
+	testshell.Run(t, repoPath, `
+		# Create a PR branch
+		git checkout -b pr-branch
+
+		# Add a bad commit to the master branch;
+		# this should not break our CI run on the PR branch which doesn't have
+		# this change yet.
+		git checkout master
+		echo 'exit 1' > exit.sh
+		git add .
+		git commit -m "Fail"
+	`)
+
+	runnerFlags := []string{
+		"--workflow_id=test-workflow",
+		"--action_name=Test",
+		"--trigger_event=pull_request",
+		"--pushed_repo_url=file://" + repoPath,
+		"--pushed_branch=pr-branch",
+		"--commit_sha=" + headCommitSHA,
+		"--target_repo_url=file://" + repoPath,
+		"--target_branch=master",
+		// Need to set the fetch_depth flag even though it's set in the config,
+		// since this is required in order to fetch the config.
+		"--git_fetch_depth=1",
+	}
+	app := buildbuddy.Run(t)
+	runnerFlags = append(runnerFlags, app.BESBazelFlags()...)
+
+	result := invokeRunner(t, runnerFlags, nil, wsPath)
+	checkRunnerResult(t, result)
+}
+
 func TestArtifactUploads_GRPCLog(t *testing.T) {
 	wsPath := testfs.MakeTempDir(t)
 	repoPath, headCommitSHA := makeGitRepo(t, workspaceContentsWithArtifactUploads)

enterprise/server/workflow/config/BUILD

@@ -20,5 +20,6 @@ go_test(
         ":config",
         "//enterprise/server/workflow/config/test_data",
         "@com_github_stretchr_testify//assert",
+        "@com_github_stretchr_testify//require",
     ],
 )

enterprise/server/workflow/config/config.go

@@ -35,6 +35,8 @@ type Action struct {
 	ResourceRequests  ResourceRequests  `yaml:"resource_requests"`
 	User              string            `yaml:"user"`
 	GitCleanExclude   []string          `yaml:"git_clean_exclude"`
+	GitFetchFilters   []string          `yaml:"git_fetch_filters"`
+	GitFetchDepth     int               `yaml:"git_fetch_depth"`
 	BazelWorkspaceDir string            `yaml:"bazel_workspace_dir"`
 	Env               map[string]string `yaml:"env"`
 	BazelCommands     []string          `yaml:"bazel_commands"`
@@ -48,6 +50,16 @@ func (a *Action) GetTriggers() *Triggers {
 	return a.Triggers
 }
 
+func (a *Action) GetGitFetchFilters() []string {
+	if a.GitFetchFilters == nil {
+		// Default to blob:none if unspecified.
+		// TODO: this seems to increase fetch time in some cases;
+		// consider removing this as the default.
+		return []string{"blob:none"}
+	}
+	return a.GitFetchFilters
+}
+
 type Triggers struct {
 	Push        *PushTrigger        `yaml:"push"`
 	PullRequest *PullRequestTrigger `yaml:"pull_request"`

enterprise/server/workflow/config/config_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/workflow/config/test_data"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestWorkflowConf_Parse_BasicConfig_Valid(t *testing.T) {
@@ -79,3 +80,34 @@ func TestMatchesAnyTrigger_SupportsBasicWildcard(t *testing.T) {
 		assert.Equal(t, testCase.shouldMatch, match, "expected match(%q, %q) => %v", testCase.branchName, testCase.pattern, testCase.shouldMatch)
 	}
 }
+
+func TestGetGitFetchFilters(t *testing.T) {
+	for _, test := range []struct {
+		Name    string
+		YAML    string
+		Filters []string
+	}{
+		{
+			Name:    "Default",
+			YAML:    "",
+			Filters: []string{"blob:none"},
+		},
+		{
+			Name:    "EmptyList",
+			YAML:    "git_fetch_filters: []",
+			Filters: []string{},
+		},
+		{
+			Name:    "NonEmptyList",
+			YAML:    `git_fetch_filters: ["tree:0"]`,
+			Filters: []string{"tree:0"},
+		},
+	} {
+		t.Run(test.Name, func(t *testing.T) {
+			s := `actions: [ { name: Test, ` + test.YAML + ` } ]`
+			cfg, err := config.NewConfig(strings.NewReader(s))
+			require.NoError(t, err)
+			require.Equal(t, test.Filters, cfg.Actions[0].GetGitFetchFilters())
+		})
+	}
+}

enterprise/server/workflow/service/service.go

@@ -1169,35 +1169,44 @@ func (ws *workflowService) createActionForWorkflow(ctx context.Context, wf *tabl
 		return nil, err
 	}
 
+	args := []string{
+		// NOTE: The executor is responsible for making sure this
+		// buildbuddy_ci_runner binary exists at the workspace root. It does so
+		// whenever it sees the `workflow-id` platform property.
+		//
+		// Also be cautious when adding new flags. See
+		// https://github.com/buildbuddy-io/buildbuddy-internal/issues/3101
+		"./buildbuddy_ci_runner",
+		"--invocation_id=" + invocationID,
+		"--action_name=" + workflowAction.Name,
+		"--bes_backend=" + events_api_url.String(),
+		"--bes_results_url=" + besResultsURL,
+		"--cache_backend=" + cache_api_url.String(),
+		"--rbe_backend=" + remote_exec_api_url.String(),
+		"--remote_instance_name=" + instanceName,
+		"--commit_sha=" + wd.SHA,
+		"--pushed_repo_url=" + wd.PushedRepoURL,
+		"--pushed_branch=" + wd.PushedBranch,
+		"--pull_request_number=" + fmt.Sprintf("%d", wd.PullRequestNumber),
+		"--target_repo_url=" + wd.TargetRepoURL,
+		"--target_branch=" + wd.TargetBranch,
+		"--visibility=" + visibility,
+		"--workflow_id=" + wf.WorkflowID,
+		"--trigger_event=" + wd.EventName,
+		"--bazel_command=" + ws.ciRunnerBazelCommand(),
+		"--debug=" + fmt.Sprintf("%v", ws.ciRunnerDebugMode()),
+	}
+	for _, filter := range workflowAction.GetGitFetchFilters() {
+		args = append(args, "--git_fetch_filters="+filter)
+	}
+	for _, path := range workflowAction.GitCleanExclude {
+		args = append(args, "--git_clean_exclude="+path)
+	}
+	args = append(args, extraArgs...)
+
 	cmd := &repb.Command{
 		EnvironmentVariables: envVars,
-		Arguments: append([]string{
-			// NOTE: The executor is responsible for making sure this
-			// buildbuddy_ci_runner binary exists at the workspace root. It does so
-			// whenever it sees the `workflow-id` platform property.
-			//
-			// Also be cautious when adding new flags. See
-			// https://github.com/buildbuddy-io/buildbuddy-internal/issues/3101
-			"./buildbuddy_ci_runner",
-			"--invocation_id=" + invocationID,
-			"--action_name=" + workflowAction.Name,
-			"--bes_backend=" + events_api_url.String(),
-			"--bes_results_url=" + besResultsURL,
-			"--cache_backend=" + cache_api_url.String(),
-			"--rbe_backend=" + remote_exec_api_url.String(),
-			"--remote_instance_name=" + instanceName,
-			"--commit_sha=" + wd.SHA,
-			"--pushed_repo_url=" + wd.PushedRepoURL,
-			"--pushed_branch=" + wd.PushedBranch,
-			"--pull_request_number=" + fmt.Sprintf("%d", wd.PullRequestNumber),
-			"--target_repo_url=" + wd.TargetRepoURL,
-			"--target_branch=" + wd.TargetBranch,
-			"--visibility=" + visibility,
-			"--workflow_id=" + wf.WorkflowID,
-			"--trigger_event=" + wd.EventName,
-			"--bazel_command=" + ws.ciRunnerBazelCommand(),
-			"--debug=" + fmt.Sprintf("%v", ws.ciRunnerDebugMode()),
-		}, extraArgs...),
+		Arguments:            args,
 		Platform: &repb.Platform{
 			Properties: []*repb.Platform_Property{
 				{Name: "Pool", Value: ws.poolForAction(workflowAction)},
@@ -1239,9 +1248,6 @@ func (ws *workflowService) createActionForWorkflow(ctx context.Context, wf *tabl
 			Value: "true",
 		})
 	}
-	for _, path := range workflowAction.GitCleanExclude {
-		cmd.Arguments = append(cmd.Arguments, "--git_clean_exclude="+path)
-	}
 	cmdDigest, err := cachetools.UploadProtoToCAS(ctx, cache, instanceName, repb.DigestFunction_SHA256, cmd)
 	if err != nil {
 		return nil, err