Skip to content

Commit

Permalink
ls-apis should fail if it finds inconsistency between Cargo deps and …
Browse files Browse the repository at this point in the history
…API metadata (#6830)
  • Loading branch information
davepacheco authored Oct 11, 2024
1 parent 5200f2a commit b0a88d5
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
9 changes: 9 additions & 0 deletions dev-tools/ls-apis/api-manifest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 25 additions & 1 deletion dev-tools/ls-apis/src/system_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()),
Expand Down
6 changes: 3 additions & 3 deletions dev-tools/ls-apis/src/workspaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>)> {
Expand Down Expand Up @@ -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
));
Expand Down

0 comments on commit b0a88d5

Please sign in to comment.