From 5f3fa6414274f22f03abe754e4c334f2ee0cf7f9 Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Tue, 7 May 2024 23:32:57 +0000 Subject: [PATCH 1/2] set backend not found status on routes with any backends that are not found Signed-off-by: Alex Leong --- policy-controller/k8s/status/src/index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/policy-controller/k8s/status/src/index.rs b/policy-controller/k8s/status/src/index.rs index 8b45d5c050553..8892c269403a7 100644 --- a/policy-controller/k8s/status/src/index.rs +++ b/policy-controller/k8s/status/src/index.rs @@ -421,7 +421,7 @@ impl Index { // If all references have been resolved (i.e exist in our services cache), // return positive status, otherwise, one of them does not exist - if backend_refs.iter().any(|backend_ref| match backend_ref { + if backend_refs.iter().all(|backend_ref| match backend_ref { BackendReference::Service(service) => self.services.contains_key(service), _ => false, }) { From 85d4ee83a1b8381039b82f691d2869574cbd08ef Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Thu, 9 May 2024 23:18:35 +0000 Subject: [PATCH 2/2] Add tests Signed-off-by: Alex Leong --- .../k8s/status/src/tests/http_routes.rs | 345 +++++++++++++++++- 1 file changed, 332 insertions(+), 13 deletions(-) diff --git a/policy-controller/k8s/status/src/tests/http_routes.rs b/policy-controller/k8s/status/src/tests/http_routes.rs index 543b350a0d1c9..19fde10a18d19 100644 --- a/policy-controller/k8s/status/src/tests/http_routes.rs +++ b/policy-controller/k8s/status/src/tests/http_routes.rs @@ -3,7 +3,8 @@ use crate::{ resource_id::NamespaceGroupKindName, Index, IndexMetrics, }; -use k8s::Resource; +use gateway::{BackendObjectReference, BackendRef, HttpBackendRef, ParentReference}; +use k8s::{Resource, ResourceExt}; use kubert::index::IndexNamespacedResource; use linkerd_policy_controller_core::{http_route::GroupKindName, POLICY_CONTROLLER_NAME}; use linkerd_policy_controller_k8s_api::{self as k8s, gateway, policy::server::Port}; @@ -27,7 +28,15 @@ fn http_route_accepted_after_server_create() { ); // Apply the route. - let http_route = make_route("ns-0", "route-foo", "srv-8080"); + let parent = ParentReference { + group: Some(POLICY_API_GROUP.to_string()), + kind: Some("Server".to_string()), + namespace: None, + name: "srv-8080".to_string(), + section_name: None, + port: None, + }; + let http_route = make_route("ns-0", "route-foo", parent, None); index.write().apply(http_route); // Create the expected update. @@ -111,7 +120,15 @@ fn http_route_rejected_after_server_delete() { // There should be no update since there are no HTTPRoutes yet. assert!(updates_rx.try_recv().is_err()); - let http_route = make_route("ns-0", "route-foo", "srv-8080"); + let parent = ParentReference { + group: Some(POLICY_API_GROUP.to_string()), + kind: Some("Server".to_string()), + namespace: None, + name: "srv-8080".to_string(), + section_name: None, + port: None, + }; + let http_route = make_route("ns-0", "route-foo", parent, None); index.write().apply(http_route); // Create the expected update. @@ -164,6 +181,298 @@ fn http_route_rejected_after_server_delete() { assert!(updates_rx.try_recv().is_err()); } +#[test] +fn http_route_with_invalid_backend() { + let hostname = "test"; + let claim = kubert::lease::Claim { + holder: "test".to_string(), + expiry: chrono::DateTime::::MAX_UTC, + }; + let (_claims_tx, claims_rx) = watch::channel(Arc::new(claim)); + let (updates_tx, mut updates_rx) = mpsc::channel(10000); + let index = Index::shared( + hostname, + claims_rx, + updates_tx, + IndexMetrics::register(&mut Default::default()), + ); + + // Apply the parent service + let parent = make_service("ns-0", "svc"); + index.write().apply(parent.clone()); + + // Apply one backend service + let backend = make_service("ns-0", "backend-1"); + index.write().apply(backend.clone()); + + // Apply the route. + let parent = ParentReference { + group: Some("core".to_string()), + kind: Some("Service".to_string()), + namespace: parent.namespace(), + name: parent.name_unchecked(), + section_name: None, + port: Some(8080), + }; + let http_route = make_route( + "ns-0", + "route-foo", + parent.clone(), + Some(vec![ + HttpBackendRef { + backend_ref: Some(BackendRef { + weight: None, + inner: BackendObjectReference { + group: Some("core".to_string()), + kind: Some("Service".to_string()), + name: backend.name_unchecked(), + namespace: backend.namespace(), + port: Some(8080), + }, + }), + filters: None, + }, + HttpBackendRef { + backend_ref: Some(BackendRef { + weight: None, + inner: BackendObjectReference { + group: Some("core".to_string()), + kind: Some("Service".to_string()), + name: "nonexistant-backend".to_string(), + namespace: backend.namespace(), + port: Some(8080), + }, + }), + filters: None, + }, + ]), + ); + index.write().apply(http_route); + + // Create the expected update. + let id = NamespaceGroupKindName { + namespace: "ns-0".to_string(), + gkn: GroupKindName { + group: k8s::policy::HttpRoute::group(&()), + kind: k8s::policy::HttpRoute::kind(&()), + name: "route-foo".into(), + }, + }; + let accepted_condition = k8s::Condition { + last_transition_time: k8s::Time(chrono::DateTime::::MIN_UTC), + message: "".to_string(), + observed_generation: None, + reason: "Accepted".to_string(), + status: "True".to_string(), + type_: "Accepted".to_string(), + }; + // One of the backends does not exist so the status should be BackendNotFound. + let backend_condition = k8s::Condition { + last_transition_time: k8s::Time(chrono::DateTime::::MIN_UTC), + message: "".to_string(), + observed_generation: None, + reason: "BackendNotFound".to_string(), + status: "False".to_string(), + type_: "ResolvedRefs".to_string(), + }; + let parent_status = gateway::RouteParentStatus { + parent_ref: parent, + controller_name: POLICY_CONTROLLER_NAME.to_string(), + conditions: vec![accepted_condition, backend_condition], + }; + let status = make_status(vec![parent_status]); + let patch = index::make_patch("route-foo", status); + + let update = updates_rx.try_recv().unwrap(); + assert_eq!(id, update.id); + assert_eq!(patch, update.patch); + assert!(updates_rx.try_recv().is_err()) +} + +#[test] +fn http_route_with_no_backends() { + let hostname = "test"; + let claim = kubert::lease::Claim { + holder: "test".to_string(), + expiry: chrono::DateTime::::MAX_UTC, + }; + let (_claims_tx, claims_rx) = watch::channel(Arc::new(claim)); + let (updates_tx, mut updates_rx) = mpsc::channel(10000); + let index = Index::shared( + hostname, + claims_rx, + updates_tx, + IndexMetrics::register(&mut Default::default()), + ); + + // Apply the parent service + let parent = make_service("ns-0", "svc"); + index.write().apply(parent.clone()); + + // Apply the route. + let parent = ParentReference { + group: Some("core".to_string()), + kind: Some("Service".to_string()), + namespace: parent.namespace(), + name: parent.name_unchecked(), + section_name: None, + port: Some(8080), + }; + let http_route = make_route("ns-0", "route-foo", parent.clone(), None); + index.write().apply(http_route); + + // Create the expected update. + let id = NamespaceGroupKindName { + namespace: "ns-0".to_string(), + gkn: GroupKindName { + group: k8s::policy::HttpRoute::group(&()), + kind: k8s::policy::HttpRoute::kind(&()), + name: "route-foo".into(), + }, + }; + let accepted_condition = k8s::Condition { + last_transition_time: k8s::Time(chrono::DateTime::::MIN_UTC), + message: "".to_string(), + observed_generation: None, + reason: "Accepted".to_string(), + status: "True".to_string(), + type_: "Accepted".to_string(), + }; + // No backends were specified, so we have vacuously have resolved them all. + let backend_condition = k8s::Condition { + last_transition_time: k8s::Time(chrono::DateTime::::MIN_UTC), + message: "".to_string(), + observed_generation: None, + reason: "ResolvedRefs".to_string(), + status: "True".to_string(), + type_: "ResolvedRefs".to_string(), + }; + let parent_status = gateway::RouteParentStatus { + parent_ref: parent, + controller_name: POLICY_CONTROLLER_NAME.to_string(), + conditions: vec![accepted_condition, backend_condition], + }; + let status = make_status(vec![parent_status]); + let patch = index::make_patch("route-foo", status); + + let update = updates_rx.try_recv().unwrap(); + assert_eq!(id, update.id); + assert_eq!(patch, update.patch); + assert!(updates_rx.try_recv().is_err()) +} + +#[test] +fn http_route_with_valid_backends() { + let hostname = "test"; + let claim = kubert::lease::Claim { + holder: "test".to_string(), + expiry: chrono::DateTime::::MAX_UTC, + }; + let (_claims_tx, claims_rx) = watch::channel(Arc::new(claim)); + let (updates_tx, mut updates_rx) = mpsc::channel(10000); + let index = Index::shared( + hostname, + claims_rx, + updates_tx, + IndexMetrics::register(&mut Default::default()), + ); + + // Apply the parent service + let parent = make_service("ns-0", "svc"); + index.write().apply(parent.clone()); + + // Apply one backend service + let backend1 = make_service("ns-0", "backend-1"); + index.write().apply(backend1.clone()); + + // Apply one backend service + let backend2 = make_service("ns-0", "backend-2"); + index.write().apply(backend2.clone()); + + // Apply the route. + let parent = ParentReference { + group: Some("core".to_string()), + kind: Some("Service".to_string()), + namespace: parent.namespace(), + name: parent.name_unchecked(), + section_name: None, + port: Some(8080), + }; + let http_route = make_route( + "ns-0", + "route-foo", + parent.clone(), + Some(vec![ + HttpBackendRef { + backend_ref: Some(BackendRef { + weight: None, + inner: BackendObjectReference { + group: Some("core".to_string()), + kind: Some("Service".to_string()), + name: backend1.name_unchecked(), + namespace: backend1.namespace(), + port: Some(8080), + }, + }), + filters: None, + }, + HttpBackendRef { + backend_ref: Some(BackendRef { + weight: None, + inner: BackendObjectReference { + group: Some("core".to_string()), + kind: Some("Service".to_string()), + name: backend2.name_unchecked(), + namespace: backend2.namespace(), + port: Some(8080), + }, + }), + filters: None, + }, + ]), + ); + index.write().apply(http_route); + + // Create the expected update. + let id = NamespaceGroupKindName { + namespace: "ns-0".to_string(), + gkn: GroupKindName { + group: k8s::policy::HttpRoute::group(&()), + kind: k8s::policy::HttpRoute::kind(&()), + name: "route-foo".into(), + }, + }; + let accepted_condition = k8s::Condition { + last_transition_time: k8s::Time(chrono::DateTime::::MIN_UTC), + message: "".to_string(), + observed_generation: None, + reason: "Accepted".to_string(), + status: "True".to_string(), + type_: "Accepted".to_string(), + }; + // All backends exist and can be resolved. + let backend_condition = k8s::Condition { + last_transition_time: k8s::Time(chrono::DateTime::::MIN_UTC), + message: "".to_string(), + observed_generation: None, + reason: "ResolvedRefs".to_string(), + status: "True".to_string(), + type_: "ResolvedRefs".to_string(), + }; + let parent_status = gateway::RouteParentStatus { + parent_ref: parent, + controller_name: POLICY_CONTROLLER_NAME.to_string(), + conditions: vec![accepted_condition, backend_condition], + }; + let status = make_status(vec![parent_status]); + let patch = index::make_patch("route-foo", status); + + let update = updates_rx.try_recv().unwrap(); + assert_eq!(id, update.id); + assert_eq!(patch, update.patch); + assert!(updates_rx.try_recv().is_err()) +} + fn make_server( namespace: impl ToString, name: impl ToString, @@ -192,10 +501,27 @@ fn make_server( } } +fn make_service(namespace: impl ToString, name: impl ToString) -> k8s::api::core::v1::Service { + k8s::api::core::v1::Service { + metadata: k8s::ObjectMeta { + namespace: Some(namespace.to_string()), + name: Some(name.to_string()), + ..Default::default() + }, + spec: Some(k8s::ServiceSpec { + cluster_ip: Some("1.2.3.4".to_string()), + cluster_ips: Some(vec!["1.2.3.4".to_string()]), + ..Default::default() + }), + status: None, + } +} + fn make_route( namespace: impl ToString, name: impl ToString, - server: impl ToString, + parent: ParentReference, + backends: Option>, ) -> k8s::policy::HttpRoute { use chrono::Utc; use k8s::{policy::httproute::*, Time}; @@ -209,14 +535,7 @@ fn make_route( }, spec: HttpRouteSpec { inner: CommonRouteSpec { - parent_refs: Some(vec![ParentReference { - group: Some(POLICY_API_GROUP.to_string()), - kind: Some("Server".to_string()), - namespace: None, - name: server.to_string(), - section_name: None, - port: None, - }]), + parent_refs: Some(vec![parent]), }, hostnames: None, rules: Some(vec![HttpRouteRule { @@ -229,7 +548,7 @@ fn make_route( method: Some("GET".to_string()), }]), filters: None, - backend_refs: None, + backend_refs: backends, timeouts: None, }]), },