swarm-controller: mark the instance webhook as a system webhook
`POST /admin/hooks` reads `is_system_webhook` out of the config map and defaults it to false, which creates a forgejo *default* webhook — a template copied into repos created later — instead of a live instance-wide one. `GET /admin/hooks` returns only hooks with the flag set, so `list_hook_urls` could never see what the create had just made: every controller start listed zero hooks and created another default webhook (12 in 26h), while the instance-wide push observation the scope exists for never fired at all. Send the key on the `Instance` arm only, via a per-scope `extra_create_config()` so repo and org scopes stay unchanged. Closes #3807
This commit is contained in:
parent
3939303287
commit
2b35ffd250
1 changed files with 31 additions and 3 deletions
|
|
@ -624,8 +624,11 @@ impl Client {
|
||||||
// arm's line reads exactly like a first, correct registration.
|
// arm's line reads exactly like a first, correct registration.
|
||||||
//
|
//
|
||||||
// The two ways to reach a create have different causes, so they are
|
// The two ways to reach a create have different causes, so they are
|
||||||
// separated: `listed=0` is a permission or scope problem, a non-zero
|
// separated: `listed=0` means the create is not landing in the set
|
||||||
// count with no match means the recorded url is not the one compared.
|
// the list reads (a bucket or permission problem — see
|
||||||
|
// [`HookScope::extra_create_config`], which is where the instance
|
||||||
|
// scope's own version of that went wrong), a non-zero count with no
|
||||||
|
// match means the recorded url is not the one compared.
|
||||||
match scope.list_hook_urls(&self.api).await {
|
match scope.list_hook_urls(&self.api).await {
|
||||||
Ok(urls) => {
|
Ok(urls) => {
|
||||||
if urls.iter().any(|u| u == target_url) {
|
if urls.iter().any(|u| u == target_url) {
|
||||||
|
|
@ -650,6 +653,9 @@ impl Client {
|
||||||
|
|
||||||
let mut additional = BTreeMap::new();
|
let mut additional = BTreeMap::new();
|
||||||
additional.insert("secret".to_owned(), secret.to_owned());
|
additional.insert("secret".to_owned(), secret.to_owned());
|
||||||
|
for (key, value) in scope.extra_create_config() {
|
||||||
|
additional.insert((*key).to_owned(), (*value).to_owned());
|
||||||
|
}
|
||||||
let hook = CreateHookOption {
|
let hook = CreateHookOption {
|
||||||
active: Some(true),
|
active: Some(true),
|
||||||
authorization_header: None,
|
authorization_header: None,
|
||||||
|
|
@ -692,11 +698,33 @@ enum HookScope<'a> {
|
||||||
/// Forgejo's "global (system) webhook" — fires for every repo in the
|
/// Forgejo's "global (system) webhook" — fires for every repo in the
|
||||||
/// instance, in every org, present or future. The `admin_*` API
|
/// instance, in every org, present or future. The `admin_*` API
|
||||||
/// namespace; needs the same `write:admin` scope
|
/// namespace; needs the same `write:admin` scope
|
||||||
/// `Client::ensure_agent_user` already requires on this token.
|
/// `Client::ensure_agent_user` already requires on this token, **and**
|
||||||
|
/// the `is_system_webhook` config key from
|
||||||
|
/// [`Self::extra_create_config`] — that endpoint's default is the
|
||||||
|
/// *other* kind of admin hook.
|
||||||
Instance,
|
Instance,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl HookScope<'_> {
|
impl HookScope<'_> {
|
||||||
|
/// Config-map keys the create call needs beyond the shared ones.
|
||||||
|
///
|
||||||
|
/// Instance scope carries `is_system_webhook`, and it is load-bearing
|
||||||
|
/// twice over. `POST /admin/hooks` reads that key **out of the config
|
||||||
|
/// map** and defaults it to `false`, which makes a forgejo *default*
|
||||||
|
/// webhook — a template copied into repos created later, not a live
|
||||||
|
/// hook — while `GET /admin/hooks` returns only webhooks with the flag
|
||||||
|
/// set. Omitting it therefore breaks both halves at once: the hook is
|
||||||
|
/// not the instance-wide one this scope exists for, and
|
||||||
|
/// [`Self::list_hook_urls`] can never see it, so every process start
|
||||||
|
/// creates another one — twelve in a day, `listed=0` each time,
|
||||||
|
/// before this key was sent.
|
||||||
|
fn extra_create_config(&self) -> &'static [(&'static str, &'static str)] {
|
||||||
|
match self {
|
||||||
|
Self::Repo { .. } | Self::Org { .. } => &[],
|
||||||
|
Self::Instance => &[("is_system_webhook", "true")],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// The `url` config value of every hook currently on this scope.
|
/// The `url` config value of every hook currently on this scope.
|
||||||
async fn list_hook_urls(&self, api: &Forgejo) -> Result<Vec<String>, ForgejoError> {
|
async fn list_hook_urls(&self, api: &Forgejo) -> Result<Vec<String>, ForgejoError> {
|
||||||
let hooks = match self {
|
let hooks = match self {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue