Skip to content

Commit

Permalink
handle VERCEL_ARTIFACTS_* env vars override (#9249)
Browse files Browse the repository at this point in the history
### Description

The prior intended state of EnvVars overrides looked like this:
<br />
<div align="center">
  <div><sub><em>`VERCEL_ARTIFACTS_*` always wins</em></sub></div>
  <br />
<img width="75%"
src="https://github.com/user-attachments/assets/b8f5428a-acaa-4323-9b47-006a3dbc9a42"
/>
</div>
<br />
<br />

But we've realized this is not good enough. We want to better handle the
scenario where users have provided custom remote cache - even if they
deploy on Vercel.

After this PR, the precedence works like this.

1. if both `TURBO_TEAMID`+`TURBO_TOKEN` are set we use those. if
`TURBO_TEAM` is also set, we pass that along, too.
1. if both `TURBO_TEAM`+`TURBO_TOKEN` are set we use those. if
`TURBO_TEAMID` is also set we pass that along, too.
1. if both `VERCEL_ARTIFACTS_OWNER`+`VERCEL_ARTIFACTS_TOKEN` are set, we
use those (all else is ignored).
1. if both `TURBO_TEAMID`+`TURBO_TEAM`, we pass those along.
1. if just `TURBO_TEAMID` is set, we pass that along.
1. if just `TURBO_TEAM` is set, we pass that along.
1. if both `TURBO_TEAM` and `VERCEL_ARTIFACTS_OWNER` are set, we pass
those along
1. if just `VERCEL_ARTIFACTS_OWNER` is set, we pass that along
1. we do not support or consider partial configurations. For example if
only `VERCEL_ARTIFACTS_TOKEN` and `TURBO_TEAMID` are set, this is
considered undefined/unsupported behavior. The reasons are that:
- this shouldn't ever happen - Vercel sets both or none of the
`VERCEL_ARTIFACTS_*` family
- we tried doing this, but the combinatorial explosion that results is
pretty major, and some of the cases are very strange to reason about,
considering the above point that we don't expect there to be any genuine
scenario where this happens, anyway.

### Testing Instructions

Take a look at the newly added tests in `override_env.rs`. You could run
turbo while manually setting those same env vars.

---------

Co-authored-by: Chris Olszewski <[email protected]>
  • Loading branch information
dimitropoulos and chris-olszewski authored Oct 18, 2024
1 parent 5c36b0b commit 9375ef9
Show file tree
Hide file tree
Showing 3 changed files with 463 additions and 91 deletions.
87 changes: 7 additions & 80 deletions crates/turborepo-lib/src/config/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,60 +189,7 @@ impl ResolvedConfigurationOptions for EnvVars {
}
}

const VERCEL_ARTIFACTS_MAPPING: &[(&str, &str)] = [
("vercel_artifacts_token", "token"),
("vercel_artifacts_owner", "team_id"),
]
.as_slice();

pub struct OverrideEnvVars<'a> {
environment: &'a HashMap<OsString, OsString>,
output_map: HashMap<&'static str, String>,
}

impl<'a> OverrideEnvVars<'a> {
pub fn new(environment: &'a HashMap<OsString, OsString>) -> Result<Self, Error> {
let vercel_artifacts_mapping: HashMap<_, _> =
VERCEL_ARTIFACTS_MAPPING.iter().copied().collect();

let output_map = map_environment(vercel_artifacts_mapping, environment)?;
Ok(Self {
environment,
output_map,
})
}

fn ui(&self) -> Option<UIMode> {
let value = self
.environment
.get(OsStr::new("ci"))
.or_else(|| self.environment.get(OsStr::new("no_color")))?;
match truth_env_var(value.to_str()?)? {
true => Some(UIMode::Stream),
false => None,
}
}
}

impl<'a> ResolvedConfigurationOptions for OverrideEnvVars<'a> {
fn get_configuration_options(
&self,
_existing_config: &ConfigurationOptions,
) -> Result<ConfigurationOptions, Error> {
let ui = self.ui();
let output = ConfigurationOptions {
team_id: self.output_map.get("team_id").cloned(),
token: self.output_map.get("token").cloned(),
api_url: None,
ui,
..Default::default()
};

Ok(output)
}
}

fn truth_env_var(s: &str) -> Option<bool> {
pub fn truth_env_var(s: &str) -> Option<bool> {
match s {
"true" | "1" => Some(true),
"false" | "0" => Some(false),
Expand All @@ -251,7 +198,13 @@ fn truth_env_var(s: &str) -> Option<bool> {
}

fn map_environment<'a>(
// keys are environment variable names
// values are properties of ConfigurationOptions we want to store the
// values in
mapping: HashMap<&str, &'a str>,

// keys are environment variable names
// values are the values of those environment variables
environment: &HashMap<OsString, OsString>,
) -> Result<HashMap<&'a str, String>, Error> {
let mut output_map = HashMap::new();
Expand Down Expand Up @@ -395,30 +348,4 @@ mod test {
assert!(!config.run_summary());
assert!(!config.allow_no_turbo_json());
}

#[test]
fn test_override_env_setting() {
let mut env: HashMap<OsString, OsString> = HashMap::new();

let vercel_artifacts_token = "correct-horse-battery-staple";
let vercel_artifacts_owner = "bobby_tables";

env.insert(
"vercel_artifacts_token".into(),
vercel_artifacts_token.into(),
);
env.insert(
"vercel_artifacts_owner".into(),
vercel_artifacts_owner.into(),
);
env.insert("ci".into(), "1".into());

let config = OverrideEnvVars::new(&env)
.unwrap()
.get_configuration_options(&ConfigurationOptions::default())
.unwrap();
assert_eq!(vercel_artifacts_token, config.token.unwrap());
assert_eq!(vercel_artifacts_owner, config.team_id.unwrap());
assert_eq!(Some(UIMode::Stream), config.ui);
}
}
21 changes: 10 additions & 11 deletions crates/turborepo-lib/src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
mod env;
mod file;
mod override_env;
mod turbo_json;

use std::{collections::HashMap, ffi::OsString, io};

use camino::{Utf8Path, Utf8PathBuf};
use convert_case::{Case, Casing};
use derive_setters::Setters;
use env::{EnvVars, OverrideEnvVars};
use env::EnvVars;
use file::{AuthFile, ConfigFile};
use merge::Merge;
use miette::{Diagnostic, NamedSource, SourceSpan};
use override_env::OverrideEnvVars;
use serde::Deserialize;
use struct_iterable::Iterable;
use thiserror::Error;
Expand Down Expand Up @@ -219,11 +221,14 @@ pub struct ConfigurationOptions {
#[serde(alias = "teamslug")]
#[serde(alias = "TeamSlug")]
#[serde(alias = "TEAMSLUG")]
/// corresponds to env var TURBO_TEAM
pub(crate) team_slug: Option<String>,
#[serde(alias = "teamid")]
#[serde(alias = "TeamId")]
#[serde(alias = "TEAMID")]
/// corresponds to env var TURBO_TEAMID
pub(crate) team_id: Option<String>,
/// corresponds to env var TURBO_TOKEN
pub(crate) token: Option<String>,
pub(crate) signature: Option<bool>,
pub(crate) preflight: Option<bool>,
Expand Down Expand Up @@ -467,9 +472,9 @@ impl TurborepoConfigBuilder {

// These are ordered from highest to lowest priority
let sources: [Box<dyn ResolvedConfigurationOptions>; 7] = [
Box::new(override_env_var_config),
Box::new(&self.override_config),
Box::new(env_var_config),
Box::new(override_env_var_config),
Box::new(local_config),
Box::new(global_auth),
Box::new(global_config),
Expand Down Expand Up @@ -561,22 +566,16 @@ mod test {
vercel_artifacts_owner.into(),
);

let override_config = ConfigurationOptions {
token: Some("unseen".into()),
team_id: Some("unseen".into()),
..Default::default()
};

let builder = TurborepoConfigBuilder {
repo_root,
override_config,
override_config: Default::default(),
global_config_path: Some(global_config_path),
environment: Some(env),
};

let config = builder.build().unwrap();
assert_eq!(config.team_id().unwrap(), vercel_artifacts_owner);
assert_eq!(config.token().unwrap(), vercel_artifacts_token);
assert_eq!(config.team_id().unwrap(), turbo_teamid);
assert_eq!(config.token().unwrap(), turbo_token);
assert_eq!(config.spaces_id().unwrap(), "my-spaces-id");
}

Expand Down
Loading

0 comments on commit 9375ef9

Please sign in to comment.