diff --git a/Cargo.lock b/Cargo.lock index 1a254a7..6c4fd53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -918,6 +918,7 @@ dependencies = [ "tracing-subscriber", "typetag", "url", + "uuid", "xz2", ] @@ -1862,6 +1863,12 @@ dependencies = [ "serde", ] +[[package]] +name = "uuid" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "422ee0de9031b5b948b97a8fc04e3aa35230001a722ddd27943e0be31564ce4c" + [[package]] name = "valuable" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 96a7fb6..f9e3bb5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,7 @@ dyn-clone = { version = "1.0.9", default-features = false } rand = { version = "0.8.5", default-features = false, features = [ "std", "std_rng" ] } semver = { version = "1.0.14", default-features = false, features = ["serde", "std"] } term = { version = "0.7.0", default-features = false } +uuid = "1.2.2" [dev-dependencies] eyre = { version = "0.6.8", default-features = false, features = [ "track-caller" ] } diff --git a/src/action/base/create_or_insert_into_file.rs b/src/action/base/create_or_insert_into_file.rs index 94dd72d..58dd601 100644 --- a/src/action/base/create_or_insert_into_file.rs +++ b/src/action/base/create_or_insert_into_file.rs @@ -185,10 +185,19 @@ impl Action for CreateOrInsertIntoFile { tokio::fs::set_permissions(&temp_file_path, PermissionsExt::from_mode(*mode)) .await .map_err(|e| ActionError::SetPermissions(*mode, path.to_owned(), e))?; - } else if orig_file.is_some() { - tokio::fs::set_permissions(&temp_file_path, PermissionsExt::from_mode(0o644)) + } else if let Some(original_file) = orig_file { + let original_file_mode = original_file + .metadata() .await - .map_err(|e| ActionError::SetPermissions(0o644, path.to_owned(), e))?; + .map_err(|e| ActionError::GettingMetadata(path.to_path_buf(), e))? + .permissions() + .mode(); + tokio::fs::set_permissions( + &temp_file_path, + PermissionsExt::from_mode(original_file_mode), + ) + .await + .map_err(|e| ActionError::SetPermissions(original_file_mode, path.to_owned(), e))?; } tokio::fs::rename(&temp_file_path, &path) diff --git a/src/action/darwin/create_fstab_entry.rs b/src/action/darwin/create_fstab_entry.rs new file mode 100644 index 0000000..5fc2184 --- /dev/null +++ b/src/action/darwin/create_fstab_entry.rs @@ -0,0 +1,237 @@ +use nix::unistd::{chown, Group, User}; +use uuid::Uuid; + +use crate::{ + action::{Action, ActionDescription, ActionError, StatefulAction}, + execute_command, +}; +use rand::Rng; +use std::{ + io::SeekFrom, + os::unix::prelude::PermissionsExt, + path::{Path, PathBuf}, + str::FromStr, +}; +use tokio::{ + fs::{remove_file, OpenOptions}, + io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt}, + process::Command, +}; +use tracing::{span, Span}; + +const FSTAB_PATH: &str = "/etc/fstab"; + +/** Create an `/etc/fstab` entry for the given volume + + +This action queries `diskutil info` on the volume to fetch it's UUID and +add the relevant information to `/etc/fstab`. + */ +// Initially, a `NAME` was used, however in https://github.com/DeterminateSystems/nix-installer/issues/212 +// several users reported issues. Using a UUID resolved the issue for them. +#[derive(Debug, serde::Deserialize, serde::Serialize, Clone)] +pub struct CreateFstabEntry { + apfs_volume_label: String, +} + +impl CreateFstabEntry { + #[tracing::instrument(level = "debug", skip_all)] + pub async fn plan(apfs_volume_label: String) -> Result, ActionError> { + let fstab_path = Path::new(FSTAB_PATH); + if fstab_path.exists() { + let fstab_buf = tokio::fs::read_to_string(&fstab_path) + .await + .map_err(|e| ActionError::Read(fstab_path.to_path_buf(), e))?; + let prelude_comment = fstab_prelude_comment(&apfs_volume_label); + if fstab_buf.contains(&prelude_comment) { + return Err(ActionError::Custom(Box::new( + CreateFstabEntryError::EntryExists(apfs_volume_label.clone()), + ))); + } + } + + Ok(Self { apfs_volume_label }.into()) + } +} + +#[async_trait::async_trait] +#[typetag::serde(name = "create_fstab_entry")] +impl Action for CreateFstabEntry { + fn tracing_synopsis(&self) -> String { + format!( + "Add a UUID based entry for the APFS volume `{}` to `/etc/fstab`", + self.apfs_volume_label + ) + } + + fn tracing_span(&self) -> Span { + let span = span!( + tracing::Level::DEBUG, + "create_fstab_entry", + apfs_volume_label = self.apfs_volume_label, + ); + + span + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] + } + + #[tracing::instrument(level = "debug", skip_all)] + async fn execute(&mut self) -> Result<(), ActionError> { + let Self { apfs_volume_label } = self; + let fstab_path = Path::new(FSTAB_PATH); + let uuid = get_uuid_for_label(&apfs_volume_label) + .await? + .ok_or_else(|| { + ActionError::Custom(Box::new(CreateFstabEntryError::NoVolume( + apfs_volume_label.clone(), + ))) + })?; + let fstab_entry = fstab_entry(&uuid, apfs_volume_label); + + let mut fstab = tokio::fs::OpenOptions::new() + .create(true) + .write(true) + .read(true) + .open(fstab_path) + .await + .map_err(|e| ActionError::Open(fstab_path.to_path_buf(), e))?; + + // Make sure it doesn't already exist before we write to it. + let mut fstab_buf = String::new(); + fstab + .read_to_string(&mut fstab_buf) + .await + .map_err(|e| ActionError::Read(fstab_path.to_owned(), e))?; + + if fstab_buf.contains(&fstab_entry) { + tracing::debug!("Skipped writing to `/etc/fstab` as the content already existed") + } else { + fstab + .write_all(fstab_entry.as_bytes()) + .await + .map_err(|e| ActionError::Write(fstab_path.to_owned(), e))?; + } + + Ok(()) + } + + fn revert_description(&self) -> Vec { + let Self { apfs_volume_label } = &self; + vec![ActionDescription::new( + format!( + "Remove the UUID based entry for the APFS volume `{}` in `/etc/fstab`", + apfs_volume_label + ), + vec![], + )] + } + + #[tracing::instrument(level = "debug", skip_all)] + async fn revert(&mut self) -> Result<(), ActionError> { + let Self { apfs_volume_label } = self; + let fstab_path = Path::new(FSTAB_PATH); + let uuid = get_uuid_for_label(&apfs_volume_label) + .await? + .ok_or_else(|| { + ActionError::Custom(Box::new(CreateFstabEntryError::NoVolume( + apfs_volume_label.clone(), + ))) + })?; + let fstab_entry = fstab_entry(&uuid, apfs_volume_label); + + let mut file = OpenOptions::new() + .create(false) + .write(true) + .read(true) + .open(&fstab_path) + .await + .map_err(|e| ActionError::Open(fstab_path.to_owned(), e))?; + + let mut file_contents = String::default(); + file.read_to_string(&mut file_contents) + .await + .map_err(|e| ActionError::Read(fstab_path.to_owned(), e))?; + + if let Some(start) = file_contents.rfind(fstab_entry.as_str()) { + let end = start + fstab_entry.len(); + file_contents.replace_range(start..end, "") + } + + file.seek(SeekFrom::Start(0)) + .await + .map_err(|e| ActionError::Seek(fstab_path.to_owned(), e))?; + file.set_len(0) + .await + .map_err(|e| ActionError::Truncate(fstab_path.to_owned(), e))?; + file.write_all(file_contents.as_bytes()) + .await + .map_err(|e| ActionError::Write(fstab_path.to_owned(), e))?; + file.flush() + .await + .map_err(|e| ActionError::Flush(fstab_path.to_owned(), e))?; + + Ok(()) + } +} + +async fn get_uuid_for_label(apfs_volume_label: &str) -> Result, ActionError> { + let output = execute_command( + Command::new("/usr/sbin/diskutil") + .process_group(0) + .arg("info") + .arg(apfs_volume_label) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()), + ) + .await + .map_err(|e| ActionError::Command(e))?; + + let stdout = String::from_utf8(output.stdout)?; + + let mut found = None; + for line in stdout.lines() { + let prefix = "Volume UUID:"; + let trimmed = line.trim(); + if let Some(index) = trimmed.find(prefix) { + let maybe_uuid = trimmed[(index + prefix.len())..].trim(); + let uuid = Uuid::parse_str(maybe_uuid).map_err(|err| { + ActionError::Custom(Box::new(CreateFstabEntryError::Uuid( + maybe_uuid.to_string(), + err, + ))) + })?; + + found = Some(uuid); + break; + } + } + + Ok(found) +} + +fn fstab_prelude_comment(apfs_volume_label: &str) -> String { + format!("# nix-installer created volume labelled `{apfs_volume_label}`") +} + +fn fstab_entry(uuid: &Uuid, apfs_volume_label: &str) -> String { + let prelude_comment = fstab_prelude_comment(apfs_volume_label); + format!( + "\ + {prelude_comment}\n\ + UUID={uuid} /nix apfs rw,noauto,nobrowse,suid,owners\n\ + " + ) +} + +#[derive(thiserror::Error, Debug)] +pub enum CreateFstabEntryError { + #[error("UUID error: {0}")] + Uuid(String, #[source] uuid::Error), + #[error("No volume labelled `{0}` present, cannot get UUID to add to /etc/fstab")] + NoVolume(String), + #[error("An `/etc/fstab` entry for the volume labelled `{0}` already exists. If a Nix Store already exists it may need to be deleted with `diskutil apfs deleteVolume \"{0}\") and should be removed from `/etc/fstab`")] + EntryExists(String), +} diff --git a/src/action/darwin/create_nix_volume.rs b/src/action/darwin/create_nix_volume.rs index 4076505..8b048fe 100644 --- a/src/action/darwin/create_nix_volume.rs +++ b/src/action/darwin/create_nix_volume.rs @@ -13,6 +13,8 @@ use std::{ use tokio::process::Command; use tracing::{span, Span}; +use super::create_fstab_entry::CreateFstabEntry; + pub const NIX_VOLUME_MOUNTD_DEST: &str = "/Library/LaunchDaemons/org.nixos.darwin-store.plist"; /// Create an APFS volume @@ -26,7 +28,7 @@ pub struct CreateNixVolume { create_synthetic_objects: StatefulAction, unmount_volume: StatefulAction, create_volume: StatefulAction, - create_or_append_fstab: StatefulAction, + create_fstab_entry: StatefulAction, encrypt_volume: Option>, setup_volume_daemon: StatefulAction, bootstrap_volume: StatefulAction, @@ -59,16 +61,9 @@ impl CreateNixVolume { let create_volume = CreateApfsVolume::plan(disk, name.clone(), case_sensitive).await?; - let create_or_append_fstab = CreateOrInsertIntoFile::plan( - "/etc/fstab", - None, - None, - 0o0655, - format!("NAME=\"{name}\" /nix apfs rw,noauto,nobrowse,suid,owners"), - create_or_insert_into_file::Position::End, - ) - .await - .map_err(|e| ActionError::Child(Box::new(e)))?; + let create_fstab_entry = CreateFstabEntry::plan(name.clone()) + .await + .map_err(|e| ActionError::Child(Box::new(e)))?; let encrypt_volume = if encrypt { Some(EncryptApfsVolume::plan(disk, &name).await?) @@ -125,7 +120,7 @@ impl CreateNixVolume { create_synthetic_objects, unmount_volume, create_volume, - create_or_append_fstab, + create_fstab_entry, encrypt_volume, setup_volume_daemon, bootstrap_volume, @@ -173,7 +168,7 @@ impl Action for CreateNixVolume { create_synthetic_objects, unmount_volume, create_volume, - create_or_append_fstab, + create_fstab_entry, encrypt_volume, setup_volume_daemon, bootstrap_volume, @@ -184,7 +179,7 @@ impl Action for CreateNixVolume { create_synthetic_objects.try_execute().await?; unmount_volume.try_execute().await.ok(); // We actually expect this may fail. create_volume.try_execute().await?; - create_or_append_fstab.try_execute().await?; + create_fstab_entry.try_execute().await?; if let Some(encrypt_volume) = encrypt_volume { encrypt_volume.try_execute().await?; } @@ -236,7 +231,7 @@ impl Action for CreateNixVolume { create_synthetic_objects, unmount_volume, create_volume, - create_or_append_fstab, + create_fstab_entry, encrypt_volume, setup_volume_daemon, bootstrap_volume, @@ -249,7 +244,7 @@ impl Action for CreateNixVolume { if let Some(encrypt_volume) = encrypt_volume { encrypt_volume.try_revert().await?; } - create_or_append_fstab.try_revert().await?; + create_fstab_entry.try_revert().await?; unmount_volume.try_revert().await?; create_volume.try_revert().await?; diff --git a/src/action/darwin/mod.rs b/src/action/darwin/mod.rs index 799ef8d..822b6a2 100644 --- a/src/action/darwin/mod.rs +++ b/src/action/darwin/mod.rs @@ -3,6 +3,7 @@ pub(crate) mod bootstrap_apfs_volume; pub(crate) mod create_apfs_volume; +pub(crate) mod create_fstab_entry; pub(crate) mod create_nix_volume; pub(crate) mod create_synthetic_objects; pub(crate) mod enable_ownership; diff --git a/tests/fixtures/darwin/darwin-multi.json b/tests/fixtures/darwin/darwin-multi.json index 1502bc2..fcd7ae7 100644 --- a/tests/fixtures/darwin/darwin-multi.json +++ b/tests/fixtures/darwin/darwin-multi.json @@ -38,14 +38,9 @@ }, "state": "Uncompleted" }, - "create_or_append_fstab": { + "create_fstab_entry": { "action": { - "path": "/etc/fstab", - "user": null, - "group": null, - "mode": 429, - "buf": "NAME=\"Nix Store\" /nix apfs rw,noauto,nobrowse,suid,owners", - "position": "End" + "apfs_volume_label": "Nix Store" }, "state": "Uncompleted" },