fix: external-prs better error messages (#1527)

* Adds an .env.example so folks know how to configure this for local
  testing
* Reports in an error message what the valid commands are if you
  specified an invalid one
* Adds a bunch of comments in places

.github/workflows/test-deploy-owners.yml

@@ -37,7 +37,7 @@ jobs:
 
       - name: Setup external pr tools
         uses: ./.github/workflows/setup-external-pr-tools
-      
+
       - name: Initialize check
         run: |
           cd ops/external-prs &&
@@ -48,9 +48,9 @@ jobs:
           echo "${{ github.event.pull_request.author_association }}"
 
       - name: Login to google
-        uses: 'google-github-actions/auth@v2'
+        uses: "google-github-actions/auth@v2"
         with:
-          credentials_json: '${{ secrets.GOOGLE_BQ_ADMIN_CREDENTIALS_JSON }}'
+          credentials_json: "${{ secrets.GOOGLE_BQ_ADMIN_CREDENTIALS_JSON }}"
           create_credentials_file: true
         if: ${{ contains(fromJson('["OWNER", "MEMBER", "COLLABORATOR", "CONTRIBUTOR"]'), github.event.pull_request.author_association) }}
 

ops/external-prs/.env.example

@@ -0,0 +1,7 @@
+# .env
+
+# GitHub App ID, which you can find here
+# https://github.com/organizations/opensource-observer/settings/apps/oso-prs
+PR_TOOLS_GITHUB_APP_ID=
+# Base64 encoded private key for the GitHub App
+PR_TOOLS_GITHUB_APP_PRIVATE_KEY=

ops/external-prs/src/base.ts

@@ -70,6 +70,11 @@ export class GHAppUtils {
     });
   }
 
+  /**
+   * Set a status comment on a PR
+   * This will try to keep updating the same comment if it exists
+   * You can have multiple comments by setting a `messageId`
+   **/
   async setStatusComment(pr: number, message: string, messageId?: string) {
     messageId = messageId || "external-pr-status-comment";
     const messageIdText = `<!-- ${messageId} -->`;
@@ -92,6 +97,7 @@ export class GHAppUtils {
     });
     console.log(appCommentRefs);
 
+    // If this app has never commented on this PR, just create it
     if (appCommentRefs.length === 0) {
       await this.octo.rest.issues.createComment({
         owner: this.repo.owner,
@@ -116,11 +122,13 @@ export class GHAppUtils {
       comments.push(appComment);
     }
 
+    // Look for the messageIdText
     const matchingComments = comments.filter((c) => {
       const body = c.data.body || "";
       return body.trimStart().indexOf(messageIdText) === 0;
     });
 
+    // Just create it if it doesn't exist yet
     if (matchingComments.length === 0) {
       await this.octo.rest.issues.createComment({
         owner: this.repo.owner,
@@ -130,6 +138,8 @@ export class GHAppUtils {
       });
       return;
     }
+
+    // Delete any duplicate comments with the same messageIdText
     if (matchingComments.length > 1) {
       logger.warn(
         "multiple matching comments found. This isn't treated as an error. Deleting extra comments",
@@ -143,6 +153,7 @@ export class GHAppUtils {
       });
     }
 
+    // Update the existing comment
     await this.octo.rest.issues.updateComment({
       owner: this.repo.owner,
       repo: this.repo.name,
@@ -169,6 +180,7 @@ export class GHAppUtils {
       commentId: comment.data.id,
       authorAssosication: comment.data.author_association,
       author: comment.data.user?.login,
+      content: comment.data.body,
     });
 
     const login = comment.data.user?.login;
@@ -200,7 +212,12 @@ export class GHAppUtils {
 
     const handler = handlers[match[1]];
     if (!handler) {
-      throw new NoCommandError(`invalid command "${match[1]}`);
+      logger.warn(
+        `Valid commands include ${Object.keys(handlers)
+          .map((x) => `'${x}'`)
+          .join(", ")}`,
+      );
+      throw new NoCommandError(`invalid command /${match[1]}`);
     }
     const issueUrl = comment.data.issue_url;
     const url = new URL(issueUrl);

ops/external-prs/src/cli.ts

@@ -20,11 +20,19 @@ type BeforeClientArgs = ArgumentsCamelCase<{
 }>;
 
 interface InitializePRCheck extends BaseArgs {
+  // Commit SHA
   sha: string;
+  // GitHub user
   login: string;
   checkName: string;
 }
 
+/**
+ * Checks if the user has write access.
+ * If yes, we signal that we've already queued a job.
+ * Otherwise, signal that we need admin approval.
+ * This is typically run with the initiator of the pull request
+ **/
 async function initializePrCheck(args: InitializePRCheck) {
   logger.info({
     message: "initializing the PR check",
@@ -49,8 +57,9 @@ async function initializePrCheck(args: InitializePRCheck) {
       head_sha: args.sha,
       status: CheckStatus.Queued,
       output: {
-        title: "Test deployment queued",
-        summary: "Test deployment queued",
+        title: "Test workflow has been queued",
+        summary:
+          "Test workflow has been queued. Please check the corresponding owners workflow for the latest job status.",
       },
     });
   } else {
@@ -62,7 +71,7 @@ async function initializePrCheck(args: InitializePRCheck) {
       conclusion: CheckConclusion.ActionRequired,
       output: {
         title: `Approval required for ${args.checkName}`,
-        summary: `Approval required for the ${args.checkName} check.`,
+        summary: `Approval required for the ${args.checkName} check. Repo admins can run '/${args.checkName} LATEST_COMMIT_SHA'. Remember to use the latest commit SHA, or validation will fail.`,
       },
     });
   }
@@ -96,8 +105,9 @@ const cli = yargs(hideBin(process.argv))
     demandOption: true,
   })
   .middleware(async (args: BeforeClientArgs) => {
+    // Get base64-encoded private key from the environment
     const buf = Buffer.from(args.githubAppPrivateKey as string, "base64"); // Ta-da
-
+    // Log into GitHub Octokit
     const app = new App({
       appId: args.githubAppId as string,
       privateKey: buf.toString("utf-8"),

ops/external-prs/src/ossd/index.ts

@@ -43,7 +43,9 @@ function jsonlExport<T>(path: string, arr: Array<T>): Promise<void> {
 }
 
 interface ParseCommentArgs extends BaseArgs {
+  // Comment ID
   comment: number;
+  // Output filename
   output: string;
   login: string;
 }
@@ -686,6 +688,11 @@ async function validatePR(args: ValidatePRArgs) {
   await pr.validate(args);
 }
 
+/**
+ * This command is called by external-prs-handle-comment as a check
+ * for whether we should run the validation logic,
+ * based on whether a valid command was called.
+ **/
 async function parseOSSDirectoryComments(args: ParseCommentArgs) {
   const enableValidation: CommmentCommandHandler<GithubOutput> = async (
     command,
@@ -712,15 +719,19 @@ async function parseOSSDirectoryComments(args: ParseCommentArgs) {
     });
   };
 
+  const commandHandlers = {
+    // /validate <sha>
+    validate: enableValidation,
+  };
+
   try {
     const output = await args.appUtils.parseCommentForCommand<GithubOutput>(
       args.comment,
-      {
-        validate: enableValidation,
-      },
+      commandHandlers,
     );
     await output.commit(args.output);
-  } catch (_e) {
+  } catch (e) {
+    logger.debug("Error", e);
     await GithubOutput.write(args.output, {
       deploy: "false",
     });