From 9bb92e8a0299b59a33220179cf5763966b0de9df Mon Sep 17 00:00:00 2001 From: blaipr Date: Mon, 26 Oct 2026 18:09:00 +0100 Subject: [PATCH 1/5] feat: notification history, and applications all or by name --- CHANGELOG.md | 2 +- docs/api.md | 1 + docs/configuration.md | 2 +- docs/notifications.md | 13 ++-- src/adapters/in/http/notifications.rs | 89 +++++++++++++++++++++------ src/adapters/in/http/openapi.rs | 2 + src/adapters/in/http/routes.rs | 4 ++ src/application/notify.rs | 50 +++++++++++++-- src/config.rs | 4 +- src/domain/notification.rs | 68 ++++++++++++++++++-- tests/notifications_test.rs | 12 +++- 11 files changed, 208 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68d00d7..dd9556c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ project adheres to [Semantic Versioning](https://semver.org/). ### Added -- Notifications: a `notifications` item sends `drift_detected`, `out_of_sync`, `sync_succeeded`, `sync_failed` and `back_in_sync` to a webhook (any receiver, with bearer, basic or header auth, a body template and optional HMAC signing) or to an Ascender or AWX job template (its webhook receiver or its launch API), one message per run, queued and retried, never with a field's value; `GET /api/v1/notifications` shows each one's last send and `POST /api/v1/notifications/{id}/test` sends a test (see `docs/notifications.md`). +- Notifications: a `notifications` item sends, for `all` applications or the ones it names, `drift_detected`, `out_of_sync`, `sync_succeeded`, `sync_failed` and `back_in_sync` to a webhook (any receiver, with bearer, basic or header auth, a body template and optional HMAC signing) or to an Ascender or AWX job template (its webhook receiver or its launch API), one message per run, queued and retried, never with a field's value; `GET /api/v1/notifications` shows each one's last send, `GET /api/v1/notifications/{id}` its last 50, and `POST /api/v1/notifications/{id}/test` sends a test (see `docs/notifications.md`). ### Changed diff --git a/docs/api.md b/docs/api.md index 5574284..2b5438f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -19,6 +19,7 @@ session also need its `X-CSRF-Token`). Every failure is `{"error": "..."}`. | `/api/v1/search?q=` | GET | any key (its apps) | Find an object by key or value: its application, kind, whether git declares it | | `/api/v1/connectors` | GET | admin | Connectors: script, project, timeout, who uses them | | `/api/v1/notifications` | GET | admin | Notifications: events, destination, last send | +| `/api/v1/notifications/{id}` | GET | admin | One notification and its last 50 sends | | `/api/v1/notifications/{id}/test` | POST | admin | Send a test message now, once; answers the outcome | | `/api/v1/projects` | GET | admin | Git checkouts, their health, what uses them | | `/api/v1/projects/{id}/sync` | POST | admin | Pull a project now | diff --git a/docs/configuration.md b/docs/configuration.md index 0b9ac0b..664638b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -182,7 +182,7 @@ notifications: - name: ntf-ascender-drift type: ascender # webhook | ascender | awx events: [drift_detected] - applications: [isilon-prod] # empty = every application + applications: [isilon-prod] # or all; left out or [] = none ascender: url: "https://ascender.example.com" job_template: 123 diff --git a/docs/notifications.md b/docs/notifications.md index 53002b8..82f9b09 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -42,7 +42,7 @@ notifications: description: "Any application that fails" type: webhook events: [sync_failed, back_in_sync] - applications: [] # empty = every application + applications: all # all, or a list; left out or [] = none webhook: url: "https://alerts.example.com/hooks/unified-gitops" method: POST # POST (default), PUT or PATCH @@ -56,6 +56,10 @@ notifications: retries: 3 ``` +- **`applications`** is `all` (every application, present and future), or the applications by name. + Left out or `[]`, it listens to none, as an API key or an LDAP rule with no applications reaches + none; Send test still works. + | `auth` | What is sent | Credential field | | --- | --- | --- | | `bearer`, or left out with a credential | `Authorization: Bearer ` | `token` | @@ -145,9 +149,10 @@ notifications: - **Retries:** an unanswered send, a `429` or a `5xx` is retried `retries` times, waiting 2, 4, 8 … up to 60 seconds. A `4xx` is not: a wrong key or template does not fix itself. - **Redirects are not followed:** the body and its credential go only where the configuration says. -- **What is kept:** each notification's last send (event, application, result, status, job, - error), in memory, on `GET /api/v1/notifications`. `POST /api/v1/notifications/{id}/test` - sends a test message now, once, and answers the outcome. +- **What is kept:** each notification's last 50 sends (event, application, result, status, job, + error), in memory: the last on `GET /api/v1/notifications`, all of them on + `GET /api/v1/notifications/{id}`. `POST /api/v1/notifications/{id}/test` sends a test message + now, once, and answers the outcome. - **Metrics:** `unified_gitops_notifications_total{notification,event,outcome}` (`success`, `error`, `dropped`) and `unified_gitops_notification_duration_seconds`. - **Reload:** notifications reload like applications. A message keeps the definition it was queued diff --git a/src/adapters/in/http/notifications.rs b/src/adapters/in/http/notifications.rs index 14425e1..a0a9645 100644 --- a/src/adapters/in/http/notifications.rs +++ b/src/adapters/in/http/notifications.rs @@ -23,7 +23,10 @@ pub struct NotificationInfo { #[serde(rename = "type")] pub notification_type: NotificationType, pub events: Vec, - /// Applications it listens to; empty = every application + /// Every application, present and future (`applications: all`) + pub all_applications: bool, + /// The applications it listens to, when it names them; empty with + /// all_applications false = none pub applications: Vec, /// Where it sends: scheme and host only, never a path or query, which /// can carry a receiver's secret @@ -58,29 +61,77 @@ pub async fn list_notifications( let mut out: Vec = config .notifications .iter() - .map(|(id, n)| { - let target = n.ascender.as_ref().or(n.awx.as_ref()); - let mut applications = n.applications.clone(); - applications.sort(); - NotificationInfo { - name: id.clone(), - description: n.description.clone(), - notification_type: n.notification_type, - events: n.events.clone(), - applications, - destination: n.destination(), - job_template: target.and_then(|t| t.job_template), - workflow_job_template: target.and_then(|t| t.workflow_job_template), - trigger: target.map(|t| t.trigger), - credentials: n.credentials().to_vec(), - last: state.notifier.log.last(id), - } - }) + .map(|(id, n)| info(&state, id, n)) .collect(); out.sort_by(|a, b| a.name.cmp(&b.name)); Ok(Json(out)) } +// One notification as the list shows it. +fn info( + state: &AppState, + id: &str, + n: &crate::domain::notification::Notification, +) -> NotificationInfo { + let target = n.ascender.as_ref().or(n.awx.as_ref()); + let mut applications = n.applications.named().to_vec(); + applications.sort(); + NotificationInfo { + name: id.to_string(), + description: n.description.clone(), + notification_type: n.notification_type, + events: n.events.clone(), + all_applications: n.applications.is_all(), + applications, + destination: n.destination(), + job_template: target.and_then(|t| t.job_template), + workflow_job_template: target.and_then(|t| t.workflow_job_template), + trigger: target.map(|t| t.trigger), + credentials: n.credentials().to_vec(), + last: state.notifier.log.last(id), + } +} + +#[derive(Serialize, ToSchema)] +pub struct NotificationDetail { + #[serde(flatten)] + pub notification: NotificationInfo, + /// Its sends since this process started, newest first, at most 50. + /// Retries of one message count as one send + pub history: Vec, +} + +#[utoipa::path( + get, + path = "/api/v1/notifications/{id}", + tag = "Notifications", + params(("id" = String, Path, description = "Notification name")), + responses( + (status = 200, description = "One notification, with its recent sends", body = NotificationDetail), + (status = 403, description = "API key is not admin", body = ErrorBody), + (status = 404, description = "No such notification", body = ErrorBody) + ) +)] +pub async fn get_notification( + State(state): State>, + axum::Extension(auth): axum::Extension, + Path(id): Path, +) -> Result, ApiError> { + if !auth.permissions.is_admin() { + return Err(ApiError::admin_only()); + } + let config = state.config(); + let Some(n) = config.notifications.get(&id) else { + return Err(ApiError::not_found(format!( + "notification '{id}' is not configured" + ))); + }; + Ok(Json(NotificationDetail { + notification: info(&state, &id, n), + history: state.notifier.log.history(&id), + })) +} + #[utoipa::path( post, path = "/api/v1/notifications/{id}/test", diff --git a/src/adapters/in/http/openapi.rs b/src/adapters/in/http/openapi.rs index 244fd57..88b4baa 100644 --- a/src/adapters/in/http/openapi.rs +++ b/src/adapters/in/http/openapi.rs @@ -41,6 +41,7 @@ impl Modify for SecurityAddon { http::search::search_objects, http::connectors::list_connectors, http::notifications::list_notifications, + http::notifications::get_notification, http::notifications::test_notification, http::projects::list_projects, http::projects::sync_project_now, @@ -78,6 +79,7 @@ impl Modify for SecurityAddon { crate::domain::run::RunResult, http::connectors::ConnectorInfo, http::notifications::NotificationInfo, + http::notifications::NotificationDetail, crate::application::notify::Delivery, crate::domain::notification::NotificationType, crate::domain::notification::Event, diff --git a/src/adapters/in/http/routes.rs b/src/adapters/in/http/routes.rs index 47c2a97..56cb609 100644 --- a/src/adapters/in/http/routes.rs +++ b/src/adapters/in/http/routes.rs @@ -71,6 +71,10 @@ pub fn create_router(state: Arc, api_keys: Arc) -> Rou "/api/v1/notifications", get(http::notifications::list_notifications), ) + .route( + "/api/v1/notifications/{id}", + get(http::notifications::get_notification), + ) .route( "/api/v1/notifications/{id}/test", post(http::notifications::test_notification), diff --git a/src/application/notify.rs b/src/application/notify.rs index cde716c..38057da 100644 --- a/src/application/notify.rs +++ b/src/application/notify.rs @@ -493,8 +493,7 @@ fn hex(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{b:02x}")).collect() } -// The last send of each notification, for the API and the UI. In memory, -// like run history. +// One send of a notification, for the API and the UI. #[derive(Debug, Clone, Serialize, ToSchema)] pub struct Delivery { pub event: String, @@ -510,18 +509,34 @@ pub struct Delivery { pub error: Option, } +// How many sends each notification keeps. In memory, like run history: a +// restart starts it over. +pub const DELIVERY_HISTORY: usize = 50; + +// Each notification's recent sends, newest first. #[derive(Default)] pub struct DeliveryLog { - last: DashMap, + sends: DashMap>, } impl DeliveryLog { pub fn last(&self, notification_id: &str) -> Option { - self.last.get(notification_id).map(|d| d.clone()) + self.sends + .get(notification_id) + .and_then(|h| h.front().cloned()) + } + + pub fn history(&self, notification_id: &str) -> Vec { + self.sends + .get(notification_id) + .map(|h| h.iter().cloned().collect()) + .unwrap_or_default() } fn record(&self, notification_id: &str, delivery: Delivery) { - self.last.insert(notification_id.to_string(), delivery); + let mut history = self.sends.entry(notification_id.to_string()).or_default(); + history.push_front(delivery); + history.truncate(DELIVERY_HISTORY); } } @@ -1135,6 +1150,31 @@ mod tests { assert!(interpret(&n, &refused).unwrap_err().starts_with("HTTP 403")); } + #[test] + fn the_log_keeps_the_newest_sends_first_and_bounded() { + let log = DeliveryLog::default(); + for i in 0..(DELIVERY_HISTORY as u64 + 5) { + log.record( + "n", + Delivery { + event: "test".into(), + application: None, + at: i, + ok: true, + attempts: 1, + status: Some(202), + job: None, + error: None, + }, + ); + } + let history = log.history("n"); + assert_eq!(history.len(), DELIVERY_HISTORY); + assert_eq!(history[0].at, DELIVERY_HISTORY as u64 + 4); + assert_eq!(log.last("n").unwrap().at, history[0].at); + assert!(log.history("other").is_empty()); + } + #[test] fn only_unanswered_or_server_failures_are_retried() { assert!(retryable(&Err("timed out".into()))); diff --git a/src/config.rs b/src/config.rs index df34b29..edd8ba8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -542,7 +542,7 @@ impl AppConfig { // to and the credentials they send with. for (id, notification) in &self.notifications { errors.extend(notification.errors(id)); - for app_id in ¬ification.applications { + for app_id in notification.applications.named() { if !self.applications.contains_key(app_id) { errors.push(format!( "Notification '{id}' references unknown application '{app_id}'" @@ -967,7 +967,7 @@ mod tests { ]); let cfg = load_config(dir.path().to_str().unwrap()).expect("loads"); let n = &cfg.notifications["ntf-ascender-drift"]; - assert_eq!(n.applications, vec!["portainer".to_string()]); + assert_eq!(n.applications.named(), ["portainer".to_string()]); } #[test] diff --git a/src/domain/notification.rs b/src/domain/notification.rs index 0a3e65e..5dfad52 100644 --- a/src/domain/notification.rs +++ b/src/domain/notification.rs @@ -16,9 +16,10 @@ pub struct Notification { #[serde(rename = "type")] pub notification_type: NotificationType, pub events: Vec, - // Empty = every application. + // `all`, or the applications by name. Left out or `[]`: none, as an API + // key or an LDAP rule with no applications reaches none. #[serde(default)] - pub applications: Vec, + pub applications: Applications, #[serde(default)] pub webhook: Option, #[serde(default)] @@ -27,6 +28,48 @@ pub struct Notification { pub awx: Option, } +// Which applications a notification listens to: the word `all`, or a list. +#[derive(Debug, Clone, PartialEq, Deserialize)] +#[serde(untagged)] +pub enum Applications { + All(AllWord), + Named(Vec), +} + +// The one word `applications:` takes instead of a list. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum AllWord { + All, +} + +impl Default for Applications { + fn default() -> Self { + Applications::Named(Vec::new()) + } +} + +impl Applications { + pub fn is_all(&self) -> bool { + matches!(self, Applications::All(_)) + } + + // The applications named; empty for `all`. + pub fn named(&self) -> &[String] { + match self { + Applications::All(_) => &[], + Applications::Named(names) => names, + } + } + + pub fn includes(&self, application: &str) -> bool { + match self { + Applications::All(_) => true, + Applications::Named(names) => names.iter().any(|a| a == application), + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, ToSchema)] #[serde(rename_all = "snake_case")] pub enum NotificationType { @@ -291,8 +334,7 @@ impl Notification { // Whether this notification wants `event` from `application`. pub fn wants(&self, event: Event, application: &str) -> bool { - self.events.contains(&event) - && (self.applications.is_empty() || self.applications.iter().any(|a| a == application)) + self.events.contains(&event) && self.applications.includes(application) } // Where it sends, without anything a URL can carry that is secret: a @@ -607,9 +649,23 @@ mod tests { assert!(n.wants(Event::SyncFailed, "a")); assert!(!n.wants(Event::SyncFailed, "b")); assert!(!n.wants(Event::DriftDetected, "a")); - let all = - parse("type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com\n"); + let all = parse( + "type: webhook\nevents: [sync_failed]\napplications: all\nwebhook:\n url: https://x.example.com\n", + ); + assert!(all.applications.is_all()); assert!(all.wants(Event::SyncFailed, "anything")); + // Left out or empty: none, as an API key with no applications. + let none = + parse("type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com\n"); + assert!(!none.wants(Event::SyncFailed, "anything")); + let empty = parse( + "type: webhook\nevents: [sync_failed]\napplications: []\nwebhook:\n url: https://x.example.com\n", + ); + assert!(!empty.wants(Event::SyncFailed, "anything")); + assert!(serde_saphyr::from_str::( + "type: webhook\nevents: [sync_failed]\napplications: every\nwebhook:\n url: https://x.example.com\n" + ) + .is_err()); } #[test] diff --git a/tests/notifications_test.rs b/tests/notifications_test.rs index 9bbe743..2f10794 100644 --- a/tests/notifications_test.rs +++ b/tests/notifications_test.rs @@ -91,7 +91,7 @@ fn fixture() -> Fixture { let project: unified_gitops::domain::project::GitProject = serde_saphyr::from_str("git_url: https://example.com/state.git\n").unwrap(); let hook: unified_gitops::domain::notification::Notification = serde_saphyr::from_str( - "type: webhook\nevents: [sync_succeeded, sync_failed]\nwebhook:\n url: https://hooks.example.com/services/T0/secret-path\n credentials: [cred-hook]\n retries: 0\n", + "type: webhook\nevents: [sync_succeeded, sync_failed]\napplications: all\nwebhook:\n url: https://hooks.example.com/services/T0/secret-path\n credentials: [cred-hook]\n retries: 0\n", ) .unwrap(); @@ -224,6 +224,16 @@ async fn send_test_sends_now_and_answers_the_outcome() { let body: Value = serde_json::from_slice(&sent[0].body).unwrap(); assert_eq!(body["event"], "test"); + // Kept in its history, newest first. + call(&f.app, "POST", "/api/v1/notifications/hook/test").await; + let (status, detail) = call(&f.app, "GET", "/api/v1/notifications/hook").await; + assert_eq!(status, StatusCode::OK, "{detail}"); + assert_eq!(detail["name"], "hook"); + assert_eq!(detail["history"].as_array().unwrap().len(), 2); + assert_eq!(detail["history"][0]["event"], "test"); + let (status, _) = call(&f.app, "GET", "/api/v1/notifications/nope").await; + assert_eq!(status, StatusCode::NOT_FOUND); + let (status, _) = call(&f.app, "POST", "/api/v1/notifications/nope/test").await; assert_eq!(status, StatusCode::NOT_FOUND); } From 3fae2bdf80396b72e0e4c94e112a1944c1baaa2a Mon Sep 17 00:00:00 2001 From: blaipr Date: Mon, 26 Oct 2026 18:27:30 +0100 Subject: [PATCH 2/5] feat: say how each notification sends, never with a value --- src/adapters/in/http/notifications.rs | 57 +++++++++++++++++++++++++++ src/adapters/in/http/openapi.rs | 1 + tests/notifications_test.rs | 8 ++++ 3 files changed, 66 insertions(+) diff --git a/src/adapters/in/http/notifications.rs b/src/adapters/in/http/notifications.rs index a0a9645..28cb95b 100644 --- a/src/adapters/in/http/notifications.rs +++ b/src/adapters/in/http/notifications.rs @@ -37,10 +37,32 @@ pub struct NotificationInfo { /// ascender and awx: through the template's webhook receiver or its launch API pub trigger: Option, pub credentials: Vec, + /// How long one attempt may take, and how many times a send is retried + pub timeout_seconds: u64, + pub retries: u32, + /// webhook: its HTTP method, how the credential is sent, the names of + /// its fixed headers (never their values), whether it sends a template or + /// the default message, and the header its signature goes in, if signed + pub webhook: Option, + /// ascender and awx, trigger launch: the names of the extra_vars it adds + /// beside the event (never their values) + pub extra_vars: Vec, /// The last send since this process started, if any pub last: Option, } +#[derive(Serialize, ToSchema)] +pub struct WebhookInfo { + pub method: String, + /// bearer, basic, none, or header (the credential in `auth_header`) + pub auth: String, + pub auth_header: Option, + pub headers: Vec, + /// true: a body template; false: the default message + pub templated: bool, + pub signature_header: Option, +} + #[utoipa::path( get, path = "/api/v1/notifications", @@ -88,6 +110,41 @@ fn info( workflow_job_template: target.and_then(|t| t.workflow_job_template), trigger: target.map(|t| t.trigger), credentials: n.credentials().to_vec(), + timeout_seconds: n + .webhook + .as_ref() + .map(|w| w.timeout_seconds) + .or(target.map(|t| t.timeout_seconds)) + .unwrap_or_default(), + retries: n + .webhook + .as_ref() + .map(|w| w.retries) + .or(target.map(|t| t.retries)) + .unwrap_or_default(), + webhook: n.webhook.as_ref().map(|w| { + use crate::domain::notification::{Auth, AuthScheme}; + let (auth, auth_header) = match &w.auth { + Auth::Header { header } => ("header", Some(header.clone())), + Auth::Scheme(AuthScheme::Basic) => ("basic", None), + Auth::Scheme(AuthScheme::None) => ("none", None), + Auth::Scheme(AuthScheme::Bearer) => ("bearer", None), + // Left out: a bearer token when there is a credential. + Auth::Default if w.credentials.is_empty() => ("none", None), + Auth::Default => ("bearer", None), + }; + WebhookInfo { + method: w.method.clone(), + auth: auth.to_string(), + auth_header, + headers: w.headers.keys().cloned().collect(), + templated: w.body.is_some(), + signature_header: w.sign.as_ref().map(|s| s.header.clone()), + } + }), + extra_vars: target + .map(|t| t.extra_vars.keys().cloned().collect()) + .unwrap_or_default(), last: state.notifier.log.last(id), } } diff --git a/src/adapters/in/http/openapi.rs b/src/adapters/in/http/openapi.rs index 88b4baa..762be00 100644 --- a/src/adapters/in/http/openapi.rs +++ b/src/adapters/in/http/openapi.rs @@ -80,6 +80,7 @@ impl Modify for SecurityAddon { http::connectors::ConnectorInfo, http::notifications::NotificationInfo, http::notifications::NotificationDetail, + http::notifications::WebhookInfo, crate::application::notify::Delivery, crate::domain::notification::NotificationType, crate::domain::notification::Event, diff --git a/tests/notifications_test.rs b/tests/notifications_test.rs index 2f10794..ce33f69 100644 --- a/tests/notifications_test.rs +++ b/tests/notifications_test.rs @@ -191,6 +191,14 @@ async fn a_sync_that_applies_a_change_sends_sync_succeeded() { assert_eq!(status, StatusCode::OK, "{list}"); assert_eq!(list[0]["name"], "hook"); assert_eq!(list[0]["destination"], "https://hooks.example.com"); + // How it sends, never a value: a bearer token (a credential, auth left + // out), the default message, no signature. + assert_eq!(list[0]["webhook"]["auth"], "bearer"); + assert_eq!(list[0]["webhook"]["method"], "POST"); + assert_eq!(list[0]["webhook"]["templated"], false); + assert_eq!(list[0]["retries"], 0); + assert_eq!(list[0]["timeout_seconds"], 30); + assert!(!list.to_string().contains("hook-token")); assert_eq!(list[0]["last"]["ok"], true); assert_eq!(list[0]["last"]["event"], "sync_succeeded"); assert_eq!(list[0]["last"]["status"], 202); From 8edccd266f3c9e7b8391074bbc703d7f6a87f6ab Mon Sep 17 00:00:00 2001 From: blaipr Date: Mon, 26 Oct 2026 18:52:00 +0100 Subject: [PATCH 3/5] refactor!: notifications are webhooks, Ascender and AWX through theirs only --- CHANGELOG.md | 2 +- CLAUDE.md | 8 +- README.md | 4 +- docs/api.md | 6 +- docs/configuration.md | 16 +- docs/observability.md | 4 +- docs/{notifications.md => webhooks.md} | 61 ++-- src/adapters/in/http/config.rs | 8 +- src/adapters/in/http/index.rs | 4 +- src/adapters/in/http/mod.rs | 2 +- src/adapters/in/http/openapi.rs | 21 +- src/adapters/in/http/routes.rs | 14 +- .../in/http/{notifications.rs => webhooks.rs} | 123 ++++---- src/adapters/out/mod.rs | 2 +- src/adapters/out/{notify => webhooks}/http.rs | 2 +- src/adapters/out/{notify => webhooks}/mod.rs | 0 src/application/config.rs | 8 +- src/application/mod.rs | 2 +- src/application/reconcile.rs | 18 +- src/application/{notify.rs => webhooks.rs} | 278 ++++++------------ src/config.rs | 44 +-- src/domain/mod.rs | 2 +- src/domain/run.rs | 4 +- src/domain/{notification.rs => webhook.rs} | 127 +++----- src/lib.rs | 25 +- src/main.rs | 8 +- src/ports/{notifier.rs => http_sender.rs} | 6 +- src/ports/mod.rs | 2 +- src/state.rs | 20 +- ...notifications_test.rs => webhooks_test.rs} | 34 +-- 30 files changed, 339 insertions(+), 516 deletions(-) rename docs/{notifications.md => webhooks.md} (71%) rename src/adapters/in/http/{notifications.rs => webhooks.rs} (62%) rename src/adapters/out/{notify => webhooks}/http.rs (96%) rename src/adapters/out/{notify => webhooks}/mod.rs (100%) rename src/application/{notify.rs => webhooks.rs} (78%) rename src/domain/{notification.rs => webhook.rs} (84%) rename src/ports/{notifier.rs => http_sender.rs} (78%) rename tests/{notifications_test.rs => webhooks_test.rs} (90%) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd9556c..ed6b3e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ project adheres to [Semantic Versioning](https://semver.org/). ### Added -- Notifications: a `notifications` item sends, for `all` applications or the ones it names, `drift_detected`, `out_of_sync`, `sync_succeeded`, `sync_failed` and `back_in_sync` to a webhook (any receiver, with bearer, basic or header auth, a body template and optional HMAC signing) or to an Ascender or AWX job template (its webhook receiver or its launch API), one message per run, queued and retried, never with a field's value; `GET /api/v1/notifications` shows each one's last send, `GET /api/v1/notifications/{id}` its last 50, and `POST /api/v1/notifications/{id}/test` sends a test (see `docs/notifications.md`). +- Webhooks: a `webhooks` item sends, for `all` applications or the ones it names, `drift_detected`, `out_of_sync`, `sync_succeeded`, `sync_failed` and `back_in_sync` to a webhook (any receiver, with bearer, basic or header auth, a body template and optional HMAC signing) or to an Ascender or AWX job template (through the template's own webhook receiver, so no user is needed), one message per run, queued and retried, never with a field's value; `GET /api/v1/webhooks` shows each one's last send, `GET /api/v1/webhooks/{id}` its last 50, and `POST /api/v1/webhooks/{id}/test` sends a test (see `docs/webhooks.md`). ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index 481a084..a6e0f74 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,17 +45,17 @@ src/ │ ├── desired.rs # desired-state files: parse, load a folder, validate, order │ ├── plan.rs # THE DIFF: only declared fields compared → Operation list │ ├── run.rs # SyncStatus, RunRecord, PlanSnapshot, AppStateRegistry -│ ├── notification.rs # Notification: type (webhook/ascender/awx), events, its block +│ ├── webhook.rs # Webhook: type (webhook/ascender/awx), events, its block │ ├── yaml.rs # YAML → JSON with exact scalars ("0644" stays a string) │ ├── credential.rs, api_key.rs, ldap.rs, project.rs, sync_health.rs ├── application/ │ ├── reconcile.rs # THE LOOP: validate → list → copy → diff → apply → re-list │ ├── projects.rs # pull a checkout (bounded by timeout_seconds) -│ ├── notify.rs # event per run (detect), message, request per type, queue + worker +│ ├── webhooks.rs # event per run (detect), message, request per type, queue + worker │ ├── scripts.rs # per-run script path resolution into checkouts │ ├── credentials.rs, login.rs, config.rs (reload) ├── ports/ # connector, state_store, git, secrets, directory, config_store, venv, -│ # notifier (sends one built HTTP request) +│ # http_sender (sends one built HTTP request) └── adapters/ ├── in/http/ # applications (+ state copies), connectors, projects, │ # credentials, settings, users (auth), config, login, session, @@ -64,7 +64,7 @@ src/ └── out/ ├── connectors/process.rs # ConnectorPort → script (scrubbed env, APPLICATION_CONFIG, timeout kills) ├── state/fs.rs # StateStorePort → //.yaml - ├── notify/http.rs # HttpSenderPort → reqwest (no redirects followed) + ├── webhooks/http.rs # HttpSenderPort → reqwest (no redirects followed) ├── git/, secrets/, directory/, config/, python.rs, process_env.rs tests/ # integration tests; tests/connectors/ holds test connector scripts # (kv.py + the SDK, copied from unified-connectors) diff --git a/README.md b/README.md index f693d49..a9603f4 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ network, and a change made by hand on a system shows up as drift. - **Self-describing connectors**: a script says what it manages, each type's key and what goes first - **One application, one system**: its address and credentials live in config, never in git - **Run history**: which commit was applied, when, and the result of every operation -- **Notifications**: drift, applied commits and failures to a webhook or an Ascender or AWX job +- **Webhooks**: drift, applied commits and failures to a webhook or an Ascender or AWX job - **Config as code**: one item per file, any folders; `name` is the identity - **Strict validation**: a typo in desired state or config fails before any system is touched - **Web UI and LDAP sign-in**: the same UI family and sign-in as unified-api, on its own address @@ -42,7 +42,7 @@ network, and a change made by hand on a system shows up as drift. | [How it works](docs/how-it-works.md) | The reconcile loop: pull, list, diff, apply, and what each status means | | [Connectors](docs/connectors.md) | The script contract, the Python SDK, and writing a connector | | [Configuration](docs/configuration.md) | Every YAML file field by field, and desired-state files | -| [Notifications](docs/notifications.md) | Drift, syncs and failures to a webhook or an Ascender or AWX job template | +| [Webhooks](docs/webhooks.md) | Drift, syncs and failures to a webhook or an Ascender or AWX job template | | [API](docs/api.md) | All routes with authentication and examples | | [Architecture](docs/architecture.md) | The layers, the dependency rule, and what is shared with unified-api | | [Configuration API](docs/config-api.md) | Pushing the config directory over HTTP and reloading it live | diff --git a/docs/api.md b/docs/api.md index 2b5438f..9410866 100644 --- a/docs/api.md +++ b/docs/api.md @@ -18,9 +18,9 @@ session also need its `X-CSRF-Token`). Every failure is `{"error": "..."}`. | `/api/v1/applications/{id}/state/{kind}` | GET | key granted the app | The system's current settings, as of its last check | | `/api/v1/search?q=` | GET | any key (its apps) | Find an object by key or value: its application, kind, whether git declares it | | `/api/v1/connectors` | GET | admin | Connectors: script, project, timeout, who uses them | -| `/api/v1/notifications` | GET | admin | Notifications: events, destination, last send | -| `/api/v1/notifications/{id}` | GET | admin | One notification and its last 50 sends | -| `/api/v1/notifications/{id}/test` | POST | admin | Send a test message now, once; answers the outcome | +| `/api/v1/webhooks` | GET | admin | Webhooks: events, destination, last send | +| `/api/v1/webhooks/{id}` | GET | admin | One webhook and its last 50 sends | +| `/api/v1/webhooks/{id}/test` | POST | admin | Send a test message now, once; answers the outcome | | `/api/v1/projects` | GET | admin | Git checkouts, their health, what uses them | | `/api/v1/projects/{id}/sync` | POST | admin | Pull a project now | | `/api/v1/credentials` | GET | admin | Credentials: where from, whether they resolve | diff --git a/docs/configuration.md b/docs/configuration.md index 664638b..c33b19f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -22,8 +22,8 @@ as startup does, and exits. │ └── cred-isilon.yaml ├── api_keys/ │ └── key-pipeline.yaml -└── notifications/ - └── ntf-failures.yaml +└── webhooks/ + └── wh-failures.yaml ``` | Top-level key | Holds | @@ -33,7 +33,7 @@ as startup does, and exits. | `projects` | Git repositories: desired state, and connector scripts | | `credentials` | Where each credential's secrets come from (never the values) | | `api_keys` | API keys by name and role (the values come from env vars) | -| `notifications` | What runs report, and where: webhooks, Ascender and AWX job templates | +| `webhooks` | What runs report, and where: webhooks, Ascender and AWX job templates | - **The key says what an item is, never the folder.** A file in `connectors/` holding `applications:` is read as an application. @@ -172,14 +172,14 @@ api_keys: `UNIFIED_GITOPS_KEY`, if set, is one more admin key. With no keys at all the API is open, which is logged loudly at startup. -## notifications +## webhooks -Which events, from which applications, go where. The full reference is [notifications.md](notifications.md): +Which events, from which applications, go where. The full reference is [webhooks.md](webhooks.md): ```yaml -# notifications/ntf-ascender-drift.yaml -notifications: - - name: ntf-ascender-drift +# webhooks/wh-ascender-drift.yaml +webhooks: + - name: wh-ascender-drift type: ascender # webhook | ascender | awx events: [drift_detected] applications: [isilon-prod] # or all; left out or [] = none diff --git a/docs/observability.md b/docs/observability.md index 3a3f095..5082c58 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -27,8 +27,8 @@ stops running pushes nothing, so only a scrape-time gauge shows the silence. | `unified_gitops_project_sync_last_success_age_seconds` | project | Seconds since the last good pull | | `unified_gitops_connector_calls_total` | application, connector, command, outcome | Calls to the systems: `list` reads, `apply` writes, `describe` starts a run | | `unified_gitops_connector_call_duration_seconds` | application, connector, command | How long each call to a system took (histogram) | -| `unified_gitops_notifications_total` | notification, event, outcome | Messages sent (`success`), failed after retries (`error`), or dropped by a full queue | -| `unified_gitops_notification_duration_seconds` | notification | How long a send took, retries included (histogram) | +| `unified_gitops_webhooks_total` | webhook, event, outcome | Messages sent (`success`), failed after retries (`error`), or dropped by a full queue | +| `unified_gitops_webhook_duration_seconds` | webhook | How long a send took, retries included (histogram) | | `unified_gitops_http_requests_total` | method, path, status | Requests, by matched route | | `unified_gitops_http_request_duration_seconds` | method, path | Latency histogram | | `unified_gitops_config_generation` | | Bumped by every applied reload | diff --git a/docs/notifications.md b/docs/webhooks.md similarity index 71% rename from docs/notifications.md rename to docs/webhooks.md index 82f9b09..a8f433e 100644 --- a/docs/notifications.md +++ b/docs/webhooks.md @@ -1,7 +1,7 @@ -# Notifications +# Webhooks -A notification tells another system what happened to an application: drift found, a commit applied, -a sync that failed. It is a configuration item like the others, under `notifications:`. Every send +A webhook tells another system what happened to an application: drift found, a commit applied, +a sync that failed. It is a configuration item like the others, under `webhooks:`. Every send goes out from this instance, so nothing has to reach in. - [Events](#events) @@ -36,9 +36,9 @@ Each run produces at most one event, so one change never arrives twice under two The `type` names the receiver, and its settings live in the block of the same name. ```yaml -# notifications/ntf-failures.yaml -notifications: - - name: ntf-failures +# webhooks/wh-failures.yaml +webhooks: + - name: wh-failures description: "Any application that fails" type: webhook events: [sync_failed, back_in_sync] @@ -75,7 +75,8 @@ notifications: - **Never in the YAML:** a token, a password, or a URL that carries one. They belong in credentials, which come from env vars or Vault. -The `ascender` and `awx` types start a job template; see [Ascender and AWX](#ascender-and-awx). +The `ascender` and `awx` types start a job template through its webhook; see +[Ascender and AWX](#ascender-and-awx). ## The message @@ -84,7 +85,7 @@ The default body, and the fields a `body` template can use: ```json { "event": "drift_detected", - "notification": "ntf-ascender-drift", + "webhook": "wh-ascender-drift", "application": "portainer", "description": "Portainer settings, environments and users", "status": "synced", @@ -110,13 +111,13 @@ The default body, and the fields a `body` template can use: ## Ascender and AWX -Both start a job template (or a workflow job template) in two ways. Their routes are the same today; -they are separate types so each follows its own product. +Both start a job template (or a workflow job template) through the template's own webhook receiver. +Their routes are the same today; they are separate types so each follows its own product. ```yaml -# notifications/ntf-ascender-drift.yaml -notifications: - - name: ntf-ascender-drift +# webhooks/wh-ascender-drift.yaml +webhooks: + - name: wh-ascender-drift description: "Drift report in Ascender" type: ascender # or awx, with an awx: block events: [drift_detected] @@ -124,22 +125,16 @@ notifications: ascender: url: "https://ascender.example.com" # the controller, without /api job_template: 123 # or workflow_job_template: 45 - trigger: webhook # webhook (default) | launch - credentials: [cred-ascender-jt123] - # extra_vars: {report: drift} # launch only + credentials: [cred-ascender-jt123] # its token is the template's webhook key ``` -| `trigger` | Route | The credential's `token` | The playbook reads the message as | -| --- | --- | --- | --- | -| `webhook` | `/api/v2/job_templates/{id}/gitlab/` | The template's webhook key | `ascender_webhook_payload` (Ascender), `tower_webhook_payload` (both) | -| `launch` | `/api/v2/job_templates/{id}/launch/` | A user's OAuth token | `unified_gitops`, beside `extra_vars` | - -- **`webhook`** needs the template's **Webhooks** option on, with service **GitLab**: the - controller then gives it a key, compared as is, and no user is needed. Any event is accepted. -- **`launch`** needs a user allowed to run the template, and **Prompt on launch** for its variables. - Without it the controller drops `extra_vars`; the send reports that as a failure instead of - starting a job without the event. -- The last send records the job a `launch` started. +- **On the template:** turn its **Webhooks** option on, with service **GitLab**. The controller then + gives it a key; put that in a credential, from Vault. No user and no token are needed, and the key + can start that one template only. +- **The send:** `POST /api/v2/job_templates/{id}/gitlab/` with the key in `X-Gitlab-Token`, compared + as is, and the message as the body. Any event is accepted. +- **In the playbook:** the message is `ascender_webhook_payload` (Ascender), `awx_webhook_payload` + (AWX), or `tower_webhook_payload` (both). ## Sending, retries and what is kept @@ -149,11 +144,11 @@ notifications: - **Retries:** an unanswered send, a `429` or a `5xx` is retried `retries` times, waiting 2, 4, 8 … up to 60 seconds. A `4xx` is not: a wrong key or template does not fix itself. - **Redirects are not followed:** the body and its credential go only where the configuration says. -- **What is kept:** each notification's last 50 sends (event, application, result, status, job, - error), in memory: the last on `GET /api/v1/notifications`, all of them on - `GET /api/v1/notifications/{id}`. `POST /api/v1/notifications/{id}/test` sends a test message +- **What is kept:** each webhook's last 50 sends (event, application, result, status, + error), in memory: the last on `GET /api/v1/webhooks`, all of them on + `GET /api/v1/webhooks/{id}`. `POST /api/v1/webhooks/{id}/test` sends a test message now, once, and answers the outcome. -- **Metrics:** `unified_gitops_notifications_total{notification,event,outcome}` (`success`, - `error`, `dropped`) and `unified_gitops_notification_duration_seconds`. -- **Reload:** notifications reload like applications. A message keeps the definition it was queued +- **Metrics:** `unified_gitops_webhooks_total{webhook,event,outcome}` (`success`, + `error`, `dropped`) and `unified_gitops_webhook_duration_seconds`. +- **Reload:** webhooks reload like applications. A message keeps the definition it was queued with. diff --git a/src/adapters/in/http/config.rs b/src/adapters/in/http/config.rs index 41406f0..fb0e68e 100644 --- a/src/adapters/in/http/config.rs +++ b/src/adapters/in/http/config.rs @@ -53,7 +53,7 @@ pub struct ConfigSummary { pub credentials: usize, pub projects: usize, pub api_keys: usize, - pub notifications: usize, + pub webhooks: usize, } #[derive(Serialize, ToSchema)] @@ -117,7 +117,7 @@ pub struct ReloadInfo { pub connectors: DeltaInfo, pub projects: DeltaInfo, pub credentials: DeltaInfo, - pub notifications: DeltaInfo, + pub webhooks: DeltaInfo, /// How many API keys are in force after the reload pub api_keys: usize, } @@ -253,7 +253,7 @@ fn summarize(cfg: &AppConfig) -> ConfigSummary { credentials: cfg.credentials.len(), projects: cfg.projects.len(), api_keys: cfg.api_keys.len(), - notifications: cfg.notifications.len(), + webhooks: cfg.webhooks.len(), } } @@ -266,7 +266,7 @@ fn reload_info(report: &reload_use_case::ReloadReport, api_keys: usize) -> Reloa connectors: (&report.connectors).into(), projects: (&report.projects).into(), credentials: (&report.credentials).into(), - notifications: (&report.notifications).into(), + webhooks: (&report.webhooks).into(), api_keys, } } diff --git a/src/adapters/in/http/index.rs b/src/adapters/in/http/index.rs index 18d3576..0f7e75b 100644 --- a/src/adapters/in/http/index.rs +++ b/src/adapters/in/http/index.rs @@ -32,7 +32,7 @@ pub struct ApiIndex { pub connectors: Option<&'static str>, /// Admin-only; absent for a restricted key #[serde(skip_serializing_if = "Option::is_none")] - pub notifications: Option<&'static str>, + pub webhooks: Option<&'static str>, /// Admin-only route; absent for a restricted key #[serde(skip_serializing_if = "Option::is_none")] pub projects: Option<&'static str>, @@ -74,7 +74,7 @@ pub async fn api_index( applications: "/api/v1/applications", search: "/api/v1/search", connectors: admin.then_some("/api/v1/connectors"), - notifications: admin.then_some("/api/v1/notifications"), + webhooks: admin.then_some("/api/v1/webhooks"), projects: admin.then_some("/api/v1/projects"), // Same condition the config handlers enforce: the routes exist in the // router either way, and answer 403 when no store is configured. diff --git a/src/adapters/in/http/mod.rs b/src/adapters/in/http/mod.rs index 1fd4394..6ad5147 100644 --- a/src/adapters/in/http/mod.rs +++ b/src/adapters/in/http/mod.rs @@ -12,7 +12,6 @@ pub mod html; pub mod index; pub mod login; pub mod metrics; -pub mod notifications; pub mod openapi; pub mod projects; pub mod routes; @@ -22,3 +21,4 @@ pub mod settings; pub mod spec; pub mod ui_assets; pub mod users; +pub mod webhooks; diff --git a/src/adapters/in/http/openapi.rs b/src/adapters/in/http/openapi.rs index 762be00..943d64f 100644 --- a/src/adapters/in/http/openapi.rs +++ b/src/adapters/in/http/openapi.rs @@ -40,9 +40,9 @@ impl Modify for SecurityAddon { http::applications::get_application_state, http::search::search_objects, http::connectors::list_connectors, - http::notifications::list_notifications, - http::notifications::get_notification, - http::notifications::test_notification, + http::webhooks::list_webhooks, + http::webhooks::get_webhook, + http::webhooks::test_webhook, http::projects::list_projects, http::projects::sync_project_now, http::credentials::list_credentials, @@ -78,13 +78,12 @@ impl Modify for SecurityAddon { crate::domain::run::RunMode, crate::domain::run::RunResult, http::connectors::ConnectorInfo, - http::notifications::NotificationInfo, - http::notifications::NotificationDetail, - http::notifications::WebhookInfo, - crate::application::notify::Delivery, - crate::domain::notification::NotificationType, - crate::domain::notification::Event, - crate::domain::notification::JobTrigger, + http::webhooks::WebhookSummary, + http::webhooks::WebhookDetail, + http::webhooks::RequestInfo, + crate::application::webhooks::Delivery, + crate::domain::webhook::WebhookType, + crate::domain::webhook::Event, crate::domain::connector::ConnectorType, crate::ports::state_store::Snapshot, crate::application::search::FoundObject, @@ -123,7 +122,7 @@ impl Modify for SecurityAddon { (name = "Health", description = "Liveness and readiness probes"), (name = "Applications", description = "Each live system and its desired state in git: status, the pending plan, run history and the local copy of the system's current settings; refresh compares, sync applies"), (name = "Connectors", description = "The scripts that talk to each kind of system, and the applications using them (admin-only)"), - (name = "Notifications", description = "What runs report and where it goes: webhooks and Ascender or AWX job templates, with each one's last send (admin-only)"), + (name = "Webhooks", description = "What runs report, sent to any HTTP receiver or to an Ascender or AWX job template's webhook, with each one's recent sends (admin-only)"), (name = "Projects", description = "Git project checkouts (admin-only operational routes)"), (name = "Credentials", description = "Configured credentials: where each one's secrets come from, whether they resolve now, and what uses it (admin-only; never a secret value)"), (name = "Auth", description = "Who the caller is, and the ways in: LDAP login and API keys (read-only; declared in git)"), diff --git a/src/adapters/in/http/routes.rs b/src/adapters/in/http/routes.rs index 56cb609..8acf20e 100644 --- a/src/adapters/in/http/routes.rs +++ b/src/adapters/in/http/routes.rs @@ -67,17 +67,11 @@ pub fn create_router(state: Arc, api_keys: Arc) -> Rou ) .route("/api/v1/search", get(http::search::search_objects)) .route("/api/v1/connectors", get(http::connectors::list_connectors)) + .route("/api/v1/webhooks", get(http::webhooks::list_webhooks)) + .route("/api/v1/webhooks/{id}", get(http::webhooks::get_webhook)) .route( - "/api/v1/notifications", - get(http::notifications::list_notifications), - ) - .route( - "/api/v1/notifications/{id}", - get(http::notifications::get_notification), - ) - .route( - "/api/v1/notifications/{id}/test", - post(http::notifications::test_notification), + "/api/v1/webhooks/{id}/test", + post(http::webhooks::test_webhook), ) .route("/api/v1/projects", get(http::projects::list_projects)) .route( diff --git a/src/adapters/in/http/notifications.rs b/src/adapters/in/http/webhooks.rs similarity index 62% rename from src/adapters/in/http/notifications.rs rename to src/adapters/in/http/webhooks.rs index 28cb95b..32b843c 100644 --- a/src/adapters/in/http/notifications.rs +++ b/src/adapters/in/http/webhooks.rs @@ -8,20 +8,20 @@ use utoipa::ToSchema; use crate::AppState; use crate::adapters::r#in::http::auth::AuthContext; use crate::adapters::r#in::http::error::{ApiError, ErrorBody}; -use crate::application::notify::{self, Delivery, Job}; -use crate::domain::notification::{Event, JobTrigger, NotificationType}; +use crate::application::webhooks::{self, Delivery, Job}; +use crate::domain::webhook::{Event, WebhookType}; -// Notifications: which events go where, and how the last send went. -// Admin-only, like connectors: a notification can name any application, and +// Webhooks: which events go where, and how the last send went. +// Admin-only, like connectors: a webhook can name any application, and // its destination says where this instance reaches out to. #[derive(Serialize, ToSchema)] -pub struct NotificationInfo { +pub struct WebhookSummary { /// The item's identity, as in the configuration pub name: String, pub description: Option, #[serde(rename = "type")] - pub notification_type: NotificationType, + pub webhook_type: WebhookType, pub events: Vec, /// Every application, present and future (`applications: all`) pub all_applications: bool, @@ -34,25 +34,20 @@ pub struct NotificationInfo { /// ascender and awx: the job template (or workflow job template) it starts pub job_template: Option, pub workflow_job_template: Option, - /// ascender and awx: through the template's webhook receiver or its launch API - pub trigger: Option, pub credentials: Vec, /// How long one attempt may take, and how many times a send is retried pub timeout_seconds: u64, pub retries: u32, - /// webhook: its HTTP method, how the credential is sent, the names of - /// its fixed headers (never their values), whether it sends a template or - /// the default message, and the header its signature goes in, if signed - pub webhook: Option, - /// ascender and awx, trigger launch: the names of the extra_vars it adds - /// beside the event (never their values) - pub extra_vars: Vec, + /// type webhook: its HTTP method, how the credential is sent, the names + /// of its fixed headers (never their values), whether it sends a template + /// or the default message, and the header its signature goes in, if signed + pub request: Option, /// The last send since this process started, if any pub last: Option, } #[derive(Serialize, ToSchema)] -pub struct WebhookInfo { +pub struct RequestInfo { pub method: String, /// bearer, basic, none, or header (the credential in `auth_header`) pub auth: String, @@ -65,23 +60,23 @@ pub struct WebhookInfo { #[utoipa::path( get, - path = "/api/v1/notifications", - tag = "Notifications", + path = "/api/v1/webhooks", + tag = "Webhooks", responses( - (status = 200, description = "Every notification: its events, applications and destination, and its last send", body = Vec), + (status = 200, description = "Every webhook: its events, applications and destination, and its last send", body = Vec), (status = 403, description = "API key is not admin", body = ErrorBody) ) )] -pub async fn list_notifications( +pub async fn list_webhooks( State(state): State>, axum::Extension(auth): axum::Extension, -) -> Result>, ApiError> { +) -> Result>, ApiError> { if !auth.permissions.is_admin() { return Err(ApiError::admin_only()); } let config = state.config(); - let mut out: Vec = config - .notifications + let mut out: Vec = config + .webhooks .iter() .map(|(id, n)| info(&state, id, n)) .collect(); @@ -89,26 +84,21 @@ pub async fn list_notifications( Ok(Json(out)) } -// One notification as the list shows it. -fn info( - state: &AppState, - id: &str, - n: &crate::domain::notification::Notification, -) -> NotificationInfo { +// One webhook as the list shows it. +fn info(state: &AppState, id: &str, n: &crate::domain::webhook::Webhook) -> WebhookSummary { let target = n.ascender.as_ref().or(n.awx.as_ref()); let mut applications = n.applications.named().to_vec(); applications.sort(); - NotificationInfo { + WebhookSummary { name: id.to_string(), description: n.description.clone(), - notification_type: n.notification_type, + webhook_type: n.webhook_type, events: n.events.clone(), all_applications: n.applications.is_all(), applications, destination: n.destination(), job_template: target.and_then(|t| t.job_template), workflow_job_template: target.and_then(|t| t.workflow_job_template), - trigger: target.map(|t| t.trigger), credentials: n.credentials().to_vec(), timeout_seconds: n .webhook @@ -122,8 +112,8 @@ fn info( .map(|w| w.retries) .or(target.map(|t| t.retries)) .unwrap_or_default(), - webhook: n.webhook.as_ref().map(|w| { - use crate::domain::notification::{Auth, AuthScheme}; + request: n.webhook.as_ref().map(|w| { + use crate::domain::webhook::{Auth, AuthScheme}; let (auth, auth_header) = match &w.auth { Auth::Header { header } => ("header", Some(header.clone())), Auth::Scheme(AuthScheme::Basic) => ("basic", None), @@ -133,7 +123,7 @@ fn info( Auth::Default if w.credentials.is_empty() => ("none", None), Auth::Default => ("bearer", None), }; - WebhookInfo { + RequestInfo { method: w.method.clone(), auth: auth.to_string(), auth_header, @@ -142,17 +132,14 @@ fn info( signature_header: w.sign.as_ref().map(|s| s.header.clone()), } }), - extra_vars: target - .map(|t| t.extra_vars.keys().cloned().collect()) - .unwrap_or_default(), - last: state.notifier.log.last(id), + last: state.dispatcher.log.last(id), } } #[derive(Serialize, ToSchema)] -pub struct NotificationDetail { +pub struct WebhookDetail { #[serde(flatten)] - pub notification: NotificationInfo, + pub summary: WebhookSummary, /// Its sends since this process started, newest first, at most 50. /// Retries of one message count as one send pub history: Vec, @@ -160,47 +147,47 @@ pub struct NotificationDetail { #[utoipa::path( get, - path = "/api/v1/notifications/{id}", - tag = "Notifications", - params(("id" = String, Path, description = "Notification name")), + path = "/api/v1/webhooks/{id}", + tag = "Webhooks", + params(("id" = String, Path, description = "Webhook name")), responses( - (status = 200, description = "One notification, with its recent sends", body = NotificationDetail), + (status = 200, description = "One webhook, with its recent sends", body = WebhookDetail), (status = 403, description = "API key is not admin", body = ErrorBody), - (status = 404, description = "No such notification", body = ErrorBody) + (status = 404, description = "No such webhook", body = ErrorBody) ) )] -pub async fn get_notification( +pub async fn get_webhook( State(state): State>, axum::Extension(auth): axum::Extension, Path(id): Path, -) -> Result, ApiError> { +) -> Result, ApiError> { if !auth.permissions.is_admin() { return Err(ApiError::admin_only()); } let config = state.config(); - let Some(n) = config.notifications.get(&id) else { + let Some(n) = config.webhooks.get(&id) else { return Err(ApiError::not_found(format!( - "notification '{id}' is not configured" + "webhook '{id}' is not configured" ))); }; - Ok(Json(NotificationDetail { - notification: info(&state, &id, n), - history: state.notifier.log.history(&id), + Ok(Json(WebhookDetail { + summary: info(&state, &id, n), + history: state.dispatcher.log.history(&id), })) } #[utoipa::path( post, - path = "/api/v1/notifications/{id}/test", - tag = "Notifications", - params(("id" = String, Path, description = "Notification name")), + path = "/api/v1/webhooks/{id}/test", + tag = "Webhooks", + params(("id" = String, Path, description = "Webhook name")), responses( - (status = 200, description = "Sent once, now, without retrying: the outcome, also kept as the notification's last send", body = Delivery), + (status = 200, description = "Sent once, now, without retrying: the outcome, also kept as the webhook's last send", body = Delivery), (status = 403, description = "API key is not admin", body = ErrorBody), - (status = 404, description = "No such notification", body = ErrorBody) + (status = 404, description = "No such webhook", body = ErrorBody) ) )] -pub async fn test_notification( +pub async fn test_webhook( State(state): State>, axum::Extension(auth): axum::Extension, headers: HeaderMap, @@ -210,20 +197,20 @@ pub async fn test_notification( return Err(ApiError::admin_only()); } let config = state.config(); - let Some(notification) = config.notifications.get(&id) else { + let Some(webhook) = config.webhooks.get(&id) else { return Err(ApiError::not_found(format!( - "notification '{id}' is not configured" + "webhook '{id}' is not configured" ))); }; - let mut message = notify::test_message(&id, config.url.as_deref()); - message.insert("notification".into(), serde_json::json!(id)); - let delivery = notify::deliver( + let mut message = webhooks::test_message(&id, config.url.as_deref()); + message.insert("webhook".into(), serde_json::json!(id)); + let delivery = webhooks::deliver( &*state.secrets, &*state.http, - &state.notifier.log, + &state.dispatcher.log, Job { - notification_id: id.clone(), - notification: notification.clone(), + webhook_id: id.clone(), + webhook: webhook.clone(), message, }, false, @@ -232,7 +219,7 @@ pub async fn test_notification( crate::adapters::r#in::http::audit::record( &auth, &headers, - "notification_test", + "webhook_test", &id, if delivery.ok { "success" } else { "error" }, ); diff --git a/src/adapters/out/mod.rs b/src/adapters/out/mod.rs index e2e172a..7193545 100644 --- a/src/adapters/out/mod.rs +++ b/src/adapters/out/mod.rs @@ -4,7 +4,7 @@ pub mod config; pub mod connectors; pub mod directory; pub mod git; -pub mod notify; +pub mod webhooks; // Not a port implementation: the scrubbed-environment Command builder shared // by every adapter that spawns a local script. pub(crate) mod process_env; diff --git a/src/adapters/out/notify/http.rs b/src/adapters/out/webhooks/http.rs similarity index 96% rename from src/adapters/out/notify/http.rs rename to src/adapters/out/webhooks/http.rs index 2c580b8..70ea826 100644 --- a/src/adapters/out/notify/http.rs +++ b/src/adapters/out/webhooks/http.rs @@ -1,4 +1,4 @@ -use crate::ports::notifier::{HttpSenderPort, OutboundRequest, OutboundResponse, SendFuture}; +use crate::ports::http_sender::{HttpSenderPort, OutboundRequest, OutboundResponse, SendFuture}; // How much of an answer is kept: a receiver's error message or a job id, not // whatever page a misconfigured URL returns. diff --git a/src/adapters/out/notify/mod.rs b/src/adapters/out/webhooks/mod.rs similarity index 100% rename from src/adapters/out/notify/mod.rs rename to src/adapters/out/webhooks/mod.rs diff --git a/src/application/config.rs b/src/application/config.rs index 3840981..fdcfdf6 100644 --- a/src/application/config.rs +++ b/src/application/config.rs @@ -69,7 +69,7 @@ pub struct ReloadReport { pub connectors: SectionDelta, pub projects: SectionDelta, pub credentials: SectionDelta, - pub notifications: SectionDelta, + pub webhooks: SectionDelta, } impl ReloadReport { @@ -148,7 +148,7 @@ fn runtime_config(cfg: &AppConfig) -> RuntimeConfig { applications: cfg.applications.clone(), connectors: cfg.connectors.clone(), credentials: cfg.credentials.clone(), - notifications: cfg.notifications.clone(), + webhooks: cfg.webhooks.clone(), projects: cfg.projects.clone(), secrets: cfg.secrets_config.clone(), metrics_require_auth: cfg.server.metrics_require_auth, @@ -178,7 +178,7 @@ fn diff( connectors: SectionDelta::between(&previous.connectors, &next.connectors), projects: SectionDelta::between(&previous.projects, &next.projects), credentials: SectionDelta::between(&previous.credentials, &next.credentials), - notifications: SectionDelta::between(&previous.notifications, &next.notifications), + webhooks: SectionDelta::between(&previous.webhooks, &next.webhooks), ..ReloadReport::default() }; @@ -188,7 +188,7 @@ fn diff( ("connectors", &report.connectors), ("projects", &report.projects), ("credentials", &report.credentials), - ("notifications", &report.notifications), + ("webhooks", &report.webhooks), ] { if !delta.is_empty() { applied.push(name.to_string()); diff --git a/src/application/mod.rs b/src/application/mod.rs index f85b330..897c1ae 100644 --- a/src/application/mod.rs +++ b/src/application/mod.rs @@ -6,8 +6,8 @@ pub mod config; pub mod credentials; pub mod login; -pub mod notify; pub mod projects; pub mod reconcile; pub mod scripts; pub mod search; +pub mod webhooks; diff --git a/src/application/reconcile.rs b/src/application/reconcile.rs index 0177bf9..7857deb 100644 --- a/src/application/reconcile.rs +++ b/src/application/reconcile.rs @@ -28,8 +28,8 @@ pub struct Catalog<'a> { pub applications: &'a HashMap, pub connectors: &'a HashMap, pub projects: &'a HashMap, - pub notifications: &'a HashMap, - // server.url, for links in notifications. + pub webhooks: &'a HashMap, + // server.url, for links in webhooks. pub url: Option<&'a str>, } @@ -48,8 +48,8 @@ pub struct Reconciler<'a> { pub project_health: &'a SyncHealthRegistry, // What each connector last described, for search. pub described: &'a crate::domain::connector::DescribedKinds, - // Queues what a run has to tell (application::notify). - pub notifier: &'a crate::application::notify::Dispatcher, + // Queues what a run has to tell (application::webhooks). + pub dispatcher: &'a crate::application::webhooks::Dispatcher, } // Pull the application's project, then reconcile. What every run should do — @@ -392,11 +392,11 @@ async fn reconcile_tree( // Where it stood before this run, read before the run is recorded over it. let before = r.states.baseline(app_id); r.states.record(app_id, status, snapshot, record.clone()); - if let Some(detected) = crate::application::notify::detect(&before, &record, status) { - crate::application::notify::after_run( - r.notifier, - catalog.notifications, - &crate::application::notify::RunOutcome { + if let Some(detected) = crate::application::webhooks::detect(&before, &record, status) { + crate::application::webhooks::after_run( + r.dispatcher, + catalog.webhooks, + &crate::application::webhooks::RunOutcome { application: app_id, description: app.description.as_deref(), base_url: catalog.url, diff --git a/src/application/notify.rs b/src/application/webhooks.rs similarity index 78% rename from src/application/notify.rs rename to src/application/webhooks.rs index 38057da..5094931 100644 --- a/src/application/notify.rs +++ b/src/application/webhooks.rs @@ -9,12 +9,11 @@ use serde_json::{Map, Value, json}; use tokio::sync::{Semaphore, mpsc}; use utoipa::ToSchema; -use crate::domain::notification::{ - Auth, AuthScheme, Event, JobTemplateTarget, JobTrigger, Notification, NotificationType, - WebhookTarget, -}; use crate::domain::run::{Baseline, RunMode, RunRecord, RunResult, SyncStatus, Trigger, now}; -use crate::ports::notifier::{HttpSenderPort, OutboundRequest, OutboundResponse}; +use crate::domain::webhook::{ + Auth, AuthScheme, Event, HttpTarget, JobTemplateTarget, Webhook, WebhookType, +}; +use crate::ports::http_sender::{HttpSenderPort, OutboundRequest, OutboundResponse}; use crate::ports::secrets::SecretsPort; // The use case "tell someone what happened to an application". @@ -22,7 +21,7 @@ use crate::ports::secrets::SecretsPort; // After every run, `detect` compares where the application stood before it // (its Baseline) with what the run found and did, and names at most one // event: one message per run, never "drift" and "synced" for the same -// change. Notifications that want that event are queued; a worker sends +// change. Webhooks that want that event are queued; a worker sends // them, so a slow receiver never holds up a run. // Why a sync changed something. @@ -199,13 +198,13 @@ pub fn message(o: &RunOutcome<'_>) -> Map { } // The message a "Send test" sends: the real shape, marked as a test. -pub fn test_message(notification_id: &str, base_url: Option<&str>) -> Map { +pub fn test_message(webhook_id: &str, base_url: Option<&str>) -> Map { let mut m = Map::new(); m.insert("event".into(), json!("test")); m.insert("application".into(), Value::Null); m.insert( "description".into(), - json!(format!("Test of notification {notification_id}")), + json!(format!("Test of webhook {webhook_id}")), ); for key in [ "status", @@ -272,27 +271,27 @@ fn placeholder_value(name: &str, message: &Map) -> String { } } -// The request one notification sends for one message. Pure: the credentials +// The request one webhook sends for one message. Pure: the credentials // arrive resolved, and nothing here touches the network. pub fn build_request( - notification: &Notification, + webhook: &Webhook, message: &Map, credentials: &HashMap, ) -> Result { - match notification.notification_type { - NotificationType::Webhook => { - let target = notification + match webhook.webhook_type { + WebhookType::Webhook => { + let target = webhook .webhook .as_ref() .ok_or("type webhook needs a 'webhook:' block")?; webhook_request(target, message, credentials) } - NotificationType::Ascender | NotificationType::Awx => { - let target = notification + WebhookType::Ascender | WebhookType::Awx => { + let target = webhook .ascender .as_ref() - .or(notification.awx.as_ref()) - .ok_or("a job template notification needs its block")?; + .or(webhook.awx.as_ref()) + .ok_or("a job template webhook needs its block")?; job_template_request(target, message, credentials) } } @@ -318,7 +317,7 @@ fn field<'a>(credentials: &'a HashMap, name: &str) -> Result<&'a } fn webhook_request( - target: &WebhookTarget, + target: &HttpTarget, message: &Map, credentials: &HashMap, ) -> Result { @@ -378,10 +377,9 @@ fn webhook_request( }) } -// Ascender and AWX speak the same routes. `webhook` posts the message to the -// template's own receiver in GitLab mode (its key, compared as is, and any -// event name; the playbook reads the message as _webhook_payload). -// `launch` starts it as a user, the message under extra_vars.unified_gitops. +// Ascender and AWX speak the same routes: the message goes to the template's +// own receiver in GitLab mode (its key, compared as is, and any event name; +// the playbook reads the message as _webhook_payload). fn job_template_request( target: &JobTemplateTarget, message: &Map, @@ -390,67 +388,26 @@ fn job_template_request( let token = field(credentials, "token")?; let event = message.get("event").and_then(Value::as_str).unwrap_or(""); let mut headers = base_headers(event); - let body = match target.trigger { - JobTrigger::Webhook => { - headers.push(("X-Gitlab-Token".into(), token.to_string())); - headers.push(("X-Gitlab-Event".into(), format!("unified-gitops {event}"))); - serde_json::to_vec(message).map_err(|e| e.to_string())? - } - JobTrigger::Launch => { - headers.push(("Authorization".into(), format!("Bearer {token}"))); - let mut extra_vars = target.extra_vars.clone(); - extra_vars.insert("unified_gitops".into(), Value::Object(message.clone())); - serde_json::to_vec(&json!({ "extra_vars": extra_vars })).map_err(|e| e.to_string())? - } - }; + headers.push(("X-Gitlab-Token".into(), token.to_string())); + headers.push(("X-Gitlab-Event".into(), format!("unified-gitops {event}"))); Ok(OutboundRequest { method: "POST".into(), url: target.endpoint(), headers, - body, + body: serde_json::to_vec(message).map_err(|e| e.to_string())?, timeout: Duration::from_secs(target.timeout_seconds), }) } -// What an answer means for this receiver: Ok with the job it started, if it -// says; Err with why it did not do what was asked. -pub fn interpret( - notification: &Notification, - response: &OutboundResponse, -) -> Result, String> { - if !(200..300).contains(&response.status) { - let excerpt: String = response.body.chars().take(300).collect(); - return Err(format!("HTTP {}: {}", response.status, excerpt.trim())); - } - let target = notification.ascender.as_ref().or(notification.awx.as_ref()); - let Some(target) = target else { - return Ok(None); - }; - let answer: Value = serde_json::from_str(&response.body).unwrap_or(Value::Null); - match target.trigger { - JobTrigger::Launch => { - // Without "Prompt on launch" for variables (or a survey), the - // controller drops extra_vars and says so only here: the job - // would run without the event. - if answer - .get("ignored_fields") - .and_then(|f| f.get("extra_vars")) - .is_some() - { - return Err( - "the job template ignored extra_vars: tick \"Prompt on launch\" \ - for its variables" - .to_string(), - ); - } - Ok(answer - .get("job") - .or_else(|| answer.get("workflow_job")) - .or_else(|| answer.get("id")) - .map(|id| id.to_string())) - } - JobTrigger::Webhook => Ok(None), +// What an answer means: any 2xx is taken (a controller answers 202, "Job +// queued.", or the same for a message it already had); anything else is why +// it was not, with the start of what the receiver said. +pub fn interpret(response: &OutboundResponse) -> Result<(), String> { + if (200..300).contains(&response.status) { + return Ok(()); } + let excerpt: String = response.body.chars().take(300).collect(); + Err(format!("HTTP {}: {}", response.status, excerpt.trim())) } // Worth sending again: no answer at all, or a receiver that is overloaded or @@ -493,7 +450,7 @@ fn hex(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{b:02x}")).collect() } -// One send of a notification, for the API and the UI. +// One send of a webhook, for the API and the UI. #[derive(Debug, Clone, Serialize, ToSchema)] pub struct Delivery { pub event: String, @@ -504,47 +461,43 @@ pub struct Delivery { pub attempts: u32, // The receiver's HTTP status, when it answered. pub status: Option, - // The job the controller started, when it says (launch). - pub job: Option, pub error: Option, } -// How many sends each notification keeps. In memory, like run history: a +// How many sends each webhook keeps. In memory, like run history: a // restart starts it over. pub const DELIVERY_HISTORY: usize = 50; -// Each notification's recent sends, newest first. +// Each webhook's recent sends, newest first. #[derive(Default)] pub struct DeliveryLog { sends: DashMap>, } impl DeliveryLog { - pub fn last(&self, notification_id: &str) -> Option { - self.sends - .get(notification_id) - .and_then(|h| h.front().cloned()) + pub fn last(&self, webhook_id: &str) -> Option { + self.sends.get(webhook_id).and_then(|h| h.front().cloned()) } - pub fn history(&self, notification_id: &str) -> Vec { + pub fn history(&self, webhook_id: &str) -> Vec { self.sends - .get(notification_id) + .get(webhook_id) .map(|h| h.iter().cloned().collect()) .unwrap_or_default() } - fn record(&self, notification_id: &str, delivery: Delivery) { - let mut history = self.sends.entry(notification_id.to_string()).or_default(); + fn record(&self, webhook_id: &str, delivery: Delivery) { + let mut history = self.sends.entry(webhook_id.to_string()).or_default(); history.push_front(delivery); history.truncate(DELIVERY_HISTORY); } } -// One message for one notification, its definition as of the run that made +// One message for one webhook, its definition as of the run that made // it: a reload while it waits does not change where it goes. pub struct Job { - pub notification_id: String, - pub notification: Notification, + pub webhook_id: String, + pub webhook: Webhook, pub message: Map, } @@ -592,8 +545,8 @@ impl Dispatcher { .and_then(Value::as_str) .unwrap_or("") .to_string(); - tracing::warn!(notification = %job.notification_id, event = %event, "notification queue full; message dropped"); - count(&job.notification_id, &event, "dropped"); + tracing::warn!(webhook = %job.webhook_id, event = %event, "webhook queue full; message dropped"); + count(&job.webhook_id, &event, "dropped"); } } @@ -651,31 +604,25 @@ pub async fn deliver( ok: false, attempts: 0, status: None, - job: None, error: None, }; - let request = match resolve(secrets, job.notification.credentials()).await { - Ok(credentials) => build_request(&job.notification, &job.message, &credentials), + let request = match resolve(secrets, job.webhook.credentials()).await { + Ok(credentials) => build_request(&job.webhook, &job.message, &credentials), Err(e) => Err(e), }; match request { Err(e) => delivery.error = Some(e), Ok(request) => { - let retries = if retry { - retries_of(&job.notification) - } else { - 0 - }; + let retries = if retry { retries_of(&job.webhook) } else { 0 }; loop { delivery.attempts += 1; let result = http.send(request.clone()).await; delivery.status = result.as_ref().ok().map(|r| r.status); let again = retryable(&result); - match result.and_then(|response| interpret(&job.notification, &response)) { - Ok(started_job) => { + match result.and_then(|response| interpret(&response)) { + Ok(()) => { delivery.ok = true; - delivery.job = started_job; delivery.error = None; break; } @@ -691,34 +638,34 @@ pub async fn deliver( } let outcome = if delivery.ok { "success" } else { "error" }; - count(&job.notification_id, &event, outcome); + count(&job.webhook_id, &event, outcome); metrics::histogram!( - "unified_gitops_notification_duration_seconds", - "notification" => job.notification_id.clone() + "unified_gitops_webhook_duration_seconds", + "webhook" => job.webhook_id.clone() ) .record(started.elapsed().as_secs_f64()); if delivery.ok { - tracing::info!(notification = %job.notification_id, event = %event, attempts = delivery.attempts, "notification sent"); + tracing::info!(webhook = %job.webhook_id, event = %event, attempts = delivery.attempts, "webhook sent"); } else { tracing::warn!( - notification = %job.notification_id, + webhook = %job.webhook_id, event = %event, attempts = delivery.attempts, error = delivery.error.as_deref().unwrap_or(""), - "notification failed" + "webhook failed" ); } - log.record(&job.notification_id, delivery.clone()); + log.record(&job.webhook_id, delivery.clone()); delivery } -fn retries_of(notification: &Notification) -> u32 { - notification +fn retries_of(webhook: &Webhook) -> u32 { + webhook .webhook .as_ref() .map(|w| w.retries) - .or_else(|| notification.ascender.as_ref().map(|t| t.retries)) - .or_else(|| notification.awx.as_ref().map(|t| t.retries)) + .or_else(|| webhook.ascender.as_ref().map(|t| t.retries)) + .or_else(|| webhook.awx.as_ref().map(|t| t.retries)) .unwrap_or(0) } @@ -731,28 +678,28 @@ async fn resolve( .map_err(|e| e.message) } -fn count(notification_id: &str, event: &str, outcome: &'static str) { +fn count(webhook_id: &str, event: &str, outcome: &'static str) { metrics::counter!( - "unified_gitops_notifications_total", - "notification" => notification_id.to_string(), + "unified_gitops_webhooks_total", + "webhook" => webhook_id.to_string(), "event" => event.to_string(), "outcome" => outcome ) .increment(1); } -// After a run: names its event, if any, and queues it for every notification +// After a run: names its event, if any, and queues it for every webhook // that wants it from this application. pub fn after_run( dispatcher: &Dispatcher, - notifications: &HashMap, + webhooks: &HashMap, outcome: &RunOutcome<'_>, ) { - if notifications.is_empty() { + if webhooks.is_empty() { return; } let event = outcome.detected.event; - let mut wanted: Vec<(&String, &Notification)> = notifications + let mut wanted: Vec<(&String, &Webhook)> = webhooks .iter() .filter(|(_, n)| n.wants(event, outcome.application)) .collect(); @@ -761,12 +708,12 @@ pub fn after_run( } wanted.sort_by(|a, b| a.0.cmp(b.0)); let base = message(outcome); - for (id, notification) in wanted { + for (id, webhook) in wanted { let mut m = base.clone(); - m.insert("notification".into(), json!(id)); + m.insert("webhook".into(), json!(id)); dispatcher.enqueue(Job { - notification_id: id.clone(), - notification: notification.clone(), + webhook_id: id.clone(), + webhook: webhook.clone(), message: m, }); } @@ -1029,7 +976,7 @@ mod tests { assert_eq!(parsed["x"], json!("")); } - fn notification(yaml: &str) -> Notification { + fn webhook(yaml: &str) -> Webhook { serde_saphyr::from_str(yaml).unwrap() } @@ -1045,7 +992,7 @@ mod tests { fn a_webhook_puts_the_credential_where_auth_says() { let creds = HashMap::from([("token".to_string(), "t0k".to_string())]); let m = outcome_message(); - let bearer = notification( + let bearer = webhook( "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com/h\n credentials: [c]\n", ); let request = build_request(&bearer, &m, &creds).unwrap(); @@ -1054,7 +1001,7 @@ mod tests { let parsed: Value = serde_json::from_slice(&request.body).unwrap(); assert_eq!(parsed["application"], json!("portainer")); - let named = notification( + let named = webhook( "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com/h\n credentials: [c]\n auth: {header: X-Gitlab-Token}\n headers: {X-Gitlab-Event: unified-gitops}\n", ); let request = build_request(&named, &m, &creds).unwrap(); @@ -1062,7 +1009,7 @@ mod tests { assert_eq!(header(&request, "Authorization"), None); assert_eq!(header(&request, "X-Gitlab-Event"), Some("unified-gitops")); - let basic = notification( + let basic = webhook( "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com/h\n credentials: [c]\n auth: basic\n", ); let err = build_request(&basic, &m, &creds).unwrap_err(); @@ -1072,7 +1019,7 @@ mod tests { #[test] fn a_signed_webhook_carries_the_hmac_of_its_body() { let creds = HashMap::from([("secret".to_string(), "Jefe".to_string())]); - let signed = notification( + let signed = webhook( "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com/h\n credentials: [c]\n auth: none\n body: 'what do ya want for nothing?'\n sign: {}\n", ); let request = build_request(&signed, &outcome_message(), &creds).unwrap(); @@ -1086,7 +1033,7 @@ mod tests { #[test] fn a_job_template_webhook_uses_the_gitlab_receiver_and_its_key() { let creds = HashMap::from([("token".to_string(), "jt-key".to_string())]); - let n = notification( + let n = webhook( "type: ascender\nevents: [drift_detected]\nascender:\n url: https://ascender.example.com\n job_template: 12\n credentials: [k]\n", ); let request = build_request(&n, &outcome_message(), &creds).unwrap(); @@ -1105,74 +1052,17 @@ mod tests { } #[test] - fn a_launch_sends_the_message_under_extra_vars() { - let creds = HashMap::from([("token".to_string(), "user-token".to_string())]); - let n = notification( - "type: awx\nevents: [drift_detected]\nawx:\n url: https://awx.example.com\n job_template: 5\n trigger: launch\n credentials: [t]\n extra_vars: {report: drift}\n", - ); - let request = build_request(&n, &outcome_message(), &creds).unwrap(); - assert_eq!( - request.url, - "https://awx.example.com/api/v2/job_templates/5/launch/" - ); - assert_eq!(header(&request, "Authorization"), Some("Bearer user-token")); - let parsed: Value = serde_json::from_slice(&request.body).unwrap(); - assert_eq!(parsed["extra_vars"]["report"], json!("drift")); - assert_eq!( - parsed["extra_vars"]["unified_gitops"]["application"], - json!("portainer") - ); - } - - #[test] - fn a_launch_whose_variables_were_ignored_is_a_failure() { - let n = notification( - "type: ascender\nevents: [drift_detected]\nascender:\n url: https://a.example.com\n job_template: 5\n trigger: launch\n credentials: [t]\n", - ); - let ignored = OutboundResponse { - status: 201, - body: r#"{"job": 77, "ignored_fields": {"extra_vars": {"unified_gitops": {}}}}"#.into(), + fn any_2xx_is_taken_and_anything_else_says_why() { + let queued = OutboundResponse { + status: 202, + body: r#"{"message": "Job queued."}"#.into(), }; - assert!( - interpret(&n, &ignored) - .unwrap_err() - .contains("Prompt on launch") - ); - let started = OutboundResponse { - status: 201, - body: r#"{"job": 77, "id": 77}"#.into(), - }; - assert_eq!(interpret(&n, &started).unwrap(), Some("77".to_string())); + assert!(interpret(&queued).is_ok()); let refused = OutboundResponse { status: 403, body: r#"{"detail": "You do not have permission"}"#.into(), }; - assert!(interpret(&n, &refused).unwrap_err().starts_with("HTTP 403")); - } - - #[test] - fn the_log_keeps_the_newest_sends_first_and_bounded() { - let log = DeliveryLog::default(); - for i in 0..(DELIVERY_HISTORY as u64 + 5) { - log.record( - "n", - Delivery { - event: "test".into(), - application: None, - at: i, - ok: true, - attempts: 1, - status: Some(202), - job: None, - error: None, - }, - ); - } - let history = log.history("n"); - assert_eq!(history.len(), DELIVERY_HISTORY); - assert_eq!(history[0].at, DELIVERY_HISTORY as u64 + 4); - assert_eq!(log.last("n").unwrap().at, history[0].at); - assert!(log.history("other").is_empty()); + assert!(interpret(&refused).unwrap_err().starts_with("HTTP 403")); } #[test] diff --git a/src/config.rs b/src/config.rs index edd8ba8..c630913 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,8 +7,8 @@ use crate::domain::api_key::{ApiKeyDef, ApiKeyRole}; use crate::domain::app::Application; use crate::domain::connector::Connector; use crate::domain::credential::Credential; -use crate::domain::notification::Notification; use crate::domain::project::GitProject; +use crate::domain::webhook::Webhook; pub struct AppConfig { pub server: ServerConfig, @@ -21,7 +21,7 @@ pub struct AppConfig { pub connectors: HashMap, pub applications: HashMap, pub api_keys: HashMap, - pub notifications: HashMap, + pub webhooks: HashMap, } // HTTP server configuration — config.yaml @@ -190,7 +190,7 @@ pub const SECTIONS: [&str; 6] = [ "projects", "credentials", "api_keys", - "notifications", + "webhooks", ]; // Whether a path, relative to the configuration directory, can be a @@ -538,21 +538,21 @@ impl AppConfig { } } - // Notifications: their own settings, then the applications they listen + // Webhooks: their own settings, then the applications they listen // to and the credentials they send with. - for (id, notification) in &self.notifications { - errors.extend(notification.errors(id)); - for app_id in notification.applications.named() { + for (id, webhook) in &self.webhooks { + errors.extend(webhook.errors(id)); + for app_id in webhook.applications.named() { if !self.applications.contains_key(app_id) { errors.push(format!( - "Notification '{id}' references unknown application '{app_id}'" + "Webhook '{id}' references unknown application '{app_id}'" )); } } - for cred in notification.credentials() { + for cred in webhook.credentials() { if !self.credentials.contains_key(cred) { errors.push(format!( - "Notification '{id}' references unknown credential '{cred}'" + "Webhook '{id}' references unknown credential '{cred}'" )); } } @@ -749,14 +749,14 @@ pub fn load_config_detailed(dir: &Path) -> Result { section("connectors"), section("applications"), section("api_keys"), - section("notifications"), + section("webhooks"), ); let credentials = take("credential", c, &mut errors); let projects = take("project", pr, &mut errors); let connectors = take("connector", pl, &mut errors); let applications = take("application", ap, &mut errors); let api_keys = take("API key", ak, &mut errors); - let notifications = take("notification", nt, &mut errors); + let webhooks = take("webhook", nt, &mut errors); if !errors.is_empty() { // Cross-references are only worth checking once every item parsed: // otherwise one typo'd connector also reports every application using it. @@ -774,7 +774,7 @@ pub fn load_config_detailed(dir: &Path) -> Result { connectors, applications, api_keys, - notifications, + webhooks, }; let errors = config.validate_errors(); @@ -954,40 +954,40 @@ mod tests { const CREDENTIAL: &str = "credentials:\n - name: cred-key\n type: token\n env_prefix: JT\n secret_keys:\n token: KEY\n"; #[test] - fn notifications_load_as_items() { + fn webhooks_load_as_items() { let dir = dir_with(&[ ("connectors/kv.yaml", CONNECTOR), ("projects.yaml", PROJECT), ("applications/portainer.yaml", APPLICATION), ("credentials/cred-key.yaml", CREDENTIAL), ( - "notifications/ntf-ascender-drift.yaml", - "notifications:\n - name: ntf-ascender-drift\n type: ascender\n events: [drift_detected]\n applications: [portainer]\n ascender:\n url: https://ascender.example.com\n job_template: 12\n credentials: [cred-key]\n", + "webhooks/ntf-ascender-drift.yaml", + "webhooks:\n - name: ntf-ascender-drift\n type: ascender\n events: [drift_detected]\n applications: [portainer]\n ascender:\n url: https://ascender.example.com\n job_template: 12\n credentials: [cred-key]\n", ), ]); let cfg = load_config(dir.path().to_str().unwrap()).expect("loads"); - let n = &cfg.notifications["ntf-ascender-drift"]; + let n = &cfg.webhooks["ntf-ascender-drift"]; assert_eq!(n.applications.named(), ["portainer".to_string()]); } #[test] - fn a_notification_must_name_known_applications_and_credentials() { + fn a_webhook_must_name_known_applications_and_credentials() { let dir = dir_with(&[ ("connectors/kv.yaml", CONNECTOR), ("projects.yaml", PROJECT), ("applications/portainer.yaml", APPLICATION), ( - "notifications/n.yaml", - "notifications:\n - name: n\n type: webhook\n events: [sync_failed]\n applications: [portainr]\n webhook:\n url: https://hooks.example.com/x\n credentials: [cred-nope]\n", + "webhooks/n.yaml", + "webhooks:\n - name: n\n type: webhook\n events: [sync_failed]\n applications: [portainr]\n webhook:\n url: https://hooks.example.com/x\n credentials: [cred-nope]\n", ), ]); let err = load_err(&dir); assert!( - err.contains("Notification 'n' references unknown application 'portainr'"), + err.contains("Webhook 'n' references unknown application 'portainr'"), "{err}" ); assert!( - err.contains("Notification 'n' references unknown credential 'cred-nope'"), + err.contains("Webhook 'n' references unknown credential 'cred-nope'"), "{err}" ); } diff --git a/src/domain/mod.rs b/src/domain/mod.rs index ddcb157..435b582 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -4,11 +4,11 @@ pub mod connector; pub mod credential; pub mod desired; pub mod ldap; -pub mod notification; pub mod plan; pub mod project; pub mod run; pub mod sync_health; +pub mod webhook; pub mod yaml; // Default timeout for git pulls and other bounded work. diff --git a/src/domain/run.rs b/src/domain/run.rs index 6800f8f..e6ca236 100644 --- a/src/domain/run.rs +++ b/src/domain/run.rs @@ -183,7 +183,7 @@ struct Entry { status: Option, revision: Option, // The commit the application was last found in sync at. Differences found - // at that same commit were made outside git: drift (application::notify). + // at that same commit were made outside git: drift (application::webhooks). synced_revision: Option, plan: Option, history: VecDeque, @@ -249,7 +249,7 @@ impl AppStateRegistry { } } - // Where the application stood before a run: what notifications compare + // Where the application stood before a run: what webhooks compare // the run's outcome with. pub fn baseline(&self, app_id: &str) -> Baseline { let entries = self.entries.read().unwrap_or_else(|e| e.into_inner()); diff --git a/src/domain/notification.rs b/src/domain/webhook.rs similarity index 84% rename from src/domain/notification.rs rename to src/domain/webhook.rs index 5dfad52..7d61c70 100644 --- a/src/domain/notification.rs +++ b/src/domain/webhook.rs @@ -3,32 +3,32 @@ use std::collections::BTreeMap; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; -// A notification: which events, from which applications, and where they go. +// A webhook: which events, from which applications, and where they go. // The `type` names the receiver; its settings live in the block of the same // name (`webhook:`, `ascender:`, `awx:`), so a block that belongs to another // type is a load error rather than settings silently ignored, and adding a // type later is a new block, with nothing to rename. #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] -pub struct Notification { +pub struct Webhook { #[serde(default)] pub description: Option, #[serde(rename = "type")] - pub notification_type: NotificationType, + pub webhook_type: WebhookType, pub events: Vec, // `all`, or the applications by name. Left out or `[]`: none, as an API // key or an LDAP rule with no applications reaches none. #[serde(default)] pub applications: Applications, #[serde(default)] - pub webhook: Option, + pub webhook: Option, #[serde(default)] pub ascender: Option, #[serde(default)] pub awx: Option, } -// Which applications a notification listens to: the word `all`, or a list. +// Which applications a webhook listens to: the word `all`, or a list. #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(untagged)] pub enum Applications { @@ -72,10 +72,10 @@ impl Applications { #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, ToSchema)] #[serde(rename_all = "snake_case")] -pub enum NotificationType { +pub enum WebhookType { // Any HTTP receiver: its URL, method, headers, body and authentication. Webhook, - // A job template in Ascender, through its webhook receiver or its launch API. + // A job template in Ascender, through its own webhook receiver. Ascender, // The same in AWX. The routes are Ascender's today; they are separate // types so neither carries the other's name, and either can follow its @@ -83,18 +83,18 @@ pub enum NotificationType { Awx, } -impl NotificationType { +impl WebhookType { pub fn as_str(self) -> &'static str { match self { - NotificationType::Webhook => "webhook", - NotificationType::Ascender => "ascender", - NotificationType::Awx => "awx", + WebhookType::Webhook => "webhook", + WebhookType::Ascender => "ascender", + WebhookType::Awx => "awx", } } } -// What happened to an application, as a notification names it. One run -// produces at most one event (application::notify::detect). +// What happened to an application, as a webhook names it. One run +// produces at most one event (application::webhooks::detect). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize, Serialize, ToSchema)] #[serde(rename_all = "snake_case")] pub enum Event { @@ -125,7 +125,7 @@ impl Event { #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] -pub struct WebhookTarget { +pub struct HttpTarget { pub url: String, #[serde(default = "default_method")] pub method: String, @@ -189,7 +189,10 @@ pub struct Signing { pub secret_field: String, } -// A job template (or workflow job template) to start, in Ascender or AWX. +// A job template (or workflow job template) to start, in Ascender or AWX, +// through the template's own webhook receiver in GitLab mode: its key in +// X-Gitlab-Token, no user needed, and any event accepted. The playbook reads +// the message as _webhook_payload (ascender_, awx_ or tower_). #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] pub struct JobTemplateTarget { @@ -199,43 +202,15 @@ pub struct JobTemplateTarget { pub job_template: Option, #[serde(default)] pub workflow_job_template: Option, - #[serde(default)] - pub trigger: JobTrigger, - // `webhook`: one credential whose `token` is the template's webhook key. - // `launch`: one credential whose `token` is a user's OAuth token. + // One credential, whose `token` is the template's webhook key. #[serde(default)] pub credentials: Vec, - // `launch` only: variables sent beside the event, which arrives as - // `unified_gitops`. The template must prompt for variables on launch. - #[serde(default)] - pub extra_vars: serde_json::Map, #[serde(default = "default_timeout_seconds")] pub timeout_seconds: u64, #[serde(default = "default_retries")] pub retries: u32, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, ToSchema, Default)] -#[serde(rename_all = "snake_case")] -pub enum JobTrigger { - // The template's own webhook receiver, in GitLab mode: its key in - // X-Gitlab-Token, no user needed. The playbook reads the event as - // `_webhook_payload`. - #[default] - Webhook, - // POST .../launch/ as a user, with extra_vars. - Launch, -} - -impl JobTrigger { - pub fn as_str(self) -> &'static str { - match self { - JobTrigger::Webhook => "webhook", - JobTrigger::Launch => "launch", - } - } -} - fn default_method() -> String { "POST".to_string() } @@ -277,22 +252,22 @@ pub const TEMPLATE_FIELDS: [&str; 15] = [ "errors", "at", "url", - "notification", + "webhook", ]; const METHODS: [&str; 3] = ["POST", "PUT", "PATCH"]; // Longest a send may take: a receiver that never answers must not hold the -// sender (and every notification queued behind it) for long. +// sender (and every webhook queued behind it) for long. const MAX_TIMEOUT_SECONDS: u64 = 300; const MAX_RETRIES: u32 = 10; -impl Notification { +impl Webhook { // Everything wrong with this item on its own; references to applications // and credentials are checked by the loader, which knows them. pub fn errors(&self, id: &str) -> Vec { let mut errors = Vec::new(); - let at = |what: String| format!("Notification '{id}': {what}"); + let at = |what: String| format!("Webhook '{id}': {what}"); if self.events.is_empty() { errors.push(at("needs at least one event".to_string())); } @@ -301,7 +276,7 @@ impl Notification { ("ascender", self.ascender.is_some()), ("awx", self.awx.is_some()), ]; - let wanted = self.notification_type.as_str(); + let wanted = self.webhook_type.as_str(); for (block, present) in blocks { if block == wanted && !present { errors.push(at(format!("type {wanted} needs a '{wanted}:' block"))); @@ -321,7 +296,7 @@ impl Notification { errors } - // The credentials this notification names, whichever its type. + // The credentials this webhook names, whichever its type. pub fn credentials(&self) -> &[String] { if let Some(webhook) = &self.webhook { return &webhook.credentials; @@ -332,7 +307,7 @@ impl Notification { } } - // Whether this notification wants `event` from `application`. + // Whether this webhook wants `event` from `application`. pub fn wants(&self, event: Event, application: &str) -> bool { self.events.contains(&event) && self.applications.includes(application) } @@ -349,7 +324,7 @@ impl Notification { } } -impl WebhookTarget { +impl HttpTarget { fn errors(&self) -> Vec { let mut errors = Vec::new(); errors.extend(url_errors("webhook.url", &self.url)); @@ -418,18 +393,8 @@ impl JobTemplateTarget { _ => {} } if self.credentials.len() != 1 { - errors.push(format!( - "trigger {} needs exactly one credential, whose token is {}", - self.trigger.as_str(), - match self.trigger { - JobTrigger::Webhook => "the template's webhook key", - JobTrigger::Launch => "a user's token", - } - )); - } - if self.trigger == JobTrigger::Webhook && !self.extra_vars.is_empty() { errors.push( - "extra_vars only go with trigger launch: a webhook sends the event itself" + "needs exactly one credential, whose token is the template's webhook key" .to_string(), ); } @@ -441,19 +406,15 @@ impl JobTemplateTarget { errors } - // The route a send goes to. + // The template's webhook receiver, GitLab mode. pub fn endpoint(&self) -> String { let (collection, id) = match (self.job_template, self.workflow_job_template) { (Some(id), _) => ("job_templates", id), (None, Some(id)) => ("workflow_job_templates", id), (None, None) => ("job_templates", 0), }; - let action = match self.trigger { - JobTrigger::Webhook => "gitlab", - JobTrigger::Launch => "launch", - }; format!( - "{}/api/v2/{collection}/{id}/{action}/", + "{}/api/v2/{collection}/{id}/gitlab/", self.url.trim_end_matches('/') ) } @@ -529,7 +490,7 @@ pub fn origin(url: &str) -> String { mod tests { use super::*; - fn parse(yaml: &str) -> Notification { + fn parse(yaml: &str) -> Webhook { serde_saphyr::from_str(yaml).unwrap() } @@ -561,12 +522,12 @@ mod tests { #[test] fn an_unknown_event_or_key_fails_to_parse() { assert!( - serde_saphyr::from_str::( + serde_saphyr::from_str::( "type: webhook\nevents: [drifted]\nwebhook:\n url: https://x.example.com\n" ) .is_err() ); - assert!(serde_saphyr::from_str::( + assert!(serde_saphyr::from_str::( "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com\n token: x\n" ) .is_err()); @@ -614,31 +575,31 @@ mod tests { ); assert!(n.errors("n").is_empty(), "{:?}", n.errors("n")); let target = n.ascender.unwrap(); - assert_eq!(target.trigger, JobTrigger::Webhook); assert_eq!( target.endpoint(), "https://ascender.example.com/api/v2/job_templates/7/gitlab/" ); - let launch = parse( - "type: awx\nevents: [drift_detected]\nawx:\n url: https://awx.example.com/\n workflow_job_template: 9\n trigger: launch\n credentials: [t]\n", + let workflow = parse( + "type: awx\nevents: [drift_detected]\nawx:\n url: https://awx.example.com/\n workflow_job_template: 9\n credentials: [k]\n", ); assert_eq!( - launch.awx.unwrap().endpoint(), - "https://awx.example.com/api/v2/workflow_job_templates/9/launch/" + workflow.awx.unwrap().endpoint(), + "https://awx.example.com/api/v2/workflow_job_templates/9/gitlab/" ); + // The launch API is not a way in: trigger and extra_vars are unknown keys. + assert!(serde_saphyr::from_str::( + "type: ascender\nevents: [drift_detected]\nascender:\n url: https://a.example.com\n job_template: 7\n credentials: [k]\n trigger: launch\n" + ) + .is_err()); let wrong = parse( - "type: ascender\nevents: [drift_detected]\nascender:\n url: https://ascender.example.com/api/v2\n job_template: 7\n workflow_job_template: 8\n extra_vars: {a: 1}\n", + "type: ascender\nevents: [drift_detected]\nascender:\n url: https://ascender.example.com/api/v2\n job_template: 7\n workflow_job_template: 8\n", ); let errors = wrong.errors("n").join("\n"); assert!(errors.contains("without /api"), "{errors}"); assert!(errors.contains("not both"), "{errors}"); assert!(errors.contains("exactly one credential"), "{errors}"); - assert!( - errors.contains("extra_vars only go with trigger launch"), - "{errors}" - ); } #[test] @@ -662,7 +623,7 @@ mod tests { "type: webhook\nevents: [sync_failed]\napplications: []\nwebhook:\n url: https://x.example.com\n", ); assert!(!empty.wants(Event::SyncFailed, "anything")); - assert!(serde_saphyr::from_str::( + assert!(serde_saphyr::from_str::( "type: webhook\nevents: [sync_failed]\napplications: every\nwebhook:\n url: https://x.example.com\n" ) .is_err()); diff --git a/src/lib.rs b/src/lib.rs index 2ac1f83..78315a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,8 +35,8 @@ pub struct AppBuilder { connectors: HashMap, projects: HashMap, credentials: HashMap, - notifications: HashMap, - http: Arc, + webhooks: HashMap, + http: Arc, secrets_settings: config::SecretsConfig, projects_dir: std::path::PathBuf, ui: crate::config::UiConfig, @@ -66,8 +66,8 @@ impl AppBuilder { connectors: HashMap::new(), projects: HashMap::new(), credentials: HashMap::new(), - notifications: HashMap::new(), - http: Arc::new(adapters::out::notify::http::ReqwestSender::new()), + webhooks: HashMap::new(), + http: Arc::new(adapters::out::webhooks::http::ReqwestSender::new()), secrets_settings: config::SecretsConfig::default(), projects_dir: std::path::PathBuf::from("projects"), ui: crate::config::UiConfig::default(), @@ -150,16 +150,13 @@ impl AppBuilder { // The credential DEFINITIONS (not their values), and the settings the // resolver chain is built from. Both only matter to a reload: it rebuilds // the chain from the new ones and reports which credential changed. - pub fn notifications( - mut self, - notifications: HashMap, - ) -> Self { - self.notifications = notifications; + pub fn webhooks(mut self, webhooks: HashMap) -> Self { + self.webhooks = webhooks; self } - // What notifications are sent through: a recorder in tests. - pub fn http(mut self, http: Arc) -> Self { + // What webhooks are sent through: a recorder in tests. + pub fn http(mut self, http: Arc) -> Self { self.http = http; self } @@ -186,7 +183,7 @@ impl AppBuilder { self.connectors = cfg.connectors.clone(); self.projects = cfg.projects.clone(); self.credentials = cfg.credentials.clone(); - self.notifications = cfg.notifications.clone(); + self.webhooks = cfg.webhooks.clone(); self.secrets_settings = cfg.secrets_config.clone(); self.projects_dir = std::path::PathBuf::from(&cfg.projects_config.dir); self.state_dir = std::path::PathBuf::from(&cfg.state.dir); @@ -293,7 +290,7 @@ impl AppBuilder { connectors: self.connectors, projects: self.projects, credentials: self.credentials, - notifications: self.notifications, + webhooks: self.webhooks, secrets: self.secrets_settings, metrics_require_auth: self.metrics_require_auth, cors_allowed_origins: self.cors_allowed_origins, @@ -318,7 +315,7 @@ impl AppBuilder { described: Arc::new(domain::connector::DescribedKinds::new()), started: std::time::Instant::now(), login_throttle: Arc::new(crate::application::login::LoginThrottle::default()), - notifier: Arc::new(application::notify::Dispatcher::new()), + dispatcher: Arc::new(application::webhooks::Dispatcher::new()), http: self.http, }); let router = adapters::r#in::http::routes::create_router( diff --git a/src/main.rs b/src/main.rs index ecdc12e..eeb757f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,13 +16,13 @@ async fn main() { Ok(cfg) => { println!( "configuration OK ({}): {} applications, {} connectors, \ - {} credentials, {} projects, {} notifications", + {} credentials, {} projects, {} webhooks", config_dir, cfg.applications.len(), cfg.connectors.len(), cfg.credentials.len(), cfg.projects.len(), - cfg.notifications.len(), + cfg.webhooks.len(), ); return; } @@ -126,9 +126,9 @@ async fn main() { } let (app, state) = builder.build_with_state(); - // Sends what runs have to tell (notifications), from the first run on: + // Sends what runs have to tell (webhooks), from the first run on: // a failure of the very first check is worth knowing. - state.start_notifier(); + state.start_webhooks(); // One switch for every background task. Flipped after the HTTP server has // drained; each task returns at its next wait point (a running apply is diff --git a/src/ports/notifier.rs b/src/ports/http_sender.rs similarity index 78% rename from src/ports/notifier.rs rename to src/ports/http_sender.rs index 9bd4b24..6638de6 100644 --- a/src/ports/notifier.rs +++ b/src/ports/http_sender.rs @@ -2,8 +2,8 @@ use std::future::Future; use std::pin::Pin; use std::time::Duration; -// One HTTP request, built whole by application::notify: the adapter only -// carries it, so what a notification sends is decided (and tested) in one +// One HTTP request, built whole by application::webhooks: the adapter only +// carries it, so what a webhook sends is decided (and tested) in one // place, without a network. #[derive(Debug, Clone, PartialEq)] pub struct OutboundRequest { @@ -24,7 +24,7 @@ pub struct OutboundResponse { pub type SendFuture<'a> = Pin> + Send + 'a>>; -// Sends notifications. Err is a request that got no answer (refused, timed +// Sends webhooks. Err is a request that got no answer (refused, timed // out, TLS); any answer, error statuses included, is Ok, and the caller // decides what it means for its receiver. pub trait HttpSenderPort: Send + Sync { diff --git a/src/ports/mod.rs b/src/ports/mod.rs index 2338236..9b869ed 100644 --- a/src/ports/mod.rs +++ b/src/ports/mod.rs @@ -2,7 +2,7 @@ pub mod config_store; pub mod connector; pub mod directory; pub mod git; -pub mod notifier; +pub mod http_sender; pub mod secrets; pub mod state_store; pub mod venv; diff --git a/src/state.rs b/src/state.rs index cd1e24b..f4a9d78 100644 --- a/src/state.rs +++ b/src/state.rs @@ -5,10 +5,10 @@ use std::sync::{Arc, RwLock}; use crate::domain::app::Application; use crate::domain::connector::Connector; use crate::domain::credential::Credential; -use crate::domain::notification::Notification; use crate::domain::project::GitProject; use crate::domain::run::AppStateRegistry; use crate::domain::sync_health::SyncHealthRegistry; +use crate::domain::webhook::Webhook; use crate::ports; // The part of the configuration a running process can adopt without being @@ -29,7 +29,7 @@ pub struct RuntimeConfig { // changed. pub credentials: HashMap, // Read after every run, so they reload like applications. - pub notifications: HashMap, + pub webhooks: HashMap, // Carried for the same reason as `credentials`: the chain is rebuilt from // it on every reload. pub secrets: crate::config::SecretsConfig, @@ -57,7 +57,7 @@ impl RuntimeConfig { applications: &self.applications, connectors: &self.connectors, projects: &self.projects, - notifications: &self.notifications, + webhooks: &self.webhooks, url: self.url.as_deref(), } } @@ -176,10 +176,10 @@ pub struct AppState { // Recent failed LDAP logins and the cap on concurrent ones (see // application::login::LoginThrottle). pub login_throttle: Arc, - // Queues what runs have to tell, and sends it (application::notify). - pub notifier: Arc, + // Queues what runs have to tell, and sends it (application::webhooks). + pub dispatcher: Arc, // What sends it: an HTTP client in production, a recorder in tests. - pub http: Arc, + pub http: Arc, } impl AppState { @@ -233,10 +233,10 @@ impl AppState { *self.restart_pending.write().expect("restart pending lock") = keys; } - // Starts sending queued notifications. Once; main calls it beside the + // Starts sending queued webhooks. Once; main calls it beside the // scheduler, and so does a test that wants to see what was sent. - pub fn start_notifier(&self) { - self.notifier + pub fn start_webhooks(&self) { + self.dispatcher .start(Arc::clone(&self.secrets), Arc::clone(&self.http)); } @@ -254,7 +254,7 @@ impl AppState { venv: &*self.venv, project_health: &self.project_health, described: &self.described, - notifier: &self.notifier, + dispatcher: &self.dispatcher, } } } diff --git a/tests/notifications_test.rs b/tests/webhooks_test.rs similarity index 90% rename from tests/notifications_test.rs rename to tests/webhooks_test.rs index ce33f69..60a69d2 100644 --- a/tests/notifications_test.rs +++ b/tests/webhooks_test.rs @@ -8,13 +8,13 @@ use std::time::Duration; use tower::ServiceExt; use unified_gitops::adapters::out::secrets::mock::MockSecrets; -use unified_gitops::ports::notifier::{ +use unified_gitops::ports::http_sender::{ HttpSenderPort, OutboundRequest, OutboundResponse, SendFuture, }; -// Notifications through the real router and the example connector: a sync +// Webhooks through the real router and the example connector: a sync // that applies a commit sends sync_succeeded to the receiver, with the -// credential where the notification says; a failing check sends sync_failed; +// credential where the webhook says; a failing check sends sync_failed; // the API lists the last send and sends a test. #[derive(Default)] @@ -90,7 +90,7 @@ fn fixture() -> Fixture { }; let project: unified_gitops::domain::project::GitProject = serde_saphyr::from_str("git_url: https://example.com/state.git\n").unwrap(); - let hook: unified_gitops::domain::notification::Notification = serde_saphyr::from_str( + let hook: unified_gitops::domain::webhook::Webhook = serde_saphyr::from_str( "type: webhook\nevents: [sync_succeeded, sync_failed]\napplications: all\nwebhook:\n url: https://hooks.example.com/services/T0/secret-path\n credentials: [cred-hook]\n retries: 0\n", ) .unwrap(); @@ -112,7 +112,7 @@ fn fixture() -> Fixture { HashMap::from([("prj-state".to_string(), project)]), projects.path().to_path_buf(), ) - .notifications(HashMap::from([("hook".to_string(), hook)])) + .webhooks(HashMap::from([("hook".to_string(), hook)])) .secrets(Arc::new( MockSecrets::new().with("cred-hook", [("token", "hook-token")]), )) @@ -120,7 +120,7 @@ fn fixture() -> Fixture { .state_dir(state.path().to_path_buf()) .api_key(Some("admin-secret".into())) .build_with_state(); - app_state.start_notifier(); + app_state.start_webhooks(); Fixture { app, @@ -178,7 +178,7 @@ async fn a_sync_that_applies_a_change_sends_sync_succeeded() { assert_eq!(body["cause"], "commit"); assert_eq!(body["applied"], 1); assert_eq!(body["changes"][0]["kind"], "namespace"); - assert_eq!(body["notification"], "hook"); + assert_eq!(body["webhook"], "hook"); // A second sync finds nothing to do: nothing more is sent. let (_, again) = call(&f.app, "POST", "/api/v1/applications/app-demo/sync").await; @@ -187,15 +187,15 @@ async fn a_sync_that_applies_a_change_sends_sync_succeeded() { assert_eq!(f.recorder.sent.lock().unwrap().len(), 1); // The list shows the last send, and where it went without its path. - let (status, list) = call(&f.app, "GET", "/api/v1/notifications").await; + let (status, list) = call(&f.app, "GET", "/api/v1/webhooks").await; assert_eq!(status, StatusCode::OK, "{list}"); assert_eq!(list[0]["name"], "hook"); assert_eq!(list[0]["destination"], "https://hooks.example.com"); // How it sends, never a value: a bearer token (a credential, auth left // out), the default message, no signature. - assert_eq!(list[0]["webhook"]["auth"], "bearer"); - assert_eq!(list[0]["webhook"]["method"], "POST"); - assert_eq!(list[0]["webhook"]["templated"], false); + assert_eq!(list[0]["request"]["auth"], "bearer"); + assert_eq!(list[0]["request"]["method"], "POST"); + assert_eq!(list[0]["request"]["templated"], false); assert_eq!(list[0]["retries"], 0); assert_eq!(list[0]["timeout_seconds"], 30); assert!(!list.to_string().contains("hook-token")); @@ -224,7 +224,7 @@ async fn a_failing_check_sends_sync_failed_once() { #[tokio::test] async fn send_test_sends_now_and_answers_the_outcome() { let f = fixture(); - let (status, delivery) = call(&f.app, "POST", "/api/v1/notifications/hook/test").await; + let (status, delivery) = call(&f.app, "POST", "/api/v1/webhooks/hook/test").await; assert_eq!(status, StatusCode::OK, "{delivery}"); assert_eq!(delivery["ok"], true); assert_eq!(delivery["event"], "test"); @@ -233,16 +233,16 @@ async fn send_test_sends_now_and_answers_the_outcome() { assert_eq!(body["event"], "test"); // Kept in its history, newest first. - call(&f.app, "POST", "/api/v1/notifications/hook/test").await; - let (status, detail) = call(&f.app, "GET", "/api/v1/notifications/hook").await; + call(&f.app, "POST", "/api/v1/webhooks/hook/test").await; + let (status, detail) = call(&f.app, "GET", "/api/v1/webhooks/hook").await; assert_eq!(status, StatusCode::OK, "{detail}"); assert_eq!(detail["name"], "hook"); assert_eq!(detail["history"].as_array().unwrap().len(), 2); assert_eq!(detail["history"][0]["event"], "test"); - let (status, _) = call(&f.app, "GET", "/api/v1/notifications/nope").await; + let (status, _) = call(&f.app, "GET", "/api/v1/webhooks/nope").await; assert_eq!(status, StatusCode::NOT_FOUND); - let (status, _) = call(&f.app, "POST", "/api/v1/notifications/nope/test").await; + let (status, _) = call(&f.app, "POST", "/api/v1/webhooks/nope/test").await; assert_eq!(status, StatusCode::NOT_FOUND); } @@ -277,7 +277,7 @@ async fn the_http_sender_delivers_and_does_not_follow_redirects() { let addr = listener.local_addr().unwrap(); tokio::spawn(async move { axum::serve(listener, receiver).await.unwrap() }); - let sender = unified_gitops::adapters::out::notify::http::ReqwestSender::new(); + let sender = unified_gitops::adapters::out::webhooks::http::ReqwestSender::new(); let request = |path: &str| OutboundRequest { method: "POST".into(), url: format!("http://{addr}{path}"), From 5d46ce0f52fbb3dc6f4f428ef80d007fcc377be7 Mon Sep 17 00:00:00 2001 From: blaipr Date: Mon, 26 Oct 2026 19:11:21 +0100 Subject: [PATCH 4/5] fix: webhook bugs from the release sweep --- CHANGELOG.md | 2 +- deny.toml | 1 - docs/webhooks.md | 32 ++- src/adapters/in/http/webhooks.rs | 4 +- src/adapters/out/git/cli.rs | 18 ++ src/adapters/out/webhooks/http.rs | 17 +- src/application/reconcile.rs | 15 +- src/application/webhooks.rs | 361 +++++++++++++++++++++++------- src/domain/run.rs | 29 ++- src/domain/webhook.rs | 101 +++++++-- src/main.rs | 11 + src/ports/git.rs | 9 + src/ports/http_sender.rs | 14 +- tests/webhooks_test.rs | 7 +- 14 files changed, 498 insertions(+), 123 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed6b3e4..7e13216 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ project adheres to [Semantic Versioning](https://semver.org/). ### Added -- Webhooks: a `webhooks` item sends, for `all` applications or the ones it names, `drift_detected`, `out_of_sync`, `sync_succeeded`, `sync_failed` and `back_in_sync` to a webhook (any receiver, with bearer, basic or header auth, a body template and optional HMAC signing) or to an Ascender or AWX job template (through the template's own webhook receiver, so no user is needed), one message per run, queued and retried, never with a field's value; `GET /api/v1/webhooks` shows each one's last send, `GET /api/v1/webhooks/{id}` its last 50, and `POST /api/v1/webhooks/{id}/test` sends a test (see `docs/webhooks.md`). +- Webhooks: a `webhooks` item sends, for `all` applications or the ones it names, `drift_detected`, `out_of_sync`, `sync_succeeded`, `sync_failed` and `back_in_sync` to a webhook (any receiver, with bearer, basic or header auth, a body template and optional HMAC signing) or to an Ascender or AWX job template (through the template's own webhook receiver, so no user is needed), one message per run, drift told apart from a commit by the application's own folder in git, queued and retried without one receiver holding up the others, its changes listed by name and never with a field's value; `GET /api/v1/webhooks` shows each one's last send, `GET /api/v1/webhooks/{id}` its last 50, and `POST /api/v1/webhooks/{id}/test` sends a test (see `docs/webhooks.md`). ### Changed diff --git a/deny.toml b/deny.toml index 7fa1be8..0cc5a8c 100644 --- a/deny.toml +++ b/deny.toml @@ -24,7 +24,6 @@ allow = [ "ISC", "Zlib", "Unicode-3.0", - "MPL-2.0", "CDLA-Permissive-2.0", ] diff --git a/docs/webhooks.md b/docs/webhooks.md index a8f433e..2711a20 100644 --- a/docs/webhooks.md +++ b/docs/webhooks.md @@ -24,12 +24,16 @@ Each run produces at most one event, so one change never arrives twice under two | `sync_failed` | A check or sync could not finish, or some operations failed | On entering the state | | `back_in_sync` | In sync again after a failure or a difference | On entering the state | -- **Drift and commits are told apart by commit:** differences found at the commit the application - was last in sync at were made on the system; at a newer commit, they come from git. +- **Drift and commits are told apart by the application's own folder:** differences found while its + folder in git is unchanged since it was last in sync were made on the system; after its folder + changed, they come from git. Commits to other folders change nothing here. Without git to ask + (a checkout that is not a repository), the `cause` is left out rather than guessed. - **With `auto_sync`, a reverted drift is one `drift_detected`** with `reverted: true`, not a `drift_detected` plus a `sync_succeeded`. - **History is in memory:** after a restart, the first run fires only failures and applied changes, so a restart does not report every pending difference again. +- **While a rollback stands,** scheduled checks report only failures: they compare the system + with git's HEAD on purpose, and what they find is the rollback. The next manual sync ends it. ## Types @@ -69,11 +73,14 @@ webhooks: - **`body`** is a template: `{{field}}` takes any field of [the message](#the-message), plus `{{revision_short}}`. Text is written escaped for a JSON string (write the quotes yourself); lists, - numbers and booleans as JSON. A name the message does not have fails the configuration. + numbers and booleans as JSON. A field with no value is `""` inside quotes and `null` outside + them, so the body stays valid JSON. A name the message does not have fails the configuration. - **`sign`** adds `X-Hub-Signature-256: sha256=` over the body, from the credential field `secret`. `header`, `prefix` and `secret_field` change those. - **Never in the YAML:** a token, a password, or a URL that carries one. They belong in credentials, which come from env vars or Vault. +- **One header, one source:** a fixed header with the name the auth or the signature writes (say, + `Authorization` with a bearer credential) fails the configuration. The `ascender` and `awx` types start a job template through its webhook; see [Ascender and AWX](#ascender-and-awx). @@ -105,8 +112,11 @@ The default body, and the fields a `body` template can use: - **`changes`** names each object, its operation and the names of the fields that changed, never their values: a value can be a secret, and the message leaves this instance. -- **`cause`** is `commit`, `drift` or `rollback`; **`reverted`** is set on `drift_detected` only. -- **`errors`**: the first five lines of a failure, for `sync_failed`. +- **`cause`** is `commit`, `drift` or `rollback` (left out when it cannot be told); **`reverted`** is + set on `drift_detected` only. +- **`errors`**: the first five lines of a failure, for `sync_failed`, as the connector or the + system reported them. Unlike `changes`, that is free text: a connector that prints a value in its + error sends it on. - **`url`** links the application in the UI when `server.url` is set. ## Ascender and AWX @@ -138,11 +148,15 @@ webhooks: ## Sending, retries and what is kept -- **A queue, not the run:** a run queues its event and moves on; a worker sends, eight at a time, - so a slow receiver never holds up a check or a sync. A full queue (1000) drops the newest message, - counted in the metrics. +- **A queue, not the run:** a run queues its event and moves on; a worker sends, eight requests at a + time. A request holds its place only while it is being sent, never through the wait before a + retry, so a receiver that is down slows its own messages, not the others'. Past 1000 messages + waiting or being sent, the newest is dropped, counted in the metrics. - **Retries:** an unanswered send, a `429` or a `5xx` is retried `retries` times, waiting 2, 4, 8 … - up to 60 seconds. A `4xx` is not: a wrong key or template does not fix itself. + up to 60 seconds. A `4xx` is not, nor a request that cannot be built (a missing credential field, + a credential with a line break in it): those are the configuration's to fix. +- **Shutdown:** after the last runs finish, what they queued is still sent, within + `server.shutdown_grace_seconds`; whatever is left then is dropped and logged. - **Redirects are not followed:** the body and its credential go only where the configuration says. - **What is kept:** each webhook's last 50 sends (event, application, result, status, error), in memory: the last on `GET /api/v1/webhooks`, all of them on diff --git a/src/adapters/in/http/webhooks.rs b/src/adapters/in/http/webhooks.rs index 32b843c..a45f055 100644 --- a/src/adapters/in/http/webhooks.rs +++ b/src/adapters/in/http/webhooks.rs @@ -115,7 +115,7 @@ fn info(state: &AppState, id: &str, n: &crate::domain::webhook::Webhook) -> Webh request: n.webhook.as_ref().map(|w| { use crate::domain::webhook::{Auth, AuthScheme}; let (auth, auth_header) = match &w.auth { - Auth::Header { header } => ("header", Some(header.clone())), + Auth::Header(h) => ("header", Some(h.header.clone())), Auth::Scheme(AuthScheme::Basic) => ("basic", None), Auth::Scheme(AuthScheme::None) => ("none", None), Auth::Scheme(AuthScheme::Bearer) => ("bearer", None), @@ -213,7 +213,7 @@ pub async fn test_webhook( webhook: webhook.clone(), message, }, - false, + None, ) .await; crate::adapters::r#in::http::audit::record( diff --git a/src/adapters/out/git/cli.rs b/src/adapters/out/git/cli.rs index 04f7467..75d082a 100644 --- a/src/adapters/out/git/cli.rs +++ b/src/adapters/out/git/cli.rs @@ -123,6 +123,24 @@ impl GitPort for CliGit { }) } + fn tree(&self, dir: &Path, folder: &str) -> crate::ports::git::RevisionFuture<'_> { + let dir = dir.to_path_buf(); + // `HEAD:` alone names the root tree. + let spec = format!("HEAD:{}", if folder == "." { "" } else { folder }); + Box::pin(async move { + let mut cmd = Command::new("git"); + cmd.args(["-C", dir.to_str()?, "rev-parse", &spec]); + cmd.env("GIT_TERMINAL_PROMPT", "0"); + cmd.kill_on_drop(true); + let output = cmd.output().await.ok()?; + if !output.status.success() { + return None; + } + let id = String::from_utf8_lossy(&output.stdout).trim().to_string(); + (!id.is_empty()).then_some(id) + }) + } + // `git archive` streams the tree at that commit straight from the object // store; `tar` unpacks it. Nothing in the checkout moves, so a pull // running at the same time cannot be confused by it, nor it by the pull. diff --git a/src/adapters/out/webhooks/http.rs b/src/adapters/out/webhooks/http.rs index 70ea826..4b6a238 100644 --- a/src/adapters/out/webhooks/http.rs +++ b/src/adapters/out/webhooks/http.rs @@ -1,4 +1,6 @@ -use crate::ports::http_sender::{HttpSenderPort, OutboundRequest, OutboundResponse, SendFuture}; +use crate::ports::http_sender::{ + HttpSenderPort, OutboundRequest, OutboundResponse, SendError, SendFuture, +}; // How much of an answer is kept: a receiver's error message or a job id, not // whatever page a misconfigured URL returns. @@ -31,8 +33,11 @@ impl Default for ReqwestSender { impl HttpSenderPort for ReqwestSender { fn send(&self, request: OutboundRequest) -> SendFuture<'_> { Box::pin(async move { - let method = reqwest::Method::from_bytes(request.method.as_bytes()) - .map_err(|e| format!("method {}: {e}", request.method))?; + let method = + reqwest::Method::from_bytes(request.method.as_bytes()).map_err(|e| SendError { + message: format!("method {}: {e}", request.method), + retryable: false, + })?; let mut builder = self .client .request(method, &request.url) @@ -42,7 +47,11 @@ impl HttpSenderPort for ReqwestSender { builder = builder.header(name, value); } // The error's text, never the request: its headers carry the credential. - let response = builder.send().await.map_err(|e| without_url(&e))?; + let response = builder.send().await.map_err(|e| SendError { + message: without_url(&e), + // Not buildable (a malformed URL or header): the same next time. + retryable: !e.is_builder(), + })?; let status = response.status().as_u16(); let bytes = response.bytes().await.unwrap_or_default(); let kept = &bytes[..bytes.len().min(KEPT_BODY_BYTES)]; diff --git a/src/application/reconcile.rs b/src/application/reconcile.rs index 7857deb..eef649f 100644 --- a/src/application/reconcile.rs +++ b/src/application/reconcile.rs @@ -125,6 +125,7 @@ pub async fn reconcile( .read_owned() .await; let revision = r.git.revision(&checkout).await; + let tree = r.git.tree(&checkout, app.folder()).await; let record = reconcile_tree( r, catalog, @@ -132,7 +133,7 @@ pub async fn reconcile( app, &checkout, Some(reading), - revision, + (revision, tree), mode, trigger, ) @@ -189,7 +190,9 @@ pub async fn rollback( app, &tree, None, - Some(sha.clone()), + // An exported folder: no tree to compare drift with, and a rollback + // is never measured against anyway. + (Some(sha.clone()), None), RunMode::Apply, Trigger::Rollback, ) @@ -302,6 +305,7 @@ pub async fn preview_rollback( started_at: now(), finished_at: now(), revision: Some(sha.clone()), + tree: None, planned: 0, operations: Vec::new(), applied: Vec::new(), @@ -333,7 +337,8 @@ async fn reconcile_tree( app: &Application, checkout: &Path, reading: Option>, - revision: Option, + // The commit and the folder's tree read (GitPort::revision, ::tree). + (revision, tree): (Option, Option), mode: RunMode, trigger: Trigger, ) -> RunRecord { @@ -346,6 +351,7 @@ async fn reconcile_tree( started_at, finished_at: started_at, revision: revision.clone(), + tree, planned: 0, operations: Vec::new(), applied: Vec::new(), @@ -390,7 +396,8 @@ async fn reconcile_tree( _ => r.health.record_success(app_id), } // Where it stood before this run, read before the run is recorded over it. - let before = r.states.baseline(app_id); + let mut before = r.states.baseline(app_id); + before.rolled_back = r.store.rollback(app_id).is_some(); r.states.record(app_id, status, snapshot, record.clone()); if let Some(detected) = crate::application::webhooks::detect(&before, &record, status) { crate::application::webhooks::after_run( diff --git a/src/application/webhooks.rs b/src/application/webhooks.rs index 5094931..88e2a54 100644 --- a/src/application/webhooks.rs +++ b/src/application/webhooks.rs @@ -13,7 +13,7 @@ use crate::domain::run::{Baseline, RunMode, RunRecord, RunResult, SyncStatus, Tr use crate::domain::webhook::{ Auth, AuthScheme, Event, HttpTarget, JobTemplateTarget, Webhook, WebhookType, }; -use crate::ports::http_sender::{HttpSenderPort, OutboundRequest, OutboundResponse}; +use crate::ports::http_sender::{HttpSenderPort, OutboundRequest, OutboundResponse, SendError}; use crate::ports::secrets::SecretsPort; // The use case "tell someone what happened to an application". @@ -68,11 +68,20 @@ impl Detected { // state is entered, not on every check that finds it still there; events that // describe something done (sync_succeeded, a reverted drift) fire every time. // +// Drift and commits are told apart by the tree of the application's own +// folder (RunRecord::tree), not by the repository's commit: a commit to +// another application's folder changes nothing here. With no tree to compare +// (the checkout is not git, or git could not answer), the cause is unknown. +// // A baseline of Unknown (first run since the process started: history is in // memory) fires only failures and applied changes, so a restart does not -// report every pending difference again. +// report every pending difference again. While a rollback stands, scheduled +// checks report only failures: they compare the system with HEAD on purpose, +// and what they find is the rollback. pub fn detect(before: &Baseline, record: &RunRecord, status: SyncStatus) -> Option { - let at_synced_commit = record.revision.is_some() && record.revision == before.synced_revision; + let known = record.tree.is_some(); + let unchanged_in_git = known && record.tree == before.synced_tree; + let entered = matches!(before.status, SyncStatus::Synced | SyncStatus::Failed); if matches!(record.result, RunResult::Failed | RunResult::Partial) || status == SyncStatus::Failed @@ -81,26 +90,34 @@ pub fn detect(before: &Baseline, record: &RunRecord, status: SyncStatus) -> Opti .then(|| Detected::new(Event::SyncFailed, None, None)); } + if before.rolled_back && record.trigger == Trigger::Schedule { + return None; + } + let applied = record.applied.iter().filter(|a| a.ok).count(); if record.mode == RunMode::Apply && applied > 0 { let cause = if record.trigger == Trigger::Rollback { - Cause::Rollback - } else if at_synced_commit { - Cause::Drift + Some(Cause::Rollback) + } else if unchanged_in_git { + Some(Cause::Drift) + } else if known { + Some(Cause::Commit) } else { - Cause::Commit + None }; return Some(match cause { - Cause::Drift => Detected::new(Event::DriftDetected, Some(cause), Some(true)), - _ => Detected::new(Event::SyncSucceeded, Some(cause), None), + Some(Cause::Drift) => Detected::new(Event::DriftDetected, cause, Some(true)), + _ => Detected::new(Event::SyncSucceeded, cause, None), }); } match status { - SyncStatus::OutOfSync if at_synced_commit => (before.status == SyncStatus::Synced) - .then(|| Detected::new(Event::DriftDetected, Some(Cause::Drift), Some(false))), - SyncStatus::OutOfSync => matches!(before.status, SyncStatus::Synced | SyncStatus::Failed) - .then(|| Detected::new(Event::OutOfSync, Some(Cause::Commit), None)), + SyncStatus::OutOfSync if unchanged_in_git => { + entered.then(|| Detected::new(Event::DriftDetected, Some(Cause::Drift), Some(false))) + } + SyncStatus::OutOfSync => { + entered.then(|| Detected::new(Event::OutOfSync, known.then_some(Cause::Commit), None)) + } SyncStatus::Synced => matches!(before.status, SyncStatus::Failed | SyncStatus::OutOfSync) .then(|| Detected::new(Event::BackInSync, None, None)), _ => None, @@ -236,22 +253,26 @@ fn rfc3339(unix_seconds: u64) -> String { // a JSON string (the template writes the quotes); lists, numbers and // booleans as JSON; an absent value as nothing. pub fn render(template: &str, message: &Map) -> String { + use crate::domain::webhook::{TemplatePart, template_parts}; + let parts: Vec> = template_parts(template).collect(); let mut out = String::with_capacity(template.len()); - let mut rest = template; - while let Some(start) = rest.find("{{") { - let Some(len) = rest[start + 2..].find("}}") else { - break; - }; - out.push_str(&rest[..start]); - let name = rest[start + 2..start + 2 + len].trim(); - out.push_str(&placeholder_value(name, message)); - rest = &rest[start + 2 + len + 2..]; + for (i, part) in parts.iter().enumerate() { + match part { + TemplatePart::Text(text) => out.push_str(text), + TemplatePart::Field(name) => { + // Inside quotes, an absent value is an empty string; outside + // them, JSON's null, so `{"reverted": {{reverted}}}` stays + // valid when there is nothing to say. + let quoted = out.ends_with('"') + && matches!(parts.get(i + 1), Some(TemplatePart::Text(t)) if t.starts_with('"')); + out.push_str(&placeholder_value(name, message, quoted)); + } + } } - out.push_str(rest); out } -fn placeholder_value(name: &str, message: &Map) -> String { +fn placeholder_value(name: &str, message: &Map, quoted: bool) -> String { let value = if name == "revision_short" { message .get("revision") @@ -262,7 +283,8 @@ fn placeholder_value(name: &str, message: &Map) -> String { message.get(name).cloned().unwrap_or(Value::Null) }; match value { - Value::Null => String::new(), + Value::Null if quoted => String::new(), + Value::Null => "null".to_string(), Value::String(s) => { let quoted = Value::String(s).to_string(); quoted[1..quoted.len() - 1].to_string() @@ -316,6 +338,28 @@ fn field<'a>(credentials: &'a HashMap, name: &str) -> Result<&'a .ok_or_else(|| format!("the credential has no '{name}' field")) } +// Sets a header, replacing any of the same name (headers are not case +// sensitive): one header, one value. +fn set_header(headers: &mut Vec<(String, String)>, name: &str, value: String) { + headers.retain(|(n, _)| !n.eq_ignore_ascii_case(name)); + headers.push((name.to_string(), value)); +} + +// A header value HTTP can carry: no line breaks or other control characters, +// which a secret read from a file often ends with. Refused here, before the +// request: it is the configuration's to fix, and retrying would not. +fn check_headers(headers: &[(String, String)]) -> Result<(), String> { + for (name, value) in headers { + if value.chars().any(|c| c.is_control() && c != '\t') { + return Err(format!( + "the value for header '{name}' has a line break or a control character \ + (a credential read from a file with a trailing newline?)" + )); + } + } + Ok(()) +} + fn webhook_request( target: &HttpTarget, message: &Map, @@ -332,42 +376,52 @@ fn webhook_request( headers.push((name.clone(), value.clone())); } match &target.auth { - Auth::Scheme(AuthScheme::Bearer) => headers.push(( - "Authorization".into(), + Auth::Scheme(AuthScheme::Bearer) => set_header( + &mut headers, + "Authorization", format!("Bearer {}", field(credentials, "token")?), - )), + ), Auth::Default => { if let Some(token) = credentials.get("token").filter(|t| !t.is_empty()) { - headers.push(("Authorization".into(), format!("Bearer {token}"))); + set_header(&mut headers, "Authorization", format!("Bearer {token}")); } } Auth::Scheme(AuthScheme::Basic) => { - let pair = format!( - "{}:{}", - field(credentials, "username")?, - field(credentials, "password")? - ); - headers.push(( - "Authorization".into(), + // An empty password is allowed: some APIs take a key as the + // username and nothing after the colon. + let password = credentials + .get("password") + .map(String::as_str) + .unwrap_or(""); + let pair = format!("{}:{password}", field(credentials, "username")?); + set_header( + &mut headers, + "Authorization", format!( "Basic {}", base64::engine::general_purpose::STANDARD.encode(pair) ), - )); + ); } - Auth::Header { header } => { - headers.push((header.clone(), field(credentials, "token")?.to_string())); + Auth::Header(crate::domain::webhook::HeaderAuth { header }) => { + set_header( + &mut headers, + header, + field(credentials, "token")?.to_string(), + ); } Auth::Scheme(AuthScheme::None) => {} } if let Some(sign) = &target.sign { let secret = field(credentials, &sign.secret_field)?; let digest = hmac_sha256(secret.as_bytes(), &body); - headers.push(( - sign.header.clone(), + set_header( + &mut headers, + &sign.header, format!("{}{}", sign.prefix, hex(&digest)), - )); + ); } + check_headers(&headers)?; Ok(OutboundRequest { method: target.method.clone(), url: target.url.clone(), @@ -390,6 +444,7 @@ fn job_template_request( let mut headers = base_headers(event); headers.push(("X-Gitlab-Token".into(), token.to_string())); headers.push(("X-Gitlab-Event".into(), format!("unified-gitops {event}"))); + check_headers(&headers)?; Ok(OutboundRequest { method: "POST".into(), url: target.endpoint(), @@ -413,9 +468,9 @@ pub fn interpret(response: &OutboundResponse) -> Result<(), String> { // Worth sending again: no answer at all, or a receiver that is overloaded or // failing. A 4xx is this side's mistake (a wrong key, a wrong template) and // will not fix itself. -fn retryable(result: &Result) -> bool { +fn retryable(result: &Result) -> bool { match result { - Err(_) => true, + Err(e) => e.retryable, Ok(response) => response.status == 429 || response.status >= 500, } } @@ -502,9 +557,12 @@ pub struct Job { } // Bounded: a receiver that is down for hours must not grow memory without -// limit. A full queue drops the new message, counted and logged. +// limit. Messages waiting or being sent count against it; past it the new +// message is dropped, counted and logged. const QUEUE: usize = 1000; -// Sends in flight at once, so one slow receiver does not hold up the others. +// Requests in flight at once. A slot is held for one attempt only, never +// through the wait before a retry, so a receiver that is down slows its own +// messages, not everyone's. const CONCURRENT: usize = 8; // Longest wait between two attempts. const MAX_BACKOFF_SECONDS: u64 = 60; @@ -514,6 +572,8 @@ pub struct Dispatcher { // Taken by start(); None once the worker runs. rx: std::sync::Mutex>>, pub log: Arc, + // Messages queued or being sent: what QUEUE bounds and drain waits for. + pending: Arc, } impl Default for Dispatcher { @@ -529,25 +589,35 @@ impl Dispatcher { tx, rx: std::sync::Mutex::new(Some(rx)), log: Arc::new(DeliveryLog::default()), + pending: Arc::new(std::sync::atomic::AtomicUsize::new(0)), } } pub fn enqueue(&self, job: Job) { - if let Err(e) = self.tx.try_send(job) { - let job = match e { - mpsc::error::TrySendError::Full(job) | mpsc::error::TrySendError::Closed(job) => { + use std::sync::atomic::Ordering; + let room = self.pending.load(Ordering::SeqCst) < QUEUE; + let job = if room { + self.pending.fetch_add(1, Ordering::SeqCst); + match self.tx.try_send(job) { + Ok(()) => return, + Err( + mpsc::error::TrySendError::Full(job) | mpsc::error::TrySendError::Closed(job), + ) => { + self.pending.fetch_sub(1, Ordering::SeqCst); job } - }; - let event = job - .message - .get("event") - .and_then(Value::as_str) - .unwrap_or("") - .to_string(); - tracing::warn!(webhook = %job.webhook_id, event = %event, "webhook queue full; message dropped"); - count(&job.webhook_id, &event, "dropped"); - } + } + } else { + job + }; + let event = job + .message + .get("event") + .and_then(Value::as_str) + .unwrap_or("") + .to_string(); + tracing::warn!(webhook = %job.webhook_id, event = %event, "webhook queue full; message dropped"); + count(&job.webhook_id, &event, "dropped"); } // Starts the worker that sends what is queued. Once: later calls do @@ -558,32 +628,47 @@ impl Dispatcher { return; }; let log = Arc::clone(&self.log); + let pending = Arc::clone(&self.pending); tokio::spawn(async move { let slots = Arc::new(Semaphore::new(CONCURRENT)); while let Some(job) = rx.recv().await { - let Ok(slot) = Arc::clone(&slots).acquire_owned().await else { - return; - }; - let (secrets, http, log) = - (Arc::clone(&secrets), Arc::clone(&http), Arc::clone(&log)); + let (secrets, http, log, slots, pending) = ( + Arc::clone(&secrets), + Arc::clone(&http), + Arc::clone(&log), + Arc::clone(&slots), + Arc::clone(&pending), + ); tokio::spawn(async move { - let _slot = slot; - deliver(&*secrets, &*http, &log, job, true).await; + deliver(&*secrets, &*http, &log, job, Some(&slots)).await; + pending.fetch_sub(1, std::sync::atomic::Ordering::SeqCst); }); } }); } + + // Waits for what is queued or being sent, at most `grace`, and answers + // how many messages were still left. Shutdown calls it after the runs + // have finished, so the events of the last runs still go out. + pub async fn drain(&self, grace: Duration) -> usize { + use std::sync::atomic::Ordering; + let deadline = Instant::now() + grace; + while self.pending.load(Ordering::SeqCst) > 0 && Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(100)).await; + } + self.pending.load(Ordering::SeqCst) + } } -// Sends one message, with retries when `retry` is set, records the outcome -// and returns it. The worker's path, and the "Send test" button's (which -// answers at once, without retrying). +// Sends one message, records the outcome and returns it. With `slots` (the +// worker's path) it retries, holding a slot for each attempt only; without +// (the "Send test" button) it tries once and answers at once. pub async fn deliver( secrets: &dyn SecretsPort, http: &dyn HttpSenderPort, log: &DeliveryLog, job: Job, - retry: bool, + slots: Option<&Semaphore>, ) -> Delivery { let started = Instant::now(); let event = job @@ -607,6 +692,8 @@ pub async fn deliver( error: None, }; + // A request that cannot be built (a missing credential field, a value + // HTTP cannot carry) is the configuration's to fix: not retried. let request = match resolve(secrets, job.webhook.credentials()).await { Ok(credentials) => build_request(&job.webhook, &job.message, &credentials), Err(e) => Err(e), @@ -614,13 +701,25 @@ pub async fn deliver( match request { Err(e) => delivery.error = Some(e), Ok(request) => { - let retries = if retry { retries_of(&job.webhook) } else { 0 }; + let retries = if slots.is_some() { + retries_of(&job.webhook) + } else { + 0 + }; loop { delivery.attempts += 1; + let slot = match slots { + Some(slots) => slots.acquire().await.ok(), + None => None, + }; let result = http.send(request.clone()).await; + drop(slot); delivery.status = result.as_ref().ok().map(|r| r.status); let again = retryable(&result); - match result.and_then(|response| interpret(&response)) { + match result + .map_err(|e| e.message) + .and_then(|response| interpret(&response)) + { Ok(()) => { delivery.ok = true; delivery.error = None; @@ -725,14 +824,17 @@ mod tests { use crate::domain::plan::{Change, OpKind, Operation}; use crate::domain::run::AppliedOperation; + // `synced`: the tree of the application's folder when it was last in sync. fn baseline(status: SyncStatus, synced: Option<&str>) -> Baseline { Baseline { status, - synced_revision: synced.map(str::to_string), + synced_tree: synced.map(str::to_string), + rolled_back: false, } } - fn record(mode: RunMode, trigger: Trigger, result: RunResult, revision: &str) -> RunRecord { + // `tree`: the folder's tree this run read (its commit is made up). + fn record(mode: RunMode, trigger: Trigger, result: RunResult, tree: &str) -> RunRecord { RunRecord { id: 1, trigger, @@ -740,7 +842,8 @@ mod tests { result, started_at: 0, finished_at: 1_700_000_000, - revision: Some(revision.to_string()), + revision: Some(format!("commit-of-{tree}")), + tree: Some(tree.to_string()), planned: 0, operations: Vec::new(), applied: Vec::new(), @@ -910,6 +1013,88 @@ mod tests { ); } + #[test] + fn a_commit_to_another_folder_does_not_hide_drift() { + // In sync with folder tree t1; someone commits elsewhere in the + // repository (a new commit, the same tree here), then edits the + // system by hand. + let mut r = record(RunMode::Plan, Trigger::Schedule, RunResult::OutOfSync, "t1"); + r.revision = Some("another-commit".into()); + let d = detect( + &baseline(SyncStatus::Synced, Some("t1")), + &r, + SyncStatus::OutOfSync, + ); + assert_eq!( + event(d), + Some((Event::DriftDetected, Some(Cause::Drift), Some(false))) + ); + } + + #[test] + fn drift_after_a_failure_is_reported() { + let r = record(RunMode::Plan, Trigger::Schedule, RunResult::OutOfSync, "t1"); + let d = detect( + &baseline(SyncStatus::Failed, Some("t1")), + &r, + SyncStatus::OutOfSync, + ); + assert_eq!( + event(d), + Some((Event::DriftDetected, Some(Cause::Drift), Some(false))) + ); + } + + #[test] + fn a_standing_rollback_keeps_scheduled_checks_quiet_but_not_failures() { + let mut rolled = baseline(SyncStatus::Synced, Some("t1")); + rolled.rolled_back = true; + let pending = record(RunMode::Plan, Trigger::Schedule, RunResult::OutOfSync, "t2"); + assert_eq!(detect(&rolled, &pending, SyncStatus::OutOfSync), None); + let failed = record(RunMode::Plan, Trigger::Schedule, RunResult::Failed, "t2"); + assert!(detect(&rolled, &failed, SyncStatus::Failed).is_some()); + } + + #[test] + fn without_a_tree_the_cause_is_unknown() { + let mut r = record( + RunMode::Apply, + Trigger::Schedule, + RunResult::Succeeded, + "t1", + ); + r.tree = None; + r.applied.push(applied(true)); + let d = detect( + &baseline(SyncStatus::Synced, Some("t1")), + &r, + SyncStatus::Synced, + ); + assert_eq!(event(d), Some((Event::SyncSucceeded, None, None))); + } + + #[test] + fn an_absent_value_is_null_outside_quotes_and_empty_inside() { + let m = outcome_message(); + let body = render(r#"{"r": {{reverted}}, "s": "{{reverted}}"}"#, &m); + let parsed: Value = serde_json::from_str(&body).expect(&body); + assert_eq!(parsed["r"], Value::Null); + assert_eq!(parsed["s"], json!("")); + } + + #[test] + fn render_reads_placeholders_as_the_load_check_does() { + // The check sees one placeholder, `application`; so does render: + // the unclosed `{{a ` is text. + let template = "x {{a {{application}} y {{}} z"; + let names: Vec<&str> = crate::domain::webhook::placeholders(template).collect(); + assert_eq!(names, ["application"]); + assert_eq!( + render(template, &outcome_message()), + "x {{a portainer y {{}} z" + ); + } + fn outcome_message() -> Map { let mut r = record( RunMode::Apply, @@ -917,6 +1102,7 @@ mod tests { RunResult::Succeeded, "0123456789abcdef", ); + r.revision = Some("0123456789abcdef".into()); r.operations.push(Operation { kind: "user".into(), key: "jane".into(), @@ -1014,6 +1200,21 @@ mod tests { ); let err = build_request(&basic, &m, &creds).unwrap_err(); assert!(err.contains("'username'"), "{err}"); + + // A key as the username and no password: allowed. + let key_only = HashMap::from([("username".to_string(), "api-key".to_string())]); + let request = build_request(&basic, &m, &key_only).unwrap(); + assert_eq!( + header(&request, "Authorization"), + Some("Basic YXBpLWtleTo=") + ); + + // A token read from a file with its trailing newline: refused before + // anything is sent. + let newline = HashMap::from([("token".to_string(), "t0k\n".to_string())]); + let err = build_request(&bearer, &m, &newline).unwrap_err(); + assert!(err.contains("line break"), "{err}"); + assert!(!err.contains("t0k"), "{err}"); } #[test] @@ -1067,7 +1268,15 @@ mod tests { #[test] fn only_unanswered_or_server_failures_are_retried() { - assert!(retryable(&Err("timed out".into()))); + assert!(retryable(&Err(SendError { + message: "timed out".into(), + retryable: true, + }))); + // A request that could not be built fails the same way every time. + assert!(!retryable(&Err(SendError { + message: "builder error".into(), + retryable: false, + }))); assert!(retryable(&Ok(OutboundResponse { status: 503, body: String::new() diff --git a/src/domain/run.rs b/src/domain/run.rs index e6ca236..185561e 100644 --- a/src/domain/run.rs +++ b/src/domain/run.rs @@ -108,6 +108,10 @@ pub struct RunRecord { pub finished_at: u64, // The commit of the checkout the run read; None when it could not tell. pub revision: Option, + // The git tree of the application's folder at that commit (GitPort::tree): + // what webhooks compare to tell drift from a commit. Not part of the API. + #[serde(skip)] + pub tree: Option, // Operations the plan found. pub planned: usize, // Those operations themselves, each with its fields' old and new values, @@ -175,16 +179,20 @@ pub const RECORDED_ERRORS: usize = 50; #[derive(Debug, Clone, PartialEq)] pub struct Baseline { pub status: SyncStatus, - pub synced_revision: Option, + pub synced_tree: Option, + // Rolled back: checks compare the system with HEAD on purpose, and what + // they find is the rollback, not news. Set by the caller, from the store. + pub rolled_back: bool, } #[derive(Default)] struct Entry { status: Option, revision: Option, - // The commit the application was last found in sync at. Differences found - // at that same commit were made outside git: drift (application::webhooks). - synced_revision: Option, + // The tree of its folder when the application was last found in sync + // (RunRecord::tree). Differences found with that same tree were made + // outside git: drift (application::webhooks). + synced_tree: Option, plan: Option, history: VecDeque, } @@ -221,8 +229,10 @@ impl AppStateRegistry { if run.revision.is_some() { entry.revision = run.revision.clone(); } - if status == SyncStatus::Synced && run.revision.is_some() { - entry.synced_revision = run.revision.clone(); + // A rollback puts an older state on the system on purpose: it is not + // what git says, so it never becomes what drift is measured against. + if status == SyncStatus::Synced && run.trigger != Trigger::Rollback && run.tree.is_some() { + entry.synced_tree = run.tree.clone(); } if plan.is_some() { entry.plan = plan; @@ -256,11 +266,13 @@ impl AppStateRegistry { match entries.get(app_id) { Some(e) => Baseline { status: e.status.unwrap_or(SyncStatus::Unknown), - synced_revision: e.synced_revision.clone(), + synced_tree: e.synced_tree.clone(), + rolled_back: false, }, None => Baseline { status: SyncStatus::Unknown, - synced_revision: None, + synced_tree: None, + rolled_back: false, }, } } @@ -295,6 +307,7 @@ mod tests { started_at: 0, finished_at: 0, revision: None, + tree: None, planned: 10_000, operations: Vec::new(), applied: (0..10_000) diff --git a/src/domain/webhook.rs b/src/domain/webhook.rs index 7d61c70..6d2336b 100644 --- a/src/domain/webhook.rs +++ b/src/domain/webhook.rs @@ -155,9 +155,7 @@ pub struct HttpTarget { #[serde(untagged)] pub enum Auth { Scheme(AuthScheme), - Header { - header: String, - }, + Header(HeaderAuth), // `auth:` left out: a bearer token when there is a credential, nothing // when there is none. #[default] @@ -165,6 +163,15 @@ pub enum Auth { Default, } +// `auth: {header: Name}`: the credential's token in a header of that name. +// Strict like every other block: `{header: X, value: y}` is a typo, not a +// setting to ignore. +#[derive(Debug, Clone, PartialEq, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct HeaderAuth { + pub header: String, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] #[serde(rename_all = "snake_case")] pub enum AuthScheme { @@ -339,13 +346,13 @@ impl HttpTarget { errors.extend(template_errors(body)); } let needs_credential = match &self.auth { - Auth::Scheme(AuthScheme::Bearer | AuthScheme::Basic) | Auth::Header { .. } => true, + Auth::Scheme(AuthScheme::Bearer | AuthScheme::Basic) | Auth::Header(_) => true, Auth::Scheme(AuthScheme::None) | Auth::Default => false, }; if needs_credential && self.credentials.is_empty() { errors.push("webhook.auth needs a credential in webhook.credentials".to_string()); } - if let Auth::Header { header } = &self.auth + if let Auth::Header(HeaderAuth { header }) = &self.auth && !is_header_name(header) { errors.push(format!( @@ -368,6 +375,37 @@ impl HttpTarget { errors.push("webhook.sign needs a credential holding its secret".to_string()); } } + // One header, one source: a fixed header with the name the auth or + // the signature writes would be sent twice. + let mut written: Vec<(&str, &str)> = Vec::new(); + match &self.auth { + Auth::Scheme(AuthScheme::Bearer | AuthScheme::Basic) => { + written.push(("Authorization", "webhook.auth")) + } + Auth::Default if !self.credentials.is_empty() => { + written.push(("Authorization", "webhook.auth")) + } + Auth::Header(HeaderAuth { header }) => written.push((header, "webhook.auth")), + _ => {} + } + if let Some(sign) = &self.sign { + written.push((&sign.header, "webhook.sign")); + } + for (i, (name, by)) in written.iter().enumerate() { + if self.headers.keys().any(|h| h.eq_ignore_ascii_case(name)) { + errors.push(format!( + "webhook.headers sets '{name}', which {by} writes: set it in one place" + )); + } + if written[..i] + .iter() + .any(|(other, _)| other.eq_ignore_ascii_case(name)) + { + errors.push(format!( + "webhook.auth and webhook.sign both write '{name}': give them different headers" + )); + } + } errors.extend(bounds_errors("webhook", self.timeout_seconds, self.retries)); errors } @@ -379,8 +417,8 @@ impl JobTemplateTarget { errors.extend(url_errors("url", &self.url)); if self.url.trim_end_matches('/').ends_with("/api") || self.url.contains("/api/v2") { errors.push(format!( - "url '{}' is the controller's address, without /api", - self.url + "url '{}…' is the controller's address, without /api", + origin(&self.url) )); } match (self.job_template, self.workflow_job_template) { @@ -459,13 +497,40 @@ fn template_errors(body: &str) -> Vec { // Every {{name}} in a template, in order. pub fn placeholders(template: &str) -> impl Iterator { - template.split("{{").skip(1).filter_map(|rest| { - rest.split_once("}}") - .map(|(name, _)| name.trim()) - .filter(|name| !name.is_empty()) + template_parts(template).filter_map(|part| match part { + TemplatePart::Field(name) => Some(name), + TemplatePart::Text(_) => None, }) } +// A template, split once, the same way for checking it at load and for +// rendering it: text, and the {{field}} placeholders in it. A `{{` with no +// `}}` after it, or with nothing between them, is text. +pub enum TemplatePart<'a> { + Text(&'a str), + Field(&'a str), +} + +pub fn template_parts(template: &str) -> impl Iterator> { + let mut pieces = template.split("{{"); + let first = pieces.next().map(TemplatePart::Text); + first.into_iter().chain(pieces.flat_map(|piece| { + match piece.split_once("}}") { + Some((name, after)) if !name.trim().is_empty() => [ + Some(TemplatePart::Field(name.trim())), + Some(TemplatePart::Text(after)), + ], + // Not a placeholder: the `{{` is text, and so is what follows it. + _ => [ + Some(TemplatePart::Text("{{")), + Some(TemplatePart::Text(piece)), + ], + } + .into_iter() + .flatten() + })) +} + fn is_header_name(name: &str) -> bool { !name.is_empty() && name @@ -558,10 +623,20 @@ mod tests { ); assert_eq!( header.webhook.unwrap().auth, - Auth::Header { + Auth::Header(HeaderAuth { header: "X-Gitlab-Token".into() - } + }) + ); + // Strict: a key the header form does not have is an error. + assert!(serde_saphyr::from_str::( + "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com\n credentials: [c]\n auth: {header: X-Key, value: y}\n" + ) + .is_err()); + // One header, one source. + let twice = parse( + "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com\n credentials: [c]\n headers: {authorization: x}\n", ); + assert!(twice.errors("n").join("").contains("set it in one place")); let missing = parse( "type: webhook\nevents: [sync_failed]\nwebhook:\n url: https://x.example.com\n auth: bearer\n", ); diff --git a/src/main.rs b/src/main.rs index eeb757f..6751436 100644 --- a/src/main.rs +++ b/src/main.rs @@ -269,6 +269,7 @@ async fn main() { // shutdown_grace_seconds — for in-flight runs to finish. Past the grace // the process exits anyway: blocking on a wedged connector would only trade // a clean exit for a SIGKILL. + let shutdown_started = std::time::Instant::now(); let _ = shutdown_tx.send(true); let handles: Vec> = std::mem::take(background_tasks.lock().expect("handle registry").as_mut()); @@ -291,6 +292,16 @@ async fn main() { info!("Background tasks drained"); } + // Then the webhooks those last runs queued, in what is left of the grace: + // a drift reverted in the very last run must still be reported. + let left = state + .dispatcher + .drain(grace.saturating_sub(shutdown_started.elapsed())) + .await; + if left > 0 { + warn!(left, "Webhook messages still unsent at shutdown — dropped"); + } + info!("Shutdown complete"); } diff --git a/src/ports/git.rs b/src/ports/git.rs index 6eb8f9c..49baa10 100644 --- a/src/ports/git.rs +++ b/src/ports/git.rs @@ -34,6 +34,15 @@ pub trait GitPort: Send + Sync { Box::pin(async { None }) } + // The git tree id of `folder` at the checkout's HEAD: it changes only + // when something in that folder does, unlike the commit, which any change + // anywhere in the repository moves. What tells drift (the system changed) + // from a commit (git changed) for one application. Works on a shallow + // clone. None when it cannot tell. + fn tree(&self, _dir: &Path, _folder: &str) -> RevisionFuture<'_> { + Box::pin(async { None }) + } + // Write `path` of the checkout in `dir` as it was at `revision` into // `dest`, without touching the checkout itself (its files, index and // HEAD stay where the pulls left them): what a rollback applies. Answers diff --git a/src/ports/http_sender.rs b/src/ports/http_sender.rs index 6638de6..4ea3bba 100644 --- a/src/ports/http_sender.rs +++ b/src/ports/http_sender.rs @@ -21,12 +21,20 @@ pub struct OutboundResponse { pub body: String, } +// A request that got no answer. `retryable` is false when it could not even +// be built (a malformed URL): sending it again would fail the same way. +#[derive(Debug, Clone, PartialEq)] +pub struct SendError { + pub message: String, + pub retryable: bool, +} + pub type SendFuture<'a> = - Pin> + Send + 'a>>; + Pin> + Send + 'a>>; // Sends webhooks. Err is a request that got no answer (refused, timed -// out, TLS); any answer, error statuses included, is Ok, and the caller -// decides what it means for its receiver. +// out, TLS, or not buildable); any answer, error statuses included, is Ok, +// and the caller decides what it means for its receiver. pub trait HttpSenderPort: Send + Sync { fn send(&self, request: OutboundRequest) -> SendFuture<'_>; } diff --git a/tests/webhooks_test.rs b/tests/webhooks_test.rs index 60a69d2..9ff107b 100644 --- a/tests/webhooks_test.rs +++ b/tests/webhooks_test.rs @@ -175,7 +175,9 @@ async fn a_sync_that_applies_a_change_sends_sync_succeeded() { let body: Value = serde_json::from_slice(&request.body).unwrap(); assert_eq!(body["event"], "sync_succeeded"); assert_eq!(body["application"], "app-demo"); - assert_eq!(body["cause"], "commit"); + // The checkout here is a plain folder, not git: no tree to tell a commit + // from drift by, so the cause is unknown rather than guessed. + assert_eq!(body["cause"], Value::Null); assert_eq!(body["applied"], 1); assert_eq!(body["changes"][0]["kind"], "namespace"); assert_eq!(body["webhook"], "hook"); @@ -314,5 +316,6 @@ async fn the_http_sender_delivers_and_does_not_follow_redirects() { }) .await .unwrap_err(); - assert!(!unreachable.contains("secret"), "{unreachable}"); + assert!(!unreachable.message.contains("secret"), "{unreachable:?}"); + assert!(unreachable.retryable, "no answer is worth retrying"); } From 0b3691182abadcaddcb81cc6b87796df203cbb29 Mon Sep 17 00:00:00 2001 From: blaipr Date: Mon, 26 Oct 2026 19:18:28 +0100 Subject: [PATCH 5/5] feat: count requests to git servers --- CHANGELOG.md | 1 + docs/observability.md | 2 ++ src/application/projects.rs | 38 ++++++++++++++++++++++++++++++++++-- src/application/reconcile.rs | 15 ++++++++++---- 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e13216..5f586f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ project adheres to [Semantic Versioning](https://semver.org/). ### Added - Webhooks: a `webhooks` item sends, for `all` applications or the ones it names, `drift_detected`, `out_of_sync`, `sync_succeeded`, `sync_failed` and `back_in_sync` to a webhook (any receiver, with bearer, basic or header auth, a body template and optional HMAC signing) or to an Ascender or AWX job template (through the template's own webhook receiver, so no user is needed), one message per run, drift told apart from a commit by the application's own folder in git, queued and retried without one receiver holding up the others, its changes listed by name and never with a field's value; `GET /api/v1/webhooks` shows each one's last send, `GET /api/v1/webhooks/{id}` its last 50, and `POST /api/v1/webhooks/{id}/test` sends a test (see `docs/webhooks.md`). +- Metrics `unified_gitops_git_requests_total{project,operation,outcome}` and `unified_gitops_git_request_duration_seconds`: every clone, pull and rollback fetch this instance makes to a git server, as `unified_gitops_connector_calls_total` counts those to the systems. ### Changed diff --git a/docs/observability.md b/docs/observability.md index 5082c58..5e2d8d5 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -27,6 +27,8 @@ stops running pushes nothing, so only a scrape-time gauge shows the silence. | `unified_gitops_project_sync_last_success_age_seconds` | project | Seconds since the last good pull | | `unified_gitops_connector_calls_total` | application, connector, command, outcome | Calls to the systems: `list` reads, `apply` writes, `describe` starts a run | | `unified_gitops_connector_call_duration_seconds` | application, connector, command | How long each call to a system took (histogram) | +| `unified_gitops_git_requests_total` | project, operation, outcome | Requests to git servers: a `clone` or a `pull` per project, the `fetch` a rollback makes | +| `unified_gitops_git_request_duration_seconds` | project, operation | How long each request to a git server took (histogram) | | `unified_gitops_webhooks_total` | webhook, event, outcome | Messages sent (`success`), failed after retries (`error`), or dropped by a full queue | | `unified_gitops_webhook_duration_seconds` | webhook | How long a send took, retries included (histogram) | | `unified_gitops_http_requests_total` | method, path, status | Requests, by matched route | diff --git a/src/application/projects.rs b/src/application/projects.rs index a719420..11d0533 100644 --- a/src/application/projects.rs +++ b/src/application/projects.rs @@ -92,7 +92,13 @@ async fn run( // never answers used to hang this future — and with it whatever awaited // the sync. Dropping the future on timeout kills the git child // (kill_on_drop in the adapter), so nothing keeps running behind our back. - match tokio::time::timeout( + let operation = if dir.join(".git").exists() { + "pull" + } else { + "clone" + }; + let started = std::time::Instant::now(); + let pulled = match tokio::time::timeout( std::time::Duration::from_secs(project.timeout_seconds), git.ensure(&dir, project, &credentials), ) @@ -103,7 +109,9 @@ async fn run( "git operation timed out after {}s", project.timeout_seconds )), - }?; + }; + count_git_request(project_id, operation, pulled.is_ok(), started); + pulled?; if !project.python_venv { return Ok(()); @@ -133,6 +141,32 @@ async fn run( } } +// Every request this instance makes to a git server, as count_call in +// application::reconcile counts those to the systems: a clone or a pull per +// project, and the fetch a rollback makes (application::reconcile). What the +// Overview's repositories chart reads. +pub fn count_git_request( + project_id: &str, + operation: &'static str, + ok: bool, + started: std::time::Instant, +) { + let outcome = if ok { "success" } else { "error" }; + metrics::counter!( + "unified_gitops_git_requests_total", + "project" => project_id.to_string(), + "operation" => operation, + "outcome" => outcome + ) + .increment(1); + metrics::histogram!( + "unified_gitops_git_request_duration_seconds", + "project" => project_id.to_string(), + "operation" => operation + ) + .record(started.elapsed().as_secs_f64()); +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/application/reconcile.rs b/src/application/reconcile.rs index eef649f..cf3397c 100644 --- a/src/application/reconcile.rs +++ b/src/application/reconcile.rs @@ -258,7 +258,8 @@ async fn export_tree( let _fetching = lock.write().await; // Bounded like a pull: fetching history from a remote that never // answers must not hold the application's lock forever. - let sha = tokio::time::timeout( + let started = std::time::Instant::now(); + let fetched = tokio::time::timeout( std::time::Duration::from_secs(project.timeout_seconds), r.git.export( &checkout, @@ -275,9 +276,15 @@ async fn export_tree( "reading commit {revision} took longer than {} s", project.timeout_seconds ) - })? - .map_err(|e| e.message)?; - Ok((exported, sha)) + }) + .and_then(|r| r.map_err(|e| e.message)); + crate::application::projects::count_git_request( + &app.project, + "fetch", + fetched.is_ok(), + started, + ); + Ok((exported, fetched?)) } // What a rollback to `revision` would change, without changing anything: