From 916acd08be83cccb6d6c096d4a538be99ef25e16 Mon Sep 17 00:00:00 2001 From: Johan Carlin Date: Thu, 30 Apr 2026 18:35:32 +0200 Subject: [PATCH 1/5] Cover upstream substitution utility cases --- cmd/devcontainer/src/config.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/cmd/devcontainer/src/config.rs b/cmd/devcontainer/src/config.rs index 6d1b65038..23f20950d 100644 --- a/cmd/devcontainer/src/config.rs +++ b/cmd/devcontainer/src/config.rs @@ -52,6 +52,7 @@ mod tests { fn substitutes_local_env_and_workspace_tokens() { let mut env = HashMap::new(); env.insert("USER".to_string(), "johan".to_string()); + env.insert("baz".to_string(), "somevalue".to_string()); let context = ConfigContext { workspace_folder: PathBuf::from("/workspace/demo"), env, @@ -61,6 +62,7 @@ mod tests { let value = json!({ "containerEnv": { "USER_NAME": "${localEnv:USER}", + "ENV_ALIAS": "bar${env:baz}bar", "WORKSPACE": "${localWorkspaceFolder}", "CONTAINER_WORKSPACE": "${containerWorkspaceFolder}", "CONTAINER_BASENAME": "${containerWorkspaceFolderBasename}" @@ -70,6 +72,7 @@ mod tests { let substituted = substitute_local_context(&value, &context); assert_eq!(substituted["containerEnv"]["USER_NAME"], "johan"); + assert_eq!(substituted["containerEnv"]["ENV_ALIAS"], "barsomevaluebar"); assert_eq!(substituted["containerEnv"]["WORKSPACE"], "/workspace/demo"); assert_eq!( substituted["containerEnv"]["CONTAINER_WORKSPACE"], @@ -180,4 +183,34 @@ mod tests { .chars() .all(|character| matches!(character, '0'..='9' | 'a'..='v'))); } + + #[test] + fn devcontainer_id_changes_when_labels_change() { + let value = json!({ + "test": "${devcontainerId}" + }); + let first = substitute_local_context( + &value, + &ConfigContext { + workspace_folder: PathBuf::from("/workspace/demo"), + env: HashMap::new(), + container_workspace_folder: None, + id_labels: HashMap::from([("a".to_string(), "b".to_string())]), + }, + ); + let second = substitute_local_context( + &value, + &ConfigContext { + workspace_folder: PathBuf::from("/workspace/demo"), + env: HashMap::new(), + container_workspace_folder: None, + id_labels: HashMap::from([ + ("a".to_string(), "b".to_string()), + ("c".to_string(), "d".to_string()), + ]), + }, + ); + + assert_ne!(first["test"], second["test"]); + } } From 8eea81d6e0bfbaa9195292be8e3375f6c5ac3d45 Mon Sep 17 00:00:00 2001 From: Johan Carlin Date: Thu, 30 Apr 2026 18:39:02 +0200 Subject: [PATCH 2/5] Cover upstream compose build utility cases --- cmd/devcontainer/src/runtime/compose/mod.rs | 3 +- .../src/runtime/compose/service.rs | 59 ++++++++++++ cmd/devcontainer/src/runtime/compose/tests.rs | 89 +++++++++++++++++++ 3 files changed, 150 insertions(+), 1 deletion(-) diff --git a/cmd/devcontainer/src/runtime/compose/mod.rs b/cmd/devcontainer/src/runtime/compose/mod.rs index ffa53891d..897319c23 100644 --- a/cmd/devcontainer/src/runtime/compose/mod.rs +++ b/cmd/devcontainer/src/runtime/compose/mod.rs @@ -54,12 +54,13 @@ pub(crate) fn load_compose_spec(resolved: &ResolvedConfig) -> Result, pub(super) has_build: bool, + pub(super) build: Option, pub(super) user: Option, pub(super) entrypoint: Option>, pub(super) command: Option>, } +#[derive(Debug, Eq, PartialEq)] +pub(super) struct ServiceBuildInfo { + pub(super) context: String, + pub(super) dockerfile_path: String, + pub(super) target: Option, + pub(super) args: Option>, +} + pub(super) fn compose_files( configuration: &Value, config_root: &Path, @@ -93,10 +103,16 @@ pub(super) fn inspect_service_definition( ) -> Result { let mut image = None; let mut has_build = false; + let mut build = None; let mut user = None; let mut entrypoint = None; let mut command = None; let mut found_service = false; + let default_build_context = compose_files + .first() + .and_then(|path| path.parent()) + .map(|path| path.display().to_string()) + .unwrap_or_else(|| ".".to_string()); for compose_file in compose_files { let raw = std::fs::read_to_string(compose_file).map_err(|error| error.to_string())?; @@ -116,6 +132,9 @@ pub(super) fn inspect_service_definition( if service_definition.contains_key(YamlValue::String("build".to_string())) { has_build = true; } + if let Some(value) = service_field(service_definition, "build") { + build = parse_service_build(value, &default_build_context); + } if let Some(value) = service_field(service_definition, "image").and_then(YamlValue::as_str) { image = Some(value.to_string()); @@ -144,6 +163,7 @@ pub(super) fn inspect_service_definition( Ok(ServiceDefinition { image, has_build, + build, user, entrypoint, command, @@ -154,6 +174,45 @@ fn service_field<'a>(mapping: &'a Mapping, key: &str) -> Option<&'a YamlValue> { mapping.get(YamlValue::String(key.to_string())) } +fn parse_service_build(value: &YamlValue, default_context: &str) -> Option { + match value { + YamlValue::String(context) => Some(ServiceBuildInfo { + context: context.to_string(), + dockerfile_path: "Dockerfile".to_string(), + target: None, + args: None, + }), + YamlValue::Mapping(mapping) => Some(ServiceBuildInfo { + context: service_field(mapping, "context") + .and_then(YamlValue::as_str) + .map(str::to_string) + .unwrap_or_else(|| default_context.to_string()), + dockerfile_path: service_field(mapping, "dockerfile") + .and_then(YamlValue::as_str) + .map(str::to_string) + .unwrap_or_else(|| "Dockerfile".to_string()), + target: service_field(mapping, "target") + .and_then(YamlValue::as_str) + .map(str::to_string), + args: service_field(mapping, "args").and_then(parse_build_args), + }), + _ => None, + } +} + +fn parse_build_args(value: &YamlValue) -> Option> { + let mapping = value.as_mapping()?; + let args = mapping + .iter() + .filter_map(|(key, value)| { + let key = yaml_scalar_to_string(key)?; + let value = yaml_scalar_to_string(value)?; + Some((key, value)) + }) + .collect::>(); + (!args.is_empty()).then_some(args) +} + pub(super) fn read_version_prefix(compose_files: &[PathBuf]) -> Result { let Some(first_compose_file) = compose_files.first() else { return Ok(String::new()); diff --git a/cmd/devcontainer/src/runtime/compose/tests.rs b/cmd/devcontainer/src/runtime/compose/tests.rs index d886d6c82..4184ea573 100644 --- a/cmd/devcontainer/src/runtime/compose/tests.rs +++ b/cmd/devcontainer/src/runtime/compose/tests.rs @@ -48,6 +48,95 @@ fn inspects_service_image_and_build_presence() { let _ = fs::remove_dir_all(root); } +#[test] +fn inspects_service_build_info_for_upstream_compose_shapes() { + let root = unique_temp_dir("devcontainer-compose-test"); + let compose_dir = root.join("somepath"); + let compose_file = compose_dir.join("docker-compose.yml"); + fs::create_dir_all(&compose_dir).expect("compose root"); + fs::write( + &compose_file, + r#" +services: + fully_specified: + image: my-image + build: + context: context-path + dockerfile: my-dockerfile + target: a-target + args: + arg1: value1 + image_only: + image: my-image + string_build: + image: my-image + build: ./a-path + default_dockerfile: + build: + context: ./a-path + default_context: + build: + dockerfile: my-dockerfile +"#, + ) + .expect("compose file"); + + let fully_specified = + inspect_service_definition(std::slice::from_ref(&compose_file), "fully_specified") + .expect("fully specified service"); + let fully_specified_build = fully_specified.build.as_ref().expect("build info"); + assert_eq!(fully_specified.image.as_deref(), Some("my-image")); + assert_eq!(fully_specified_build.context, "context-path"); + assert_eq!(fully_specified_build.dockerfile_path, "my-dockerfile"); + assert_eq!(fully_specified_build.target.as_deref(), Some("a-target")); + assert_eq!( + fully_specified_build + .args + .as_ref() + .and_then(|args| args.get("arg1")) + .map(String::as_str), + Some("value1") + ); + + let image_only = inspect_service_definition(std::slice::from_ref(&compose_file), "image_only") + .expect("image-only service"); + assert_eq!(image_only.image.as_deref(), Some("my-image")); + assert!(image_only.build.is_none()); + + let string_build = + inspect_service_definition(std::slice::from_ref(&compose_file), "string_build") + .expect("string build service"); + let string_build_info = string_build.build.as_ref().expect("string build info"); + assert_eq!(string_build.image.as_deref(), Some("my-image")); + assert_eq!(string_build_info.context, "./a-path"); + assert_eq!(string_build_info.dockerfile_path, "Dockerfile"); + assert_eq!(string_build_info.target, None); + assert_eq!(string_build_info.args, None); + + let default_dockerfile = + inspect_service_definition(std::slice::from_ref(&compose_file), "default_dockerfile") + .expect("default dockerfile service"); + let default_dockerfile_build = default_dockerfile.build.as_ref().expect("build info"); + assert_eq!(default_dockerfile_build.context, "./a-path"); + assert_eq!(default_dockerfile_build.dockerfile_path, "Dockerfile"); + assert_eq!(default_dockerfile_build.target, None); + assert_eq!(default_dockerfile_build.args, None); + + let default_context = + inspect_service_definition(std::slice::from_ref(&compose_file), "default_context") + .expect("default context service"); + let default_context_build = default_context.build.as_ref().expect("build info"); + assert_eq!( + default_context_build.context, + compose_dir.display().to_string() + ); + assert_eq!(default_context_build.dockerfile_path, "my-dockerfile"); + assert_eq!(default_context_build.target, None); + assert_eq!(default_context_build.args, None); + + let _ = fs::remove_dir_all(root); +} + #[test] fn compose_project_name_defaults_to_workspace_devcontainer() { let root = unique_temp_dir("devcontainer-compose-test"); From 97c83ec58b6a953cf231e3c1eb58dc4babbad549 Mon Sep 17 00:00:00 2001 From: Johan Carlin Date: Thu, 30 Apr 2026 18:47:15 +0200 Subject: [PATCH 3/5] Cover upstream Dockerfile utility cases --- cmd/devcontainer/src/runtime/dockerfile.rs | 1068 ++++++++++++++++++++ cmd/devcontainer/src/runtime/mod.rs | 1 + docs/upstream/parity-inventory.json | 3 +- 3 files changed, 1071 insertions(+), 1 deletion(-) create mode 100644 cmd/devcontainer/src/runtime/dockerfile.rs diff --git a/cmd/devcontainer/src/runtime/dockerfile.rs b/cmd/devcontainer/src/runtime/dockerfile.rs new file mode 100644 index 000000000..12b896cba --- /dev/null +++ b/cmd/devcontainer/src/runtime/dockerfile.rs @@ -0,0 +1,1068 @@ +//! Dockerfile parsing helpers that mirror deterministic upstream utility behavior. + +#![cfg_attr(not(test), allow(dead_code))] + +use std::collections::{HashMap, HashSet}; + +#[derive(Debug, Eq, PartialEq)] +struct FinalStageName { + last_stage_name: String, + modified_dockerfile: Option, +} + +#[derive(Debug, Eq, PartialEq)] +struct Dockerfile { + preamble: Preamble, + stages: Vec, + stages_by_label: HashMap, +} + +#[derive(Debug, Eq, PartialEq)] +struct Preamble { + version: Option, + directives: HashMap, + instructions: Vec, +} + +#[derive(Debug, Eq, PartialEq)] +struct Stage { + from: From, + instructions: Vec, +} + +#[derive(Debug, Eq, PartialEq)] +struct From { + platform: Option, + image: String, + label: Option, +} + +#[derive(Debug, Eq, PartialEq)] +struct Instruction { + instruction: String, + name: String, + value: Option, +} + +#[derive(Debug, Eq, PartialEq)] +enum BuildContextSupport { + Supported, + Unsupported, + Unknown, +} + +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +enum ScopeId { + Preamble, + Stage(usize), +} + +struct FromLine { + from: From, + matched_end: usize, +} + +struct Token { + text: String, + end: usize, +} + +enum VariableOperator { + Default, + Alternate, +} + +struct VariableExpression<'a> { + name: &'a str, + operator: Option, + word: Option<&'a str>, +} + +fn ensure_dockerfile_has_final_stage_name( + dockerfile: &str, + default_last_stage_name: &str, +) -> Result { + let Some((line_start, line)) = last_from_line(dockerfile) else { + return Err( + "Error parsing Dockerfile: Dockerfile contains no FROM instructions".to_string(), + ); + }; + let parsed = parse_from_line(line) + .ok_or_else(|| "Error parsing Dockerfile: failed to parse final FROM line".to_string())?; + if let Some(label) = parsed.from.label { + return Ok(FinalStageName { + last_stage_name: label, + modified_dockerfile: None, + }); + } + + let insert_at = line_start + parsed.matched_end; + let mut modified_dockerfile = + String::with_capacity(dockerfile.len() + " AS ".len() + default_last_stage_name.len()); + modified_dockerfile.push_str(&dockerfile[..insert_at]); + modified_dockerfile.push_str(" AS "); + modified_dockerfile.push_str(default_last_stage_name); + modified_dockerfile.push_str(&dockerfile[insert_at..]); + + Ok(FinalStageName { + last_stage_name: default_last_stage_name.to_string(), + modified_dockerfile: Some(modified_dockerfile), + }) +} + +fn extract_dockerfile(dockerfile: &str) -> Dockerfile { + let mut preamble = String::new(); + let mut stage_strings = Vec::new(); + let mut current_stage = None::; + + for line in dockerfile.split_inclusive('\n') { + if is_from_line(line) { + if let Some(stage) = current_stage.replace(line.to_string()) { + stage_strings.push(stage); + } + } else if let Some(stage) = current_stage.as_mut() { + stage.push_str(line); + } else { + preamble.push_str(line); + } + } + if let Some(stage) = current_stage { + stage_strings.push(stage); + } + + let directives = extract_directives(&preamble); + let version = directives + .get("syntax") + .and_then(|syntax| dockerfile_syntax_version(syntax)); + let stages = stage_strings + .iter() + .map(|stage| Stage { + from: stage + .lines() + .find_map(parse_from_line) + .map(|line| line.from) + .unwrap_or_else(|| From { + platform: None, + image: "unknown".to_string(), + label: None, + }), + instructions: extract_instructions(stage), + }) + .collect::>(); + let stages_by_label = stages + .iter() + .enumerate() + .filter_map(|(index, stage)| { + stage + .from + .label + .as_ref() + .map(|label| (label.clone(), index)) + }) + .collect::>(); + + Dockerfile { + preamble: Preamble { + version, + directives, + instructions: extract_instructions(&preamble), + }, + stages, + stages_by_label, + } +} + +fn find_base_image( + dockerfile: &Dockerfile, + build_args: &HashMap, + target: Option<&str>, + global_buildx_platform_args: &HashMap, +) -> Option { + let mut stage = target + .and_then(|target| dockerfile.stages_by_label.get(target).copied()) + .or_else(|| dockerfile.stages.len().checked_sub(1))?; + let mut seen = HashSet::new(); + + loop { + if !seen.insert(stage) { + return None; + } + let image = replace_variables( + dockerfile, + build_args, + &HashMap::new(), + global_buildx_platform_args, + &dockerfile.stages[stage].from.image, + ScopeId::Preamble, + dockerfile.preamble.instructions.len(), + ); + let Some(next_stage) = dockerfile.stages_by_label.get(&image).copied() else { + return Some(image); + }; + stage = next_stage; + } +} + +fn find_user_statement( + dockerfile: &Dockerfile, + build_args: &HashMap, + base_image_env: &HashMap, + global_buildx_platform_args: &HashMap, + target: Option<&str>, +) -> Option { + let mut stage = target + .and_then(|target| dockerfile.stages_by_label.get(target).copied()) + .or_else(|| dockerfile.stages.len().checked_sub(1))?; + let mut seen = HashSet::new(); + + loop { + if !seen.insert(stage) { + return None; + } + if let Some(index) = find_last_instruction_index( + &dockerfile.stages[stage].instructions, + |instruction| instruction.instruction == "USER", + dockerfile.stages[stage].instructions.len(), + ) { + return non_empty(replace_variables( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + &dockerfile.stages[stage].instructions[index].name, + ScopeId::Stage(stage), + index, + )); + } + + let image = replace_variables( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + &dockerfile.stages[stage].from.image, + ScopeId::Preamble, + dockerfile.preamble.instructions.len(), + ); + let next_stage = dockerfile.stages_by_label.get(&image).copied()?; + stage = next_stage; + } +} + +fn supports_build_contexts(dockerfile: &Dockerfile) -> BuildContextSupport { + let Some(version) = dockerfile.preamble.version.as_deref() else { + return if dockerfile.preamble.directives.contains_key("syntax") { + BuildContextSupport::Unknown + } else { + BuildContextSupport::Unsupported + }; + }; + let numeric_version = version + .chars() + .take_while(|character| character.is_ascii_digit() || *character == '.') + .collect::(); + if numeric_version.is_empty() { + return BuildContextSupport::Supported; + } + if numeric_version_supports_build_contexts(&numeric_version) { + BuildContextSupport::Supported + } else { + BuildContextSupport::Unsupported + } +} + +fn last_from_line(dockerfile: &str) -> Option<(usize, &str)> { + let mut offset = 0; + let mut result = None; + for raw_line in dockerfile.split_inclusive('\n') { + let line = raw_line.trim_end_matches(['\r', '\n']); + if is_from_line(line) { + result = Some((offset, line)); + } + offset += raw_line.len(); + } + result +} + +fn is_from_line(line: &str) -> bool { + tokenize_line(line) + .first() + .is_some_and(|token| token.text.eq_ignore_ascii_case("FROM")) +} + +fn parse_from_line(line: &str) -> Option { + let tokens = tokenize_line(line); + if !tokens + .first() + .is_some_and(|token| token.text.eq_ignore_ascii_case("FROM")) + { + return None; + } + + let mut index = 1; + let platform = tokens + .get(index) + .filter(|token| token.text.starts_with("--platform=")) + .map(|token| { + index += 1; + token.text.clone() + }); + let image = tokens.get(index)?; + index += 1; + let label = tokens + .get(index) + .filter(|token| token.text.eq_ignore_ascii_case("AS")) + .and_then(|_| tokens.get(index + 1)) + .map(|token| strip_edge_quotes(&token.text)); + let matched_end = label + .as_ref() + .and_then(|_| tokens.get(index + 1)) + .map(|token| token.end) + .unwrap_or(image.end); + + Some(FromLine { + from: From { + platform, + image: strip_edge_quotes(&image.text), + label, + }, + matched_end, + }) +} + +fn tokenize_line(line: &str) -> Vec { + let mut tokens = Vec::new(); + let mut token_start = None; + let mut quote = None; + + for (index, character) in line.char_indices() { + if token_start.is_none() { + if character.is_whitespace() { + continue; + } + if character == '#' { + break; + } + token_start = Some(index); + } + + if let Some(active_quote) = quote { + if character == active_quote { + quote = None; + } + continue; + } + if character == '\'' || character == '"' { + quote = Some(character); + continue; + } + if character.is_whitespace() { + let start = token_start.take().expect("token start"); + tokens.push(Token { + text: line[start..index].to_string(), + end: index, + }); + } + } + + if let Some(start) = token_start { + tokens.push(Token { + text: line[start..].to_string(), + end: line.len(), + }); + } + + tokens +} + +fn extract_directives(preamble: &str) -> HashMap { + let mut directives = HashMap::new(); + for line in preamble.lines() { + let trimmed = line.trim_start(); + let Some(after_comment) = trimmed.strip_prefix('#') else { + break; + }; + let Some((name, value)) = after_comment.split_once('=') else { + break; + }; + let name = name.trim().to_ascii_lowercase(); + let value = value.trim().to_string(); + if name.is_empty() || value.is_empty() { + break; + } + directives.entry(name).or_insert(value); + } + directives +} + +fn dockerfile_syntax_version(syntax: &str) -> Option { + let syntax = syntax.to_ascii_lowercase(); + for prefix in ["docker/dockerfile", "docker.io/docker/dockerfile"] { + if syntax == prefix { + return Some("latest".to_string()); + } + if let Some(version) = syntax + .strip_prefix(prefix) + .and_then(|rest| rest.strip_prefix(':')) + { + return Some(version.to_string()); + } + } + None +} + +fn numeric_version_supports_build_contexts(version: &str) -> bool { + let parts = version + .split('.') + .filter_map(|part| part.parse::().ok()) + .collect::>(); + match parts.as_slice() { + [major] => *major >= 1, + [major, minor, ..] => *major > 1 || (*major == 1 && *minor >= 4), + _ => false, + } +} + +fn extract_instructions(section: &str) -> Vec { + section.lines().filter_map(parse_instruction_line).collect() +} + +fn parse_instruction_line(line: &str) -> Option { + let trimmed = line.trim_start(); + for keyword in ["ARG", "ENV", "USER"] { + if let Some(rest) = strip_instruction_keyword(trimmed, keyword) { + let (name, value) = parse_instruction_name_value(rest)?; + return Some(Instruction { + instruction: keyword.to_string(), + name, + value, + }); + } + } + None +} + +fn strip_instruction_keyword<'a>(line: &'a str, keyword: &str) -> Option<&'a str> { + if line.len() < keyword.len() { + return None; + } + let (candidate, rest) = line.split_at(keyword.len()); + if !candidate.eq_ignore_ascii_case(keyword) { + return None; + } + rest.chars() + .next() + .is_some_and(char::is_whitespace) + .then(|| rest.trim_start()) +} + +fn parse_instruction_name_value(rest: &str) -> Option<(String, Option)> { + let name_end = rest + .char_indices() + .find_map(|(index, character)| { + (character.is_whitespace() || character == '=').then_some(index) + }) + .unwrap_or(rest.len()); + let name = rest[..name_end].to_string(); + if name.is_empty() { + return None; + } + + let mut value = rest[name_end..].trim_start(); + if let Some(after_equals) = value.strip_prefix('=') { + value = after_equals.trim_start(); + } + let value = first_field(value).map(strip_edge_quotes); + Some((name, value)) +} + +fn first_field(value: &str) -> Option<&str> { + let trimmed = value.trim_start(); + if trimmed.is_empty() { + return None; + } + let end = trimmed + .char_indices() + .find_map(|(index, character)| character.is_whitespace().then_some(index)) + .unwrap_or(trimmed.len()); + Some(&trimmed[..end]) +} + +fn replace_variables( + dockerfile: &Dockerfile, + build_args: &HashMap, + base_image_env: &HashMap, + global_buildx_platform_args: &HashMap, + input: &str, + scope: ScopeId, + before_instruction_index: usize, +) -> String { + let mut output = String::with_capacity(input.len()); + let mut index = 0; + + while index < input.len() { + if !input[index..].starts_with('$') { + let character = input[index..].chars().next().expect("character"); + output.push(character); + index += character.len_utf8(); + continue; + } + + if input[index + 1..].starts_with('{') { + let content_start = index + 2; + let Some(close_offset) = input[content_start..].find('}') else { + output.push('$'); + index += 1; + continue; + }; + let content_end = content_start + close_offset; + let content = &input[content_start..content_end]; + if let Some(expression) = parse_variable_expression(content) { + output.push_str(&resolve_variable_expression( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + scope, + before_instruction_index, + expression, + )); + index = content_end + 1; + } else { + output.push('$'); + index += 1; + } + continue; + } + + let variable_start = index + 1; + let variable_end = input[variable_start..] + .char_indices() + .find_map(|(offset, character)| { + (!is_variable_character(character)).then_some(variable_start + offset) + }) + .unwrap_or(input.len()); + if variable_end == variable_start { + output.push('$'); + index += 1; + continue; + } + let expression = VariableExpression { + name: &input[variable_start..variable_end], + operator: None, + word: None, + }; + output.push_str(&resolve_variable_expression( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + scope, + before_instruction_index, + expression, + )); + index = variable_end; + } + + output +} + +fn parse_variable_expression(content: &str) -> Option> { + let variable_end = content + .char_indices() + .find_map(|(index, character)| (!is_variable_character(character)).then_some(index)) + .unwrap_or(content.len()); + if variable_end == 0 { + return None; + } + + let name = &content[..variable_end]; + let rest = &content[variable_end..]; + if rest.is_empty() { + return Some(VariableExpression { + name, + operator: None, + word: None, + }); + } + let (operator, word) = if let Some(word) = rest.strip_prefix(":-") { + (VariableOperator::Default, word) + } else if let Some(word) = rest.strip_prefix(":+") { + (VariableOperator::Alternate, word) + } else { + return None; + }; + Some(VariableExpression { + name, + operator: Some(operator), + word: Some(word), + }) +} + +fn resolve_variable_expression( + dockerfile: &Dockerfile, + build_args: &HashMap, + base_image_env: &HashMap, + global_buildx_platform_args: &HashMap, + scope: ScopeId, + before_instruction_index: usize, + expression: VariableExpression<'_>, +) -> String { + let value = find_value( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + expression.name, + scope, + before_instruction_index, + ) + .unwrap_or_default(); + match (expression.operator, expression.word) { + (Some(VariableOperator::Default), Some(word)) => { + if value.is_empty() { + strip_edge_quotes(word) + } else { + value + } + } + (Some(VariableOperator::Alternate), Some(word)) => { + if value.is_empty() { + value + } else { + strip_edge_quotes(word) + } + } + _ => value, + } +} + +fn find_value( + dockerfile: &Dockerfile, + build_args: &HashMap, + base_image_env: &HashMap, + global_buildx_platform_args: &HashMap, + variable: &str, + initial_scope: ScopeId, + initial_before_instruction_index: usize, +) -> Option { + let mut scope = initial_scope; + let mut before_instruction_index = initial_before_instruction_index; + let mut consider_arg = true; + let mut seen = HashSet::new(); + + loop { + if !seen.insert(scope) { + return None; + } + let instructions = scope_instructions(dockerfile, scope); + if let Some(index) = find_last_instruction_index( + instructions, + |instruction| { + instruction.name == variable + && (instruction.instruction == "ENV" + || (consider_arg + && (build_args.contains_key(&instruction.name) + || instruction.value.is_some()))) + }, + before_instruction_index.min(instructions.len()), + ) { + let instruction = &instructions[index]; + if instruction.instruction == "ENV" { + return instruction.value.as_ref().map(|value| { + replace_variables( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + value, + scope, + index, + ) + }); + } + if instruction.instruction == "ARG" { + let value = build_args + .get(&instruction.name) + .or(instruction.value.as_ref())?; + return Some(replace_variables( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + value, + scope, + index, + )); + } + } + + let Some(from) = scope_from(dockerfile, scope) else { + return base_image_env + .get(variable) + .or_else(|| global_buildx_platform_args.get(variable)) + .cloned(); + }; + let image = replace_variables( + dockerfile, + build_args, + base_image_env, + global_buildx_platform_args, + &from.image, + ScopeId::Preamble, + dockerfile.preamble.instructions.len(), + ); + scope = dockerfile + .stages_by_label + .get(&image) + .copied() + .map(ScopeId::Stage) + .unwrap_or(ScopeId::Preamble); + before_instruction_index = scope_instructions(dockerfile, scope).len(); + consider_arg = matches!(scope, ScopeId::Preamble); + } +} + +fn find_last_instruction_index( + instructions: &[Instruction], + predicate: impl Fn(&Instruction) -> bool, + before_instruction_index: usize, +) -> Option { + (0..before_instruction_index) + .rev() + .find(|index| predicate(&instructions[*index])) +} + +fn scope_instructions(dockerfile: &Dockerfile, scope: ScopeId) -> &[Instruction] { + match scope { + ScopeId::Preamble => &dockerfile.preamble.instructions, + ScopeId::Stage(index) => &dockerfile.stages[index].instructions, + } +} + +fn scope_from(dockerfile: &Dockerfile, scope: ScopeId) -> Option<&From> { + match scope { + ScopeId::Preamble => None, + ScopeId::Stage(index) => Some(&dockerfile.stages[index].from), + } +} + +fn non_empty(value: String) -> Option { + (!value.is_empty()).then_some(value) +} + +fn is_variable_character(character: char) -> bool { + character.is_ascii_alphanumeric() || character == '_' +} + +fn strip_edge_quotes(value: &str) -> String { + let mut start = 0; + let mut end = value.len(); + if value + .as_bytes() + .first() + .is_some_and(|byte| matches!(byte, b'\'' | b'"')) + { + start += 1; + } + if end > start + && value + .as_bytes() + .get(end - 1) + .is_some_and(|byte| matches!(byte, b'\'' | b'"')) + { + end -= 1; + } + value[start..end].to_string() +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::{ + ensure_dockerfile_has_final_stage_name, extract_dockerfile, find_base_image, + find_user_statement, supports_build_contexts, BuildContextSupport, + }; + + #[test] + fn ensures_final_stage_names_without_rewriting_named_stages() { + let dockerfile = r#" +FROM ubuntu:latest as base + +RUN some command + +FROM base as final + +COPY src dest +"#; + + let result = + ensure_dockerfile_has_final_stage_name(dockerfile, "placeholder").expect("stage name"); + + assert_eq!(result.last_stage_name, "final"); + assert_eq!(result.modified_dockerfile, None); + } + + #[test] + fn inserts_final_stage_names_before_comments_and_preserves_spacing() { + let dockerfile = r#" +FROM ubuntu:latest as base + +RUN some command + + FROM --platform=my-platform base # deliberately includes: as something here + +COPY src dest +"#; + + let result = + ensure_dockerfile_has_final_stage_name(dockerfile, "placeholder").expect("stage name"); + + assert_eq!(result.last_stage_name, "placeholder"); + assert_eq!( + result.modified_dockerfile.as_deref(), + Some( + r#" +FROM ubuntu:latest as base + +RUN some command + + FROM --platform=my-platform base AS placeholder # deliberately includes: as something here + +COPY src dest +"# + ) + ); + } + + #[test] + fn rejects_dockerfiles_without_from_instructions() { + let error = ensure_dockerfile_has_final_stage_name("RUN some command\n", "placeholder") + .expect_err("expected parse error"); + + assert!(error.contains("Dockerfile contains no FROM instructions")); + } + + #[test] + fn extracts_env_forms_and_lowercase_instructions() { + let dockerfile = "from E\nenv A=B\nenv C = D\nenv E F\narg G\nuser H\n"; + let extracted = extract_dockerfile(dockerfile); + + assert_eq!(extracted.stages.len(), 1); + let stage = &extracted.stages[0]; + assert_eq!(stage.from.image, "E"); + assert_eq!(stage.instructions[0].instruction, "ENV"); + assert_eq!(stage.instructions[0].name, "A"); + assert_eq!(stage.instructions[0].value.as_deref(), Some("B")); + assert_eq!(stage.instructions[1].name, "C"); + assert_eq!(stage.instructions[1].value.as_deref(), Some("D")); + assert_eq!(stage.instructions[2].name, "E"); + assert_eq!(stage.instructions[2].value.as_deref(), Some("F")); + assert_eq!(stage.instructions[3].instruction, "ARG"); + assert_eq!(stage.instructions[3].name, "G"); + assert_eq!(stage.instructions[4].instruction, "USER"); + assert_eq!(stage.instructions[4].name, "H"); + } + + #[test] + fn resolves_base_images_from_args_quotes_aliases_and_expressions() { + let extracted = extract_dockerfile( + r#" +ARG BASE_IMAGE="image2" +FROM "${BASE_IMAGE}" +"#, + ); + assert_eq!( + find_base_image(&extracted, &HashMap::new(), None, &HashMap::new()).as_deref(), + Some("image2") + ); + assert_eq!( + find_base_image( + &extracted, + &HashMap::from([("BASE_IMAGE".to_string(), "image3".to_string())]), + None, + &HashMap::new(), + ) + .as_deref(), + Some("image3") + ); + + let multistage = extract_dockerfile( + r#" +FROM image1 as stage1 +FROM stage3 as stage2 +FROM image3 as stage3 +FROM image4 as stage4 +"#, + ); + assert_eq!( + find_base_image( + &multistage, + &HashMap::new(), + Some("stage2"), + &HashMap::new() + ) + .as_deref(), + Some("image3") + ); + + let positive_expression = extract_dockerfile( + r#" +ARG cloud +FROM ${cloud:+"mcr.microsoft.com/"}azure-cli:latest" +"#, + ); + assert_eq!( + find_base_image( + &positive_expression, + &HashMap::from([("cloud".to_string(), "true".to_string())]), + None, + &HashMap::new(), + ) + .as_deref(), + Some("mcr.microsoft.com/azure-cli:latest") + ); + assert_eq!( + find_base_image(&positive_expression, &HashMap::new(), None, &HashMap::new(),) + .as_deref(), + Some("azure-cli:latest") + ); + + let negative_expression = extract_dockerfile( + r#" +ARG cloud +FROM "${cloud:-"mcr.microsoft.com/"}azure-cli:latest" +"#, + ); + assert_eq!( + find_base_image( + &negative_expression, + &HashMap::from([("cloud".to_string(), "ghcr.io/".to_string())]), + None, + &HashMap::new(), + ) + .as_deref(), + Some("ghcr.io/azure-cli:latest") + ); + assert_eq!( + find_base_image(&negative_expression, &HashMap::new(), None, &HashMap::new(),) + .as_deref(), + Some("mcr.microsoft.com/azure-cli:latest") + ); + } + + #[test] + fn resolves_user_statements_with_arg_env_and_stage_precedence() { + let arg_overwritten = extract_dockerfile( + r#" +FROM debian +ARG IMAGE_USER=user2 +USER $IMAGE_USER +"#, + ); + assert_eq!( + find_user_statement( + &arg_overwritten, + &HashMap::from([("IMAGE_USER".to_string(), "user3".to_string())]), + &HashMap::new(), + &HashMap::new(), + None, + ) + .as_deref(), + Some("user3") + ); + + let env_after_arg = extract_dockerfile( + r#" +FROM debian +ARG USERNAME=user1 +ENV USERNAME=user2 +USER ${USERNAME} +"#, + ); + assert_eq!( + find_user_statement( + &env_after_arg, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + None, + ) + .as_deref(), + Some("user2") + ); + + let inherited_env_not_arg = extract_dockerfile( + r#" +FROM debian as one +ENV USERNAME=user1 +ARG USERNAME=user2 + +FROM one as two +USER ${USERNAME} +"#, + ); + assert_eq!( + find_user_statement( + &inherited_env_not_arg, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + None, + ) + .as_deref(), + Some("user1") + ); + + let base_env = extract_dockerfile( + r#" +FROM mybase +USER ${USERNAME} +"#, + ); + assert_eq!( + find_user_statement( + &base_env, + &HashMap::new(), + &HashMap::from([("USERNAME".to_string(), "user1".to_string())]), + &HashMap::new(), + None, + ) + .as_deref(), + Some("user1") + ); + } + + #[test] + fn detects_build_context_support_from_syntax_directives() { + assert_eq!( + supports_build_contexts(&extract_dockerfile("FROM debian")), + BuildContextSupport::Unsupported + ); + assert_eq!( + supports_build_contexts(&extract_dockerfile( + "# syntax=docker/dockerfile:1.4\nFROM debian" + )), + BuildContextSupport::Supported + ); + assert_eq!( + supports_build_contexts(&extract_dockerfile( + "# syntax=docker.io/docker/dockerfile:1.2\nFROM debian" + )), + BuildContextSupport::Unsupported + ); + assert_eq!( + supports_build_contexts(&extract_dockerfile( + "# syntax=docker.io/docker/dockerfile:latest\nFROM debian" + )), + BuildContextSupport::Supported + ); + assert_eq!( + supports_build_contexts(&extract_dockerfile( + "# syntax=mycompany/myimage:1.4\nFROM debian" + )), + BuildContextSupport::Unknown + ); + } +} diff --git a/cmd/devcontainer/src/runtime/mod.rs b/cmd/devcontainer/src/runtime/mod.rs index 78ddb6452..6f577925c 100644 --- a/cmd/devcontainer/src/runtime/mod.rs +++ b/cmd/devcontainer/src/runtime/mod.rs @@ -4,6 +4,7 @@ mod build; pub(crate) mod compose; mod container; pub(crate) mod context; +mod dockerfile; pub(crate) mod engine; mod exec; mod lifecycle; diff --git a/docs/upstream/parity-inventory.json b/docs/upstream/parity-inventory.json index c97ac18c5..017139f6a 100644 --- a/docs/upstream/parity-inventory.json +++ b/docs/upstream/parity-inventory.json @@ -696,7 +696,8 @@ "evidence": [ "cmd/devcontainer/src/runtime/build.rs", "cmd/devcontainer/src/runtime/compose/args.rs", - "cmd/devcontainer/src/runtime/container/uid_update.rs" + "cmd/devcontainer/src/runtime/container/uid_update.rs", + "cmd/devcontainer/src/runtime/dockerfile.rs" ] }, { From c832dd9518c41cba468f98cdc533113901413fd7 Mon Sep 17 00:00:00 2001 From: Johan Carlin Date: Thu, 30 Apr 2026 18:50:13 +0200 Subject: [PATCH 4/5] Update upstream utility coverage map --- cmd/devcontainer/src/runtime/image.rs | 29 +++++++++++++++++++++++++++ cmd/devcontainer/src/runtime/mod.rs | 1 + docs/upstream/test-coverage-map.json | 22 ++++++++++---------- docs/upstream/test-coverage-map.md | 12 +++++------ 4 files changed, 47 insertions(+), 17 deletions(-) create mode 100644 cmd/devcontainer/src/runtime/image.rs diff --git a/cmd/devcontainer/src/runtime/image.rs b/cmd/devcontainer/src/runtime/image.rs new file mode 100644 index 000000000..6a87d70b2 --- /dev/null +++ b/cmd/devcontainer/src/runtime/image.rs @@ -0,0 +1,29 @@ +//! Container image reference helpers. + +#![cfg_attr(not(test), allow(dead_code))] + +fn qualify_image_name(name: &str) -> String { + let segments = name.split('/').collect::>(); + match segments.as_slice() { + [_] => format!("docker.io/library/{name}"), + ["docker.io", image] => format!("docker.io/library/{image}"), + [_, _] => format!("docker.io/{name}"), + _ => name.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::qualify_image_name; + + #[test] + fn qualifies_docker_io_shorthands() { + assert_eq!(qualify_image_name("ubuntu"), "docker.io/library/ubuntu"); + assert_eq!( + qualify_image_name("docker.io/ubuntu"), + "docker.io/library/ubuntu" + ); + assert_eq!(qualify_image_name("random/image"), "docker.io/random/image"); + assert_eq!(qualify_image_name("foo/random/image"), "foo/random/image"); + } +} diff --git a/cmd/devcontainer/src/runtime/mod.rs b/cmd/devcontainer/src/runtime/mod.rs index 6f577925c..0688face0 100644 --- a/cmd/devcontainer/src/runtime/mod.rs +++ b/cmd/devcontainer/src/runtime/mod.rs @@ -7,6 +7,7 @@ pub(crate) mod context; mod dockerfile; pub(crate) mod engine; mod exec; +mod image; mod lifecycle; pub(crate) mod metadata; pub(crate) mod mounts; diff --git a/docs/upstream/test-coverage-map.json b/docs/upstream/test-coverage-map.json index 3fb1f8b23..bb09de270 100644 --- a/docs/upstream/test-coverage-map.json +++ b/docs/upstream/test-coverage-map.json @@ -224,29 +224,29 @@ }, { "upstreamTest": "upstream/src/test/dockerComposeUtils.test.ts", - "status": "partial", + "status": "covered", "nativeTests": [ - "cmd/devcontainer/src/runtime/compose/tests.rs", - "cmd/devcontainer/tests/runtime_container_smoke/compose_project.rs", - "cmd/devcontainer/tests/runtime_container_smoke/compose_flow.rs" + "cmd/devcontainer/src/runtime/compose/tests.rs" ], - "notes": "Compose project naming and mount behavior are covered, but not every upstream utility case." + "notes": "Native compose utility coverage now mirrors upstream build-info parsing for image-only services, string and object build forms, default Dockerfile/default context behavior, build targets, and build args." }, { "upstreamTest": "upstream/src/test/dockerUtils.test.ts", "status": "partial", "nativeTests": [ - "cmd/devcontainer/src/runtime/container/engine_run.rs" + "cmd/devcontainer/src/runtime/container/engine_run.rs", + "cmd/devcontainer/src/runtime/image.rs" ], - "notes": "Native coverage now exercises concurrent container removal retries; registry image inspection and Docker image-name qualification remain uncovered." + "notes": "Native coverage exercises concurrent container removal retries and Docker image-name qualification; live registry image inspection remains out of scope." }, { "upstreamTest": "upstream/src/test/dockerfileUtils.test.ts", - "status": "partial", + "status": "covered", "nativeTests": [ + "cmd/devcontainer/src/runtime/dockerfile.rs", "cmd/devcontainer/tests/runtime_build_smoke/dockerfile.rs" ], - "notes": "Dockerfile build behavior is tested natively, but utility-level edge cases are not fully mirrored." + "notes": "Native Dockerfile utility coverage now mirrors upstream final-stage naming, Dockerfile extraction, base image and USER resolution, ARG/ENV precedence, stage alias traversal, build arg expressions, syntax directives, quoted images, lowercase instructions, and ENV forms." }, { "upstreamTest": "upstream/src/test/dotfiles.test.ts", @@ -313,12 +313,12 @@ }, { "upstreamTest": "upstream/src/test/variableSubstitution.test.ts", - "status": "partial", + "status": "covered", "nativeTests": [ "cmd/devcontainer/src/config.rs", "cmd/devcontainer/tests/cli_smoke/read_configuration.rs" ], - "notes": "Native substitution coverage exists, but the supported substitution surface is still narrower than upstream." + "notes": "Native substitution coverage mirrors upstream env/localEnv aliases, local and container workspace tokens, default handling, containerEnv defaults, and devcontainerId stability and label sensitivity." }, { "upstreamTest": "upstream/src/test/workspaceConfiguration.test.ts", diff --git a/docs/upstream/test-coverage-map.md b/docs/upstream/test-coverage-map.md index f02814baf..994bfc836 100644 --- a/docs/upstream/test-coverage-map.md +++ b/docs/upstream/test-coverage-map.md @@ -4,8 +4,8 @@ Machine-readable upstream test coverage inventory for the native Rust CLI. - Upstream commit: `2d81ee3c9ed96a7312c18c7513a17933f8f66d41` - Upstream tests inventoried: `35` -- Covered: `9` -- Partial: `26` +- Covered: `12` +- Partial: `23` - Missing: `0` ## Summary @@ -35,9 +35,9 @@ Machine-readable upstream test coverage inventory for the native Rust CLI. | `upstream/src/test/container-templates/containerTemplatesOCI.test.ts` | partial | `cmd/devcontainer/src/commands/collections/tests/templates.rs`
`cmd/devcontainer/tests/cli_smoke/collections.rs` | Native coverage now includes workspace-local OCI metadata lookup and archive-backed template apply flows, but live registry template fetch behavior is still missing. | | `upstream/src/test/container-templates/templatesCLICommands.test.ts` | partial | `cmd/devcontainer/tests/cli_smoke/collections.rs`
`cmd/devcontainer/src/commands/collections/tests/templates.rs`
`cmd/devcontainer/src/commands/collections/tests/publish.rs` | Template CLI commands now cover workspace-local OCI metadata and apply flows, but published-template behavior is still partial without live registry fetches. | | `upstream/src/test/disallowedFeatures.test.ts` | covered | `cmd/devcontainer/src/commands/configuration/features/control.rs`
`cmd/devcontainer/src/commands/configuration/tests/read.rs`
`cmd/devcontainer/src/commands/collections/tests/features.rs` | Native coverage now includes prefix matching plus command-level failures for read-configuration and features resolve-dependencies. | -| `upstream/src/test/dockerComposeUtils.test.ts` | partial | `cmd/devcontainer/src/runtime/compose/tests.rs`
`cmd/devcontainer/tests/runtime_container_smoke/compose_project.rs`
`cmd/devcontainer/tests/runtime_container_smoke/compose_flow.rs` | Compose project naming and mount behavior are covered, but not every upstream utility case. | -| `upstream/src/test/dockerfileUtils.test.ts` | partial | `cmd/devcontainer/tests/runtime_build_smoke/dockerfile.rs` | Dockerfile build behavior is tested natively, but utility-level edge cases are not fully mirrored. | -| `upstream/src/test/dockerUtils.test.ts` | partial | `cmd/devcontainer/src/runtime/container/engine_run.rs` | Native coverage now exercises concurrent container removal retries; registry image inspection and Docker image-name qualification remain uncovered. | +| `upstream/src/test/dockerComposeUtils.test.ts` | covered | `cmd/devcontainer/src/runtime/compose/tests.rs` | Native compose utility coverage now mirrors upstream build-info parsing for image-only services, string and object build forms, default Dockerfile/default context behavior, build targets, and build args. | +| `upstream/src/test/dockerfileUtils.test.ts` | covered | `cmd/devcontainer/src/runtime/dockerfile.rs`
`cmd/devcontainer/tests/runtime_build_smoke/dockerfile.rs` | Native Dockerfile utility coverage now mirrors upstream final-stage naming, Dockerfile extraction, base image and USER resolution, ARG/ENV precedence, stage alias traversal, build arg expressions, syntax directives, quoted images, lowercase instructions, and ENV forms. | +| `upstream/src/test/dockerUtils.test.ts` | partial | `cmd/devcontainer/src/runtime/container/engine_run.rs`
`cmd/devcontainer/src/runtime/image.rs` | Native coverage exercises concurrent container removal retries and Docker image-name qualification; live registry image inspection remains out of scope. | | `upstream/src/test/dotfiles.test.ts` | covered | `cmd/devcontainer/tests/runtime_lifecycle_smoke/dotfiles.rs` | Native dotfiles coverage includes ordering, reinstall markers, and personalization stop behavior. | | `upstream/src/test/getEntPasswd.test.ts` | covered | `cmd/devcontainer/src/runtime/user_resolution.rs` | Native unit coverage matches passwd row parsing and upstream getent/grep command generation, including empty lookup and escaping cases. | | `upstream/src/test/getHomeFolder.test.ts` | covered | `cmd/devcontainer/src/runtime/user_resolution.rs`
`cmd/devcontainer/tests/runtime_exec_smoke.rs`
`cmd/devcontainer/tests/runtime_lifecycle_smoke/commands.rs` | Native unit and fake-engine smoke coverage validates non-root HOME fallback, root HOME acceptance, explicit remote HOME precedence, and lifecycle/exec injection. | @@ -45,5 +45,5 @@ Machine-readable upstream test coverage inventory for the native Rust CLI. | `upstream/src/test/labelPathNormalization.test.ts` | covered | `cmd/devcontainer/src/commands/common/labels.rs`
`cmd/devcontainer/src/runtime/container/discovery.rs` | Native unit coverage now exercises Windows label normalization plus legacy workspace-only matching for default devcontainer labels. | | `upstream/src/test/updateUID.test.ts` | covered | `cmd/devcontainer/src/runtime/container/uid_update/tests.rs` | Native UID-update coverage includes image inspection, platform preservation, local tags, and podman behavior. | | `upstream/src/test/utils.test.ts` | covered | `cmd/devcontainer/src/runtime/build.rs` | Native build utility coverage matches upstream buildx inline cache detection cases and verifies build argument selection. | -| `upstream/src/test/variableSubstitution.test.ts` | partial | `cmd/devcontainer/src/config.rs`
`cmd/devcontainer/tests/cli_smoke/read_configuration.rs` | Native substitution coverage exists, but the supported substitution surface is still narrower than upstream. | +| `upstream/src/test/variableSubstitution.test.ts` | covered | `cmd/devcontainer/src/config.rs`
`cmd/devcontainer/tests/cli_smoke/read_configuration.rs` | Native substitution coverage mirrors upstream env/localEnv aliases, local and container workspace tokens, default handling, containerEnv defaults, and devcontainerId stability and label sensitivity. | | `upstream/src/test/workspaceConfiguration.test.ts` | partial | `cmd/devcontainer/src/runtime/context/workspace.rs`
`cmd/devcontainer/tests/runtime_container_smoke/basic.rs`
`cmd/devcontainer/tests/runtime_container_smoke/compose_project.rs` | Workspace mount behavior is covered, including lexical worktree common-dir normalization, but upstream cross-platform path cases are broader. | From 8ac62d760823878b25631f063001f6c5a6d86110 Mon Sep 17 00:00:00 2001 From: Johan Carlin Date: Thu, 30 Apr 2026 19:27:07 +0200 Subject: [PATCH 5/5] Fix missing Dockerfile target handling --- cmd/devcontainer/src/runtime/dockerfile.rs | 48 +++++++++++++++++++--- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/cmd/devcontainer/src/runtime/dockerfile.rs b/cmd/devcontainer/src/runtime/dockerfile.rs index 12b896cba..e82d4e9bd 100644 --- a/cmd/devcontainer/src/runtime/dockerfile.rs +++ b/cmd/devcontainer/src/runtime/dockerfile.rs @@ -178,9 +178,7 @@ fn find_base_image( target: Option<&str>, global_buildx_platform_args: &HashMap, ) -> Option { - let mut stage = target - .and_then(|target| dockerfile.stages_by_label.get(target).copied()) - .or_else(|| dockerfile.stages.len().checked_sub(1))?; + let mut stage = target_stage_index(dockerfile, target)?; let mut seen = HashSet::new(); loop { @@ -210,9 +208,7 @@ fn find_user_statement( global_buildx_platform_args: &HashMap, target: Option<&str>, ) -> Option { - let mut stage = target - .and_then(|target| dockerfile.stages_by_label.get(target).copied()) - .or_else(|| dockerfile.stages.len().checked_sub(1))?; + let mut stage = target_stage_index(dockerfile, target)?; let mut seen = HashSet::new(); loop { @@ -249,6 +245,13 @@ fn find_user_statement( } } +fn target_stage_index(dockerfile: &Dockerfile, target: Option<&str>) -> Option { + match target { + Some(target) => dockerfile.stages_by_label.get(target).copied(), + None => dockerfile.stages.len().checked_sub(1), + } +} + fn supports_build_contexts(dockerfile: &Dockerfile) -> BuildContextSupport { let Some(version) = dockerfile.preamble.version.as_deref() else { return if dockerfile.preamble.directives.contains_key("syntax") { @@ -952,6 +955,39 @@ FROM "${cloud:-"mcr.microsoft.com/"}azure-cli:latest" ); } + #[test] + fn missing_targets_do_not_fall_back_to_final_stage() { + let dockerfile = extract_dockerfile( + r#" +FROM debian AS base +USER base-user + +FROM ubuntu AS final +USER final-user +"#, + ); + + assert_eq!( + find_base_image( + &dockerfile, + &HashMap::new(), + Some("missing"), + &HashMap::new() + ), + None + ); + assert_eq!( + find_user_statement( + &dockerfile, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + Some("missing"), + ), + None + ); + } + #[test] fn resolves_user_statements_with_arg_env_and_stage_precedence() { let arg_overwritten = extract_dockerfile(