Skip to content

Implement launcher abstraction within data plane to improve diagnostics - #337

Open
Liam Farrelly (lfarrel6) wants to merge 5 commits into
mainfrom
liam/fou-567-surface-more-context-from-enclave-process
Open

Implement launcher abstraction within data plane to improve diagnostics#337
Liam Farrelly (lfarrel6) wants to merge 5 commits into
mainfrom
liam/fou-567-surface-more-context-from-enclave-process

Conversation

@lfarrel6

Copy link
Copy Markdown
Member

Why

The data plane launch process requires many fallible steps to complete to become healthy. Each step is currently implemented independent of one another and then wired up. This is making it awkward to implement any real diagnostics during the launch process.

To make this easier, and to get better guarantees around the chain of events taking place, this PR introduces a launcher module which allows us to chain fallible async workloads in a generic manner and publish any unexpected failures to a preconfigured observer.

How

  • implement launcher module which adds the Bootchain abstraction
    • Each step in the launcher is implemented as a Stage trait, which accepts In and forwards Out
    • The chain is composed through a series of then function chains, which enforces a causal type chain A -> B -> C
    • The chain carries an observer which can report on stage start, end, and failure

This PR does not integrate the launcher, it is only introducing the abstraction.

@ev-vaultkeeper ev-vaultkeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the launcher abstraction. This is a well-structured, unusually well-documented PR, and CI is green across all feature combinations. The abstraction (structural ordering via the Stage::In/Out type chain, forward composition, observer fan-out, Infallible composition) is clean, and the accompanying tests — especially the exhaustive wildcard-free Service label gate and the compile_fail ordering doctest — are a nice touch.

I also want to call out the genuine correctness improvements that ride along with the abstraction, independent of it being wired up:

  • enclave_trusted_cert no longer calls std::process::exit(1); the failure is now surfaced as an error for the healthcheck server to report. This is the right model.
  • finalize_env().unwrap()map_err(..)? and the acme .expect(..)? both remove panics from the boot path.

Nothing below is blocking — they're mostly documentation/testing nits. Since the entire value proposition of this PR is precise diagnostics and precise documentation, the stale doc references stood out.

One design consideration worth a thought (no change required): stub.rs (~315 lines of throwaway fixtures) is deliberately not #[cfg(test)] so the cargo check matrix can type-check the chain shape. The rationale is well documented, but it does mean unused pub scaffolding is compiled into the shipped library. An alternative would be a dedicated cfg/feature enabled only by the check matrix, or a CI job running cargo check --tests / cargo test --no-run, which would let the fixtures (and the enclave-gated test flagged below) live under #[cfg(test)] while still being gated by CI. Not necessary to land this.

Comment thread crates/data-plane/src/launcher/mod.rs Outdated
Comment thread crates/data-plane/src/launcher/stage.rs Outdated
Comment thread crates/data-plane/src/server/tls/tls_server.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants