Conversation
Bumps [rustls-webpki](https://github.com/rustls/webpki) from 0.103.10 to 0.103.13. - [Release notes](https://github.com/rustls/webpki/releases) - [Commits](rustls/webpki@v/0.103.10...v/0.103.13) --- updated-dependencies: - dependency-name: rustls-webpki dependency-version: 0.103.13 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [openssl](https://github.com/rust-openssl/rust-openssl) from 0.10.76 to 0.10.80. - [Release notes](https://github.com/rust-openssl/rust-openssl/releases) - [Commits](rust-openssl/rust-openssl@openssl-v0.10.76...openssl-v0.10.80) --- updated-dependencies: - dependency-name: openssl dependency-version: 0.10.80 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [russh](https://github.com/warp-tech/russh) from 0.58.0 to 0.61.1. - [Release notes](https://github.com/warp-tech/russh/releases) - [Commits](Eugeny/russh@v0.58.0...v0.61.1) --- updated-dependencies: - dependency-name: russh dependency-version: 0.61.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [tar](https://github.com/composefs/tar-rs) from 0.4.45 to 0.4.46. - [Release notes](https://github.com/composefs/tar-rs/releases) - [Commits](composefs/tar-rs@0.4.45...0.4.46) --- updated-dependencies: - dependency-name: tar dependency-version: 0.4.46 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…ker/npm/ws-8.21.0' into dev
…webpki-0.103.13' into dev
…/stacker/openssl-0.10.80' into dev
…/stacker/russh-0.61.1' into dev
…/stacker/tar-0.4.46' into dev
… fetches via HTTP, no curl needed in container)
Add source_url field to TriggerPipeCommand, ActivatePipeCommand, and PipeRegistration. When source_url is set, the agent fetches source data via reqwest HTTP client instead of exec'ing curl inside a container. This eliminates the need for curl/wget in source containers.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Docker-facing behavior in the agent by making container resolution more robust (label-based), bounding certain Docker operations with timeouts, and shifting HTTP probing to an “exec-first” strategy to better reach localhost services inside containers. It also extends trigger_pipe to support source_url and optional target_headers, adds a Docker-backed integration test, and bumps a few dependencies (Rust + Node).
Changes:
- Prefer
my.stacker.serviceovercom.docker.compose.servicefor container resolution; add helper(s) to reduce redundant lookups. - Add timeouts for some Docker control-plane calls and update probing to run inside the container namespace first, falling back to agent HTTP.
- Extend
trigger_pipewithsource_urland custom outbound headers; update tests accordingly; bumprusshandws.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/agent/docker.rs |
Adds label-based container matching and a with_docker_timeout helper; refactors exec APIs to support “resolved name” fast paths. |
src/commands/stacker.rs |
Adds source_url + target_headers to pipe commands, updates trigger logic, adds redaction for persisted registrations, and adjusts probe/container request behavior. |
tests/probe_exec_integration.rs |
New ignored integration test validating label-based resolution + exec-in-container probing against a real Docker daemon. |
stacker/stacker/Cargo.toml |
Bumps russh dependency. |
stacker/stacker/Cargo.lock |
Updates lockfile for Rust dependency resolution changes (incl. transitive upgrades). |
stacker/stacker/package.json |
Bumps ws dependency. |
stacker/stacker/package-lock.json |
Updates Node lockfile for the ws bump. |
Cargo.lock |
Updates root lockfile (e.g., rustls-webpki). |
Files not reviewed (1)
- stacker/stacker/package-lock.json: Generated file
Suppressed comments (1)
src/commands/stacker.rs:4115
send_trigger_pipe_container_requestappears unused in non-test code now that the container target sends via HTTP. Keeping it compiled with#[allow(dead_code)]increases maintenance surface; prefer gating it to tests (or removing it).
#[cfg(feature = "docker")]
#[allow(dead_code)]
Comment on lines
+5046
to
+5052
| let port = crate::agent::docker::get_container_port(&target_value) | ||
| .await | ||
| .unwrap_or(80); | ||
| let container_url = build_pipe_target_url( | ||
| &format!("http://{}:{}", target_value, port), | ||
| &resolved.target_endpoint, | ||
| ); |
Comment on lines
873
to
877
| // Create exec instance | ||
| let exec = docker | ||
| .create_exec( | ||
| &resolved_name, | ||
| resolved_name, | ||
| CreateExecOptions { |
Comment on lines
+3793
to
+3797
| if let Some(ref mut headers) = registration.target_headers { | ||
| for value in headers.values_mut() { | ||
| if is_sensitive_header_value(value) { | ||
| *value = "[REDACTED]".into(); | ||
| } |
Comment on lines
+16
to
+39
| /// Upper bound for a single Docker control-plane operation (list/inspect/exec | ||
| /// setup). Wraps bollard calls so an unresponsive daemon can never hang a | ||
| /// resolver — it covers both connection and execution, since the timeout spans | ||
| /// the whole request including connect. | ||
| const DOCKER_OP_TIMEOUT: Duration = Duration::from_secs(10); | ||
|
|
||
| /// Run a Docker control-plane future with [`DOCKER_OP_TIMEOUT`], mapping an | ||
| /// elapsed timeout into an error rather than hanging. Accepts any error type | ||
| /// bollard returns (it maps into `anyhow`). | ||
| async fn with_docker_timeout<T, E>( | ||
| what: &str, | ||
| fut: impl std::future::Future<Output = std::result::Result<T, E>>, | ||
| ) -> Result<T> | ||
| where | ||
| E: std::error::Error + Send + Sync + 'static, | ||
| { | ||
| match tokio::time::timeout(DOCKER_OP_TIMEOUT, fut).await { | ||
| Ok(Ok(value)) => Ok(value), | ||
| Ok(Err(err)) => Err(anyhow::Error::new(err).context(format!("docker {what}"))), | ||
| Err(_) => Err(anyhow::anyhow!( | ||
| "docker {what} timed out after {}s", | ||
| DOCKER_OP_TIMEOUT.as_secs() | ||
| )), | ||
| } |
Comment on lines
3976
to
+3977
| #[cfg(feature = "docker")] | ||
| #[allow(dead_code)] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements to the Docker agent, focusing on more robust and reliable container resolution via labels, improved timeout handling for Docker API calls, and better test coverage. It also includes dependency updates for both Rust and Node.js components.
Docker agent improvements:
label_matches_app, that prefers themy.stacker.servicelabel for container resolution over the Docker Compose service label, making container identification more robust in scenarios where service names are generic or have changed.with_docker_timeouthelper to wrap all Docker API calls with a 10-second timeout, preventing the agent from hanging if the Docker daemon is unresponsive. All relevant API calls now use this helper. [1] [2] [3] [4]New and improved tests:
Dependency updates:
russhcrate from 0.58 to 0.61 for SSH functionality, and updated thewsNode.js package from 8.18.3 to 8.21.0 for WebSocket support. [1] [2] [3]