refactor: re-organize PR linter (#32969)

The PR linter code was a bit of a mess; evaluating rules and mutating the PR was interspersed, generic GitHub code was mixed with CDK-specific code, the linter could be triggered from multiple sources, none of them were documented very well.

Try to rectify all of that in this PR to make it easier to extend the PR linter in the future:

- Split the linter into clear evaluate/act responsibilities.
- Split code across more than 1 file.
- Document how the "PR Linter Trigger" works
- Streamline how we get a PR number into the linter.
- Give an example of how to run it locally to test the rule evaluation on real PRs

Not every crazy design decision has been rectified yet, but at least we have a start of something a little more comprehensible. Another change I made: the old PR linter creates a comment + a review with the same content (but not quite). In this PR, make it just do reviews and don't do comments.

This started from a PR that had CodeCov changes added, but I want to do a refactor without feature changes first before adding new code.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

.github/workflows/pr-linter-trigger.yml → .github/workflows/pr-linter-review-trigger.yml

@@ -1,3 +1,12 @@
+# Re-evaluate the PR linter after reviews. This is used to upgrade the label
+# of a PR to `needs-maintainer-review` after a trusted community members leaves
+# an approving review.
+#
+# Unprivileged workflow that runs in the context of the PR, when a review is changed.
+#
+# Save the PR number, and download it again in the PR Linter workflow which
+# needs to run in privileged `workflow_run` context (but then must restore the
+# PR context).
 name: PR Linter Trigger
 
 on:

.github/workflows/pr-linter.yml

@@ -26,39 +26,29 @@ jobs:
     # if conditions on all individual steps because subsequent jobs depend on this job
     # and we cannot skip it entirely
     steps:
-      - name: 'Download artifact'
+      - name: 'Download workflow_run artifact'
         if: github.event_name == 'workflow_run'
-        uses: actions/github-script@v7
+        uses: dawidd6/action-download-artifact@v7
         with:
-          script: |
-            let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
-               owner: context.repo.owner,
-               repo: context.repo.repo,
-               run_id: context.payload.workflow_run.id,
-            });
-            let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
-              return artifact.name == "pr_info"
-            })[0];
-            let download = await github.rest.actions.downloadArtifact({
-               owner: context.repo.owner,
-               repo: context.repo.repo,
-               artifact_id: matchArtifact.id,
-               archive_format: 'zip',
-            });
-            let fs = require('fs');
-            fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/pr_info.zip`, Buffer.from(download.data));
-      - name: 'Unzip artifact'
-        if: github.event_name == 'workflow_run'
-        run: unzip pr_info.zip
+          run_id: ${{ github.event.workflow_run.id }}
+          name: pr_info
+          path: pr/
+          search_artifacts: true
 
-      - name: 'Make GitHub output'
+      - name: 'Determine PR info'
+        # PR info comes from the artifact if downloaded, or GitHub context if not.
         if: github.event_name == 'workflow_run'
         id: 'pr_output'
         run: |
-          echo "cat pr_number"
-          echo "pr_number=$(cat pr_number)" >> "$GITHUB_OUTPUT"
-          echo "cat pr_sha"
-          echo "pr_sha=$(cat pr_sha)" >> "$GITHUB_OUTPUT"
+          if [[ ! -f pr/pr_number ]]; then
+            echo "${{ github.event.pull_request.number }}" > pr/pr_number
+          fi
+          if [[ ! -f pr/pr_sha ]]; then
+            echo "${{ github.event.pull_request.head.sha }}" > pr/pr_sha
+          fi
+          cat pr/*
+          echo "pr_number=$(cat pr/pr_number)" >> "$GITHUB_OUTPUT"
+          echo "pr_sha=$(cat pr/pr_sha)" >> "$GITHUB_OUTPUT"
 
   validate-pr:
     # Necessary to have sufficient permissions to write to the PR
@@ -80,7 +70,7 @@ jobs:
         uses: ./tools/@aws-cdk/prlint
         env:
           GITHUB_TOKEN: ${{ secrets.PROJEN_GITHUB_TOKEN }}
-          # PR_NUMBER and PR_SHA is empty if triggered by pull_request_target, since we already have that info
           PR_NUMBER: ${{ needs.download-if-workflow-run.outputs.pr_number }}
           PR_SHA: ${{ needs.download-if-workflow-run.outputs.pr_sha }}
+          LINTER_LOGIN: ${{ vars.LINTER_LOGIN }}
           REPO_ROOT: ${{ github.workspace }}

tools/@aws-cdk/prlint/constants.ts

@@ -0,0 +1,18 @@
+
+export const DEFEAULT_LINTER_LOGIN = 'aws-cdk-automation';
+
+export const CODE_BUILD_CONTEXT = 'AWS CodeBuild us-east-1 (AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv)';
+
+/**
+ * Types of exemption labels in aws-cdk project.
+ */
+export enum Exemption {
+  README = 'pr-linter/exempt-readme',
+  TEST = 'pr-linter/exempt-test',
+  INTEG_TEST = 'pr-linter/exempt-integ-test',
+  BREAKING_CHANGE = 'pr-linter/exempt-breaking-change',
+  CLI_INTEG_TESTED = 'pr-linter/cli-integ-tested',
+  REQUEST_CLARIFICATION = 'pr/reviewer-clarification-requested',
+  REQUEST_EXEMPTION = 'pr-linter/exemption-requested',
+  CODECOV = "pr-linter/exempt-codecov",
+}

tools/@aws-cdk/prlint/github.ts

@@ -0,0 +1,33 @@
+import { Endpoints } from '@octokit/types';
+import { StatusEvent } from '@octokit/webhooks-definitions/schema';
+
+export type GitHubPr =
+  Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}']['response']['data'];
+
+
+export interface GitHubComment {
+  id: number;
+}
+
+export interface Review {
+  id: number;
+  user: {
+    login: string;
+  } | null;
+  body: string;
+  state: string;
+}
+
+export interface GithubStatusEvent {
+  readonly sha: string;
+  readonly state?: StatusEvent['state'];
+  readonly context?: string;
+}
+
+export interface GitHubLabel {
+  readonly name: string;
+}
+
+export interface GitHubFile {
+  readonly filename: string;
+}

tools/@aws-cdk/prlint/index.ts

@@ -2,55 +2,109 @@ import * as core from '@actions/core';
 import * as github from '@actions/github';
 import { Octokit } from '@octokit/rest';
 import { StatusEvent, PullRequestEvent } from '@octokit/webhooks-definitions/schema';
-import * as linter from './lint';
+import { PullRequestLinter } from './lint';
+import { LinterActions } from './linter-base';
+import { DEFEAULT_LINTER_LOGIN } from './constants';
 
+/**
+ * Entry point for PR linter
+ *
+ * This gets run as a GitHub action.
+ *
+ * To test locally, do the following:
+ *
+ * ```
+ * env GITHUB_TOKEN=$(cat ~/.my-github-token) LINTER_LOGIN=my-gh-alias GITHUB_REPOSITORY=aws/aws-cdk PR_NUMBER=1234 npx ts-node ./index
+ * ```
+ */
 async function run() {
   const token: string = process.env.GITHUB_TOKEN!;
   const client = new Octokit({ auth: token });
 
+  const owner = github.context.repo.owner;
+  const repo = github.context.repo.repo;
+
+  const number = await determinePrNumber(client);
+  if (!number) {
+    if (github.context.eventName === 'status') {
+      console.error(`Could not find PR belonging to status event, but that's not unusual. Skipping.`);
+      process.exit(0);
+    }
+    throw new Error(`Could not find PR number from event: ${github.context.eventName}`);
+  }
+
   try {
+    const prLinter = new PullRequestLinter({
+      client,
+      owner,
+      repo,
+      number,
+      // On purpose || instead of ??, also collapse empty string
+      linterLogin: process.env.LINTER_LOGIN || DEFEAULT_LINTER_LOGIN,
+    });
+
+    let actions: LinterActions | undefined;
+
     switch (github.context.eventName) {
       case 'status':
+        // Triggering on a 'status' event is solely used to see if the CodeBuild
+        // job just turned green, and adding certain 'ready for review' labels
+        // if it does.
         const statusPayload = github.context.payload as StatusEvent;
-        const pr = await linter.PullRequestLinter.getPRFromCommit(client, 'aws', 'aws-cdk', statusPayload.sha);
-        if (pr) {
-          const prLinter = new linter.PullRequestLinter({
-            client,
-            owner: 'aws',
-            repo: 'aws-cdk',
-            number: pr.number,
-          });
-          console.log('validating status event');
-          await prLinter.validateStatusEvent(pr, github.context.payload as StatusEvent);
-        }
-        break;
-      case 'workflow_run':
-        const prNumber = process.env.PR_NUMBER;
-        const prSha = process.env.PR_SHA;
-        if (!prNumber || !prSha) {
-          throw new Error(`Cannot have undefined values in: ${prNumber} pr number and ${prSha} pr sha.`)
-        }
-        const workflowPrLinter = new linter.PullRequestLinter({
-          client,
-          owner: github.context.repo.owner,
-          repo: github.context.repo.repo,
-          number: Number(prNumber),
-        });
-        await workflowPrLinter.validatePullRequestTarget(prSha);
+        console.log('validating status event');
+        actions = await prLinter.validateStatusEvent(statusPayload);
         break;
+
       default:
-        const payload = github.context.payload as PullRequestEvent;
-        const prLinter = new linter.PullRequestLinter({
-          client,
-          owner: github.context.repo.owner,
-          repo: github.context.repo.repo,
-          number: github.context.issue.number,
-        });
-        await prLinter.validatePullRequestTarget(payload.pull_request.head.sha);
+        // This is the main PR validator action.
+        actions = await prLinter.validatePullRequestTarget();
+        break;
+    }
+
+    if (actions) {
+      console.log(actions);
+
+      if (github.context.eventName || process.env.FOR_REAL) {
+        console.log('Carrying out actions');
+
+        // Actually running in GitHub actions, do it (otherwise we're just testing)
+        await prLinter.executeActions(actions);
+      }
+
+      await prLinter.actionsToException(actions);
     }
+
   } catch (error: any) {
+    // Fail the action
     core.setFailed(error.message);
   }
 }
 
-void run();
+async function determinePrNumber(client: Octokit): Promise<number | undefined> {
+  const owner = github.context.repo.owner;
+  const repo = github.context.repo.repo;
+
+  if (process.env.PR_NUMBER) {
+    return Number(process.env.PR_NUMBER);
+  }
+  if (github.context.eventName.startsWith('pull_request')) {
+    return (github.context.payload as PullRequestEvent).pull_request.number;
+  }
+
+  // If we don't have PR_NUMBER, try to find a SHA and convert that into a PR number
+  let sha = process.env.PR_SHA;
+  if (!sha && github.context.eventName === 'status') {
+    sha = (github.context.payload as StatusEvent)?.sha;
+  }
+  if (!sha) {
+    throw new Error(`Could not determine a SHA from either \$PR_SHA or ${JSON.stringify(github.context.payload)}`);
+  }
+
+  const pr = await PullRequestLinter.getPRFromCommit(client, owner, repo, sha);
+  return pr?.number;
+}
+
+run().catch(e => {
+  console.error(e);
+  process.exitCode = 1;
+});