Skip to content

Commit 8b6d2fe

Browse files
committed
fix: gate docker backends in orchestrator
Remove production docker fallback and require broker unless dev mode. Gate lifecycle manager docker usage behind DEVELOPMENT_MODE and update tests. Refresh backend selection docs and tighten error handling.
1 parent cb34e86 commit 8b6d2fe

3 files changed

Lines changed: 76 additions & 59 deletions

File tree

crates/challenge-orchestrator/src/backend.rs

Lines changed: 15 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use secure_container_runtime::{
2828
};
2929
use std::path::Path;
3030
use std::sync::Arc;
31-
use tracing::{error, info, warn};
31+
use tracing::{info, warn};
3232

3333
/// Default broker socket path
3434
pub const DEFAULT_BROKER_SOCKET: &str = "/var/run/platform/broker.sock";
@@ -490,34 +490,17 @@ pub async fn create_backend() -> anyhow::Result<Box<dyn ContainerBackend>> {
490490
info!("Using secure container broker (production mode)");
491491
Ok(Box::new(secure))
492492
} else {
493-
warn!(
494-
"Secure backend reported as available but failed to initialize; falling back to Docker"
495-
);
496-
create_docker_fallback_backend().await
493+
let default_socket = default_broker_socket_path();
494+
Err(anyhow::anyhow!(
495+
"Secure container broker is unavailable. Start broker at {} or set CONTAINER_BROKER_SOCKET.",
496+
default_socket
497+
))
497498
}
498499
}
499-
BackendMode::Fallback => create_docker_fallback_backend().await,
500-
}
501-
}
502-
503-
async fn create_docker_fallback_backend() -> anyhow::Result<Box<dyn ContainerBackend>> {
504-
warn!("Broker not available. Attempting Docker fallback...");
505-
warn!("This should only happen in local development!");
506-
warn!("Set DEVELOPMENT_MODE=true to suppress this warning, or start the broker.");
507-
508-
match DirectDockerBackend::new().await {
509-
Ok(direct) => {
510-
warn!("Using direct Docker - NOT RECOMMENDED FOR PRODUCTION");
511-
Ok(Box::new(direct))
512-
}
513-
Err(e) => {
514-
error!("Cannot connect to Docker: {}", e);
515-
error!("For production: Start the container-broker service");
516-
error!("For development: Set DEVELOPMENT_MODE=true and ensure Docker is running");
500+
BackendMode::Unavailable => {
517501
let default_socket = default_broker_socket_path();
518502
Err(anyhow::anyhow!(
519-
"No container backend available. \
520-
Start broker at {} or set DEVELOPMENT_MODE=true for local Docker",
503+
"No container backend available. Start broker at {} or set DEVELOPMENT_MODE=true for local Docker.",
521504
default_socket
522505
))
523506
}
@@ -528,7 +511,7 @@ async fn create_docker_fallback_backend() -> anyhow::Result<Box<dyn ContainerBac
528511
pub enum BackendMode {
529512
Development,
530513
Secure,
531-
Fallback,
514+
Unavailable,
532515
}
533516

534517
pub fn select_backend_mode() -> BackendMode {
@@ -537,7 +520,7 @@ pub fn select_backend_mode() -> BackendMode {
537520
} else if SecureBackend::is_available() {
538521
BackendMode::Secure
539522
} else {
540-
BackendMode::Fallback
523+
BackendMode::Unavailable
541524
}
542525
}
543526

@@ -660,13 +643,13 @@ mod tests {
660643

661644
#[test]
662645
#[serial]
663-
fn test_select_backend_mode_falls_back_without_broker() {
646+
fn test_select_backend_mode_unavailable_without_broker() {
664647
reset_env();
665648
let dir = tempdir().expect("temp dir");
666649
let missing_socket = dir.path().join("missing.sock");
667650
std::env::set_var(BROKER_SOCKET_OVERRIDE_ENV, &missing_socket);
668651

669-
assert_eq!(select_backend_mode(), BackendMode::Fallback);
652+
assert_eq!(select_backend_mode(), BackendMode::Unavailable);
670653

671654
reset_env();
672655
}
@@ -903,34 +886,18 @@ mod tests {
903886

904887
#[tokio::test]
905888
#[serial]
906-
async fn test_create_backend_falls_back_when_secure_missing() {
889+
async fn test_create_backend_reports_error_without_broker() {
907890
reset_env();
908891
let dir = tempdir().expect("temp dir");
909892
let missing_socket = dir.path().join("missing.sock");
910893
std::env::set_var(BROKER_SOCKET_OVERRIDE_ENV, &missing_socket);
911-
DirectDockerBackend::set_test_result(Ok(DirectDockerBackend::with_docker(
912-
RecordingChallengeDocker::default(),
913-
)));
914894

915-
let backend = create_backend().await.expect("fallback backend");
916-
backend
917-
.pull_image("ghcr.io/platformnetwork/fallback:v1")
918-
.await
919-
.unwrap();
920-
921-
reset_env();
922-
}
923-
924-
#[tokio::test]
925-
#[serial]
926-
async fn test_create_docker_fallback_backend_reports_error() {
927-
reset_env();
928-
DirectDockerBackend::set_test_result(Err(anyhow::anyhow!("boom")));
929-
let err = match create_docker_fallback_backend().await {
895+
let err = match create_backend().await {
930896
Ok(_) => panic!("expected error"),
931897
Err(err) => err,
932898
};
933899
assert!(err.to_string().contains("No container backend available"));
900+
934901
reset_env();
935902
}
936903

crates/challenge-orchestrator/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
//! Priority order:
2222
//! 1. `DEVELOPMENT_MODE=true` -> Direct Docker (local dev only)
2323
//! 2. Broker socket exists -> Secure broker (production default)
24-
//! 3. No broker + not dev mode -> Fallback to Docker with warnings
24+
//! 3. No broker + not dev mode -> Error (production requires broker)
2525
//!
2626
//! Default broker socket: `/var/run/platform/broker.sock`
2727

crates/challenge-orchestrator/src/lifecycle.rs

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use tracing::{error, info};
1717
/// Manages the lifecycle of challenge containers, retaining both the live
1818
/// container handles and the configs needed to recreate them during restarts.
1919
pub struct LifecycleManager {
20-
docker: Box<dyn ChallengeDocker>,
20+
docker: Option<Box<dyn ChallengeDocker>>,
2121
challenges: Arc<RwLock<HashMap<ChallengeId, ChallengeInstance>>>,
2222
configs: Arc<RwLock<HashMap<ChallengeId, ChallengeContainerConfig>>>,
2323
}
@@ -27,22 +27,42 @@ impl LifecycleManager {
2727
docker: impl ChallengeDocker + 'static,
2828
challenges: Arc<RwLock<HashMap<ChallengeId, ChallengeInstance>>>,
2929
) -> Self {
30+
Self::new_with_dev_docker(Some(Box::new(docker)), challenges)
31+
}
32+
33+
pub fn new_with_dev_docker(
34+
docker: Option<Box<dyn ChallengeDocker>>,
35+
challenges: Arc<RwLock<HashMap<ChallengeId, ChallengeInstance>>>,
36+
) -> Self {
37+
let docker = if crate::backend::is_development_mode() {
38+
docker
39+
} else {
40+
None
41+
};
42+
3043
Self {
31-
docker: Box::new(docker),
44+
docker,
3245
challenges,
3346
configs: Arc::new(RwLock::new(HashMap::new())),
3447
}
3548
}
3649

50+
fn docker(&self) -> anyhow::Result<&dyn ChallengeDocker> {
51+
self.docker.as_deref().ok_or_else(|| {
52+
anyhow::anyhow!("LifecycleManager docker backend is disabled outside DEVELOPMENT_MODE")
53+
})
54+
}
55+
3756
/// Add a challenge configuration (will start container)
3857
pub async fn add(&mut self, config: ChallengeContainerConfig) -> anyhow::Result<()> {
3958
let challenge_id = config.challenge_id;
59+
let docker = self.docker()?;
4060

4161
// Pull image first
42-
self.docker.pull_image(&config.docker_image).await?;
62+
docker.pull_image(&config.docker_image).await?;
4363

4464
// Start container
45-
let instance = self.docker.start_challenge(&config).await?;
65+
let instance = docker.start_challenge(&config).await?;
4666

4767
// Store config and instance
4868
self.configs.write().insert(challenge_id, config);
@@ -55,6 +75,7 @@ impl LifecycleManager {
5575
/// Update a challenge (new image version)
5676
pub async fn update(&mut self, config: ChallengeContainerConfig) -> anyhow::Result<()> {
5777
let challenge_id = config.challenge_id;
78+
let docker = self.docker()?;
5879

5980
// Stop existing container - get container_id first, then release lock before await
6081
let container_id = self
@@ -63,15 +84,15 @@ impl LifecycleManager {
6384
.get(&challenge_id)
6485
.map(|i| i.container_id.clone());
6586
if let Some(container_id) = container_id {
66-
self.docker.stop_container(&container_id).await?;
67-
self.docker.remove_container(&container_id).await?;
87+
docker.stop_container(&container_id).await?;
88+
docker.remove_container(&container_id).await?;
6889
}
6990

7091
// Pull new image
71-
self.docker.pull_image(&config.docker_image).await?;
92+
docker.pull_image(&config.docker_image).await?;
7293

7394
// Start new container
74-
let instance = self.docker.start_challenge(&config).await?;
95+
let instance = docker.start_challenge(&config).await?;
7596

7697
// Update config and instance
7798
self.configs.write().insert(challenge_id, config);
@@ -83,11 +104,13 @@ impl LifecycleManager {
83104

84105
/// Remove a challenge
85106
pub async fn remove(&mut self, challenge_id: ChallengeId) -> anyhow::Result<()> {
107+
let docker = self.docker()?;
108+
86109
// Remove instance and get container_id before await
87110
let instance = self.challenges.write().remove(&challenge_id);
88111
if let Some(instance) = instance {
89-
self.docker.stop_container(&instance.container_id).await?;
90-
self.docker.remove_container(&instance.container_id).await?;
112+
docker.stop_container(&instance.container_id).await?;
113+
docker.remove_container(&instance.container_id).await?;
91114
}
92115

93116
self.configs.write().remove(&challenge_id);
@@ -264,6 +287,7 @@ mod tests {
264287

265288
#[tokio::test]
266289
async fn test_restart_unhealthy_restarts_only_unhealthy() {
290+
std::env::set_var("DEVELOPMENT_MODE", "true");
267291
let mock = MockDocker::default();
268292
let mut manager =
269293
LifecycleManager::new(mock.clone(), Arc::new(RwLock::new(HashMap::new())));
@@ -318,10 +342,13 @@ mod tests {
318342
assert!(!ops
319343
.iter()
320344
.any(|op| op == &format!("stop:{healthy_container_id}")));
345+
346+
std::env::remove_var("DEVELOPMENT_MODE");
321347
}
322348

323349
#[tokio::test]
324350
async fn test_sync_handles_add_update_remove() {
351+
std::env::set_var("DEVELOPMENT_MODE", "true");
325352
let mock = MockDocker::default();
326353
let challenges = Arc::new(RwLock::new(HashMap::new()));
327354
let mut manager = LifecycleManager::new(mock.clone(), challenges);
@@ -387,10 +414,13 @@ mod tests {
387414
assert!(ops.iter().any(|op| op == "remove:container-update-old"));
388415
assert!(ops.iter().any(|op| op == "stop:container-remove-old"));
389416
assert!(ops.iter().any(|op| op == "remove:container-remove-old"));
417+
418+
std::env::remove_var("DEVELOPMENT_MODE");
390419
}
391420

392421
#[tokio::test]
393422
async fn test_add_records_config_and_instance_state() {
423+
std::env::set_var("DEVELOPMENT_MODE", "true");
394424
let mock = MockDocker::default();
395425
let challenges = Arc::new(RwLock::new(HashMap::new()));
396426
let mut manager = LifecycleManager::new(mock.clone(), challenges);
@@ -405,10 +435,13 @@ mod tests {
405435
let ops = mock.operations();
406436
assert!(ops.contains(&format!("pull:{}", config.docker_image)));
407437
assert!(ops.contains(&format!("start:{}", challenge_id)));
438+
439+
std::env::remove_var("DEVELOPMENT_MODE");
408440
}
409441

410442
#[tokio::test]
411443
async fn test_stop_all_removes_every_challenge() {
444+
std::env::set_var("DEVELOPMENT_MODE", "true");
412445
let mock = MockDocker::default();
413446
let challenges = Arc::new(RwLock::new(HashMap::new()));
414447
let mut manager = LifecycleManager::new(mock.clone(), challenges);
@@ -456,6 +489,23 @@ mod tests {
456489
assert!(ops.contains(&"remove:container-first".to_string()));
457490
assert!(ops.contains(&"stop:container-second".to_string()));
458491
assert!(ops.contains(&"remove:container-second".to_string()));
492+
493+
std::env::remove_var("DEVELOPMENT_MODE");
494+
}
495+
496+
#[tokio::test]
497+
async fn test_lifecycle_manager_disabled_outside_dev_mode() {
498+
std::env::remove_var("DEVELOPMENT_MODE");
499+
let mock = MockDocker::default();
500+
let challenges = Arc::new(RwLock::new(HashMap::new()));
501+
let mut manager = LifecycleManager::new(mock, challenges);
502+
let challenge_id = ChallengeId::new();
503+
let config = sample_config(challenge_id, "ghcr.io/org/add:v1");
504+
505+
let err = manager.add(config).await.expect_err("expected error");
506+
assert!(err
507+
.to_string()
508+
.contains("LifecycleManager docker backend is disabled"));
459509
}
460510

461511
#[derive(Clone, Default)]

0 commit comments

Comments
 (0)