acl: add tridentVersion to the update-status annotation

crates/trident-acl-agent/src/annotations/orchestrator.rs

@@ -234,6 +234,7 @@ impl Orchestrator {
                     trident_error: None,
                     from_version: None,
                     to_version: None,
+                    trident_version: Some(crate::AGENT_VERSION.to_string()),
                     started_utc: now,
                     last_updated_utc: now,
                     finished_utc: Some(now),

crates/trident-acl-agent/src/annotations/protocol.rs

@@ -19,8 +19,9 @@ use serde::{Deserialize, Serialize};
 use url::Url;
 use uuid::Uuid;
 
-use crate::core::{
-    config::DEFAULT_ANNOTATION_PREFIX, error::AgentError, trident::TridentClientError,
+use crate::{
+    core::{config::DEFAULT_ANNOTATION_PREFIX, error::AgentError, trident::TridentClientError},
+    AGENT_VERSION,
 };
 
 /// Sentinel `kind` used when Trident reported a remote failure without any
@@ -150,6 +151,13 @@ pub struct UpdateStatus {
     pub from_version: Option<String>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub to_version: Option<String>,
+    /// Version of trident-acl-agent that wrote this status (`AGENT_VERSION`).
+    /// Packaged and versioned together with tridentd via the same RPM build
+    /// (see `packaging/rpm/trident.spec`'s single `TRIDENT_VERSION`-stamped
+    /// `%build` step for both `-p trident` and `-p trident-acl-agent`), so
+    /// this value doubles as the tridentd version for that install.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub trident_version: Option<String>,
     pub started_utc: DateTime<Utc>,
     pub last_updated_utc: DateTime<Utc>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -271,16 +279,27 @@ impl UpdateStatus {
             trident_error: None,
             from_version,
             to_version,
+            trident_version: Some(AGENT_VERSION.to_string()),
             started_utc,
             last_updated_utc: finished_or_started,
             finished_utc,
         }
     }
 
+    /// Refreshes a status immediately before it is written to the node
+    /// (the common path for every annotation write - see `publish_status`).
+    /// Restamps `trident_version` with the currently-running agent's
+    /// `AGENT_VERSION` rather than trusting whatever value the status
+    /// already carries: a completed status can be loaded from
+    /// `state.json` (written by a possibly older agent) and republished
+    /// later by a newer agent (e.g. `recover_from_trident_state` replaying
+    /// a cached commit/operation status), so only re-stamping here
+    /// guarantees the annotation reflects the agent that actually wrote it.
     pub fn refreshed_for_write(&self) -> Self {
         let mut refreshed = self.clone();
         refreshed.last_updated_utc = Utc::now();
         refreshed.message = truncate_message(refreshed.message);
+        refreshed.trident_version = Some(AGENT_VERSION.to_string());
         refreshed
     }
 
@@ -307,16 +326,22 @@ impl UpdateStatus {
         self
     }
 
-    /// Compares two statuses ignoring `last_updated_utc`.
+    /// Compares two statuses ignoring `last_updated_utc` and
+    /// `trident_version`.
     ///
     /// `publish_status` stamps a fresh `last_updated_utc` on every write via
     /// `refreshed_for_write`, so a straight `PartialEq` between an
     /// already-on-the-node status and a cached/completed one to decide
     /// whether a re-publish is needed would never be equal after the first
     /// publish - triggering another watch event, another "different"
-    /// comparison, and another publish, forever. Callers that only care
-    /// whether the *content* already matches (and so a re-publish would be a
-    /// no-op) must use this instead of `==`/`!=`.
+    /// comparison, and another publish, forever. `trident_version` is
+    /// excluded for the same reason: `refreshed_for_write` also
+    /// unconditionally restamps it with the currently-running agent's
+    /// `AGENT_VERSION`, so after an upgrade a cached status carrying the old
+    /// version would otherwise never compare equal to the just-republished
+    /// one, reintroducing the same infinite re-publish loop. Callers that
+    /// only care whether the *content* already matches (and so a re-publish
+    /// would be a no-op) must use this instead of `==`/`!=`.
     pub fn same_content(&self, other: &Self) -> bool {
         self.schema_version == other.schema_version
             && self.node_update_id == other.node_update_id
@@ -441,6 +466,39 @@ mod tests {
         );
     }
 
+    #[test]
+    fn refreshed_for_write_restamps_trident_version() {
+        // Regression test: a completed status can be loaded from
+        // state.json (written by a possibly older agent) and republished
+        // later by a newer agent (recover_from_trident_state replaying a
+        // cached commit/operation status via publish_status). Verify
+        // refreshed_for_write - the common path for every annotation write
+        // - always overwrites trident_version with the currently-running
+        // AGENT_VERSION, rather than preserving whatever an older/stale
+        // status carries.
+        let request = sample_request(RequestedOperation::Finalize);
+        let mut stale = UpdateStatus::new(
+            &request,
+            Operation::Finalize,
+            request.operation_id.clone(),
+            StatusCode::Success,
+            "finalize completed",
+            Some("1.0.0".to_string()),
+            Some("2.0.0".to_string()),
+            fixed_time(0),
+            Some(fixed_time(5)),
+        );
+        stale.trident_version = Some("0.0.1-old-agent".to_string());
+
+        let republished = stale.refreshed_for_write();
+
+        assert_eq!(
+            republished.trident_version.as_deref(),
+            Some(AGENT_VERSION),
+            "refreshed_for_write must restamp trident_version with the current agent's AGENT_VERSION"
+        );
+    }
+
     #[test]
     fn same_content_detects_real_differences() {
         let request = sample_request(RequestedOperation::Finalize);
@@ -470,6 +528,34 @@ mod tests {
         assert!(!success.same_content(&failed));
     }
 
+    #[test]
+    fn same_content_ignores_trident_version() {
+        // Regression test: refreshed_for_write() always restamps
+        // trident_version with the currently-running agent's AGENT_VERSION
+        // (see refreshed_for_write_restamps_trident_version). If
+        // same_content compared trident_version, a cached status from
+        // before an agent upgrade would never again compare equal to the
+        // freshly-republished one, reintroducing the infinite re-publish
+        // loop same_content exists to prevent.
+        let request = sample_request(RequestedOperation::Finalize);
+        let mut cached = UpdateStatus::new(
+            &request,
+            Operation::Finalize,
+            request.operation_id.clone(),
+            StatusCode::Success,
+            "finalize completed",
+            Some("1.0.0".to_string()),
+            Some("2.0.0".to_string()),
+            fixed_time(0),
+            Some(fixed_time(5)),
+        );
+        cached.trident_version = Some("0.0.1-old-agent".to_string());
+        let republished = cached.refreshed_for_write();
+
+        assert_ne!(cached.trident_version, republished.trident_version);
+        assert!(cached.same_content(&republished));
+    }
+
     #[test]
     fn truncates_messages_longer_than_2048_bytes() {
         let request = sample_request(RequestedOperation::Stage);
@@ -525,13 +611,32 @@ mod tests {
         assert_eq!(json["message"], "stage completed");
         assert_eq!(json["fromVersion"], "1.0.0");
         assert_eq!(json["toVersion"], "2.0.0");
+        assert_eq!(json["tridentVersion"], AGENT_VERSION);
         assert!(json.get("startedUtc").is_some());
         assert!(json.get("lastUpdatedUtc").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 new_always_populates_trident_version_with_agent_version() {
+        let request = sample_request(RequestedOperation::Stage);
+        let status = UpdateStatus::new(
+            &request,
+            Operation::Stage,
+            request.operation_id.clone(),
+            StatusCode::Success,
+            "stage completed",
+            None,
+            None,
+            fixed_time(0),
+            Some(fixed_time(5)),
+        );
+
+        assert_eq!(status.trident_version.as_deref(), Some(AGENT_VERSION));
+    }
+
     #[test]
     fn stage_failure_annotation_has_operation_failed_code() {
         let request = sample_request(RequestedOperation::Stage);
@@ -839,6 +944,7 @@ mod tests {
     },
     "fromVersion":   { "type": "string" },
     "toVersion":     { "type": "string" },
+    "tridentVersion": { "type": "string", "description": "Version of trident-acl-agent that wrote this status. Packaged and released together with tridentd, so this is also the tridentd version." },
     "startedUtc":    { "type": "string", "format": "date-time" },
     "lastUpdatedUtc": { "type": "string", "format": "date-time", "description": "When the agent last wrote this status. The agent refreshes it on every write, including a periodic InProgress heartbeat, so AKS-RP and the watchdog can tell a working agent from a stuck one." },
     "finishedUtc":   { "type": "string", "format": "date-time" }

crates/trident-acl-agent/src/annotations/state.rs

@@ -185,8 +185,8 @@ impl StateStore {
                 .entry(status.operation_id.clone())
                 .or_default();
             match status.operation {
-                Operation::Commit => entry.commit = Some(status.clone()),
-                _ => entry.operation = Some(status.clone()),
+                Operation::Commit => entry.commit = Some(strip_trident_version(&status)),
+                _ => entry.operation = Some(strip_trident_version(&status)),
             }
         })
     }
@@ -215,14 +215,27 @@ impl StateStore {
                 .entry(status.operation_id.clone())
                 .or_default();
             match status.operation {
-                Operation::Commit => entry.commit = Some(status.clone()),
-                _ => entry.operation = Some(status.clone()),
+                Operation::Commit => entry.commit = Some(strip_trident_version(&status)),
+                _ => entry.operation = Some(strip_trident_version(&status)),
             }
             state.pending_commit = None;
         })
     }
 }
 
+// `UpdateStatus::trident_version` is stamped on every annotation publish
+// (see `refreshed_for_write`), so the persisted copy is never read back as
+// a trusted value - only ever overwritten before republish. Keeping it out
+// of `state.json` entirely avoids a cross-version rollback hazard: an
+// older trident-acl-agent binary (from before this field existed) still
+// derives `UpdateStatus` with `deny_unknown_fields`, so it would fail to
+// parse a state file containing an unrecognized `tridentVersion` key.
+fn strip_trident_version(status: &UpdateStatus) -> UpdateStatus {
+    let mut persisted = status.clone();
+    persisted.trident_version = None;
+    persisted
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -359,6 +372,33 @@ mod tests {
         assert!(entry.commit.is_none());
     }
 
+    #[test]
+    fn remember_completed_strips_trident_version_before_persisting() {
+        // Regression test: UpdateStatus is embedded directly in
+        // CompletedEntry with deny_unknown_fields, so persisting
+        // trident_version to state.json would break a pre-this-field
+        // trident-acl-agent binary reading the file back after an A/B
+        // rollback. The persisted value is never trusted anyway -
+        // refreshed_for_write always restamps it before republish - so it
+        // must never round-trip through disk.
+        let (_dir, store) = store();
+        let mut status = sample_status(Operation::Finalize);
+        status.trident_version = Some("1.2.3".to_string());
+        store
+            .remember_completed(status)
+            .expect("remember_completed should succeed");
+
+        let raw = fs::read_to_string(store.path()).expect("state.json should exist");
+        assert!(
+            !raw.contains("tridentVersion"),
+            "state.json must not persist tridentVersion: {raw}"
+        );
+
+        let state = store.load().expect("load should succeed");
+        let entry = state.completed.get("op-1").expect("entry should exist");
+        assert_eq!(entry.operation.as_ref().unwrap().trident_version, None);
+    }
+
     #[test]
     fn set_and_clear_pending_commit_round_trip() {
         let (_dir, store) = store();

docs/Explanation/Trident-ACL-Agent.md

@@ -164,6 +164,7 @@ A status annotation (`<prefix>/update-status` or
   "message":         "staged update to 202606.29.0",
   "fromVersion":     "202606.15.0",
   "toVersion":       "202606.29.0",
+  "tridentVersion":  "0.22.0",
   "startedUtc":      "2026-06-29T12:00:00Z",
   "lastUpdatedUtc":  "2026-06-29T12:03:41Z",
   "finishedUtc":     "2026-06-29T12:03:41Z"
@@ -190,6 +191,11 @@ A status annotation (`<prefix>/update-status` or
   Trident reported failure without a structured error payload) and an
   optional `location` (`path`/`line` in Trident's source), letting an
   orchestrator key off structured fields instead of parsing `message`.
+- `tridentVersion` is the agent's own build version (`AGENT_VERSION`),
+  stamped on every status write. `tridentd` and `trident-acl-agent` are
+  built and released together from a single RPM spec
+  (`packaging/rpm/trident.spec`), so this value doubles as the `tridentd`
+  version for that install.
 - `startedUtc`/`lastUpdatedUtc`/`finishedUtc` bound the operation:
   `lastUpdatedUtc` refreshes on a heartbeat cadence while `code` is
   `InProgress` (see [below](#pre-post-reboot-state-and-the-watchdog));