istio/ztunnel

Fixes around workload.hostname

howardjohn opened this issue · 0 comments

Currently, Istiod will only generate a hostname workload for SE/WE of DNS type. In ztunnel, we try to match DNS requests to it:

ztunnel/src/dns/server.rs

Lines 390 to 396 in 2e41538

} 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,
});
.

This is broken.

apiVersion: networking.istio.io/v1
kind: ServiceEntry
metadata:
  name: external-service
  namespace: echo
spec:
  addresses:
  - 240.240.240.239
  endpoints:
  - address: external.external.svc.cluster.local
  exportTo:
  - .
  hosts:
  - fake.external.com

This config should NOT trigger responses for 'external.external.svc.cluster.local'.

What we should be allowing is looking up pods in headless services. The logic is tricky, see https://github.com/istio/istio/blob/f69047b03842074eb2f56fef5d9ad7ba326de4cc/pkg/dns/server/name_table.go#L58. We don't do that today.

Finally, WorkloadStore has a by_hostname->Workload. This is not right; a hostname is not unique (definitely not across clusters!).

So a few changes needed

  • Turn off DNS lookups for SE/WE endpoints (ill do this)
  • Turn on DNS lookups for headless pods (not super high priority, we will forward them anyways)
  • Make by_hostname correct, assuming we still need it

Most of by_hostname's usage beyond DNS is in gateway addresses. I am not sure actually these should allow workload hostnames, only service ones. So maybe we can remove it entirely. However, we will probably need it to do headless pods... but we should make sure its not used in all those other cases anyways. So, regardless, we need to clean it up