From 0098cf8831bfef78c7eec437cf3c18ad65b2264a Mon Sep 17 00:00:00 2001
From: David Pacheco <dap@oxidecomputer.com>
Date: Thu, 10 Oct 2024 15:38:31 -0700
Subject: [PATCH] ls-apis should fail if it finds inconsistency between Cargo
 deps and API metadata

---
 dev-tools/ls-apis/api-manifest.toml  |  9 +++++++++
 dev-tools/ls-apis/src/system_apis.rs | 26 +++++++++++++++++++++++++-
 dev-tools/ls-apis/src/workspaces.rs  |  6 +++---
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/dev-tools/ls-apis/api-manifest.toml b/dev-tools/ls-apis/api-manifest.toml
index c9a2430fa7..b9ca4bc5df 100644
--- a/dev-tools/ls-apis/api-manifest.toml
+++ b/dev-tools/ls-apis/api-manifest.toml
@@ -138,6 +138,15 @@ client_package_name = "bootstrap-agent-client"
 label = "Bootstrap Agent"
 server_package_name = "bootstrap-agent-api"
 
+[[apis]]
+client_package_name = "clickhouse-admin-client"
+label = "Clickhouse Cluster Admin"
+server_package_name = "clickhouse-admin-api"
+notes = """
+This is the server running inside multi-node Clickhouse zones that's \
+responsible for local configuration and monitoring.
+"""
+
 [[apis]]
 client_package_name = "cockroach-admin-client"
 label = "CockroachDB Cluster Admin"
diff --git a/dev-tools/ls-apis/src/system_apis.rs b/dev-tools/ls-apis/src/system_apis.rs
index 6d624d4e57..ad61f31f33 100644
--- a/dev-tools/ls-apis/src/system_apis.rs
+++ b/dev-tools/ls-apis/src/system_apis.rs
@@ -67,9 +67,17 @@ impl SystemApis {
         // Load Cargo metadata and validate it against the manifest.
         let (workspaces, warnings) = Workspaces::load(&api_metadata)?;
         if !warnings.is_empty() {
+            // We treat these warnings as fatal here.
             for e in warnings {
-                eprintln!("warning: {:#}", e);
+                eprintln!("error: {:#}", e);
             }
+
+            bail!(
+                "found inconsistency between API manifest ({}) and \
+                 information found from the Cargo dependency tree \
+                 (see above)",
+                &args.api_manifest_path
+            );
         }
 
         // Create an index of server package names, mapping each one to the API
@@ -436,6 +444,22 @@ impl<'a> ServerComponentsTracker<'a> {
             return;
         }
 
+        // TODO Work around omicron#6829.
+        // Through the dependency tree, omicron-nexus appears to export the
+        // clickhouse-admin-api, but it doesn't really.
+        // This is just like the Crucible Pantry one above in that we can't
+        // build our data model unless we ignore it so we have to hardcode this
+        // here rather than use "dependency_filter_rules".
+        if **server_pkgname == "omicron-nexus"
+            && *api.client_package_name == "clickhouse-admin-client"
+        {
+            eprintln!(
+                "note: ignoring Cargo dependency from crucible-pantry -> \
+                 ... -> crucible-control-client",
+            );
+            return;
+        }
+
         if let Some((previous, _)) = self.api_producers.insert(
             api.client_package_name.clone(),
             (server_pkgname.clone(), dep_path.clone()),
diff --git a/dev-tools/ls-apis/src/workspaces.rs b/dev-tools/ls-apis/src/workspaces.rs
index ef1ba0ee79..ace565e011 100644
--- a/dev-tools/ls-apis/src/workspaces.rs
+++ b/dev-tools/ls-apis/src/workspaces.rs
@@ -28,7 +28,7 @@ impl Workspaces {
     /// The data found is validated against `api_metadata`.
     ///
     /// On success, returns `(workspaces, warnings)`, where `warnings` is a list
-    /// of potential inconsistencies between API metadata and Cargo metadata.
+    /// of inconsistencies between API metadata and Cargo metadata.
     pub fn load(
         api_metadata: &AllApiMetadata,
     ) -> Result<(Workspaces, Vec<anyhow::Error>)> {
@@ -114,8 +114,8 @@ impl Workspaces {
                     client_pkgnames_unused.remove(client_pkgname);
                 } else {
                     warnings.push(anyhow!(
-                        "workspace {}: found client package missing from API \
-                         manifest: {}",
+                        "workspace {}: found Progenitor-based client package \
+                         missing from API manifest: {}",
                         workspace.name(),
                         client_pkgname
                     ));