mirror of https://github.com/linkerd/linkerd2.git
				
				
				
			Consider HTTPRoute `status` in policy controller (#10486)
When the policy controller is creating the inbound server which contains the HTTPRoutes that a policy client should consider, we now consider the `status` field and filter out HTTPRoutes which have not been accepted by the status controller. When creating the HTTPRoutes for an inbound server, the policy controller now looks at the `status` field for each HTTPRoute. It filters out statuses which are not from the policy status controller. For statuses which are from the policy status controller, it ensures that it has been accepted by a Server on the cluster. The tests have been updated in the index; these were failing without an `Accepted` status. Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
This commit is contained in:
		
							parent
							
								
									6e9244f485
								
							
						
					
					
						commit
						3ae3890593
					
				|  | @ -3,15 +3,18 @@ use anyhow::{anyhow, bail, Error, Result}; | |||
| use k8s_gateway_api as api; | ||||
| use linkerd_policy_controller_core::http_route; | ||||
| use linkerd_policy_controller_k8s_api::{ | ||||
|     self as k8s, | ||||
|     self as k8s, gateway, | ||||
|     policy::{httproute as policy, Server}, | ||||
| }; | ||||
| use std::num::NonZeroU16; | ||||
| use std::{fmt, num::NonZeroU16}; | ||||
| 
 | ||||
| const STATUS_CONTROLLER_NAME: &str = "policy.linkerd.io/status-controller"; | ||||
| 
 | ||||
| #[derive(Clone, Debug, Eq, PartialEq)] | ||||
| pub struct InboundRouteBinding { | ||||
|     pub parents: Vec<InboundParentRef>, | ||||
|     pub route: http_route::InboundHttpRoute, | ||||
|     pub statuses: Vec<Status>, | ||||
| } | ||||
| 
 | ||||
| #[derive(Clone, Debug, Eq, PartialEq)] | ||||
|  | @ -19,6 +22,23 @@ pub enum InboundParentRef { | |||
|     Server(String), | ||||
| } | ||||
| 
 | ||||
| #[derive(Clone, Debug, Eq, PartialEq)] | ||||
| pub struct Status { | ||||
|     pub parent: InboundParentRef, | ||||
|     pub conditions: Vec<Condition>, | ||||
| } | ||||
| 
 | ||||
| #[derive(Clone, Debug, Eq, PartialEq)] | ||||
| pub struct Condition { | ||||
|     pub type_: ConditionType, | ||||
|     pub status: bool, | ||||
| } | ||||
| 
 | ||||
| #[derive(Clone, Debug, Eq, PartialEq)] | ||||
| pub enum ConditionType { | ||||
|     Accepted, | ||||
| } | ||||
| 
 | ||||
| #[derive(Clone, Debug, thiserror::Error)] | ||||
| pub enum InvalidParentRef { | ||||
|     #[error("HTTPRoute resource does not reference a Server resource")] | ||||
|  | @ -63,6 +83,10 @@ impl TryFrom<api::HttpRoute> for InboundRouteBinding { | |||
|             ) | ||||
|             .collect::<Result<_>>()?; | ||||
| 
 | ||||
|         let statuses = route | ||||
|             .status | ||||
|             .map_or_else(Vec::new, |status| Status::collect_from(status.inner)); | ||||
| 
 | ||||
|         Ok(InboundRouteBinding { | ||||
|             parents, | ||||
|             route: http_route::InboundHttpRoute { | ||||
|  | @ -71,6 +95,7 @@ impl TryFrom<api::HttpRoute> for InboundRouteBinding { | |||
|                 authorizations: HashMap::default(), | ||||
|                 creation_timestamp, | ||||
|             }, | ||||
|             statuses, | ||||
|         }) | ||||
|     } | ||||
| } | ||||
|  | @ -102,6 +127,10 @@ impl TryFrom<policy::HttpRoute> for InboundRouteBinding { | |||
|             ) | ||||
|             .collect::<Result<_>>()?; | ||||
| 
 | ||||
|         let statuses = route | ||||
|             .status | ||||
|             .map_or_else(Vec::new, |status| Status::collect_from(status.inner)); | ||||
| 
 | ||||
|         Ok(InboundRouteBinding { | ||||
|             parents, | ||||
|             route: http_route::InboundHttpRoute { | ||||
|  | @ -110,6 +139,7 @@ impl TryFrom<policy::HttpRoute> for InboundRouteBinding { | |||
|                 authorizations: HashMap::default(), | ||||
|                 creation_timestamp, | ||||
|             }, | ||||
|             statuses, | ||||
|         }) | ||||
|     } | ||||
| } | ||||
|  | @ -122,6 +152,17 @@ impl InboundRouteBinding { | |||
|             .any(|p| matches!(p, InboundParentRef::Server(n) if n == name)) | ||||
|     } | ||||
| 
 | ||||
|     #[inline] | ||||
|     pub fn accepted_by_server(&self, name: &str) -> bool { | ||||
|         self.statuses.iter().any(|status| { | ||||
|             status.parent == InboundParentRef::Server(name.to_string()) | ||||
|                 && status | ||||
|                     .conditions | ||||
|                     .iter() | ||||
|                     .any(|condition| condition.type_ == ConditionType::Accepted && condition.status) | ||||
|         }) | ||||
|     } | ||||
| 
 | ||||
|     pub fn try_match( | ||||
|         api::HttpRouteMatch { | ||||
|             path, | ||||
|  | @ -273,6 +314,62 @@ impl InboundParentRef { | |||
|     } | ||||
| } | ||||
| 
 | ||||
| impl Status { | ||||
|     pub fn collect_from(status: gateway::RouteStatus) -> Vec<Self> { | ||||
|         status | ||||
|             .parents | ||||
|             .iter() | ||||
|             .filter(|status| status.controller_name == STATUS_CONTROLLER_NAME) | ||||
|             .filter_map(Self::from_parent_status) | ||||
|             .collect::<Vec<_>>() | ||||
|     } | ||||
| 
 | ||||
|     fn from_parent_status(status: &gateway::RouteParentStatus) -> Option<Self> { | ||||
|         // Only match parent statuses that belong to resources of
 | ||||
|         // `kind: Server`.
 | ||||
|         match status.parent_ref.kind.as_deref() { | ||||
|             Some("Server") => (), | ||||
|             _ => return None, | ||||
|         } | ||||
| 
 | ||||
|         let conditions = status | ||||
|             .conditions | ||||
|             .iter() | ||||
|             .filter_map(|condition| { | ||||
|                 let type_ = match condition.type_.as_ref() { | ||||
|                     "Accepted" => ConditionType::Accepted, | ||||
|                     condition_type => { | ||||
|                         tracing::error!(%status.parent_ref.name, %condition_type, "Unexpected condition type found in parent status"); | ||||
|                         return None; | ||||
|                     } | ||||
|                 }; | ||||
|                 let status = match condition.status.as_ref() { | ||||
|                     "True" => true, | ||||
|                     "False" => false, | ||||
|                     condition_status => { | ||||
|                         tracing::error!(%status.parent_ref.name, %type_, %condition_status, "Unexpected condition status found in parent status"); | ||||
|                         return None | ||||
|                     }, | ||||
|                 }; | ||||
|                 Some(Condition { type_, status }) | ||||
|             }) | ||||
|             .collect(); | ||||
| 
 | ||||
|         Some(Status { | ||||
|             parent: InboundParentRef::Server(status.parent_ref.name.to_string()), | ||||
|             conditions, | ||||
|         }) | ||||
|     } | ||||
| } | ||||
| 
 | ||||
| impl fmt::Display for ConditionType { | ||||
|     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||||
|         match self { | ||||
|             Self::Accepted => write!(f, "Accepted"), | ||||
|         } | ||||
|     } | ||||
| } | ||||
| 
 | ||||
| pub mod convert { | ||||
|     use super::*; | ||||
| 
 | ||||
|  |  | |||
|  | @ -1297,6 +1297,7 @@ impl PolicyIndex { | |||
|             .http_routes | ||||
|             .iter() | ||||
|             .filter(|(_, route)| route.selects_server(server_name)) | ||||
|             .filter(|(_, route)| route.accepted_by_server(server_name)) | ||||
|             .map(|(name, route)| { | ||||
|                 let mut route = route.route.clone(); | ||||
|                 route.authorizations = self.route_client_authzs(name, authentications); | ||||
|  |  | |||
|  | @ -247,7 +247,29 @@ fn mk_route( | |||
|                 backend_refs: None, | ||||
|             }]), | ||||
|         }, | ||||
|         status: None, | ||||
|         status: Some(k8s::policy::httproute::HttpRouteStatus { | ||||
|             inner: k8s::gateway::RouteStatus { | ||||
|                 parents: vec![k8s::gateway::RouteParentStatus { | ||||
|                     parent_ref: k8s::gateway::ParentReference { | ||||
|                         group: Some(POLICY_API_GROUP.to_string()), | ||||
|                         kind: Some("Server".to_string()), | ||||
|                         namespace: None, | ||||
|                         name: server.to_string(), | ||||
|                         section_name: None, | ||||
|                         port: None, | ||||
|                     }, | ||||
|                     controller_name: "policy.linkerd.io/status-controller".to_string(), | ||||
|                     conditions: vec![k8s::Condition { | ||||
|                         last_transition_time: k8s::Time(chrono::DateTime::<chrono::Utc>::MIN_UTC), | ||||
|                         message: "".to_string(), | ||||
|                         observed_generation: None, | ||||
|                         reason: "Accepted".to_string(), | ||||
|                         status: "True".to_string(), | ||||
|                         type_: "Accepted".to_string(), | ||||
|                     }], | ||||
|                 }], | ||||
|             }, | ||||
|         }), | ||||
|     } | ||||
| } | ||||
| fn mk_authorization_policy( | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue