From 4f2fd7fddebab72b0c2518d5d4718d7e2e39a001 Mon Sep 17 00:00:00 2001
From: John Howard <howardjohn@google.com>
Date: Thu, 6 Jun 2024 20:14:34 -0700
Subject: [PATCH] Drop improper workload DNS lookups (#1120)

Part 1 of https://github.com/istio/ztunnel/issues/1119. This removes the
broken stuff. TODO is to add it back properly, but for now this PR
strictly removes broken code.

(cherry picked from commit fac3e11777a160273ec8cb1663adbb1182ce8c45)
---
 src/dns/server.rs     | 22 +++++++++-------------
 src/state.rs          | 14 +++++---------
 src/state/workload.rs | 12 ------------
 3 files changed, 14 insertions(+), 34 deletions(-)

diff --git a/src/dns/server.rs b/src/dns/server.rs
index 0a4f47963..b27bb7143 100644
--- a/src/dns/server.rs
+++ b/src/dns/server.rs
@@ -380,14 +380,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
             }
         }
 
@@ -1212,18 +1206,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 {
diff --git a/src/state.rs b/src/state.rs
index 157b4734a..73d10e122 100644
--- a/src/state.rs
+++ b/src/state.rs
@@ -227,15 +227,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(
diff --git a/src/state/workload.rs b/src/state/workload.rs
index b5ce19f63..a658169a4 100644
--- a/src/state/workload.rs
+++ b/src/state/workload.rs
@@ -601,8 +601,6 @@ pub struct WorkloadStore {
     pub(super) by_addr: HashMap<NetworkAddress, Arc<Workload>>,
     /// byUid maps workload UIDs to workloads
     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>>,
 }
@@ -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(),
         }
@@ -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 {
@@ -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) {
@@ -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()