ls-apis needs to detect cycles in dependency unit graph

dev-tools/ls-apis/api-manifest.toml

@@ -593,3 +593,97 @@ note = """
 Sled Agent uses the Crucible Agent client types only, and only in the simulated
 sled agent.
 """
+
+################################################################################
+# Localhost-only edges
+#
+# These edges are excluded from the deployment unit dependency graph because
+# they represent communication that only happens locally within a deployment
+# unit, not across deployment unit boundaries. A single deployment unit is
+# updated atomically, so there's no risk of version skew with localhost-only
+# edges.
+################################################################################
+
+[[localhost_only_edges]]
+server = "ddmd"
+client = "dpd-client"
+note = "verified: sled-agent configures ddmd with dpd_host=[::1] (services.rs:3127)"
+
+[[localhost_only_edges]]
+server = "dpd"
+client = "gateway-client"
+note = "verified: dpd defaults to [::1]:MGS_PORT (dendrite switch_identifiers.rs:60)"
+
+[[localhost_only_edges]]
+server = "lldpd"
+client = "dpd-client"
+note = "verified: lldpd defaults to localhost for dpd (lldp dendrite.rs:194)"
+
+[[localhost_only_edges]]
+server = "mgd"
+client = "ddm-admin-client"
+note = "verified: mg-lower hardcodes localhost:8000 (mg-lower ddm.rs:110)"
+
+[[localhost_only_edges]]
+server = "mgd"
+client = "dpd-client"
+note = "verified: mg-lower hardcodes localhost (mg-lower dendrite.rs:491)"
+
+[[localhost_only_edges]]
+server = "mgd"
+client = "gateway-client"
+note = "verified: mgd defaults to [::1]:12225 (mgd main.rs:93)"
+
+# NOTE: The following sled-agent edges are BUGS - they use underlay IPs, not
+# localhost. They are kept here to avoid breaking the build, but need to be
+# fixed, either through client-side versioning or by some other means.
+
+[[localhost_only_edges]]
+server = "omicron-sled-agent"
+client = "gateway-client"
+note = "BUG: uses underlay IP, not localhost (early_networking.rs:376)"
+
+[[localhost_only_edges]]
+server = "omicron-sled-agent"
+client = "ddm-admin-client"
+note = "verified: uses Client::localhost() (ddm_reconciler.rs:56)"
+
+[[localhost_only_edges]]
+server = "omicron-sled-agent"
+client = "dpd-client"
+note = "BUG: uses underlay IP, not localhost (services.rs:1090)"
+
+[[localhost_only_edges]]
+server = "omicron-sled-agent"
+client = "mg-admin-client"
+note = "BUG: uses underlay IP, not localhost (early_networking.rs:483)"
+
+[[localhost_only_edges]]
+server = "omicron-sled-agent"
+client = "propolis-client"
+note = "BUG: uses zone IP, not localhost (instance.rs:2283)"
+
+[[localhost_only_edges]]
+server = "tfportd"
+client = "dpd-client"
+note = "verified: configured with dpd_host=[::1] (services.rs:2852)"
+
+[[localhost_only_edges]]
+server = "tfportd"
+client = "lldpd-client"
+note = "verified: hardcodes localhost (tfportd tfport.rs:88)"
+
+[[localhost_only_edges]]
+server = "wicketd"
+client = "ddm-admin-client"
+note = "verified: uses DdmAdminClient::localhost() (bootstrap_addrs.rs:162)"
+
+[[localhost_only_edges]]
+server = "wicketd"
+client = "dpd-client"
+note = "verified: hardcodes [::1]:DENDRITE_PORT (preflight_check/uplink.rs:83)"
+
+[[localhost_only_edges]]
+server = "wicketd"
+client = "gateway-client"
+note = "verified: --mgs-address CLI expects localhost (bin/wicketd.rs:35)"

dev-tools/ls-apis/src/api_metadata.rs

@@ -30,6 +30,7 @@ pub struct AllApiMetadata {
     deployment_units: BTreeMap<DeploymentUnitName, DeploymentUnitInfo>,
     dependency_rules: BTreeMap<ClientPackageName, Vec<DependencyFilterRule>>,
     ignored_non_clients: BTreeSet<ClientPackageName>,
+    localhost_only_edges: Vec<LocalhostOnlyEdge>,
 }
 
 impl AllApiMetadata {
@@ -73,6 +74,11 @@ impl AllApiMetadata {
         &self.ignored_non_clients
     }
 
+    /// Returns the list of localhost-only edges
+    pub fn localhost_only_edges(&self) -> &[LocalhostOnlyEdge] {
+        &self.localhost_only_edges
+    }
+
     /// Returns how we should filter the given dependency
     pub(crate) fn evaluate_dependency(
         &self,
@@ -138,6 +144,7 @@ struct RawApiMetadata {
     deployment_units: Vec<DeploymentUnitInfo>,
     dependency_filter_rules: Vec<DependencyFilterRule>,
     ignored_non_clients: Vec<ClientPackageName>,
+    localhost_only_edges: Vec<LocalhostOnlyEdge>,
 }
 
 impl TryFrom<RawApiMetadata> for AllApiMetadata {
@@ -188,17 +195,35 @@ impl TryFrom<RawApiMetadata> for AllApiMetadata {
         for client_pkg in raw.ignored_non_clients {
             if !ignored_non_clients.insert(client_pkg.clone()) {
                 bail!(
-                    "entry in ignored_non_clients appearead twice: {:?}",
+                    "entry in ignored_non_clients appeared twice: {:?}",
                     &client_pkg
                 );
             }
         }
 
+        // Validate localhost_only_edges reference known server components and
+        // APIs.
+        let known_components: BTreeSet<_> =
+            deployment_units.values().flat_map(|u| u.packages.iter()).collect();
+        for edge in &raw.localhost_only_edges {
+            if !known_components.contains(&edge.server) {
+                bail!(
+                    "localhost_only_edges: unknown server component {:?}",
+                    edge.server
+                );
+            }
+            let client_name = edge.client.as_specific();
+            if !apis.contains_key(client_name) {
+                bail!("localhost_only_edges: unknown client {:?}", client_name);
+            }
+        }
+
         Ok(AllApiMetadata {
             apis,
             deployment_units,
             dependency_rules,
             ignored_non_clients,
+            localhost_only_edges: raw.localhost_only_edges,
         })
     }
 }
@@ -416,3 +441,61 @@ pub enum Evaluation {
     /// This dependency should be part of the update DAG
     Dag,
 }
+
+/// Specifies which client to match in a localhost-only edge rule.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub enum ClientMatcher {
+    /// Match a specific client package.
+    Specific(ClientPackageName),
+}
+
+impl<'de> Deserialize<'de> for ClientMatcher {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let s = String::deserialize(deserializer)?;
+        Ok(ClientMatcher::Specific(ClientPackageName::from(s)))
+    }
+}
+
+impl ClientMatcher {
+    /// Returns true if this matcher matches the given client package.
+    pub fn matches(&self, client: &ClientPackageName) -> bool {
+        match self {
+            ClientMatcher::Specific(name) => name == client,
+        }
+    }
+
+    /// Returns the specific client name.
+    pub fn as_specific(&self) -> &ClientPackageName {
+        match self {
+            ClientMatcher::Specific(name) => name,
+        }
+    }
+}
+
+/// An edge that should be excluded from the deployment unit dependency graph
+/// because it represents communication that only happens locally within a
+/// deployment unit.
+#[derive(Deserialize)]
+#[serde(deny_unknown_fields)]
+pub struct LocalhostOnlyEdge {
+    /// The server component that consumes the API.
+    pub server: ServerComponentName,
+    /// The client package consumed.
+    pub client: ClientMatcher,
+    /// Explanation of why this edge is localhost-only.
+    pub note: String,
+}
+
+impl LocalhostOnlyEdge {
+    /// Returns true if this rule matches the given (server, client) pair.
+    pub fn matches(
+        &self,
+        server: &ServerComponentName,
+        client: &ClientPackageName,
+    ) -> bool {
+        self.server == *server && self.client.matches(client)
+    }
+}

dev-tools/ls-apis/src/bin/ls-apis.rs

@@ -252,7 +252,14 @@ fn print_server_components<'a>(
             );
         }
         for (c, path) in apis.component_apis_consumed(s, filter)? {
-            println!("{}    consumes: {}", prefix, c);
+            if let Some(note) = apis.localhost_only_edge_note(s, c) {
+                println!(
+                    "{}    consumes: {} (localhost-only: {})",
+                    prefix, c, note
+                );
+            } else {
+                println!("{}    consumes: {}", prefix, c);
+            }
             if show_deps {
                 for p in path.nodes() {
                     println!("{}        via: {}", prefix, p);