diff --git a/internal/envoyinit/Cargo.lock b/internal/envoyinit/Cargo.lock index 5ce77f0c04a..4fd66c9e967 100644 --- a/internal/envoyinit/Cargo.lock +++ b/internal/envoyinit/Cargo.lock @@ -241,12 +241,6 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" -[[package]] -name = "lazy_static" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" - [[package]] name = "libc" version = "0.2.177" @@ -527,10 +521,10 @@ version = "0.1.0" dependencies = [ "anyhow", "envoy-proxy-dynamic-modules-rust-sdk", - "lazy_static", "matchers", "minijinja", "mockall", + "once_cell", "rand", "serde", "serde_json", @@ -682,6 +676,26 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" +[[package]] +name = "thiserror" +version = "2.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f63587ca0f12b72a0600bcba1d40081f830876000bb46dd2337a3051618f4fc8" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ff15c8ecd7de3849db632e14d18d2571fa09dfc5ed93479bc4485c7a517c913" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "time" version = "0.3.44" @@ -708,10 +722,12 @@ dependencies = [ "anyhow", "base64", "minijinja", + "once_cell", "rand", "serde", "serde_json", "serde_with", + "thiserror", ] [[package]] diff --git a/internal/envoyinit/rustformations/Cargo.toml b/internal/envoyinit/rustformations/Cargo.toml index 2aa8dfff4b4..b681f709c7f 100644 --- a/internal/envoyinit/rustformations/Cargo.toml +++ b/internal/envoyinit/rustformations/Cargo.toml @@ -14,8 +14,8 @@ minijinja = { version = "2.7.0" } tempfile = "3.16.0" mockall = "0.13.1" transformations = { path = "../transformations" } -lazy_static = "1.5.0" anyhow = "1.0.100" +once_cell = "1.21.3" [lib] name = "rust_module" diff --git a/internal/envoyinit/rustformations/src/http_simple_mutations.rs b/internal/envoyinit/rustformations/src/http_simple_mutations.rs index aef4ba326be..c3402db27a9 100644 --- a/internal/envoyinit/rustformations/src/http_simple_mutations.rs +++ b/internal/envoyinit/rustformations/src/http_simple_mutations.rs @@ -1,15 +1,17 @@ +use anyhow::{Context, Result}; use envoy_proxy_dynamic_modules_rust_sdk::*; -use lazy_static::lazy_static; +use once_cell::sync::Lazy; use serde::Deserialize; +use serde_json::Value as JsonValue; use std::collections::HashMap; -use transformations::{LocalTransformationConfig, TransformationOps}; +use transformations::{ + LocalTransform, LocalTransformationConfig, TransformationError, TransformationOps, +}; #[cfg(test)] use mockall::*; -lazy_static! { - static ref EMPTY_MAP: HashMap = HashMap::new(); -} +static EMPTY_MAP: Lazy> = Lazy::new(HashMap::new); #[derive(Deserialize, Clone)] pub struct FilterConfig { transformations: LocalTransformationConfig, @@ -17,10 +19,13 @@ pub struct FilterConfig { struct EnvoyTransformationOps<'a> { envoy_filter: &'a mut dyn EnvoyHttpFilter, - // TODO: see comment for get_random_pattern() below - // random_pattern_map: &'a mut Option>, } +impl<'a> EnvoyTransformationOps<'a> { + fn new(envoy_filter: &'a mut dyn EnvoyHttpFilter) -> EnvoyTransformationOps<'a> { + EnvoyTransformationOps { envoy_filter } + } +} impl TransformationOps for EnvoyTransformationOps<'_> { fn set_request_header(&mut self, key: &str, value: &[u8]) -> bool { self.envoy_filter.set_request_header(key, value) @@ -28,35 +33,65 @@ impl TransformationOps for EnvoyTransformationOps<'_> { fn remove_request_header(&mut self, key: &str) -> bool { self.envoy_filter.remove_request_header(key) } + fn parse_request_json_body(&mut self) -> Result { + let Some(buffers) = self.envoy_filter.get_request_body() else { + return Ok(JsonValue::Null); + }; + + if buffers.is_empty() { + return Ok(JsonValue::Null); + } + // TODO: implement Reader for EnvoyBuffer and use serde_json::from_reader to avoid making copy first? + let chunks: Vec<_> = buffers.iter().map(|b| b.as_slice()).collect(); + let body = chunks.concat(); + serde_json::from_slice(&body).context("failed to parse request body as json") + } + fn get_request_body(&mut self) -> Vec { + if let Some(buffers) = self.envoy_filter.get_request_body() { + // TODO: implement Reader for EnvoyBuffer and use serde_json::from_reader to avoid making copy first? + let chunks: Vec<_> = buffers.iter().map(|b| b.as_slice()).collect(); + chunks.concat(); + } + + Vec::default() + } + fn drain_request_body(&mut self, number_of_bytes: usize) -> bool { + self.envoy_filter.drain_request_body(number_of_bytes) + } + fn append_request_body(&mut self, data: &[u8]) -> bool { + self.envoy_filter.append_request_body(data) + } + fn set_response_header(&mut self, key: &str, value: &[u8]) -> bool { self.envoy_filter.set_response_header(key, value) } fn remove_response_header(&mut self, key: &str) -> bool { self.envoy_filter.remove_response_header(key) } - /* - TODO: was trying to use this to store the pattern in the request context that can be re-used - for all replace_with_random() custom function but have not been able to find a way to - do that yet with rust and minijinja - - fn get_random_pattern(&mut self, key: &str) -> String { - let map = self.random_pattern_map.get_or_insert_with(HashMap::new); - - if let Some(pattern) = map.get(key) { - return pattern.clone(); - } - - let new_pattern = rand::thread_rng() - .sample_iter(&Alphanumeric) - .take(8) - .map(char::from) - .collect() - - map.insert(key.to_string(), new_pattern.clone()); + fn parse_response_json_body(&mut self) -> Result { + let Some(buffers) = self.envoy_filter.get_response_body() else { + return Ok(JsonValue::Null); + }; + // TODO: implement Reader for EnvoyBuffer and use serde_json::from_reader to avoid making copy first? + let chunks: Vec<_> = buffers.iter().map(|b| b.as_slice()).collect(); + let body = chunks.concat(); + serde_json::from_slice(&body).context("failed to parse response body as json") + } + fn get_response_body(&mut self) -> Vec { + let Some(buffers) = self.envoy_filter.get_response_body() else { + return Vec::default(); + }; - new_pattern - } - */ + // TODO: implement Reader for EnvoyBuffer and use serde_json::from_reader to avoid making copy first? + let chunks: Vec<_> = buffers.iter().map(|b| b.as_slice()).collect(); + chunks.concat() + } + fn drain_response_body(&mut self, number_of_bytes: usize) -> bool { + self.envoy_filter.drain_response_body(number_of_bytes) + } + fn append_response_body(&mut self, data: &[u8]) -> bool { + self.envoy_filter.append_response_body(data) + } } impl FilterConfig { @@ -69,7 +104,7 @@ impl FilterConfig { Ok(cfg) => cfg, Err(err) => { // Dont panic if there is incorrect configuration - envoy_log_error!("Error parsing filter config: {filter_config} {err}"); + envoy_log_error!("error parsing filter config: {filter_config} {err}"); return None; } }; @@ -88,7 +123,6 @@ impl HttpFilterConfig for FilterConfig { Box::new(Filter { filter_config: self.clone(), per_route_config: None, - env: transformations::jinja::new_jinja_env(), request_headers_map: None, }) } @@ -97,7 +131,6 @@ impl HttpFilterConfig for FilterConfig { pub struct Filter { filter_config: FilterConfig, per_route_config: Option>, - env: minijinja::Environment<'static>, request_headers_map: Option>, } @@ -156,44 +189,103 @@ impl Filter { self.request_headers_map.as_ref().unwrap_or(&EMPTY_MAP) } - fn transform_request_headers(&self, envoy_filter: &mut EHF) { - let request_transform = match self.get_per_route_config() { + // set_per_route_config() has to be called before calling this function + fn get_request_transform(&self) -> &Option { + match self.get_per_route_config() { Some(config) => &config.transformations.request, None => &self.filter_config.transformations.request, + } + } + + // set_per_route_config() has to be called before calling this function + fn has_request_transform(&self) -> bool { + let Some(transform) = self.get_request_transform() else { + return false; + }; + + !transform.is_empty() + } + + // set_per_route_config() has to be called before calling this function + fn get_response_transform(&self) -> &Option { + match self.get_per_route_config() { + Some(config) => &config.transformations.response, + None => &self.filter_config.transformations.response, + } + } + + // set_per_route_config() has to be called before calling this function + fn has_response_transform(&self) -> bool { + let Some(transform) = self.get_response_transform() else { + return false; }; - if let Some(transform) = request_transform { - if let Err(e) = transformations::jinja::transform_request_headers( + !transform.is_empty() + } + + fn transform_request(&self, envoy_filter: &mut EHF) -> bool { + if let Some(transform) = self.get_request_transform() { + match transformations::jinja::transform_request( transform, - &self.env, self.get_request_headers_map(), - EnvoyTransformationOps { envoy_filter }, + EnvoyTransformationOps::new(envoy_filter), ) { - envoy_log_warn!("{e}"); + Ok(()) => {} + Err(err) => { + if let Some(e) = err.downcast_ref::() { + match e { + TransformationError::UndeclaredJsonVariables(_msg) => { + envoy_log_error!("{:#}", err); + envoy_filter.send_response(400, Vec::default(), None); + return false; + } + } + } else if let Some(e) = err.downcast_ref::() { + envoy_log_error!("json parsing error: {:#}", e); + envoy_filter.send_response(400, Vec::default(), None); + return false; + } else { + envoy_log_warn!("{:#}", err); + } + } } } - } - fn transform_response_headers(&self, envoy_filter: &mut EHF) { - let response_transform = match self.get_per_route_config() { - Some(config) => &config.transformations.response, - None => &self.filter_config.transformations.response, - }; + true + } - if let Some(transform) = response_transform { - // TODO(nfuden): find someone who knows rust to see if we really need this Hash map for serialization + fn transform_response(&self, envoy_filter: &mut EHF) -> bool { + if let Some(transform) = self.get_response_transform() { let response_headers_map = self.create_headers_map(envoy_filter.get_response_headers()); - if let Err(e) = transformations::jinja::transform_response_headers( + match transformations::jinja::transform_response( transform, - &self.env, self.get_request_headers_map(), &response_headers_map, - EnvoyTransformationOps { envoy_filter }, + EnvoyTransformationOps::new(envoy_filter), ) { - envoy_log_warn!("{e}"); + Ok(()) => {} + Err(err) => { + if let Some(e) = err.downcast_ref::() { + match e { + TransformationError::UndeclaredJsonVariables(_msg) => { + envoy_log_error!("{:#}", err); + envoy_filter.send_response(400, Vec::default(), None); + return false; + } + } + } else if let Some(e) = err.downcast_ref::() { + envoy_log_error!("json parsing error: {:#}", e); + envoy_filter.send_response(400, Vec::default(), None); + return false; + } else { + envoy_log_warn!("{:#}", err); + } + } } } + + true } } @@ -204,20 +296,29 @@ impl HttpFilter for Filter { envoy_filter: &mut EHF, _end_of_stream: bool, ) -> abi::envoy_dynamic_module_type_on_http_filter_request_headers_status { - envoy_log_trace!("on_request_headers"); - // TODO: need to test if we get called even if there is no transformation setting - // if yes, we need to short circuit here and return Continue + self.set_per_route_config(envoy_filter); + if !self.has_request_transform() { + envoy_log_trace!("on_request_headers skipping"); + return abi::envoy_dynamic_module_type_on_http_filter_request_headers_status::Continue; + } + if !_end_of_stream { // TODO: this here always stop iteration to wait for the full request body, // need to support body passthrough + envoy_log_trace!("on_request_headers buffering"); + // return abi::envoy_dynamic_module_type_on_http_filter_request_headers_status::StopAllIterationAndBuffer; return abi::envoy_dynamic_module_type_on_http_filter_request_headers_status::StopIteration; } + envoy_log_trace!("on_request_headers"); - self.set_per_route_config(envoy_filter); - // TODO(nfuden): find someone who knows rust to see if we really need this Hash map for serialization self.populate_request_headers_map(envoy_filter.get_request_headers()); - self.transform_request_headers(envoy_filter); - abi::envoy_dynamic_module_type_on_http_filter_request_headers_status::Continue + if self.transform_request(envoy_filter) { + return abi::envoy_dynamic_module_type_on_http_filter_request_headers_status::Continue; + } + + // If transform had a critical error, it would have sent a local reply with 400 already, + // so return StopIteration here + abi::envoy_dynamic_module_type_on_http_filter_request_headers_status::StopIteration } fn on_request_body( @@ -225,35 +326,84 @@ impl HttpFilter for Filter { envoy_filter: &mut EHF, end_of_stream: bool, ) -> abi::envoy_dynamic_module_type_on_http_filter_request_body_status { - envoy_log_trace!("on_request_body"); - // TODO: need to test if we get called even if there is no transformation setting - // if yes, we need to short circuit here and return Continue + self.set_per_route_config(envoy_filter); + if !self.has_request_transform() { + envoy_log_trace!("on_request_body skipping"); + return abi::envoy_dynamic_module_type_on_http_filter_request_body_status::Continue; + } + if !end_of_stream { - // TODO: Technically, we don't need to buffer the body yet as we don't support parsing the body now - // but it will be coming next. This is mimicking the C++ transformation filter behavior to - // always buffer the request body by default unless passthrough is set. Will revisit and consider - // if this is the desired behavior when we implement parsing the body + envoy_log_trace!("on_request_body buffering"); + // This is mimicking the C++ transformation filter behavior to always buffer the request body by + // default unless passthrough is set but kgateway doesn't support body passthrough in + // transformation API. return abi::envoy_dynamic_module_type_on_http_filter_request_body_status::StopIterationAndBuffer; } + envoy_log_trace!("on_request_body"); - self.set_per_route_config(envoy_filter); - // TODO(nfuden): find someone who knows rust to see if we really need this Hash map for serialization self.populate_request_headers_map(envoy_filter.get_request_headers()); - self.transform_request_headers(envoy_filter); - abi::envoy_dynamic_module_type_on_http_filter_request_body_status::Continue + if self.transform_request(envoy_filter) { + return abi::envoy_dynamic_module_type_on_http_filter_request_body_status::Continue; + } + + // If transform had a critical error, it would have sent a local reply with 400 already, + // so return StopIteration here + abi::envoy_dynamic_module_type_on_http_filter_request_body_status::StopIterationAndBuffer } fn on_response_headers( &mut self, envoy_filter: &mut EHF, - _end_of_stream: bool, + end_of_stream: bool, ) -> abi::envoy_dynamic_module_type_on_http_filter_response_headers_status { + self.set_per_route_config(envoy_filter); + if !self.has_response_transform() { + envoy_log_trace!("on_response_header skipping"); + return abi::envoy_dynamic_module_type_on_http_filter_response_headers_status::Continue; + } + + if !end_of_stream { + envoy_log_trace!("on_response_headers buffering"); + return abi::envoy_dynamic_module_type_on_http_filter_response_headers_status::StopIteration; + } envoy_log_trace!("on_response_headers"); + self.populate_request_headers_map(envoy_filter.get_request_headers()); + if self.transform_response(envoy_filter) { + return abi::envoy_dynamic_module_type_on_http_filter_response_headers_status::Continue; + } + + // If transform had a critical error, it would have sent a local reply with 400 already, + // so return StopIteration here + abi::envoy_dynamic_module_type_on_http_filter_response_headers_status::StopIteration + } + + fn on_response_body( + &mut self, + envoy_filter: &mut EHF, + end_of_stream: bool, + ) -> abi::envoy_dynamic_module_type_on_http_filter_response_body_status { self.set_per_route_config(envoy_filter); - // TODO(nfuden): find someone who knows rust to see if we really need this Hash map for serialization + if !self.has_response_transform() { + envoy_log_trace!("on_response_body skipping"); + return abi::envoy_dynamic_module_type_on_http_filter_response_body_status::Continue; + } + if !end_of_stream { + envoy_log_trace!("on_response_body buffering"); + // This is mimicking the C++ transformation filter behavior to always buffer the response body by + // default unless passthrough is set but kgateway doesn't support body passthrough in + // transformation API. + return abi::envoy_dynamic_module_type_on_http_filter_response_body_status::StopIterationAndBuffer; + } + envoy_log_trace!("on_response_body"); + self.populate_request_headers_map(envoy_filter.get_request_headers()); - self.transform_response_headers(envoy_filter); - abi::envoy_dynamic_module_type_on_http_filter_response_headers_status::Continue + if self.transform_response(envoy_filter) { + return abi::envoy_dynamic_module_type_on_http_filter_response_body_status::Continue; + } + + // If transform had a critical error, it would have sent a local reply with 400 already, + // so return StopIteration here + abi::envoy_dynamic_module_type_on_http_filter_response_body_status::StopIterationAndBuffer } } diff --git a/internal/envoyinit/rustformations/src/lib.rs b/internal/envoyinit/rustformations/src/lib.rs index c9ce44b7546..49b5489375a 100644 --- a/internal/envoyinit/rustformations/src/lib.rs +++ b/internal/envoyinit/rustformations/src/lib.rs @@ -1,3 +1,10 @@ +/* +TODO: look into enabling this to avoid accidental use of unwrap() and +crash the process. However, there are many tests using unwrap() that +will make the linter unhappy. +#![deny(clippy::unwrap_used, clippy::expect_used)] + */ + use envoy_proxy_dynamic_modules_rust_sdk::*; use std::any::Any; @@ -38,7 +45,7 @@ fn new_http_filter_config_fn( let filter_config = match std::str::from_utf8(filter_config) { Ok(config) => config, Err(_) => { - envoy_log_error!("Invalid UTF-8 in filter configuration"); + envoy_log_error!("invalid UTF-8 in filter configuration"); return None; } }; @@ -57,7 +64,7 @@ fn new_http_filter_per_route_config_fn(name: &str, config: &[u8]) -> Option config, Err(_) => { - envoy_log_error!("Invalid UTF-8 in per route filter configuration"); + envoy_log_error!("invalid UTF-8 in per route filter configuration"); return None; } }; diff --git a/internal/envoyinit/transformations/Cargo.toml b/internal/envoyinit/transformations/Cargo.toml index f863c4885de..fceb2d5b82f 100644 --- a/internal/envoyinit/transformations/Cargo.toml +++ b/internal/envoyinit/transformations/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" anyhow = "1.0.100" base64 = "0.22.1" minijinja = "2.12.0" +once_cell = "1.21.3" rand = "0.9.2" serde = { version = "1.0", features = ["derive", "rc"] } serde_json = "1.0" @@ -15,3 +16,4 @@ serde_with = { version = "3.14", features = [ "macros", "alloc", ], default-features = false } +thiserror = "2.0.17" diff --git a/internal/envoyinit/transformations/src/jinja.rs b/internal/envoyinit/transformations/src/jinja.rs index ebb177c17d6..101e86767b0 100644 --- a/internal/envoyinit/transformations/src/jinja.rs +++ b/internal/envoyinit/transformations/src/jinja.rs @@ -1,17 +1,34 @@ +use crate::BodyParseBehavior; use crate::LocalTransform; use crate::NameValuePair; +use crate::TransformationError; use crate::TransformationOps; use anyhow::{Context, Error, Result}; use base64::{ - engine::general_purpose::{STANDARD, STANDARD_NO_PAD}, + engine::general_purpose::{STANDARD, STANDARD_NO_PAD, URL_SAFE}, Engine, }; -use minijinja::{context, Environment, State}; +use minijinja::{Environment, State}; +use once_cell::sync::Lazy; use rand::Rng; use serde::Deserialize; -use std::collections::HashMap; +use serde_json::Value as JsonValue; +use std::collections::BTreeMap; +use std::collections::{HashMap, HashSet}; use std::env; +// These keys are used in a shared scope in the State where we will also put the parsed json body in. +// So, they needs to be as uniq as possible to minimize collision. +const STATE_LOOKUP_KEY_BODY: &str = "body.dev.kgateway"; +const STATE_LOOKUP_KEY_CONTEXT: &str = "context.dev.kgateway"; +const STATE_LOOKUP_KEY_HEADERS: &str = "headers.dev.kgateway"; +const STATE_LOOKUP_KEY_REQ_HEADERS: &str = "request_headers.dev.kgateway"; + +static ENV: Lazy> = Lazy::new(new_jinja_env); + +static GLOBALS_LOOKUP: Lazy> = + Lazy::new(|| ENV.globals().map(|(k, _)| k).collect()); + // substring can be called with either two or three arguments -- // the first argument is the string to be modified, the second is the start position // of the substring, and the optional third argument is the length of the substring. @@ -20,7 +37,7 @@ use std::env; fn substring(input: &str, start: usize, len: Option) -> String { let input_len = input.len(); if start >= input_len { - return "".to_string(); + return String::default(); } let mut end = input_len; @@ -34,30 +51,54 @@ fn substring(input: &str, start: usize, len: Option) -> String { } fn header(state: &State, key: &str) -> String { - let headers = state.lookup("headers"); + let headers = state.lookup(STATE_LOOKUP_KEY_HEADERS); let Some(headers) = headers else { - return "".to_string(); + return String::default(); }; let Some(header_map) = >::deserialize(headers.clone()).ok() else { - return "".to_string(); + return String::default(); }; header_map.get(key).cloned().unwrap_or_default() } fn request_header(state: &State, key: &str) -> String { - let headers = state.lookup("request_headers"); + let headers = state.lookup(STATE_LOOKUP_KEY_REQ_HEADERS); let Some(headers) = headers else { - return "".to_string(); + return String::default(); }; let Some(header_map) = >::deserialize(headers.clone()).ok() else { - return "".to_string(); + return String::default(); }; header_map.get(key).cloned().unwrap_or_default() } +fn trim_outer_quotes(s: &str) -> &str { + if s.starts_with('"') && s.ends_with('"') && s.len() >= 2 { + &s[1..s.len() - 1] + } else { + s + } +} + +fn raw_string(value: &str) -> String { + // Not sure if this is exactly the correct behavior for this function. In the C++ version, + // the native json object can be added to the context directly and that json object can dump + // out the raw string without un-escaping. Here, it's several layers of deserializing and serializing + // from serde_json::from_slice() -> constructing a BTreeMap -> adding that to the context. + // There is no way to get back the original raw_string. So, escaping the string again is the closest I + // can get. After escaping, the resulting string has extra double quote around the original string, so + // need to trim them + // Interesting Note: somehow the need for trimming the double quotes is exactly the same in the C++ + // code. So, maybe the C++ json object dumps() is also doing something similar behind the scene + match serde_json::to_string(value) { + Ok(s) => trim_outer_quotes(&s).to_string(), + Err(_) => String::default(), + } +} + fn base64_encode(input: &[u8]) -> String { STANDARD.encode(input) } @@ -70,11 +111,20 @@ fn base64_decode(input: &str) -> String { .unwrap_or_default() } +fn base64url_encode(input: &[u8]) -> String { + URL_SAFE.encode(input) +} + +fn base64url_decode(input: &str) -> String { + URL_SAFE + .decode(input) + .ok() + .and_then(|bytes| String::from_utf8(bytes).ok()) + .unwrap_or_default() +} + fn get_env(env_var: &str) -> String { - match env::var(env_var) { - Ok(val) => val, - Err(_e) => "".to_string(), - } + env::var(env_var).unwrap_or_default() } fn replace_with_random(input: &str, to_replace: &str) -> String { @@ -92,27 +142,49 @@ fn replace_with_random(input: &str, to_replace: &str) -> String { input.replace(to_replace, &pattern) } -pub fn new_jinja_env() -> Environment<'static> { +fn replace_with_string(input: &str, to_replace: &str, with_string: &str) -> String { + input.replace(to_replace, with_string) +} + +fn body(state: &State) -> String { + state + .lookup(STATE_LOOKUP_KEY_BODY) + .unwrap_or_default() + .to_string() +} + +fn context(state: &State) -> minijinja::Value { + state.lookup(STATE_LOOKUP_KEY_CONTEXT).unwrap_or_default() +} + +fn new_jinja_env() -> Environment<'static> { let mut env = Environment::new(); + // if parseAsJson is used for body parsing. minijinja would prefer the json instead of custom function + // when rendering the template. For example, we have this `env()` function here, if the json body also has + // a field named `env`, the `env()` call in the template will fail to be rendered because minijinja resolves + // `env` to the json value from the body and then will complain it's not callable. + // If we are adding any new functions, we should make the function name more uniq to minimize the chance + // of collision. env.add_function("env", get_env); env.add_function("substring", substring); // !! Standard string manipulation // env.add_function("trim", trim); env.add_function("base64_encode", base64_encode); - // env.add_function("base64url_encode", base64url_encode); + env.add_function("base64url_encode", base64url_encode); env.add_function("base64_decode", base64_decode); - // env.add_function("base64url_decode", base64url_decode); + env.add_function("base64url_decode", base64url_decode); env.add_function("replace_with_random", replace_with_random); - // env.add_function("raw_string", raw_string); + env.add_function("replace_with_string", replace_with_string); + env.add_function("raw_string", raw_string); // env.add_function("word_count", word_count); // !! Envoy context accessors env.add_function("header", header); env.add_function("request_header", request_header); // env.add_function("extraction", extraction); - // env.add_function("body", body); + env.add_function("body", body); // env.add_function("dynamic_metadata", dynamic_metadata); // !! Datasource Puller needed @@ -123,27 +195,60 @@ pub fn new_jinja_env() -> Environment<'static> { // env.add_function("cluster_metadata", cluster_metadata); // !! Possibly not relevant old inja internal debug stuff - // env.add_function("context", context); - // env.add_function("env", env); - - // specific.extend(self.route_specific.into_iter()); + env.add_function("context", context); env } -fn render(env: &Environment<'static>, ctx: minijinja::Value, template: &str) -> Result { +fn render( + env: &Environment<'static>, + ctx: &minijinja::Value, + template: &str, + parsed_body_as_json: bool, +) -> Result { let tmpl = env .template_from_str(template) - .context("error creating jinja template {template}")?; + .with_context(|| format!("error creating jinja template {}", template))?; + if !parsed_body_as_json { + // This is to mimic the C++ behavior when a transformation is used that needs + // the body is parsed as json but it's not enabled. So, we try to detect if + // the transformation template has any undeclared variables when parseAsJson + // is not turned on. Returning a TransformationError type here will cause + // the envoy layer code to return a local reply with 400 status code. + // Other errors would be logged but they are not critical to stop the request + let undeclared_variables = tmpl.undeclared_variables(true); + if !undeclared_variables.is_empty() { + for v in &undeclared_variables { + // Unfortunately, custom function is also reported as undeclared variables + // by minijinja, so only return error if the undeclared variables are not + // customer functions. GLOBALS_LOCKUP is lazily constructed once and is + // static throughout the lifetime of the process. + if !GLOBALS_LOOKUP.contains(v.as_str()) { + return Err(TransformationError::UndeclaredJsonVariables(format!( + "{:?} from template {}", + undeclared_variables, template + )) + .into()); + } + } + } + } tmpl.render(ctx) - .context("error rendering jinja template {template}") + .with_context(|| format!("error rendering jinja template {}", template)) } fn combine_errors(msg: &str, errors: Vec) -> Result<()> { + // Each error can have multiple level of errors, that's why there is + // the e.chain() iterating through each error and combine them if !errors.is_empty() { let combined = errors .into_iter() - .map(|e| e.to_string()) + .map(|e| { + e.chain() + .map(|cause| cause.to_string()) + .collect::>() + .join(":") + }) .collect::>() .join("; "); return Err(anyhow::anyhow!("{}: {}", msg, combined)); @@ -152,38 +257,122 @@ fn combine_errors(msg: &str, errors: Vec) -> Result<()> { Ok(()) } -/// Transform Request Headers +/// Transform Request /// -/// On any rendering errors, we will remove the header and continue +/// On any header rendering errors, we will remove the header and continue /// All the errors are collected and bubble up the chain so they can be logged -pub fn transform_request_headers( +/// On body parsing as json error, we return error immediately so we can send a +/// 400 response back +pub fn transform_request( transform: &LocalTransform, - env: &Environment<'static>, request_headers_map: &HashMap, mut ops: T, ) -> Result<()> { + let env = &*ENV; let mut errors = Vec::new(); + // let mut m = BTreeMap::new(); + let mut m = HashMap::new(); + // for request rendering, both the header() and request_header() use the request_headers + // so, setting both to the request_headers_map in the context + m.insert( + STATE_LOOKUP_KEY_HEADERS.to_string(), + minijinja::Value::from_serialize(request_headers_map), + ); + m.insert( + STATE_LOOKUP_KEY_REQ_HEADERS.to_string(), + minijinja::Value::from_serialize(request_headers_map), + ); + let mut parsed_body_as_json = false; + if let Some(body_transform) = transform.body.as_ref() { + if matches!(body_transform.parse_as, BodyParseBehavior::AsJson) { + let json_body = ops.parse_request_json_body()?; + + if json_body != JsonValue::Null { + if body_transform.value.contains("context()") { + m.insert( + STATE_LOOKUP_KEY_CONTEXT.to_string(), + minijinja::Value::from_serialize(&json_body), + ); + } + + if let JsonValue::Object(map) = json_body { + for (k, v) in map { + m.insert(k, minijinja::Value::from_serialize(&v)); + } + } + + parsed_body_as_json = true; + } + } + } + + if let Some(body_transform) = transform.body.as_ref() { + if body_transform.value.contains("body()") { + let body = ops.get_request_body(); + m.insert( + STATE_LOOKUP_KEY_BODY.to_string(), + minijinja::Value::from_serialize(String::from_utf8_lossy(&body)), + ); + } + } + + let ctx = minijinja::Value::from(m); + + if let Some(body_transform) = transform.body.as_ref() { + if !body_transform.value.is_empty() { + ops.drain_request_body(u64::MAX.try_into().unwrap()); + let rendered = match render(env, &ctx, &body_transform.value, parsed_body_as_json) { + Ok(str) => Some(str), + Err(e) => { + errors.push(e); + None + } + }; + if rendered.as_deref().is_some_and(|s| !s.is_empty()) { + let rendered_body = rendered.as_deref().unwrap().as_bytes(); + ops.set_request_header( + "content-length", + rendered_body.len().to_string().as_bytes(), + ); + ops.append_request_body(rendered_body); + } else { + ops.set_request_header("content-length", b"0"); + // In classic transformation, we remove content-type only when "passthrough_body" + // is set to true (even the body is not transformed but it comes in as 0 bytes) + // Here, we are only removing content-type if we have an override that ended up + // removing the body as we don't have passthrough_body setting in kgateway + ops.remove_request_header("content-type"); + } + } + } + + let mut abort_processing = false; for NameValuePair { name: key, value } in &transform.set { if value.is_empty() { - // This is following the legacy transformation filter behavior + // This is following the classic transformation filter behavior ops.remove_request_header(key); continue; } - let rendered = match render( - env, - // for request rendering, both the header() and request_header() use the request_headers - // so, setting both to the request_headers_map in the context - context!(headers => request_headers_map, request_headers => request_headers_map), - value, - ) { + let rendered = match render(env, &ctx, value, parsed_body_as_json) { Ok(str) => Some(str), - Err(e) => { - errors.push(e); + Err(err) => { + if let Some(e) = err.downcast_ref::() { + match e { + TransformationError::UndeclaredJsonVariables(_) => { + abort_processing = true; + } + } + } + errors.push(err); None } }; + if abort_processing { + return Err(errors.pop().unwrap()); + } + if rendered.as_deref().is_some_and(|s| !s.is_empty()) { ops.set_request_header(key, rendered.as_deref().unwrap().as_bytes()); } else { @@ -197,42 +386,127 @@ pub fn transform_request_headers( ops.remove_request_header(key); } - combine_errors("transform_request_headers()", errors) + combine_errors("transform_request()", errors) } -/// Transform Resposne Headers +/// Transform Response /// /// On any rendering errors, we will remove the header and continue /// All the errors are collected and bubble up the chain so they can be logged -pub fn transform_response_headers( +/// On body parsing as json error, we return error immediately so we can send a +/// 400 response back +pub fn transform_response( transform: &LocalTransform, - env: &Environment<'static>, request_headers_map: &HashMap, response_headers_map: &HashMap, mut ops: T, ) -> Result<()> { + let env = &*ENV; let mut errors = Vec::new(); + let mut m = BTreeMap::new(); + // for response rendering, header() uses response_headers and request_header() + // uses the request_headers. So, setting them in the context accordingly + m.insert( + STATE_LOOKUP_KEY_HEADERS.to_string(), + minijinja::Value::from_serialize(response_headers_map), + ); + m.insert( + STATE_LOOKUP_KEY_REQ_HEADERS.to_string(), + minijinja::Value::from_serialize(request_headers_map), + ); + let mut parsed_body_as_json = false; + if let Some(body_transform) = transform.body.as_ref() { + if matches!(body_transform.parse_as, BodyParseBehavior::AsJson) { + let json_body = ops.parse_response_json_body()?; + + if json_body != JsonValue::Null { + if body_transform.value.contains("context()") { + m.insert( + STATE_LOOKUP_KEY_CONTEXT.to_string(), + minijinja::Value::from_serialize(&json_body), + ); + } + + if let JsonValue::Object(map) = json_body { + for (k, v) in map { + m.insert(k, minijinja::Value::from_serialize(&v)); + } + } + parsed_body_as_json = true; + } + } + } + + if let Some(body_transform) = transform.body.as_ref() { + if body_transform.value.contains("body()") { + let body = ops.get_response_body(); + m.insert( + STATE_LOOKUP_KEY_BODY.to_string(), + minijinja::Value::from_serialize(String::from_utf8_lossy(&body)), + ); + } + } + + let ctx = minijinja::Value::from(m); + + if let Some(body_transform) = transform.body.as_ref() { + if !body_transform.value.is_empty() { + // The envoy sdk function would drain all the bytes if the number passed in is greater + // than the content length. This is to avoid having to iterate through the buffer to + // calculate the size. + ops.drain_response_body(u64::MAX.try_into().unwrap()); + let rendered = match render(env, &ctx, &body_transform.value, parsed_body_as_json) { + Ok(str) => Some(str), + Err(e) => { + errors.push(e); + None + } + }; + if rendered.as_deref().is_some_and(|s| !s.is_empty()) { + let rendered_body = rendered.as_deref().unwrap().as_bytes(); + ops.set_response_header( + "content-length", + rendered_body.len().to_string().as_bytes(), + ); + ops.append_response_body(rendered_body); + } else { + ops.set_response_header("content-length", b"0"); + // In classic transformation, we remove content-type only when "passthrough_body" + // is set to true (even the body is not transformed but it comes in as 0 bytes) + // Here, we are only removing content-type if we have an override that ended up + // removing the body as we don't have passthrough_body setting in kgateway + ops.remove_response_header("content-type"); + } + } + } + + let mut abort_processing = false; for NameValuePair { name: key, value } in &transform.set { if value.is_empty() { - // This is following the legacy transformation filter behavior + // This is following the classic transformation filter behavior ops.remove_response_header(key); continue; } - let rendered = match render( - env, - // for response rendering, header() uses response_headers and request_header() - // uses the request_headers. So, setting them in the context accordingly - context!(headers => response_headers_map, request_headers => request_headers_map), - value, - ) { + let rendered = match render(env, &ctx, value, parsed_body_as_json) { Ok(str) => Some(str), - Err(e) => { - errors.push(e); + Err(err) => { + if let Some(e) = err.downcast_ref::() { + match e { + TransformationError::UndeclaredJsonVariables(_) => { + abort_processing = true; + } + } + } + errors.push(err); None } }; + if abort_processing { + return Err(errors.pop().unwrap()); + } + if rendered.as_deref().is_some_and(|s| !s.is_empty()) { ops.set_response_header(key, rendered.as_deref().unwrap().as_bytes()); } else { @@ -246,5 +520,5 @@ pub fn transform_response_headers( ops.remove_response_header(key); } - combine_errors("transform_response_headers()", errors) + combine_errors("transform_response()", errors) } diff --git a/internal/envoyinit/transformations/src/lib.rs b/internal/envoyinit/transformations/src/lib.rs index a20edcb39bb..0e72cfd3c25 100644 --- a/internal/envoyinit/transformations/src/lib.rs +++ b/internal/envoyinit/transformations/src/lib.rs @@ -1,4 +1,13 @@ +/* +TODO: look into enabling this to avoid accidental use of unwrap() and +crash the process. However, there are many tests using unwrap() that +will make the linter unhappy. :w +#![deny(clippy::unwrap_used, clippy::expect_used)] +*/ + +use anyhow::Result; use serde::Deserialize; +use serde_json::Value as JsonValue; pub mod jinja; @@ -22,14 +31,41 @@ pub struct LocalTransform { pub body: Option, } +impl LocalTransform { + pub fn is_empty(&self) -> bool { + if !self.add.is_empty() { + return false; + } + if !self.set.is_empty() { + return false; + } + if !self.remove.is_empty() { + return false; + } + + match &self.body { + Some(config) => config.is_empty(), + None => true, + } + } +} + #[derive(Default, Clone, Deserialize)] pub struct BodyTransform { - #[serde(default)] + #[serde(default, rename = "parseAs")] pub parse_as: BodyParseBehavior, #[serde(default)] pub value: String, } +impl BodyTransform { + pub fn is_empty(&self) -> bool { + if self.value.is_empty() && matches!(self.parse_as, BodyParseBehavior::AsString) { + return true; + } + false + } +} #[derive(Default, Clone, Deserialize)] pub struct NameValuePair { pub name: String, @@ -49,4 +85,18 @@ pub trait TransformationOps { fn remove_request_header(&mut self, key: &str) -> bool; fn set_response_header(&mut self, key: &str, value: &[u8]) -> bool; fn remove_response_header(&mut self, key: &str) -> bool; + fn parse_request_json_body(&mut self) -> Result; + fn get_request_body(&mut self) -> Vec; + fn drain_request_body(&mut self, number_of_bytes: usize) -> bool; + fn append_request_body(&mut self, data: &[u8]) -> bool; + fn parse_response_json_body(&mut self) -> Result; + fn get_response_body(&mut self) -> Vec; + fn drain_response_body(&mut self, number_of_bytes: usize) -> bool; + fn append_response_body(&mut self, data: &[u8]) -> bool; +} + +#[derive(thiserror::Error, Debug)] +pub enum TransformationError { + #[error("undeclared json variables: {0}")] + UndeclaredJsonVariables(String), } diff --git a/test/e2e/features/transformation/suite.go b/test/e2e/features/transformation/suite.go index 743cec41a8c..9253995f650 100644 --- a/test/e2e/features/transformation/suite.go +++ b/test/e2e/features/transformation/suite.go @@ -39,19 +39,26 @@ import ( var _ e2e.NewSuiteFunc = NewTestingSuite +const ( + httpbin_echo_base_path = "/anything/:anything" +) + var ( // manifests - simpleServiceManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "service.yaml") - gatewayManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "gateway.yaml") - transformForCustomFunctionsManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-custom-functions.yaml") - transformForHeadersManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-headers.yaml") - transformForBodyJsonManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-body-json.yaml") - transformForBodyAsStringManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-body-as-string.yaml") - gatewayAttachedTransformManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "gateway-attached-transform.yaml") - transformForMatchPathManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-path.yaml") - transformForMatchHeaderManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-header.yaml") - transformForMatchQueryManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-query.yaml") - transformForMatchMethodManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-method.yaml") + simpleServiceManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "service.yaml") + gatewayManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "gateway.yaml") + transformForCustomFunctionsManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-custom-functions.yaml") + transformForHeadersManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-headers.yaml") + transformForPseudoHeadersManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-pseudo-headers.yaml") + transformForBodyJsonManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-body-json.yaml") + rustformationForBodyJsonManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-body-json-rust.yaml") + transformForBodyAsStringManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-body-as-string.yaml") + gatewayAttachedTransformManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "gateway-attached-transform.yaml") + transformForMatchPathManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-path.yaml") + transformForMatchHeaderManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-header.yaml") + transformForMatchQueryManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-query.yaml") + transformForMatchMethodManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-match-method.yaml") + transformForHeaderToBodyJsonManifest = filepath.Join(fsutils.MustGetThisDir(), "testdata", "transform-for-header-to-body-json.yaml") proxyObjectMeta = metav1.ObjectMeta{ Name: "gw", @@ -66,18 +73,43 @@ var ( gatewayManifest, transformForCustomFunctionsManifest, transformForHeadersManifest, - transformForBodyJsonManifest, + transformForPseudoHeadersManifest, transformForBodyAsStringManifest, gatewayAttachedTransformManifest, transformForMatchHeaderManifest, transformForMatchMethodManifest, transformForMatchPathManifest, transformForMatchQueryManifest, + transformForHeaderToBodyJsonManifest, }, } - // everything is applied during setup; there are no additional test-specific manifests - testCases = map[string]*base.TestCase{} + // Because the jinja template syntax are slightly different between C++ and rust when + // accessing the json object after parsing the body as json, we need to use different + // resources for the same test case when switching between the C++ (classic transformation) + // and Rust (rustformation). Also because there is no hook in the testsuite frame work + // to run custom function right before applying the resource, if you look at the log from envoy + // you will see something like this: + // [2025-11-17 15:37:40.956][1][warning][config] + // [external/envoy/source/extensions/config_subscription/grpc/grpc_subscription_impl.cc:138] + // gRPC config for type.googleapis.com/envoy.config.route.v3.RouteConfiguration rejected: + // Failed to parse response template: Failed to parse header template 'from-incoming': + // [inja.exception.parser_error] (at 1:67) malformed expression + // This is because envoy is still configured to use the classic transformation while the rust + // specific resource is applied. Once the rust test starts, it will switch envoy to the + // rust dynamic module filter and the route will be accepted (and the error will go away) + testCases = map[string]*base.TestCase{ + "TestGatewayWithTransformedRoute": { + Manifests: []string{ + transformForBodyJsonManifest, + }, + }, + "TestGatewayRustformationsWithTransformedRoute": { + Manifests: []string{ + rustformationForBodyJsonManifest, + }, + }, + } ) type transformationTestCase struct { @@ -86,6 +118,11 @@ type transformationTestCase struct { opts []curl.Option resp *testmatchers.HttpResponse req *testmatchers.HttpRequest + // with go-httpbin, cannot use curl.WithPath directly in opts because we + // need to add a path prefix (anything/:anything) to get the request data. + // so, use the to add something to the path if you need to match it in the + // test + url string } // testingSuite is a suite of basic routing / "happy path" tests @@ -97,421 +134,417 @@ type testingSuite struct { commonTestCases []transformationTestCase } -func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite { - return &testingSuite{ - base.NewBaseTestingSuite(ctx, testInst, setup, testCases), - []transformationTestCase{ - { - name: "basic-gateway-attached", - routeName: "gateway-attached-transform", - opts: []curl.Option{ - // in testdata/gateway-attached-transform.yaml, - // for x-empty, the value is set to "" - // for x-not-set, the value is not set - // The behavior for both is removing the existing header - // Testing this to make sure rustformation behaves the same - curl.WithHeader("x-empty", "not empty"), - curl.WithHeader("x-not-set", "set"), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "response-gateway": "goodbye", - }, - NotHeaders: []string{ - "x-foo-response", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "request-gateway": "hello", - }, - NotHeaders: []string{ - "x-not-set", - "x-empty", - }, - }, - }, - { - name: "basic", - routeName: "headers", - opts: []curl.Option{ - curl.WithBody("hello"), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-foo-response": "notsuper", - "x-foo-response-status": "200", - // These are commented out so the testcase will pass on both classic and rustformation - // and left here for documentation purpose - // There should be a space at the beginning and end but - // rust minijinja template rendering seems to right trim the space at the end - // "x-space-test": " foobar", - // while C++ inja leave the space untouched. - // "x-space-test": " foobar ", - }, - NotHeaders: []string{ - "response-gateway", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-foo-bar": "foolen_5", - "x-foo-bar2": "foolen_5", - // There should be a space at the beginning and end but - // there might be a side effect from the echo server where the header values are trimmed - "x-space-test": "foobar", - }, - NotHeaders: []string{ - // looks like the way we set up transformation targeting gateway, we are - // also using RouteTransformation instead of FilterTransformation and it's - // set , so it's set at the route table level and if there is a more specific - // transformation (eg in vhost or prefix match), the gateway attached transformation - // will not apply. Make sure it's not there. - "request-gateway", - }, - }, - }, - { - name: "remove headers", - routeName: "headers", - opts: []curl.Option{ - curl.WithBody("hello"), - curl.WithHeader("x-remove-me", "test"), - curl.WithHeader("x-dont-remove-me", "in request"), - // This instruct the echo server to set the response headers - curl.WithHeader("X-Echo-Set-Header", "x-remove-me:test,x-dont-remove-me:in response"), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-dont-remove-me": "in response", - }, - NotHeaders: []string{ - "x-remove-me", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-dont-remove-me": "in request", - }, - NotHeaders: []string{ - "x-remove-me", - }, - }, - }, - { - name: "set headers with headers already exists multiple times", - routeName: "headers", - opts: []curl.Option{ - curl.WithBody("hello"), - // The 2 x-foo-bar headers will be replaced with a single one when we set the header - // to a value using transformation - curl.WithMultiHeader("x-foo-bar", []string{"original_1", "original_2"}), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-foo-response": "notsuper", - "x-foo-response-status": "200", - }, - NotHeaders: []string{ - "response-gateway", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-foo-bar": "foolen_5", - "x-foo-bar2": "foolen_5", - }, - NotHeaders: []string{ - // looks like the way we set up transformation targeting gateway, we are - // also using RouteTransformation instead of FilterTransformation and it's - // set , so it's set at the route table level and if there is a more specific - // transformation (eg in vhost or prefix match), the gateway attached transformation - // will not apply. Make sure it's not there. - "request-gateway", - }, - }, - }, - { - name: "conditional set by request header", // inja and the request_header function in use - routeName: "headers", - opts: []curl.Option{ - curl.WithBody("hello-world"), - curl.WithHeader("x-add-bar", "super"), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-foo-response": "supersupersuper", - "x-foo-response-status": "200", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-foo-bar": "foolen_11", - "x-foo-bar2": "foolen_11", - }, - NotHeaders: []string{ - // looks like the way we set up transformation targeting gateway, we are - // also using RouteTransformation instead of FilterTransformation and it's - // set , so it's set at the route table level and if there is a more specific - // transformation (eg in vhost or prefix match), the gateway attached transformation - // will not apply. Make sure it's not there. - "request-gateway", - }, - }, - }, - { - // When all matching criterion are met, path match takes precedence - name: "match-all", - routeName: "match", - opts: []curl.Option{ - curl.WithHeader("foo", "bar"), - curl.WithPath("/path_match/index.html"), - curl.WithQueryParameters(map[string]string{"test": "123"}), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-foo-response": "path matched", - "x-path-response": "matched", - }, - NotHeaders: []string{ - "response-gateway", - "x-method-response", - "x-header-response", - "x-query-response", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-foo-request": "path matched", - "x-path-request": "matched", - }, - NotHeaders: []string{ - "request-gateway", - "x-method-request", - "x-header-request", - "x-query-request", - }, - }, - }, - { - // When all matching criterion are met except path, method match takes precedence - name: "match-method-header-and-query", - routeName: "match", - opts: []curl.Option{ - curl.WithHeader("foo", "bar"), - curl.WithPath("/index.html"), - curl.WithQueryParameters(map[string]string{"test": "123"}), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-foo-response": "method matched", - "x-method-response": "matched", - }, - NotHeaders: []string{ - "response-gateway", - "x-path-response", - "x-header-response", - "x-query-response", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-foo-request": "method matched", - "x-method-request": "matched", - }, - NotHeaders: []string{ - "request-gateway", - "x-path-request", - "x-header-request", - "x-query-request", - }, - }, - }, - { - // When all matching criterion are met except path and method, header match takes precedence - name: "match-header-and-query", - routeName: "match", - opts: []curl.Option{ - curl.WithBody("hello"), - curl.WithHeader("foo", "bar"), - curl.WithPath("/index.html"), - curl.WithQueryParameters(map[string]string{"test": "123"}), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-foo-response": "header matched", - "x-header-response": "matched", - }, - NotHeaders: []string{ - "response-gateway", - "x-path-response", - "x-method-response", - "x-query-response", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-foo-request": "header matched", - "x-header-request": "matched", - }, - NotHeaders: []string{ - "request-gateway", - "x-path-request", - "x-method-request", - "x-query-request", - }, - }, - }, - { - name: "match-query", - routeName: "match", - opts: []curl.Option{ - curl.WithBody("hello"), - curl.WithPath("/index.html"), - curl.WithQueryParameters(map[string]string{"test": "123"}), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-foo-response": "query matched", - "x-query-response": "matched", - }, - NotHeaders: []string{ - "response-gateway", - "x-path-response", - "x-method-response", - "x-header-response", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-foo-request": "query matched", - "x-query-request": "matched", - }, - NotHeaders: []string{ - "request-gateway", - "x-path-request", - "x-method-request", - "x-header-request", - }, - }, - }, - { - // Interesting Note: because when a transformation attached to the gateway is set at route-table - // level, when nothing match and envoy returns 404, that transformation won't ge applied neither! - name: "match-none", - routeName: "match", - opts: []curl.Option{ - curl.WithBody("hello"), - curl.WithPath("/index.html"), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusNotFound, - Headers: map[string]any{ - // The Gateway attached transformation never apply when no route match - // "response-gateway": "goodbyte", - }, - NotHeaders: []string{ - "response-gateway", - "x-path-response", - "x-method-response", - "x-header-response", - "x-query-response", - "x-foo-response", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - // The Gateway attached transformation never apply when no route match - // "request-gateway": "hello", - }, - NotHeaders: []string{ - "request-gateway", - "x-path-request", - "x-method-request", - "x-header-request", - "x-foo-request", - "x-query-request", - }, - }, - }, - { - name: "custom functions", - routeName: "custom-functions", - opts: []curl.Option{ - curl.WithBody(`{"foo":"\"bar\""}`), - }, - resp: &testmatchers.HttpResponse{ - StatusCode: http.StatusOK, - Headers: map[string]any{ - "x-base64-encode": "YmFzZTY0IGVuY29kZSBpbiByZXNwb25zZSBoZWFkZXI=", - "x-base64-decode": "base64 decode in response header", - "x-base64-decode-invalid-non-empty": "foobar", - "x-substring": "response", - "x-substring2": "resp", - // when the len is invalid, we default to the end of the string - "x-substring-invalid2": "response", - "x-env": gomega.MatchRegexp(`default/gw-[a-f0-9]*-[a-z0-9]*`), - "x-replace-random": gomega.MatchRegexp(`.+ be or not .+ be`), - }, - NotHeaders: []string{ - // When decode fail, we return an empty string which in turn becomes a "remove" header ops - "x-base64-decode-invalid", - // when start is invalid, we return an empty string which in turn becomes a "remove" header ops - "x-substring-invalid", - "x-env-not-set", - }, - }, - req: &testmatchers.HttpRequest{ - Headers: map[string]any{ - "x-base64-encode": "YmFzZTY0IGVuY29kZSBpbiByZXF1ZXN0IGhlYWRlcg==", - "x-base64-decode": "base64 decode in request header", - "x-base64-decode-invalid-non-empty": "foobar", - "x-substring": "request", - "x-substring2": "req", - // when the len is invalid, we default to the end of the string - "x-substring-invalid2": "request", - "x-env": gomega.MatchRegexp(`default/gw-[a-f0-9]*-[a-z0-9]*`), - "x-replace-random": gomega.MatchRegexp(`.+ be or not .+ be`), - }, - NotHeaders: []string{ - // When decode fail, we return an empty string which in turn becomes a "remove" header ops - "x-base64-decode-invalid", - // when start is invalid, we return an empty string which in turn becomes a "remove" header ops - "x-substring-invalid", - "x-env-not-set", - }, +// select specific test cases to run. Mainly for speeding up local testing +// when working on a specific test case. By default, when indices is empty, +// it returns all test cases. -1 index select the last one. +func selectCommonTestCases(indices ...int) []transformationTestCase { + commonTestCases := []transformationTestCase{ + { + // test 0 + name: "basic-gateway-attached", + routeName: "gateway-attached-transform", + opts: []curl.Option{ + // in testdata/gateway-attached-transform.yaml, + // for x-empty, the value is set to "" + // for x-not-set, the value is not set + // The behavior for both is removing the existing header + // Testing this to make sure rustformation behaves the same + curl.WithHeader("x-empty", "not empty"), + curl.WithHeader("x-not-set", "set"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "response-gateway": "goodbye", + }, + NotHeaders: []string{ + "x-foo-response", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "request-gateway": "hello", + }, + NotHeaders: []string{ + "x-not-set", + "x-empty", }, }, }, - } -} - -func (s *testingSuite) SetupSuite() { - s.BaseTestingSuite.SetupSuite() - - s.assertStatus() -} - -func (s *testingSuite) TestGatewayWithTransformedRoute() { - s.SetRustformationInController(false) - - s.TestInstallation.Assertions.AssertEnvoyAdminApi( - s.Ctx, - proxyObjectMeta, - s.dynamicModuleAssertion(false), - ) - - testCases := []transformationTestCase{ { + // test 1 + name: "basic", + routeName: "headers", + opts: []curl.Option{ + curl.WithBody("hello"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-foo-response": "notsuper", + "x-foo-response-status": "200", + // These are commented out so the testcase will pass on both classic and rustformation + // and left here for documentation purpose + // There should be a space at the beginning and end but + // rust minijinja template rendering seems to right trim the space at the end + // "x-space-test": " foobar", + // while C++ inja leave the space untouched. + // "x-space-test": " foobar ", + }, + NotHeaders: []string{ + "response-gateway", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-foo-bar": "foolen_5", + "x-foo-bar2": "foolen_5", + // There should be a space at the beginning and end but + // there might be a side effect from the echo server where the header values are trimmed + "x-space-test": "foobar", + }, + NotHeaders: []string{ + // looks like the way we set up transformation targeting gateway, we are + // also using RouteTransformation instead of FilterTransformation and it's + // set , so it's set at the route table level and if there is a more specific + // transformation (eg in vhost or prefix match), the gateway attached transformation + // will not apply. Make sure it's not there. + "request-gateway", + }, + }, + }, + { + // test 2 + name: "remove headers", + routeName: "headers", + opts: []curl.Option{ + curl.WithBody("hello"), + curl.WithHeader("x-remove-me", "test"), + curl.WithHeader("x-dont-remove-me", "in request"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + // go-httpbin doesn't allow setting custom response header, so make sure + // we get one of the default access-control header and removed the other + Headers: map[string]any{ + "access-control-allow-credentials": "true", + }, + NotHeaders: []string{ + "access-control-allow-origin", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-dont-remove-me": "in request", + }, + NotHeaders: []string{ + "x-remove-me", + }, + }, + }, + { + // test 3 + name: "set headers with headers already exists multiple times", + routeName: "headers", + opts: []curl.Option{ + curl.WithBody("hello"), + // The 2 x-foo-bar headers will be replaced with a single one when we set the header + // to a value using transformation + curl.WithMultiHeader("x-foo-bar", []string{"original_1", "original_2"}), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-foo-response": "notsuper", + "x-foo-response-status": "200", + }, + NotHeaders: []string{ + "response-gateway", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-foo-bar": "foolen_5", + "x-foo-bar2": "foolen_5", + }, + NotHeaders: []string{ + // looks like the way we set up transformation targeting gateway, we are + // also using RouteTransformation instead of FilterTransformation and it's + // set , so it's set at the route table level and if there is a more specific + // transformation (eg in vhost or prefix match), the gateway attached transformation + // will not apply. Make sure it's not there. + "request-gateway", + }, + Body: "hello", + }, + }, + { + // test 4 + name: "conditional set by request header", // inja and the request_header function in use + routeName: "headers", + opts: []curl.Option{ + curl.WithBody("hello-world"), + curl.WithHeader("x-add-bar", "super"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-foo-response": "supersupersuper", + "x-foo-response-status": "200", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-foo-bar": "foolen_11", + "x-foo-bar2": "foolen_11", + }, + NotHeaders: []string{ + // looks like the way we set up transformation targeting gateway, we are + // also using RouteTransformation instead of FilterTransformation and it's + // set , so it's set at the route table level and if there is a more specific + // transformation (eg in vhost or prefix match), the gateway attached transformation + // will not apply. Make sure it's not there. + "request-gateway", + }, + }, + }, + { + // test 5 + // When all matching criterion are met, path match takes precedence + name: "match-all", + routeName: "match", + opts: []curl.Option{ + curl.WithHeader("foo", "bar"), + curl.WithQueryParameters(map[string]string{"test": "123"}), + }, + url: "/path_match/index.html", + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-foo-response": "path matched", + "x-path-response": "matched", + }, + NotHeaders: []string{ + "response-gateway", + "x-method-response", + "x-header-response", + "x-query-response", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-foo-request": "path matched", + "x-path-request": "matched", + }, + NotHeaders: []string{ + "request-gateway", + "x-method-request", + "x-header-request", + "x-query-request", + }, + }, + }, + { + // test 6 + // When all matching criterion are met except path, method match takes precedence + name: "match-method-header-and-query", + routeName: "match", + opts: []curl.Option{ + curl.WithHeader("foo", "bar"), + curl.WithQueryParameters(map[string]string{"test": "123"}), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-foo-response": "method matched", + "x-method-response": "matched", + }, + NotHeaders: []string{ + "response-gateway", + "x-path-response", + "x-header-response", + "x-query-response", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-foo-request": "method matched", + "x-method-request": "matched", + }, + NotHeaders: []string{ + "request-gateway", + "x-path-request", + "x-header-request", + "x-query-request", + }, + }, + }, + { + // test 7 + // When all matching criterion are met except path and method, header match takes precedence + name: "match-header-and-query", + routeName: "match", + opts: []curl.Option{ + curl.WithBody("hello"), + curl.WithHeader("foo", "bar"), + curl.WithQueryParameters(map[string]string{"test": "123"}), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-foo-response": "header matched", + "x-header-response": "matched", + }, + NotHeaders: []string{ + "response-gateway", + "x-path-response", + "x-method-response", + "x-query-response", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-foo-request": "header matched", + "x-header-request": "matched", + }, + NotHeaders: []string{ + "request-gateway", + "x-path-request", + "x-method-request", + "x-query-request", + }, + }, + }, + { + // test 8 + name: "match-query", + routeName: "match", + opts: []curl.Option{ + curl.WithBody("hello"), + curl.WithQueryParameters(map[string]string{"test": "123"}), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-foo-response": "query matched", + "x-query-response": "matched", + }, + NotHeaders: []string{ + "response-gateway", + "x-path-response", + "x-method-response", + "x-header-response", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-foo-request": "query matched", + "x-query-request": "matched", + }, + NotHeaders: []string{ + "request-gateway", + "x-path-request", + "x-method-request", + "x-header-request", + }, + }, + }, + { + // test 9 + // Interesting Note: because when a transformation attached to the gateway is set at route-table + // level, when nothing match and envoy returns 404, that transformation won't ge applied neither! + name: "match-none", + routeName: "match", + opts: []curl.Option{ + curl.WithBody("hello"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusNotFound, + Headers: map[string]any{ + // The Gateway attached transformation never apply when no route match + // "response-gateway": "goodbyte", + }, + NotHeaders: []string{ + "response-gateway", + "x-path-response", + "x-method-response", + "x-header-response", + "x-query-response", + "x-foo-response", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + // The Gateway attached transformation never apply when no route match + // "request-gateway": "hello", + }, + NotHeaders: []string{ + "request-gateway", + "x-path-request", + "x-method-request", + "x-header-request", + "x-foo-request", + "x-query-request", + }, + }, + }, + { + // test 10 + name: "custom functions", + routeName: "custom-functions", + opts: []curl.Option{ + curl.WithBody(`{"foo":"\"bar\""}`), + curl.WithHeader("x-nested-call", "my name is andy"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{ + "x-base64-encode": "YmFzZTY0IGVuY29kZSBpbiByZXNwb25zZSBoZWFkZXI=", + "x-base64-decode": "base64 decode in response header", + "x-base64-decode-invalid-non-empty": "foobar", + "x-substring": "response", + "x-substring2": "resp", + // when the len is invalid, we default to the end of the string + "x-substring-invalid2": "response", + "x-env": gomega.MatchRegexp(`default/gw-[a-f0-9]*-[a-z0-9]*`), + "x-replace-random": gomega.MatchRegexp(`.+ be or not .+ be`), + // replace_with_random creates a string longer then 4 characters, so if `andy` + // is not replaced in the x-nested-call request header, this will not match + "x-replace-nested": gomega.MatchRegexp(`my name is .....+`), + }, + NotHeaders: []string{ + // When decode fail, we return an empty string which in turn becomes a "remove" header ops + "x-base64-decode-invalid", + // when start is invalid, we return an empty string which in turn becomes a "remove" header ops + "x-substring-invalid", + "x-env-not-set", + }, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + "x-base64-encode": "YmFzZTY0IGVuY29kZSBpbiByZXF1ZXN0IGhlYWRlcg==", + "x-base64-decode": "base64 decode in request header", + "x-base64-decode-invalid-non-empty": "foobar", + "x-substring": "request", + "x-substring2": "req", + // when the len is invalid, we default to the end of the string + "x-substring-invalid2": "request", + "x-env": gomega.MatchRegexp(`default/gw-[a-f0-9]*-[a-z0-9]*`), + "x-replace-random": gomega.MatchRegexp(`.+ be or not .+ be`), + "content-length": "31", + }, + NotHeaders: []string{ + // When decode fail, we return an empty string which in turn becomes a "remove" header ops + "x-base64-decode-invalid", + // when start is invalid, we return an empty string which in turn becomes a "remove" header ops + "x-substring-invalid", + "x-env-not-set", + }, + Body: testmatchers.JSONContains([]byte(`{"Foo":"\"bar\""}`)), + }, + }, + { + // test 11 name: "pull json info", // shows we parse the body as json routeName: "route-for-body-json", opts: []curl.Option{ @@ -524,6 +557,9 @@ func (s *testingSuite) TestGatewayWithTransformedRoute() { "x-how-great": "level_super", "from-incoming": "key_level_myinnervalue", }, + // The test dump the headers field from the echo response into the top level of + // the body, so all the request headers would be at the top level of the json body + Body: testmatchers.JSONContains([]byte(`{"X-Incoming-Stuff":["super"],"X-Transformed-Incoming":["level_myinnervalue"]}`)), }, // Note: for this test, there is a response body transformation setup which extracts just the headers field // When we create the Request Object from the echo response, we accounted for that @@ -534,6 +570,7 @@ func (s *testingSuite) TestGatewayWithTransformedRoute() { }, }, { + // test 12 // The default for Body parsing is AsString which translate to body passthrough (no buffering in envoy) // For this test, the response header transformation is set to try to use the `headers` field in the response // json body, because the body is never parse, so `headers` is undefine and envoy returns 400 response @@ -546,11 +583,12 @@ func (s *testingSuite) TestGatewayWithTransformedRoute() { resp: &testmatchers.HttpResponse{ StatusCode: http.StatusBadRequest, // bad transformation results in 400 NotHeaders: []string{ - "x-how-great", + "x-what-method", }, }, }, { + // test 13 name: "dont pull json info if not json", // shows we parse the body as json routeName: "route-for-body-json", opts: []curl.Option{ @@ -560,7 +598,110 @@ func (s *testingSuite) TestGatewayWithTransformedRoute() { StatusCode: http.StatusBadRequest, // transformation should choke }, }, + { + // test 14 + name: "header to body with json parsing", + routeName: "route-for-header-to-body-json", + opts: []curl.Option{ + curl.WithBody(`[3,2,1]`), + curl.WithHeader("X-my-name", "andy"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusOK, + Headers: map[string]any{}, + }, + req: &testmatchers.HttpRequest{ + Headers: map[string]any{ + // The original value is "andy" but we use body modification to change the httpbin + // response to a random string which is longer than 5 characters. It will fail if + // the modification did not happen because "andy" is only 4 characters + "x-my-name": gomega.MatchRegexp(`.....+`), + }, + Body: fmt.Sprintf("321-%s", httpbin_echo_base_path), + }, + }, + { + // test 15 + name: "modify :method and :status header foo=bar", + routeName: "pseudo-headers", + opts: []curl.Option{ + curl.WithHeader("foo", "bar"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusCreated, + Headers: map[string]any{}, + }, + req: &testmatchers.HttpRequest{ + Method: "POST", + }, + }, + { + // test 16 + name: "modify :method and :status header foo=baz", + routeName: "pseudo-headers", + opts: []curl.Option{ + curl.WithHeader("foo", "baz"), + }, + resp: &testmatchers.HttpResponse{ + StatusCode: http.StatusAccepted, + Headers: map[string]any{}, + }, + req: &testmatchers.HttpRequest{ + Method: "POST", + }, + }, + } + + // If no indices are provided, return the full original slice. + if len(indices) == 0 { + return commonTestCases + } + + var selected []transformationTestCase + + for _, index := range indices { + if index < 0 { + index = len(commonTestCases) + index + } + + if index >= 0 && index < len(commonTestCases) { + selected = append(selected, commonTestCases[index]) + } else { + fmt.Printf("warning: Index %d out of bounds. Skipping.\n", index) + } + } + + return selected +} + +func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite { + return &testingSuite{ + base.NewBaseTestingSuite(ctx, testInst, setup, testCases), + // For local development only! + // Enter a list of indices to select specific tests, -1 means the last test. + // Default will return all common test cases. + // reviewers: please flag the PR if the argument is not empty! + selectCommonTestCases(), } +} + +func (s *testingSuite) SetupSuite() { + s.BaseTestingSuite.SetupSuite() + + s.assertSuiteResourceStatus() +} + +func (s *testingSuite) TestGatewayWithTransformedRoute() { + s.SetRustformationInController(false) + s.assertTestResourceStatus() + + s.TestInstallation.Assertions.AssertEnvoyAdminApi( + s.Ctx, + proxyObjectMeta, + s.dynamicModuleAssertion(false), + ) + + testCases := []transformationTestCase{} testCases = append(testCases, s.commonTestCases...) s.runTestCases((testCases)) } @@ -623,6 +764,7 @@ func (s *testingSuite) SetRustformationInController(enabled bool) { func (s *testingSuite) TestGatewayRustformationsWithTransformedRoute() { s.SetRustformationInController(true) + s.assertTestResourceStatus() testutils.Cleanup(s.T(), func() { s.SetRustformationInController(false) @@ -658,12 +800,13 @@ func (s *testingSuite) runTestCases(testCases []transformationTestCase) { curl.WithHost(kubeutils.ServiceFQDN(proxyObjectMeta)), curl.WithHostHeader(fmt.Sprintf("example-%s.com", tc.routeName)), curl.WithPort(8080), + curl.WithPath(httpbin_echo_base_path+tc.url), // This is the endpoint for httpbin to return the request in json ), tc.resp, 6, /* timeout */ 2 /* retry interval */) if resp.StatusCode == http.StatusOK { - req, err := helper.CreateRequestFromEchoResponse(resp.Body) + req, err := helper.CreateRequestFromHttpBinResponse(resp.Body) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(req).To(testmatchers.HaveHttpRequest(tc.req)) } else { @@ -673,21 +816,8 @@ func (s *testingSuite) runTestCases(testCases []transformationTestCase) { } } -func (s *testingSuite) assertStatus() { +func (s *testingSuite) assertRouteAndTrafficPolicyStatus(routesToCheck, trafficPoliciesToCheck []string) { currentTimeout, pollingInterval := helpers.GetTimeouts() - routesToCheck := []string{ - "example-route-for-headers", - "example-route-for-body-json", - "example-route-for-body-as-string", - "example-route-for-gateway-attached-transform", - } - trafficPoliciesToCheck := []string{ - "example-traffic-policy-for-headers", - "example-traffic-policy-for-body-json", - "example-traffic-policy-for-body-as-string", - "example-traffic-policy-for-gateway-attached-transform", - } - for i, routeName := range routesToCheck { trafficPolicyName := trafficPoliciesToCheck[i] @@ -720,7 +850,7 @@ func (s *testingSuite) assertStatus() { } actualPolicyStatus := tp.Status - g.Expect(actualPolicyStatus.Ancestors).To(gomega.HaveLen(1), "should have one ancestor") + g.Expect(actualPolicyStatus.Ancestors).To(gomega.HaveLen(1), "%s should have one ancestor", trafficPolicyName) ancestorStatus := actualPolicyStatus.Ancestors[0] cond := meta.FindStatusCondition(ancestorStatus.Conditions, expectedCond.Type) g.Expect(cond).NotTo(gomega.BeNil()) @@ -732,6 +862,48 @@ func (s *testingSuite) assertStatus() { } } +func (s *testingSuite) assertSuiteResourceStatus() { + routesToCheck := []string{ + "example-route-for-body-as-string", + // This route is apply right before that test as this is test specific. Cannot check at suite. + // "example-route-for-body-json", + "example-route-for-custom-functions", + "example-route-for-gateway-attached-transform", + "example-route-for-header-match", + "example-route-for-header-to-body-json", + "example-route-for-headers", + "example-route-for-method-match", + "example-route-for-path-match", + "example-route-for-pseudo-headers", + "example-route-for-query-match", + } + trafficPoliciesToCheck := []string{ + "example-traffic-policy-for-body-as-string", + // This policy is applied right before that test as this is test specific. Cannot check at suite. + // "example-traffic-policy-for-body-json", + "example-traffic-policy-for-custom-functions", + "example-traffic-policy-for-gateway-attached-transform", + "example-traffic-policy-for-header-match", + "example-traffic-policy-for-header-to-body-json", + "example-traffic-policy-for-headers", + "example-traffic-policy-for-method-match", + "example-traffic-policy-for-path-match", + "example-traffic-policy-for-pseudo-headers", + "example-traffic-policy-for-query-match", + } + s.assertRouteAndTrafficPolicyStatus(routesToCheck, trafficPoliciesToCheck) +} + +func (s *testingSuite) assertTestResourceStatus() { + routesToCheck := []string{ + "example-route-for-body-json", + } + trafficPoliciesToCheck := []string{ + "example-traffic-policy-for-body-json", + } + s.assertRouteAndTrafficPolicyStatus(routesToCheck, trafficPoliciesToCheck) +} + func (s *testingSuite) dynamicModuleAssertion(shouldBeLoaded bool) func(ctx context.Context, adminClient *envoyadmincli.Client) { return func(ctx context.Context, adminClient *envoyadmincli.Client) { s.TestInstallation.Assertions.Gomega.Eventually(func(g gomega.Gomega) { diff --git a/test/e2e/features/transformation/testdata/service.yaml b/test/e2e/features/transformation/testdata/service.yaml index 08fc4495616..ba1f18f3066 100644 --- a/test/e2e/features/transformation/testdata/service.yaml +++ b/test/e2e/features/transformation/testdata/service.yaml @@ -8,7 +8,7 @@ spec: ports: - name: http port: 8080 - targetPort: 3000 + targetPort: 8080 selector: app.kubernetes.io/name: backend-0 --- @@ -29,11 +29,11 @@ spec: version: v1 spec: containers: - - image: gcr.io/k8s-staging-gateway-api/echo-basic:v20231214-v1.0.0-140-gf544a46e + - image: ghcr.io/mccutchen/go-httpbin:2.19 imagePullPolicy: IfNotPresent name: backend-0 ports: - - containerPort: 3000 + - containerPort: 8080 env: - name: POD_NAME valueFrom: diff --git a/test/e2e/features/transformation/testdata/transform-for-body-as-string.yaml b/test/e2e/features/transformation/testdata/transform-for-body-as-string.yaml index db51efe5cef..027d84893ce 100644 --- a/test/e2e/features/transformation/testdata/transform-for-body-as-string.yaml +++ b/test/e2e/features/transformation/testdata/transform-for-body-as-string.yaml @@ -25,5 +25,5 @@ spec: transformation: response: set: - - name: "x-how-great" - value: "level_{%- if headers != \"\" -%}{{headers.X-Incoming-Stuff.0}}{% else %}unknown{% endif %}" \ No newline at end of file + - name: "x-what-method" + value: "method_{%- if method != \"\" -%}{{method}}{% else %}unknown{% endif %}" \ No newline at end of file diff --git a/test/e2e/features/transformation/testdata/transform-for-body-json-rust.yaml b/test/e2e/features/transformation/testdata/transform-for-body-json-rust.yaml new file mode 100644 index 00000000000..668edf8755e --- /dev/null +++ b/test/e2e/features/transformation/testdata/transform-for-body-json-rust.yaml @@ -0,0 +1,40 @@ + +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: example-route-for-body-json +spec: + parentRefs: + - name: gw + hostnames: + - "example-route-for-body-json.com" + rules: + - backendRefs: + - name: simple-svc + port: 8080 +--- +apiVersion: gateway.kgateway.dev/v1alpha1 +kind: TrafficPolicy +metadata: + name: example-traffic-policy-for-body-json +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: example-route-for-body-json + transformation: + request: + set: + - name: "x-transformed-incoming" + value: "level_{%- if mykey != \"\" -%}{{mykey.myinnerkey}}{% else %}unknown{% endif %}" + body: + parseAs: "AsJson" + response: + set: + - name: "x-how-great" + value: "level_{%- if headers != \"\" -%}{{headers[\"X-Incoming-Stuff\"][0]}}{% else %}unknown{% endif %}" + - name: "from-incoming" + value: "key_{%- if headers != \"\" -%}{{headers[\"X-Transformed-Incoming\"][0]}}{% else %}unknown{% endif %}" + body: + parseAs: "AsJson" + value: "{{headers}}" # use the parsed body of the returned format (pull from input headers as returned by echo service) \ No newline at end of file diff --git a/test/e2e/features/transformation/testdata/transform-for-custom-functions.yaml b/test/e2e/features/transformation/testdata/transform-for-custom-functions.yaml index 778b85ff16f..11e5b43709f 100644 --- a/test/e2e/features/transformation/testdata/transform-for-custom-functions.yaml +++ b/test/e2e/features/transformation/testdata/transform-for-custom-functions.yaml @@ -48,7 +48,7 @@ spec: value: "{{replace_with_random(\"to be or not to be\", \"to\")}}" body: parseAs: "AsJson" - value: "{\"Foo\":\"{{ raw_string(foo) }}\"}" + value: "{\"Foo\":\"{{ raw_string(foo) }}\", \"test\":\"123\"}" response: set: - name: x-base64-encode @@ -73,3 +73,5 @@ spec: value: "{{env(\"FOOBAR\")}}" - name: x-replace-random value: "{{replace_with_random(\"to be or not to be\", \"to\")}}" + - name: x-replace-nested + value: '{{ replace_with_random(request_header("x-nested-call"), "andy") }}' diff --git a/test/e2e/features/transformation/testdata/transform-for-header-to-body-json.yaml b/test/e2e/features/transformation/testdata/transform-for-header-to-body-json.yaml new file mode 100644 index 00000000000..b5998ba7e54 --- /dev/null +++ b/test/e2e/features/transformation/testdata/transform-for-header-to-body-json.yaml @@ -0,0 +1,37 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: example-route-for-header-to-body-json +spec: + parentRefs: + - name: gw + hostnames: + - "example-route-for-header-to-body-json.com" + rules: + - backendRefs: + - name: simple-svc + port: 8080 +--- +apiVersion: gateway.kgateway.dev/v1alpha1 +kind: TrafficPolicy +metadata: + name: example-traffic-policy-for-header-to-body-json +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: example-route-for-header-to-body-json + transformation: + request: + body: + parseAs: "AsJson" + # the request body is a json array, here we convert that into a string by pulling + # out all the elements and added a request header value + value: '{% for i in context() %}{{ i }}{% endfor %}-{{request_header(":path")}}' + response: + body: + parseAs: "AsString" + # Because the test relies on the json body from http-bin to re-construct the request, we cannot + # just replace the body. Here, we are replacing a known request header value put in the body by + # http-bin and then check for the request header is changed + value: '{{ replace_with_random(body(), "andy") }}' \ No newline at end of file diff --git a/test/e2e/features/transformation/testdata/transform-for-headers.yaml b/test/e2e/features/transformation/testdata/transform-for-headers.yaml index b9cac2be376..a6f77d54727 100644 --- a/test/e2e/features/transformation/testdata/transform-for-headers.yaml +++ b/test/e2e/features/transformation/testdata/transform-for-headers.yaml @@ -46,4 +46,6 @@ spec: - name: "x-space-test" value: " foobar " remove: - - "x-remove-me" +# - "x-remove-me" +# go-httpbin does not support custom response header, so remove one of the Access-Control header + - "access-control-allow-origin" diff --git a/test/e2e/features/transformation/testdata/transform-for-match-path.yaml b/test/e2e/features/transformation/testdata/transform-for-match-path.yaml index 7ba4cfe0012..21cc0d5d786 100644 --- a/test/e2e/features/transformation/testdata/transform-for-match-path.yaml +++ b/test/e2e/features/transformation/testdata/transform-for-match-path.yaml @@ -11,8 +11,8 @@ spec: rules: - matches: - path: - type: PathPrefix - value: /path_match/ + type: RegularExpression + value: ".*/path_match/.*" backendRefs: - name: simple-svc port: 8080 diff --git a/test/e2e/features/transformation/testdata/transform-for-pseudo-headers.yaml b/test/e2e/features/transformation/testdata/transform-for-pseudo-headers.yaml new file mode 100644 index 00000000000..d772b97db63 --- /dev/null +++ b/test/e2e/features/transformation/testdata/transform-for-pseudo-headers.yaml @@ -0,0 +1,33 @@ + +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: example-route-for-pseudo-headers +spec: + parentRefs: + - name: gw + hostnames: + - "example-pseudo-headers.com" + rules: + - backendRefs: + - name: simple-svc + port: 8080 +--- +apiVersion: gateway.kgateway.dev/v1alpha1 +kind: TrafficPolicy +metadata: + name: example-traffic-policy-for-pseudo-headers +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: example-route-for-pseudo-headers + transformation: + request: + set: + - name: :method + value: "POST" + response: + set: + - name: :status + value: '{% if request_header("foo") == "bar" %}201{% else %}202{% endif %}' diff --git a/test/e2e/tests/base/base_suite.go b/test/e2e/tests/base/base_suite.go index 5399309b2d8..825ab84b9dc 100644 --- a/test/e2e/tests/base/base_suite.go +++ b/test/e2e/tests/base/base_suite.go @@ -364,6 +364,7 @@ func (s *BaseTestingSuite) BeforeTest(suiteName, testName string) { return } + fmt.Printf("testName: %s applying %v\n", testName, testCase) s.ApplyManifests(testCase) } diff --git a/test/e2e/testutils/assertions/curl.go b/test/e2e/testutils/assertions/curl.go index 5281a04fb1e..9ba869e5ef5 100644 --- a/test/e2e/testutils/assertions/curl.go +++ b/test/e2e/testutils/assertions/curl.go @@ -3,9 +3,11 @@ package assertions import ( + "bytes" "context" "errors" "fmt" + "io" "net/http" "os/exec" "strings" @@ -46,6 +48,7 @@ func (p *Provider) AssertEventualCurlReturnResponse( currentTimeout, pollingInterval := helpers.GetTimeouts(timeout...) var curlHttpResponse *http.Response + var cachedBodyBytes []byte p.Gomega.Eventually(func(g Gomega) { curlResponse, err := p.clusterContext.Cli.CurlFromPod(ctx, podOpts, curlOptions...) fmt.Printf("want:\n%+v\nstdout:\n%s\nstderr:%s\n\n", expectedResponse, curlResponse.StdOut, curlResponse.StdErr) @@ -62,6 +65,16 @@ func (p *Provider) AssertEventualCurlReturnResponse( // Do the transform in a separate step instead of a WithTransform to avoid having to do it twice //nolint:bodyclose // The caller of this assertion should be responsible for ensuring the body close - if the response is not needed for the test, AssertEventualCurlResponse should be used instead curlHttpResponse = transforms.WithCurlResponse(curlResponse) + + // Read the body, clone and put it back into the response so the gomega matcher + // will still work but the return response object will also has the body + bodyBytes, err := io.ReadAll(curlHttpResponse.Body) + curlHttpResponse.Body.Close() + g.Expect(err).NotTo(HaveOccurred()) + cachedBodyBytes = make([]byte, len(bodyBytes)) + copy(cachedBodyBytes, bodyBytes) + curlHttpResponse.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) + g.Expect(curlHttpResponse).To(matchers.HaveHttpResponse(expectedResponse)) fmt.Printf("success: %+v", curlResponse) }). @@ -70,6 +83,10 @@ func (p *Provider) AssertEventualCurlReturnResponse( WithContext(ctx). Should(Succeed(), "failed to get expected response") + if len(cachedBodyBytes) > 0 { + curlHttpResponse.Body.Close() + curlHttpResponse.Body = io.NopCloser(bytes.NewBuffer(cachedBodyBytes)) + } return curlHttpResponse } diff --git a/test/e2e/testutils/helper/http_bin.go b/test/e2e/testutils/helper/http_bin.go new file mode 100644 index 00000000000..e190e7a055f --- /dev/null +++ b/test/e2e/testutils/helper/http_bin.go @@ -0,0 +1,86 @@ +package helper + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "maps" + "net/http" + "net/url" +) + +type httpbinResponse struct { + Path string `json:"url"` + Host string `json:"host"` + Method string `json:"method"` + Headers map[string][]string `json:"headers"` + Body string `json:"data"` + // other fields like namespace, ingress, service, pod ignored +} + +// ToHttpRequest reconstructs an http.Request from the httpbinResponse +// The httpbinResponse is from docker.io/mccutchen/go-httpbin +func (r *httpbinResponse) ToHttpRequest() (*http.Request, error) { + // Construct a URL (you may want to prepend scheme, default http://) + u := &url.URL{ + Scheme: "http", + Host: r.Host, + Path: r.Path, + } + + // Create a body if Content-Length > 0 (dummy body here) + var body io.ReadCloser + if cl, ok := r.Headers["Content-Length"]; ok && len(cl) > 0 && cl[0] != "0" { + body = io.NopCloser(bytes.NewBuffer([]byte(r.Body))) + } + + // Build request + req := &http.Request{ + Method: r.Method, + URL: u, + Host: r.Host, + Header: http.Header{}, + Body: body, + Proto: "http", // http-bin doesn't return the proto back + } + + // Add headers + for k, v := range r.Headers { + for _, val := range v { + fmt.Printf("%s: %s\n", k, val) + req.Header.Add(k, val) + } + } + + return req, nil +} + +func CreateRequestFromHttpBinResponse(r io.ReadCloser) (*http.Request, error) { + bytes, err := io.ReadAll(r) + if err != nil { + return nil, err + } + r.Close() + + var response httpbinResponse + if err := json.Unmarshal(bytes, &response); err != nil { + // Hack Alert: + // For a request, the headers will at least contain `:path` and `:method` and should + // never be empty. + // some transformation tests extract just the headers field from the original echo + // response and return that as the json body, so just try parse that as a map of key + // and value and put that into Headers + fmt.Printf("json bytes:\n%s\n", string(bytes)) + var m map[string][]string + if err := json.Unmarshal(bytes, &m); err != nil { + return nil, err + } + + if response.Headers == nil { + response.Headers = make(map[string][]string) + } + maps.Copy(response.Headers, m) + } + return response.ToHttpRequest() +} diff --git a/test/e2e/testutils/helper/http_echo.go b/test/e2e/testutils/helper/http_echo.go index 1e2e7daf882..386aae3df76 100644 --- a/test/e2e/testutils/helper/http_echo.go +++ b/test/e2e/testutils/helper/http_echo.go @@ -19,6 +19,7 @@ type echoResponse struct { } // ToHttpRequest reconstructs an http.Request from the EchoResponse +// The EchoResponse is from the gcr.io/k8s-staging-gateway-api/echo-basic func (er *echoResponse) ToHttpRequest() (*http.Request, error) { // Construct a URL (you may want to prepend scheme, default http://) u := &url.URL{ diff --git a/test/gomega/matchers/have_http_request.go b/test/gomega/matchers/have_http_request.go index c7fce135433..6d119420289 100644 --- a/test/gomega/matchers/have_http_request.go +++ b/test/gomega/matchers/have_http_request.go @@ -4,10 +4,13 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "github.com/onsi/gomega" + "github.com/onsi/gomega/format" "github.com/onsi/gomega/gstruct" + "github.com/onsi/gomega/matchers" "github.com/onsi/gomega/types" ) @@ -39,9 +42,7 @@ type HttpRequest struct { // Body is the expected request body for an http.Request // Body can be of type: {string, bytes, GomegaMatcher} // Optional: If not provided, defaults to an empty string - // TODO: currently, the http echo service we use in our test does not - // return the request body. So, this is not implemented yet and commented out - // Body interface{} + Body any // Headers is the set of expected header values for an http.Request // Each header can be of type: {string, GomegaMatcher} @@ -95,6 +96,11 @@ func HaveHttpRequest(expected *HttpRequest) types.GomegaMatcher { }) } partialRequestMatchers = append(partialRequestMatchers, expectedCustomMatcher) + if expected.Body != nil { + partialRequestMatchers = append(partialRequestMatchers, &HaveHTTPRequestBodyMatcher{ + Expected: expected.Body, + }) + } return &HaveHttpRequestMatcher{ Expected: expected, @@ -234,6 +240,95 @@ func (m *NotHaveHTTPRequestHeaderMatcher) NegatedFailureMessage(actual any) stri return fmt.Sprintf("Expected HTTP request to have header '%s', but it was not present", m.Header) } +type HaveHTTPRequestBodyMatcher struct { + Expected any + cachedRequest any + cachedBody []byte +} + +func (matcher *HaveHTTPRequestBodyMatcher) Match(actual any) (bool, error) { + body, err := matcher.body(actual) + if err != nil { + return false, err + } + + switch e := matcher.Expected.(type) { + case string: + return (&matchers.EqualMatcher{Expected: e}).Match(string(body)) + case []byte: + return (&matchers.EqualMatcher{Expected: e}).Match(body) + case types.GomegaMatcher: + return e.Match(body) + default: + return false, fmt.Errorf("HaveHTTPBody matcher expects string, []byte, or GomegaMatcher. Got:\n%s", format.Object(matcher.Expected, 1)) + } +} + +func (matcher *HaveHTTPRequestBodyMatcher) FailureMessage(actual any) (message string) { + body, err := matcher.body(actual) + if err != nil { + return fmt.Sprintf("failed to read body: %s", err) + } + + switch e := matcher.Expected.(type) { + case string: + return (&matchers.EqualMatcher{Expected: e}).FailureMessage(string(body)) + case []byte: + return (&matchers.EqualMatcher{Expected: e}).FailureMessage(body) + case types.GomegaMatcher: + return e.FailureMessage(body) + default: + return fmt.Sprintf("HaveHTTPBody matcher expects string, []byte, or GomegaMatcher. Got:\n%s", format.Object(matcher.Expected, 1)) + } +} + +func (matcher *HaveHTTPRequestBodyMatcher) NegatedFailureMessage(actual any) (message string) { + body, err := matcher.body(actual) + if err != nil { + return fmt.Sprintf("failed to read body: %s", err) + } + + switch e := matcher.Expected.(type) { + case string: + return (&matchers.EqualMatcher{Expected: e}).NegatedFailureMessage(string(body)) + case []byte: + return (&matchers.EqualMatcher{Expected: e}).NegatedFailureMessage(body) + case types.GomegaMatcher: + return e.NegatedFailureMessage(body) + default: + return fmt.Sprintf("HaveHTTPBody matcher expects string, []byte, or GomegaMatcher. Got:\n%s", format.Object(matcher.Expected, 1)) + } +} + +// body returns the body. It is cached because once we read it in Match() +// the Reader is closed and it is not readable again in FailureMessage() +// or NegatedFailureMessage() +func (matcher *HaveHTTPRequestBodyMatcher) body(actual any) ([]byte, error) { + if matcher.cachedRequest == actual && matcher.cachedBody != nil { + return matcher.cachedBody, nil + } + + body := func(a *http.Request) ([]byte, error) { + if a.Body != nil { + defer a.Body.Close() + var err error + matcher.cachedBody, err = io.ReadAll(a.Body) + if err != nil { + return nil, fmt.Errorf("error reading request body: %w", err) + } + } + return matcher.cachedBody, nil + } + + switch a := actual.(type) { + case *http.Request: + matcher.cachedRequest = a + return body(a) + default: + return nil, fmt.Errorf("HaveHTTPRequestBody matcher expects *http.Request. Got:\n%s", format.Object(actual, 1)) + } +} + // informativeRequestComparison returns a string which presents data to the user to help them understand why a failure occurred. // The HaveHttpRequestMatcher uses an And matcher, which intentionally short-circuits and only // logs the first failure that occurred.