diff --git a/crates/arc-devcontainer/src/features.rs b/crates/arc-devcontainer/src/features.rs index fa4ccc965..abba9d256 100644 --- a/crates/arc-devcontainer/src/features.rs +++ b/crates/arc-devcontainer/src/features.rs @@ -17,6 +17,13 @@ pub struct FeatureLayer { pub dockerfile_snippet: String, } +/// All resolved feature data: layers, environment, and lifecycle hooks. +#[derive(Debug, Clone, Default)] +pub struct ResolvedFeatures { + pub layers: Vec, + pub container_env: HashMap, +} + /// Extract the directory name from a feature ID. /// e.g. "ghcr.io/devcontainers/features/node:1" -> "node" fn dir_name_from_id(feature_id: &str) -> String { @@ -333,9 +340,9 @@ fn generate_layer( pub async fn resolve_features( features: &HashMap, _build_context: &Path, -) -> crate::Result> { +) -> crate::Result { if features.is_empty() { - return Ok(Vec::new()); + return Ok(ResolvedFeatures::default()); } ensure_oras().await?; @@ -387,8 +394,8 @@ pub async fn resolve_features( // Topologically sort features let sorted_ids = topo_sort(&feature_ids, &metadata_map); - // Generate layers - let mut layers = Vec::new(); + // Generate layers and collect container_env + let mut resolved = ResolvedFeatures::default(); for id in &sorted_ids { let dir_name = dir_name_from_id(id); let options = all_options.get(id).cloned().unwrap_or(serde_json::Value::Object( @@ -404,17 +411,23 @@ pub async fn resolve_features( options: HashMap::new(), installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }); + // Collect feature containerEnv (later features override earlier) + for (k, v) in &metadata.container_env { + resolved.container_env.insert(k.clone(), v.clone()); + } + let dockerfile_snippet = generate_layer(id, &dir_name, &options, &metadata); - layers.push(FeatureLayer { + resolved.layers.push(FeatureLayer { id: id.clone(), dir_name, dockerfile_snippet, }); } - Ok(layers) + Ok(resolved) } #[cfg(test)] @@ -458,6 +471,7 @@ mod tests { options: HashMap::new(), installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }, ) }) @@ -481,6 +495,7 @@ mod tests { options: HashMap::new(), installs_after: vec!["b".to_string()], depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); metadata.insert( @@ -492,6 +507,7 @@ mod tests { options: HashMap::new(), installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); @@ -519,6 +535,7 @@ mod tests { options: HashMap::new(), installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); metadata.insert( @@ -530,6 +547,7 @@ mod tests { options: HashMap::new(), installs_after: vec!["a".to_string()], depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); metadata.insert( @@ -541,6 +559,7 @@ mod tests { options: HashMap::new(), installs_after: vec!["a".to_string()], depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); metadata.insert( @@ -552,6 +571,7 @@ mod tests { options: HashMap::new(), installs_after: vec!["b".to_string(), "c".to_string()], depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); @@ -586,6 +606,7 @@ mod tests { options: meta_options, installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }; let snippet = generate_layer( @@ -621,6 +642,7 @@ mod tests { options: meta_options, installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }; let snippet = generate_layer( @@ -644,6 +666,7 @@ mod tests { options: HashMap::new(), installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }; let snippet = generate_layer( @@ -675,6 +698,7 @@ mod tests { options: HashMap::new(), installs_after: Vec::new(), depends_on: depends, + container_env: HashMap::new(), }, ); metadata.insert( @@ -686,6 +710,7 @@ mod tests { options: HashMap::new(), installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); @@ -709,6 +734,7 @@ mod tests { options: HashMap::new(), installs_after: vec!["b".to_string()], depends_on: depends, + container_env: HashMap::new(), }, ); metadata.insert( @@ -720,6 +746,7 @@ mod tests { options: HashMap::new(), installs_after: Vec::new(), depends_on: HashMap::new(), + container_env: HashMap::new(), }, ); @@ -748,9 +775,53 @@ mod tests { "ghcr.io/devcontainers/features/node:1".to_string(), serde_json::json!({"version": "20"}), ); - let layers = resolve_features(&features, tmp.path()).await.unwrap(); - assert_eq!(layers.len(), 1); - assert_eq!(layers[0].dir_name, "node"); - assert!(layers[0].dockerfile_snippet.contains("export VERSION=\"20\"")); + let resolved = resolve_features(&features, tmp.path()).await.unwrap(); + assert_eq!(resolved.layers.len(), 1); + assert_eq!(resolved.layers[0].dir_name, "node"); + assert!(resolved.layers[0].dockerfile_snippet.contains("export VERSION=\"20\"")); + } + + #[test] + fn feature_container_env_collected() { + // Simulate what resolve_features does: collect container_env from metadata in sort order + let mut resolved = ResolvedFeatures::default(); + + let meta_a = FeatureMetadata { + id: Some("a".to_string()), + name: None, + version: None, + options: HashMap::new(), + installs_after: Vec::new(), + depends_on: HashMap::new(), + container_env: { + let mut env = HashMap::new(); + env.insert("FOO".to_string(), "from_a".to_string()); + env.insert("BAR".to_string(), "from_a".to_string()); + env + }, + }; + let meta_b = FeatureMetadata { + id: Some("b".to_string()), + name: None, + version: None, + options: HashMap::new(), + installs_after: Vec::new(), + depends_on: HashMap::new(), + container_env: { + let mut env = HashMap::new(); + env.insert("FOO".to_string(), "from_b".to_string()); + env + }, + }; + + // A is sorted first, then B — B's FOO overrides A's + for meta in [&meta_a, &meta_b] { + for (k, v) in &meta.container_env { + resolved.container_env.insert(k.clone(), v.clone()); + } + } + + assert_eq!(resolved.container_env.get("FOO").map(String::as_str), Some("from_b")); + assert_eq!(resolved.container_env.get("BAR").map(String::as_str), Some("from_a")); } } diff --git a/crates/arc-devcontainer/src/lib.rs b/crates/arc-devcontainer/src/lib.rs index 0c242366c..a53f58a2d 100644 --- a/crates/arc-devcontainer/src/lib.rs +++ b/crates/arc-devcontainer/src/lib.rs @@ -248,17 +248,31 @@ impl DevcontainerResolver { }; // Features - let feature_layers = if !devcontainer.features.is_empty() { + let resolved_features = if !devcontainer.features.is_empty() { features::resolve_features(&devcontainer.features, &build_context).await? } else { - Vec::new() + features::ResolvedFeatures::default() + }; + + // Merge feature containerEnv with devcontainer.json containerEnv + // (devcontainer.json wins on conflicts) + let mut merged_container_env = resolved_features.container_env; + if let Some(env) = &devcontainer.container_env { + for (k, v) in env { + merged_container_env.insert(k.clone(), variables::substitute(v, &vars)); + } + } + let container_env_option = if merged_container_env.is_empty() { + None + } else { + Some(merged_container_env.clone()) }; // Generate final Dockerfile let dockerfile_content = dockerfile::generate( &base_dockerfile, - &feature_layers, - &devcontainer.container_env, + &resolved_features.layers, + &container_env_option, devcontainer.remote_user.as_deref(), ); @@ -281,7 +295,7 @@ impl DevcontainerResolver { post_create_commands: Self::collect_commands(&devcontainer.post_create_command, &vars), post_start_commands: Self::collect_commands(&devcontainer.post_start_command, &vars), environment, - container_env: Self::collect_container_env(&devcontainer.container_env, &vars), + container_env: merged_container_env, remote_user: devcontainer.remote_user.clone(), workspace_folder, forwarded_ports, diff --git a/crates/arc-devcontainer/src/types.rs b/crates/arc-devcontainer/src/types.rs index 80bb1b827..1d70d0ef2 100644 --- a/crates/arc-devcontainer/src/types.rs +++ b/crates/arc-devcontainer/src/types.rs @@ -122,6 +122,10 @@ pub struct FeatureMetadata { /// Hard dependencies: feature IDs that must be present (auto-installed if missing) #[serde(default)] pub depends_on: HashMap, + + /// Environment variables contributed by this feature + #[serde(default)] + pub container_env: HashMap, } /// A single option for a devcontainer feature. @@ -244,6 +248,20 @@ mod tests { assert_eq!(config.image.as_deref(), Some("ubuntu")); } + #[test] + fn parse_feature_metadata_container_env() { + let json = r#"{ + "id": "node", + "containerEnv": { + "NODE_ENV": "development", + "PATH": "/usr/local/bin:${PATH}" + } + }"#; + let meta: FeatureMetadata = serde_json::from_str(json).unwrap(); + assert_eq!(meta.container_env.len(), 2); + assert_eq!(meta.container_env.get("NODE_ENV").map(String::as_str), Some("development")); + } + #[test] fn parse_feature_metadata_depends_on() { let json = r#"{