move cockroach-admin-client NodeId to cockroach-admin-types

[sunshowers]

Currently, the OpenAPI manager has a circular dependency with cockroach-admin-client and cockroach-admin-api, making it hard to make changes to the API that need to happen before the client is updated. The only dependency is NodeId, though, which can be moved to this more central location.

[davepacheco]

Thanks!

Is this something that the ls-apis tool could look for?

[sunshowers]

@davepacheco by "this" do you mean these kinds of circular deps? Yeah probably I think.

[davepacheco]

Yeah. What exactly is the condition? The client crate -> API crate dependency is implicit, so if the API crate somehow depends on the client crate for real, that's the problem? It's not just an API depending on its own client...but it's also not just any API crate depending on any client crate (because I think we have that successfully and it works out as long as it's in the right direction down the API DAG).

[sunshowers]

I think it would be any openapi-manager-managed API crate depending on any openapi-manager-managed API's corresponding client crate, right? Otherwise you'll run into the issue described here where you can't generate the API until the client compiles properly.

I could very well be missing something though.

[davepacheco]

The cases I was thinking of were that:

• Nexus internal API exposes types that (used to?) come from the DNS client.
• Bootstrap agent API exposes types that (used to?) come from the sled agent client

I just checked and these both use the "-types" crate rather than the "-client" crate. Is that somehow okay?

I'm trying to resolve this with the fact that while working on stuff I'm pretty sure I've had to run cargo xtask openapi generate to generate a new spec, which alone caused another thing to need updating with cargo xtask openapi generate. This was admittedly several months ago and I could also be misremembering.

[sunshowers]

Yeah using the types crate is okay (this change does that). The types crate is meant to be shared by the client and API in this fashion.

It's possible that not everything obeys this today (bootstrap-agent-client is likely an exception to this), so we may need an allowlist that we ratchet down.