diff --git a/config.toml b/config.toml index 7050adda0..48b154bdf 100644 --- a/config.toml +++ b/config.toml @@ -83,89 +83,6 @@ members-without-zulip-id = [ "rust-timer", ] -enable-rulesets-repos = [ - "rust-analyzer/expect-test", - "rust-lang-nursery/rust-toolstate", - "rust-lang/annotate-snippets-rs", - "rust-lang/arewewebyet", - "rust-lang/async-fundamentals-initiative", - "rust-lang/backtrace-rs", - "rust-lang/beyond-refs", - "rust-lang/blog.rust-lang.org", - "rust-lang/book", - "rust-lang/bors", - "rust-lang/calendar-generation", - "rust-lang/calendar", - "rust-lang/cargo-bisect-rustc", - "rust-lang/cargo", - "rust-lang/cc-rs", - "rust-lang/ci-mirrors", - "rust-lang/cmake-rs", - "rust-lang/compiler-builtins", - "rust-lang/compiler-team", - "rust-lang/crater", - "rust-lang/crates_io_og_image", - "rust-lang/crates-build-env", - "rust-lang/crates-io-auth-action", - "rust-lang/crates.io", - "rust-lang/crates.io-index-archive", - "rust-lang/crates.io-index", - "rust-lang/docs.rs", - "rust-lang/edition-guide", - "rust-lang/ena", - "rust-lang/enzyme", - "rust-lang/flate2-rs", - "rust-lang/fls", - "rust-lang/futures-rs", - "rust-lang/gha-self-hosted", - "rust-lang/git2-rs", - "rust-lang/gll", - "rust-lang/glob", - "rust-lang/hashbrown", - "rust-lang/impl-trait-utils", - "rust-lang/infra-smoke-tests", - "rust-lang/josh-sync", - "rust-lang/leadership-council", - "rust-lang/libc", - "rust-lang/llvm-project", - "rust-lang/mdBook", - "rust-lang/measureme", - "rust-lang/miri", - "rust-lang/miri-test-libstd", - "rust-lang/nomicon", - "rust-lang/odht", - "rust-lang/portable-simd", - "rust-lang/project-stable-mir", - "rust-lang/project-exploit-mitigations", - "rust-lang/promote-release", - "rust-lang/reference", - "rust-lang/relnotes", - "rust-lang/rfcs", - "rust-lang/rust-analyzer", - "rust-lang/rust-bindgen", - "rust-lang/rust-clippy", - "rust-lang/rust-forge", - "rust-lang/rust-log-analyzer", - "rust-lang/rust-playground", - "rust-lang/rustc_codegen_c", - "rust-lang/rustc_codegen_gcc", - "rust-lang/rustc-demangle", - "rust-lang/rustc-dev-guide", - "rust-lang/rustc-hash", - "rust-lang/rustc-perf", - "rust-lang/rustc-stable-hash", - "rust-lang/rustfmt", - "rust-lang/rustup", - "rust-lang/simpleinfra", - "rust-lang/socket2", - "rust-lang/staging.crates.io-index", - "rust-lang/std-dev-guide", - "rust-lang/stdarch", - "rust-lang/team", - "rust-lang/this-week-in-rust", - "rust-lang/thorin", - "rust-lang/triagebot", - "rust-lang/wg-async", - "rust-lang/wg-macros", - "rust-lang/www.rust-lang.org", +disable-rulesets-repos = [ + "rust-lang/rust", ] diff --git a/src/data.rs b/src/data.rs index 9ca212e4d..c2d5b6b43 100644 --- a/src/data.rs +++ b/src/data.rs @@ -242,7 +242,7 @@ impl Data { Ok(sync::Config { special_org_members, independent_github_orgs: self.config.independent_github_orgs().clone(), - enable_rulesets_repos: self.config.enable_rulesets_repos().clone(), + disable_rulesets_repos: self.config.disable_rulesets_repos().clone(), }) } } diff --git a/src/schema.rs b/src/schema.rs index ef8e3a3eb..8249b1c73 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -17,7 +17,7 @@ pub(crate) struct Config { special_org_members: BTreeSet, members_without_zulip_id: BTreeSet, #[serde(default)] - enable_rulesets_repos: BTreeSet, + disable_rulesets_repos: BTreeSet, } impl Config { @@ -49,8 +49,8 @@ impl Config { &self.members_without_zulip_id } - pub(crate) fn enable_rulesets_repos(&self) -> &BTreeSet { - &self.enable_rulesets_repos + pub(crate) fn disable_rulesets_repos(&self) -> &BTreeSet { + &self.disable_rulesets_repos } } diff --git a/src/sync/github/api/mod.rs b/src/sync/github/api/mod.rs index 563d2a16c..2ee49ca5f 100644 --- a/src/sync/github/api/mod.rs +++ b/src/sync/github/api/mod.rs @@ -414,7 +414,7 @@ where } /// An object with a `login` field -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[derive(Deserialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub(crate) struct Login { pub(crate) login: String, } @@ -456,7 +456,7 @@ fn team_node_id(id: u64) -> String { BASE64_STANDARD.encode(format!("04:Team{id}")) } -#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, serde::Deserialize)] #[serde(rename_all = "camelCase")] pub(crate) struct BranchProtection { pub(crate) pattern: String, @@ -501,7 +501,7 @@ where } /// Entities that can be allowed to push to a branch in a repo -#[derive(Clone, Deserialize, Debug, PartialEq, Eq)] +#[derive(Clone, Deserialize, Debug, PartialEq, Eq, PartialOrd, Ord)] #[serde(untagged)] pub(crate) enum PushAllowanceActor { User(UserPushAllowanceActor), @@ -510,20 +510,20 @@ pub(crate) enum PushAllowanceActor { } /// User who can be allowed to push to a branch in a repo -#[derive(Clone, Deserialize, Debug, PartialEq, Eq)] +#[derive(Clone, Deserialize, Debug, PartialEq, Eq, PartialOrd, Ord)] pub(crate) struct UserPushAllowanceActor { pub(crate) login: String, } /// Team that can be allowed to push to a branch in a repo -#[derive(Clone, Deserialize, Debug, PartialEq, Eq)] +#[derive(Clone, Deserialize, Debug, PartialEq, Eq, PartialOrd, Ord)] pub(crate) struct TeamPushAllowanceActor { pub(crate) organization: Login, pub(crate) name: String, } /// GitHub app that can be allowed to push to a branch in a repo -#[derive(Clone, Deserialize, Debug, PartialEq, Eq)] +#[derive(Clone, Deserialize, Debug, PartialEq, Eq, PartialOrd, Ord)] pub(crate) struct AppPushAllowanceActor { pub(crate) name: String, /// Node ID, which can be used as a push actor ID diff --git a/src/sync/github/api/read.rs b/src/sync/github/api/read.rs index b0080d4de..06642ddce 100644 --- a/src/sync/github/api/read.rs +++ b/src/sync/github/api/read.rs @@ -11,7 +11,7 @@ use anyhow::Context as _; use async_trait::async_trait; use reqwest::{Method, StatusCode}; use rust_team_data::v1::Environment; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; #[async_trait] pub(crate) trait GithubRead { @@ -74,7 +74,7 @@ pub(crate) trait GithubRead { &self, org: &str, repo: &str, - ) -> anyhow::Result>; + ) -> anyhow::Result>; /// Get environments for a repository /// Returns a map of environment names to their Environment data @@ -512,7 +512,7 @@ impl GithubRead for GitHubApiRead { &self, org: &str, repo: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { #[derive(serde::Serialize)] struct Params<'a> { org: &'a str, @@ -576,7 +576,7 @@ impl GithubRead for GitHubApiRead { protection: BranchProtection, } - let mut result = HashMap::new(); + let mut result = BTreeMap::new(); let res: Wrapper = self .client .graphql(QUERY, Params { org, repo }, org) diff --git a/src/sync/github/mod.rs b/src/sync/github/mod.rs index adee87835..5f4dc2bf1 100644 --- a/src/sync/github/mod.rs +++ b/src/sync/github/mod.rs @@ -444,7 +444,7 @@ impl SyncGitHub { /// Check if a repository should use rulesets instead of branch protections fn should_use_rulesets(&self, repo: &rust_team_data::v1::Repo) -> bool { let repo_full_name = format!("{}/{}", repo.org, repo.name); - self.config.enable_rulesets_repos.contains(&repo_full_name) + !self.config.disable_rulesets_repos.contains(&repo_full_name) } async fn diff_repo( @@ -671,7 +671,7 @@ impl SyncGitHub { branch_protection_diffs.push(BranchProtectionDiff { pattern: branch_protection.pattern.clone(), operation, - }) + }); } // `actual_branch_protections` now contains the branch protections that were not expected diff --git a/src/sync/github/tests/mod.rs b/src/sync/github/tests/mod.rs index be18a858b..e3a787005 100644 --- a/src/sync/github/tests/mod.rs +++ b/src/sync/github/tests/mod.rs @@ -333,27 +333,55 @@ async fn repo_create() { ), }, ], - branch_protections: [ - ( - "main", - BranchProtection { - pattern: "main", - is_admin_enforced: true, - allows_force_pushes: false, - dismisses_stale_reviews: false, - requires_conversation_resolution: false, - requires_linear_history: false, - requires_strict_status_checks: false, - required_approving_review_count: 1, - required_status_check_contexts: [ - "test", - ], - push_allowances: [], - requires_approving_reviews: true, + branch_protections: [], + rulesets: [ + Ruleset { + id: None, + name: "main", + target: Branch, + source_type: Repository, + enforcement: Active, + bypass_actors: [], + conditions: RulesetConditions { + ref_name: RulesetRefNameCondition { + include: [ + "refs/heads/main", + ], + exclude: [], + }, }, - ), + rules: { + Creation, + Deletion, + PullRequest { + parameters: PullRequestParameters { + dismiss_stale_reviews_on_push: false, + require_code_owner_review: false, + require_last_push_approval: false, + required_approving_review_count: 1, + required_review_thread_resolution: false, + }, + }, + RequiredStatusChecks { + parameters: RequiredStatusChecksParameters { + do_not_enforce_on_create: Some( + false, + ), + required_status_checks: [ + RequiredStatusCheck { + context: "test", + integration_id: Some( + 15368, + ), + }, + ], + strict_required_status_checks_policy: false, + }, + }, + NonFastForward, + }, + }, ], - rulesets: [], environments: [], app_installations: [], }, @@ -760,48 +788,92 @@ async fn repo_add_branch_protection() { }, ), permission_diffs: [], - branch_protection_diffs: [ - BranchProtectionDiff { - pattern: "master", + branch_protection_diffs: [], + ruleset_diffs: [ + RulesetDiff { + name: "master", operation: Create( - BranchProtection { - pattern: "master", - is_admin_enforced: true, - allows_force_pushes: false, - dismisses_stale_reviews: false, - requires_conversation_resolution: false, - requires_linear_history: false, - requires_strict_status_checks: false, - required_approving_review_count: 0, - required_status_check_contexts: [ - "test", - "test 2", - ], - push_allowances: [], - requires_approving_reviews: true, + Ruleset { + id: None, + name: "master", + target: Branch, + source_type: Repository, + enforcement: Active, + bypass_actors: [], + conditions: RulesetConditions { + ref_name: RulesetRefNameCondition { + include: [ + "refs/heads/master", + ], + exclude: [], + }, + }, + rules: { + Creation, + Deletion, + PullRequest { + parameters: PullRequestParameters { + dismiss_stale_reviews_on_push: false, + require_code_owner_review: false, + require_last_push_approval: false, + required_approving_review_count: 0, + required_review_thread_resolution: false, + }, + }, + RequiredStatusChecks { + parameters: RequiredStatusChecksParameters { + do_not_enforce_on_create: Some( + false, + ), + required_status_checks: [ + RequiredStatusCheck { + context: "test", + integration_id: Some( + 15368, + ), + }, + RequiredStatusCheck { + context: "test 2", + integration_id: Some( + 15368, + ), + }, + ], + strict_required_status_checks_policy: false, + }, + }, + NonFastForward, + }, }, ), }, - BranchProtectionDiff { - pattern: "beta", + RulesetDiff { + name: "beta", operation: Create( - BranchProtection { - pattern: "beta", - is_admin_enforced: true, - allows_force_pushes: false, - dismisses_stale_reviews: false, - requires_conversation_resolution: false, - requires_linear_history: false, - requires_strict_status_checks: false, - required_approving_review_count: 0, - required_status_check_contexts: [], - push_allowances: [], - requires_approving_reviews: false, + Ruleset { + id: None, + name: "beta", + target: Branch, + source_type: Repository, + enforcement: Active, + bypass_actors: [], + conditions: RulesetConditions { + ref_name: RulesetRefNameCondition { + include: [ + "refs/heads/beta", + ], + exclude: [], + }, + }, + rules: { + Creation, + Deletion, + NonFastForward, + }, }, ), }, ], - ruleset_diffs: [], environment_diffs: [], app_installation_diffs: [], }, @@ -867,46 +939,115 @@ async fn repo_update_branch_protection() { }, ), permission_diffs: [], - branch_protection_diffs: [ - BranchProtectionDiff { - pattern: "master", + branch_protection_diffs: [], + ruleset_diffs: [ + RulesetDiff { + name: "master", operation: Update( - "0", - BranchProtection { - pattern: "master", - is_admin_enforced: true, - allows_force_pushes: false, - dismisses_stale_reviews: false, - requires_conversation_resolution: false, - requires_linear_history: false, - requires_strict_status_checks: false, - required_approving_review_count: 1, - required_status_check_contexts: [ - "test", - ], - push_allowances: [], - requires_approving_reviews: true, + 0, + Ruleset { + id: Some( + 0, + ), + name: "master", + target: Branch, + source_type: Repository, + enforcement: Active, + bypass_actors: [], + conditions: RulesetConditions { + ref_name: RulesetRefNameCondition { + include: [ + "refs/heads/master", + ], + exclude: [], + }, + }, + rules: { + Creation, + Deletion, + PullRequest { + parameters: PullRequestParameters { + dismiss_stale_reviews_on_push: false, + require_code_owner_review: false, + require_last_push_approval: false, + required_approving_review_count: 1, + required_review_thread_resolution: false, + }, + }, + RequiredStatusChecks { + parameters: RequiredStatusChecksParameters { + do_not_enforce_on_create: Some( + false, + ), + required_status_checks: [ + RequiredStatusCheck { + context: "test", + integration_id: Some( + 15368, + ), + }, + ], + strict_required_status_checks_policy: false, + }, + }, + NonFastForward, + }, }, - BranchProtection { - pattern: "master", - is_admin_enforced: true, - allows_force_pushes: true, - dismisses_stale_reviews: true, - requires_conversation_resolution: true, - requires_linear_history: true, - requires_strict_status_checks: true, - required_approving_review_count: 0, - required_status_check_contexts: [ - "Test", - "test", - ], - push_allowances: [], - requires_approving_reviews: true, + Ruleset { + id: None, + name: "master", + target: Branch, + source_type: Repository, + enforcement: Active, + bypass_actors: [], + conditions: RulesetConditions { + ref_name: RulesetRefNameCondition { + include: [ + "refs/heads/master", + ], + exclude: [], + }, + }, + rules: { + Creation, + Deletion, + RequiredLinearHistory, + PullRequest { + parameters: PullRequestParameters { + dismiss_stale_reviews_on_push: true, + require_code_owner_review: false, + require_last_push_approval: false, + required_approving_review_count: 0, + required_review_thread_resolution: true, + }, + }, + RequiredStatusChecks { + parameters: RequiredStatusChecksParameters { + do_not_enforce_on_create: Some( + false, + ), + required_status_checks: [ + RequiredStatusCheck { + context: "Test", + integration_id: Some( + 15368, + ), + }, + RequiredStatusCheck { + context: "test", + integration_id: Some( + 15368, + ), + }, + ], + strict_required_status_checks_policy: true, + }, + }, + }, }, ), }, ], - ruleset_diffs: [], environment_diffs: [], app_installation_diffs: [], }, @@ -954,15 +1095,15 @@ async fn repo_remove_branch_protection() { }, ), permission_diffs: [], - branch_protection_diffs: [ - BranchProtectionDiff { - pattern: "stable", + branch_protection_diffs: [], + ruleset_diffs: [ + RulesetDiff { + name: "stable", operation: Delete( - "1", + 1, ), }, ], - ruleset_diffs: [], environment_diffs: [], app_installation_diffs: [], }, diff --git a/src/sync/github/tests/test_utils.rs b/src/sync/github/tests/test_utils.rs index 1f3bc3327..feb9522cd 100644 --- a/src/sync/github/tests/test_utils.rs +++ b/src/sync/github/tests/test_utils.rs @@ -1,6 +1,6 @@ use async_trait::async_trait; use indexmap::IndexMap; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::vec; use derive_builder::Builder; @@ -16,8 +16,7 @@ use crate::sync::github::api::{ RepoTeam, RepoUser, Ruleset, Team, TeamMember, TeamPrivacy, TeamRole, }; use crate::sync::github::{ - OrgMembershipDiff, RepoDiff, SyncGitHub, TeamDiff, api, construct_branch_protection, - convert_permission, + OrgMembershipDiff, RepoDiff, SyncGitHub, TeamDiff, api, construct_ruleset, convert_permission, }; pub const DEFAULT_ORG: &str = "rust-lang"; @@ -174,16 +173,19 @@ impl DataModel { org.repo_members .insert(repo.name.clone(), RepoMembers { teams, members }); - let repo_v1: v1::Repo = repo.clone().into(); - let mut protections = vec![]; - for protection in &repo.branch_protections { - protections.push(( - format!("{}", protections.len()), - construct_branch_protection(&repo_v1, protection), - )); - } - org.branch_protections - .insert(repo.name.clone(), protections); + // Branch protections are deprecated so we don't test them anymore + org.branch_protections.insert(repo.name.clone(), Vec::new()); + + let protections = repo + .branch_protections + .iter() + .enumerate() + .map(|(idx, protection)| Ruleset { + id: Some(idx as i64), + ..construct_ruleset(protection) + }) + .collect(); + org.rulesets.insert(repo.name.clone(), protections); let environments: HashMap = repo.environments.clone().into_iter().collect(); @@ -702,11 +704,11 @@ impl GithubRead for GithubMock { &self, org: &str, repo: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let Some(protections) = self.get_org(org).branch_protections.get(repo) else { return Ok(Default::default()); }; - let mut result = HashMap::default(); + let mut result = BTreeMap::default(); for (id, protection) in protections { result.insert(protection.pattern.clone(), (id.clone(), protection.clone())); } diff --git a/src/sync/mod.rs b/src/sync/mod.rs index 3c79db8de..d803f5e98 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -19,7 +19,7 @@ use zulip::SyncZulip; pub struct Config { pub special_org_members: BTreeSet, pub independent_github_orgs: BTreeSet, - pub enable_rulesets_repos: BTreeSet, + pub disable_rulesets_repos: BTreeSet, } pub async fn run_sync_team(