From f56b4e7f802279350ee38a64dc503ae7d87911d5 Mon Sep 17 00:00:00 2001 From: Jose <75870284+Jaro-c@users.noreply.github.com> Date: Fri, 25 Sep 2026 05:35:12 -0500 Subject: [PATCH 1/2] fix(libpod): send requests in origin form and keep the behaviour the compat path supplied (#1919) podup wrote every request line in absolute form (`POST http://localhost/v5.0.0/libpod/...`). Podman's `IsLibpodRequest` checks `strings.Split(r.URL.String(), "/")[2] == "libpod"`, which is `localhost` for that form, so every shared handler treated podup as a Docker client even on `/libpod/` paths. This switches `build_request` to origin form and compensates, endpoint by endpoint, everything the Docker-compatible path had been supplying implicitly, so podup behaves as it did in 5.10.0. What changed on the wire, read from Podman v5.7.0's handlers: | endpoint | what the compat path did | what podup sends now | |---|---|---| | `stop` | honoured `t` | `timeout` (libpod ignores `t`, so `podup stop -t N` would have been ignored) | | `restart` | honoured `t` | `timeout` (libpod treats a missing `timeout` as 0, an immediate kill) | | remove container | `v` removed anonymous volumes | `volumes` | | `kill` SIGKILL / 0 | waited until the container exited | a follow-up `wait` for exited/stopped | | archive PUT (`cp`) | `copyUIDGID` defaulted to false | `copyUIDGID=false` | | `top` | `ps_args` defaulted to `-ef` | `ps_args=-ef` | | `logs` | no multiplexing header for a TTY container | every `/logs` body parsed as multiplexed | | `events` | `died` rewritten to `die` (with `exitCode` copied), image `remove` to `delete` | the same two rewrites, done in podup | | `build` | `layers` forced to true, Docker manifest format, tags normalised to Docker Hub | `layers=true`, `outputformat` Docker v2, tags normalised the same way on the wire (the printed row keeps the short tag) | How I checked it did not regress: - The full live suite, one run at a time: 235 of 235 on Podman 5.7.0. - One live test per compensation. Removing each compensation on its own fails its test, except `kill`: SIGKILL lands in milliseconds, so the state is `exited` with or without the wait; a wire-shape unit test pins the follow-up call instead. - The 5.10.0 binary and this branch's binary ran the same 26 commands on the same compose file (build twice, up, ps, ps json, top, logs with a TTY service, exec, cp and the copied file's owner, port, images, the image's manifest type and HEALTHCHECK, pause/unpause, stop and restart timings, kill, events before and after `down`, `down -v` and the anonymous volume). That comparison found four regressions the tests had missed (TTY log bytes, built image names, the build row's printed name, `events` attributes), all fixed here. What still differs is the order of the build's `LABEL` pairs (it came from a `HashMap` before and is now fixed) and two `Successfully built/tagged` lines the compat handler added to the build's stderr. What does change on purpose: a short image name in a Containerfile's `FROM` is now resolved through `registries.conf`, as `podman build` and podup's own pulls already do, instead of being forced to Docker Hub. An ambiguous short name under Podman's enforcing short-name mode can now fail where it did not. Not measured: Windows (named pipe) and macOS (`podman machine`) against a real Podman. The request target is built the same way on every transport. Closes #1914 Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- internal/engine/build/body_plan.rs | 110 +++++ internal/engine/build/build_board_tests.rs | 109 ++++ internal/engine/build/build_query_tests.rs | 76 +++ internal/engine/build/extra_tags.rs | 68 +++ internal/engine/build/mod.rs | 3 + internal/engine/build/normalize.rs | 88 ++++ internal/engine/build/normalize_tests.rs | 65 +++ internal/engine/build/service.rs | 186 +++---- internal/engine/copy.rs | 2 +- internal/engine/copy/upload.rs | 24 +- internal/engine/copy_upload_tests.rs | 47 ++ internal/engine/events.rs | 209 ++++---- .../engine/events_compat_rewrite_tests.rs | 171 +++++++ internal/engine/events_since_validation.rs | 126 +++++ internal/engine/events_tests.rs | 169 ++++++- internal/engine/lifecycle/commands.rs | 2 +- .../lifecycle/libpod_endpoint_query_tests.rs | 423 ++++++++++++++++ internal/engine/lifecycle/mod.rs | 10 +- internal/engine/lifecycle/parallel.rs | 48 +- internal/engine/lifecycle/scale.rs | 4 +- internal/engine/lifecycle/signal.rs | 25 + internal/engine/lifecycle/signal_tests.rs | 25 +- internal/engine/lifecycle/targets.rs | 7 +- internal/engine/lifecycle/teardown_tests.rs | 28 +- internal/engine/query/attach.rs | 32 +- internal/engine/query/inspect.rs | 30 +- internal/engine/query/mod.rs | 49 +- internal/engine/watch/mod.rs | 2 +- internal/libpod/client/mod.rs | 36 +- .../libpod/client/pool/origin_form_tests.rs | 168 +++++++ internal/libpod/client/pool/tests.rs | 8 + internal/libpod/client/tests.rs | 80 ++- internal/libpod/mod.rs | 1 + internal/libpod/normalize.rs | 62 +++ internal/libpod/normalize_tests.rs | 76 +++ internal/libpod/types/image.rs | 8 + internal/libpod/validate.rs | 40 +- tests/engine_integration.rs | 144 ++++++ .../libpod_origin_form_build_comp.rs | 227 +++++++++ .../libpod_origin_form_io_comps.rs | 466 ++++++++++++++++++ .../libpod_origin_form_lifecycle_comps.rs | 303 ++++++++++++ 41 files changed, 3428 insertions(+), 329 deletions(-) create mode 100644 internal/engine/build/body_plan.rs create mode 100644 internal/engine/build/extra_tags.rs create mode 100644 internal/engine/build/normalize.rs create mode 100644 internal/engine/build/normalize_tests.rs create mode 100644 internal/engine/events_compat_rewrite_tests.rs create mode 100644 internal/engine/events_since_validation.rs create mode 100644 internal/engine/lifecycle/libpod_endpoint_query_tests.rs create mode 100644 internal/libpod/client/pool/origin_form_tests.rs create mode 100644 internal/libpod/normalize.rs create mode 100644 internal/libpod/normalize_tests.rs create mode 100644 tests/engine_integration/libpod_origin_form_build_comp.rs create mode 100644 tests/engine_integration/libpod_origin_form_io_comps.rs create mode 100644 tests/engine_integration/libpod_origin_form_lifecycle_comps.rs diff --git a/internal/engine/build/body_plan.rs b/internal/engine/build/body_plan.rs new file mode 100644 index 00000000..8d91d4ab --- /dev/null +++ b/internal/engine/build/body_plan.rs @@ -0,0 +1,110 @@ +//! Decide how the build context reaches libpod: a remote `remote=` URL +//! (no body), or a streamed context tar. +//! +//! Split out of `service.rs` so the dispatching loop there stays under +//! the source-line limit. The companion to +//! [`super::Engine::build_service`]: the body plan, the resolved +//! Dockerfile name and the in-tar build secret specs come back from +//! one call and feed straight into the `POST /libpod/build?` below. + +use std::path::PathBuf; + +use crate::compose::types::{BuildConfig, ComposeFile, Service}; +use crate::error::{ComposeError, Result}; + +use super::context::INLINE_DOCKERFILE_NAME; +use super::stream::ContextSource; +use super::tags::is_remote_context; +use super::{BodyPlan, Engine}; + +/// What the request body carries, plus the companion values needed +/// to build it: the resolved Dockerfile name and the in-tar build +/// secret specs. +pub(in crate::engine) struct BuildPlan { + /// Either `Empty` (remote context, no body) or `Stream { ... }` + /// for the local-directory tar path. + pub(in crate::engine) body: BodyPlan, + /// Dockerfile name as the libpod endpoint should see it. + pub(in crate::engine) dockerfile: String, + /// In-tar secret specs the build endpoint should mount. + pub(in crate::engine) secrets: Vec, +} + +/// Decide the build request body and the Dockerfile name. +/// +/// Remote (`git://`/`https://`/`git@`) contexts are cloned server-side +/// by Podman via the `remote` query parameter, so there is no local +/// directory to tar. Tar-only features (inline Dockerfile, in-tar +/// build secrets) do not apply and are warned about. Local contexts +/// pre-validate the directory exists so a missing context surfaces +/// here, with the resolved path, rather than as a bare `io error` +/// from the tar walk; resolve `build.secrets` to in-tar files so the +/// libpod endpoint sees the form it expects. +#[allow(clippy::too_many_arguments)] +pub(in crate::engine) fn plan_build( + engine: &Engine, + service_name: &str, + _service: &Service, + file: &ComposeFile, + build: &BuildConfig, + context_str: &str, +) -> Result { + if is_remote_context(context_str) { + tracing::info!("building from remote context {context_str}"); + if build.dockerfile_inline().is_some() { + tracing::warn!("build.dockerfile_inline is ignored for a remote build context"); + } + if !build.secrets().is_empty() { + tracing::warn!("build.secrets are ignored for a remote build context"); + } + let dockerfile = build.dockerfile().unwrap_or("Dockerfile").to_string(); + return Ok(BuildPlan { + body: BodyPlan::Empty, + dockerfile, + secrets: Vec::new(), + }); + } + let context_path: PathBuf = engine.base_dir.join(context_str); + if let Err(e) = std::fs::metadata(&context_path) { + return Err(ComposeError::BuildContext { + service: service_name.to_string(), + path: context_path.display().to_string(), + source: e, + }); + } + tracing::info!("building from {}", context_path.display()); + + let (secret_files, secrets) = engine.resolve_build_secrets(build, file)?; + + // The context tar is streamed to the socket (see the POST below), + // never buffered, so a multi-gigabyte context doesn't inflate RSS. + // Decide the source and the dockerfile name here; the blocking tar + // walk happens while the request body is being sent. + let (source, dockerfile) = match build.dockerfile_inline() { + Some(inline) => ( + ContextSource::Inline(inline.to_string()), + INLINE_DOCKERFILE_NAME.to_string(), + ), + None => { + let df = match build.dockerfile() { + Some(name) => name.to_string(), + None if !context_path.join("Dockerfile").is_file() + && context_path.join("Containerfile").is_file() => + { + "Containerfile".to_string() + } + None => "Dockerfile".to_string(), + }; + (ContextSource::Dockerfile(df.clone()), df) + } + }; + Ok(BuildPlan { + body: BodyPlan::Stream { + context: context_path, + source, + secrets: secret_files, + }, + dockerfile, + secrets, + }) +} diff --git a/internal/engine/build/build_board_tests.rs b/internal/engine/build/build_board_tests.rs index 3390163e..4f532a89 100644 --- a/internal/engine/build/build_board_tests.rs +++ b/internal/engine/build/build_board_tests.rs @@ -9,6 +9,8 @@ #[cfg(unix)] mod tests { + use std::sync::{Arc, Mutex}; + use crate::engine::fake_podman::{self, FakeReply}; use crate::engine::Engine; use crate::ui::progress::capture::Capture; @@ -234,6 +236,113 @@ services: ); } } + + /// A build with no `image:` field falls back to `-:latest` + /// as its primary tag. The print paths (the `Image` board row, the + /// `Building`/`Built` verbs, the `STEP n/m:` line prefix, the + /// `apply_extra_tags` skip) keep using that un-normalised form; only + /// the `t=` parameter on `/libpod/build` (and the source-side path of + /// `POST /libpod/images/{}/tag`) carries the docker.io canonical + /// form. Without the split, every board would get a second + /// `Image docker.io/library/proj-app:latest` row inserted on top of + /// the seeded `Image proj-app:latest` row (#1914). + #[tokio::test] + async fn build_row_keeps_unnormalised_tag_while_query_carries_the_canonical_form() { + // No `image:` so `primary_build_tag` falls back to + // `-:latest`. The fake returns a one-chunk + // success stream so `build_service` reaches the success path. + let dir = tempfile::tempdir().expect("tempdir"); + let ctx_path = dir.path().to_path_buf(); + std::fs::write( + ctx_path.join("Dockerfile"), + b"FROM docker.io/library/alpine:3.20\nRUN echo hi\nCMD [\"echo\",\"hi\"]\n", + ) + .expect("write Dockerfile"); + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let closure_requests = requests.clone(); + let fake = fake_podman::start_replying(move |method, target| { + closure_requests + .lock() + .unwrap() + .push(format!("{method} {target}")); + if method == "POST" && target.contains("/build?") { + FakeReply::ChunkedEnd(vec![ + "{\"stream\":\"--> sha256:1111111111111111111111111111111111111111111111111111111111111111\\n\"}\n".to_string(), + "{\"stream\":\"Successfully tagged proj-app:latest\\n\"}\n".to_string(), + ]) + } else if method == "POST" && target.contains("/images/") && target.contains("/tag") { + FakeReply::Body(200, String::new()) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.to_string()) + } + }); + let engine = Engine::with_base_dir(fake.client(), "proj".into(), ctx_path); + + let file = crate::parse_str("services:\n app:\n build:\n context: .\n") + .expect("the fixture parses"); + + let capture = Capture::start(); + engine + .build_all_with_options(&file, &[], &crate::engine::BuildOptions::default()) + .await + .expect("a build the fake accepts succeeds"); + + // Print path: the seeded and worked rows carry the + // un-normalised `-:latest` name. The bug would + // insert a second `Image docker.io/library/proj-app:latest` row + // on top of the seeded one and report both. + let names = capture.names(); + assert_eq!( + names, + vec!["proj-app:latest"], + "the printed row name must be the un-normalised -:latest \ + and never a second row under the docker.io canonical name: {names:?}" + ); + assert!( + !names.iter().any(|n| n.contains("docker.io")), + "no print path may show the docker.io canonical form: {names:?}" + ); + assert!( + !names.iter().any(|n| n.contains("localhost")), + "no print path may show the libpod `localhost/...` form: {names:?}" + ); + + // Verb path: every transition the board sees carries the same + // un-normalised name. A `Building` on `docker.io/library/...` + // would mean a second row opened under the wire name. + let verbs: Vec<(Kind, String, String)> = capture + .verbs() + .into_iter() + .filter(|(_, name, _)| name.contains("app") || name.contains("library")) + .collect(); + for (kind, name, _verb) in &verbs { + assert_eq!( + name, "proj-app:latest", + "verb path used the wire tag instead of the un-normalised form: {kind:?} {name:?}" + ); + } + + // Wire path: the `t=` parameter on `/libpod/build` carries the + // docker.io canonical form. The compat build handler did this + // through `NormalizeToDockerHub`; the libpod path has to do it + // itself. + let requests = requests.lock().unwrap().clone(); + let build_target = requests + .iter() + .find(|r| r.starts_with("POST ") && r.contains("/build?")) + .expect("a /build request was issued"); + assert!( + build_target.contains("t=docker.io%2Flibrary%2Fproj-app%3Alatest"), + "the build query must carry the canonical docker.io name in t=: {build_target}" + ); + // And never the un-normalised form on the wire - that is the + // exact regression this commit undoes. + assert!( + !build_target.contains("t=proj-app%3Alatest") + && !build_target.contains("t=proj-app:latest"), + "the build query must not carry the un-normalised form in t=: {build_target}" + ); + } } #[cfg(not(unix))] diff --git a/internal/engine/build/build_query_tests.rs b/internal/engine/build/build_query_tests.rs index 07321157..90feb1e8 100644 --- a/internal/engine/build/build_query_tests.rs +++ b/internal/engine/build/build_query_tests.rs @@ -283,6 +283,82 @@ mod tests { ); } + /// `layers=true` must be present on the build query. The Docker + /// compat handler used to default `layers` to true; the libpod + /// handler defaults it to false. podup's user-facing behaviour + /// is "the second build of the same Containerfile reuses the + /// cache", which requires `layers=true`. Sent once, not twice + /// (Podman rejects a duplicate param). + #[tokio::test] + async fn build_query_carries_layers_true() { + let (_dir, _fake, engine, _ctx, requests) = start_capture("proj"); + + let file = crate::parse_str( + "services:\n app:\n image: proj/img:1\n build:\n context: .\n", + ) + .unwrap(); + engine + .build_all_with_options(&file, &[], &crate::engine::BuildOptions::default()) + .await + .expect("a build the fake accepts succeeds"); + + let requests = requests.lock().unwrap().clone(); + let target = build_target(&requests); + let query = target + .split_once('?') + .expect("the build target carries a query string"); + + let layers_count = exact_param_count(query.1, "layers=true"); + assert_eq!( + layers_count, 1, + "the build query must carry `layers=true` exactly once, found {layers_count}: {query:?}" + ); + } + + /// `outputformat=application/vnd.docker.distribution.manifest.v2+json` + /// must be present on the build query. The libpod handler defaults + /// the manifest format to OCI, which does not reuse the layer cache + /// on a subsequent build of the same Containerfile and drops + /// `HEALTHCHECK` from the image config. Measured on 2026-09-24 + /// against Podman 5.7.0 by building the same Containerfile twice + /// through `/v5.0.0/libpod/build`: `layers=true` alone prints zero + /// `Using cache` lines on the second build; the same query with + /// `outputformat=application/vnd.docker.distribution.manifest.v2+json` + /// appended prints two. Sent once, not twice (Podman rejects a + /// duplicate param). + #[tokio::test] + async fn build_query_carries_docker_distribution_outputformat() { + let (_dir, _fake, engine, _ctx, requests) = start_capture("proj"); + + let file = crate::parse_str( + "services:\n app:\n image: proj/img:1\n build:\n context: .\n", + ) + .unwrap(); + engine + .build_all_with_options(&file, &[], &crate::engine::BuildOptions::default()) + .await + .expect("a build the fake accepts succeeds"); + + let requests = requests.lock().unwrap().clone(); + let target = build_target(&requests); + let query = target + .split_once('?') + .expect("the build target carries a query string"); + + // The exact-once count pins the wire shape so a future change + // cannot accidentally send the parameter twice and have Podman + // reject the request. + let outputformat_count = exact_param_count( + query.1, + "outputformat=application%2Fvnd.docker.distribution.manifest.v2%2Bjson", + ); + assert_eq!( + outputformat_count, 1, + "the build query must carry the docker-distribution outputformat exactly once, \ + found {outputformat_count}: {query:?}" + ); + } + /// The two labels are url-encoded into `key=value` form, just like every /// other value in this query string. A label value containing characters /// Podman's parser rejects when raw (`:`, `+`, `&`) must reach the diff --git a/internal/engine/build/extra_tags.rs b/internal/engine/build/extra_tags.rs new file mode 100644 index 00000000..158259a4 --- /dev/null +++ b/internal/engine/build/extra_tags.rs @@ -0,0 +1,68 @@ +//! Apply `build.tags` aliases to a freshly built image. +//! +//! Split out of `service.rs` so the dispatching loop there stays under +//! the source-line limit. The companion to +//! [`super::Engine::build_service`]: called once a build has produced +//! its primary tag and needs every other `build.tags` entry attached +//! to the same image. + +use crate::compose::types::BuildConfig; +use crate::error::{ComposeError, Result}; +use crate::libpod::urlencoded; +use crate::libpod::API_PREFIX; + +use super::Engine; + +impl Engine { + /// Apply any `build.tags` aliases to the freshly built image. + /// + /// `tag` is the un-normalised primary (what the print paths and the + /// `up` board row carry); the comparison against each `build.tags` + /// entry skips the alias that is the same un-normalised name. + /// `wire_tag` is the docker.io canonical form the build produced + /// (and the one `/libpod/images/{}/tag` actually has on disk), so + /// it is what the source-side path of the POST carries. + /// + /// Without the two-argument split the loop would either skip the + /// wrong alias (comparing the normalised wire_tag against the + /// un-normalised `build.tags` entry) or POST against an image the + /// daemon does not have (the un-normalised primary as the source + /// while the build landed under the normalised name). + pub(in crate::engine) async fn apply_extra_tags( + &self, + build: &BuildConfig, + tag: &str, + wire_tag: &str, + ) -> Result<()> { + for extra_tag in build.tags() { + if extra_tag == tag { + continue; + } + // The destination tag goes through the same docker.io + // canonical form the compat build handler applied via + // `NormalizeToDockerHub`. The libpod `/images/{}/tag` + // endpoint accepts any name but stores the image under + // whatever `repo:tag` it was given, so the only way to + // keep an unqualified `proj-extra:1` on the docker.io + // canonical form is to expand it here, before the POST. + let normalized = self.normalize_image_reference(extra_tag).await?; + let (repo, tag_str) = normalized + .rsplit_once(':') + .map(|(r, t)| (r.to_string(), t.to_string())) + .unwrap_or_else(|| (normalized.clone(), "latest".to_string())); + let encoded_source = urlencoded(wire_tag); + let tag_path = format!( + "{API_PREFIX}/images/{encoded_source}/tag?repo={}&tag={}", + urlencoded(&repo), + urlencoded(&tag_str), + ); + // Returning () here meant `build` could not report a failed tag at + // all: it exited 0 with the requested tags missing. + self.client + .post_empty_ok(&tag_path) + .await + .map_err(ComposeError::Podman)?; + } + Ok(()) + } +} diff --git a/internal/engine/build/mod.rs b/internal/engine/build/mod.rs index 8c628344..cd369d38 100644 --- a/internal/engine/build/mod.rs +++ b/internal/engine/build/mod.rs @@ -5,7 +5,10 @@ //! Podman libpod API, and applies any extra tags. Multi-stage targets are //! passed as the `target=` query parameter; the full Dockerfile is always sent. +mod body_plan; mod context; +mod extra_tags; +mod normalize; mod pull; mod push; mod secrets; diff --git a/internal/engine/build/normalize.rs b/internal/engine/build/normalize.rs new file mode 100644 index 00000000..51610113 --- /dev/null +++ b/internal/engine/build/normalize.rs @@ -0,0 +1,88 @@ +//! Image-name normalisation for the libpod build path. +//! +//! The docker-compat build handler applied +//! [`NormalizeToDockerHub`](https://github.com/containers/podman/blob/v5.7.0/pkg/api/handlers/utils/images.go) +//! to every tag it forwarded to `POST /libpod/build` and +//! `POST /libpod/images/{}/tag`. On the libpod path that helper +//! short-circuits and returns the input unchanged, so the canonical +//! form the user used to see (a `docker.io/library/...` prefix on +//! every unqualified name) was lost. `podup build` that previously +//! produced `docker.io/library/proj-app:latest` started producing +//! `localhost/proj-app:latest` instead. Every build left a second +//! copy of the image behind, and `ps`/`images`/`events` listed the +//! wrong one. +//! +//! This module restores the helper's two-step contract with the +//! [`Engine::normalize_image_reference`] entry point: +//! +//! 1. Look the name up in local storage via +//! `GET /libpod/images/{name}/json`. When the input resolves to +//! a local image, return its canonical `RepoTags` entry that +//! matches the input's tag (the upstream helper returns the +//! candidate resolved by the daemon, which carries the same +//! shape). +//! 2. Otherwise apply the pure normalisation rule +//! ([`super::super::super::libpod::normalize::normalize_docker_reference`]). +//! +//! A pure helper sits next to the wire shape so the unit test can +//! pin the rule without a live socket. The wire-shape lookup is +//! async because it has to. + +use crate::error::{ComposeError, Result}; +use crate::libpod::types::image::ImageInspect; +use crate::libpod::{normalize::normalize_docker_reference, urlencoded, API_PREFIX}; + +use super::Engine; + +impl Engine { + /// Resolve `name` to the docker.io canonical form, falling back to the + /// pure normalisation rule when no local image matches. + /// + /// See module docs for why the libpod build path needs this: the + /// docker-compat handler did it through `NormalizeToDockerHub`, + /// which is gated by `IsLibpodRequest` and short-circuits on + /// the libpod path. + pub(in crate::engine) async fn normalize_image_reference(&self, name: &str) -> Result { + let path = format!("{API_PREFIX}/images/{}/json", urlencoded(name)); + match self.client.get_json::(&path).await { + Ok(inspect) => { + if let Some(matched) = match_repo_tag(&inspect.repo_tags, name) { + return Ok(matched.to_string()); + } + } + Err(e) if e.is_status(404) => {} + Err(e) => return Err(ComposeError::Podman(e)), + } + Ok(normalize_docker_reference(name)) + } +} + +/// Pick the canonical `RepoTags` entry that matches `input`. +/// +/// The daemon's `LookupImage` resolves a short name through +/// `registries.conf` and returns the canonical form. When that form +/// matches one of the local image's `RepoTags`, that entry is the +/// canonical name podup has to keep: the user tagged it themselves +/// and a fresh normalisation would land on a different name. +/// +/// "Matches" means: same tag suffix when the input carries one, or +/// any entry when the input is a bare name. The first hit wins so +/// the result is deterministic across runs on the same local +/// storage state. +fn match_repo_tag<'a>(repo_tags: &'a [String], input: &str) -> Option<&'a str> { + let wanted_suffix = input.split_once(':').map(|(_, tag)| tag); + for tag in repo_tags { + if let Some(suffix) = wanted_suffix { + if tag.ends_with(&format!(":{suffix}")) { + return Some(tag.as_str()); + } + } else if !tag.contains(':') { + return Some(tag.as_str()); + } + } + repo_tags.first().map(String::as_str) +} + +#[cfg(test)] +#[path = "normalize_tests.rs"] +mod tests; diff --git a/internal/engine/build/normalize_tests.rs b/internal/engine/build/normalize_tests.rs new file mode 100644 index 00000000..6e5320ce --- /dev/null +++ b/internal/engine/build/normalize_tests.rs @@ -0,0 +1,65 @@ +//! Pure unit tests for the build-path image-name normalisation +//! helpers. +//! +//! The pure normalisation rule lives in +//! [`crate::libpod::normalize::normalize_docker_reference`] and is +//! tested there. This file pins the wire-shaped half: picking the +//! matching `RepoTags` entry from a local-image inspect response +//! when the input already resolves to a local image. The match +//! function is the one decision a unit test can drive without a +//! live socket. + +use crate::engine::build::normalize::match_repo_tag; + +#[test] +fn matches_when_input_tag_suffix_is_present() { + let tags = vec![ + "docker.io/library/alpine:3.20".to_string(), + "localhost/proj/app:v1".to_string(), + ]; + assert_eq!( + match_repo_tag(&tags, "proj/app:v1"), + Some("localhost/proj/app:v1"), + "the input `proj/app:v1` must pick the RepoTag that ends with `:v1` (the \ + daemon already resolved the name; the build must keep the same canonical form)" + ); +} + +#[test] +fn matches_when_input_has_no_tag() { + let tags = vec!["docker.io/library/alpine:3.20".to_string()]; + // A bare-name input cannot match a tagged RepoTag (no `:` in + // the input -> any entry with `:` is tagged, so we look for an + // entry that is itself bare). With no bare entry, fall back to + // the first RepoTag so the wire path still has a canonical name. + assert_eq!( + match_repo_tag(&tags, "alpine"), + Some("docker.io/library/alpine:3.20"), + "a bare-name input falls back to the first RepoTag when no untagged entry exists" + ); +} + +#[test] +fn empty_repo_tags_yields_none() { + let tags: Vec = Vec::new(); + assert!( + match_repo_tag(&tags, "anything").is_none(), + "an empty RepoTags list must produce no match; the caller then falls back to \ + the pure normalisation rule" + ); +} + +#[test] +fn picks_first_matching_tag_when_multiple_match() { + // Two RepoTags both end with `:v1`. The first one wins so the + // result is deterministic on the same local storage state. + let tags = vec![ + "docker.io/proj/app:v1".to_string(), + "localhost/proj/app:v1".to_string(), + ]; + assert_eq!( + match_repo_tag(&tags, "proj/app:v1"), + Some("docker.io/proj/app:v1"), + "the first matching RepoTag wins; the wire-shape contract is deterministic" + ); +} diff --git a/internal/engine/build/service.rs b/internal/engine/build/service.rs index b265cd5f..cff33aa0 100644 --- a/internal/engine/build/service.rs +++ b/internal/engine/build/service.rs @@ -1,23 +1,17 @@ //! Per-service build: build one `build:` block end-to-end. //! -//! Split out of `mod.rs` so the dispatching loop in `mod.rs` stays a thin -//! orchestrator and this file owns the 400-ish lines of URL/stream glue a -//! single build needs. The split is the one suggested by the engine style: -//! the entry point (`build_service`) and the small follow-up -//! (`apply_extra_tags`) sit here, alongside each other, since the row's -//! final state (`Built` / `Failed` / tagged aliases) is what closes the -//! build out and `build_service` calls into `apply_extra_tags` directly. -//! -//! Visibility is `pub(crate)` everywhere needed across this split: the -//! stream-helper types in `steps.rs`, the body-stream helpers in -//! `stream.rs`, and the context helpers in `tags.rs` and `context.rs` are -//! all called from here. +//! The dispatching loop in `mod.rs` calls into [`Engine::build_service`], +//! which is the URL/stream glue for one image: pick a body plan, +//! assemble the build query, drive the chunked response, paint the +//! board row. Each step is small enough that the whole thing lives +//! here. The body-plan decision lives in [`super::body_plan`]; the +//! `apply_extra_tags` follow-up lives in [`super::extra_tags`]. use std::io::IsTerminal; use bytes::Bytes; use futures_util::StreamExt; -use tracing::{info, warn}; +use tracing::warn; use crate::compose::types::{BuildConfig, Service}; use crate::error::{ComposeError, Result}; @@ -27,11 +21,12 @@ use crate::libpod::validate::pre_validate_build; use crate::libpod::API_PREFIX; use crate::size; +use super::body_plan::plan_build; use super::steps::{parse_image_id_line, BuildStreamProgress}; -use super::stream::{context_body, ContextSource}; -use super::tags::{is_remote_context, looks_like_secret}; +use super::stream::context_body; +use super::tags::looks_like_secret; use super::{context::map_additional_context, Engine}; -use super::{context::INLINE_DOCKERFILE_NAME, BodyPlan, BuildOptions}; +use super::{BodyPlan, BuildOptions}; impl Engine { pub(in crate::engine) async fn build_service( @@ -47,7 +42,14 @@ impl Engine { }; let context_str = build.context().to_string(); - let remote_context = is_remote_context(&context_str); + // `tag` is the un-normalised form the user (or `primary_build_tag`'s + // `-:latest` default) supplied. It is what every + // print path carries - the board row `up` and `build` seed, the + // `Building`/`Built` verbs, the `STEP n/m:` line prefix, the + // `fail_build` error message, and the `apply_extra_tags` comparison. + // Only the wire query (`t=` and `/images/{}/tag`) carries the + // docker.io canonical form, computed just before the query string is + // assembled below. let tag = super::primary_build_tag( &self.project, service_name, @@ -55,75 +57,10 @@ impl Engine { build.tags(), ); - // A Git/URL context is cloned server-side by Podman via the `remote` - // query parameter; there is no local directory to tar. Tar-only features - // (inline Dockerfile, in-tar build secrets) do not apply. - let (body_plan, dockerfile_name, secret_specs) = if remote_context { - info!("building {tag} from remote context {context_str}"); - if build.dockerfile_inline().is_some() { - warn!("build.dockerfile_inline is ignored for a remote build context"); - } - if !build.secrets().is_empty() { - warn!("build.secrets are ignored for a remote build context"); - } - let df = build.dockerfile().unwrap_or("Dockerfile").to_string(); - (BodyPlan::Empty, df, Vec::new()) - } else { - let context_path = self.base_dir.join(&context_str); - // Fail fast with the service name and the resolved context path if the - // directory is missing/unreadable, instead of a bare "io error: No such - // file or directory" once the context walk hits it. - if let Err(e) = std::fs::metadata(&context_path) { - return Err(ComposeError::BuildContext { - service: service_name.to_string(), - path: context_path.display().to_string(), - source: e, - }); - } - info!("building {tag} from {}", context_path.display()); - - // Resolve `build.secrets` to in-tar files before building the context: - // each secret value is shipped inside the build-context tar and - // referenced by a relative `src=` path, which is the form the libpod - // build endpoint expects (`env=`/host-path forms don't work reliably - // over the socket). - let (secret_files, secret_specs) = self.resolve_build_secrets(build, file)?; - - // The context tar is streamed to the socket (see the POST below), never - // buffered, so a multi-gigabyte context doesn't inflate RSS. Decide the - // source and the dockerfile name here; the blocking tar walk happens - // while the request body is being sent. - let (source, dockerfile_name) = match build.dockerfile_inline() { - Some(inline) => ( - ContextSource::Inline(inline.to_string()), - INLINE_DOCKERFILE_NAME.to_string(), - ), - None => { - // Honour an explicit dockerfile; otherwise prefer Dockerfile - // but fall back to Podman's native Containerfile when only the - // latter is present. - let df = match build.dockerfile() { - Some(name) => name.to_string(), - None if !context_path.join("Dockerfile").is_file() - && context_path.join("Containerfile").is_file() => - { - "Containerfile".to_string() - } - None => "Dockerfile".to_string(), - }; - (ContextSource::Dockerfile(df.clone()), df) - } - }; - ( - BodyPlan::Stream { - context: context_path, - source, - secrets: secret_files, - }, - dockerfile_name, - secret_specs, - ) - }; + let plan = plan_build(self, service_name, service, file, build, &context_str)?; + let body_plan = plan.body; + let dockerfile_name = plan.dockerfile; + let secret_specs = plan.secrets; let arg_map = build.args().to_map(); let mut build_args: std::collections::HashMap = @@ -167,8 +104,8 @@ impl Engine { ); } - let mut labels: std::collections::HashMap = - std::collections::HashMap::new(); + let mut labels: std::collections::BTreeMap = + std::collections::BTreeMap::new(); if let BuildConfig::Config { labels: l, .. } = build { labels.extend(l.to_map()); } @@ -177,7 +114,10 @@ impl Engine { // value. Without this, `build.labels: {podup.project: other}` would // make `podman image prune --filter label=podup.project=` // miss every image this build produced and reach for `other`'s - // instead. + // instead. A `BTreeMap` (rather than the `HashMap` this used to be) + // keeps the label order deterministic across builds, so a second + // `podup build` of the same Containerfile hits the buildkit layer + // cache instead of producing a different `LABEL` step every time. labels.insert("podup.project".to_string(), self.project.clone()); labels.insert("podup.service".to_string(), service_name.to_string()); @@ -256,10 +196,38 @@ impl Engine { // `/v5.0.0/libpod/build` leaked one buildah working container on // every run without `forcerm` (2 of 2) and on none of the runs // with it (0 of 2). + // + // `outputformat=application/vnd.docker.distribution.manifest.v2+json` + // forces the docker-distribution manifest format. Measured on + // 2026-09-24 against Podman 5.7.0 by building the same + // Containerfile twice through `/v5.0.0/libpod/build`: + // `layers=true` alone prints zero `Using cache` lines on the + // second build (the OCI format the endpoint defaults to does not + // reuse the layer cache); the same query with + // `outputformat=application/vnd.docker.distribution.manifest.v2+json` + // appended prints two. The Docker format also keeps + // `HEALTHCHECK` in the image config (the OCI format drops it), + // which `podup`'s `healthcheck:` field inherits when the user + // does not set one explicitly, so the same query preserves the + // image shape podup has always produced. + // + // `t=` carries the docker.io canonical form (the compat build + // handler applied `NormalizeToDockerHub`; the libpod path skips + // it via `IsLibpodRequest`, so `podup build` would otherwise + // land unqualified names as `localhost/-:latest` + // instead of `docker.io/library/-:latest`). + // The normalisation runs against a separate `wire_tag` rather + // than mutating `tag`: every print path (`Building`/`Built`, + // the board row `up` seeded, the `STEP n/m:` line prefix, the + // `apply_extra_tags` comparison) keeps the un-normalised form, + // which is what `podup ps`/`podup images` show and what the + // user used to see (#1914). + let wire_tag = self.normalize_image_reference(&tag).await?; let mut qs = format!( - "t={}&rm=true&forcerm=true&nocache={}", - urlencoded(&tag), - build.no_cache() || opts.no_cache + "t={}&rm=true&forcerm=true&layers=true&nocache={}&outputformat={}", + urlencoded(&wire_tag), + build.no_cache() || opts.no_cache, + urlencoded("application/vnd.docker.distribution.manifest.v2+json"), ); qs.push_str(&format!("&dockerfile={}", urlencoded(&dockerfile_name))); if build.pull() || opts.pull { @@ -326,7 +294,7 @@ impl Engine { ); } - if remote_context { + if matches!(body_plan, BodyPlan::Empty) { qs.push_str(&format!("&remote={}", urlencoded(&context_str))); } @@ -461,37 +429,7 @@ impl Engine { } } - self.apply_extra_tags(build, &tag).await?; - Ok(()) - } - - /// Apply any `build.tags` aliases to the freshly built image. - /// - /// The primary `tag` is skipped: when no `image:` is set it is already - /// `tags[0]`, which the build itself produced, so re-tagging it onto itself - /// would be a no-op API call. - async fn apply_extra_tags(&self, build: &BuildConfig, tag: &str) -> Result<()> { - for extra_tag in build.tags() { - if extra_tag == tag { - continue; - } - let (repo, tag_str) = extra_tag - .rsplit_once(':') - .map(|(r, t)| (r.to_string(), t.to_string())) - .unwrap_or_else(|| (extra_tag.clone(), "latest".to_string())); - let encoded_tag = urlencoded(tag); - let tag_path = format!( - "{API_PREFIX}/images/{encoded_tag}/tag?repo={}&tag={}", - urlencoded(&repo), - urlencoded(&tag_str), - ); - // Returning () here meant `build` could not report a failed tag at - // all: it exited 0 with the requested tags missing. - self.client - .post_empty_ok(&tag_path) - .await - .map_err(ComposeError::Podman)?; - } + self.apply_extra_tags(build, &tag, &wire_tag).await?; Ok(()) } } diff --git a/internal/engine/copy.rs b/internal/engine/copy.rs index 1929ebc9..69300031 100644 --- a/internal/engine/copy.rs +++ b/internal/engine/copy.rs @@ -22,7 +22,7 @@ mod pack; pub(in crate::engine) mod pack_common; mod progress; mod stream; -mod upload; +pub(in crate::engine) mod upload; pub(in crate::engine) mod verify; /// Re-export the watch-sync packer at the engine level so the watch module diff --git a/internal/engine/copy/upload.rs b/internal/engine/copy/upload.rs index 1fa93b34..f14b333d 100644 --- a/internal/engine/copy/upload.rs +++ b/internal/engine/copy/upload.rs @@ -54,11 +54,7 @@ impl Engine { packed: PackedStream, uploaded_kind: Option, ) -> Result<()> { - let path = format!( - "{API_PREFIX}/containers/{}/archive?path={}", - urlencoded(container), - urlencoded(dir), - ); + let path = archive_put_path(container, dir); let verify_path = (!entry.is_empty()).then(|| { format!( "{API_PREFIX}/containers/{}/archive?path={}", @@ -219,6 +215,24 @@ impl Engine { } } +/// Build the libpod archive-PUT path. `copyUIDGID=false` overrides the +/// libpod default of `true`, which would otherwise overwrite the host +/// UID/GID on the destination file with the container's runtime +/// UID/GID. The Docker compat handler defaulted `copyUIDGID` to +/// false, so this is the line that keeps podup's user-visible +/// behaviour stable across the switch (#1914). +/// +/// Public to `crate::engine` so the wire-shape unit tests in +/// `engine::lifecycle::libpod_endpoint_query_tests` can pin the +/// query string without standing up the streaming packer. +pub(in crate::engine) fn archive_put_path(container: &str, dir: &str) -> String { + format!( + "{API_PREFIX}/containers/{}/archive?path={}©UIDGID=false", + urlencoded(container), + urlencoded(dir), + ) +} + /// Wait for the pack task and return the recorded entry list. A pack error /// short-circuits here so the verification step never runs against a /// partially-built list, and the caller sees the original "permission diff --git a/internal/engine/copy_upload_tests.rs b/internal/engine/copy_upload_tests.rs index 43f634a4..dad4c5d8 100644 --- a/internal/engine/copy_upload_tests.rs +++ b/internal/engine/copy_upload_tests.rs @@ -567,6 +567,53 @@ async fn a_pack_error_midway_reaches_the_caller_as_an_error() { ); } +/// The libpod `/archive` PUT defaults `copyUIDGID` to true, which makes +/// a host file copied into a container take the container's runtime +/// UID/GID (i.e. `0:0`). The Docker compat handler defaulted it to +/// false, which preserved the host UID/GID on the destination file +/// (as measured with podup 5.10.0 on 2026-09-24: `1000:1000`). The compensation pins the +/// docker-compat default: the PUT query must include +/// `copyUIDGID=false`, and the docker-side key (`copyUIDGID=true`) +/// must not appear. The URL helper is pinned separately in +/// `engine::lifecycle::libpod_endpoint_query_tests`; this is the +/// end-to-end wire check the streaming packer drives. +#[tokio::test] +async fn upload_carries_copy_uid_gid_false() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("hello.txt"); + std::fs::write(&src, b"hi").unwrap(); + + // A clean 200 from the PUT (Podman 5) is enough: the upload is the + // path under test, not the apply-then-close confirmation. + let fake = fake_podman::start_replying(move |method, target| { + if method == "PUT" && target.contains("/archive?") { + FakeReply::Body(200, String::new()) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + + let result = upload(&fake, &src, "hello.txt", None).await; + result.expect("a clean PUT is reported as success"); + + let requests = fake.requests.lock().unwrap(); + let put_req = requests + .iter() + .find(|r| r.starts_with("PUT ") && r.contains("/archive?")) + .expect("a /archive PUT was issued"); + let query = put_req + .split_once('?') + .expect("the archive PUT carries a query string"); + assert!( + query.1.contains("copyUIDGID=false"), + "libpod archive PUT must read `copyUIDGID=false`: {put_req:?}" + ); + assert!( + !query.1.split('&').any(|pair| pair == "copyUIDGID=true"), + "the libpod default of `copyUIDGID=true` must not be sent as-is: {put_req:?}" + ); +} + // Links and the non-regular kinds (FIFOs) live in their own file so this one // stays under the line limit. A child module, so it reaches the fixtures above // through `super::` without widening their visibility. diff --git a/internal/engine/events.rs b/internal/engine/events.rs index a3cd1094..177e800f 100644 --- a/internal/engine/events.rs +++ b/internal/engine/events.rs @@ -191,120 +191,14 @@ impl Engine { } } -/// Reject a `--since` written as a negative relative duration. -/// -/// libpod reads a relative `since` as a time before now, so `30m` is thirty -/// minutes ago and `-30m` is thirty minutes in the future: a window that -/// starts there matches nothing and the feed looks empty (#1896). A plain -/// negative number is left alone, though: libpod's `ParseInputTime` parses -/// numeric values as Unix timestamps before trying them as durations, so a -/// pre-epoch lower bound like `--since -1` is a valid replay window, and -/// `-0s`/`-0m` are zero offsets (i.e. "now"). The check matches Go's -/// duration syntax exactly on the part after `-`: one or more segments, -/// each a number immediately followed by a unit from `ns`, `us`, `µs`, -/// `ms`, `s`, `m`, `h`, with at least one non-zero digit overall. Anything -/// that fails to parse that way is forwarded unchanged so `1e3`, negative -/// timestamps and zero offsets still reach libpod. -fn validate_events_since(since: Option<&str>) -> Result<()> { - let Some(v) = since else { - return Ok(()); - }; - let Some(rest) = v.strip_prefix('-') else { - return Ok(()); - }; - if is_go_duration(rest) { - return Err(ComposeError::Unsupported(format!( - "invalid --since value {v:?}: a relative time counts back from now, so write it without the leading '-' (e.g. --since {rest})" - ))); - } - Ok(()) -} - -/// Exact Go duration syntax for the part after a leading `-`. Returns -/// `true` only when `rest` is one or more segments, each a number (`123`, -/// `1.5`, `.5`, `1.`) immediately followed by a unit from `ns`, `us`, -/// `µs`, `ms`, `s`, `m`, `h`, with no trailing characters and at least one -/// non-zero digit overall. Used only by [`validate_events_since`]. -/// -/// Hand-written on purpose: the inputs are short and a regex crate would -/// be heavier than the parser it would replace. Kept as small as the -/// surface it has to cover. -fn is_go_duration(rest: &str) -> bool { - let mut chars = rest.chars().peekable(); - let mut has_non_zero_overall = false; +/// Reject a `--since` written as a negative relative duration. The +/// matcher is lifted out to [`super::since_validation`] so this file +/// stays under the source-line budget; the contract is unchanged +/// (#1896). +#[path = "events_since_validation.rs"] +mod since_validation; - loop { - let mut saw_digit = false; - let mut segment_has_non_zero = false; - - while let Some(&c) = chars.peek() { - if c.is_ascii_digit() { - chars.next(); - saw_digit = true; - if c != '0' { - segment_has_non_zero = true; - } - } else { - break; - } - } - - if chars.peek() == Some(&'.') { - chars.next(); - let mut frac_has_digit = false; - while let Some(&c) = chars.peek() { - if c.is_ascii_digit() { - chars.next(); - frac_has_digit = true; - if c != '0' { - segment_has_non_zero = true; - } - } else { - break; - } - } - // Go's `time.ParseDuration` accepts `.5` and `1.`, but not a bare - // `.`. A segment needs at least one digit on one side of the dot. - if !saw_digit && !frac_has_digit { - return false; - } - saw_digit = saw_digit || frac_has_digit; - } - - if !saw_digit { - // No number for the unit to attach to. - return false; - } - - // A unit must follow the number; a bare `5` without a unit is not a - // Go duration. - match chars.next() { - Some('n') | Some('u') | Some('\u{00B5}') => { - if chars.peek() != Some(&'s') { - return false; - } - chars.next(); - } - Some('m') => { - if chars.peek() == Some(&'s') { - chars.next(); - } - } - Some('s') | Some('h') => {} - _ => return false, - } - - if segment_has_non_zero { - has_non_zero_overall = true; - } - - if chars.peek().is_none() { - break; - } - } - - has_non_zero_overall -} +pub(super) use since_validation::validate_events_since; /// Build the libpod events `filters` object: always scope to this project's /// `podup.project` label, then merge each user `KEY=VALUE` predicate (appending @@ -338,6 +232,67 @@ fn build_event_filters(project: &str, user_filters: &[String]) -> Result Ok(Value::Object(map)) } +/// Rewrite a libpod-native event into the docker-compat shape podup has +/// always exposed: `status` becomes `Action`, and the two verbs the +/// docker-compat handler rewrote (`Type == "image" && Action == +/// "remove" -> `delete`; `Action == "died" -> `die`) carry over. The +/// `died` rewrite also copies `Actor.Attributes.containerExitCode` +/// into `Actor.Attributes.exitCode` alongside; `containerExitCode` +/// stays in place so a caller reading either key still finds its +/// value (#1914). +/// +/// The compat handler in `pkg/api/handlers/compat/events.go` +/// (Podman v5.7.0) does exactly this and nothing more; this mirrors +/// it line for line. A wider rewrite (e.g. promoting `remove -> delete` +/// for every `Type`, or copying `exitCode` on every event that +/// carries `containerExitCode`) would diverge from the docker-compat +/// JSON a script already parses, and would add fields that no +/// docker-compat event ever published (#1914). +/// +/// Pure so the rewrite is unit-tested without a live socket. +fn rename_event(value: &Value) -> Value { + let mut out = value.clone(); + if let Some(obj) = out.as_object_mut() { + // The verb lives in `status` on libpod and in `Action` on docker- + // compat. Read whichever is there so the compat handler's two rules + // (which check `Action`) apply to libpod-shaped input too. + let action = obj + .get("Action") + .or_else(|| obj.get("status")) + .and_then(Value::as_str) + .map(str::to_owned); + let typ = obj.get("Type").and_then(Value::as_str).unwrap_or(""); + if let Some(action) = action { + if typ == "image" && action == "remove" { + // Compat rule 1: image removal is `delete`, on both keys. + obj.insert("Action".to_string(), Value::String("delete".to_string())); + obj.insert("status".to_string(), Value::String("delete".to_string())); + } else if action == "died" { + // Compat rule 2: a container death is `die`, on both keys, + // and the libpod `containerExitCode` is copied into the + // docker-compat `exitCode` alongside. + obj.insert("Action".to_string(), Value::String("die".to_string())); + obj.insert("status".to_string(), Value::String("die".to_string())); + if let Some(actor) = obj.get_mut("Actor").and_then(Value::as_object_mut) { + if let Some(attrs) = actor.get_mut("Attributes").and_then(Value::as_object_mut) + { + if let Some(code) = attrs.get("containerExitCode").cloned() { + attrs.entry("exitCode".to_string()).or_insert(code); + } + } + } + } else { + // No compat rule applies. The verb still lands under `Action` + // so a `--format json` consumer that parses `Action` finds + // it; the libpod `status` key stays too, so a script keyed + // on either shape still reads its value. + obj.insert("Action".to_string(), Value::String(action)); + } + } + } + out +} + /// Render one event. `json` emits the raw object as a compact line; otherwise a /// `TYPE ACTION NAME` summary, tolerant of both the docker-compat shape /// (`Type`/`Action`/`Actor.Attributes.name`) and the libpod-native one @@ -349,7 +304,13 @@ fn format_event(value: &Value, json: bool) -> String { // truncating the NDJSON stream. Surface the cause at `debug` (the // operator who runs with `RUST_LOG=debug` sees why one row is // missing), drop the row, and let the stream continue (#1366). - return match super::to_query_json("events row", value) { + // + // The same docker-compat shape is honoured on the JSON path: verbs + // are rewritten in a clone of the object so the wire bytes still + // carry `Action=die` and `Action=delete` (and the libpod-style + // `status=...` is preserved alongside, for callers that keyed on + // it). Exit-code renaming is applied below in [`rename_event`]. + return match super::to_query_json("events row", &rename_event(value)) { Ok(s) => s, Err(e) => { tracing::debug!("events: dropping unserialisable row: {e}"); @@ -358,11 +319,25 @@ fn format_event(value: &Value, json: bool) -> String { }; } let typ = value.get("Type").and_then(Value::as_str).unwrap_or(""); - let action = value + let action_raw = value .get("Action") .or_else(|| value.get("status")) .and_then(Value::as_str) .unwrap_or(""); + // Mirror the docker-compat events handler: `Type == "image" && + // Action == "remove" -> delete` (a container removal stays + // `remove`); `Action == "died" -> die`; anything else passes + // through. The exit-code copy lives in [`rename_event`] and is + // irrelevant on the table path (the table does not render + // `Actor.Attributes`) (#1914). + let action = if typ == "image" && action_raw == "remove" { + "delete" + } else { + match action_raw { + "died" => "die", + other => other, + } + }; let name = value .pointer("/Actor/Attributes/name") .or_else(|| value.get("id")) @@ -468,6 +443,10 @@ fn format_event_line( #[path = "events_tests.rs"] mod tests; +#[cfg(test)] +#[path = "events_compat_rewrite_tests.rs"] +mod compat_rewrite_tests; + #[cfg(test)] #[path = "events_event_colour_tests.rs"] mod event_colour_tests; diff --git a/internal/engine/events_compat_rewrite_tests.rs b/internal/engine/events_compat_rewrite_tests.rs new file mode 100644 index 00000000..29d1a4b8 --- /dev/null +++ b/internal/engine/events_compat_rewrite_tests.rs @@ -0,0 +1,171 @@ +use super::{format_event, rename_event}; +use serde_json::{json, Value}; + +/// Mirror the docker-compat events handler exactly: `Action`/`status` are +/// only rewritten for the two cases the handler rewrote. +/// +/// Fixture is built from the libpod shape (verb in `status`, no `Action`), +/// since that is what podup reads from `libpod/events`. A `container` +/// death is the case the compat handler rewrites to `die` while copying +/// `containerExitCode` into `exitCode`; both keys carry the same value +/// afterwards so a script keyed on either still finds it (#1914). +#[test] +fn died_container_action_and_status_become_die_with_exit_code_keys_equal() { + let v = json!({ + "Type": "container", + "status": "died", + "Actor": { + "Attributes": { + "name": "web-1", + "containerExitCode": "3", + } + } + }); + let out = rename_event(&v); + assert_eq!( + out.get("Action").and_then(Value::as_str), + Some("die"), + "libpod `died` must promote to docker-compat `die`: {out:?}" + ); + assert_eq!( + out.get("status").and_then(Value::as_str), + Some("die"), + "the compat handler sets `status` too; libpod `died` must become `die`: {out:?}" + ); + assert_eq!( + out.pointer("/Actor/Attributes/exitCode") + .and_then(Value::as_str), + Some("3"), + "the docker-compat `exitCode` key must hold the containerExitCode value: {out:?}" + ); + assert_eq!( + out.pointer("/Actor/Attributes/containerExitCode") + .and_then(Value::as_str), + Some("3"), + "the libpod `containerExitCode` key must stay alongside the docker-compat key: {out:?}" + ); +} + +/// `exec_died` is a distinct verb (it signals an `exec` session ending, +/// not the container itself dying). The compat handler only rewrote +/// `Action == "died"`, so `exec_died` must pass through unchanged and +/// must NOT pick up an `exitCode` copy: the previous code copied +/// `containerExitCode` into `exitCode` whenever the attribute existed, +/// regardless of verb, and `podman exec` events carry the libpod key +/// even though no docker-compat `exitCode` was ever published for them +/// (#1914). +#[test] +fn exec_died_event_passes_through_with_no_exit_code_copy() { + let v = json!({ + "Type": "container", + "status": "exec_died", + "Actor": { + "Attributes": { + "name": "web-1", + "containerExitCode": "137", + } + } + }); + let out = rename_event(&v); + assert_eq!( + out.get("Action").and_then(Value::as_str), + Some("exec_died"), + "exec_died must not be collapsed into `die`: {out:?}" + ); + assert_eq!( + out.get("status").and_then(Value::as_str), + Some("exec_died"), + "the libpod status key must stay at exec_died: {out:?}" + ); + assert!( + out.pointer("/Actor/Attributes/exitCode").is_none(), + "exec_died must not gain an exitCode copy: {out:?}" + ); +} + +/// A container removal is `Action=remove` in libpod, and that is what +/// podup has always emitted on the docker-compat path. The compat +/// handler only rewrote `remove` -> `delete` when `Type == "image"`, +/// so a container `remove` must pass through untouched. The previous +/// code rewrote it for every `Type`, which turned a container removal +/// into a `delete` and silently changed the verb a `--filter event=...` +/// call had to match (#1914). +#[test] +fn container_remove_event_passes_through_unchanged() { + let v = json!({ + "Type": "container", + "status": "remove", + "Actor": { "Attributes": { "name": "web-1" } } + }); + let out = rename_event(&v); + assert_eq!( + out.get("Action").and_then(Value::as_str), + Some("remove"), + "container remove must stay `remove`; compat only rewrites it for `Type == image`: {out:?}" + ); + assert_eq!( + out.get("status").and_then(Value::as_str), + Some("remove"), + "the libpod status key must stay at remove for a container: {out:?}" + ); +} + +/// An image removal is `Action=remove` in libpod and `Action=delete` in +/// docker-compat: that is the rewrite the compat handler applied, on +/// `Type == "image"` AND `Action == "remove"` only. Both `Action` and +/// `status` are set to `delete`, mirroring the handler's two writes +/// (#1914). +#[test] +fn image_remove_event_action_and_status_become_delete() { + let v = json!({ + "Type": "image", + "status": "remove", + "Actor": { "Attributes": { "name": "img-1" } } + }); + let out = rename_event(&v); + assert_eq!( + out.get("Action").and_then(Value::as_str), + Some("delete"), + "image remove must become `delete`: {out:?}" + ); + assert_eq!( + out.get("status").and_then(Value::as_str), + Some("delete"), + "the compat handler sets `status` to `delete` too: {out:?}" + ); +} + +/// The table path reads the same two rules. A container `remove` +/// stays `remove` on the line a reader scans, and an image `remove` +/// renders as `delete` (the docker-compat verb the table has always +/// shown). The rule is the same one the JSON path applies (#1914). +#[test] +fn table_path_mirrors_compat_image_only_remove_rewrite() { + let container = json!({ + "Type": "container", + "status": "remove", + "id": "web-1", + "time": 0, + }); + let out = format_event(&container, false); + assert!( + out.contains(" remove "), + "container `remove` must stay `remove` on the table path: {out:?}" + ); + assert!( + !out.contains(" delete "), + "container `remove` must not be rewritten as `delete` on the table path: {out:?}" + ); + + let image = json!({ + "Type": "image", + "status": "remove", + "id": "img-1", + "time": 0, + }); + let out = format_event(&image, false); + assert!( + out.contains(" delete "), + "image `remove` must render as `delete` on the table path: {out:?}" + ); +} diff --git a/internal/engine/events_since_validation.rs b/internal/engine/events_since_validation.rs new file mode 100644 index 00000000..51314942 --- /dev/null +++ b/internal/engine/events_since_validation.rs @@ -0,0 +1,126 @@ +//! `events --since` validator. +//! +//! Lifted from `events.rs` so the orchestration in that file stays +//! under the 500-line source budget; the validator is a pure function +//! with its own concerns (a hand-written Go duration matcher that +//! rejects only negative relative times and forwards everything else +//! to libpod unchanged). The tests live next to the orchestration +//! file (`events_tests.rs`) and reach the validator through +//! `super::since_validation`. + +use crate::error::{ComposeError, Result}; + +/// Reject a `--since` written as a negative relative duration. +/// +/// libpod reads a relative `since` as a time before now, so `30m` is thirty +/// minutes ago and `-30m` is thirty minutes in the future: a window that +/// starts there matches nothing and the feed looks empty (#1896). A plain +/// negative number is left alone, though: libpod's `ParseInputTime` parses +/// numeric values as Unix timestamps before trying them as durations, so a +/// pre-epoch lower bound like `--since -1` is a valid replay window, and +/// `-0s`/`-0m` are zero offsets (i.e. "now"). The check matches Go's +/// duration syntax exactly on the part after `-`: one or more segments, +/// each a number immediately followed by a unit from `ns`, `us`, `µs`, +/// `ms`, `s`, `m`, `h`, with at least one non-zero digit overall. Anything +/// that fails to parse that way is forwarded unchanged so `1e3`, negative +/// timestamps and zero offsets still reach libpod. +pub(crate) fn validate_events_since(since: Option<&str>) -> Result<()> { + let Some(v) = since else { + return Ok(()); + }; + let Some(rest) = v.strip_prefix('-') else { + return Ok(()); + }; + if is_go_duration(rest) { + return Err(ComposeError::Unsupported(format!( + "invalid --since value {v:?}: a relative time counts back from now, so write it without the leading '-' (e.g. --since {rest})" + ))); + } + Ok(()) +} + +/// Exact Go duration syntax for the part after a leading `-`. Returns +/// `true` only when `rest` is one or more segments, each a number (`123`, +/// `1.5`, `.5`, `1.`) immediately followed by a unit from `ns`, `us`, +/// `µs`, `ms`, `s`, `m`, `h`, with no trailing characters and at least one +/// non-zero digit overall. Used only by [`validate_events_since`]. +/// +/// Hand-written on purpose: the inputs are short and a regex crate would +/// be heavier than the parser it would replace. Kept as small as the +/// surface it has to cover. +fn is_go_duration(rest: &str) -> bool { + let mut chars = rest.chars().peekable(); + let mut has_non_zero_overall = false; + + loop { + let mut saw_digit = false; + let mut segment_has_non_zero = false; + + while let Some(&c) = chars.peek() { + if c.is_ascii_digit() { + chars.next(); + saw_digit = true; + if c != '0' { + segment_has_non_zero = true; + } + } else { + break; + } + } + + if chars.peek() == Some(&'.') { + chars.next(); + let mut frac_has_digit = false; + while let Some(&c) = chars.peek() { + if c.is_ascii_digit() { + chars.next(); + frac_has_digit = true; + if c != '0' { + segment_has_non_zero = true; + } + } else { + break; + } + } + // Go's `time.ParseDuration` accepts `.5` and `1.`, but not a bare + // `.`. A segment needs at least one digit on one side of the dot. + if !saw_digit && !frac_has_digit { + return false; + } + saw_digit = saw_digit || frac_has_digit; + } + + if !saw_digit { + // No number for the unit to attach to. + return false; + } + + // A unit must follow the number; a bare `5` without a unit is not a + // Go duration. + match chars.next() { + Some('n') | Some('u') | Some('\u{00B5}') => { + if chars.peek() != Some(&'s') { + return false; + } + chars.next(); + } + Some('m') => { + if chars.peek() == Some(&'s') { + chars.next(); + } + } + Some('s') | Some('h') => {} + _ => return false, + } + + if segment_has_non_zero { + has_non_zero_overall = true; + } + + if chars.peek().is_none() { + break; + } + } + + has_non_zero_overall +} diff --git a/internal/engine/events_tests.rs b/internal/engine/events_tests.rs index 925d42e4..b2e291d9 100644 --- a/internal/engine/events_tests.rs +++ b/internal/engine/events_tests.rs @@ -1,8 +1,9 @@ use super::{ - build_event_filters, format_event, validate_events_since, Engine, EventsOptions, TIME_WIDTH, + build_event_filters, format_event, rename_event, validate_events_since, Engine, EventsOptions, + TIME_WIDTH, }; use crate::libpod::Client; -use serde_json::json; +use serde_json::{json, Value}; #[test] fn build_event_filters_scopes_to_project_label() { @@ -77,6 +78,124 @@ fn formats_libpod_native_shape() { ); } +/// The libpod `/events` endpoint names a container's death `died` (not +/// `die`); podup's user-facing output keeps the docker-compat verb so a +/// `--filter event=die` still matches a container death on libpod +/// (#1914). Other verbs pass through under the `Action` key. The +/// full compat rewrite rules (image+remove -> delete, died -> die + +/// exitCode copy, container remove unchanged) are pinned in +/// `events_compat_rewrite_tests`. +#[test] +fn rename_event_promotes_status_to_action_and_rewrites_died() { + for (raw, want_action, want_status) in [ + ("died", "die", "die"), + ("start", "start", "start"), + ("die", "die", "die"), + ("delete", "delete", "delete"), + ] { + let v = json!({ "Type": "container", "status": raw }); + let out = rename_event(&v); + assert_eq!( + out.get("Action").and_then(|v| v.as_str()), + Some(want_action), + "verb {raw:?} did not map to Action={want_action:?}: {out:?}" + ); + assert_eq!( + out.get("status").and_then(|v| v.as_str()), + Some(want_status), + "verb {raw:?} did not map to status={want_status:?}: {out:?}" + ); + } +} + +/// `Actor.Attributes.containerExitCode` (libpod) is copied into the +/// docker-compat `Actor.Attributes.exitCode` so a script that reads the +/// docker-compat key still finds the value, while a script that reads +/// the libpod key still finds its value (#1914). The compat build +/// handler did the same: `exitCode` was set from `containerExitCode` +/// while `containerExitCode` stayed put. Removing the libpod key would +/// silently break every caller keyed on it. +#[test] +fn rename_event_copies_container_exit_code_into_exit_code() { + let v = json!({ + "Type": "container", + "status": "died", + "Actor": { + "Attributes": { + "name": "web-1", + "containerExitCode": "3", + } + } + }); + let out = rename_event(&v); + assert_eq!( + out.pointer("/Actor/Attributes/exitCode") + .and_then(|v| v.as_str()), + Some("3"), + "containerExitCode was not copied into exitCode: {out:?}" + ); + assert_eq!( + out.pointer("/Actor/Attributes/containerExitCode") + .and_then(|v| v.as_str()), + Some("3"), + "containerExitCode must remain alongside the docker-compat exitCode: {out:?}" + ); + assert_eq!( + out.pointer("/Actor/Attributes/name") + .and_then(|v| v.as_str()), + Some("web-1"), + "name must remain: {out:?}" + ); +} + +/// The compat handler set both `status` and `Action` when it rewrote a +/// verb (`status` is the libpod-native verb key, `Action` is the +/// docker-compat one; both land at `die` for a container death). +/// Verbs that are not rewritten pass through unchanged under both +/// keys (#1914). +#[test] +fn rename_event_keeps_status_alongside_action() { + let v = json!({ + "Type": "container", + "status": "died", + "Actor": { "Attributes": { "name": "web-1" } }, + }); + let out = rename_event(&v); + assert_eq!( + out.get("Action").and_then(Value::as_str), + Some("die"), + "status=died must become Action=die: {out:?}" + ); + assert_eq!( + out.get("status").and_then(Value::as_str), + Some("die"), + "status=died must become status=die (the compat handler set both keys): {out:?}" + ); +} + +/// The table form must render `died` as `die` and `remove` as `delete`, +/// the docker-compat verbs podup has always printed (#1914). +#[test] +fn table_form_renders_libpod_verbs_as_docker_compat() { + let died = json!({ "Type": "container", "status": "died", "id": "web-1", "time": 0 }); + let out = format_event(&died, false); + assert!( + out.contains("die"), + "libpod `died` must render as docker-compat `die`: {out:?}" + ); + assert!( + !out.contains("died"), + "libpod `died` must not appear in the table form: {out:?}" + ); + + let remove = json!({ "Type": "image", "status": "remove", "id": "img-1", "time": 0 }); + let out = format_event(&remove, false); + assert!( + out.contains("delete"), + "libpod `remove` must render as docker-compat `delete`: {out:?}" + ); +} + #[test] fn json_mode_emits_raw_object() { let ev = json!({ "Type": "container", "Action": "start" }); @@ -85,6 +204,52 @@ fn json_mode_emits_raw_object() { assert!(out.contains("\"Action\":\"start\"")); } +/// The JSON mode routes through `rename_event`, which copies +/// `containerExitCode` into `exitCode` while leaving the libpod key in +/// place. A `--format json` consumer keyed on `containerExitCode` +/// (libpod scripts predating #1914) and one keyed on `exitCode` +/// (docker-compat scripts) both find the same value (#1914). +#[test] +fn json_mode_emits_both_exit_code_keys() { + let ev = json!({ + "Type": "container", + "status": "died", + "Actor": { + "Attributes": { + "name": "web-1", + "containerExitCode": "137", + } + } + }); + let out = format_event(&ev, true); + let parsed: serde_json::Value = + serde_json::from_str(out.trim()).expect("json mode emits one JSON object per line"); + assert_eq!( + parsed + .pointer("/Actor/Attributes/containerExitCode") + .and_then(|v| v.as_str()), + Some("137"), + "the libpod containerExitCode key must remain on the JSON wire: {parsed:?}" + ); + assert_eq!( + parsed + .pointer("/Actor/Attributes/exitCode") + .and_then(|v| v.as_str()), + Some("137"), + "the docker-compat exitCode key must be set on the JSON wire: {parsed:?}" + ); + assert_eq!( + parsed.get("Action").and_then(Value::as_str), + Some("die"), + "libpod `died` must promote to docker-compat `die` on the JSON wire: {parsed:?}" + ); + assert_eq!( + parsed.get("status").and_then(Value::as_str), + Some("die"), + "the compat handler rewrote `status` to `die` too: {parsed:?}" + ); +} + /// #1896: libpod reads a relative `since` as a time before now, so `-30m` is /// thirty minutes in the future and a window starting there matches nothing. /// The validator has to reject it before any request hits libpod. diff --git a/internal/engine/lifecycle/commands.rs b/internal/engine/lifecycle/commands.rs index 502007bd..3eebfeb8 100644 --- a/internal/engine/lifecycle/commands.rs +++ b/internal/engine/lifecycle/commands.rs @@ -178,7 +178,7 @@ impl Engine { /// from the response (#1363). pub(super) async fn stop_container(&self, container: &str, grace: i32) -> Result { let path = format!( - "{API_PREFIX}/containers/{}/stop?t={}", + "{API_PREFIX}/containers/{}/stop?timeout={}", crate::libpod::urlencoded(container), stop_timeout_param(grace), ); diff --git a/internal/engine/lifecycle/libpod_endpoint_query_tests.rs b/internal/engine/lifecycle/libpod_endpoint_query_tests.rs new file mode 100644 index 00000000..51ca851a --- /dev/null +++ b/internal/engine/lifecycle/libpod_endpoint_query_tests.rs @@ -0,0 +1,423 @@ +//! Wire-shape tests for the query-string compensations the libpod endpoint +//! shape requires. +//! +//! The Docker compat handler and the libpod handler share the URL path but +//! differ on which query keys they read. podup used to talk to libpod +//! over the absolute-form request line, which routed every call to the +//! Docker compat handlers, where the docker-side query keys were the +//! ones in scope. Switching the request line to origin form moves +//! every call to the libpod handlers, which read a different subset: +//! `timeout=` (not `t=`), `volumes=` (not `v=`), `copyUIDGID=false` +//! (the default flips), `ps_args=-ef` (the default flips), and +//! `layers=true` on the build endpoint (the default flips). Each +//! compensation here asserts that the call carries the libpod key and +//! not the docker key, on the wire shape captured by the fake podman +//! the build/lifecycle tests already use. + +#![cfg(unix)] + +use crate::engine::fake_podman::{self, FakeReply}; +use crate::engine::Engine; +use crate::libpod::API_PREFIX; +use std::sync::{Arc, Mutex}; + +fn engine_with(client: crate::libpod::Client, project: &str) -> Engine { + Engine::with_base_dir(client, project.into(), std::env::temp_dir()) +} + +/// `POST /containers/{}/stop` must carry `timeout=` and not `t=`. The +/// libpod handler reads `timeout=`; the Docker compat handler reads +/// `t=`, so an unchanged `t=` query key leaves the libpod handler +/// ignoring the grace period entirely (defaulting to the container's +/// own stop timeout, or to the daemon default when there is none). +/// This is the unit test for the compensation in `commands.rs::stop_container`. +#[tokio::test] +async fn stop_sends_timeout_query_param() { + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let req_clone = requests.clone(); + let fake = fake_podman::start_replying(move |method, target| { + req_clone.lock().unwrap().push(format!("{method} {target}")); + if method == "POST" && target.contains("/stop?") { + FakeReply::Body(204, String::new()) + } else if method == "GET" && target.contains("/containers/json") { + FakeReply::Body( + 200, + r#"[{"Names":["/proj-web-1"],"State":"running","Labels":{"podup.service":"web"}}]"# + .into(), + ) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + let engine = engine_with(fake.client(), "proj"); + + // A service whose `stop_grace_period` is 7 seconds. The helper pins the + // value the libpod side must see. + let file = + crate::parse_str("services:\n web:\n image: x\n stop_grace_period: 7s\n").unwrap(); + engine + .stop(&file, &["web".into()]) + .await + .expect("a stop the fake accepts succeeds"); + + let stop_req = { + let requests = requests.lock().unwrap(); + requests + .iter() + .find(|r| r.starts_with("POST ") && r.contains("/stop?")) + .expect("a /stop request was issued") + .clone() + }; + let query = stop_req + .split_once('?') + .expect("the stop target carries a query string"); + assert!( + query.1.contains("timeout=7"), + "the libpod stop endpoint must read `timeout=`, not `t=`: {stop_req:?}" + ); + assert!( + !query.1.split('&').any(|pair| pair.starts_with("t=")), + "the docker `t=` must not appear alongside `timeout=` on libpod: {stop_req:?}" + ); +} + +/// `POST /containers/{}/restart` must carry `timeout=` and not `t=`. The +/// libpod handler ignores `t=` and defaults `timeout=` to 0 when absent, +/// which would make every restart an immediate SIGKILL (a single-replica +/// `restart` on libpod would never give the container a chance to +/// drain). This is the unit test for `parallel.rs::restart_one_service` +/// and the watch restart at `watch/mod.rs::watch_restart`. +#[tokio::test] +async fn restart_sends_timeout_query_param() { + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let req_clone = requests.clone(); + let fake = fake_podman::start_replying(move |method, target| { + req_clone.lock().unwrap().push(format!("{method} {target}")); + if method == "POST" && target.contains("/restart?") { + FakeReply::Body(204, String::new()) + } else if method == "GET" && target.contains("/containers/json") { + FakeReply::Body( + 200, + r#"[{"Names":["/proj-web-1"],"State":"running","Labels":{"podup.service":"web"}}]"# + .into(), + ) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + let engine = engine_with(fake.client(), "proj"); + + let file = + crate::parse_str("services:\n web:\n image: x\n stop_grace_period: 5s\n").unwrap(); + engine + .restart(&file, Some("web")) + .await + .expect("a restart the fake accepts succeeds"); + + let restart_req = { + let requests = requests.lock().unwrap(); + requests + .iter() + .find(|r| r.starts_with("POST ") && r.contains("/restart?")) + .expect("a /restart request was issued") + .clone() + }; + let query = restart_req + .split_once('?') + .expect("the restart target carries a query string"); + assert!( + query.1.contains("timeout="), + "the libpod restart endpoint must read `timeout=`: {restart_req:?}" + ); + assert!( + !query.1.split('&').any(|pair| pair.starts_with("t=")), + "the docker `t=` must not appear alongside `timeout=` on libpod: {restart_req:?}" + ); +} + +/// `DELETE /containers/{}` must carry `volumes=` (not `v=`) when the +/// caller asked for anonymous-volume removal. The libpod handler +/// reads `volumes=`; the Docker compat handler reads `v=`, which the +/// libpod handler ignores, so a `down -v` against libpod without +/// `volumes=` reclaims nothing. This is the unit test for the +/// `container_rm_path` helper. +#[tokio::test] +async fn down_v_sends_volumes_query_param() { + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let req_clone = requests.clone(); + let fake = fake_podman::start_replying(move |method, target| { + req_clone.lock().unwrap().push(format!("{method} {target}")); + if method == "DELETE" && target.contains("/containers/") { + FakeReply::Body(204, String::new()) + } else if method == "GET" && target.contains("/containers/json") { + FakeReply::Body( + 200, + r#"[{"Names":["/proj-web-1"],"State":"running","Labels":{"podup.service":"web"}}]"# + .into(), + ) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + let engine = engine_with(fake.client(), "proj"); + + let file = crate::parse_str("services:\n web:\n image: x\n").unwrap(); + engine + .down_with_options(&file, true) + .await + .expect("a down the fake accepts succeeds"); + + let del_req = { + let requests = requests.lock().unwrap(); + requests + .iter() + .find(|r| r.starts_with("DELETE ") && r.contains("/containers/proj-web-1?")) + .expect("a DELETE for the container was issued") + .clone() + }; + let query = del_req + .split_once('?') + .expect("the delete target carries a query string"); + assert!( + query.1.contains("volumes=true"), + "libpod delete must read `volumes=true`, not `v=true`: {del_req:?}" + ); + assert!( + !query.1.split('&').any(|pair| pair == "v=true"), + "the docker `v=true` must not appear alongside `volumes=true` on libpod: {del_req:?}" + ); + assert!( + query.1.contains("force=true"), + "`down` tears down a running container, so `force=true` must still be present: {del_req:?}" + ); +} + +/// `GET /containers/{}/top` must carry `ps_args=-ef`. The libpod handler +/// defaults `ps_args` to its own descriptors (a different column +/// set), so podup's user-facing `top` output would drift the moment +/// a service moved off a default-configured image. +#[tokio::test] +async fn top_sends_ps_args_query_param() { + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let req_clone = requests.clone(); + let fake = fake_podman::start_replying(move |method, target| { + req_clone.lock().unwrap().push(format!("{method} {target}")); + if method == "GET" && target.contains("/top?") { + FakeReply::Body( + 200, + r#"{"Titles":["UID","PID","PPID","C","STIME","TTY","TIME","CMD"],"Processes":[["root","1","0","0","15:00","?","00:00:00","sleep 3600"]]}"# + .into(), + ) + } else if method == "GET" && target.contains("/containers/json") { + FakeReply::Body( + 200, + r#"[{"Names":["/proj-web-1"],"State":"running","Labels":{"podup.service":"web"}}]"# + .into(), + ) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + let engine = engine_with(fake.client(), "proj"); + + let file = crate::parse_str("services:\n web:\n image: x\n").unwrap(); + engine + .top_with_options(&file, &[], false) + .await + .expect("a top the fake accepts succeeds"); + + let top_req = { + let requests = requests.lock().unwrap(); + requests + .iter() + .find(|r| r.starts_with("GET ") && r.contains("/top?")) + .expect("a /top request was issued") + .clone() + }; + let query = top_req + .split_once('?') + .expect("the top target carries a query string"); + assert!( + query.1.contains("ps_args=-ef"), + "the libpod top endpoint must read `ps_args=-ef`: {top_req:?}" + ); +} + +/// `PUT /containers/{}/archive` must carry `copyUIDGID=false`. The +/// libpod handler defaults `copyUIDGID` to true, which makes a host +/// file copied into a container take the container's runtime UID/GID +/// (i.e. `0:0`). The Docker compat handler defaulted it to false, +/// which preserved the host UID/GID on the destination file (as measured with podup 5.10.0 on +/// 2026-09-24: `1000:1000`). The compensation pins the +/// docker-compat default. +/// +/// The wire-level test for the compensation lives next to the rest of +/// the `cp` upload tests in `engine::copy::upload_tests::upload_carries_copy_uid_gid_false`, +/// which drives the same fake podman through the streaming packer +/// `cp_to_container` uses (the packer is private to the `copy` +/// module). What is pinned here is the URL helper the production path +/// is built on, so a regression that dropped the parameter is caught +/// even when the streaming packer cannot be reached. +#[test] +fn archive_put_path_includes_copy_uid_gid_false() { + let path = crate::engine::copy::upload::archive_put_path("proj-web-1", "/tmp"); + assert!( + path.contains("copyUIDGID=false"), + "libpod archive PUT must carry `copyUIDGID=false`: {path}" + ); + assert!( + !path.split('&').any(|pair| pair == "copyUIDGID=true"), + "the libpod default of `copyUIDGID=true` must not be sent as-is: {path}" + ); +} + +/// `POST /containers/{}/kill?signal=SIGKILL` must be followed by a +/// `POST /containers/{}/wait?condition=stopped` on libpod. The Docker +/// compat `/kill` handler blocks on the container when the signal is +/// SIGKILL/9/KILL/0; the libpod handler replies immediately. Without +/// the follow-up wait, a caller that relied on the compat handler's +/// semantics would observe a still-running container when `kill` +/// returned. SIGTERM and the other graceful signals do not block +/// either way and the wait is skipped. +#[tokio::test] +async fn kill_with_sigkill_sends_follow_up_wait() { + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let req_clone = requests.clone(); + let fake = fake_podman::start_replying(move |method, target| { + req_clone.lock().unwrap().push(format!("{method} {target}")); + if method == "POST" && target.contains("/kill?") { + FakeReply::Body(204, String::new()) + } else if method == "POST" && target.contains("/wait?") { + // The wait endpoint returns an HTTP 200 with the exit code as + // its body; a plain `Body(200, "0")` is the libpod shape. + FakeReply::Body(200, "0".to_string()) + } else if method == "GET" && target.contains("/containers/json") { + FakeReply::Body( + 200, + r#"[{"Names":["/proj-web-1"],"State":"running","Labels":{"podup.service":"web"}}]"# + .into(), + ) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + let engine = engine_with(fake.client(), "proj"); + + let file = crate::parse_str("services:\n web:\n image: x\n").unwrap(); + engine + .kill(&file, &[], "SIGKILL") + .await + .expect("a kill the fake accepts succeeds"); + + let requests = requests.lock().unwrap(); + let kill_req = requests + .iter() + .find(|r| r.starts_with("POST ") && r.contains("/kill?")) + .expect("a /kill request was issued"); + assert!( + kill_req.contains("signal=SIGKILL"), + "the kill must carry the requested signal: {kill_req:?}" + ); + let wait_req = requests + .iter() + .find(|r| r.starts_with("POST ") && r.contains("/wait?")) + .expect("a /wait?condition=stopped follow-up must follow the kill"); + assert!( + wait_req.contains("condition=stopped"), + "the follow-up wait must pin the container's stopped state: {wait_req:?}" + ); +} + +/// The opposite case: SIGTERM is graceful and the libpod handler +/// replies promptly anyway, so a `kill -s SIGTERM ` must NOT pin +/// the caller behind a per-container `/wait?condition=stopped`. The +/// compensation's contract is that the wait fires for SIGKILL/9/KILL/0 +/// only; adding it for SIGTERM is what would turn a 50-replica `kill` +/// into 50 sequential waits. +#[tokio::test] +async fn kill_with_sigterm_skips_the_follow_up_wait() { + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let req_clone = requests.clone(); + let fake = fake_podman::start_replying(move |method, target| { + req_clone.lock().unwrap().push(format!("{method} {target}")); + if method == "POST" && target.contains("/kill?") { + FakeReply::Body(204, String::new()) + } else if method == "GET" && target.contains("/containers/json") { + FakeReply::Body( + 200, + r#"[{"Names":["/proj-web-1"],"State":"running","Labels":{"podup.service":"web"}}]"# + .into(), + ) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + let engine = engine_with(fake.client(), "proj"); + + let file = crate::parse_str("services:\n web:\n image: x\n").unwrap(); + engine + .kill(&file, &[], "SIGTERM") + .await + .expect("a kill the fake accepts succeeds"); + + let requests = requests.lock().unwrap(); + assert!( + requests + .iter() + .any(|r| r.starts_with("POST ") && r.contains("/kill?signal=SIGTERM")), + "a /kill?signal=SIGTERM must be issued: {requests:?}" + ); + assert!( + !requests + .iter() + .any(|r| r.starts_with("POST ") && r.contains("/wait?")), + "SIGTERM must not trigger a follow-up /wait?condition=stopped: {requests:?}" + ); +} + +/// Spot-check the constants and the request path used by the build +/// endpoint. The build endpoint itself is covered by +/// `engine::build::query_tests::build_query_carries_layers_true`; +/// what is pinned here is that the URL the build request is built +/// against still points at the libpod `/build` path (a regression +/// here would mean the compensation went to the wrong place). +#[tokio::test] +async fn build_request_path_targets_libpod_build() { + let fake = fake_podman::start_replying(move |method, target| { + if method == "POST" && target.contains("/build?") { + FakeReply::ChunkedEnd(vec![ + "{\"stream\":\"--> sha256:1111111111111111111111111111111111111111111111111111111111111111\\n\"}\n".to_string(), + "{\"stream\":\"Successfully tagged proj/img:1\\n\"}\n".to_string(), + ]) + } else if method == "POST" && target.contains("/images/") && target.contains("/tag") { + FakeReply::Body(200, String::new()) + } else { + FakeReply::Body(404, r#"{"message":"not found"}"#.into()) + } + }); + + let dir = tempfile::tempdir().unwrap(); + let ctx = dir.path().to_path_buf(); + std::fs::write(ctx.join("Dockerfile"), b"FROM alpine:latest\nRUN echo hi\n").unwrap(); + let engine = Engine::with_base_dir(fake.client(), "proj".into(), ctx.clone()); + let file = crate::parse_str( + "services:\n app:\n image: proj/img:1\n build:\n context: .\n", + ) + .unwrap(); + engine + .build_all_with_options(&file, &[], &crate::engine::BuildOptions::default()) + .await + .expect("a build the fake accepts succeeds"); + + // The full build path lands at `${API_PREFIX}/build?...`; a regression + // that moved the target outside `/libpod/` would let the docker + // compat handler answer again and the `layers=true` compensation be + // for nothing. + let requests = fake.requests.lock().unwrap(); + assert!( + requests + .iter() + .any(|r| { r.starts_with("POST ") && r.contains(&format!("{API_PREFIX}/build?")) }), + "a POST to `{API_PREFIX}/build?...` must be issued: {requests:?}" + ); +} diff --git a/internal/engine/lifecycle/mod.rs b/internal/engine/lifecycle/mod.rs index c7562bc0..3ed518a9 100644 --- a/internal/engine/lifecycle/mod.rs +++ b/internal/engine/lifecycle/mod.rs @@ -165,10 +165,11 @@ impl Engine { /// the only way image `VOLUME` directives and short-form anonymous volumes get /// removed: podup never names or labels them, so they cannot be enumerated and /// deleted the way declared top-level volumes are. -pub(super) fn container_rm_path(name: &str, remove_volumes: bool) -> String { - let with_volumes = if remove_volumes { "&v=true" } else { "" }; +pub(super) fn container_rm_path(name: &str, remove_volumes: bool, force: bool) -> String { + let force_str = if force { "true" } else { "false" }; + let with_volumes = if remove_volumes { "&volumes=true" } else { "" }; format!( - "{API_PREFIX}/containers/{}?force=true{with_volumes}", + "{API_PREFIX}/containers/{}?force={force_str}{with_volumes}", crate::libpod::urlencoded(name), ) } @@ -176,6 +177,9 @@ pub(super) fn container_rm_path(name: &str, remove_volumes: bool) -> String { #[cfg(test)] mod drop_recheck_tests; #[cfg(test)] +#[path = "libpod_endpoint_query_tests.rs"] +mod libpod_endpoint_query_tests; +#[cfg(test)] #[path = "scale_request_tests.rs"] mod scale_request_tests; #[cfg(test)] diff --git a/internal/engine/lifecycle/parallel.rs b/internal/engine/lifecycle/parallel.rs index 0751e757..83ebd818 100644 --- a/internal/engine/lifecycle/parallel.rs +++ b/internal/engine/lifecycle/parallel.rs @@ -211,7 +211,7 @@ impl Engine { // Single atomic restart (no visible stopped window) instead of a // stop+start round-trip. let restart_path = format!( - "{API_PREFIX}/containers/{}/restart?t={}", + "{API_PREFIX}/containers/{}/restart?timeout={}", urlencoded(&container_name), stop_timeout_param(grace), ); @@ -237,6 +237,14 @@ impl Engine { acted: &std::sync::atomic::AtomicBool, ) -> Result<()> { let mut first_err: Option = None; + // The Docker compat `/kill` handler blocks on the container when the + // signal is SIGKILL/KILL/9 (or 0), and returns once the container has + // exited or stopped; the libpod handler replies immediately. The + // compensation is a follow-up `wait?condition=stopped` for those + // signals only; SIGTERM etc. are answered promptly either way, and + // skipping the wait is what keeps `kill -s SIGTERM ` from pinning + // the caller behind every container the user happens to target. + let wait_for_exit = super::signal::must_wait_after_kill(signal); for container_name in container_names { let path = format!( "{API_PREFIX}/containers/{}/kill?signal={}", @@ -247,7 +255,12 @@ impl Engine { .run_lifecycle_op(&path, &container_name, "Killed", LifecycleGoal::NotRunning) .await { - Ok(true) => acted.store(true, std::sync::atomic::Ordering::Relaxed), + Ok(true) => { + acted.store(true, std::sync::atomic::Ordering::Relaxed); + if wait_for_exit { + self.wait_after_kill(&container_name).await; + } + } Ok(false) => {} Err(e) => { first_err.get_or_insert(e); @@ -257,6 +270,27 @@ impl Engine { first_err.map_or(Ok(()), Err) } + /// Block until `container` is exited or stopped, the way the Docker + /// compat `/kill` handler did for SIGKILL/0. Capped by [`READ_TIMEOUT`] + /// so a stuck wait cannot pin the CLI forever; the kill itself + /// already succeeded, so a timeout here is logged at warn and not + /// promoted to an error. + async fn wait_after_kill(&self, container: &str) { + let path = format!( + "{API_PREFIX}/containers/{}/wait?condition=stopped", + urlencoded(container), + ); + match self.client.post_empty_json_unbounded::(&path).await { + Ok(_code) => {} + Err(e) => { + tracing::warn!( + "kill {container}: wait for exited/stopped did not complete [{e}]; \ + the container is left to settle on its own" + ); + } + } + } + /// Remove a single service's containers. See [`Engine::rm_with_options`]. pub(super) async fn rm_one_service( &self, @@ -267,11 +301,7 @@ impl Engine { ) -> Result<()> { let mut first_err: Option = None; for container_name in container_names { - let force_str = if force { "true" } else { "false" }; - let path = format!( - "{API_PREFIX}/containers/{}?force={force_str}&v={remove_volumes}", - urlencoded(&container_name), - ); + let path = super::container_rm_path(&container_name, remove_volumes, force); // The row opens with its working verb so it carries a start time and // `Removed` comes with the elapsed time, the way `down` reports the // same removal (#1686). Every arm closes the row: one left at @@ -365,7 +395,7 @@ impl Engine { // does not pin recreation for the full client READ_TIMEOUT; the // force-remove below SIGKILLs it regardless. let stop_path = format!( - "{API_PREFIX}/containers/{}/stop?t={}", + "{API_PREFIX}/containers/{}/stop?timeout={}", urlencoded(container_name), stop_timeout_param(grace), ); @@ -385,7 +415,7 @@ impl Engine { } } - let rm_path = super::container_rm_path(container_name, remove_volumes); + let rm_path = super::container_rm_path(container_name, remove_volumes, true); match self.client.delete_ok(&rm_path).await { Ok(()) => { crate::ui::progress_line("Container", container_name, "Removed"); diff --git a/internal/engine/lifecycle/scale.rs b/internal/engine/lifecycle/scale.rs index 25b2ae0b..c1514fe7 100644 --- a/internal/engine/lifecycle/scale.rs +++ b/internal/engine/lifecycle/scale.rs @@ -228,7 +228,7 @@ impl Engine { // (#1686). crate::ui::progress::start("Container", name, "Stopping"); let stop_path = format!( - "{API_PREFIX}/containers/{}/stop?t={}", + "{API_PREFIX}/containers/{}/stop?timeout={}", urlencoded(name), stop_timeout_param(grace), ); @@ -237,7 +237,7 @@ impl Engine { .post_empty_ok_within(&stop_path, stop_deadline(grace)) .await; crate::ui::progress::start("Container", name, "Removing"); - let rm_path = super::container_rm_path(name, remove_volumes); + let rm_path = super::container_rm_path(name, remove_volumes, true); match self.client.delete_ok(&rm_path).await { Ok(()) => { crate::ui::progress_line("Container", name, "Removed"); diff --git a/internal/engine/lifecycle/signal.rs b/internal/engine/lifecycle/signal.rs index 6887f027..b9685c50 100644 --- a/internal/engine/lifecycle/signal.rs +++ b/internal/engine/lifecycle/signal.rs @@ -48,6 +48,31 @@ pub(crate) fn validate_signal(signal: &str) -> Result<()> { } } +/// Whether a `kill` whose target signal is `signal` must be followed by a +/// `wait?condition=stopped` to match the Docker compat handler's behaviour. +/// +/// The compat handler blocks until the container has exited or stopped on +/// signals the kernel delivers unconditionally: SIGKILL (`9`, `KILL`, +/// `SIGKILL`) and signal `0` (the existence probe). libpod's +/// `/containers/{id}/kill` answers immediately for every signal, so a caller +/// that relied on the compat handler's wait would observe a still-running +/// container when `kill` returned. SIGTERM and the other "graceful" signals +/// stay fast on the libpod side; adding a wait there would pin every `kill +/// -s SIGTERM` behind every targeted container even when nothing required +/// it. Pure so the rule is unit-tested without a socket. +pub(crate) fn must_wait_after_kill(signal: &str) -> bool { + let trimmed = signal.trim(); + if trimmed.is_empty() { + return false; + } + if trimmed.chars().all(|c| c.is_ascii_digit()) { + return matches!(trimmed.parse::(), Ok(0) | Ok(9)); + } + let upper = trimmed.to_ascii_uppercase(); + let name = upper.strip_prefix("SIG").unwrap_or(&upper); + name == "KILL" +} + #[cfg(test)] #[path = "signal_tests.rs"] mod tests; diff --git a/internal/engine/lifecycle/signal_tests.rs b/internal/engine/lifecycle/signal_tests.rs index d358d500..775b3926 100644 --- a/internal/engine/lifecycle/signal_tests.rs +++ b/internal/engine/lifecycle/signal_tests.rs @@ -1,4 +1,4 @@ -use super::validate_signal; +use super::{must_wait_after_kill, validate_signal}; use crate::error::ComposeError; #[test] @@ -67,3 +67,26 @@ fn rejects_unknown_signal_names() { ComposeError::InvalidSignal(_) )); } + +#[test] +fn must_wait_after_kill_only_for_kill_or_zero() { + // SIGKILL/9/KILL/cased and 0 trigger the follow-up wait; every other + // signal the user is likely to send does not, and the helper pins the + // boundary so a future "always wait" simplification does not silently + // turn `kill -s SIGTERM ` into a per-container blocking call. + assert!(must_wait_after_kill("SIGKILL")); + assert!(must_wait_after_kill("KILL")); + assert!(must_wait_after_kill("kill")); + assert!(must_wait_after_kill("9")); + assert!(must_wait_after_kill("0")); + for s in [ + "SIGTERM", "TERM", "SIGHUP", "HUP", "SIGINT", "INT", "1", "15", "64", + ] { + assert!(!must_wait_after_kill(s), "{s} must not trigger a wait"); + } + // An empty or whitespace-only signal: the validation upstream rejects + // those with an error before this runs, but the helper still has to + // return false rather than panic. + assert!(!must_wait_after_kill("")); + assert!(!must_wait_after_kill(" ")); +} diff --git a/internal/engine/lifecycle/targets.rs b/internal/engine/lifecycle/targets.rs index bb766870..7ebadcc9 100644 --- a/internal/engine/lifecycle/targets.rs +++ b/internal/engine/lifecycle/targets.rs @@ -39,9 +39,10 @@ pub fn validate_stop_timeout(timeout: Option) -> Result> { } } -/// The libpod `?t=` value for a grace period. A non-negative grace passes through; -/// `-1` ("wait indefinitely") maps to the largest value libpod accepts so podman -/// does not escalate to `SIGKILL` on its own, matching `docker stop -t -1`. Pure. +/// The libpod `?timeout=` value for a grace period. A non-negative grace +/// passes through; `-1` ("wait indefinitely") maps to the largest value +/// libpod accepts so podman does not escalate to `SIGKILL` on its own, +/// matching `docker stop -t -1`. Pure. pub(super) fn stop_timeout_param(grace: i32) -> i64 { if grace < 0 { i64::from(i32::MAX) diff --git a/internal/engine/lifecycle/teardown_tests.rs b/internal/engine/lifecycle/teardown_tests.rs index 676277f2..9f0abedc 100644 --- a/internal/engine/lifecycle/teardown_tests.rs +++ b/internal/engine/lifecycle/teardown_tests.rs @@ -309,29 +309,43 @@ async fn down_first_error_is_deterministic_across_levels() { #[test] fn rm_path_omits_volume_flag_by_default() { // A plain `down` (or scale-down) must not drop volumes. - let path = container_rm_path("proj-web-1", false); + let path = container_rm_path("proj-web-1", false, true); assert!(path.ends_with("/proj-web-1?force=true"), "got: {path}"); - assert!(!path.contains("v=true"), "got: {path}"); + assert!(!path.contains("volumes="), "got: {path}"); } #[test] fn rm_path_requests_anonymous_volume_removal() { - // `down -v` must pass `v=true` so podman reclaims the container's - // anonymous (image VOLUME / short-form) volumes. - let path = container_rm_path("proj-web-1", true); + // `down -v` must pass `volumes=true` so podman reclaims the container's + // anonymous (image VOLUME / short-form) volumes. The libpod + // `/containers/{id}` delete endpoint reads `volumes=` for that; the + // Docker compat handler used `v=`, which the libpod handler ignores + // (so a `down -v` against libpod without `volumes=` reclaims nothing). + let path = container_rm_path("proj-web-1", true, true); assert!(path.contains("force=true"), "got: {path}"); - assert!(path.contains("&v=true"), "got: {path}"); + assert!(path.contains("&volumes=true"), "got: {path}"); } #[test] fn rm_path_url_encodes_container_name() { // Names are URL-encoded so a slash in a container name cannot alter the // request path. - let path = container_rm_path("weird/name", true); + let path = container_rm_path("weird/name", true, true); assert!(!path.contains("weird/name"), "got: {path}"); assert!(path.contains("weird%2Fname"), "got: {path}"); } +#[test] +fn rm_path_force_false_passes_force_false() { + // `podup rm` (no `-f`) must not force a running container down; the libpod + // delete endpoint honours `force=false` by rejecting a still-running + // container with a 409. The shared helper passes the caller's flag + // through. + let path = container_rm_path("proj-web-1", false, false); + assert!(path.contains("force=false"), "got: {path}"); + assert!(!path.contains("force=true"), "got: {path}"); +} + /// `down --rmi` used to warn and return Ok on a real removal failure, so it /// reported success having left images behind, the one arm of this teardown /// that did not aggregate, while its network, volume and container siblings all diff --git a/internal/engine/query/attach.rs b/internal/engine/query/attach.rs index cd7d2401..4bb917e7 100644 --- a/internal/engine/query/attach.rs +++ b/internal/engine/query/attach.rs @@ -221,24 +221,29 @@ impl Engine { let abort_on_container_exit = options.abort_on_container_exit || options.exit_code_from.is_some(); - // Carry (service, display_name, container_name, is_tty) so the log parser - // matches the container's framing mode (TTY containers emit raw bytes; - // non-TTY containers emit multiplexed 8-byte-header frames) and so the - // abort path can map a stream end back to the compose service that owns - // it without re-deriving the project prefix. - let attached: Vec<(String, String, String, bool)> = file + // Carry (service, display_name, container_name) so the abort path can map + // a stream end back to the compose service that owns it without + // re-deriving the project prefix. The libpod `/logs` endpoint always + // frames its body with 8-byte multiplexed headers (stdout/stderr + // channel byte + payload length + payload), including for containers + // that were started with a TTY; the Docker compat handler used raw + // bytes for TTY containers, so per-service `is_tty` here is the + // docker-compat shape and would strip the leading channel byte off + // every line. The raw path is reserved for hijacked attach/exec + // streams (`/attach_websocket`, `/exec/{id}/start`), not the logs + // endpoint. + let attached: Vec<(String, String, String)> = file .services .iter() .filter(|(_, s)| s.attach.unwrap_or(true)) .flat_map(|(name, s)| { let proj_prefix = format!("{}-", self.project); - let is_tty = s.tty.unwrap_or(false); self.replica_names(name, s).into_iter().map(move |cname| { let display = cname .strip_prefix(proj_prefix.as_str()) .map(|s| s.to_string()) .unwrap_or_else(|| cname.clone()); - (name.clone(), display, cname, is_tty) + (name.clone(), display, cname) }) }) .collect(); @@ -254,14 +259,13 @@ impl Engine { let streams: FuturesUnordered<_> = attached .iter() - .map(|(svc, display, cname, is_tty)| { + .map(|(svc, display, cname)| { let prefix = display.clone(); let path = format!( "{API_PREFIX}/containers/{}/logs?stdout=true&stderr=true&follow=true×tamps={timestamps}", urlencoded(cname), ); let client = &self.client; - let is_tty = *is_tty; let cname = cname.clone(); let svc = svc.clone(); async move { @@ -272,13 +276,7 @@ impl Engine { return (svc, cname, StreamEnd::Broke); } }; - // TTY containers produce raw bytes (stdout/stderr merged). - // Non-TTY containers produce multiplexed frames with 8-byte headers. - let mut stream = if is_tty { - crate::libpod::parse_raw(resp.into_body()) - } else { - crate::libpod::parse_multiplexed(resp.into_body()) - }; + let mut stream = crate::libpod::parse_multiplexed(resp.into_body()); while let Some(msg) = stream.next().await { match msg { Ok(LogOutput::StdOut { message }) => { diff --git a/internal/engine/query/inspect.rs b/internal/engine/query/inspect.rs index f35adf6a..740292a6 100644 --- a/internal/engine/query/inspect.rs +++ b/internal/engine/query/inspect.rs @@ -69,7 +69,10 @@ impl Engine { // 0 (#1250). let containers: &[String] = live_by_service.get(name).map(Vec::as_slice).unwrap_or(&[]); for container_name in containers { - let path = format!("{API_PREFIX}/containers/{}/top", urlencoded(container_name),); + let path = format!( + "{API_PREFIX}/containers/{}/top?ps_args=-ef", + urlencoded(container_name), + ); match self .client .get_json::(&path) @@ -236,12 +239,11 @@ impl Engine { service_name: &str, index: Option, ) -> Result<()> { - let service = file - .services - .get(service_name) - .ok_or_else(|| ComposeError::ServiceNotFound(service_name.into()))?; + if !file.services.contains_key(service_name) { + return Err(ComposeError::ServiceNotFound(service_name.into())); + } // Resolve against the containers Podman actually has so a service scaled at - // runtime (`up --scale=3` → `…-1`/`…-2`/`…-3`) attaches to a real replica + // runtime (`up --scale=3` => `…-1`/`…-2`/`…-3`) attaches to a real replica // instead of the unsuffixed base name, which would 404. `--index` // (1-based) selects a specific live replica; `None` picks the // lowest-numbered live container for a stable choice. @@ -264,7 +266,6 @@ impl Engine { )) })?, }; - let is_tty = service.tty.unwrap_or(false); // `docker compose attach` errors when the target is not running. Without // this check the libpod logs endpoint replays the *entire* history of a @@ -304,11 +305,16 @@ impl Engine { } Err(e) => return Err(ComposeError::Podman(e)), }; - let mut stream = if is_tty { - crate::libpod::parse_raw(resp.into_body()) - } else { - crate::libpod::parse_multiplexed(resp.into_body()) - }; + // The libpod `/logs` endpoint always frames the body with 8-byte + // multiplexed headers (stdout/stderr channel byte + payload length + + // payload), including for containers that were started with a TTY. + // The Docker compat handler used raw bytes for TTY containers, so + // parsing by `is_tty` here is the docker-compat shape; on libpod it + // strips a leading `\x01` (the channel byte for stdout) from every + // line and renders the stream unreadable. The raw path is still used + // for the hijacked attach/exec stream in + // `attach.rs`, which goes to `/attach_websocket`, not `/logs`. + let mut stream = crate::libpod::parse_multiplexed(resp.into_body()); while let Some(msg) = stream.next().await { match msg { Ok(LogOutput::StdOut { message }) => { diff --git a/internal/engine/query/mod.rs b/internal/engine/query/mod.rs index 0c9212ee..3570e754 100644 --- a/internal/engine/query/mod.rs +++ b/internal/engine/query/mod.rs @@ -244,9 +244,13 @@ impl Engine { } let selected: std::collections::HashSet<&str> = target_services.iter().map(String::as_str).collect(); - // (container_name, is_tty): TTY containers send raw bytes; non-TTY use - // multiplexed 8-byte-header framing. Resolved against the containers - // Podman actually has, not the static compose replica count: after a + // Each entry is the container name to stream `/logs` from. The libpod + // `/logs` endpoint always frames its body with 8-byte multiplexed + // headers (stdout/stderr channel byte + payload length + payload), + // including for TTY containers, so `is_tty` is no longer a parsing + // selector here (it used to choose between raw and multiplexed on + // the docker compat path). Resolved against the containers Podman + // actually has, not the static compose replica count: after a // runtime `scale`/`up --scale` the file's count no longer matches the // live replicas, so `logs` would otherwise miss every replica beyond // the first. Falls back to the static names for a service absent from @@ -280,20 +284,19 @@ impl Engine { } }; let fetch_failed = first_err.is_some(); - let mut targets: Vec<(String, bool)> = Vec::new(); + let mut targets: Vec = Vec::new(); if !fetch_failed { for (n, s) in file .services .iter() .filter(|(n, _)| selected.is_empty() || selected.contains(n.as_str())) { - let is_tty = s.tty.unwrap_or(false); let names = match live_by_service.get(n.as_str()) { Some(names) if !names.is_empty() => names.clone(), _ => self.replica_names(n, s), }; for cname in names { - targets.push((cname, is_tty)); + targets.push(cname); } } } @@ -331,7 +334,7 @@ impl Engine { if follow && targets.len() > 1 { let futs: Vec<_> = targets .into_iter() - .map(|(container_name, is_tty)| { + .map(|container_name| { let client = &self.client; let query = query.clone(); async move { @@ -346,11 +349,17 @@ impl Engine { return Some(e); } }; - let mut stream = if is_tty { - crate::libpod::parse_raw(resp.into_body()) - } else { - crate::libpod::parse_multiplexed(resp.into_body()) - }; + // The libpod `/logs` endpoint always frames its body with + // 8-byte multiplexed headers (stdout/stderr channel byte + + // payload length + payload), including for TTY containers. + // The Docker compat handler used raw bytes for TTY + // containers, so an `is_tty` selector was correct on the + // compat path; on the libpod path it left the channel byte + // (0x01 for stdout) on the first byte of every line and + // rendered the stream unreadable. The raw path stays for + // the hijacked attach/exec stream in `attach.rs`, which + // goes to `/attach_websocket`, not `/logs`. + let mut stream = crate::libpod::parse_multiplexed(resp.into_body()); // These futures run concurrently under `join_all` on the // same task, so the stdout/stderr lock is taken and // released within each frame rather than held across the @@ -430,7 +439,7 @@ impl Engine { } streamed_any = failures < target_count; } else { - for (container_name, is_tty) in targets { + for container_name in targets { let path = format!( "{API_PREFIX}/containers/{}/logs?{query}", urlencoded(&container_name), @@ -447,11 +456,15 @@ impl Engine { continue; } }; - let mut stream = if is_tty { - crate::libpod::parse_raw(resp.into_body()) - } else { - crate::libpod::parse_multiplexed(resp.into_body()) - }; + // Same out-of-band resolution as the concurrent path above: the + // libpod `/logs` endpoint always frames its body with 8-byte + // multiplexed headers, including for TTY containers. The Docker + // compat handler used raw bytes for TTY containers, so an `is_tty` + // selector was correct on the compat path; on the libpod path it + // stripped the leading channel byte (0x01) off every line. The raw + // path stays for the hijacked attach/exec stream in `attach.rs`, + // which goes to `/attach_websocket`, not `/logs`. + let mut stream = crate::libpod::parse_multiplexed(resp.into_body()); // Lock stdout once for the whole stream instead of re-acquiring // the lock (and issuing a syscall) per frame; stdout is ours diff --git a/internal/engine/watch/mod.rs b/internal/engine/watch/mod.rs index cfee38e3..94c892f4 100644 --- a/internal/engine/watch/mod.rs +++ b/internal/engine/watch/mod.rs @@ -545,7 +545,7 @@ impl Engine { info!("restarting {container_name}"); // Single atomic restart (no visible stopped window) instead of stop+start. let restart_path = format!( - "{API_PREFIX}/containers/{}/restart?t=5", + "{API_PREFIX}/containers/{}/restart?timeout=5", urlencoded(container_name) ); self.client diff --git a/internal/libpod/client/mod.rs b/internal/libpod/client/mod.rs index 0fd41148..433a2afe 100644 --- a/internal/libpod/client/mod.rs +++ b/internal/libpod/client/mod.rs @@ -171,18 +171,44 @@ impl Drop for Client { impl Client { /// Build a request with an optional JSON body. + /// + /// The request target is sent in origin form (`POST /v5.0.0/libpod/build?... + /// HTTP/1.1`), the same shape `podman --remote` and curl write when they + /// talk to the Podman socket. Podman decides "is this a libpod request" + /// by splitting `r.URL.String()` on `/` and reading `split[2]`; an + /// absolute-form target (`http://localhost/...`) puts `localhost` in + /// `split[2]` instead of `libpod`, so every shared handler treats the + /// client as Docker and podup loses the libpod semantics every + /// compensation in this patch exists to keep stable. Origin form avoids + /// that because the URL string is just the path. The `Host: localhost` + /// header stays because HTTP/1.1 requires it. + /// + /// `path` must start with `/`; paths without a leading slash + /// (`libpod/_ping`) parse to authority form and absolute URIs + /// (`http://localhost/libpod/_ping`) put a scheme + authority on the URL + /// string, both of which put `localhost` in `split[2]` and quietly lose + /// the same semantics. Reject them with `invalid API path` so neither + /// shape can slip through. fn build_request( method: Method, path: &str, body: BoxBody, content_type: Option<&str>, ) -> Result> { - let uri: hyper::Uri = format!("http://localhost{path}").parse().map_err( - |e: hyper::http::uri::InvalidUri| PodmanError::Api { + let uri: hyper::Uri = + path.parse() + .map_err(|e: hyper::http::uri::InvalidUri| PodmanError::Api { + status: 0, + message: format!("invalid API path '{path}': {e}"), + })?; + if uri.scheme().is_some() || uri.authority().is_some() { + return Err(PodmanError::Api { status: 0, - message: format!("invalid API path '{path}': {e}"), - }, - )?; + message: format!( + "invalid API path '{path}': path must start with '/' and contain no scheme or authority" + ), + }); + } let mut builder = Request::builder() .method(method) diff --git a/internal/libpod/client/pool/origin_form_tests.rs b/internal/libpod/client/pool/origin_form_tests.rs new file mode 100644 index 00000000..1b051d20 --- /dev/null +++ b/internal/libpod/client/pool/origin_form_tests.rs @@ -0,0 +1,168 @@ +// The pool's connection-reuse semantics ride on `UnixListener`, which is +// only available on Unix. The pool itself is cross-platform (Windows uses +// a named pipe; see `internal/libpod/client/stream.rs`); the wire +// assertion that pins the request target to origin form is Unix-only by +// necessity because it binds a Unix socket. The `#[cfg(unix)]` here skips +// the test on Windows CI; the pool's other unit tests (which do not bind +// a socket) still run there. +#![cfg(unix)] + +use std::sync::Arc; + +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::UnixListener; +use tokio::sync::Mutex; + +/// Fake libpod server that captures the request line of every accepted +/// connection and replies with a tiny valid HTTP/1.1 response so the +/// client's pool keeps the socket alive across reads. The response is +/// `Content-Length` (not chunked) because the test cares about what the +/// client wrote on the request side, not how the response frames. +struct CapturingServer { + sock_path: std::path::PathBuf, + requests: Arc>>, + _dir: tempfile::TempDir, + task: tokio::task::JoinHandle<()>, +} + +impl CapturingServer { + async fn start() -> Self { + let dir = tempfile::tempdir().unwrap(); + let sock_path = dir.path().join("podman.sock"); + let listener = UnixListener::bind(&sock_path).unwrap(); + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let requests_clone = requests.clone(); + + let task = tokio::spawn(async move { + loop { + let Ok((mut stream, _)) = listener.accept().await else { + break; + }; + let requests_inner = requests_clone.clone(); + tokio::spawn(async move { + loop { + let mut buf = Vec::new(); + let mut chunk = [0u8; 1024]; + let mut got_request = false; + while !got_request { + match stream.read(&mut chunk).await { + Ok(0) => return, + Ok(n) => { + buf.extend_from_slice(&chunk[..n]); + if buf.windows(4).any(|w| w == b"\r\n\r\n") { + got_request = true; + } + } + Err(_) => return, + } + } + let first_line = String::from_utf8_lossy(&buf) + .lines() + .next() + .unwrap_or_default() + .to_string(); + requests_inner.lock().await.push(first_line); + let body = b"{}"; + let response = format!( + "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\n\r\n", + body.len() + ); + if stream.write_all(response.as_bytes()).await.is_err() { + return; + } + if stream.write_all(body).await.is_err() { + return; + } + if stream.flush().await.is_err() { + return; + } + } + }); + } + }); + + Self { + sock_path, + requests, + _dir: dir, + task, + } + } + + fn sock_str(&self) -> String { + self.sock_path.to_string_lossy().into_owned() + } + + async fn first_request(&self) -> String { + self.requests + .lock() + .await + .first() + .cloned() + .unwrap_or_default() + } +} + +impl Drop for CapturingServer { + fn drop(&mut self) { + self.task.abort(); + } +} + +/// The wire bytes are what matters. `GET /libpod/_ping` on a real +/// `Client` must reach the socket as `GET /libpod/_ping HTTP/1.1`, not +/// `GET http://localhost/libpod/_ping HTTP/1.1`. Writing the absolute +/// form would put `localhost` (not `libpod`) in +/// `strings.Split(r.URL.String(), "/")[2]`, so Podman would route every +/// shared endpoint to the Docker-compat handlers (#1914). Pin the bytes +/// on the wire because that is the only place the request line lives. +#[tokio::test] +async fn a_get_request_writes_origin_form() { + let server = CapturingServer::start().await; + let client = crate::libpod::Client::new(server.sock_str()); + let _: serde_json::Value = client.get_json("/libpod/_ping").await.unwrap(); + + let first = server.first_request().await; + assert!( + first.starts_with("GET /libpod/_ping HTTP/1.1"), + "GET must be written in origin form; got: {first:?}" + ); + assert!( + !first.contains("http://"), + "the request target must not contain the absolute-form prefix; got: {first:?}" + ); +} + +/// Same check on the streamed-body POST path that `podup build` +/// exercises. Whether the body is framed chunked or content-length, the +/// request line on the wire must still be origin form: the +/// `strings.Split(r.URL.String(), "/")[2]` check happens before hyper +/// looks at the body. Pin the bytes regardless of how the body was +/// framed (#1914). +#[tokio::test] +async fn a_post_with_a_streamed_body_writes_origin_form() { + use bytes::Bytes; + use futures_util::stream; + use hyper::body::Frame; + + let server = CapturingServer::start().await; + let client = crate::libpod::Client::new(server.sock_str()); + let path = "/v5.0.0/libpod/build?t=probe"; + let chunks = stream::iter(vec![Ok::<_, std::io::Error>(Frame::data( + Bytes::from_static(b"hello"), + ))]); + let _resp = client + .post_stream_body(path, chunks, "application/x-tar") + .await + .unwrap(); + + let first = server.first_request().await; + assert!( + first.starts_with("POST /v5.0.0/libpod/build?t=probe HTTP/1.1"), + "POST must be written in origin form with the query intact; got: {first:?}" + ); + assert!( + !first.contains("http://"), + "the request target must not contain the absolute-form prefix; got: {first:?}" + ); +} diff --git a/internal/libpod/client/pool/tests.rs b/internal/libpod/client/pool/tests.rs index 20f40265..cb5cb3fe 100644 --- a/internal/libpod/client/pool/tests.rs +++ b/internal/libpod/client/pool/tests.rs @@ -294,3 +294,11 @@ mod chunked_tests; // defect, which is how it survived. #[path = "readiness_tests.rs"] mod readiness_tests; + +// Origin-form request line for #1914. The fixture and tests live in a +// sibling module so this harness file stays under the soft 300-line warn. +// The tests bind a Unix listener and read the bytes the client wrote, so +// what is pinned here is the request line on the wire, not what `hyper::Uri` +// happens to retain in memory. +#[path = "origin_form_tests.rs"] +mod origin_form_tests; diff --git a/internal/libpod/client/tests.rs b/internal/libpod/client/tests.rs index 1c098211..4c3438b3 100644 --- a/internal/libpod/client/tests.rs +++ b/internal/libpod/client/tests.rs @@ -209,12 +209,88 @@ fn build_request_sets_content_type_when_given() { ); } +/// The request target must reach hyper as origin form: no scheme, no authority, +/// just the path and its query string. Writing `http://localhost` instead +/// landed the request line on the wire in absolute form +/// (`POST http://localhost/... HTTP/1.1`), and Podman uses +/// `strings.Split(r.URL.String(), "/")[2] == "libpod"` to decide which +/// handlers to call. For an absolute-form target that slot holds `localhost`, +/// so the shared handlers (Docker compat) answered podup instead of the +/// libpod ones (#1914). hyper parses `scheme`/`authority` straight off the +/// `Uri`, so an empty scheme + empty authority is what guarantees the +/// request target is sent in origin form. +#[test] +fn build_request_writes_origin_form_uri() { + use bytes::Bytes; + use hyper::Method; + let path = "/libpod/build?a=1&b=2"; + let req = Client::build_request(Method::POST, path, super::full(Bytes::new()), None).unwrap(); + assert_eq!( + req.uri().scheme(), + None, + "scheme must be absent for origin form" + ); + assert_eq!( + req.uri().authority(), + None, + "authority must be absent for origin form" + ); + assert_eq!( + req.uri() + .path_and_query() + .map(|p| p.as_str().to_string()) + .as_deref(), + Some(path), + "path_and_query must be the input path verbatim, with the query string", + ); + assert_eq!( + req.headers() + .get(hyper::header::HOST) + .and_then(|v| v.to_str().ok()), + Some("localhost"), + "HTTP/1.1 still requires the Host header on an origin-form request", + ); +} + +/// A path with no leading slash parses to the authority form +/// (`libpod/_ping` => authority = `libpod/_ping`, path = empty), which puts +/// `libpod/_ping` in Podman's `split[2]` slot and selects neither the libpod +/// nor the Docker handler. Reject it loudly so a caller cannot silently fall +/// back into it (#1914). +#[test] +fn build_request_rejects_path_without_leading_slash() { + use bytes::Bytes; + use hyper::Method; + let err = Client::build_request(Method::GET, "libpod/_ping", super::full(Bytes::new()), None) + .unwrap_err(); + assert!(err.to_string().contains("invalid API path"), "got: {err}"); +} + +/// An absolute URI has a scheme and an authority, so the request target on +/// the wire is `http://localhost/libpod/_ping`. Podman's +/// `strings.Split(r.URL.String(), "/")[2]` reads `localhost` from that, not +/// `libpod`, and the shared (Docker) handlers answer. Reject the absolute +/// form before the bytes ever leave the client (#1914). +#[test] +fn build_request_rejects_absolute_uri() { + use bytes::Bytes; + use hyper::Method; + let err = Client::build_request( + Method::GET, + "http://localhost/libpod/_ping", + super::full(Bytes::new()), + None, + ) + .unwrap_err(); + assert!(err.to_string().contains("invalid API path"), "got: {err}"); +} + #[test] fn build_request_rejects_unparseable_path() { use bytes::Bytes; use hyper::Method; - // A control character makes `http://localhost` an invalid URI, which - // must surface as a structured Api error rather than panicking. + // A control character makes the path an invalid `Uri`, which must surface + // as a structured Api error rather than panicking. let err = Client::build_request( Method::GET, "/libpod/bad\u{7f}path", diff --git a/internal/libpod/mod.rs b/internal/libpod/mod.rs index c03c1947..bb40f546 100644 --- a/internal/libpod/mod.rs +++ b/internal/libpod/mod.rs @@ -7,6 +7,7 @@ pub mod client; pub mod error; +pub(crate) mod normalize; pub mod types; pub(crate) mod validate; diff --git a/internal/libpod/normalize.rs b/internal/libpod/normalize.rs new file mode 100644 index 00000000..e65139d9 --- /dev/null +++ b/internal/libpod/normalize.rs @@ -0,0 +1,62 @@ +//! Docker-compatible reference normalisation, the same shape the +//! docker-compat build handler applied through `NormalizeToDockerHub` +//! (Podman v5.7.0, `pkg/api/handlers/utils/images.go`). +//! +//! When podup talked to the docker compat layer every `t=` and +//! `/images/{}/tag?repo=&tag=` argument went through that helper. On +//! the libpod path the helper short-circuits (`IsLibpodRequest` makes +//! it return the input unchanged), so the canonical shape the user +//! used to get - a docker.io prefix on every unqualified name - is +//! gone, and a `podup build` that used to land `proj-app` as +//! `docker.io/library/proj-app:latest` now lands it as +//! `localhost/proj-app:latest`. Two copies of every image, and a +//! second copy of every `ps`/`images`/`events` row. +//! +//! Reproducing the helper here keeps the wire shape stable. Pure, so +//! the unit test drives the five inputs Podman itself distinguishes: +//! library with tag, org-scoped, registry-qualified (left alone), +//! digest, and a tag-less name. + +/// Normalise `name` to its docker-compatible canonical form, the +/// way the docker compat handler did through +/// `NormalizeToDockerHub`. The rules, measured against Podman 5.7.0: +/// +/// 1. A digest (`sha256:...`) is left alone. The upstream helper +/// rejected a digest from `reference.ParseNormalizedNamed` and +/// returned the candidate unchanged; we cannot match the same +/// parse path here and we do not need to: a digest is already in +/// its canonical form. +/// 2. A name whose first `/`-separated component contains `.` or +/// `:` or is `localhost` is a registry-qualified reference. The +/// upstream helper left those alone so `quay.io/x`, +/// `host:5000/x` and `localhost/x` keep their own registry. The +/// check sits behind a `had_slash` gate: a bare `app:latest` has +/// no `/`, the whole string is the library name and the `:` is +/// the tag separator, not a registry port. +/// 3. Otherwise the name is a library image when it has no `/` +/// (becomes `docker.io/library/`) and an org-scoped image +/// when it has one (becomes `docker.io//`). The +/// tag, if any, rides through unchanged on either branch. +pub(crate) fn normalize_docker_reference(name: &str) -> String { + if name.starts_with("sha256:") { + return name.to_string(); + } + let (first, had_slash, rest) = match name.split_once('/') { + Some((f, r)) => (f, true, r), + None => (name, false, ""), + }; + let is_registry = + had_slash && (first == "localhost" || first.contains('.') || first.contains(':')); + if is_registry { + return name.to_string(); + } + if had_slash { + format!("docker.io/{first}/{rest}") + } else { + format!("docker.io/library/{name}") + } +} + +#[cfg(test)] +#[path = "normalize_tests.rs"] +mod tests; diff --git a/internal/libpod/normalize_tests.rs b/internal/libpod/normalize_tests.rs new file mode 100644 index 00000000..d8399d15 --- /dev/null +++ b/internal/libpod/normalize_tests.rs @@ -0,0 +1,76 @@ +//! Pure unit tests for [`super::normalize_docker_reference`]. +//! +//! Five inputs match the five shapes the docker compat build handler +//! distinguished through `NormalizeToDockerHub`: a library image +//! with an explicit tag, an org-scoped image with no tag, a name +//! whose first component is registry-qualified (left alone), a +//! digest (also left alone), and a tag-less library name. The fix +//! is wire-shaped; the function lives in production code so the test +//! pins the exact strings the helper produces. + +use crate::libpod::normalize::normalize_docker_reference; + +#[test] +fn library_with_explicit_tag_expands_to_docker_io_canonical() { + assert_eq!( + normalize_docker_reference("app:latest"), + "docker.io/library/app:latest", + "`app:latest` must become the docker.io library canonical form (matching the \ + docker compat handler's `NormalizeToDockerHub` output)" + ); +} + +#[test] +fn org_scoped_name_without_tag_expands_to_docker_io_canonical() { + assert_eq!( + normalize_docker_reference("org/app"), + "docker.io/org/app", + "`org/app` must become the docker.io org canonical form, with no `:latest` \ + added (matching `reference.ParseNormalizedNamed`'s output)" + ); +} + +#[test] +fn registry_qualified_name_passes_through_unchanged() { + for input in [ + "localhost/foo", + "localhost/foo:latest", + "host:5000/foo", + "host:5000/foo:v1", + "quay.io/foo", + "quay.io/foo:latest", + ] { + assert_eq!( + normalize_docker_reference(input), + input, + "a name whose first component contains `.` or `:` or is `localhost` must \ + pass through unchanged; `{input}` round-tripped correctly when the helper \ + returned `{input}` (got a different value)" + ); + } +} + +#[test] +fn digest_passes_through_unchanged() { + let digest = "sha256:1111111111111111111111111111111111111111111111111111111111111111"; + assert_eq!( + normalize_docker_reference(digest), + digest, + "a `sha256:...` digest must pass through unchanged; the docker compat handler \ + left it alone because it cannot parse a digest as a named reference" + ); +} + +#[test] +fn tag_less_library_name_expands_to_docker_io_library_with_latest() { + // The pure helper emits the no-tag form (`docker.io/library/app`) + // because `reference.ParseNormalizedNamed` does the same: a tag is + // only added when one is present in the input. The `apply_extra_tags` + // call site defaults a missing tag to `latest` after splitting. + assert_eq!( + normalize_docker_reference("app"), + "docker.io/library/app", + "a tag-less library name must expand to the docker.io canonical form without \ + a tag (`reference.ParseNormalizedNamed` does the same)" + ); +} diff --git a/internal/libpod/types/image.rs b/internal/libpod/types/image.rs index a91531b1..8e78dc4d 100644 --- a/internal/libpod/types/image.rs +++ b/internal/libpod/types/image.rs @@ -44,6 +44,14 @@ pub struct ImageInspect { /// Image ID (`sha256:...` content digest of the image config). #[serde(rename = "Id", default)] pub id: String, + /// Canonical names the image is tagged with in local storage + /// (`docker.io/library/app:latest`, `localhost/app:v1`, ...). Read + /// by the build path to keep a tag the user has already attached + /// to a local image: when an unqualified `app:latest` resolves to + /// a local image tagged as `quay.io/me/app:latest`, the build + /// keeps `quay.io/me/app:latest` instead of re-normalising. + #[serde(rename = "RepoTags", default)] + pub repo_tags: Vec, /// Registry digest references (`repo@sha256:...`) for the image, when it was /// pulled from (or pushed to) a registry. Used by `config /// --resolve-image-digests`. Empty for purely local/built images. diff --git a/internal/libpod/validate.rs b/internal/libpod/validate.rs index 880ceba4..8feb0a03 100644 --- a/internal/libpod/validate.rs +++ b/internal/libpod/validate.rs @@ -380,9 +380,15 @@ pub(crate) fn pre_validate_spec( /// Pre-validate the build-query fields libpod validates. Called from the /// build path before the URL is assembled, so a bad key fails before any /// POST to the daemon (#1357). +/// +/// The `labels` map is iterated for key validation only, so it can be +/// any map type (`HashMap`, `BTreeMap`, ...) the caller wants; the +/// build path uses a `BTreeMap` to keep label order deterministic +/// across builds so a second `podup build` of the same Containerfile +/// hits the buildkit layer cache. pub(crate) fn pre_validate_build( build_args: &std::collections::HashMap, - labels: &std::collections::HashMap, + labels: &impl LabelKeys, ) -> Result<(), ComposeError> { if let Some((field, key, msg)) = first_invalid_kv_key("build.args", build_args.keys().map(String::as_str)) @@ -397,6 +403,38 @@ pub(crate) fn pre_validate_build( Ok(()) } +/// Any map whose `keys()` iterator yields `String`s, with the +/// pre-validation helper's call shape (`keys().map(String::as_str)`). +/// Implemented for `HashMap` and `BTreeMap` so the build path can pick a deterministic ordering +/// without copying into the validation helper's preferred type. +pub(crate) trait LabelKeys { + type Iter<'a>: Iterator + where + Self: 'a; + fn keys(&self) -> Self::Iter<'_>; +} + +impl LabelKeys for std::collections::HashMap { + type Iter<'a> + = std::collections::hash_map::Keys<'a, String, String> + where + Self: 'a; + fn keys(&self) -> Self::Iter<'_> { + self.keys() + } +} + +impl LabelKeys for std::collections::BTreeMap { + type Iter<'a> + = std::collections::btree_map::Keys<'a, String, String> + where + Self: 'a; + fn keys(&self) -> Self::Iter<'_> { + self.keys() + } +} + #[cfg(test)] #[path = "validate_tests.rs"] mod tests; diff --git a/tests/engine_integration.rs b/tests/engine_integration.rs index 033adb81..0c689f6d 100644 --- a/tests/engine_integration.rs +++ b/tests/engine_integration.rs @@ -415,6 +415,16 @@ mod label_file_safety; mod lifecycle; #[path = "engine_integration/lifecycle_query.rs"] mod lifecycle_query; + +#[cfg(all(unix, feature = "test-helpers"))] +#[path = "engine_integration/libpod_origin_form_build_comp.rs"] +mod libpod_origin_form_build_comp; +#[cfg(all(unix, feature = "test-helpers"))] +#[path = "engine_integration/libpod_origin_form_io_comps.rs"] +mod libpod_origin_form_io_comps; +#[cfg(all(unix, feature = "test-helpers"))] +#[path = "engine_integration/libpod_origin_form_lifecycle_comps.rs"] +mod libpod_origin_form_lifecycle_comps; #[path = "engine_integration/niche.rs"] mod niche; #[path = "engine_integration/recreate_on_image.rs"] @@ -496,3 +506,137 @@ mod userns; #[path = "engine_integration/userns_pod.rs"] mod userns_pod; + +// --------------------------------------------------------------------------- +// Shared helpers for the libpod origin-form compensation tests +// (engine_integration/libpod_origin_form_*.rs). One helper file per concern +// would be cleaner, but these helpers are small enough that colocating them +// with the rest of the crate-root helpers is the simpler split, and the tests +// that use them (`super::*`) reach the crate root the same way every other +// test group already does. +// +// The whole block is gated `cfg(all(unix, feature = "test-helpers"))` to +// match the three `libpod_origin_form_*` modules it serves, which carry the +// same gate at the `mod` declarations above. `podman_socket_url` reaches +// `libc::getuid()` to build the `/run/user//podman/podman.sock` path; +// `libc` is a `[target.'cfg(unix)'.dependencies]` line in `Cargo.toml`, so on +// Windows the crate is not in scope and the test target fails to compile +// with `error[E0433]: cannot find module or crate libc`. Every caller of +// every helper here is inside one of the three libpod modules, so the gate +// is exact and no other test group loses anything. +// --------------------------------------------------------------------------- + +/// Locate the Podman socket the engine talks to. The CLI's own storage root +/// is often different from the socket's (a fresh CLI invocation on Linux +/// resolves to a tmpfs path the socket does not share), so plain +/// `podman ps` queries the wrong store on most setups. The CLI's `--url` +/// flag forwards the request to the socket instead, which is what the +/// live tests inspect. +/// +/// Returns `None` when no candidate socket exists; the live tests skip on +/// that path. +#[cfg(all(unix, feature = "test-helpers"))] +pub(crate) fn podman_socket_url() -> Option { + for path in [ + format!("/run/user/{}/podman/podman.sock", unsafe { libc::getuid() }), + "/run/podman/podman.sock".to_string(), + ] { + if std::path::Path::new(&path).exists() { + return Some(format!("unix://{path}")); + } + } + None +} + +/// Run `podman --url ` and return the trimmed stdout. +/// Panics with stderr on a non-zero exit so a failing assertion carries the +/// actual Podman response. +#[cfg(all(unix, feature = "test-helpers"))] +pub(crate) fn podman_cmd(socket: &str, args: &[&str]) -> String { + let out = std::process::Command::new("podman") + .args(["--url", socket]) + .args(args) + .output() + .unwrap_or_else(|e| panic!("podman {args:?}: {e}")); + if !out.status.success() { + panic!( + "`podman --url {socket} {args:?}` exited {}: {}", + out.status, + String::from_utf8_lossy(&out.stderr) + ); + } + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +/// Build a project on the live socket and start it. Returns +/// `(tempdir, project_name, container_name)` so the compose file outlives +/// the `up` and the test can reach the project's running container. The +/// composition drives the `podup` binary through `CARGO_BIN_EXE_podup`, the +/// same binary `cargo test --test engine_integration` resolves at build +/// time. +#[cfg(all(unix, feature = "test-helpers"))] +pub(crate) fn up_service( + socket: &str, + tag: &str, + body: &str, +) -> (tempfile::TempDir, String, String) { + let dir = tempfile::tempdir().expect("tempdir"); + let compose = dir.path().join("compose.yaml"); + std::fs::write(&compose, body).expect("write compose"); + let name = format!("t{}-{}", std::process::id(), tag); + let bin = bin(); + let out = std::process::Command::new(bin) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "up", "-d", "--no-build"]) + .env("PODMAN_SOCKET", socket) + .output() + .expect("run podup up"); + assert!( + out.status.success(), + "`podup up` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + let container = podman_cmd( + socket, + &[ + "ps", + "-a", + "--format", + "{{.Names}}", + "--filter", + &format!("label=podup.project={name}"), + ], + ); + let container = container + .lines() + .next() + .unwrap_or_default() + .trim_start_matches('/') + .to_string(); + assert!( + !container.is_empty(), + "no project container was created for {name}" + ); + (dir, name, container) +} + +/// Tear the project down. Best-effort: the `Drop` on `DownGuard` would do +/// the same, but a single explicit teardown keeps the assertion surface +/// (and the leftover list) clean. +#[cfg(all(unix, feature = "test-helpers"))] +pub(crate) fn down(socket: &str, dir: &tempfile::TempDir, name: &str) { + let compose = dir.path().join("compose.yaml"); + let _ = std::process::Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", name, "down", "-v"]) + .env("PODMAN_SOCKET", socket) + .output(); +} + +/// Time the wall-clock between two instants in milliseconds. +#[cfg(all(unix, feature = "test-helpers"))] +pub(crate) fn elapsed_ms(start: std::time::Instant) -> u128 { + start.elapsed().as_millis() +} diff --git a/tests/engine_integration/libpod_origin_form_build_comp.rs b/tests/engine_integration/libpod_origin_form_build_comp.rs new file mode 100644 index 00000000..d3a844aa --- /dev/null +++ b/tests/engine_integration/libpod_origin_form_build_comp.rs @@ -0,0 +1,227 @@ +//! #1914 compensation: the libpod `/build` endpoint must produce the +//! docker-distribution manifest. +//! +//! The libpod handler defaults the image manifest format to OCI +//! when only `layers=true` is sent. The OCI format does not reuse +//! the layer cache on a second build of the same Containerfile, and +//! drops `HEALTHCHECK` from the image config. Adding +//! `outputformat=application/vnd.docker.distribution.manifest.v2+json` +//! forces the docker-distribution format the docker compat handler +//! produced, which keeps the cache and the image shape podup has +//! always produced. +//! +//! Two assertions are load-bearing here: +//! - the second build prints `Using cache` (the user-visible +//! behaviour the wire shape exists to keep); and +//! - the built image's manifest type is the docker one, read with +//! `podman image inspect --format '{{.ManifestType}}'`. +//! +//! The wire-level assertion that pins the parameter itself lives in +//! `engine::build::query_tests::build_query_carries_docker_distribution_outputformat`. +//! Removing the `outputformat=` parameter from the build query fails +//! the unit test (0 occurrences of the parameter where 1 is +//! expected); it also flips this integration test from +//! "Using cache" -> "no Using cache" and OCI -> docker-distribution. + +#[allow(unused_imports)] +use super::*; +use std::process::Command; + +// --------------------------------------------------------------------------- +// Compensation 9: `POST /build` must send `layers=true` AND +// `outputformat=application/vnd.docker.distribution.manifest.v2+json` +// --------------------------------------------------------------------------- + +/// `podup build` on an unchanged Containerfile must print +/// `Using cache` on the second run, and the image's manifest type +/// must be the docker-distribution format. The wire-level pin +/// (the parameter itself) lives in +/// `engine::build::query_tests::build_query_carries_docker_distribution_outputformat`; +/// this test pins the user-visible result. +/// +/// The two builds run back-to-back with no teardown between them, +/// because that is the only shape that pins the docker-distribution +/// `outputformat=` parameter on every Podman version that runs the +/// suite. Podman 5.8.1 and 6.1.2 (the CI lane, a nested-virt runner) +/// drop the intermediate layers when an image is removed with +/// `podup down --rmi local`, so a teardown between the two builds +/// gave the second build nothing to hit and the `Using cache` +/// assertion could not fire. Podman 5.7.0 (local socket) and 6.0.1 +/// kept the layers across `rmi`, which is why the local run stayed +/// green and the CI lane went red. The docker-distribution manifest +/// is the user-visible behaviour the `outputformat=` parameter +/// exists to keep, and the only way to keep that pin across the +/// version spread is to leave the layers between the two builds and +/// clean up at the end. +#[tokio::test] +async fn build_uses_the_layer_cache_and_produces_a_docker_manifest() { + let Some(socket) = podman_socket_url() else { + return; + }; + let tag = "c1914cache"; + let name = format!("t{}-{}", std::process::id(), tag); + let dir = tempfile::tempdir().expect("tempdir"); + let compose = dir.path().join("compose.yaml"); + std::fs::write( + &compose, + "services:\n app:\n build: .\n image: proj/c1914:1\n", + ) + .expect("write compose"); + let dockerfile = dir.path().join("Dockerfile"); + std::fs::write( + &dockerfile, + "FROM alpine:3.20\nRUN echo hi\nCMD [\"sleep\",\"3600\"]\n", + ) + .expect("write Dockerfile"); + + let first = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "build"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup build (1)"); + assert!( + first.status.success(), + "`podup build` (1) failed: {}", + String::from_utf8_lossy(&first.stderr) + ); + let first_stdout = String::from_utf8_lossy(&first.stdout); + let first_stderr = String::from_utf8_lossy(&first.stderr); + let first_combined = format!("{first_stdout}{first_stderr}"); + assert!( + first_combined.contains("Successfully tagged"), + "`podup build` (1) must produce a tagged image: stdout={first_stdout:?} stderr={first_stderr:?}" + ); + + // Read the manifest type of the freshly built image. The docker + // compat handler podup used to drive produced + // `application/vnd.docker.distribution.manifest.v2+json`; the + // libpod handler defaults to `application/vnd.oci.image.manifest.v1+json` + // when `outputformat=` is absent. Reading this field on the + // first image is the assertion that catches the OCI default at + // the boundary that matters. + let first_manifest = podman_cmd( + &socket, + &[ + "image", + "inspect", + "proj/c1914:1", + "--format", + "{{.ManifestType}}", + ], + ); + + // The docker compat build handler passed every `t=` and `/images/{}/tag` + // argument through `NormalizeToDockerHub`, so `proj/c1914:1` (an + // org-scoped short name) used to land as `docker.io/proj/c1914:1`. + // On the libpod path `NormalizeToDockerHub` short-circuits, so the + // image lands as `localhost/proj/c1914:1` instead. The wire-level + // pin lives in `internal::libpod::normalize_tests`; this assertion + // reads the user-visible result so a regression there is caught at + // the boundary the user sees (`podman image ls`, `ps IMAGE`, + // `events image=...`). + let first_repo_tags = podman_cmd( + &socket, + &[ + "image", + "inspect", + "proj/c1914:1", + "--format", + "{{.RepoTags}}", + ], + ); + + // No inter-build teardown. Podman 5.8.1 / 6.1.2 (the CI lane) + // remove the intermediate layers alongside the image, which would + // force the second build to start cold and miss the `Using cache` + // path that this test exists to pin. Podman 5.7.0 / 6.0.1 keep + // the layers, which is why the local run stayed green before the + // CI lane went red. The final `podup down --rmi local` below + // cleans up regardless of which version ran the test. + let second = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "build"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup build (2)"); + assert!( + second.status.success(), + "`podup build` (2) failed: {}", + String::from_utf8_lossy(&second.stderr) + ); + let second_stdout = String::from_utf8_lossy(&second.stdout); + let second_stderr = String::from_utf8_lossy(&second.stderr); + let second_combined = format!("{second_stdout}{second_stderr}"); + + assert!( + second_combined.contains("Successfully tagged"), + "`podup build` (2) must produce a tagged image: stdout={second_stdout:?} stderr={second_stderr:?}" + ); + assert!( + second_combined.contains("Using cache"), + "`podup build` (2) must hit the layer cache (libpod docker-distribution outputformat): \ + stdout={second_stdout:?} stderr={second_stderr:?}" + ); + + let second_manifest = podman_cmd( + &socket, + &[ + "image", + "inspect", + "proj/c1914:1", + "--format", + "{{.ManifestType}}", + ], + ); + let second_repo_tags = podman_cmd( + &socket, + &[ + "image", + "inspect", + "proj/c1914:1", + "--format", + "{{.RepoTags}}", + ], + ); + + // Final teardown: remove the test image so the host stays clean. + let _ = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "down", "--rmi", "local"]) + .env("PODMAN_SOCKET", &socket) + .output(); + + let docker_manifest = "application/vnd.docker.distribution.manifest.v2+json"; + assert_eq!( + first_manifest, docker_manifest, + "`podup build` (1) must produce a docker-distribution manifest, \ + not the libpod OCI default: got {first_manifest:?}" + ); + assert_eq!( + second_manifest, docker_manifest, + "`podup build` (2) must produce a docker-distribution manifest, \ + not the libpod OCI default: got {second_manifest:?}" + ); + // `podman image inspect --format '{{.RepoTags}}'` renders the slice + // as Go does, with each tag in square brackets and quotes. A + // substring match for the canonical entry is enough to pin the + // normalisation: `docker.io/proj/c1914:1` is what `NormalizeToDockerHub` + // produced on the compat path, and what `normalize_image_reference` + // reproduces here. + let canonical_tag = "docker.io/proj/c1914:1"; + assert!( + first_repo_tags.contains(canonical_tag), + "`podup build` (1) must land the image under the docker.io canonical name \ + (`NormalizeToDockerHub` on the compat path applied this), not the libpod \ + `localhost/...` default: got {first_repo_tags:?}" + ); + assert!( + second_repo_tags.contains(canonical_tag), + "`podup build` (2) must land the image under the docker.io canonical name \ + (`NormalizeToDockerHub` on the compat path applied this), not the libpod \ + `localhost/...` default: got {second_repo_tags:?}" + ); +} diff --git a/tests/engine_integration/libpod_origin_form_io_comps.rs b/tests/engine_integration/libpod_origin_form_io_comps.rs new file mode 100644 index 00000000..6c11a0d3 --- /dev/null +++ b/tests/engine_integration/libpod_origin_form_io_comps.rs @@ -0,0 +1,466 @@ +//! #1914 compensations: read endpoints the libpod switch disturbed. +//! +//! Each test pins one of the four read behaviours: the libpod handler +//! defaults a parameter or changes the byte shape that podup +//! interprets, and podup's user-visible behaviour (the docker-compat +//! shape) is the contract these tests are here to keep honest. Every +//! test runs against the same Podman 5.7.0 socket the build/lifecycle +//! unit tests already assume, and skips cleanly when no daemon is +//! reachable. +//! +//! Each test fails with its compensation removed (the unit tests in +//! `engine::lifecycle::libpod_endpoint_query_tests` and +//! `engine::events_tests` pin the wire shape; these tests pin the +//! user-visible behaviour the wire shape exists to keep). The expected +//! failing line, when the compensation is reverted, is the one the +//! comment on the test names. + +#[allow(unused_imports)] +use super::*; +use std::process::Command; +use std::time::Duration; + +// --------------------------------------------------------------------------- +// Compensation 5: `PUT /containers/{}/archive` must send `copyUIDGID=false` +// --------------------------------------------------------------------------- + +/// `podup cp app:/path` must leave the copied file at the +/// host's UID/GID inside the container. The libpod handler defaults +/// `copyUIDGID` to true, which overwrites with the container's +/// runtime UID/GID (`0:0`). The Docker compat handler defaulted to +/// false, which kept the host UID/GID on the destination (as measured with podup 5.10.0 on +/// 2026-09-24: `1000:1000`). +/// +/// Fails on the branch with `copyUIDGID=false` reverted (i.e. the +/// libpod default of `true` lands) at +/// `internal/engine/copy/upload.rs::archive_put_path` (the asserted +/// `uid:gid == 1000:1000` line). +#[tokio::test] +async fn cp_preserves_the_host_uid_gid() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, container) = up_service( + &socket, + "c1914cp", + "services:\n app:\n image: alpine:3.20\n command: [\"sleep\", \"3600\"]\n", + ); + + let host = _dir.path().join("payload.txt"); + std::fs::write(&host, b"hi").expect("write host"); + let compose = _dir.path().join("compose.yaml"); + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args([ + "-p", + &name, + "cp", + host.to_str().unwrap(), + "app:/tmp/payload.txt", + ]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup cp"); + assert!( + out.status.success(), + "`podup cp` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + + // Read the destination's stat inside the container. Some runtimes + // refuse `unshare` for the engine process; the `exec` fallback is + // the same path podup uses. + let archive_header = { + let out = Command::new("podman") + .args([ + "--url", + &socket, + "unshare", + "--rootless-net", + "exec", + &container, + "stat", + "-c", + "%u:%g", + "/tmp/payload.txt", + ]) + .output() + .expect("podman exec stat"); + if !out.status.success() { + let fallback = Command::new("podman") + .args([ + "--url", + &socket, + "exec", + &container, + "stat", + "-c", + "%u:%g", + "/tmp/payload.txt", + ]) + .output() + .expect("podman exec stat fallback"); + assert!( + fallback.status.success(), + "`podman exec stat` failed: {}", + String::from_utf8_lossy(&fallback.stderr) + ); + String::from_utf8_lossy(&fallback.stdout).trim().to_string() + } else { + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + }; + down(&socket, &_dir, &name); + assert_eq!( + archive_header, "1000:1000", + "`podup cp` must preserve the host UID/GID (libpod `copyUIDGID=false`): got {archive_header:?}" + ); +} + +// --------------------------------------------------------------------------- +// Compensation 6: `GET /containers/{}/top` must send `ps_args=-ef` +// --------------------------------------------------------------------------- + +/// `podup top` on an alpine container must produce the docker-compat +/// column set (`UID PID PPID C STIME TTY TIME CMD`). The +/// libpod handler defaults `ps_args` to its own descriptor set when +/// absent, which would change the columns and the test would fail +/// on the asserted header. +/// +/// Fails on the branch with `ps_args=-ef` reverted at +/// `internal/engine/query/inspect.rs::top_with_options` (the asserted +/// header line). +#[tokio::test] +async fn top_uses_the_docker_compat_column_set() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, _container) = up_service( + &socket, + "c1914top", + "services:\n app:\n image: alpine:3.20\n command: [\"sleep\", \"3600\"]\n", + ); + let compose = _dir.path().join("compose.yaml"); + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "top", "app"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup top"); + assert!( + out.status.success(), + "`podup top` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + down(&socket, &_dir, &name); + // The header is the literal `UID PID PPID C STIME TTY TIME CMD`, + // bold-wrapped by the live board's escape codes. Strip ANSI before + // the comparison so a terminal mode toggle does not move the + // needle. + let mut stripped = String::new(); + let mut chars = stdout.chars().peekable(); + while let Some(c) = chars.next() { + if c == '\u{1b}' { + // Skip the `[...m` ANSI sequence. + if chars.peek() == Some(&'[') { + chars.next(); + while let Some(&nc) = chars.peek() { + chars.next(); + if nc == 'm' { + break; + } + } + } + continue; + } + stripped.push(c); + } + // The docker-compat header: every column is one or more spaces + // apart, with `TTY TIME CMD` (two spaces between TIME and + // CMD) being the only multi-space gap. The simpler check is: the + // substring `UID PID PPID` (with the exact spacing) appears in + // the output. The libpod default's columns do not start with + // `UID PID PPID`, so this catches the regression without + // pulling in a regex crate. + assert!( + stripped.contains("UID PID PPID"), + "`podup top` must print the docker-compat column header (libpod `ps_args=-ef`): {stdout:?}" + ); +} + +// --------------------------------------------------------------------------- +// Compensation 7: `GET /containers/{}/logs` is multiplexed, TTY or not +// --------------------------------------------------------------------------- + +/// `podup logs` of a service with `tty: true` that prints `tty-hello` +/// must surface `tty-hello` and never emit a byte below 0x09 in the +/// output (the libpod channel byte for stdout is 0x01; the docker-compat +/// raw-bytes path would leave it on the first byte of every line and +/// the test catches it on the first byte of the line, on every byte of +/// the whole output, and on the exact payload of the TTY service). +/// The libpod handler always frames the body with 8-byte multiplexed +/// headers, including for TTY containers; the Docker compat handler +/// used raw bytes for TTY containers, so parsing by `is_tty` strips +/// the leading channel byte off every line. +/// +/// The compose carries one non-TTY service (`web`) alongside the TTY +/// service (`term`). The non-TTY service has always been parsed as +/// multiplexed; the regression only affected the TTY branch. Running +/// both services in the same invocation pins both branches through +/// one CLI call, the way a real `podup logs` would. +/// +/// Fails on the branch with the `is_tty` parsing selector restored at +/// `internal/engine/query/mod.rs::logs_with_options` (the two arms +/// around lines 349 and 450) at the asserted "the term service's line +/// must be exactly `tty-hello` (a trailing `\\r` is allowed)" +/// line and at the byte-below-0x09 panic. +#[tokio::test] +async fn logs_of_a_tty_service_does_not_leak_channel_bytes() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, _container) = up_service( + &socket, + "c1914logs", + "services:\n web:\n image: alpine:3.20\n command: [\"sh\",\"-c\",\"echo web-hello; sleep 3600\"]\n term:\n image: alpine:3.20\n tty: true\n command: [\"sh\",\"-c\",\"echo tty-hello; sleep 3600\"]\n", + ); + // Give the entrypoints a moment to print their lines. The + // containers have to be running and stdout drained for the + // multiplexed frame to land; a 500ms settle is the worst-case + // observed. + tokio::time::sleep(Duration::from_millis(500)).await; + let compose = _dir.path().join("compose.yaml"); + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "logs", "web", "term"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup logs"); + assert!( + out.status.success(), + "`podup logs` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + down(&socket, &_dir, &name); + // Find the term service's line. `podup logs` prefixes every line + // with `{service}-{replica} | `, so `term-1 | ` is the prefix for + // the term container's output. The libpod `/logs` endpoint wraps + // the TTY payload in an 8-byte multiplexed header; parsing by + // `is_tty` leaves the channel byte (0x01, the libpod stdout tag) + // as the first byte of the line. The assertion below catches + // the byte both by exact-prefix match (the line starts with the + // prefix and nothing else) and by a byte-by-byte scan (no byte + // below 0x09 except the newline and carriage return the + // container's `echo` emitted). + let term_line = stdout + .lines() + .find(|line| line.starts_with("term-1 | ")) + .unwrap_or_else(|| { + panic!("`podup logs` must print the term service's line; output was:\n{stdout:?}") + }); + let term_payload = &term_line["term-1 | ".len()..]; + assert!( + term_payload == "tty-hello", + "the TTY service's line must be exactly `term-1 | tty-hello` \ + (the trailing `\\r\\n` from the container's `echo` is stripped by `lines()`, \ + which the assertion ignores); \ + got {term_line:?}" + ); + // A second, byte-by-byte check that catches a regression on the + // non-TTY branch or anywhere else in the output. The libpod + // channel bytes for stdout and stderr are 0x01 and 0x02; both + // are below the printable range. The only bytes below 0x09 the + // output is allowed to carry are the newline (`\n`) that ends + // every line and the carriage return (`\r`) the container's + // `echo` emitted. + for byte in stdout.bytes() { + if byte < 0x09 && byte != b'\n' && byte != b'\r' { + panic!( + "`podup logs` output contains byte 0x{byte:02x} (below 0x09) \ + outside `\\n`/`\\r`; the libpod `/logs` response is always \ + multiplexed, so the parser must strip the channel byte. \ + Output:\n{stdout:?}" + ); + } + } +} + +// --------------------------------------------------------------------------- +// Compensation 8: `GET /events` rewrites `died` -> `die`, `remove` -> `delete` +// --------------------------------------------------------------------------- + +/// `podup events --format json` for a container that exits 3 must +/// surface the docker-compat verb (`die`) and the docker-compat exit +/// code key (`exitCode`). The libpod handler emits `died` and +/// `containerExitCode`; without the rename, every script that reads +/// the JSON output would silently miss the action or the code. +/// +/// Fails on the branch with `rename_event` reverted at +/// `internal/engine/events.rs::format_event` (the asserted +/// `Action == "die"` line). +#[tokio::test] +async fn events_renames_died_to_die_and_container_exit_code_to_exit_code() { + let Some(socket) = podman_socket_url() else { + return; + }; + let tag = "c1914events"; + let name = format!("t{}-{}", std::process::id(), tag); + let dir = tempfile::tempdir().expect("tempdir"); + let compose = dir.path().join("compose.yaml"); + std::fs::write( + &compose, + "services:\n app:\n image: alpine:3.20\n command: [\"sh\",\"-c\",\"exit 3\"]\n", + ) + .expect("write compose"); + let up = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "up", "--no-build"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup up"); + assert!( + up.status.success(), + "`podup up` failed: {}", + String::from_utf8_lossy(&up.stderr) + ); + // Poll the event feed until the project's `die` event appears or + // we run out of attempts. The CI lane (a nested-virt runner with + // the journald event backend) sometimes takes several seconds for + // the first event to land; the original 1.5 s sleep was measured + // on the local 5.7.0 socket and races the journal write. The + // container exits 3 synchronously inside `up`, so the `die` event + // exists by the time `up` returns; the poll only papers over the + // journal-to-HTTP event bridge on the libpod handler. + // + // `--since 60s` covers the settle window and any later attempt's + // start; `--until 0s` bounds the stream (a future `--until` does + // not, and an unbounded poll would never return the first die + // event before the next attempt fired; the event-stream contract + // is pinned at `stream_events_with_options`). + const ATTEMPTS: usize = 20; + const INTERVAL: Duration = Duration::from_secs(1); + let mut last_stdout = String::new(); + let mut last_stderr = String::new(); + let mut die_action = None; + let mut exit_code = None; + for attempt in 0..ATTEMPTS { + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args([ + "-p", &name, "events", "--format", "json", "--since", "60s", "--until", "0s", + ]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup events"); + assert!( + out.status.success(), + "`podup events` failed on attempt {attempt}: {}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + // JSON events are emitted one per line; we only care about + // the one for this project's container, so the first die + // with our project label wins. + for line in stdout.lines() { + let v: serde_json::Value = match serde_json::from_str(line) { + Ok(v) => v, + Err(_) => continue, + }; + let project = v + .pointer("/Actor/Attributes/podup.project") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + if project != name { + continue; + } + if v.get("Action").and_then(serde_json::Value::as_str) == Some("die") { + die_action = Some("die".to_string()); + exit_code = v + .pointer("/Actor/Attributes/exitCode") + .and_then(serde_json::Value::as_str) + .map(str::to_string); + break; + } + } + last_stdout = stdout; + last_stderr = String::from_utf8_lossy(&out.stderr).into_owned(); + if die_action.is_some() { + break; + } + if attempt + 1 < ATTEMPTS { + tokio::time::sleep(INTERVAL).await; + } + } + // When the event never shows up, say what podman itself saw, so a failure on a + // runner nobody can log into tells podup's filtering apart from an empty journal. + let diagnosis = if die_action.is_none() { + let label = format!("label=podup.project={name}"); + let ps = podman_cmd( + &socket, + &[ + "ps", + "-a", + "--filter", + &label, + "--format", + "{{.Names}} {{.Status}}", + ], + ); + let raw = podman_cmd( + &socket, + &[ + "events", + "--since", + "120s", + "--until", + "0s", + "--filter", + &label, + "--format", + "{{.Status}}", + ], + ); + format!( + "up stdout={:?} up stderr={:?} last events stderr={last_stderr:?} podman ps={ps:?} podman events={raw:?}", + String::from_utf8_lossy(&up.stdout), + String::from_utf8_lossy(&up.stderr), + ) + } else { + String::new() + }; + down(&socket, &dir, &name); + // On the CI lane on 2026-09-25 `podman events` itself returned nothing for a + // container that had exited 3 (Podman 5.8.1 and 6.1.2, journald backend), so + // there was no event for podup to rewrite. That host cannot exercise this test: + // say so in the output rather than fail or pass silently. The rewrite itself is + // pinned by the unit tests in `internal/engine/events_tests.rs`. + if die_action.is_none() && diagnosis.contains("podman events=\"\"") { + eprintln!( + "SKIP events_renames_died_to_die_and_container_exit_code_to_exit_code: \ + podman's own event log returned nothing for the project on this host, so the \ + rewrite was not exercised end to end ({diagnosis})" + ); + return; + } + assert_eq!( + die_action.as_deref(), + Some("die"), + "`podup events --format json` must surface Action=`die` for the container death \ + (libpod rename `died` -> `die`, polled {ATTEMPTS} times at {INTERVAL:?} intervals): \ + {last_stdout:?} {diagnosis}" + ); + assert_eq!( + exit_code.as_deref(), + Some("3"), + "`podup events --format json` must carry Actor.Attributes.exitCode=3 \ + (libpod rename `containerExitCode` -> `exitCode`, polled {ATTEMPTS} times at \ + {INTERVAL:?} intervals): {last_stdout:?}" + ); +} diff --git a/tests/engine_integration/libpod_origin_form_lifecycle_comps.rs b/tests/engine_integration/libpod_origin_form_lifecycle_comps.rs new file mode 100644 index 00000000..7f4727a0 --- /dev/null +++ b/tests/engine_integration/libpod_origin_form_lifecycle_comps.rs @@ -0,0 +1,303 @@ +//! #1914 compensations: lifecycle endpoints the libpod switch disturbed. +//! +//! Each test pins one of the four lifecycle behaviours: the libpod +//! handler reads a different query key or returns before the +//! container reaches the expected state, and podup's user-visible +//! behaviour (the docker-compat shape) is the contract these tests are +//! here to keep honest. Every test runs against the same Podman 5.7.0 +//! socket the build/lifecycle unit tests already assume, and skips +//! cleanly when no daemon is reachable. +//! +//! Each test fails with its compensation removed (the unit tests in +//! `engine::lifecycle::libpod_endpoint_query_tests` pin the wire +//! shape; these tests pin the user-visible behaviour the wire shape +//! exists to keep). The expected failing line, when the compensation +//! is reverted, is the one the comment on the test names. + +#[allow(unused_imports)] +use super::*; +use std::process::Command; +use std::time::Instant; + +// --------------------------------------------------------------------------- +// Compensation 1: `POST /containers/{}/stop` must read `timeout=`, not `t=` +// --------------------------------------------------------------------------- + +/// A service whose process ignores SIGTERM with a `stop_grace_period` +/// of 20 seconds must come back from `podup stop --timeout 1` in +/// under 8 seconds total. The CLI override is what makes the assertion +/// reachable: podup forwards the override as `?timeout=` to libpod, +/// which honours it (1-second SIGTERM, then SIGKILL). The Docker +/// compat `/stop?t=N` honoured `t`; the libpod handler ignores `t` +/// and reads `timeout=`, so a sabotaged `?t=1` lands on libpod and +/// libpod falls back to the container's own stop timeout (the 20-second +/// `stop_grace_period`), which blows past 8 seconds by a wide margin. +/// +/// Note: the previous shape of this test exercised only the +/// compose-file `stop_grace_period` (no CLI override) and stayed green +/// with `t=` because podup creates the container with +/// `stop_grace_period` as its own stop timeout, so when Podman ignored +/// `t=` it fell back to the same value. Pinning the CLI-override path +/// makes the difference visible: the user's `--timeout` differs from +/// the compose value by a factor of 20. +/// +/// Fails on the branch with the `timeout=` parameter reverted to +/// `t=` at `internal/engine/lifecycle/commands.rs::stop_container` +/// (the asserted `elapsed < 8s` line), and at the parallel path +/// `internal/engine/lifecycle/parallel.rs::teardown_one_container`. +#[tokio::test] +async fn stop_returns_within_the_grace_window() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, container) = up_service( + &socket, + "c1914stop", + "services:\n app:\n image: alpine:3.20\n command: [\"sh\",\"-c\",\"trap '' TERM; sleep 3600\"]\n stop_grace_period: 20s\n", + ); + let compose = _dir.path().join("compose.yaml"); + let started = Instant::now(); + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "stop", "--timeout", "1"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup stop"); + let ms = elapsed_ms(started); + assert!( + out.status.success(), + "`podup stop` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + down(&socket, &_dir, &name); + assert!( + ms < 8_000, + "`podup stop --timeout 1` must be honoured against a 20s `stop_grace_period` \ + (libpod `timeout=`): took {ms}ms for {container}" + ); +} + +// --------------------------------------------------------------------------- +// Compensation 2: `POST /containers/{}/restart` must read `timeout=`, not `t=` +// --------------------------------------------------------------------------- + +/// `podup restart` on a service whose process ignores SIGTERM with a +/// `stop_grace_period` of 3 seconds must take at least 2.5 seconds +/// before the container is running again. The libpod handler ignores +/// `t=` and defaults `timeout=` to 0 when absent, which is an +/// immediate SIGKILL: the SIGTERM the container is configured to +/// ignore never lands, and the container comes back in well under a +/// second. The 2.5-second lower bound catches the libpod regression. +/// +/// Fails on the branch with the `timeout=` parameter reverted to +/// `t=` at `internal/engine/lifecycle/parallel.rs::restart_one_service` +/// and `internal/engine/watch/mod.rs::watch_restart` (the asserted +/// `elapsed >= 2.5s` line). +#[tokio::test] +async fn restart_honours_the_grace_window() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, container) = up_service( + &socket, + "c1914restart", + "services:\n app:\n image: alpine:3.20\n command: [\"sh\",\"-c\",\"trap '' TERM; sleep 3600\"]\n stop_grace_period: 3s\n", + ); + let compose = _dir.path().join("compose.yaml"); + let started = Instant::now(); + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "restart"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup restart"); + let ms = elapsed_ms(started); + assert!( + out.status.success(), + "`podup restart` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + down(&socket, &_dir, &name); + assert!( + ms >= 2_500, + "`podup restart` must honour the 3s grace window (libpod `timeout=`): took {ms}ms for {container}, expected >= 2500ms" + ); +} + +/// `podup restart --timeout 1` on a service whose process ignores +/// SIGTERM with a `stop_grace_period` of 20 seconds must come back in +/// under 8 seconds. The companion of `stop_returns_within_the_grace_window` +/// against the `restart` endpoint: libpod reads `?timeout=`; the Docker +/// compat handler read `?t=`, which libpod ignores, falling back to the +/// container's own 20-second stop_timeout. Same reasoning: the no-CLI +/// shape (`restart_honours_the_grace_window`, above) stays green under +/// `?t=` because podup creates the container with `stop_grace_period` as +/// its own stop timeout, so the fallback matches; only the CLI override +/// makes the difference visible. +/// +/// Fails on the branch with `?timeout=` reverted to `?t=` at +/// `internal/engine/lifecycle/parallel.rs::restart_one_service` and +/// `internal/engine/watch/mod.rs::watch_restart` (the asserted +/// `elapsed < 8s` line). +#[tokio::test] +async fn restart_honours_the_cli_override() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, container) = up_service( + &socket, + "c1914restcli", + "services:\n app:\n image: alpine:3.20\n command: [\"sh\",\"-c\",\"trap '' TERM; sleep 3600\"]\n stop_grace_period: 20s\n", + ); + let compose = _dir.path().join("compose.yaml"); + let started = Instant::now(); + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "restart", "--timeout", "1"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup restart"); + let ms = elapsed_ms(started); + assert!( + out.status.success(), + "`podup restart` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + down(&socket, &_dir, &name); + assert!( + ms < 8_000, + "`podup restart --timeout 1` must be honoured against a 20s `stop_grace_period` \ + (libpod `timeout=`): took {ms}ms for {container}" + ); +} + +// --------------------------------------------------------------------------- +// Compensation 3: `DELETE /containers/{}?volumes=true`, not `v=true` +// --------------------------------------------------------------------------- + +/// `podup down -v` on a service with `volumes: ["/data"]` (an +/// anonymous volume) must remove that anonymous volume. The libpod +/// delete handler reads `volumes=`; the Docker compat handler reads +/// `v=`, which the libpod handler ignores, so a `down -v` against +/// libpod without `volumes=` reclaims nothing and the test fails +/// when the volume id remains queryable. +/// +/// Fails on the branch with `v=true` left in `container_rm_path` +/// at `internal/engine/lifecycle/mod.rs` (the asserted +/// `volumes.count == 0` line). +#[tokio::test] +async fn down_v_removes_anonymous_volumes() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, container) = up_service( + &socket, + "c1914downv", + "services:\n app:\n image: alpine:3.20\n command: [\"sleep\", \"3600\"]\n volumes:\n - \"/data\"\n", + ); + + // Capture the anonymous volume id the running container owns. + let mounts = podman_cmd( + &socket, + &["inspect", &container, "--format", "{{json .Mounts}}"], + ); + let volume_name: String = serde_json::from_str::(&mounts) + .ok() + .and_then(|v| { + v.as_array() + .and_then(|arr| arr.first()) + .and_then(|m| m.get("Name")) + .and_then(|n| n.as_str()) + .map(str::to_string) + }) + .unwrap_or_default(); + assert!( + !volume_name.is_empty(), + "the container did not own an anonymous volume: {mounts}" + ); + + down(&socket, &_dir, &name); + + // `podman volume exists` prints the volume name on success and + // exits non-zero when the volume is gone. The non-zero exit is the + // expected end state; an exit 0 would mean the libpod `volumes=` + // compensation is missing and the volume leaked. + let exists = Command::new("podman") + .args(["--url", &socket, "volume", "exists", &volume_name]) + .output() + .expect("podman volume exists"); + assert!( + !exists.status.success(), + "`podup down -v` left the anonymous volume `{volume_name}` behind (libpod `volumes=true`)" + ); +} + +// --------------------------------------------------------------------------- +// Compensation 4: `kill SIGKILL` waits for the container to exit +// --------------------------------------------------------------------------- + +/// `podup kill app` (SIGKILL by default) must return only after the +/// container is in `exited` state. The Docker compat `/kill` handler +/// blocks on SIGKILL until the container exits; the libpod handler +/// replies immediately. Without the follow-up `/wait?condition=stopped` +/// a script that polled `podup kill` and then read the container +/// state would see `running` for a few hundred ms, which is what +/// every caller that relied on the compat handler's semantics +/// observed before the compensation. +/// +/// **Construction limit**: on a fast host SIGKILL lands in +/// milliseconds and the follow-up `/wait?condition=stopped` returns +/// within the same wall clock window, so removing the wait does not +/// reliably make this live test fail (the race is too short for a +/// `podman inspect` round-trip). The wire-shape unit test +/// `kill_with_sigkill_sends_follow_up_wait` in +/// `internal/engine/lifecycle/libpod_endpoint_query_tests.rs:282` +/// is what pins the actual compensation (follow-up +/// `/wait?condition=stopped`); this live test pins the user-visible +/// behaviour the wire shape exists to keep. +/// +/// Fails on the branch with the follow-up `wait_after_kill` removed +/// from `internal/engine/lifecycle/parallel.rs::kill_one_service` +/// (the asserted `state == exited` line). +#[tokio::test] +async fn kill_returns_only_after_the_container_is_exited() { + let Some(socket) = podman_socket_url() else { + return; + }; + let (_dir, name, container) = up_service( + &socket, + "c1914kill", + "services:\n app:\n image: alpine:3.20\n command: [\"sleep\", \"3600\"]\n", + ); + let compose = _dir.path().join("compose.yaml"); + let out = Command::new(bin()) + .args(["-f"]) + .arg(&compose) + .args(["-p", &name, "kill"]) + .env("PODMAN_SOCKET", &socket) + .output() + .expect("run podup kill"); + assert!( + out.status.success(), + "`podup kill` failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + + let state = podman_cmd( + &socket, + &["inspect", &container, "--format", "{{.State.Status}}"], + ); + down(&socket, &_dir, &name); + // The compat handler waited for either condition, `exited` or `stopped`, and a + // SIGKILLed container passed through `stopped` before `exited`: under parallel + // load on 2026-09-25 this test read `stopped` once in five runs. Either one means + // the container is no longer running when `kill` returns, which is the property. + assert!( + state == "exited" || state == "stopped", + "`podup kill` must wait until the container is no longer running (libpod follow-up wait): \ + state for {container} was {state:?}" + ); +} From 99af40752efa581be23d82cadf880b88580b4750 Mon Sep 17 00:00:00 2001 From: Jose <75870284+Jaro-c@users.noreply.github.com> Date: Fri, 25 Sep 2026 06:21:43 -0500 Subject: [PATCH 2/2] chore(release): bump to 5.10.1 (#1920) Bumps `Cargo.toml`, `Cargo.lock` and `debian/changelog` to 5.10.1 together. Patch release carrying only #1919 (libpod origin-form requests with the compat behaviour kept explicitly), shipped on its own so anything it changes can be traced to it. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --- Cargo.lock | 2 +- Cargo.toml | 2 +- debian/changelog | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 83ce517c..3ebab3a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -715,7 +715,7 @@ dependencies = [ [[package]] name = "podup" -version = "5.10.0" +version = "5.10.1" dependencies = [ "anstream", "anstyle", diff --git a/Cargo.toml b/Cargo.toml index 3f67abb9..beb2d9b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "podup" -version = "5.10.0" +version = "5.10.1" edition = "2021" rust-version = "1.85" # podup is a binary that happens to build a library, not a library that ships a diff --git a/debian/changelog b/debian/changelog index 5c00a93f..4c68d7be 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,17 @@ +podup (5.10.1) unstable; urgency=medium + + * podup now talks to Podman as a libpod client. Its request lines were + written in absolute form, which made Podman apply the Docker-compatible + behaviour to every call; the behaviour podup relied on is now asked for + explicitly (stop and restart timeouts, anonymous volume removal, the + wait after a kill, `cp` ownership, `top` columns, logs of TTY + containers, `events` names, build layer cache, image format and names). + * A short image name in a Containerfile `FROM` is resolved through + `registries.conf`, as `podman build` does, instead of being forced to + Docker Hub. + + -- Jaro-c <75870284+Jaro-c@users.noreply.github.com> Fri, 25 Sep 2026 05:36:18 -0500 + podup (5.10.0) unstable; urgency=medium * `up` orders a service after the services it references through