Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't assume docker host is a url #709

Merged
merged 4 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions testcontainers/src/core/client.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -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;
Expand Down Expand Up @@ -312,15 +312,15 @@ impl Client {

pub(crate) async fn docker_hostname(&self) -> Result<url::Host, ClientError> {
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");

DDtKey marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
30 changes: 11 additions & 19 deletions testcontainers/src/core/client/bollard_client.rs
Original file line number Diff line number Diff line change
@@ -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<Docker, bollard::errors::Error> {
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(),
}),
Expand All @@ -44,7 +36,7 @@ fn connect_with_ssl(config: &env::Config) -> Result<Docker, bollard::errors::Err
let cert_path = config.cert_path().expect("cert path not found");

Docker::connect_with_ssl(
config.docker_host().as_str(),
config.docker_host(),
&cert_path.join("key.pem"),
&cert_path.join("cert.pem"),
&cert_path.join("ca.pem"),
Expand Down
31 changes: 11 additions & 20 deletions testcontainers/src/core/env/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use std::{
str::FromStr,
};

use url::Url;

use crate::core::env::GetEnvValue;

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -35,8 +33,8 @@ pub const DEFAULT_DOCKER_HOST: &str = "npipe:////./pipe/docker_engine";

#[derive(Debug, Default)]
pub(crate) struct Config {
tc_host: Option<Url>,
host: Option<Url>,
tc_host: Option<String>,
host: Option<String>,
tls_verify: Option<bool>,
cert_path: Option<PathBuf>,
command: Option<Command>,
Expand All @@ -48,9 +46,9 @@ pub(crate) struct Config {
#[derive(Debug, Default, serde::Deserialize)]
struct TestcontainersProperties {
#[serde(rename = "tc.host")]
tc_host: Option<Url>,
tc_host: Option<String>,
#[serde(rename = "docker.host")]
host: Option<Url>,
host: Option<String>,
#[serde_as(as = "Option<serde_with::BoolFromInt>")]
#[serde(rename = "docker.tls.verify")]
tls_verify: Option<bool>,
Expand Down Expand Up @@ -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")
Expand All @@ -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 {
Expand Down Expand Up @@ -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";

Expand Down
2 changes: 2 additions & 0 deletions testcontainers/src/runners/async_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand Down Expand Up @@ -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?;
Expand Down
3 changes: 2 additions & 1 deletion testcontainers/tests/async_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<String>(None)
Expand Down
Loading