mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-07 08:27:12 +00:00
Fix 3 feature spec bugs: shorthand version, option env names, install user vars
- Normalize string option values ("1.18") to {"version": "1.18"} so
shorthand syntax correctly sets the VERSION env var
- Add option_id_to_env_name() to convert option IDs like "node-version"
to NODE_VERSION per the dev container spec
- Emit _REMOTE_USER, _CONTAINER_USER, _REMOTE_USER_HOME, and
_CONTAINER_USER_HOME in feature install snippets, threading remoteUser
from devcontainer.json through resolve_features/generate_layer
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8168c85ac9
commit
864cb5d853
6 changed files with 247 additions and 18 deletions
|
|
@ -415,29 +415,66 @@ fn topo_sort(
|
|||
sorted
|
||||
}
|
||||
|
||||
/// Convert an option ID to an environment variable name per the dev container spec.
|
||||
/// Replaces non-alphanumeric, non-underscore chars with `_`, strips leading digits/underscores,
|
||||
/// and uppercases the result.
|
||||
fn option_id_to_env_name(id: &str) -> String {
|
||||
let replaced: String = id
|
||||
.chars()
|
||||
.map(|c| if c.is_alphanumeric() || c == '_' { c } else { '_' })
|
||||
.collect();
|
||||
let trimmed = replaced.trim_start_matches(|c: char| c == '_' || c.is_ascii_digit());
|
||||
if trimmed.is_empty() {
|
||||
"_".to_string()
|
||||
} else {
|
||||
trimmed.to_uppercase()
|
||||
}
|
||||
}
|
||||
|
||||
/// Generate a Dockerfile snippet for a single feature layer.
|
||||
fn generate_layer(
|
||||
feature_id: &str,
|
||||
dir_name: &str,
|
||||
options: &serde_json::Value,
|
||||
metadata: &FeatureMetadata,
|
||||
remote_user: Option<&str>,
|
||||
) -> String {
|
||||
let mut env_lines = Vec::new();
|
||||
|
||||
// Collect all option names from metadata to set defaults
|
||||
let user_options: HashMap<String, String> = match options.as_object() {
|
||||
Some(obj) => obj
|
||||
.iter()
|
||||
.map(|(k, v)| {
|
||||
let val = match v {
|
||||
serde_json::Value::String(s) => s.clone(),
|
||||
other => other.to_string(),
|
||||
};
|
||||
(k.clone(), val)
|
||||
})
|
||||
.collect(),
|
||||
None => HashMap::new(),
|
||||
// Emit built-in user env vars expected by community features
|
||||
let ru = remote_user.unwrap_or("root");
|
||||
let ru_home = if ru == "root" {
|
||||
"/root".to_string()
|
||||
} else {
|
||||
format!("/home/{ru}")
|
||||
};
|
||||
env_lines.push(format!(" export _REMOTE_USER=\"{ru}\" && \\"));
|
||||
env_lines.push(" export _CONTAINER_USER=\"root\" && \\".to_string());
|
||||
env_lines.push(format!(" export _REMOTE_USER_HOME=\"{ru_home}\" && \\"));
|
||||
env_lines.push(" export _CONTAINER_USER_HOME=\"/root\" && \\".to_string());
|
||||
|
||||
// Normalize shorthand version syntax: "1.18" → {"version": "1.18"}
|
||||
let options_obj = match options {
|
||||
serde_json::Value::String(s) => {
|
||||
let mut map = serde_json::Map::new();
|
||||
map.insert("version".to_string(), serde_json::Value::String(s.clone()));
|
||||
map
|
||||
}
|
||||
serde_json::Value::Object(obj) => obj.clone(),
|
||||
_ => serde_json::Map::new(),
|
||||
};
|
||||
|
||||
// Collect all option names from metadata to set defaults
|
||||
let user_options: HashMap<String, String> = options_obj
|
||||
.iter()
|
||||
.map(|(k, v)| {
|
||||
let val = match v {
|
||||
serde_json::Value::String(s) => s.clone(),
|
||||
other => other.to_string(),
|
||||
};
|
||||
(k.clone(), val)
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Merge metadata defaults with user-provided options
|
||||
let mut merged_options: Vec<(String, String)> = Vec::new();
|
||||
|
|
@ -467,7 +504,7 @@ fn generate_layer(
|
|||
merged_options.sort_by(|a, b| a.0.cmp(&b.0));
|
||||
|
||||
for (name, value) in &merged_options {
|
||||
let env_name = name.to_uppercase();
|
||||
let env_name = option_id_to_env_name(name);
|
||||
env_lines.push(format!(" export {env_name}=\"{value}\" && \\"));
|
||||
}
|
||||
|
||||
|
|
@ -492,6 +529,7 @@ fn generate_layer(
|
|||
pub async fn resolve_features(
|
||||
features: &HashMap<String, serde_json::Value>,
|
||||
devcontainer_dir: &Path,
|
||||
remote_user: Option<&str>,
|
||||
) -> crate::Result<ResolvedFeatures> {
|
||||
if features.is_empty() {
|
||||
return Ok(ResolvedFeatures::default());
|
||||
|
|
@ -592,7 +630,7 @@ pub async fn resolve_features(
|
|||
resolved.post_start_commands.push(cmd.clone());
|
||||
}
|
||||
|
||||
let dockerfile_snippet = generate_layer(id, &dir_name, &options, &metadata);
|
||||
let dockerfile_snippet = generate_layer(id, &dir_name, &options, &metadata, remote_user);
|
||||
resolved.layers.push(FeatureLayer {
|
||||
id: id.clone(),
|
||||
dir_name,
|
||||
|
|
@ -869,6 +907,7 @@ mod tests {
|
|||
"node",
|
||||
&options,
|
||||
&metadata,
|
||||
None,
|
||||
);
|
||||
|
||||
assert!(snippet.contains("# Feature: ghcr.io/devcontainers/features/node:1"));
|
||||
|
|
@ -908,6 +947,7 @@ mod tests {
|
|||
"node",
|
||||
&options,
|
||||
&metadata,
|
||||
None,
|
||||
);
|
||||
|
||||
// Default value "lts" should be used
|
||||
|
|
@ -935,12 +975,15 @@ mod tests {
|
|||
"common-utils",
|
||||
&options,
|
||||
&metadata,
|
||||
None,
|
||||
);
|
||||
|
||||
assert!(snippet.contains("# Feature: ghcr.io/devcontainers/features/common-utils:1"));
|
||||
assert!(snippet.contains("COPY common-utils/ /tmp/devcontainer-features/common-utils/"));
|
||||
assert!(snippet.contains("chmod +x install.sh"));
|
||||
assert!(!snippet.contains("export "));
|
||||
// Should have built-in user env vars but no feature-specific options
|
||||
assert!(snippet.contains("_REMOTE_USER"));
|
||||
assert!(!snippet.contains("export VERSION"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -1048,7 +1091,7 @@ mod tests {
|
|||
"ghcr.io/devcontainers/features/node:1".to_string(),
|
||||
serde_json::json!({"version": "20"}),
|
||||
);
|
||||
let resolved = resolve_features(&features, tmp.path()).await.unwrap();
|
||||
let resolved = resolve_features(&features, tmp.path(), None).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\""));
|
||||
|
|
@ -1124,4 +1167,85 @@ mod tests {
|
|||
assert!(matches!(&resolved.post_create_commands[0], LifecycleCommand::Array(arr) if arr.len() == 2));
|
||||
assert_eq!(resolved.post_start_commands.len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn option_id_to_env_name_hyphenated() {
|
||||
assert_eq!(option_id_to_env_name("node-version"), "NODE_VERSION");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn option_id_to_env_name_leading_digit() {
|
||||
assert_eq!(option_id_to_env_name("2fast"), "FAST");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn option_id_to_env_name_simple() {
|
||||
assert_eq!(option_id_to_env_name("simple"), "SIMPLE");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generate_layer_shorthand_version() {
|
||||
let options = serde_json::json!("20");
|
||||
let mut meta_options = HashMap::new();
|
||||
meta_options.insert(
|
||||
"version".to_string(),
|
||||
FeatureOption {
|
||||
option_type: Some("string".to_string()),
|
||||
default: Some(serde_json::Value::String("lts".to_string())),
|
||||
description: Some("Node.js version".to_string()),
|
||||
},
|
||||
);
|
||||
let metadata = FeatureMetadata {
|
||||
id: Some("node".to_string()),
|
||||
name: None,
|
||||
version: None,
|
||||
options: meta_options,
|
||||
installs_after: Vec::new(),
|
||||
depends_on: HashMap::new(),
|
||||
container_env: HashMap::new(),
|
||||
on_create_command: None,
|
||||
post_create_command: None,
|
||||
post_start_command: None,
|
||||
};
|
||||
|
||||
let snippet = generate_layer(
|
||||
"ghcr.io/devcontainers/features/node:1",
|
||||
"node",
|
||||
&options,
|
||||
&metadata,
|
||||
None,
|
||||
);
|
||||
|
||||
assert!(snippet.contains("export VERSION=\"20\""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generate_layer_install_env_vars() {
|
||||
let options = serde_json::json!({});
|
||||
let metadata = FeatureMetadata {
|
||||
id: Some("node".to_string()),
|
||||
name: None,
|
||||
version: None,
|
||||
options: HashMap::new(),
|
||||
installs_after: Vec::new(),
|
||||
depends_on: HashMap::new(),
|
||||
container_env: HashMap::new(),
|
||||
on_create_command: None,
|
||||
post_create_command: None,
|
||||
post_start_command: None,
|
||||
};
|
||||
|
||||
let snippet = generate_layer(
|
||||
"ghcr.io/devcontainers/features/node:1",
|
||||
"node",
|
||||
&options,
|
||||
&metadata,
|
||||
Some("vscode"),
|
||||
);
|
||||
|
||||
assert!(snippet.contains("_REMOTE_USER=\"vscode\""));
|
||||
assert!(snippet.contains("_CONTAINER_USER=\"root\""));
|
||||
assert!(snippet.contains("_REMOTE_USER_HOME=\"/home/vscode\""));
|
||||
assert!(snippet.contains("_CONTAINER_USER_HOME=\"/root\""));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -249,7 +249,7 @@ impl DevcontainerResolver {
|
|||
|
||||
// Features
|
||||
let resolved_features = if !devcontainer.features.is_empty() {
|
||||
features::resolve_features(&devcontainer.features, base_dir).await?
|
||||
features::resolve_features(&devcontainer.features, base_dir, devcontainer.remote_user.as_deref()).await?
|
||||
} else {
|
||||
features::ResolvedFeatures::default()
|
||||
};
|
||||
|
|
|
|||
|
|
@ -429,6 +429,86 @@ async fn feature_lifecycle_hooks_appended() {
|
|||
assert!(post_start.contains(&"echo node-started"));
|
||||
}
|
||||
|
||||
/// Fix 1: Shorthand version syntax "1.21" is normalized to {"version": "1.21"}.
|
||||
#[tokio::test]
|
||||
async fn feature_shorthand_version_syntax() {
|
||||
let config = DevcontainerResolver::resolve(&fixture_path("feature-options"))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// "1.21" string should become version=1.21 env var
|
||||
assert!(
|
||||
config.dockerfile.contains("export VERSION=\"1.21\""),
|
||||
"shorthand string \"1.21\" should set VERSION env var, got:\n{}",
|
||||
config.dockerfile,
|
||||
);
|
||||
}
|
||||
|
||||
/// Fix 2: Hyphenated option IDs are converted to valid env var names (node-version → NODE_VERSION).
|
||||
#[tokio::test]
|
||||
async fn feature_option_id_hyphen_to_underscore() {
|
||||
let config = DevcontainerResolver::resolve(&fixture_path("feature-options"))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// node-version default "none" should export as NODE_VERSION (not NODE-VERSION)
|
||||
assert!(
|
||||
config.dockerfile.contains("export NODE_VERSION=\"none\""),
|
||||
"hyphenated option 'node-version' should become NODE_VERSION env var, got:\n{}",
|
||||
config.dockerfile,
|
||||
);
|
||||
assert!(
|
||||
!config.dockerfile.contains("NODE-VERSION"),
|
||||
"NODE-VERSION (with hyphen) should not appear in Dockerfile",
|
||||
);
|
||||
}
|
||||
|
||||
/// Fix 3: _REMOTE_USER and related env vars are emitted in feature install snippets.
|
||||
#[tokio::test]
|
||||
async fn feature_install_user_env_vars() {
|
||||
let config = DevcontainerResolver::resolve(&fixture_path("feature-options"))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// remoteUser is "developer", so _REMOTE_USER should be "developer"
|
||||
assert!(
|
||||
config.dockerfile.contains("_REMOTE_USER=\"developer\""),
|
||||
"_REMOTE_USER should be set to remoteUser value, got:\n{}",
|
||||
config.dockerfile,
|
||||
);
|
||||
assert!(
|
||||
config.dockerfile.contains("_CONTAINER_USER=\"root\""),
|
||||
"_CONTAINER_USER should always be root",
|
||||
);
|
||||
assert!(
|
||||
config.dockerfile.contains("_REMOTE_USER_HOME=\"/home/developer\""),
|
||||
"_REMOTE_USER_HOME should be /home/developer",
|
||||
);
|
||||
assert!(
|
||||
config.dockerfile.contains("_CONTAINER_USER_HOME=\"/root\""),
|
||||
"_CONTAINER_USER_HOME should always be /root",
|
||||
);
|
||||
}
|
||||
|
||||
/// Fix 3: _REMOTE_USER defaults to root when remoteUser is not set.
|
||||
#[tokio::test]
|
||||
async fn feature_install_user_env_vars_default_root() {
|
||||
let config = DevcontainerResolver::resolve(&fixture_path("local-features"))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// local-features has remoteUser: "vscode"
|
||||
assert!(
|
||||
config.dockerfile.contains("_REMOTE_USER=\"vscode\""),
|
||||
"_REMOTE_USER should be set to vscode, got:\n{}",
|
||||
config.dockerfile,
|
||||
);
|
||||
assert!(
|
||||
config.dockerfile.contains("_REMOTE_USER_HOME=\"/home/vscode\""),
|
||||
"_REMOTE_USER_HOME should be /home/vscode",
|
||||
);
|
||||
}
|
||||
|
||||
/// Gap 2+3: Feature ordering affects both containerEnv and lifecycle hook collection.
|
||||
/// python-feature installsAfter node-feature, so node's env/hooks come first.
|
||||
#[tokio::test]
|
||||
|
|
|
|||
7
crates/arc-devcontainer/tests/fixtures/feature-options/.devcontainer/devcontainer.json
vendored
Normal file
7
crates/arc-devcontainer/tests/fixtures/feature-options/.devcontainer/devcontainer.json
vendored
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
{
|
||||
"image": "mcr.microsoft.com/devcontainers/base:ubuntu",
|
||||
"features": {
|
||||
"./go-feature": "1.21"
|
||||
},
|
||||
"remoteUser": "developer"
|
||||
}
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
{
|
||||
"id": "go-feature",
|
||||
"version": "1.0.0",
|
||||
"options": {
|
||||
"version": {
|
||||
"type": "string",
|
||||
"default": "latest",
|
||||
"description": "Go version"
|
||||
},
|
||||
"node-version": {
|
||||
"type": "string",
|
||||
"default": "none",
|
||||
"description": "Optional Node.js version (hyphenated option ID)"
|
||||
}
|
||||
}
|
||||
}
|
||||
2
crates/arc-devcontainer/tests/fixtures/feature-options/.devcontainer/go-feature/install.sh
vendored
Normal file
2
crates/arc-devcontainer/tests/fixtures/feature-options/.devcontainer/go-feature/install.sh
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
#!/bin/sh
|
||||
echo "Installing go ${VERSION} with node ${NODE_VERSION}"
|
||||
Loading…
Add table
Reference in a new issue