From ab010ce4f3a0628d4699f558bd463e657a8f1a97 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 21 Jan 2024 01:04:13 +0100 Subject: [PATCH] feat: improve handling of pinned versions for bun --- backend/Cargo.lock | 1 + backend/parsers/windmill-parser-ts/src/lib.rs | 22 ++++++++++++ backend/windmill-api/Cargo.toml | 1 + backend/windmill-api/src/scripts.rs | 27 +++++++++++++- backend/windmill-worker/loader.bun.ts | 4 +-- backend/windmill-worker/src/bun_executor.rs | 35 +++++++------------ 6 files changed, 64 insertions(+), 26 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 1907218cdb..c4db4be430 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -9645,6 +9645,7 @@ dependencies = [ "windmill-git-sync", "windmill-parser", "windmill-parser-py-imports", + "windmill-parser-ts", "windmill-queue", ] diff --git a/backend/parsers/windmill-parser-ts/src/lib.rs b/backend/parsers/windmill-parser-ts/src/lib.rs index de4ff80095..fb4a3323c8 100644 --- a/backend/parsers/windmill-parser-ts/src/lib.rs +++ b/backend/parsers/windmill-parser-ts/src/lib.rs @@ -286,6 +286,28 @@ fn binding_ident_to_arg(BindingIdent { id, type_ann }: &BindingIdent) -> (String lazy_static::lazy_static! { static ref RE_SNK_CASE: Regex = Regex::new(r"_(\d)").unwrap(); + static ref IMPORTS_VERSION: Regex = Regex::new(r"^((?:\@[^\/\@]+\/[^\/\@]+)|(?:[^\/\@]+))(?:\@(?:[^\/]+))?(.*)$").unwrap(); + +} + +pub fn remove_pinned_imports(code: &str) -> anyhow::Result { + let imports = parse_expr_for_imports(code)?; + let mut content = code.to_string(); + for import in imports { + let to_c = IMPORTS_VERSION.captures(&import); + if let Some(to) = to_c.and_then(|x| { + x.get(1).map(|y| { + format!( + "{}{}", + y.as_str(), + x.get(2).map(|z| z.as_str()).unwrap_or("") + ) + }) + }) { + content = content.replace(&import, &to); + } + } + Ok(content) } fn to_snake_case(s: &str) -> String { diff --git a/backend/windmill-api/Cargo.toml b/backend/windmill-api/Cargo.toml index cd2e6c49f9..0bd21d2a03 100644 --- a/backend/windmill-api/Cargo.toml +++ b/backend/windmill-api/Cargo.toml @@ -28,6 +28,7 @@ windmill-common = { workspace = true, features = [ windmill-audit.workspace = true windmill-parser.workspace = true windmill-parser-py-imports.workspace = true +windmill-parser-ts.workspace = true windmill-git-sync.workspace = true tokio.workspace = true anyhow.workspace = true diff --git a/backend/windmill-api/src/scripts.rs b/backend/windmill-api/src/scripts.rs index e53d635091..a69b088fd5 100644 --- a/backend/windmill-api/src/scripts.rs +++ b/backend/windmill-api/src/scripts.rs @@ -47,6 +47,7 @@ use windmill_common::{ }, }; use windmill_git_sync::{handle_deployment_metadata, DeployedObject}; +use windmill_parser_ts::remove_pinned_imports; use windmill_queue::{self, schedule::push_scheduled_job, PushIsolationLevel, QueueTransaction}; const MAX_HASH_HISTORY_LENGTH_STORED: usize = 20; @@ -113,6 +114,7 @@ pub fn workspaced_service() -> Router { .route("/get/draft/*path", get(get_script_by_path_w_draft)) .route("/get/p/*path", get(get_script_by_path)) .route("/raw/p/*path", get(raw_script_by_path)) + .route("/raw_unpinned/p/*path", get(raw_script_by_path_unpinned)) .route("/exists/p/*path", get(exists_script_by_path)) .route("/archive/h/:hash", post(archive_script_by_hash)) .route("/delete/h/:hash", post(delete_script_by_hash)) @@ -870,6 +872,24 @@ async fn raw_script_by_path( authed: ApiAuthed, Extension(user_db): Extension, Path((w_id, path)): Path<(String, StripPath)>, +) -> Result { + raw_script_by_path_internal(path, user_db, authed, w_id, false).await +} + +async fn raw_script_by_path_unpinned( + authed: ApiAuthed, + Extension(user_db): Extension, + Path((w_id, path)): Path<(String, StripPath)>, +) -> Result { + raw_script_by_path_internal(path, user_db, authed, w_id, true).await +} + +async fn raw_script_by_path_internal( + path: StripPath, + user_db: UserDB, + authed: ApiAuthed, + w_id: String, + unpin: bool, ) -> Result { let path = path.to_path(); if !path.ends_with(".py") @@ -904,7 +924,12 @@ async fn raw_script_by_path( tx.commit().await?; let content = not_found_if_none(content_o, "Script", path)?; - Ok(content) + + if unpin { + return Ok(remove_pinned_imports(&content)?); + } else { + return Ok(content); + } } async fn exists_script_by_path( diff --git a/backend/windmill-worker/loader.bun.ts b/backend/windmill-worker/loader.bun.ts index 3419fed0f0..f4c143dab7 100644 --- a/backend/windmill-worker/loader.bun.ts +++ b/backend/windmill-worker/loader.bun.ts @@ -38,8 +38,8 @@ const p = { const isRelative = !args.path.startsWith("/"); const url = isRelative - ? `${base_internal_url}/api/w/${w_id}/scripts/raw/p/${file_path}/../${args.path}` - : `${base_internal_url}/api/w/${w_id}/scripts/raw/p/${args.path}`; + ? `${base_internal_url}/api/w/${w_id}/scripts/RAW_GET_ENDPOINT/p/${file_path}/../${args.path}` + : `${base_internal_url}/api/w/${w_id}/scripts/RAW_GET_ENDPOINT/p/${args.path}`; const file = isRelative ? resolve("./" + current_path + "/../" + args.path + ".url") : resolve("./" + args.path + ".url"); diff --git a/backend/windmill-worker/src/bun_executor.rs b/backend/windmill-worker/src/bun_executor.rs index 9055a02d0f..3d4a767c26 100644 --- a/backend/windmill-worker/src/bun_executor.rs +++ b/backend/windmill-worker/src/bun_executor.rs @@ -5,6 +5,7 @@ use itertools::Itertools; use regex::Regex; use serde_json::value::RawValue; use uuid::Uuid; +use windmill_parser_ts::remove_pinned_imports; use windmill_queue::CanceledBy; #[cfg(feature = "enterprise")] @@ -19,10 +20,7 @@ use crate::{ DISABLE_NUSER, HOME_ENV, NODE_PATH, NPM_CONFIG_REGISTRY, NSJAIL_PATH, PATH_ENV, TZ_ENV, }; -use tokio::{ - fs::{remove_dir_all, File}, - process::Command, -}; +use tokio::{fs::File, process::Command}; use tokio::io::AsyncReadExt; @@ -81,6 +79,7 @@ pub async fn gen_lockfile( .replace("BASE_INTERNAL_URL", base_internal_url) .replace("TOKEN", token) .replace("CURRENT_PATH", script_path) + .replace("RAW_GET_ENDPOINT", "raw") ), ) .await?; @@ -307,9 +306,6 @@ pub async fn handle_bun_job( )); } - let has_custom_config_registry = - NPM_CONFIG_REGISTRY.read().await.is_some() || BUNFIG_INSTALL_SCOPES.read().await.is_some(); - if let Some(reqs) = requirements_o { let splitted = reqs.split(BUN_LOCKB_SPLIT).collect::>(); if splitted.len() != 2 { @@ -347,14 +343,10 @@ pub async fn handle_bun_job( common_bun_proc_envs.clone(), ) .await?; - if !has_trusted_deps && !has_custom_config_registry && !nodejs_mode { - remove_dir_all(format!("{}/node_modules", job_dir)).await?; - } } } else { // TODO: remove once bun implement a reasonable set of trusted deps let trusted_deps = get_trusted_deps(inner_content); - let empty_trusted_deps = trusted_deps.len() == 0; // if !*DISABLE_NSJAIL || !empty_trusted_deps || has_custom_config_registry { logs.push_str("\n\n--- BUN INSTALL ---\n"); @@ -376,16 +368,11 @@ pub async fn handle_bun_job( ) .await?; // } - - if empty_trusted_deps && !has_custom_config_registry && !nodejs_mode { - let node_modules_path = format!("{}/node_modules", job_dir); - let node_modules_exists = tokio::fs::metadata(&node_modules_path).await.is_ok(); - if node_modules_exists { - remove_dir_all(&node_modules_path).await?; - } - } } + let main_code = remove_pinned_imports(inner_content)?; + let _ = write_file(job_dir, "main.ts", &main_code).await?; + if nodejs_mode { logs.push_str("\n\n--- NODE CODE EXECUTION ---\n"); } else { @@ -469,7 +456,8 @@ run().catch(async (e) => {{ .replace("W_ID", &job.workspace_id) .replace("BASE_INTERNAL_URL", base_internal_url) .replace("TOKEN", &client.get_token().await) - .replace("CURRENT_PATH", job.script_path()); + .replace("CURRENT_PATH", job.script_path()) + .replace("RAW_GET_ENDPOINT", "raw_unpinned"); let write_loader_f = async move { if nodejs_mode { write_file( @@ -775,9 +763,6 @@ pub async fn start_worker( common_bun_proc_envs.clone(), ) .await?; - if !has_trusted_deps { - remove_dir_all(format!("{}/node_modules", job_dir)).await?; - } tracing::info!("dedicated worker requirements installed: {reqs}"); } } else if !*DISABLE_NSJAIL { @@ -801,6 +786,9 @@ pub async fn start_worker( .await?; } + let main_code = remove_pinned_imports(inner_content)?; + let _ = write_file(job_dir, "main.ts", &main_code).await?; + { // let mut start = Instant::now(); let args = windmill_parser_ts::parse_deno_signature(inner_content, true)?.args; @@ -875,6 +863,7 @@ plugin(p) .replace("BASE_INTERNAL_URL", base_internal_url) .replace("TOKEN", token) .replace("CURRENT_PATH", script_path) + .replace("RAW_GET_ENDPOINT", "raw_unpinned") ), ) .await?;