From 719224bffdb0c22b2430e09935b8f08af8c91fe0 Mon Sep 17 00:00:00 2001
From: Joey Perrott <josephperrott@gmail.com>
Date: Mon, 30 Mar 2020 08:44:30 -0700
Subject: [PATCH] feat(dev-infra): add support for new global approvers in
 pullapprove (#36324)

Pullapprove as added a few new features to allow for us to better
execute our expectation for global approvals. We need to allow for
an expectation that our global approver groups are not in the list
of approved groups. Additionally, since approval groups apply to
all files in the repo, the global approval groups also do not have
conditions defined for them, which means pullapprove verification
need to allow for no conditions need to be defined.

PR Close #36324
---
 dev-infra/pullapprove/group.ts      | 75 +++++++++++++++--------------
 dev-infra/pullapprove/parse-yaml.ts |  6 ++-
 2 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/dev-infra/pullapprove/group.ts b/dev-infra/pullapprove/group.ts
index 247ea4054a..4dc34d012c 100644
--- a/dev-infra/pullapprove/group.ts
+++ b/dev-infra/pullapprove/group.ts
@@ -34,6 +34,7 @@ const CONDITION_TYPES = {
   INCLUDE_GLOBS: /^contains_any_globs/,
   EXCLUDE_GLOBS: /^not contains_any_globs/,
   ATTR_LENGTH: /^len\(.*\)/,
+  GLOBAL_APPROVAL: /^global-(docs-)?approvers not in groups.approved$/,
 };
 
 /** A PullApprove group to be able to test files against. */
@@ -48,43 +49,47 @@ export class PullApproveGroup {
   public hasMatchers = false;
 
   constructor(public groupName: string, group: PullApproveGroupConfig) {
-    for (let condition of group.conditions) {
-      condition = condition.trim();
+    if (group.conditions) {
+      for (let condition of group.conditions) {
+        condition = condition.trim();
 
-      if (condition.match(CONDITION_TYPES.INCLUDE_GLOBS)) {
-        const [conditions, misconfiguredLines] = getLinesForContainsAnyGlobs(condition);
-        conditions.forEach(globString => this.includeConditions.push({
-          glob: globString,
-          matcher: new Minimatch(globString, {dot: true}),
-          matchedFiles: new Set<string>(),
-        }));
-        this.misconfiguredLines.push(...misconfiguredLines);
-        this.hasMatchers = true;
-      } else if (condition.match(CONDITION_TYPES.EXCLUDE_GLOBS)) {
-        const [conditions, misconfiguredLines] = getLinesForContainsAnyGlobs(condition);
-        conditions.forEach(globString => this.excludeConditions.push({
-          glob: globString,
-          matcher: new Minimatch(globString, {dot: true}),
-          matchedFiles: new Set<string>(),
-        }));
-        this.misconfiguredLines.push(...misconfiguredLines);
-        this.hasMatchers = true;
-      } else if (condition.match(CONDITION_TYPES.ATTR_LENGTH)) {
-        // Currently a noop as we do not take any action on this condition type.
-      } else {
-        const errMessage =
-            `Unrecognized condition found, unable to parse the following condition: \n\n` +
-            `From the [${groupName}] group:\n` +
-            ` - ${condition}` +
-            `\n\n` +
-            `Known condition regexs:\n` +
-            `${Object.entries(CONDITION_TYPES).map(([k, v]) => ` ${k} - $ {
-          v
+        if (condition.match(CONDITION_TYPES.INCLUDE_GLOBS)) {
+          const [conditions, misconfiguredLines] = getLinesForContainsAnyGlobs(condition);
+          conditions.forEach(globString => this.includeConditions.push({
+            glob: globString,
+            matcher: new Minimatch(globString, {dot: true}),
+            matchedFiles: new Set<string>(),
+          }));
+          this.misconfiguredLines.push(...misconfiguredLines);
+          this.hasMatchers = true;
+        } else if (condition.match(CONDITION_TYPES.EXCLUDE_GLOBS)) {
+          const [conditions, misconfiguredLines] = getLinesForContainsAnyGlobs(condition);
+          conditions.forEach(globString => this.excludeConditions.push({
+            glob: globString,
+            matcher: new Minimatch(globString, {dot: true}),
+            matchedFiles: new Set<string>(),
+          }));
+          this.misconfiguredLines.push(...misconfiguredLines);
+          this.hasMatchers = true;
+        } else if (condition.match(CONDITION_TYPES.ATTR_LENGTH)) {
+          // Currently a noop as we do not take any action on this condition type.
+        } else if (condition.match(CONDITION_TYPES.GLOBAL_APPROVAL)) {
+          // Currently a noop as we don't take any action for global approval conditions.
+        } else {
+          const errMessage =
+              `Unrecognized condition found, unable to parse the following condition: \n\n` +
+              `From the [${groupName}] group:\n` +
+              ` - ${condition}` +
+              `\n\n` +
+              `Known condition regexs:\n` +
+              `${Object.entries(CONDITION_TYPES).map(([k, v]) => ` ${k} - $ {
+            v
+          }
+          `).join('\n')}` +
+              `\n\n`;
+          console.error(errMessage);
+          process.exit(1);
         }
-        `).join('\n')}` +
-            `\n\n`;
-        console.error(errMessage);
-        process.exit(1);
       }
     }
   }
diff --git a/dev-infra/pullapprove/parse-yaml.ts b/dev-infra/pullapprove/parse-yaml.ts
index a74eb3a0cb..5b3d89d47e 100644
--- a/dev-infra/pullapprove/parse-yaml.ts
+++ b/dev-infra/pullapprove/parse-yaml.ts
@@ -8,9 +8,11 @@
 import {parse as parseYaml} from 'yaml';
 
 export interface PullApproveGroupConfig {
-  conditions: string;
+  conditions?: string;
   reviewers: {
     users: string[],
+    teams?: string[],
+  }|{
     teams: string[],
   };
 }
@@ -30,4 +32,4 @@ export interface PullApproveConfig {
 
 export function parsePullApproveYaml(rawYaml: string): PullApproveConfig {
   return parseYaml(rawYaml) as PullApproveConfig;
-}
\ No newline at end of file
+}