From 25d02eb4fc5374496e3637d33ff85b5b98db8313 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 09:35:39 -0700 Subject: [PATCH 01/10] Remove deprecated methods, allow sync and async features simultaneously --- zone/src/lib.rs | 94 ------------------------------------------------- 1 file changed, 94 deletions(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index 2a630a6..380dc3b 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -16,12 +16,6 @@ use std::string::ToString; use thiserror::Error; use zone_cfg_derive::Resource; -#[cfg(all(feature = "sync", feature = "async"))] -compile_error!( - "The 'sync' and 'async' features are currently mutually exclusive. \ - This restriction may be lifted in the future." -); - const PFEXEC: &str = "/bin/pfexec"; const ZONENAME: &str = "/usr/bin/zonename"; const ZONEADM: &str = "/usr/sbin/zoneadm"; @@ -639,14 +633,6 @@ impl Config { self } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'run_blocking', to use the blocking variant, or \n\ - - Call 'run' using the 'async' feature to call asynchronously.")] - pub fn run(&mut self) -> Result { - self.run_blocking() - } - /// Executes the queued commands for the zone, and clears the /// current queued arguments. #[cfg(feature = "sync")] @@ -686,14 +672,6 @@ impl Config { Ok(out) } } -/// Returns the name of the current zone, if one exists. -#[cfg(feature = "sync")] -#[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'current_blocking', to use the blocking variant, or \n\ - - Call 'current' using the 'async' feature to call asynchronously.")] -pub fn current() -> Result { - current_blocking() -} /// Returns the name of the current zone, if one exists. #[cfg(feature = "sync")] @@ -853,14 +831,6 @@ impl Adm { } } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'boot_blocking', to use the blocking variant, or \n\ - - Call 'boot' using the 'async' feature to call asynchronously.")] - pub fn boot(&mut self) -> Result { - self.boot_blocking() - } - /// Boots (or activates) the zone. #[cfg(feature = "sync")] pub fn boot_blocking(&mut self) -> Result { @@ -872,14 +842,6 @@ impl Adm { self.run(&["boot"]).await } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'clone_blocking', to use the blocking variant, or \n\ - - Call 'clone' using the 'async' feature to call asynchronously.")] - pub fn clone(&mut self, source: impl AsRef) -> Result { - self.clone_blocking(source) - } - /// Installs a zone by copying an existing installed zone. #[cfg(feature = "sync")] pub fn clone_blocking(&mut self, source: impl AsRef) -> Result { @@ -891,14 +853,6 @@ impl Adm { self.run(&[OsStr::new("clone"), source.as_ref()]).await } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'halt_blocking', to use the blocking variant, or \n\ - - Call 'halt' using the 'async' feature to call asynchronously.")] - pub fn halt(&mut self) -> Result { - self.halt_blocking() - } - /// Halts the specified zone. #[cfg(feature = "sync")] pub fn halt_blocking(&mut self) -> Result { @@ -910,14 +864,6 @@ impl Adm { self.run(&["halt"]).await } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'mount_blocking', to use the blocking variant, or \n\ - - Call 'mount' using the 'async' feature to call asynchronously.")] - pub fn mount(&mut self) -> Result { - self.mount_blocking() - } - // TODO: Not documented in manpage, but apparently this exists. #[cfg(feature = "sync")] pub fn mount_blocking(&mut self) -> Result { @@ -929,14 +875,6 @@ impl Adm { self.run(&["mount"]).await } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'unmount_blocking', to use the blocking variant, or \n\ - - Call 'unmount' using the 'async' feature to call asynchronously.")] - pub fn unmount(&mut self) -> Result { - self.unmount_blocking() - } - // TODO: Not documented in manpage, but apparently this exists. #[cfg(feature = "sync")] pub fn unmount_blocking(&mut self) -> Result { @@ -948,14 +886,6 @@ impl Adm { self.run(&["unmount"]).await } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'install_blocking', to use the blocking variant, or \n\ - - Call 'install' using the 'async' feature to call asynchronously.")] - pub fn install(&mut self, brand_specific_options: &[&OsStr]) -> Result { - self.install_blocking(brand_specific_options) - } - /// Install the specified zone on the system. #[cfg(feature = "sync")] pub fn install_blocking( @@ -975,14 +905,6 @@ impl Adm { self.run([command, brand_specific_options].concat()).await } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'uninstall_blocking', to use the blocking variant, or \n\ - - Call 'uninstall' using the 'async' feature to call asynchronously.")] - pub fn uninstall(&mut self, force: bool) -> Result { - self.uninstall_blocking(force) - } - /// Uninstalls the zone from the system. #[cfg(feature = "sync")] pub fn uninstall_blocking(&mut self, force: bool) -> Result { @@ -1002,14 +924,6 @@ impl Adm { self.run(&args).await } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'list_blocking', to use the blocking variant, or \n\ - - Call 'list' using the 'async' feature to call asynchronously.")] - pub fn list() -> Result, ZoneError> { - Self::list_blocking() - } - /// List all zones. #[cfg(feature = "sync")] pub fn list_blocking() -> Result, ZoneError> { @@ -1092,14 +1006,6 @@ impl Zlogin { } } - #[cfg(feature = "sync")] - #[deprecated(note = "'zone' now provides asynchronous support. Please either: \n\ - - Call 'exec_blocking', to use the blocking variant, or \n\ - - Call 'exec' using the 'async' feature to call asynchronously.")] - pub fn exec(&self, cmd: impl AsRef) -> Result { - self.exec_blocking(cmd) - } - /// Executes a command in the zone and returns the result. #[cfg(feature = "sync")] pub fn exec_blocking(&self, cmd: impl AsRef) -> Result { From 80ece80e33b4cf5715d85b282e8eaf2219933468 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 11:00:57 -0700 Subject: [PATCH 02/10] Split out 'as_command' functions, publish 0.3.0 --- zone/Cargo.toml | 4 +- zone/src/lib.rs | 213 ++++++++++++++++++++++++------------- zone_cfg_derive/Cargo.toml | 2 +- 3 files changed, 145 insertions(+), 74 deletions(-) diff --git a/zone/Cargo.toml b/zone/Cargo.toml index 38f2451..e843285 100644 --- a/zone/Cargo.toml +++ b/zone/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "zone" -version = "0.2.0" +version = "0.3.0" authors = ["Sean Klein "] edition = "2018" repository = "https://github.com/oxidecomputer/zone" @@ -12,7 +12,7 @@ license = "MPL-2.0" [dependencies] thiserror = "1.0" itertools = "0.10" -zone_cfg_derive = { version = "0.2.0", path = "../zone_cfg_derive" } +zone_cfg_derive = { version = "0.3.0", path = "../zone_cfg_derive" } tokio = { version = "1.23", features = [ "process" ], optional = true } [features] diff --git a/zone/src/lib.rs b/zone/src/lib.rs index 380dc3b..f01773a 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -11,6 +11,7 @@ use itertools::Itertools; use std::collections::BTreeSet; use std::ffi::OsStr; use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; use std::str::FromStr; use std::string::ToString; use thiserror::Error; @@ -633,60 +634,69 @@ impl Config { self } - /// Executes the queued commands for the zone, and clears the - /// current queued arguments. - #[cfg(feature = "sync")] - pub fn run_blocking(&mut self) -> Result { + /// Creates a [Command] which can be executed to send the queued + /// command to the host. + /// + /// Addiitonally, clears the previously queued arguments. + pub fn as_command(&mut self) -> Command { let separator = ";".to_string(); let args = Itertools::intersperse(self.args.iter(), &separator); + let mut cmd = std::process::Command::new(PFEXEC); - let out = std::process::Command::new(PFEXEC) - .env_clear() + cmd.env_clear() .arg(ZONECFG) .arg("-z") .arg(&self.name) - .args(args) - .output() - .map_err(ZoneError::Command)? - .read_stdout()?; + .args(args); + self.args.clear(); + cmd + } + + /// Parses the output from a command which was emitted from + /// [Self::as_command]. + pub fn parse_output(output: &Output) -> Result { + let out = output.read_stdout()?; Ok(out) } + /// Executes the queued commands for the zone, and clears the + /// current queued arguments. + #[cfg(feature = "sync")] + pub fn run_blocking(&mut self) -> Result { + let output = self.as_command().output().map_err(ZoneError::Command)?; + Self::parse_output(&output) + } + #[cfg(feature = "async")] pub async fn run(&mut self) -> Result { - let separator = ";".to_string(); - let args = Itertools::intersperse(self.args.iter(), &separator); - - let out = tokio::process::Command::new(PFEXEC) - .env_clear() - .arg(ZONECFG) - .arg("-z") - .arg(&self.name) - .args(args) + let output = tokio::process::Command::from(self.as_command()) .output() .await - .map_err(ZoneError::Command)? - .read_stdout()?; - self.args.clear(); - Ok(out) + .map_err(ZoneError::Command)?; + Self::parse_output(&output) } } +pub fn current_command() -> Command { + let mut cmd = std::process::Command::new(ZONENAME); + cmd.env_clear(); + cmd +} + /// Returns the name of the current zone, if one exists. #[cfg(feature = "sync")] pub fn current_blocking() -> Result { - Ok(std::process::Command::new(ZONENAME) - .env_clear() + Ok(current_command() .output() .map_err(ZoneError::Command)? .read_stdout()?) } +/// Returns the name of the current zone, if one exists. #[cfg(feature = "async")] pub async fn current() -> Result { - Ok(tokio::process::Command::new(ZONENAME) - .env_clear() + Ok(tokio::process::Command::from(current_command()) .output() .await .map_err(ZoneError::Command)? @@ -831,6 +841,14 @@ impl Adm { } } + pub fn boot_command(&mut self) -> Command { + self.as_command(&["boot"]) + } + + pub fn parse_boot_output(output: &Output) -> Result { + Ok(output.read_stdout()?) + } + /// Boots (or activates) the zone. #[cfg(feature = "sync")] pub fn boot_blocking(&mut self) -> Result { @@ -842,6 +860,14 @@ impl Adm { self.run(&["boot"]).await } + pub fn clone_command(&mut self, source: impl AsRef) -> Command { + self.as_command(&[OsStr::new("clone"), source.as_ref()]) + } + + pub fn parse_clone_output(output: &Output) -> Result { + Ok(output.read_stdout()?) + } + /// Installs a zone by copying an existing installed zone. #[cfg(feature = "sync")] pub fn clone_blocking(&mut self, source: impl AsRef) -> Result { @@ -853,6 +879,14 @@ impl Adm { self.run(&[OsStr::new("clone"), source.as_ref()]).await } + pub fn halt_command(&mut self) -> Command { + self.as_command(&["halt"]) + } + + pub fn parse_halt_output(output: &Output) -> Result { + Ok(output.read_stdout()?) + } + /// Halts the specified zone. #[cfg(feature = "sync")] pub fn halt_blocking(&mut self) -> Result { @@ -864,6 +898,14 @@ impl Adm { self.run(&["halt"]).await } + pub fn mount_command(&mut self) -> Command { + self.as_command(&["mount"]) + } + + pub fn parse_mount_output(output: &Output) -> Result { + Ok(output.read_stdout()?) + } + // TODO: Not documented in manpage, but apparently this exists. #[cfg(feature = "sync")] pub fn mount_blocking(&mut self) -> Result { @@ -875,6 +917,14 @@ impl Adm { self.run(&["mount"]).await } + pub fn unmount_command(&mut self) -> Command { + self.as_command(&["unmount"]) + } + + pub fn parse_unmount_output(output: &Output) -> Result { + Ok(output.read_stdout()?) + } + // TODO: Not documented in manpage, but apparently this exists. #[cfg(feature = "sync")] pub fn unmount_blocking(&mut self) -> Result { @@ -886,6 +936,15 @@ impl Adm { self.run(&["unmount"]).await } + pub fn install_command(&mut self, brand_specific_options: &[&OsStr]) -> Command { + let command = &[OsStr::new("install")]; + self.as_command([command, brand_specific_options].concat()) + } + + pub fn parse_install_output(output: &Output) -> Result { + Ok(output.read_stdout()?) + } + /// Install the specified zone on the system. #[cfg(feature = "sync")] pub fn install_blocking( @@ -905,6 +964,18 @@ impl Adm { self.run([command, brand_specific_options].concat()).await } + pub fn uninstall_command(&mut self, force: bool) -> Command { + let mut args = vec!["uninstall"]; + if force { + args.push("-F"); + } + self.as_command(&args) + } + + pub fn parse_uninstall_output(output: &Output) -> Result { + Ok(output.read_stdout()?) + } + /// Uninstalls the zone from the system. #[cfg(feature = "sync")] pub fn uninstall_blocking(&mut self, force: bool) -> Result { @@ -924,36 +995,44 @@ impl Adm { self.run(&args).await } - /// List all zones. - #[cfg(feature = "sync")] - pub fn list_blocking() -> Result, ZoneError> { - std::process::Command::new(PFEXEC) - .env_clear() - .arg(ZONEADM) - .arg("list") - .arg("-cip") - .output() - .map_err(ZoneError::Command)? + pub fn list_command() -> Command { + let mut cmd = std::process::Command::new(PFEXEC); + cmd.env_clear().arg(ZONEADM).arg("list").arg("-cip"); + cmd + } + + pub fn parse_list_output(output: &Output) -> Result, ZoneError> { + output .read_stdout()? .split('\n') .map(|s| s.parse::()) .collect::, _>>() } + /// List all zones. + #[cfg(feature = "sync")] + pub fn list_blocking() -> Result, ZoneError> { + let output = Self::list_command().output().map_err(ZoneError::Command)?; + Self::parse_list_output(&output) + } + #[cfg(feature = "async")] pub async fn list() -> Result, ZoneError> { - tokio::process::Command::new(PFEXEC) - .env_clear() - .arg(ZONEADM) - .arg("list") - .arg("-cip") + let output = tokio::process::Command::from(Self::list_command()) .output() .await - .map_err(ZoneError::Command)? - .read_stdout()? - .split('\n') - .map(|s| s.parse::()) - .collect::, _>>() + .map_err(ZoneError::Command)?; + Self::parse_list_output(&output) + } + + fn as_command(&self, args: impl IntoIterator>) -> Command { + let mut cmd = std::process::Command::new(PFEXEC); + cmd.env_clear() + .arg(ZONEADM) + .arg("-z") + .arg(&self.name) + .args(args); + cmd } #[cfg(feature = "sync")] @@ -961,12 +1040,8 @@ impl Adm { &self, args: impl IntoIterator>, ) -> Result { - let out = std::process::Command::new(PFEXEC) - .env_clear() - .arg(ZONEADM) - .arg("-z") - .arg(&self.name) - .args(args) + let out = self + .as_command(args) .output() .map_err(ZoneError::Command)? .read_stdout()?; @@ -978,12 +1053,7 @@ impl Adm { &self, args: impl IntoIterator>, ) -> Result { - let out = tokio::process::Command::new(PFEXEC) - .env_clear() - .arg(ZONEADM) - .arg("-z") - .arg(&self.name) - .args(args) + let out = tokio::process::Command::from(self.as_command(args)) .output() .await .map_err(ZoneError::Command)? @@ -1006,15 +1076,21 @@ impl Zlogin { } } - /// Executes a command in the zone and returns the result. - #[cfg(feature = "sync")] - pub fn exec_blocking(&self, cmd: impl AsRef) -> Result { - Ok(std::process::Command::new(PFEXEC) - .env_clear() + pub fn as_command(&self, cmds: impl AsRef) -> Command { + let mut cmd = std::process::Command::new(PFEXEC); + cmd.env_clear() .arg(ZLOGIN) .arg("-Q") .arg(&self.name) - .arg(cmd) + .arg(cmds); + cmd + } + + /// Executes a command in the zone and returns the result. + #[cfg(feature = "sync")] + pub fn exec_blocking(&self, cmd: impl AsRef) -> Result { + Ok(self + .as_command(cmd) .output() .map_err(ZoneError::Command)? .read_stdout()?) @@ -1022,12 +1098,7 @@ impl Zlogin { #[cfg(feature = "async")] pub async fn exec(&self, cmd: impl AsRef) -> Result { - Ok(tokio::process::Command::new(PFEXEC) - .env_clear() - .arg(ZLOGIN) - .arg("-Q") - .arg(&self.name) - .arg(cmd) + Ok(tokio::process::Command::from(self.as_command(cmd)) .output() .await .map_err(ZoneError::Command)? diff --git a/zone_cfg_derive/Cargo.toml b/zone_cfg_derive/Cargo.toml index 6552105..75aa76a 100644 --- a/zone_cfg_derive/Cargo.toml +++ b/zone_cfg_derive/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "zone_cfg_derive" -version = "0.2.0" +version = "0.3.0" authors = ["Sean Klein "] edition = "2018" repository = "https://github.com/oxidecomputer/zone" From 6a1583bcb4e1ac7379f49bbe045306bb2e00c07d Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 11:03:36 -0700 Subject: [PATCH 03/10] Changelog --- CHANGELOG.adoc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 273b1bf..6a6f86a 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -5,6 +5,12 @@ = Zone Changelog +== 0.3.0 (released 2023-08-09) + +=== Breaking Changes + +* https://github.com/oxidecomputer/zone/pull/22[#22] Removes deprecated methods, and makes the `sync` and `async` features usable together. + == 0.2.0 (released 2022-11-14) === Breaking Changes From c93bb6bdb7cdb440f0e430c2e1021e5efd6d1906 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 11:04:48 -0700 Subject: [PATCH 04/10] Patch doctests --- zone/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index f01773a..4bccf56 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -532,7 +532,7 @@ pub struct Admin { /// }); /// /// // Issues the previously enqueued operations to zonecfg. -/// cfg.run().unwrap(); +/// cfg.run_blocking().unwrap(); /// ``` /// /// ## Selection and modification of an existing zone. @@ -550,7 +550,7 @@ pub struct Admin { /// cfg.remove_all_capped_memory(); /// /// // Issues the previously enqueued operations to zonecfg. -/// cfg.run().unwrap(); +/// cfg.run_blocking().unwrap(); /// ``` pub struct Config { /// Name of the zone. From 7aabdcfbc5730dee52ac085e0cb56599234f0a33 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 12:09:05 -0700 Subject: [PATCH 05/10] Fix tests --- zone/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index 4bccf56..7dc80a6 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -1113,7 +1113,7 @@ mod tests { #[cfg(target_os = "illumos")] #[test] fn test_current_zone() { - let zone = current().unwrap(); + let zone = current_blocking().unwrap(); assert_eq!("global", zone); } @@ -1279,14 +1279,14 @@ mod tests { ..Default::default() }); - cfg.run().unwrap(); - cfg.delete(true).run().unwrap(); + cfg.run_blocking().unwrap(); + cfg.delete(true).run_blocking().unwrap(); } #[cfg(target_os = "illumos")] #[test] fn test_list_global() { - let zones = Adm::list().unwrap(); + let zones = Adm::list_blocking().unwrap(); let global = zones.iter().find(|zone| zone.global()).unwrap(); @@ -1304,10 +1304,10 @@ mod tests { // Create a zone. let mut cfg = Config::create(name, true, CreationOptions::Default); cfg.get_global().set_path(path).set_autoboot(true); - cfg.run().unwrap(); + cfg.run_blocking().unwrap(); // Observe that the zone exists. - let zone = Adm::list() + let zone = Adm::list_blocking() .unwrap() .into_iter() .find(|z| z.name() == name) @@ -1315,10 +1315,10 @@ mod tests { assert_eq!(zone.path(), path); // Destroy the zone - cfg.delete(true).run().unwrap(); + cfg.delete(true).run_blocking().unwrap(); // Observe the zone does not exist - assert!(Adm::list() + assert!(Adm::list_blocking() .unwrap() .into_iter() .find(|z| z.name() == name) From 89734f46fbf96f728dd57e495e9b912103ce4611 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 15:44:36 -0700 Subject: [PATCH 06/10] Fix spacing, path to pfexec --- zone/src/lib.rs | 104 ++++++++++++++++++++++++------------- zone/tests/zlogin_test.rs | 2 +- zone_cfg_derive/src/lib.rs | 27 ++++++---- 3 files changed, 86 insertions(+), 47 deletions(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index 7dc80a6..3e158dd 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -17,7 +17,7 @@ use std::string::ToString; use thiserror::Error; use zone_cfg_derive::Resource; -const PFEXEC: &str = "/bin/pfexec"; +const PFEXEC: &str = "/usr/bin/pfexec"; const ZONENAME: &str = "/usr/bin/zonename"; const ZONEADM: &str = "/usr/sbin/zoneadm"; const ZONECFG: &str = "/usr/sbin/zonecfg"; @@ -597,22 +597,24 @@ impl Config { /// This method does not execute `zonecfg` until [`Config::run`] /// is invoked. pub fn create(name: impl AsRef, overwrite: bool, options: CreationOptions) -> Self { - let overwrite_flag = if overwrite { - "-F".to_string() - } else { - "".to_string() + let mut cfg = Self::new(name); + cfg.push("create"); + if overwrite { + cfg.push("-F"); }; - let options = match options { + match options { CreationOptions::FromDetached(path) => { - format!("-a {}", path.into_os_string().to_string_lossy()) + cfg.push("-a"); + cfg.push(path.into_os_string().to_string_lossy()); + } + CreationOptions::Blank => cfg.push("-b"), + CreationOptions::Default => (), + CreationOptions::Template(zone) => { + cfg.push("-t"); + cfg.push(zone); } - CreationOptions::Blank => "-b".to_string(), - CreationOptions::Default => "".to_string(), - CreationOptions::Template(zone) => format!("-t {}", zone), }; - let mut cfg = Self::new(name); - cfg.push(format!("create {} {}", overwrite_flag, options)); cfg } @@ -621,7 +623,9 @@ impl Config { /// This method does not execute `zonecfg` until [`Config::run`] /// is invoked. pub fn export(&mut self, p: impl AsRef) -> &mut Self { - self.push(format!("export -f {}", p.as_ref().to_string_lossy())); + self.push("export"); + self.push("-f"); + self.push(p.as_ref().to_string_lossy()); self } @@ -630,7 +634,10 @@ impl Config { /// This method does not execute `zonecfg` until [`Config::run`] /// is invoked. pub fn delete(&mut self, force: bool) -> &mut Self { - self.push(format!("delete {}", if force { "-F" } else { "" })); + self.push("delete"); + if force { + self.push("-F"); + } self } @@ -1141,18 +1148,27 @@ mod tests { cfg.args, &[ // Initial resource creation. - "add fs", - "set type=my-type", - "set dir=/path/to/dir", - "set special=/path/to/special", - "set options=[]", + "add", + "fs", + "set", + "type=my-type", + "set", + "dir=/path/to/dir", + "set", + "special=/path/to/special", + "set", + "options=[]", // Set mandatory field. - "set dir=/path/to/other/dir", + "set", + "dir=/path/to/other/dir", // Clear and set optional fied. - "clear raw", - "set raw=/raw", + "clear", + "raw", + "set", + "raw=/raw", // Set a list field. - "set options=[abc,def]", + "set", + "options=[abc,def]", "end" ] ); @@ -1172,10 +1188,14 @@ mod tests { assert_eq!( cfg.args, &[ - "add attr", - "set name=my-attr", - "set type=uint", - "set value=10", + "add", + "attr", + "set", + "name=my-attr", + "set", + "type=uint", + "set", + "value=10", "end" ] ); @@ -1202,9 +1222,12 @@ mod tests { assert_eq!( cfg.args, &[ - "add security-flags", - "set default=ASLR,FORBIDNULLMAP", - "set lower=ASLR", + "add", + "security-flags", + "set", + "default=ASLR,FORBIDNULLMAP", + "set", + "lower=ASLR", "end" ] ); @@ -1231,13 +1254,20 @@ mod tests { assert_eq!( cfg.args, &[ - "set zonename=my-new-zone", - "set autoboot=false", - "clear limitpriv", - "set fs-allowed=pcfs,ufs", - "set pool=my-pool", - "clear pool", - "set ip-type=exclusive", + "set", + "zonename=my-new-zone", + "set", + "autoboot=false", + "clear", + "limitpriv", + "set", + "fs-allowed=pcfs,ufs", + "set", + "pool=my-pool", + "clear", + "pool", + "set", + "ip-type=exclusive", ] ); } diff --git a/zone/tests/zlogin_test.rs b/zone/tests/zlogin_test.rs index d3d7ec4..c465a25 100644 --- a/zone/tests/zlogin_test.rs +++ b/zone/tests/zlogin_test.rs @@ -3,7 +3,7 @@ use std::path::Path; use zone::{Adm, Config, CreationOptions, Zlogin}; -const PFEXEC: &str = "/bin/pfexec"; +const PFEXEC: &str = "/usr/bin/pfexec"; // This test is marked with ignore because it involves booting a newly created // zone which can take over a minute. This test assumes that the sparse brand is diff --git a/zone_cfg_derive/src/lib.rs b/zone_cfg_derive/src/lib.rs index d14c5de..ea8485e 100644 --- a/zone_cfg_derive/src/lib.rs +++ b/zone_cfg_derive/src/lib.rs @@ -136,14 +136,16 @@ fn setters(scope_name: &Ident, parsed_fields: &Vec) -> proc_macro2: PropertyName::Implicit => #name, PropertyName::Explicit(name) => name, }; - self.push(format!("set {}={}", name, property.value)); + self.push("set"); + self.push(format!("{}={}", name, property.value)); } for property_name in value.get_clearables() { let name = match &property_name { PropertyName::Implicit => #name, PropertyName::Explicit(name) => name, }; - self.push(format!("clear {}", name)); + self.push("clear"); + self.push(name); } self } @@ -190,9 +192,10 @@ fn selectors(input_name: &Ident, parsed_fields: &Vec) -> proc_macro let mut scope = #scope_name { config: self }; + scope.push("select"); + scope.push(#input_name_kebab); scope.push( - format!("select {} {}={}", - #input_name_kebab, + format!("{}={}", #name, value, ) @@ -203,10 +206,12 @@ fn selectors(input_name: &Ident, parsed_fields: &Vec) -> proc_macro #[doc = #remover_msg] pub fn #remover(&mut self, value: impl Into<#ty>) { let value: #ty = value.into(); + self.push("remove"); + self.push("-F"); + self.push(#input_name_kebab); self.push( format!( - "remove -F {} {}={}", - #input_name_kebab, + "{}={}", #name, value, ) @@ -249,7 +254,8 @@ fn constructor( PropertyName::Implicit => #name, PropertyName::Explicit(name) => name, }; - scope.push(format!("set {}={}", name, property.value)); + scope.push("set"); + scope.push(format!("{}={}", name, property.value)); } } }) @@ -310,14 +316,17 @@ fn constructor( config: self }; - scope.push(format!("add {}", #input_name_kebab)); + scope.push("add"); + scope.push(#input_name_kebab); #initial_set_values scope } #[doc = #scope_removal_msg] pub fn #scope_removal(&mut self) { - self.push(format!("remove -F {}", #input_name_kebab)); + self.push("remove"); + self.push("-F"); + self.push(#input_name_kebab); } } } From 903fab6f07f92f7085db971ee9fee9242a5fc140 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 15:49:42 -0700 Subject: [PATCH 07/10] Revert "Fix spacing, path to pfexec" This reverts commit 89734f46fbf96f728dd57e495e9b912103ce4611. --- zone/src/lib.rs | 104 +++++++++++++------------------------ zone/tests/zlogin_test.rs | 2 +- zone_cfg_derive/src/lib.rs | 27 ++++------ 3 files changed, 47 insertions(+), 86 deletions(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index 3e158dd..7dc80a6 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -17,7 +17,7 @@ use std::string::ToString; use thiserror::Error; use zone_cfg_derive::Resource; -const PFEXEC: &str = "/usr/bin/pfexec"; +const PFEXEC: &str = "/bin/pfexec"; const ZONENAME: &str = "/usr/bin/zonename"; const ZONEADM: &str = "/usr/sbin/zoneadm"; const ZONECFG: &str = "/usr/sbin/zonecfg"; @@ -597,24 +597,22 @@ impl Config { /// This method does not execute `zonecfg` until [`Config::run`] /// is invoked. pub fn create(name: impl AsRef, overwrite: bool, options: CreationOptions) -> Self { - let mut cfg = Self::new(name); - cfg.push("create"); - if overwrite { - cfg.push("-F"); + let overwrite_flag = if overwrite { + "-F".to_string() + } else { + "".to_string() }; - match options { + let options = match options { CreationOptions::FromDetached(path) => { - cfg.push("-a"); - cfg.push(path.into_os_string().to_string_lossy()); - } - CreationOptions::Blank => cfg.push("-b"), - CreationOptions::Default => (), - CreationOptions::Template(zone) => { - cfg.push("-t"); - cfg.push(zone); + format!("-a {}", path.into_os_string().to_string_lossy()) } + CreationOptions::Blank => "-b".to_string(), + CreationOptions::Default => "".to_string(), + CreationOptions::Template(zone) => format!("-t {}", zone), }; + let mut cfg = Self::new(name); + cfg.push(format!("create {} {}", overwrite_flag, options)); cfg } @@ -623,9 +621,7 @@ impl Config { /// This method does not execute `zonecfg` until [`Config::run`] /// is invoked. pub fn export(&mut self, p: impl AsRef) -> &mut Self { - self.push("export"); - self.push("-f"); - self.push(p.as_ref().to_string_lossy()); + self.push(format!("export -f {}", p.as_ref().to_string_lossy())); self } @@ -634,10 +630,7 @@ impl Config { /// This method does not execute `zonecfg` until [`Config::run`] /// is invoked. pub fn delete(&mut self, force: bool) -> &mut Self { - self.push("delete"); - if force { - self.push("-F"); - } + self.push(format!("delete {}", if force { "-F" } else { "" })); self } @@ -1148,27 +1141,18 @@ mod tests { cfg.args, &[ // Initial resource creation. - "add", - "fs", - "set", - "type=my-type", - "set", - "dir=/path/to/dir", - "set", - "special=/path/to/special", - "set", - "options=[]", + "add fs", + "set type=my-type", + "set dir=/path/to/dir", + "set special=/path/to/special", + "set options=[]", // Set mandatory field. - "set", - "dir=/path/to/other/dir", + "set dir=/path/to/other/dir", // Clear and set optional fied. - "clear", - "raw", - "set", - "raw=/raw", + "clear raw", + "set raw=/raw", // Set a list field. - "set", - "options=[abc,def]", + "set options=[abc,def]", "end" ] ); @@ -1188,14 +1172,10 @@ mod tests { assert_eq!( cfg.args, &[ - "add", - "attr", - "set", - "name=my-attr", - "set", - "type=uint", - "set", - "value=10", + "add attr", + "set name=my-attr", + "set type=uint", + "set value=10", "end" ] ); @@ -1222,12 +1202,9 @@ mod tests { assert_eq!( cfg.args, &[ - "add", - "security-flags", - "set", - "default=ASLR,FORBIDNULLMAP", - "set", - "lower=ASLR", + "add security-flags", + "set default=ASLR,FORBIDNULLMAP", + "set lower=ASLR", "end" ] ); @@ -1254,20 +1231,13 @@ mod tests { assert_eq!( cfg.args, &[ - "set", - "zonename=my-new-zone", - "set", - "autoboot=false", - "clear", - "limitpriv", - "set", - "fs-allowed=pcfs,ufs", - "set", - "pool=my-pool", - "clear", - "pool", - "set", - "ip-type=exclusive", + "set zonename=my-new-zone", + "set autoboot=false", + "clear limitpriv", + "set fs-allowed=pcfs,ufs", + "set pool=my-pool", + "clear pool", + "set ip-type=exclusive", ] ); } diff --git a/zone/tests/zlogin_test.rs b/zone/tests/zlogin_test.rs index c465a25..d3d7ec4 100644 --- a/zone/tests/zlogin_test.rs +++ b/zone/tests/zlogin_test.rs @@ -3,7 +3,7 @@ use std::path::Path; use zone::{Adm, Config, CreationOptions, Zlogin}; -const PFEXEC: &str = "/usr/bin/pfexec"; +const PFEXEC: &str = "/bin/pfexec"; // This test is marked with ignore because it involves booting a newly created // zone which can take over a minute. This test assumes that the sparse brand is diff --git a/zone_cfg_derive/src/lib.rs b/zone_cfg_derive/src/lib.rs index ea8485e..d14c5de 100644 --- a/zone_cfg_derive/src/lib.rs +++ b/zone_cfg_derive/src/lib.rs @@ -136,16 +136,14 @@ fn setters(scope_name: &Ident, parsed_fields: &Vec) -> proc_macro2: PropertyName::Implicit => #name, PropertyName::Explicit(name) => name, }; - self.push("set"); - self.push(format!("{}={}", name, property.value)); + self.push(format!("set {}={}", name, property.value)); } for property_name in value.get_clearables() { let name = match &property_name { PropertyName::Implicit => #name, PropertyName::Explicit(name) => name, }; - self.push("clear"); - self.push(name); + self.push(format!("clear {}", name)); } self } @@ -192,10 +190,9 @@ fn selectors(input_name: &Ident, parsed_fields: &Vec) -> proc_macro let mut scope = #scope_name { config: self }; - scope.push("select"); - scope.push(#input_name_kebab); scope.push( - format!("{}={}", + format!("select {} {}={}", + #input_name_kebab, #name, value, ) @@ -206,12 +203,10 @@ fn selectors(input_name: &Ident, parsed_fields: &Vec) -> proc_macro #[doc = #remover_msg] pub fn #remover(&mut self, value: impl Into<#ty>) { let value: #ty = value.into(); - self.push("remove"); - self.push("-F"); - self.push(#input_name_kebab); self.push( format!( - "{}={}", + "remove -F {} {}={}", + #input_name_kebab, #name, value, ) @@ -254,8 +249,7 @@ fn constructor( PropertyName::Implicit => #name, PropertyName::Explicit(name) => name, }; - scope.push("set"); - scope.push(format!("{}={}", name, property.value)); + scope.push(format!("set {}={}", name, property.value)); } } }) @@ -316,17 +310,14 @@ fn constructor( config: self }; - scope.push("add"); - scope.push(#input_name_kebab); + scope.push(format!("add {}", #input_name_kebab)); #initial_set_values scope } #[doc = #scope_removal_msg] pub fn #scope_removal(&mut self) { - self.push("remove"); - self.push("-F"); - self.push(#input_name_kebab); + self.push(format!("remove -F {}", #input_name_kebab)); } } } From 79e399cb6fd04b90cf3d9d69064d603dff81560b Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 15:50:26 -0700 Subject: [PATCH 08/10] Patch pfexec path again --- zone/src/lib.rs | 2 +- zone/tests/zlogin_test.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index 7dc80a6..6b74db9 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -17,7 +17,7 @@ use std::string::ToString; use thiserror::Error; use zone_cfg_derive::Resource; -const PFEXEC: &str = "/bin/pfexec"; +const PFEXEC: &str = "/usr/bin/pfexec"; const ZONENAME: &str = "/usr/bin/zonename"; const ZONEADM: &str = "/usr/sbin/zoneadm"; const ZONECFG: &str = "/usr/sbin/zonecfg"; diff --git a/zone/tests/zlogin_test.rs b/zone/tests/zlogin_test.rs index d3d7ec4..c465a25 100644 --- a/zone/tests/zlogin_test.rs +++ b/zone/tests/zlogin_test.rs @@ -3,7 +3,7 @@ use std::path::Path; use zone::{Adm, Config, CreationOptions, Zlogin}; -const PFEXEC: &str = "/bin/pfexec"; +const PFEXEC: &str = "/usr/bin/pfexec"; // This test is marked with ignore because it involves booting a newly created // zone which can take over a minute. This test assumes that the sparse brand is From c75fd99d4f0af8ef00d6405072c18ac01aa53901 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 9 Aug 2023 16:16:30 -0700 Subject: [PATCH 09/10] Split args between ';' --- zone/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index 6b74db9..b9635f6 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -640,7 +640,10 @@ impl Config { /// Addiitonally, clears the previously queued arguments. pub fn as_command(&mut self) -> Command { let separator = ";".to_string(); - let args = Itertools::intersperse(self.args.iter(), &separator); + let args = Itertools::intersperse(self.args.iter(), &separator) + .flat_map(|arg| { + arg.split(' ') + }); let mut cmd = std::process::Command::new(PFEXEC); cmd.env_clear() From 96cf031b238e9f7d64c1e78561c788ee2b274240 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 10 Aug 2023 13:42:22 -0700 Subject: [PATCH 10/10] fmt --- zone/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/zone/src/lib.rs b/zone/src/lib.rs index b9635f6..6127f27 100644 --- a/zone/src/lib.rs +++ b/zone/src/lib.rs @@ -640,10 +640,8 @@ impl Config { /// Addiitonally, clears the previously queued arguments. pub fn as_command(&mut self) -> Command { let separator = ";".to_string(); - let args = Itertools::intersperse(self.args.iter(), &separator) - .flat_map(|arg| { - arg.split(' ') - }); + let args = + Itertools::intersperse(self.args.iter(), &separator).flat_map(|arg| arg.split(' ')); let mut cmd = std::process::Command::new(PFEXEC); cmd.env_clear()