From 498659bf7c4c9c97fef3488ef33de6c5acec4460 Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Wed, 5 Aug 2026 19:35:04 +0000 Subject: [PATCH 1/6] trident-acl-agent: rewrite as annotation-based ACL update agent Rewrites trident-acl-agent from the earlier label-protocol prototype into the accepted annotation-based design: it watches a Node's acl.azure.com/update-request annotation, drives Trident's stage/finalize/ rollback/commit gRPC operations against tridentd, and writes back acl.azure.com/update-status, including the post-reboot commit half of a finalize/rollback via a small persisted state file (/var/lib/trident-acl-agent/state.json). New modules: - annotations.rs: UpdateRequest/UpdateStatus wire types, schema validation, and design-doc conformance tests (parses the request/status JSON examples from docs/update-trigger-design.md with the real (de)serialization code, and validates both those examples and our own constructed annotations against the formal JSON Schema embedded in that document). - orchestrator.rs: the reconcile loop - stage/finalize/rollback handlers, the post-reboot commit resume/reconstruction path, and the terminal status-mapping logic (including its unit tests against a mock tridentd). - trident.rs: gRPC client wrapper for tridentd's stable v1 Update/Commit/ Rollback services. - mock_tridentd.rs: in-process fake tridentd server (dev-only) used by orchestrator.rs's unit tests. - k8s.rs: thin kube-rs wrapper for watching/patching the agent's own Node. - config.rs, state.rs: agent configuration and the persisted state file. Depends on the stable RollbackService promoted in the parent branch (user/bfjelds/rollback-grpc-promotion) for its rollback support. Packaging: adds the trident-acl-agent.service systemd unit and RPM spec entries so the agent ships and starts on ACL images. New workspace dependencies (Cargo.toml): k8s-openapi, kube, toml - all for k8s.rs's Node watch/patch client. Split out from user/bfjelds/acl-agent-rollback-grpc for isolated review: this covers the Rust agent implementation and packaging, not the storm Go E2E test harness, test images, or pipeline definitions (stacked as a separate PR on top of this one). Verified: cargo test -p trident-acl-agent (78 passed), cargo clippy -p trident-acl-agent --all-targets -- -D warnings (clean), cargo fmt --check (clean), cargo build --workspace (clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c06585d-a82d-475f-a802-83fdfa012d86 --- Cargo.lock | 733 ++++++- Cargo.toml | 3 + crates/trident-acl-agent/Cargo.toml | 19 + crates/trident-acl-agent/src/annotations.rs | 899 +++++++++ crates/trident-acl-agent/src/config.rs | 375 ++++ crates/trident-acl-agent/src/k8s.rs | 128 ++ crates/trident-acl-agent/src/lib.rs | 458 +++++ crates/trident-acl-agent/src/main.rs | 621 +----- crates/trident-acl-agent/src/mock_tridentd.rs | 253 +++ crates/trident-acl-agent/src/orchestrator.rs | 1712 +++++++++++++++++ crates/trident-acl-agent/src/state.rs | 280 +++ crates/trident-acl-agent/src/trident.rs | 380 ++++ packaging/rpm/trident.spec | 11 + packaging/systemd/trident-acl-agent.service | 12 + 14 files changed, 5305 insertions(+), 579 deletions(-) create mode 100644 crates/trident-acl-agent/src/annotations.rs create mode 100644 crates/trident-acl-agent/src/config.rs create mode 100644 crates/trident-acl-agent/src/k8s.rs create mode 100644 crates/trident-acl-agent/src/lib.rs create mode 100644 crates/trident-acl-agent/src/mock_tridentd.rs create mode 100644 crates/trident-acl-agent/src/orchestrator.rs create mode 100644 crates/trident-acl-agent/src/state.rs create mode 100644 crates/trident-acl-agent/src/trident.rs create mode 100644 packaging/systemd/trident-acl-agent.service diff --git a/Cargo.lock b/Cargo.lock index a7fef8faf5..da53c76252 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,19 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "512761e0bb2578dd7380c6baaa0f4ce03e84f95e960231d1dec8bf4d7d6e2627" +[[package]] +name = "ahash" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" +dependencies = [ + "cfg-if", + "getrandom 0.3.1", + "once_cell", + "version_check", + "zerocopy 0.8.27", +] + [[package]] name = "aho-corasick" version = "1.1.3" @@ -26,6 +39,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" + [[package]] name = "android-tzdata" version = "0.1.1" @@ -109,6 +128,40 @@ dependencies = [ "serde_json", ] +[[package]] +name = "async-broadcast" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "435a87a52755b8f27fcf321ac4f04b2802e337c8c4872923137471ec39c37532" +dependencies = [ + "event-listener", + "event-listener-strategy", + "futures-core", + "pin-project-lite", +] + +[[package]] +name = "async-stream" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b5a71a6f37880a80d1d7f19efd781e4b5de42c88f0722cc13bcb6cc2cfe8476" +dependencies = [ + "async-stream-impl", + "futures-core", + "pin-project-lite", +] + +[[package]] +name = "async-stream-impl" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7c24de15d275a1ecfd47a380fb4d5ec9bfe0933f309ed5e705b775596a3574d" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.90", +] + [[package]] name = "async-trait" version = "0.1.89" @@ -117,7 +170,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -175,6 +228,17 @@ dependencies = [ "tower-service", ] +[[package]] +name = "backoff" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b62ddb9cb1ec0a098ad4bbf9344d0713fa193ae1a80af55febcff2627b6a00c1" +dependencies = [ + "getrandom 0.2.15", + "instant", + "rand 0.8.6", +] + [[package]] name = "backtrace" version = "0.3.74" @@ -210,11 +274,11 @@ checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" [[package]] name = "bitflags" -version = "2.6.0" +version = "2.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" +checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" dependencies = [ - "serde", + "serde_core", ] [[package]] @@ -256,10 +320,11 @@ checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" [[package]] name = "cc" -version = "1.2.2" +version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f34d93e62b03caf570cccc334cbc6c2fceca82f39211051345108adcba3eebdc" +checksum = "5add81bb678e6cb321aff7fa0dc7689ad82b112dbc032cea19f91d6b8e3582b9" dependencies = [ + "find-msvc-tools", "jobserver", "libc", "shlex", @@ -346,7 +411,7 @@ dependencies = [ "heck 0.4.1", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -425,6 +490,16 @@ dependencies = [ "libc", ] +[[package]] +name = "core-foundation" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2a6cd9ae233e7f62ba4e9353e81a88df7fc8a5987b8d445b4d90c879bd156f6" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "core-foundation-sys" version = "0.8.7" @@ -520,7 +595,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn", + "syn 2.0.90", ] [[package]] @@ -531,7 +606,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -552,7 +627,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -562,7 +637,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab63b0e2bf4d5928aff72e83a7dace85d7bba5fe12dcc3c5a572d78caffd3f3c" dependencies = [ "derive_builder_core", - "syn", + "syn 2.0.90", ] [[package]] @@ -575,7 +650,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn", + "syn 2.0.90", ] [[package]] @@ -603,7 +678,7 @@ checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -663,7 +738,7 @@ dependencies = [ "optfield", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -684,6 +759,18 @@ version = "1.0.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d6ef0072f8a535281e4876be788938b528e9a1d43900b82c2569af7da799125" +[[package]] +name = "educe" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d7bc049e1bd8cdeb31b68bbd586a9464ecf9f3944af3958a7a9d0f8b9799417" +dependencies = [ + "enum-ordinalize", + "proc-macro2", + "quote", + "syn 2.0.90", +] + [[package]] name = "either" version = "1.13.0" @@ -699,6 +786,26 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "enum-ordinalize" +version = "4.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "89dd01549b09589510cf0647475075d12071456586d70f5c75c98ae2a5537677" +dependencies = [ + "enum-ordinalize-derive", +] + +[[package]] +name = "enum-ordinalize-derive" +version = "4.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a65863d15a4ce2888bd2f0f543cc963d3879c3a022c8ee43f6141d479a3ac815" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", +] + [[package]] name = "enumflags2" version = "0.7.10" @@ -717,7 +824,7 @@ checksum = "de0d48a183585823424a4ce1aa132d174a6a81bd540895822eb4c8373a8e49e8" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -772,6 +879,26 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "event-listener" +version = "5.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a23add41df1562121a9393cb065eab5146a1242410f23a644851e90cfd669d2" +dependencies = [ + "parking", + "pin-project-lite", +] + +[[package]] +name = "event-listener-strategy" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8be9f3dfaaffdae2972880079a491a1a8bb7cbed0b8dd7a347f668b4150a3b93" +dependencies = [ + "event-listener", + "pin-project-lite", +] + [[package]] name = "fastrand" version = "2.2.0" @@ -790,6 +917,12 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "find-msvc-tools" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" + [[package]] name = "fixedbitset" version = "0.4.2" @@ -812,6 +945,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "foreign-types" version = "0.3.2" @@ -892,7 +1031,7 @@ checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -966,7 +1105,7 @@ dependencies = [ "proc-macro-error2", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -1041,6 +1180,35 @@ name = "hashbrown" version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf151400ff0baff5465007dd2f3e717f3fe502074ca563069ce3a6629d07b289" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] + +[[package]] +name = "headers" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b3314d5adb5d94bcdf56771f2e50dbbc80bb4bdf88967526706205ac9eff24eb" +dependencies = [ + "base64 0.22.1", + "bytes", + "headers-core", + "http", + "httpdate", + "mime", + "sha1", +] + +[[package]] +name = "headers-core" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54b4a22553d4242c49fddb9ba998a99962b5cc6f22cb5a3482bec22522403ce4" +dependencies = [ + "http", +] [[package]] name = "heck" @@ -1188,6 +1356,42 @@ dependencies = [ "want", ] +[[package]] +name = "hyper-http-proxy" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8021e0ae20c08eadc94d0bdafdeda66d4f0858541c146ae6e46b219bfe58497e" +dependencies = [ + "bytes", + "futures-util", + "headers", + "http", + "hyper", + "hyper-rustls", + "hyper-util", + "pin-project-lite", + "tokio", + "tokio-rustls", + "tower-service", +] + +[[package]] +name = "hyper-rustls" +version = "0.27.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33ca68d021ef39cf6463ab54c1d0f5daf03377b70561305bb89a8f83aab66e0f" +dependencies = [ + "http", + "hyper", + "hyper-util", + "log", + "rustls", + "rustls-native-certs", + "tokio", + "tokio-rustls", + "tower-service", +] + [[package]] name = "hyper-timeout" version = "0.5.2" @@ -1376,7 +1580,7 @@ checksum = "1ec89e9337638ecdc08744df490b221a7399bf8d164eb52a665454e60e075ad6" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -1438,6 +1642,15 @@ version = "2.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b248f5224d1d606005e02c97f5aa4e88eeb230488bcc03bc9ca4d7991399f2b5" +[[package]] +name = "instant" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0242819d153cba4b4b05a5a8f2a7e9bbf97b6055b2a002b395c96b5ff3c0222" +dependencies = [ + "cfg-if", +] + [[package]] name = "inventory" version = "0.3.15" @@ -1501,6 +1714,41 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "json-patch" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "863726d7afb6bc2590eeff7135d923545e5e964f004c2ccf8716c25e70a86f08" +dependencies = [ + "jsonptr", + "serde", + "serde_json", + "thiserror 1.0.69", +] + +[[package]] +name = "jsonpath-rust" +version = "0.7.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c00ae348f9f8fd2d09f82a98ca381c60df9e0820d8d79fce43e649b4dc3128b" +dependencies = [ + "pest", + "pest_derive", + "regex", + "serde_json", + "thiserror 2.0.12", +] + +[[package]] +name = "jsonptr" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5dea2b27dd239b2556ed7a25ba842fe47fd602e7fc7433c2a8d6106d4d9edd70" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "jwt" version = "0.16.0" @@ -1516,6 +1764,115 @@ dependencies = [ "sha2", ] +[[package]] +name = "k8s-openapi" +version = "0.24.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c75b990324f09bef15e791606b7b7a296d02fc88a344f6eba9390970a870ad5" +dependencies = [ + "base64 0.22.1", + "chrono", + "serde", + "serde-value", + "serde_json", +] + +[[package]] +name = "kube" +version = "0.98.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32053dc495efad4d188c7b33cc7c02ef4a6e43038115348348876efd39a53cba" +dependencies = [ + "k8s-openapi", + "kube-client", + "kube-core", + "kube-runtime", +] + +[[package]] +name = "kube-client" +version = "0.98.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d34ad38cdfbd1fa87195d42569f57bb1dda6ba5f260ee32fef9570b7937a0c9" +dependencies = [ + "base64 0.22.1", + "bytes", + "chrono", + "either", + "futures", + "home", + "http", + "http-body", + "http-body-util", + "hyper", + "hyper-http-proxy", + "hyper-rustls", + "hyper-timeout", + "hyper-util", + "jsonpath-rust", + "k8s-openapi", + "kube-core", + "pem", + "rustls", + "rustls-pemfile", + "secrecy", + "serde", + "serde_json", + "serde_yaml", + "thiserror 2.0.12", + "tokio", + "tokio-util", + "tower", + "tower-http", + "tracing", +] + +[[package]] +name = "kube-core" +version = "0.98.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97aa830b288a178a90e784d1b0f1539f2d200d2188c7b4a3146d9dc983d596f3" +dependencies = [ + "chrono", + "form_urlencoded", + "http", + "json-patch", + "k8s-openapi", + "serde", + "serde-value", + "serde_json", + "thiserror 2.0.12", +] + +[[package]] +name = "kube-runtime" +version = "0.98.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a41af186a0fe80c71a13a13994abdc3ebff80859ca6a4b8a6079948328c135b" +dependencies = [ + "ahash", + "async-broadcast", + "async-stream", + "async-trait", + "backoff", + "educe", + "futures", + "hashbrown", + "hostname", + "json-patch", + "jsonptr", + "k8s-openapi", + "kube-client", + "parking_lot", + "pin-project", + "serde", + "serde_json", + "thiserror 2.0.12", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "lazy_static" version = "1.5.0" @@ -1671,10 +2028,10 @@ dependencies = [ "libc", "log", "openssl", - "openssl-probe", + "openssl-probe 0.1.5", "openssl-sys", "schannel", - "security-framework", + "security-framework 2.11.1", "security-framework-sys", "tempfile", ] @@ -1819,7 +2176,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -1828,6 +2185,12 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf" +[[package]] +name = "openssl-probe" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" + [[package]] name = "openssl-sys" version = "0.9.116" @@ -1848,7 +2211,16 @@ checksum = "fa59f025cde9c698fcb4fcb3533db4621795374065bee908215263488f2d2a1d" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", +] + +[[package]] +name = "ordered-float" +version = "2.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68f19d67e5a2795c94e73e0bb1cc1a7edeb2e28efd39e2e1c9b7a40c1108b11c" +dependencies = [ + "num-traits", ] [[package]] @@ -1916,6 +2288,12 @@ dependencies = [ "which", ] +[[package]] +name = "parking" +version = "2.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f38d5652c16fde515bb1ecef450ab0f6a219d619a7274976324d5e377f7dceba" + [[package]] name = "parking_lot" version = "0.12.3" @@ -1948,6 +2326,16 @@ dependencies = [ "regex", ] +[[package]] +name = "pem" +version = "3.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d30c53c26bc5b31a98cd02d20f25a7c8567146caf63ed593a9d87b2775291be" +dependencies = [ + "base64 0.22.1", + "serde_core", +] + [[package]] name = "percent-encoding" version = "2.3.1" @@ -1985,7 +2373,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2049,7 +2437,7 @@ dependencies = [ "phf_shared", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2078,7 +2466,7 @@ checksum = "6e918e4ff8c4549eb882f14b3a4bc8c8bc93de829416eacf579f1207a8fbf861" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2125,7 +2513,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "64d1ec885c64d0457d564db4ec299b2dae3f9c02808b8ad9c3a089c591b18033" dependencies = [ "proc-macro2", - "syn", + "syn 2.0.90", ] [[package]] @@ -2147,7 +2535,7 @@ dependencies = [ "proc-macro-error-attr2", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2212,7 +2600,7 @@ dependencies = [ "pulldown-cmark", "pulldown-cmark-to-cmark", "regex", - "syn", + "syn 2.0.90", "tempfile", ] @@ -2226,7 +2614,7 @@ dependencies = [ "itertools", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2278,7 +2666,7 @@ dependencies = [ "proc-macro2", "pytest", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2357,7 +2745,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b08f3c9802962f7e1b25113931d94f43ed9725bebc59db9d0c3e9a23b67e15ff" dependencies = [ "getrandom 0.3.1", - "zerocopy 0.8.17", + "zerocopy 0.8.27", ] [[package]] @@ -2461,6 +2849,20 @@ dependencies = [ "windows-registry", ] +[[package]] +name = "ring" +version = "0.17.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" +dependencies = [ + "cc", + "cfg-if", + "getrandom 0.2.15", + "libc", + "untrusted", + "windows-sys 0.52.0", +] + [[package]] name = "rustc-demangle" version = "0.1.24" @@ -2502,6 +2904,33 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustls" +version = "0.23.43" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0283386ce02abc0151e1761d08802dfe86c173b0b494af5cbc086574e453da06" +dependencies = [ + "log", + "once_cell", + "ring", + "rustls-pki-types", + "rustls-webpki", + "subtle", + "zeroize", +] + +[[package]] +name = "rustls-native-certs" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dab5152771c58876a2146916e53e35057e1a4dfa2b9df0f0305b07f611fdea4d" +dependencies = [ + "openssl-probe 0.2.1", + "rustls-pki-types", + "schannel", + "security-framework 3.7.0", +] + [[package]] name = "rustls-pemfile" version = "2.2.0" @@ -2513,9 +2942,23 @@ dependencies = [ [[package]] name = "rustls-pki-types" -version = "1.10.0" +version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "16f1201b3c9a7ee8039bcadc17b7e605e2945b27eee7631788c1bd2b0643674b" +checksum = "2f4925028c7eb5d1fcdaf196971378ed9d2c1c4efc7dc5d011256f76c99c0a96" +dependencies = [ + "zeroize", +] + +[[package]] +name = "rustls-webpki" +version = "0.103.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" +dependencies = [ + "ring", + "rustls-pki-types", + "untrusted", +] [[package]] name = "rustversion" @@ -2570,7 +3013,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn", + "syn 2.0.90", ] [[package]] @@ -2579,6 +3022,15 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "secrecy" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e891af845473308773346dc847b2c23ee78fe442e0472ac50e22a18a93d3ae5a" +dependencies = [ + "zeroize", +] + [[package]] name = "security-framework" version = "2.11.1" @@ -2586,7 +3038,20 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "897b2245f0b511c87893af39b033e5ca9cce68824c4d7e7630b5a1d339658d02" dependencies = [ "bitflags", - "core-foundation", + "core-foundation 0.9.4", + "core-foundation-sys", + "libc", + "security-framework-sys", +] + +[[package]] +name = "security-framework" +version = "3.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" +dependencies = [ + "bitflags", + "core-foundation 0.10.1", "core-foundation-sys", "libc", "security-framework-sys", @@ -2594,9 +3059,9 @@ dependencies = [ [[package]] name = "security-framework-sys" -version = "2.12.1" +version = "2.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa39c7303dc58b5543c94d22c1766b0d31f2ee58306363ea622b10bbc075eaa2" +checksum = "6ce2691df843ecc5d231c0b14ece2acc3efb62c0a398c7e1d875f3983ce020e3" dependencies = [ "core-foundation-sys", "libc", @@ -2621,6 +3086,16 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-value" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3a1a3341211875ef120e117ea7fd5228530ae7e7036a779fdc9117be6b3282c" +dependencies = [ + "ordered-float", + "serde", +] + [[package]] name = "serde_core" version = "1.0.228" @@ -2638,7 +3113,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2649,7 +3124,7 @@ checksum = "18d26a20a969b9e3fdf2fc2d9f21eda6c40e2de84c9408bb5d3b05d499aae711" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2675,6 +3150,15 @@ dependencies = [ "serde_core", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + [[package]] name = "serde_urlencoded" version = "0.7.1" @@ -2709,6 +3193,17 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "sha1" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a978451301f4db1d02937a4ab3ccce137717b81826e79b7d49ffe3244a13c3b8" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "sha2" version = "0.10.8" @@ -2741,9 +3236,9 @@ dependencies = [ [[package]] name = "shlex" -version = "1.3.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" [[package]] name = "signal-hook" @@ -2815,7 +3310,7 @@ checksum = "0eb01866308440fc64d6c44d9e86c5cc17adfe33c4d6eed55da9145044d0ffc1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -2902,7 +3397,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn", + "syn 2.0.90", ] [[package]] @@ -2915,7 +3410,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn", + "syn 2.0.90", ] [[package]] @@ -2941,6 +3436,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "sync_wrapper" version = "1.0.2" @@ -2958,7 +3464,7 @@ checksum = "c8af7666ab7b6390ab78131fb5b0fce11d6b7a6951602017c35fa82800708971" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -3115,7 +3621,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -3126,7 +3632,7 @@ checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -3189,7 +3695,7 @@ checksum = "af407857209536a95c8e56f8231ef2c2e2aff839b22e07a1ffcbc617e9db9fa5" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -3202,6 +3708,16 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-rustls" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" +dependencies = [ + "rustls", + "tokio", +] + [[package]] name = "tokio-stream" version = "0.1.17" @@ -3223,9 +3739,51 @@ dependencies = [ "futures-core", "futures-sink", "pin-project-lite", + "slab", "tokio", ] +[[package]] +name = "toml" +version = "0.8.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + +[[package]] +name = "toml_datetime" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +dependencies = [ + "serde", +] + +[[package]] +name = "toml_edit" +version = "0.22.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "toml_write", + "winnow", +] + +[[package]] +name = "toml_write" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" + [[package]] name = "tonic" version = "0.14.2" @@ -3264,7 +3822,7 @@ dependencies = [ "prettyplease", "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -3301,7 +3859,7 @@ dependencies = [ "prost-build", "prost-types", "quote", - "syn", + "syn 2.0.90", "tempfile", "tonic-build", ] @@ -3325,6 +3883,24 @@ dependencies = [ "tracing", ] +[[package]] +name = "tower-http" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cfcf7e2740e6fc6d4d688b4ef00650406bb94adf4731e43c096c3a19fe40840" +dependencies = [ + "base64 0.22.1", + "bitflags", + "bytes", + "http", + "http-body", + "mime", + "pin-project-lite", + "tower-layer", + "tower-service", + "tracing", +] + [[package]] name = "tower-layer" version = "0.3.3" @@ -3357,7 +3933,7 @@ checksum = "395ae124c09f9e6918a2310af6038fba074bcf474ac352496d5910dd59a2226d" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -3495,10 +4071,15 @@ name = "trident-acl-agent" version = "0.1.0" dependencies = [ "anyhow", + "chrono", "clap", "env_logger 0.11.5", "futures", + "humantime", + "hyper-util", "indoc", + "k8s-openapi", + "kube", "log", "maplit", "mockito", @@ -3509,12 +4090,17 @@ dependencies = [ "serde", "serde_json", "serde_path_to_error", + "serde_yaml", "sha2", "sysdefs", "systemd-journal-logger", + "tempfile", "thiserror 1.0.69", "tokio", + "tokio-stream", + "toml", "tonic", + "tower", "trident-proto", "url", "uuid", @@ -3685,6 +4271,12 @@ version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" +[[package]] +name = "untrusted" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" + [[package]] name = "url" version = "2.5.4" @@ -3799,7 +4391,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 2.0.90", "wasm-bindgen-shared", ] @@ -3834,7 +4426,7 @@ checksum = "98c9ae5a76e46f4deecd0f0255cc223cfa18dc9b261213b8aa0c7b36f61b3f1d" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -4197,6 +4789,15 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650" +[[package]] +name = "winnow" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" +dependencies = [ + "memchr", +] + [[package]] name = "winsafe" version = "0.0.19" @@ -4255,7 +4856,7 @@ checksum = "2380878cad4ac9aac1e2435f3eb4020e8374b5f13c296cb75b4620ff8e229154" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", "synstructure", ] @@ -4271,11 +4872,11 @@ dependencies = [ [[package]] name = "zerocopy" -version = "0.8.17" +version = "0.8.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa91407dacce3a68c56de03abe2760159582b846c6a4acd2f456618087f12713" +checksum = "0894878a5fa3edfd6da3f88c4805f4c8558e2b996227a3d864f47fe11e38282c" dependencies = [ - "zerocopy-derive 0.8.17", + "zerocopy-derive 0.8.27", ] [[package]] @@ -4286,18 +4887,18 @@ checksum = "fa4f8080344d4671fb4e831a13ad1e68092748387dfc4f55e356242fae12ce3e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] name = "zerocopy-derive" -version = "0.8.17" +version = "0.8.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06718a168365cad3d5ff0bb133aad346959a2074bd4a85c121255a11304a8626" +checksum = "88d2b8d9c68ad2b9e4340d7832716a4d21a22a1154777ad56ea55c51a9cf3831" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] @@ -4317,10 +4918,16 @@ checksum = "595eed982f7d355beb85837f651fa22e90b3c044842dc7f2c2842c086f295808" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", "synstructure", ] +[[package]] +name = "zeroize" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e13c156562582aa81c60cb29407084cdb54c4164760106ab78e6c5b0858cf64e" + [[package]] name = "zerovec" version = "0.10.4" @@ -4340,7 +4947,7 @@ checksum = "6eafa6dfb17584ea3e2bd6e76e0cc15ad7af12b09abdd1ca55961bed9b1063c6" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.90", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index d0baaae608..d2ac28ee96 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,9 @@ humantime = "2.3.0" hyper = "1.8.1" indoc = "2.0.5" inventory = "0.3.15" +k8s-openapi = { version = "0.24.0", features = ["v1_32"] } itertools = "0.13.0" +kube = { version = "0.98.0", default-features = false, features = ["client", "rustls-tls", "runtime"] } lazy_static = "1.5.0" libc = "0.2.167" log = "0.4.22" @@ -90,6 +92,7 @@ tar = "0.4.46" tempfile = "3.14.0" tera = "1.20.0" textwrap = "0.16.2" +toml = "0.8.23" thiserror = "1.0.69" tokio = { version = "1.48.0", features = ["full"] } tokio-stream = { version = "0.1.17", features = ["net"] } diff --git a/crates/trident-acl-agent/Cargo.toml b/crates/trident-acl-agent/Cargo.toml index 21c46aaace..1c40afa3f8 100644 --- a/crates/trident-acl-agent/Cargo.toml +++ b/crates/trident-acl-agent/Cargo.toml @@ -7,18 +7,26 @@ publish = false [dependencies] anyhow = { workspace = true, features = ["backtrace"] } clap = { workspace = true, features = ["derive"] } +chrono = { workspace = true } env_logger = { workspace = true } futures = { workspace = true } +humantime = { workspace = true } +k8s-openapi = { workspace = true } +kube = { workspace = true } log = { workspace = true } quick-xml = { workspace = true, features = ["serialize"] } reqwest = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +serde_yaml = { workspace = true } serde_path_to_error = { workspace = true } sha2 = { workspace = true } systemd-journal-logger = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } +tokio-stream = { workspace = true } +toml = { workspace = true } tonic = { workspace = true } url = { workspace = true, features = ["serde"] } uuid = { workspace = true, features = ["v4", "serde"] } @@ -29,7 +37,18 @@ trident-proto = { path = "../trident-proto" } [dev-dependencies] +tempfile = { workspace = true } indoc = { workspace = true } maplit = { workspace = true } mockito = { workspace = true } serde_json = { workspace = true } +serde_yaml = { workspace = true } +tower = { workspace = true } +# The generated gRPC *server* stubs (UpdateServiceServer/CommitServiceServer) +# are only needed to build an in-process fake tridentd for unit tests -- the +# production trident-acl-agent binary is a gRPC client only. Declaring the +# "server" feature here (dev-dependencies) rather than in [dependencies] +# keeps it out of the production binary's dependency graph; it is only +# pulled in for `cargo test`. +trident-proto = { path = "../trident-proto", features = ["grpc-preview", "server"] } +hyper-util = { version = "0.1", features = ["tokio"] } diff --git a/crates/trident-acl-agent/src/annotations.rs b/crates/trident-acl-agent/src/annotations.rs new file mode 100644 index 0000000000..c02f0a7649 --- /dev/null +++ b/crates/trident-acl-agent/src/annotations.rs @@ -0,0 +1,899 @@ +//! Request/status annotation protocol types for the Trident ACL agent. +//! +//! This module (schema types, `UpdateRequest::validate()`, and the +//! `#[cfg(test)]` design-doc conformance tests below) implements the +//! `acl.azure.com/update-request` / `acl.azure.com/update-status` node +//! annotation protocol designed in `docs/update-trigger-design.md`: +//! https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md +//! Keep `UpdateRequest`/`UpdateStatus`/`StatusCode` and `validate()` in sync +//! with that document's formal JSON Schema (its section "Formal JSON +//! Schema") - the `design_doc_*`/`agent_built_*_conform_to_formal_schema` +//! tests in this file's test module pin that JSON Schema in literally and +//! check both the doc's own examples and our constructed annotations +//! against it. + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +pub const UPDATE_REQUEST_ANNOTATION: &str = "acl.azure.com/update-request"; +pub const UPDATE_STATUS_ANNOTATION: &str = "acl.azure.com/update-status"; +pub const SCHEMA_VERSION: &str = "1.0"; +// TODO(DR-001): This is a temporary stub. The active OS version cannot yet be +// determined on-node because the OS image does not currently ship a required +// file/manifest describing the running version. Once that file exists (planned +// as part of the image build), replace CURRENT_VERSION_STUB and +// current_active_version() below with real logic that reads it. Until then, +// this stub can cause current_active_version() to spuriously equal a real +// requested target version, making handle_stage/handle_finalize incorrectly +// short-circuit to AlreadyAtTarget. Do not remove this comment when bumping the +// stub value; keep it until the real probe lands. +pub const CURRENT_VERSION_STUB: &str = "202601.1.0"; + +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Hash)] +#[serde(rename_all = "camelCase")] +pub enum RequestedOperation { + Stage, + Finalize, + Rollback, +} + +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Hash)] +#[serde(rename_all = "camelCase")] +pub enum Operation { + Stage, + Finalize, + Rollback, + Commit, +} + +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Hash)] +pub enum StatusCode { + InProgress, + Success, + AlreadyAtTarget, + NotStaged, + OperationFailed, + RevertedToPrevious, + AgentInternalError, + InvalidRequest, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct UpdateRequest { + pub schema_version: String, + pub node_update_id: Uuid, + pub operation_id: String, + pub operation: RequestedOperation, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub target_version: Option, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct UpdateStatus { + pub schema_version: String, + pub node_update_id: Uuid, + pub operation_id: String, + pub operation: Operation, + pub code: StatusCode, + pub message: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub from_version: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub to_version: Option, + pub started_utc: DateTime, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub finished_utc: Option>, +} + +impl UpdateRequest { + /// Enforces the same constraints as the request annotation's formal + /// JSON Schema in docs/update-trigger-design.md (https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md): + /// schemaVersion match, and targetVersion required for stage/finalize + /// but disallowed for rollback. See this file's module doc. + pub fn validate(self) -> Result { + if self.schema_version != SCHEMA_VERSION { + return Err(format!("unsupported schemaVersion {}", self.schema_version)); + } + match self.operation { + RequestedOperation::Stage | RequestedOperation::Finalize => { + if self.target_version.as_deref().unwrap_or("").is_empty() { + return Err("targetVersion is required for stage/finalize".to_string()); + } + } + RequestedOperation::Rollback => { + if self.target_version.is_some() { + return Err("targetVersion must be omitted for rollback".to_string()); + } + } + } + Ok(self) + } +} + +impl UpdateStatus { + // This constructor mirrors UpdateStatus's wire schema field-for-field + // (see accepted-design.md's two-annotation JSON protocol); splitting it + // into a builder would add ceremony across ~25 call sites in + // orchestrator.rs without making any of them clearer. + #[allow(clippy::too_many_arguments)] + pub fn new( + request: &UpdateRequest, + operation: Operation, + operation_id: String, + code: StatusCode, + message: impl Into, + from_version: Option, + to_version: Option, + started_utc: DateTime, + finished_utc: Option>, + ) -> Self { + Self { + schema_version: SCHEMA_VERSION.to_string(), + node_update_id: request.node_update_id, + operation_id, + operation, + code, + message: message.into(), + from_version, + to_version, + started_utc, + finished_utc, + } + } +} + +pub fn current_active_version() -> String { + // STUB: see CURRENT_VERSION_STUB above. Replace with a real probe (e.g. reading + // a version manifest file the OS image will ship) once available. Known risk: + // while stubbed, this can equal a real request's targetVersion and cause a + // false AlreadyAtTarget short-circuit in handle_stage/handle_finalize. + CURRENT_VERSION_STUB.to_string() +} + +pub fn commit_operation_id(operation_id: &str) -> String { + format!("{operation_id}.commit") +} + +impl From for Operation { + fn from(value: RequestedOperation) -> Self { + match value { + RequestedOperation::Stage => Operation::Stage, + RequestedOperation::Finalize => Operation::Finalize, + RequestedOperation::Rollback => Operation::Rollback, + } + } +} + +#[cfg(test)] +mod tests { + use chrono::TimeZone; + use serde_json::Value; + use uuid::Uuid; + + use super::*; + + fn sample_request(operation: RequestedOperation) -> UpdateRequest { + UpdateRequest { + schema_version: SCHEMA_VERSION.to_string(), + node_update_id: Uuid::parse_str("11111111-1111-1111-1111-111111111111").unwrap(), + operation_id: "op-1".to_string(), + operation, + target_version: Some("2.0.0".to_string()), + } + } + + fn fixed_time(secs: i64) -> DateTime { + Utc.timestamp_opt(1_700_000_000 + secs, 0).unwrap() + } + + /// Round-trips `status` through JSON and returns the parsed `Value`, also + /// asserting the annotation is valid JSON and that deserializing it back + /// produces an identical `UpdateStatus` (guards against any field being + /// silently dropped or renamed by a future schema change). + fn to_annotation_json(status: &UpdateStatus) -> Value { + let text = serde_json::to_string(status).expect("UpdateStatus must serialize to JSON"); + let value: Value = serde_json::from_str(&text).expect("annotation must be valid JSON"); + let round_tripped: UpdateStatus = + serde_json::from_str(&text).expect("annotation must deserialize back to UpdateStatus"); + assert_eq!(&round_tripped, status); + value + } + + #[test] + fn stage_success_annotation_has_expected_shape() { + let request = sample_request(RequestedOperation::Stage); + let status = UpdateStatus::new( + &request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::Success, + "stage completed", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + + let json = to_annotation_json(&status); + assert_eq!(json["schemaVersion"], "1.0"); + assert_eq!(json["operationId"], "op-1"); + assert_eq!(json["operation"], "stage"); + assert_eq!(json["code"], "Success"); + assert_eq!(json["message"], "stage completed"); + assert_eq!(json["fromVersion"], "1.0.0"); + assert_eq!(json["toVersion"], "2.0.0"); + assert!(json.get("startedUtc").is_some()); + assert!(json.get("finishedUtc").is_some()); + // Confirms camelCase renaming applies to every field, not just a subset. + assert!(json.get("nodeUpdateId").is_some()); + } + + #[test] + fn stage_failure_annotation_has_operation_failed_code() { + let request = sample_request(RequestedOperation::Stage); + let status = UpdateStatus::new( + &request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::OperationFailed, + "stage failed: disk full", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + + let json = to_annotation_json(&status); + assert_eq!(json["code"], "OperationFailed"); + assert_eq!(json["message"], "stage failed: disk full"); + } + + #[test] + fn finalize_success_annotation_records_operation_and_no_finish_before_reboot() { + let request = sample_request(RequestedOperation::Finalize); + // In-progress finalize status published before reboot has no + // finishedUtc yet - confirm the annotation omits the field entirely + // (skip_serializing_if) rather than emitting `null`. + let in_progress = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::InProgress, + "finalizing update", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + None, + ); + + let json = to_annotation_json(&in_progress); + assert_eq!(json["operation"], "finalize"); + assert_eq!(json["code"], "InProgress"); + assert!( + json.get("finishedUtc").is_none(), + "finishedUtc should be omitted, not null, while in progress" + ); + } + + #[test] + fn finalize_failure_reverted_annotation_has_reverted_to_previous_code() { + let request = sample_request(RequestedOperation::Finalize); + let status = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::RevertedToPrevious, + "finalize failed: trident reported ab-update-reboot-check failure", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + + let json = to_annotation_json(&status); + assert_eq!(json["code"], "RevertedToPrevious"); + assert_eq!(json["operationId"], "op-1"); + } + + #[test] + fn commit_success_annotation_uses_commit_suffixed_operation_id() { + let request = sample_request(RequestedOperation::Finalize); + let commit_id = commit_operation_id(&request.operation_id); + let status = UpdateStatus::new( + &request, + Operation::Commit, + commit_id.clone(), + StatusCode::Success, + "commit completed", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + + let json = to_annotation_json(&status); + assert_eq!(json["operationId"], "op-1.commit"); + assert_eq!(json["operation"], "commit"); + assert_eq!(json["code"], "Success"); + } + + #[test] + fn commit_reboot_required_annotation_uses_agent_internal_error_code() { + let request = sample_request(RequestedOperation::Finalize); + let commit_id = commit_operation_id(&request.operation_id); + let status = UpdateStatus::new( + &request, + Operation::Commit, + commit_id, + StatusCode::AgentInternalError, + "commit requested another reboot", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + + let json = to_annotation_json(&status); + assert_eq!(json["code"], "AgentInternalError"); + assert!(json["message"].as_str().unwrap().contains("another reboot")); + } + + #[test] + fn commit_failure_reverted_annotations_cover_both_reverted_subkinds() { + let request = sample_request(RequestedOperation::Finalize); + let commit_id = commit_operation_id(&request.operation_id); + + for message in [ + "commit failed: trident reported ab-update-reboot-check failure", + "commit failed: trident reported ab-update-health-check-commit-check failure", + ] { + let status = UpdateStatus::new( + &request, + Operation::Commit, + commit_id.clone(), + StatusCode::RevertedToPrevious, + message, + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + + let json = to_annotation_json(&status); + assert_eq!(json["code"], "RevertedToPrevious"); + assert_eq!(json["operationId"], "op-1.commit"); + } + } + + #[test] + fn commit_failure_generic_annotation_has_operation_failed_code() { + let request = sample_request(RequestedOperation::Finalize); + let commit_id = commit_operation_id(&request.operation_id); + let status = UpdateStatus::new( + &request, + Operation::Commit, + commit_id, + StatusCode::OperationFailed, + "commit failed: commit rpc failed", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + + let json = to_annotation_json(&status); + assert_eq!(json["code"], "OperationFailed"); + assert_eq!(json["operationId"], "op-1.commit"); + } + + #[test] + fn optional_version_fields_are_omitted_not_null_when_absent() { + let request = sample_request(RequestedOperation::Rollback); + let status = UpdateStatus::new( + &request, + Operation::Rollback, + request.operation_id.clone(), + StatusCode::InProgress, + "staging rollback", + Some("1.0.0".to_string()), + None, + fixed_time(0), + None, + ); + + let json = to_annotation_json(&status); + assert!(json.get("toVersion").is_none()); + assert!(json.get("finishedUtc").is_none()); + assert_eq!(json["fromVersion"], "1.0.0"); + } + + // --- docs/update-trigger-design.md conformance -------------------------- + // + // Pins our annotation (de)serialization/validation code against two + // things lifted verbatim from docs/update-trigger-design.md + // (https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md), + // section 2.1 "Trigger mechanism", so a doc/code drift shows up as a + // test failure instead of being discovered against a real AKS-RP: + // 1. The three example JSON payloads (request, finalize status, and + // the derived commit status) parse with our real UpdateRequest / + // UpdateStatus (de)serialization and UpdateRequest::validate(). + // 2. Annotations our own code constructs conform to the two formal + // JSON Schema documents embedded in the same section. + // + // Keep these constants byte-for-byte in sync with the design doc. + + /// docs/update-trigger-design.md 2.1, "Request annotation" example. + const DESIGN_DOC_FINALIZE_REQUEST_EXAMPLE: &str = r#"{ + "schemaVersion": "1.0", + "nodeUpdateId": "550e8400-e29b-41d4-a716-446655440000", + "operationId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", + "operation": "finalize", + "targetVersion": "202606.29.0" +}"#; + + /// docs/update-trigger-design.md 2.1, "Status annotation" example. + const DESIGN_DOC_FINALIZE_STATUS_EXAMPLE: &str = r#"{ + "schemaVersion": "1.0", + "nodeUpdateId": "550e8400-e29b-41d4-a716-446655440000", + "operationId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", + "operation": "finalize", + "code": "Success", + "message": "boot armed, rebooting, awaiting commit", + "fromVersion": "202605.15.0", + "toVersion": "202606.29.0", + "startedUtc": "2026-06-04T12:00:00Z", + "finishedUtc": "2026-06-04T12:00:32Z" +}"#; + + /// docs/update-trigger-design.md 2.1, the derived post-reboot commit status example. + const DESIGN_DOC_COMMIT_STATUS_EXAMPLE: &str = r#"{ + "schemaVersion": "1.0", + "nodeUpdateId": "550e8400-e29b-41d4-a716-446655440000", + "operationId": "f47ac10b-58cc-4372-a567-0e02b2c3d479.commit", + "operation": "commit", + "code": "Success", + "message": "booted expected volume, boot order promoted", + "fromVersion": "202605.15.0", + "toVersion": "202606.29.0", + "startedUtc": "2026-06-04T12:01:18Z", + "finishedUtc": "2026-06-04T12:01:32Z" +}"#; + + /// The formal JSON Schema for the request annotation, from + /// docs/update-trigger-design.md (https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md), + /// section 2.1 "Formal JSON Schema". Keep byte-for-byte in sync with + /// that document. + const DESIGN_DOC_REQUEST_SCHEMA: &str = r#"{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://acl.azure.com/schemas/update-request/1.0.json", + "title": "ACL A/B update request annotation", + "type": "object", + "additionalProperties": false, + "required": ["schemaVersion", "nodeUpdateId", "operationId", "operation"], + "properties": { + "schemaVersion": { "type": "string", "const": "1.0" }, + "nodeUpdateId": { "type": "string", "format": "uuid", "pattern": "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" }, + "operationId": { "type": "string", "format": "uuid", "pattern": "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" }, + "operation": { "type": "string", "enum": ["stage", "finalize", "rollback"] }, + "targetVersion": { "type": "string", "description": "ACL image release version, e.g. 202606.29.0." } + }, + "allOf": [ + { + "if": { "properties": { "operation": { "enum": ["stage", "finalize"] } }, "required": ["operation"] }, + "then": { "required": ["targetVersion"] } + } + ] +}"#; + + /// The formal JSON Schema for the status annotation, from + /// docs/update-trigger-design.md (https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md), + /// section 2.1 "Formal JSON Schema". Keep byte-for-byte in sync with + /// that document. + const DESIGN_DOC_STATUS_SCHEMA: &str = r#"{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://acl.azure.com/schemas/update-status/1.0.json", + "title": "ACL A/B update status annotation", + "type": "object", + "additionalProperties": false, + "required": ["schemaVersion", "nodeUpdateId", "operationId", "operation", "code"], + "properties": { + "schemaVersion": { "type": "string", "const": "1.0" }, + "nodeUpdateId": { "type": "string", "format": "uuid", "pattern": "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" }, + "operationId": { "type": "string", "pattern": "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}(\\.commit)?$", "description": "The request operationId; the implicit post-reboot commit status appends a '.commit' suffix to the finalize/rollback operationId." }, + "operation": { "type": "string", "enum": ["stage", "finalize", "rollback", "commit"] }, + "code": { "type": "string", "enum": ["InProgress", "Success", "AlreadyAtTarget", "NotStaged", "OperationFailed", "RevertedToPrevious", "AgentInternalError", "InvalidRequest"] }, + "message": { "type": "string" }, + "fromVersion": { "type": "string" }, + "toVersion": { "type": "string" }, + "startedUtc": { "type": "string", "format": "date-time" }, + "finishedUtc": { "type": "string", "format": "date-time" } + }, + "allOf": [ + { + "if": { "properties": { "code": { "const": "InProgress" } }, "required": ["code"] }, + "then": { "required": ["startedUtc"] }, + "else": { "required": ["startedUtc", "finishedUtc"] } + } + ] +}"#; + + // --- minimal JSON Schema subset validator ------------------------------ + // + // Deliberately not a general-purpose JSON Schema engine: supports only + // the exact vocabulary the two schemas above actually use (type, + // additionalProperties, required, properties.{type,const,enum,format, + // pattern}, and a single-level allOf/if/then/else). Panics loudly on any + // schema keyword/pattern/type/format it doesn't recognize, so if + // accepted-design.md's schemas grow new constraints, this validator's + // blind spots don't silently mask them - the test fails instead, + // prompting an update here. + + fn schema_validate(schema: &Value, instance: &Value) -> Result<(), String> { + let schema_obj = schema.as_object().ok_or("schema is not a JSON object")?; + let obj = instance + .as_object() + .ok_or("instance is not a JSON object")?; + + let properties = schema_obj.get("properties").and_then(Value::as_object); + + if schema_obj + .get("additionalProperties") + .and_then(Value::as_bool) + == Some(false) + { + if let Some(props) = properties { + for key in obj.keys() { + if !props.contains_key(key) { + return Err(format!( + "property {key:?} not declared in schema (additionalProperties: false)" + )); + } + } + } + } + + if let Some(required) = schema_obj.get("required").and_then(Value::as_array) { + for req in required { + let name = req.as_str().ok_or("required entry is not a string")?; + if !obj.contains_key(name) { + return Err(format!("missing required property {name:?}")); + } + } + } + + if let Some(props) = properties { + for (name, prop_schema) in props { + if let Some(value) = obj.get(name) { + schema_validate_property(name, prop_schema, value)?; + } + } + } + + if let Some(all_of) = schema_obj.get("allOf").and_then(Value::as_array) { + for clause in all_of { + let clause_obj = clause.as_object().ok_or("allOf entry is not an object")?; + let condition_met = match clause_obj.get("if") { + Some(if_schema) => schema_if_matches(if_schema, obj), + None => true, + }; + let branch = if condition_met { + clause_obj.get("then") + } else { + clause_obj.get("else") + }; + if let Some(branch) = branch { + schema_validate(branch, instance)?; + } + } + } + + Ok(()) + } + + fn schema_if_matches(if_schema: &Value, obj: &serde_json::Map) -> bool { + let Some(if_obj) = if_schema.as_object() else { + return false; + }; + if let Some(required) = if_obj.get("required").and_then(Value::as_array) { + for req in required { + let Some(name) = req.as_str() else { + return false; + }; + if !obj.contains_key(name) { + return false; + } + } + } + if let Some(props) = if_obj.get("properties").and_then(Value::as_object) { + for (name, prop_schema) in props { + let Some(value) = obj.get(name) else { + return false; + }; + if let Some(enum_values) = prop_schema.get("enum").and_then(Value::as_array) { + if !enum_values.iter().any(|v| v == value) { + return false; + } + } + if let Some(const_value) = prop_schema.get("const") { + if value != const_value { + return false; + } + } + } + } + true + } + + fn schema_validate_property( + name: &str, + prop_schema: &Value, + value: &Value, + ) -> Result<(), String> { + if let Some(expected_type) = prop_schema.get("type").and_then(Value::as_str) { + let matches = match expected_type { + "string" => value.is_string(), + "object" => value.is_object(), + "array" => value.is_array(), + "boolean" => value.is_boolean(), + "number" | "integer" => value.is_number(), + other => panic!( + "test schema validator does not support type {other:?} - extend schema_validate_property" + ), + }; + if !matches { + return Err(format!( + "property {name:?}: expected type {expected_type}, got {value:?}" + )); + } + } + if let Some(const_value) = prop_schema.get("const") { + if value != const_value { + return Err(format!( + "property {name:?}: expected const {const_value:?}, got {value:?}" + )); + } + } + if let Some(enum_values) = prop_schema.get("enum").and_then(Value::as_array) { + if !enum_values.iter().any(|v| v == value) { + return Err(format!( + "property {name:?}: value {value:?} not in enum {enum_values:?}" + )); + } + } + if let Some(format) = prop_schema.get("format").and_then(Value::as_str) { + let s = value.as_str().ok_or_else(|| { + format!("property {name:?}: expected string for format {format:?}") + })?; + match format { + "uuid" => { + Uuid::parse_str(s).map_err(|err| { + format!("property {name:?}: {s:?} is not a valid uuid: {err}") + })?; + } + "date-time" => { + DateTime::parse_from_rfc3339(s).map_err(|err| { + format!("property {name:?}: {s:?} is not a valid date-time: {err}") + })?; + } + other => panic!( + "test schema validator does not support format {other:?} - extend schema_validate_property" + ), + } + } + if let Some(pattern) = prop_schema.get("pattern").and_then(Value::as_str) { + let s = value + .as_str() + .ok_or_else(|| format!("property {name:?}: expected string to check pattern"))?; + if !schema_pattern_matches(pattern, s) { + return Err(format!( + "property {name:?}: {s:?} does not match pattern {pattern:?}" + )); + } + } + Ok(()) + } + + /// Bespoke stand-in for full regex support: the two schemas above use + /// exactly two distinct patterns, both UUID-shaped, so this matches them + /// by exact pattern text rather than pulling in a regex engine for two + /// known cases. Panics on an unrecognized pattern so a future schema + /// change can't silently pass unchecked. + fn schema_pattern_matches(pattern: &str, value: &str) -> bool { + const BARE_UUID: &str = + r"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$"; + const UUID_OPTIONAL_COMMIT_SUFFIX: &str = r"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}(\.commit)?$"; + match pattern { + BARE_UUID => Uuid::parse_str(value).is_ok(), + UUID_OPTIONAL_COMMIT_SUFFIX => match value.strip_suffix(".commit") { + Some(prefix) => Uuid::parse_str(prefix).is_ok(), + None => Uuid::parse_str(value).is_ok(), + }, + other => panic!( + "test schema validator does not recognize pattern {other:?} - extend schema_pattern_matches" + ), + } + } + + // --- example payload parsing tests ------------------------------------- + + #[test] + fn design_doc_finalize_request_example_parses_and_validates() { + let request: UpdateRequest = serde_json::from_str(DESIGN_DOC_FINALIZE_REQUEST_EXAMPLE) + .expect("design doc's finalize request example must parse as UpdateRequest"); + let request = request + .validate() + .expect("design doc's finalize request example must pass UpdateRequest::validate()"); + assert_eq!(request.schema_version, "1.0"); + assert_eq!( + request.node_update_id, + Uuid::parse_str("550e8400-e29b-41d4-a716-446655440000").unwrap() + ); + assert_eq!(request.operation_id, "f47ac10b-58cc-4372-a567-0e02b2c3d479"); + assert_eq!(request.operation, RequestedOperation::Finalize); + assert_eq!(request.target_version.as_deref(), Some("202606.29.0")); + } + + #[test] + fn design_doc_finalize_status_example_parses() { + let status: UpdateStatus = serde_json::from_str(DESIGN_DOC_FINALIZE_STATUS_EXAMPLE) + .expect("design doc's finalize status example must parse as UpdateStatus"); + assert_eq!(status.operation, Operation::Finalize); + assert_eq!(status.code, StatusCode::Success); + assert_eq!(status.from_version.as_deref(), Some("202605.15.0")); + assert_eq!(status.to_version.as_deref(), Some("202606.29.0")); + assert!(status.finished_utc.is_some()); + } + + #[test] + fn design_doc_commit_status_example_parses() { + let status: UpdateStatus = serde_json::from_str(DESIGN_DOC_COMMIT_STATUS_EXAMPLE) + .expect("design doc's commit status example must parse as UpdateStatus"); + assert_eq!(status.operation, Operation::Commit); + assert_eq!(status.code, StatusCode::Success); + assert_eq!( + status.operation_id, + "f47ac10b-58cc-4372-a567-0e02b2c3d479.commit" + ); + } + + // --- example payloads validated against the embedded formal schema ---- + + #[test] + fn design_doc_finalize_request_example_matches_formal_schema() { + let schema: Value = serde_json::from_str(DESIGN_DOC_REQUEST_SCHEMA).unwrap(); + let instance: Value = serde_json::from_str(DESIGN_DOC_FINALIZE_REQUEST_EXAMPLE).unwrap(); + schema_validate(&schema, &instance) + .expect("design doc's own finalize request example must satisfy its own schema"); + } + + #[test] + fn design_doc_finalize_status_example_matches_formal_schema() { + let schema: Value = serde_json::from_str(DESIGN_DOC_STATUS_SCHEMA).unwrap(); + let instance: Value = serde_json::from_str(DESIGN_DOC_FINALIZE_STATUS_EXAMPLE).unwrap(); + schema_validate(&schema, &instance) + .expect("design doc's own finalize status example must satisfy its own schema"); + } + + #[test] + fn design_doc_commit_status_example_matches_formal_schema() { + let schema: Value = serde_json::from_str(DESIGN_DOC_STATUS_SCHEMA).unwrap(); + let instance: Value = serde_json::from_str(DESIGN_DOC_COMMIT_STATUS_EXAMPLE).unwrap(); + schema_validate(&schema, &instance) + .expect("design doc's own commit status example must satisfy its own schema"); + } + + // --- annotations *we construct* validated against the embedded schema - + + #[test] + fn agent_built_requests_conform_to_formal_schema() { + let schema: Value = serde_json::from_str(DESIGN_DOC_REQUEST_SCHEMA).unwrap(); + let node_update_id = Uuid::new_v4(); + + for (operation, target_version) in [ + (RequestedOperation::Stage, Some("202606.29.0".to_string())), + ( + RequestedOperation::Finalize, + Some("202606.29.0".to_string()), + ), + (RequestedOperation::Rollback, None), + ] { + let request = UpdateRequest { + schema_version: SCHEMA_VERSION.to_string(), + node_update_id, + operation_id: Uuid::new_v4().to_string(), + operation, + target_version, + }; + let request = request + .validate() + .unwrap_or_else(|err| panic!("{operation:?} request must validate: {err}")); + let instance: Value = serde_json::to_value(&request).unwrap(); + schema_validate(&schema, &instance).unwrap_or_else(|err| { + panic!( + "agent-constructed {operation:?} request must conform to the formal schema: {err}" + ) + }); + } + } + + #[test] + fn agent_built_statuses_conform_to_formal_schema() { + let schema: Value = serde_json::from_str(DESIGN_DOC_STATUS_SCHEMA).unwrap(); + let request = UpdateRequest { + schema_version: SCHEMA_VERSION.to_string(), + node_update_id: Uuid::new_v4(), + operation_id: Uuid::new_v4().to_string(), + operation: RequestedOperation::Finalize, + target_version: Some("202606.29.0".to_string()), + }; + + // InProgress: startedUtc only, no finishedUtc yet. + let in_progress = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::InProgress, + "finalizing update", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + None, + ); + schema_validate(&schema, &serde_json::to_value(&in_progress).unwrap()) + .expect("agent-constructed InProgress status must conform to the formal schema"); + + // Terminal Success: both startedUtc and finishedUtc present. + let success = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::Success, + "boot armed, rebooting, awaiting commit", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(0), + Some(fixed_time(5)), + ); + schema_validate(&schema, &serde_json::to_value(&success).unwrap()) + .expect("agent-constructed terminal Success status must conform to the formal schema"); + + // Rollback status: no toVersion - still conforms (toVersion is optional). + let rollback_request = UpdateRequest { + operation: RequestedOperation::Rollback, + target_version: None, + ..request.clone() + }; + let rollback_status = UpdateStatus::new( + &rollback_request, + Operation::Rollback, + rollback_request.operation_id.clone(), + StatusCode::Success, + "rollback finalize completed; rebooting for commit", + Some("2.0.0".to_string()), + None, + fixed_time(0), + Some(fixed_time(5)), + ); + schema_validate(&schema, &serde_json::to_value(&rollback_status).unwrap()) + .expect("agent-constructed rollback status must conform to the formal schema"); + + // Derived post-reboot commit status: operationId gets a ".commit" + // suffix - exercises the status schema's optional-suffix pattern. + let commit_status = UpdateStatus::new( + &request, + Operation::Commit, + commit_operation_id(&request.operation_id), + StatusCode::Success, + "booted expected volume, boot order promoted", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + fixed_time(10), + Some(fixed_time(15)), + ); + schema_validate(&schema, &serde_json::to_value(&commit_status).unwrap()) + .expect("agent-constructed commit status must conform to the formal schema"); + } +} diff --git a/crates/trident-acl-agent/src/config.rs b/crates/trident-acl-agent/src/config.rs new file mode 100644 index 0000000000..432212cd43 --- /dev/null +++ b/crates/trident-acl-agent/src/config.rs @@ -0,0 +1,375 @@ +//! Config loading for Harpoon. +//! +//! See the design doc's endpoint override section (§12). Label mode is opt-in +//! through config only; defaults intentionally preserve the historical +//! `omaha-only` one-shot behavior. + +use std::{ + env, fs, + path::{Path, PathBuf}, + time::Duration, +}; + +use anyhow::Context; +use serde::Deserialize; +use url::Url; + +use crate::DEFAULT_NEBRASKA_APP_ID; + +pub const DEFAULT_CONFIG_PATH: &str = "/etc/trident/trident-acl-agent.conf"; +pub const DEFAULT_KUBERNETES_API_SERVER: &str = "https://kubernetes.default.svc"; +const DEFAULT_KUBERNETES_POLL_INTERVAL: Duration = Duration::from_secs(2); +const DEFAULT_NEBRASKA_POLL_INTERVAL: Duration = Duration::from_secs(5 * 60); +const DEFAULT_STAGE_TIMEOUT: Duration = Duration::from_secs(20 * 60); +const DEFAULT_FINALIZE_TIMEOUT: Duration = Duration::from_secs(10 * 60); +pub const DEFAULT_STATE_PATH: &str = "/var/lib/trident-acl-agent/state.json"; +pub const DEFAULT_KUBELET_KUBECONFIG: &str = "/var/lib/kubelet/kubeconfig"; + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct AgentConfig { + pub nebraska: NebraskaConfig, + pub kubernetes: KubernetesConfig, + pub trident: TridentConfig, + pub orchestration: OrchestrationConfig, +} + +impl AgentConfig { + pub fn load(path: &Path, explicit: bool) -> Result, anyhow::Error> { + match fs::read_to_string(path) { + Ok(contents) => Ok(Some(Self::from_toml(&contents)?)), + Err(err) if err.kind() == std::io::ErrorKind::NotFound && !explicit => Ok(None), + Err(err) => Err(anyhow::Error::new(err).context(format!( + "failed to read Harpoon config at {}", + path.display() + ))), + } + } + + pub fn from_toml(contents: &str) -> Result { + let raw: RawAgentConfig = + toml::from_str(contents).context("failed to parse config.toml")?; + raw.into_effective() + } + + pub fn with_cli_endpoint(mut self, cli_endpoint: Option) -> Self { + if cli_endpoint.is_some() { + self.nebraska.endpoint = cli_endpoint; + } + self + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NebraskaConfig { + pub endpoint: Option, + pub app_id: String, + pub poll_interval: Duration, +} + +impl Default for NebraskaConfig { + fn default() -> Self { + Self { + endpoint: None, + app_id: DEFAULT_NEBRASKA_APP_ID.to_string(), + poll_interval: DEFAULT_NEBRASKA_POLL_INTERVAL, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct KubernetesConfig { + pub api_server: Url, + pub kubeconfig: String, + pub node_name: String, + pub watch_poll_interval: Duration, +} + +impl Default for KubernetesConfig { + fn default() -> Self { + Self { + api_server: Url::parse(DEFAULT_KUBERNETES_API_SERVER).expect("static url"), + kubeconfig: DEFAULT_KUBELET_KUBECONFIG.to_string(), + node_name: default_node_name(), + watch_poll_interval: DEFAULT_KUBERNETES_POLL_INTERVAL, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TridentConfig { + pub socket: String, +} + +impl Default for TridentConfig { + fn default() -> Self { + Self { + socket: trident_proto::TRIDENT_DEFAULT_SOCKET_URI.to_string(), + } + } +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum GoalSource { + #[default] + OmahaOnly, + Labels, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OrchestrationConfig { + pub goal_source: GoalSource, + pub state_path: PathBuf, + /// Placeholder default pending real data from storm aclagent scenario runs. + pub stage_timeout: Duration, + /// Placeholder default pending real data from storm aclagent scenario runs. + pub finalize_timeout: Duration, +} + +impl Default for OrchestrationConfig { + fn default() -> Self { + Self { + goal_source: GoalSource::OmahaOnly, + state_path: PathBuf::from(DEFAULT_STATE_PATH), + stage_timeout: DEFAULT_STAGE_TIMEOUT, + finalize_timeout: DEFAULT_FINALIZE_TIMEOUT, + } + } +} + +#[derive(Debug, Default, Deserialize)] +struct RawAgentConfig { + #[serde(default)] + nebraska: RawNebraskaConfig, + #[serde(default)] + kubernetes: RawKubernetesConfig, + #[serde(default)] + trident: RawTridentConfig, + #[serde(default)] + orchestration: RawOrchestrationConfig, +} + +impl RawAgentConfig { + fn into_effective(self) -> Result { + Ok(AgentConfig { + nebraska: NebraskaConfig { + endpoint: self.nebraska.endpoint, + app_id: self + .nebraska + .app_id + .unwrap_or_else(|| DEFAULT_NEBRASKA_APP_ID.to_string()), + poll_interval: parse_duration( + self.nebraska.poll_interval.as_deref(), + DEFAULT_NEBRASKA_POLL_INTERVAL, + "nebraska.poll_interval", + )?, + }, + kubernetes: KubernetesConfig { + api_server: self.kubernetes.api_server.unwrap_or_else(|| { + Url::parse(DEFAULT_KUBERNETES_API_SERVER).expect("static url") + }), + kubeconfig: self + .kubernetes + .kubeconfig + .unwrap_or_else(|| DEFAULT_KUBELET_KUBECONFIG.to_string()), + node_name: self + .kubernetes + .node_name + .map(expand_env_token) + .transpose()? + .unwrap_or_else(default_node_name), + watch_poll_interval: DEFAULT_KUBERNETES_POLL_INTERVAL, + }, + trident: TridentConfig { + socket: self + .trident + .socket + .unwrap_or_else(|| trident_proto::TRIDENT_DEFAULT_SOCKET_URI.to_string()), + }, + orchestration: OrchestrationConfig { + goal_source: self.orchestration.goal_source.unwrap_or_default(), + state_path: self + .orchestration + .state_path + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from(DEFAULT_STATE_PATH)), + stage_timeout: parse_duration( + self.orchestration.stage_timeout.as_deref(), + DEFAULT_STAGE_TIMEOUT, + "orchestration.stage_timeout", + )?, + finalize_timeout: parse_duration( + self.orchestration.finalize_timeout.as_deref(), + DEFAULT_FINALIZE_TIMEOUT, + "orchestration.finalize_timeout", + )?, + }, + }) + } +} + +#[derive(Debug, Default, Deserialize)] +struct RawNebraskaConfig { + endpoint: Option, + app_id: Option, + poll_interval: Option, +} + +#[derive(Debug, Default, Deserialize)] +struct RawKubernetesConfig { + api_server: Option, + kubeconfig: Option, + node_name: Option, +} + +#[derive(Debug, Default, Deserialize)] +struct RawTridentConfig { + socket: Option, +} + +#[derive(Debug, Default, Deserialize)] +struct RawOrchestrationConfig { + goal_source: Option, + state_path: Option, + stage_timeout: Option, + finalize_timeout: Option, +} + +fn parse_duration( + value: Option<&str>, + default: Duration, + field: &str, +) -> Result { + value + .map(|value| { + humantime::parse_duration(value) + .map_err(|err| anyhow::anyhow!("invalid duration for {field}: {err}")) + }) + .transpose()? + .unwrap_or(default) + .pipe(Ok) +} + +fn expand_env_token(value: String) -> Result { + if let Some(name) = value.strip_prefix("${").and_then(|v| v.strip_suffix('}')) { + return env::var(name).map_err(|_| { + anyhow::anyhow!("environment variable {name} is not set for kubernetes.node_name") + }); + } + if let Some(name) = value.strip_prefix('$') { + return env::var(name).map_err(|_| { + anyhow::anyhow!("environment variable {name} is not set for kubernetes.node_name") + }); + } + Ok(value) +} + +fn default_node_name() -> String { + // Kubernetes Node names must be valid RFC 1123 DNS labels, which are + // lowercase-only; kubelet itself lowercases the hostname when it + // registers the Node object. Match that behavior here so a mixed-case + // hostname doesn't produce a node_name that can never match the actual + // Node the agent is supposed to reconcile against. + osutils::hostname::read() + .unwrap_or_else(|_| "localhost".to_string()) + .to_lowercase() +} + +trait Pipe: Sized { + fn pipe(self, f: impl FnOnce(Self) -> T) -> T { + f(self) + } +} + +impl Pipe for T {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_defaults() { + let config = AgentConfig::from_toml("").unwrap(); + assert_eq!(config.nebraska.endpoint, None); + assert_eq!(config.nebraska.app_id, DEFAULT_NEBRASKA_APP_ID); + assert_eq!( + config.nebraska.poll_interval, + DEFAULT_NEBRASKA_POLL_INTERVAL + ); + assert_eq!( + config.kubernetes.api_server.as_str(), + "https://kubernetes.default.svc/" + ); + assert_eq!( + config.trident.socket, + trident_proto::TRIDENT_DEFAULT_SOCKET_URI + ); + assert_eq!(config.orchestration.goal_source, GoalSource::OmahaOnly); + assert_eq!( + config.orchestration.state_path, + PathBuf::from(DEFAULT_STATE_PATH) + ); + assert_eq!(config.orchestration.stage_timeout, DEFAULT_STAGE_TIMEOUT); + assert_eq!( + config.orchestration.finalize_timeout, + DEFAULT_FINALIZE_TIMEOUT + ); + } + + #[test] + fn parses_overrides() { + let config = AgentConfig::from_toml( + r#" + [nebraska] + endpoint = "https://nebraska.example.invalid/v1/update" + app_id = "custom-app" + poll_interval = "7m" + + [kubernetes] + api_server = "https://cluster.example.invalid" + kubeconfig = "/etc/harpoon/kubeconfig" + node_name = "node-42" + + [trident] + socket = "unix:///custom/trident.sock" + + [orchestration] + goal_source = "labels" + state_path = "/var/lib/trident-acl-agent/custom-state.json" + stage_timeout = "21m" + finalize_timeout = "11m" + "#, + ) + .unwrap(); + + assert_eq!( + config.nebraska.endpoint.unwrap().as_str(), + "https://nebraska.example.invalid/v1/update" + ); + assert_eq!(config.nebraska.app_id, "custom-app"); + assert_eq!(config.nebraska.poll_interval, Duration::from_secs(7 * 60)); + assert_eq!( + config.kubernetes.api_server.as_str(), + "https://cluster.example.invalid/" + ); + assert_eq!( + config.kubernetes.kubeconfig.as_str(), + "/etc/harpoon/kubeconfig" + ); + assert_eq!(config.kubernetes.node_name, "node-42"); + assert_eq!(config.trident.socket, "unix:///custom/trident.sock"); + assert_eq!(config.orchestration.goal_source, GoalSource::Labels); + assert_eq!( + config.orchestration.state_path, + PathBuf::from("/var/lib/trident-acl-agent/custom-state.json") + ); + assert_eq!( + config.orchestration.stage_timeout, + Duration::from_secs(21 * 60) + ); + assert_eq!( + config.orchestration.finalize_timeout, + Duration::from_secs(11 * 60) + ); + } +} diff --git a/crates/trident-acl-agent/src/k8s.rs b/crates/trident-acl-agent/src/k8s.rs new file mode 100644 index 0000000000..600fe46d91 --- /dev/null +++ b/crates/trident-acl-agent/src/k8s.rs @@ -0,0 +1,128 @@ +//! Thin Kubernetes client wrapper for Harpoon's node self-patching protocol. +//! +//! Implements the Node get/watch/patch access described in +//! `docs/update-trigger-design.md`: +//! https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md +//! +//! The design calls for get/patch access to exactly one Node object (§3–§5). +//! Node changes are observed via the Kubernetes watch API (`kube::runtime::watcher`) +//! rather than polling, so label updates are delivered promptly and without +//! placing repeated load on the API server. `watch_poll_interval` still bounds +//! how quickly the watcher notices a dropped/re-established connection (used +//! as the watcher's backoff ceiling) and how often the fake test API server +//! needs to support being polled if it does not support real watches. + +use std::{collections::BTreeMap, path::Path}; + +use anyhow::Context; +use futures::{stream::BoxStream, StreamExt, TryStreamExt}; +use k8s_openapi::api::core::v1::Node; +use kube::{ + api::{Patch, PatchParams}, + config::{KubeConfigOptions, Kubeconfig}, + runtime::{watcher, WatchStreamExt}, + Api, Client, Config, +}; +use serde_json::json; + +use crate::config::KubernetesConfig; + +#[derive(Debug, thiserror::Error)] +pub enum K8sClientError { + #[error("failed to build Kubernetes client config: {0}")] + Config(#[from] anyhow::Error), + #[error("failed Kubernetes API call: {0}")] + Api(#[from] kube::Error), + #[error("Kubernetes watch stream failed: {0}")] + Watch(#[from] kube::runtime::watcher::Error), +} + +#[derive(Clone)] +pub struct NodeClient { + api: Api, + poll_interval: std::time::Duration, +} + +impl NodeClient { + pub async fn new(config: &KubernetesConfig) -> Result { + let client_config = load_client_config(config).await?; + let client = Client::try_from(client_config).map_err(anyhow::Error::new)?; + Ok(Self { + api: Api::all(client), + poll_interval: config.watch_poll_interval, + }) + } + + pub async fn get_node(&self, name: &str) -> Result { + Ok(self.api.get(name).await?) + } + + pub async fn patch_node_labels( + &self, + name: &str, + labels: BTreeMap, + ) -> Result { + let patch = json!({ "metadata": { "labels": labels } }); + Ok(self + .api + .patch(name, &PatchParams::default(), &Patch::Merge(&patch)) + .await?) + } + + pub async fn patch_node_annotations( + &self, + name: &str, + annotations: BTreeMap, + ) -> Result { + let patch = json!({ "metadata": { "annotations": annotations } }); + Ok(self + .api + .patch(name, &PatchParams::default(), &Patch::Merge(&patch)) + .await?) + } + + pub async fn patch_node_metadata( + &self, + name: &str, + labels: BTreeMap>, + annotations: BTreeMap>, + ) -> Result { + let patch = json!({ + "metadata": { + "labels": labels, + "annotations": annotations, + } + }); + Ok(self + .api + .patch(name, &PatchParams::default(), &Patch::Merge(&patch)) + .await?) + } + + /// Streams Node updates for `name` using the Kubernetes watch API instead + /// of polling. `watcher::Config::default().fields(...)` scopes the watch + /// server-side to the single node we care about, and `.default_backoff()` + /// governs reconnect timing (capped near `poll_interval`) if the watch + /// connection drops. + pub fn watch_node(&self, name: String) -> BoxStream<'static, Result> { + let watcher_config = watcher::Config::default() + .fields(&format!("metadata.name={name}")) + .timeout(self.poll_interval.as_secs().max(1) as u32); + + watcher(self.api.clone(), watcher_config) + .default_backoff() + .touched_objects() + .map_err(K8sClientError::from) + .boxed() + } +} + +async fn load_client_config(config: &KubernetesConfig) -> Result { + let path = Path::new(&config.kubeconfig); + let kubeconfig = Kubeconfig::read_from(path) + .with_context(|| format!("failed to read kubeconfig {}", path.display()))?; + let mut client_config = + Config::from_custom_kubeconfig(kubeconfig, &KubeConfigOptions::default()).await?; + client_config.cluster_url = config.api_server.as_str().parse()?; + Ok(client_config) +} diff --git a/crates/trident-acl-agent/src/lib.rs b/crates/trident-acl-agent/src/lib.rs new file mode 100644 index 0000000000..72abad8807 --- /dev/null +++ b/crates/trident-acl-agent/src/lib.rs @@ -0,0 +1,458 @@ +//! # Harpoon +//! +//! Harpoon is Trident's ACL update sidecar. Historically it was a one-shot +//! Omaha client that called Trident's combined `Update()` RPC once and exited. +//! This crate now also supports the AKS label protocol described in the local +//! design doc (`aks-rp ↔ trident-acl-agent`, especially §3–§6 and §12–§13), +//! while preserving the original `omaha-only` mode as the default. + +use semver::Version; +use sha2::{Digest, Sha256}; +use url::Url; +use uuid::Uuid; + +pub mod annotations; +pub mod config; +pub mod error; +pub mod id; +pub mod k8s; +pub mod omaha; +pub mod orchestrator; +pub mod state; +pub mod trident; + +/// Only built for `cargo test` (relies on trident-proto's `server` feature, +/// which is only enabled via trident-acl-agent's dev-dependencies - see +/// mock_tridentd.rs's module docs). +#[cfg(test)] +pub mod mock_tridentd; + +use error::HarpoonError; +use omaha::{ + event::{OmahaEvent, OmahaEventType}, + request::{AppRequest, Request}, + response::Package, +}; +use trident::TridentClient; + +pub use id::IdSource; +pub use omaha::event::EventResult; + +pub const DEFAULT_NEBRASKA_APP_ID: &str = "b0ec8f0d-1c13-4bf4-9efd-ea54464a7098"; +pub const DEFAULT_NEBRASKA_TRACK: &str = "west-us"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OmahaUpdate { + pub url: Url, + pub version: Version, + pub hash: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HarpoonQueryResponse { + pub session_id: Uuid, + pub result: QueryResult, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum QueryResult { + NoUpdate, + NewDocument(OmahaUpdate), +} + +pub async fn run_omaha_only(config: &config::AgentConfig) -> Result<(), anyhow::Error> { + let endpoint = config.nebraska.endpoint.clone().ok_or_else(|| { + anyhow::anyhow!( + "no Nebraska endpoint configured: pass on the CLI or set [nebraska].endpoint in config.toml" + ) + })?; + + let response = query_for_update( + &endpoint, + &config.nebraska.app_id, + DEFAULT_NEBRASKA_TRACK, + &Version::new(0, 0, 0), + IdSource::MachineIdHashed, + )?; + + match response.result { + QueryResult::NoUpdate => { + log::debug!("No update available from Nebraska"); + Ok(()) + } + QueryResult::NewDocument(update) => { + log::info!("Triggering one-shot Omaha update to {}", update.version); + let mut client = TridentClient::connect(&config.trident.socket).await?; + let combined_timeout = + config.orchestration.stage_timeout + config.orchestration.finalize_timeout; + client + .update(&update.url, update.hash.as_deref(), combined_timeout) + .await?; + Ok(()) + } + } +} + +/// Query the Omaha server at the given URL for the given app and track. +pub fn query_for_update( + url: &Url, + app_id: &str, + track: &str, + document_version: &Version, + machine_id_source: IdSource, +) -> Result { + let request = Request::default().with_app( + AppRequest::new(app_id, document_version, track, machine_id_source)?.with_update_check(), + ); + + let response = omaha::send(url, &request)?; + + log::debug!( + "Received response from Omaha server at '{}' for app '{}' on track '{}': {response:#?}", + url, + app_id, + track, + ); + if response.apps().len() != 1 { + return Err(HarpoonError::InvalidResponse( + "Expected exactly one app in response".to_string(), + )); + } + + let app = response.apps().first().expect("validated len above"); + + if app.app_id() != app_id { + return Err(HarpoonError::InvalidResponse( + "Unexpected app ID in response".to_string(), + )); + } + + if app.status().is_error() { + return Err(HarpoonError::QueryError(format!( + "Received a non-OK app status: {}", + app.status() + ))); + } + + let update_check = app.update_check().ok_or_else(|| { + HarpoonError::InvalidResponse("Missing update check in response".to_string()) + })?; + log::debug!("Received update check response: {update_check:#?}"); + + if update_check.status().is_error() { + return Err(HarpoonError::QueryError(format!( + "Received an error status in update check: {}", + update_check.status() + ))); + } + + if update_check.status().is_no_update() { + log::debug!( + "No update available for app '{}' v{}", + app_id, + document_version + ); + return Ok(HarpoonQueryResponse { + session_id: request.session_id(), + result: QueryResult::NoUpdate, + }); + } + + let new_version = update_check.version().ok_or_else(|| { + HarpoonError::InvalidResponse("Missing new version in update check response".to_string()) + })?; + + let update_base_url = update_check.urls().next().ok_or_else(|| { + HarpoonError::InvalidResponse("Missing URL in update check response".to_string()) + })?; + + if update_check.packages().len() != 1 { + return Err(HarpoonError::InvalidResponse( + "Expected exactly one package in update check response".to_string(), + )); + } + + let package = update_check + .packages() + .first() + .expect("validated len above"); + let package_url = update_base_url.join(&package.name).map_err(|err| { + HarpoonError::InvalidResponse(format!("Failed to join URL with package name: {err}")) + })?; + + log::debug!( + "Update available for app '{}' v{} -> v{} ({})", + app_id, + document_version, + new_version, + package_url, + ); + + Ok(HarpoonQueryResponse { + session_id: request.session_id(), + result: QueryResult::NewDocument(OmahaUpdate { + url: package_url, + version: new_version.as_version().clone(), + hash: normalized_sha384_hash(package), + }), + }) +} + +fn normalized_sha384_hash(package: &Package) -> Option { + fn is_sha384(candidate: &str) -> bool { + candidate.len() == 96 && candidate.chars().all(|c| c.is_ascii_hexdigit()) + } + + package + .hash_sha256 + .as_deref() + .filter(|candidate| is_sha384(candidate)) + .map(str::to_owned) + .or_else(|| { + if is_sha384(&package.hash) { + Some(package.hash.clone()) + } else { + None + } + }) +} + +/// Downloads an update package provided by the Omaha server at the given base URL. +#[allow(unused)] +fn download_document( + update_base_url: &Url, + package: &Package, + file_extension: &str, +) -> Result<(String, Url), HarpoonError> { + if !package.name.ends_with(file_extension) { + return Err(HarpoonError::ExpectedYamlDocument(package.name.clone())); + } + + if package.size >= 1024 * 1024 { + log::warn!( + "Reported document size is larger than 1MB ({}). This may NOT be a '{}' text document.", + package.size, + file_extension + ); + } + + let package_url = update_base_url.join(&package.name).map_err(|err| { + HarpoonError::InvalidResponse(format!("Failed to join URL with package name: {err}")) + })?; + + let document = reqwest::blocking::Client::new() + .get(package_url.clone()) + .send() + .map_err(|err| HarpoonError::FetchError(err.to_string()))? + .text() + .map_err(|err| HarpoonError::FetchError(err.to_string()))?; + + log::trace!( + "Validating document size: actual [{}] == expected [{}]", + document.len(), + package.size + ); + if package.size != document.len() as u64 { + return Err(HarpoonError::FetchError(format!( + "Downloaded document size does not match package size: {} != {}", + document.len(), + package.size + ))); + } + + if !package.hash.is_empty() { + let actual = format!("{:x}", Sha256::digest(document.as_bytes())); + let expected = package.hash.to_lowercase(); + log::trace!( + "Validating document hash: actual [{}] == expected [{}]", + actual, + expected + ); + if actual != expected { + return Err(HarpoonError::FetchError(format!( + "Downloaded document hash does not match package hash: {actual} != {expected}" + ))); + } + } + + Ok((document, package_url)) +} + +/// A wrapper to hide away the details of what Omaha events are actually relevant. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EventType { + Install, + Update, +} + +impl From for OmahaEventType { + fn from(event_type: EventType) -> Self { + match event_type { + EventType::Install => OmahaEventType::EventUpdateInstalled, + EventType::Update => OmahaEventType::UpdateComplete, + } + } +} + +fn report_omaha_event( + url: &Url, + app_id: &str, + track: &str, + event: OmahaEventType, + result: EventResult, + machine_id_source: IdSource, +) -> Result<(), HarpoonError> { + omaha::send_event( + url, + &Request::default().with_app( + AppRequest::new_event(app_id, track, machine_id_source)? + .with_event(OmahaEvent::new(event, result)), + ), + )?; + Ok(()) +} + +pub fn report_event( + url: &Url, + app_id: &str, + track: &str, + event: EventType, + result: EventResult, + machine_id_source: IdSource, +) -> Result<(), HarpoonError> { + report_omaha_event(url, app_id, track, event.into(), result, machine_id_source) +} + +#[cfg(test)] +mod tests { + use mockito::Matcher; + + use super::*; + + #[test] + fn test_download_document() { + let mut server = mockito::Server::new(); + + let data = "test document"; + + let document_mock = server + .mock("GET", "/test.yaml") + .with_body(data) + .with_header("content-length", &data.len().to_string()) + .with_header("content-type", "text/plain") + .with_status(200) + .expect(1) + .create(); + + let url = Url::parse(&server.url()).unwrap(); + let package = Package { + name: "test.yaml".to_string(), + size: 13, + hash: format!("{:x}", Sha256::digest(data.as_bytes())), + hash_sha256: None, + required: true, + }; + + let (document, package_url) = download_document(&url, &package, ".yaml").unwrap(); + + document_mock.assert(); + + assert_eq!(document, data); + assert_eq!( + package_url, + Url::parse(&format!("{}/test.yaml", server.url())).unwrap() + ); + } + + #[test] + fn test_query_for_update() { + let mut server = mockito::Server::new(); + let data = "test document"; + + let omaha_mock = server + .mock("POST", "/") + .with_status(200) + .match_body(Matcher::Regex(".* + + + + + + + + + + + + + + + "#}, + server.url(), + Sha256::digest(data.as_bytes()), + data.len() + )) + .expect(1) + .create(); + + let response = query_for_update( + &Url::parse(&server.url()).unwrap(), + "test", + "track", + &Version::new(0, 1, 0), + IdSource::MachineIdHashed, + ) + .unwrap(); + + omaha_mock.assert(); + + assert_eq!( + response, + HarpoonQueryResponse { + session_id: response.session_id, + result: QueryResult::NewDocument(OmahaUpdate { + url: Url::parse(&format!("{}/test.yaml", server.url())).unwrap(), + version: Version::new(1, 0, 0), + hash: None, + }) + } + ); + } + + #[test] + fn test_query_for_update_no_update() { + let mut server = mockito::Server::new(); + + let omaha_mock = server + .mock("POST", "/") + .with_status(200) + .match_body(Matcher::Regex(".* + + + + + + + + "#}) + .expect(1) + .create(); + + let response = query_for_update( + &Url::parse(&server.url()).unwrap(), + "test", + "track", + &Version::new(0, 1, 0), + IdSource::MachineIdHashed, + ) + .unwrap(); + + omaha_mock.assert(); + assert!(matches!(response.result, QueryResult::NoUpdate)); + } +} diff --git a/crates/trident-acl-agent/src/main.rs b/crates/trident-acl-agent/src/main.rs index 75677af298..039a4a763a 100644 --- a/crates/trident-acl-agent/src/main.rs +++ b/crates/trident-acl-agent/src/main.rs @@ -1,549 +1,138 @@ -//! # Harpoon -//! -//! Harpoon is a lightweight Omaha protocol client for documents. It queries a -//! server at a given address for a specific app and track to fetch an updated -//! document. -//! -//! This crate is specifically meant to function as an Omaha client for Trident -//! to fetch updated Host Configuration documents. -//! -//! -//! +use std::path::PathBuf; use clap::Parser; -use futures::StreamExt; -use log::{debug, error, info, trace, warn, LevelFilter}; -use semver::Version; -use sha2::{Digest, Sha256}; -use tonic::{transport::Endpoint, Streaming}; -use trident_proto::v1::{ - servicing_response::Response as ResponseBody, update_service_client::UpdateServiceClient, - FinalizeUpdateRequest, HostConfiguration, LogLevel, RebootHandling, RebootManagement, - ServicingResponse, StageUpdateRequest, StatusCode, UpdateRequest, +use log::{LevelFilter, Log, Metadata, Record}; + +use trident_acl_agent::{ + config::{AgentConfig, GoalSource, DEFAULT_CONFIG_PATH}, + orchestrator::Orchestrator, + run_omaha_only, }; -use url::Url; -use uuid::Uuid; -pub mod error; -pub mod id; -pub mod omaha; +/// Module/target prefixes for the underlying HTTP/gRPC/watch client stack. +/// These crates emit very verbose `log`-facade tracing (connection setup, +/// per-frame HTTP2 detail, watch reconnect churn) that is rarely useful at +/// the same verbosity as the agent's own orchestration logic, so it's +/// filtered independently via `--network-verbosity`. +const NETWORK_LOG_TARGETS: &[&str] = &[ + "hyper", + "h2", + "tower", + "tonic", + "reqwest", + "rustls", + "kube", + "kube_client", + "kube_runtime", +]; + +/// A `log::Log` wrapper that applies a separate level filter to the noisy +/// HTTP/gRPC/watch client crates (see [`NETWORK_LOG_TARGETS`]) while leaving +/// every other target (the agent's own code) at the main `--verbosity` +/// level. +struct FilteredLogger { + inner: L, + verbosity: LevelFilter, + network_verbosity: LevelFilter, +} -use error::HarpoonError; -use omaha::{ - event::{OmahaEvent, OmahaEventType}, - request::{AppRequest, Request}, - response::Package, -}; +impl Log for FilteredLogger { + fn enabled(&self, metadata: &Metadata) -> bool { + let level = if is_network_target(metadata.target()) { + self.network_verbosity + } else { + self.verbosity + }; + metadata.level() <= level + } -pub use id::IdSource; -pub use omaha::event::EventResult; + fn log(&self, record: &Record) { + if self.enabled(record.metadata()) { + self.inner.log(record); + } + } -#[derive(Debug, PartialEq, Eq)] -pub struct HarpoonQueryResponse { - pub session_id: Uuid, - pub result: QueryResult, + fn flush(&self) { + self.inner.flush(); + } } -#[derive(Debug, PartialEq, Eq)] -pub enum QueryResult { - NoUpdate, - NewDocument { url: Url, version: Version }, +fn is_network_target(target: &str) -> bool { + NETWORK_LOG_TARGETS + .iter() + .any(|prefix| target == *prefix || target.starts_with(&format!("{prefix}::"))) } +/// Harpoon can either run its original one-shot Omaha flow or the new +/// label-driven orchestrator. Activation of label mode is intentionally gated by +/// config file only: shipping defaults stay on `omaha-only`, while a VM +/// extension or AgentBaker-dropped config is expected to opt a node into the +/// AKS label protocol. #[derive(Parser, Debug)] #[command(version, about, long_about = None)] struct Args { /// Logging verbosity [OFF, ERROR, WARN, INFO, DEBUG, TRACE] #[arg(global = true, short, long, default_value_t = LevelFilter::Debug)] - pub verbosity: LevelFilter, - - /// The URL of the Nebraska server to use. Likely should end in `/v1/update` + verbosity: LevelFilter, + + /// Logging verbosity for the underlying HTTP/gRPC/watch client stack + /// (hyper, h2, tower, tonic, reqwest, rustls, kube). Kept separate from + /// `--verbosity` because it can be extremely noisy (per-frame HTTP2 + /// detail, watch reconnect churn) [OFF, ERROR, WARN, INFO, DEBUG, TRACE]. + #[arg(global = true, long, default_value_t = LevelFilter::Warn)] + network_verbosity: LevelFilter, + + /// Optional path to /etc/trident/trident-acl-agent.conf. + #[arg(long)] + config: Option, + + /// Optional Omaha/Nebraska URL override. When omitted, Harpoon uses the + /// endpoint from config.toml. When both are missing, startup fails with a + /// clear error. #[arg()] - pub url: Url, + url: Option, } -fn main() { +#[tokio::main] +async fn main() -> Result<(), anyhow::Error> { let args = Args::parse(); + let max_level = args.verbosity.max(args.network_verbosity); if let Some(Ok(journal_logger)) = systemd_journal_logger::connected_to_journal().then(systemd_journal_logger::JournalLog::new) { - journal_logger - .install() - .expect("Failed to install systemd journal logger"); - log::set_max_level(args.verbosity); + log::set_boxed_logger(Box::new(FilteredLogger { + inner: journal_logger, + verbosity: args.verbosity, + network_verbosity: args.network_verbosity, + })) + .expect("Failed to install systemd journal logger"); + log::set_max_level(max_level); } else { - env_logger::builder() + let inner = env_logger::builder() .format_timestamp(None) - .filter_level(args.verbosity) - .init(); - } - - let r = query_and_fetch_yaml_document( - &args.url, - "b0ec8f0d-1c13-4bf4-9efd-ea54464a7098", - "west-us", - &Version::new(0, 0, 0), - IdSource::MachineIdHashed, - ) - .expect("Failed to query Omaha server"); - - match r.result { - QueryResult::NoUpdate => { - debug!("No update available"); - } - QueryResult::NewDocument { url, version } => { - debug!("Updating to version {version}"); - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("Failed to create tokio runtime"); - - rt.block_on(trigger(&url, None)) - .expect("Failed to run update"); - } - } -} - -async fn trigger(url: &Url, hash: Option) -> Result<(), anyhow::Error> { - // For now, we will just log the trigger. In the future, this function can be - // used to trigger an update check on the server side, for example by sending - // a specific event or making a specific API call to the server. - debug!("Triggering update with URL: {url} and hash: {hash:?}"); - - let channel = Endpoint::new(trident_proto::TRIDENT_DEFAULT_SOCKET_URI)? - .connect() - .await?; - let mut client = UpdateServiceClient::new(channel); - - let response = client - .update(tonic::Request::new(UpdateRequest { - stage: Some(StageUpdateRequest { - config: Some(HostConfiguration { - // TODO: Handle escaping of URL and hash. - config: match hash { - Some(hash) => format!("image:\n url: {url}\n sha384: {hash}"), - None => { - format!("image:\n url: {url}\n sha384: ignored") - } - }, - }), - }), - finalize: Some(FinalizeUpdateRequest { - reboot: Some(RebootManagement { - handling: RebootHandling::CallerHandlesReboot.into(), - }), - }), + .filter_level(max_level) + .build(); + log::set_boxed_logger(Box::new(FilteredLogger { + inner, + verbosity: args.verbosity, + network_verbosity: args.network_verbosity, })) - .await?; - - handle_servicing_stream(response.into_inner()).await -} - -async fn handle_servicing_stream( - mut stream: Streaming, -) -> Result<(), anyhow::Error> { - // Iterate through the stream until we get a Completed message - loop { - match stream.next().await { - Some(Ok(response)) => match response.response { - Some(ResponseBody::Started(_)) => { - info!("[Trident] Install started"); - // Continue to next message - } - Some(ResponseBody::Log(log)) => { - let msg = format!("[Trident] {}", log.message); - match log.level() { - LogLevel::Unspecified | LogLevel::Trace => trace!("{msg}"), - LogLevel::Debug => debug!("{msg}"), - LogLevel::Info => info!("{msg}"), - LogLevel::Warn => warn!("{msg}"), - LogLevel::Error => error!("{msg}"), - } - } - Some(ResponseBody::Completed(final_status)) => { - if final_status.status() == StatusCode::Success { - info!( - "Trident install succeeded: status={:?}", - final_status.status() - ); - break Ok(()); - } else { - error!("Trident install failed: status={:?}", final_status.status()); - match final_status.error { - Some(err) => { - error!("Trident reported error: {}", err.message); - break Err(anyhow::anyhow!(err.message)); - } - None => { - break Err(anyhow::anyhow!("Trident install failed")); - } - } - } - } - None => { - // Empty response, continue - continue; - } - }, - Some(Err(e)) => { - break Err(anyhow::anyhow!("Error reading from Trident stream: {e}")); - } - None => { - break Err(anyhow::anyhow!( - "Trident install stream ended without control message" - )); - } - } - } -} - -/// Query the Omaha server at the given URL for the given app and track to fetch -/// an updated YAML document. -/// -/// Returns the session ID and the result of the query. If an update is -/// available, the new version and the updated document are returned. -/// -/// This function should ONLY be used for querying YAML documents (i.e YAML text -/// files) because the whole file will be downloaded, and the function will only -/// look at the first package returned by the omaha server to fetch the -/// document. The function expects the document to be a single file with `.yaml` -/// extension. -pub fn query_and_fetch_yaml_document( - url: &Url, - app_id: &str, - track: &str, - document_version: &Version, - machine_id_source: IdSource, -) -> Result { - let request = Request::default().with_app( - AppRequest::new(app_id, document_version, track, machine_id_source)?.with_update_check(), - ); - - let response = omaha::send(url, &request)?; - - debug!( - "Received response from Omaha server at '{url}' for app '{app_id}' on track '{track}': {response:#?}", - url = url, - app_id = app_id, - track = track, - response = response - ); - if response.apps().len() != 1 { - return Err(HarpoonError::InvalidResponse( - "Expected exactly one app in response".to_string(), - )); - } - - let app = response.apps().first().unwrap(); - - if app.app_id() != app_id { - return Err(HarpoonError::InvalidResponse( - "Unexpected app ID in response".to_string(), - )); - } - - if app.status().is_error() { - return Err(HarpoonError::QueryError(format!( - "Received a non-OK app status: {0}", - app.status() - ))); - } - - let update_check = app.update_check().ok_or_else(|| { - HarpoonError::InvalidResponse("Missing update check in response".to_string()) - })?; - debug!("Received update check response: {update_check:#?}"); - - if update_check.status().is_error() { - return Err(HarpoonError::QueryError(format!( - "Received an error status in update check: {0}", - update_check.status() - ))); - } - - if update_check.status().is_no_update() { - // Successfully checked that there is no update available! - debug!( - "No update available for app '{}' v{}", - app_id, document_version - ); - return Ok(HarpoonQueryResponse { - session_id: request.session_id(), - result: QueryResult::NoUpdate, - }); - } - - // If we got here, an update is available! - let new_version = update_check.version().ok_or_else(|| { - HarpoonError::InvalidResponse("Missing new version in update check response".to_string()) - })?; - - let update_base_url = update_check.urls().next().ok_or_else(|| { - HarpoonError::InvalidResponse("Missing URL in update check response".to_string()) - })?; - - if update_check.packages().len() != 1 { - return Err(HarpoonError::InvalidResponse( - "Expected exactly one package in update check response".to_string(), - )); + .expect("Failed to install env logger"); + log::set_max_level(max_level); } - let package_url = update_base_url - .join(&update_check.packages().first().unwrap().name) - .map_err(|err| { - HarpoonError::InvalidResponse(format!("Failed to join URL with package name: {err}")) - })?; - - debug!( - "Downloaded update for app '{}' v{} to v{}", - app_id, document_version, new_version - ); - debug!("Document URL: {package_url}"); - - Ok(HarpoonQueryResponse { - session_id: request.session_id(), - result: QueryResult::NewDocument { - url: package_url, - version: new_version.as_version().clone(), - }, - }) -} - -/// Downloads an update package provided by the Omaha server at the given base -/// URL. -/// -/// On success, returns the document as a string and the URL from which it was -/// downloaded. -/// -/// The function takes care of validating the size and hash of the downloaded -/// document. -#[allow(unused)] -fn download_document( - update_base_url: &Url, - package: &Package, - file_extension: &str, -) -> Result<(String, Url), HarpoonError> { - if !package.name.ends_with(file_extension) { - return Err(HarpoonError::ExpectedYamlDocument(package.name.clone())); - } - - // If the package size is larger than 1MB, log a warning. This may mean that - // we are not downloading the correct document. - if package.size >= 1024 * 1024 { - warn!( - "Reported document size is larger than 1MB ({}). This may NOT be a '{}' text document.", - package.size, file_extension - ); - } - - let package_url = update_base_url.join(&package.name).map_err(|err| { - HarpoonError::InvalidResponse(format!("Failed to join URL with package name: {err}")) - })?; - - let document = reqwest::blocking::Client::new() - .get(package_url.clone()) - .send() - .map_err(|err| HarpoonError::FetchError(err.to_string()))? - .text() - .map_err(|err| HarpoonError::FetchError(err.to_string()))?; - - // Check that the downloaded document size matches the package size. - trace!( - "Validating document size: actual [{}] == expected [{}]", - document.len(), - package.size - ); - if package.size != document.len() as u64 { - return Err(HarpoonError::FetchError(format!( - "Downloaded document size does not match package size: {} != {}", - document.len(), - package.size - ))); - } - - // If we have a hash, validate it. - if !package.hash.is_empty() { - let actual = format!("{:x}", Sha256::digest(document.as_bytes())); - let expected = package.hash.to_lowercase(); - trace!( - "Validating document hash: actual [{}] == expected [{}]", - actual, - expected - ); - if actual != expected { - return Err(HarpoonError::FetchError(format!( - "Downloaded document hash does not match package hash: {actual} != {expected}" - ))); - } - } - - Ok((document, package_url)) -} - -/// A wrapper to hide away the details of what Omaha events are actually -/// relevant. Trident only needs to know about Install and Update events. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum EventType { - Install, - Update, -} - -impl From for OmahaEventType { - fn from(event_type: EventType) -> Self { - match event_type { - EventType::Install => OmahaEventType::EventUpdateInstalled, - EventType::Update => OmahaEventType::UpdateComplete, - } - } -} - -/// Reports an Omaha event to the server at the given URL for the given app and -/// track. -fn report_omaha_event( - url: &Url, - app_id: &str, - track: &str, - event: OmahaEventType, - result: EventResult, - machine_id_source: IdSource, -) -> Result<(), HarpoonError> { - omaha::send_event( - url, - &Request::default().with_app( - AppRequest::new_event(app_id, track, machine_id_source)? - .with_event(OmahaEvent::new(event, result)), - ), - )?; - Ok(()) -} - -/// Reports a generic event to the Omaha server at the given URL for the given -/// app and track. -pub fn report_event( - url: &Url, - app_id: &str, - track: &str, - event: EventType, - result: EventResult, - machine_id_source: IdSource, -) -> Result<(), HarpoonError> { - report_omaha_event(url, app_id, track, event.into(), result, machine_id_source) -} - -#[cfg(test)] -mod tests { - use mockito::Matcher; - - use super::*; - - #[test] - fn test_download_document() { - let mut server = mockito::Server::new(); - - let data = "test document"; - - let document_mock = server - .mock("GET", "/test.yaml") - .with_body(data) - .with_header("content-length", &data.len().to_string()) - .with_header("content-type", "text/plain") - .with_status(200) - .expect(1) - .create(); - - let url = Url::parse(&server.url()).unwrap(); - let package = Package { - name: "test.yaml".to_string(), - size: 13, - hash: format!("{:x}", Sha256::digest(data.as_bytes())), - hash_sha256: None, - required: true, - }; - - let (document, package_url) = download_document(&url, &package, ".yaml").unwrap(); - - document_mock.assert(); - - assert_eq!(document, data); - - assert_eq!( - package_url, - Url::parse(&format!("{}/test.yaml", server.url())).unwrap() - ); - } - - #[test] - fn test_query_and_fetch_document() { - let mut server = mockito::Server::new(); - - let data = "test document"; - - let omaha_mock = server - .mock("POST", "/") - .with_status(200) - .match_body(Matcher::Regex(".* - - - - - - - - - - - - - - - "#}, - server.url(), - Sha256::digest(data.as_bytes()), - data.len() - )) - .expect(1) - .create(); - - // let omaha_event_mock = server - // .mock("POST", "/") - // .with_status(200) - // .match_body(Matcher::Regex(".* - // - // - // - // - // - // "#}) - // .expect(1) - // .create(); - - let response = query_and_fetch_yaml_document( - &Url::parse(&server.url()).unwrap(), - "test", - "track", - &Version::new(0, 1, 0), - IdSource::MachineIdHashed, - ) - .unwrap(); + let config_path = args + .config + .clone() + .unwrap_or_else(|| PathBuf::from(DEFAULT_CONFIG_PATH)); + let explicit_config = args.config.is_some(); - omaha_mock.assert(); - // omaha_event_mock.assert(); + let config = AgentConfig::load(&config_path, explicit_config)?.unwrap_or_default(); + let config = config.with_cli_endpoint(args.url.clone()); - assert_eq!( - response, - HarpoonQueryResponse { - session_id: response.session_id, - result: QueryResult::NewDocument { - url: Url::parse(&format!("{}/test.yaml", server.url())).unwrap(), - version: Version::new(1, 0, 0), - } - } - ); + match config.orchestration.goal_source { + GoalSource::OmahaOnly => run_omaha_only(&config).await, + GoalSource::Labels => Orchestrator::from_config(config).await?.run().await, } } diff --git a/crates/trident-acl-agent/src/mock_tridentd.rs b/crates/trident-acl-agent/src/mock_tridentd.rs new file mode 100644 index 0000000000..d0e60670f0 --- /dev/null +++ b/crates/trident-acl-agent/src/mock_tridentd.rs @@ -0,0 +1,253 @@ +//! In-process mock tridentd used only by unit tests. +//! +//! Implements the real generated `UpdateService`/`CommitService` server +//! traits (gated behind trident-proto's `server` feature, enabled only in +//! `[dev-dependencies]` - see trident-acl-agent/Cargo.toml) so tests can +//! exercise the *real* `TridentClient` request/response/error-mapping code +//! against canned stage/finalize/commit outcomes, without a real tridentd +//! process or unix socket. +//! +//! Tests wire a `TridentClient` to this mock server over an in-memory +//! `tokio::io::duplex` transport via `Endpoint::connect_with_connector` + +//! `TridentClient::from_channel` - see `connect_mock_client` below. + +use std::sync::{Arc, Mutex}; + +use hyper_util::rt::TokioIo; +use tokio_stream::wrappers::ReceiverStream; +use tonic::{transport::Endpoint, Request, Response, Status}; +use trident_proto::v1::{ + commit_service_server::{CommitService, CommitServiceServer}, + rollback_service_server::{RollbackService, RollbackServiceServer}, + servicing_response::Response as ResponseBody, + update_service_server::{UpdateService, UpdateServiceServer}, + CommitRequest, Completed, FinalizeUpdateRequest, RebootStatus, RollbackFinalizeRequest, + RollbackRequest, RollbackStageRequest, ServicingKind, ServicingResponse, StageUpdateRequest, + StatusCode as ProtoStatusCode, TridentError, UpdateRequest, +}; + +use crate::trident::TridentClient; + +/// Canned outcome a `MockTridentd` should return for a given RPC call. +#[derive(Clone, Debug)] +pub enum Outcome { + /// Respond with a successful `Completed` message. `servicing_kind` + /// mirrors what a real tridentd populates on every servicing RPC + /// (`ServicingKind::NoneRequired` for a no-op, the real kind + /// otherwise) - tests that care about the no-op-detection path (see + /// orchestrator.rs's `handle_rollback`) set this explicitly; other + /// tests that don't inspect it can pass `None`. + Success { + reboot_status: RebootStatus, + servicing_kind: Option, + }, + /// Respond with a failed `Completed` message carrying the given error + /// subkind (e.g. "ab-update-reboot-check"). + Failure { + subkind: &'static str, + message: &'static str, + }, +} + +impl Outcome { + fn into_servicing_response(self) -> ServicingResponse { + let completed = match self { + Outcome::Success { + reboot_status, + servicing_kind, + } => Completed { + status: ProtoStatusCode::Success as i32, + error: None, + reboot_status: reboot_status as i32, + image_hash: None, + servicing_kind: servicing_kind.map(|k| k as i32), + }, + Outcome::Failure { subkind, message } => Completed { + status: ProtoStatusCode::Failure as i32, + error: Some(TridentError { + kind: 0, + subkind: subkind.to_string(), + message: message.to_string(), + error_message: message.to_string(), + location: None, + }), + reboot_status: RebootStatus::Unspecified as i32, + image_hash: None, + servicing_kind: None, + }, + }; + ServicingResponse { + timestamp: None, + response: Some(ResponseBody::Completed(completed)), + } + } +} + +/// Configurable canned responses for the three RPCs `TridentClient` calls. +/// Each field defaults to `None`; a test sets only the outcome(s) it cares +/// about, and the mock server panics if a call arrives with no outcome +/// configured (surfacing test-setup bugs immediately rather than silently +/// hanging or defaulting). +#[derive(Default)] +pub struct MockTridentdConfig { + pub stage: Option, + pub finalize: Option, + pub commit: Option, + pub rollback_stage: Option, + pub rollback_finalize: Option, +} + +#[derive(Clone)] +struct MockTridentd { + config: Arc>, +} + +async fn respond_with( + outcome: Outcome, +) -> Result>>, Status> { + let (tx, rx) = tokio::sync::mpsc::channel(4); + tx.send(Ok(outcome.into_servicing_response())) + .await + .expect("mock tridentd channel send should not fail"); + Ok(Response::new(ReceiverStream::new(rx))) +} + +#[tonic::async_trait] +impl UpdateService for MockTridentd { + type UpdateStream = ReceiverStream>; + type UpdateStageStream = ReceiverStream>; + type UpdateFinalizeStream = ReceiverStream>; + + async fn update( + &self, + _request: Request, + ) -> Result, Status> { + Err(Status::unimplemented( + "update() is not used by trident-acl-agent", + )) + } + + async fn update_stage( + &self, + _request: Request, + ) -> Result, Status> { + let outcome = + self.config.lock().unwrap().stage.clone().expect( + "test must configure MockTridentdConfig::stage before calling update_stage", + ); + respond_with(outcome).await + } + + async fn update_finalize( + &self, + _request: Request, + ) -> Result, Status> { + let outcome = self.config.lock().unwrap().finalize.clone().expect( + "test must configure MockTridentdConfig::finalize before calling update_finalize", + ); + respond_with(outcome).await + } +} + +#[tonic::async_trait] +impl CommitService for MockTridentd { + type CommitStream = ReceiverStream>; + + async fn commit( + &self, + _request: Request, + ) -> Result, Status> { + let outcome = self + .config + .lock() + .unwrap() + .commit + .clone() + .expect("test must configure MockTridentdConfig::commit before calling commit"); + respond_with(outcome).await + } +} + +#[tonic::async_trait] +impl RollbackService for MockTridentd { + type RollbackStream = ReceiverStream>; + type RollbackStageStream = ReceiverStream>; + type RollbackFinalizeStream = ReceiverStream>; + + // check_rollback is no longer part of the stable v1 RollbackService + // trait (demoted back to trident.v1preview - trident-acl-agent detects + // a no-op rollback via RollbackStage's servicing_kind now instead, see + // orchestrator.rs's handle_rollback), so this mock no longer needs to + // implement it. + + async fn rollback( + &self, + _request: Request, + ) -> Result, Status> { + Err(Status::unimplemented( + "rollback() is not used by trident-acl-agent", + )) + } + + async fn rollback_stage( + &self, + _request: Request, + ) -> Result, Status> { + let outcome = self.config.lock().unwrap().rollback_stage.clone().expect( + "test must configure MockTridentdConfig::rollback_stage before calling rollback_stage", + ); + respond_with(outcome).await + } + + async fn rollback_finalize( + &self, + _request: Request, + ) -> Result, Status> { + let outcome = self + .config + .lock() + .unwrap() + .rollback_finalize + .clone() + .expect( + "test must configure MockTridentdConfig::rollback_finalize before calling rollback_finalize", + ); + respond_with(outcome).await + } +} + +/// Starts an in-process mock tridentd wired to `client` over an in-memory +/// duplex transport (no real socket/subprocess), and returns a +/// `TridentClient` connected to it. `config` is shared (`Arc>`) +/// so the caller can reconfigure outcomes between calls if a test needs to +/// simulate stage-then-finalize-then-commit in one session. +pub async fn connect_mock_client(config: Arc>) -> TridentClient { + let (client_io, server_io) = tokio::io::duplex(64 * 1024); + + let mock = MockTridentd { config }; + tokio::spawn(async move { + tonic::transport::Server::builder() + .add_service(UpdateServiceServer::new(mock.clone())) + .add_service(CommitServiceServer::new(mock.clone())) + .add_service(RollbackServiceServer::new(mock)) + .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server_io))) + .await + .expect("mock tridentd server should not fail"); + }); + + let mut client_io = Some(client_io); + let channel = Endpoint::try_from("http://[::]:50051") + .expect("static endpoint URI should always parse") + .connect_with_connector(tower::service_fn(move |_: tonic::transport::Uri| { + let client_io = client_io.take(); + async move { + client_io.map(TokioIo::new).ok_or_else(|| { + std::io::Error::other("mock client connector called more than once") + }) + } + })) + .await + .expect("in-memory duplex connection should succeed"); + + TridentClient::from_channel(channel) +} diff --git a/crates/trident-acl-agent/src/orchestrator.rs b/crates/trident-acl-agent/src/orchestrator.rs new file mode 100644 index 0000000000..8ac9020520 --- /dev/null +++ b/crates/trident-acl-agent/src/orchestrator.rs @@ -0,0 +1,1712 @@ +//! The Trident ACL agent's reconcile loop: watches the Node's request +//! annotation, drives Trident (stage/finalize/rollback/commit) over gRPC, +//! and writes the status annotation back, including post-reboot. +//! +//! Implements the node-side control flow from `docs/update-trigger-design.md`: +//! https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md +//! (sections 2.1 "Trigger mechanism", 2.3 "Stage/finalize/rollback split +//! and post-reboot commit", and 2.5 "Rollback"). See that document for the +//! full state-machine rationale; keep it in sync with this file if the +//! design changes. + +use std::{collections::BTreeMap, process::Command}; + +use chrono::{DateTime, Utc}; +use futures::StreamExt; +use k8s_openapi::api::core::v1::Node; +use semver::Version; +use trident_proto::v1::{RebootStatus, ServicingKind}; +use uuid::Uuid; + +use crate::{ + annotations::{ + commit_operation_id, current_active_version, Operation, RequestedOperation, StatusCode, + UpdateRequest, UpdateStatus, SCHEMA_VERSION, UPDATE_REQUEST_ANNOTATION, + UPDATE_STATUS_ANNOTATION, + }, + config::AgentConfig, + k8s::NodeClient, + query_for_update, + state::{PendingCommit, StateStore}, + trident::{CompletedResponse, TridentClient, TridentClientError}, + IdSource, QueryResult, DEFAULT_NEBRASKA_TRACK, +}; + +const FINAL_STATUS_PATCH_RETRIES: usize = 3; +const FINAL_STATUS_PATCH_BACKOFF: std::time::Duration = std::time::Duration::from_secs(2); + +#[derive(Clone, Default)] +pub struct SystemRebooter; + +pub trait RebootHandle: Clone + Send + Sync + 'static { + fn reboot(&self) -> Result<(), anyhow::Error>; +} + +impl RebootHandle for SystemRebooter { + fn reboot(&self) -> Result<(), anyhow::Error> { + for candidate in [ + ("reboot", Vec::<&str>::new()), + ("systemctl", vec!["reboot"]), + ] { + match Command::new(candidate.0) + .args(candidate.1.iter().copied()) + .status() + { + Ok(status) if status.success() => return Ok(()), + Ok(status) => log::warn!("{} exited with {}", candidate.0, status), + Err(err) => log::warn!("failed to invoke {}: {err}", candidate.0), + } + } + Err(anyhow::anyhow!( + "failed to issue reboot via reboot or systemctl reboot" + )) + } +} + +pub struct Orchestrator { + config: AgentConfig, + k8s: NodeClient, + rebooter: R, + state: StateStore, +} + +impl Orchestrator { + pub async fn from_config(config: AgentConfig) -> Result { + let k8s = NodeClient::new(&config.kubernetes).await?; + Ok(Self { + state: StateStore::new(config.orchestration.state_path.clone()), + config, + k8s, + rebooter: SystemRebooter, + }) + } +} + +impl Orchestrator +where + R: RebootHandle, +{ + pub async fn run(&self) -> Result<(), anyhow::Error> { + self.recover_from_trident_state().await?; + let mut stream = self + .k8s + .watch_node(self.config.kubernetes.node_name.clone()); + while let Some(node) = stream.next().await { + match self.reconcile_node(&node?).await? { + LoopControl::Continue => {} + LoopControl::ExitForReboot => return Ok(()), + } + } + Ok(()) + } + + async fn recover_from_trident_state(&self) -> Result<(), anyhow::Error> { + let node = self.k8s.get_node(&self.config.kubernetes.node_name).await?; + let snapshot = Snapshot::from_node(&node); + let persisted = self.state.load()?; + + if let Some(pending) = persisted.pending_commit.clone() { + return self.resume_pending_commit(pending).await; + } + + if let Some(request) = snapshot.request.clone() { + // See reconcile_node() for why we must also check the + // commit-suffixed id: a finalize/rollback's post-reboot outcome + // is recorded under `.commit`, not the original + // request's plain operationId. + let cached = persisted + .completed + .get(&commit_operation_id(&request.operation_id)) + .or_else(|| persisted.completed.get(&request.operation_id)) + .cloned(); + if let Some(status) = cached { + self.publish_status(&status).await?; + return Ok(()); + } + } + + // No pendingCommit survived (or none was ever written) and there's no + // cached terminal status for the current request's operationId. If + // the outstanding request is a finalize/rollback, this is exactly the + // "state.json did not survive the reboot" degraded path from + // accepted-design.md §2.3: the *status* annotation from before the + // reboot (e.g. finalize's InProgress/Success) is still sitting in the + // API server untouched - annotations live in etcd, not on the node - + // so we cannot use "is there a status annotation at all" to detect + // this case. We must always attempt reconstruction here rather than + // falling through to the normal watch loop, which would otherwise + // re-run handle_finalize from scratch against an empty local + // `completed` map and incorrectly report NotStaged. Stage requests + // don't reboot, so a crash there is safely retried by the normal + // watch loop instead. + if let Some(request) = snapshot.request { + if matches!( + request.operation, + RequestedOperation::Finalize | RequestedOperation::Rollback + ) { + let status = self.reconstruct_without_state(&request, None, None).await; + self.record_and_publish(status).await?; + } + } + Ok(()) + } + + async fn reconcile_node(&self, node: &Node) -> Result { + let snapshot = Snapshot::from_node(node); + log::debug!( + "received node update: request={:?} status={:?}", + snapshot.request, + snapshot.status + ); + let persisted = self.state.load()?; + + if let Some(invalid) = snapshot.invalid_request.clone() { + // Dedupe the same way the completed-status cache above does: + // only publish once per operationId, so a persistently invalid + // annotation doesn't re-PATCH on every reconcile. + if !persisted.completed.contains_key(&invalid.operation_id) { + let now = Utc::now(); + let status = UpdateStatus { + schema_version: SCHEMA_VERSION.to_string(), + node_update_id: invalid.node_update_id, + operation_id: invalid.operation_id, + operation: invalid.operation, + code: StatusCode::InvalidRequest, + message: invalid.reason, + from_version: None, + to_version: None, + started_utc: now, + finished_utc: Some(now), + }; + self.record_and_publish(status).await?; + } + return Ok(LoopControl::Continue); + } + + let Some(request) = snapshot.request.clone() else { + return Ok(LoopControl::Continue); + }; + + // A finalize/rollback request yields two terminal statuses under two + // operationIds on opposite sides of the reboot: the plain id (the + // pre-reboot `finalize`/`rollback` terminal) and the `.commit`-suffixed + // id (the post-reboot `commit` terminal written by + // resume_pending_commit()/reconstruct_without_state() - see + // accepted-design.md §2.3). Once the commit half has landed, the + // *request* annotation is still the original finalize/rollback + // request (annotations don't get cleared), so this reconcile can + // fire again for the same request after the commit already + // completed. Checking only the plain operationId here missed the + // commit-suffixed entry and caused this reconcile to treat the + // request as unfinished, re-publishing the stale pre-reboot + // `finalize` status and clobbering the correct post-reboot `commit` + // status the caller (AKS-RP) is actually waiting on. Prefer the + // commit-suffixed entry when present since it reflects the more + // recent, authoritative outcome. + let cached = persisted + .completed + .get(&commit_operation_id(&request.operation_id)) + .or_else(|| persisted.completed.get(&request.operation_id)) + .cloned(); + if let Some(status) = cached { + // Only (re-)publish if the status annotation isn't already + // up to date. Publishing unconditionally here is dangerous: + // publish_status() PATCHes the Node, which is itself an + // update the watch stream observes, which re-triggers + // reconcile_node() for the very same (already-completed) + // request, causing an infinite self-sustaining PATCH loop + // (observed as thousands of PATCH/watch cycles per second + // with no forward progress). Comparing against the annotation + // already on the node breaks that cycle while still repairing + // a stale/missing annotation exactly once. + if snapshot.status.as_ref() != Some(&status) { + self.publish_status(&status).await?; + } + return Ok(LoopControl::Continue); + } + + if let Some(pending) = persisted.pending_commit.as_ref() { + // Reject on operationId, not nodeUpdateId: the actual conflict + // this guard exists to prevent is "a second finalize/rollback + // starts while one is still waiting for its post-reboot + // commit" (accepted-design.md's in-flight conflict rule). + // Keying on nodeUpdateId alone let a retried/re-issued request + // that reused the same nodeUpdateId but a new operationId slip + // through this guard entirely and re-enter handle_finalize/ + // handle_rollback concurrently with the still-outstanding + // original operation. + if request.operation_id != pending.request.operation_id { + let started = Utc::now(); + let status = UpdateStatus::new( + &request, + request.operation.into(), + request.operation_id.clone(), + StatusCode::InvalidRequest, + format!( + "another finalize/rollback (operationId {}) is waiting for post-reboot commit", + pending.request.operation_id + ), + pending.from_version.clone(), + pending.to_version.clone(), + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(LoopControl::Continue); + } + } + + match request.operation { + RequestedOperation::Stage => { + self.handle_stage(request).await?; + Ok(LoopControl::Continue) + } + RequestedOperation::Finalize => self.handle_finalize(request).await, + RequestedOperation::Rollback => self.handle_rollback(request).await, + } + } + + async fn handle_stage(&self, request: UpdateRequest) -> Result<(), anyhow::Error> { + let started = Utc::now(); + let from_version = Some(current_active_version()); + let to_version = request.target_version.clone(); + if from_version == to_version { + let status = UpdateStatus::new( + &request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::AlreadyAtTarget, + "node already running requested target version", + from_version, + to_version, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(()); + } + + self.publish_status(&UpdateStatus::new( + &request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::InProgress, + "staging update", + from_version.clone(), + to_version.clone(), + started, + None, + )) + .await?; + let endpoint = self.config.nebraska.endpoint.clone().ok_or_else(|| { + anyhow::anyhow!("annotation mode requires [nebraska].endpoint or CLI override") + })?; + let response = query_for_update( + &endpoint, + &self.config.nebraska.app_id, + DEFAULT_NEBRASKA_TRACK, + &Version::new(0, 0, 0), + IdSource::MachineIdHashed, + )?; + let offered = match response.result { + QueryResult::NoUpdate => { + let status = UpdateStatus::new( + &request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::OperationFailed, + "Nebraska currently offers no update for the requested target", + from_version, + to_version, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(()); + } + QueryResult::NewDocument(update) => update, + }; + if request.target_version.as_deref() != Some(offered.version.to_string().as_str()) { + let status = UpdateStatus::new( + &request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::InvalidRequest, + format!( + "requested target version {:?} but Nebraska offers {}", + request.target_version, offered.version + ), + from_version, + to_version, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(()); + } + + let mut client = TridentClient::connect(&self.config.trident.socket).await?; + let result = client + .update_stage( + &offered.url, + offered.hash.as_deref(), + self.config.orchestration.stage_timeout, + ) + .await; + let status = stage_result_to_status(&request, from_version, to_version, started, result); + self.record_and_publish(status).await + } + + async fn handle_finalize(&self, request: UpdateRequest) -> Result { + let started = Utc::now(); + let from_version = Some(current_active_version()); + let to_version = request.target_version.clone(); + if from_version == to_version { + let status = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::AlreadyAtTarget, + "node already running requested target version", + from_version, + to_version, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(LoopControl::Continue); + } + + let completed = self.state.load()?.completed; + let staged = completed.values().find(|s| { + s.node_update_id == request.node_update_id + && s.operation == Operation::Stage + && matches!(s.code, StatusCode::Success | StatusCode::AlreadyAtTarget) + }); + let staged = match staged { + None => { + let status = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::NotStaged, + "finalize requested without prior successful stage for nodeUpdateId", + from_version, + to_version, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(LoopControl::Continue); + } + Some(staged) => staged, + }; + // finalize must apply to whatever was actually staged: tridentd's + // update_finalize() carries no target version of its own, it just + // finalizes whatever is currently staged on disk. Without this + // check, a finalize whose targetVersion differs from the recorded + // stage's targetVersion would silently finalize the *staged* + // version while the status annotation reported the *requested* + // (different) toVersion - a silent version-skew in the status + // channel that AKS-RP has no way to detect. + if staged.to_version != to_version { + let status = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::InvalidRequest, + format!( + "finalize targetVersion {:?} does not match the version staged for this nodeUpdateId ({:?})", + to_version, staged.to_version + ), + from_version, + to_version, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(LoopControl::Continue); + } + + self.publish_status(&UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::InProgress, + "finalizing update", + from_version.clone(), + to_version.clone(), + started, + None, + )) + .await?; + self.state.set_pending_commit(PendingCommit { + request: request.clone(), + operation_id: request.operation_id.clone(), + operation: Operation::Finalize, + from_version: from_version.clone(), + to_version: to_version.clone(), + started_utc: started, + })?; + let mut client = TridentClient::connect(&self.config.trident.socket).await?; + match client + .update_finalize(self.config.orchestration.finalize_timeout) + .await + { + Ok(_) => { + let terminal = finalize_success_status( + &request, + from_version.clone(), + to_version.clone(), + started, + ); + // Record this terminal status under the *finalize* operationId + // (not just the eventual ".commit" one written after + // commit()) before rebooting. Without this, if state.json + // doesn't survive the reboot, the still-present finalize + // request annotation gets reconciled again post-reboot with + // an empty local `completed` map for "finalize-op" and + // re-runs handle_finalize from scratch - incorrectly + // reporting NotStaged even though finalize already + // succeeded and a commit reconstruction already ran. + if let Err(err) = self.state.remember_completed(terminal.clone()) { + log::warn!("failed to record finalize completion in state.json: {err}"); + } + self.best_effort_publish_terminal(&terminal).await; + match self.rebooter.reboot() { + Ok(()) => Ok(LoopControl::ExitForReboot), + Err(err) => { + self.state.clear_pending_commit()?; + let status = UpdateStatus::new( + &request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::AgentInternalError, + format!("finalize succeeded but reboot failed: {err}"), + from_version, + to_version, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + Ok(LoopControl::Continue) + } + } + } + Err(err) => { + self.state.clear_pending_commit()?; + let status = + finalize_failure_status(&request, from_version, to_version, started, &err); + self.record_and_publish(status).await?; + Ok(LoopControl::Continue) + } + } + } + + async fn handle_rollback(&self, request: UpdateRequest) -> Result { + let started = Utc::now(); + let from_version = Some(current_active_version()); + + let mut client = TridentClient::connect(&self.config.trident.socket).await?; + + self.publish_status(&UpdateStatus::new( + &request, + Operation::Rollback, + request.operation_id.clone(), + StatusCode::InProgress, + "staging rollback", + from_version.clone(), + None, + started, + None, + )) + .await?; + + let stage_response = match client + .rollback_stage(self.config.orchestration.stage_timeout) + .await + { + Ok(response) => response, + Err(err) => { + let status = rollback_stage_failure_status(&request, from_version, started, &err); + self.record_and_publish(status).await?; + return Ok(LoopControl::Continue); + } + }; + + // tridentd's RollbackStage reports a no-op ("nothing to roll back") + // the same way update/install do: Ok/Success with servicing_kind == + // NoneRequired, not an error - see servicing.proto's + // ServicingResponse.servicing_kind and execute_rollback() in + // engine/manual_rollback/mod.rs. Detect that here and stop before + // finalize/reboot - otherwise a rollback request against a node + // with an empty rollback chain (or in a non-rollbackable state) + // would be reported as false Success to AKS-RP and would trigger + // an unnecessary reboot for nothing. `None` is treated the same as + // `NoneRequired` (fail closed) in case a future response omits the + // field. + if !matches!( + stage_response.servicing_kind, + Some(ServicingKind::ManualRollbackAb) + ) { + let status = UpdateStatus::new( + &request, + Operation::Rollback, + request.operation_id.clone(), + StatusCode::OperationFailed, + "no AB rollback available to perform for this node", + from_version, + None, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + return Ok(LoopControl::Continue); + } + + self.publish_status(&UpdateStatus::new( + &request, + Operation::Rollback, + request.operation_id.clone(), + StatusCode::InProgress, + "finalizing rollback", + from_version.clone(), + None, + started, + None, + )) + .await?; + self.state.set_pending_commit(PendingCommit { + request: request.clone(), + operation_id: request.operation_id.clone(), + operation: Operation::Rollback, + from_version: from_version.clone(), + to_version: None, + started_utc: started, + })?; + match client + .rollback_finalize(self.config.orchestration.finalize_timeout) + .await + { + Ok(_) => { + let terminal = + rollback_finalize_success_status(&request, from_version.clone(), started); + // Same rationale as handle_finalize(): record the terminal + // status under the rollback's plain operationId before + // rebooting, so a lost state.json doesn't cause + // recover_from_trident_state() to re-run handle_rollback + // from scratch after finalize already succeeded. + if let Err(err) = self.state.remember_completed(terminal.clone()) { + log::warn!("failed to record rollback completion in state.json: {err}"); + } + self.best_effort_publish_terminal(&terminal).await; + match self.rebooter.reboot() { + Ok(()) => Ok(LoopControl::ExitForReboot), + Err(err) => { + self.state.clear_pending_commit()?; + let status = UpdateStatus::new( + &request, + Operation::Rollback, + request.operation_id.clone(), + StatusCode::AgentInternalError, + format!("rollback finalize succeeded but reboot failed: {err}"), + from_version, + None, + started, + Some(Utc::now()), + ); + self.record_and_publish(status).await?; + Ok(LoopControl::Continue) + } + } + } + Err(err) => { + self.state.clear_pending_commit()?; + let status = + rollback_finalize_failure_status(&request, from_version, started, &err); + self.record_and_publish(status).await?; + Ok(LoopControl::Continue) + } + } + } + + async fn resume_pending_commit(&self, pending: PendingCommit) -> Result<(), anyhow::Error> { + let mut client = match TridentClient::connect(&self.config.trident.socket).await { + Ok(client) => client, + Err(err) => { + let status = self + .reconstruct_without_state( + &pending.request, + pending.from_version.clone(), + Some(err.to_string()), + ) + .await; + self.state.clear_pending_commit()?; + self.record_and_publish(status).await?; + return Ok(()); + } + }; + self.publish_status(&UpdateStatus::new( + &pending.request, + Operation::Commit, + commit_operation_id(&pending.operation_id), + StatusCode::InProgress, + "committing post-reboot state", + pending.from_version.clone(), + pending.to_version.clone(), + pending.started_utc, + None, + )) + .await?; + let result = client + .commit(self.config.orchestration.finalize_timeout) + .await; + let status = self.map_commit_result(&pending, result); + self.state.clear_pending_commit()?; + self.record_and_publish(status).await + } + + fn map_commit_result( + &self, + pending: &PendingCommit, + result: Result, + ) -> UpdateStatus { + commit_result_to_status(pending, result) + } + + async fn reconstruct_without_state( + &self, + request: &UpdateRequest, + from_version: Option, + connect_error: Option, + ) -> UpdateStatus { + // state.json did not survive the reboot (or was never written, e.g. + // the agent crashed before persisting pendingCommit). Per + // accepted-design.md §2.3's degraded path, reconstruct the answer by + // calling commit() unconditionally rather than guessing from labels + // or the target version alone - tridentd's commit() is self-checking + // and its own (ServicingKind/RebootStatus/Result) response already + // distinguishes "swap happened, run commit" from "reboot hasn't + // happened yet" from "target armed but firmware fell back" far more + // reliably than a bare version-string comparison could. + if let Some(status) = + reconstruct_precheck_status(request, from_version.clone(), connect_error.as_deref()) + { + return status; + } + + let mut client = match TridentClient::connect(&self.config.trident.socket).await { + Ok(client) => client, + Err(err) => { + return UpdateStatus::new( + request, + request.operation.into(), + request.operation_id.clone(), + StatusCode::AgentInternalError, + format!("state.json missing after reboot and tridentd unreachable: {err}"), + from_version, + request.target_version.clone(), + Utc::now(), + Some(Utc::now()), + ); + } + }; + + let started = Utc::now(); + let result = client + .commit(self.config.orchestration.finalize_timeout) + .await; + reconstruct_commit_result_to_status(request, from_version, started, result) + } + + async fn record_and_publish(&self, status: UpdateStatus) -> Result<(), anyhow::Error> { + self.state.remember_completed(status.clone())?; + // Once the status is recorded in state.json, publishing it to the + // Node annotation is not allowed to be fatal: right after a reboot + // the fake-apiserver/kubelet networking can still be settling + // (transient "connection refused"), and letting that error + // propagate would crash the whole process via `?` up through + // run()/main(). Systemd then restarts the agent in a tight loop + // (visible as repeated "Scheduled restart job" entries), and the + // already-recorded status never makes it onto the Node - the + // annotation just stays stale until something else nudges a + // reconcile. Retry with backoff instead; the write is idempotent + // since remember_completed() already happened. + self.best_effort_publish_terminal(&status).await; + Ok(()) + } + + async fn publish_status(&self, status: &UpdateStatus) -> Result<(), anyhow::Error> { + let mut annotations = BTreeMap::new(); + annotations.insert( + UPDATE_STATUS_ANNOTATION.to_string(), + Some(serde_json::to_string(status)?), + ); + log::info!( + "sending {UPDATE_STATUS_ANNOTATION} annotation to node {}: {status:?}", + self.config.kubernetes.node_name + ); + self.k8s + .patch_node_metadata( + &self.config.kubernetes.node_name, + BTreeMap::new(), + annotations, + ) + .await?; + Ok(()) + } + + async fn best_effort_publish_terminal(&self, status: &UpdateStatus) { + for _ in 0..FINAL_STATUS_PATCH_RETRIES { + if self.publish_status(status).await.is_ok() { + return; + } + tokio::time::sleep(FINAL_STATUS_PATCH_BACKOFF).await; + } + } +} + +/// A request annotation that parsed as JSON but failed schema/semantic +/// validation (e.g. wrong schemaVersion, missing targetVersion for +/// stage/finalize, or a targetVersion present on a rollback request). Kept +/// distinct from "no request at all" so reconcile_node can surface an +/// InvalidRequest status instead of silently ignoring the annotation. +#[derive(Debug, Clone)] +struct InvalidRequest { + node_update_id: Uuid, + operation_id: String, + operation: Operation, + reason: String, +} + +#[derive(Debug, Clone, Default)] +struct Snapshot { + request: Option, + invalid_request: Option, + #[allow(dead_code)] + status: Option, +} + +impl Snapshot { + fn from_node(node: &Node) -> Self { + let annotations = node.metadata.annotations.as_ref(); + let raw_request = annotations.and_then(|a| a.get(UPDATE_REQUEST_ANNOTATION)); + let (request, invalid_request) = match raw_request + .map(|v| serde_json::from_str::(v)) + { + None => (None, None), + Some(Ok(candidate)) => match candidate.clone().validate() { + Ok(valid) => (Some(valid), None), + Err(reason) => ( + None, + Some(InvalidRequest { + node_update_id: candidate.node_update_id, + operation_id: candidate.operation_id, + operation: candidate.operation.into(), + reason, + }), + ), + }, + Some(Err(err)) => { + // Cannot attribute a status to an operationId we couldn't + // even parse out of the annotation - log loudly instead so + // this doesn't fail silently, but there's no request to + // surface an InvalidRequest status against. + log::warn!( + "ignoring malformed {UPDATE_REQUEST_ANNOTATION} annotation (JSON parse failed): {err}" + ); + (None, None) + } + }; + let status = annotations + .and_then(|a| a.get(UPDATE_STATUS_ANNOTATION)) + .and_then(|v| serde_json::from_str::(v).ok()); + Self { + request, + invalid_request, + status, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LoopControl { + Continue, + ExitForReboot, +} + +/// Maps a stage() result to the terminal `UpdateStatus` for that stage +/// attempt. Pure function: no I/O, no side effects - exists so tests can +/// exercise the full success/failure matrix against a fake tridentd without +/// needing a real Kubernetes API or state store. +fn stage_result_to_status( + request: &UpdateRequest, + from_version: Option, + to_version: Option, + started: chrono::DateTime, + result: Result, +) -> UpdateStatus { + match result { + Ok(_) => UpdateStatus::new( + request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::Success, + "stage completed", + from_version, + to_version, + started, + Some(Utc::now()), + ), + Err(err) => UpdateStatus::new( + request, + Operation::Stage, + request.operation_id.clone(), + StatusCode::OperationFailed, + format!("stage failed: {err}"), + from_version, + to_version, + started, + Some(Utc::now()), + ), + } +} + +/// Builds the terminal `UpdateStatus` for a successful finalize() call. Pure +/// function - see `stage_result_to_status` for rationale. +fn finalize_success_status( + request: &UpdateRequest, + from_version: Option, + to_version: Option, + started: chrono::DateTime, +) -> UpdateStatus { + UpdateStatus::new( + request, + Operation::Finalize, + request.operation_id.clone(), + StatusCode::Success, + "finalize completed; rebooting for commit", + from_version, + to_version, + started, + Some(Utc::now()), + ) +} + +/// Builds the terminal `UpdateStatus` for a failed finalize() call. Pure +/// function - see `stage_result_to_status` for rationale. +fn finalize_failure_status( + request: &UpdateRequest, + from_version: Option, + to_version: Option, + started: chrono::DateTime, + err: &TridentClientError, +) -> UpdateStatus { + UpdateStatus::new( + request, + Operation::Finalize, + request.operation_id.clone(), + map_trident_failure(err), + format!("finalize failed: {err}"), + from_version, + to_version, + started, + Some(Utc::now()), + ) +} + +fn map_trident_failure(error: &TridentClientError) -> StatusCode { + if indicates_reverted(error) { + StatusCode::RevertedToPrevious + } else { + StatusCode::OperationFailed + } +} + +fn indicates_reverted(error: &TridentClientError) -> bool { + error + .remote() + .map(|remote| { + // "ab-update-reboot-check"/"ab-update-health-check-commit-check" + // are the forward-update (finalize/commit) reboot-check + // subkinds (ServicingError::AbUpdateRebootCheck / + // HealthChecksError::AbUpdateHealthCheckCommitCheck). + // "manual-rollback-reboot-check" is the *rollback*-specific + // sibling (ServicingError::ManualRollbackRebootCheck), emitted + // when a post-rollback reboot's firmware A/B fallback lands on + // the wrong slot. It is a distinct enum variant with its own + // kebab-case serde subkind, not a copy of the forward-update + // one - both must be checked here, or a real rollback + // boot-fallback silently reports as generic OperationFailed + // instead of RevertedToPrevious. + remote.subkind == "ab-update-reboot-check" + || remote.subkind == "ab-update-health-check-commit-check" + || remote.subkind == "manual-rollback-reboot-check" + }) + .unwrap_or(false) +} + +/// Pure function extracted from `Orchestrator::map_commit_result` so tests +/// can exercise it directly (with a mock-tridentd-driven `Result`) without +/// needing a full `Orchestrator` instance. See `stage_result_to_status` for +/// rationale. +/// Pre-flight checks for the state.json-missing degraded reconstruction +/// path (accepted-design.md §2.3). Returns `Some(status)` when reconstruction +/// cannot proceed (tridentd already known-unreachable, or the outstanding +/// request isn't a finalize/rollback), or `None` when the caller should go +/// on to call tridentd's commit() to determine the real outcome. +fn reconstruct_precheck_status( + request: &UpdateRequest, + from_version: Option, + connect_error: Option<&str>, +) -> Option { + if let Some(err) = connect_error { + return Some(UpdateStatus::new( + request, + request.operation.into(), + request.operation_id.clone(), + StatusCode::AgentInternalError, + format!("state.json missing after reboot and tridentd unreachable: {err}"), + from_version, + request.target_version.clone(), + Utc::now(), + Some(Utc::now()), + )); + } + + if !matches!( + request.operation, + RequestedOperation::Finalize | RequestedOperation::Rollback + ) { + return Some(UpdateStatus::new( + request, + request.operation.into(), + request.operation_id.clone(), + StatusCode::AgentInternalError, + "unable to reconstruct operation without state.json", + from_version, + request.target_version.clone(), + Utc::now(), + Some(Utc::now()), + )); + } + + None +} + +/// Maps tridentd's commit() result to the terminal status for the +/// state.json-missing degraded reconstruction path (accepted-design.md +/// §2.3). Always reports under the `.commit`-suffixed operationId, mirroring +/// the normal post-reboot commit path in `commit_result_to_status`. +fn reconstruct_commit_result_to_status( + request: &UpdateRequest, + from_version: Option, + started: DateTime, + result: Result, +) -> UpdateStatus { + match result { + Ok(response) if response.reboot_status == RebootStatus::RebootRequired => UpdateStatus::new( + request, + Operation::Commit, + commit_operation_id(&request.operation_id), + StatusCode::AgentInternalError, + "state.json missing after reboot; commit requested another reboot", + from_version, + request.target_version.clone(), + started, + Some(Utc::now()), + ), + Ok(_) => UpdateStatus::new( + request, + Operation::Commit, + commit_operation_id(&request.operation_id), + StatusCode::Success, + "state.json missing after reboot; commit() confirmed the swap and completed", + from_version, + request.target_version.clone(), + started, + Some(Utc::now()), + ), + Err(err) if indicates_reverted(&err) => UpdateStatus::new( + request, + Operation::Commit, + commit_operation_id(&request.operation_id), + StatusCode::RevertedToPrevious, + format!( + "state.json missing after reboot; commit detected rollback to previous version: {err}" + ), + from_version, + request.target_version.clone(), + started, + Some(Utc::now()), + ), + Err(err) => UpdateStatus::new( + request, + Operation::Commit, + commit_operation_id(&request.operation_id), + map_trident_failure(&err), + format!("state.json missing after reboot; commit failed: {err}"), + from_version, + request.target_version.clone(), + started, + Some(Utc::now()), + ), + } +} + +fn commit_result_to_status( + pending: &PendingCommit, + result: Result, +) -> UpdateStatus { + match result { + Ok(response) if response.reboot_status == RebootStatus::RebootRequired => { + UpdateStatus::new( + &pending.request, + Operation::Commit, + commit_operation_id(&pending.operation_id), + StatusCode::AgentInternalError, + "commit requested another reboot", + pending.from_version.clone(), + pending.to_version.clone(), + pending.started_utc, + Some(Utc::now()), + ) + } + Ok(_) => UpdateStatus::new( + &pending.request, + Operation::Commit, + commit_operation_id(&pending.operation_id), + StatusCode::Success, + "commit completed", + pending.from_version.clone(), + pending.to_version.clone(), + pending.started_utc, + Some(Utc::now()), + ), + Err(err) if indicates_reverted(&err) => UpdateStatus::new( + &pending.request, + Operation::Commit, + commit_operation_id(&pending.operation_id), + StatusCode::RevertedToPrevious, + format!("commit detected rollback to previous version: {err}"), + pending.from_version.clone(), + pending.to_version.clone(), + pending.started_utc, + Some(Utc::now()), + ), + Err(err) => UpdateStatus::new( + &pending.request, + Operation::Commit, + commit_operation_id(&pending.operation_id), + map_trident_failure(&err), + format!("commit failed: {err}"), + pending.from_version.clone(), + pending.to_version.clone(), + pending.started_utc, + Some(Utc::now()), + ), + } +} + +/// Builds the terminal `UpdateStatus` for a failed rollback_stage() call. +/// Pure function - see `stage_result_to_status` for rationale. +fn rollback_stage_failure_status( + request: &UpdateRequest, + from_version: Option, + started: chrono::DateTime, + err: &TridentClientError, +) -> UpdateStatus { + UpdateStatus::new( + request, + Operation::Rollback, + request.operation_id.clone(), + map_trident_failure(err), + format!("rollback stage failed: {err}"), + from_version, + None, + started, + Some(Utc::now()), + ) +} + +/// Builds the terminal `UpdateStatus` for a successful rollback_finalize() +/// call. Pure function - see `stage_result_to_status` for rationale. +fn rollback_finalize_success_status( + request: &UpdateRequest, + from_version: Option, + started: chrono::DateTime, +) -> UpdateStatus { + UpdateStatus::new( + request, + Operation::Rollback, + request.operation_id.clone(), + StatusCode::Success, + "rollback finalize completed; rebooting for commit", + from_version, + None, + started, + Some(Utc::now()), + ) +} + +/// Builds the terminal `UpdateStatus` for a failed rollback_finalize() call. +/// Pure function - see `stage_result_to_status` for rationale. +fn rollback_finalize_failure_status( + request: &UpdateRequest, + from_version: Option, + started: chrono::DateTime, + err: &TridentClientError, +) -> UpdateStatus { + UpdateStatus::new( + request, + Operation::Rollback, + request.operation_id.clone(), + map_trident_failure(err), + format!("rollback finalize failed: {err}"), + from_version, + None, + started, + Some(Utc::now()), + ) +} + +#[cfg(test)] +mod tests { + use std::sync::{Arc, Mutex}; + + use chrono::Utc; + use uuid::Uuid; + + use super::*; + use crate::{ + annotations::{RequestedOperation, SCHEMA_VERSION}, + mock_tridentd::{connect_mock_client, MockTridentdConfig, Outcome}, + }; + + fn request(operation: RequestedOperation) -> UpdateRequest { + UpdateRequest { + schema_version: SCHEMA_VERSION.to_string(), + node_update_id: Uuid::new_v4(), + operation_id: "op-1".to_string(), + operation, + target_version: Some("2.0.0".to_string()), + } + } + + fn pending(operation: Operation) -> PendingCommit { + PendingCommit { + request: request(RequestedOperation::Finalize), + operation_id: "op-1".to_string(), + operation, + from_version: Some("1.0.0".to_string()), + to_version: Some("2.0.0".to_string()), + started_utc: Utc::now(), + } + } + + // --- rollback --- + + #[tokio::test] + async fn rollback_stage_failure_maps_to_operation_failed() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + rollback_stage: Some(Outcome::Failure { + subkind: "some-rollback-stage-error", + message: "disk full", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client + .rollback_stage(std::time::Duration::from_secs(5)) + .await; + + let request = request(RequestedOperation::Rollback); + let status = rollback_stage_failure_status( + &request, + Some("2.0.0".to_string()), + Utc::now(), + &result.unwrap_err(), + ); + + assert_eq!(status.code, StatusCode::OperationFailed); + assert_eq!(status.operation, Operation::Rollback); + assert!(status.message.contains("rollback stage failed")); + } + + /// Regression coverage for the "rollback with nothing to roll back" + /// fix: RollbackStage's response must carry the real ServicingKind + /// (ManualRollbackAb for a real rollback, NoneRequired for a no-op) so + /// handle_rollback() in this module can distinguish the two before + /// finalizing/rebooting - see the `matches!(stage_response.servicing_kind, ..)` + /// check there. This test pins the wire plumbing `TridentClient` + /// depends on: a mocked RollbackStage response's servicing_kind must + /// survive unchanged into `CompletedResponse`. + #[tokio::test] + async fn rollback_stage_success_reports_servicing_kind() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + rollback_stage: Some(Outcome::Success { + reboot_status: RebootStatus::RebootNotRequired, + servicing_kind: Some(ServicingKind::ManualRollbackAb), + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let response = client + .rollback_stage(std::time::Duration::from_secs(5)) + .await + .expect("mocked rollback_stage should succeed"); + assert_eq!( + response.servicing_kind, + Some(ServicingKind::ManualRollbackAb) + ); + } + + #[tokio::test] + async fn rollback_stage_noop_reports_none_required_servicing_kind() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + rollback_stage: Some(Outcome::Success { + reboot_status: RebootStatus::RebootNotRequired, + servicing_kind: Some(ServicingKind::NoneRequired), + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let response = client + .rollback_stage(std::time::Duration::from_secs(5)) + .await + .expect("mocked rollback_stage should succeed"); + assert_eq!(response.servicing_kind, Some(ServicingKind::NoneRequired)); + } + + #[tokio::test] + async fn rollback_finalize_success_maps_to_success_status() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + rollback_finalize: Some(Outcome::Success { + reboot_status: RebootStatus::RebootRequired, + servicing_kind: None, + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client + .rollback_finalize(std::time::Duration::from_secs(5)) + .await; + assert!(result.is_ok()); + + let request = request(RequestedOperation::Rollback); + let status = + rollback_finalize_success_status(&request, Some("2.0.0".to_string()), Utc::now()); + + assert_eq!(status.code, StatusCode::Success); + assert_eq!(status.operation, Operation::Rollback); + assert_eq!(status.to_version, None); + } + + #[tokio::test] + async fn rollback_finalize_failure_maps_to_operation_failed() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + rollback_finalize: Some(Outcome::Failure { + subkind: "some-rollback-finalize-error", + message: "boom", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client + .rollback_finalize(std::time::Duration::from_secs(5)) + .await; + + let request = request(RequestedOperation::Rollback); + let status = rollback_finalize_failure_status( + &request, + Some("2.0.0".to_string()), + Utc::now(), + &result.unwrap_err(), + ); + + assert_eq!(status.code, StatusCode::OperationFailed); + assert_eq!(status.operation, Operation::Rollback); + assert!(status.message.contains("rollback finalize failed")); + } + + #[tokio::test] + async fn rollback_finalize_reverted_maps_to_reverted_to_previous() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + rollback_finalize: Some(Outcome::Failure { + subkind: "ab-update-reboot-check", + message: "reverted", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client + .rollback_finalize(std::time::Duration::from_secs(5)) + .await; + + let request = request(RequestedOperation::Rollback); + let status = rollback_finalize_failure_status( + &request, + Some("2.0.0".to_string()), + Utc::now(), + &result.unwrap_err(), + ); + + assert_eq!(status.code, StatusCode::RevertedToPrevious); + } + + // --- stage --- + + #[tokio::test] + async fn stage_success_maps_to_success_status() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + stage: Some(Outcome::Success { + reboot_status: RebootStatus::RebootNotRequired, + servicing_kind: None, + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client + .update_stage( + &"http://example.test/image".parse().unwrap(), + None, + std::time::Duration::from_secs(5), + ) + .await; + + let request = request(RequestedOperation::Stage); + let status = stage_result_to_status( + &request, + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + Utc::now(), + result, + ); + + assert_eq!(status.code, StatusCode::Success); + assert_eq!(status.operation, Operation::Stage); + assert_eq!(status.from_version, Some("1.0.0".to_string())); + assert_eq!(status.to_version, Some("2.0.0".to_string())); + } + + #[tokio::test] + async fn stage_failure_maps_to_operation_failed() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + stage: Some(Outcome::Failure { + subkind: "some-stage-error", + message: "disk full", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client + .update_stage( + &"http://example.test/image".parse().unwrap(), + None, + std::time::Duration::from_secs(5), + ) + .await; + + let request = request(RequestedOperation::Stage); + let status = stage_result_to_status(&request, None, None, Utc::now(), result); + + assert_eq!(status.code, StatusCode::OperationFailed); + assert!(status.message.contains("stage failed")); + assert!(status.message.contains("disk full")); + } + + // --- finalize --- + + #[tokio::test] + async fn finalize_success_maps_to_success_status() { + let status = finalize_success_status( + &request(RequestedOperation::Finalize), + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + Utc::now(), + ); + + assert_eq!(status.code, StatusCode::Success); + assert_eq!(status.operation, Operation::Finalize); + assert!(status.message.contains("rebooting")); + } + + #[tokio::test] + async fn finalize_failure_with_generic_error_maps_to_operation_failed() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + finalize: Some(Outcome::Failure { + subkind: "some-finalize-error", + message: "partition swap failed", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let err = client + .update_finalize(std::time::Duration::from_secs(5)) + .await + .unwrap_err(); + + let status = finalize_failure_status( + &request(RequestedOperation::Finalize), + None, + None, + Utc::now(), + &err, + ); + + assert_eq!(status.code, StatusCode::OperationFailed); + assert!(status.message.contains("finalize failed")); + assert!(status.message.contains("partition swap failed")); + } + + #[tokio::test] + async fn finalize_failure_with_reboot_check_subkind_maps_to_reverted() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + finalize: Some(Outcome::Failure { + subkind: "ab-update-reboot-check", + message: "boot did not land on target partition", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let err = client + .update_finalize(std::time::Duration::from_secs(5)) + .await + .unwrap_err(); + + let status = finalize_failure_status( + &request(RequestedOperation::Finalize), + None, + None, + Utc::now(), + &err, + ); + + assert_eq!(status.code, StatusCode::RevertedToPrevious); + } + + // --- commit --- + + #[tokio::test] + async fn commit_success_maps_to_success_status() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Success { + reboot_status: RebootStatus::RebootNotRequired, + servicing_kind: None, + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let status = commit_result_to_status(&pending(Operation::Finalize), result); + + assert_eq!(status.code, StatusCode::Success); + assert_eq!(status.operation, Operation::Commit); + } + + #[tokio::test] + async fn commit_success_but_reboot_required_maps_to_agent_internal_error() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Success { + reboot_status: RebootStatus::RebootRequired, + servicing_kind: None, + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let status = commit_result_to_status(&pending(Operation::Finalize), result); + + assert_eq!(status.code, StatusCode::AgentInternalError); + assert!(status.message.contains("another reboot")); + } + + #[tokio::test] + async fn commit_failure_with_generic_error_maps_to_operation_failed() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Failure { + subkind: "some-commit-error", + message: "commit rpc failed", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let status = commit_result_to_status(&pending(Operation::Finalize), result); + + assert_eq!(status.code, StatusCode::OperationFailed); + assert!(status.message.contains("commit failed")); + } + + #[tokio::test] + async fn commit_failure_with_reboot_check_subkind_maps_to_reverted() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Failure { + subkind: "ab-update-reboot-check", + message: "boot did not land on target partition", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let status = commit_result_to_status(&pending(Operation::Finalize), result); + + assert_eq!(status.code, StatusCode::RevertedToPrevious); + } + + #[tokio::test] + async fn commit_failure_with_health_check_subkind_maps_to_reverted() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Failure { + subkind: "ab-update-health-check-commit-check", + message: "post-commit health check failed", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let status = commit_result_to_status(&pending(Operation::Finalize), result); + + assert_eq!(status.code, StatusCode::RevertedToPrevious); + } + + // --- reconstruct_without_state (state.json missing after reboot) --- + + #[test] + fn reconstruct_precheck_reports_agent_internal_error_when_tridentd_unreachable() { + let request = request(RequestedOperation::Finalize); + let status = reconstruct_precheck_status( + &request, + Some("1.0.0".to_string()), + Some("connection refused"), + ) + .expect("connect error should short-circuit reconstruction"); + + assert_eq!(status.code, StatusCode::AgentInternalError); + assert!(status.message.contains("tridentd unreachable")); + assert_eq!(status.operation_id, request.operation_id); + } + + #[test] + fn reconstruct_precheck_reports_agent_internal_error_for_non_finalize_rollback_operation() { + let request = request(RequestedOperation::Stage); + let status = reconstruct_precheck_status(&request, None, None) + .expect("stage requests cannot be reconstructed without state.json"); + + assert_eq!(status.code, StatusCode::AgentInternalError); + assert!(status + .message + .contains("unable to reconstruct operation without state.json")); + } + + #[test] + fn reconstruct_precheck_allows_finalize_and_rollback_through() { + for operation in [RequestedOperation::Finalize, RequestedOperation::Rollback] { + let request = request(operation); + assert!( + reconstruct_precheck_status(&request, None, None).is_none(), + "expected {operation:?} to proceed to commit() reconstruction" + ); + } + } + + #[tokio::test] + async fn reconstruct_commit_result_success_maps_to_success_status() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Success { + reboot_status: RebootStatus::RebootNotRequired, + servicing_kind: None, + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let request = request(RequestedOperation::Finalize); + let status = reconstruct_commit_result_to_status( + &request, + Some("1.0.0".to_string()), + Utc::now(), + result, + ); + + assert_eq!(status.code, StatusCode::Success); + assert_eq!(status.operation, Operation::Commit); + assert_eq!( + status.operation_id, + commit_operation_id(&request.operation_id) + ); + assert!(status.message.contains("commit() confirmed the swap")); + } + + #[tokio::test] + async fn reconstruct_commit_result_reboot_required_maps_to_agent_internal_error() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Success { + reboot_status: RebootStatus::RebootRequired, + servicing_kind: None, + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let request = request(RequestedOperation::Finalize); + let status = reconstruct_commit_result_to_status(&request, None, Utc::now(), result); + + assert_eq!(status.code, StatusCode::AgentInternalError); + assert!(status.message.contains("requested another reboot")); + } + + #[tokio::test] + async fn reconstruct_commit_result_reverted_subkind_maps_to_reverted_to_previous() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Failure { + subkind: "ab-update-reboot-check", + message: "boot did not land on target partition", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let request = request(RequestedOperation::Rollback); + let status = reconstruct_commit_result_to_status(&request, None, Utc::now(), result); + + assert_eq!(status.code, StatusCode::RevertedToPrevious); + assert!(status.message.contains("detected rollback")); + } + + #[tokio::test] + async fn reconstruct_commit_result_generic_failure_maps_to_operation_failed() { + let config = Arc::new(Mutex::new(MockTridentdConfig { + commit: Some(Outcome::Failure { + subkind: "some-commit-error", + message: "commit rpc failed", + }), + ..Default::default() + })); + let mut client = connect_mock_client(config).await; + let result = client.commit(std::time::Duration::from_secs(5)).await; + + let request = request(RequestedOperation::Finalize); + let status = reconstruct_commit_result_to_status(&request, None, Utc::now(), result); + + assert_eq!(status.code, StatusCode::OperationFailed); + assert!(status.message.contains("commit failed")); + // Regression check for DR-003: the generic-failure branch must use the + // same Operation::Commit / `.commit`-suffixed operationId as every other + // branch of this function, matching commit_result_to_status and the + // doc comment above reconstruct_commit_result_to_status. + assert_eq!(status.operation, Operation::Commit); + assert_eq!( + status.operation_id, + commit_operation_id(&request.operation_id) + ); + } +} diff --git a/crates/trident-acl-agent/src/state.rs b/crates/trident-acl-agent/src/state.rs new file mode 100644 index 0000000000..485a410b7f --- /dev/null +++ b/crates/trident-acl-agent/src/state.rs @@ -0,0 +1,280 @@ +//! Persistent agent state (`/var/lib/trident-acl-agent/state.json`): +//! completed-operation cache and the pending post-reboot commit record. +//! +//! Implements the `state.json` mechanism from `docs/update-trigger-design.md`: +//! https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md +//! (section 2.3), which bridges the pre-reboot finalize/rollback half and +//! the post-reboot commit half of an operation across the reboot. + +use std::{ + collections::BTreeMap, + fs, + path::{Path, PathBuf}, +}; + +use anyhow::Context; +use serde::{Deserialize, Serialize}; + +use crate::annotations::{UpdateRequest, UpdateStatus}; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct PersistentState { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pending_commit: Option, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub completed: BTreeMap, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct PendingCommit { + pub request: UpdateRequest, + pub operation_id: String, + pub operation: crate::annotations::Operation, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub from_version: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub to_version: Option, + pub started_utc: chrono::DateTime, +} + +#[derive(Debug, Clone)] +pub struct StateStore { + path: PathBuf, +} + +impl StateStore { + pub fn new(path: impl Into) -> Self { + Self { path: path.into() } + } + + pub fn path(&self) -> &Path { + &self.path + } + + pub fn load(&self) -> Result { + match fs::read_to_string(&self.path) { + Ok(raw) => Ok(serde_json::from_str(&raw).context("failed to parse state.json")?), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + Ok(PersistentState::default()) + } + Err(err) => { + Err(anyhow::Error::new(err) + .context(format!("failed to read {}", self.path.display()))) + } + } + } + + pub fn save(&self, state: &PersistentState) -> Result<(), anyhow::Error> { + if let Some(parent) = self.path.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + fs::write(&self.path, serde_json::to_string_pretty(state)?) + .with_context(|| format!("failed to write {}", self.path.display())) + } + + pub fn remember_completed(&self, status: UpdateStatus) -> Result<(), anyhow::Error> { + let mut state = self.load()?; + state.completed.insert(status.operation_id.clone(), status); + self.save(&state) + } + + pub fn set_pending_commit(&self, pending: PendingCommit) -> Result<(), anyhow::Error> { + let mut state = self.load()?; + state.pending_commit = Some(pending); + self.save(&state) + } + + pub fn clear_pending_commit(&self) -> Result<(), anyhow::Error> { + let mut state = self.load()?; + state.pending_commit = None; + self.save(&state) + } +} + +#[cfg(test)] +mod tests { + use chrono::Utc; + use uuid::Uuid; + + use super::*; + use crate::annotations::{Operation, RequestedOperation, StatusCode, SCHEMA_VERSION}; + + fn store() -> (tempfile::TempDir, StateStore) { + let dir = tempfile::tempdir().expect("failed to create temp dir"); + let path = dir.path().join("state.json"); + let store = StateStore::new(path); + (dir, store) + } + + fn sample_request() -> UpdateRequest { + UpdateRequest { + schema_version: SCHEMA_VERSION.to_string(), + node_update_id: Uuid::new_v4(), + operation_id: "op-1".to_string(), + operation: RequestedOperation::Finalize, + target_version: Some("2.0.0".to_string()), + } + } + + fn sample_status(operation_id: &str) -> UpdateStatus { + UpdateStatus::new( + &sample_request(), + Operation::Finalize, + operation_id.to_string(), + StatusCode::Success, + "finalize completed", + Some("1.0.0".to_string()), + Some("2.0.0".to_string()), + Utc::now(), + Some(Utc::now()), + ) + } + + fn sample_pending() -> PendingCommit { + PendingCommit { + request: sample_request(), + operation_id: "op-1".to_string(), + operation: Operation::Finalize, + from_version: Some("1.0.0".to_string()), + to_version: Some("2.0.0".to_string()), + started_utc: Utc::now(), + } + } + + #[test] + fn load_returns_default_when_file_missing() { + let (_dir, store) = store(); + let state = store + .load() + .expect("load should not fail when file is absent"); + assert_eq!(state, PersistentState::default()); + assert!(state.pending_commit.is_none()); + assert!(state.completed.is_empty()); + } + + #[test] + fn save_then_load_round_trips_full_state() { + let (_dir, store) = store(); + let mut completed = std::collections::BTreeMap::new(); + completed.insert("op-1".to_string(), sample_status("op-1")); + let state = PersistentState { + pending_commit: Some(sample_pending()), + completed, + }; + store.save(&state).expect("save should succeed"); + let loaded = store.load().expect("load should succeed after save"); + + assert_eq!(loaded, state); + } + + #[test] + fn save_creates_parent_directories() { + let dir = tempfile::tempdir().expect("failed to create temp dir"); + let nested_path = dir.path().join("nested").join("deeper").join("state.json"); + let store = StateStore::new(nested_path.clone()); + + store + .save(&PersistentState::default()) + .expect("save should create missing parent directories"); + + assert!(nested_path.exists()); + } + + #[test] + fn remember_completed_inserts_by_operation_id_without_clobbering_others() { + let (_dir, store) = store(); + store + .remember_completed(sample_status("op-1")) + .expect("remember_completed should succeed"); + store + .remember_completed(sample_status("op-2")) + .expect("remember_completed should succeed"); + + let state = store.load().expect("load should succeed"); + assert_eq!(state.completed.len(), 2); + assert!(state.completed.contains_key("op-1")); + assert!(state.completed.contains_key("op-2")); + } + + #[test] + fn remember_completed_overwrites_same_operation_id() { + let (_dir, store) = store(); + store + .remember_completed(sample_status("op-1")) + .expect("first remember_completed should succeed"); + + let mut updated = sample_status("op-1"); + updated.message = "updated message".to_string(); + store + .remember_completed(updated) + .expect("second remember_completed should succeed"); + + let state = store.load().expect("load should succeed"); + assert_eq!(state.completed.len(), 1); + assert_eq!(state.completed["op-1"].message, "updated message"); + } + + #[test] + fn set_and_clear_pending_commit_round_trip() { + let (_dir, store) = store(); + assert!(store.load().unwrap().pending_commit.is_none()); + + let pending = sample_pending(); + store + .set_pending_commit(pending.clone()) + .expect("set_pending_commit should succeed"); + let state = store.load().expect("load should succeed"); + assert_eq!(state.pending_commit, Some(pending)); + + store + .clear_pending_commit() + .expect("clear_pending_commit should succeed"); + let state = store.load().expect("load should succeed"); + assert!(state.pending_commit.is_none()); + } + + #[test] + fn set_pending_commit_preserves_existing_completed_entries() { + let (_dir, store) = store(); + store + .remember_completed(sample_status("op-1")) + .expect("remember_completed should succeed"); + store + .set_pending_commit(sample_pending()) + .expect("set_pending_commit should succeed"); + + let state = store.load().expect("load should succeed"); + assert!(state.pending_commit.is_some()); + assert_eq!(state.completed.len(), 1); + } + + #[test] + fn load_fails_with_context_on_corrupt_json() { + let (_dir, store) = store(); + std::fs::write(store.path(), "not valid json").expect("failed to write corrupt state"); + + let err = store.load().expect_err("load must fail on corrupt JSON"); + assert!( + err.to_string().contains("failed to parse state.json"), + "error should mention state.json parsing, got: {err}" + ); + } + + #[test] + fn deny_unknown_fields_rejects_unrecognized_state_json_keys() { + let (_dir, store) = store(); + std::fs::write( + store.path(), + r#"{"pendingCommit": null, "completed": {}, "unexpectedField": true}"#, + ) + .expect("failed to write state with unknown field"); + + let err = store + .load() + .expect_err("load must reject unknown fields per deny_unknown_fields"); + assert!(err.to_string().contains("failed to parse state.json")); + } +} diff --git a/crates/trident-acl-agent/src/trident.rs b/crates/trident-acl-agent/src/trident.rs new file mode 100644 index 0000000000..79a0a38470 --- /dev/null +++ b/crates/trident-acl-agent/src/trident.rs @@ -0,0 +1,380 @@ +//! gRPC helpers for talking to `tridentd`. +//! +//! Implements the Trident-invocation half of `docs/update-trigger-design.md`: +//! https://msazure.visualstudio.com/One/_git/Compute-ACL-Update-Service?version=GC67946fff8f296e10217b70e063c896e6028ea843&path=/docs/update-trigger-design.md +//! (the "Trident invocation" column of section 2.1's operations table, +//! and the stage/finalize/rollback-finalize CallerHandlesReboot split in +//! section 2.3). +//! +//! The label protocol drives stage/finalize/commit directly against tridentd's +//! stable v1 API (§4–§5). Startup recovery no longer pre-queries the preview +//! `StatusService::GetServicingState`: commit() is self-checking (tridentd +//! only commits from a valid servicing_state and otherwise returns +//! ServicingKind::NoneRequired as a harmless no-op), so the orchestrator +//! always calls commit() unconditionally and falls back to label-based +//! progress for anything commit() reports nothing to do for. See +//! orchestrator.rs's recover_from_trident_state for the full rationale. + +use std::time::Duration; + +use anyhow::anyhow; +use futures::StreamExt; +use tonic::{transport::Endpoint, Request, Streaming}; +use trident_proto::v1::{ + commit_service_client::CommitServiceClient, rollback_service_client::RollbackServiceClient, + servicing_response::Response as ResponseBody, update_service_client::UpdateServiceClient, + CommitRequest, FinalizeUpdateRequest, HostConfiguration, LogLevel, ManualRollbackKind, + RebootHandling, RebootManagement, RebootStatus, RollbackFinalizeRequest, RollbackStageRequest, + ServicingKind, ServicingResponse, StageUpdateRequest, StatusCode, TridentErrorKind, + UpdateRequest, +}; +use url::Url; + +#[derive(Debug, Clone)] +pub struct CompletedResponse { + pub reboot_status: RebootStatus, + pub servicing_kind: Option, +} + +#[derive(Debug, Clone)] +pub struct RemoteError { + pub kind: Option, + pub subkind: String, + pub message: String, + pub error_message: String, +} + +#[derive(Debug, thiserror::Error)] +pub enum TridentClientError { + #[error("failed to connect to trident socket {socket}: {source}")] + Connect { + socket: String, + #[source] + source: tonic::transport::Error, + }, + #[error("failed to start trident request {operation}: {source}")] + Request { + operation: &'static str, + #[source] + source: tonic::Status, + }, + #[error("trident stream for {operation} ended before a Completed message")] + MissingCompletion { operation: &'static str }, + #[error("trident stream for {operation} failed: {source}")] + Stream { + operation: &'static str, + #[source] + source: anyhow::Error, + }, + #[error("trident reported {operation} failure: {details:?}")] + Remote { + operation: &'static str, + details: RemoteError, + }, + #[error("{operation} timed out after {timeout:?}")] + Timeout { + operation: &'static str, + timeout: Duration, + }, +} + +impl TridentClientError { + pub fn remote(&self) -> Option<&RemoteError> { + match self { + Self::Remote { details, .. } => Some(details), + _ => None, + } + } +} + +pub struct TridentClient { + update_client: UpdateServiceClient, + commit_client: CommitServiceClient, + rollback_client: RollbackServiceClient, +} + +impl TridentClient { + pub async fn connect(socket: &str) -> Result { + let endpoint = + Endpoint::new(socket.to_string()).map_err(|source| TridentClientError::Connect { + socket: socket.to_string(), + source, + })?; + let channel = endpoint + .connect() + .await + .map_err(|source| TridentClientError::Connect { + socket: socket.to_string(), + source, + })?; + + Ok(Self::from_channel(channel)) + } + + /// Builds a client directly from an existing tonic Channel, bypassing + /// socket/URI resolution entirely. Production code always goes through + /// connect(); this exists so tests can hand the client a channel wired + /// to an in-process fake tridentd (e.g. via Endpoint::connect_with_connector + /// over an in-memory duplex stream) and exercise the exact same + /// request/response/error-mapping code as production, without a real + /// unix socket or subprocess. + pub fn from_channel(channel: tonic::transport::Channel) -> Self { + Self { + update_client: UpdateServiceClient::new(channel.clone()), + commit_client: CommitServiceClient::new(channel.clone()), + rollback_client: RollbackServiceClient::new(channel), + } + } + + pub async fn update( + &mut self, + url: &Url, + hash: Option<&str>, + timeout: Duration, + ) -> Result { + let response = self + .update_client + .update(Request::new(UpdateRequest { + stage: Some(StageUpdateRequest { + config: Some(host_configuration_from_image(url, hash)), + }), + finalize: Some(FinalizeUpdateRequest { + reboot: Some(RebootManagement { + handling: RebootHandling::CallerHandlesReboot.into(), + }), + }), + })) + .await + .map_err(|source| TridentClientError::Request { + operation: "update", + source, + })? + .into_inner(); + + run_with_timeout( + "update", + timeout, + consume_servicing_stream("update", response), + ) + .await + } + + pub async fn update_stage( + &mut self, + url: &Url, + hash: Option<&str>, + timeout: Duration, + ) -> Result { + let response = self + .update_client + .update_stage(Request::new(StageUpdateRequest { + config: Some(host_configuration_from_image(url, hash)), + })) + .await + .map_err(|source| TridentClientError::Request { + operation: "update_stage", + source, + })? + .into_inner(); + + run_with_timeout( + "update_stage", + timeout, + consume_servicing_stream("update_stage", response), + ) + .await + } + + pub async fn update_finalize( + &mut self, + timeout: Duration, + ) -> Result { + let response = self + .update_client + .update_finalize(Request::new(FinalizeUpdateRequest { + reboot: Some(RebootManagement { + handling: RebootHandling::CallerHandlesReboot.into(), + }), + })) + .await + .map_err(|source| TridentClientError::Request { + operation: "update_finalize", + source, + })? + .into_inner(); + + run_with_timeout( + "update_finalize", + timeout, + consume_servicing_stream("update_finalize", response), + ) + .await + } + + pub async fn commit( + &mut self, + timeout: Duration, + ) -> Result { + let response = self + .commit_client + .commit(Request::new(CommitRequest { + reboot: Some(RebootManagement { + // The agent, not tridentd, must own every reboot + // decision: AKS-RP is the sole authority over + // reboot/rollback (accepted-design.md §2.5). If commit() + // ever reports NeedsReboot (e.g. a health-check failure, + // were health checks ever re-enabled), the agent needs + // to see that as a RebootRequired response it controls + // and reports via labels, not have tridentd reboot out + // from under it. + handling: RebootHandling::CallerHandlesReboot.into(), + }), + })) + .await + .map_err(|source| TridentClientError::Request { + operation: "commit", + source, + })? + .into_inner(); + + run_with_timeout( + "commit", + timeout, + consume_servicing_stream("commit", response), + ) + .await + } + + /// Stages an A/B rollback. Only `AbRollbackRequested` is used - per the + /// accepted design, trident-acl-agent only ever drives AB-kind manual + /// rollback; runtime-kind and "any" rollback are out of scope for the + /// annotation-driven protocol. + pub async fn rollback_stage( + &mut self, + timeout: Duration, + ) -> Result { + let response = self + .rollback_client + .rollback_stage(Request::new(RollbackStageRequest { + kind: ManualRollbackKind::AbRollbackRequested.into(), + })) + .await + .map_err(|source| TridentClientError::Request { + operation: "rollback_stage", + source, + })? + .into_inner(); + + run_with_timeout( + "rollback_stage", + timeout, + consume_servicing_stream("rollback_stage", response), + ) + .await + } + + pub async fn rollback_finalize( + &mut self, + timeout: Duration, + ) -> Result { + let response = self + .rollback_client + .rollback_finalize(Request::new(RollbackFinalizeRequest { + reboot: Some(RebootManagement { + // Same rationale as commit()/update_finalize(): AKS-RP, + // via the agent, is the sole authority over reboot + // timing (accepted-design.md §2.5). + handling: RebootHandling::CallerHandlesReboot.into(), + }), + })) + .await + .map_err(|source| TridentClientError::Request { + operation: "rollback_finalize", + source, + })? + .into_inner(); + + run_with_timeout( + "rollback_finalize", + timeout, + consume_servicing_stream("rollback_finalize", response), + ) + .await + } +} + +pub fn host_configuration_from_image(url: &Url, hash: Option<&str>) -> HostConfiguration { + HostConfiguration { + config: match hash { + Some(hash) => format!("image:\n url: {url}\n sha384: {hash}"), + None => format!("image:\n url: {url}\n sha384: ignored"), + }, + } +} + +async fn run_with_timeout( + operation: &'static str, + timeout: Duration, + future: impl std::future::Future>, +) -> Result { + tokio::time::timeout(timeout, future) + .await + .map_err(|_| TridentClientError::Timeout { operation, timeout })? +} + +async fn consume_servicing_stream( + operation: &'static str, + mut stream: Streaming, +) -> Result { + while let Some(item) = stream.next().await { + let response = item.map_err(|source| TridentClientError::Stream { + operation, + source: anyhow!(source), + })?; + + match response.response { + Some(ResponseBody::Started(_)) => { + log::info!("[Trident:{operation}] started"); + } + Some(ResponseBody::Log(log_record)) => { + let msg = format!("[Trident:{operation}] {}", log_record.message); + match log_record.level() { + LogLevel::Unspecified | LogLevel::Trace => log::trace!("{msg}"), + LogLevel::Debug => log::debug!("{msg}"), + LogLevel::Info => log::info!("{msg}"), + LogLevel::Warn => log::warn!("{msg}"), + LogLevel::Error => log::error!("{msg}"), + } + } + Some(ResponseBody::Completed(completed)) => { + if completed.status() == StatusCode::Success { + return Ok(CompletedResponse { + reboot_status: completed.reboot_status(), + servicing_kind: completed + .servicing_kind + .and_then(|value| ServicingKind::try_from(value).ok()), + }); + } + + let details = completed + .error + .map(|error| RemoteError { + kind: TridentErrorKind::try_from(error.kind).ok(), + subkind: error.subkind, + message: error.message, + error_message: error.error_message, + }) + .unwrap_or(RemoteError { + kind: None, + subkind: "unknown".to_string(), + message: format!("Trident {operation} failed without structured error"), + error_message: String::new(), + }); + return Err(TridentClientError::Remote { operation, details }); + } + None => continue, + } + } + + Err(TridentClientError::MissingCompletion { operation }) +} diff --git a/packaging/rpm/trident.spec b/packaging/rpm/trident.spec index 411c195d55..324e75e904 100644 --- a/packaging/rpm/trident.spec +++ b/packaging/rpm/trident.spec @@ -221,6 +221,16 @@ The Trident ACL Agent triggers updates of ACL images. %files acl-agent %{_bindir}/%{name}-acl-agent +%{_unitdir}/%{name}-acl-agent.service + +%post acl-agent +%systemd_post %{name}-acl-agent.service + +%preun acl-agent +%systemd_preun %{name}-acl-agent.service + +%postun acl-agent +%systemd_postun_with_restart %{name}-acl-agent.service %endif # ------------------------------------------------------------------------------ @@ -289,6 +299,7 @@ cargo test --all --no-fail-fast -- --skip test_run_systemd_check --skip test_pre install -D -m 755 target/release/%{name} %{buildroot}/%{_bindir}/%{name} %if %{defined rpm_ver} install -D -m 755 target/release/%{name}-acl-agent %{buildroot}/%{_bindir}/%{name}-acl-agent +install -D -m 644 packaging/systemd/%{name}-acl-agent.service %{buildroot}%{_unitdir}/%{name}-acl-agent.service %endif # Copy Trident SELinux policy module to /usr/share/selinux/packages diff --git a/packaging/systemd/trident-acl-agent.service b/packaging/systemd/trident-acl-agent.service new file mode 100644 index 0000000000..eee90451e2 --- /dev/null +++ b/packaging/systemd/trident-acl-agent.service @@ -0,0 +1,12 @@ +[Unit] +Description=Trident ACL Agent +After=network-online.target tridentd.socket +Wants=network-online.target tridentd.socket + +[Service] +ExecStart=trident-acl-agent +Restart=on-failure +RestartSec=5 + +[Install] +WantedBy=multi-user.target From c3dfd8d978dd31bd3c2ea9071797e54162db1dda Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Wed, 5 Aug 2026 20:00:18 +0000 Subject: [PATCH 2/6] trident-acl-agent: pin enum-ordinalize to rustc-1.86-compatible version Cargo.lock had resolved enum-ordinalize/enum-ordinalize-derive 4.4.2 (a transitive dependency via kube-runtime -> educe), which requires rustc 1.89+. The RPM build pins rust-1.86.0, causing: error: rustc 1.86.0 is not supported by the following packages: enum-ordinalize@4.4.2 requires rustc 1.89 enum-ordinalize-derive@4.4.2 requires rustc 1.89 Pinned both to 4.3.2 (rust-version 1.68, well under 1.86) via cargo update --precise. This also collapses the dependency graph back to a single syn version (2.0.90) - 4.4.2s derive crate needed syn 3.x, which no longer resolves once removed. Verified: cargo build/test/clippy -p trident-acl-agent clean, and the real docker-based "make bin/trident-rpms.tar.gz" RPM build (which uses the actual pinned rust-1.86.0 toolchain, not the newer local dev rustc that produced the original lockfile) now succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c06585d-a82d-475f-a802-83fdfa012d86 --- Cargo.lock | 111 ++++++++++++++++++++++++----------------------------- 1 file changed, 50 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index da53c76252..22ee3e3ac4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -159,7 +159,7 @@ checksum = "c7c24de15d275a1ecfd47a380fb4d5ec9bfe0933f309ed5e705b775596a3574d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -170,7 +170,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -411,7 +411,7 @@ dependencies = [ "heck 0.4.1", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -595,7 +595,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.90", + "syn", ] [[package]] @@ -606,7 +606,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -627,7 +627,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -637,7 +637,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab63b0e2bf4d5928aff72e83a7dace85d7bba5fe12dcc3c5a572d78caffd3f3c" dependencies = [ "derive_builder_core", - "syn 2.0.90", + "syn", ] [[package]] @@ -650,7 +650,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn 2.0.90", + "syn", ] [[package]] @@ -678,7 +678,7 @@ checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -738,7 +738,7 @@ dependencies = [ "optfield", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -768,7 +768,7 @@ dependencies = [ "enum-ordinalize", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -788,22 +788,22 @@ dependencies = [ [[package]] name = "enum-ordinalize" -version = "4.4.2" +version = "4.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89dd01549b09589510cf0647475075d12071456586d70f5c75c98ae2a5537677" +checksum = "4a1091a7bb1f8f2c4b28f1fe2cef4980ca2d410a3d727d67ecc3178c9b0800f0" dependencies = [ "enum-ordinalize-derive", ] [[package]] name = "enum-ordinalize-derive" -version = "4.4.2" +version = "4.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a65863d15a4ce2888bd2f0f543cc963d3879c3a022c8ee43f6141d479a3ac815" +checksum = "8ca9601fb2d62598ee17836250842873a413586e5d7ed88b356e38ddbb0ec631" dependencies = [ "proc-macro2", "quote", - "syn 3.0.3", + "syn", ] [[package]] @@ -824,7 +824,7 @@ checksum = "de0d48a183585823424a4ce1aa132d174a6a81bd540895822eb4c8373a8e49e8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -1031,7 +1031,7 @@ checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -1105,7 +1105,7 @@ dependencies = [ "proc-macro-error2", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -1580,7 +1580,7 @@ checksum = "1ec89e9337638ecdc08744df490b221a7399bf8d164eb52a665454e60e075ad6" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2176,7 +2176,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2211,7 +2211,7 @@ checksum = "fa59f025cde9c698fcb4fcb3533db4621795374065bee908215263488f2d2a1d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2373,7 +2373,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2437,7 +2437,7 @@ dependencies = [ "phf_shared", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2466,7 +2466,7 @@ checksum = "6e918e4ff8c4549eb882f14b3a4bc8c8bc93de829416eacf579f1207a8fbf861" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2513,7 +2513,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "64d1ec885c64d0457d564db4ec299b2dae3f9c02808b8ad9c3a089c591b18033" dependencies = [ "proc-macro2", - "syn 2.0.90", + "syn", ] [[package]] @@ -2535,7 +2535,7 @@ dependencies = [ "proc-macro-error-attr2", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2600,7 +2600,7 @@ dependencies = [ "pulldown-cmark", "pulldown-cmark-to-cmark", "regex", - "syn 2.0.90", + "syn", "tempfile", ] @@ -2614,7 +2614,7 @@ dependencies = [ "itertools", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -2666,7 +2666,7 @@ dependencies = [ "proc-macro2", "pytest", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3013,7 +3013,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 2.0.90", + "syn", ] [[package]] @@ -3113,7 +3113,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3124,7 +3124,7 @@ checksum = "18d26a20a969b9e3fdf2fc2d9f21eda6c40e2de84c9408bb5d3b05d499aae711" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3310,7 +3310,7 @@ checksum = "0eb01866308440fc64d6c44d9e86c5cc17adfe33c4d6eed55da9145044d0ffc1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3397,7 +3397,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.90", + "syn", ] [[package]] @@ -3410,7 +3410,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.90", + "syn", ] [[package]] @@ -3436,17 +3436,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "syn" -version = "3.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", -] - [[package]] name = "sync_wrapper" version = "1.0.2" @@ -3464,7 +3453,7 @@ checksum = "c8af7666ab7b6390ab78131fb5b0fce11d6b7a6951602017c35fa82800708971" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3621,7 +3610,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3632,7 +3621,7 @@ checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3695,7 +3684,7 @@ checksum = "af407857209536a95c8e56f8231ef2c2e2aff839b22e07a1ffcbc617e9db9fa5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3822,7 +3811,7 @@ dependencies = [ "prettyplease", "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -3859,7 +3848,7 @@ dependencies = [ "prost-build", "prost-types", "quote", - "syn 2.0.90", + "syn", "tempfile", "tonic-build", ] @@ -3933,7 +3922,7 @@ checksum = "395ae124c09f9e6918a2310af6038fba074bcf474ac352496d5910dd59a2226d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -4391,7 +4380,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.90", + "syn", "wasm-bindgen-shared", ] @@ -4426,7 +4415,7 @@ checksum = "98c9ae5a76e46f4deecd0f0255cc223cfa18dc9b261213b8aa0c7b36f61b3f1d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -4856,7 +4845,7 @@ checksum = "2380878cad4ac9aac1e2435f3eb4020e8374b5f13c296cb75b4620ff8e229154" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", "synstructure", ] @@ -4887,7 +4876,7 @@ checksum = "fa4f8080344d4671fb4e831a13ad1e68092748387dfc4f55e356242fae12ce3e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -4898,7 +4887,7 @@ checksum = "88d2b8d9c68ad2b9e4340d7832716a4d21a22a1154777ad56ea55c51a9cf3831" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] @@ -4918,7 +4907,7 @@ checksum = "595eed982f7d355beb85837f651fa22e90b3c044842dc7f2c2842c086f295808" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", "synstructure", ] @@ -4947,7 +4936,7 @@ checksum = "6eafa6dfb17584ea3e2bd6e76e0cc15ad7af12b09abdd1ca55961bed9b1063c6" dependencies = [ "proc-macro2", "quote", - "syn 2.0.90", + "syn", ] [[package]] From f4edacdd79e549b065cf414f71990c7b06889f3e Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Wed, 5 Aug 2026 21:43:32 +0000 Subject: [PATCH 3/6] trident-acl-agent: fix panic when querying Nebraska from async context query_for_update() is a blocking call (reqwest::blocking under the hood in omaha::send), which was called directly from two async fns: run_omaha_only() and orchestrator.rs's handle_stage(). This panics with "Cannot drop a runtime in a context where blocking is not allowed" the moment a real response is received, because reqwest::blocking spins up its own inner Tokio runtime per call, which isn't safe to tear down from inside an already-running async task. Confirmed this is a real, standard-usage bug, not specific to any one code path: reproduced the panic both via the default omaha-only invocation (no flags) and would affect handle_stage() identically, since it uses the exact same call pattern. Fixed both call sites by running query_for_update() on a dedicated blocking thread via tokio::task::spawn_blocking, matching the fix already applied to the --validate-connection nebraska check on user/bfjelds/acl-agent-connection-check. Verified: cargo test -p trident-acl-agent (78 passed), cargo clippy --all-targets -- -D warnings (clean), cargo fmt --check (clean). Reproduced the panic against a local mock Omaha server before the fix, confirmed clean exit 0 with no panic after. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c06585d-a82d-475f-a802-83fdfa012d86 --- crates/trident-acl-agent/src/lib.rs | 27 ++++++++++++++----- crates/trident-acl-agent/src/orchestrator.rs | 28 +++++++++++++++----- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/crates/trident-acl-agent/src/lib.rs b/crates/trident-acl-agent/src/lib.rs index 72abad8807..799d8bdf75 100644 --- a/crates/trident-acl-agent/src/lib.rs +++ b/crates/trident-acl-agent/src/lib.rs @@ -6,6 +6,7 @@ //! design doc (`aks-rp ↔ trident-acl-agent`, especially §3–§6 and §12–§13), //! while preserving the original `omaha-only` mode as the default. +use anyhow::Context; use semver::Version; use sha2::{Digest, Sha256}; use url::Url; @@ -67,13 +68,25 @@ pub async fn run_omaha_only(config: &config::AgentConfig) -> Result<(), anyhow:: ) })?; - let response = query_for_update( - &endpoint, - &config.nebraska.app_id, - DEFAULT_NEBRASKA_TRACK, - &Version::new(0, 0, 0), - IdSource::MachineIdHashed, - )?; + // query_for_update() is a blocking call (reqwest::blocking under the + // hood, see omaha::send) - calling it directly from this async fn can + // panic ("Cannot drop a runtime in a context where blocking is not + // allowed") because reqwest::blocking spins up its own inner Tokio + // runtime per call, which isn't safe to tear down from inside an + // already-running async task. Run it on a dedicated blocking thread. + let app_id = config.nebraska.app_id.clone(); + let endpoint_for_task = endpoint.clone(); + let response = tokio::task::spawn_blocking(move || { + query_for_update( + &endpoint_for_task, + &app_id, + DEFAULT_NEBRASKA_TRACK, + &Version::new(0, 0, 0), + IdSource::MachineIdHashed, + ) + }) + .await + .context("Nebraska query task panicked")??; match response.result { QueryResult::NoUpdate => { diff --git a/crates/trident-acl-agent/src/orchestrator.rs b/crates/trident-acl-agent/src/orchestrator.rs index 8ac9020520..1249e3483a 100644 --- a/crates/trident-acl-agent/src/orchestrator.rs +++ b/crates/trident-acl-agent/src/orchestrator.rs @@ -11,6 +11,7 @@ use std::{collections::BTreeMap, process::Command}; +use anyhow::Context; use chrono::{DateTime, Utc}; use futures::StreamExt; use k8s_openapi::api::core::v1::Node; @@ -301,13 +302,26 @@ where let endpoint = self.config.nebraska.endpoint.clone().ok_or_else(|| { anyhow::anyhow!("annotation mode requires [nebraska].endpoint or CLI override") })?; - let response = query_for_update( - &endpoint, - &self.config.nebraska.app_id, - DEFAULT_NEBRASKA_TRACK, - &Version::new(0, 0, 0), - IdSource::MachineIdHashed, - )?; + // query_for_update() is a blocking call (reqwest::blocking under the + // hood, see omaha::send) - calling it directly from this async fn + // can panic ("Cannot drop a runtime in a context where blocking is + // not allowed") because reqwest::blocking spins up its own inner + // Tokio runtime per call, which isn't safe to tear down from inside + // an already-running async task. Run it on a dedicated blocking + // thread instead. + let app_id = self.config.nebraska.app_id.clone(); + let endpoint_for_task = endpoint.clone(); + let response = tokio::task::spawn_blocking(move || { + query_for_update( + &endpoint_for_task, + &app_id, + DEFAULT_NEBRASKA_TRACK, + &Version::new(0, 0, 0), + IdSource::MachineIdHashed, + ) + }) + .await + .context("Nebraska query task panicked")??; let offered = match response.result { QueryResult::NoUpdate => { let status = UpdateStatus::new( From e277fcaa2356f6e9e54a907b22ad8e75c92f2e80 Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Wed, 5 Aug 2026 21:43:59 +0000 Subject: [PATCH 4/6] trident-acl-agent: remove dead nebraska.poll_interval; add default nebraska.endpoint nebraska.poll_interval was declared, defaulted, parsed from TOML, and unit-tested, but never actually consumed anywhere: neither run_omaha_only() (a genuine one-shot, no internal loop) nor the label/annotation orchestrator (event-driven off the Kubernetes watch, not a timer) ever read it. There's also no companion systemd .timer unit to periodically re-invoke the agent. Removed the field, its default constant, its TOML parsing, and the corresponding test assertions/fixtures. Also added a default for nebraska.endpoint (previously None, requiring an explicit config or CLI override or the agent would fail to start). Defaults to a `.invalid`-TLD placeholder (RFC 2606, guaranteed to never resolve) until the real production endpoint is known - deployments that forget to override it fail loudly at the network layer instead of silently querying a real-looking but wrong host. The existing `ok_or_else` "no Nebraska endpoint configured" guards in run_omaha_only/ handle_stage/validate_connection are left in place as harmless defensive code, even though endpoint is now effectively always populated. Verified: cargo test -p trident-acl-agent (78 passed, including updated config parsing tests), cargo clippy --all-targets -- -D warnings (clean), cargo fmt --check (clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c06585d-a82d-475f-a802-83fdfa012d86 --- crates/trident-acl-agent/src/config.rs | 35 ++++++++++++-------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/crates/trident-acl-agent/src/config.rs b/crates/trident-acl-agent/src/config.rs index 432212cd43..ba157bcf20 100644 --- a/crates/trident-acl-agent/src/config.rs +++ b/crates/trident-acl-agent/src/config.rs @@ -19,7 +19,12 @@ use crate::DEFAULT_NEBRASKA_APP_ID; pub const DEFAULT_CONFIG_PATH: &str = "/etc/trident/trident-acl-agent.conf"; pub const DEFAULT_KUBERNETES_API_SERVER: &str = "https://kubernetes.default.svc"; const DEFAULT_KUBERNETES_POLL_INTERVAL: Duration = Duration::from_secs(2); -const DEFAULT_NEBRASKA_POLL_INTERVAL: Duration = Duration::from_secs(5 * 60); +// TODO: placeholder until the real production Nebraska/Omaha endpoint is +// known. `.invalid` is reserved by RFC 2606 and is guaranteed to never +// resolve, so a deployment that forgets to override this fails loudly at +// the network layer instead of silently querying a real-looking but wrong +// host. +pub const DEFAULT_NEBRASKA_ENDPOINT: &str = "https://nebraska.example.invalid/v1/update"; const DEFAULT_STAGE_TIMEOUT: Duration = Duration::from_secs(20 * 60); const DEFAULT_FINALIZE_TIMEOUT: Duration = Duration::from_secs(10 * 60); pub const DEFAULT_STATE_PATH: &str = "/var/lib/trident-acl-agent/state.json"; @@ -63,15 +68,13 @@ impl AgentConfig { pub struct NebraskaConfig { pub endpoint: Option, pub app_id: String, - pub poll_interval: Duration, } impl Default for NebraskaConfig { fn default() -> Self { Self { - endpoint: None, + endpoint: Some(Url::parse(DEFAULT_NEBRASKA_ENDPOINT).expect("static url")), app_id: DEFAULT_NEBRASKA_APP_ID.to_string(), - poll_interval: DEFAULT_NEBRASKA_POLL_INTERVAL, } } } @@ -153,16 +156,14 @@ impl RawAgentConfig { fn into_effective(self) -> Result { Ok(AgentConfig { nebraska: NebraskaConfig { - endpoint: self.nebraska.endpoint, + endpoint: self + .nebraska + .endpoint + .or_else(|| Some(Url::parse(DEFAULT_NEBRASKA_ENDPOINT).expect("static url"))), app_id: self .nebraska .app_id .unwrap_or_else(|| DEFAULT_NEBRASKA_APP_ID.to_string()), - poll_interval: parse_duration( - self.nebraska.poll_interval.as_deref(), - DEFAULT_NEBRASKA_POLL_INTERVAL, - "nebraska.poll_interval", - )?, }, kubernetes: KubernetesConfig { api_server: self.kubernetes.api_server.unwrap_or_else(|| { @@ -212,7 +213,6 @@ impl RawAgentConfig { struct RawNebraskaConfig { endpoint: Option, app_id: Option, - poll_interval: Option, } #[derive(Debug, Default, Deserialize)] @@ -290,12 +290,11 @@ mod tests { #[test] fn parses_defaults() { let config = AgentConfig::from_toml("").unwrap(); - assert_eq!(config.nebraska.endpoint, None); - assert_eq!(config.nebraska.app_id, DEFAULT_NEBRASKA_APP_ID); assert_eq!( - config.nebraska.poll_interval, - DEFAULT_NEBRASKA_POLL_INTERVAL + config.nebraska.endpoint.unwrap().as_str(), + DEFAULT_NEBRASKA_ENDPOINT ); + assert_eq!(config.nebraska.app_id, DEFAULT_NEBRASKA_APP_ID); assert_eq!( config.kubernetes.api_server.as_str(), "https://kubernetes.default.svc/" @@ -321,9 +320,8 @@ mod tests { let config = AgentConfig::from_toml( r#" [nebraska] - endpoint = "https://nebraska.example.invalid/v1/update" + endpoint = "https://custom-nebraska.example.invalid/v1/update" app_id = "custom-app" - poll_interval = "7m" [kubernetes] api_server = "https://cluster.example.invalid" @@ -344,10 +342,9 @@ mod tests { assert_eq!( config.nebraska.endpoint.unwrap().as_str(), - "https://nebraska.example.invalid/v1/update" + "https://custom-nebraska.example.invalid/v1/update" ); assert_eq!(config.nebraska.app_id, "custom-app"); - assert_eq!(config.nebraska.poll_interval, Duration::from_secs(7 * 60)); assert_eq!( config.kubernetes.api_server.as_str(), "https://cluster.example.invalid/" From 60e66ec3c6cbd30458343b75e6a315e525816c8e Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Wed, 5 Aug 2026 22:10:42 +0000 Subject: [PATCH 5/6] trident-acl-agent: add nebraska.track config, default to annotation mode, rename Labels->Annotations Three related config changes: 1. Adds `nebraska.track` (defaults to DEFAULT_NEBRASKA_TRACK = "west-us") as a configurable field, threaded through run_omaha_only() and orchestrator.rs's handle_stage() instead of the hardcoded constant. 2. Swaps GoalSource's default from OmahaOnly to the annotation-driven orchestrator mode. The historical one-shot omaha-only behavior remains available as an explicit opt-out (`goal_source = "omaha-only"`), just no longer the shipping default. 3. Renames GoalSource::Labels -> GoalSource::Annotations (TOML value "labels" -> "annotations"). The old name was a holdover from an earlier design iteration that used Kubernetes labels before switching to annotations - flagged previously as a naming-drift risk since the wire protocol has used annotations for a while. Updated the handful of directly-coupled doc comments (main.rs, lib.rs's crate doc, trident.rs) that described this feature as "label protocol"/"label mode" for consistency; `k8s.rs`'s patch_node_labels (real Kubernetes label support, a separate feature) is untouched. Added doc comments on both GoalSource variants and on both match arms in main.rs explaining what each mode actually does. Verified: cargo test -p trident-acl-agent (78 passed, including new nebraska.track parsing coverage), cargo clippy --all-targets -- -D warnings (clean), cargo fmt --check (clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c06585d-a82d-475f-a802-83fdfa012d86 --- crates/trident-acl-agent/src/config.rs | 39 +++++++++++++++----- crates/trident-acl-agent/src/lib.rs | 10 +++-- crates/trident-acl-agent/src/main.rs | 17 ++++++--- crates/trident-acl-agent/src/orchestrator.rs | 5 ++- crates/trident-acl-agent/src/trident.rs | 10 ++--- 5 files changed, 54 insertions(+), 27 deletions(-) diff --git a/crates/trident-acl-agent/src/config.rs b/crates/trident-acl-agent/src/config.rs index ba157bcf20..b7ad8a4330 100644 --- a/crates/trident-acl-agent/src/config.rs +++ b/crates/trident-acl-agent/src/config.rs @@ -1,8 +1,8 @@ //! Config loading for Harpoon. //! -//! See the design doc's endpoint override section (§12). Label mode is opt-in -//! through config only; defaults intentionally preserve the historical -//! `omaha-only` one-shot behavior. +//! See the design doc's endpoint override section (§12). Annotation mode is +//! the default; `omaha-only` (the historical one-shot behavior) remains +//! available as an explicit opt-out via `goal_source = "omaha-only"`. use std::{ env, fs, @@ -14,7 +14,7 @@ use anyhow::Context; use serde::Deserialize; use url::Url; -use crate::DEFAULT_NEBRASKA_APP_ID; +use crate::{DEFAULT_NEBRASKA_APP_ID, DEFAULT_NEBRASKA_TRACK}; pub const DEFAULT_CONFIG_PATH: &str = "/etc/trident/trident-acl-agent.conf"; pub const DEFAULT_KUBERNETES_API_SERVER: &str = "https://kubernetes.default.svc"; @@ -68,6 +68,7 @@ impl AgentConfig { pub struct NebraskaConfig { pub endpoint: Option, pub app_id: String, + pub track: String, } impl Default for NebraskaConfig { @@ -75,6 +76,7 @@ impl Default for NebraskaConfig { Self { endpoint: Some(Url::parse(DEFAULT_NEBRASKA_ENDPOINT).expect("static url")), app_id: DEFAULT_NEBRASKA_APP_ID.to_string(), + track: DEFAULT_NEBRASKA_TRACK.to_string(), } } } @@ -114,9 +116,19 @@ impl Default for TridentConfig { #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize)] #[serde(rename_all = "kebab-case")] pub enum GoalSource { - #[default] + /// Historical one-shot behavior: query Nebraska/Omaha once, and if an + /// update is offered, call tridentd's combined `update()` RPC once and + /// exit. No Kubernetes involvement at all - no annotations, no watch, + /// no Node access. Kept as an explicit opt-out for nodes that don't + /// participate in the AKS annotation-driven update protocol. OmahaOnly, - Labels, + /// The annotation-driven reconcile loop: watches the Node's + /// `acl.azure.com/update-request` annotation and drives Trident's + /// stage/finalize/rollback/commit operations against tridentd + /// accordingly, writing progress back to `acl.azure.com/update-status` + /// (see docs/update-trigger-design.md). This is the default mode. + #[default] + Annotations, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -132,7 +144,7 @@ pub struct OrchestrationConfig { impl Default for OrchestrationConfig { fn default() -> Self { Self { - goal_source: GoalSource::OmahaOnly, + goal_source: GoalSource::Annotations, state_path: PathBuf::from(DEFAULT_STATE_PATH), stage_timeout: DEFAULT_STAGE_TIMEOUT, finalize_timeout: DEFAULT_FINALIZE_TIMEOUT, @@ -164,6 +176,10 @@ impl RawAgentConfig { .nebraska .app_id .unwrap_or_else(|| DEFAULT_NEBRASKA_APP_ID.to_string()), + track: self + .nebraska + .track + .unwrap_or_else(|| DEFAULT_NEBRASKA_TRACK.to_string()), }, kubernetes: KubernetesConfig { api_server: self.kubernetes.api_server.unwrap_or_else(|| { @@ -213,6 +229,7 @@ impl RawAgentConfig { struct RawNebraskaConfig { endpoint: Option, app_id: Option, + track: Option, } #[derive(Debug, Default, Deserialize)] @@ -303,7 +320,7 @@ mod tests { config.trident.socket, trident_proto::TRIDENT_DEFAULT_SOCKET_URI ); - assert_eq!(config.orchestration.goal_source, GoalSource::OmahaOnly); + assert_eq!(config.orchestration.goal_source, GoalSource::Annotations); assert_eq!( config.orchestration.state_path, PathBuf::from(DEFAULT_STATE_PATH) @@ -322,6 +339,7 @@ mod tests { [nebraska] endpoint = "https://custom-nebraska.example.invalid/v1/update" app_id = "custom-app" + track = "custom-track" [kubernetes] api_server = "https://cluster.example.invalid" @@ -332,7 +350,7 @@ mod tests { socket = "unix:///custom/trident.sock" [orchestration] - goal_source = "labels" + goal_source = "omaha-only" state_path = "/var/lib/trident-acl-agent/custom-state.json" stage_timeout = "21m" finalize_timeout = "11m" @@ -345,6 +363,7 @@ mod tests { "https://custom-nebraska.example.invalid/v1/update" ); assert_eq!(config.nebraska.app_id, "custom-app"); + assert_eq!(config.nebraska.track, "custom-track"); assert_eq!( config.kubernetes.api_server.as_str(), "https://cluster.example.invalid/" @@ -355,7 +374,7 @@ mod tests { ); assert_eq!(config.kubernetes.node_name, "node-42"); assert_eq!(config.trident.socket, "unix:///custom/trident.sock"); - assert_eq!(config.orchestration.goal_source, GoalSource::Labels); + assert_eq!(config.orchestration.goal_source, GoalSource::OmahaOnly); assert_eq!( config.orchestration.state_path, PathBuf::from("/var/lib/trident-acl-agent/custom-state.json") diff --git a/crates/trident-acl-agent/src/lib.rs b/crates/trident-acl-agent/src/lib.rs index 799d8bdf75..64cb212431 100644 --- a/crates/trident-acl-agent/src/lib.rs +++ b/crates/trident-acl-agent/src/lib.rs @@ -2,9 +2,10 @@ //! //! Harpoon is Trident's ACL update sidecar. Historically it was a one-shot //! Omaha client that called Trident's combined `Update()` RPC once and exited. -//! This crate now also supports the AKS label protocol described in the local -//! design doc (`aks-rp ↔ trident-acl-agent`, especially §3–§6 and §12–§13), -//! while preserving the original `omaha-only` mode as the default. +//! This crate now defaults to the AKS annotation protocol described in the +//! local design doc (`aks-rp ↔ trident-acl-agent`, especially §3–§6 and +//! §12–§13), while preserving the original `omaha-only` mode as an explicit +//! opt-out (see `config::GoalSource`). use anyhow::Context; use semver::Version; @@ -75,12 +76,13 @@ pub async fn run_omaha_only(config: &config::AgentConfig) -> Result<(), anyhow:: // runtime per call, which isn't safe to tear down from inside an // already-running async task. Run it on a dedicated blocking thread. let app_id = config.nebraska.app_id.clone(); + let track = config.nebraska.track.clone(); let endpoint_for_task = endpoint.clone(); let response = tokio::task::spawn_blocking(move || { query_for_update( &endpoint_for_task, &app_id, - DEFAULT_NEBRASKA_TRACK, + &track, &Version::new(0, 0, 0), IdSource::MachineIdHashed, ) diff --git a/crates/trident-acl-agent/src/main.rs b/crates/trident-acl-agent/src/main.rs index 039a4a763a..3c07aceecb 100644 --- a/crates/trident-acl-agent/src/main.rs +++ b/crates/trident-acl-agent/src/main.rs @@ -63,11 +63,11 @@ fn is_network_target(target: &str) -> bool { .any(|prefix| target == *prefix || target.starts_with(&format!("{prefix}::"))) } -/// Harpoon can either run its original one-shot Omaha flow or the new -/// label-driven orchestrator. Activation of label mode is intentionally gated by -/// config file only: shipping defaults stay on `omaha-only`, while a VM -/// extension or AgentBaker-dropped config is expected to opt a node into the -/// AKS label protocol. +/// Harpoon can either run the annotation-driven orchestrator (the default) +/// or fall back to its original one-shot Omaha flow. Mode selection is +/// config-file only (`[orchestration] goal_source`): shipping defaults +/// enable the AKS annotation protocol, while a VM extension or +/// AgentBaker-dropped config can opt a node out to `omaha-only` if needed. #[derive(Parser, Debug)] #[command(version, about, long_about = None)] struct Args { @@ -132,7 +132,12 @@ async fn main() -> Result<(), anyhow::Error> { let config = config.with_cli_endpoint(args.url.clone()); match config.orchestration.goal_source { + // Historical one-shot flow: query Nebraska once, apply an update if + // offered, and exit. No Kubernetes/annotation involvement. GoalSource::OmahaOnly => run_omaha_only(&config).await, - GoalSource::Labels => Orchestrator::from_config(config).await?.run().await, + // Default: the annotation-driven reconcile loop (watches + // acl.azure.com/update-request, drives stage/finalize/rollback/ + // commit against tridentd, writes acl.azure.com/update-status). + GoalSource::Annotations => Orchestrator::from_config(config).await?.run().await, } } diff --git a/crates/trident-acl-agent/src/orchestrator.rs b/crates/trident-acl-agent/src/orchestrator.rs index 1249e3483a..82183a9d1f 100644 --- a/crates/trident-acl-agent/src/orchestrator.rs +++ b/crates/trident-acl-agent/src/orchestrator.rs @@ -30,7 +30,7 @@ use crate::{ query_for_update, state::{PendingCommit, StateStore}, trident::{CompletedResponse, TridentClient, TridentClientError}, - IdSource, QueryResult, DEFAULT_NEBRASKA_TRACK, + IdSource, QueryResult, }; const FINAL_STATUS_PATCH_RETRIES: usize = 3; @@ -310,12 +310,13 @@ where // an already-running async task. Run it on a dedicated blocking // thread instead. let app_id = self.config.nebraska.app_id.clone(); + let track = self.config.nebraska.track.clone(); let endpoint_for_task = endpoint.clone(); let response = tokio::task::spawn_blocking(move || { query_for_update( &endpoint_for_task, &app_id, - DEFAULT_NEBRASKA_TRACK, + &track, &Version::new(0, 0, 0), IdSource::MachineIdHashed, ) diff --git a/crates/trident-acl-agent/src/trident.rs b/crates/trident-acl-agent/src/trident.rs index 79a0a38470..7992828a55 100644 --- a/crates/trident-acl-agent/src/trident.rs +++ b/crates/trident-acl-agent/src/trident.rs @@ -6,12 +6,12 @@ //! and the stage/finalize/rollback-finalize CallerHandlesReboot split in //! section 2.3). //! -//! The label protocol drives stage/finalize/commit directly against tridentd's -//! stable v1 API (§4–§5). Startup recovery no longer pre-queries the preview -//! `StatusService::GetServicingState`: commit() is self-checking (tridentd -//! only commits from a valid servicing_state and otherwise returns +//! The annotation protocol drives stage/finalize/commit directly against +//! tridentd's stable v1 API (§4–§5). Startup recovery no longer pre-queries +//! the preview `StatusService::GetServicingState`: commit() is self-checking +//! (tridentd only commits from a valid servicing_state and otherwise returns //! ServicingKind::NoneRequired as a harmless no-op), so the orchestrator -//! always calls commit() unconditionally and falls back to label-based +//! always calls commit() unconditionally and falls back to annotation-based //! progress for anything commit() reports nothing to do for. See //! orchestrator.rs's recover_from_trident_state for the full rationale. From fb484130944e97109f2eb1f73be6cfceed69df6e Mon Sep 17 00:00:00 2001 From: Brian Fjeldstad Date: Wed, 5 Aug 2026 23:37:22 +0000 Subject: [PATCH 6/6] fix Copilot-flagged issues on PR #730 - SystemRebooter now routes through osutils::dependencies::Dependency instead of raw process::Command, matching the rest of the codebase (crates/trident/src/reboot.rs uses the same pattern) and getting uniform actionable errors on a missing/failing systemctl - host_configuration_from_image now builds YAML via serde_yaml instead of format!, avoiding malformed/misinterpreted YAML if a URL or hash contains YAML-special characters - from_toml error message now names trident-acl-agent.conf instead of the generic "config.toml" - CURRENT_VERSION_STUB changed to a sentinel that cannot collide with a real AKS/Trident release version string, preventing a spurious AlreadyAtTarget short-circuit - StateStore::save now writes to a temp file and renames atomically instead of truncate-then-write, so a crash/power-loss around a real reboot cannot corrupt state.json - hyper-util moved to [workspace.dependencies] and referenced via workspace = true, matching repo convention Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c06585d-a82d-475f-a802-83fdfa012d86 --- Cargo.toml | 1 + crates/trident-acl-agent/Cargo.toml | 2 +- crates/trident-acl-agent/src/annotations.rs | 14 ++++---- crates/trident-acl-agent/src/config.rs | 2 +- crates/trident-acl-agent/src/orchestrator.rs | 29 +++++++-------- crates/trident-acl-agent/src/state.rs | 37 ++++++++++++++++---- crates/trident-acl-agent/src/trident.rs | 27 +++++++++++--- 7 files changed, 77 insertions(+), 35 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d2ac28ee96..15d0c73504 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,7 @@ hex = "0.4.0" hostname = "0.4.0" humantime = "2.3.0" hyper = "1.8.1" +hyper-util = "0.1" indoc = "2.0.5" inventory = "0.3.15" k8s-openapi = { version = "0.24.0", features = ["v1_32"] } diff --git a/crates/trident-acl-agent/Cargo.toml b/crates/trident-acl-agent/Cargo.toml index 1c40afa3f8..0755991b1a 100644 --- a/crates/trident-acl-agent/Cargo.toml +++ b/crates/trident-acl-agent/Cargo.toml @@ -51,4 +51,4 @@ tower = { workspace = true } # keeps it out of the production binary's dependency graph; it is only # pulled in for `cargo test`. trident-proto = { path = "../trident-proto", features = ["grpc-preview", "server"] } -hyper-util = { version = "0.1", features = ["tokio"] } +hyper-util = { workspace = true, features = ["tokio"] } diff --git a/crates/trident-acl-agent/src/annotations.rs b/crates/trident-acl-agent/src/annotations.rs index c02f0a7649..a273fea49e 100644 --- a/crates/trident-acl-agent/src/annotations.rs +++ b/crates/trident-acl-agent/src/annotations.rs @@ -23,12 +23,14 @@ pub const SCHEMA_VERSION: &str = "1.0"; // determined on-node because the OS image does not currently ship a required // file/manifest describing the running version. Once that file exists (planned // as part of the image build), replace CURRENT_VERSION_STUB and -// current_active_version() below with real logic that reads it. Until then, -// this stub can cause current_active_version() to spuriously equal a real -// requested target version, making handle_stage/handle_finalize incorrectly -// short-circuit to AlreadyAtTarget. Do not remove this comment when bumping the -// stub value; keep it until the real probe lands. -pub const CURRENT_VERSION_STUB: &str = "202601.1.0"; +// current_active_version() below with real logic that reads it. The stub +// value below is an explicit sentinel that cannot collide with a real +// AKS/Trident release version string (those look like "YYYYMM.N.N"), so it +// can never accidentally match a real requested target version and cause +// handle_stage/handle_finalize to incorrectly short-circuit to +// AlreadyAtTarget. Do not remove this comment when bumping the stub value; +// keep it (and its non-colliding shape) until the real probe lands. +pub const CURRENT_VERSION_STUB: &str = "0.0.0-unprobed-trident-acl-agent-stub"; #[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Hash)] #[serde(rename_all = "camelCase")] diff --git a/crates/trident-acl-agent/src/config.rs b/crates/trident-acl-agent/src/config.rs index b7ad8a4330..3a34634c5a 100644 --- a/crates/trident-acl-agent/src/config.rs +++ b/crates/trident-acl-agent/src/config.rs @@ -52,7 +52,7 @@ impl AgentConfig { pub fn from_toml(contents: &str) -> Result { let raw: RawAgentConfig = - toml::from_str(contents).context("failed to parse config.toml")?; + toml::from_str(contents).context("failed to parse trident-acl-agent.conf")?; raw.into_effective() } diff --git a/crates/trident-acl-agent/src/orchestrator.rs b/crates/trident-acl-agent/src/orchestrator.rs index 82183a9d1f..8f87b5d54c 100644 --- a/crates/trident-acl-agent/src/orchestrator.rs +++ b/crates/trident-acl-agent/src/orchestrator.rs @@ -9,7 +9,7 @@ //! full state-machine rationale; keep it in sync with this file if the //! design changes. -use std::{collections::BTreeMap, process::Command}; +use std::collections::BTreeMap; use anyhow::Context; use chrono::{DateTime, Utc}; @@ -19,6 +19,8 @@ use semver::Version; use trident_proto::v1::{RebootStatus, ServicingKind}; use uuid::Uuid; +use osutils::dependencies::Dependency; + use crate::{ annotations::{ commit_operation_id, current_active_version, Operation, RequestedOperation, StatusCode, @@ -45,22 +47,15 @@ pub trait RebootHandle: Clone + Send + Sync + 'static { impl RebootHandle for SystemRebooter { fn reboot(&self) -> Result<(), anyhow::Error> { - for candidate in [ - ("reboot", Vec::<&str>::new()), - ("systemctl", vec!["reboot"]), - ] { - match Command::new(candidate.0) - .args(candidate.1.iter().copied()) - .status() - { - Ok(status) if status.success() => return Ok(()), - Ok(status) => log::warn!("{} exited with {}", candidate.0, status), - Err(err) => log::warn!("failed to invoke {}: {err}", candidate.0), - } - } - Err(anyhow::anyhow!( - "failed to issue reboot via reboot or systemctl reboot" - )) + // Route through the repo's centralized dependency runner so a + // missing systemctl binary or non-zero exit produces the same + // uniform, actionable error type used everywhere else in the + // codebase (see crates/trident/src/reboot.rs for the same pattern). + Dependency::Systemctl + .cmd() + .arg("reboot") + .run_and_check() + .map_err(|err| anyhow::anyhow!("failed to issue systemctl reboot: {err}")) } } diff --git a/crates/trident-acl-agent/src/state.rs b/crates/trident-acl-agent/src/state.rs index 485a410b7f..e7926ccf4d 100644 --- a/crates/trident-acl-agent/src/state.rs +++ b/crates/trident-acl-agent/src/state.rs @@ -67,12 +67,37 @@ impl StateStore { } pub fn save(&self, state: &PersistentState) -> Result<(), anyhow::Error> { - if let Some(parent) = self.path.parent() { - fs::create_dir_all(parent) - .with_context(|| format!("failed to create {}", parent.display()))?; - } - fs::write(&self.path, serde_json::to_string_pretty(state)?) - .with_context(|| format!("failed to write {}", self.path.display())) + let parent = match self.path.parent() { + Some(parent) => { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + parent + } + None => Path::new("."), + }; + + // Write to a temp file in the same directory and rename over the + // real path, so a crash or power loss mid-write (plausible here, + // since this file is written right around a real reboot) can't + // leave state.json truncated/corrupted -- rename is atomic on the + // same filesystem, unlike a direct truncate-then-write. + let temp_path = parent.join(format!( + "{}.tmp-{}", + self.path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("state.json"), + std::process::id() + )); + fs::write(&temp_path, serde_json::to_string_pretty(state)?) + .with_context(|| format!("failed to write {}", temp_path.display()))?; + fs::rename(&temp_path, &self.path).with_context(|| { + format!( + "failed to atomically replace {} with {}", + self.path.display(), + temp_path.display() + ) + }) } pub fn remember_completed(&self, status: UpdateStatus) -> Result<(), anyhow::Error> { diff --git a/crates/trident-acl-agent/src/trident.rs b/crates/trident-acl-agent/src/trident.rs index 7992828a55..1d3ad0b364 100644 --- a/crates/trident-acl-agent/src/trident.rs +++ b/crates/trident-acl-agent/src/trident.rs @@ -303,12 +303,31 @@ impl TridentClient { } } +#[derive(serde::Serialize)] +struct ImageSpec<'a> { + url: &'a str, + sha384: &'a str, +} + +#[derive(serde::Serialize)] +struct HostConfigurationYaml<'a> { + image: ImageSpec<'a>, +} + pub fn host_configuration_from_image(url: &Url, hash: Option<&str>) -> HostConfiguration { - HostConfiguration { - config: match hash { - Some(hash) => format!("image:\n url: {url}\n sha384: {hash}"), - None => format!("image:\n url: {url}\n sha384: ignored"), + // Build via serde_yaml rather than raw string formatting so a URL or + // hash containing YAML-special characters (e.g. ':' or '#') can't + // produce invalid YAML or silently change the parsed structure fed to + // tridentd as configuration. + let spec = HostConfigurationYaml { + image: ImageSpec { + url: url.as_str(), + sha384: hash.unwrap_or("ignored"), }, + }; + HostConfiguration { + config: serde_yaml::to_string(&spec) + .expect("serializing a simple struct to YAML cannot fail"), } }