feat: Support an Impacts All PATH Filter (#39)

- Move the sha computation into `prerequisites`. - If any paths are added/removed/modified via [dorny's path filter](https://github.com/dorny/paths-filter), skip impacted target computation and upload `ALL`. Tested using `netkos/act` against a temporary merge graph in staging: https://www.loom.com/share/Library-or-Loom-14-September-2023-4edd8ed7f45240718f4180a3543585dc Closes https://linear.app/trunk/issue/TRUNK-8676/support-wildcard-impacted-target
trunk-io · Sep 15, 2023 · 6a6b856 · 6a6b856
1 parent 6c5a6fc
commit 6a6b856
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 55 deletions.
diff --git a/.github/workflows/compute_impacted_tests.yaml b/.github/workflows/compute_impacted_tests.yaml
@@ -10,6 +10,8 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
 
       - name: Setup Bazel
         # trunk-ignore(semgrep): Trust third-party `bazelbuild` GH Action
@@ -20,8 +22,10 @@ jobs:
         run: ./src/scripts/compute_impacted_targets.sh
         shell: bash
         env:
-          MERGE_INSTANCE_BRANCH: main
+          MERGE_INSTANCE_BRANCH: do_not_delete/stable_test_branch
+          MERGE_INSTANCE_BRANCH_HEAD_SHA: 097c8259c2e18da92f6189849ebc0f7f6dc624e5
           PR_BRANCH: do_not_delete/stable_test_branch
+          PR_BRANCH_HEAD_SHA: 097c8259c2e18da92f6189849ebc0f7f6dc624e5
           VERBOSE: 1
           WORKSPACE_PATH: ./tests/simple_bazel_workspace
           BAZEL_STARTUP_OPTIONS: --host_jvm_args=-Xmx12G,--block_for_lock,--client_debug
@@ -31,6 +35,6 @@ jobs:
         shell: bash
         run: |
           target_count=`cat ${{ steps.compute.outputs.impacted_targets_out }} | wc -l`
-          if [[ $target_count -ne 2 ]]; then
+          if [[ $target_count -ne 0 ]]; then
             exit 1
           fi
diff --git a/action.yaml b/action.yaml
@@ -29,36 +29,56 @@ inputs:
     description: A path to the Bazel executable. Defaults to PATH.
     required: false
     default: bazel
+  impact-all-filters-path:
+    description:
+      A path to a list of filters to identify whether `ALL` impacted targets should be considered.
+      See https://github.com/dorny/paths-filter/blob/master/.github/filters.yml for an example.
+    required: false
+    default: ""
 
 runs:
   using: composite
   steps:
+    - name: Detect changed paths
+      id: detect-changed-paths
+      if: ${{ inputs.impact-all-filters-path != '' }}
+      # trunk-ignore(semgrep/yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha.third-party-action-not-pinned-to-commit-sha)
+      uses: dorny/paths-filter@v2
+      with:
+        filters: ${{ inputs.impact-all-filters-path }}
+
     - name: Prerequisites
       id: prerequisites
       run: ${GITHUB_ACTION_PATH}/src/scripts/prerequisites.sh
       shell: bash
       env:
         DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
         TARGET_BRANCH: ${{ inputs.target-branch }}
+        PR_BRANCH: ${{ github.head_ref }}
         WORKSPACE_PATH: ${{ inputs.bazel-workspace-path }}
         BAZEL_PATH: ${{ inputs.bazel-path }}
+        IMPACTS_FILTERS_CHANGES: ${{ steps.detect-changed-paths.outputs.changes }}
+
+    - name: Setup jq
+      # trunk-ignore(semgrep): Trust third-party action to install JQ. Source code: https://github.com/dcarbone/install-jq-action/
+      uses: dcarbone/[email protected]
 
     - name: Install Bazel in PATH
       if: ${{ steps.prerequisites.outputs.requires_default_bazel_installation == 'true' }}
       # trunk-ignore(semgrep): Trust third-party `bazelbuild` GH Action
       uses: bazelbuild/setup-bazelisk@v2
 
-    - name: Setup jq
-      # trunk-ignore(semgrep): Trust third-party action to install JQ. Source code: https://github.com/dcarbone/install-jq-action/
-      uses: dcarbone/[email protected]
-
     - name: Compute Impacted Targets
       id: compute-impacted-targets
       run: ${GITHUB_ACTION_PATH}/src/scripts/compute_impacted_targets.sh
+      if: ${{ steps.prerequisites.outputs.impacts_all_detected == 'false' }}
       shell: bash
       env:
         MERGE_INSTANCE_BRANCH: ${{ steps.prerequisites.outputs.merge_instance_branch }}
-        PR_BRANCH: ${{ github.head_ref }}
+        MERGE_INSTANCE_BRANCH_HEAD_SHA:
+          ${{ steps.prerequisites.outputs.merge_instance_branch_head_sha }}
+        PR_BRANCH: ${{ steps.prerequisites.outputs.pr_branch }}
+        PR_BRANCH_HEAD_SHA: ${{ steps.prerequisites.outputs.pr_branch_head_sha }}
         VERBOSE: ${{ inputs.verbose }}
         WORKSPACE_PATH: ${{ steps.prerequisites.outputs.workspace_path }}
         BAZEL_PATH: ${{ inputs.bazel-path }}
@@ -73,5 +93,6 @@ runs:
         REPOSITORY: ${{ github.repository }}
         TARGET_BRANCH: ${{ steps.prerequisites.outputs.merge_instance_branch }}
         PR_NUMBER: ${{ github.event.pull_request.number }}
-        PR_SHA: ${{ steps.compute-impacted-targets.outputs.git_commit }}
+        PR_SHA: ${{ steps.prerequisites.outputs.pr_branch_head_sha }}
         IMPACTED_TARGETS_FILE: ${{ steps.compute-impacted-targets.outputs.impacted_targets_out }}
+        IMPACTS_ALL_DETECTED: ${{ steps.prerequisites.outputs.impacts_all_detected }}
diff --git a/src/scripts/compute_impacted_targets.sh b/src/scripts/compute_impacted_targets.sh
@@ -8,6 +8,11 @@ if [[ (-z ${MERGE_INSTANCE_BRANCH}) || (-z ${PR_BRANCH}) ]]; then
 	exit 2
 fi
 
+if [[ (-z ${MERGE_INSTANCE_BRANCH_HEAD_SHA}) || (-z ${PR_BRANCH_HEAD_SHA}) ]]; then
+	echo "Missing sha"
+	exit 2
+fi
+
 if [[ -z ${WORKSPACE_PATH} ]]; then
 	echo "Missing workspace path"
 	exit 2
@@ -56,32 +61,21 @@ fetchRemoteGitHistory() {
 	logIfVerbose "...done!"
 }
 
-fetchRemoteGitHistory "${MERGE_INSTANCE_BRANCH}"
-fetchRemoteGitHistory "${PR_BRANCH}"
-
-git switch "${MERGE_INSTANCE_BRANCH}"
-merge_instance_branch_head_sha=$(git rev-parse "${MERGE_INSTANCE_BRANCH}")
-ifVerbose echo "Merge Instance Branch Head= ${merge_instance_branch_head_sha}"
-
-git switch "${PR_BRANCH}"
-pr_branch_head_sha=$(git rev-parse "${PR_BRANCH}")
-ifVerbose echo "PR Branch Head= ${pr_branch_head_sha}"
-
 ## Verbose logging for the Merge Instance and PR branch.
 if [[ -n ${VERBOSE} ]]; then
 	# Find the merge base of the two branches
-	merge_base_sha=$(git merge-base "${merge_instance_branch_head_sha}" "${pr_branch_head_sha}")
+	merge_base_sha=$(git merge-base "${MERGE_INSTANCE_BRANCH_HEAD_SHA}" "${PR_BRANCH_HEAD_SHA}")
 	echo "Merge Base= ${merge_base_sha}"
 
 	# Find the number of commits between the merge base and the merge instance's HEAD
-	merge_instance_depth=$(git rev-list "${merge_base_sha}".."${merge_instance_branch_head_sha}" | wc -l)
+	merge_instance_depth=$(git rev-list "${merge_base_sha}".."${MERGE_INSTANCE_BRANCH_HEAD_SHA}" | wc -l)
 	echo "Merge Instance Depth= ${merge_instance_depth}"
 
 	git switch "${MERGE_INSTANCE_BRANCH}"
 	git log -n "${merge_instance_depth}" --oneline
 
 	# Find the number of commits between the merge base and the PR's HEAD
-	pr_depth=$(git rev-list "${merge_base_sha}".."${pr_branch_head_sha}" | wc -l)
+	pr_depth=$(git rev-list "${merge_base_sha}".."${PR_BRANCH_HEAD_SHA}" | wc -l)
 	echo "PR Depth= ${pr_depth}"
 
 	git switch "${PR_BRANCH}"
@@ -94,9 +88,9 @@ _java -jar bazel-diff.jar -V
 _bazel version # Does not require running with startup options.
 
 # Output Files
-merge_instance_branch_out=./${merge_instance_branch_head_sha}
-merge_instance_with_pr_branch_out=./${pr_branch_head_sha}_${merge_instance_branch_head_sha}
-impacted_targets_out=./impacted_targets_${pr_branch_head_sha}
+merge_instance_branch_out=./${MERGE_INSTANCE_BRANCH_HEAD_SHA}
+merge_instance_with_pr_branch_out=./${PR_BRANCH_HEAD_SHA}_${MERGE_INSTANCE_BRANCH_HEAD_SHA}
+impacted_targets_out=./impacted_targets_${PR_BRANCH_HEAD_SHA}
 
 # Generate Hashes for the Merge Instance Branch
 git switch "${MERGE_INSTANCE_BRANCH}"
@@ -110,8 +104,7 @@ bazelDiff generate-hashes --bazelPath="${BAZEL_PATH}" --workspacePath="${WORKSPA
 bazelDiff get-impacted-targets --startingHashes="${merge_instance_branch_out}" --finalHashes="${merge_instance_with_pr_branch_out}" --output="${impacted_targets_out}"
 
 num_impacted_targets=$(wc -l <"${impacted_targets_out}")
-echo "Computed ${num_impacted_targets} targets for sha ${pr_branch_head_sha}"
+echo "Computed ${num_impacted_targets} targets for sha ${PR_BRANCH_HEAD_SHA}"
 
 # Outputs
-echo "git_commit=${pr_branch_head_sha}" >>"${GITHUB_OUTPUT}"
 echo "impacted_targets_out=${impacted_targets_out}" >>"${GITHUB_OUTPUT}"
diff --git a/src/scripts/prerequisites.sh b/src/scripts/prerequisites.sh
@@ -2,6 +2,15 @@
 
 set -euo pipefail
 
+# NOTE: We cannot assume that the checked out Git repo (e.g. via actions-checkout)
+# was a shallow vs a complete clone. The `--depth` options deepens the commit history
+# in both clone modes: https://git-scm.com/docs/fetch-options#Documentation/fetch-options.txt---depthltdepthgt
+fetchRemoteGitHistory() {
+	git fetch --quiet --depth=2147483647 origin "$@"
+}
+
+# trunk-ignore(shellcheck)
+pr_branch="${PR_BRANCH}"
 merge_instance_branch="${TARGET_BRANCH}"
 if [[ -z ${merge_instance_branch} ]]; then
 	merge_instance_branch="${DEFAULT_BRANCH}"
@@ -25,8 +34,33 @@ if [[ ${BAZEL_PATH} == "bazel" ]]; then
 	fi
 fi
 
+changes_count=0
+impacts_all_detected="false"
+if [[ -n ${IMPACTS_FILTERS_CHANGES+x} ]]; then
+	changes_count=$(echo "${IMPACTS_FILTERS_CHANGES}" | jq length)
+	if [[ ${changes_count} -gt 0 ]]; then
+		impacts_all_detected="true"
+		requires_default_bazel_installation="false"
+	fi
+fi
+
+fetchRemoteGitHistory "${merge_instance_branch}"
+fetchRemoteGitHistory "${pr_branch}"
+
+git switch "${merge_instance_branch}"
+merge_instance_branch_head_sha=$(git rev-parse "${merge_instance_branch}")
+
+git switch "${pr_branch}"
+pr_branch_head_sha=$(git rev-parse "${pr_branch}")
+
+echo "Identified changes: " "${impacts_all_detected}"
+
 # Outputs
 # trunk-ignore(shellcheck/SC2129)
 echo "merge_instance_branch=${merge_instance_branch}" >>"${GITHUB_OUTPUT}"
+echo "merge_instance_branch_head_sha=${merge_instance_branch_head_sha}" >>"${GITHUB_OUTPUT}"
+echo "pr_branch=${pr_branch}" >>"${GITHUB_OUTPUT}"
+echo "pr_branch_head_sha=${pr_branch_head_sha}" >>"${GITHUB_OUTPUT}"
+echo "impacts_all_detected=${impacts_all_detected}" >>"${GITHUB_OUTPUT}"
 echo "workspace_path=${workspace_path}" >>"${GITHUB_OUTPUT}"
 echo "requires_default_bazel_installation=${requires_default_bazel_installation}" >>"${GITHUB_OUTPUT}"
diff --git a/src/scripts/upload_impacted_targets.sh b/src/scripts/upload_impacted_targets.sh
@@ -22,27 +22,11 @@ if [[ (-z ${PR_NUMBER}) || (-z ${PR_SHA}) ]]; then
 	exit 2
 fi
 
-if [[ -z ${IMPACTED_TARGETS_FILE+x} ]]; then
-	echo "Missing Impacted Targets File"
-	exit 2
-fi
-
 # API URL
 if [[ -z ${API_URL+x} ]]; then
 	API_URL="https://api.trunk.io:443/v1/setImpactedTargets"
 fi
 
-# Reformat the impacted targets into JSON array and pipe into a new file.
-IMPACTED_TARGETS_JSON_TMP="./impacted_targets_json_tmp"
-touch "${IMPACTED_TARGETS_JSON_TMP}"
-mapfile -t impacted_targets_array <"${IMPACTED_TARGETS_FILE}"
-IMPACTED_TARGETS=$(printf '%s\n' "${impacted_targets_array[@]}" | jq -R . | jq -s .)
-if [[ -z ${IMPACTED_TARGETS} ]]; then
-	echo "[]" >"${IMPACTED_TARGETS_JSON_TMP}"
-else
-	echo "${IMPACTED_TARGETS}" >"${IMPACTED_TARGETS_JSON_TMP}"
-fi
-
 REPO_BODY=$(
 	jq --null-input \
 		--arg host "github.com" \
@@ -58,14 +42,40 @@ PR_BODY=$(
 		'{ "number": $number, "sha": $sha }'
 )
 
+num_impacted_targets=""
 POST_BODY="./post_body_tmp"
-jq --null-input \
-	--argjson repo "${REPO_BODY}" \
-	--argjson pr "${PR_BODY}" \
-	--slurpfile impactedTargets "${IMPACTED_TARGETS_JSON_TMP}" \
-	--arg targetBranch "${TARGET_BRANCH}" \
-	'{ "repo": $repo, "pr": $pr, "targetBranch": $targetBranch, "impactedTargets": $impactedTargets | .[0] | map(select(length > 0)) }' \
-	>"${POST_BODY}"
+if [[ ${IMPACTS_ALL_DETECTED} == 'true' ]]; then
+	jq --null-input \
+		--argjson repo "${REPO_BODY}" \
+		--argjson pr "${PR_BODY}" \
+		--arg impactedTargets "ALL" \
+		--arg targetBranch "${TARGET_BRANCH}" \
+		'{ "repo": $repo, "pr": $pr, "targetBranch": $targetBranch, "impactedTargets": $impactedTargets }' \
+		>"${POST_BODY}"
+
+	num_impacted_targets="'ALL'"
+else
+	# Reformat the impacted targets into JSON array and pipe into a new file.
+	IMPACTED_TARGETS_JSON_TMP="./impacted_targets_json_tmp"
+	touch "${IMPACTED_TARGETS_JSON_TMP}"
+	mapfile -t impacted_targets_array <"${IMPACTED_TARGETS_FILE}"
+	IMPACTED_TARGETS=$(printf '%s\n' "${impacted_targets_array[@]}" | jq -R . | jq -s .)
+	if [[ -z ${IMPACTED_TARGETS} ]]; then
+		echo "[]" >"${IMPACTED_TARGETS_JSON_TMP}"
+	else
+		echo "${IMPACTED_TARGETS}" >"${IMPACTED_TARGETS_JSON_TMP}"
+	fi
+
+	jq --null-input \
+		--argjson repo "${REPO_BODY}" \
+		--argjson pr "${PR_BODY}" \
+		--slurpfile impactedTargets "${IMPACTED_TARGETS_JSON_TMP}" \
+		--arg targetBranch "${TARGET_BRANCH}" \
+		'{ "repo": $repo, "pr": $pr, "targetBranch": $targetBranch, "impactedTargets": $impactedTargets | .[0] | map(select(length > 0)) }' \
+		>"${POST_BODY}"
+
+	num_impacted_targets=$(wc -l <"${IMPACTED_TARGETS_FILE}")
+fi
 
 HTTP_STATUS_CODE=$(
 	curl -s -o /dev/null -w '%{http_code}' -X POST \
@@ -77,7 +87,6 @@ HTTP_STATUS_CODE=$(
 EXIT_CODE=0
 COMMENT_TEXT=""
 if [[ ${HTTP_STATUS_CODE} == 200 ]]; then
-	num_impacted_targets=$(wc -l <"${IMPACTED_TARGETS_FILE}")
 	COMMENT_TEXT="✨ Uploaded ${num_impacted_targets} impacted targets for ${PR_NUMBER} @ ${PR_SHA}"
 else
 	EXIT_CODE=1

diff --git a/tests/upload.test.ts b/tests/upload.test.ts
@@ -9,6 +9,8 @@ import util from "node:util";
 
 const PORT = 4567;
 
+type ImpactedTargets = string[] | "ALL";
+
 const fetchUrl = (path: string) => `http://localhost:${PORT}${path}`;
 const UPLOAD_IMPACTED_TARGETS_SCRIPT = "src/scripts/upload_impacted_targets.sh";
 const ENV_VARIABLES: Record<string, string> = {
@@ -18,6 +20,7 @@ const ENV_VARIABLES: Record<string, string> = {
   PR_NUMBER: "123",
   PR_SHA: "test-pr-sha",
   IMPACTED_TARGETS_FILE: "/tmp/test-impacted-targets-file",
+  IMPACTS_ALL_DETECTED: "false",
   API_URL: fetchUrl("/testUploadImpactedTargets"),
 };
 const exportEnv = (env: Record<string, string>) =>
@@ -32,12 +35,14 @@ let server: http.Server;
 let uploadedImpactedTargetsPayload = [null, null];
 
 const runUploadTargets = async (
-  impactedTargets: string[],
+  impactedTargets: ImpactedTargets,
   env: Record<string, string> = ENV_VARIABLES,
 ) => {
   // The bazel / glob / ... scripts are responsible for populating these files.
   // Verify that the upload works as intended.
-  fs.writeFileSync(env.IMPACTED_TARGETS_FILE, impactedTargets.join("\n"));
+  if (impactedTargets !== "ALL") {
+    fs.writeFileSync(env.IMPACTED_TARGETS_FILE, impactedTargets.join("\n"));
+  }
 
   const runScript = util.promisify(exec.exec)(
     `${exportEnv(env)} ${UPLOAD_IMPACTED_TARGETS_SCRIPT}`,
@@ -46,7 +51,7 @@ const runUploadTargets = async (
   await runScript;
 };
 
-const expectImpactedTargetsUpload = (impactedTargets: string[]): void => {
+const expectImpactedTargetsUpload = (impactedTargets: ImpactedTargets): void => {
   const { API_TOKEN, REPOSITORY, TARGET_BRANCH, PR_NUMBER, PR_SHA } = ENV_VARIABLES;
   const [actualToken, actualBody] = uploadedImpactedTargetsPayload;
   expect(actualToken).toEqual(API_TOKEN);
@@ -121,11 +126,17 @@ test("supports 1K targets", async function () {
 });
 
 test("supports 100K targets", async function () {
-  const impactedTargets = [...new Array(1_000)].map((_, i) => `target-${i}`);
+  const impactedTargets = [...new Array(100_000)].map((_, i) => `target-${i}`);
   await runUploadTargets(impactedTargets);
   expectImpactedTargetsUpload(impactedTargets);
 });
 
+test("supports IMPACTS_ALL", async function () {
+  const env = { ...ENV_VARIABLES, IMPACTS_ALL_DETECTED: "true" };
+  await runUploadTargets("ALL", env);
+  expectImpactedTargetsUpload("ALL");
+});
+
 test("rejects when missing API token", async function () {
   await expect(runUploadTargets([], _.omit(ENV_VARIABLES, "API_TOKEN"))).rejects.toBeTruthy();
 });