Skip to content

Commit 0dc1471

Browse files
committed
feat(fleet): redesign gateway handshake around agent_version
Vendored proto synced with platform PR #112: EnrollRequest carries agent_version (dropping protocol_version), EnrollResponse is a oneof (Enrolled | AgentUpdate). AttachRequest carries a first-message Attach { agent_version, capabilities, host_info_json }; AttachResponse gains AttachAccepted, AttachRejected(AgentUpdate), and HeartbeatAck (rename of Keepalive). Heartbeat trims to just telemetry — the only field that actually changes over time; capabilities and host_info are per-process constants and belong on the Attach handshake. connect_and_serve now sends the Attach message as its first outbound (still speaks-first for PLAT-34) with the full declarative payload, waits for AttachAccepted, and only then starts the heartbeat loop. AttachRejected surfaces as an AgentUpdateRequired error that the reconnect loop parks on (analogous to the RUN-40 credential-rejection park). enroll() branches on EnrollResponse.result and surfaces the expected version on rejection instead of a stringly-typed error. Deletes the x-arcbox-protocol-version metadata header and the PROTOCOL_VERSION const on both sides of the wire.
1 parent cef3a22 commit 0dc1471

6 files changed

Lines changed: 237 additions & 78 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

fleet/arcbox-fleet-agent/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ arcbox-grpc.workspace = true
4242
arcbox-protocol.workspace = true
4343
arcbox-constants.workspace = true
4444
russh = { version = "0.62.1", default-features = false, features = ["ring", "flate2"] }
45+
thiserror.workspace = true
4546

4647
[lints]
4748
workspace = true

fleet/arcbox-fleet-agent/src/attach.rs

Lines changed: 122 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use anyhow::{Context, Result};
1212
use arcbox_fleet_control_proto::v1 as control_proto;
1313
use arcbox_fleet_proto::v1::fleet_gateway_service_client::FleetGatewayServiceClient;
1414
use arcbox_fleet_proto::v1::{
15-
AttachRequest, Capability, Heartbeat, HostTelemetry, attach_request, attach_response,
15+
Attach, AttachRequest, Capability, Heartbeat, HostTelemetry, attach_request, attach_response,
1616
};
1717
use tokio::sync::mpsc;
1818
use tokio_stream::wrappers::ReceiverStream;
@@ -22,7 +22,7 @@ use tonic::Request;
2222
use tonic::metadata::MetadataValue;
2323
use tracing::{info, warn};
2424

25-
use crate::config::{AgentConfig, PROTOCOL_VERSION};
25+
use crate::config::AgentConfig;
2626
use crate::credentials::Credential;
2727
use crate::docker;
2828
use crate::host;
@@ -38,32 +38,40 @@ const INITIAL_BACKOFF: Duration = Duration::from_secs(1);
3838
const MAX_BACKOFF: Duration = Duration::from_secs(60);
3939
const OUTBOUND_CAPACITY: usize = 64;
4040
const MACHINE_TOKEN_HEADER: &str = "x-arcbox-machine-token";
41-
const PROTOCOL_VERSION_HEADER: &str = "x-arcbox-protocol-version";
4241
/// How long to wait for runners to be torn down and reaped on shutdown before
4342
/// giving up. Killing a process group or container is near-instant, so this is a
4443
/// generous ceiling rather than an expected wait.
4544
const SHUTDOWN_GRACE: Duration = Duration::from_secs(15);
4645

47-
/// Wrap `message` with the machine-credential and protocol-version metadata
48-
/// every authenticated gateway RPC carries (`Attach` here,
49-
/// `enroll::unenroll`'s `Unenroll`).
46+
/// Wrap `message` with the machine-credential metadata every authenticated
47+
/// gateway RPC carries (`Attach` here, `enroll::unenroll`'s `Unenroll`). The
48+
/// agent build identity travels as the first in-band `Attach` message rather
49+
/// than as a metadata header; see [`connect_and_serve`].
5050
pub fn authenticated_request<T>(message: T, machine_token: &str) -> Result<Request<T>> {
5151
let mut request = Request::new(message);
5252
let token: MetadataValue<_> = machine_token
5353
.parse()
5454
.context("machine token is not a valid metadata value")?;
5555
request.metadata_mut().insert(MACHINE_TOKEN_HEADER, token);
56-
// Protocol-version handshake: the gateway rejects an unsupported version.
57-
let version: MetadataValue<_> = PROTOCOL_VERSION
58-
.to_string()
59-
.parse()
60-
.expect("protocol version is a valid metadata value");
61-
request
62-
.metadata_mut()
63-
.insert(PROTOCOL_VERSION_HEADER, version);
6456
Ok(request)
6557
}
6658

59+
/// Terminal error returned by the attach loop when the gateway refuses the
60+
/// build. The reconnect loop matches on this to park visibly (analogous to
61+
/// [`is_unauthenticated`]) instead of retrying — a different binary is what's
62+
/// required, not another connection attempt.
63+
#[derive(Debug, thiserror::Error)]
64+
#[error("gateway refused agent build; expected version {expected_version}")]
65+
struct AgentUpdateRequired {
66+
expected_version: String,
67+
}
68+
69+
fn as_agent_update_required(error: &anyhow::Error) -> Option<&AgentUpdateRequired> {
70+
error
71+
.chain()
72+
.find_map(|cause| cause.downcast_ref::<AgentUpdateRequired>())
73+
}
74+
6775
/// Whether the error chain contains an `UNAUTHENTICATED` gRPC status — the
6876
/// gateway's definitive "this credential is revoked" (a RUN-40
6977
/// decommission), surfaced either by the Attach handshake or as a
@@ -190,6 +198,29 @@ pub async fn run(
190198
shutdown.cancelled().await;
191199
break;
192200
}
201+
Err(e) if as_agent_update_required(&e).is_some() => {
202+
// The gateway pins a specific agent build and this binary
203+
// isn't it. Retrying will keep hitting the same rejection;
204+
// park until the operator installs the expected binary.
205+
// Reuses `CredentialRejected` for the parked-state signal
206+
// because it's the closest existing variant — a proper
207+
// "update required" state can land alongside push-update.
208+
let expected = as_agent_update_required(&e)
209+
.expect("just matched above")
210+
.expected_version
211+
.clone();
212+
warn!(
213+
expected = %expected,
214+
current = env!("CARGO_PKG_VERSION"),
215+
"gateway refused the agent build; parked until the expected binary is installed"
216+
);
217+
state.set_enrollment(
218+
control_proto::Enrollment::CredentialRejected,
219+
&credential.machine_id,
220+
);
221+
shutdown.cancelled().await;
222+
break;
223+
}
193224
Err(e) => {
194225
warn!(error = %e, backoff_secs = backoff.as_secs(), "attach failed; retrying");
195226
}
@@ -248,18 +279,27 @@ async fn connect_and_serve(
248279
let gateway = state.gateway_target();
249280
let (req_tx, req_rx) = mpsc::channel::<AttachRequest>(OUTBOUND_CAPACITY);
250281

251-
// Start heartbeating before the request is even sent, not after the
252-
// response arrives. The gateway acks each heartbeat on the response
253-
// stream (its only keepalive signal for a proxy/load balancer sitting in
254-
// front), so if the client waits for a response before saying anything,
255-
// neither side ever sends a byte: the connection sits fully idle until
256-
// an intermediary's own idle-connection timeout (AWS ALB defaults to
257-
// 60s) resets it, and the response headers the origin sent immediately
258-
// are only ever delivered bundled with that reset. Speaking first breaks
259-
// the standoff.
260-
// Held only for its abort-on-drop side effect: it must outlive every exit
261-
// path of this function, not be read.
262-
let _heartbeat = spawn_heartbeat(req_tx.clone(), capabilities.to_vec(), state.clone());
282+
// Send `Attach` as the very first outbound message, buffered on `req_tx`
283+
// before the gRPC call runs. Three reasons: it's the handshake message
284+
// the gateway checks agent_version against; it declares the capabilities
285+
// and host facts that are constant for this process's lifetime (so
286+
// Heartbeat can carry only what actually changes); and it satisfies the
287+
// PLAT-34 "speak first" property — the gateway (or a proxy in front)
288+
// will not release its response headers until it sees the client say
289+
// something, and if the client waits for the response before sending
290+
// anything the whole stream idle-times-out. The mpsc buffer holds the
291+
// message until tonic drains it once the stream opens.
292+
let attach_msg = AttachRequest {
293+
msg: Some(attach_request::Msg::Attach(Attach {
294+
agent_version: env!("CARGO_PKG_VERSION").to_owned(),
295+
capabilities: capabilities.to_vec(),
296+
host_info_json: host::host_info_json(),
297+
})),
298+
};
299+
req_tx
300+
.send(attach_msg)
301+
.await
302+
.context("outbound stream closed before Attach could be buffered")?;
263303

264304
// The connect + Attach-RPC handshake can block indefinitely — no connect
265305
// timeout is configured on the tonic `Endpoint` — and, unlike the
@@ -287,13 +327,36 @@ async fn connect_and_serve(
287327
.context("Attach RPC failed")
288328
.map(|response| response.into_inner())
289329
};
290-
// `_heartbeat` (an `AbortOnDropHandle`) is aborted automatically on every
291-
// exit from this function, including `?` above and the early return here.
292330
let mut inbound = tokio::select! {
293331
biased;
294332
() = shutdown.cancelled() => return Ok(()),
295333
result = connect => result?,
296334
};
335+
336+
// First inbound message is the handshake result: `AttachAccepted` and we
337+
// proceed, or `AttachRejected(AgentUpdate)` and we park the reconnect
338+
// loop by returning [`AgentUpdateRequired`].
339+
let handshake = inbound
340+
.message()
341+
.await
342+
.context("Attach handshake response failed")?
343+
.ok_or_else(|| anyhow::anyhow!("attach stream closed before handshake response"))?;
344+
match handshake.msg {
345+
Some(attach_response::Msg::AttachAccepted(_)) => {}
346+
Some(attach_response::Msg::AttachRejected(update)) => {
347+
return Err(anyhow::Error::new(AgentUpdateRequired {
348+
expected_version: update.expected_version,
349+
}));
350+
}
351+
other => anyhow::bail!("unexpected first response from gateway: {other:?}"),
352+
}
353+
354+
// Only start heartbeating once the handshake succeeded; a rejected
355+
// machine has no business pushing telemetry, and the mpsc would fill
356+
// with heartbeats a closing stream can never deliver. Held only for its
357+
// abort-on-drop side effect: it must outlive every exit path below.
358+
let _heartbeat = spawn_heartbeat(req_tx.clone(), state.clone());
359+
297360
*backoff = INITIAL_BACKOFF;
298361
state.set_enrollment(control_proto::Enrollment::Attached, &credential.machine_id);
299362
state.set_gateway_current(&gateway);
@@ -336,7 +399,9 @@ async fn connect_and_serve(
336399

337400
/// Route one inbound `AttachResponse` to the supervisor. Synchronous: every
338401
/// handler only mutates supervisor state and spawns work, so dispatch can never
339-
/// stall the stream loop.
402+
/// stall the stream loop. The handshake variants (`AttachAccepted`/
403+
/// `AttachRejected`) are consumed by [`connect_and_serve`] before this runs;
404+
/// seeing either mid-stream is a gateway bug, logged and ignored.
340405
fn dispatch(supervisor: &RunnerSupervisor, msg: Option<attach_response::Msg>) {
341406
match msg {
342407
Some(attach_response::Msg::ProvisionRunner(order)) => {
@@ -347,11 +412,17 @@ fn dispatch(supervisor: &RunnerSupervisor, msg: Option<attach_response::Msg>) {
347412
Some(attach_response::Msg::OfferVerdictAck(ack)) => {
348413
supervisor.handle_ack(&ack.offer_token);
349414
}
350-
// No-op the gateway acks each heartbeat with, purely so a proxy in
351-
// front of the gateway (e.g. Cloudflare) sees server->client traffic
352-
// and never idle-times-out the stream.
353-
Some(attach_response::Msg::Keepalive(_)) => {
354-
tracing::debug!("received keepalive from gateway");
415+
// Each heartbeat gets an ack, whose sole purpose is to keep
416+
// server->client traffic flowing so a proxy in front of the gateway
417+
// (e.g. Cloudflare) doesn't idle-cut the stream (PLAT-34).
418+
Some(attach_response::Msg::HeartbeatAck(_)) => {
419+
tracing::debug!("received heartbeat ack from gateway");
420+
}
421+
Some(attach_response::Msg::AttachAccepted(_)) => {
422+
warn!("unexpected AttachAccepted after handshake; ignoring");
423+
}
424+
Some(attach_response::Msg::AttachRejected(_)) => {
425+
warn!("unexpected AttachRejected after handshake; ignoring");
355426
}
356427
None => {}
357428
}
@@ -382,15 +453,15 @@ fn spawn_verdict_resend(
382453
})
383454
}
384455

385-
/// Periodically push a declarative capability + telemetry heartbeat until the
386-
/// channel closes.
456+
/// Periodically push a live telemetry pulse until the channel closes.
387457
///
388458
/// Heartbeats are connection-scoped: the task is spawned per connection and
389459
/// aborted when it drops, so a momentary disconnect does not leave stale
390-
/// heartbeats queued for the next stream.
460+
/// heartbeats queued for the next stream. Capability set and host facts are
461+
/// declared once per stream in the `Attach` handshake (they're constant for
462+
/// the process lifetime), so this loop carries only what actually changes.
391463
fn spawn_heartbeat(
392464
outbound: mpsc::Sender<AttachRequest>,
393-
capabilities: Vec<Capability>,
394465
state: AgentState,
395466
) -> AbortOnDropHandle<()> {
396467
AbortOnDropHandle::new(tokio::spawn(async move {
@@ -400,8 +471,6 @@ fn spawn_heartbeat(
400471
let telemetry = host::telemetry();
401472
state.set_telemetry(telemetry_to_control(&telemetry));
402473
let msg = attach_request::Msg::Heartbeat(Heartbeat {
403-
capabilities: capabilities.clone(),
404-
host_info_json: host::host_info_json(),
405474
telemetry: Some(telemetry),
406475
});
407476
if outbound
@@ -582,9 +651,19 @@ mod tests {
582651
.await
583652
.map_err(|e| tonic::Status::internal(e.to_string()))?
584653
.ok_or_else(|| tonic::Status::internal("closed before any message arrived"))?;
585-
// Empty, immediately-closed response: this test only cares that
586-
// headers were unblocked, not about dispatch.
587-
let (_tx, rx) = mpsc::channel(1);
654+
// Send AttachAccepted then immediately close — under the redesigned
655+
// handshake, the client waits for this before proceeding. Without
656+
// it the client would treat the empty stream as a handshake
657+
// failure. This test only cares that response headers were
658+
// unblocked once the client spoke first, not about dispatch.
659+
let (tx, rx) = mpsc::channel(1);
660+
let _ = tx
661+
.send(Ok(arcbox_fleet_proto::v1::AttachResponse {
662+
msg: Some(attach_response::Msg::AttachAccepted(
663+
arcbox_fleet_proto::v1::AttachAccepted {},
664+
)),
665+
}))
666+
.await;
588667
Ok(tonic::Response::new(
589668
tokio_stream::wrappers::ReceiverStream::new(rx),
590669
))

fleet/arcbox-fleet-agent/src/config.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ const DEFAULT_LINUX_RUNNER_IMAGE: &str = "ghcr.io/actions/actions-runner:latest"
3333
/// Published macOS base-image stream with the Actions runner baked in.
3434
pub const DEFAULT_MACOS_RUNNER_IMAGE: &str = "tahoe-base";
3535

36-
/// Wire-protocol version this agent speaks; the gateway rejects a mismatch.
37-
pub const PROTOCOL_VERSION: u32 = 1;
38-
3936
/// Whether Docker-based Linux job execution is enabled.
4037
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
4138
pub enum DockerMode {

fleet/arcbox-fleet-agent/src/enroll.rs

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
55
use anyhow::{Context, Result};
66
use arcbox_fleet_proto::v1::fleet_gateway_service_client::FleetGatewayServiceClient;
7-
use arcbox_fleet_proto::v1::{Capability, EnrollRequest, UnenrollRequest};
7+
use arcbox_fleet_proto::v1::{Capability, EnrollRequest, UnenrollRequest, enroll_response};
88
use tracing::info;
99

1010
use crate::attach::authenticated_request;
11-
use crate::config::{AgentConfig, PROTOCOL_VERSION};
11+
use crate::config::AgentConfig;
1212
use crate::credentials::Credential;
1313
use crate::host;
1414

@@ -21,6 +21,10 @@ use crate::host;
2121
/// `gateway`: the CLI uses the persisted-settings (or configured default)
2222
/// gateway; the RPC uses its `control_plane` override if given, else the
2323
/// current settings target.
24+
///
25+
/// A gateway that expects a different agent build refuses enrollment with
26+
/// `EnrollResponse::update_required`; surface the expected version so the
27+
/// operator installs the right binary instead of a stringly-typed error.
2428
pub async fn enroll(
2529
config: &AgentConfig,
2630
token: String,
@@ -42,7 +46,7 @@ pub async fn enroll(
4246
mem_mib: host::mem_mib(),
4347
capabilities,
4448
host_info_json: host::host_info_json(),
45-
protocol_version: PROTOCOL_VERSION,
49+
agent_version: env!("CARGO_PKG_VERSION").to_owned(),
4650
};
4751

4852
let response = client
@@ -51,12 +55,24 @@ pub async fn enroll(
5155
.context("Enroll RPC failed")?
5256
.into_inner();
5357

54-
let credential = Credential {
55-
machine_id: response.machine_id,
56-
machine_token: response.machine_token,
57-
};
58-
info!(machine_id = %credential.machine_id, "enrolled");
59-
Ok(credential)
58+
match response.result {
59+
Some(enroll_response::Result::Enrolled(enrolled)) => {
60+
let credential = Credential {
61+
machine_id: enrolled.machine_id,
62+
machine_token: enrolled.machine_token,
63+
};
64+
info!(machine_id = %credential.machine_id, "enrolled");
65+
Ok(credential)
66+
}
67+
Some(enroll_response::Result::UpdateRequired(update)) => {
68+
anyhow::bail!(
69+
"enrollment refused: gateway expects agent version {}, this binary is {}",
70+
update.expected_version,
71+
env!("CARGO_PKG_VERSION"),
72+
);
73+
}
74+
None => anyhow::bail!("gateway returned an empty EnrollResponse"),
75+
}
6076
}
6177

6278
/// Call `Unenroll` on `gateway`, authenticated with `machine_token` — the

0 commit comments

Comments
 (0)