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

netavark ,aardvark: accept and populate custom dns_servers for containers #452

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion src/commands/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ impl Setup {
}
}
debug!("{:?}", "Setting up...");

let network_options = match network::types::NetworkOptions::load(&input_file) {
Ok(opts) => opts,
Err(e) => {
Expand Down Expand Up @@ -95,6 +94,7 @@ impl Setup {
firewall: firewall_driver.as_ref(),
container_id: &network_options.container_id,
container_name: &network_options.container_name,
container_dns_servers: &network_options.dns_servers,
netns_host: hostns.fd,
netns_container: netns.fd,
network,
Expand Down
1 change: 1 addition & 0 deletions src/commands/teardown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ impl Teardown {
firewall: firewall_driver.as_ref(),
container_id: &network_options.container_id,
container_name: &network_options.container_name,
container_dns_servers: &network_options.dns_servers,
netns_host: hostns.fd,
netns_container: netns.fd,
network,
Expand Down
20 changes: 18 additions & 2 deletions src/dns/aardvark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub struct AardvarkEntry<'a> {
pub container_ips_v4: Vec<Ipv4Addr>,
pub container_ips_v6: Vec<Ipv6Addr>,
pub container_names: Vec<String>,
pub container_dns_servers: &'a Option<Vec<IpAddr>>,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -249,9 +250,24 @@ impl Aardvark {
.collect::<Vec<String>>()
.join(",");

let dns_server = if let Some(dns_servers) = &entry.container_dns_servers {
if !dns_servers.is_empty() {
let dns_server_collected = dns_servers
.iter()
.map(|g| g.to_string())
.collect::<Vec<String>>()
.join(",");
format!(" {}", dns_server_collected)
} else {
"".to_string()
}
} else {
"".to_string()
};

let data = format!(
"{} {} {} {}\n",
entry.container_id, ipv4s, ipv6s, container_names
"{} {} {} {}{}\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are we prepending a space so that this is separated from the previous field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry.container_id, ipv4s, ipv6s, container_names, dns_server
);

file.write_all(data.as_bytes())?; // return error if write fails
Expand Down
8 changes: 8 additions & 0 deletions src/network/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,16 @@ impl driver::NetworkDriver for Bridge<'_> {
container_ips_v4: ipv4,
container_ips_v6: ipv6,
container_names: names,
container_dns_servers: self.info.container_dns_servers,
})
} else {
// If --dns-enable=false and --dns was set then return following DNS servers
// in status_block so podman can use these and populate resolv.conf
if let Some(container_dns_servers) = self.info.container_dns_servers {
let _ = response
.dns_server_ips
.insert(container_dns_servers.clone());
}
None
};

Expand Down
3 changes: 3 additions & 0 deletions src/network/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use crate::{
firewall::FirewallDriver,
};

use std::net::IpAddr;

use super::{
bridge::Bridge,
constants,
Expand All @@ -17,6 +19,7 @@ pub struct DriverInfo<'a> {
pub firewall: &'a dyn FirewallDriver,
pub container_id: &'a String,
pub container_name: &'a String,
pub container_dns_servers: &'a Option<Vec<IpAddr>>,
pub netns_host: RawFd,
pub netns_container: RawFd,
pub network: &'a Network,
Expand Down
4 changes: 4 additions & 0 deletions src/network/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ pub struct NetworkOptions {
/// The port mappings for this container.
#[serde(rename = "port_mappings")]
pub port_mappings: Option<Vec<PortMapping>>,

/// Custom DNS servers for aardvark-dns.
#[serde(rename = "dns_servers")]
pub dns_servers: Option<Vec<IpAddr>>,
}

// PerNetworkOptions are options which should be set on a per network basis
Expand Down
33 changes: 33 additions & 0 deletions test/100-bridge-iptables.bats
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,39 @@ fw_driver=iptables
run_netavark --file ${TESTSDIR}/testfiles/ipv6-bridge.json teardown $(get_container_netns_path)
}

@test "$fw_driver - bridge driver must generate config for aardvark with custom dns server" {
# TODO !!! Unkip after https://github.com/containers/aardvark-dns/pull/240
skip "unskip after https://github.com/containers/aardvark-dns/pull/240"
# get a random port directly to avoid low ports e.g. 53 would not create iptables
flouthoc marked this conversation as resolved.
Show resolved Hide resolved
dns_port=$((RANDOM+10000))

# hack to make aardvark-dns run when really root or when running as user with
# podman unshare --rootless-netns; since netavark runs aardvark with systemd-run
# it needs to know if it should use systemd user instance or not.
# iptables are still setup identically.
rootless=false
if [[ ! -e "/run/dbus/system_bus_socket" ]]; then
rootless=true
fi

mkdir -p "$NETAVARK_TMPDIR/config"

NETAVARK_DNS_PORT="$dns_port" run_netavark --file ${TESTSDIR}/testfiles/dualstack-bridge-custom-dns-server.json \
--rootless "$rootless" --config "$NETAVARK_TMPDIR/config" \
setup $(get_container_netns_path)

# check aardvark config and running
run_helper cat "$NETAVARK_TMPDIR/config/aardvark-dns/podman1"
assert "${lines[0]}" =~ "10.89.3.1,fd10:88:a::1" "aardvark set to listen to all IPs"
assert "${lines[1]}" =~ "^[0-9a-f]{64} 10.89.3.2 fd10:88:a::2 somename 8.8.8.8$" "aardvark config's container"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also test at least two custom servers but since we cannot test it right now I fine with keeping like this and add a second test for this once aardvark is ready

assert "${#lines[@]}" = 2 "too many lines in aardvark config"

aardvark_pid=$(cat "$NETAVARK_TMPDIR/config/aardvark-dns/aardvark.pid")
assert "$ardvark_pid" =~ "[0-9]*" "aardvark pid not found"
run_helper ps "$aardvark_pid"
assert "${lines[1]}" =~ ".*aardvark-dns --config $NETAVARK_TMPDIR/config/aardvark-dns -p $dns_port run" "aardvark not running or bad options"
}

@test "$fw_driver - dual stack dns with alt port" {
# get a random port directly to avoid low ports e.g. 53 would not create iptables
dns_port=$((RANDOM+10000))
Expand Down
38 changes: 38 additions & 0 deletions test/testfiles/dualstack-bridge-custom-dns-server.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"container_id": "f031bf33eecba75d0d84952337b1ceef6a239eb8e94b48aee0993d0791345325",
"container_name": "somename",
"dns_servers": ["8.8.8.8"],
"networks": {
"podman1": {
"static_ips": [
"10.89.3.2",
"fd10:88:a::2"
],
"interface_name": "eth0"
}
},
"network_info": {
"podman1": {
"name": "podman1",
"id": "ec79dd0cad82083c8ac5cc23e9542e4ddea813dff60d68258d36e84f6393b63b",
"driver": "bridge",
"network_interface": "podman1",
"subnets": [
{
"subnet": "10.89.3.0/24",
"gateway": "10.89.3.1"
},
{
"subnet": "fd10:88:a::/64",
"gateway": "fd10:88:a::1"
}
],
"ipv6_enabled": true,
"internal": false,
"dns_enabled": true,
"ipam_options": {
"driver": "host-local"
}
}
}
}