discovery: consume server_name and UriLikeIdentity from proto (#2618)

This change is a follow-up to the work to split the concepts of `ServerId` and `ServerName`. To do that we consume the changes to the protobuf API introduced in: linkerd/linkerd2-proxy-api#285. while keeping things backward compatible. Signed-off-by: Zahari Dichev <[email protected]>
linkerd · Jan 15, 2024 · 9c23fdd · 9c23fdd
1 parent b3dea15
commit 9c23fdd
Show file tree

Hide file tree

Showing 15 changed files with 126 additions and 30 deletions.
diff --git a/Cargo.lock b/Cargo.lock
@@ -1715,6 +1715,7 @@ dependencies = [
  "http-body",
  "linkerd-addr",
  "linkerd-error",
+ "linkerd-identity",
  "linkerd-proxy-core",
  "linkerd-stack",
  "linkerd-tls",
@@ -2253,9 +2254,9 @@ dependencies = [
 
 [[package]]
 name = "linkerd2-proxy-api"
-version = "0.12.0"
+version = "0.13.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "52ad088d0a89f71b0a32a719c082e34fc9ba5c11c670dceab759b5700e3fa914"
+checksum = "d1713a2bd4b823395b97c89a15aeb9d7ec3cd99603e8f46190f1f0b97b6f880a"
 dependencies = [
  "h2",
  "http",

diff --git a/Cargo.toml b/Cargo.toml
@@ -82,5 +82,3 @@ members = [
 debug = 1
 lto = true
 
-# [patch.crates-io]
-# linkerd2-proxy-api = { git = "https://github.com/linkerd/linkerd2-proxy-api", branch = "ver/deps" }
diff --git a/linkerd/app/inbound/Cargo.toml b/linkerd/app/inbound/Cargo.toml
@@ -30,7 +30,7 @@ linkerd-meshtls-rustls = { path = "../../meshtls/rustls", optional = true }
 linkerd-proxy-client-policy = { path = "../../proxy/client-policy" }
 linkerd-tonic-stream = { path = "../../tonic-stream" }
 linkerd-tonic-watch = { path = "../../tonic-watch" }
-linkerd2-proxy-api = { version = "0.12", features = ["inbound"] }
+linkerd2-proxy-api = { version = "0.13", features = ["inbound"] }
 once_cell = "1"
 parking_lot = "0.12"
 rangemap = "1"

diff --git a/linkerd/app/integration/Cargo.toml b/linkerd/app/integration/Cargo.toml
@@ -33,7 +33,7 @@ ipnet = "2"
 linkerd-app = { path = "..", features = ["allow-loopback"] }
 linkerd-app-core = { path = "../core" }
 linkerd-metrics = { path = "../../metrics", features = ["test_util"] }
-linkerd2-proxy-api = { version = "0.12", features = [
+linkerd2-proxy-api = { version = "0.13", features = [
     "destination",
     "arbitrary",
 ] }

diff --git a/linkerd/app/integration/src/controller.rs b/linkerd/app/integration/src/controller.rs
@@ -515,8 +515,9 @@ impl From<DestinationBuilder> for pb::Update {
 
         let tls_identity = identity.map(|name| pb::TlsIdentity {
             strategy: Some(pb::tls_identity::Strategy::DnsLikeIdentity(
-                pb::tls_identity::DnsLikeIdentity { name },
+                pb::tls_identity::DnsLikeIdentity { name: name.clone() },
             )),
+            server_name: Some(pb::tls_identity::DnsLikeIdentity { name }),
         });
 
         pb::Update {

diff --git a/linkerd/app/outbound/Cargo.toml b/linkerd/app/outbound/Cargo.toml
@@ -20,7 +20,7 @@ ahash = "0.8"
 bytes = "1"
 http = "0.2"
 futures = { version = "0.3", default-features = false }
-linkerd2-proxy-api = { version = "0.12", features = ["outbound"] }
+linkerd2-proxy-api = { version = "0.13", features = ["outbound"] }
 linkerd-app-core = { path = "../core" }
 linkerd-app-test = { path = "../test", optional = true }
 linkerd-distribute = { path = "../../distribute" }

diff --git a/linkerd/http-route/Cargo.toml b/linkerd/http-route/Cargo.toml
@@ -17,7 +17,7 @@ tracing = "0.1"
 url = "2"
 
 [dependencies.linkerd2-proxy-api]
-version = "0.12"
+version = "0.13"
 features = ["http-route", "grpc-route"]
 optional = true
 

diff --git a/linkerd/proxy/api-resolve/Cargo.toml b/linkerd/proxy/api-resolve/Cargo.toml
@@ -13,11 +13,12 @@ Implements the Resolve trait using the proxy's gRPC API
 futures = { version = "0.3", default-features = false }
 linkerd-addr = { path = "../../addr" }
 linkerd-error = { path = "../../error" }
-linkerd2-proxy-api = { version = "0.12", features = ["destination"] }
+linkerd2-proxy-api = { version = "0.13", features = ["destination"] }
 linkerd-proxy-core = { path = "../core" }
 linkerd-stack = { path = "../../stack" }
 linkerd-tonic-stream = { path = "../../tonic-stream" }
 linkerd-tls = { path = "../../tls" }
+linkerd-identity = { path = "../../identity" }
 http = "0.2"
 http-body = "0.4"
 pin-project = "1"

diff --git a/linkerd/proxy/api-resolve/src/pb.rs b/linkerd/proxy/api-resolve/src/pb.rs
@@ -7,6 +7,7 @@ use crate::{
     metadata::{Metadata, ProtocolHint},
 };
 use http::uri::Authority;
+use linkerd_identity::Id;
 use linkerd_tls::{client::ServerId, ClientTls, ServerName};
 use std::{collections::HashMap, net::SocketAddr, str::FromStr};
 
@@ -51,18 +52,32 @@ pub fn to_addr_meta(
 }
 
 fn to_identity(pb: TlsIdentity) -> Option<ClientTls> {
-    // TODO: differenciate between id and name, once this is
-    // available in the API
     use crate::api::destination::tls_identity::Strategy;
 
-    let Strategy::DnsLikeIdentity(i) = pb.strategy?;
-    match (ServerId::from_str(&i.name), ServerName::from_str(&i.name)) {
-        (Ok(i), Ok(n)) => Some(ClientTls::new(i, n)),
-        (_, _) => {
-            tracing::warn!("Ignoring invalid identity: {}", i.name);
+    let server_id = pb
+        .strategy
+        .and_then(|s| match s {
+            Strategy::DnsLikeIdentity(dns) => Id::parse_dns_name(&dns.name)
+                .map_err(|_| tracing::warn!("Ignoring invalid DNS identity: {}", dns.name))
+                .ok(),
+            Strategy::UriLikeIdentity(uri) => Id::parse_uri(&uri.uri)
+                .map_err(|_| tracing::warn!("Ignoring invalid URI identity: {}", uri.uri))
+                .ok(),
+        })
+        .map(ServerId)?;
+
+    let server_name = match (pb.server_name, &server_id) {
+        (Some(name), _) => ServerName::from_str(&name.name)
+            .map_err(|_| tracing::warn!("Ignoring invalid Server name: {}", name.name))
+            .ok(),
+        (None, ServerId(Id::Dns(dns_id))) => Some(ServerName(dns_id.clone())),
+        (None, ServerId(Id::Uri(_))) => {
+            tracing::warn!("server name missing for URI type identity");
             None
         }
-    }
+    }?;
+
+    Some(ClientTls::new(server_id, server_name))
 }
 
 pub(crate) fn to_authority(o: AuthorityOverride) -> Option<Authority> {
@@ -129,3 +144,86 @@ pub(crate) fn to_sock_addr(pb: TcpAddress) -> Option<SocketAddr> {
         None => None,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::api::destination::TlsIdentity;
+    use linkerd2_proxy_api::destination::tls_identity::{
+        DnsLikeIdentity, Strategy, UriLikeIdentity,
+    };
+    use linkerd_identity as id;
+
+    #[test]
+    fn dns_identity_no_server_name_works() {
+        let pb_id = TlsIdentity {
+            server_name: None,
+            strategy: Some(Strategy::DnsLikeIdentity(DnsLikeIdentity {
+                name: "system.local".to_string(),
+            })),
+        };
+
+        assert!(to_identity(pb_id).is_some());
+    }
+
+    #[test]
+    fn uri_identity_no_server_name_does_not_work() {
+        let pb_id = TlsIdentity {
+            server_name: None,
+            strategy: Some(Strategy::UriLikeIdentity(UriLikeIdentity {
+                uri: "spiffe://root".to_string(),
+            })),
+        };
+
+        assert!(to_identity(pb_id).is_none());
+    }
+
+    #[test]
+    fn uri_identity_with_server_name_works() {
+        let pb_id = TlsIdentity {
+            server_name: Some(DnsLikeIdentity {
+                name: "system.local".to_string(),
+            }),
+            strategy: Some(Strategy::UriLikeIdentity(UriLikeIdentity {
+                uri: "spiffe://root".to_string(),
+            })),
+        };
+
+        assert!(to_identity(pb_id).is_some());
+    }
+
+    #[test]
+    fn dns_identity_with_server_name_works() {
+        let dns_id = DnsLikeIdentity {
+            name: "system.local".to_string(),
+        };
+        let pb_id = TlsIdentity {
+            server_name: Some(dns_id.clone()),
+            strategy: Some(Strategy::DnsLikeIdentity(dns_id)),
+        };
+
+        assert!(to_identity(pb_id).is_some());
+    }
+
+    #[test]
+    fn dns_identity_with_server_name_works_different_values() {
+        let name = DnsLikeIdentity {
+            name: "name.some".to_string(),
+        };
+        let dns_id = DnsLikeIdentity {
+            name: "system.local".to_string(),
+        };
+
+        let pb_id = TlsIdentity {
+            server_name: Some(name),
+            strategy: Some(Strategy::DnsLikeIdentity(dns_id)),
+        };
+
+        let expected_identity = Some(ClientTls::new(
+            ServerId(id::Id::parse_dns_name("system.local").expect("should parse")),
+            ServerName::from_str("name.some").expect("should parse"),
+        ));
+
+        assert_eq!(expected_identity, to_identity(pb_id));
+    }
+}
diff --git a/linkerd/proxy/client-policy/Cargo.toml b/linkerd/proxy/client-policy/Cargo.toml
@@ -18,7 +18,7 @@ proto = [
 ahash = "0.8"
 ipnet = "2"
 http = "0.2"
-linkerd2-proxy-api = { version = "0.12", optional = true, features = [
+linkerd2-proxy-api = { version = "0.13", optional = true, features = [
     "outbound",
 ] }
 linkerd-error = { path = "../../error" }

diff --git a/linkerd/proxy/identity-client/Cargo.toml b/linkerd/proxy/identity-client/Cargo.toml
@@ -8,7 +8,7 @@ publish = false
 
 [dependencies]
 futures = { version = "0.3", default-features = false }
-linkerd2-proxy-api = { version = "0.12", features = ["identity"] }
+linkerd2-proxy-api = { version = "0.13", features = ["identity"] }
 linkerd-dns-name = { path = "../../dns/name" }
 linkerd-error = { path = "../../error" }
 linkerd-identity = { path = "../../identity" }

diff --git a/linkerd/proxy/server-policy/Cargo.toml b/linkerd/proxy/server-policy/Cargo.toml
@@ -17,7 +17,7 @@ prost-types = { version = "0.12", optional = true }
 thiserror = "1"
 
 [dependencies.linkerd2-proxy-api]
-version = "0.12"
+version = "0.13"
 features = ["inbound"]
 optional = true
 

diff --git a/linkerd/proxy/tap/Cargo.toml b/linkerd/proxy/tap/Cargo.toml
@@ -11,7 +11,7 @@ http = "0.2"
 hyper = { version = "0.14", features = ["http1", "http2"] }
 futures = { version = "0.3", default-features = false }
 ipnet = "2.7"
-linkerd2-proxy-api = { version = "0.12", features = ["tap"] }
+linkerd2-proxy-api = { version = "0.13", features = ["tap"] }
 linkerd-conditional = { path = "../../conditional" }
 linkerd-error = { path = "../../error" }
 linkerd-meshtls = { path = "../../meshtls" }
@@ -30,5 +30,5 @@ tracing = "0.1"
 pin-project = "1"
 
 [dev-dependencies]
-linkerd2-proxy-api = { version = "0.12", features = ["arbitrary"] }
+linkerd2-proxy-api = { version = "0.13", features = ["arbitrary"] }
 quickcheck = { version = "1", default-features = false }
diff --git a/linkerd/service-profiles/Cargo.toml b/linkerd/service-profiles/Cargo.toml
@@ -22,7 +22,7 @@ linkerd-proxy-api-resolve = { path = "../proxy/api-resolve" }
 linkerd-stack = { path = "../stack" }
 linkerd-tonic-stream = { path = "../tonic-stream" }
 linkerd-tonic-watch = { path = "../tonic-watch" }
-linkerd2-proxy-api = { version = "0.12", features = ["destination"] }
+linkerd2-proxy-api = { version = "0.13", features = ["destination"] }
 once_cell = "1.17"
 prost-types = "0.12"
 regex = "1"
@@ -34,5 +34,5 @@ thiserror = "1"
 tracing = "0.1"
 
 [dev-dependencies]
-linkerd2-proxy-api = { version = "0.12", features = ["arbitrary"] }
+linkerd2-proxy-api = { version = "0.13", features = ["arbitrary"] }
 quickcheck = { version = "1", default-features = false }
diff --git a/linkerd/tls/src/client.rs b/linkerd/tls/src/client.rs
@@ -1,7 +1,6 @@
 use crate::{NegotiatedProtocol, ServerName};
 use futures::prelude::*;
 use linkerd_conditional::Conditional;
-use linkerd_dns_name as dns;
 use linkerd_identity as id;
 use linkerd_io as io;
 use linkerd_stack::{layer, MakeConnection, NewService, Oneshot, Param, Service, ServiceExt};
@@ -210,11 +209,9 @@ impl Deref for ServerId {
 }
 
 impl FromStr for ServerId {
-    type Err = dns::InvalidName;
+    type Err = linkerd_error::Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        // TODO Handle SPIFFE IDs.
-        let n = dns::Name::from_str(s)?;
-        Ok(Self(n.into()))
+        id::Id::from_str(s).map(Self)
     }
 }