mirror of https://github.com/linkerd/linkerd2.git
Refactor FullyQualifiedAuthority::normalize to always return authority (#476)
As requested by @briansmith in https://github.com/runconduit/conduit/issues/415#issuecomment-369026560 and https://github.com/runconduit/conduit/issues/415#issuecomment-369032059, I've refactored `FullyQualifiedAuthority::normalize` to _always_ return a `FullyQualifiedAuthority`, along with a boolean value indicating whether or not the Destination service should be used for that authority. This is in contrast to returning an `Option<FullyQualifiedAuthority>` where `None` indicated that the Destination service should not be used, which is what this function did previously. This is required for further progress on #415. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This commit is contained in:
parent
1e611c21c6
commit
41bef41eb5
|
@ -263,7 +263,8 @@ where
|
|||
Entry::Vacant(vac) => {
|
||||
let req = Destination {
|
||||
scheme: "k8s".into(),
|
||||
path: vac.key().without_trailing_dot().into(),
|
||||
path: vac.key().without_trailing_dot()
|
||||
.as_str().into(),
|
||||
};
|
||||
// TODO: Can grpc::Request::new be removed?
|
||||
let mut svc = DestinationSvc::new(client.lift_ref());
|
||||
|
@ -298,7 +299,7 @@ where
|
|||
trace!("Destination.Get reconnect {:?}", auth);
|
||||
let req = Destination {
|
||||
scheme: "k8s".into(),
|
||||
path: auth.without_trailing_dot().into(),
|
||||
path: auth.without_trailing_dot().as_str().into(),
|
||||
};
|
||||
let mut svc = DestinationSvc::new(client.lift_ref());
|
||||
let response = svc.get(grpc::Request::new(req));
|
||||
|
|
|
@ -5,9 +5,16 @@ use std::str::FromStr;
|
|||
|
||||
use http::uri::Authority;
|
||||
|
||||
/// A normalized `Authority`.
|
||||
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
|
||||
pub struct FullyQualifiedAuthority(Authority);
|
||||
|
||||
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
|
||||
pub struct NamedAddress {
|
||||
pub name: FullyQualifiedAuthority,
|
||||
pub use_destination_service: bool
|
||||
}
|
||||
|
||||
impl FullyQualifiedAuthority {
|
||||
/// Normalizes the name according to Kubernetes service naming conventions.
|
||||
/// Case folding is not done; that is done internally inside `Authority`.
|
||||
|
@ -16,11 +23,15 @@ impl FullyQualifiedAuthority {
|
|||
///
|
||||
/// Returns `None` is authority doesn't look like a local Kubernetes service.
|
||||
pub fn normalize(authority: &Authority, default_namespace: Option<&str>,
|
||||
default_zone: Option<&str>)
|
||||
-> Option<FullyQualifiedAuthority> {
|
||||
default_zone: Option<&str>)
|
||||
-> NamedAddress
|
||||
{
|
||||
// Don't change IP-address-based authorities.
|
||||
if IpAddr::from_str(authority.host()).is_ok() {
|
||||
return None;
|
||||
return NamedAddress {
|
||||
name: FullyQualifiedAuthority(authority.clone()),
|
||||
use_destination_service: false,
|
||||
}
|
||||
};
|
||||
|
||||
// TODO: `Authority` doesn't have a way to get the serialized form of the
|
||||
|
@ -38,7 +49,12 @@ impl FullyQualifiedAuthority {
|
|||
if name.ends_with('.') {
|
||||
let authority = authority.clone().into_bytes();
|
||||
let normalized = authority.slice(0, authority.len() - 1);
|
||||
return Some(FullyQualifiedAuthority(Authority::from_shared(normalized).unwrap()));
|
||||
let authority = Authority::from_shared(normalized).unwrap();
|
||||
let name = FullyQualifiedAuthority(authority);
|
||||
return NamedAddress {
|
||||
name,
|
||||
use_destination_service: true,
|
||||
}
|
||||
}
|
||||
|
||||
// parts should have a maximum 4 of pieces (name, namespace, svc, zone)
|
||||
|
@ -59,7 +75,10 @@ impl FullyQualifiedAuthority {
|
|||
let append_svc = if let Some(part) = parts.next() {
|
||||
if !part.eq_ignore_ascii_case("svc") {
|
||||
// if not "$name.$namespace.svc", treat as external
|
||||
return None;
|
||||
return NamedAddress {
|
||||
name: FullyQualifiedAuthority(authority.clone()),
|
||||
use_destination_service: false,
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
|
@ -67,7 +86,10 @@ impl FullyQualifiedAuthority {
|
|||
true
|
||||
} else if namespace_to_append.is_none() {
|
||||
// We can't append ".svc" without a namespace, so treat as external.
|
||||
return None;
|
||||
return NamedAddress {
|
||||
name: FullyQualifiedAuthority(authority.clone()),
|
||||
use_destination_service: false,
|
||||
}
|
||||
} else {
|
||||
true
|
||||
};
|
||||
|
@ -78,7 +100,10 @@ impl FullyQualifiedAuthority {
|
|||
if !zone.eq_ignore_ascii_case(default_zone) {
|
||||
// if "a.b.svc.foo" and zone is not "foo",
|
||||
// treat as external
|
||||
return None;
|
||||
return NamedAddress {
|
||||
name: FullyQualifiedAuthority(authority.clone()),
|
||||
use_destination_service: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
|
@ -99,7 +124,10 @@ impl FullyQualifiedAuthority {
|
|||
|
||||
// If we're not going to change anything then don't allocate anything.
|
||||
if additional_len == 0 {
|
||||
return Some(FullyQualifiedAuthority(authority.clone()));
|
||||
return NamedAddress {
|
||||
name: FullyQualifiedAuthority(authority.clone()),
|
||||
use_destination_service: true,
|
||||
}
|
||||
}
|
||||
|
||||
// `authority.as_str().len()` includes the length of `colon_port`.
|
||||
|
@ -119,12 +147,17 @@ impl FullyQualifiedAuthority {
|
|||
}
|
||||
normalized.extend_from_slice(colon_port.as_bytes());
|
||||
|
||||
Some(FullyQualifiedAuthority(Authority::from_shared(normalized.freeze())
|
||||
.expect("syntactically-valid authority")))
|
||||
let name = Authority::from_shared(normalized.freeze())
|
||||
.expect("syntactically-valid authority");
|
||||
let name = FullyQualifiedAuthority(name);
|
||||
NamedAddress {
|
||||
name,
|
||||
use_destination_service: true,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn without_trailing_dot(&self) -> &str {
|
||||
self.0.as_str()
|
||||
pub fn without_trailing_dot(&self) -> &Authority {
|
||||
&self.0
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -132,141 +165,137 @@ impl FullyQualifiedAuthority {
|
|||
mod tests {
|
||||
#[test]
|
||||
fn test_normalized_authority() {
|
||||
fn f(input: &str, default_namespace: Option<&str>,
|
||||
fn local(input: &str, default_namespace: Option<&str>,
|
||||
default_zone: Option<&str>)
|
||||
-> String {
|
||||
use bytes::Bytes;
|
||||
use http::uri::Authority;
|
||||
|
||||
let input = Authority::from_shared(Bytes::from(input.as_bytes())).unwrap();
|
||||
let output = match super::FullyQualifiedAuthority::normalize(
|
||||
&input, default_namespace, default_zone) {
|
||||
Some(output) => output,
|
||||
None => panic!(
|
||||
"unexpected None for input={:?}, default_namespace={:?}, default_zone={:?}",
|
||||
input,
|
||||
default_namespace,
|
||||
default_zone
|
||||
),
|
||||
};
|
||||
output.without_trailing_dot().into()
|
||||
let input = Authority::from_shared(Bytes::from(input.as_bytes()))
|
||||
.unwrap();
|
||||
let output = super::FullyQualifiedAuthority::normalize(
|
||||
&input, default_namespace, default_zone);
|
||||
assert_eq!(output.use_destination_service, true);
|
||||
output.name.without_trailing_dot().as_str().into()
|
||||
}
|
||||
|
||||
fn none(input: &str, default_namespace: Option<&str>,
|
||||
fn external(input: &str, default_namespace: Option<&str>,
|
||||
default_zone: Option<&str>) {
|
||||
use bytes::Bytes;
|
||||
use http::uri::Authority;
|
||||
|
||||
let input = Authority::from_shared(Bytes::from(input.as_bytes())).unwrap();
|
||||
assert_eq!(None, super::FullyQualifiedAuthority::normalize(
|
||||
&input, default_namespace, default_zone));
|
||||
let output = super::FullyQualifiedAuthority::normalize(
|
||||
&input, default_namespace, default_zone);
|
||||
assert_eq!(output.use_destination_service, false);
|
||||
assert_eq!(output.name.without_trailing_dot().as_str(), input);
|
||||
}
|
||||
|
||||
none("name", None, None);
|
||||
external("name", None, None);
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name.namespace", None, None));
|
||||
local("name.namespace", None, None));
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name.namespace.svc", None, None));
|
||||
local("name.namespace.svc", None, None));
|
||||
assert_eq!("name.namespace.svc.cluster",
|
||||
f("name.namespace.svc.cluster", None, None));
|
||||
local("name.namespace.svc.cluster", None, None));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc.cluster.local", None, None));
|
||||
local("name.namespace.svc.cluster.local", None, None));
|
||||
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name", Some("namespace"), None));
|
||||
local("name", Some("namespace"), None));
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name.namespace", Some("namespace"), None));
|
||||
local("name.namespace", Some("namespace"), None));
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name.namespace.svc", Some("namespace"), None));
|
||||
local("name.namespace.svc", Some("namespace"), None));
|
||||
assert_eq!("name.namespace.svc.cluster",
|
||||
f("name.namespace.svc.cluster", Some("namespace"), None));
|
||||
local("name.namespace.svc.cluster", Some("namespace"), None));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc.cluster.local", Some("namespace"), None));
|
||||
local("name.namespace.svc.cluster.local", Some("namespace"), None));
|
||||
|
||||
none("name", None, Some("cluster.local"));
|
||||
external("name", None, Some("cluster.local"));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace", None, Some("cluster.local")));
|
||||
local("name.namespace", None, Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc", None, Some("cluster.local")));
|
||||
none("name.namespace.svc.cluster", None, Some("cluster.local"));
|
||||
local("name.namespace.svc", None, Some("cluster.local")));
|
||||
external("name.namespace.svc.cluster", None, Some("cluster.local"));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc.cluster.local", None, Some("cluster.local")));
|
||||
local("name.namespace.svc.cluster.local", None, Some("cluster.local")));
|
||||
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name", Some("namespace"), Some("cluster.local")));
|
||||
local("name", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc", Some("namespace"), Some("cluster.local")));
|
||||
none("name.namespace.svc.cluster", Some("namespace"), Some("cluster.local"));
|
||||
local("name.namespace.svc", Some("namespace"), Some("cluster.local")));
|
||||
external("name.namespace.svc.cluster", Some("namespace"), Some("cluster.local"));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc.cluster.local", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace.svc.cluster.local", Some("namespace"), Some("cluster.local")));
|
||||
|
||||
// Fully-qualified names end with a dot and aren't modified except by removing the dot.
|
||||
assert_eq!("name",
|
||||
f("name.", None, None));
|
||||
local("name.", None, None));
|
||||
assert_eq!("name.namespace",
|
||||
f("name.namespace.", None, None));
|
||||
local("name.namespace.", None, None));
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name.namespace.svc.", None, None));
|
||||
local("name.namespace.svc.", None, None));
|
||||
assert_eq!("name.namespace.svc.cluster",
|
||||
f("name.namespace.svc.cluster.", None, None));
|
||||
local("name.namespace.svc.cluster.", None, None));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc.cluster.local.", None, None));
|
||||
local("name.namespace.svc.cluster.local.", None, None));
|
||||
assert_eq!("name",
|
||||
f("name.", Some("namespace"), None));
|
||||
local("name.", Some("namespace"), None));
|
||||
assert_eq!("name.namespace",
|
||||
f("name.namespace.", Some("namespace"), None));
|
||||
local("name.namespace.", Some("namespace"), None));
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name.namespace.svc.", Some("namespace"), None));
|
||||
local("name.namespace.svc.", Some("namespace"), None));
|
||||
assert_eq!("name.namespace.svc.cluster",
|
||||
f("name.namespace.svc.cluster.", Some("namespace"), None));
|
||||
local("name.namespace.svc.cluster.", Some("namespace"), None));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc.cluster.local.", Some("namespace"), None));
|
||||
local("name.namespace.svc.cluster.local.", Some("namespace"), None));
|
||||
assert_eq!("name",
|
||||
f("name.", Some("namespace"), Some("cluster.local")));
|
||||
local("name.", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace",
|
||||
f("name.namespace.", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace.", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc",
|
||||
f("name.namespace.svc.", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace.svc.", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc.cluster",
|
||||
f("name.namespace.svc.cluster.", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace.svc.cluster.", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc.cluster.local",
|
||||
f("name.namespace.svc.cluster.local.", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace.svc.cluster.local.", Some("namespace"), Some("cluster.local")));
|
||||
|
||||
// Ports are preserved.
|
||||
assert_eq!("name.namespace.svc.cluster.local:1234",
|
||||
f("name:1234", Some("namespace"), Some("cluster.local")));
|
||||
local("name:1234", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc.cluster.local:1234",
|
||||
f("name.namespace:1234", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace:1234", Some("namespace"), Some("cluster.local")));
|
||||
assert_eq!("name.namespace.svc.cluster.local:1234",
|
||||
f("name.namespace.svc:1234", Some("namespace"), Some("cluster.local")));
|
||||
none("name.namespace.svc.cluster:1234", Some("namespace"), Some("cluster.local"));
|
||||
local("name.namespace.svc:1234", Some("namespace"), Some("cluster.local")));
|
||||
external("name.namespace.svc.cluster:1234", Some("namespace"), Some("cluster.local"));
|
||||
assert_eq!("name.namespace.svc.cluster.local:1234",
|
||||
f("name.namespace.svc.cluster.local:1234", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace.svc.cluster.local:1234", Some("namespace"), Some("cluster.local")));
|
||||
|
||||
// "SVC" is recognized as being equivalent to "svc"
|
||||
assert_eq!("name.namespace.SVC.cluster.local",
|
||||
f("name.namespace.SVC", Some("namespace"), Some("cluster.local")));
|
||||
none("name.namespace.SVC.cluster", Some("namespace"), Some("cluster.local"));
|
||||
local("name.namespace.SVC", Some("namespace"), Some("cluster.local")));
|
||||
external("name.namespace.SVC.cluster", Some("namespace"), Some("cluster.local"));
|
||||
assert_eq!("name.namespace.SVC.cluster.local",
|
||||
f("name.namespace.SVC.cluster.local", Some("namespace"), Some("cluster.local")));
|
||||
local("name.namespace.SVC.cluster.local", Some("namespace"), Some("cluster.local")));
|
||||
|
||||
// IPv4 addresses are left unchanged.
|
||||
none("1.2.3.4", Some("namespace"), Some("cluster.local"));
|
||||
none("1.2.3.4:1234", Some("namespace"), Some("cluster.local"));
|
||||
none("127.0.0.1", Some("namespace"), Some("cluster.local"));
|
||||
none("127.0.0.1:8080", Some("namespace"), Some("cluster.local"));
|
||||
none("127.0.0.1", None, Some("cluster.local"));
|
||||
none("127.0.0.1", Some("namespace"), None);
|
||||
none("127.0.0.1", None, None);
|
||||
none("127.0.0.1", Some("1"), Some("1"));
|
||||
external("1.2.3.4", Some("namespace"), Some("cluster.local"));
|
||||
external("1.2.3.4:1234", Some("namespace"), Some("cluster.local"));
|
||||
external("127.0.0.1", Some("namespace"), Some("cluster.local"));
|
||||
external("127.0.0.1:8080", Some("namespace"), Some("cluster.local"));
|
||||
external("127.0.0.1", None, Some("cluster.local"));
|
||||
external("127.0.0.1", Some("namespace"), None);
|
||||
external("127.0.0.1", None, None);
|
||||
external("127.0.0.1", Some("1"), Some("1"));
|
||||
|
||||
// IPv6 addresses are left unchanged.
|
||||
none("[::1]", Some("namespace"), Some("cluster.local"));
|
||||
none("[::1]:1234", Some("namespace"), Some("cluster.local"));
|
||||
none("[::1]", None, Some("cluster.local"));
|
||||
none("[::1]", Some("namespace"), None);
|
||||
none("[::1]", None, None);
|
||||
external("[::1]", Some("namespace"), Some("cluster.local"));
|
||||
external("[::1]:1234", Some("namespace"), Some("cluster.local"));
|
||||
external("[::1]", None, Some("cluster.local"));
|
||||
external("[::1]", Some("namespace"), None);
|
||||
external("[::1]", None, None);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -17,7 +17,7 @@ use bind::{self, Bind, Protocol};
|
|||
use control::{self, discovery};
|
||||
use control::discovery::Bind as BindTrait;
|
||||
use ctx;
|
||||
use fully_qualified_authority::FullyQualifiedAuthority;
|
||||
use fully_qualified_authority::{FullyQualifiedAuthority, NamedAddress};
|
||||
use timeout::Timeout;
|
||||
|
||||
type BindProtocol<B> = bind::BindProtocol<Arc<ctx::Proxy>, B>;
|
||||
|
@ -72,7 +72,7 @@ where
|
|||
>>>>;
|
||||
|
||||
fn recognize(&self, req: &Self::Request) -> Option<Self::Key> {
|
||||
let local = req.uri().authority_part().and_then(|authority| {
|
||||
let local = req.uri().authority_part().map(|authority| {
|
||||
FullyQualifiedAuthority::normalize(
|
||||
authority,
|
||||
self.default_namespace.as_ref().map(|s| s.as_ref()),
|
||||
|
@ -87,8 +87,11 @@ where
|
|||
// In practice, this shouldn't ever happen, since we expect the proxy
|
||||
// to be run on Linux servers, with iptables setup, so there should
|
||||
// always be an original destination.
|
||||
let dest = if let Some(local) = local {
|
||||
Destination::LocalSvc(local)
|
||||
let dest = if let Some(NamedAddress {
|
||||
name,
|
||||
use_destination_service: true
|
||||
}) = local {
|
||||
Destination::LocalSvc(name)
|
||||
} else {
|
||||
let orig_dst = req.extensions()
|
||||
.get::<Arc<ctx::transport::Server>>()
|
||||
|
|
Loading…
Reference in New Issue