From b09b0ff7925ef69af0fc6972e4328058c8b46dbf Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 24 Jul 2025 14:11:29 +0200 Subject: [PATCH] feat: enable workspace exclusion in custom tags (#6263) * feat: enable tags that exclude workspaces * feat: frontend tooltip * fix: use method to check if tag applies * refactor: change conversion method * fix: change operator comparison * remove redundant tests --- backend/windmill-api/src/workers.rs | 30 +- backend/windmill-common/src/jobs.rs | 12 +- backend/windmill-common/src/worker.rs | 283 ++++++++++++++++-- .../lib/components/AssignableTagsInner.svelte | 5 + 4 files changed, 273 insertions(+), 57 deletions(-) diff --git a/backend/windmill-api/src/workers.rs b/backend/windmill-api/src/workers.rs index 2f93b55a6e..7e2593cf0e 100644 --- a/backend/windmill-api/src/workers.rs +++ b/backend/windmill-api/src/workers.rs @@ -15,6 +15,7 @@ use axum::{ use serde::{Deserialize, Serialize}; use sqlx::FromRow; use uuid::Uuid; +use windmill_common::worker::SpecificTagType; use windmill_common::{ db::UserDB, error::JsonResult, @@ -146,33 +147,12 @@ async fn get_custom_tags(Query(query): Query) -> JsonResult>(); - let all_tags = tags_o.0.clone(); - return Ok(Json( - all_tags - .into_iter() - .chain(workspace_tags.into_iter()) - .collect(), - )); + let all_tags = tags_o.to_string_vec(Some(workspace)); + return Ok(Json(all_tags)); } else if query.show_workspace_restriction.is_some_and(|x| x) { let tags_o = CUSTOM_TAGS_PER_WORKSPACE.read().await; - let workspace_tags = tags_o - .1 - .iter() - .map(|(tag, workspaces)| format!("{}({})", tag, workspaces.join("+"))) - .collect::>(); - let all_tags = tags_o.0.clone(); - return Ok(Json( - all_tags - .into_iter() - .chain(workspace_tags.into_iter()) - .collect(), - )); + let all_tags = tags_o.to_string_vec(None); + return Ok(Json(all_tags)); } Ok(Json(ALL_TAGS.read().await.clone().into())) } diff --git a/backend/windmill-common/src/jobs.rs b/backend/windmill-common/src/jobs.rs index 968de73500..2b4a9a3067 100644 --- a/backend/windmill-common/src/jobs.rs +++ b/backend/windmill-common/src/jobs.rs @@ -663,16 +663,10 @@ pub async fn check_tag_available_for_workspace_internal( } let custom_tags_per_w = CUSTOM_TAGS_PER_WORKSPACE.read().await; - if custom_tags_per_w.0.contains(&tag.to_string()) { - is_tag_in_workspace_custom_tags = true; - } else if custom_tags_per_w.1.contains_key(tag) - && custom_tags_per_w - .1 - .get(tag) - .unwrap() - .contains(&w_id.to_string()) - { + if custom_tags_per_w.global.contains(&tag.to_string()) { is_tag_in_workspace_custom_tags = true; + } else if let Some(specific_tag) = custom_tags_per_w.specific.get(tag) { + is_tag_in_workspace_custom_tags = specific_tag.applies_to_workspace(w_id); } match is_tag_in_scope_tags { diff --git a/backend/windmill-common/src/worker.rs b/backend/windmill-common/src/worker.rs index 91ab306c31..8bc095f482 100644 --- a/backend/windmill-common/src/worker.rs +++ b/backend/windmill-common/src/worker.rs @@ -35,6 +35,102 @@ use crate::{ KillpillSender, DB, }; +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)] +pub struct CustomTags { + pub global: Vec, + pub specific: HashMap, +} + +impl CustomTags { + pub fn from(tags: Vec) -> Self { + let mut global = vec![]; + let mut specific: HashMap = HashMap::new(); + for e in tags { + if let Some(cap) = CUSTOM_TAG_REGEX.captures(&e) { + let tag_name = cap.get(1).unwrap().as_str().to_string(); + let workspace_str = cap.get(2).unwrap().as_str(); + let tag_type = SpecificTagType::from_regex_string(workspace_str); + let workspaces: Vec = workspace_str + .split(tag_type.corresponding_separator()) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .collect(); + if workspaces.is_empty() { + tracing::warn!("Ignoring tag `{}` with empty exclusion/inclusion list", e); + global.push(e); + continue; + } + specific.insert(tag_name, SpecificTagData { tag_type, workspaces }); + } else { + global.push(e.to_string()); + } + } + Self { global, specific } + } + + pub fn to_string_vec(&self, filter_with_workspace: Option) -> Vec { + let specific = if let Some(workspace) = filter_with_workspace { + self.specific + .iter() + .filter(|(_, tag_data)| tag_data.applies_to_workspace(&workspace)) + .map(|(tag, _)| tag.clone()) + .collect::>() + } else { + self.specific + .iter() + .map(|(tag, tag_data)| { + let separator = tag_data.tag_type.corresponding_separator(); + let mut workspaces = tag_data.workspaces.join(&*separator.to_string()); + if tag_data.tag_type == SpecificTagType::AllExcluding { + // the AllExcluding tag syntax has a leading separator + workspaces.insert(0, separator); + } + format!("{}({})", tag, workspaces) + }) + .collect::>() + }; + let all_tags = self.global.clone(); + all_tags.into_iter().chain(specific.into_iter()).collect() + } +} +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct SpecificTagData { + pub tag_type: SpecificTagType, + pub workspaces: Vec, +} + +impl SpecificTagData { + pub fn applies_to_workspace(&self, workspace_id: &str) -> bool { + match self.tag_type { + SpecificTagType::AllExcluding => !self.workspaces.contains(&workspace_id.to_string()), + SpecificTagType::NoneExcept => self.workspaces.contains(&workspace_id.to_string()), + } + } +} +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub enum SpecificTagType { + AllExcluding, + NoneExcept, +} + +impl SpecificTagType { + pub fn corresponding_separator(&self) -> char { + match self { + SpecificTagType::AllExcluding => '^', + SpecificTagType::NoneExcept => '+', + } + } + + pub fn from_regex_string(workspaces_str: &str) -> Self { + if workspaces_str.contains(SpecificTagType::AllExcluding.corresponding_separator()) { + SpecificTagType::AllExcluding + } else { + // Regex match ensures the second branch is correct + SpecificTagType::NoneExcept + } + } +} + pub const DEFAULT_CLOUD_TIMEOUT: u64 = 900; pub const DEFAULT_SELFHOSTED_TIMEOUT: u64 = 604800; // 7 days pub const MIN_PERIODIC_SCRIPT_INTERVAL_SECONDS: u64 = 60; @@ -130,11 +226,21 @@ lazy_static::lazy_static! { .map(|x| x.split(',').map(|x| x.to_string()).collect::>()).unwrap_or_default(); - pub static ref CUSTOM_TAGS_PER_WORKSPACE: Arc, HashMap>)>> = Arc::new(RwLock::new((vec![], HashMap::new()))); + pub static ref CUSTOM_TAGS_PER_WORKSPACE: Arc> = Arc::new(RwLock::new(CustomTags::default())); pub static ref ALL_TAGS: Arc>> = Arc::new(RwLock::new(vec![])); - static ref CUSTOM_TAG_REGEX: Regex = Regex::new(r"^([\w-]+)\(((?:[\w-]+\+)*[\w-]+)\)$").unwrap(); + + + // ^([\w-]+) # Group 1: tag name + // \( # Literal '(' + // ( # Group 2: the full workspace list + // (?:[\w-]+\+)*[\w-]+ # NoneExcept pattern: ws1+ws2 + // | # OR + // (?:\^[\w-]+)+ # AllExcluding pattern: ^ws1^ws2 + // ) + // \)$ # Closing ')' + static ref CUSTOM_TAG_REGEX: Regex = Regex::new(r"^([\w-]+)\(((?:[\w-]+\+)*[\w-]+|(?:\^[\w-]+)+)\)$").unwrap(); pub static ref DISABLE_BUNDLING: bool = std::env::var("DISABLE_BUNDLING") .ok() @@ -473,12 +579,12 @@ pub async fn reload_custom_tags_setting(db: &DB) -> error::Result<()> { CUSTOM_TAGS.clone() }; - let custom_tags = process_custom_tags(tags); + let custom_tags = CustomTags::from(tags); tracing::info!( "Loaded setting custom_tags, common: {:?}, per-workspace: {:?}", - custom_tags.0, - custom_tags.1, + custom_tags.global, + custom_tags.specific, ); { @@ -488,29 +594,18 @@ pub async fn reload_custom_tags_setting(db: &DB) -> error::Result<()> { { let mut l = ALL_TAGS.write().await; *l = [ - custom_tags.0.clone(), - custom_tags.1.keys().map(|x| x.to_string()).collect_vec(), + custom_tags.global.clone(), + custom_tags + .specific + .keys() + .map(|x| x.to_string()) + .collect_vec(), ] - .concat(); + .concat(); } Ok(()) } -fn process_custom_tags(tags: Vec) -> (Vec, HashMap>) { - let mut global = vec![]; - let mut specific: HashMap> = HashMap::new(); - for e in tags { - if let Some(cap) = CUSTOM_TAG_REGEX.captures(&e) { - let tag = cap.get(1).unwrap().as_str().to_string(); - let workspaces = cap.get(2).unwrap().as_str().split("+"); - specific.insert(tag, workspaces.map(|x| x.to_string()).collect_vec()); - } else { - global.push(e.to_string()); - } - } - (global, specific) -} - fn parse_file(path: &str) -> Option { std::process::Command::new("cat") .args([path]) @@ -1616,3 +1711,145 @@ pub fn to_raw_value_owned(result: serde_json::Value) -> Box { serde_json::value::to_raw_value(&result) .unwrap_or_else(|_| RawValue::from_string("{}".to_string()).unwrap()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + + + #[test] + fn test_mixed_tags() { + let input = vec![ + "global".to_string(), + "feat(ws1+ws2)".to_string(), + "hotfix(^ws3^ws4)".to_string(), + ]; + let result = CustomTags::from(input); + + assert_eq!(result.global, vec!["global"]); + + let mut expected = HashMap::new(); + expected.insert( + "feat".to_string(), + SpecificTagData { + tag_type: SpecificTagType::NoneExcept, + workspaces: vec!["ws1".to_string(), "ws2".to_string()], + }, + ); + expected.insert( + "hotfix".to_string(), + SpecificTagData { + tag_type: SpecificTagType::AllExcluding, + workspaces: vec!["ws3".to_string(), "ws4".to_string()], + }, + ); + + assert_eq!(result.specific, expected); + } + + #[test] + fn test_invalid_specific_tag_format() { + let input = vec!["invalid(custom+format".to_string()]; + let result = CustomTags::from(input); + + // Regex does not match, so it's treated as a global tag. + assert_eq!(result.global, vec!["invalid(custom+format"]); + assert!(result.specific.is_empty()); + } + + #[test] + fn test_empty_input() { + let input: Vec = vec![]; + let result = CustomTags::from(input); + assert!(result.global.is_empty()); + assert!(result.specific.is_empty()); + } + + #[test] + fn test_custom_tags_from_parses_global_tags_correctly() { + let input = vec!["frontend".to_string(), "urgent".to_string()]; + let tags = CustomTags::from(input.clone()); + assert_eq!(tags.global, input); + assert!(tags.specific.is_empty()); + } + + #[test] + fn test_custom_tags_from_parses_specific_tags_none_except() { + let input = vec!["urgent(ws1+ws2)".to_string()]; + let tags = CustomTags::from(input.clone()); + + assert!(tags.global.is_empty()); + assert_eq!(tags.specific.len(), 1); + + let data = tags.specific.get("urgent").unwrap(); + assert_eq!(data.tag_type, SpecificTagType::NoneExcept); + assert_eq!(data.workspaces, vec!["ws1", "ws2"]); + } + + #[test] + fn test_custom_tags_from_parses_specific_tags_all_excluding() { + let input = vec!["legacy(^ws1^ws2)".to_string()]; + let tags = CustomTags::from(input.clone()); + + assert!(tags.global.is_empty()); + assert_eq!(tags.specific.len(), 1); + + let data = tags.specific.get("legacy").unwrap(); + assert_eq!(data.tag_type, SpecificTagType::AllExcluding); + assert_eq!(data.workspaces, vec!["ws1", "ws2"]); + } + + #[test] + fn test_custom_tags_from_ignores_empty_workspace_list_as_global() { + let input = vec!["foo()".to_string(), "bar(^)".to_string()]; + let tags = CustomTags::from(input.clone()); + + assert_eq!(tags.global, input); + assert!(tags.specific.is_empty()); + } + + #[test] + fn test_custom_tags_from_filters_specific_tags_by_workspace_none_except() { + let input = vec!["urgent(ws1+ws2)".to_string()]; + let tags = CustomTags::from(input); + + let output = tags.to_string_vec(Some("ws1".to_string())); + assert_eq!(output, vec!["urgent"]); + + let output_none = tags.to_string_vec(Some("ws3".to_string())); + assert!(output_none.is_empty()); + } + + #[test] + fn test_custom_tags_from_filters_specific_tags_by_workspace_all_excluding() { + let input = vec!["legacy(^ws1^ws2)".to_string()]; + let tags = CustomTags::from(input); + + let output = tags.to_string_vec(Some("ws3".to_string())); + assert_eq!(output, vec!["legacy"]); + + let output_excluded = tags.to_string_vec(Some("ws1".to_string())); + assert!(output_excluded.is_empty()); + } + + #[test] + fn test_custom_tags_from_reconstructs_all_tags_when_no_filter() { + let input = vec![ + "foo".to_string(), + "urgent(ws1+ws2)".to_string(), + "legacy(^ws1^ws2)".to_string(), + ]; + let tags = CustomTags::from(input); + + let result = tags.to_string_vec(None); + assert_eq!( + result, + vec![ + "foo", + "urgent(ws1+ws2)", + "legacy(^ws1^ws2)" + ] + ); + } +} diff --git a/frontend/src/lib/components/AssignableTagsInner.svelte b/frontend/src/lib/components/AssignableTagsInner.svelte index 09a848c505..af77125b87 100644 --- a/frontend/src/lib/components/AssignableTagsInner.svelte +++ b/frontend/src/lib/components/AssignableTagsInner.svelte @@ -95,6 +95,11 @@ >For tags specific to some workspaces, use 
tag(workspace1+workspace2)
+ To exclude 'workspace1' and 'workspace2' from a tag, use 
tag(^workspace1^workspace2)
 For dynamic tags based on the workspace, use 
$workspace
, e.g: 
tag-$workspace