Skip to content

Commit f7e3c17

Browse files
author
Roy Lin
committed
fix(cli): image-management Docker parity (inspect array, rmi by short-id/--force, commit --change, tag validation)
Found via the runtime/CLI parity probe (round 2): - image-inspect now prints a top-level JSON array (like docker image inspect), so `inspect X | jq '.[0]'` works. - rmi resolves a bare lowercase-hex image id (Docker 12-hex short id), not only sha256:-prefixed refs; digest_matches prefix-matches the stored hex. With multiple tags sharing a digest, rmi errors 'ambiguous' (correct) and `rmi -f <id>` now removes ALL matching references instead of erroring. - commit --change CMD/ENTRYPOINT honor exec form (["a","b"]) and ENV/LABEL accept the legacy space-separated form, by reusing import.rs's parsers (previously commit wrapped exec arrays in sh -c => exit 127, and dropped space-form ENV). - tag rejects an uppercase repository name ('repository name must be lowercase'), like Docker, instead of silently creating a bad tag. All verified on Linux.
1 parent 4727c33 commit f7e3c17

6 files changed

Lines changed: 135 additions & 44 deletions

File tree

src/cli/src/commands/commit.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -246,16 +246,17 @@ fn apply_changes(config: &mut serde_json::Value, changes: &[String]) {
246246
for change in changes {
247247
let trimmed = change.trim();
248248
if let Some(rest) = trimmed.strip_prefix("CMD ") {
249-
config["config"]["Cmd"] = serde_json::json!(["/bin/sh", "-c", rest]);
249+
// Honor exec form (CMD ["a","b"]) vs shell form, like Docker/import.
250+
config["config"]["Cmd"] = super::import::parse_exec_or_shell(rest);
250251
} else if let Some(rest) = trimmed.strip_prefix("ENTRYPOINT ") {
251-
config["config"]["Entrypoint"] = serde_json::json!(["/bin/sh", "-c", rest]);
252+
config["config"]["Entrypoint"] = super::import::parse_exec_or_shell(rest);
252253
} else if let Some(rest) = trimmed.strip_prefix("ENV ") {
253-
if let Some((k, v)) = rest.split_once('=') {
254-
let env = config["config"]["Env"]
255-
.as_array_mut()
256-
.map(|a| a.clone())
254+
// Accept both KEY=VALUE and the legacy space-separated KEY VALUE form.
255+
if let Ok((k, v)) = super::import::parse_key_value(rest) {
256+
let mut env = config["config"]["Env"]
257+
.as_array()
258+
.cloned()
257259
.unwrap_or_default();
258-
let mut env = env;
259260
env.push(serde_json::json!(format!("{k}={v}")));
260261
config["config"]["Env"] = serde_json::json!(env);
261262
}
@@ -272,13 +273,12 @@ fn apply_changes(config: &mut serde_json::Value, changes: &[String]) {
272273
} else if let Some(rest) = trimmed.strip_prefix("USER ") {
273274
config["config"]["User"] = serde_json::json!(rest);
274275
} else if let Some(rest) = trimmed.strip_prefix("LABEL ") {
275-
if let Some((k, v)) = rest.split_once('=') {
276-
let labels = config["config"]["Labels"]
276+
if let Ok((k, v)) = super::import::parse_key_value(rest) {
277+
let mut labels = config["config"]["Labels"]
277278
.as_object()
278279
.cloned()
279280
.unwrap_or_default();
280-
let mut labels = labels;
281-
labels.insert(k.to_string(), serde_json::json!(v));
281+
labels.insert(k, serde_json::json!(v));
282282
config["config"]["Labels"] = serde_json::json!(labels);
283283
}
284284
}

src/cli/src/commands/image_inspect.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ fn build_image_inspect_json(
7575
"LayerCount": oci.layer_paths().len(),
7676
});
7777

78-
Ok(serde_json::to_string_pretty(&output)?)
78+
// Docker's inspect family always returns a top-level JSON array (even for a
79+
// single image), so tooling can do `inspect X | jq '.[0].Config'`.
80+
Ok(serde_json::to_string_pretty(&serde_json::json!([output]))?)
7981
}
8082

8183
#[cfg(test)]

src/cli/src/commands/image_tag.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,28 @@ pub struct ImageTagArgs {
1313
pub target: String,
1414
}
1515

16+
/// Validate a tag target's reference grammar. Docker requires the repository
17+
/// name (everything before the `:tag`/`@digest`) to be lowercase.
18+
fn validate_tag_target(target: &str) -> Result<(), String> {
19+
let without_digest = target.split('@').next().unwrap_or(target);
20+
let last_slash = without_digest.rfind('/');
21+
// Strip a trailing `:tag` only when the colon is part of the tag, not a
22+
// `registry:port`.
23+
let repo = match without_digest.rfind(':') {
24+
Some(colon) if last_slash.is_none_or(|slash| colon > slash) => &without_digest[..colon],
25+
_ => without_digest,
26+
};
27+
if repo.bytes().any(|b| b.is_ascii_uppercase()) {
28+
return Err(format!(
29+
"invalid reference format: repository name must be lowercase: '{target}'"
30+
));
31+
}
32+
Ok(())
33+
}
34+
1635
pub async fn execute(args: ImageTagArgs) -> Result<(), Box<dyn std::error::Error>> {
36+
validate_tag_target(&args.target)?;
37+
1738
let store = super::open_image_store()?;
1839
let images = store.list().await;
1940

@@ -47,6 +68,15 @@ mod tests {
4768
}
4869
}
4970

71+
#[test]
72+
fn test_validate_tag_target_rejects_uppercase_repo() {
73+
assert!(validate_tag_target("BadRepo:Tag").is_err());
74+
assert!(validate_tag_target("myrepo:V1").is_ok()); // uppercase tag is fine
75+
assert!(validate_tag_target("localhost:5000/myrepo:tag").is_ok());
76+
assert!(validate_tag_target("alpine:latest").is_ok());
77+
assert!(validate_tag_target("ns/Sub:tag").is_err());
78+
}
79+
5080
#[test]
5181
fn test_tag_source_resolution_accepts_normalized_alias() {
5282
let images = vec![stored("docker.io/library/alpine:latest")];

src/cli/src/commands/import.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ fn apply_changes(
209209
}
210210

211211
/// Parse a CMD/ENTRYPOINT value as a JSON array (exec form) or a shell string.
212-
fn parse_exec_or_shell(rest: &str) -> serde_json::Value {
212+
pub(crate) fn parse_exec_or_shell(rest: &str) -> serde_json::Value {
213213
let t = rest.trim();
214214
if t.starts_with('[') {
215215
if let Ok(v) = serde_json::from_str::<Vec<String>>(t) {
@@ -220,7 +220,7 @@ fn parse_exec_or_shell(rest: &str) -> serde_json::Value {
220220
serde_json::json!(["/bin/sh", "-c", t])
221221
}
222222

223-
fn parse_key_value(rest: &str) -> Result<(String, String), String> {
223+
pub(crate) fn parse_key_value(rest: &str) -> Result<(String, String), String> {
224224
if let Some((k, v)) = rest.split_once('=') {
225225
Ok((k.trim().to_string(), v.trim().to_string()))
226226
} else if let Some((k, v)) = rest.split_once(char::is_whitespace) {

src/cli/src/commands/rmi.rs

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,37 +27,48 @@ pub async fn execute(args: RmiArgs) -> Result<(), Box<dyn std::error::Error>> {
2727

2828
for query in &args.images {
2929
let images = store.list().await;
30-
let target = match image_usage::resolve_stored_image(&images, query) {
31-
Ok(Some(target)) => target,
32-
Ok(None) if args.force => continue,
33-
Ok(None) => {
34-
errors.push(format!("{query}: Image not found"));
35-
continue;
30+
// `--force` removes every reference matching the query (Docker: a digest
31+
// id with multiple tags is untagged-all); without force a single
32+
// unambiguous match is required.
33+
let targets: Vec<_> = if args.force {
34+
let all = image_usage::all_matching_images(&images, query);
35+
if all.is_empty() {
36+
continue; // force: ignore not-found
3637
}
37-
Err(error) => {
38-
errors.push(format!("{query}: {error}"));
39-
continue;
38+
all
39+
} else {
40+
match image_usage::resolve_stored_image(&images, query) {
41+
Ok(Some(target)) => vec![target],
42+
Ok(None) => {
43+
errors.push(format!("{query}: Image not found"));
44+
continue;
45+
}
46+
Err(error) => {
47+
errors.push(format!("{query}: {error}"));
48+
continue;
49+
}
4050
}
4151
};
4252

43-
if image_usage::is_protected_reference(&target.reference, &protected_images) {
44-
errors.push(format!(
45-
"{}: image is referenced by an existing box; remove the box before removing this image",
46-
target.reference
47-
));
48-
continue;
49-
}
50-
51-
match store.remove(&target.reference).await {
52-
Ok(()) => {
53-
println!("Removed: {}", target.reference);
53+
for target in targets {
54+
if image_usage::is_protected_reference(&target.reference, &protected_images) {
55+
errors.push(format!(
56+
"{}: image is referenced by an existing box; remove the box before removing this image",
57+
target.reference
58+
));
59+
continue;
5460
}
55-
Err(e) => {
56-
if args.force {
57-
// Silently skip not-found errors in force mode
58-
continue;
61+
62+
match store.remove(&target.reference).await {
63+
Ok(()) => {
64+
println!("Removed: {}", target.reference);
65+
}
66+
Err(e) => {
67+
if args.force {
68+
continue; // silently skip not-found in force mode
69+
}
70+
errors.push(format!("{}: {e}", target.reference));
5971
}
60-
errors.push(format!("{}: {e}", target.reference));
6172
}
6273
}
6374
}

src/cli/src/image_usage.rs

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,23 @@ pub fn resolve_stored_image(
120120
Ok(None)
121121
}
122122

123+
/// All images matching `query` (exact, then alias, then digest — first mode
124+
/// with any match wins). Used by `rmi --force`, where an ambiguous digest
125+
/// should remove every reference sharing it rather than erroring.
126+
pub fn all_matching_images(images: &[StoredImage], query: &str) -> Vec<StoredImage> {
127+
let query = query.trim();
128+
if query.is_empty() {
129+
return Vec::new();
130+
}
131+
for mode in [MatchMode::Exact, MatchMode::Alias, MatchMode::Digest] {
132+
let matches = matching_images(images, query, mode);
133+
if !matches.is_empty() {
134+
return matches;
135+
}
136+
}
137+
Vec::new()
138+
}
139+
123140
pub fn resolve_required_stored_image(
124141
images: &[StoredImage],
125142
query: &str,
@@ -179,17 +196,33 @@ fn ambiguous_reference_error(query: &str, matches: &[StoredImage]) -> String {
179196
}
180197

181198
fn is_digest_reference(reference: &str) -> bool {
182-
reference.starts_with("sha256:")
199+
if reference.starts_with("sha256:") {
200+
return true;
201+
}
202+
// A bare lowercase-hex string is a Docker image id (short id = 12 hex,
203+
// full = 64). Tags/repos that happen to be all-hex are still matched first
204+
// by Exact/Alias, so this only catches genuine id queries.
205+
let len = reference.len();
206+
(12..=64).contains(&len)
207+
&& reference
208+
.bytes()
209+
.all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b))
183210
}
184211

185212
fn digest_matches(stored_digest: &str, query: &str) -> bool {
186213
if stored_digest == query {
187214
return true;
188215
}
189-
let Some(query_hex) = query.strip_prefix("sha256:") else {
216+
// Accept both `sha256:<hex>` and a bare `<hex>` query, prefix-matching the
217+
// stored digest's hex (so `rmi 5b10f432ef3d` resolves the image).
218+
let query_hex = query.strip_prefix("sha256:").unwrap_or(query);
219+
if query_hex.is_empty() {
190220
return false;
191-
};
192-
!query_hex.is_empty() && stored_digest.starts_with(query)
221+
}
222+
let stored_hex = stored_digest
223+
.strip_prefix("sha256:")
224+
.unwrap_or(stored_digest);
225+
stored_hex.starts_with(query_hex)
193226
}
194227

195228
pub fn normalize_reference(reference: &str) -> String {
@@ -396,6 +429,21 @@ mod tests {
396429
assert!(digest_matches("sha256:abcdef123456", "sha256:abcdef123456"));
397430
assert!(digest_matches("sha256:abcdef123456", "sha256:abcdef"));
398431
assert!(!digest_matches("sha256:abcdef123456", "sha256:"));
399-
assert!(!digest_matches("sha256:abcdef123456", "abcdef"));
432+
// A bare hex query prefix-matches the stored digest's hex (Docker short id).
433+
assert!(digest_matches("sha256:abcdef123456", "abcdef"));
434+
assert!(!digest_matches("sha256:abcdef123456", "ffffff"));
435+
}
436+
437+
#[test]
438+
fn test_is_digest_reference_bare_hex_short_id() {
439+
assert!(is_digest_reference("sha256:abc"));
440+
// 12-hex Docker short id.
441+
assert!(is_digest_reference("5b10f432ef3d"));
442+
// Normal tags/repos are not treated as digests.
443+
assert!(!is_digest_reference("alpine"));
444+
assert!(!is_digest_reference("v1"));
445+
assert!(!is_digest_reference("latest"));
446+
// Uppercase hex is not a valid image id.
447+
assert!(!is_digest_reference("5B10F432EF3D"));
400448
}
401449
}

0 commit comments

Comments
 (0)