Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor FullyQualifiedAuthority::normalize to always return authority #476

Merged
merged 3 commits into from
Feb 28, 2018
Merged

Refactor FullyQualifiedAuthority::normalize to always return authority #476

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
abec410
Refactor FullyQualifiedAuthority::normalize to always return authority
hawkw Feb 27, 2018
86a641a
Review feedback
hawkw Feb 27, 2018
4401cac
unstick ci
hawkw Feb 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions proxy/src/control/discovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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));
Expand Down
197 changes: 113 additions & 84 deletions proxy/src/fully_qualified_authority.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -59,15 +75,21 @@ 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
} else if has_explicit_namespace {
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
};
Expand All @@ -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
Expand All @@ -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`.
Expand All @@ -119,154 +147,155 @@ 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
}
}

#[cfg(test)]
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);
}
}
11 changes: 7 additions & 4 deletions proxy/src/outbound.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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()),
Expand All @@ -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>>()
Expand Down