feat(acl-agent): nebraska client module (Omaha protocol, invariant-encoded) - #733
Draft
Paco Huelsz (frhuelsz) wants to merge 6 commits into
Draft
feat(acl-agent): nebraska client module (Omaha protocol, invariant-encoded)#733Paco Huelsz (frhuelsz) wants to merge 6 commits into
Paco Huelsz (frhuelsz) wants to merge 6 commits into
Conversation
Add a self-contained `nebraska` module (crates/trident-acl-agent/src/nebraska) implementing the Nebraska/Omaha update protocol, scoped strictly to the protocol (no Trident gRPC, reboot, commit, or CLI concerns). It encodes in the type system the protocol invariants that otherwise fail silently, per the behavioural spec knowledge/topics/nebraska-client-protocol.md. Public API: Client (check_for_update / report_progress / complete_after_reboot / report_failure), CheckOutcome, UpdateOffer, ProgressEvent, MachineId, the AppStatus/UpdateCheckStatus response types, a Transport seam (ReqwestTransport), and NebraskaError. Invariants encoded: - Only the six whitelisted (eventtype,eventresult) pairs are constructible; no raw integers in the public API. - `track` is a Client field, so it cannot be omitted from any request. - error-updateInProgressOnInstance is a first-class CheckOutcome, and unknown status strings map to an Other catch-all rather than failing to parse. - MachineId is a validated, unbraced newtype. - Versions are semver::Version, not String. - Terminal events are emitted only via named methods, and completion uses the batched 3/2 + ping + updatecheck request. The all-or-nothing rule is a documented plain API (a cross-reboot commitment cannot be an in-process typestate); see nebraska/README.md for the rationale. Exposes the crate as a library (new lib.rs) so the module is reusable by a future TAA. Tests are hermetic (mock transport): wire-format, response parsing incl. the in-progress/unknown-status shapes, whitelist coverage, machine-id and semver handling. cargo fmt / clippy -D warnings clean. The existing agent is left unchanged; adoption is described in nebraska/README.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
… supersession Follow-up to the nebraska module review: - Add NebraskaError::is_retryable() to distinguish transient (transport/HTTP) failures from permanent protocol errors, so a caller retrying the post-reboot completion loops only on retryable errors and never spins on a permanent one (the inverse of the gRPC-UNIMPLEMENTED-retried-30s bug). complete_after_reboot's rustdoc now states loudly that it MUST be retried, and that losing it wedges the instance permanently; retry policy stays the caller's. - Document that the sync Transport is deliberate and that an async transport is a non-breaking addition (Client is generic; an AsyncTransport + async client can be added alongside), so the seam does not force a future rewrite. - Make the omaha module's supersession by nebraska explicit: a module-level note with the call-to-call migration mapping (no #[deprecated] since the agent still depends on it and the crate builds with -D warnings). - Emit the batched completion elements in logical order (event → ping → updatecheck) and assert that order in a test, so a refactor cannot silently split or reorder the request that closes the wedge window. cargo fmt / clippy --all-targets -D warnings clean; 26 module unit tests + doctest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… README Prepare the module for a public repo: - Remove all references to the internal knowledge-base doc (repo name and file path) from mod.rs and the README — these named internal material a public reader cannot resolve. - Replace every "protocol spec §N" citation across the module with the reasoning stated inline, so each doc comment stands on its own. The facts are observable behaviour of the open-source Nebraska server; only the internal pointer is removed. - Replace demo-specific values in examples and tests (real app id, POC hostname, 192.168.122.x addresses, date-stamped build versions, package name) with obviously-generic ones (example-app, updates.example.com, 1.0.0/2.0.0). - Rewrite the README to be short and usage-focused: a brief intro plus worked examples for polling, the full event sequence with retry, and wedge recovery. No behavioural change. cargo fmt / clippy --all-targets -D warnings clean; 26 module tests + doctest pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Nebraska returns 501 Not Implemented when an Omaha secret is configured and the client's URL lacks it — a permanent client misconfiguration. Under the previous classification (all HTTP errors retryable) a bounded-retry caller would spin on it. Carry the HTTP status code on NebraskaError::Http and treat 4xx and 501 as permanent, other 5xx as transient. Carrying the code also makes it available for programmatic use, which the flattened String discarded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…attempt variant Losing the post-reboot terminal event wedges the instance permanently, so retrying it is a correctness requirement rather than a quality-of-service choice. Make the safe behaviour the default: complete_after_reboot now retries retryable failures with a bounded exponential backoff, and callers no longer hand-roll the loop. try_complete_after_reboot is the single-attempt escape hatch for callers that own their own scheduler (e.g. an existing poll loop). The retry classification and a sensible default policy live in the library (where the protocol knowledge is); a caller opts out only when it has a reason to. Adds a small internal RetryPolicy + retry helper with unit tests (first-success, transient-then-success, permanent-not-retried, exhaustion) and a test that the single-attempt variant makes exactly one call. Simplifies the README example to a single call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Previously UpdateOffer exposed only version + package_url and dropped the
package hash Nebraska sends. Parse and surface it: add package_hash
(PackageHash { sha1, sha256 }) and package_size to UpdateOffer, and capture the
hash/hash_sha256/size attributes at the wire layer.
The hash is documented clearly as a hash of the package *file* (Nebraska sends a
base64 SHA-1, optionally SHA-256), for integrity-checking the download — not a
hash of any content embedded within the package. An unparseable size is treated
as absent rather than failing the offer.
Adds tests for hash+size present and absent, and asserts the wire layer captures
them. cargo fmt / clippy --all-targets -D warnings clean; 33 module tests + doctest.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a self-contained
nebraskamodule incrates/trident-acl-agent/src/nebraska/implementing the Nebraska/Omaha update protocol. Scoped strictly to the protocol — no Trident gRPC, reboot, commit, or CLI logic — so it is reusable by the current agent and a future TAA.The design goal is to make illegal protocol states unrepresentable. Several Nebraska behaviours fail silently (a client can look fine over the wire while wedging the fleet's state); this module encodes those invariants in the type system. Behavioural reference:
knowledge/topics/nebraska-client-protocol.md(pacobot repo), cited throughout the rustdoc.Public API
Client<T = ReqwestTransport>— bound to one app + track +MachineId; methodscheck_for_update,report_progress,complete_after_reboot,report_failure.CheckOutcome=UpToDate | UpdateAvailable(UpdateOffer) | UpdateInProgress.UpdateOffer { version: semver::Version, package_url: Url }.ProgressEvent=DownloadStarted | DownloadFinished | Installed(the only publicly constructible events).MachineId(validated, unbraced),AppStatus/UpdateCheckStatus(withOthercatch-all),Transport/ReqwestTransport,NebraskaError.Invariants encoded
(eventtype,eventresult)pairs are constructible — no raw integers in the public API.trackis aClientfield → impossible to omit on any request (incl. event-only).error-updateInProgressOnInstanceis a first-classCheckOutcome, not an error; unknown status strings map toOtherrather than failing to parse.MachineIdrejects braced ids;from_uuidis unbraced.semver::Version, notString.3/2 + ping + updatecheckrequest.Design note: all-or-nothing (invariant #2)
Chosen a documented plain API with a typed progress/terminal split, not a typestate/RAII guard: the terminal event fires after a reboot, in a different process, so no in-process typestate can span the commitment, and an RAII guard would fire at the wrong moment (at reboot). Rationale in
nebraska/README.md.Tests (hermetic — mock
Transport, no network)<actions>),noupdate,error-updateInProgressOnInstance+error-internal, and an unknown status (must not panic).MachineIdunbraced invariant (incl. rejecting a braced input) and empty rejection.track+ the right event.24 module unit tests + 1 doctest; the crate's existing 26 tests still pass.
cargo fmt/cargo clippy --all-targets -- -D warningsclean.Notes
lib.rs) so the module is importable.omahamodule are left unchanged; adoption path is described innebraska/README.md. Migrating the agent is a deliberate follow-up.