[common] switch nexus config to using camino paths (#4831)

In #4690 I was dealing with some of these paths, and conversion to and
fro is a bit annoying.

There is no loss of generality here because all of these paths are read
from a config file -- and are therefore expected to be UTF-8 anyway.

common/src/nexus_config.rs

@@ -11,6 +11,7 @@ use crate::api::internal::shared::SwitchLocation;
 use super::address::{Ipv6Subnet, RACK_PREFIX};
 use super::postgres_config::PostgresConfigWithUrl;
 use anyhow::anyhow;
+use camino::{Utf8Path, Utf8PathBuf};
 use dropshot::ConfigDropshot;
 use dropshot::ConfigLogging;
 use schemars::JsonSchema;
@@ -24,13 +25,12 @@ use std::collections::HashMap;
 use std::fmt;
 use std::net::IpAddr;
 use std::net::SocketAddr;
-use std::path::{Path, PathBuf};
 use std::time::Duration;
 use uuid::Uuid;
 
 #[derive(Debug)]
 pub struct LoadError {
-    pub path: PathBuf,
+    pub path: Utf8PathBuf,
     pub kind: LoadErrorKind,
 }
 
@@ -54,14 +54,14 @@ pub enum LoadErrorKind {
     InvalidTunable(InvalidTunable),
 }
 
-impl From<(PathBuf, std::io::Error)> for LoadError {
-    fn from((path, err): (PathBuf, std::io::Error)) -> Self {
+impl From<(Utf8PathBuf, std::io::Error)> for LoadError {
+    fn from((path, err): (Utf8PathBuf, std::io::Error)) -> Self {
         LoadError { path, kind: LoadErrorKind::Io(err) }
     }
 }
 
-impl From<(PathBuf, toml::de::Error)> for LoadError {
-    fn from((path, err): (PathBuf, toml::de::Error)) -> Self {
+impl From<(Utf8PathBuf, toml::de::Error)> for LoadError {
+    fn from((path, err): (Utf8PathBuf, toml::de::Error)) -> Self {
         LoadError { path, kind: LoadErrorKind::Parse(err) }
     }
 }
@@ -72,18 +72,13 @@ impl fmt::Display for LoadError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match &self.kind {
             LoadErrorKind::Io(e) => {
-                write!(f, "read \"{}\": {}", self.path.display(), e)
+                write!(f, "read \"{}\": {}", self.path, e)
             }
             LoadErrorKind::Parse(e) => {
-                write!(f, "parse \"{}\": {}", self.path.display(), e.message())
+                write!(f, "parse \"{}\": {}", self.path, e.message())
             }
             LoadErrorKind::InvalidTunable(inner) => {
-                write!(
-                    f,
-                    "invalid tunable \"{}\": {}",
-                    self.path.display(),
-                    inner,
-                )
+                write!(f, "invalid tunable \"{}\": {}", self.path, inner)
             }
         }
     }
@@ -170,7 +165,7 @@ impl DeploymentConfig {
     ///
     /// This config object can then be used to create a new `Nexus`.
     /// The format is described in the README.
-    pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Self, LoadError> {
+    pub fn from_file<P: AsRef<Utf8Path>>(path: P) -> Result<Self, LoadError> {
         let path = path.as_ref();
         let file_contents = std::fs::read_to_string(path)
             .map_err(|e| (path.to_path_buf(), e))?;
@@ -207,7 +202,7 @@ pub struct AuthnConfig {
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct ConsoleConfig {
-    pub static_dir: PathBuf,
+    pub static_dir: Utf8PathBuf,
     /// how long a session can be idle before expiring
     pub session_idle_timeout_minutes: u32,
     /// how long a session can exist before expiring
@@ -217,15 +212,15 @@ pub struct ConsoleConfig {
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct UpdatesConfig {
     /// Trusted root.json role for the TUF updates repository.
-    pub trusted_root: PathBuf,
+    pub trusted_root: Utf8PathBuf,
     /// Default base URL for the TUF repository.
     pub default_base_url: String,
 }
 
 /// Options to tweak database schema changes.
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct SchemaConfig {
-    pub schema_dir: PathBuf,
+    pub schema_dir: Utf8PathBuf,
 }
 
 /// Optional configuration for the timeseries database.
@@ -464,7 +459,7 @@ impl Config {
     ///
     /// This config object can then be used to create a new `Nexus`.
     /// The format is described in the README.
-    pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Self, LoadError> {
+    pub fn from_file<P: AsRef<Utf8Path>>(path: P) -> Result<Self, LoadError> {
         let path = path.as_ref();
         let file_contents = std::fs::read_to_string(path)
             .map_err(|e| (path.to_path_buf(), e))?;
@@ -524,6 +519,7 @@ mod test {
     };
     use crate::address::{Ipv6Subnet, RACK_PREFIX};
     use crate::api::internal::shared::SwitchLocation;
+    use camino::{Utf8Path, Utf8PathBuf};
     use dropshot::ConfigDropshot;
     use dropshot::ConfigLogging;
     use dropshot::ConfigLoggingIfExists;
@@ -532,21 +528,18 @@ mod test {
     use std::collections::HashMap;
     use std::fs;
     use std::net::{Ipv6Addr, SocketAddr};
-    use std::path::Path;
-    use std::path::PathBuf;
     use std::str::FromStr;
     use std::time::Duration;
 
     /// Generates a temporary filesystem path unique for the given label.
-    fn temp_path(label: &str) -> PathBuf {
+    fn temp_path(label: &str) -> Utf8PathBuf {
         let arg0str = std::env::args().next().expect("expected process arg0");
-        let arg0 = Path::new(&arg0str)
+        let arg0 = Utf8Path::new(&arg0str)
             .file_name()
-            .expect("expected arg0 filename")
-            .to_str()
-            .expect("expected arg0 filename to be valid Unicode");
+            .expect("expected arg0 filename");
         let pid = std::process::id();
-        let mut pathbuf = std::env::temp_dir();
+        let mut pathbuf = Utf8PathBuf::try_from(std::env::temp_dir())
+            .expect("expected temp dir to be valid UTF-8");
         pathbuf.push(format!("{}.{}.{}", arg0, pid, label));
         pathbuf
     }
@@ -559,7 +552,7 @@ mod test {
     fn read_config(label: &str, contents: &str) -> Result<Config, LoadError> {
         let pathbuf = temp_path(label);
         let path = pathbuf.as_path();
-        eprintln!("writing test config {}", path.display());
+        eprintln!("writing test config {}", path);
         fs::write(path, contents).expect("write to tempfile failed");
 
         let result = Config::from_file(path);
@@ -572,7 +565,7 @@ mod test {
 
     #[test]
     fn test_config_nonexistent() {
-        let error = Config::from_file(Path::new("/nonexistent"))
+        let error = Config::from_file(Utf8Path::new("/nonexistent"))
             .expect_err("expected config to fail from /nonexistent");
         let expected = std::io::Error::from_raw_os_error(libc::ENOENT);
         assert_eq!(error, expected);
@@ -734,7 +727,7 @@ mod test {
                         address: Some("[::1]:8123".parse().unwrap())
                     },
                     updates: Some(UpdatesConfig {
-                        trusted_root: PathBuf::from("/path/to/root.json"),
+                        trusted_root: Utf8PathBuf::from("/path/to/root.json"),
                         default_base_url: "http://example.invalid/".into(),
                     }),
                     schema: None,

nexus/Cargo.toml

@@ -88,6 +88,7 @@ omicron-workspace-hack.workspace = true
 
 [dev-dependencies]
 async-bb8-diesel.workspace = true
+camino-tempfile.workspace = true
 criterion.workspace = true
 diesel.workspace = true
 dns-server.workspace = true

nexus/db-queries/src/db/datastore/db_metadata.rs

@@ -210,7 +210,7 @@ impl DataStore {
         // - Look in the schema directory for all the changes, in-order, to
         // migrate from our current version to the desired version.
 
-        info!(log, "Reading schemas from {}", config.schema_dir.display());
+        info!(log, "Reading schemas from {}", config.schema_dir);
         let mut dir = tokio::fs::read_dir(&config.schema_dir)
             .await
             .map_err(|e| format!("Failed to read schema config dir: {e}"))?;
@@ -239,14 +239,14 @@ impl DataStore {
         if !all_versions.contains(&current_version) {
             return Err(format!(
                 "Current DB version {current_version} was not found in {}",
-                config.schema_dir.display()
+                config.schema_dir
             ));
         }
         // TODO: Test this?
         if !all_versions.contains(&desired_version) {
             return Err(format!(
                 "Target DB version {desired_version} was not found in {}",
-                config.schema_dir.display()
+                config.schema_dir
             ));
         }
 
@@ -263,10 +263,7 @@ impl DataStore {
                 "target_version" => target_version.to_string(),
             );
 
-            let target_dir = Utf8PathBuf::from_path_buf(
-                config.schema_dir.join(target_version.to_string()),
-            )
-            .map_err(|e| format!("Invalid schema path: {}", e.display()))?;
+            let target_dir = config.schema_dir.join(target_version.to_string());
 
             let schema_change =
                 all_sql_for_version_migration(&target_dir).await?;
@@ -709,9 +706,8 @@ mod test {
             .await;
 
         // Show that the datastores can be created concurrently.
-        let config = SchemaConfig {
-            schema_dir: config_dir.path().to_path_buf().into_std_path_buf(),
-        };
+        let config =
+            SchemaConfig { schema_dir: config_dir.path().to_path_buf() };
         let _ = futures::future::join_all((0..10).map(|_| {
             let log = log.clone();
             let pool = pool.clone();

nexus/src/bin/nexus.rs

@@ -11,14 +11,14 @@
 //   omicron#2184, omicron#2414.
 
 use anyhow::anyhow;
+use camino::Utf8PathBuf;
 use clap::Parser;
 use omicron_common::cmd::fatal;
 use omicron_common::cmd::CmdError;
 use omicron_nexus::run_openapi_external;
 use omicron_nexus::run_openapi_internal;
 use omicron_nexus::run_server;
 use omicron_nexus::Config;
-use std::path::PathBuf;
 
 #[derive(Debug, Parser)]
 #[clap(name = "nexus", about = "See README.adoc for more information")]
@@ -41,7 +41,7 @@ struct Args {
     openapi_internal: bool,
 
     #[clap(name = "CONFIG_FILE_PATH", action)]
-    config_file_path: Option<PathBuf>,
+    config_file_path: Option<Utf8PathBuf>,
 }
 
 #[tokio::main]

nexus/src/bin/schema-updater.rs

@@ -72,8 +72,7 @@ async fn main() -> anyhow::Result<()> {
 
     let crdb_cfg = db::Config { url: args.url };
     let pool = Arc::new(db::Pool::new(&log, &crdb_cfg));
-    let schema_config =
-        SchemaConfig { schema_dir: args.schema_directory.into() };
+    let schema_config = SchemaConfig { schema_dir: args.schema_directory };
 
     // We use the unchecked constructor of the datastore because we
     // don't want to block on someone else applying an upgrade.

nexus/src/context.rs

@@ -10,6 +10,7 @@ use authn::external::session_cookie::HttpAuthnSessionCookie;
 use authn::external::spoof::HttpAuthnSpoof;
 use authn::external::token::HttpAuthnToken;
 use authn::external::HttpAuthnScheme;
+use camino::Utf8PathBuf;
 use chrono::Duration;
 use internal_dns::ServiceName;
 use nexus_db_queries::authn::external::session_cookie::SessionStore;
@@ -24,7 +25,6 @@ use oximeter::types::ProducerRegistry;
 use oximeter_instruments::http::{HttpService, LatencyTracker};
 use slog::Logger;
 use std::env;
-use std::path::PathBuf;
 use std::str::FromStr;
 use std::sync::Arc;
 use uuid::Uuid;
@@ -60,7 +60,7 @@ pub(crate) struct ConsoleConfig {
     /// how long a session can exist before expiring
     pub session_absolute_timeout: Duration,
     /// directory containing static file to serve
-    pub static_dir: Option<PathBuf>,
+    pub static_dir: Option<Utf8PathBuf>,
 }
 
 impl ServerContext {
@@ -119,14 +119,33 @@ impl ServerContext {
         let static_dir = if config.pkg.console.static_dir.is_absolute() {
             Some(config.pkg.console.static_dir.to_owned())
         } else {
-            env::current_dir()
-                .map(|root| root.join(&config.pkg.console.static_dir))
-                .ok()
+            match env::current_dir() {
+                Ok(root) => {
+                    match Utf8PathBuf::try_from(root) {
+                        Ok(root) => {
+                            Some(root.join(&config.pkg.console.static_dir))
+                        }
+                        Err(err) => {
+                            error!(log, "Failed to convert current directory to UTF-8, \
+                                         setting assets dir to None: {}", err);
+                            None
+                        }
+                    }
+                }
+                Err(error) => {
+                    error!(
+                        log,
+                        "Failed to get current directory, \
+                         setting assets dir to None: {}",
+                        error
+                    );
+                    None
+                }
+            }
         };
 
         // We don't want to fail outright yet, but we do want to try to make
-        // problems slightly easier to debug. The only way it's None is if
-        // current_dir() fails.
+        // problems slightly easier to debug.
         if static_dir.is_none() {
             error!(log, "No assets directory configured. All console page and asset requests will 404.");
         }