Add branch protection mode

Author: Kobzol

docs/toml-schema.md

@@ -333,13 +333,18 @@ Admins cannot override these branch protections. If an admin needs to do that, t
 # The pattern matching the branches to be protected (required)
 pattern = "master"
 # Which CI checks to are required for merging (optional)
+# Cannot be set if `pr-required` is `false`.
 ci-checks = ["CI"]
 # Whether new commits after a reviewer's approval of a PR 
 # merging into this branch require another review. 
 # (optional - default `false`)
 dismiss-stale-review = false
+# Is a PR required when making changes to this branch?
+# (optional - default `true`)
+pr-required = true
 # How many approvals are required for a PR to be merged.
 # This option is only relevant if bors is not used.
+# Cannot be set if `pr-required` is `false`.
 # (optional - default `1`)
 required-approvals = 1
 # Which GitHub teams have access to push/merge to this branch.

rust_team_data/src/v1.rs

@@ -209,12 +209,21 @@ pub enum RepoPermission {
     Triage,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
+#[serde(rename_all = "snake_case")]
+pub enum BranchProtectionMode {
+    PrRequired {
+        ci_checks: Vec<String>,
+        required_approvals: u32,
+    },
+    PrNotRequired,
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 pub struct BranchProtection {
     pub pattern: String,
-    pub ci_checks: Vec<String>,
     pub dismiss_stale_review: bool,
-    pub required_approvals: u32,
+    pub mode: BranchProtectionMode,
     pub allowed_merge_teams: Vec<String>,
 }
 

src/schema.rs

@@ -797,6 +797,8 @@ pub(crate) struct BranchProtection {
     pub dismiss_stale_review: bool,
     #[serde(default)]
     pub required_approvals: Option<u32>,
+    #[serde(default = "default_true")]
+    pub pr_required: bool,
     #[serde(default)]
     pub allowed_merge_teams: Vec<String>,
 }

src/static_api.rs

@@ -4,6 +4,7 @@ use anyhow::{ensure, Context as _, Error};
 use indexmap::IndexMap;
 use log::info;
 use rust_team_data::v1;
+use rust_team_data::v1::BranchProtectionMode;
 use std::collections::HashMap;
 use std::path::Path;
 
@@ -48,9 +49,15 @@ impl<'a> Generator<'a> {
                 .iter()
                 .map(|b| v1::BranchProtection {
                     pattern: b.pattern.clone(),
-                    ci_checks: b.ci_checks.clone(),
                     dismiss_stale_review: b.dismiss_stale_review,
-                    required_approvals: b.required_approvals.unwrap_or(1),
+                    mode: if b.pr_required {
+                        BranchProtectionMode::PrRequired {
+                            ci_checks: b.ci_checks.clone(),
+                            required_approvals: b.required_approvals.unwrap_or(1),
+                        }
+                    } else {
+                        BranchProtectionMode::PrNotRequired
+                    },
                     allowed_merge_teams: b.allowed_merge_teams.clone(),
                 })
                 .collect();

src/validate.rs

@@ -839,6 +839,25 @@ but that team does not seem to exist"#,
                 }
             }
 
+            if !protection.pr_required {
+                // It does not make sense to use CI checks when a PR is not required, because with a
+                // CI check, it would not be possible to push into the branch without a PR anyway.
+                if !protection.ci_checks.is_empty() {
+                    bail!(
+                        r#"repo '{}' uses a branch protection for {} that does not require a PR, but has non-empty `ci-checks`"#,
+                        repo.name,
+                        protection.pattern,
+                    );
+                }
+                if protection.required_approvals.is_some() {
+                    bail!(
+                        r#"repo '{}' uses a branch protection for {} that does not require a PR, but sets the `required-approvals` attribute"#,
+                        repo.name,
+                        protection.pattern,
+                    );
+                }
+            }
+
             if bors_used {
                 if protection.required_approvals.is_some() {
                     bail!(

tests/static-api/_expected/v1/repos.json

@@ -16,11 +16,15 @@
       "branch_protections": [
         {
           "pattern": "master",
-          "ci_checks": [
-            "CI"
-          ],
           "dismiss_stale_review": false,
-          "required_approvals": 1,
+          "mode": {
+            "pr_required": {
+              "ci_checks": [
+                "CI"
+              ],
+              "required_approvals": 1
+            }
+          },
           "allowed_merge_teams": []
         }
       ],
@@ -44,11 +48,15 @@
       "branch_protections": [
         {
           "pattern": "master",
-          "ci_checks": [
-            "CI"
-          ],
           "dismiss_stale_review": false,
-          "required_approvals": 1,
+          "mode": {
+            "pr_required": {
+              "ci_checks": [
+                "CI"
+              ],
+              "required_approvals": 1
+            }
+          },
           "allowed_merge_teams": [
             "foo"
           ]

tests/static-api/_expected/v1/repos/archived_repo.json

@@ -14,11 +14,15 @@
   "branch_protections": [
     {
       "pattern": "master",
-      "ci_checks": [
-        "CI"
-      ],
       "dismiss_stale_review": false,
-      "required_approvals": 1,
+      "mode": {
+        "pr_required": {
+          "ci_checks": [
+            "CI"
+          ],
+          "required_approvals": 1
+        }
+      },
       "allowed_merge_teams": []
     }
   ],

tests/static-api/_expected/v1/repos/some_repo.json

@@ -14,11 +14,15 @@
   "branch_protections": [
     {
       "pattern": "master",
-      "ci_checks": [
-        "CI"
-      ],
       "dismiss_stale_review": false,
-      "required_approvals": 1,
+      "mode": {
+        "pr_required": {
+          "ci_checks": [
+            "CI"
+          ],
+          "required_approvals": 1
+        }
+      },
       "allowed_merge_teams": [
         "foo"
       ]