diff --git a/examples/localhost.yaml b/examples/localhost.yaml index a75800680b..8e37583449 100644 --- a/examples/localhost.yaml +++ b/examples/localhost.yaml @@ -46,6 +46,7 @@ services: 80: 8080 subjectAltNames: - spiffe://cluster.local/ns/default/sa/local + canonical: true - name: remote namespace: default hostname: example2.com @@ -53,3 +54,4 @@ services: - remote/127.10.0.2 ports: 80: 8080 + canonical: true diff --git a/proto/workload.proto b/proto/workload.proto index 74ba6ed9f3..df398d2eae 100644 --- a/proto/workload.proto +++ b/proto/workload.proto @@ -89,6 +89,10 @@ message Service { // Extension provides a mechanism to attach arbitrary additional configuration to an object. repeated Extension extensions = 10; + + // canonical marks this Service as taking priority during hostname lookups, + // when there is not a match in the namespace of the client. + bool canonical = 11; } enum IPFamilies { diff --git a/src/admin.rs b/src/admin.rs index c6e5890de0..382426d90d 100644 --- a/src/admin.rs +++ b/src/admin.rs @@ -693,6 +693,7 @@ mod tests { }), // ..Default::default() // intentionally don't default. we want all fields populated ip_families: 0, extensions: Default::default(), + canonical: true, }; let auth = XdsAuthorization { diff --git a/src/dns/server.rs b/src/dns/server.rs index 886956ba29..68aa78c694 100644 --- a/src/dns/server.rs +++ b/src/dns/server.rs @@ -187,6 +187,26 @@ impl Server { } } +enum MatchReason<'a> { + Canonical(&'a Arc), + First(&'a Arc), + Namespace(&'a Arc), + PreferredNamespace(&'a Arc), + None, +} + +impl<'a> From> for Option<&'a Arc> { + fn from(value: MatchReason<'a>) -> Option<&'a Arc> { + match value { + MatchReason::Canonical(s) + | MatchReason::First(s) + | MatchReason::Namespace(s) + | MatchReason::PreferredNamespace(s) => Some(s), + MatchReason::None => None, + } + } +} + /// A DNS [Resolver] backed by the ztunnel [DemandProxyState]. struct Store { state: DemandProxyState, @@ -390,21 +410,35 @@ impl Store { .cloned() .collect(); - // TODO: ideally we'd sort these by creation time so that the oldest would be used if there are no namespace matches - // presently service doesn't have creation time in WDS, but we could add it - // TODO: if the local namespace doesn't define a service, kube service should be prioritized over se - let service = match services + let service: Option<&Arc> = services .iter() - .find(|service| service.namespace == client.namespace) - { - Some(service) => Some(service), - None => match self.prefered_service_namespace.as_ref() { - Some(prefered_namespace) => services.iter().find_or_first(|service| { - service.namespace == prefered_namespace.as_str() - }), - None => services.first(), - }, - }; + .fold_while(MatchReason::None, |r, s| { + if s.namespace == client.namespace { + itertools::FoldWhile::Done(MatchReason::Namespace(s)) + } else if s.canonical { + itertools::FoldWhile::Continue(MatchReason::Canonical(s)) + } else { + // TODO: deprecate preferred_service_namespace + // https://github.com/istio/ztunnel/issues/1709 + if let Some(preferred_namespace) = + self.prefered_service_namespace.as_ref() + && preferred_namespace.as_str() == s.namespace + && !matches!(r, MatchReason::Canonical(_)) + { + return itertools::FoldWhile::Continue( + MatchReason::PreferredNamespace(s), + ); + } + match r { + MatchReason::None => { + itertools::FoldWhile::Continue(MatchReason::First(s)) + } + _ => itertools::FoldWhile::Continue(r), + } + } + }) + .into_inner() + .into(); // First, lookup the host as a service. if let Some(service) = service { @@ -963,6 +997,7 @@ mod tests { const NS1: &str = "ns1"; const NS2: &str = "ns2"; + const NS3: &str = "ns3"; const PREFERRED: &str = "preferred-ns"; const NW1: Strng = strng::literal!("nw1"); const NW2: Strng = strng::literal!("nw2"); @@ -1394,6 +1429,18 @@ mod tests { expect_records: vec![a(n("everywhere.io."), ipv4("10.10.10.112"))], ..Default::default() }, + Case { + name: "success: canonical services are preferred when no ns-local hostname is present", + host: "canonical.svc", + expect_records: vec![a(n("canonical.svc."), ipv4("10.10.10.141"))], + ..Default::default() + }, + Case { + name: "success: namespace-local service should be preferred over canonical", + host: "canonical.with.local", + expect_records: vec![a(n("canonical.with.local."), ipv4("10.10.10.150"))], + ..Default::default() + }, ]; // Create and start the proxy. @@ -1713,6 +1760,24 @@ mod tests { // Service with the same name in the same namespace // Client in NS1 should use this service xds_namespaced_external_service("everywhere.io", NS1, &[na(NW1, "10.10.10.112")]), + // Service that is canonical should be preferrred when no ns-local definition + xds_namespaced_external_service("canonical.svc", NS2, &[na(NW1, "10.10.10.140")]), + xds_namespaced_external_canonical_service( + "canonical.svc", + NS3, + &[na(NW1, "10.10.10.141")], + ), + // Client in NS1 should prefer local over canonical + xds_namespaced_external_service( + "canonical.with.local", + NS1, + &[na(NW1, "10.10.10.150")], + ), + xds_namespaced_external_canonical_service( + "canonical.with.local", + NS2, + &[na(NW1, "10.10.10.151")], + ), with_fqdn( "details.ns2.svc.cluster.remote", xds_service( @@ -1838,6 +1903,11 @@ mod tests { svc } + fn with_canonical(canonical: bool, mut svc: XdsService) -> XdsService { + svc.canonical = canonical; + svc + } + fn xds_service, S2: AsRef>( name: S1, ns: S2, @@ -1877,6 +1947,14 @@ mod tests { ) } + fn xds_namespaced_external_canonical_service, S2: AsRef>( + hostname: S1, + ns: S2, + vips: &[NetworkAddress], + ) -> XdsService { + with_canonical(true, xds_namespaced_external_service(hostname, ns, vips)) + } + fn xds_workload( name: &str, ns: &str, diff --git a/src/proxy/inbound.rs b/src/proxy/inbound.rs index 51a41615af..72c79a5270 100644 --- a/src/proxy/inbound.rs +++ b/src/proxy/inbound.rs @@ -962,6 +962,7 @@ mod tests { waypoint: waypoint.service_attached(), load_balancer: None, ip_families: None, + canonical: true, }); let workloads = vec![ diff --git a/src/state/service.rs b/src/state/service.rs index ceca1705ac..e621607a4b 100644 --- a/src/state/service.rs +++ b/src/state/service.rs @@ -55,6 +55,9 @@ pub struct Service { #[serde(default, skip_serializing_if = "is_default")] pub ip_families: Option, + + #[serde(default)] + pub canonical: bool, } /// EndpointSet is an abstraction over a set of endpoints. @@ -328,6 +331,7 @@ impl TryFrom<&XdsService> for Service { waypoint, load_balancer: lb, ip_families, + canonical: s.canonical, }; Ok(svc) } diff --git a/src/state/workload.rs b/src/state/workload.rs index 509f20bc0e..021a104175 100644 --- a/src/state/workload.rs +++ b/src/state/workload.rs @@ -1123,6 +1123,7 @@ mod tests { load_balancing: None, ip_families: 0, extensions: Default::default(), + canonical: true, }, ) .unwrap(); @@ -1157,6 +1158,7 @@ mod tests { load_balancing: None, ip_families: 0, extensions: Default::default(), + canonical: true, }, ) .unwrap(); @@ -1214,6 +1216,7 @@ mod tests { load_balancing: None, ip_families: 0, extensions: Default::default(), + canonical: true, }, ) .unwrap(); @@ -1525,6 +1528,7 @@ mod tests { load_balancing: None, ip_families: 0, extensions: Default::default(), + canonical: true, }, ) .unwrap(); @@ -1552,6 +1556,7 @@ mod tests { }), ip_families: 0, extensions: Default::default(), + canonical: true, }, ) .unwrap(); @@ -1603,6 +1608,7 @@ mod tests { }), ip_families: 0, extensions: Default::default(), + canonical: true, }; updater .insert_service( @@ -1624,6 +1630,7 @@ mod tests { load_balancing: None, ip_families: 0, extensions: Default::default(), + canonical: true, }, ) .unwrap(); diff --git a/src/test_helpers.rs b/src/test_helpers.rs index 568e8742ba..f6560829ac 100644 --- a/src/test_helpers.rs +++ b/src/test_helpers.rs @@ -195,6 +195,7 @@ pub fn mock_default_service() -> Service { waypoint: None, load_balancer: None, ip_families: None, + canonical: true, } } @@ -289,6 +290,7 @@ fn test_custom_svc( waypoint: None, load_balancer: None, ip_families: None, + canonical: true, }) } diff --git a/src/test_helpers/linux.rs b/src/test_helpers/linux.rs index cf40cb7a05..d7c45879e4 100644 --- a/src/test_helpers/linux.rs +++ b/src/test_helpers/linux.rs @@ -406,6 +406,7 @@ impl<'a> TestServiceBuilder<'a> { waypoint: None, load_balancer: None, ip_families: None, + canonical: true, }, manager, }