From 1d88cede1bc811f8ba60bced466051137a365abf Mon Sep 17 00:00:00 2001 From: James Hodgkinson Date: Tue, 5 Sep 2023 11:50:51 +1000 Subject: [PATCH] Yak hassling (#2059) * trying this query thing again * if error show error not panic * clippyism * moving dependencies around and fixing log messages for healthcheck * cleaning up some comment mess * fixing the "debug thing breaks packaging" issue and test failures --- .gitignore | 2 + server/core/src/https/v1.rs | 15 +++-- server/daemon/src/main.rs | 4 +- server/lib/Cargo.toml | 4 +- server/lib/src/be/idl_arc_sqlite.rs | 2 +- server/lib/src/be/idl_sqlite.rs | 96 +++++++++++++++++++---------- server/lib/src/be/mod.rs | 2 +- server/lib/src/server/mod.rs | 4 +- server/web_ui/build_wasm.sh | 10 +-- server/web_ui/src/views/mod.rs | 1 + 10 files changed, 90 insertions(+), 50 deletions(-) diff --git a/.gitignore b/.gitignore index 965960715..28a59475b 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,8 @@ vendor.tar.* tools/orca/example_profiles/small/orca-edited.toml /docs/ .vscode/ +# webui things we don't need +*.d.ts # kanidm simple packaging deployment-config/ diff --git a/server/core/src/https/v1.rs b/server/core/src/https/v1.rs index ad1e0f6f6..bbbe3b2e9 100644 --- a/server/core/src/https/v1.rs +++ b/server/core/src/https/v1.rs @@ -1425,7 +1425,6 @@ pub async fn auth_valid( to_axum_response(res) } -#[cfg(debug_assertions)] pub async fn debug_ipinfo( State(_state): State, TrustedClientIp(ip_addr): TrustedClientIp, @@ -1435,7 +1434,7 @@ pub async fn debug_ipinfo( #[instrument(skip(state))] pub fn router(state: ServerState) -> Router { - let mut router = Router::new() + let router = Router::new() .route("/v1/oauth2", get(super::oauth2::oauth2_get)) .route("/v1/oauth2/_basic", post(super::oauth2::oauth2_basic_post)) .route( @@ -1737,8 +1736,14 @@ pub fn router(state: ServerState) -> Router { ) .with_state(state) .layer(from_fn(dont_cache_me)); - if cfg!(debug_assertions) { - router = router.route("/v1/debug/ipinfo", get(debug_ipinfo)); + if cfg!(debug_assertions) || cfg!(test) { + add_debug_info(router) + } else { + router } - router +} + +// #[cfg(any(debug_assertions, test))] +fn add_debug_info(router: Router) -> Router { + router.route("/v1/debug/ipinfo", get(debug_ipinfo)) } diff --git a/server/daemon/src/main.rs b/server/daemon/src/main.rs index cc1ef9fa7..e33cf6ccd 100644 --- a/server/daemon/src/main.rs +++ b/server/daemon/src/main.rs @@ -638,7 +638,7 @@ async fn main() -> ExitCode { client = match &sconfig.tls_chain { None => client, Some(ca_cert) => { - debug!("Trying to load {}", ca_cert); + debug!("Trying to load {} to build a CA cert path", ca_cert); // if the ca_cert file exists, then we'll use it let ca_cert_path = PathBuf::from(ca_cert); match ca_cert_path.exists() { @@ -666,7 +666,7 @@ async fn main() -> ExitCode { let ca_cert_parsed = match reqwest::Certificate::from_pem(content.as_bytes()) { Ok(val) => val, Err(e) =>{ - error!("Failed to parse {} as a valid certificate!\nError: {:?}", ca_cert, e); + error!("Failed to parse {} into CA certificate!\nError: {:?}", ca_cert, e); return ExitCode::FAILURE } }; diff --git a/server/lib/Cargo.toml b/server/lib/Cargo.toml index f811cda0a..44501721c 100644 --- a/server/lib/Cargo.toml +++ b/server/lib/Cargo.toml @@ -50,6 +50,8 @@ regex = { workspace = true, features = [ "unicode", "unicode-gencat", ] } +rusqlite = { workspace = true, features = ["array", "bundled"] } + serde = { workspace = true, features = ["derive"] } serde_cbor = { workspace = true } serde_json = { workspace = true } @@ -80,11 +82,9 @@ serde_with = { workspace = true } # because windows really can't build without the bundled one [target.'cfg(target_family = "windows")'.dependencies] -rusqlite = { workspace = true, features = ["bundled"] } whoami = { workspace = true } [target.'cfg(not(target_family = "windows"))'.dependencies] -rusqlite = { workspace = true } kanidm_utils_users = { workspace = true } [features] diff --git a/server/lib/src/be/idl_arc_sqlite.rs b/server/lib/src/be/idl_arc_sqlite.rs index a40b0eb76..abea72a15 100644 --- a/server/lib/src/be/idl_arc_sqlite.rs +++ b/server/lib/src/be/idl_arc_sqlite.rs @@ -565,7 +565,7 @@ impl<'a> IdlArcSqliteTransaction for IdlArcSqliteWriteTransaction<'a> { } impl<'a> IdlArcSqliteWriteTransaction<'a> { - #[cfg(debug_assertions)] + #[cfg(any(test, debug_assertions))] #[instrument(level = "debug", name = "idl_arc_sqlite::clear_cache", skip_all)] pub fn clear_cache(&mut self) -> Result<(), OperationError> { // I'm not sure rn if I want to reload these? If we reload these we kind of diff --git a/server/lib/src/be/idl_sqlite.rs b/server/lib/src/be/idl_sqlite.rs index 9415b7a52..431bca116 100644 --- a/server/lib/src/be/idl_sqlite.rs +++ b/server/lib/src/be/idl_sqlite.rs @@ -8,6 +8,7 @@ use std::time::Duration; use hashbrown::HashMap; use idlset::v2::IDLBitRange; use kanidm_proto::v1::{ConsistencyError, OperationError}; +use rusqlite::vtab::array::Array; use rusqlite::{Connection, OpenFlags, OptionalExtension}; use uuid::Uuid; @@ -160,44 +161,53 @@ pub trait IdlSqliteTransaction { let mut stmt = self .get_conn()? .prepare(&format!( - "SELECT id, data FROM {}.id2entry WHERE id = :idl", + "SELECT id, data FROM {}.id2entry + WHERE id IN rarray(:idli)", self.get_db_name() )) .map_err(sqlite_error)?; - // TODO #258: Can this actually just load in a single select? - // TODO #258: I have no idea how to make this an iterator chain ... so what - // I have now is probably really bad :( - let mut results = Vec::new(); - - // Faster to iterate over the compressed form believe it or not. - /* - let decompressed: Result, _> = idli.into_iter() - .map(|u| i64::try_from(u).map_err(|_| OperationError::InvalidEntryId)) - .collect(); - */ - + // turn them into i64's + let mut id_list: Vec = vec![]; for id in idli { - let iid = i64::try_from(id).map_err(|_| OperationError::InvalidEntryId)?; - let id2entry_iter = stmt - .query_map([&iid], |row| { - Ok(IdSqliteEntry { - id: row.get(0)?, - data: row.get(1)?, - }) - }) - .map_err(sqlite_error)?; + id_list.push(i64::try_from(id).map_err(|_| OperationError::InvalidEntryId)?); + } + // turn them into rusqlite values + let id_list: Array = std::rc::Rc::new( + id_list + .into_iter() + .map(rusqlite::types::Value::from) + .collect::>(), + ); - let r: Result, _> = id2entry_iter - .map(|v| { - v.map_err(sqlite_error).and_then(|ise| { - // Convert the idsqlite to id raw - ise.try_into() - }) - }) - .collect(); - let mut r = r?; - results.append(&mut r); + let mut results: Vec = vec![]; + + let rows = stmt.query_map(named_params! {":idli": &id_list}, |row| { + Ok(IdSqliteEntry { + id: row.get(0)?, + data: row.get(1)?, + }) + }); + let rows = match rows { + Ok(rows) => rows, + Err(e) => { + error!("query failed in get_identry_raw: {:?}", e); + return Err(OperationError::SqliteError); + } + }; + + for row in rows { + match row { + Ok(ise) => { + // Convert the idsqlite to id raw + results.push(ise.try_into()?); + } + // TODO: make this a better error + Err(e) => { + admin_error!(?e, "SQLite Error in get_identry_raw"); + return Err(OperationError::SqliteError); + } + } } Ok(results) } @@ -1695,7 +1705,27 @@ impl IdlSqlite { let pool = (0..cfg.pool_size) .map(|i| { trace!("Opening Connection {}", i); - Connection::open_with_flags(cfg.path.as_str(), flags).map_err(sqlite_error) + let conn = + Connection::open_with_flags(cfg.path.as_str(), flags).map_err(sqlite_error); + match conn { + Ok(conn) => { + // load the rusqlite vtab module to allow for virtual tables + rusqlite::vtab::array::load_module(&conn).map_err(|e| { + admin_error!( + "Failed to load rarray virtual module for sqlite, cannot start! {:?}", e + ); + sqlite_error(e) + })?; + Ok(conn) + } + Err(err) => { + admin_error!( + "Failed to start database connection, cannot start! {:?}", + err + ); + Err(err) + } + } }) .collect::, OperationError>>() .map_err(|e| { diff --git a/server/lib/src/be/mod.rs b/server/lib/src/be/mod.rs index 38232156f..8cc93fa0a 100644 --- a/server/lib/src/be/mod.rs +++ b/server/lib/src/be/mod.rs @@ -1792,7 +1792,7 @@ impl<'a> BackendWriteTransaction<'a> { self.set_db_index_version(0) } - #[cfg(debug_assertions)] + #[cfg(any(test, debug_assertions))] pub fn clear_cache(&mut self) -> Result<(), OperationError> { self.get_idlayer().clear_cache() } diff --git a/server/lib/src/server/mod.rs b/server/lib/src/server/mod.rs index 8038f95c6..0dfffcabd 100644 --- a/server/lib/src/server/mod.rs +++ b/server/lib/src/server/mod.rs @@ -1220,7 +1220,7 @@ impl QueryServer { } } - #[cfg(debug_assertions)] + #[cfg(any(test, debug_assertions))] pub async fn clear_cache(&self) -> Result<(), OperationError> { let ct = duration_from_epoch_now(); let mut w_txn = self.write(ct).await; @@ -1608,7 +1608,7 @@ impl<'a> QueryServerWriteTransaction<'a> { Ok(()) } - #[cfg(debug_assertions)] + #[cfg(any(test, debug_assertions))] #[instrument(level = "debug", skip_all)] pub fn clear_cache(&mut self) -> Result<(), OperationError> { self.be_txn.clear_cache() diff --git a/server/web_ui/build_wasm.sh b/server/web_ui/build_wasm.sh index 5683f6b6e..0e2c75d3a 100755 --- a/server/web_ui/build_wasm.sh +++ b/server/web_ui/build_wasm.sh @@ -29,7 +29,7 @@ fi # we can disable this since we want it to expand # shellcheck disable=SC2086 -wasm-pack build ${BUILD_FLAGS} --target web --mode no-install --no-pack || exit 1 +wasm-pack build ${BUILD_FLAGS} --target web --mode no-install --no-pack touch ./pkg/ANYTHING_HERE_WILL_BE_DELETED_ADD_TO_SRC && \ rsync --delete-after -r --copy-links -v ./static/* ./pkg/ && \ @@ -37,6 +37,8 @@ touch ./pkg/ANYTHING_HERE_WILL_BE_DELETED_ADD_TO_SRC && \ cp ../../LICENSE.md ./pkg/ rm ./pkg/.gitignore -# updates the brotli-compressed files -echo "brotli-compressing the WASM file..." -find ./pkg -name '*.wasm' -exec ./find_best_brotli.sh "{}" \; || exit 1 +if [ -z "${SKIP_BROTLI}" ]; then + # updates the brotli-compressed files + echo "brotli-compressing the WASM file..." + find ./pkg -name '*.wasm' -exec ./find_best_brotli.sh "{}" \; || exit 1 +fi \ No newline at end of file diff --git a/server/web_ui/src/views/mod.rs b/server/web_ui/src/views/mod.rs index f8012e019..95bae14f0 100644 --- a/server/web_ui/src/views/mod.rs +++ b/server/web_ui/src/views/mod.rs @@ -1,3 +1,4 @@ +#![allow(non_camel_case_types)] use gloo::console; use kanidm_proto::v1::{UiHint, UserAuthToken}; use serde::{Deserialize, Serialize};