fix(#2287): reconcile team settings on 409 (upsert via org_edit_team)
On 409 (team already exists), list the org teams to find the operators team id, then unconditionally PATCH to the desired settings via org_edit_team. This self-heals a team that was created with the wrong shape by an older code path (missing units, wrong permission) without touching membership (separate endpoint, operator-managed). Addresses mara's review: 'shouldnt we get, then change, then update'. Unconditional PATCH is simpler than GET→diff→conditional PATCH and safe here since we own units/permission/description fully.
This commit is contained in:
parent
beaa220dc2
commit
e9667f9c1a
1 changed files with 50 additions and 18 deletions
|
|
@ -10,7 +10,8 @@ use anyhow::{Context, Result};
|
||||||
use forgejo_api::structs::{
|
use forgejo_api::structs::{
|
||||||
AddCollaboratorOption, AddCollaboratorOptionPermission, CreateBranchProtectionOption,
|
AddCollaboratorOption, AddCollaboratorOptionPermission, CreateBranchProtectionOption,
|
||||||
CreateOrgOption, CreateRepoOption, CreateTeamOption, CreateTeamOptionPermission,
|
CreateOrgOption, CreateRepoOption, CreateTeamOption, CreateTeamOptionPermission,
|
||||||
EditRepoOption, MigrateRepoOptions, MigrateRepoOptionsService, Repository,
|
EditRepoOption, EditTeamOption, EditTeamOptionPermission, MigrateRepoOptions,
|
||||||
|
MigrateRepoOptionsService, Repository,
|
||||||
};
|
};
|
||||||
use forgejo_api::{ApiErrorKind, ForgejoError};
|
use forgejo_api::{ApiErrorKind, ForgejoError};
|
||||||
use reqwest::StatusCode;
|
use reqwest::StatusCode;
|
||||||
|
|
@ -575,37 +576,68 @@ async fn ensure_mirror_repo(
|
||||||
/// real validation error (bad request shape, missing units, etc.) that
|
/// real validation error (bad request shape, missing units, etc.) that
|
||||||
/// must surface so it can be fixed; the previous 422-swallowing hid the
|
/// must surface so it can be fixed; the previous 422-swallowing hid the
|
||||||
/// true cause and left the team silently uncreated every boot.
|
/// true cause and left the team silently uncreated every boot.
|
||||||
|
/// Repo-unit access flags for the `operators` team.
|
||||||
|
/// Explicit list so Forgejo doesn't reject a null/absent `units` field;
|
||||||
|
/// a `write`-permission team needs at least `repo.code` + `repo.pulls`
|
||||||
|
/// to review and merge PRs.
|
||||||
|
const OPERATORS_TEAM_UNITS: &[&str] = &[
|
||||||
|
"repo.code",
|
||||||
|
"repo.issues",
|
||||||
|
"repo.pulls",
|
||||||
|
"repo.releases",
|
||||||
|
"repo.wiki",
|
||||||
|
"repo.projects",
|
||||||
|
"repo.packages",
|
||||||
|
];
|
||||||
|
|
||||||
pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()> {
|
pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()> {
|
||||||
// Explicit units so Forgejo doesn't reject a null/absent units field.
|
let units_vec: Vec<String> = OPERATORS_TEAM_UNITS.iter().map(|&s| s.to_owned()).collect();
|
||||||
// A `write`-permission team needs at minimum repo.code and repo.pulls
|
|
||||||
// to review and merge PRs. Include the full standard set so members
|
|
||||||
// can see the whole repo surface.
|
|
||||||
let units = Some(vec![
|
|
||||||
"repo.code".to_owned(),
|
|
||||||
"repo.issues".to_owned(),
|
|
||||||
"repo.pulls".to_owned(),
|
|
||||||
"repo.releases".to_owned(),
|
|
||||||
"repo.wiki".to_owned(),
|
|
||||||
"repo.projects".to_owned(),
|
|
||||||
"repo.packages".to_owned(),
|
|
||||||
]);
|
|
||||||
let team = CreateTeamOption {
|
let team = CreateTeamOption {
|
||||||
can_create_org_repo: Some(false),
|
can_create_org_repo: Some(false),
|
||||||
description: Some("hyperhive operators — merge gate for agent repos".to_owned()),
|
description: Some("hyperhive operators — merge gate for agent repos".to_owned()),
|
||||||
includes_all_repositories: Some(true),
|
includes_all_repositories: Some(true),
|
||||||
name: OPERATORS_TEAM.to_owned(),
|
name: OPERATORS_TEAM.to_owned(),
|
||||||
permission: Some(CreateTeamOptionPermission::Write),
|
permission: Some(CreateTeamOptionPermission::Write),
|
||||||
units,
|
units: Some(units_vec.clone()),
|
||||||
units_map: None,
|
units_map: None,
|
||||||
};
|
};
|
||||||
match api(token)?.org_create_team(org, team).await {
|
let client = api(token)?;
|
||||||
|
match client.org_create_team(org, team).await {
|
||||||
Ok(_) => {
|
Ok(_) => {
|
||||||
tracing::info!(%org, "forge: created {OPERATORS_TEAM} team");
|
tracing::info!(%org, "forge: created {OPERATORS_TEAM} team");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
// 409: team already exists — idempotent.
|
// 409: team already exists — reconcile settings to desired state so
|
||||||
|
// a team created with an older/wrong shape self-heals on next boot.
|
||||||
|
// List teams to find the id (required by org_edit_team), then
|
||||||
|
// unconditionally PATCH to the desired settings. Members are a
|
||||||
|
// separate endpoint; this never touches membership.
|
||||||
Err(e) if is_conflict(&e) => {
|
Err(e) if is_conflict(&e) => {
|
||||||
tracing::debug!(%org, "forge: {OPERATORS_TEAM} team already exists");
|
let (_headers, teams) = client
|
||||||
|
.org_list_teams(org)
|
||||||
|
.await
|
||||||
|
.with_context(|| format!("list teams for {org}"))?;
|
||||||
|
let team_id = teams
|
||||||
|
.into_iter()
|
||||||
|
.find(|t| t.name.as_deref() == Some(OPERATORS_TEAM))
|
||||||
|
.and_then(|t| t.id)
|
||||||
|
.with_context(|| {
|
||||||
|
format!("{OPERATORS_TEAM} team not found in {org} after 409 conflict")
|
||||||
|
})?;
|
||||||
|
let edit = EditTeamOption {
|
||||||
|
can_create_org_repo: Some(false),
|
||||||
|
description: Some("hyperhive operators — merge gate for agent repos".to_owned()),
|
||||||
|
includes_all_repositories: Some(true),
|
||||||
|
name: OPERATORS_TEAM.to_owned(),
|
||||||
|
permission: Some(EditTeamOptionPermission::Write),
|
||||||
|
units: Some(units_vec),
|
||||||
|
units_map: None,
|
||||||
|
};
|
||||||
|
client
|
||||||
|
.org_edit_team(team_id, edit)
|
||||||
|
.await
|
||||||
|
.with_context(|| format!("reconcile {org}/{OPERATORS_TEAM} team settings"))?;
|
||||||
|
tracing::debug!(%org, "forge: reconciled {OPERATORS_TEAM} team settings");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
// 422 and any other error: surface it — don't mask a real failure
|
// 422 and any other error: surface it — don't mask a real failure
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue