Remove unneeded uses of indexmap (#1048)

We initially adopted `indexmap` in many places where it's not strictly
necessary: we don't actually rely on the data being insert-ordered and
don't need to mutate the structure while iterating over it. In fact, the
traffic split module is the only place in the codebase where we access
the data structure by index (rather than by key).

In all other cases, we do not care about insertion-order. In the vast
majority of cases we can simply use a `HashMap`. Endpoint and route
labels need to be reliably ordered so that they can be compared (i.e.
implementing `Eq`), in which case the standard `BTreeMap` works
sufficiently.

This change eliminates unnecessary uses of `indexmap` to reduce
dependency and conceptual overhead.

Co-authored-by: Eliza Weisman <[email protected]>

Author: olix0r

Cargo.lock

@@ -618,7 +618,6 @@ name = "linkerd-app"
 version = "0.1.0"
 dependencies = [
  "futures",
- "indexmap",
  "ipnet",
  "linkerd-app-admin",
  "linkerd-app-core",
@@ -663,7 +662,6 @@ dependencies = [
  "http",
  "http-body",
  "hyper",
- "indexmap",
  "ipnet",
  "linkerd-addr",
  "linkerd-cache",
@@ -723,7 +721,6 @@ version = "0.1.0"
 dependencies = [
  "futures",
  "http",
- "indexmap",
  "linkerd-app-core",
  "linkerd-app-inbound",
  "linkerd-app-outbound",
@@ -745,7 +742,6 @@ dependencies = [
  "futures",
  "http",
  "hyper",
- "indexmap",
  "libfuzzer-sys",
  "linkerd-app-core",
  "linkerd-app-test",
@@ -796,7 +792,6 @@ dependencies = [
  "futures",
  "http",
  "hyper",
- "indexmap",
  "ipnet",
  "linkerd-app-core",
  "linkerd-app-test",
@@ -931,7 +926,6 @@ name = "linkerd-error-metrics"
 version = "0.1.0"
 dependencies = [
  "futures",
- "indexmap",
  "linkerd-metrics",
  "pin-project",
  "tower",
@@ -992,7 +986,6 @@ dependencies = [
  "http",
  "http-body",
  "hyper",
- "indexmap",
  "linkerd-error",
  "linkerd-http-classify",
  "linkerd-metrics",
@@ -1056,7 +1049,6 @@ dependencies = [
  "hdrhistogram",
  "http",
  "hyper",
- "indexmap",
  "parking_lot",
  "quickcheck",
  "tokio",
@@ -1088,7 +1080,6 @@ dependencies = [
  "futures",
  "http",
  "http-body",
- "indexmap",
  "linkerd-addr",
  "linkerd-error",
  "linkerd-proxy-core",
@@ -1158,7 +1149,6 @@ dependencies = [
  "httparse",
  "hyper",
  "hyper-balance",
- "indexmap",
  "linkerd-detect",
  "linkerd-duplex",
  "linkerd-error",
@@ -1216,7 +1206,6 @@ dependencies = [
  "futures",
  "http",
  "hyper",
- "indexmap",
  "ipnet",
  "linkerd-conditional",
  "linkerd-error",
@@ -1357,7 +1346,6 @@ dependencies = [
 name = "linkerd-stack-metrics"
 version = "0.1.0"
 dependencies = [
- "indexmap",
  "linkerd-metrics",
  "tokio",
  "tower",

linkerd/app/Cargo.toml

@@ -16,7 +16,6 @@ allow-loopback = ["linkerd-app-outbound/allow-loopback"]
 
 [dependencies]
 futures = { version = "0.3", default-features = false }
-indexmap = "1.6"
 ipnet = "2.3"
 linkerd-app-admin = { path = "./admin" }
 linkerd-app-core = { path = "./core" }

linkerd/app/core/Cargo.toml

@@ -19,7 +19,6 @@ http = "0.2"
 http-body = "0.4"
 hyper = { version = "0.14.2", features = ["http1", "http2"] }
 futures = { version = "0.3", default-features = false }
-indexmap = "1.6"
 ipnet = "2.3"
 linkerd-addr = { path = "../../addr" }
 linkerd-cache = { path = "../../cache" }

linkerd/app/gateway/Cargo.toml

@@ -9,7 +9,6 @@ publish = false
 [dependencies]
 http = "0.2"
 futures = { version = "0.3", default-features = false }
-indexmap = "1.6"
 linkerd-app-core = { path = "../core" }
 linkerd-app-inbound = { path = "../inbound" }
 linkerd-app-outbound = { path = "../outbound" }

linkerd/app/inbound/Cargo.toml

@@ -13,7 +13,6 @@ Configures and runs the inbound proxy
 bytes = "1"
 http = "0.2"
 futures = { version = "0.3", default-features = false }
-indexmap = "1.6"
 linkerd-app-core = { path = "../core" }
 thiserror = "1.0"
 tokio = { version = "1", features = ["sync"] }

linkerd/app/inbound/src/require_identity.rs

@@ -1,14 +1,13 @@
 use crate::target::TcpAccept;
-use indexmap::IndexSet;
 use linkerd_app_core::{svc::stack::Predicate, tls, Conditional, Error};
-use std::sync::Arc;
+use std::{collections::HashSet, sync::Arc};
 use thiserror::Error;
 
 /// A connection policy that fails connections that don't have a client identity
 /// if they target one of the configured local ports.
 #[derive(Clone, Debug)]
 pub struct RequireIdentityForPorts {
-    ports: Arc<IndexSet<u16>>,
+    ports: Arc<HashSet<u16>>,
 }
 
 #[derive(Debug, Error)]

linkerd/app/inbound/src/target.rs

@@ -1,4 +1,3 @@
-use indexmap::IndexMap;
 use linkerd_app_core::{
     classify, dst, http_request_authority_addr, http_request_host_addr, identity, metrics,
     profiles,
@@ -279,15 +278,15 @@ impl tap::Inspect for Target {
         Some(self.target_addr)
     }
 
-    fn dst_labels<B>(&self, _: &http::Request<B>) -> Option<&IndexMap<String, String>> {
+    fn dst_labels<B>(&self, _: &http::Request<B>) -> Option<&tap::Labels> {
         None
     }
 
     fn dst_tls<B>(&self, _: &http::Request<B>) -> tls::ConditionalClientTls {
         Conditional::None(tls::NoClientTls::Loopback)
     }
 
-    fn route_labels<B>(&self, req: &http::Request<B>) -> Option<Arc<IndexMap<String, String>>> {
+    fn route_labels<B>(&self, req: &http::Request<B>) -> Option<Arc<tap::Labels>> {
         req.extensions()
             .get::<dst::Route>()
             .map(|r| r.route.labels().clone())

linkerd/app/outbound/Cargo.toml

@@ -18,7 +18,6 @@ test-subscriber = []
 bytes = "1"
 http = "0.2"
 futures = { version = "0.3", default-features = false }
-indexmap = "1.6"
 linkerd-app-core = { path = "../core" }
 linkerd-http-retry = { path = "../../http-retry" }
 linkerd-identity = { path = "../../identity" }

linkerd/app/outbound/src/http/endpoint.rs

@@ -182,7 +182,7 @@ mod test {
             opaque_protocol: false,
             tls: tls::ConditionalClientTls::None(tls::NoClientTls::Disabled),
             metadata: Metadata::new(
-                Default::default(),
+                None,
                 support::resolver::ProtocolHint::Http2,
                 None,
                 None,
@@ -230,7 +230,7 @@ mod test {
             opaque_protocol: false,
             tls: tls::ConditionalClientTls::None(tls::NoClientTls::Disabled),
             metadata: Metadata::new(
-                Default::default(),
+                None,
                 support::resolver::ProtocolHint::Http2,
                 None,
                 None,

linkerd/app/outbound/src/http/mod.rs

@@ -5,7 +5,6 @@ mod require_id_header;
 mod server;
 
 use crate::tcp;
-use indexmap::IndexMap;
 pub use linkerd_app_core::proxy::http::*;
 use linkerd_app_core::{
     dst,
@@ -173,15 +172,15 @@ impl tap::Inspect for Endpoint {
         Some(self.addr.into())
     }
 
-    fn dst_labels<B>(&self, _: &Request<B>) -> Option<&IndexMap<String, String>> {
+    fn dst_labels<B>(&self, _: &Request<B>) -> Option<&tap::Labels> {
         Some(self.metadata.labels())
     }
 
     fn dst_tls<B>(&self, _: &Request<B>) -> tls::ConditionalClientTls {
         self.tls.clone()
     }
 
-    fn route_labels<B>(&self, req: &Request<B>) -> Option<Arc<IndexMap<String, String>>> {
+    fn route_labels<B>(&self, req: &Request<B>) -> Option<Arc<tap::Labels>> {
         req.extensions()
             .get::<dst::Route>()
             .map(|r| r.route.labels().clone())