From 5d040e4df7ebecc9a137236850c58190111bc44a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Israel=20Pe=C3=B1a?= Date: Sun, 28 Jul 2024 00:48:12 -0700 Subject: [PATCH] fix: don't assume docker host is a url --- testcontainers/src/core/client.rs | 44 +++++++++---------- .../src/core/client/bollard_client.rs | 30 +++++-------- testcontainers/src/core/env/config.rs | 31 +++++-------- testcontainers/src/runners/async_runner.rs | 2 + testcontainers/tests/async_runner.rs | 3 +- 5 files changed, 48 insertions(+), 62 deletions(-) diff --git a/testcontainers/src/core/client.rs b/testcontainers/src/core/client.rs index 9e7f96a7..ba29e13c 100644 --- a/testcontainers/src/core/client.rs +++ b/testcontainers/src/core/client.rs @@ -1,5 +1,13 @@ -use std::io; - +use crate::core::{ + client::exec::ExecResult, + env, + env::ConfigurationError, + logs::{ + stream::{LogStream, RawLogStream}, + LogFrame, LogSource, WaitingStreamWrapper, + }, + ports::{PortMappingError, Ports}, +}; use bollard::{ auth::DockerCredentials, container::{Config, CreateContainerOptions, LogOutput, LogsOptions, RemoveContainerOptions}, @@ -11,18 +19,10 @@ use bollard::{ }; use bollard_stubs::models::{ContainerInspectResponse, ExecInspectResponse, Network}; use futures::{StreamExt, TryStreamExt}; +use std::io; +use std::str::FromStr; use tokio::sync::OnceCell; - -use crate::core::{ - client::exec::ExecResult, - env, - env::ConfigurationError, - logs::{ - stream::{LogStream, RawLogStream}, - LogFrame, LogSource, WaitingStreamWrapper, - }, - ports::{PortMappingError, Ports}, -}; +use url::Url; mod bollard_client; mod exec; @@ -312,15 +312,15 @@ impl Client { pub(crate) async fn docker_hostname(&self) -> Result { let docker_host = self.config.docker_host(); - match docker_host.scheme() { - "tcp" | "http" | "https" => { - docker_host - .host() - .map(|host| host.to_owned()) - .ok_or_else(|| { - ConfigurationError::InvalidDockerHost(docker_host.to_string()).into() - }) - } + let docker_host_url = Url::from_str(docker_host).expect("expected a valid Docker Host URL"); + + match docker_host_url.scheme() { + "tcp" | "http" | "https" => docker_host_url + .host() + .map(|host| host.to_owned()) + .ok_or_else(|| { + ConfigurationError::InvalidDockerHost(docker_host.to_string()).into() + }), "unix" | "npipe" => { if is_in_container().await { let host = self diff --git a/testcontainers/src/core/client/bollard_client.rs b/testcontainers/src/core/client/bollard_client.rs index 11139803..8da7206d 100644 --- a/testcontainers/src/core/client/bollard_client.rs +++ b/testcontainers/src/core/client/bollard_client.rs @@ -1,39 +1,31 @@ +use std::str::FromStr; use std::time::Duration; -use bollard::{Docker, API_DEFAULT_VERSION}; - use crate::core::env; +use bollard::{Docker, API_DEFAULT_VERSION}; +use url::Url; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(2 * 60); pub(super) fn init(config: &env::Config) -> Result { let host = config.docker_host(); + let host_url = Url::from_str(host).expect("expected a valid Docker Host URL"); - match host.scheme() { + match host_url.scheme() { "https" => connect_with_ssl(config), "http" | "tcp" => { if config.tls_verify() { connect_with_ssl(config) } else { - Docker::connect_with_http( - host.as_str(), - DEFAULT_TIMEOUT.as_secs(), - API_DEFAULT_VERSION, - ) + Docker::connect_with_http(host, DEFAULT_TIMEOUT.as_secs(), API_DEFAULT_VERSION) } } #[cfg(unix)] - "unix" => Docker::connect_with_unix( - host.as_str(), - DEFAULT_TIMEOUT.as_secs(), - API_DEFAULT_VERSION, - ), + "unix" => Docker::connect_with_unix(host, DEFAULT_TIMEOUT.as_secs(), API_DEFAULT_VERSION), #[cfg(windows)] - "npipe" => Docker::connect_with_named_pipe( - host.as_str(), - DEFAULT_TIMEOUT.as_secs(), - API_DEFAULT_VERSION, - ), + "npipe" => { + Docker::connect_with_named_pipe(host, DEFAULT_TIMEOUT.as_secs(), API_DEFAULT_VERSION) + } _ => Err(bollard::errors::Error::UnsupportedURISchemeError { uri: host.to_string(), }), @@ -44,7 +36,7 @@ fn connect_with_ssl(config: &env::Config) -> Result, - host: Option, + tc_host: Option, + host: Option, tls_verify: Option, cert_path: Option, command: Option, @@ -48,9 +46,9 @@ pub(crate) struct Config { #[derive(Debug, Default, serde::Deserialize)] struct TestcontainersProperties { #[serde(rename = "tc.host")] - tc_host: Option, + tc_host: Option, #[serde(rename = "docker.host")] - host: Option, + host: Option, #[serde_as(as = "Option")] #[serde(rename = "docker.tls.verify")] tls_verify: Option, @@ -103,11 +101,7 @@ impl Config { where E: GetEnvValue, { - let host = E::get_env_value("DOCKER_HOST") - .as_deref() - .map(FromStr::from_str) - .transpose() - .map_err(|e: url::ParseError| ConfigurationError::InvalidDockerHost(e.to_string()))?; + let host = E::get_env_value("DOCKER_HOST"); let tls_verify = E::get_env_value("DOCKER_TLS_VERIFY").map(|v| v == "1"); let cert_path = E::get_env_value("DOCKER_CERT_PATH").map(PathBuf::from); let command = E::get_env_value("TESTCONTAINERS_COMMAND") @@ -132,14 +126,11 @@ impl Config { /// 2. `DOCKER_HOST` environment variable. /// 3. Docker host from the `docker.host` property in the `~/.testcontainers.properties` file. /// 4. Else, the default Docker socket will be returned. - pub(crate) fn docker_host(&self) -> Url { + pub(crate) fn docker_host(&self) -> &str { self.tc_host - .as_ref() - .or(self.host.as_ref()) - .cloned() - .unwrap_or_else(|| { - Url::from_str(DEFAULT_DOCKER_HOST).expect("default host is valid URL") - }) + .as_deref() + .or(self.host.as_deref()) + .unwrap_or(DEFAULT_DOCKER_HOST) } pub(crate) fn tls_verify(&self) -> bool { @@ -211,8 +202,8 @@ mod tests { #[test] fn deserialize_java_properties() { - let tc_host = Url::parse("http://tc-host").unwrap(); - let docker_host = Url::parse("http://docker-host").unwrap(); + let tc_host = "http://tc-host"; + let docker_host = "http://docker-host"; let tls_verify = 1; let cert_path = "/path/to/cert"; diff --git a/testcontainers/src/runners/async_runner.rs b/testcontainers/src/runners/async_runner.rs index 460706f1..50d6c3ba 100644 --- a/testcontainers/src/runners/async_runner.rs +++ b/testcontainers/src/runners/async_runner.rs @@ -297,6 +297,7 @@ mod tests { Ok(()) } + #[cfg(unix)] #[tokio::test] async fn async_run_command_should_map_exposed_port_udp_sctp() -> anyhow::Result<()> { let client = Client::lazy_client().await?; @@ -370,6 +371,7 @@ mod tests { Ok(()) } + #[cfg(unix)] #[tokio::test] async fn async_run_command_should_map_ports_udp_sctp() -> anyhow::Result<()> { let client = Client::lazy_client().await?; diff --git a/testcontainers/tests/async_runner.rs b/testcontainers/tests/async_runner.rs index c3bc8f86..b8636a7d 100644 --- a/testcontainers/tests/async_runner.rs +++ b/testcontainers/tests/async_runner.rs @@ -41,7 +41,8 @@ async fn bollard_can_run_hello_world_with_multi_thread() -> anyhow::Result<()> { } async fn cleanup_hello_world_image() -> anyhow::Result<()> { - let docker = Docker::connect_with_unix_defaults()?; + let docker = Docker::connect_with_local_defaults()?; + futures::future::join_all( docker .list_images::(None)