Merge pull request #78 from Kuadrant/service-from-config

Setup services from configuration
Kuadrant · Aug 29, 2024 · ed8eec1 · ed8eec1
2 parents 870cafd + a78e962
commit ed8eec1
Show file tree

Hide file tree

Showing 6 changed files with 202 additions and 104 deletions.
diff --git a/src/configuration.rs b/src/configuration.rs
@@ -1,5 +1,7 @@
 use std::cell::OnceCell;
+use std::collections::HashMap;
 use std::fmt::{Debug, Display, Formatter};
+use std::rc::Rc;
 use std::sync::Arc;
 
 use cel_interpreter::objects::ValueType;
@@ -10,6 +12,7 @@ use serde::Deserialize;
 use crate::attribute::Attribute;
 use crate::policy::Policy;
 use crate::policy_index::PolicyIndex;
+use crate::service::GrpcService;
 
 #[derive(Deserialize, Debug, Clone)]
 pub struct SelectorItem {
@@ -439,15 +442,14 @@ pub fn type_of(path: &str) -> Option<ValueType> {
 
 pub struct FilterConfig {
     pub index: PolicyIndex,
-    // Deny/Allow request when faced with an irrecoverable failure.
-    pub failure_mode: FailureMode,
+    pub services: Rc<HashMap<String, Rc<GrpcService>>>,
 }
 
 impl Default for FilterConfig {
     fn default() -> Self {
         Self {
             index: PolicyIndex::new(),
-            failure_mode: FailureMode::Deny,
+            services: Rc::new(HashMap::new()),
         }
     }
 }
@@ -480,16 +482,33 @@ impl TryFrom<PluginConfiguration> for FilterConfig {
             }
         }
 
+        // configure grpc services from the extensions in config
+        let services = config
+            .extensions
+            .into_iter()
+            .map(|(name, ext)| {
+                (
+                    name,
+                    Rc::new(GrpcService::new(
+                        ext.extension_type,
+                        ext.endpoint,
+                        ext.failure_mode,
+                    )),
+                )
+            })
+            .collect();
+
         Ok(Self {
             index,
-            failure_mode: config.failure_mode,
+            services: Rc::new(services),
         })
     }
 }
 
-#[derive(Deserialize, Debug, Clone)]
+#[derive(Deserialize, Debug, Clone, Default)]
 #[serde(rename_all = "lowercase")]
 pub enum FailureMode {
+    #[default]
     Deny,
     Allow,
 }
@@ -504,8 +523,16 @@ pub enum ExtensionType {
 #[derive(Deserialize, Debug, Clone)]
 #[serde(rename_all = "camelCase")]
 pub struct PluginConfiguration {
-    #[serde(rename = "rateLimitPolicies")]
+    pub extensions: HashMap<String, Extension>,
     pub policies: Vec<Policy>,
+}
+
+#[derive(Deserialize, Debug, Clone)]
+#[serde(rename_all = "camelCase")]
+pub struct Extension {
+    #[serde(rename = "type")]
+    pub extension_type: ExtensionType,
+    pub endpoint: String,
     // Deny/Allow request when faced with an irrecoverable failure.
     pub failure_mode: FailureMode,
 }
@@ -515,12 +542,17 @@ mod test {
     use super::*;
 
     const CONFIG: &str = r#"{
-        "failureMode": "deny",
-        "rateLimitPolicies": [
+        "extensions": {
+            "limitador": {
+                "type": "ratelimit",
+                "endpoint": "limitador-cluster",
+                "failureMode": "deny"
+            }
+        },
+        "policies": [
         {
             "name": "rlp-ns-A/rlp-name-A",
             "domain": "rlp-ns-A/rlp-name-A",
-            "service": "limitador-cluster",
             "hostnames": ["*.toystore.com", "example.com"],
             "rules": [
             {
@@ -612,8 +644,8 @@ mod test {
     #[test]
     fn parse_config_min() {
         let config = r#"{
-            "failureMode": "deny",
-            "rateLimitPolicies": []
+            "extensions": {},
+            "policies": []
         }"#;
         let res = serde_json::from_str::<PluginConfiguration>(config);
         if let Err(ref e) = res {
@@ -628,12 +660,17 @@ mod test {
     #[test]
     fn parse_config_data_selector() {
         let config = r#"{
-            "failureMode": "deny",
-            "rateLimitPolicies": [
+            "extensions": {
+                "limitador": {
+                    "type": "ratelimit",
+                    "endpoint": "limitador-cluster",
+                    "failureMode": "deny"
+                }
+            },
+            "policies": [
             {
                 "name": "rlp-ns-A/rlp-name-A",
                 "domain": "rlp-ns-A/rlp-name-A",
-                "service": "limitador-cluster",
                 "hostnames": ["*.toystore.com", "example.com"],
                 "rules": [
                 {
@@ -678,12 +715,17 @@ mod test {
     #[test]
     fn parse_config_condition_selector_operators() {
         let config = r#"{
-            "failureMode": "deny",
-            "rateLimitPolicies": [
+            "extensions": {
+                "limitador": {
+                    "type": "ratelimit",
+                    "endpoint": "limitador-cluster",
+                    "failureMode": "deny"
+                }
+            },
+            "policies": [
             {
                 "name": "rlp-ns-A/rlp-name-A",
                 "domain": "rlp-ns-A/rlp-name-A",
-                "service": "limitador-cluster",
                 "hostnames": ["*.toystore.com", "example.com"],
                 "rules": [
                 {
@@ -757,12 +799,17 @@ mod test {
     #[test]
     fn parse_config_conditions_optional() {
         let config = r#"{
-            "failureMode": "deny",
-            "rateLimitPolicies": [
+            "extensions": {
+                "limitador": {
+                    "type": "ratelimit",
+                    "endpoint": "limitador-cluster",
+                    "failureMode": "deny"
+                }
+            },
+            "policies": [
             {
                 "name": "rlp-ns-A/rlp-name-A",
                 "domain": "rlp-ns-A/rlp-name-A",
-                "service": "limitador-cluster",
                 "hostnames": ["*.toystore.com", "example.com"],
                 "rules": [
                 {
@@ -801,12 +848,17 @@ mod test {
     fn parse_config_invalid_data() {
         // data item fields are mutually exclusive
         let bad_config = r#"{
-        "failureMode": "deny",
-        "rateLimitPolicies": [
+        "extensions": {
+            "limitador": {
+                "type": "ratelimit",
+                "endpoint": "limitador-cluster",
+                "failureMode": "deny"
+            }
+        },
+        "policies": [
         {
             "name": "rlp-ns-A/rlp-name-A",
             "domain": "rlp-ns-A/rlp-name-A",
-            "service": "limitador-cluster",
             "hostnames": ["*.toystore.com", "example.com"],
             "rules": [
             {
@@ -828,8 +880,14 @@ mod test {
 
         // data item unknown fields are forbidden
         let bad_config = r#"{
-        "failureMode": "deny",
-        "rateLimitPolicies": [
+        "extensions": {
+            "limitador": {
+                "type": "ratelimit",
+                "endpoint": "limitador-cluster",
+                "failureMode": "deny"
+            }
+        },
+        "policies": [
         {
             "name": "rlp-ns-A/rlp-name-A",
             "domain": "rlp-ns-A/rlp-name-A",
@@ -852,12 +910,17 @@ mod test {
 
         // condition selector operator unknown
         let bad_config = r#"{
-            "failureMode": "deny",
-            "rateLimitPolicies": [
+            "extensions": {
+                "limitador": {
+                    "type": "ratelimit",
+                    "endpoint": "limitador-cluster",
+                    "failureMode": "deny"
+                }
+            },
+            "policies": [
             {
                 "name": "rlp-ns-A/rlp-name-A",
                 "domain": "rlp-ns-A/rlp-name-A",
-                "service": "limitador-cluster",
                 "hostnames": ["*.toystore.com", "example.com"],
                 "rules": [
                 {

diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs
@@ -1,4 +1,4 @@
-use crate::configuration::{ExtensionType, FailureMode, FilterConfig};
+use crate::configuration::{FailureMode, FilterConfig};
 use crate::envoy::{RateLimitResponse, RateLimitResponse_Code};
 use crate::policy::Policy;
 use crate::service::rate_limit::RateLimitService;
@@ -40,14 +40,19 @@ impl Filter {
             return Action::Continue;
         }
 
-        let rls = GrpcServiceHandler::new(
-            ExtensionType::RateLimit,
-            rlp.service.clone(),
-            Rc::clone(&self.header_resolver),
-        );
+        // todo(adam-cattermole): For now we just get the first GrpcService but we expect to have
+        // an action which links to the service that should be used
+        let rls = self
+            .config
+            .services
+            .values()
+            .next()
+            .expect("expect a value");
+
+        let handler = GrpcServiceHandler::new(Rc::clone(rls), Rc::clone(&self.header_resolver));
         let message = RateLimitService::message(rlp.domain.clone(), descriptors);
 
-        match rls.send(message) {
+        match handler.send(message) {
             Ok(call_id) => {
                 debug!(
                     "#{} initiated gRPC call (id# {}) to Limitador",
@@ -57,7 +62,7 @@ impl Filter {
             }
             Err(e) => {
                 warn!("gRPC call to Limitador failed! {e:?}");
-                if let FailureMode::Deny = self.config.failure_mode {
+                if let FailureMode::Deny = rls.failure_mode() {
                     self.send_http_response(500, vec![], Some(b"Internal Server Error.\n"))
                 }
                 Action::Continue
@@ -66,7 +71,15 @@ impl Filter {
     }
 
     fn handle_error_on_grpc_response(&self) {
-        match &self.config.failure_mode {
+        // todo(adam-cattermole): We need a method of knowing which service is the one currently
+        // being used (the current action) so that we can get the failure mode
+        let rls = self
+            .config
+            .services
+            .values()
+            .next()
+            .expect("expect a value");
+        match rls.failure_mode() {
             FailureMode::Deny => {
                 self.send_http_response(500, vec![], Some(b"Internal Server Error.\n"))
             }

diff --git a/src/policy.rs b/src/policy.rs
@@ -26,24 +26,16 @@ pub struct Rule {
 pub struct Policy {
     pub name: String,
     pub domain: String,
-    pub service: String,
     pub hostnames: Vec<String>,
     pub rules: Vec<Rule>,
 }
 
 impl Policy {
     #[cfg(test)]
-    pub fn new(
-        name: String,
-        domain: String,
-        service: String,
-        hostnames: Vec<String>,
-        rules: Vec<Rule>,
-    ) -> Self {
+    pub fn new(name: String, domain: String, hostnames: Vec<String>, rules: Vec<Rule>) -> Self {
         Policy {
             name,
             domain,
-            service,
             hostnames,
             rules,
         }

diff --git a/src/policy_index.rs b/src/policy_index.rs
@@ -41,13 +41,7 @@ mod tests {
     use crate::policy_index::PolicyIndex;
 
     fn build_ratelimit_policy(name: &str) -> Policy {
-        Policy::new(
-            name.to_owned(),
-            "".to_owned(),
-            "".to_owned(),
-            Vec::new(),
-            Vec::new(),
-        )
+        Policy::new(name.to_owned(), "".to_owned(), Vec::new(), Vec::new())
     }
 
     #[test]