Skip to content

Commit

Permalink
Drop improper workload DNS lookups
Browse files Browse the repository at this point in the history
Part 1 of istio#1119. This removes the
broken stuff. TODO is to add it back properly, but for now this PR
strictly removes broken code.
  • Loading branch information
howardjohn committed Jun 6, 2024
1 parent 83f9791 commit 74e6ceb
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 34 deletions.
22 changes: 9 additions & 13 deletions src/dns/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,14 +387,8 @@ impl Store {
name: search_name,
alias,
});
} else if let Some(wl) = state.workloads.find_hostname(&search_name_str) {
// Didn't find a service, try a workload.
return Some(ServerMatch {
server: Address::Workload(wl),
name: search_name,
alias,
});
}
// TODO(): add support for workload lookups for headless pods
}
}

Expand Down Expand Up @@ -1219,18 +1213,20 @@ mod tests {
a(n("headless.ns1.svc.cluster.local."), ipv4("31.31.31.31"))],
..Default::default()
},
// TODO(https://github.com/istio/ztunnel/issues/1119)
Case {
name: "success: k8s pod - fqdn",
name: "todo: k8s pod - fqdn",
host: "headless.pod0.ns1.svc.cluster.local.",
expect_records: vec![
a(n("headless.pod0.ns1.svc.cluster.local."), ipv4("30.30.30.30"))],
expect_authoritative: false, // forwarded.
expect_code: ResponseCode::NXDomain,
..Default::default()
},
// TODO(https://github.com/istio/ztunnel/issues/1119)
Case {
name: "success: k8s pod - name.domain.ns",
name: "todo: k8s pod - name.domain.ns",
host: "headless.pod0.ns1.",
expect_records: vec![
a(n("headless.pod0.ns1."), ipv4("30.30.30.30"))],
expect_authoritative: false, // forwarded.
expect_code: ResponseCode::NXDomain,
..Default::default()
},
Case {
Expand Down
14 changes: 5 additions & 9 deletions src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,11 @@ impl ProxyState {
pub fn find_hostname(&self, name: &NamespacedHostname) -> Option<Address> {
// Hostnames for services are more common, so lookup service first and fallback
// to workload.
match self.services.get_by_namespaced_host(name) {
None => {
// Workload hostnames are globally unique, so ignore the namespace.
self.workloads
.find_hostname(&name.hostname)
.map(Address::Workload)
}
Some(svc) => Some(Address::Service(svc)),
}
// We do not looking up workloads by hostname. We could, but we only allow referencing "frontends",
// not backends
self.services
.get_by_namespaced_host(name)
.map(Address::Service)
}

fn find_upstream(
Expand Down
12 changes: 0 additions & 12 deletions src/state/workload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,6 @@ pub struct WorkloadStore {
by_addr: HashMap<NetworkAddress, Arc<Workload>>,
/// byUid maps workload UIDs to workloads
pub(super) by_uid: HashMap<Strng, Arc<Workload>>,
/// byHostname maps workload hostname to workloads.
by_hostname: HashMap<Strng, Arc<Workload>>,
// Identity->Set of UIDs. Only stores local nodes
by_identity: HashMap<Identity, HashSet<Strng>>,
}
Expand All @@ -612,7 +610,6 @@ impl Default for WorkloadStore {
WorkloadStore {
insert_notifier: Sender::new(()),
by_addr: Default::default(),
by_hostname: Default::default(),
by_identity: Default::default(),
by_uid: Default::default(),
}
Expand All @@ -635,9 +632,6 @@ impl WorkloadStore {
self.by_addr
.insert(network_addr(w.network.clone(), *ip), w.clone());
}
if !w.hostname.is_empty() {
self.by_hostname.insert(w.hostname.clone(), w.clone());
}
self.by_uid.insert(w.uid.clone(), w.clone());
// Only track local nodes to avoid overhead
if track_identity {
Expand All @@ -663,7 +657,6 @@ impl WorkloadStore {
self.by_addr
.remove(&network_addr(prev.network.clone(), *wip));
}
self.by_hostname.remove(&prev.hostname);

let id = prev.identity();
if let Some(set) = self.by_identity.get_mut(&id) {
Expand All @@ -682,11 +675,6 @@ impl WorkloadStore {
self.by_addr.get(addr).cloned()
}

/// Finds the workload by hostname.
pub fn find_hostname(&self, hostname: &Strng) -> Option<Arc<Workload>> {
self.by_hostname.get(hostname).cloned()
}

/// Finds the workload by uid.
pub fn find_uid(&self, uid: &Strng) -> Option<Arc<Workload>> {
self.by_uid.get(uid).cloned()
Expand Down

0 comments on commit 74e6ceb

Please sign in to comment.