From 97c484e6740ef6354e0415a59f4c7c1b68bd7921 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 12 Mar 2024 13:19:04 -0400 Subject: [PATCH 01/17] add sled-hardware-types crate Break's wicketd's dependency on libnvme --- Cargo.lock | 20 ++++- Cargo.toml | 3 + bootstore/Cargo.toml | 2 +- clients/bootstrap-agent-client/Cargo.toml | 2 +- clients/bootstrap-agent-client/src/lib.rs | 22 +++--- clients/ddm-admin-client/Cargo.toml | 2 +- clients/ddm-admin-client/src/lib.rs | 6 +- installinator/Cargo.toml | 1 + installinator/src/bootstrap.rs | 2 +- installinator/src/peers.rs | 2 +- sled-hardware/Cargo.toml | 1 + sled-hardware/src/illumos/mod.rs | 4 +- sled-hardware/src/lib.rs | 89 ----------------------- sled-hardware/src/non_illumos/mod.rs | 3 +- sled-hardware/src/underlay.rs | 50 ------------- wicketd/Cargo.toml | 2 +- wicketd/src/bin/wicketd.rs | 2 +- wicketd/src/bootstrap_addrs.rs | 4 +- wicketd/src/context.rs | 2 +- wicketd/src/http_entrypoints.rs | 2 +- wicketd/src/lib.rs | 2 +- wicketd/src/rss_config.rs | 2 +- 22 files changed, 54 insertions(+), 171 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5900cf3decc..796d005b2b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -637,7 +637,7 @@ dependencies = [ "serde", "serde_with", "sha3", - "sled-hardware", + "sled-hardware-types", "slog", "slog-async", "slog-term", @@ -660,7 +660,7 @@ dependencies = [ "reqwest", "schemars", "serde", - "sled-hardware", + "sled-hardware-types", "slog", "uuid", ] @@ -1595,7 +1595,7 @@ dependencies = [ "rustfmt-wrapper", "serde", "serde_json", - "sled-hardware", + "sled-hardware-types", "slog", "thiserror", "tokio", @@ -3556,6 +3556,7 @@ dependencies = [ "reqwest", "sha2", "sled-hardware", + "sled-hardware-types", "sled-storage", "slog", "slog-async", @@ -8329,6 +8330,7 @@ dependencies = [ "rand 0.8.5", "schemars", "serde", + "sled-hardware-types", "slog", "slog-error-chain", "thiserror", @@ -8337,6 +8339,16 @@ dependencies = [ "uuid", ] +[[package]] +name = "sled-hardware-types" +version = "0.1.0" +dependencies = [ + "illumos-utils", + "omicron-common", + "schemars", + "serde", +] + [[package]] name = "sled-storage" version = "0.1.0" @@ -10606,7 +10618,7 @@ dependencies = [ "serde", "serde_json", "sha2", - "sled-hardware", + "sled-hardware-types", "slog", "slog-dtrace", "subprocess", diff --git a/Cargo.toml b/Cargo.toml index f291d9642ac..36478a9a160 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,6 +63,7 @@ members = [ "rpaths", "sled-agent", "sled-hardware", + "sled-hardware/types", "sled-storage", "sp-sim", "test-utils", @@ -140,6 +141,7 @@ default-members = [ "rpaths", "sled-agent", "sled-hardware", + "sled-hardware/types", "sled-storage", "sp-sim", "test-utils", @@ -370,6 +372,7 @@ similar-asserts = "1.5.0" sled = "0.34" sled-agent-client = { path = "clients/sled-agent-client" } sled-hardware = { path = "sled-hardware" } +sled-hardware-types = { path = "sled-hardware/types" } sled-storage = { path = "sled-storage" } slog = { version = "2.7", features = [ "dynamic-keys", "max_level_trace", "release_max_level_debug" ] } slog-async = "2.8" diff --git a/bootstore/Cargo.toml b/bootstore/Cargo.toml index 37280f6dcbf..5e9bcd1ef44 100644 --- a/bootstore/Cargo.toml +++ b/bootstore/Cargo.toml @@ -22,7 +22,7 @@ secrecy.workspace = true serde.workspace = true serde_with.workspace = true sha3.workspace = true -sled-hardware.workspace = true +sled-hardware-types.workspace = true slog.workspace = true thiserror.workspace = true tokio.workspace = true diff --git a/clients/bootstrap-agent-client/Cargo.toml b/clients/bootstrap-agent-client/Cargo.toml index 3474c5814ae..ea5d5358f92 100644 --- a/clients/bootstrap-agent-client/Cargo.toml +++ b/clients/bootstrap-agent-client/Cargo.toml @@ -12,7 +12,7 @@ regress.workspace = true reqwest = { workspace = true, features = [ "json", "rustls-tls", "stream" ] } schemars.workspace = true serde.workspace = true -sled-hardware.workspace = true +sled-hardware-types.workspace = true slog.workspace = true uuid.workspace = true omicron-workspace-hack.workspace = true diff --git a/clients/bootstrap-agent-client/src/lib.rs b/clients/bootstrap-agent-client/src/lib.rs index 19ecb599f30..61ebd04e1bc 100644 --- a/clients/bootstrap-agent-client/src/lib.rs +++ b/clients/bootstrap-agent-client/src/lib.rs @@ -31,32 +31,36 @@ impl omicron_common::api::external::ClientError for types::Error { } } -impl From for sled_hardware::Baseboard { +impl From for sled_hardware_types::Baseboard { fn from(value: types::Baseboard) -> Self { match value { types::Baseboard::Gimlet { identifier, model, revision } => { - sled_hardware::Baseboard::new_gimlet( + sled_hardware_types::Baseboard::new_gimlet( identifier, model, revision, ) } - types::Baseboard::Unknown => sled_hardware::Baseboard::unknown(), + types::Baseboard::Unknown => { + sled_hardware_types::Baseboard::unknown() + } types::Baseboard::Pc { identifier, model } => { - sled_hardware::Baseboard::new_pc(identifier, model) + sled_hardware_types::Baseboard::new_pc(identifier, model) } } } } -impl From for types::Baseboard { - fn from(value: sled_hardware::Baseboard) -> Self { +impl From for types::Baseboard { + fn from(value: sled_hardware_types::Baseboard) -> Self { match value { - sled_hardware::Baseboard::Gimlet { + sled_hardware_types::Baseboard::Gimlet { identifier, model, revision, } => types::Baseboard::Gimlet { identifier, model, revision }, - sled_hardware::Baseboard::Unknown => types::Baseboard::Unknown, - sled_hardware::Baseboard::Pc { identifier, model } => { + sled_hardware_types::Baseboard::Unknown => { + types::Baseboard::Unknown + } + sled_hardware_types::Baseboard::Pc { identifier, model } => { types::Baseboard::Pc { identifier, model } } } diff --git a/clients/ddm-admin-client/Cargo.toml b/clients/ddm-admin-client/Cargo.toml index 4d00f329e78..f4aee0a9615 100644 --- a/clients/ddm-admin-client/Cargo.toml +++ b/clients/ddm-admin-client/Cargo.toml @@ -14,7 +14,7 @@ thiserror.workspace = true tokio.workspace = true omicron-common.workspace = true -sled-hardware.workspace = true +sled-hardware-types.workspace = true omicron-workspace-hack.workspace = true [build-dependencies] diff --git a/clients/ddm-admin-client/src/lib.rs b/clients/ddm-admin-client/src/lib.rs index c9dea15964d..b88ea51e00f 100644 --- a/clients/ddm-admin-client/src/lib.rs +++ b/clients/ddm-admin-client/src/lib.rs @@ -26,9 +26,9 @@ use omicron_common::address::Ipv6Subnet; use omicron_common::address::SLED_PREFIX; use omicron_common::backoff::retry_notify; use omicron_common::backoff::retry_policy_internal_service_aggressive; -use sled_hardware::underlay::BootstrapInterface; -use sled_hardware::underlay::BOOTSTRAP_MASK; -use sled_hardware::underlay::BOOTSTRAP_PREFIX; +use sled_hardware_types::underlay::BootstrapInterface; +use sled_hardware_types::underlay::BOOTSTRAP_MASK; +use sled_hardware_types::underlay::BOOTSTRAP_PREFIX; use slog::info; use slog::Logger; use std::net::Ipv6Addr; diff --git a/installinator/Cargo.toml b/installinator/Cargo.toml index 43966d12025..1f92913aa1d 100644 --- a/installinator/Cargo.toml +++ b/installinator/Cargo.toml @@ -27,6 +27,7 @@ omicron-common.workspace = true reqwest.workspace = true sha2.workspace = true sled-hardware.workspace = true +sled-hardware-types.workspace = true sled-storage.workspace = true slog.workspace = true slog-async.workspace = true diff --git a/installinator/src/bootstrap.rs b/installinator/src/bootstrap.rs index 71c76809db5..59541df28ea 100644 --- a/installinator/src/bootstrap.rs +++ b/installinator/src/bootstrap.rs @@ -16,7 +16,7 @@ use illumos_utils::dladm::Dladm; use illumos_utils::zone::Zones; use omicron_common::address::Ipv6Subnet; use sled_hardware::underlay; -use sled_hardware::underlay::BootstrapInterface; +use sled_hardware_types::underlay::BootstrapInterface; use slog::info; use slog::Logger; diff --git a/installinator/src/peers.rs b/installinator/src/peers.rs index 3627ae861b1..33ba0de0e5d 100644 --- a/installinator/src/peers.rs +++ b/installinator/src/peers.rs @@ -25,7 +25,7 @@ use itertools::Itertools; use omicron_common::address::BOOTSTRAP_ARTIFACT_PORT; use omicron_common::update::ArtifactHashId; use reqwest::StatusCode; -use sled_hardware::underlay::BootstrapInterface; +use sled_hardware_types::underlay::BootstrapInterface; use tokio::{sync::mpsc, time::Instant}; use update_engine::events::ProgressUnits; use uuid::Uuid; diff --git a/sled-hardware/Cargo.toml b/sled-hardware/Cargo.toml index 28cec20041d..1c914e2897d 100644 --- a/sled-hardware/Cargo.toml +++ b/sled-hardware/Cargo.toml @@ -18,6 +18,7 @@ omicron-common.workspace = true rand.workspace = true schemars.workspace = true serde.workspace = true +sled-hardware-types.workspace = true slog.workspace = true slog-error-chain.workspace = true thiserror.workspace = true diff --git a/sled-hardware/src/illumos/mod.rs b/sled-hardware/src/illumos/mod.rs index f2db424bda6..7dd6f9e20d4 100644 --- a/sled-hardware/src/illumos/mod.rs +++ b/sled-hardware/src/illumos/mod.rs @@ -3,13 +3,13 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use crate::{ - Baseboard, DendriteAsic, DiskVariant, HardwareUpdate, SledMode, - UnparsedDisk, + DendriteAsic, DiskVariant, HardwareUpdate, SledMode, UnparsedDisk, }; use camino::Utf8PathBuf; use gethostname::gethostname; use illumos_devinfo::{DevInfo, DevLinkType, DevLinks, Node, Property}; use omicron_common::disk::DiskIdentity; +use sled_hardware_types::Baseboard; use slog::debug; use slog::error; use slog::info; diff --git a/sled-hardware/src/lib.rs b/sled-hardware/src/lib.rs index 2e3fd4a576f..607f72e25c6 100644 --- a/sled-hardware/src/lib.rs +++ b/sled-hardware/src/lib.rs @@ -74,92 +74,3 @@ pub enum SledMode { /// Force sled to run as a Scrimlet Scrimlet { asic: DendriteAsic }, } - -/// Describes properties that should uniquely identify a Gimlet. -#[derive( - Clone, - Debug, - PartialOrd, - Ord, - PartialEq, - Eq, - Hash, - Serialize, - Deserialize, - JsonSchema, -)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum Baseboard { - Gimlet { identifier: String, model: String, revision: i64 }, - - Unknown, - - Pc { identifier: String, model: String }, -} - -impl Baseboard { - #[allow(dead_code)] - pub fn new_gimlet( - identifier: String, - model: String, - revision: i64, - ) -> Self { - Self::Gimlet { identifier, model, revision } - } - - pub fn new_pc(identifier: String, model: String) -> Self { - Self::Pc { identifier, model } - } - - // XXX This should be removed, but it requires a refactor in how devices are - // polled. - pub fn unknown() -> Self { - Self::Unknown - } - - pub fn type_string(&self) -> &str { - match &self { - Self::Gimlet { .. } => "gimlet", - Self::Pc { .. } => "pc", - Self::Unknown => "unknown", - } - } - - pub fn identifier(&self) -> &str { - match &self { - Self::Gimlet { identifier, .. } => &identifier, - Self::Pc { identifier, .. } => &identifier, - Self::Unknown => "unknown", - } - } - - pub fn model(&self) -> &str { - match self { - Self::Gimlet { model, .. } => &model, - Self::Pc { model, .. } => &model, - Self::Unknown => "unknown", - } - } - - pub fn revision(&self) -> i64 { - match self { - Self::Gimlet { revision, .. } => *revision, - Self::Pc { .. } => 0, - Self::Unknown => 0, - } - } -} - -impl std::fmt::Display for Baseboard { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Baseboard::Gimlet { identifier, model, revision } => { - write!(f, "gimlet-{identifier}-{model}-{revision}") - } - Baseboard::Unknown => write!(f, "unknown"), - Baseboard::Pc { identifier, model } => { - write!(f, "pc-{identifier}-{model}") - } - } - } -} diff --git a/sled-hardware/src/non_illumos/mod.rs b/sled-hardware/src/non_illumos/mod.rs index 8518aae4957..e990567b7c4 100644 --- a/sled-hardware/src/non_illumos/mod.rs +++ b/sled-hardware/src/non_illumos/mod.rs @@ -5,8 +5,9 @@ use crate::disk::{ DiskPaths, DiskVariant, Partition, PooledDiskError, UnparsedDisk, }; -use crate::{Baseboard, SledMode}; +use crate::SledMode; use omicron_common::disk::DiskIdentity; +use sled_hardware_types::Baseboard; use slog::Logger; use std::collections::HashSet; use tokio::sync::broadcast; diff --git a/sled-hardware/src/underlay.rs b/sled-hardware/src/underlay.rs index 48d1dd2d643..41d43d685cc 100644 --- a/sled-hardware/src/underlay.rs +++ b/sled-hardware/src/underlay.rs @@ -7,7 +7,6 @@ use crate::is_gimlet; use illumos_utils::addrobj; use illumos_utils::addrobj::AddrObject; -use illumos_utils::dladm; use illumos_utils::dladm::Dladm; use illumos_utils::dladm::FindPhysicalLinkError; use illumos_utils::dladm::GetLinkpropError; @@ -15,14 +14,6 @@ use illumos_utils::dladm::PhysicalLink; use illumos_utils::dladm::SetLinkpropError; use illumos_utils::dladm::CHELSIO_LINK_PREFIX; use illumos_utils::zone::Zones; -use omicron_common::api::external::MacAddr; -use std::net::Ipv6Addr; - -/// Initial octet of IPv6 for bootstrap addresses. -pub const BOOTSTRAP_PREFIX: u16 = 0xfdb0; - -/// IPv6 prefix mask for bootstrap addresses. -pub const BOOTSTRAP_MASK: u8 = 64; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -108,47 +99,6 @@ pub fn ensure_links_have_global_zone_link_local_v6_addresses( Ok(addr_objs) } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BootstrapInterface { - GlobalZone, - SwitchZone, -} - -impl BootstrapInterface { - pub fn interface_id(self) -> u64 { - match self { - BootstrapInterface::GlobalZone => 1, - BootstrapInterface::SwitchZone => 2, - } - } - - // TODO(https://github.com/oxidecomputer/omicron/issues/945): This address - // could be randomly generated when it no longer needs to be durable. - pub fn ip( - self, - link: &PhysicalLink, - ) -> Result { - let mac = Dladm::get_mac(link)?; - Ok(mac_to_bootstrap_ip(mac, self.interface_id())) - } -} - -fn mac_to_bootstrap_ip(mac: MacAddr, interface_id: u64) -> Ipv6Addr { - let mac_bytes = mac.into_array(); - assert_eq!(6, mac_bytes.len()); - - Ipv6Addr::new( - BOOTSTRAP_PREFIX, - ((mac_bytes[0] as u16) << 8) | mac_bytes[1] as u16, - ((mac_bytes[2] as u16) << 8) | mac_bytes[3] as u16, - ((mac_bytes[4] as u16) << 8) | mac_bytes[5] as u16, - (interface_id >> 48 & 0xffff).try_into().unwrap(), - (interface_id >> 32 & 0xffff).try_into().unwrap(), - (interface_id >> 16 & 0xffff).try_into().unwrap(), - (interface_id & 0xfff).try_into().unwrap(), - ) -} - #[cfg(test)] mod tests { use super::*; diff --git a/wicketd/Cargo.toml b/wicketd/Cargo.toml index 26e54eb3bcd..4b6e6d6f5f2 100644 --- a/wicketd/Cargo.toml +++ b/wicketd/Cargo.toml @@ -53,7 +53,7 @@ installinator-common.workspace = true omicron-certificates.workspace = true omicron-common.workspace = true omicron-passwords.workspace = true -sled-hardware.workspace = true +sled-hardware-types.workspace = true tufaceous-lib.workspace = true update-common.workspace = true update-engine.workspace = true diff --git a/wicketd/src/bin/wicketd.rs b/wicketd/src/bin/wicketd.rs index 24fa802c792..4037bc4c23d 100644 --- a/wicketd/src/bin/wicketd.rs +++ b/wicketd/src/bin/wicketd.rs @@ -11,7 +11,7 @@ use omicron_common::{ address::Ipv6Subnet, cmd::{fatal, CmdError}, }; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::net::{Ipv6Addr, SocketAddrV6}; use std::path::PathBuf; use wicketd::{self, run_openapi, Config, Server, SmfConfigValues}; diff --git a/wicketd/src/bootstrap_addrs.rs b/wicketd/src/bootstrap_addrs.rs index f49e5f8ba8d..f7256ae6ac5 100644 --- a/wicketd/src/bootstrap_addrs.rs +++ b/wicketd/src/bootstrap_addrs.rs @@ -4,8 +4,8 @@ use ddm_admin_client::Client as DdmAdminClient; use futures::stream::FuturesUnordered; -use sled_hardware::underlay::BootstrapInterface; -use sled_hardware::Baseboard; +use sled_hardware_types::underlay::BootstrapInterface; +use sled_hardware_types::Baseboard; use slog::warn; use slog::Logger; use std::collections::BTreeMap; diff --git a/wicketd/src/context.rs b/wicketd/src/context.rs index eeecc3fa647..68d04f35dc0 100644 --- a/wicketd/src/context.rs +++ b/wicketd/src/context.rs @@ -14,7 +14,7 @@ use anyhow::bail; use anyhow::Result; use gateway_client::types::SpIdentifier; use internal_dns::resolver::Resolver; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::info; use std::net::Ipv6Addr; use std::net::SocketAddrV6; diff --git a/wicketd/src/http_entrypoints.rs b/wicketd/src/http_entrypoints.rs index 9748a93bd54..e87581b7bef 100644 --- a/wicketd/src/http_entrypoints.rs +++ b/wicketd/src/http_entrypoints.rs @@ -38,7 +38,7 @@ use omicron_common::update::ArtifactId; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::o; use std::collections::BTreeMap; use std::collections::BTreeSet; diff --git a/wicketd/src/lib.rs b/wicketd/src/lib.rs index 32188d77dea..5926fc468d8 100644 --- a/wicketd/src/lib.rs +++ b/wicketd/src/lib.rs @@ -32,7 +32,7 @@ use nexus_proxy::NexusTcpProxy; use omicron_common::address::{Ipv6Subnet, AZ_PREFIX}; use omicron_common::FileKv; use preflight_check::PreflightCheckerHandler; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::{debug, error, o, Drain}; use std::sync::{Mutex, OnceLock}; use std::time::Duration; diff --git a/wicketd/src/rss_config.rs b/wicketd/src/rss_config.rs index 4bc1a6b62bb..3fc123480de 100644 --- a/wicketd/src/rss_config.rs +++ b/wicketd/src/rss_config.rs @@ -26,7 +26,7 @@ use gateway_client::types::SpType; use omicron_certificates::CertificateError; use omicron_common::address; use omicron_common::address::Ipv4Range; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::warn; use std::collections::BTreeSet; use std::mem; From c0cfacbd3c65fb257cd6090555b39cfd4b2528ce Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 12 Mar 2024 13:36:21 -0400 Subject: [PATCH 02/17] add sled-hardware-types crate Break's wicketd's dependency on libnvme --- Cargo.lock | 2 ++ bootstore/src/schemes/v0/fsm.rs | 2 +- bootstore/src/schemes/v0/messages.rs | 2 +- bootstore/src/schemes/v0/peer.rs | 2 +- bootstore/src/schemes/v0/peer_networking.rs | 2 +- bootstore/src/schemes/v0/request_manager.rs | 2 +- bootstore/src/schemes/v0/share_pkg.rs | 2 +- bootstore/src/schemes/v0/storage.rs | 2 +- bootstore/tests/common/generators.rs | 2 +- bootstore/tests/common/mod.rs | 2 +- bootstore/tests/v0-fsm-proptest-learner.rs | 2 +- .../tests/v0-fsm-proptest-rack-coordinator.rs | 2 +- sled-agent/Cargo.toml | 1 + sled-agent/src/bin/sled-agent-sim.rs | 2 +- sled-agent/src/bootstrap/bootstore_setup.rs | 4 ++-- sled-agent/src/bootstrap/http_entrypoints.rs | 2 +- sled-agent/src/bootstrap/params.rs | 2 +- sled-agent/src/bootstrap/pre_server.rs | 5 +++-- sled-agent/src/hardware_monitor.rs | 3 ++- sled-agent/src/metrics.rs | 2 +- sled-agent/src/nexus.rs | 6 ++++-- sled-agent/src/params.rs | 2 +- sled-agent/src/rack_setup/service.rs | 2 +- sled-agent/src/services.rs | 4 ++-- sled-agent/src/sim/config.rs | 2 +- sled-agent/src/sled_agent.rs | 8 ++++---- sled-hardware/src/underlay.rs | 16 ---------------- 27 files changed, 38 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 796d005b2b6..c3dc0987efc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5431,6 +5431,7 @@ dependencies = [ "sha3", "sled-agent-client", "sled-hardware", + "sled-hardware-types", "sled-storage", "slog", "slog-async", @@ -8344,6 +8345,7 @@ name = "sled-hardware-types" version = "0.1.0" dependencies = [ "illumos-utils", + "macaddr", "omicron-common", "schemars", "serde", diff --git a/bootstore/src/schemes/v0/fsm.rs b/bootstore/src/schemes/v0/fsm.rs index 6382ca7df19..4f2bcc2d11e 100644 --- a/bootstore/src/schemes/v0/fsm.rs +++ b/bootstore/src/schemes/v0/fsm.rs @@ -23,7 +23,7 @@ use secrecy::ExposeSecret; use serde::{Deserialize, Serialize}; use serde_with::serde_as; use sha3::{Digest, Sha3_256}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::collections::{BTreeMap, BTreeSet}; use std::fmt::Debug; use std::time::Instant; diff --git a/bootstore/src/schemes/v0/messages.rs b/bootstore/src/schemes/v0/messages.rs index db6ec4f2817..a524df5c883 100644 --- a/bootstore/src/schemes/v0/messages.rs +++ b/bootstore/src/schemes/v0/messages.rs @@ -7,7 +7,7 @@ use super::{LearnedSharePkg, RackUuid, Share, SharePkg}; use derive_more::From; use serde::{Deserialize, Serialize}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::net::SocketAddrV6; use thiserror::Error; use uuid::Uuid; diff --git a/bootstore/src/schemes/v0/peer.rs b/bootstore/src/schemes/v0/peer.rs index 1175e641436..efb916a61fa 100644 --- a/bootstore/src/schemes/v0/peer.rs +++ b/bootstore/src/schemes/v0/peer.rs @@ -14,7 +14,7 @@ use super::{ApiError, ApiOutput, Fsm, FsmConfig, RackUuid}; use crate::trust_quorum::RackSecret; use camino::Utf8PathBuf; use derive_more::From; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::{error, info, o, warn, Logger}; use std::collections::{BTreeMap, BTreeSet}; use std::net::{SocketAddr, SocketAddrV6}; diff --git a/bootstore/src/schemes/v0/peer_networking.rs b/bootstore/src/schemes/v0/peer_networking.rs index 33673cdab82..13afd27fa23 100644 --- a/bootstore/src/schemes/v0/peer_networking.rs +++ b/bootstore/src/schemes/v0/peer_networking.rs @@ -11,7 +11,7 @@ use crate::schemes::Hello; use bytes::Buf; use derive_more::From; use serde::{Deserialize, Serialize}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::{debug, error, info, o, warn, Logger}; use std::collections::VecDeque; use std::io::Cursor; diff --git a/bootstore/src/schemes/v0/request_manager.rs b/bootstore/src/schemes/v0/request_manager.rs index 90466fdc07f..2ad907aaa15 100644 --- a/bootstore/src/schemes/v0/request_manager.rs +++ b/bootstore/src/schemes/v0/request_manager.rs @@ -7,7 +7,7 @@ use super::{ Envelope, FsmConfig, Msg, RackUuid, Request, RequestType, Share, SharePkg, }; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::collections::{BTreeMap, BTreeSet}; use std::time::Instant; use uuid::Uuid; diff --git a/bootstore/src/schemes/v0/share_pkg.rs b/bootstore/src/schemes/v0/share_pkg.rs index 2446f1904d2..f89d5b73137 100644 --- a/bootstore/src/schemes/v0/share_pkg.rs +++ b/bootstore/src/schemes/v0/share_pkg.rs @@ -12,7 +12,7 @@ use rand::{rngs::OsRng, RngCore}; use secrecy::{ExposeSecret, Secret}; use serde::{Deserialize, Serialize}; use sha3::{Digest, Sha3_256}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::collections::BTreeSet; use std::fmt; use uuid::Uuid; diff --git a/bootstore/src/schemes/v0/storage.rs b/bootstore/src/schemes/v0/storage.rs index 327acc6058a..624d0ee6edf 100644 --- a/bootstore/src/schemes/v0/storage.rs +++ b/bootstore/src/schemes/v0/storage.rs @@ -15,7 +15,7 @@ use super::{Fsm, FsmConfig, State}; use camino::Utf8PathBuf; use omicron_common::ledger::{Ledger, Ledgerable}; use serde::{Deserialize, Serialize}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::{info, Logger}; /// A persistent version of `Fsm::State` diff --git a/bootstore/tests/common/generators.rs b/bootstore/tests/common/generators.rs index 1760aefc96d..4f1a2fdbc92 100644 --- a/bootstore/tests/common/generators.rs +++ b/bootstore/tests/common/generators.rs @@ -6,7 +6,7 @@ use bootstore::schemes::v0::{FsmConfig, MsgError}; use proptest::prelude::*; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::collections::BTreeSet; use std::ops::RangeInclusive; use std::time::Duration; diff --git a/bootstore/tests/common/mod.rs b/bootstore/tests/common/mod.rs index 0cff91cfa0a..ed396efde58 100644 --- a/bootstore/tests/common/mod.rs +++ b/bootstore/tests/common/mod.rs @@ -12,7 +12,7 @@ use bootstore::schemes::v0::{ ApiError, Envelope, Fsm, FsmConfig, Msg, MsgError, RackUuid, Request, RequestType, Response, ResponseType, }; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::collections::{BTreeMap, BTreeSet}; use std::time::Instant; diff --git a/bootstore/tests/v0-fsm-proptest-learner.rs b/bootstore/tests/v0-fsm-proptest-learner.rs index 2295f5fc8e0..b70c3c3a9d3 100644 --- a/bootstore/tests/v0-fsm-proptest-learner.rs +++ b/bootstore/tests/v0-fsm-proptest-learner.rs @@ -19,7 +19,7 @@ use bootstore::trust_quorum::RackSecret; use common::CommonTestState; use proptest::prelude::*; use secrecy::ExposeSecret; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::collections::{BTreeMap, BTreeSet}; use uuid::Uuid; diff --git a/bootstore/tests/v0-fsm-proptest-rack-coordinator.rs b/bootstore/tests/v0-fsm-proptest-rack-coordinator.rs index 8bf149568a9..0de1f83ba7f 100644 --- a/bootstore/tests/v0-fsm-proptest-rack-coordinator.rs +++ b/bootstore/tests/v0-fsm-proptest-rack-coordinator.rs @@ -20,7 +20,7 @@ use bootstore::schemes::v0::{ Request, RequestType, Response, ResponseType, Share, }; use proptest::prelude::*; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::collections::{BTreeMap, BTreeSet}; use uuid::Uuid; diff --git a/sled-agent/Cargo.toml b/sled-agent/Cargo.toml index 7f46a277ce9..239887e7894 100644 --- a/sled-agent/Cargo.toml +++ b/sled-agent/Cargo.toml @@ -63,6 +63,7 @@ serde_json = { workspace = true, features = ["raw_value"] } sha3.workspace = true sled-agent-client.workspace = true sled-hardware.workspace = true +sled-hardware-types.workspace = true sled-storage.workspace = true slog.workspace = true slog-async.workspace = true diff --git a/sled-agent/src/bin/sled-agent-sim.rs b/sled-agent/src/bin/sled-agent-sim.rs index 8de9a3c423a..fcd7d02623e 100644 --- a/sled-agent/src/bin/sled-agent-sim.rs +++ b/sled-agent/src/bin/sled-agent-sim.rs @@ -20,7 +20,7 @@ use omicron_sled_agent::sim::{ run_standalone_server, Config, ConfigHardware, ConfigStorage, ConfigZpool, SimMode, }; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::net::SocketAddr; use std::net::SocketAddrV6; use uuid::Uuid; diff --git a/sled-agent/src/bootstrap/bootstore_setup.rs b/sled-agent/src/bootstrap/bootstore_setup.rs index 9eb0a87c033..120c7df000e 100644 --- a/sled-agent/src/bootstrap/bootstore_setup.rs +++ b/sled-agent/src/bootstrap/bootstore_setup.rs @@ -12,8 +12,8 @@ use super::server::StartError; use bootstore::schemes::v0 as bootstore; use camino::Utf8PathBuf; use ddm_admin_client::Client as DdmAdminClient; -use sled_hardware::underlay::BootstrapInterface; -use sled_hardware::Baseboard; +use sled_hardware_types::underlay::BootstrapInterface; +use sled_hardware_types::Baseboard; use sled_storage::dataset::CLUSTER_DATASET; use sled_storage::resources::StorageResources; use slog::Logger; diff --git a/sled-agent/src/bootstrap/http_entrypoints.rs b/sled-agent/src/bootstrap/http_entrypoints.rs index 7c32bf48a5f..68098f431e5 100644 --- a/sled-agent/src/bootstrap/http_entrypoints.rs +++ b/sled-agent/src/bootstrap/http_entrypoints.rs @@ -23,7 +23,7 @@ use http::StatusCode; use omicron_common::api::external::Error; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use sled_storage::manager::StorageHandle; use slog::Logger; use std::net::Ipv6Addr; diff --git a/sled-agent/src/bootstrap/params.rs b/sled-agent/src/bootstrap/params.rs index 48444af8d4f..4835edf4a9d 100644 --- a/sled-agent/src/bootstrap/params.rs +++ b/sled-agent/src/bootstrap/params.rs @@ -12,7 +12,7 @@ use omicron_common::ledger::Ledgerable; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use sha3::{Digest, Sha3_256}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use std::borrow::Cow; use std::collections::BTreeSet; use std::net::{IpAddr, Ipv6Addr, SocketAddrV6}; diff --git a/sled-agent/src/bootstrap/pre_server.rs b/sled-agent/src/bootstrap/pre_server.rs index e61e15d370a..6769af8ca2d 100644 --- a/sled-agent/src/bootstrap/pre_server.rs +++ b/sled-agent/src/bootstrap/pre_server.rs @@ -38,6 +38,7 @@ use omicron_common::FileKv; use sled_hardware::underlay; use sled_hardware::DendriteAsic; use sled_hardware::SledMode; +use sled_hardware_types::underlay::BootstrapInterface; use slog::Drain; use slog::Logger; use std::net::IpAddr; @@ -344,7 +345,7 @@ pub(crate) struct BootstrapNetworking { impl BootstrapNetworking { fn setup(config: &Config) -> Result { let link_for_mac = config.get_link().map_err(StartError::ConfigLink)?; - let global_zone_bootstrap_ip = underlay::BootstrapInterface::GlobalZone + let global_zone_bootstrap_ip = BootstrapInterface::GlobalZone .ip(&link_for_mac) .map_err(StartError::BootstrapLinkMac)?; @@ -381,7 +382,7 @@ impl BootstrapNetworking { IpAddr::V6(addr) => addr, }; - let switch_zone_bootstrap_ip = underlay::BootstrapInterface::SwitchZone + let switch_zone_bootstrap_ip = BootstrapInterface::SwitchZone .ip(&link_for_mac) .map_err(StartError::BootstrapLinkMac)?; diff --git a/sled-agent/src/hardware_monitor.rs b/sled-agent/src/hardware_monitor.rs index 5dcd060f98e..34e9d8c9c71 100644 --- a/sled-agent/src/hardware_monitor.rs +++ b/sled-agent/src/hardware_monitor.rs @@ -8,7 +8,8 @@ use crate::services::ServiceManager; use crate::sled_agent::SledAgent; -use sled_hardware::{Baseboard, HardwareManager, HardwareUpdate}; +use sled_hardware::{HardwareManager, HardwareUpdate}; +use sled_hardware_types::Baseboard; use sled_storage::disk::RawDisk; use sled_storage::manager::StorageHandle; use slog::Logger; diff --git a/sled-agent/src/metrics.rs b/sled-agent/src/metrics.rs index a9d5acfff8e..c41037fead3 100644 --- a/sled-agent/src/metrics.rs +++ b/sled-agent/src/metrics.rs @@ -6,7 +6,7 @@ use oximeter::types::MetricsError; use oximeter::types::ProducerRegistry; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; use slog::Logger; use std::sync::Arc; use std::time::Duration; diff --git a/sled-agent/src/nexus.rs b/sled-agent/src/nexus.rs index 7e7d60f6a4a..59ae797ab46 100644 --- a/sled-agent/src/nexus.rs +++ b/sled-agent/src/nexus.rs @@ -140,7 +140,9 @@ impl ConvertInto } } -impl ConvertInto for sled_hardware::Baseboard { +impl ConvertInto + for sled_hardware_types::Baseboard +{ fn convert(self) -> nexus_client::types::Baseboard { nexus_client::types::Baseboard { serial: self.identifier().to_string(), @@ -644,7 +646,7 @@ mod test { ByteCount, Error, Generation, LookupType, MessagePair, ResourceType, }; use omicron_test_utils::dev::test_setup_log; - use sled_hardware::Baseboard; + use sled_hardware_types::Baseboard; /// Pretend to be CRDB storing info about a sled-agent #[derive(Default, Clone)] diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index f30c910efc4..05a8ec5a58b 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -21,7 +21,7 @@ use omicron_common::api::internal::shared::{ }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use sled_hardware::Baseboard; +use sled_hardware_types::Baseboard; pub use sled_hardware::DendriteAsic; use sled_storage::dataset::DatasetKind; use sled_storage::dataset::DatasetName; diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 2788e189cc5..453495a870d 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -103,7 +103,7 @@ use serde::{Deserialize, Serialize}; use sled_agent_client::{ types as SledAgentTypes, Client as SledAgentClient, Error as SledAgentError, }; -use sled_hardware::underlay::BootstrapInterface; +use sled_hardware_types::underlay::BootstrapInterface; use sled_storage::dataset::CONFIG_DATASET; use sled_storage::manager::StorageHandle; use slog::Logger; diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 2be4b8ec45a..11ccfad69b9 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -91,9 +91,9 @@ use once_cell::sync::OnceCell; use rand::prelude::SliceRandom; use sled_hardware::is_gimlet; use sled_hardware::underlay; -use sled_hardware::underlay::BOOTSTRAP_PREFIX; -use sled_hardware::Baseboard; use sled_hardware::SledMode; +use sled_hardware_types::underlay::BOOTSTRAP_PREFIX; +use sled_hardware_types::Baseboard; use sled_storage::dataset::{ DatasetKind, DatasetName, CONFIG_DATASET, INSTALL_DATASET, ZONE_DATASET, }; diff --git a/sled-agent/src/sim/config.rs b/sled-agent/src/sim/config.rs index 7a20dd57099..8da6f8773f2 100644 --- a/sled-agent/src/sim/config.rs +++ b/sled-agent/src/sim/config.rs @@ -9,7 +9,7 @@ use camino::Utf8Path; use dropshot::ConfigDropshot; use serde::Deserialize; use serde::Serialize; -pub use sled_hardware::Baseboard; +pub use sled_hardware_types::Baseboard; use std::net::Ipv6Addr; use std::net::{IpAddr, SocketAddr}; use uuid::Uuid; diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index 4a21a6fe89b..cff13dd43a9 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -66,7 +66,9 @@ use omicron_common::backoff::{ retry_policy_internal_service_aggressive, BackoffError, }; use oximeter::types::ProducerRegistry; -use sled_hardware::{underlay, Baseboard, HardwareManager}; +use sled_hardware::{underlay, HardwareManager}; +use sled_hardware_types::underlay::BootstrapInterface; +use sled_hardware_types::Baseboard; use sled_storage::manager::StorageHandle; use slog::Logger; use std::collections::BTreeMap; @@ -1173,9 +1175,7 @@ pub async fn sled_add( // Get all known bootstrap addresses via DDM let ddm_admin_client = DdmAdminClient::localhost(&log)?; let addrs = ddm_admin_client - .derive_bootstrap_addrs_from_prefixes(&[ - underlay::BootstrapInterface::GlobalZone, - ]) + .derive_bootstrap_addrs_from_prefixes(&[BootstrapInterface::GlobalZone]) .await?; // Create a set of futures to concurrently map the baseboard to bootstrap ip diff --git a/sled-hardware/src/underlay.rs b/sled-hardware/src/underlay.rs index 41d43d685cc..e0b40f443b6 100644 --- a/sled-hardware/src/underlay.rs +++ b/sled-hardware/src/underlay.rs @@ -98,19 +98,3 @@ pub fn ensure_links_have_global_zone_link_local_v6_addresses( Ok(addr_objs) } - -#[cfg(test)] -mod tests { - use super::*; - use macaddr::MacAddr6; - - #[test] - fn test_mac_to_bootstrap_ip() { - let mac = MacAddr("a8:40:25:10:00:01".parse::().unwrap()); - - assert_eq!( - mac_to_bootstrap_ip(mac, 1), - "fdb0:a840:2510:1::1".parse::().unwrap(), - ); - } -} From 4dddd26c2e37054c9779f4b7904507f7feb09be6 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 12 Mar 2024 13:49:35 -0400 Subject: [PATCH 03/17] remove phantom libnvme dependencies in debug builds --- Cargo.lock | 1 - clients/mg-admin-client/Cargo.toml | 1 - dev-tools/omdb/Cargo.toml | 2 +- dev-tools/omdb/src/bin/omdb/db.rs | 2 +- 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3dc0987efc..8eb87d55b9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4182,7 +4182,6 @@ dependencies = [ "rustfmt-wrapper", "serde", "serde_json", - "sled-hardware", "slog", "thiserror", "tokio", diff --git a/clients/mg-admin-client/Cargo.toml b/clients/mg-admin-client/Cargo.toml index c444fee32ff..e426fa23717 100644 --- a/clients/mg-admin-client/Cargo.toml +++ b/clients/mg-admin-client/Cargo.toml @@ -13,7 +13,6 @@ slog.workspace = true thiserror.workspace = true tokio.workspace = true omicron-common.workspace = true -sled-hardware.workspace = true omicron-workspace-hack.workspace = true [build-dependencies] diff --git a/dev-tools/omdb/Cargo.toml b/dev-tools/omdb/Cargo.toml index 2b0480be2d8..1b697561a95 100644 --- a/dev-tools/omdb/Cargo.toml +++ b/dev-tools/omdb/Cargo.toml @@ -49,11 +49,11 @@ unicode-width.workspace = true uuid.workspace = true ipnetwork.workspace = true omicron-workspace-hack.workspace = true -nexus-test-utils.workspace = true multimap.workspace = true [dev-dependencies] expectorate.workspace = true +nexus-test-utils.workspace = true nexus-test-utils-macros.workspace = true omicron-nexus.workspace = true omicron-test-utils.workspace = true diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index 98e77480549..1b99a815eff 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -77,9 +77,9 @@ use nexus_db_queries::db::datastore::InstanceAndActiveVmm; use nexus_db_queries::db::identity::Asset; use nexus_db_queries::db::lookup::LookupPath; use nexus_db_queries::db::model::ServiceKind; +use nexus_db_queries::db::queries::ALLOW_FULL_TABLE_SCAN_SQL; use nexus_db_queries::db::DataStore; use nexus_reconfigurator_preparation::policy_from_db; -use nexus_test_utils::db::ALLOW_FULL_TABLE_SCAN_SQL; use nexus_types::deployment::Blueprint; use nexus_types::deployment::OmicronZoneType; use nexus_types::deployment::UnstableReconfiguratorState; From adb563bf44c3024a8b9d181e7e2c8f6cd8fe7f12 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 12 Mar 2024 13:49:47 -0400 Subject: [PATCH 04/17] cargo fmt --- sled-agent/src/params.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 05a8ec5a58b..82813d3f1e0 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -21,8 +21,8 @@ use omicron_common::api::internal::shared::{ }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use sled_hardware_types::Baseboard; pub use sled_hardware::DendriteAsic; +use sled_hardware_types::Baseboard; use sled_storage::dataset::DatasetKind; use sled_storage::dataset::DatasetName; use std::collections::BTreeSet; From f6c32d3516583e622caef65df923ef9ba84eb91e Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 12 Mar 2024 14:27:15 -0400 Subject: [PATCH 05/17] actually commit sled-hardware-types --- sled-hardware/types/Cargo.toml | 14 +++++ sled-hardware/types/src/lib.rs | 97 +++++++++++++++++++++++++++++ sled-hardware/types/src/underlay.rs | 72 +++++++++++++++++++++ 3 files changed, 183 insertions(+) create mode 100644 sled-hardware/types/Cargo.toml create mode 100644 sled-hardware/types/src/lib.rs create mode 100644 sled-hardware/types/src/underlay.rs diff --git a/sled-hardware/types/Cargo.toml b/sled-hardware/types/Cargo.toml new file mode 100644 index 00000000000..75a5f157ee7 --- /dev/null +++ b/sled-hardware/types/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "sled-hardware-types" +version = "0.1.0" +edition = "2021" +license = "MPL-2.0" + +[dependencies] +illumos-utils.workspace = true +omicron-common.workspace = true +schemars.workspace = true +serde.workspace = true + +[dev-dependencies] +macaddr.workspace = true diff --git a/sled-hardware/types/src/lib.rs b/sled-hardware/types/src/lib.rs new file mode 100644 index 00000000000..e589498ff8e --- /dev/null +++ b/sled-hardware/types/src/lib.rs @@ -0,0 +1,97 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +pub mod underlay; + +/// Describes properties that should uniquely identify a Gimlet. +#[derive( + Clone, + Debug, + PartialOrd, + Ord, + PartialEq, + Eq, + Hash, + Serialize, + Deserialize, + JsonSchema, +)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum Baseboard { + Gimlet { identifier: String, model: String, revision: i64 }, + + Unknown, + + Pc { identifier: String, model: String }, +} + +impl Baseboard { + #[allow(dead_code)] + pub fn new_gimlet( + identifier: String, + model: String, + revision: i64, + ) -> Self { + Self::Gimlet { identifier, model, revision } + } + + pub fn new_pc(identifier: String, model: String) -> Self { + Self::Pc { identifier, model } + } + + // XXX This should be removed, but it requires a refactor in how devices are + // polled. + pub fn unknown() -> Self { + Self::Unknown + } + + pub fn type_string(&self) -> &str { + match &self { + Self::Gimlet { .. } => "gimlet", + Self::Pc { .. } => "pc", + Self::Unknown => "unknown", + } + } + + pub fn identifier(&self) -> &str { + match &self { + Self::Gimlet { identifier, .. } => &identifier, + Self::Pc { identifier, .. } => &identifier, + Self::Unknown => "unknown", + } + } + + pub fn model(&self) -> &str { + match self { + Self::Gimlet { model, .. } => &model, + Self::Pc { model, .. } => &model, + Self::Unknown => "unknown", + } + } + + pub fn revision(&self) -> i64 { + match self { + Self::Gimlet { revision, .. } => *revision, + Self::Pc { .. } => 0, + Self::Unknown => 0, + } + } +} + +impl std::fmt::Display for Baseboard { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Baseboard::Gimlet { identifier, model, revision } => { + write!(f, "gimlet-{identifier}-{model}-{revision}") + } + Baseboard::Unknown => write!(f, "unknown"), + Baseboard::Pc { identifier, model } => { + write!(f, "pc-{identifier}-{model}") + } + } + } +} diff --git a/sled-hardware/types/src/underlay.rs b/sled-hardware/types/src/underlay.rs new file mode 100644 index 00000000000..bbeb43bd4d2 --- /dev/null +++ b/sled-hardware/types/src/underlay.rs @@ -0,0 +1,72 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use illumos_utils::dladm; +use illumos_utils::dladm::Dladm; +use illumos_utils::dladm::PhysicalLink; +use omicron_common::api::external::MacAddr; +use std::net::Ipv6Addr; + +/// Initial octet of IPv6 for bootstrap addresses. +pub const BOOTSTRAP_PREFIX: u16 = 0xfdb0; + +/// IPv6 prefix mask for bootstrap addresses. +pub const BOOTSTRAP_MASK: u8 = 64; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BootstrapInterface { + GlobalZone, + SwitchZone, +} + +impl BootstrapInterface { + pub fn interface_id(self) -> u64 { + match self { + BootstrapInterface::GlobalZone => 1, + BootstrapInterface::SwitchZone => 2, + } + } + + // TODO(https://github.com/oxidecomputer/omicron/issues/945): This address + // could be randomly generated when it no longer needs to be durable. + pub fn ip( + self, + link: &PhysicalLink, + ) -> Result { + let mac = Dladm::get_mac(link)?; + Ok(mac_to_bootstrap_ip(mac, self.interface_id())) + } +} + +fn mac_to_bootstrap_ip(mac: MacAddr, interface_id: u64) -> Ipv6Addr { + let mac_bytes = mac.into_array(); + assert_eq!(6, mac_bytes.len()); + + Ipv6Addr::new( + BOOTSTRAP_PREFIX, + ((mac_bytes[0] as u16) << 8) | mac_bytes[1] as u16, + ((mac_bytes[2] as u16) << 8) | mac_bytes[3] as u16, + ((mac_bytes[4] as u16) << 8) | mac_bytes[5] as u16, + (interface_id >> 48 & 0xffff).try_into().unwrap(), + (interface_id >> 32 & 0xffff).try_into().unwrap(), + (interface_id >> 16 & 0xffff).try_into().unwrap(), + (interface_id & 0xfff).try_into().unwrap(), + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use macaddr::MacAddr6; + + #[test] + fn test_mac_to_bootstrap_ip() { + let mac = MacAddr("a8:40:25:10:00:01".parse::().unwrap()); + + assert_eq!( + mac_to_bootstrap_ip(mac, 1), + "fdb0:a840:2510:1::1".parse::().unwrap(), + ); + } +} From 2445f20fd77455f1eddf60178b052a11ac2fcb40 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Wed, 13 Mar 2024 20:38:13 +0000 Subject: [PATCH 06/17] Provide a way of detecting system library leakage --- .github/buildomat/build-and-test.sh | 8 +++ Cargo.lock | 2 + dev-tools/xtask/Cargo.toml | 2 + dev-tools/xtask/src/main.rs | 96 ++++++++++++++++++++++++++++- library-verification.toml | 9 +++ 5 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 library-verification.toml diff --git a/.github/buildomat/build-and-test.sh b/.github/buildomat/build-and-test.sh index 30abd02f90a..e514a171f7a 100755 --- a/.github/buildomat/build-and-test.sh +++ b/.github/buildomat/build-and-test.sh @@ -76,6 +76,14 @@ export RUSTC_BOOTSTRAP=1 # We report build progress to stderr, and the "--timings=json" output goes to stdout. ptime -m cargo build -Z unstable-options --timings=json --workspace --tests --locked --verbose 1> "$OUTPUT_DIR/crate-build-timings.json" +# If we are running on illumos we want to verify that we are not requiring system +# libraries outside of specific binaries. If we encounter this situation we bail +# before running any tests. +# NB: This must be ran after we have built the binaries with cargo build. +if [[ $target_os == "illumos" ]]; then + ptime -m cargo xtask verify-libraries +fi + # # We apply our own timeout to ensure that we get a normal failure on timeout # rather than a buildomat timeout. See oxidecomputer/buildomat#8. diff --git a/Cargo.lock b/Cargo.lock index 8eb87d55b9e..d19055e7817 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10905,6 +10905,8 @@ dependencies = [ "cargo_metadata", "cargo_toml", "clap 4.5.1", + "serde", + "toml 0.8.10", ] [[package]] diff --git a/dev-tools/xtask/Cargo.toml b/dev-tools/xtask/Cargo.toml index 0429fcae791..fdb2e4efb56 100644 --- a/dev-tools/xtask/Cargo.toml +++ b/dev-tools/xtask/Cargo.toml @@ -10,3 +10,5 @@ camino.workspace = true cargo_toml = "0.19" cargo_metadata = "0.18" clap.workspace = true +serde.workspace = true +toml.workspace = true diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index 42e6c10d64e..030fdd777f4 100644 --- a/dev-tools/xtask/src/main.rs +++ b/dev-tools/xtask/src/main.rs @@ -11,7 +11,12 @@ use camino::Utf8Path; use cargo_metadata::Metadata; use cargo_toml::{Dependency, Manifest}; use clap::{Parser, Subcommand}; -use std::{collections::BTreeMap, process::Command}; +use serde::Deserialize; +use std::{ + collections::{BTreeMap, BTreeSet}, + fs, + process::Command, +}; #[derive(Parser)] #[command(name = "cargo xtask", about = "Workspace-related developer tools")] @@ -27,6 +32,9 @@ enum Cmds { CheckWorkspaceDeps, /// Run configured clippy checks Clippy(ClippyArgs), + /// Verify we are not leaking library bindings outside of intended + /// crates + VerifyLibraries, } #[derive(Parser)] @@ -36,11 +44,27 @@ struct ClippyArgs { fix: bool, } +#[derive(Parser)] +struct VerifyLibraryArgs { + library: String, +} + +#[derive(Deserialize)] +struct BinaryWhitelist { + binaries: BTreeSet, +} + +#[derive(Deserialize)] +struct LibraryVerificationConfig { + libraries: BTreeMap, +} + fn main() -> Result<()> { let args = Args::parse(); match args.cmd { Cmds::Clippy(args) => cmd_clippy(args), Cmds::CheckWorkspaceDeps => cmd_check_workspace_deps(), + Cmds::VerifyLibraries => cmd_verify_library(), } } @@ -213,6 +237,68 @@ fn cmd_check_workspace_deps() -> Result<()> { Ok(()) } +fn cmd_verify_library() -> Result<()> { + let metadata = load_workspace()?; + let mut config_path = metadata.workspace_root; + config_path.push("library-verification.toml"); + let config = read_library_verification_toml(&config_path)?; + + let mut offenders = Vec::new(); + for entry in fs::read_dir(metadata.target_directory)? { + let entry = entry?; + if !entry.file_type()?.is_dir() { + continue; + } + + if entry.file_name() != "release" && entry.file_name() != "debug" { + continue; + } + + for binary in metadata + .packages + .iter() + .flat_map(|p| &p.targets) + .filter(|t| t.kind == vec!["bin"]) + .map(|x| &x.name) + { + let mut path = entry.path(); + path.push(binary); + + if !path.is_file() { + continue; + } + + let command = Command::new("elfedit") + .arg("-o") + .arg("simple") + .arg("-r") + .arg("-e") + .arg("dyn:tag NEEDED") + .arg(&path) + .output() + .context("exec elfedit")?; + + assert!(command.status.success()); + + let stdout = String::from_utf8(command.stdout)?; + for library in stdout.lines() { + if let Some(whitelist) = config.libraries.get(library.trim()) { + if !whitelist.binaries.contains(binary) { + offenders + .push(anyhow::anyhow!("{binary} NEEDS {library} but it is not whitelisted")); + } + } + } + } + } + + if !offenders.is_empty() { + bail!("Found the following issues: {offenders:#?}") + } + + Ok(()) +} + fn read_cargo_toml(path: &Utf8Path) -> Result { let bytes = std::fs::read(path).with_context(|| format!("read {:?}", path))?; @@ -224,3 +310,11 @@ fn load_workspace() -> Result { .exec() .context("loading cargo metadata") } + +fn read_library_verification_toml( + path: &Utf8Path, +) -> Result { + let config_str = std::fs::read_to_string(path) + .with_context(|| format!("read {:?}", path))?; + toml::from_str(&config_str).with_context(|| format!("parse {:?}", path)) +} diff --git a/library-verification.toml b/library-verification.toml new file mode 100644 index 00000000000..ce2c290abbe --- /dev/null +++ b/library-verification.toml @@ -0,0 +1,9 @@ +[libraries."libnvme.so.1"] +binaries = [ + "installinator", + "omicron-dev", + "omicron-package", + "services-ledger-check-migrate", + "sled-agent", + "sled-agent-sim", +] From 8decf7c4699ed775378cab851e9256fe19dd0240 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Wed, 13 Mar 2024 20:53:47 +0000 Subject: [PATCH 07/17] Add timeout to buildomat job --- .github/buildomat/build-and-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/buildomat/build-and-test.sh b/.github/buildomat/build-and-test.sh index e514a171f7a..dd94597881e 100755 --- a/.github/buildomat/build-and-test.sh +++ b/.github/buildomat/build-and-test.sh @@ -81,7 +81,7 @@ ptime -m cargo build -Z unstable-options --timings=json --workspace --tests --lo # before running any tests. # NB: This must be ran after we have built the binaries with cargo build. if [[ $target_os == "illumos" ]]; then - ptime -m cargo xtask verify-libraries + ptime -m timeout 5m cargo xtask verify-libraries fi # From c8791e9f7bea91e10c5cfaacff86931598a5dc67 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Wed, 13 Mar 2024 22:20:17 +0000 Subject: [PATCH 08/17] Add some documentation --- README.adoc | 4 ++++ dev-tools/xtask/src/main.rs | 25 ++++++++++++++++++------- library-verification.toml | 3 +++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/README.adoc b/README.adoc index 93d1fa4fb8a..b5e7ea3c141 100644 --- a/README.adoc +++ b/README.adoc @@ -106,6 +106,10 @@ When you're happy with things and want to make sure you haven't missed something cargo nextest run ``` +=== Adding a new system library dependency + +We check that certain system library dependencies are not leaked outside of their intended binaries via `cargo xtask verify-libraries` in CI. If you are adding a new dependency on a illumos/helios library it is recommended that you update xref:verify-libraries.toml[] with a whitelist of where you expect the dependency to show up. For example some libraries such as `libnvme.so.1` are only available in the global zone and therefore will not be present in any other zone. This check is here to help us catch any leakage before we go to deploy on a rack. You can inspect a compiled binary in the target directory for what it requires by using `elfedit` - for example `elfedit -r -e 'dyn:tag NEEDED' /path/to/omicron/target/debug/sled-agent`. + === Rust packages in Omicron NOTE: The term "package" is overloaded: most programming languages and operating systems have their own definitions of a package. On top of that, Omicron bundles up components into our own kind of "package" that gets delivered via the install and update systems. These are described in the `package-manifest.toml` file in the root of the repo. In this section, we're just concerned with Rust packages. diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index 030fdd777f4..31302b1d7f7 100644 --- a/dev-tools/xtask/src/main.rs +++ b/dev-tools/xtask/src/main.rs @@ -244,12 +244,15 @@ fn cmd_verify_library() -> Result<()> { let config = read_library_verification_toml(&config_path)?; let mut offenders = Vec::new(); - for entry in fs::read_dir(metadata.target_directory)? { - let entry = entry?; + for entry in fs::read_dir(&metadata.target_directory) + .with_context(|| format!("read_dir {:?}", &metadata.target_directory))? + { + let entry = entry.context("dirent")?; if !entry.file_type()?.is_dir() { continue; } + // Only search for binaries in target/release and target/debug if entry.file_name() != "release" && entry.file_name() != "debug" { continue; } @@ -278,14 +281,19 @@ fn cmd_verify_library() -> Result<()> { .output() .context("exec elfedit")?; - assert!(command.status.success()); - let stdout = String::from_utf8(command.stdout)?; + // `elfedit -o simple -r -e "dyn:tag NEEDED" /file/path` will return + // a new line seperated list of required libraries so we walk over + // them looking for a match in our configuration file. If we find + // the library we make sure the binary is allowed to pull it in via + // the whitelist. for library in stdout.lines() { if let Some(whitelist) = config.libraries.get(library.trim()) { if !whitelist.binaries.contains(binary) { - offenders - .push(anyhow::anyhow!("{binary} NEEDS {library} but it is not whitelisted")); + offenders.push(anyhow::anyhow!( + "{binary} NEEDS {library} but \ + it is not whitelisted" + )); } } } @@ -293,7 +301,10 @@ fn cmd_verify_library() -> Result<()> { } if !offenders.is_empty() { - bail!("Found the following issues: {offenders:#?}") + bail!( + "Found the following issues: {offenders:#?}\n\nIf you believe any \ + of these issues are wrong please update the whitelist." + ) } Ok(()) diff --git a/library-verification.toml b/library-verification.toml index ce2c290abbe..ea4b014c50d 100644 --- a/library-verification.toml +++ b/library-verification.toml @@ -1,3 +1,6 @@ +# libnvme is a global zone only library and therefore we must be sure that only +# programs running in the gz require it. Additionally only sled-agent should be +# managing a sled's hardware. [libraries."libnvme.so.1"] binaries = [ "installinator", From 8029cbeb4d289f2d88b51aadd1a9804a079ffe8e Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Wed, 13 Mar 2024 22:22:05 +0000 Subject: [PATCH 09/17] Typo --- README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.adoc b/README.adoc index b5e7ea3c141..2da3f1aa761 100644 --- a/README.adoc +++ b/README.adoc @@ -108,7 +108,7 @@ cargo nextest run === Adding a new system library dependency -We check that certain system library dependencies are not leaked outside of their intended binaries via `cargo xtask verify-libraries` in CI. If you are adding a new dependency on a illumos/helios library it is recommended that you update xref:verify-libraries.toml[] with a whitelist of where you expect the dependency to show up. For example some libraries such as `libnvme.so.1` are only available in the global zone and therefore will not be present in any other zone. This check is here to help us catch any leakage before we go to deploy on a rack. You can inspect a compiled binary in the target directory for what it requires by using `elfedit` - for example `elfedit -r -e 'dyn:tag NEEDED' /path/to/omicron/target/debug/sled-agent`. +We check that certain system library dependencies are not leaked outside of their intended binaries via `cargo xtask verify-libraries` in CI. If you are adding a new dependency on a illumos/helios library it is recommended that you update xref:library-verification.toml[] with a whitelist of where you expect the dependency to show up. For example some libraries such as `libnvme.so.1` are only available in the global zone and therefore will not be present in any other zone. This check is here to help us catch any leakage before we go to deploy on a rack. You can inspect a compiled binary in the target directory for what it requires by using `elfedit` - for example `elfedit -r -e 'dyn:tag NEEDED' /path/to/omicron/target/debug/sled-agent`. === Rust packages in Omicron From 4e31ae8e0412260a386ba8b65a54e53405737c44 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Wed, 13 Mar 2024 23:19:20 +0000 Subject: [PATCH 10/17] Check the exit status of elfedit --- dev-tools/xtask/src/main.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index 31302b1d7f7..ebcb3213d48 100644 --- a/dev-tools/xtask/src/main.rs +++ b/dev-tools/xtask/src/main.rs @@ -281,6 +281,13 @@ fn cmd_verify_library() -> Result<()> { .output() .context("exec elfedit")?; + if !command.status.success() { + bail!( + "Failed to execute elfedit successfully {}", + command.status + ); + } + let stdout = String::from_utf8(command.stdout)?; // `elfedit -o simple -r -e "dyn:tag NEEDED" /file/path` will return // a new line seperated list of required libraries so we walk over From 276e13521b8782feff5b4d387ab86e56750e17b7 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Mon, 18 Mar 2024 20:49:06 +0000 Subject: [PATCH 11/17] Address Rain's feedback --- .cargo/xtask.toml | 41 ++++++ .github/buildomat/build-and-test.sh | 19 +-- Cargo.lock | 1 + README.adoc | 2 +- dev-tools/xtask/Cargo.toml | 1 + dev-tools/xtask/src/main.rs | 198 ++++++++++++++++------------ library-verification.toml | 12 -- 7 files changed, 168 insertions(+), 106 deletions(-) create mode 100644 .cargo/xtask.toml delete mode 100644 library-verification.toml diff --git a/.cargo/xtask.toml b/.cargo/xtask.toml new file mode 100644 index 00000000000..0055cf723c6 --- /dev/null +++ b/.cargo/xtask.toml @@ -0,0 +1,41 @@ +# This config file is used by `cargo xtask verify-libraries` + + +# These are libraries that we expect to show up in any executable produced +# by the omicron repo. +[libraries."libc.so.1"] +[libraries."libcontract.so.1"] +[libraries."libcrypto.so.3"] +[libraries."libdevinfo.so.1"] +[libraries."libdlpi.so.1"] +[libraries."libdoor.so.1"] +[libraries."libefi.so.1"] +[libraries."libgcc_s.so.1"] +[libraries."libipcc.so.1"] +[libraries."libkstat.so.1"] +[libraries."libm.so.2"] +[libraries."libnsl.so.1"] +[libraries."libnvpair.so.1"] +[libraries."libpq.so.5"] +[libraries."libpthread.so.1"] +[libraries."libresolv.so.2"] +[libraries."librt.so.1"] +[libraries."libscf.so.1"] +[libraries."libsocket.so.1"] +[libraries."libssl.so.3"] +[libraries."libumem.so.1"] +[libraries."libxml2.so.2"] +[libraries."libxmlsec1.so.1"] + +# libnvme is a global zone only library and therefore we must be sure that only +# programs running in the gz require it. Additionally only sled-agent should be +# managing a sled's hardware. +[libraries."libnvme.so.1"] +binary_allow_list = [ + "installinator", + "omicron-dev", + "omicron-package", + "services-ledger-check-migrate", + "sled-agent", + "sled-agent-sim", +] diff --git a/.github/buildomat/build-and-test.sh b/.github/buildomat/build-and-test.sh index dd94597881e..22511ae369f 100755 --- a/.github/buildomat/build-and-test.sh +++ b/.github/buildomat/build-and-test.sh @@ -72,24 +72,25 @@ export CARGO_INCREMENTAL=0 # If we remove "--timings=json" below, this would no longer be needed. export RUSTC_BOOTSTRAP=1 +verify_libraries() { + # If we are running on illumos we want to verify that we are not requiring system + # libraries outside of specific binaries. If we encounter this situation we bail + # NB: This runs `cargo build --bins` to ensure it can check the final executable. + if [[ $target_os == "illumos" ]]; then + cargo xtask verify-libraries + fi +} + # Build all the packages and tests, and keep track of how long each took to build. # We report build progress to stderr, and the "--timings=json" output goes to stdout. ptime -m cargo build -Z unstable-options --timings=json --workspace --tests --locked --verbose 1> "$OUTPUT_DIR/crate-build-timings.json" -# If we are running on illumos we want to verify that we are not requiring system -# libraries outside of specific binaries. If we encounter this situation we bail -# before running any tests. -# NB: This must be ran after we have built the binaries with cargo build. -if [[ $target_os == "illumos" ]]; then - ptime -m timeout 5m cargo xtask verify-libraries -fi - # # We apply our own timeout to ensure that we get a normal failure on timeout # rather than a buildomat timeout. See oxidecomputer/buildomat#8. # banner test -ptime -m timeout 2h cargo nextest run --profile ci --locked --verbose +ptime -m timeout 2h verify_libraries && cargo nextest run --profile ci --locked --verbose # # https://github.com/nextest-rs/nextest/issues/16 diff --git a/Cargo.lock b/Cargo.lock index a7a0ab48473..1e3c0e24ec1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10952,6 +10952,7 @@ dependencies = [ "cargo_metadata", "cargo_toml", "clap 4.5.1", + "fs-err", "serde", "toml 0.8.10", ] diff --git a/README.adoc b/README.adoc index 2da3f1aa761..04bc33b2afd 100644 --- a/README.adoc +++ b/README.adoc @@ -108,7 +108,7 @@ cargo nextest run === Adding a new system library dependency -We check that certain system library dependencies are not leaked outside of their intended binaries via `cargo xtask verify-libraries` in CI. If you are adding a new dependency on a illumos/helios library it is recommended that you update xref:library-verification.toml[] with a whitelist of where you expect the dependency to show up. For example some libraries such as `libnvme.so.1` are only available in the global zone and therefore will not be present in any other zone. This check is here to help us catch any leakage before we go to deploy on a rack. You can inspect a compiled binary in the target directory for what it requires by using `elfedit` - for example `elfedit -r -e 'dyn:tag NEEDED' /path/to/omicron/target/debug/sled-agent`. +We check that certain system library dependencies are not leaked outside of their intended binaries via `cargo xtask verify-libraries` in CI. If you are adding a new dependency on a illumos/helios library it is recommended that you update xref:.cargo/xtask.toml[] with an allow list of where you expect the dependency to show up. For example some libraries such as `libnvme.so.1` are only available in the global zone and therefore will not be present in any other zone. This check is here to help us catch any leakage before we go to deploy on a rack. You can inspect a compiled binary in the target directory for what it requires by using `elfedit` - for example `elfedit -r -e 'dyn:tag NEEDED' /path/to/omicron/target/debug/sled-agent`. === Rust packages in Omicron diff --git a/dev-tools/xtask/Cargo.toml b/dev-tools/xtask/Cargo.toml index fdb2e4efb56..8b6a5396f3b 100644 --- a/dev-tools/xtask/Cargo.toml +++ b/dev-tools/xtask/Cargo.toml @@ -12,3 +12,4 @@ cargo_metadata = "0.18" clap.workspace = true serde.workspace = true toml.workspace = true +fs-err.workspace = true diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index ebcb3213d48..48ea3ed9e00 100644 --- a/dev-tools/xtask/src/main.rs +++ b/dev-tools/xtask/src/main.rs @@ -8,14 +8,15 @@ use anyhow::{bail, Context, Result}; use camino::Utf8Path; -use cargo_metadata::Metadata; +use cargo_metadata::{Message, Metadata}; use cargo_toml::{Dependency, Manifest}; use clap::{Parser, Subcommand}; +use fs_err as fs; use serde::Deserialize; use std::{ - collections::{BTreeMap, BTreeSet}, - fs, - process::Command, + collections::{BTreeMap, BTreeSet, HashMap}, + io::BufReader, + process::{Command, Stdio}, }; #[derive(Parser)] @@ -44,19 +45,20 @@ struct ClippyArgs { fix: bool, } -#[derive(Parser)] -struct VerifyLibraryArgs { - library: String, +#[derive(Deserialize, Debug)] +struct LibraryConfig { + binary_allow_list: Option>, } -#[derive(Deserialize)] -struct BinaryWhitelist { - binaries: BTreeSet, +#[derive(Deserialize, Debug)] +struct XtaskConfig { + libraries: BTreeMap, } -#[derive(Deserialize)] -struct LibraryVerificationConfig { - libraries: BTreeMap, +#[derive(Debug)] +enum LibraryError { + Unexpected(String), + NotAllowed(String), } fn main() -> Result<()> { @@ -64,7 +66,7 @@ fn main() -> Result<()> { match args.cmd { Cmds::Clippy(args) => cmd_clippy(args), Cmds::CheckWorkspaceDeps => cmd_check_workspace_deps(), - Cmds::VerifyLibraries => cmd_verify_library(), + Cmds::VerifyLibraries => cmd_verify_libraries(), } } @@ -237,89 +239,120 @@ fn cmd_check_workspace_deps() -> Result<()> { Ok(()) } -fn cmd_verify_library() -> Result<()> { - let metadata = load_workspace()?; - let mut config_path = metadata.workspace_root; - config_path.push("library-verification.toml"); - let config = read_library_verification_toml(&config_path)?; - - let mut offenders = Vec::new(); - for entry in fs::read_dir(&metadata.target_directory) - .with_context(|| format!("read_dir {:?}", &metadata.target_directory))? - { - let entry = entry.context("dirent")?; - if !entry.file_type()?.is_dir() { - continue; - } +/// Verify that the binary at the provided path complies with the rules laid out +/// in the xtask.toml config file. Errors are pushed to a hashmap so that we can +/// display to a user the entire list of issues in one go. +fn verify_executable( + config: &XtaskConfig, + path: &Utf8Path, + errors: &mut HashMap>, +) -> Result<()> { + let binary = path.file_name().context("basename of executable")?; + + let command = Command::new("elfedit") + .arg("-o") + .arg("simple") + .arg("-r") + .arg("-e") + .arg("dyn:tag NEEDED") + .arg(&path) + .output() + .context("exec elfedit")?; + + if !command.status.success() { + bail!("Failed to execute elfedit successfully {}", command.status); + } - // Only search for binaries in target/release and target/debug - if entry.file_name() != "release" && entry.file_name() != "debug" { - continue; - } + let stdout = String::from_utf8(command.stdout)?; + // `elfedit -o simple -r -e "dyn:tag NEEDED" /file/path` will return + // a new line seperated list of required libraries so we walk over + // them looking for a match in our configuration file. If we find + // the library we make sure the binary is allowed to pull it in via + // the whitelist. + for library in stdout.lines() { + let library_config = match config.libraries.get(library.trim()) { + Some(config) => config, + None => { + errors + .entry(binary.to_string()) + .or_default() + .push(LibraryError::Unexpected(library.to_string())); - for binary in metadata - .packages - .iter() - .flat_map(|p| &p.targets) - .filter(|t| t.kind == vec!["bin"]) - .map(|x| &x.name) - { - let mut path = entry.path(); - path.push(binary); - - if !path.is_file() { continue; } - - let command = Command::new("elfedit") - .arg("-o") - .arg("simple") - .arg("-r") - .arg("-e") - .arg("dyn:tag NEEDED") - .arg(&path) - .output() - .context("exec elfedit")?; - - if !command.status.success() { - bail!( - "Failed to execute elfedit successfully {}", - command.status - ); + }; + + if let Some(allowed) = &library_config.binary_allow_list { + if !allowed.contains(binary) { + errors + .entry(binary.to_string()) + .or_default() + .push(LibraryError::NotAllowed(library.to_string())); } + } + } - let stdout = String::from_utf8(command.stdout)?; - // `elfedit -o simple -r -e "dyn:tag NEEDED" /file/path` will return - // a new line seperated list of required libraries so we walk over - // them looking for a match in our configuration file. If we find - // the library we make sure the binary is allowed to pull it in via - // the whitelist. - for library in stdout.lines() { - if let Some(whitelist) = config.libraries.get(library.trim()) { - if !whitelist.binaries.contains(binary) { - offenders.push(anyhow::anyhow!( - "{binary} NEEDS {library} but \ - it is not whitelisted" - )); - } + Ok(()) +} + +fn cmd_verify_libraries() -> Result<()> { + let metadata = load_workspace()?; + let mut config_path = metadata.workspace_root; + config_path.push(".cargo/xtask.toml"); + let config = read_xtask_toml(&config_path)?; + + let mut command = Command::new("cargo") + .args(&["build", "--message-format=json-render-diagnostics"]) + .stdout(Stdio::piped()) + .spawn() + .context("failed to spawn cargo build")?; + + let reader = BufReader::new(command.stdout.take().context("take stdout")?); + + let mut errors = Default::default(); + for message in cargo_metadata::Message::parse_stream(reader) { + match message? { + Message::CompilerArtifact(artifact) => { + // We are only interested in artifacts that are binaries + if let Some(executable) = artifact.executable { + verify_executable(&config, &executable, &mut errors)?; } } + _ => (), } } - if !offenders.is_empty() { + let status = command.wait()?; + if !status.success() { + bail!("Failed to execute cargo build successfully {}", status); + } + + if !errors.is_empty() { + let mut msg = String::new(); + use std::fmt::Write; + errors.iter().for_each(|(binary, errors)| { + write!(msg, "{binary}\n").unwrap(); + errors.iter().for_each(|error| match error { + LibraryError::Unexpected(lib) => { + write!(msg, "\tUNEXPECTED dependency on {lib}\n").unwrap() + } + LibraryError::NotAllowed(lib) => { + write!(msg, "\tNEEDS {lib} but is not allowed\n").unwrap() + } + }); + }); + bail!( - "Found the following issues: {offenders:#?}\n\nIf you believe any \ - of these issues are wrong please update the whitelist." - ) + "Found library issues with the following:\n{msg}\n\n\ + If depending on a new library was intended please add it to xtask.toml" + ); } Ok(()) } fn read_cargo_toml(path: &Utf8Path) -> Result { - let bytes = - std::fs::read(path).with_context(|| format!("read {:?}", path))?; + let bytes = fs::read(path)?; Manifest::from_slice(&bytes).with_context(|| format!("parse {:?}", path)) } @@ -329,10 +362,7 @@ fn load_workspace() -> Result { .context("loading cargo metadata") } -fn read_library_verification_toml( - path: &Utf8Path, -) -> Result { - let config_str = std::fs::read_to_string(path) - .with_context(|| format!("read {:?}", path))?; +fn read_xtask_toml(path: &Utf8Path) -> Result { + let config_str = fs::read_to_string(path)?; toml::from_str(&config_str).with_context(|| format!("parse {:?}", path)) } diff --git a/library-verification.toml b/library-verification.toml deleted file mode 100644 index ea4b014c50d..00000000000 --- a/library-verification.toml +++ /dev/null @@ -1,12 +0,0 @@ -# libnvme is a global zone only library and therefore we must be sure that only -# programs running in the gz require it. Additionally only sled-agent should be -# managing a sled's hardware. -[libraries."libnvme.so.1"] -binaries = [ - "installinator", - "omicron-dev", - "omicron-package", - "services-ledger-check-migrate", - "sled-agent", - "sled-agent-sim", -] From dd71b07e2d6b68fdeedb52cfd458366423d1c70c Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Mon, 18 Mar 2024 21:00:22 +0000 Subject: [PATCH 12/17] Add a stub impl for non illumos systems --- dev-tools/xtask/src/main.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index 48ea3ed9e00..b0a3373c3b4 100644 --- a/dev-tools/xtask/src/main.rs +++ b/dev-tools/xtask/src/main.rs @@ -295,6 +295,12 @@ fn verify_executable( Ok(()) } +#[cfg(not(target_os = "illumos"))] +fn cmd_verify_libraries() -> Result<()> { + unimplemented!("Library verification is only available on illumos!") +} + +#[cfg(target_os = "illumos")] fn cmd_verify_libraries() -> Result<()> { let metadata = load_workspace()?; let mut config_path = metadata.workspace_root; From da25241115a7d56585959189ce388926ef6038b5 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Mon, 18 Mar 2024 21:46:31 -0400 Subject: [PATCH 13/17] Pull illumos bits into a module to make clippy happy --- dev-tools/xtask/src/illumos.rs | 144 ++++++++++++++++++++++++++++++++ dev-tools/xtask/src/main.rs | 148 ++------------------------------- 2 files changed, 151 insertions(+), 141 deletions(-) create mode 100644 dev-tools/xtask/src/illumos.rs diff --git a/dev-tools/xtask/src/illumos.rs b/dev-tools/xtask/src/illumos.rs new file mode 100644 index 00000000000..7e2f72ea6fc --- /dev/null +++ b/dev-tools/xtask/src/illumos.rs @@ -0,0 +1,144 @@ +use anyhow::{bail, Context, Result}; +use camino::Utf8Path; +use cargo_metadata::Message; +use fs_err as fs; +use serde::Deserialize; +use std::{ + collections::{BTreeMap, BTreeSet, HashMap}, + io::BufReader, + process::{Command, Stdio}, +}; + +use crate::load_workspace; + +#[derive(Deserialize, Debug)] +struct LibraryConfig { + binary_allow_list: Option>, +} + +#[derive(Deserialize, Debug)] +struct XtaskConfig { + libraries: BTreeMap, +} + +#[derive(Debug)] +enum LibraryError { + Unexpected(String), + NotAllowed(String), +} + +/// Verify that the binary at the provided path complies with the rules laid out +/// in the xtask.toml config file. Errors are pushed to a hashmap so that we can +/// display to a user the entire list of issues in one go. +fn verify_executable( + config: &XtaskConfig, + path: &Utf8Path, + errors: &mut HashMap>, +) -> Result<()> { + let binary = path.file_name().context("basename of executable")?; + + let command = Command::new("elfedit") + .arg("-o") + .arg("simple") + .arg("-r") + .arg("-e") + .arg("dyn:tag NEEDED") + .arg(&path) + .output() + .context("exec elfedit")?; + + if !command.status.success() { + bail!("Failed to execute elfedit successfully {}", command.status); + } + + let stdout = String::from_utf8(command.stdout)?; + // `elfedit -o simple -r -e "dyn:tag NEEDED" /file/path` will return + // a new line seperated list of required libraries so we walk over + // them looking for a match in our configuration file. If we find + // the library we make sure the binary is allowed to pull it in via + // the whitelist. + for library in stdout.lines() { + let library_config = match config.libraries.get(library.trim()) { + Some(config) => config, + None => { + errors + .entry(binary.to_string()) + .or_default() + .push(LibraryError::Unexpected(library.to_string())); + + continue; + } + }; + + if let Some(allowed) = &library_config.binary_allow_list { + if !allowed.contains(binary) { + errors + .entry(binary.to_string()) + .or_default() + .push(LibraryError::NotAllowed(library.to_string())); + } + } + } + + Ok(()) +} +fn cmd_verify_libraries() -> Result<()> { + let metadata = load_workspace()?; + let mut config_path = metadata.workspace_root; + config_path.push(".cargo/xtask.toml"); + let config = read_xtask_toml(&config_path)?; + + let mut command = Command::new("cargo") + .args(&["build", "--message-format=json-render-diagnostics"]) + .stdout(Stdio::piped()) + .spawn() + .context("failed to spawn cargo build")?; + + let reader = BufReader::new(command.stdout.take().context("take stdout")?); + + let mut errors = Default::default(); + for message in cargo_metadata::Message::parse_stream(reader) { + match message? { + Message::CompilerArtifact(artifact) => { + // We are only interested in artifacts that are binaries + if let Some(executable) = artifact.executable { + verify_executable(&config, &executable, &mut errors)?; + } + } + _ => (), + } + } + + let status = command.wait()?; + if !status.success() { + bail!("Failed to execute cargo build successfully {}", status); + } + + if !errors.is_empty() { + let mut msg = String::new(); + use std::fmt::Write; + errors.iter().for_each(|(binary, errors)| { + write!(msg, "{binary}\n").unwrap(); + errors.iter().for_each(|error| match error { + LibraryError::Unexpected(lib) => { + write!(msg, "\tUNEXPECTED dependency on {lib}\n").unwrap() + } + LibraryError::NotAllowed(lib) => { + write!(msg, "\tNEEDS {lib} but is not allowed\n").unwrap() + } + }); + }); + + bail!( + "Found library issues with the following:\n{msg}\n\n\ + If depending on a new library was intended please add it to xtask.toml" + ); + } + + Ok(()) +} + +fn read_xtask_toml(path: &Utf8Path) -> Result { + let config_str = fs::read_to_string(path)?; + toml::from_str(&config_str).with_context(|| format!("parse {:?}", path)) +} diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index b0a3373c3b4..c682fc247e0 100644 --- a/dev-tools/xtask/src/main.rs +++ b/dev-tools/xtask/src/main.rs @@ -8,16 +8,16 @@ use anyhow::{bail, Context, Result}; use camino::Utf8Path; -use cargo_metadata::{Message, Metadata}; +use cargo_metadata::Metadata; use cargo_toml::{Dependency, Manifest}; use clap::{Parser, Subcommand}; use fs_err as fs; -use serde::Deserialize; -use std::{ - collections::{BTreeMap, BTreeSet, HashMap}, - io::BufReader, - process::{Command, Stdio}, -}; +use std::{collections::BTreeMap, process::Command}; + +#[cfg(target_os = "illumos")] +mod illumos; +#[cfg(target_os = "illumos")] +use illumos::cmd_verify_libraries; #[derive(Parser)] #[command(name = "cargo xtask", about = "Workspace-related developer tools")] @@ -45,22 +45,6 @@ struct ClippyArgs { fix: bool, } -#[derive(Deserialize, Debug)] -struct LibraryConfig { - binary_allow_list: Option>, -} - -#[derive(Deserialize, Debug)] -struct XtaskConfig { - libraries: BTreeMap, -} - -#[derive(Debug)] -enum LibraryError { - Unexpected(String), - NotAllowed(String), -} - fn main() -> Result<()> { let args = Args::parse(); match args.cmd { @@ -239,124 +223,11 @@ fn cmd_check_workspace_deps() -> Result<()> { Ok(()) } -/// Verify that the binary at the provided path complies with the rules laid out -/// in the xtask.toml config file. Errors are pushed to a hashmap so that we can -/// display to a user the entire list of issues in one go. -fn verify_executable( - config: &XtaskConfig, - path: &Utf8Path, - errors: &mut HashMap>, -) -> Result<()> { - let binary = path.file_name().context("basename of executable")?; - - let command = Command::new("elfedit") - .arg("-o") - .arg("simple") - .arg("-r") - .arg("-e") - .arg("dyn:tag NEEDED") - .arg(&path) - .output() - .context("exec elfedit")?; - - if !command.status.success() { - bail!("Failed to execute elfedit successfully {}", command.status); - } - - let stdout = String::from_utf8(command.stdout)?; - // `elfedit -o simple -r -e "dyn:tag NEEDED" /file/path` will return - // a new line seperated list of required libraries so we walk over - // them looking for a match in our configuration file. If we find - // the library we make sure the binary is allowed to pull it in via - // the whitelist. - for library in stdout.lines() { - let library_config = match config.libraries.get(library.trim()) { - Some(config) => config, - None => { - errors - .entry(binary.to_string()) - .or_default() - .push(LibraryError::Unexpected(library.to_string())); - - continue; - } - }; - - if let Some(allowed) = &library_config.binary_allow_list { - if !allowed.contains(binary) { - errors - .entry(binary.to_string()) - .or_default() - .push(LibraryError::NotAllowed(library.to_string())); - } - } - } - - Ok(()) -} - #[cfg(not(target_os = "illumos"))] fn cmd_verify_libraries() -> Result<()> { unimplemented!("Library verification is only available on illumos!") } -#[cfg(target_os = "illumos")] -fn cmd_verify_libraries() -> Result<()> { - let metadata = load_workspace()?; - let mut config_path = metadata.workspace_root; - config_path.push(".cargo/xtask.toml"); - let config = read_xtask_toml(&config_path)?; - - let mut command = Command::new("cargo") - .args(&["build", "--message-format=json-render-diagnostics"]) - .stdout(Stdio::piped()) - .spawn() - .context("failed to spawn cargo build")?; - - let reader = BufReader::new(command.stdout.take().context("take stdout")?); - - let mut errors = Default::default(); - for message in cargo_metadata::Message::parse_stream(reader) { - match message? { - Message::CompilerArtifact(artifact) => { - // We are only interested in artifacts that are binaries - if let Some(executable) = artifact.executable { - verify_executable(&config, &executable, &mut errors)?; - } - } - _ => (), - } - } - - let status = command.wait()?; - if !status.success() { - bail!("Failed to execute cargo build successfully {}", status); - } - - if !errors.is_empty() { - let mut msg = String::new(); - use std::fmt::Write; - errors.iter().for_each(|(binary, errors)| { - write!(msg, "{binary}\n").unwrap(); - errors.iter().for_each(|error| match error { - LibraryError::Unexpected(lib) => { - write!(msg, "\tUNEXPECTED dependency on {lib}\n").unwrap() - } - LibraryError::NotAllowed(lib) => { - write!(msg, "\tNEEDS {lib} but is not allowed\n").unwrap() - } - }); - }); - - bail!( - "Found library issues with the following:\n{msg}\n\n\ - If depending on a new library was intended please add it to xtask.toml" - ); - } - - Ok(()) -} - fn read_cargo_toml(path: &Utf8Path) -> Result { let bytes = fs::read(path)?; Manifest::from_slice(&bytes).with_context(|| format!("parse {:?}", path)) @@ -367,8 +238,3 @@ fn load_workspace() -> Result { .exec() .context("loading cargo metadata") } - -fn read_xtask_toml(path: &Utf8Path) -> Result { - let config_str = fs::read_to_string(path)?; - toml::from_str(&config_str).with_context(|| format!("parse {:?}", path)) -} From c5ac263ada25ec6e20e128987f02bb9f4cbab699 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Tue, 19 Mar 2024 01:48:37 +0000 Subject: [PATCH 14/17] Mark fn pub --- dev-tools/xtask/src/illumos.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/xtask/src/illumos.rs b/dev-tools/xtask/src/illumos.rs index 7e2f72ea6fc..464e298d935 100644 --- a/dev-tools/xtask/src/illumos.rs +++ b/dev-tools/xtask/src/illumos.rs @@ -82,7 +82,7 @@ fn verify_executable( Ok(()) } -fn cmd_verify_libraries() -> Result<()> { +pub fn cmd_verify_libraries() -> Result<()> { let metadata = load_workspace()?; let mut config_path = metadata.workspace_root; config_path.push(".cargo/xtask.toml"); From b78e0567d19f252c9cb1fe846e0a59ca612e0714 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Tue, 19 Mar 2024 19:21:56 +0000 Subject: [PATCH 15/17] timeout expects external commands --- .github/buildomat/build-and-test.sh | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/buildomat/build-and-test.sh b/.github/buildomat/build-and-test.sh index 22511ae369f..7d0aadcb56f 100755 --- a/.github/buildomat/build-and-test.sh +++ b/.github/buildomat/build-and-test.sh @@ -72,25 +72,29 @@ export CARGO_INCREMENTAL=0 # If we remove "--timings=json" below, this would no longer be needed. export RUSTC_BOOTSTRAP=1 -verify_libraries() { - # If we are running on illumos we want to verify that we are not requiring system - # libraries outside of specific binaries. If we encounter this situation we bail - # NB: This runs `cargo build --bins` to ensure it can check the final executable. - if [[ $target_os == "illumos" ]]; then - cargo xtask verify-libraries - fi -} - # Build all the packages and tests, and keep track of how long each took to build. # We report build progress to stderr, and the "--timings=json" output goes to stdout. ptime -m cargo build -Z unstable-options --timings=json --workspace --tests --locked --verbose 1> "$OUTPUT_DIR/crate-build-timings.json" +# If we are running on illumos we want to verify that we are not requiring +# system libraries outside of specific binaries. If we encounter this situation +# we bail. +# NB: `cargo xtask verify-libraries` runs `cargo build --bins` to ensure it can +# check the final executables. +if [[ $target_os == "illumos" ]]; then + banner verify-libraries + # This has a separate timeout from `cargo nextest` since `timeout` expects + # to run an external command and therefore we cannot run bash functions or + # subshells. + ptime -m timeout 10m cargo xtask verify-libraries +fi + # # We apply our own timeout to ensure that we get a normal failure on timeout # rather than a buildomat timeout. See oxidecomputer/buildomat#8. # banner test -ptime -m timeout 2h verify_libraries && cargo nextest run --profile ci --locked --verbose +ptime -m timeout 2h cargo nextest run --profile ci --locked --verbose # # https://github.com/nextest-rs/nextest/issues/16 From fa5b905ef38a8a5f50130329967961514346ec67 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Wed, 20 Mar 2024 16:09:38 +0000 Subject: [PATCH 16/17] Another round of Rain feedback and cleanup --- Cargo.lock | 1 + dev-tools/xtask/Cargo.toml | 1 + dev-tools/xtask/src/illumos.rs | 40 ++++++++++++++++------------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1e3c0e24ec1..509a79e2f45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10954,6 +10954,7 @@ dependencies = [ "clap 4.5.1", "fs-err", "serde", + "swrite", "toml 0.8.10", ] diff --git a/dev-tools/xtask/Cargo.toml b/dev-tools/xtask/Cargo.toml index 8b6a5396f3b..73bfe0b37a5 100644 --- a/dev-tools/xtask/Cargo.toml +++ b/dev-tools/xtask/Cargo.toml @@ -13,3 +13,4 @@ clap.workspace = true serde.workspace = true toml.workspace = true fs-err.workspace = true +swrite.workspace = true diff --git a/dev-tools/xtask/src/illumos.rs b/dev-tools/xtask/src/illumos.rs index 464e298d935..9497362929f 100644 --- a/dev-tools/xtask/src/illumos.rs +++ b/dev-tools/xtask/src/illumos.rs @@ -1,13 +1,18 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + use anyhow::{bail, Context, Result}; use camino::Utf8Path; use cargo_metadata::Message; use fs_err as fs; use serde::Deserialize; use std::{ - collections::{BTreeMap, BTreeSet, HashMap}, + collections::{BTreeMap, BTreeSet}, io::BufReader, process::{Command, Stdio}, }; +use swrite::{swriteln, SWrite}; use crate::load_workspace; @@ -33,17 +38,13 @@ enum LibraryError { fn verify_executable( config: &XtaskConfig, path: &Utf8Path, - errors: &mut HashMap>, + errors: &mut BTreeMap>, ) -> Result<()> { let binary = path.file_name().context("basename of executable")?; let command = Command::new("elfedit") - .arg("-o") - .arg("simple") - .arg("-r") - .arg("-e") - .arg("dyn:tag NEEDED") - .arg(&path) + .args(["-o", "simple", "-r", "-e", "dyn:tag NEEDED"]) + .arg(path) .output() .context("exec elfedit")?; @@ -88,8 +89,9 @@ pub fn cmd_verify_libraries() -> Result<()> { config_path.push(".cargo/xtask.toml"); let config = read_xtask_toml(&config_path)?; - let mut command = Command::new("cargo") - .args(&["build", "--message-format=json-render-diagnostics"]) + let cargo = std::env::var("CARGO").context("CARGO env variable")?; + let mut command = Command::new(cargo) + .args(["build", "--bins", "--message-format=json-render-diagnostics"]) .stdout(Stdio::piped()) .spawn() .context("failed to spawn cargo build")?; @@ -98,14 +100,11 @@ pub fn cmd_verify_libraries() -> Result<()> { let mut errors = Default::default(); for message in cargo_metadata::Message::parse_stream(reader) { - match message? { - Message::CompilerArtifact(artifact) => { - // We are only interested in artifacts that are binaries - if let Some(executable) = artifact.executable { - verify_executable(&config, &executable, &mut errors)?; - } + if let Message::CompilerArtifact(artifact) = message? { + // We are only interested in artifacts that are binaries + if let Some(executable) = artifact.executable { + verify_executable(&config, &executable, &mut errors)?; } - _ => (), } } @@ -116,15 +115,14 @@ pub fn cmd_verify_libraries() -> Result<()> { if !errors.is_empty() { let mut msg = String::new(); - use std::fmt::Write; errors.iter().for_each(|(binary, errors)| { - write!(msg, "{binary}\n").unwrap(); + swriteln!(msg, "{binary}"); errors.iter().for_each(|error| match error { LibraryError::Unexpected(lib) => { - write!(msg, "\tUNEXPECTED dependency on {lib}\n").unwrap() + swriteln!(msg, "\tUNEXPECTED dependency on {lib}"); } LibraryError::NotAllowed(lib) => { - write!(msg, "\tNEEDS {lib} but is not allowed\n").unwrap() + swriteln!(msg, "\tNEEDS {lib} but is not allowed"); } }); }); From c2b849d242fea71eabbafdf2b4305f047f885f69 Mon Sep 17 00:00:00 2001 From: Mike Zeller Date: Wed, 20 Mar 2024 20:42:09 +0000 Subject: [PATCH 17/17] Fallback to "cargo" if $CARGO is unset --- dev-tools/xtask/src/illumos.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/xtask/src/illumos.rs b/dev-tools/xtask/src/illumos.rs index 9497362929f..a2daab2c9e2 100644 --- a/dev-tools/xtask/src/illumos.rs +++ b/dev-tools/xtask/src/illumos.rs @@ -89,7 +89,7 @@ pub fn cmd_verify_libraries() -> Result<()> { config_path.push(".cargo/xtask.toml"); let config = read_xtask_toml(&config_path)?; - let cargo = std::env::var("CARGO").context("CARGO env variable")?; + let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string()); let mut command = Command::new(cargo) .args(["build", "--bins", "--message-format=json-render-diagnostics"]) .stdout(Stdio::piped())