From 0085f277d08d89c15d83f88bd6360f21974522fc Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 2 Oct 2024 14:21:08 +0100 Subject: [PATCH 01/12] Initial prototype of adding static data to bundle --- bundler/src/bundle.rs | 62 ++++++++++++++++++++++++++++++++++++++++--- bundler/src/main.rs | 24 +++++++++++++---- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/bundler/src/bundle.rs b/bundler/src/bundle.rs index 164e1cae..11c0b846 100644 --- a/bundler/src/bundle.rs +++ b/bundler/src/bundle.rs @@ -3,13 +3,15 @@ use schemars::{schema::RootSchema, schema_for, JsonSchema}; use serde::Serialize; use sqlx::MySqlPool; use std::{ - collections::{hash_map::DefaultHasher, BTreeMap}, + collections::{hash_map::DefaultHasher, BTreeMap, HashMap}, + ffi::OsStr, fmt::Debug, hash::{Hash, Hasher}, + path::Path, }; use tar::Header; use tokio::try_join; -use tracing::instrument; +use tracing::{instrument, trace, warn}; use crate::permissionables::{ beamlines::Beamlines, proposals::Proposals, sessions::Sessions, subjects::Subjects, @@ -74,6 +76,8 @@ where proposals: Proposals, /// A mapping of beamlines to their various attributes beamlines: Beamlines, + /// A map (name to data) of static files to include in the bundle + static_data: HashMap>, } /// The prefix applied to data files in the bundle. Open Policy Agent does not support loading bundles with overlapping prefixes @@ -90,6 +94,7 @@ where sessions: Sessions, proposals: Proposals, beamlines: Beamlines, + static_data: HashMap>, ) -> Self { let mut hasher = DefaultHasher::new(); metadata.hash(&mut hasher); @@ -110,20 +115,35 @@ where sessions, proposals, beamlines, + static_data, } } /// Fetches [`Subjects`] from ISPyB and constructs a [`Bundle`] #[instrument(name = "fetch_bundle")] - pub async fn fetch(metadata: Metadata, ispyb_pool: &MySqlPool) -> Result { + pub async fn fetch( + metadata: Metadata, + static_data_directory: Option<&Path>, + ispyb_pool: &MySqlPool, + ) -> Result { let (subjects, sessions, proposals, beamlines) = try_join!( Subjects::fetch(ispyb_pool), Sessions::fetch(ispyb_pool), Proposals::fetch(ispyb_pool), Beamlines::fetch(ispyb_pool), )?; + // Ignore all static data if any of the reading fails + let static_data = match static_data_directory { + Some(dir) => static_data(dir).await.unwrap_or_default(), + None => HashMap::default(), + }; Ok(Self::new( - metadata, subjects, sessions, proposals, beamlines, + metadata, + subjects, + sessions, + proposals, + beamlines, + static_data, )) } @@ -172,6 +192,15 @@ where beamlines.as_slice(), )?; + for (name, data) in &self.static_data { + let mut header = Header::from_bytes(data); + bundle_builder.append_data( + &mut header, + format!("{BUNDLE_PREFIX}/{name}/data.json"), + data.as_slice(), + )?; + } + Ok(bundle_builder.into_inner()?.finish()?) } @@ -185,3 +214,28 @@ where ]) } } + +/// Read static data from files that should be included in the compiled bundle +async fn static_data(root: &Path) -> Result>, std::io::Error> { + let mut data = HashMap::new(); + let mut contents = tokio::fs::read_dir(root).await?; + while let Some(entry) = contents.next_entry().await? { + let path = entry.path(); + if !path.is_file() || !path.extension().is_some_and(|ext| ext == "json") { + // Not explicitly a json file so ignore + trace!("Skipping non file in static data directory: {path:?}"); + continue; + } + let name = path.file_stem(); + let Some(name) = name.and_then(OsStr::to_str) else { + // Save having to think about non-utf8 in OPA rules + trace!("Skipping non-utf8 static file: {name:?}"); + continue; + }; + match tokio::fs::read(&path).await { + Ok(file_data) => _ = data.insert(name.to_string(), file_data), + Err(e) => warn!("Failed to read static data from {path:?}: {e}"), + } + } + Ok(data) +} diff --git a/bundler/src/main.rs b/bundler/src/main.rs index 55e05e60..12e0875b 100644 --- a/bundler/src/main.rs +++ b/bundler/src/main.rs @@ -35,6 +35,7 @@ use std::{ io::Write, net::{Ipv4Addr, SocketAddr, SocketAddrV4}, ops::Add, + path::{Path, PathBuf}, str::FromStr, sync::Arc, time::Duration, @@ -84,7 +85,7 @@ type CurrentBundle = Arc>>; #[command(author, version, about, long_about= None)] enum Cli { /// Run the service providing bundle data - Serve(ServeArgs), + Serve(Box), /// Output the bundle schema BundleSchema(BundleSchemaArgs), } @@ -110,6 +111,9 @@ struct ServeArgs { /// The URL of the OpenTelemetry collector to send traces to #[arg(long, env = "BUNDLER_OTEL_COLLECTOR_URL")] otel_collector_url: Option, + /// Directory containing any static data files that should be included in the bundle + #[arg(long, env = "BUNDLER_STATIC_DATA_DIRECTORY")] + static_data_directory: Option, } /// Arguments to output the schema with @@ -126,7 +130,7 @@ async fn main() { let args = Cli::parse(); match args { - Cli::Serve(args) => serve(args).await, + Cli::Serve(args) => serve(*args).await, Cli::BundleSchema(args) => bundle_schema(args), } } @@ -136,7 +140,9 @@ async fn serve(args: ServeArgs) { setup_telemetry(args.log_level, args.otel_collector_url).unwrap(); let ispyb_pool = connect_ispyb(args.database_url).await.unwrap(); - let current_bundle = fetch_initial_bundle(&ispyb_pool).await.unwrap(); + let current_bundle = fetch_initial_bundle(args.static_data_directory.as_deref(), &ispyb_pool) + .await + .unwrap(); let app = Router::new() .route("/bundle.tar.gz", get(bundle_endpoint)) .with_state(current_bundle.clone()) @@ -152,8 +158,10 @@ async fn serve(args: ServeArgs) { ); let mut tasks = tokio::task::JoinSet::new(); + let static_dir = args.static_data_directory.clone(); tasks.spawn(update_bundle( current_bundle, + static_dir, ispyb_pool, args.polling_interval.into(), )); @@ -234,11 +242,14 @@ async fn connect_ispyb(database_url: Url) -> Result { /// Fetches the intial [`Bundle`] from ISPyB and produces the correspoinding [`BundleFile`] #[instrument] async fn fetch_initial_bundle( + static_data: Option<&Path>, ispyb_pool: &MySqlPool, ) -> Result>>, anyhow::Error> { tracing::info!("Fetching initial bundle"); let bundle = Arc::new(RwLock::new(BundleFile::try_from( - Bundle::fetch(NoMetadata, ispyb_pool).await.unwrap(), + Bundle::fetch(NoMetadata, static_data, ispyb_pool) + .await + .unwrap(), )?)); tracing::info!( "Using bundle with revison: {}", @@ -258,6 +269,7 @@ async fn serve_endpoints(port: u16, app: Router) { /// Periodically update the bundle with new data from ISPyB async fn update_bundle( current_bundle: impl AsRef>>, + static_data: Option, ispyb_pool: MySqlPool, polling_interval: Duration, ) { @@ -267,7 +279,9 @@ async fn update_bundle( sleep_until(next_fetch).await; next_fetch = next_fetch.add(polling_interval); tracing::info!("Updating bundle"); - let bundle = Bundle::fetch(NoMetadata, &ispyb_pool).await.unwrap(); + let bundle = Bundle::fetch(NoMetadata, static_data.as_deref(), &ispyb_pool) + .await + .unwrap(); let bundle_file = BundleFile::try_from(bundle).unwrap(); let old_revision = current_bundle .as_ref() From 1bda7151e6cebbecb9b35fc1eaf73a64ec6dc603 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 2 Oct 2024 14:21:25 +0100 Subject: [PATCH 02/12] Add sample static data --- static/admin.json | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 static/admin.json diff --git a/static/admin.json b/static/admin.json new file mode 100644 index 00000000..e79e3bfe --- /dev/null +++ b/static/admin.json @@ -0,0 +1,36 @@ +{ + "em_admin": [], + "epsic_admin": [], + "fault_admin": [], + "gen_admin": [], + "pow_admin": [], + "mx_admin": ["i02-1", "i02-2", "i03", "i04", "i04-1", "i23", "i24"], + "saxs_admin": ["b21", "i22", "p38"], + "sm_admin": [], + "tomo_admin": [], + "xpdf_admin": ["i15", "i15-1"], + + "b07_admin": ["b07"], + "b16_admin": ["b16"], + "b18_admin": ["b18"], + "b22_admin": ["b22"], + "b23_admin": ["b23"], + "b24_admin": ["b24"], + "i05_admin": ["i05"], + "i06_admin": ["i06"], + "i07_admin": ["i07"], + "i08_admin": ["i08"], + "i09_admin": ["i09"], + "i10_admin": ["i10"], + "i11_admin": ["i11"], + "i12_admin": ["i12"], + "i13_admin": ["i13"], + "i14_admin": ["i14"], + "i16_admin": ["i16"], + "i18_admin": ["i18"], + "i20_admin": ["i20"], + "i21_admin": ["i21"], + "k11_admin": ["k11"], + "p45_admin": ["p45"], + "p99_admin": ["p99"] +} From cf40d51be07f670278b54eb28d5fc468ad55e595 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 3 Oct 2024 13:55:52 +0100 Subject: [PATCH 03/12] Review fixings * Unbox CLI args * Include static data in revision info * Fail on fs errors when reading static data --- bundler/src/bundle.rs | 52 +++++++++++++++++++++++++++++++++++++------ bundler/src/main.rs | 5 +++-- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/bundler/src/bundle.rs b/bundler/src/bundle.rs index 11c0b846..eddd163a 100644 --- a/bundler/src/bundle.rs +++ b/bundler/src/bundle.rs @@ -5,7 +5,7 @@ use sqlx::MySqlPool; use std::{ collections::{hash_map::DefaultHasher, BTreeMap, HashMap}, ffi::OsStr, - fmt::Debug, + fmt::{Debug, Display}, hash::{Hash, Hasher}, path::Path, }; @@ -102,6 +102,9 @@ where sessions.hash(&mut hasher); proposals.hash(&mut hasher); beamlines.hash(&mut hasher); + for entry in &static_data { + entry.hash(&mut hasher); + } let hash = hasher.finish(); Self { @@ -132,9 +135,8 @@ where Proposals::fetch(ispyb_pool), Beamlines::fetch(ispyb_pool), )?; - // Ignore all static data if any of the reading fails let static_data = match static_data_directory { - Some(dir) => static_data(dir).await.unwrap_or_default(), + Some(dir) => static_data(dir).await?, None => HashMap::default(), }; Ok(Self::new( @@ -232,10 +234,46 @@ async fn static_data(root: &Path) -> Result>, std::io::E trace!("Skipping non-utf8 static file: {name:?}"); continue; }; - match tokio::fs::read(&path).await { - Ok(file_data) => _ = data.insert(name.to_string(), file_data), - Err(e) => warn!("Failed to read static data from {path:?}: {e}"), - } + data.insert(name.to_string(), tokio::fs::read(&path).await?); } Ok(data) } + +/// Combination of possible errors when fetching data to create bundle +#[derive(Debug)] +pub enum BundleDataError { + /// Error fetching data from database + Sql(sqlx::Error), + /// Error fetching data from file + Static(std::io::Error), +} + +impl From for BundleDataError { + fn from(value: sqlx::Error) -> Self { + Self::Sql(value) + } +} + +impl From for BundleDataError { + fn from(value: std::io::Error) -> Self { + Self::Static(value) + } +} + +impl Display for BundleDataError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + BundleDataError::Sql(e) => write!(f, "Error reading dynamic data: {e}"), + BundleDataError::Static(e) => write!(f, "Error reading static data: {e}"), + } + } +} + +impl std::error::Error for BundleDataError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + BundleDataError::Sql(e) => Some(e), + BundleDataError::Static(e) => Some(e), + } + } +} diff --git a/bundler/src/main.rs b/bundler/src/main.rs index 12e0875b..46177873 100644 --- a/bundler/src/main.rs +++ b/bundler/src/main.rs @@ -83,9 +83,10 @@ type CurrentBundle = Arc>>; /// Bundler acts as a Open Policy Agent bundle server, providing permissionable data from the ISPyB database #[derive(Debug, Parser)] #[command(author, version, about, long_about= None)] +#[allow(clippy::large_enum_variant)] enum Cli { /// Run the service providing bundle data - Serve(Box), + Serve(ServeArgs), /// Output the bundle schema BundleSchema(BundleSchemaArgs), } @@ -130,7 +131,7 @@ async fn main() { let args = Cli::parse(); match args { - Cli::Serve(args) => serve(*args).await, + Cli::Serve(args) => serve(args).await, Cli::BundleSchema(args) => bundle_schema(args), } } From 546ea0e6bc51dc050d012dec72686a40aeed55f4 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 3 Oct 2024 14:27:30 +0100 Subject: [PATCH 04/12] Set static data via globs instead of directory --- bundler/Cargo.lock | 1 + bundler/Cargo.toml | 1 + bundler/src/bundle.rs | 37 +++++++++++++++---------------------- bundler/src/main.rs | 19 +++++++++---------- 4 files changed, 26 insertions(+), 32 deletions(-) diff --git a/bundler/Cargo.lock b/bundler/Cargo.lock index 95510f50..ce41ae5e 100644 --- a/bundler/Cargo.lock +++ b/bundler/Cargo.lock @@ -348,6 +348,7 @@ dependencies = [ "derive_more", "dotenvy", "flate2", + "glob", "headers", "humantime", "opentelemetry", diff --git a/bundler/Cargo.toml b/bundler/Cargo.toml index da66d3fd..c7709a6d 100644 --- a/bundler/Cargo.toml +++ b/bundler/Cargo.toml @@ -12,6 +12,7 @@ clio = { version = "0.3.5", features = ["clap-parse"] } derive_more = { version = "1.0.0", features = ["deref", "deref_mut"] } dotenvy = { version = "0.15.7" } flate2 = { version = "1.0.34" } +glob = "0.3.1" headers = { version = "0.4.0" } humantime = { version = "2.1.0" } opentelemetry = { version = "0.23.0" } diff --git a/bundler/src/bundle.rs b/bundler/src/bundle.rs index eddd163a..dba231c7 100644 --- a/bundler/src/bundle.rs +++ b/bundler/src/bundle.rs @@ -1,4 +1,5 @@ use flate2::{write::GzEncoder, Compression}; +use glob::Pattern; use schemars::{schema::RootSchema, schema_for, JsonSchema}; use serde::Serialize; use sqlx::MySqlPool; @@ -7,11 +8,10 @@ use std::{ ffi::OsStr, fmt::{Debug, Display}, hash::{Hash, Hasher}, - path::Path, }; use tar::Header; use tokio::try_join; -use tracing::{instrument, trace, warn}; +use tracing::{instrument, trace}; use crate::permissionables::{ beamlines::Beamlines, proposals::Proposals, sessions::Sessions, subjects::Subjects, @@ -126,7 +126,7 @@ where #[instrument(name = "fetch_bundle")] pub async fn fetch( metadata: Metadata, - static_data_directory: Option<&Path>, + static_data_directory: &[Pattern], ispyb_pool: &MySqlPool, ) -> Result { let (subjects, sessions, proposals, beamlines) = try_join!( @@ -135,10 +135,7 @@ where Proposals::fetch(ispyb_pool), Beamlines::fetch(ispyb_pool), )?; - let static_data = match static_data_directory { - Some(dir) => static_data(dir).await?, - None => HashMap::default(), - }; + let static_data = static_data(static_data_directory).await?; Ok(Self::new( metadata, subjects, @@ -218,23 +215,19 @@ where } /// Read static data from files that should be included in the compiled bundle -async fn static_data(root: &Path) -> Result>, std::io::Error> { +async fn static_data(patterns: &[Pattern]) -> Result>, std::io::Error> { let mut data = HashMap::new(); - let mut contents = tokio::fs::read_dir(root).await?; - while let Some(entry) = contents.next_entry().await? { - let path = entry.path(); - if !path.is_file() || !path.extension().is_some_and(|ext| ext == "json") { - // Not explicitly a json file so ignore - trace!("Skipping non file in static data directory: {path:?}"); - continue; + for pattern in patterns { + for file in glob::glob(pattern.as_str()).expect("Pattern was validated by CLI") { + let file = file.map_err(|e| e.into_error())?; + let name = file.file_stem(); + let Some(name) = name.and_then(OsStr::to_str) else { + // Save having to think about non-utf8 in OPA rules + trace!("Skipping non-utf8 static file: {name:?}"); + continue; + }; + data.insert(name.to_string(), tokio::fs::read(&file).await?); } - let name = path.file_stem(); - let Some(name) = name.and_then(OsStr::to_str) else { - // Save having to think about non-utf8 in OPA rules - trace!("Skipping non-utf8 static file: {name:?}"); - continue; - }; - data.insert(name.to_string(), tokio::fs::read(&path).await?); } Ok(data) } diff --git a/bundler/src/main.rs b/bundler/src/main.rs index 46177873..0af2084b 100644 --- a/bundler/src/main.rs +++ b/bundler/src/main.rs @@ -23,6 +23,7 @@ use axum::{ use axum_extra::TypedHeader; use clap::Parser; use clio::ClioPath; +use glob::Pattern; use headers::{ETag, HeaderMapExt, IfNoneMatch}; use opentelemetry_otlp::WithExportConfig; use require_bearer::RequireBearerLayer; @@ -35,7 +36,6 @@ use std::{ io::Write, net::{Ipv4Addr, SocketAddr, SocketAddrV4}, ops::Add, - path::{Path, PathBuf}, str::FromStr, sync::Arc, time::Duration, @@ -112,9 +112,9 @@ struct ServeArgs { /// The URL of the OpenTelemetry collector to send traces to #[arg(long, env = "BUNDLER_OTEL_COLLECTOR_URL")] otel_collector_url: Option, - /// Directory containing any static data files that should be included in the bundle - #[arg(long, env = "BUNDLER_STATIC_DATA_DIRECTORY")] - static_data_directory: Option, + /// Paths to any static data files that should be included in the bundle - can be globs + #[arg(long, env = "BUNDLER_STATIC_DATA")] + static_data: Vec, } /// Arguments to output the schema with @@ -141,7 +141,7 @@ async fn serve(args: ServeArgs) { setup_telemetry(args.log_level, args.otel_collector_url).unwrap(); let ispyb_pool = connect_ispyb(args.database_url).await.unwrap(); - let current_bundle = fetch_initial_bundle(args.static_data_directory.as_deref(), &ispyb_pool) + let current_bundle = fetch_initial_bundle(&args.static_data, &ispyb_pool) .await .unwrap(); let app = Router::new() @@ -159,10 +159,9 @@ async fn serve(args: ServeArgs) { ); let mut tasks = tokio::task::JoinSet::new(); - let static_dir = args.static_data_directory.clone(); tasks.spawn(update_bundle( current_bundle, - static_dir, + args.static_data, ispyb_pool, args.polling_interval.into(), )); @@ -243,7 +242,7 @@ async fn connect_ispyb(database_url: Url) -> Result { /// Fetches the intial [`Bundle`] from ISPyB and produces the correspoinding [`BundleFile`] #[instrument] async fn fetch_initial_bundle( - static_data: Option<&Path>, + static_data: &[Pattern], ispyb_pool: &MySqlPool, ) -> Result>>, anyhow::Error> { tracing::info!("Fetching initial bundle"); @@ -270,7 +269,7 @@ async fn serve_endpoints(port: u16, app: Router) { /// Periodically update the bundle with new data from ISPyB async fn update_bundle( current_bundle: impl AsRef>>, - static_data: Option, + static_data: Vec, ispyb_pool: MySqlPool, polling_interval: Duration, ) { @@ -280,7 +279,7 @@ async fn update_bundle( sleep_until(next_fetch).await; next_fetch = next_fetch.add(polling_interval); tracing::info!("Updating bundle"); - let bundle = Bundle::fetch(NoMetadata, static_data.as_deref(), &ispyb_pool) + let bundle = Bundle::fetch(NoMetadata, &static_data, &ispyb_pool) .await .unwrap(); let bundle_file = BundleFile::try_from(bundle).unwrap(); From ab5d7580039764ce6860ccff6e438c3d52e0fed0 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 3 Oct 2024 15:37:26 +0100 Subject: [PATCH 05/12] Update doc strings --- bundler/src/main.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bundler/src/main.rs b/bundler/src/main.rs index 0af2084b..dce7c190 100644 --- a/bundler/src/main.rs +++ b/bundler/src/main.rs @@ -80,7 +80,8 @@ where /// A thread safe, mutable, wrapper around the [`BundleFile`] type CurrentBundle = Arc>>; -/// Bundler acts as a Open Policy Agent bundle server, providing permissionable data from the ISPyB database +/// Bundler acts as an Open Policy Agent bundle server, providing permissionable data from the +/// ISPyB database and static data from local files #[derive(Debug, Parser)] #[command(author, version, about, long_about= None)] #[allow(clippy::large_enum_variant)] @@ -136,7 +137,7 @@ async fn main() { } } -/// Runs the service, pulling fresh bundles from ISPyB and serving them via the API +/// Runs the service, pulling fresh bundles from ISPyB/local files and serving them via the API async fn serve(args: ServeArgs) { setup_telemetry(args.log_level, args.otel_collector_url).unwrap(); @@ -239,7 +240,8 @@ async fn connect_ispyb(database_url: Url) -> Result { connection } -/// Fetches the intial [`Bundle`] from ISPyB and produces the correspoinding [`BundleFile`] +/// Fetches the initial [`Bundle`] from ISPyB and any static files, and produces the corresponding +/// [`BundleFile`] #[instrument] async fn fetch_initial_bundle( static_data: &[Pattern], @@ -266,7 +268,8 @@ async fn serve_endpoints(port: u16, app: Router) { axum::serve(listener, app).await.unwrap() } -/// Periodically update the bundle with new data from ISPyB +/// Periodically update the bundle with new data from ISPyB and any static files matching the given +/// glob patterns. async fn update_bundle( current_bundle: impl AsRef>>, static_data: Vec, From f7d27f3f52ce17f250744f471e522d21d92f3785 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 3 Oct 2024 15:38:39 +0100 Subject: [PATCH 06/12] Use thiserror instead of manually implementing error --- bundler/Cargo.lock | 9 +++++---- bundler/Cargo.toml | 1 + bundler/src/bundle.rs | 40 ++++++---------------------------------- 3 files changed, 12 insertions(+), 38 deletions(-) diff --git a/bundler/Cargo.lock b/bundler/Cargo.lock index ce41ae5e..7aee1a63 100644 --- a/bundler/Cargo.lock +++ b/bundler/Cargo.lock @@ -360,6 +360,7 @@ dependencies = [ "serde_json", "sqlx", "tar", + "thiserror", "tokio", "tower 0.5.1", "tower-http", @@ -2214,18 +2215,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.56" +version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d54378c645627613241d077a3a79db965db602882668f9136ac42af9ecb730ad" +checksum = "d50af8abc119fb8bb6dbabcfa89656f46f84aa0ac7688088608076ad2b459a84" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.56" +version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa0faa943b50f3db30a20aa7e265dbc66076993efed8463e8de414e5d06d3471" +checksum = "08904e7672f5eb876eaaf87e0ce17857500934f4981c4a0ab2b4aa98baac7fc3" dependencies = [ "proc-macro2", "quote", diff --git a/bundler/Cargo.toml b/bundler/Cargo.toml index c7709a6d..a44e0fae 100644 --- a/bundler/Cargo.toml +++ b/bundler/Cargo.toml @@ -28,6 +28,7 @@ sqlx = { version = "0.8.2", features = [ "mysql", ] } tar = { version = "0.4.42" } +thiserror = "1.0.64" tokio = { version = "1.40.0", features = ["macros", "rt-multi-thread"] } tower = { version = "0.5.1" } tower-http = { version = "0.6.1", features = ["trace"] } diff --git a/bundler/src/bundle.rs b/bundler/src/bundle.rs index dba231c7..aa2f1da5 100644 --- a/bundler/src/bundle.rs +++ b/bundler/src/bundle.rs @@ -6,7 +6,7 @@ use sqlx::MySqlPool; use std::{ collections::{hash_map::DefaultHasher, BTreeMap, HashMap}, ffi::OsStr, - fmt::{Debug, Display}, + fmt::Debug, hash::{Hash, Hasher}, }; use tar::Header; @@ -233,40 +233,12 @@ async fn static_data(patterns: &[Pattern]) -> Result>, s } /// Combination of possible errors when fetching data to create bundle -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum BundleDataError { /// Error fetching data from database - Sql(sqlx::Error), + #[error("Error reading dynamic data: {0}")] + Sql(#[from] sqlx::Error), /// Error fetching data from file - Static(std::io::Error), -} - -impl From for BundleDataError { - fn from(value: sqlx::Error) -> Self { - Self::Sql(value) - } -} - -impl From for BundleDataError { - fn from(value: std::io::Error) -> Self { - Self::Static(value) - } -} - -impl Display for BundleDataError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - BundleDataError::Sql(e) => write!(f, "Error reading dynamic data: {e}"), - BundleDataError::Static(e) => write!(f, "Error reading static data: {e}"), - } - } -} - -impl std::error::Error for BundleDataError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - BundleDataError::Sql(e) => Some(e), - BundleDataError::Static(e) => Some(e), - } - } + #[error("Error reading static data: {0}")] + Static(#[from] std::io::Error), } From 17b5efb11682c28a7c0131f1bf132e556eed8587 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Mon, 7 Oct 2024 11:36:41 +0100 Subject: [PATCH 07/12] Add beamlines to em_admin and epsic_admin groups These are the most common beamlines for visits of users with the respective admin permissions. --- static/admin.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/static/admin.json b/static/admin.json index e79e3bfe..cf048930 100644 --- a/static/admin.json +++ b/static/admin.json @@ -1,7 +1,6 @@ { - "em_admin": [], - "epsic_admin": [], - "fault_admin": [], + "em_admin": ["m01", "m02", "m03", "m04", "m05", "m06", "m07", "m08", "m09", "m10", "m11", "m12", "m13", "m14", "m15"], + "epsic_admin": ["e01", "e02", "e03"], "gen_admin": [], "pow_admin": [], "mx_admin": ["i02-1", "i02-2", "i03", "i04", "i04-1", "i23", "i24"], From f19b70465bb89992831aaa5727a3fefd833798a2 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 9 Oct 2024 11:13:08 +0100 Subject: [PATCH 08/12] Move static data to bundler helm charts --- {static => charts/bundler/static}/admin.json | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {static => charts/bundler/static}/admin.json (100%) diff --git a/static/admin.json b/charts/bundler/static/admin.json similarity index 100% rename from static/admin.json rename to charts/bundler/static/admin.json From 3735d05b2d0d7b03e2123167b997f29336b08a1e Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 9 Oct 2024 12:16:02 +0100 Subject: [PATCH 09/12] Store bundler static data globs as strings They are used as strings so storing them as strings makes more sense. It also makes the tracing sane again as debug format for patterns is incredibly verbose. Strings are still validated by the CLI. --- bundler/src/bundle.rs | 10 +++++----- bundler/src/main.rs | 11 ++++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/bundler/src/bundle.rs b/bundler/src/bundle.rs index aa2f1da5..506e9166 100644 --- a/bundler/src/bundle.rs +++ b/bundler/src/bundle.rs @@ -1,5 +1,4 @@ use flate2::{write::GzEncoder, Compression}; -use glob::Pattern; use schemars::{schema::RootSchema, schema_for, JsonSchema}; use serde::Serialize; use sqlx::MySqlPool; @@ -126,7 +125,7 @@ where #[instrument(name = "fetch_bundle")] pub async fn fetch( metadata: Metadata, - static_data_directory: &[Pattern], + static_data: &[String], ispyb_pool: &MySqlPool, ) -> Result { let (subjects, sessions, proposals, beamlines) = try_join!( @@ -135,7 +134,7 @@ where Proposals::fetch(ispyb_pool), Beamlines::fetch(ispyb_pool), )?; - let static_data = static_data(static_data_directory).await?; + let static_data = read_static_data(static_data).await?; Ok(Self::new( metadata, subjects, @@ -215,11 +214,12 @@ where } /// Read static data from files that should be included in the compiled bundle -async fn static_data(patterns: &[Pattern]) -> Result>, std::io::Error> { +async fn read_static_data(patterns: &[String]) -> Result>, std::io::Error> { let mut data = HashMap::new(); for pattern in patterns { - for file in glob::glob(pattern.as_str()).expect("Pattern was validated by CLI") { + for file in glob::glob(pattern).expect("Pattern was validated by CLI") { let file = file.map_err(|e| e.into_error())?; + trace!(glob = pattern.as_str(), file = ?file, "Reading static data from {file:?}"); let name = file.file_stem(); let Some(name) = name.and_then(OsStr::to_str) else { // Save having to think about non-utf8 in OPA rules diff --git a/bundler/src/main.rs b/bundler/src/main.rs index dce7c190..24f11475 100644 --- a/bundler/src/main.rs +++ b/bundler/src/main.rs @@ -23,7 +23,7 @@ use axum::{ use axum_extra::TypedHeader; use clap::Parser; use clio::ClioPath; -use glob::Pattern; +use glob::{Pattern, PatternError}; use headers::{ETag, HeaderMapExt, IfNoneMatch}; use opentelemetry_otlp::WithExportConfig; use require_bearer::RequireBearerLayer; @@ -114,8 +114,9 @@ struct ServeArgs { #[arg(long, env = "BUNDLER_OTEL_COLLECTOR_URL")] otel_collector_url: Option, /// Paths to any static data files that should be included in the bundle - can be globs - #[arg(long, env = "BUNDLER_STATIC_DATA")] - static_data: Vec, + // validate that string can be converted to a glob pattern but store it in its string form + #[arg(long, env = "BUNDLER_STATIC_DATA", value_parser = |raw: &'_ str| Pattern::new(raw).map(|_| raw.to_string()))] + static_data: Vec, } /// Arguments to output the schema with @@ -244,7 +245,7 @@ async fn connect_ispyb(database_url: Url) -> Result { /// [`BundleFile`] #[instrument] async fn fetch_initial_bundle( - static_data: &[Pattern], + static_data: &[String], ispyb_pool: &MySqlPool, ) -> Result>>, anyhow::Error> { tracing::info!("Fetching initial bundle"); @@ -272,7 +273,7 @@ async fn serve_endpoints(port: u16, app: Router) { /// glob patterns. async fn update_bundle( current_bundle: impl AsRef>>, - static_data: Vec, + static_data: Vec, ispyb_pool: MySqlPool, polling_interval: Duration, ) { From c21d075b1844f9a8eedd73ae786d6b3dba881009 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 9 Oct 2024 16:00:03 +0100 Subject: [PATCH 10/12] Add StaticDataGlob newtype to wrap glob validation --- bundler/src/bundle.rs | 17 +++++++++++------ bundler/src/main.rs | 28 +++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/bundler/src/bundle.rs b/bundler/src/bundle.rs index 506e9166..bb696a21 100644 --- a/bundler/src/bundle.rs +++ b/bundler/src/bundle.rs @@ -12,8 +12,11 @@ use tar::Header; use tokio::try_join; use tracing::{instrument, trace}; -use crate::permissionables::{ - beamlines::Beamlines, proposals::Proposals, sessions::Sessions, subjects::Subjects, +use crate::{ + permissionables::{ + beamlines::Beamlines, proposals::Proposals, sessions::Sessions, subjects::Subjects, + }, + StaticDataGlob, }; /// A compiled Web Assembly module @@ -125,7 +128,7 @@ where #[instrument(name = "fetch_bundle")] pub async fn fetch( metadata: Metadata, - static_data: &[String], + static_data: &[StaticDataGlob], ispyb_pool: &MySqlPool, ) -> Result { let (subjects, sessions, proposals, beamlines) = try_join!( @@ -214,12 +217,14 @@ where } /// Read static data from files that should be included in the compiled bundle -async fn read_static_data(patterns: &[String]) -> Result>, std::io::Error> { +async fn read_static_data( + patterns: &[StaticDataGlob], +) -> Result>, std::io::Error> { let mut data = HashMap::new(); for pattern in patterns { - for file in glob::glob(pattern).expect("Pattern was validated by CLI") { + for file in glob::glob(pattern.as_ref()).expect("Pattern was validated by CLI") { let file = file.map_err(|e| e.into_error())?; - trace!(glob = pattern.as_str(), file = ?file, "Reading static data from {file:?}"); + trace!(glob = pattern.as_ref(), file = ?file, "Reading static data from {file:?}"); let name = file.file_stem(); let Some(name) = name.and_then(OsStr::to_str) else { // Save having to think about non-utf8 in OPA rules diff --git a/bundler/src/main.rs b/bundler/src/main.rs index 24f11475..e51ac756 100644 --- a/bundler/src/main.rs +++ b/bundler/src/main.rs @@ -77,6 +77,25 @@ where } } +/// Wrapper to ensure that globs passed via the CLI are valid file globs +#[derive(Debug, Clone)] +struct StaticDataGlob(String); + +impl FromStr for StaticDataGlob { + type Err = PatternError; + + fn from_str(value: &str) -> Result { + _ = Pattern::new(value)?; + Ok(Self(value.into())) + } +} + +impl AsRef for StaticDataGlob { + fn as_ref(&self) -> &str { + &self.0 + } +} + /// A thread safe, mutable, wrapper around the [`BundleFile`] type CurrentBundle = Arc>>; @@ -114,9 +133,8 @@ struct ServeArgs { #[arg(long, env = "BUNDLER_OTEL_COLLECTOR_URL")] otel_collector_url: Option, /// Paths to any static data files that should be included in the bundle - can be globs - // validate that string can be converted to a glob pattern but store it in its string form - #[arg(long, env = "BUNDLER_STATIC_DATA", value_parser = |raw: &'_ str| Pattern::new(raw).map(|_| raw.to_string()))] - static_data: Vec, + #[arg(long, env = "BUNDLER_STATIC_DATA")] + static_data: Vec, } /// Arguments to output the schema with @@ -245,7 +263,7 @@ async fn connect_ispyb(database_url: Url) -> Result { /// [`BundleFile`] #[instrument] async fn fetch_initial_bundle( - static_data: &[String], + static_data: &[StaticDataGlob], ispyb_pool: &MySqlPool, ) -> Result>>, anyhow::Error> { tracing::info!("Fetching initial bundle"); @@ -273,7 +291,7 @@ async fn serve_endpoints(port: u16, app: Router) { /// glob patterns. async fn update_bundle( current_bundle: impl AsRef>>, - static_data: Vec, + static_data: Vec, ispyb_pool: MySqlPool, polling_interval: Duration, ) { From ec4a4443229b7f1a3c8f6d4d15e884488fade830 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 9 Oct 2024 16:11:55 +0100 Subject: [PATCH 11/12] Remove static bundler data This can be added at a later time and is not required for static data support. --- charts/bundler/static/admin.json | 35 -------------------------------- 1 file changed, 35 deletions(-) delete mode 100644 charts/bundler/static/admin.json diff --git a/charts/bundler/static/admin.json b/charts/bundler/static/admin.json deleted file mode 100644 index cf048930..00000000 --- a/charts/bundler/static/admin.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "em_admin": ["m01", "m02", "m03", "m04", "m05", "m06", "m07", "m08", "m09", "m10", "m11", "m12", "m13", "m14", "m15"], - "epsic_admin": ["e01", "e02", "e03"], - "gen_admin": [], - "pow_admin": [], - "mx_admin": ["i02-1", "i02-2", "i03", "i04", "i04-1", "i23", "i24"], - "saxs_admin": ["b21", "i22", "p38"], - "sm_admin": [], - "tomo_admin": [], - "xpdf_admin": ["i15", "i15-1"], - - "b07_admin": ["b07"], - "b16_admin": ["b16"], - "b18_admin": ["b18"], - "b22_admin": ["b22"], - "b23_admin": ["b23"], - "b24_admin": ["b24"], - "i05_admin": ["i05"], - "i06_admin": ["i06"], - "i07_admin": ["i07"], - "i08_admin": ["i08"], - "i09_admin": ["i09"], - "i10_admin": ["i10"], - "i11_admin": ["i11"], - "i12_admin": ["i12"], - "i13_admin": ["i13"], - "i14_admin": ["i14"], - "i16_admin": ["i16"], - "i18_admin": ["i18"], - "i20_admin": ["i20"], - "i21_admin": ["i21"], - "k11_admin": ["k11"], - "p45_admin": ["p45"], - "p99_admin": ["p99"] -} From 6d16519bdfd3a3a2216deec0c147d65ff1719ccb Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 9 Oct 2024 16:21:28 +0100 Subject: [PATCH 12/12] Use derive_more instead of manual AsRef --- bundler/Cargo.toml | 2 +- bundler/src/main.rs | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/bundler/Cargo.toml b/bundler/Cargo.toml index a44e0fae..b13af731 100644 --- a/bundler/Cargo.toml +++ b/bundler/Cargo.toml @@ -9,7 +9,7 @@ axum = { version = "0.7.7" } axum-extra = { version = "0.9.4", features = ["typed-header"] } clap = { version = "4.5.18", features = ["derive", "env"] } clio = { version = "0.3.5", features = ["clap-parse"] } -derive_more = { version = "1.0.0", features = ["deref", "deref_mut"] } +derive_more = { version = "1.0.0", features = ["deref", "deref_mut", "as_ref"] } dotenvy = { version = "0.15.7" } flate2 = { version = "1.0.34" } glob = "0.3.1" diff --git a/bundler/src/main.rs b/bundler/src/main.rs index e51ac756..2c81327f 100644 --- a/bundler/src/main.rs +++ b/bundler/src/main.rs @@ -78,7 +78,7 @@ where } /// Wrapper to ensure that globs passed via the CLI are valid file globs -#[derive(Debug, Clone)] +#[derive(Debug, Clone, derive_more::AsRef)] struct StaticDataGlob(String); impl FromStr for StaticDataGlob { @@ -90,12 +90,6 @@ impl FromStr for StaticDataGlob { } } -impl AsRef for StaticDataGlob { - fn as_ref(&self) -> &str { - &self.0 - } -} - /// A thread safe, mutable, wrapper around the [`BundleFile`] type CurrentBundle = Arc>>;