diff --git a/api/src/apps/mod.rs b/api/src/apps/mod.rs index b0d6b776..27c8d941 100644 --- a/api/src/apps/mod.rs +++ b/api/src/apps/mod.rs @@ -34,9 +34,7 @@ use crate::config::{Config, ConfigError}; use crate::deployment::deployment_unit::DeploymentTemplatingError; use crate::deployment::deployment_unit::DeploymentUnitBuilder; use crate::deployment::hooks::HooksError; -use crate::infrastructure::HttpForwarder; -use crate::infrastructure::Infrastructure; -use crate::infrastructure::TraefikIngressRoute; +use crate::infrastructure::{HttpForwarder, Infrastructure, TraefikIngressRoute}; use crate::models::user_defined_parameters::UserDefinedParameters; use crate::models::Owner; use crate::models::{App, AppName, ContainerType, LogChunk, Service, ServiceConfig, ServiceStatus}; @@ -363,6 +361,9 @@ impl Apps { ); deployment_unit_builder .apply_base_traefik_ingress_route(base_traefik_ingress_route.clone()) + .map_err(|e| AppsError::BaseRouteNotMergeable { + error: e.to_string(), + })? .build() } else { deployment_unit_builder.build() @@ -472,6 +473,8 @@ pub enum AppsError { AppNotFound { app_name: AppName }, #[error("Cannot create more than {limit} apps")] AppLimitExceeded { limit: usize }, + #[error("Cannot merge the base route: {error}")] + BaseRouteNotMergeable { error: String }, /// Will be used when the service cannot interact correctly with the infrastructure. #[error("Cannot interact with infrastructure: {error}")] InfrastructureError { error: String }, diff --git a/api/src/apps/routes/mod.rs b/api/src/apps/routes/mod.rs index 975b009d..4a0ff54e 100644 --- a/api/src/apps/routes/mod.rs +++ b/api/src/apps/routes/mod.rs @@ -579,6 +579,7 @@ impl From for HttpApiError { }, AppsError::AppNotFound { .. } => StatusCode::NOT_FOUND, AppsError::InfrastructureError { .. } + | AppsError::BaseRouteNotMergeable { .. } | AppsError::InvalidServerConfiguration { .. } | AppsError::TemplatingIssue { .. } | AppsError::UnapplicableHook { .. } => { diff --git a/api/src/deployment/deployment_unit.rs b/api/src/deployment/deployment_unit.rs index cfb8645b..90d208b4 100644 --- a/api/src/deployment/deployment_unit.rs +++ b/api/src/deployment/deployment_unit.rs @@ -28,7 +28,9 @@ use url::Url; use super::hooks::HooksError; use crate::config::{Config, StorageStrategy}; use crate::deployment::hooks::Hooks; -use crate::infrastructure::{TraefikIngressRoute, TraefikMiddleware, TraefikRouterRule}; +use crate::infrastructure::{ + TraefikIngressRoute, TraefikIngressRouteMergeError, TraefikMiddleware, TraefikRouterRule, +}; use crate::models::user_defined_parameters::UserDefinedParameters; use crate::models::{AppName, ContainerType, Image, Owner, ServiceConfig}; use crate::registry::ImageInfo; @@ -567,12 +569,12 @@ impl DeploymentUnitBuilder { storage_strategy: &StorageStrategy, image_infos: &HashMap, ) -> Result { + let service_name = raw_service_config.service_name(); + let app_name = &self.stage.app_name; + let ingress_route = match raw_service_config.routing() { - None => TraefikIngressRoute::with_defaults( - &self.stage.app_name, - raw_service_config.service_name(), - ), + None => TraefikIngressRoute::with_defaults(&self.stage.app_name, service_name), Some(routing) => match &routing.rule { Some(rule) => TraefikIngressRoute::with_rule_and_middlewares( TraefikRouterRule::from_str(rule).map_err(|err| { @@ -586,7 +588,7 @@ impl DeploymentUnitBuilder { .iter() .enumerate() .map(|(i, (name, spec))| TraefikMiddleware { - name: format!("custom-middleware-{i}"), + name: format!("{app_name}-{service_name}-custom-middleware-{i}"), spec: serde_value::to_value(serde_json::json!({ name: spec.clone() })) @@ -596,10 +598,10 @@ impl DeploymentUnitBuilder { ), None => TraefikIngressRoute::with_defaults_and_additional_middleware( &self.stage.app_name, - raw_service_config.service_name(), + service_name, routing.additional_middlewares.iter().enumerate().map( |(i, (name, spec))| TraefikMiddleware { - name: format!("custom-middleware-{i}"), + name: format!("{app_name}-{service_name}-custom-middleware-{i}"), spec: serde_value::to_value(serde_json::json!({ name: spec.clone() })) @@ -692,18 +694,18 @@ impl DeploymentUnitBuilder { pub fn apply_base_traefik_ingress_route( mut self, route: TraefikIngressRoute, - ) -> DeploymentUnitBuilder { + ) -> Result, TraefikIngressRouteMergeError> { for service in &mut self.stage.services { let service_route = std::mem::replace(&mut service.ingress_route, route.clone()); - service.ingress_route.merge_with(service_route); + service.ingress_route.merge_with(service_route)?; } let mut route = route; route.merge_with(TraefikIngressRoute::with_app_only_defaults( &self.stage.app_name, - )); + ))?; - DeploymentUnitBuilder { + Ok(DeploymentUnitBuilder { stage: WithAppliedIngressRoute { app_name: self.stage.app_name, services: self.stage.services, @@ -711,7 +713,7 @@ impl DeploymentUnitBuilder { owners: self.stage.owners, user_defined_parameters: self.stage.user_defined_parameters, }, - } + }) } pub fn build(self) -> DeploymentUnit { @@ -1339,7 +1341,7 @@ mod tests { .await? .apply_base_traefik_ingress_route(TraefikIngressRoute::with_rule( TraefikRouterRule::path_prefix_rule(vec![String::from("my-path-prefix")]), - )) + ))? .build(); let service = unit.services.into_iter().next().unwrap(); @@ -1407,7 +1409,7 @@ mod tests { configs[0].ingress_route().routes()[0].middlewares(), &vec![ crate::infrastructure::TraefikMiddleware { - name: String::from("custom-middleware-0"), + name: String::from("master-adminer-custom-middleware-0"), spec: serde_value::to_value(serde_json::json!({ "headers": { "customRequestHeaders": { @@ -1418,7 +1420,7 @@ mod tests { .unwrap() }, crate::infrastructure::TraefikMiddleware { - name: String::from("custom-middleware-1"), + name: String::from("master-adminer-custom-middleware-1"), spec: serde_value::to_value(serde_json::json!({ "stripPrefix": { "prefixes": [ @@ -1482,7 +1484,7 @@ mod tests { .unwrap() }, crate::infrastructure::TraefikMiddleware { - name: String::from("custom-middleware-0"), + name: String::from("master-adminer-custom-middleware-0"), spec: serde_value::to_value(serde_json::json!({ "headers": { "customRequestHeaders": { diff --git a/api/src/infrastructure/docker.rs b/api/src/infrastructure/docker.rs index 9f798f57..6d64f013 100644 --- a/api/src/infrastructure/docker.rs +++ b/api/src/infrastructure/docker.rs @@ -460,14 +460,14 @@ impl DockerInfrastructure { let mut applied_middle_wares = String::new(); for middleware in route.middlewares() { for (key, value) in middleware.to_key_value_spec() { + let middleware_name = &middleware.name; if !applied_middle_wares.is_empty() { - applied_middle_wares += ","; + applied_middle_wares += ", "; } - applied_middle_wares += &format!("{}@docker", middleware.name); + applied_middle_wares += &middleware_name; labels.insert( format!( - "traefik.http.middlewares.{}.{}", - middleware.name, + "traefik.http.middlewares.{middleware_name}.{}", key.to_lowercase() ), value, @@ -1779,7 +1779,7 @@ mod tests { expected: serde_json::json!({ "Labels": { "traefik.http.routers.master-db.rule": "PathPrefix(`/master/db/`)", - "traefik.http.routers.master-db.middlewares": "master-db-middleware@docker", + "traefik.http.routers.master-db.middlewares": "master-db-middleware", "traefik.http.middlewares.master-db-middleware.stripprefix.prefixes": "/master/db/", "traefik.docker.network": "master-net", } diff --git a/api/src/infrastructure/kubernetes/deployment_unit.rs b/api/src/infrastructure/kubernetes/deployment_unit.rs index ef2bc57d..5690fcd7 100644 --- a/api/src/infrastructure/kubernetes/deployment_unit.rs +++ b/api/src/infrastructure/kubernetes/deployment_unit.rs @@ -1442,6 +1442,7 @@ mod tests { &AppName::master(), ), ) + .unwrap() .build(); K8sDeploymentUnit::parse_from_log_streams(&deployment_unit, log_streams) diff --git a/api/src/infrastructure/kubernetes/payloads.rs b/api/src/infrastructure/kubernetes/payloads.rs index 8bc6fd1a..e54682ac 100644 --- a/api/src/infrastructure/kubernetes/payloads.rs +++ b/api/src/infrastructure/kubernetes/payloads.rs @@ -148,15 +148,21 @@ pub fn convert_k8s_ingress_to_traefik_ingress( ingress: Ingress, base_route: TraefikIngressRoute, services: &[V1Service], -) -> Result<(IngressRoute, Vec), (Ingress, &'static str)> { +) -> Result<(IngressRoute, Vec), (Ingress, String)> { let Some(name) = ingress.metadata.name.as_ref() else { - return Err((ingress, "Ingress object does not provide a name")); + return Err(( + ingress, + String::from("Ingress object does not provide a name"), + )); }; let Some(spec) = ingress.spec.as_ref() else { - return Err((ingress, "Ingress does not provide spec")); + return Err((ingress, String::from("Ingress does not provide spec"))); }; let Some(rules) = spec.rules.as_ref() else { - return Err((ingress, "Ingress' spec does not provide rules")); + return Err(( + ingress, + String::from("Ingress' spec does not provide rules"), + )); }; let Some(path) = rules @@ -166,12 +172,15 @@ pub fn convert_k8s_ingress_to_traefik_ingress( else { return Err(( ingress, - "Ingress' rule does not a provide http paths object", + String::from("Ingress' rule does not a provide http paths object"), )); }; let Some(path_value) = path.path.as_ref() else { - return Err((ingress, "Ingress' path does not provide a HTTP path value")); + return Err(( + ingress, + String::from("Ingress' path does not provide a HTTP path value"), + )); }; let (rule, middleware) = match &spec.ingress_class_name { @@ -246,7 +255,9 @@ pub fn convert_k8s_ingress_to_traefik_ingress( let mut route = base_route; if let Some(rule) = rule { - route.merge_with(rule); + if let Err(e) = route.merge_with(rule) { + return Err((ingress, e.to_string())); + } } let mut middlewares_refs = route @@ -270,7 +281,10 @@ pub fn convert_k8s_ingress_to_traefik_ingress( })); let Some(service) = path.backend.service.as_ref() else { - return Err((ingress, "Expecting a service name for the path")); + return Err(( + ingress, + String::from("Expecting a service name for the path"), + )); }; let port = if let Some(port) = service @@ -289,7 +303,7 @@ pub fn convert_k8s_ingress_to_traefik_ingress( else { return Err(( ingress, - "There is no service matching to the ingress' service.", + String::from("There is no service matching to the ingress' service."), )); }; @@ -307,7 +321,7 @@ pub fn convert_k8s_ingress_to_traefik_ingress( else { return Err(( ingress, - "There is no service matching to the ingress' service and port name.", + String::from("There is no service matching to the ingress' service and port name."), )); }; port as u16 @@ -1894,7 +1908,6 @@ mod tests { }), ..Default::default() }, - ..Default::default() }], }), ..Default::default() @@ -1992,7 +2005,6 @@ mod tests { }), ..Default::default() }, - ..Default::default() }], }), ..Default::default() diff --git a/api/src/infrastructure/mod.rs b/api/src/infrastructure/mod.rs index 67268fdb..7b2977c0 100644 --- a/api/src/infrastructure/mod.rs +++ b/api/src/infrastructure/mod.rs @@ -31,7 +31,9 @@ pub use dummy_infrastructure::DummyInfrastructure as Dummy; pub use infrastructure::{HttpForwarder, Infrastructure}; pub use kubernetes::KubernetesInfrastructure as Kubernetes; use serde_json::{map::Map, Value}; -pub use traefik::{TraefikIngressRoute, TraefikMiddleware, TraefikRouterRule}; +pub use traefik::{ + TraefikIngressRoute, TraefikIngressRouteMergeError, TraefikMiddleware, TraefikRouterRule, +}; mod docker; #[cfg(test)] diff --git a/api/src/infrastructure/traefik.rs b/api/src/infrastructure/traefik.rs index 024d2e17..aebdb99f 100644 --- a/api/src/infrastructure/traefik.rs +++ b/api/src/infrastructure/traefik.rs @@ -13,6 +13,17 @@ pub struct TraefikIngressRoute { tls: Option, } +#[derive(Debug, thiserror::Error, PartialEq)] +pub enum TraefikIngressRouteMergeError { + #[error("The ingress route merging is limited to one route each")] + TooManyRoutes, + #[error("Cannot merge {rule_1} with {rule_2} because there is currently no known way of implementing this merge logic.")] + PathRegexpNotMergable { + rule_1: TraefikRouterRule, + rule_2: TraefikRouterRule, + }, +} + impl TraefikIngressRoute { pub fn entry_points(&self) -> &Vec { &self.entry_points @@ -129,11 +140,13 @@ impl TraefikIngressRoute { } } - pub fn merge_with(&mut self, other: Self) { + pub fn merge_with(&mut self, other: Self) -> Result<(), TraefikIngressRouteMergeError> { self.entry_points.extend(other.entry_points); - // FIXME: at the moment there is no handling of multiple routes which needs to be addessed - // in the future when it is required. + if self.routes.len() > 1 || other.routes.len() > 1 { + return Err(TraefikIngressRouteMergeError::TooManyRoutes); + } + match ( self.routes.iter_mut().next(), other.routes.into_iter().next(), @@ -144,7 +157,7 @@ impl TraefikIngressRoute { } (Some(_), None) => {} (Some(route1), Some(route2)) => { - route1.rule.merge_with(route2.rule); + route1.rule.merge_with(route2.rule)?; route1.middlewares.extend(route2.middlewares); } }; @@ -155,6 +168,8 @@ impl TraefikIngressRoute { (None, Some(tls)) => Some(tls), (Some(_), Some(tls)) => Some(tls), }; + + Ok(()) } pub fn to_url(&self) -> Option { @@ -312,8 +327,20 @@ impl TraefikRouterRule { }) } - pub fn merge_with(&mut self, other: TraefikRouterRule) { - for other_match in other.matches { + pub fn merge_with( + &mut self, + other: TraefikRouterRule, + ) -> Result<(), TraefikIngressRouteMergeError> { + for m in &self.matches { + if matches!(m, Matcher::PathRegexp { .. }) { + return Err(TraefikIngressRouteMergeError::PathRegexpNotMergable { + rule_1: self.clone(), + rule_2: other, + }); + } + } + + for other_match in other.clone().matches { match other_match { Matcher::Headers { key, value } => { self.matches.push(Matcher::Headers { key, value }); @@ -363,8 +390,16 @@ impl TraefikRouterRule { .push(Matcher::PathPrefix { paths: other_paths }); } } + Matcher::PathRegexp { .. } => { + return Err(TraefikIngressRouteMergeError::PathRegexpNotMergable { + rule_1: self.clone(), + rule_2: other, + }) + } } } + + Ok(()) } } @@ -446,7 +481,7 @@ fn value_to_string(value: &Value) -> String { Value::F32(v) => format!("{v}"), Value::F64(v) => format!("{v}"), Value::Char(v) => format!("{v}"), - Value::String(v) => format!("{v}"), + Value::String(v) => v.to_string(), Value::Option(value) if value.is_some() => { // SAFETY: match case is protected by is_some() value_to_string(unsafe { value.as_ref().unwrap_unchecked() }) @@ -454,7 +489,7 @@ fn value_to_string(value: &Value) -> String { Value::Newtype(value) => value_to_string(value), Value::Seq(values) => values .iter() - .map(|v| value_to_string(v)) + .map(value_to_string) .reduce(|acc, s| format!("{acc},{s}")) .unwrap_or_default(), _ => String::new(), @@ -464,12 +499,14 @@ fn value_to_string(value: &Value) -> String { #[derive(pest_derive::Parser)] #[grammar_inline = r#" ident = { (ASCII_ALPHANUMERIC | PUNCTUATION)+ } +regexp = { (ASCII_ALPHANUMERIC | PUNCTUATION | "\\" | "/" | "^" | "$" | "[" | "]" | "(" | ")" | "+" | "*" | "|")+ } -Root = _{ (Headers | Host | PathPrefix) ~ ( " "* ~ "&&" ~ " "* ~ (Headers | Host | PathPrefix))* } +Root = _{ (Headers | Host | PathPrefix | PathRegexp) ~ ( " "* ~ "&&" ~ " "* ~ (Headers | Host | PathPrefix | PathRegexp))* } Headers = { "Headers" ~ "(`" ~ ident ~ "`, `" ~ ident ~ "`)" } Host = { "Host" ~ "(`" ~ ident ~ ("`, `" ~ ident)* ~ "`)" } PathPrefix = { "PathPrefix" ~ "(`" ~ ident ~ ("`, `" ~ ident)* ~ "`)" } +PathRegexp = { "PathRegexp" ~ "(`" ~ regexp ~ "`)" } "#] struct RuleParser; @@ -478,6 +515,7 @@ pub enum Matcher { Headers { key: String, value: String }, Host { domains: Vec }, PathPrefix { paths: Vec }, + PathRegexp { regexp: String }, } impl FromStr for TraefikRouterRule { @@ -519,7 +557,18 @@ impl FromStr for TraefikRouterRule { rule.matches.push(Matcher::PathPrefix { paths }); } - Rule::ident | Rule::Root => {} + Rule::PathRegexp => { + let Some(regexp) = pair + .into_inner() + .map(|pair| pair.as_str().to_string()) + .next() + else { + unreachable!("regexp value should be always available once the grammar has been validated.") + }; + + rule.matches.push(Matcher::PathRegexp { regexp }); + } + Rule::ident | Rule::Root | Rule::regexp => {} } } @@ -563,6 +612,9 @@ impl Display for TraefikRouterRule { write!(f, ")")?; } + Matcher::PathRegexp { regexp } => { + write!(f, "PathRegexp(`{regexp}`)")?; + } } } Ok(()) @@ -765,7 +817,7 @@ mod test { let mut base_prefix_rule = "PathPrefix(`/base`)".parse::().unwrap(); let path_prefix_rule = "PathPrefix(`/test`)".parse::().unwrap(); - base_prefix_rule.merge_with(path_prefix_rule); + base_prefix_rule.merge_with(path_prefix_rule).unwrap(); assert_eq!( Ok(base_prefix_rule), @@ -773,12 +825,36 @@ mod test { ); } + #[rstest::rstest] + #[case( + "PathPrefix(`/base`)", + "PathRegexp(`^/products/(shoes\\|socks)/[0-9]+$`)" + )] + #[case( + "PathRegexp(`^/products/(shoes\\|socks)/[0-9]+$`)", + "PathPrefix(`/base`)" + )] + fn merge_path_prefix_rule_with_regexp_rule(#[case] base: &str, #[case] other: &str) { + let mut base_prefix_rule = base.parse::().unwrap(); + let path_prefix_rule = other.parse::().unwrap(); + + let err = base_prefix_rule.merge_with(path_prefix_rule).unwrap_err(); + + assert_eq!( + err, + TraefikIngressRouteMergeError::PathRegexpNotMergable { + rule_1: base.parse().unwrap(), + rule_2: other.parse().unwrap() + } + ); + } + #[test] fn merge_host_path_prefix_rules() { let mut host_rule = "Host(`example.com`)".parse::().unwrap(); let path_prefix_rule = "PathPrefix(`/test`)".parse::().unwrap(); - host_rule.merge_with(path_prefix_rule); + host_rule.merge_with(path_prefix_rule).unwrap(); assert_eq!( host_rule, @@ -802,7 +878,7 @@ mod test { .parse::() .unwrap(); - host_rule1.merge_with(host_rule2); + host_rule1.merge_with(host_rule2).unwrap(); assert_eq!( host_rule1, @@ -819,7 +895,7 @@ mod test { let host_rule = "Host(`example.com`)".parse::().unwrap(); let mut path_prefix_rule = "PathPrefix(`/test`)".parse::().unwrap(); - path_prefix_rule.merge_with(host_rule); + path_prefix_rule.merge_with(host_rule).unwrap(); assert_eq!( path_prefix_rule, @@ -845,7 +921,7 @@ mod test { .parse::() .unwrap(); - headers_rule_1.merge_with(headers_rule_2); + headers_rule_1.merge_with(headers_rule_2).unwrap(); assert_eq!( headers_rule_1, @@ -862,7 +938,7 @@ mod test { .parse::() .unwrap(); - path_prefix_rule.merge_with(headers_rule); + path_prefix_rule.merge_with(headers_rule).unwrap(); assert_eq!( path_prefix_rule, @@ -901,7 +977,7 @@ mod test { }; let route2 = TraefikIngressRoute::with_defaults(&AppName::master(), "whoami"); - route1.merge_with(route2); + route1.merge_with(route2).unwrap(); assert_eq!( route1, @@ -963,7 +1039,7 @@ mod test { }; let mut route2 = TraefikIngressRoute::with_defaults(&AppName::master(), "whoami"); - route2.merge_with(route1); + route2.merge_with(route1).unwrap(); assert_eq!( route2, @@ -1007,7 +1083,7 @@ mod test { let mut route1 = TraefikIngressRoute::empty(); let route2 = TraefikIngressRoute::empty(); - route1.merge_with(route2); + route1.merge_with(route2).unwrap(); assert_eq!(route1, TraefikIngressRoute::empty()); } @@ -1016,10 +1092,12 @@ mod test { fn merge_empty_with_none_empty_ingress_routes() { let mut route1 = TraefikIngressRoute::empty(); - route1.merge_with(TraefikIngressRoute::with_defaults( - &AppName::master(), - "test", - )); + route1 + .merge_with(TraefikIngressRoute::with_defaults( + &AppName::master(), + "test", + )) + .unwrap(); assert_eq!( route1, @@ -1038,7 +1116,7 @@ mod test { cert_resolver: String::from("second"), }); - route1.merge_with(route2); + route1.merge_with(route2).unwrap(); assert_eq!( route1, @@ -1052,6 +1130,42 @@ mod test { ); } + #[test] + fn cannot_merge_ingress_routes() { + let route1 = TraefikIngressRoute { + entry_points: vec![String::from("web")], + routes: vec![TraefikRoute { + rule: TraefikRouterRule::host_rule(vec![String::from("prevant.example.com")]), + middlewares: vec![TraefikMiddleware { + name: String::from("traefik-forward-auth"), + spec: serde_value::to_value(serde_json::json!({ + "forwardAuth": { + "address": "http://traefik-forward-auth.my-namespace.svc.cluster.local:4181" + } + })).unwrap() + }], + }, TraefikRoute { + rule: TraefikRouterRule::host_rule(vec![String::from("prevant.example.com")]), + middlewares: vec![TraefikMiddleware { + name: String::from("traefik-forward-auth"), + spec: serde_value::to_value(serde_json::json!({ + "forwardAuth": { + "address": "http://traefik-forward-auth.my-namespace.svc.cluster.local:4181" + } + })).unwrap() + }], + }], + tls: Some(TraefikTLS { + cert_resolver: String::from("letsencrypt"), + }), + }; + let mut route2 = TraefikIngressRoute::with_defaults(&AppName::master(), "whoami"); + + let err = route2.merge_with(route1).unwrap_err(); + + assert_eq!(err, TraefikIngressRouteMergeError::TooManyRoutes) + } + mod from_url { use super::*;