feat: add deployment branch support #2142

amustaque97 · 2025-12-02T18:21:17Z

Closes: #2132
Extends environment configuration to support specifying deployment branch restrictions, allowing teams to control which branches can deploy to each environment via a new field in the TOML schema.

rustbot · 2025-12-02T18:48:24Z

rust_team_data/src/v1.rs has been modified, it is used (as a git dependency) by multiple sub-projects like triagebot, the www.rust-lang.org website and others.

If you are changing the data structures, please make sure that the changes are not going to break serde deserialization (adding a field is fine; removing or renaming a field isn't).

If you must do a breaking change to the format, make sure to coordinate it with all the users of the rust_team_data crate.

cc @Urgau

Kobzol · 2025-12-02T19:45:36Z

repos/rust-lang/edition-guide.toml

-[[environments]]
-name = "github-pages"
+[environments.github-pages]
+branches = []


Just to avoid a common default case, if the branches array is empty, I'd remove it from the current files, and keep the default.

Copilot

Pull request overview

This pull request adds support for deployment branch restrictions to repository environment configurations. It transforms the environment schema from a simple list of environment names to a map structure that can specify which branches are allowed to deploy to each environment.

Key changes:

Schema migration from array-of-tables [[environments]] to table-of-tables [environments.{name}] with optional branches field
New EnvironmentDiff::Update variant to handle changes to existing environment branch policies
Implementation of GitHub API integration for managing deployment branch policies via separate API endpoints

Reviewed changes

Copilot reviewed 112 out of 112 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`src/schema.rs`	Changed `environments` from `Vec<Environment>` to `HashMap<String, Environment>` and replaced `name` field with `branches: Vec<String>`
`rust_team_data/src/v1.rs`	Updated public API schema to use `IndexMap<String, Environment>` with `branches` field
`src/validate.rs`	Updated validation to check environment names as HashMap keys and validate branch names are non-empty
`src/static_api.rs`	Updated static API generation to sort and collect environments from HashMap into IndexMap
`src/main.rs`	Updated environment listing to display branch restrictions when present
`sync-team/src/github/mod.rs`	Added `Update` variant to `EnvironmentDiff` and updated diff logic to detect branch policy changes
`sync-team/src/github/api/write.rs`	Added `update_environment`, `get_environment_branch_policies`, `set_environment_branch_policies`, and `delete_environment_branch_policy` methods
`sync-team/src/github/api/read.rs`	Changed `repo_environments` return type to `HashMap<String, Environment>`
`sync-team/src/github/tests/test_utils.rs`	Updated test utilities to use HashMap-based environments and added `environment_with_branches` helper
`sync-team/src/github/tests/mod.rs`	Updated test cases to use new environment structure
`tests/static-api/_expected/v1/repos/*.json`	Updated expected JSON output to use object `{}` instead of array `[]` for environments
`repos/rust-lang/*.toml`	Migrated all repository TOML files from `[[environments]]` to `[environments.{name}]` syntax
`repos/rust-lang/bors.toml`	Example of environment with branch restrictions: `branches = ["main"]`
`docs/toml-schema.md`	Updated documentation with new table-of-tables syntax and branch restrictions examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-03T05:19:54Z

sync-team/src/github/mod.rs

+                    EnvironmentDiff::Update(name, branches) => {
+                        writeln!(f, "    🔄 Update: {name}")?;
+                        if !branches.is_empty() {
+                            writeln!(f, "        Branches: {}", branches.join(", "))?;
+                        }
+                    }


The display output for EnvironmentDiff::Update doesn't clearly communicate when branch restrictions are being removed. When updating an environment from having branch restrictions to having none (empty branches list), the output only shows "🔄 Update: {name}" without indicating that restrictions are being cleared.

Suggested improvement:

EnvironmentDiff::Update(name, branches) => { writeln!(f, " 🔄 Update: {name}")?; if branches.is_empty() { writeln!(f, " Remove branch restrictions")?; } else { writeln!(f, " Branches: {}", branches.join(", "))?; } }

This makes it clearer to users what configuration change is being applied.

sync-team/src/github/api/write.rs

sync-team/src/github/api/read.rs

src/validate.rs

src/schema.rs

sync-team/src/github/api/read.rs

sync-team/src/github/api/write.rs

marcoieni · 2025-12-05T16:58:09Z

sync-team/src/github/mod.rs

    permissions: Vec<RepoPermissionAssignmentDiff>,
    branch_protections: Vec<(String, api::BranchProtection)>,
-    environments: Vec<String>,
+    environments: Vec<(String, Vec<String>)>,


I would substitute Vec<String> with a struct

I would like to change either whole Vec<(String, Vec<String>)> as a struct, not just Vec<String>. IMO, it will make it unnecessarily complex.

struct Branches { names: Vec<String> }

I would like to keep it as it is. WDYT?

marcoieni · 2025-12-06T16:56:49Z

I ran a dry run of this in #2142
the diff is like this:

 Environments:
        🔄 Update: github-pages

Imo the diff should show that (in this case) it's deleting the branch protection.
So please show in the diff whether you are adding/removing branches 👍

marcoieni · 2025-12-09T15:50:14Z

From #2159 (comment) you can update the toml files to import the branches 👍

amustaque97 · 2025-12-10T03:49:57Z

From #2159 (comment) you can update the toml files to import the branches 👍

I made the required changes 🙌🏻

marcoieni · 2025-12-10T13:19:50Z

the dry run doesn't look clean #2159 (comment)

Kobzol

Left some comments. It looks great :)

Kobzol · 2025-12-10T16:59:06Z

rust_team_data/src/v1.rs

    pub branch_protections: Vec<BranchProtection>,
    pub crates: Vec<Crate>,
-    pub environments: Vec<Environment>,
+    pub environments: IndexMap<String, Environment>,


This is a breaking change. I think that we can leave the environments as they were in the v1 API, and just add a branches field to the environment.

This would not be a problem in any way. All the migrations/changes have been taken care of. Please don't hesitate to correct me.

To clarify, the contents of the v1 API is used by several other projects. If this PR is merged as-is, those projects will try to deserialize this field as a Vec (because they will still have the old serde definition), but the API will now expose it as a JSON dictionary. So the repository endpoint will stop working for them.

I'm not sure. Is there any way to confirm this? As environment feature was implemented very recently #2127, if we still have time, I would like to use the current structure because IMO this is how the schema should be.

I would love to hear your thoughts @Kobzol

Hmm, in theory if the projects haven't updated their team version yet, it would be doable. Here is a list of some projects that use the API: https://github.com/search?type=code&q=%22rust_team_data+%3D+%22+org%3Arust-lang We'd need to check what version of team they use, and if they use the repos endpoint.

Thanks for the nudge, I got lucky this time. I verified all the repos using the link and can confirm that it is safe to accept the proposed changes.

Looks like the only repo that uses the repos endpoint is triagebot, and that should be fine, yeah, as it doesn't have the environments field yet.

Kobzol · 2025-12-10T16:59:55Z

src/validate.rs

+                        env_name
+                    );
+                }
+                if !seen_branches.insert(branch) {


Is this really an issue? Why couldn't I have two environments on the same branch?

Is this really an issue?

Not really, this is for better logging and checks. This will be helpful when we try to add the same branch twice or more in any environment.

Why couldn't I have two environments on the same branch?

It’s allowed — a single branch can belong to multiple environments. As you can see in the screenshot below, we iterate over each environment and check for duplicates within that scope.

Sorry 🤦 I read the code wrong. Nevermind!

Kobzol · 2025-12-10T17:01:18Z

sync-team/src/github/mod.rs

 #[derive(Debug)]
 enum EnvironmentDiff {
-    Create(String),
+    Create(String, Vec<String>),


Could you please use named structs here? It's not clear what String, Vec<String>, Vec<String> represents :)

This change has been made.

Kobzol · 2025-12-10T17:02:26Z

sync-team/src/github/mod.rs

+                            writeln!(f, "        Branches: {}", branches.join(", "))?;
+                        }
+                    }
+                    EnvironmentDiff::Update(name, old_branches, new_branches) => {


Also, we usually precompute the "what to add" and "what to remove" things before creating the diff. Could you please modify it so that the update diff already contains branches to add and branches to remove, rather than old/new branches?

I got you, and it 100% makes sense. I have made the required changes.

marcoieni · 2025-12-10T17:14:56Z

the dry run now is clean, thanks!

Kobzol · 2025-12-11T21:50:03Z

Some repositories use tags for protection rules, not branches. This PR will (I think) set everything as branches. I wonder if that will work when a repository then tries to deploy from a tag, have you tried that? The type (branch/tag) is not mandatory when creating the branch policy, but I think that if it doesn't match, it might be an issue. So in that case we might want to explicitly model both branches and tags in team.

amustaque97 · 2025-12-12T08:58:18Z

Some repositories use tags for protection rules, not branches. This PR will (I think) set everything as branches. I wonder if that will work when a repository then tries to deploy from a tag. Have you tried that? The type (branch/tag) is not mandatory when creating the branch policy, but I think that if it doesn't match, it might be an issue. So in that case we might want to explicitly model both branches and tags in team.

Whoops 😬 , I implemented changes based on the comment #2132 (comment); I have added support for tags, too. Can you please review the PR again?

Kobzol · 2025-12-12T10:06:58Z

I see, I didn't know that. It seemed in https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/mdbook.20trusted.20publishing.20broken/with/563418198 that tags would be required. I'll ask t-infra on the next meeting.

marcoieni · 2025-12-12T11:51:37Z

I see, I didn't know that. It seemed in https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/mdbook.20trusted.20publishing.20broken/with/563418198 that tags would be required. I'll ask t-infra on the next meeting.

I don't see why we wouldn't want to do it 🤔

docs/toml-schema.md

amustaque97 · 2025-12-12T12:39:13Z

All the comments are addressed

consistent naming of attributes, i.e. branches and tags
Updated repos directory with required tags as asked in comment Allow configuring deployment branches in environments #2132 (comment)

It is ready for review 👍🏻

amustaque97 marked this pull request as ready for review December 2, 2025 18:48

amustaque97 requested review from Mark-Simulacrum, jdno, marcoieni and pietroalbini as code owners December 2, 2025 18:48

amustaque97 mentioned this pull request Dec 2, 2025

add validation: if trusted publishing is defined, corresponding environment should exist #2140

Open

Kobzol reviewed Dec 2, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings December 3, 2025 05:14

Copilot started reviewing on behalf of amustaque97 December 3, 2025 05:14 View session

Copilot finished reviewing on behalf of amustaque97 December 3, 2025 05:18

Copilot AI reviewed Dec 3, 2025

View reviewed changes

amustaque97 mentioned this pull request Dec 4, 2025

feat: add validation check for publishing environment #2150

Draft

marcoieni reviewed Dec 4, 2025

View reviewed changes

src/schema.rs Outdated Show resolved Hide resolved

sync-team/src/github/api/read.rs Outdated Show resolved Hide resolved

sync-team/src/github/api/write.rs Outdated Show resolved Hide resolved

sync-team/src/github/api/write.rs Show resolved Hide resolved

jieyouxu added needs-infra-admin-review This change requires one of the `infra-admins` to review. S-waiting-on-review Status: waiting on review from a team/WG/PG lead, an infra-admin, and/or a team-repo-admin. labels Dec 5, 2025

marcoieni reviewed Dec 5, 2025

View reviewed changes

marcoieni mentioned this pull request Dec 6, 2025

Test https://github.com/rust-lang/team/pull/2142 #2159

Draft

feat: configure branches in environments

b0ecc58

amustaque97 force-pushed the feat/2132-deploy-branches branch from f182c1d to b0ecc58 Compare December 7, 2025 07:42

amustaque97 requested a review from marcoieni December 7, 2025 07:44

feat: update branches array in repos toml files

752e173

fix: ordering issue

d626fd4

amustaque97 force-pushed the feat/2132-deploy-branches branch from 0b27efe to d626fd4 Compare December 10, 2025 14:00

Kobzol reviewed Dec 10, 2025

View reviewed changes

refactor: address review comments

73ac937

feat: add support for tags

cf58411

marcoieni reviewed Dec 12, 2025

View reviewed changes

docs/toml-schema.md Outdated Show resolved Hide resolved

marcoieni reviewed Dec 12, 2025

View reviewed changes

docs/toml-schema.md Outdated Show resolved Hide resolved

amustaque97 added 2 commits December 12, 2025 18:05

chore: branches and tags

1b94e4a

feat: add tags in repos dir toml files

609d6cf

feat: add deployment branch support #2142

Are you sure you want to change the base?

feat: add deployment branch support #2142

Uh oh!

Conversation

amustaque97 commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcoieni commented Dec 9, 2025

Uh oh!

amustaque97 commented Dec 10, 2025

Uh oh!

marcoieni commented Dec 10, 2025

Uh oh!

Kobzol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kobzol Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni commented Dec 10, 2025

Uh oh!

Kobzol commented Dec 11, 2025

Uh oh!

amustaque97 commented Dec 12, 2025

amustaque97 commented Dec 2, 2025 •

edited

Loading

marcoieni commented Dec 6, 2025 •

edited

Loading

Kobzol Dec 11, 2025 •

edited

Loading