diff --git a/policy-controller/grpc/src/outbound.rs b/policy-controller/grpc/src/outbound.rs index 3b631043b77d8..fbe7f1f1674b4 100644 --- a/policy-controller/grpc/src/outbound.rs +++ b/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( diff --git a/policy-controller/k8s/api/src/policy/httproute.rs b/policy-controller/k8s/api/src/policy/httproute.rs index 9844b34b2c9fd..ace2ee88b0beb 100644 --- a/policy-controller/k8s/api/src/policy/httproute.rs +++ b/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::( backend_ref.group.as_deref(), - backend_ref.kind.as_deref().unwrap_or("service"), + backend_ref.kind.as_deref().unwrap_or("Service"), ) } diff --git a/policy-controller/k8s/index/src/outbound/index.rs b/policy-controller/k8s/index/src/outbound/index.rs index 20defded04574..b41f6c32df0ae 100644 --- a/policy-controller/k8s/index/src/outbound/index.rs +++ b/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, })) diff --git a/policy-test/src/lib.rs b/policy-test/src/lib.rs index 91f173a911705..1ca9645915000 100644 --- a/policy-test/src/lib.rs +++ b/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, 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, diff --git a/policy-test/tests/outbound_api_gateway.rs b/policy-test/tests/outbound_api_gateway.rs index 0aeafff7bf94b..9242864d67a5e 100644 --- a/policy-test/tests/outbound_api_gateway.rs +++ b/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] diff --git a/policy-test/tests/outbound_api_linkerd.rs b/policy-test/tests/outbound_api_linkerd.rs index 1431b561a318b..36af6ca2593ea 100644 --- a/policy-test/tests/outbound_api_linkerd.rs +++ b/policy-test/tests/outbound_api_linkerd.rs @@ -4,8 +4,9 @@ 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 tokio::time; @@ -46,6 +47,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 +72,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 +89,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 +114,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 +135,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 +162,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 +187,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 +216,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 +256,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 +288,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 +311,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 +340,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 +365,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 +381,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); @@ -405,6 +434,8 @@ async fn service_with_consecutive_failure_accrual() { .expect("watch must return an initial config"); tracing::trace!(?config); + assert_svc_meta(&config.metadata, &svc, 4191); + detect_failure_accrual(&config, |accrual| { let consecutive = failure_accrual_consecutive(accrual); assert_eq!(8, consecutive.max_failures); @@ -449,6 +480,8 @@ async fn service_with_consecutive_failure_accrual_defaults() { .expect("watch must return an initial config"); tracing::trace!(?config); + assert_svc_meta(&config.metadata, &svc, 4191); + // Expect default max_failures and default backoff detect_failure_accrual(&config, |accrual| { let consecutive = failure_accrual_consecutive(accrual); @@ -614,6 +647,8 @@ async fn opaque_service() { .expect("watch must return an initial config"); tracing::trace!(?config); + assert_svc_meta(&config.metadata, &svc, 4191); + // Proxy protocol should be opaque. match config.protocol.unwrap().kind.unwrap() { grpc::outbound::proxy_protocol::Kind::Opaque(_) => {} @@ -754,6 +789,8 @@ async fn backend_with_filters() { .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); @@ -804,6 +841,8 @@ async fn backend_with_filters() { .expect("watch must return an updated config"); tracing::trace!(?config); + assert_svc_meta(&config.metadata, &svc, 4191); + // There should be a route without rule filters. detect_http_routes(&config, |routes| { let route = assert_singleton(routes); @@ -873,6 +912,8 @@ async fn http_route_with_no_port() { .expect("watch must return an initial config"); tracing::trace!(?config_4191); + assert_svc_meta(&config_4191.metadata, &svc, 4191); + let mut rx_9999 = retry_watch_outbound_policy(&client, &ns, &svc, 9999).await; let config_9999 = rx_9999 .next() @@ -881,6 +922,8 @@ async fn http_route_with_no_port() { .expect("watch must return an initial config"); tracing::trace!(?config_9999); + assert_svc_meta(&config_9999.metadata, &svc, 9999); + // There should be a default route. detect_http_routes(&config_4191, |routes| { let route = assert_singleton(routes); @@ -1399,8 +1442,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 { @@ -1421,6 +1464,8 @@ fn assert_backend_matches_service( port ) ); + + assert_svc_meta(&backend.metadata, svc, port) } #[track_caller]