policy: Set correct backend metadata (#11842)

The policy controller sets incorrect backend metadata when (1) there is
no explicit backend reference specified, and (2) when a backend
reference crosses namespaces.

This change fixes these backend references so that proxy logs and
metrics have the proper metadata references. Outbound policy tests are
updated to validate this.

policy-controller/grpc/src/outbound.rs

@@ -467,7 +467,14 @@ fn convert_http_backend(
 fn default_backend(outbound: &OutboundPolicy) -> outbound::Backend {
     outbound::Backend {
         metadata: Some(Metadata {
-            kind: Some(metadata::Kind::Default("service".to_string())),
+            kind: Some(metadata::Kind::Resource(api::meta::Resource {
+                group: "core".to_string(),
+                kind: "Service".to_string(),
+                name: outbound.name.clone(),
+                namespace: outbound.namespace.clone(),
+                section: Default::default(),
+                port: u16::from(outbound.port).into(),
+            })),
         }),
         queue: Some(default_queue_config()),
         kind: Some(outbound::backend::Kind::Balancer(

policy-controller/k8s/api/src/policy/httproute.rs

@@ -259,6 +259,6 @@ where
     // Default kind is assumed to be service for backend ref objects
     super::targets_kind::<T>(
         backend_ref.group.as_deref(),
-        backend_ref.kind.as_deref().unwrap_or("service"),
+        backend_ref.kind.as_deref().unwrap_or("Service"),
     )
 }

policy-controller/k8s/index/src/outbound/index.rs

@@ -611,7 +611,7 @@ fn convert_backend(
         weight: weight.into(),
         authority: cluster.service_dns_authority(&service_ref.namespace, &name, port),
         name,
-        namespace: ns.to_string(),
+        namespace: service_ref.namespace.to_string(),
         port,
         filters,
     }))

policy-test/src/lib.rs

@@ -267,6 +267,24 @@ pub fn mk_service(ns: &str, name: &str, port: i32) -> k8s::Service {
     }
 }
 
+#[track_caller]
+pub fn assert_svc_meta(meta: &Option<grpc::meta::Metadata>, svc: &k8s::Service, port: u16) {
+    tracing::debug!(?meta, ?svc, port, "Asserting service metadata");
+    assert_eq!(
+        meta,
+        &Some(grpc::meta::Metadata {
+            kind: Some(grpc::meta::metadata::Kind::Resource(grpc::meta::Resource {
+                group: "core".to_string(),
+                kind: "Service".to_string(),
+                name: svc.name_unchecked(),
+                namespace: svc.namespace().unwrap(),
+                section: "".to_string(),
+                port: port.into()
+            })),
+        })
+    );
+}
+
 pub fn mk_route(
     ns: &str,
     name: &str,

policy-test/tests/outbound_api_gateway.rs

@@ -1,13 +1,13 @@
-use std::{collections::BTreeMap, time::Duration};
-
 use futures::prelude::*;
 use kube::ResourceExt;
 use linkerd_policy_controller_k8s_api as k8s;
 use linkerd_policy_test::{
-    assert_default_accrual_backoff, create, create_annotated_service, create_cluster_scoped,
-    create_opaque_service, create_service, delete_cluster_scoped, grpc, mk_service, with_temp_ns,
+    assert_default_accrual_backoff, assert_svc_meta, create, create_annotated_service,
+    create_cluster_scoped, create_opaque_service, create_service, delete_cluster_scoped, grpc,
+    mk_service, with_temp_ns,
 };
 use maplit::{btreemap, convert_args};
+use std::{collections::BTreeMap, time::Duration};
 use tokio::time;
 
 // These tests are copies of the tests in outbound_api_gateway.rs but using the
@@ -46,6 +46,8 @@ async fn service_with_no_http_routes() {
             .expect("watch must return an initial config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a default route.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -69,6 +71,8 @@ async fn service_with_http_route_without_rules() {
             .expect("watch must return an initial config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a default route.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -84,6 +88,8 @@ async fn service_with_http_route_without_rules() {
             .expect("watch must return an updated config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a route with no rules.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -107,6 +113,8 @@ async fn service_with_http_routes_without_backends() {
             .expect("watch must return an initial config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a default route.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -126,6 +134,8 @@ async fn service_with_http_routes_without_backends() {
             .expect("watch must return an updated config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a route with the logical backend.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -151,6 +161,8 @@ async fn service_with_http_routes_with_backend() {
             .expect("watch must return an initial config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a default route.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -174,6 +186,8 @@ async fn service_with_http_routes_with_backend() {
             .expect("watch must return an updated config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a route with a backend with no filters.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -201,6 +215,8 @@ async fn service_with_http_routes_with_cross_namespace_backend() {
             .expect("watch must return an initial config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a default route.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -239,6 +255,8 @@ async fn service_with_http_routes_with_cross_namespace_backend() {
             .expect("watch must return an updated config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a route with a backend with no filters.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -269,6 +287,8 @@ async fn service_with_http_routes_with_invalid_backend() {
             .expect("watch must return an initial config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a default route.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -290,6 +310,8 @@ async fn service_with_http_routes_with_invalid_backend() {
             .expect("watch must return an updated config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a route with a backend.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -317,6 +339,8 @@ async fn service_with_multiple_http_routes() {
             .expect("watch must return an initial config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be a default route.
         detect_http_routes(&config, |routes| {
             let route = assert_singleton(routes);
@@ -340,6 +364,8 @@ async fn service_with_multiple_http_routes() {
             .expect("watch must return an updated config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         let _b_route = create(
             &client,
             mk_http_route(&ns, "b-route", &svc, Some(4191)).build(),
@@ -354,6 +380,8 @@ async fn service_with_multiple_http_routes() {
             .expect("watch must return an updated config");
         tracing::trace!(?config);
 
+        assert_svc_meta(&config.metadata, &svc, 4191);
+
         // There should be 2 routes, returned in order.
         detect_http_routes(&config, |routes| {
             assert_eq!(routes.len(), 2);
@@ -1387,8 +1415,8 @@ fn assert_backend_matches_service(
     svc: &k8s::Service,
     port: u16,
 ) {
-    let kind = backend.backend.as_ref().unwrap().kind.as_ref().unwrap();
-    let dst = match kind {
+    let backend = backend.backend.as_ref().unwrap();
+    let dst = match backend.kind.as_ref().unwrap() {
         grpc::outbound::backend::Kind::Balancer(balance) => {
             let kind = balance.discovery.as_ref().unwrap().kind.as_ref().unwrap();
             match kind {
@@ -1409,6 +1437,8 @@ fn assert_backend_matches_service(
             port
         )
     );
+
+    assert_svc_meta(&backend.metadata, svc, port)
 }
 
 #[track_caller]