From 171d7b0e946f498200d64ea3e168ea94f7214f18 Mon Sep 17 00:00:00 2001 From: tknickman Date: Fri, 6 Sep 2024 15:41:43 -0400 Subject: [PATCH 1/2] feat(turbo): add platform env support --- Cargo.lock | 2 + crates/turborepo-env/Cargo.toml | 2 + crates/turborepo-env/src/lib.rs | 2 + crates/turborepo-env/src/platform.rs | 178 ++++++++++++++++++ crates/turborepo-lib/src/run/cache.rs | 4 + .../turborepo-lib/src/task_graph/visitor.rs | 59 +++++- 6 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 crates/turborepo-env/src/platform.rs diff --git a/Cargo.lock b/Cargo.lock index c4dbcc163caa5..b0731cdba1357 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6218,6 +6218,8 @@ dependencies = [ "sha2", "test-case", "thiserror", + "turborepo-ci", + "turborepo-ui", ] [[package]] diff --git a/crates/turborepo-env/Cargo.toml b/crates/turborepo-env/Cargo.toml index 507d716b1fd06..4751cb2141f7b 100644 --- a/crates/turborepo-env/Cargo.toml +++ b/crates/turborepo-env/Cargo.toml @@ -15,6 +15,8 @@ regex = { workspace = true } serde = { workspace = true } sha2 = { workspace = true } thiserror = { workspace = true } +turborepo-ci = { workspace = true } +turborepo-ui = { workspace = true } [dev-dependencies] test-case = { workspace = true } diff --git a/crates/turborepo-env/src/lib.rs b/crates/turborepo-env/src/lib.rs index 399f0b5261565..938aff039f252 100644 --- a/crates/turborepo-env/src/lib.rs +++ b/crates/turborepo-env/src/lib.rs @@ -11,6 +11,8 @@ use serde::Serialize; use sha2::{Digest, Sha256}; use thiserror::Error; +pub mod platform; + const DEFAULT_ENV_VARS: [&str; 1] = ["VERCEL_ANALYTICS_ID"]; #[derive(Clone, Debug, Error)] diff --git a/crates/turborepo-env/src/platform.rs b/crates/turborepo-env/src/platform.rs new file mode 100644 index 0000000000000..1b22c79ff2821 --- /dev/null +++ b/crates/turborepo-env/src/platform.rs @@ -0,0 +1,178 @@ +use turborepo_ci::Vendor; +use turborepo_ui::{cprint, cprintln, ColorConfig, BOLD, GREY, YELLOW}; + +use crate::EnvironmentVariableMap; + +pub struct PlatformEnv { + env_keys: Vec, +} + +const TURBO_PLATFORM_ENV_KEY: &str = "TURBO_PLATFORM_ENV"; +const TURBO_PLATFORM_ENV_DISABLED_KEY: &str = "TURBO_PLATFORM_ENV_DISABLED"; + +impl PlatformEnv { + pub fn new() -> Self { + let env_keys = std::env::var(TURBO_PLATFORM_ENV_KEY) + .unwrap_or_default() + .split(',') + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()) + .collect(); + + Self { env_keys } + } + + pub fn disabled() -> bool { + let turbo_platform_env_disabled = + std::env::var(TURBO_PLATFORM_ENV_DISABLED_KEY).unwrap_or_default(); + turbo_platform_env_disabled == "1" || turbo_platform_env_disabled == "true" + } + + pub fn validate(&self, execution_env: &EnvironmentVariableMap) -> Vec { + if Self::disabled() { + return vec![]; + } + + self.diff(execution_env) + } + + pub fn diff(&self, execution_env: &EnvironmentVariableMap) -> Vec { + self.env_keys + .iter() + .filter(|key| !execution_env.contains_key(*key)) + .map(|s| s.to_string()) + .collect() + } + + pub fn output_header(is_strict: bool, color_config: ColorConfig) { + let ci = Vendor::get_constant().unwrap_or("unknown"); + + let strict_message = if is_strict { + "These variables WILL NOT be available to your application and may cause your build to \ + fail." + } else { + "These variables WILL NOT be considered in your cache key and could cause inadvertent \ + cache hits." + }; + + match ci { + "VERCEL" => { + cprintln!( + color_config, + BOLD, + "The following environment variables are set on your Vercel project, but \ + missing from \"turbo.json\". {}", + strict_message + ); + } + _ => { + cprintln!( + color_config, + BOLD, + "The following environment variables are missing from \"turbo.json\". {}", + strict_message + ); + } + } + } + + pub fn output_for_task( + missing: Vec, + task_id_for_display: &str, + color_config: ColorConfig, + ) { + cprintln!(color_config, YELLOW, "{}", task_id_for_display); + for key in missing { + cprint!(color_config, GREY, " - "); + cprint!(color_config, GREY, "{}\n", key); + } + println!(); + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::*; + + fn set_env_var(key: &str, value: &str) { + std::env::set_var(key, value); + } + + fn clear_env_var(key: &str) { + std::env::remove_var(key); + } + + #[test] + fn test_platform_env_new() { + set_env_var(TURBO_PLATFORM_ENV_KEY, "VAR1,VAR2,VAR3"); + let platform_env = PlatformEnv::new(); + assert_eq!(platform_env.env_keys, vec!["VAR1", "VAR2", "VAR3"]); + clear_env_var(TURBO_PLATFORM_ENV_KEY); + } + + #[test] + fn test_platform_env_new_empty() { + set_env_var(TURBO_PLATFORM_ENV_KEY, ""); + let platform_env = PlatformEnv::new(); + assert!(platform_env.env_keys.is_empty()); + clear_env_var(TURBO_PLATFORM_ENV_KEY); + } + + #[test] + fn test_disabled_true() { + set_env_var(TURBO_PLATFORM_ENV_DISABLED_KEY, "1"); + assert!(PlatformEnv::disabled()); + clear_env_var(TURBO_PLATFORM_ENV_DISABLED_KEY); + } + + #[test] + fn test_disabled_false() { + set_env_var(TURBO_PLATFORM_ENV_DISABLED_KEY, "0"); + assert!(!PlatformEnv::disabled()); + clear_env_var(TURBO_PLATFORM_ENV_DISABLED_KEY); + } + + #[test] + fn test_validate_disabled() { + set_env_var(TURBO_PLATFORM_ENV_DISABLED_KEY, "1"); + let platform_env = PlatformEnv::new(); + let execution_env = EnvironmentVariableMap(HashMap::new()); + let missing = platform_env.validate(&execution_env); + assert!(missing.is_empty()); + clear_env_var(TURBO_PLATFORM_ENV_DISABLED_KEY); + } + + #[test] + fn test_validate_missing_keys() { + set_env_var(TURBO_PLATFORM_ENV_KEY, "VAR1,VAR2"); + clear_env_var(TURBO_PLATFORM_ENV_DISABLED_KEY); + + let platform_env = PlatformEnv::new(); + + let mut execution_env = EnvironmentVariableMap(HashMap::new()); + execution_env.insert("VAR2".to_string(), "value".to_string()); + + let missing = platform_env.validate(&execution_env); + + assert_eq!(missing, vec!["VAR1".to_string()]); + + clear_env_var(TURBO_PLATFORM_ENV_KEY); + } + + #[test] + fn test_diff_all_keys_present() { + set_env_var(TURBO_PLATFORM_ENV_KEY, "VAR1,VAR2"); + let platform_env = PlatformEnv::new(); + + let mut execution_env = EnvironmentVariableMap(HashMap::new()); + execution_env.insert("VAR1".to_string(), "value1".to_string()); + execution_env.insert("VAR2".to_string(), "value2".to_string()); + + let missing = platform_env.diff(&execution_env); + assert!(missing.is_empty()); + + clear_env_var(TURBO_PLATFORM_ENV_KEY); + } +} diff --git a/crates/turborepo-lib/src/run/cache.rs b/crates/turborepo-lib/src/run/cache.rs index cfb7483ea9291..02c9eedce5b9b 100644 --- a/crates/turborepo-lib/src/run/cache.rs +++ b/crates/turborepo-lib/src/run/cache.rs @@ -161,6 +161,10 @@ impl TaskCache { self.task_output_logs } + pub fn is_caching_disabled(&self) -> bool { + self.caching_disabled + } + /// Will read log file and write to output a line at a time pub fn replay_log_file(&self, output: &mut impl CacheOutput) -> Result<(), Error> { if self.log_file_path.exists() { diff --git a/crates/turborepo-lib/src/task_graph/visitor.rs b/crates/turborepo-lib/src/task_graph/visitor.rs index f2313e4ad9584..3dac06c3ccc9b 100644 --- a/crates/turborepo-lib/src/task_graph/visitor.rs +++ b/crates/turborepo-lib/src/task_graph/visitor.rs @@ -13,10 +13,10 @@ use itertools::Itertools; use miette::{Diagnostic, NamedSource, SourceSpan}; use regex::Regex; use tokio::sync::{mpsc, oneshot}; -use tracing::{debug, error, Instrument, Span}; +use tracing::{debug, error, warn, Instrument, Span}; use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, AnchoredSystemPath}; use turborepo_ci::{Vendor, VendorBehavior}; -use turborepo_env::EnvironmentVariableMap; +use turborepo_env::{platform::PlatformEnv, EnvironmentVariableMap}; use turborepo_repository::{ package_graph::{PackageGraph, PackageName, ROOT_PKG_NAME}, package_manager::PackageManager, @@ -68,6 +68,7 @@ pub struct Visitor<'a> { color_config: ColorConfig, is_watch: bool, ui_sender: Option, + warnings: Arc>>, } #[derive(Debug, thiserror::Error, Diagnostic)] @@ -153,6 +154,7 @@ impl<'a> Visitor<'a> { global_env, ui_sender, is_watch, + warnings: Default::default(), } } @@ -293,6 +295,7 @@ impl<'a> Visitor<'a> { } else { TaskOutput::Direct(self.output_client(&info, vendor_behavior)) }; + let tracker = self.run_tracker.track_task(info.clone().into_owned()); let spaces_client = self.run_tracker.spaces_task_client(); let parent_span = Span::current(); @@ -381,6 +384,35 @@ impl<'a> Visitor<'a> { let global_hash_summary = GlobalHashSummary::try_from(global_hash_inputs)?; + // output any warnings that we collected while running tasks + if let Ok(warnings) = self.warnings.lock() { + if !warnings.is_empty() { + println!(); + warn!("finished with warnings"); + println!(); + + let has_missing_platform_env: bool = warnings + .iter() + .any(|warning| !warning.missing_platform_env.is_empty()); + if has_missing_platform_env { + PlatformEnv::output_header( + global_env_mode == EnvMode::Strict, + self.color_config, + ); + + for warning in warnings.iter() { + if !warning.missing_platform_env.is_empty() { + PlatformEnv::output_for_task( + warning.missing_platform_env.clone(), + &warning.task_id, + self.color_config, + ) + } + } + } + } + } + Ok(self .run_tracker .finish( @@ -564,6 +596,13 @@ fn turbo_regex() -> &'static Regex { RE.get_or_init(|| Regex::new(r"(?:^|\s)turbo(?:$|\s)").unwrap()) } +// Warning that comes from the execution of the task +#[derive(Debug, Clone)] +pub struct TaskWarning { + task_id: String, + missing_platform_env: Vec, +} + // Error that comes from the execution of the task #[derive(Debug, thiserror::Error, Clone)] #[error("{task_id}: {cause}")] @@ -691,8 +730,10 @@ impl<'a> ExecContextFactory<'a> { continue_on_error: self.visitor.run_opts.continue_on_error, pass_through_args, errors: self.errors.clone(), + warnings: self.visitor.warnings.clone(), takes_input, task_access, + platform_env: PlatformEnv::new(), } } @@ -727,8 +768,10 @@ struct ExecContext { continue_on_error: bool, pass_through_args: Option>, errors: Arc>>, + warnings: Arc>>, takes_input: bool, task_access: TaskAccess, + platform_env: PlatformEnv, } enum ExecOutcome { @@ -881,6 +924,18 @@ impl ExecContext { } } + if !self.task_cache.is_caching_disabled() { + let missing_platform_env = self.platform_env.validate(&self.execution_env); + if !missing_platform_env.is_empty() { + let _ = self.warnings.lock().map(|mut warnings| { + warnings.push(TaskWarning { + task_id: self.task_id_for_display.clone(), + missing_platform_env, + }); + }); + } + } + match self .task_cache .restore_outputs(&mut prefixed_ui, telemetry) From 524ac9df7e753d4c50f5719b3771dd2702ef39c6 Mon Sep 17 00:00:00 2001 From: tknickman Date: Fri, 11 Oct 2024 17:20:06 -0400 Subject: [PATCH 2/2] Ok clippy --- crates/turborepo-env/src/platform.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/turborepo-env/src/platform.rs b/crates/turborepo-env/src/platform.rs index 1b22c79ff2821..f916c5bb97ec2 100644 --- a/crates/turborepo-env/src/platform.rs +++ b/crates/turborepo-env/src/platform.rs @@ -7,6 +7,12 @@ pub struct PlatformEnv { env_keys: Vec, } +impl Default for PlatformEnv { + fn default() -> Self { + Self::new() + } +} + const TURBO_PLATFORM_ENV_KEY: &str = "TURBO_PLATFORM_ENV"; const TURBO_PLATFORM_ENV_DISABLED_KEY: &str = "TURBO_PLATFORM_ENV_DISABLED";