Merge pull request #74 from joao-paulo-parity/action-inputs

Move action inputs to the configuration file
paritytech · Apr 4, 2022 · f7ca44d · f7ca44d
2 parents b0ac5a9 + 5a65ac2
commit f7ca44d
Show file tree

Hide file tree

Showing 12 changed files with 718 additions and 714 deletions.
diff --git a/README.md b/README.md
@@ -9,6 +9,7 @@ This is a GitHub Action created for complex pull request approval scenarios whic
 - [Built-in checks](#built-in-checks)
 - [Configuration](#configuration)
   - [Configuration file](#configuration-file)
+  - [Configuration syntax](#configuration-syntax)
   - [Rules syntax](#rules-syntax)
     - [Basic Rule syntax](#basic-rule-syntax)
     - [AND Rule syntax](#and-rule-syntax)
@@ -49,7 +50,6 @@ This action has the following non-configurable built-in checks:
 
 - If the action's files are changed, 1 approval from
   [action-review-team](#workflow-configuration) is required
-  - `.github/workflows/pr-custom-review.yml`
   - `.github/pr-custom-review.yml`
 
 Customizable rules should be enabled through [configuration](#action-configuration).
@@ -67,9 +67,32 @@ action's workflow file is added, otherwise the action will fail with
 `RequestError [HttpError]: Not Found` because the configuration does not yet
 exist in the default branch.
 
+### Configuration syntax <a name="configuration-syntax"></a>
+
+```yaml
+inputs:
+  # locks-review-team defines the team which will handle the "locks touched"
+  # built-in rule. We recommend protecting this input with "🔒" so that it
+  # won't be changed unless someone from locks-review-team approves it.
+  # 🔒 PROTECTED: Changes to locks-review-team should be approved by custom-locks-team
+  locks-review-team: custom-locks-team
+
+  # The second team which will handle the "locks touched" built-in rule.
+  team-leads-team: my-custom-leads-team
+
+  # The team which will handle the changes to the action's configuration.
+  action-review-team: my-action-review-team
+
+# This is an example of a basic rule which enforces one approval from anyone
+# More complex rule types are explained in-depth in the "Rules syntax" section
+rules:
+  - name: A single approval
+    min_approvals: 1
+```
+
 ### Rules syntax <a name="rules-syntax"></a>
 
-Three kinds of rules are available:
+Four kinds of rules are available:
 
 - Basic Rule, through which you specify **top-level** `users` and `teams` for
   reaching `min_approvals`
@@ -78,6 +101,9 @@ Three kinds of rules are available:
   with its own `min_approvals`, and **all** of them (logical `AND`) should
   reach their respective `min_approvals`
 
+- AND DISTINCT Rule, which works like AND Rule except that each approval needs
+  to come from a different user
+
 - OR Rule, through which you specify subconditions of `users` and `teams`, each
   with its own `min_approvals`, and **any** of them (logical `OR`) should reach
   their respective `min_approvals`
@@ -225,9 +251,7 @@ field.
 
 ### Workflow configuration <a name="workflow-configuration"></a>
 
-The workflow configuration should be placed in
-`.github/workflows/pr-custom-review.yml` (related to
-[built-in checks](#built-in-checks)).
+The workflow configuration should be placed in `.github/workflows`.
 
 ```yaml
 name: PR Custom Review Status    # The PR status will be created with this name.
@@ -256,19 +280,6 @@ jobs:
           # - `workflow` for being able to request the workflow's job
           #    information; used to track lines in the job's output
           token: ${{ secrets.REVIEWS_TOKEN }}
-
-          # locks-review-team defines the team which will handle the "locks
-          # touched" built-in rule. We recommend protecting this input with
-          # "🔒" so that it won't be changed unless someone from
-          # locks-review-team approves it.
-          # 🔒 PROTECTED: Changes to locks-review-team should be approved by custom-locks-team
-          locks-review-team: custom-locks-team
-
-          # The second team which will handle the "locks touched" built-in rule.
-          team-leads-team: my-custom-leads-team
-
-          # The team which will handle the changes to the action's configuration.
-          action-review-team: my-action-review-team
 ```
 
 ### GitHub repository configuration  <a name="github-repository-configuration"></a>
@@ -389,7 +400,6 @@ installed.
 5. Add the [workflow configuration](#workflow-configuration), as demonstrated in
   <https://github.com/paritytech/substrate/pull/10951/files>
 
-    - Team inputs should use the teams created on Step 1
     - `token` input should use the Personal Access Token generated on Step 2
 
 6. Trigger one of the events defined in the

diff --git a/action.yml b/action.yml
@@ -8,15 +8,6 @@ inputs:
   token:
     required: true
     description: 'Repository token'
-  locks-review-team:
-    required: true
-    description: 'The team which will be asked for review if locks are touched'
-  team-leads-team:
-    required: true
-    description: 'The team leads which will be asked for review if locks are touched'
-  action-review-team:
-    required: true
-    description: "The team which will be asked for review if the action's files are changed"
 runs:
   using: 'node16'
   main: 'dist/index.js'
diff --git a/src/constants.ts b/src/constants.ts
@@ -1,8 +1,7 @@
 import { CommitState, RulesConfigurations } from "./types"
 
 export const configFilePath = ".github/pr-custom-review.yml"
-export const workflowFilePath = ".github/workflows/pr-custom-review.yml"
-export const actionReviewTeamFiles = [configFilePath, workflowFilePath]
+export const actionReviewTeamFiles = [configFilePath]
 
 export const commitStateSuccess: CommitState = "success"
 export const commitStateFailure: CommitState = "failure"
@@ -32,9 +31,3 @@ export const rulesConfigurations: RulesConfigurations = {
     invalidFields: ["min_approvals", "teams", "users", "all", "any"],
   },
 }
-
-export const variableNameToActionInputName = {
-  teamLeadsTeam: "team-leads-team",
-  locksReviewTeam: "locks-review-team",
-  actionReviewTeam: "action-review-team",
-}
diff --git a/src/core.ts b/src/core.ts
@@ -9,7 +9,6 @@ import {
   configFilePath,
   maxGithubApiFilesPerPage,
   maxGithubApiTeamMembersPerPage,
-  variableNameToActionInputName,
 } from "./constants"
 import { LoggerInterface } from "./logger"
 import {
@@ -95,35 +94,51 @@ export const runChecks = async function (
   pr: PR,
   octokit: Octokit,
   logger: LoggerInterface,
-  {
-    actionReviewTeam,
-    locksReviewTeam,
-    teamLeadsTeam,
-  }: {
-    actionReviewTeam: string
-    locksReviewTeam: string
-    teamLeadsTeam: string
-  },
 ) {
-  if (locksReviewTeam.length === 0) {
+  const configFileResponse = await octokit.rest.repos.getContent({
+    owner: pr.base.repo.owner.login,
+    repo: pr.base.repo.name,
+    path: configFilePath,
+  })
+  if (!("content" in configFileResponse.data)) {
     logger.failure(
-      `Locks Review Team (action input: ${variableNameToActionInputName.locksReviewTeam}) should be provided`,
+      `Did not find "content" key in the response for ${configFilePath}`,
     )
+    logger.log(configFileResponse.data)
     return commitStateFailure
   }
-  if (teamLeadsTeam.length === 0) {
+
+  const { content: configFileContentsEnconded } = configFileResponse.data
+  if (typeof configFileContentsEnconded !== "string") {
     logger.failure(
-      `Team Leads Team (action input: ${variableNameToActionInputName.teamLeadsTeam}) should be provided`,
+      `Content response for ${configFilePath} had unexpected type (expected string)`,
     )
+    logger.log(configFileResponse.data)
     return commitStateFailure
   }
-  if (actionReviewTeam.length === 0) {
-    logger.failure(
-      `Action Review Team (action input: ${variableNameToActionInputName.actionReviewTeam}) should be provided`,
-    )
+
+  const configFileContents = Buffer.from(
+    configFileContentsEnconded,
+    "base64",
+  ).toString("utf-8")
+  const configValidationResult = configurationSchema.validate(
+    YAML.parse(configFileContents),
+  )
+  if (configValidationResult.error) {
+    logger.failure("Configuration file is invalid")
+    logger.log(configValidationResult.error)
     return commitStateFailure
   }
 
+  const {
+    inputs: {
+      "locks-review-team": locksReviewTeam,
+      "team-leads-team": teamLeadsTeam,
+      "action-review-team": actionReviewTeam,
+    },
+    rules,
+  } = configValidationResult.value
+
   // Set up a teams cache so that teams used multiple times don't have to be
   // requested more than once
   const teamsCache: TeamsCache = new Map()
@@ -134,13 +149,6 @@ export const runChecks = async function (
     pull_number: pr.number,
     mediaType: { format: "diff" },
   })) /* Octokit doesn't inform the right return type for mediaType: { format: "diff" } */ as unknown as OctokitResponse<string>
-  if (diffResponse.status !== 200) {
-    logger.failure(
-      `Failed to get the diff from ${pr.html_url} (code ${diffResponse.status})`,
-    )
-    logger.log(diffResponse.data)
-    return commitStateFailure
-  }
   const { data: diff } = diffResponse
 
   const matchedRules: MatchedRule[] = []
@@ -218,57 +226,6 @@ export const runChecks = async function (
     }
   }
 
-  const configFileResponse = await octokit.rest.repos.getContent({
-    owner: pr.base.repo.owner.login,
-    repo: pr.base.repo.name,
-    path: configFilePath,
-  })
-  if (configFileResponse.status !== 200) {
-    logger.failure(
-      `Failed to get the contents of ${configFilePath} (code ${configFileResponse.status})`,
-    )
-    logger.log(configFileResponse.data)
-    return commitStateFailure
-  }
-  const { data } = configFileResponse
-  if (typeof data !== "object" || data === null) {
-    logger.failure(
-      `Data response for ${configFilePath} had unexpected type (expected object)`,
-    )
-    logger.log(configFileResponse.data)
-    return commitStateFailure
-  }
-  if (!("content" in data)) {
-    logger.failure(
-      `Did not find "content" key in the response for ${configFilePath}`,
-    )
-    logger.log(configFileResponse.data)
-    return commitStateFailure
-  }
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-  const { content: configFileContentsEnconded } = data
-  if (typeof configFileContentsEnconded !== "string") {
-    logger.failure(
-      `Content response for ${configFilePath} had unexpected type (expected string)`,
-    )
-    logger.log(configFileResponse.data)
-    return commitStateFailure
-  }
-
-  const configFileContents = Buffer.from(
-    configFileContentsEnconded,
-    "base64",
-  ).toString("utf-8")
-  const validationResult = configurationSchema.validate(
-    YAML.parse(configFileContents),
-  )
-  if (validationResult.error) {
-    logger.failure("Configuration file is invalid")
-    logger.log(validationResult.error)
-    return commitStateFailure
-  }
-  const config = validationResult.value
-
   const processComplexRule = async function (
     id: MatchedRule["id"],
     name: string,
@@ -327,7 +284,7 @@ export const runChecks = async function (
     }
   }
 
-  for (const rule of config.rules) {
+  for (const rule of rules) {
     const includeCondition = (function () {
       switch (typeof rule.condition) {
         case "string": {
@@ -456,13 +413,6 @@ export const runChecks = async function (
       repo: pr.base.repo.name,
       pull_number: pr.number,
     })
-    if (reviewsResponse.status !== 200) {
-      logger.failure(
-        `Failed to fetch reviews from ${pr.html_url} (code ${reviewsResponse.status})`,
-      )
-      logger.log(reviewsResponse.data)
-      return commitStateFailure
-    }
     const { data: reviews } = reviewsResponse
 
     const latestReviews: Map<