feedback

Signed-off-by: Alex Leong <[email protected]>
linkerd · Jul 17, 2023 · dbc611c · dbc611c
1 parent 7935f2e
commit dbc611c
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 9 deletions.
diff --git a/policy-controller/k8s/index/src/outbound/index.rs b/policy-controller/k8s/index/src/outbound/index.rs
@@ -42,6 +42,8 @@ struct NamespaceIndex {
 
 #[derive(Debug)]
 struct Namespace {
+    // A map of Service name and port to all HttpRoutes which target that
+    // Service and port specifically.
     service_port_routes: HashMap<ServicePort, ServiceRoutes>,
     // A map of Service name to all HttpRoutes that target that Service and do
     // not specify a port. We keep track of these so that whenever a new entry
@@ -237,8 +239,8 @@ impl Namespace {
                 continue;
             }
 
-            if let Some(port) = parent_ref.port {
-                if let Some(port) = NonZeroU16::new(port) {
+            let port = parent_ref.port.and_then(NonZeroU16::new);
+            if let Some(port) = port {
                     let service_port = ServicePort {
                         port,
                         service: parent_ref.name.clone(),
@@ -251,9 +253,6 @@ impl Namespace {
                     let service_routes =
                         self.service_routes_or_default(service_port, cluster_info, service_info);
                     service_routes.apply(route.gkn(), outbound_route.clone());
-                } else {
-                    tracing::warn!(?parent_ref, "ignoring parent_ref with port 0");
-                }
             } else {
                 // If the parent_ref doesn't include a port, apply this route
                 // to all ServiceRoutes which match the Service name.

diff --git a/policy-test/tests/outbound_api_gateway.rs b/policy-test/tests/outbound_api_gateway.rs
@@ -1180,7 +1180,9 @@ fn assert_route_is_default(route: &grpc::outbound::HttpRoute, svc: &k8s::Service
     let kind = route.metadata.as_ref().unwrap().kind.as_ref().unwrap();
     match kind {
         grpc::meta::metadata::Kind::Default(_) => {}
-        grpc::meta::metadata::Kind::Resource(_) => panic!("route expected to be default"),
+        grpc::meta::metadata::Kind::Resource(r) => {
+            panic!("route expected to be default but got resource {r:?}")
+        }
     }
 
     let backends = route_backends_first_available(route);
@@ -1236,7 +1238,9 @@ fn assert_singleton<T>(ts: &[T]) -> &T {
 fn assert_route_name_eq(route: &grpc::outbound::HttpRoute, name: &str) {
     let kind = route.metadata.as_ref().unwrap().kind.as_ref().unwrap();
     match kind {
-        grpc::meta::metadata::Kind::Default(_) => panic!("route expected to not be default"),
+        grpc::meta::metadata::Kind::Default(d) => {
+            panic!("route expected to not be default, but got default {d:?}")
+        }
         grpc::meta::metadata::Kind::Resource(resource) => assert_eq!(resource.name, *name),
     }
 }
diff --git a/policy-test/tests/outbound_api_linkerd.rs b/policy-test/tests/outbound_api_linkerd.rs
@@ -1198,7 +1198,9 @@ fn assert_route_is_default(route: &grpc::outbound::HttpRoute, svc: &k8s::Service
     let kind = route.metadata.as_ref().unwrap().kind.as_ref().unwrap();
     match kind {
         grpc::meta::metadata::Kind::Default(_) => {}
-        grpc::meta::metadata::Kind::Resource(_) => panic!("route expected to be default"),
+        grpc::meta::metadata::Kind::Resource(r) => {
+            panic!("route expected to be default but got resource {r:?}")
+        }
     }
 
     let backends = route_backends_first_available(route);
@@ -1254,7 +1256,9 @@ fn assert_singleton<T>(ts: &[T]) -> &T {
 fn assert_route_name_eq(route: &grpc::outbound::HttpRoute, name: &str) {
     let kind = route.metadata.as_ref().unwrap().kind.as_ref().unwrap();
     match kind {
-        grpc::meta::metadata::Kind::Default(_) => panic!("route expected to not be default"),
+        grpc::meta::metadata::Kind::Default(d) => {
+            panic!("route expected to not be default, but got default {d:?}")
+        }
         grpc::meta::metadata::Kind::Resource(resource) => assert_eq!(resource.name, *name),
     }
 }