Merge pull request #118 from fensak-io/main

yorinasub17 · web-flow · commit c232709c7069 · 2023-10-18T11:32:40.000-05:00
Release
diff --git a/deno.lock b/deno.lock
diff --git a/fskconfig/fixtures/invalid-zero-required-approvals-machine-when-no-requiredrule.json b/fskconfig/fixtures/invalid-zero-required-approvals-machine-when-no-requiredrule.json
@@ -0,0 +1,9 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "ruleFile": "app_deploy_rule.ts",
+      "ruleLang": "ts",
+      "requiredApprovalsForMachineUsers": 0
+    }
+  }
+}
diff --git a/fskconfig/fixtures/invalid-zero-required-approvals-trusted-when-no-requiredrule.json b/fskconfig/fixtures/invalid-zero-required-approvals-trusted-when-no-requiredrule.json
@@ -0,0 +1,9 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "ruleFile": "app_deploy_rule.ts",
+      "ruleLang": "ts",
+      "requiredApprovalsForTrustedUsers": 0
+    }
+  }
+}
diff --git a/fskconfig/fixtures/invalid-zero-required-approvals-when-no-requiredrule.json b/fskconfig/fixtures/invalid-zero-required-approvals-when-no-requiredrule.json
@@ -0,0 +1,9 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "ruleFile": "app_deploy_rule.ts",
+      "ruleLang": "ts",
+      "requiredApprovals": 0
+    }
+  }
+}
diff --git a/fskconfig/fixtures/valid-only-required-rule.json b/fskconfig/fixtures/valid-only-required-rule.json
@@ -0,0 +1,8 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "requiredRuleFile": "source_branch_rule.ts",
+      "requiredApprovals": 0
+    }
+  }
+}
diff --git a/fskconfig/fixtures/valid-with-required-rule.json b/fskconfig/fixtures/valid-with-required-rule.json
@@ -0,0 +1,10 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "ruleFile": "app_deploy_rule.ts",
+      "ruleLang": "ts",
+      "requiredRuleFile": "source_branch_rule.ts",
+      "requiredApprovals": 2
+    }
+  }
+}
diff --git a/fskconfig/fixtures/valid-zero-required-approvals-machine-with-requiredrule.json b/fskconfig/fixtures/valid-zero-required-approvals-machine-with-requiredrule.json
@@ -0,0 +1,10 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "ruleFile": "app_deploy_rule.ts",
+      "ruleLang": "ts",
+      "requiredRuleFile": "source_branch.ts",
+      "requiredApprovalsForMachineUsers": 0
+    }
+  }
+}
diff --git a/fskconfig/fixtures/valid-zero-required-approvals-trusted-with-requiredrule.json b/fskconfig/fixtures/valid-zero-required-approvals-trusted-with-requiredrule.json
@@ -0,0 +1,10 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "ruleFile": "app_deploy_rule.ts",
+      "ruleLang": "ts",
+      "requiredRuleFile": "source_branch.ts",
+      "requiredApprovalsForTrustedUsers": 0
+    }
+  }
+}
diff --git a/fskconfig/fixtures/valid-zero-required-approvals-with-requiredrule.json b/fskconfig/fixtures/valid-zero-required-approvals-with-requiredrule.json
@@ -0,0 +1,10 @@
+{
+  "repos": {
+    "test-fensak-rules-engine": {
+      "ruleFile": "app_deploy_rule.ts",
+      "ruleLang": "ts",
+      "requiredRuleFile": "source_branch.ts",
+      "requiredApprovals": 0
+    }
+  }
+}
diff --git a/fskconfig/loader_github.ts b/fskconfig/loader_github.ts
@@ -268,17 +268,24 @@ async function loadRuleFiles(
   const ruleFilesToLoad: Record<string, reng.RuleFnSourceLang> = {};
   for (const repoName in orgCfg.repos) {
     const repoCfg = orgCfg.repos[repoName];
-    if (ruleFilesToLoad[repoCfg.ruleFile]) {
-      // Skip because it is already accounted for
-      continue;
-    }
+    if (repoCfg.ruleFile && !ruleFilesToLoad[repoCfg.ruleFile]) {
+      // This is redundant and unnecessary, but it makes the compiler happy.
+      if (!repoCfg.ruleLang) {
+        repoCfg.ruleLang = getRuleLang(repoCfg.ruleFile);
+      }
 
-    // This is redundant and unnecessary, but it makes the compiler happy.
-    if (!repoCfg.ruleLang) {
-      repoCfg.ruleLang = getRuleLang(repoCfg.ruleFile);
+      ruleFilesToLoad[repoCfg.ruleFile] = repoCfg.ruleLang;
     }
+    if (
+      repoCfg.requiredRuleFile && !ruleFilesToLoad[repoCfg.requiredRuleFile]
+    ) {
+      // This is redundant and unnecessary, but it makes the compiler happy.
+      if (!repoCfg.requiredRuleLang) {
+        repoCfg.requiredRuleLang = getRuleLang(repoCfg.requiredRuleFile);
+      }
 
-    ruleFilesToLoad[repoCfg.ruleFile] = repoCfg.ruleLang;
+      ruleFilesToLoad[repoCfg.requiredRuleFile] = repoCfg.requiredRuleLang;
+    }
   }
 
   const out: RuleLookup = {};
diff --git a/fskconfig/parser.ts b/fskconfig/parser.ts
@@ -54,9 +54,38 @@ export function parseConfigFile(
   }
   const typedData: OrgConfig = data as OrgConfig;
 
+  // Further validation on the data that requires more logic than json schema.
+  for (const repoName in typedData.repos) {
+    const cfg = typedData.repos[repoName];
+    if (cfg.requiredRuleFile) {
+      // Skip since it makes sense to allow 0 for required reviews if required rules are configured
+      continue;
+    }
+
+    // Check to make sure the required reviews is greater than 0. If it is set to 0, then the check has no effect since
+    // it will always pass.
+    if (cfg.requiredApprovals == 0) {
+      throw new Error(
+        `requiredApprovals for repo ${repoName} must be greater than 0 when there are no required rules`,
+      );
+    }
+    if (cfg.requiredApprovalsForTrustedUsers == 0) {
+      throw new Error(
+        `requiredApprovalsForTrustedUsers for repo ${repoName} must be greater than 0 when there are no required rules`,
+      );
+    }
+    if (cfg.requiredApprovalsForMachineUsers == 0) {
+      throw new Error(
+        `requiredApprovalsForMachineUsers for repo ${repoName} must be greater than 0 when there are no required rules`,
+      );
+    }
+  }
+
   // Configure defaults:
   // - set the machineUsers top level key to empty array if unset.
-  // - set the repoLang based on the filename extension if any entry is missing it.
+  // - set the ruleLang based on the filename extension if any entry is missing it.
+  // - set the requiredRuleLang based on the filename extension if any entry is missing it and requiredRuleFile is
+  //   configured.
   // - set the requiredApprovals to 1 if it is unset.
   // - set the requiredApprovalsForMachineUsers and requiredApprovalsForTrustedUsers to requiredApprovals if it is
   //   unset.
@@ -65,17 +94,22 @@ export function parseConfigFile(
   }
   for (const repoName in typedData.repos) {
     const cfg = typedData.repos[repoName];
-    if (!cfg.ruleLang) {
+    if (cfg.ruleFile && !cfg.ruleLang) {
       typedData.repos[repoName].ruleLang = getRuleLang(cfg.ruleFile);
     }
-    if (!cfg.requiredApprovals) {
+    if (cfg.requiredRuleFile && !cfg.requiredRuleLang) {
+      typedData.repos[repoName].requiredRuleLang = getRuleLang(
+        cfg.requiredRuleFile,
+      );
+    }
+    if (cfg.requiredApprovals === undefined) {
       typedData.repos[repoName].requiredApprovals = 1;
     }
-    if (!cfg.requiredApprovalsForMachineUsers) {
+    if (cfg.requiredApprovalsForMachineUsers === undefined) {
       typedData.repos[repoName].requiredApprovalsForMachineUsers =
         typedData.repos[repoName].requiredApprovals;
     }
-    if (!cfg.requiredApprovalsForTrustedUsers) {
+    if (cfg.requiredApprovalsForTrustedUsers === undefined) {
       typedData.repos[repoName].requiredApprovalsForTrustedUsers =
         typedData.repos[repoName].requiredApprovals;
     }
diff --git a/fskconfig/parser_test.ts b/fskconfig/parser_test.ts
@@ -53,6 +53,140 @@ Deno.test("parseConfigFile rejects invalid requiredApprovals", async () => {
   );
 });
 
+Deno.test("parseConfigFile rejects 0 requiredApprovals when no requiredRuleFile", async () => {
+  const fname = "invalid-zero-required-approvals-when-no-requiredrule.json";
+  const fixtureInvalidReqAppTxt = await Deno.readTextFile(
+    path.join(
+      __dirname,
+      "fixtures",
+      fname,
+    ),
+  );
+  assertThrows(
+    () => {
+      parseConfigFile(fname, fixtureInvalidReqAppTxt);
+    },
+    Error,
+    "requiredApprovals for repo test-fensak-rules-engine must be greater than 0",
+  );
+});
+
+Deno.test("parseConfigFile rejects 0 requiredApprovalsForTrustedUsers when no requiredRuleFile", async () => {
+  const fname =
+    "invalid-zero-required-approvals-trusted-when-no-requiredrule.json";
+  const fixtureInvalidReqAppTxt = await Deno.readTextFile(
+    path.join(
+      __dirname,
+      "fixtures",
+      fname,
+    ),
+  );
+  assertThrows(
+    () => {
+      parseConfigFile(fname, fixtureInvalidReqAppTxt);
+    },
+    Error,
+    "requiredApprovalsForTrustedUsers for repo test-fensak-rules-engine must be greater than 0",
+  );
+});
+
+Deno.test("parseConfigFile rejects 0 requiredApprovalsForMachineUsers when no requiredRuleFile", async () => {
+  const fname =
+    "invalid-zero-required-approvals-machine-when-no-requiredrule.json";
+  const fixtureInvalidReqAppTxt = await Deno.readTextFile(
+    path.join(
+      __dirname,
+      "fixtures",
+      fname,
+    ),
+  );
+  assertThrows(
+    () => {
+      parseConfigFile(fname, fixtureInvalidReqAppTxt);
+    },
+    Error,
+    "requiredApprovalsForMachineUsers for repo test-fensak-rules-engine must be greater than 0",
+  );
+});
+
+Deno.test("parseConfigFile allows 0 requiredApprovals when requiredRuleFile is set", async () => {
+  const fname = "valid-zero-required-approvals-with-requiredrule.json";
+  const fixtureTxt = await Deno.readTextFile(
+    path.join(
+      __dirname,
+      "fixtures",
+      fname,
+    ),
+  );
+  const cfg = parseConfigFile(fname, fixtureTxt);
+  assertEquals(cfg, {
+    repos: {
+      "test-fensak-rules-engine": {
+        ruleFile: "app_deploy_rule.ts",
+        ruleLang: reng.RuleFnSourceLang.Typescript,
+        requiredRuleFile: "source_branch.ts",
+        requiredRuleLang: reng.RuleFnSourceLang.Typescript,
+        requiredApprovals: 0,
+        requiredApprovalsForTrustedUsers: 0,
+        requiredApprovalsForMachineUsers: 0,
+      },
+    },
+    machineUsers: [],
+  });
+});
+
+Deno.test("parseConfigFile allows 0 requiredApprovalsForTrustedUsers when requiredRuleFile is set", async () => {
+  const fname = "valid-zero-required-approvals-trusted-with-requiredrule.json";
+  const fixtureTxt = await Deno.readTextFile(
+    path.join(
+      __dirname,
+      "fixtures",
+      fname,
+    ),
+  );
+  const cfg = parseConfigFile(fname, fixtureTxt);
+  assertEquals(cfg, {
+    repos: {
+      "test-fensak-rules-engine": {
+        ruleFile: "app_deploy_rule.ts",
+        ruleLang: reng.RuleFnSourceLang.Typescript,
+        requiredRuleFile: "source_branch.ts",
+        requiredRuleLang: reng.RuleFnSourceLang.Typescript,
+        requiredApprovals: 1,
+        requiredApprovalsForTrustedUsers: 0,
+        requiredApprovalsForMachineUsers: 1,
+      },
+    },
+    machineUsers: [],
+  });
+});
+
+Deno.test("parseConfigFile allows 0 requiredApprovalsForMachineUsers when requiredRuleFile is set", async () => {
+  const fname = "valid-zero-required-approvals-machine-with-requiredrule.json";
+  const fixtureTxt = await Deno.readTextFile(
+    path.join(
+      __dirname,
+      "fixtures",
+      fname,
+    ),
+  );
+  const cfg = parseConfigFile(fname, fixtureTxt);
+  assertEquals(cfg, {
+    repos: {
+      "test-fensak-rules-engine": {
+        ruleFile: "app_deploy_rule.ts",
+        ruleLang: reng.RuleFnSourceLang.Typescript,
+        requiredRuleFile: "source_branch.ts",
+        requiredRuleLang: reng.RuleFnSourceLang.Typescript,
+        requiredApprovals: 1,
+        requiredApprovalsForTrustedUsers: 1,
+        requiredApprovalsForMachineUsers: 0,
+      },
+    },
+    machineUsers: [],
+  });
+});
+
 Deno.test("parseConfigFile single repo config", async () => {
   const fixtureFname = "valid-single-repo.json";
   const contents = await Deno.readTextFile(
@@ -73,6 +207,48 @@ Deno.test("parseConfigFile single repo config", async () => {
   });
 });
 
+Deno.test("parseConfigFile single repo config with only requiredRuleFile", async () => {
+  const fixtureFname = "valid-only-required-rule.json";
+  const contents = await Deno.readTextFile(
+    path.join(__dirname, "fixtures", fixtureFname),
+  );
+  const cfg = parseConfigFile(fixtureFname, contents);
+  assertEquals(cfg, {
+    repos: {
+      "test-fensak-rules-engine": {
+        requiredRuleFile: "source_branch_rule.ts",
+        requiredRuleLang: reng.RuleFnSourceLang.Typescript,
+        requiredApprovals: 0,
+        requiredApprovalsForTrustedUsers: 0,
+        requiredApprovalsForMachineUsers: 0,
+      },
+    },
+    machineUsers: [],
+  });
+});
+
+Deno.test("parseConfigFile accepts required rule file", async () => {
+  const fixtureFname = "valid-with-required-rule.json";
+  const contents = await Deno.readTextFile(
+    path.join(__dirname, "fixtures", fixtureFname),
+  );
+  const cfg = parseConfigFile(fixtureFname, contents);
+  assertEquals(cfg, {
+    repos: {
+      "test-fensak-rules-engine": {
+        ruleFile: "app_deploy_rule.ts",
+        ruleLang: reng.RuleFnSourceLang.Typescript,
+        requiredRuleFile: "source_branch_rule.ts",
+        requiredRuleLang: reng.RuleFnSourceLang.Typescript,
+        requiredApprovals: 2,
+        requiredApprovalsForTrustedUsers: 2,
+        requiredApprovalsForMachineUsers: 2,
+      },
+    },
+    machineUsers: [],
+  });
+});
+
 Deno.test("parseConfigFile multi repo config (different formats)", async (t) => {
   const names = [
     "valid-multiple-repo.json",
diff --git a/fskconfig/schema.json b/fskconfig/schema.json
diff --git a/ghevent/pullrequest.ts b/ghevent/pullrequest.ts
diff --git a/ghstd/checks.ts b/ghstd/checks.ts
diff --git a/svcdata/models.ts b/svcdata/models.ts
