Skip to content

bugfixes for target separation #1365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/alerts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use serde_json::Error as SerdeError;
use std::collections::{HashMap, HashSet};
use std::fmt::{self, Display};
use std::thread;
use std::time::Duration;
use tokio::sync::oneshot::{Receiver, Sender};
use tokio::sync::{mpsc, RwLock};
use tokio::task::JoinHandle;
Expand Down Expand Up @@ -610,10 +611,12 @@ impl AlertConfig {
// validate that target repeat notifs !> eval_frequency
for target_id in &self.targets {
let target = TARGETS.get_target_by_id(target_id).await?;
match &target.timeout.times {
match &target.notification_config.times {
target::Retry::Infinite => {}
target::Retry::Finite(repeat) => {
let notif_duration = target.timeout.interval * *repeat as u32;
let notif_duration =
Duration::from_secs(60 * target.notification_config.interval)
* *repeat as u32;
if (notif_duration.as_secs_f64()).gt(&((eval_frequency * 60) as f64)) {
return Err(AlertError::Metadata(
"evalFrequency should be greater than target repetition interval",
Expand Down Expand Up @@ -853,8 +856,8 @@ pub enum AlertError {
FromStrError(#[from] FromStrError),
#[error("Invalid Target ID- {0}")]
InvalidTargetID(String),
#[error("Target already exists")]
DuplicateTargetConfig,
#[error("Invalid target modification request: {0}")]
InvalidTargetModification(String),
#[error("Can't delete a Target which is being used")]
TargetInUse,
}
Expand All @@ -875,7 +878,7 @@ impl actix_web::ResponseError for AlertError {
Self::InvalidAlertModifyRequest => StatusCode::BAD_REQUEST,
Self::FromStrError(_) => StatusCode::BAD_REQUEST,
Self::InvalidTargetID(_) => StatusCode::BAD_REQUEST,
Self::DuplicateTargetConfig => StatusCode::BAD_REQUEST,
Self::InvalidTargetModification(_) => StatusCode::BAD_REQUEST,
Self::TargetInUse => StatusCode::CONFLICT,
}
}
Expand Down
109 changes: 82 additions & 27 deletions src/alerts/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ use base64::Engine;
use bytes::Bytes;
use chrono::Utc;
use http::{header::AUTHORIZATION, HeaderMap, HeaderValue};
use humantime_serde::re::humantime;
use itertools::Itertools;
use once_cell::sync::Lazy;
use reqwest::ClientBuilder;
use serde_json::{json, Value};
use tokio::sync::RwLock;
use tracing::{error, trace, warn};
use ulid::Ulid;
Expand Down Expand Up @@ -66,14 +66,6 @@ impl TargetConfigs {

pub async fn update(&self, target: Target) -> Result<(), AlertError> {
let mut map = self.target_configs.write().await;
if map.values().any(|t| {
t.target == target.target
&& t.timeout.interval == target.timeout.interval
&& t.timeout.times == target.timeout.times
&& t.id != target.id
}) {
return Err(AlertError::DuplicateTargetConfig);
}
map.insert(target.id, target.clone());

let path = target_json_path(&target.id);
Expand Down Expand Up @@ -148,16 +140,84 @@ pub struct Target {
pub name: String,
#[serde(flatten)]
pub target: TargetType,
#[serde(default, rename = "repeat")]
pub timeout: Timeout,
pub notification_config: Timeout,
#[serde(default = "Ulid::new")]
pub id: Ulid,
}

impl Target {
pub fn mask(self) -> Value {
match self.target {
TargetType::Slack(slack_web_hook) => {
let endpoint = slack_web_hook.endpoint.to_string();
let masked_endpoint = if endpoint.len() > 20 {
format!("{}********", &endpoint[..20])
} else {
"********".to_string()
};
json!({
"name":self.name,
"type":"slack",
"endpoint":masked_endpoint,
"notificationConfig":self.notification_config,
"id":self.id
})
}
TargetType::Other(other_web_hook) => {
let endpoint = other_web_hook.endpoint.to_string();
let masked_endpoint = if endpoint.len() > 20 {
format!("{}********", &endpoint[..20])
} else {
"********".to_string()
};
json!({
"name":self.name,
"type":"webhook",
"endpoint":masked_endpoint,
"headers":other_web_hook.headers,
"skipTlsCheck":other_web_hook.skip_tls_check,
"notificationConfig":self.notification_config,
"id":self.id
})
}
TargetType::AlertManager(alert_manager) => {
let endpoint = alert_manager.endpoint.to_string();
let masked_endpoint = if endpoint.len() > 20 {
format!("{}********", &endpoint[..20])
} else {
"********".to_string()
};
if let Some(auth) = alert_manager.auth {
let password = "********";
json!({
"name":self.name,
"type":"webhook",
"endpoint":masked_endpoint,
"username":auth.username,
"password":password,
"skipTlsCheck":alert_manager.skip_tls_check,
"notificationConfig":self.notification_config,
"id":self.id
})
} else {
json!({
"name":self.name,
"type":"webhook",
"endpoint":masked_endpoint,
"username":Value::Null,
"password":Value::Null,
"skipTlsCheck":alert_manager.skip_tls_check,
"notificationConfig":self.notification_config,
"id":self.id
})
}
}
}
}

pub fn call(&self, context: Context) {
trace!("target.call context- {context:?}");
let timeout = &self.timeout;
let timeout = &self.notification_config;
let resolves = context.alert_info.alert_state;
let mut state = timeout.state.lock().unwrap();
trace!("target.call state- {state:?}");
Expand Down Expand Up @@ -205,7 +265,7 @@ impl Target {
let sleep_and_check_if_call =
move |timeout_state: Arc<Mutex<TimeoutState>>, current_state: AlertState| {
async move {
tokio::time::sleep(timeout).await;
tokio::time::sleep(Duration::from_secs(timeout * 60)).await;

let mut state = timeout_state.lock().unwrap();

Expand Down Expand Up @@ -276,8 +336,8 @@ fn call_target(target: TargetType, context: Context) {
}

#[derive(Debug, serde::Deserialize)]
pub struct RepeatVerifier {
interval: Option<String>,
pub struct NotificationConfigVerifier {
interval: Option<u64>,
times: Option<usize>,
}

Expand All @@ -288,7 +348,7 @@ pub struct TargetVerifier {
#[serde(flatten)]
pub target: TargetType,
#[serde(default)]
pub repeat: Option<RepeatVerifier>,
pub notification_config: Option<NotificationConfigVerifier>,
#[serde(default = "Ulid::new")]
pub id: Ulid,
}
Expand All @@ -304,26 +364,22 @@ impl TryFrom<TargetVerifier> for Target {
timeout.times = Retry::Infinite
}

if let Some(repeat_config) = value.repeat {
let interval = repeat_config
.interval
.map(|ref interval| humantime::parse_duration(interval))
.transpose()
.map_err(|err| err.to_string())?;
if let Some(notification_config) = value.notification_config {
let interval = notification_config.interval.map(|ref interval| *interval);

if let Some(interval) = interval {
timeout.interval = interval
}

if let Some(times) = repeat_config.times {
if let Some(times) = notification_config.times {
timeout.times = Retry::Finite(times)
}
}

Ok(Target {
name: value.name,
target: value.target,
timeout,
notification_config: timeout,
id: value.id,
})
}
Expand Down Expand Up @@ -518,8 +574,7 @@ impl CallableTarget for AlertManager {

#[derive(Debug, serde::Serialize, serde::Deserialize, Clone)]
pub struct Timeout {
#[serde(with = "humantime_serde")]
pub interval: Duration,
pub interval: u64,
#[serde(default = "Retry::default")]
pub times: Retry,
#[serde(skip)]
Expand All @@ -529,7 +584,7 @@ pub struct Timeout {
impl Default for Timeout {
fn default() -> Self {
Self {
interval: Duration::from_secs(60),
interval: 1,
times: Retry::default(),
state: Arc::<Mutex<TimeoutState>>::default(),
}
Expand Down
32 changes: 19 additions & 13 deletions src/handlers/http/modal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use tokio::sync::oneshot;
use tracing::{error, info, warn};

use crate::{
alerts::ALERTS,
alerts::{target::TARGETS, ALERTS},
cli::Options,
correlation::CORRELATIONS,
oidc::Claims,
Expand Down Expand Up @@ -173,18 +173,20 @@ pub trait ParseableServer {

pub async fn load_on_init() -> anyhow::Result<()> {
// Run all loading operations concurrently
let (correlations_result, filters_result, dashboards_result, alerts_result) = future::join4(
async {
CORRELATIONS
.load()
.await
.context("Failed to load correlations")
},
async { FILTERS.load().await.context("Failed to load filters") },
async { DASHBOARDS.load().await.context("Failed to load dashboards") },
async { ALERTS.load().await.context("Failed to load alerts") },
)
.await;
let (correlations_result, filters_result, dashboards_result, alerts_result, targets_result) =
future::join5(
async {
CORRELATIONS
.load()
.await
.context("Failed to load correlations")
},
async { FILTERS.load().await.context("Failed to load filters") },
async { DASHBOARDS.load().await.context("Failed to load dashboards") },
async { ALERTS.load().await.context("Failed to load alerts") },
async { TARGETS.load().await.context("Failed to load targets") },
)
.await;

// Handle errors from each operation
if let Err(e) = correlations_result {
Expand All @@ -203,6 +205,10 @@ pub async fn load_on_init() -> anyhow::Result<()> {
error!("{err}");
}

if let Err(err) = targets_result {
error!("{err}");
}

Ok(())
}

Expand Down
25 changes: 19 additions & 6 deletions src/handlers/http/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use actix_web::{
web::{self, Json, Path},
HttpRequest, Responder,
};
use itertools::Itertools;
use ulid::Ulid;

use crate::alerts::{
Expand All @@ -18,13 +19,18 @@ pub async fn post(
// add to the map
TARGETS.update(target.clone()).await?;

Ok(web::Json(target))
Ok(web::Json(target.mask()))
}

// GET /targets
pub async fn list(_req: HttpRequest) -> Result<impl Responder, AlertError> {
// add to the map
let list = TARGETS.list().await?;
let list = TARGETS
.list()
.await?
.into_iter()
.map(|t| t.mask())
.collect_vec();

Ok(web::Json(list))
}
Expand All @@ -35,7 +41,7 @@ pub async fn get(_req: HttpRequest, target_id: Path<Ulid>) -> Result<impl Respon

let target = TARGETS.get_target_by_id(&target_id).await?;

Ok(web::Json(target))
Ok(web::Json(target.mask()))
}

// PUT /targets/{target_id}
Expand All @@ -47,7 +53,14 @@ pub async fn update(
let target_id = target_id.into_inner();

// if target_id does not exist, error
TARGETS.get_target_by_id(&target_id).await?;
let old_target = TARGETS.get_target_by_id(&target_id).await?;

// do not allow modifying name
if old_target.name != target.name {
return Err(AlertError::InvalidTargetModification(
"Can't modify target name".to_string(),
));
}

// esnure that the supplied target id is assigned to the target config
target.id = target_id;
Expand All @@ -56,7 +69,7 @@ pub async fn update(
// add to the map
TARGETS.update(target.clone()).await?;

Ok(web::Json(target))
Ok(web::Json(target.mask()))
}

// DELETE /targets/{target_id}
Expand All @@ -68,5 +81,5 @@ pub async fn delete(

let target = TARGETS.delete(&target_id).await?;

Ok(web::Json(target))
Ok(web::Json(target.mask()))
}
Loading