Skip to content

Commit

Permalink
Merge pull request #511 from Luap99/ttl
Browse files Browse the repository at this point in the history 
coredns: use a TTL of 0 for our names
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Oct 4, 2024
2 parents bf6e536 + edbe4e9 commit 51cb505
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 41 deletions.
58 changes: 25 additions & 33 deletions src/dns/coredns.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ use std::time::Duration;
use tokio::net::TcpListener;
use tokio::net::UdpSocket;

// Containers can be recreated with different ips quickly so
// do not let the clients cache to dns response for to long,
// aardvark-dns runs on the same host so caching is not that important.
// see https://github.com/containers/netavark/discussions/644
const CONTAINER_TTL: u32 = 60;

pub struct CoreDns {
rx: flume::Receiver<()>, // kill switch receiver
inner: CoreDnsData,
Expand Down Expand Up @@ -428,15 +422,13 @@ fn reply_ptr(
let mut req_clone = req.clone();
for entry in reverse_lookup {
if let Ok(answer) = Name::from_ascii(format!("{}.", entry)) {
req_clone.add_answer(
Record::new()
.set_name(Name::from_str_relaxed(name).unwrap_or_default())
.set_ttl(CONTAINER_TTL)
.set_rr_type(RecordType::PTR)
.set_dns_class(DNSClass::IN)
.set_data(Some(RData::PTR(rdata::PTR(answer))))
.clone(),
);
let mut record = Record::new();
record
.set_name(Name::from_str_relaxed(name).unwrap_or_default())
.set_rr_type(RecordType::PTR)
.set_dns_class(DNSClass::IN)
.set_data(Some(RData::PTR(rdata::PTR(answer))));
req_clone.add_answer(record);
}
}
return Some(req_clone);
Expand Down Expand Up @@ -464,29 +456,29 @@ fn reply_ip<'a>(
if record_type == RecordType::A {
for record_addr in resolved_ip_list {
if let IpAddr::V4(ipv4) = record_addr {
req.add_answer(
Record::new()
.set_name(request_name.clone())
.set_ttl(CONTAINER_TTL)
.set_rr_type(RecordType::A)
.set_dns_class(DNSClass::IN)
.set_data(Some(RData::A(rdata::A(ipv4))))
.clone(),
);
let mut record = Record::new();
// DO NOT SET A TTL, the default is 0 which means client should not cache it.
// Containers can be be restarted with a different ip at any time so allowing
// caches here doesn't make much sense given the server is local and queries
// should be fast enough anyway.
record
.set_name(request_name.clone())
.set_rr_type(RecordType::A)
.set_dns_class(DNSClass::IN)
.set_data(Some(RData::A(rdata::A(ipv4))));
req.add_answer(record);
}
}
} else if record_type == RecordType::AAAA {
for record_addr in resolved_ip_list {
if let IpAddr::V6(ipv6) = record_addr {
req.add_answer(
Record::new()
.set_name(request_name.clone())
.set_ttl(CONTAINER_TTL)
.set_rr_type(RecordType::AAAA)
.set_dns_class(DNSClass::IN)
.set_data(Some(RData::AAAA(rdata::AAAA(ipv6))))
.clone(),
);
let mut record = Record::new();
record
.set_name(request_name.clone())
.set_rr_type(RecordType::AAAA)
.set_dns_class(DNSClass::IN)
.set_data(Some(RData::AAAA(rdata::AAAA(ipv6))));
req.add_answer(record);
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions test/100-basic-name-resolution.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ load helpers
gw=$(echo "$config_a1" | jq -r .network_info.podman1.subnets[0].gateway)
create_container "$config_a1"
a1_pid=$CONTAINER_NS_PID
run_in_container_netns "$a1_pid" "dig" "+short" "aone" "@$gw"
assert "$ip_a1"
run_in_container_netns "$a1_pid" "dig" "aone" "@$gw"
# check for TTL 0 here as well
assert "$output" =~ "aone\.[[:space:]]*0[[:space:]]*IN[[:space:]]*A[[:space:]]*$ip_a1"
# Set recursion bit is already set if requested so output must not
# contain unexpected warning.
assert "$output" !~ "WARNING: recursion requested but not available"
Expand Down
12 changes: 6 additions & 6 deletions test/500-reverse-lookups.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ load helpers
dig_reverse "$a1_pid" "$a2_ip" "$gw"
echo -e "Output:\n${output}\n"
a2_expected_name=$(echo $a2_ip | awk -F. '{printf "%d.%d.%d.%d.in-addr.arpa.", $4, $3, $2, $1}')
assert "$output" =~ "$a2_expected_name[ ].*[ ]atwo\."
assert "$output" =~ "$a2_expected_name[ ].*[ ]a2\."
assert "$output" =~ "$a2_expected_name[ ].*[ ]2a\."
assert "$output" =~ "$a2_expected_name[[:space:]]*0[[:space:]]*IN[[:space:]]*PTR[[:space:]]*atwo\."
assert "$output" =~ "$a2_expected_name[[:space:]]*0[[:space:]]*IN[[:space:]]*PTR[[:space:]]*a2\."
assert "$output" =~ "$a2_expected_name[[:space:]]*0[[:space:]]*IN[[:space:]]*PTR[[:space:]]*2a\."
dig_reverse "$a2_pid" "$a1_ip" "$gw"
echo -e "Output:\n${output}\n"
a1_expected_name=$(echo $a1_ip | awk -F. '{printf "%d.%d.%d.%d.in-addr.arpa.", $4, $3, $2, $1}')
assert "$output" =~ "$a1_expected_name[ ].*[ ]aone\."
assert "$output" =~ "$a1_expected_name[ ].*[ ]a1\."
assert "$output" =~ "$a1_expected_name[ ].*[ ]1a\."
assert "$output" =~ "$a1_expected_name[[:space:]]*0[[:space:]]*IN[[:space:]]*PTR[[:space:]]*aone\."
assert "$output" =~ "$a1_expected_name[[:space:]]*0[[:space:]]*IN[[:space:]]*PTR[[:space:]]*a1\."
assert "$output" =~ "$a1_expected_name[[:space:]]*0[[:space:]]*IN[[:space:]]*PTR[[:space:]]*1a\."
}

@test "check reverse lookups on ipaddress v6" {
Expand Down

1 comment on commit 51cb505

@packit-as-a-service
Copy link

Choose a reason for hiding this comment

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

podman-next COPR build failed. @containers/packit-build please check.

Please sign in to comment.