Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ls-apis cannot see dependency from Propolis/Crucible upstairs to Nexus internal API #6811

Open
davepacheco opened this issue Oct 9, 2024 · 1 comment · May be fixed by #6812
Open

ls-apis cannot see dependency from Propolis/Crucible upstairs to Nexus internal API #6811

davepacheco opened this issue Oct 9, 2024 · 1 comment · May be fixed by #6812

Comments

@davepacheco
Copy link
Collaborator

Summary: The ls-apis tool tries to identify API dependencies between components using Progenitor-based clients of Dropshot servers. Propolis includes Crucible [upstairs], which uses the Nexus internal API. But ls-apis does not see this dependency because Crucible's use of it is behind the non-default feature flag 'notify-nexus':
https://github.com/oxidecomputer/crucible/blob/2cacedab67c5af7515a8ea69abaa7937a44fe482/upstairs/Cargo.toml#L14

More precisely, as part of constructing a shipping TUF repo:

  1. The Omicron build packages up the "propolis-server" package from the "propolis" repo via the buildomat artifact produced by that repo:
    # Refer to
    # https://github.com/oxidecomputer/propolis/blob/master/package/README.md
    # for instructions on building this manually.
    [package.propolis-server]
    service_name = "propolis-server"
    only_for_targets.image = "standard"
    source.type = "prebuilt"
    source.repo = "propolis"
    source.commit = "11371b0f3743f8df5b047dc0edc2699f4bdf3927"
    # The SHA256 digest is automatically posted to:
    # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/propolis/image/<commit>/propolis-server.sha256.txt
    source.sha256 = "07383cbad45bc032de1b65d3553839751fde96342cc76249ca4a45b89872aae9"
    output.type = "zone"
  2. The build of that artifact explicitly enables the "omicron-build" feature of the propolis-server package:
    https://github.com/oxidecomputer/propolis/blob/5fe523a12734d6a0630c5cf07eba7cf4bfadfb2e/.github/buildomat/jobs/image.sh#L31-L36
  3. That omicron-build feature of propolis-server enables the omicron-build feature of its dependency on propolis:
    https://github.com/oxidecomputer/propolis/blob/5fe523a12734d6a0630c5cf07eba7cf4bfadfb2e/bin/propolis-server/Cargo.toml#L81-L83
  4. That omicron-build feature of propolis enables the notify-nexus feature of its dependency on crucible (which is the Crucible upstairs package):
    https://github.com/oxidecomputer/propolis/blob/5fe523a12734d6a0630c5cf07eba7cf4bfadfb2e/lib/propolis/Cargo.toml#L61-L63

and that gets us a Propolis binary containing a Crucible that uses the Nexus internal API.

However, ls-apis works by looking at cargo metadata in clones of these repos, which only enables the default features. As a result, when it looks at dependencies of propolis-server, it sees the dependency on propolis and then crucible, but without crucible's notify-nexus feature, and so it doesn't see the dependency on nexus-client. That's bad!

In practice, this isn't a huge issue right now because Propolis itself depends on nexus-client. And since ls-apis (and the upgrade project) doesn't really care about which part of a shipped component uses some other component, this is sufficient. This would be a bigger problem if for whatever reason Propolis stopped using the same client that Crucible is using here (e.g., if Propolis stopped needing it, or if we break up the Nexus internal API into separate APIs that Propolis and Crucible separately consume).

Related: oxidecomputer/crucible#1280, which would fix this.

@davepacheco
Copy link
Collaborator Author

For my future reference and to make this super clear, here's what happens in a fresh clone of Propolis (commit fae5334bcad5e864794332c6fed5e6bb9ec88831).

First, what happens when you see how propolis-server depends on nexus-client using default features:

$ cargo tree -p propolis-server -i nexus-client
nexus-client v0.1.0 (https://github.com/oxidecomputer/omicron?branch=main#888f6a1e)
├── oximeter-producer v0.1.0 (https://github.com/oxidecomputer/omicron?branch=main#888f6a1e)
│   ├── crucible v0.0.1 (https://github.com/oxidecomputer/crucible?rev=2b88ab88461fb06aaf2aab11c5e381a3cad25eac#2b88ab88)
│   │   └── propolis v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/lib/propolis)
│   │       └── propolis-server v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server)
│   └── propolis-server v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server)
├── propolis v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/lib/propolis) (*)
└── propolis-server v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server)

Now, when we enable the omicron-build feature:

$ cargo tree -p propolis-server --features omicron-build -i nexus-client
nexus-client v0.1.0 (https://github.com/oxidecomputer/omicron?branch=main#888f6a1e)
├── crucible v0.0.1 (https://github.com/oxidecomputer/crucible?rev=2b88ab88461fb06aaf2aab11c5e381a3cad25eac#2b88ab88)
│   └── propolis v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/lib/propolis)
│       └── propolis-server v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server)
├── oximeter-producer v0.1.0 (https://github.com/oxidecomputer/omicron?branch=main#888f6a1e)
│   ├── crucible v0.0.1 (https://github.com/oxidecomputer/crucible?rev=2b88ab88461fb06aaf2aab11c5e381a3cad25eac#2b88ab88) (*)
│   └── propolis-server v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server)
├── propolis v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/lib/propolis) (*)
└── propolis-server v0.1.0 (/home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server)

Enabling that feature gets us the additional path from propolis-server -> propolis -> crucible -> nexus-client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant