Centralise IP address object address retrieval and existence checks. (#…

…6204)

Following [illumos 16677](https://www.illumos.org/issues/16677), the
error message that indicates that
an address object does not exist has been changed to be
consistent, whereas it was previously one of two different messages
or sometimes even completely blank.

A couple of places in omicron relied on the specific error
message so this change centralises the check, and updates it
to cater for illumos pre- and post-16677. Longer term this
could be replaced by bindings to libipadm.

illumos-utils/src/ipadm.rs

@@ -7,17 +7,31 @@
 use crate::addrobj::{IPV6_LINK_LOCAL_ADDROBJ_NAME, IPV6_STATIC_ADDROBJ_NAME};
 use crate::zone::IPADM;
 use crate::{execute, ExecutionError, PFEXEC};
-use std::net::Ipv6Addr;
+use oxnet::IpNet;
+use std::net::{IpAddr, Ipv6Addr};
 
 /// Wraps commands for interacting with interfaces.
 pub struct Ipadm {}
 
 /// Expected error message contents when showing an addrobj that doesn't exist.
-const ADDROBJ_NOT_FOUND_ERR: &str = "Address object not found";
+// The message changed to be consistent regardless of the state of the
+// system in illumos 16677. It is now always `ERR1` below. Prior to that, it
+// would most often be `ERR2` but could sometimes be blank or `ERR1`.
+const ADDROBJ_NOT_FOUND_ERR1: &str = "address: Object not found";
+const ADDROBJ_NOT_FOUND_ERR2: &str = "Address object not found";
 
 /// Expected error message when an interface already exists.
 const INTERFACE_ALREADY_EXISTS: &str = "Interface already exists";
 
+/// Expected error message when an addrobj already exists.
+const ADDROBJ_ALREADY_EXISTS: &str = "Address object already exists";
+
+pub enum AddrObjType {
+    DHCP,
+    AddrConf,
+    Static(IpAddr),
+}
+
 #[cfg_attr(any(test, feature = "testing"), mockall::automock)]
 impl Ipadm {
     /// Ensure that an IP interface exists on the provided datalink.
@@ -37,6 +51,96 @@ impl Ipadm {
         }
     }
 
+    /// Create an address object with the provided parameters. If an object
+    /// with the requested name already exists, return success. Note that in
+    /// this case, the existing object is not checked to ensure it is
+    /// consistent with the provided parameters.
+    pub fn ensure_ip_addrobj_exists(
+        addrobj: &str,
+        addrtype: AddrObjType,
+    ) -> Result<(), ExecutionError> {
+        let mut cmd = std::process::Command::new(PFEXEC);
+        let cmd = cmd.args(&[IPADM, "create-addr", "-t", "-T"]);
+        let cmd = match addrtype {
+            AddrObjType::DHCP => cmd.args(&["dhcp"]),
+            AddrObjType::AddrConf => cmd.args(&["addrconf"]),
+            AddrObjType::Static(addr) => {
+                cmd.args(&["static", "-a", &addr.to_string()])
+            }
+        };
+        let cmd = cmd.arg(&addrobj);
+        match execute(cmd) {
+            Ok(_) => Ok(()),
+            Err(ExecutionError::CommandFailure(info))
+                if info.stderr.contains(ADDROBJ_ALREADY_EXISTS) =>
+            {
+                Ok(())
+            }
+            Err(e) => Err(e),
+        }
+    }
+
+    /// Remove any scope from an IPv6 address.
+    /// e.g. fe80::8:20ff:fed0:8687%oxControlService1/10 ->
+    ///      fe80::8:20ff:fed0:8687/10
+    fn remove_addr_scope(input: &str) -> String {
+        if let Some(pos) = input.find('%') {
+            let (base, rest) = input.split_at(pos);
+            if let Some(slash_pos) = rest.find('/') {
+                format!("{}{}", base, &rest[slash_pos..])
+            } else {
+                base.to_string()
+            }
+        } else {
+            input.to_string()
+        }
+    }
+
+    /// Return the IP network associated with an address object, or None if
+    /// there is no address object with this name.
+    pub fn addrobj_addr(
+        addrobj: &str,
+    ) -> Result<Option<IpNet>, ExecutionError> {
+        // Note that additional privileges are not required to list address
+        // objects, and so there is no `pfexec` here.
+        let mut cmd = std::process::Command::new(IPADM);
+        let cmd = cmd.args(&["show-addr", "-po", "addr", addrobj]);
+        match execute(cmd) {
+            Err(ExecutionError::CommandFailure(info))
+                if [ADDROBJ_NOT_FOUND_ERR1, ADDROBJ_NOT_FOUND_ERR2]
+                    .iter()
+                    .any(|&ss| info.stderr.contains(ss)) =>
+            {
+                // The address object does not exist.
+                Ok(None)
+            }
+            Err(e) => Err(e),
+            Ok(output) => {
+                let out = std::str::from_utf8(&output.stdout).map_err(|e| {
+                    let s = String::from_utf8_lossy(&output.stdout);
+                    ExecutionError::ParseFailure(format!("{}: {}", e, s))
+                })?;
+                let lines: Vec<_> = out.trim().lines().collect();
+                if lines.is_empty() {
+                    return Ok(None);
+                }
+                match Self::remove_addr_scope(lines[0].trim()).parse() {
+                    Ok(ipnet) => Ok(Some(ipnet)),
+                    Err(e) => Err(ExecutionError::ParseFailure(format!(
+                        "{}: {}",
+                        lines[0].trim(),
+                        e
+                    ))),
+                }
+            }
+        }
+    }
+
+    /// Determine if a named address object exists
+    pub fn addrobj_exists(addrobj: &str) -> Result<bool, ExecutionError> {
+        Ok(Self::addrobj_addr(addrobj)?.is_some())
+    }
+
     // Set MTU to 9000 on both IPv4 and IPv6
     pub fn set_interface_mtu(datalink: &str) -> Result<(), ExecutionError> {
         let mut cmd = std::process::Command::new(PFEXEC);
@@ -71,77 +175,26 @@ impl Ipadm {
         datalink: &str,
         listen_addr: &Ipv6Addr,
     ) -> Result<(), ExecutionError> {
-        // Create auto-configured address on the IP interface if it doesn't already exist
+        // Create auto-configured address on the IP interface if it doesn't
+        // already exist
         let addrobj = format!("{}/{}", datalink, IPV6_LINK_LOCAL_ADDROBJ_NAME);
-        let mut cmd = std::process::Command::new(PFEXEC);
-        let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]);
-        match execute(cmd) {
-            Err(ExecutionError::CommandFailure(info))
-                if info.stderr.contains(ADDROBJ_NOT_FOUND_ERR) =>
-            {
-                let mut cmd = std::process::Command::new(PFEXEC);
-                let cmd = cmd.args(&[
-                    IPADM,
-                    "create-addr",
-                    "-t",
-                    "-T",
-                    "addrconf",
-                    &addrobj,
-                ]);
-                execute(cmd)?;
-            }
-            Err(other) => return Err(other),
-            Ok(_) => (),
-        };
+        Self::ensure_ip_addrobj_exists(&addrobj, AddrObjType::AddrConf)?;
 
         // Create static address on the IP interface if it doesn't already exist
         let addrobj = format!("{}/{}", datalink, IPV6_STATIC_ADDROBJ_NAME);
-        let mut cmd = std::process::Command::new(PFEXEC);
-        let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]);
-        match execute(cmd) {
-            Err(ExecutionError::CommandFailure(info))
-                if info.stderr.contains(ADDROBJ_NOT_FOUND_ERR) =>
-            {
-                let mut cmd = std::process::Command::new(PFEXEC);
-                let cmd = cmd.args(&[
-                    IPADM,
-                    "create-addr",
-                    "-t",
-                    "-T",
-                    "static",
-                    "-a",
-                    &listen_addr.to_string(),
-                    &addrobj,
-                ]);
-                execute(cmd).map(|_| ())
-            }
-            Err(other) => Err(other),
-            Ok(_) => Ok(()),
-        }
+        Self::ensure_ip_addrobj_exists(
+            &addrobj,
+            AddrObjType::Static((*listen_addr).into()),
+        )?;
+        Ok(())
     }
 
     // Create gateway on the IP interface if it doesn't already exist
     pub fn create_opte_gateway(
         opte_iface: &String,
     ) -> Result<(), ExecutionError> {
         let addrobj = format!("{}/public", opte_iface);
-        let mut cmd = std::process::Command::new(PFEXEC);
-        let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]);
-        match execute(cmd) {
-            Err(_) => {
-                let mut cmd = std::process::Command::new(PFEXEC);
-                let cmd = cmd.args(&[
-                    IPADM,
-                    "create-addr",
-                    "-t",
-                    "-T",
-                    "dhcp",
-                    &addrobj,
-                ]);
-                execute(cmd)?;
-            }
-            Ok(_) => (),
-        };
+        Self::ensure_ip_addrobj_exists(&addrobj, AddrObjType::DHCP)?;
         Ok(())
     }
 }

illumos-utils/src/lib.rs

@@ -64,6 +64,9 @@ pub enum ExecutionError {
     #[error("Failed to manipulate process contract: {err}")]
     ContractFailure { err: std::io::Error },
 
+    #[error("Failed to parse command output")]
+    ParseFailure(String),
+
     #[error("Zone is not running")]
     NotRunning,
 }

sled-agent/src/bin/zone-bundle.rs

@@ -246,54 +246,25 @@ async fn fetch_underlay_address() -> anyhow::Result<Ipv6Addr> {
     return Ok(Ipv6Addr::LOCALHOST);
     #[cfg(target_os = "illumos")]
     {
+        use illumos_utils::ipadm::Ipadm;
+        use std::net::IpAddr;
         const EXPECTED_ADDR_OBJ: &str = "underlay0/sled6";
-        let output = Command::new("ipadm")
-            .arg("show-addr")
-            .arg("-p")
-            .arg("-o")
-            .arg("addr")
-            .arg(EXPECTED_ADDR_OBJ)
-            .output()
-            .await?;
-        // If we failed because there was no such interface, then fall back to
-        // localhost.
-        if !output.status.success() {
-            match std::str::from_utf8(&output.stderr) {
-                Err(_) => bail!(
-                    "ipadm command failed unexpectedly, stderr:\n{}",
-                    String::from_utf8_lossy(&output.stderr)
+        match Ipadm::addrobj_addr(EXPECTED_ADDR_OBJ) {
+            // If we failed because there was no such interface, then fall back
+            // to localhost.
+            Ok(None) => Ok(Ipv6Addr::LOCALHOST),
+            Ok(Some(addr)) => match addr.addr() {
+                IpAddr::V6(ipv6) => Ok(ipv6),
+                IpAddr::V4(ipv4) => bail!(
+                    "Unexpectedly got IPv4 address for {}: {}",
+                    EXPECTED_ADDR_OBJ,
+                    ipv4
                 ),
-                Ok(out) => {
-                    if out.contains("Address object not found") {
-                        eprintln!(
-                            "Expected addrobj '{}' not found, using localhost",
-                            EXPECTED_ADDR_OBJ,
-                        );
-                        return Ok(Ipv6Addr::LOCALHOST);
-                    } else {
-                        bail!(
-                            "ipadm subcommand failed unexpectedly, stderr:\n{}",
-                            String::from_utf8_lossy(&output.stderr),
-                        );
-                    }
-                }
-            }
+            },
+            Err(e) => bail!(
+                "failed to get address for addrobj {EXPECTED_ADDR_OBJ}: {e}",
+            ),
         }
-        let out = std::str::from_utf8(&output.stdout)
-            .context("non-UTF8 output in ipadm")?;
-        let lines: Vec<_> = out.trim().lines().collect();
-        anyhow::ensure!(
-            lines.len() == 1,
-            "No addresses or more than one address on expected interface '{}'",
-            EXPECTED_ADDR_OBJ
-        );
-        lines[0]
-            .trim()
-            .split_once('/')
-            .context("expected a /64 subnet")?
-            .0
-            .parse()
-            .context("invalid IPv6 address")
     }
 }