Merge pull request #107 from github/config-issues

Debugging and Documentation

__tests__/github-provider.test.js

@@ -1,6 +1,11 @@
 const nock = require("nock");
 const { GitHubProvider } = require("../src/github-provider");
 
+import * as core from "@actions/core";
+
+jest.spyOn(core, "debug").mockImplementation(() => {});
+jest.spyOn(core, "info").mockImplementation(() => {});
+
 test("It creates an approved review", async () => {
   process.env["GITHUB_REPOSITORY"] = "foo/bar";
 

__tests__/privileged-requester.test.js

@@ -1,6 +1,11 @@
 import { GitHubProvider } from "../src/github-provider";
 import { PrivilegedRequester } from "../src/privileged-requester";
 
+import * as core from "@actions/core";
+
+jest.spyOn(core, "debug").mockImplementation(() => {});
+jest.spyOn(core, "info").mockImplementation(() => {});
+
 test("We receive the expected config content", async () => {
   let configContent = `---
 requesters:
@@ -28,9 +33,7 @@ requesters:
 
 test("We do not receive the expected config content", async () => {
   let provider = new GitHubProvider("token");
-  jest
-    .spyOn(provider, "getConfigContent")
-    .mockImplementation(() => false);
+  jest.spyOn(provider, "getConfigContent").mockImplementation(() => false);
   expect(provider.getConfigContent()).toBe(false);
 
   let privilegedRequester = new PrivilegedRequester(provider);

__tests__/pull-request.test.js

@@ -26,9 +26,7 @@ test("We can list commits", async () => {
     { author: { login: "danhoerst" } },
   ];
   let provider = new GitHubProvider("token");
-  jest
-    .spyOn(provider, "listPRCommits")
-    .mockImplementation(() => prCommits);
+  jest.spyOn(provider, "listPRCommits").mockImplementation(() => prCommits);
   expect(provider.listPRCommits()).toBe(prCommits);
 
   let pullRequest = new PullRequest(provider);
@@ -39,9 +37,7 @@ test("We can list commits", async () => {
 test("We can list labels", async () => {
   let prLabels = [{ name: "bug" }, { name: "feature-request" }];
   let provider = new GitHubProvider("token");
-  jest
-    .spyOn(provider, "listLabelsOnPR")
-    .mockImplementation(() => prLabels);
+  jest.spyOn(provider, "listLabelsOnPR").mockImplementation(() => prLabels);
   expect(provider.listLabelsOnPR()).toBe(prLabels);
 
   let pullRequest = new PullRequest(provider);

__tests__/runner.test.js

@@ -25,9 +25,7 @@ afterEach(() => {
 describe("processCommits", () => {
   test("We process commits successfully", async () => {
     let prCommits = [{ author: { login: "robot" } }];
-    jest
-      .spyOn(pullRequest, "listCommits")
-      .mockImplementation(() => prCommits);
+    jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
     expect(pullRequest.listCommits()).toBe(prCommits);
 
     let commits = await runner.processCommits("robot");
@@ -39,9 +37,7 @@ describe("processCommits", () => {
       { author: { login: "robot" } },
       { author: { login: "danhoerst" } },
     ];
-    jest
-      .spyOn(pullRequest, "listCommits")
-      .mockImplementation(() => prCommits);
+    jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
     expect(pullRequest.listCommits()).toBe(prCommits);
 
     let commits = await runner.processCommits("robot");
@@ -63,9 +59,7 @@ index 2f4e8d9..93c2072 100644
          if: blah
          with:
            name: dist`;
-    jest
-      .spyOn(pullRequest, "getDiff")
-      .mockImplementation(() => prDiff);
+    jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
     expect(pullRequest.getDiff()).toBe(prDiff);
 
     let diff = await runner.processDiff();
@@ -86,9 +80,7 @@ index 2f4e8d9..93c2072 100644
          if: blah
          with:
            name: dist`;
-    jest
-      .spyOn(pullRequest, "getDiff")
-      .mockImplementation(() => prDiff);
+    jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
     expect(pullRequest.getDiff()).toBe(prDiff);
 
     let diff = await runner.processDiff();
@@ -117,9 +109,7 @@ describe("labelsEqual", () => {
 describe("processLabels", () => {
   test("We process labels successfully", async () => {
     let prLabels = [{ name: "bug" }, { name: "feature-request" }];
-    jest
-      .spyOn(pullRequest, "listLabels")
-      .mockImplementation(() => prLabels);
+    jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
     expect(pullRequest.listLabels()).toBe(prLabels);
 
     let commits = await runner.processLabels({
@@ -130,9 +120,7 @@ describe("processLabels", () => {
 
   test("We process labels unsuccessfully", async () => {
     let prLabels = [{ name: "bug" }, { name: "feature-request" }];
-    jest
-      .spyOn(pullRequest, "listLabels")
-      .mockImplementation(() => prLabels);
+    jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
     expect(pullRequest.listLabels()).toBe(prLabels);
 
     let commits = await runner.processLabels({
@@ -148,15 +136,11 @@ describe("processPrivilegedReviewer", () => {
     process.env["INPUT_CHECKLABELS"] = "true";
     process.env["INPUT_CHECKDIFF"] = "true";
     let prLabels = [{ name: "bug" }, { name: "feature-request" }];
-    jest
-      .spyOn(pullRequest, "listLabels")
-      .mockImplementation(() => prLabels);
+    jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
     expect(pullRequest.listLabels()).toBe(prLabels);
 
     let prCommits = [{ author: { login: "robot" } }];
-    jest
-      .spyOn(pullRequest, "listCommits")
-      .mockImplementation(() => prCommits);
+    jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
     expect(pullRequest.listCommits()).toBe(prCommits);
 
     let prDiff = `| diff --git a/.github/workflows/check-dist.yml b/.github/workflows/check-dist.yml
@@ -171,9 +155,7 @@ index 2f4e8d9..93c2072 100644
          if: blah
          with:
            name: dist`;
-    jest
-      .spyOn(pullRequest, "getDiff")
-      .mockImplementation(() => prDiff);
+    jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
     expect(pullRequest.getDiff()).toBe(prDiff);
 
     let processed = await runner.processPrivilegedReviewer("robot", {
@@ -189,9 +171,7 @@ index 2f4e8d9..93c2072 100644
       { author: { login: "robot" } },
       { author: { login: "malicious" } },
     ];
-    jest
-      .spyOn(pullRequest, "listCommits")
-      .mockImplementation(() => prCommits);
+    jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
     expect(pullRequest.listCommits()).toBe(prCommits);
 
     let processed = await runner.processPrivilegedReviewer("robot", {
@@ -205,9 +185,7 @@ index 2f4e8d9..93c2072 100644
       { author: { login: "robot" } },
       { author: { login: "malicious" } },
     ];
-    jest
-      .spyOn(pullRequest, "listCommits")
-      .mockImplementation(() => prCommits);
+    jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
     expect(pullRequest.listCommits()).toBe(prCommits);
 
     let processed = await runner.processPrivilegedReviewer("robot", {
@@ -219,9 +197,7 @@ index 2f4e8d9..93c2072 100644
   test("We process labels unsuccessfully with the option enabled", async () => {
     process.env["INPUT_CHECKLABELS"] = "true";
     let prLabels = [{ name: "bug" }, { name: "feature-request" }];
-    jest
-      .spyOn(pullRequest, "listLabels")
-      .mockImplementation(() => prLabels);
+    jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
     expect(pullRequest.listLabels()).toBe(prLabels);
 
     let processed = await runner.processPrivilegedReviewer("robot", {
@@ -232,9 +208,7 @@ index 2f4e8d9..93c2072 100644
 
   test("We allow bad labels when the option to check them is not set", async () => {
     let prLabels = [{ name: "bug" }, { name: "feature-request" }];
-    jest
-      .spyOn(pullRequest, "listLabels")
-      .mockImplementation(() => prLabels);
+    jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
     expect(pullRequest.listLabels()).toBe(prLabels);
 
     let processed = await runner.processPrivilegedReviewer("robot", {
@@ -258,9 +232,7 @@ index 2f4e8d9..93c2072 100644
          if: blah
          with:
            name: dist`;
-    jest
-      .spyOn(pullRequest, "getDiff")
-      .mockImplementation(() => prDiff);
+    jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
     expect(pullRequest.getDiff()).toBe(prDiff);
 
     let processed = await runner.processPrivilegedReviewer("robot", {
@@ -283,9 +255,7 @@ index 2f4e8d9..93c2072 100644
          if: blah
          with:
            name: dist`;
-    jest
-      .spyOn(pullRequest, "getDiff")
-      .mockImplementation(() => prDiff);
+    jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
     expect(pullRequest.getDiff()).toBe(prDiff);
 
     let processed = await runner.processPrivilegedReviewer("robot", {

.github/workflows/privileged-requester.yml

@@ -5,12 +5,15 @@ on:
 
 permissions:
   pull-requests: write
+  contents: read
 
 jobs:
   check:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - name: checkout
+      uses: actions/checkout@v4
+
     - uses: ./
       with:
         myToken: ${{ secrets.GITHUB_TOKEN }}

README.md

@@ -11,15 +11,32 @@ The workflow should be configured like:
 > Where `vX.X.X` is the latest release version found on the releases page
 
 ```yaml
+name: privileged-requester
+on:
+  pull_request:
+    types: [opened, synchronize, reopened, labeled, unlabeled]
+
+permissions:
+  pull-requests: write
+  contents: read
+
 jobs:
   check:
     runs-on: ubuntu-latest
     steps:
-    # checkout the repository
-    - uses: actions/checkout@v4
+      - name: checkout
+        uses: actions/checkout@v4
 
-    # run privileged-requester
-    - uses: github/privileged-requester@vX.X.X
+      - uses: github/privileged-requester@vX.X.X
+        with:
+          myToken: ${{ secrets.GITHUB_TOKEN }}
+          robotUserToken: ${{ secrets.REPO_GITHUB_TOKEN }}
+          path: config/privileged-requester.yaml
+          prCreator: ${{ github.event.pull_request.user.login }}
+          prNumber: ${{ github.event.pull_request.number }}
+          checkCommits: "true"
+          checkDiff: "true"
+          checkLabels: "true"
 ```
 
 See the example in [the workflow folder](.github/workflows/privileged-requester.yml)
@@ -51,7 +68,7 @@ However, you can configure the Action to run with a different repo scoped token
 
 Here are the configuration options for this Action:
 
-## Inputs 📥
+### Inputs 📥
 
 | Input     | Required? | Default                                     | Description |
 |-----------| --------- |---------------------------------------------| ----------- |
@@ -64,8 +81,12 @@ Here are the configuration options for this Action:
 | `checkDiff` | yes | `"true"`                                       | An option to check that the PR diff only has a removal diff, with no additions |
 | `checkLabels` | yes | `"true"`                                       | An option to check that the labels on the PR match those defined in the privileged requester config |
 
-## Outputs 📤
+### Outputs 📤
 
 | Output | Description |
 | ------ | ----------- |
 | `approved` | The string `"true"` if the privileged-requester approved the pull request |
+
+## First Time Setup
+
+It should be noted that this Action looks at the `default` branch for its configuration file. This means that if you add this Action through a pull request, it will look at the default branch and _fail_ because it cannot find the config file that has not landed on `main` / `master` yet. After merging the pull request that adds this Action to your repository, it should work as expected.

dist/index.js

@@ -36930,6 +36930,9 @@ class GitHubProvider {
   }
 
   async createReview(prNumber, reviewEvent) {
+    core.debug(`prNumber: ${prNumber}`);
+    core.debug(`reviewEvent: ${reviewEvent}`);
+
     await this.octokit.rest.pulls.createReview({
       owner: github.context.repo.owner,
       repo: github.context.repo.repo,
@@ -36939,11 +36942,14 @@ class GitHubProvider {
   }
 
   async getConfigContent() {
+    const path = core.getInput("path", { required: true });
+    core.info(`config path: ${path}`);
+
     // getContent defaults to the main branch
     const { data: configContent } = await this.octokit.rest.repos.getContent({
       owner: github.context.repo.owner,
       repo: github.context.repo.repo,
-      path: core.getInput("path"),
+      path: path,
       mediaType: { format: "raw" },
     });
     return configContent;
@@ -37001,11 +37007,13 @@ class PrivilegedRequester {
     if (this.requesters === false) {
       try {
         const config = await this.github.getConfigContent();
+        lib_core.debug(`config: ${config}`);
+
         this.configContents = yaml.load(config);
         this.requesters = this.configContents["requesters"];
       } catch (err) {
         lib_core.error(
-          "There was a problem with the privileged requester configuration."
+          `There was a problem with the privileged requester configuration.\n${err}\n${err.stack}`
         );
         lib_core.setFailed(err.message);
         return false;
@@ -37237,7 +37245,11 @@ class Runner {
 const index_core = __nccwpck_require__(2186);
 let myToken = index_core.getInput("myToken");
 const robotUserToken = index_core.getInput("robotUserToken");
-if (robotUserToken !== "") {
+if (
+  robotUserToken !== "" ||
+  robotUserToken !== undefined ||
+  robotUserToken !== null
+) {
   index_core.info("Robot User configured. I will use that PAT instead.");
   myToken = robotUserToken;
 }

index.js

@@ -6,7 +6,11 @@ import { Runner } from "./src/runner";
 const core = require("@actions/core");
 let myToken = core.getInput("myToken");
 const robotUserToken = core.getInput("robotUserToken");
-if (robotUserToken !== "") {
+if (
+  robotUserToken !== "" ||
+  robotUserToken !== undefined ||
+  robotUserToken !== null
+) {
   core.info("Robot User configured. I will use that PAT instead.");
   myToken = robotUserToken;
 }

package.json

@@ -11,7 +11,7 @@
     "prepare": "ncc build index.js -o dist --source-map --license licenses.txt",
     "test": "jest",
     "bundle": "npm run format && npm run package",
-    "all": "npm run lint && npm run prepare && npm run test"
+    "all": "npm run lint && npm run format && npm run prepare && npm run test"
   },
   "repository": {
     "type": "git",

src/github-provider.js

@@ -11,6 +11,9 @@ class GitHubProvider {
   }
 
   async createReview(prNumber, reviewEvent) {
+    core.debug(`prNumber: ${prNumber}`);
+    core.debug(`reviewEvent: ${reviewEvent}`);
+
     await this.octokit.rest.pulls.createReview({
       owner: github.context.repo.owner,
       repo: github.context.repo.repo,
@@ -20,11 +23,14 @@ class GitHubProvider {
   }
 
   async getConfigContent() {
+    const path = core.getInput("path", { required: true });
+    core.info(`config path: ${path}`);
+
     // getContent defaults to the main branch
     const { data: configContent } = await this.octokit.rest.repos.getContent({
       owner: github.context.repo.owner,
       repo: github.context.repo.repo,
-      path: core.getInput("path"),
+      path: path,
       mediaType: { format: "raw" },
     });
     return configContent;

src/privileged-requester.js

@@ -14,11 +14,13 @@ class PrivilegedRequester {
     if (this.requesters === false) {
       try {
         const config = await this.github.getConfigContent();
+        core.debug(`config: ${config}`);
+
         this.configContents = yaml.load(config);
         this.requesters = this.configContents["requesters"];
       } catch (err) {
         core.error(
-          "There was a problem with the privileged requester configuration."
+          `There was a problem with the privileged requester configuration.\n${err}\n${err.stack}`
         );
         core.setFailed(err.message);
         return false;