fix(#2570): recognize 422 team-already-exists so operators-team provisioning stops warning
Forgejo returns team-already-exists as HTTP 422 ValidationFailed, not 409 Conflict, so the 409-only guard in ensure_operators_team missed it and logged a spurious warning every boot (and skipped the settings reconcile). Add a lenient discriminator that also treats a 422 whose message says already-exists as benign.
This commit is contained in:
parent
a35b67b7c5
commit
6a4382bee8
1 changed files with 64 additions and 10 deletions
|
|
@ -119,6 +119,33 @@ fn is_conflict(e: &ForgejoError) -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
/// Whether a rendered error message names the "already exists" case. The
|
||||
/// discriminator that tells a *benign* already-exists 422 apart from a
|
||||
/// *real* validation 422 (invalid units, etc.). Case-insensitive.
|
||||
fn message_says_already_exists(rendered: &str) -> bool {
|
||||
rendered.to_lowercase().contains("already exists")
|
||||
}
|
||||
|
||||
/// Whether `e` is Forgejo saying the resource already exists — matching
|
||||
/// the 409 conflict shape *and* the 422 shape this Forgejo build actually
|
||||
/// returns for a duplicate team: `validation failed: team already exists`.
|
||||
/// A 409-only [`is_conflict`] check misses that 422, so the caller fires
|
||||
/// a spurious "provisioning failed" warning every boot and skips its
|
||||
/// settings reconcile. This still surfaces *other* 422s (bad request
|
||||
/// body) as real failures — only a 422 whose message names the
|
||||
/// already-exists case is folded in.
|
||||
fn is_already_exists_lenient(e: &ForgejoError) -> bool {
|
||||
if is_conflict(e) {
|
||||
return true;
|
||||
}
|
||||
let is_unprocessable = match e {
|
||||
ForgejoError::ApiError(api) => matches!(api.error_kind(), ApiErrorKind::ValidationFailed),
|
||||
ForgejoError::UnexpectedStatusCode(s) => *s == StatusCode::UNPROCESSABLE_ENTITY,
|
||||
_ => false,
|
||||
};
|
||||
is_unprocessable && message_says_already_exists(&e.to_string())
|
||||
}
|
||||
|
||||
/// Whether an error is Forgejo saying 404 — the resource is absent,
|
||||
/// as opposed to a transport / auth / server failure.
|
||||
fn is_not_found(e: &ForgejoError) -> bool {
|
||||
|
|
@ -714,12 +741,15 @@ pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()>
|
|||
tracing::info!(%org, "forge: created {OPERATORS_TEAM} team");
|
||||
Ok(())
|
||||
}
|
||||
// 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) => {
|
||||
// Team already exists — reconcile settings to desired state so a
|
||||
// team created with an older/wrong shape self-heals on next boot.
|
||||
// Forgejo signals the duplicate as a 409 conflict OR (this build) a
|
||||
// 422 `validation failed: team already exists`; both mean the same
|
||||
// thing, so fold both in via `is_already_exists_lenient` — a 409-only
|
||||
// check missed the 422 and warned every boot. List teams to find the
|
||||
// id (required by org_edit_team), then unconditionally PATCH to the
|
||||
// desired settings. Members are a separate endpoint; untouched here.
|
||||
Err(e) if is_already_exists_lenient(&e) => {
|
||||
let (_headers, teams) = client
|
||||
.org_list_teams(org)
|
||||
.await
|
||||
|
|
@ -747,10 +777,10 @@ pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()>
|
|||
tracing::debug!(%org, "forge: reconciled {OPERATORS_TEAM} team settings");
|
||||
Ok(())
|
||||
}
|
||||
// 422 and any other error: surface it — don't mask a real failure
|
||||
// as "already exists". A 422 here typically means the request body
|
||||
// is invalid (e.g. Forgejo rejected the units list or another
|
||||
// field); it will repeat every boot until fixed.
|
||||
// Any OTHER error (a 422 that is NOT already-exists, or a transport
|
||||
// / auth failure): surface it — don't mask a real failure. A
|
||||
// non-already-exists 422 means the request body is invalid (e.g.
|
||||
// Forgejo rejected the units list); it repeats every boot until fixed.
|
||||
Err(e) => Err(e).with_context(|| format!("create team {org}/{OPERATORS_TEAM}")),
|
||||
}
|
||||
}
|
||||
|
|
@ -992,3 +1022,27 @@ pub async fn create_agent_repo(agent: &str, repo: &str, core_token: &str) -> Res
|
|||
tracing::info!(%agent, %repo, "forge: created agent repo in {AGENTS_ORG} with operator merge gate");
|
||||
Ok(format!("{AGENTS_ORG}/{repo}"))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::message_says_already_exists;
|
||||
|
||||
#[test]
|
||||
fn already_exists_message_is_recognised() {
|
||||
// The exact 422 body this Forgejo build returns for a duplicate team.
|
||||
assert!(message_says_already_exists(
|
||||
"validation failed: team already exists [org_id: 10, name: operators]"
|
||||
));
|
||||
// Case-insensitive.
|
||||
assert!(message_says_already_exists("Repository Already Exists"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn real_validation_error_is_not_treated_as_already_exists() {
|
||||
// A genuine bad-request 422 must still surface, not be folded in.
|
||||
assert!(!message_says_already_exists(
|
||||
"validation failed: units must not be empty"
|
||||
));
|
||||
assert!(!message_says_already_exists("not found"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue