diff --git a/src/action/base/create_directory.rs b/src/action/base/create_directory.rs index 19b68cff..cbe718d2 100644 --- a/src/action/base/create_directory.rs +++ b/src/action/base/create_directory.rs @@ -21,11 +21,11 @@ If `force_prune_on_revert` is set, the folder will always be deleted on #[serde(tag = "action_name", rename = "create_directory")] pub struct CreateDirectory { pub(crate) path: PathBuf, - user: Option, - group: Option, - mode: Option, - is_mountpoint: bool, - force_prune_on_revert: bool, + pub(crate) user: Option, + pub(crate) group: Option, + pub(crate) mode: Option, + pub(crate) is_mountpoint: bool, + pub(crate) force_prune_on_revert: bool, } impl CreateDirectory { diff --git a/src/action/base/create_or_merge_nix_config.rs b/src/action/base/create_or_merge_nix_config.rs index 3e70ee8f..bcb67c18 100644 --- a/src/action/base/create_or_merge_nix_config.rs +++ b/src/action/base/create_or_merge_nix_config.rs @@ -15,9 +15,13 @@ use crate::action::{ Action, ActionDescription, ActionError, ActionErrorKind, ActionTag, StatefulAction, }; +pub(crate) const TRUSTED_USERS_CONF_NAME: &str = "trusted-users"; +pub(crate) const EXPERIMENTAL_FEATURES_CONF_NAME: &str = "experimental-features"; +pub(crate) const EXTRA_EXPERIMENTAL_FEATURES_CONF_NAME: &str = "extra-experimental-features"; /// The `nix.conf` configuration names that are safe to merge. // FIXME(@cole-h): make configurable by downstream users? -const MERGEABLE_CONF_NAMES: &[&str] = &["experimental-features"]; +// NOTE(cole-h): evaluate if any additions here need to be handled in PlaceNixConfiguration::setup_extra_config +const MERGEABLE_CONF_NAMES: &[&str] = &[EXPERIMENTAL_FEATURES_CONF_NAME]; const NIX_CONF_MODE: u32 = 0o644; const NIX_CONF_COMMENT_CHAR: char = '#'; @@ -48,6 +52,7 @@ pub struct CreateOrMergeNixConfig { pub(crate) path: PathBuf, pending_nix_config: NixConfig, header: String, + footer: Option, } impl CreateOrMergeNixConfig { @@ -56,6 +61,7 @@ impl CreateOrMergeNixConfig { path: impl AsRef, pending_nix_config: NixConfig, header: String, + footer: Option, ) -> Result, ActionError> { let path = path.as_ref().to_path_buf(); @@ -63,6 +69,7 @@ impl CreateOrMergeNixConfig { path, pending_nix_config, header, + footer, }; if this.path.exists() { @@ -405,6 +412,12 @@ impl Action for CreateOrMergeNixConfig { new_config.push('\n'); } + if let Some(footer) = &self.footer { + new_config.push('\n'); + new_config.push_str(footer); + new_config.push('\n'); + } + temp_file .write_all(new_config.as_bytes()) .await @@ -466,15 +479,21 @@ mod test { nix_config .settings_mut() .insert("experimental-features".into(), "ca-references".into()); - let mut action = - CreateOrMergeNixConfig::plan(&test_file, nix_config, "# Generated by".to_string()) - .await?; + let mut action = CreateOrMergeNixConfig::plan( + &test_file, + nix_config, + "# Generated by".to_string(), + Some("# opa".into()), + ) + .await?; action.try_execute().await?; let s = std::fs::read_to_string(&test_file)?; assert!(s.contains("# Generated by")); assert!(s.contains("ca-references")); + + assert!(s.contains("# opa")); assert!(NixConfig::parse_file(&test_file).is_ok()); action.try_revert().await?; @@ -494,9 +513,13 @@ mod test { nix_config .settings_mut() .insert("experimental-features".into(), "ca-references".into()); - let mut action = - CreateOrMergeNixConfig::plan(&test_file, nix_config, "# Generated by".to_string()) - .await?; + let mut action = CreateOrMergeNixConfig::plan( + &test_file, + nix_config, + "# Generated by".to_string(), + None, + ) + .await?; action.try_execute().await?; @@ -524,9 +547,13 @@ mod test { nix_config .settings_mut() .insert("experimental-features".into(), "flakes".into()); - let mut action = - CreateOrMergeNixConfig::plan(&test_file, nix_config, "# Generated by".to_string()) - .await?; + let mut action = CreateOrMergeNixConfig::plan( + &test_file, + nix_config, + "# Generated by".to_string(), + None, + ) + .await?; action.try_execute().await?; @@ -558,9 +585,13 @@ mod test { nix_config .settings_mut() .insert("allow-dirty".into(), "false".into()); - let mut action = - CreateOrMergeNixConfig::plan(&test_file, nix_config, "# Generated by".to_string()) - .await?; + let mut action = CreateOrMergeNixConfig::plan( + &test_file, + nix_config, + "# Generated by".to_string(), + None, + ) + .await?; action.try_execute().await?; @@ -605,7 +636,7 @@ mod test { nix_config .settings_mut() .insert("warn-dirty".into(), "false".into()); - match CreateOrMergeNixConfig::plan(&test_file, nix_config, "".to_string()).await { + match CreateOrMergeNixConfig::plan(&test_file, nix_config, "".to_string(), None).await { Err(err) => { if let ActionErrorKind::Custom(e) = err.kind() { match e.downcast_ref::() { @@ -647,9 +678,13 @@ mod test { nix_config .settings_mut() .insert("experimental-features".into(), "ca-references".into()); - let mut action = - CreateOrMergeNixConfig::plan(&test_file, nix_config, "# Generated by".to_string()) - .await?; + let mut action = CreateOrMergeNixConfig::plan( + &test_file, + nix_config, + "# Generated by".to_string(), + None, + ) + .await?; action.try_execute().await?; @@ -681,9 +716,13 @@ mod test { nix_config .settings_mut() .insert("experimental-features".into(), "ca-references".into()); - let mut action = - CreateOrMergeNixConfig::plan(&test_file, nix_config, "# Generated by".to_string()) - .await?; + let mut action = CreateOrMergeNixConfig::plan( + &test_file, + nix_config, + "# Generated by".to_string(), + None, + ) + .await?; action.try_execute().await?; diff --git a/src/action/common/place_nix_configuration.rs b/src/action/common/place_nix_configuration.rs index 7f990554..b397a651 100644 --- a/src/action/common/place_nix_configuration.rs +++ b/src/action/common/place_nix_configuration.rs @@ -1,7 +1,10 @@ use tracing::{span, Span}; use url::Url; -use crate::action::base::create_or_merge_nix_config::CreateOrMergeNixConfigError; +use crate::action::base::create_or_merge_nix_config::{ + CreateOrMergeNixConfigError, EXPERIMENTAL_FEATURES_CONF_NAME, + EXTRA_EXPERIMENTAL_FEATURES_CONF_NAME, TRUSTED_USERS_CONF_NAME, +}; use crate::action::base::{CreateDirectory, CreateOrMergeNixConfig}; use crate::action::{ Action, ActionDescription, ActionError, ActionErrorKind, ActionTag, StatefulAction, @@ -14,15 +17,13 @@ pub const NIX_CONF_FOLDER: &str = "/etc/nix"; const NIX_CONF: &str = "/etc/nix/nix.conf"; const CUSTOM_NIX_CONF: &str = "/etc/nix/nix.custom.conf"; -const NIX_CONFIG_HEADER: &str = r#" -# Generated by https://github.com/DeterminateSystems/nix-installer. +const NIX_CONFIG_HEADER: &str = r#"# Generated by https://github.com/DeterminateSystems/nix-installer. # See `/nix/nix-installer --version` for the version details. - -!include nix.custom.conf "#; -const CUSTOM_NIX_CONFIG_HEADER: &str = r#" -# Written by https://github.com/DeterminateSystems/nix-installer. +const NIX_CONFIG_FOOTER: &str = "!include nix.custom.conf"; + +const CUSTOM_NIX_CONFIG_HEADER: &str = r#"# Written by https://github.com/DeterminateSystems/nix-installer. # The contents below are based on options specified at installation time. "#; @@ -47,14 +48,18 @@ impl PlaceNixConfiguration { force: bool, determinate_nix: bool, ) -> Result, ActionError> { + let extra_conf = Self::parse_extra_conf(proxy, ssl_cert_file.as_ref(), extra_conf).await?; + let standard_nix_config = if !determinate_nix { - Some(Self::setup_standard_config(ssl_cert_file.as_ref()).await?) + let maybe_trusted_users = extra_conf.settings().get(TRUSTED_USERS_CONF_NAME); + + Some(Self::setup_standard_config(maybe_trusted_users).await?) } else { None }; let custom_nix_config = - Self::setup_extra_config(nix_build_group_name, proxy, ssl_cert_file, extra_conf) + Self::setup_extra_config(extra_conf, nix_build_group_name, ssl_cert_file.as_ref()) .await?; let create_directory = CreateDirectory::plan(NIX_CONF_FOLDER, None, None, 0o0755, force) @@ -68,6 +73,7 @@ impl PlaceNixConfiguration { NIX_CONF, standard_nix_config, NIX_CONFIG_HEADER.to_string(), + Some(NIX_CONFIG_FOOTER.to_string()), ) .await .map_err(Self::error)?, @@ -80,9 +86,11 @@ impl PlaceNixConfiguration { CUSTOM_NIX_CONF, custom_nix_config, CUSTOM_NIX_CONFIG_HEADER.to_string(), + None, ) .await .map_err(Self::error)?; + Ok(Self { create_directory, create_or_merge_standard_nix_config, @@ -92,14 +100,14 @@ impl PlaceNixConfiguration { } async fn setup_standard_config( - ssl_cert_file: Option<&PathBuf>, + maybe_trusted_users: Option<&String>, ) -> Result { let mut nix_config = nix_config_parser::NixConfig::new(); let settings = nix_config.settings_mut(); let experimental_features = ["nix-command", "flakes"]; settings.insert( - "experimental-features".to_string(), + "extra-experimental-features".to_string(), experimental_features.join(" "), ); @@ -143,15 +151,6 @@ impl PlaceNixConfiguration { "(nix:$name)\\040".to_string(), ); settings.insert("max-jobs".to_string(), "auto".to_string()); - if let Some(ssl_cert_file) = ssl_cert_file { - let ssl_cert_file_canonical = ssl_cert_file.canonicalize().map_err(|e| { - Self::error(ActionErrorKind::Canonicalize(ssl_cert_file.to_owned(), e)) - })?; - settings.insert( - "ssl-cert-file".to_string(), - ssl_cert_file_canonical.display().to_string(), - ); - } settings.insert( "extra-nix-path".to_string(), "nixpkgs=flake:nixpkgs".to_string(), @@ -161,13 +160,32 @@ impl PlaceNixConfiguration { "https://install.determinate.systems/nix-upgrade/stable/universal".to_string(), ); + // NOTE(cole-h): This is a workaround to hopefully unbreak users of Cachix. + // When `cachix use`ing a cache, the Cachix CLI will sanity-check the system configuration + // at `/etc/nix/nix.conf` to ensure that the user doing this will actually be able to + // configure trusted settings (such as `trusted-public-keys`). + // However, because we now write the `--extra-conf` into the `nix.custom.conf` (which is how + // users, including our first-party DeterminateSystems/nix-installer-action, would configure + // the `trusted-users` setting), and Cachix does not currently handle `include`s + // properly[1][2], Cachix bails out thinking that the user is not a trusted user[3] even + // though it is (it's just configured in another file). + // + // [1]: https://github.com/cachix/cachix/issues/680 + // [2]: https://github.com/cachix/cachix/pull/681 + // [3]: https://github.com/DeterminateSystems/nix-installer/issues/1389 + if let Some(trusted_users) = maybe_trusted_users { + settings.insert( + TRUSTED_USERS_CONF_NAME.to_string(), + trusted_users.to_owned(), + ); + } + Ok(nix_config) } - async fn setup_extra_config( - nix_build_group_name: String, + async fn parse_extra_conf( proxy: Option, - ssl_cert_file: Option, + ssl_cert_file: Option<&PathBuf>, extra_conf: Vec, ) -> Result { let mut extra_conf_text = vec![]; @@ -223,17 +241,57 @@ impl PlaceNixConfiguration { } let extra_conf = extra_conf_text.join("\n"); - let mut nix_config = nix_config_parser::NixConfig::parse_string(extra_conf, None) + let nix_config = nix_config_parser::NixConfig::parse_string(extra_conf, None) .map_err(CreateOrMergeNixConfigError::ParseNixConfig) .map_err(Self::error)?; - let settings = nix_config.settings_mut(); + Ok(nix_config) + } + + async fn setup_extra_config( + mut extra_conf: nix_config_parser::NixConfig, + nix_build_group_name: String, + ssl_cert_file: Option<&PathBuf>, + ) -> Result { + let settings = extra_conf.settings_mut(); if nix_build_group_name != crate::settings::DEFAULT_NIX_BUILD_USER_GROUP_NAME { settings.insert("build-users-group".to_string(), nix_build_group_name); } - Ok(nix_config) + if let Some(ssl_cert_file) = ssl_cert_file { + let ssl_cert_file_canonical = ssl_cert_file.canonicalize().map_err(|e| { + Self::error(ActionErrorKind::Canonicalize(ssl_cert_file.to_owned(), e)) + })?; + settings.insert( + "ssl-cert-file".to_string(), + ssl_cert_file_canonical.display().to_string(), + ); + } + + // NOTE(cole-h): We want to ensure our experimental-features are not clobbered by user + // config, so if a user specifies that, we exchange it for the `extra-` variant that just + // appends to the list of experimental features. + // If the user does indeed want to completely overwrite our experimental features, they can + // modify nix.custom.conf to their heart's desire. In most cases, however, they likely just + // want their setting to take effect (and probably have no opinion on or would even prefer + // our standard experimental features continue to take effect). + if let Some((idx, _key, experimental_features)) = + settings.shift_remove_full(EXPERIMENTAL_FEATURES_CONF_NAME) + { + tracing::debug!( + "User specified {EXPERIMENTAL_FEATURES_CONF_NAME} in extra-conf, but this would \ + override our desired configuration, so force it to be \ + {EXTRA_EXPERIMENTAL_FEATURES_CONF_NAME} instead" + ); + settings.shift_insert( + idx, + EXTRA_EXPERIMENTAL_FEATURES_CONF_NAME.to_string(), + experimental_features, + ); + } + + Ok(extra_conf) } } @@ -355,9 +413,8 @@ mod tests { use super::*; #[tokio::test] - async fn extra_trusted_no_error() -> eyre::Result<()> { - let nix_config = PlaceNixConfiguration::setup_extra_config( - String::from("foo"), + async fn extra_trusted_cache() -> eyre::Result<()> { + let extra_conf = PlaceNixConfiguration::parse_extra_conf( None, None, vec![ @@ -367,6 +424,10 @@ mod tests { ) .await?; + let nix_config = + PlaceNixConfiguration::setup_extra_config(extra_conf, String::from("foo"), None) + .await?; + assert!( nix_config .settings() @@ -387,4 +448,179 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn experimental_features() -> eyre::Result<()> { + let nix_conf_dir = tempfile::tempdir()?; + let nix_conf_path = nix_conf_dir.path().join("nix.conf"); + let nix_custom_conf_path = nix_conf_dir.path().join("nix.custom.conf"); + + let extra_conf = PlaceNixConfiguration::parse_extra_conf( + None, + None, + vec![UrlOrPathOrString::String(format!( + "{EXPERIMENTAL_FEATURES_CONF_NAME} = foobar" + ))], + ) + .await?; + + let standard_nix_config = PlaceNixConfiguration::setup_standard_config(None).await?; + let custom_nix_config = + PlaceNixConfiguration::setup_extra_config(extra_conf, String::from("foo"), None) + .await?; + dbg!(&custom_nix_config); + dbg!(custom_nix_config.settings()); + dbg!(custom_nix_config + .settings() + .get(EXTRA_EXPERIMENTAL_FEATURES_CONF_NAME)); + + assert!( + custom_nix_config + .settings() + .get(EXPERIMENTAL_FEATURES_CONF_NAME) + .is_none(), + "experimental-features in `--extra-conf` was renamed to extra-experimental-features" + ); + assert!( + custom_nix_config + .settings() + .get(EXTRA_EXPERIMENTAL_FEATURES_CONF_NAME) + .unwrap() + .contains("foobar"), + "experimental-features in `--extra-conf` was renamed to extra-experimental-features" + ); + + let mut place_nix_configuration = StatefulAction::uncompleted(PlaceNixConfiguration { + create_directory: StatefulAction::completed(CreateDirectory { + path: nix_conf_dir.path().to_owned(), + user: None, + group: None, + mode: None, + is_mountpoint: false, + force_prune_on_revert: false, + }), + create_or_merge_standard_nix_config: Some( + CreateOrMergeNixConfig::plan( + &nix_conf_path, + standard_nix_config, + NIX_CONFIG_HEADER.to_string(), + Some(NIX_CONFIG_FOOTER.to_string()), + ) + .await + .map_err(PlaceNixConfiguration::error)?, + ), + create_or_merge_custom_nix_config: CreateOrMergeNixConfig::plan( + &nix_custom_conf_path, + custom_nix_config, + CUSTOM_NIX_CONFIG_HEADER.to_string(), + None, + ) + .await + .map_err(PlaceNixConfiguration::error)?, + }); + + place_nix_configuration + .try_execute() + .await + .expect("place nix config should succeed"); + + let custom_conf = tokio::fs::read_to_string(nix_custom_conf_path) + .await + .unwrap(); + assert!( + custom_conf.contains(EXTRA_EXPERIMENTAL_FEATURES_CONF_NAME) + && custom_conf.contains("foobar"), + "experimental-features in `--extra-conf` was renamed to extra-experimental-features" + ); + + Ok(()) + } + + #[tokio::test] + async fn extra_trusted_users() -> eyre::Result<()> { + let nix_conf_dir = tempfile::tempdir()?; + let nix_conf_path = nix_conf_dir.path().join("nix.conf"); + let nix_custom_conf_path = nix_conf_dir.path().join("nix.custom.conf"); + + let extra_conf = PlaceNixConfiguration::parse_extra_conf( + None, + None, + vec![UrlOrPathOrString::String(String::from( + "trusted-users = bob alice", + ))], + ) + .await?; + + let maybe_trusted_users = extra_conf.settings().get(TRUSTED_USERS_CONF_NAME); + + let standard_nix_config = + PlaceNixConfiguration::setup_standard_config(maybe_trusted_users).await?; + let custom_nix_config = + PlaceNixConfiguration::setup_extra_config(extra_conf, String::from("foo"), None) + .await?; + + assert!( + custom_nix_config + .settings() + .get("trusted-users") + .unwrap() + .contains("bob"), + "User config and internal defaults are both respected" + ); + assert!( + standard_nix_config + .settings() + .get("trusted-users") + .unwrap() + .contains("bob"), + "User config and internal defaults are both respected" + ); + + let mut place_nix_configuration = StatefulAction::uncompleted(PlaceNixConfiguration { + create_directory: StatefulAction::completed(CreateDirectory { + path: nix_conf_dir.path().to_owned(), + user: None, + group: None, + mode: None, + is_mountpoint: false, + force_prune_on_revert: false, + }), + create_or_merge_standard_nix_config: Some( + CreateOrMergeNixConfig::plan( + &nix_conf_path, + standard_nix_config, + NIX_CONFIG_HEADER.to_string(), + Some(NIX_CONFIG_FOOTER.to_string()), + ) + .await + .map_err(PlaceNixConfiguration::error)?, + ), + create_or_merge_custom_nix_config: CreateOrMergeNixConfig::plan( + &nix_custom_conf_path, + custom_nix_config, + CUSTOM_NIX_CONFIG_HEADER.to_string(), + None, + ) + .await + .map_err(PlaceNixConfiguration::error)?, + }); + + place_nix_configuration + .try_execute() + .await + .expect("place nix config should succeed"); + + let standard_conf = tokio::fs::read_to_string(nix_conf_path).await.unwrap(); + assert!(standard_conf.contains("trusted-user"), "trusted-user setting should exist in standard conf so that we don't break cachix users"); + + let custom_conf = tokio::fs::read_to_string(nix_custom_conf_path) + .await + .unwrap(); + assert!( + custom_conf.contains("trusted-user"), + "trusted-user setting should exist in custom conf as well" + ); + + Ok(()) + } }