From 04e5df2e889746e05c8493feb80f37204d110b29 Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Fri, 30 Dec 2022 13:47:05 +0100 Subject: [PATCH 1/8] Implementing module usage in policies --- charts/bridgekeeper/templates/crds.yaml | 206 ++++++++++++++---------- src/audit.rs | 47 +++++- src/crd.rs | 15 ++ src/evaluator.rs | 56 +++++-- src/events.rs | 100 +++++++----- src/helper/gencrd.rs | 32 +++- src/main.rs | 1 + src/manager.rs | 105 ++++++++++-- src/module.rs | 98 +++++++++++ src/policy.rs | 39 ++--- src/server.rs | 13 +- src/util/mod.rs | 2 + src/util/traits.rs | 6 + src/util/types.rs | 21 +++ 14 files changed, 548 insertions(+), 193 deletions(-) create mode 100644 src/module.rs create mode 100644 src/util/traits.rs create mode 100644 src/util/types.rs diff --git a/charts/bridgekeeper/templates/crds.yaml b/charts/bridgekeeper/templates/crds.yaml index 0e54ce5..a38bc00 100644 --- a/charts/bridgekeeper/templates/crds.yaml +++ b/charts/bridgekeeper/templates/crds.yaml @@ -1,5 +1,4 @@ {{- if .Values.installCRDs }} ---- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: @@ -14,88 +13,129 @@ spec: singular: policy scope: Cluster versions: - - additionalPrinterColumns: [] - name: v1alpha1 - schema: - openAPIV3Schema: - description: "Auto-generated derived type for PolicySpec via `CustomResource`" - properties: - spec: - properties: - audit: - nullable: true - type: boolean - enforce: - nullable: true - type: boolean - rule: - properties: - python: + - additionalPrinterColumns: [] + name: v1alpha1 + schema: + openAPIV3Schema: + description: Auto-generated derived type for PolicySpec via `CustomResource` + properties: + spec: + properties: + audit: + nullable: true + type: boolean + enforce: + nullable: true + type: boolean + modules: + items: + type: string + nullable: true + type: array + rule: + properties: + python: + type: string + required: + - python + type: object + target: + properties: + excludedNamespaces: + items: type: string - required: - - python - type: object - target: - properties: - excludedNamespaces: - items: - type: string - nullable: true - type: array - matches: - items: - properties: - apiGroup: - type: string - kind: - type: string - required: - - apiGroup - - kind - type: object - type: array - namespaces: - items: - type: string - nullable: true - type: array - required: - - matches - type: object - required: - - rule - - target - type: object - status: - nullable: true - properties: - audit: - nullable: true - properties: - timestamp: - nullable: true + nullable: true + type: array + matches: + items: + properties: + apiGroup: + type: string + kind: + type: string + required: + - apiGroup + - kind + type: object + type: array + namespaces: + items: type: string - violations: - items: - properties: - identifier: - type: string - message: - type: string - required: - - identifier - - message - type: object - nullable: true - type: array - type: object - type: object - required: - - spec - title: Policy - type: object - served: true - storage: true - subresources: - status: {} + nullable: true + type: array + required: + - matches + type: object + required: + - rule + - target + type: object + status: + nullable: true + properties: + audit: + nullable: true + properties: + timestamp: + nullable: true + type: string + violations: + items: + properties: + identifier: + type: string + message: + type: string + required: + - identifier + - message + type: object + nullable: true + type: array + type: object + type: object + required: + - spec + title: Policy + type: object + served: true + storage: true + subresources: + status: {} +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: modules.bridgekeeper.maibornwolff.de +spec: + group: bridgekeeper.maibornwolff.de + names: + categories: [] + kind: Module + plural: modules + shortNames: [] + singular: module + scope: Cluster + versions: + - additionalPrinterColumns: [] + name: v1alpha1 + schema: + openAPIV3Schema: + description: Auto-generated derived type for ModuleSpec via `CustomResource` + properties: + spec: + properties: + python: + type: string + required: + - python + type: object + required: + - spec + title: Module + type: object + served: true + storage: true + subresources: {} +--- {{- end }} diff --git a/src/audit.rs b/src/audit.rs index d0b509b..65985a5 100644 --- a/src/audit.rs +++ b/src/audit.rs @@ -1,6 +1,7 @@ use crate::crd::{Policy, PolicyStatus, Violation}; use crate::events::init_event_watcher; use crate::manager::Manager; +use crate::module::{ModuleStore, ModuleStoreRef, ModuleInfo}; use crate::policy::{load_policies_from_file, PolicyInfo, PolicyStore, PolicyStoreRef}; use crate::util::error::{kube_err, load_err, BridgekeeperError, Result}; use crate::util::k8s_client::{list_with_retry, patch_status_with_retry}; @@ -19,6 +20,7 @@ use prometheus::{ register_counter, register_gauge, register_gauge_vec, Counter, Encoder, Gauge, GaugeVec, }; use serde_json::json; +use std::collections::HashMap; use std::time::SystemTime; use tokio::task; use tokio::time::{sleep, Duration}; @@ -77,6 +79,7 @@ pub struct Args { pub struct Auditor { k8s_client: Client, policies: PolicyStoreRef, + modules: ModuleStoreRef, //event_sender: EventSender, } @@ -84,12 +87,14 @@ impl Auditor { pub fn new( client: Client, policies: PolicyStoreRef, + modules: ModuleStoreRef, //event_sender: EventSender, ) -> Auditor { pyo3::prepare_freethreaded_python(); Auditor { k8s_client: client, policies, + modules, //event_sender, } } @@ -101,7 +106,8 @@ impl Auditor { all: bool, ) -> Result<()> { let mut policies = Vec::new(); - // While holding the lock only collect the policies, directly auditing them would make the future of the method not implement Send which breaks the task spawn + let mut modules = HashMap::new(); + // While holding the lock only collect the policies or modules, directly auditing them would make the future of the method not implement Send which breaks the task spawn { let policy_store = self.policies.lock().expect("lock failed. Cannot continue"); for policy in policy_store.policies.values() { @@ -110,9 +116,15 @@ impl Auditor { } } } + + { + let module_store = self.modules.lock().expect("lock failed. Cannot continue"); + modules.extend(module_store.modules.clone()); + } + for policy in policies.iter() { if let Err(err) = self - .audit_policy(policy, print_violations, update_status) + .audit_policy(policy, &modules, print_violations, update_status) .await { return Err(err); @@ -128,10 +140,28 @@ impl Auditor { async fn audit_policy( &self, policy: &PolicyInfo, + modules: &HashMap, print_violations: bool, update_status: bool, ) -> Result<()> { println!("Auditing policy {}", policy.name); + + let mut module_code = String::new(); + + if let Some(used_modules) = &policy.policy.modules { + for module_name in used_modules.iter() { + match modules.get(module_name) { + Some(module_info) => { + module_code.push_str(&module_info.module.python); + module_code.push_str("\n"); + }, + None => { + log::warn!("Could not find module '{}'", module_name) + } + }; + } + } + let (valid, reason) = crate::evaluator::validate_policy(&policy.name, &policy.policy); if !valid { println!( @@ -416,6 +446,7 @@ pub async fn run(args: Args) { .await .expect("Fail early if kube client cannot be created"); let policies = PolicyStore::new(); + let modules = ModuleStore::new(); let event_sender = init_event_watcher(&client); // Load policies either from kubernetes or from file if !args.file.is_empty() { @@ -423,14 +454,18 @@ pub async fn run(args: Args) { load_policies_from_file(policies.clone(), filename).expect("failed to load policy"); } } else { - let mut manager = Manager::new(client.clone(), policies.clone(), event_sender.clone()); + let mut manager = Manager::new(client.clone(), policies.clone(), modules.clone(), event_sender.clone()); manager .load_existing_policies() .await .expect("Could not load existing policies"); + manager + .load_existing_modules() + .await + .expect("Could not load existing policies"); } // Run audit - let auditor = Auditor::new(client, policies); + let auditor = Auditor::new(client, policies, modules); match auditor .audit_policies(!args.silent, args.status, args.all) .await @@ -447,9 +482,9 @@ pub async fn run(args: Args) { push_metrics(metric_families).await; } -pub async fn launch_loop(client: kube::Client, policies: PolicyStoreRef, interval: u32) { +pub async fn launch_loop(client: kube::Client, policies: PolicyStoreRef, modules: ModuleStoreRef, interval: u32) { task::spawn(async move { - let auditor = Auditor::new(client, policies); + let auditor = Auditor::new(client, policies, modules); loop { sleep(Duration::from_secs(interval as u64)).await; log::info!("Starting audit run"); diff --git a/src/crd.rs b/src/crd.rs index 9b1ae9b..b8d9f3e 100644 --- a/src/crd.rs +++ b/src/crd.rs @@ -16,6 +16,19 @@ pub struct PolicySpec { pub rule: Rule, pub audit: Option, pub enforce: Option, + pub modules: Option>, +} + +#[derive( + CustomResource, Serialize, Deserialize, Debug, Default, Clone, Hash, PartialEq, Eq, JsonSchema, +)] +#[kube( + group = "bridgekeeper.maibornwolff.de", + version = "v1alpha1", + kind = "Module" +)] +pub struct ModuleSpec { + pub python: String, } #[derive(Serialize, Deserialize, Debug, Default, Clone, Hash, PartialEq, Eq, JsonSchema)] @@ -105,6 +118,7 @@ impl PolicySpec { enforce: Some(true), target, rule: Default::default(), + modules: None, } } @@ -114,6 +128,7 @@ impl PolicySpec { enforce: Some(true), target: Default::default(), rule: Rule { python }, + modules: None, } } } diff --git a/src/evaluator.rs b/src/evaluator.rs index b742e78..4bb206d 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -1,7 +1,8 @@ use crate::{ crd::{Policy, PolicySpec}, - events::{EventSender, PolicyEvent, PolicyEventData}, + events::{EventSender, Event, PolicyEventData, EventType}, policy::{PolicyInfo, PolicyStoreRef}, + module::{ModuleStoreRef}, }; use kube::core::{ admission::{self, Operation}, @@ -69,6 +70,7 @@ impl ValidationRequest { pub struct PolicyEvaluator { policies: PolicyStoreRef, + modules: ModuleStoreRef, event_sender: EventSender, } @@ -82,9 +84,10 @@ pub struct EvaluationResult { pub type PolicyEvaluatorRef = Arc; impl PolicyEvaluator { - pub fn new(policies: PolicyStoreRef, event_sender: EventSender) -> PolicyEvaluatorRef { + pub fn new(policies: PolicyStoreRef, modules: ModuleStoreRef, event_sender: EventSender) -> PolicyEvaluatorRef { let evaluator = PolicyEvaluator { policies, + modules, event_sender, }; pyo3::prepare_freethreaded_python(); @@ -111,6 +114,8 @@ impl PolicyEvaluator { } }; let policies = self.policies.lock().expect("lock failed. Cannot continue"); + let modules = self.modules.lock().expect("lock failed. Cannot continue"); + let mut matching_policies = Vec::new(); @@ -144,7 +149,24 @@ impl PolicyEvaluator { namespace.clone().unwrap_or_else(|| "-".to_string()), name ); - let res = evaluate_policy(value, &request); + + let mut module_code = String::new(); + + if let Some(used_modules) = &value.policy.modules { + for module_name in used_modules.iter() { + match modules.modules.get(module_name) { + Some(module_info) => { + module_code.push_str(&module_info.module.python); + module_code.push_str("\n"); + }, + None => { + log::warn!("Could not find module '{}'", module_name) + } + }; + } + } + + let res = evaluate_policy(value, &request, &module_code); if let Some(mut patch) = res.2 { if let Some(patches) = patches.as_mut() { patches.0.append(&mut patch.0); @@ -153,13 +175,15 @@ impl PolicyEvaluator { } } self.event_sender - .send(PolicyEvent { - policy_reference: value.ref_info.clone(), - event_data: PolicyEventData::Evaluated { + .send(Event { + object_reference: value.ref_info.clone(), + event_data: EventType::Policy( + PolicyEventData::Evaluated { target_identifier, result: res.0, reason: res.1.clone(), - }, + } + ), }) .unwrap_or_else(|err| log::warn!("Could not send event: {:?}", err)); if res.0 { @@ -232,6 +256,7 @@ pub fn validate_policy(name: &str, policy: &PolicySpec) -> (bool, Option fn evaluate_policy( policy: &PolicyInfo, request: &ValidationRequest, + module_code: &str, ) -> (bool, Option, Option) { let name = &policy.name; Python::with_gil(|py| { @@ -240,7 +265,10 @@ fn evaluate_policy( Err(err) => return fail(name, &format!("Failed to initialize python: {}", err)), }; - match PyModule::from_code(py, &policy.policy.rule.python, "rule.py", "bridgekeeper") { + let mut full_code = String::from(module_code); + full_code.push_str(&policy.policy.rule.python); + + match PyModule::from_code(py, &full_code, "rule.py", "bridgekeeper") { Ok(rule_code) => { if let Ok(validation_function) = rule_code.getattr("validate") { match validation_function.call1((obj,)) { @@ -267,7 +295,7 @@ pub fn evaluate_policy_audit( object, operation: Operation::Update, }; - evaluate_policy(policy, &request) + evaluate_policy(policy, &request, "") // TODO: load module_code } fn extract_result( @@ -338,7 +366,7 @@ def validate(request): operation: Operation::Create, }; - let (res, reason, patch) = evaluate_policy(&policy, &request); + let (res, reason, patch) = evaluate_policy(&policy, &request, ""); assert!(res, "validate function failed: {}", reason.unwrap()); assert!(reason.is_none()); assert!(patch.is_none()); @@ -364,7 +392,7 @@ def validate(request): operation: Operation::Create, }; - let (res, reason, patch) = evaluate_policy(&policy, &request); + let (res, reason, patch) = evaluate_policy(&policy, &request, ""); assert!(!res); assert!(reason.is_some()); assert_eq!("foobar".to_string(), reason.unwrap()); @@ -391,7 +419,7 @@ def validate(request): operation: Operation::Create, }; - let (res, reason, patch) = evaluate_policy(&policy, &request); + let (res, reason, patch) = evaluate_policy(&policy, &request, ""); assert!(!res); assert!(reason.is_some()); assert_eq!( @@ -424,7 +452,7 @@ def validate(request): operation: Operation::Create, }; - let (res, reason, patch) = evaluate_policy(&policy, &request); + let (res, reason, patch) = evaluate_policy(&policy, &request, ""); assert!(res, "validate function failed: {}", reason.unwrap()); assert!(reason.is_none()); assert!(patch.is_some()); @@ -438,4 +466,6 @@ def validate(request): serde_json::to_value(patch.0).unwrap() ); } + + // TODO: Test modules } diff --git a/src/events.rs b/src/events.rs index be789b4..2ff9244 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1,4 +1,3 @@ -use crate::policy::PolicyObjectReference; use k8s_openapi::api::core::v1::{Event as KubeEvent, EventSource as KubeEventSource}; use k8s_openapi::apimachinery::pkg::apis::meta::v1::Time; use k8s_openapi::chrono::offset::Utc; @@ -9,13 +8,9 @@ use kube::{ use tokio::sync::mpsc::{self, UnboundedSender}; use tokio::task; -pub type EventSender = UnboundedSender; +use crate::util::types::ObjectReference; -#[derive(Debug)] -pub struct PolicyEvent { - pub policy_reference: PolicyObjectReference, - pub event_data: PolicyEventData, -} +pub type EventSender = UnboundedSender; #[derive(Debug, PartialEq, Eq)] pub enum PolicyEventData { @@ -27,50 +22,81 @@ pub enum PolicyEventData { }, } +#[derive(Debug, PartialEq, Eq)] +pub enum ModuleEventData { + Loaded, +} + +#[derive(Debug)] +pub enum EventType { + Policy(PolicyEventData), + Module(ModuleEventData), +} + +#[derive(Debug)] +pub struct Event { + pub object_reference: ObjectReference, + pub event_data: EventType, +} + pub fn init_event_watcher(client: &Client) -> EventSender { - let (sender, mut receiver) = mpsc::unbounded_channel::(); + let (sender, mut receiver) = mpsc::unbounded_channel::(); let events_api: Api = Api::namespaced(client.clone(), "default"); task::spawn(async move { let instance = std::env::var("POD_NAME").unwrap_or_else(|_| "dev".to_string()); while let Some(event) = receiver.recv().await { let mut kube_event = KubeEvent::default(); - kube_event.metadata.generate_name = event.policy_reference.name.clone(); - kube_event.involved_object = event.policy_reference.to_object_reference(); + + kube_event.metadata.generate_name = event.object_reference.name.clone(); + kube_event.involved_object = event.object_reference.to_object_reference(); kube_event.type_ = Some("Normal".to_string()); kube_event.first_timestamp = Some(Time(Utc::now())); kube_event.source = Some(KubeEventSource { component: Some(format!("bridgekeeper/{}", instance)), host: None, }); + match event.event_data { - PolicyEventData::Loaded => { - kube_event.reason = Some("Loaded".to_string()); - kube_event.message = Some("Policy loaded by bridgekeeper".to_string()); + EventType::Policy(event_data) => { + match event_data { + PolicyEventData::Loaded => { + kube_event.reason = Some("Loaded".to_string()); + kube_event.message = Some("Policy loaded by bridgekeeper".to_string()); + } + PolicyEventData::Evaluated { + target_identifier, + result, + reason, + } => { + kube_event.reason = Some("Evaluated".to_string()); + kube_event.message = Some(format!( + "Target: {}, Result: {}, Reason: {}", + target_identifier, + result, + reason.unwrap_or_else(|| "-".to_string()) + )); + } + } + if let Err(err) = events_api.create(&PostParams::default(), &kube_event).await { + log::warn!( + "Could not create event for policy {}. Reason: {}", + event + .object_reference + .name + .clone() + .unwrap_or_else(|| "-".to_string()), + err + ); + } + }, + EventType::Module(event_data) => { + match event_data { + ModuleEventData::Loaded => { + kube_event.reason = Some("Loaded".to_string()); + kube_event.message = Some("Module loaded by bridgekeeper".to_string()); + } + } } - PolicyEventData::Evaluated { - target_identifier, - result, - reason, - } => { - kube_event.reason = Some("Evaluated".to_string()); - kube_event.message = Some(format!( - "Target: {}, Result: {}, Reason: {}", - target_identifier, - result, - reason.unwrap_or_else(|| "-".to_string()) - )); - } - } - if let Err(err) = events_api.create(&PostParams::default(), &kube_event).await { - log::warn!( - "Could not create event for policy {}. Reason: {}", - event - .policy_reference - .name - .clone() - .unwrap_or_else(|| "-".to_string()), - err - ); } } }); diff --git a/src/helper/gencrd.rs b/src/helper/gencrd.rs index 2a3b27b..10cc357 100644 --- a/src/helper/gencrd.rs +++ b/src/helper/gencrd.rs @@ -1,7 +1,8 @@ -use crate::{constants::CRD_FILEPATH, crd::Policy}; +use crate::{constants::CRD_FILEPATH, crd::Policy, crd::Module}; use argh::FromArgs; use kube::CustomResourceExt; use std::fs; +use k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition; #[derive(FromArgs, PartialEq, Eq, Debug)] #[argh(subcommand, name = "gencrd")] @@ -16,20 +17,37 @@ pub struct Args { } pub fn run(args: Args) { - let data = - serde_yaml::to_string(&Policy::crd()).expect("Could not generate yaml from CRD definition"); + let policy_crd = Policy::crd(); + let module_crd = Module::crd(); let filepath = args.file.unwrap_or_else(|| CRD_FILEPATH.to_string()); - let wrapped_data = "{{- if .Values.installCRDs }}\n".to_string() + &data + "{{- end }}\n"; + + let mut crd = String::new(); + crd.push_str(&gen_crd_data(&policy_crd)); + crd.push_str(&gen_crd_data(&module_crd)); + write_crd_str(&filepath, &crd, args.no_wrapping); +} + +fn gen_crd_data(data: &CustomResourceDefinition) -> String { + let mut string_data = serde_yaml::to_string(data).expect("Could not generate yaml from CRD definition"); + string_data.push_str("---\n"); + + string_data +} + +fn write_crd_str(filepath: &str, data: &str, no_wrapping: bool) { if filepath == "-" { println!("{}\n", data); } else { + + let wrapped_data = "{{- if .Values.installCRDs }}\n".to_string() + &data + "{{- end }}\n"; + fs::write( filepath, - match args.no_wrapping { + match no_wrapping { true => data, - false => wrapped_data, + false => &wrapped_data, }, ) .expect("Unable to write crd yaml"); } -} +} \ No newline at end of file diff --git a/src/main.rs b/src/main.rs index d0be5b7..f69b2b3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,6 +9,7 @@ mod events; mod helper; mod manager; mod policy; +mod module; mod server; mod util; diff --git a/src/manager.rs b/src/manager.rs index 82b58da..d952224 100644 --- a/src/manager.rs +++ b/src/manager.rs @@ -1,11 +1,15 @@ +use crate::events::{EventType}; use crate::util::error::{kube_err, Result}; +use crate::util::traits::ObjectStore; use crate::{ crd::Policy, - events::{EventSender, PolicyEvent, PolicyEventData}, + crd::Module, + events::{EventSender, Event, PolicyEventData, ModuleEventData}, policy::PolicyStoreRef, + module::ModuleStoreRef }; use futures::StreamExt; -use kube::runtime::{watcher, watcher::Event}; +use kube::runtime::{watcher, watcher::Event as KubeEvent}; use kube::{ api::{Api, ListParams}, Client, @@ -15,14 +19,16 @@ use tokio::task; pub struct Manager { k8s_client: Client, policies: PolicyStoreRef, + modules: ModuleStoreRef, event_sender: EventSender, } impl Manager { - pub fn new(client: Client, policies: PolicyStoreRef, event_sender: EventSender) -> Manager { + pub fn new(client: Client, policies: PolicyStoreRef, modules: ModuleStoreRef, event_sender: EventSender) -> Manager { Manager { k8s_client: client, policies, + modules, event_sender, } } @@ -36,11 +42,33 @@ impl Manager { { let mut policies = self.policies.lock().expect("lock failed. Cannot continue"); for policy in res { - if let Some(ref_info) = policies.add_policy(policy) { + if let Some(ref_info) = policies.add_object(policy) { self.event_sender - .send(PolicyEvent { - policy_reference: ref_info, - event_data: PolicyEventData::Loaded, + .send(Event { + object_reference: ref_info, + event_data: EventType::Policy(PolicyEventData::Loaded), + }) + .unwrap_or_else(|err| log::warn!("Could not send event: {:?}", err)); + } + } + } + Ok(()) + } + + pub async fn load_existing_modules(&mut self) -> Result<()> { + let modules_api = self.modules_api(); + let res = modules_api + .list(&ListParams::default()) + .await + .map_err(kube_err)?; + { + let mut modules = self.modules.lock().expect("lock failed. Cannot continue"); + for module in res { + if let Some(ref_info) = modules.add_object(module) { + self.event_sender + .send(Event { + object_reference: ref_info, + event_data: EventType::Module(ModuleEventData::Loaded), }) .unwrap_or_else(|err| log::warn!("Could not send event: {:?}", err)); } @@ -50,10 +78,16 @@ impl Manager { } pub async fn start(&mut self) { + self.watch_policies(); + self.watch_modules(); + } + + fn watch_policies(&mut self) { let policies_api = self.policies_api(); let policies = self.policies.clone(); let event_sender = self.event_sender.clone(); + // Watcher for policies task::spawn(async move { let watcher = watcher(policies_api.clone(), ListParams::default()); let mut pinned_watcher = Box::pin(watcher); @@ -61,24 +95,63 @@ impl Manager { let res = pinned_watcher.next().await; if let Some(Ok(event)) = res { match event { - Event::Applied(policy) => { + KubeEvent::Applied(policy) => { let mut policies = policies.lock().expect("lock failed. Cannot continue"); - if let Some(ref_info) = policies.add_policy(policy) { + if let Some(ref_info) = policies.add_object(policy) { event_sender - .send(PolicyEvent { - policy_reference: ref_info, - event_data: PolicyEventData::Loaded, + .send(Event { + object_reference: ref_info, + event_data: EventType::Policy(PolicyEventData::Loaded), }) .unwrap_or_else(|err| { log::warn!("Could not send event: {:?}", err) }); } } - Event::Deleted(policy) => { + KubeEvent::Deleted(policy) => { let mut policies = policies.lock().expect("lock failed. Cannot continue"); - policies.remove_policy(policy); + policies.remove_object(policy); + } + _ => (), + } + } + } + }); + } + + fn watch_modules(&mut self) { + let modules_api = self.modules_api(); + let modules = self.modules.clone(); + let event_sender = self.event_sender.clone(); + + // Watcher for modules + task::spawn(async move { + let watcher = watcher(modules_api.clone(), ListParams::default()); + let mut pinned_watcher = Box::pin(watcher); + loop { + let res = pinned_watcher.next().await; + if let Some(Ok(event)) = res { + match event { + KubeEvent::Applied(module) => { + let mut modules = + modules.lock().expect("lock failed. Cannot continue"); + if let Some(ref_info) = modules.add_object(module) { + event_sender + .send(Event { + object_reference: ref_info, + event_data: EventType::Module(ModuleEventData::Loaded), + }) + .unwrap_or_else(|err| { + log::warn!("Could not send event: {:?}", err) + }); + } + } + KubeEvent::Deleted(module) => { + let mut modules = + modules.lock().expect("lock failed. Cannot continue"); + modules.remove_object(module); } _ => (), } @@ -90,4 +163,8 @@ impl Manager { fn policies_api(&mut self) -> Api { Api::all(self.k8s_client.clone()) } + + fn modules_api(&mut self) -> Api { + Api::all(self.k8s_client.clone()) + } } diff --git a/src/module.rs b/src/module.rs new file mode 100644 index 0000000..abea4ec --- /dev/null +++ b/src/module.rs @@ -0,0 +1,98 @@ +use crate::crd::{Module, ModuleSpec}; +use crate::util::traits::ObjectStore; +use crate::util::types::ObjectReference; +use kube::core::Resource; +use lazy_static::lazy_static; +use prometheus::{register_gauge, Gauge}; +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; + +lazy_static! { + static ref ACTIVE_MODULES: Gauge = + register_gauge!("bridgekeeper_modules_active", "Number of active modules.") + .expect("creating metric always works"); +} + +pub struct ModuleStore { + pub modules: HashMap, +} + +pub type ModuleStoreRef = Arc>; + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ModuleInfo { + pub name: String, + pub module: ModuleSpec, + pub ref_info: ObjectReference, +} + +impl ModuleStore { + pub fn new() -> ModuleStoreRef { + let store = ModuleStore { + modules: HashMap::new(), + }; + Arc::new(Mutex::new(store)) + } +} + +fn create_object_reference(obj: &Module) -> ObjectReference { + ObjectReference { + api_version: Some(Module::api_version(&()).to_string()), + kind: Some(Module::kind(&()).to_string()), + name: obj.metadata.name.clone(), + uid: obj.metadata.uid.clone(), + } +} + +impl ModuleInfo { + pub fn new(name: String, module: ModuleSpec, ref_info: ObjectReference) -> ModuleInfo { + ModuleInfo { + name, + module, + ref_info, + } + } +} + +impl ObjectStore for ModuleStore { + fn add_object(&mut self, module: Module) -> Option { + let ref_info = create_object_reference(&module); + let name = module.metadata.name.expect("name is always set"); + if let Some(existing_module_info) = self.modules.get(&name) { + if existing_module_info.module != module.spec { + let module_info = ModuleInfo::new(name.clone(), module.spec, ref_info.clone()); + log::info!("Module '{}' updated", name); + self.modules.insert(name, module_info); + Some(ref_info) + } else { + None + } + } else { + let module_info = ModuleInfo::new(name.clone(), module.spec, ref_info.clone()); + log::info!("Module '{}' added", name); + self.modules.insert(name, module_info); + ACTIVE_MODULES.inc(); + Some(ref_info) + } + } + + fn remove_object(&mut self, module: Module) { + let name = module.metadata.name.expect("name is always set"); + log::info!("Module '{}' removed", name); + self.modules.remove(&name); + ACTIVE_MODULES.dec(); + } +} + +// pub fn load_modules_from_file(modules: ModuleStoreRef, filename: &str) -> Result { +// let mut modules = modules.lock().expect("Lock failed"); +// let data = std::fs::read_to_string(filename).map_err(load_err)?; + +// let mut count = 0; +// for document in serde_yaml::Deserializer::from_str(&data) { +// let module = Module::deserialize(document).map_err(load_err)?; +// modules.add_object(module); +// count += 1; +// } +// Ok(count) +// } diff --git a/src/policy.rs b/src/policy.rs index 8c5251e..ba6cead 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -1,6 +1,7 @@ use crate::crd::{Policy, PolicySpec}; +use crate::util::traits::ObjectStore; use crate::util::error::{load_err, Result}; -use k8s_openapi::api::core::v1::ObjectReference as KubeObjectReference; +use crate::util::types::ObjectReference; use kube::api::GroupVersionKind; use kube::core::Resource; use lazy_static::lazy_static; @@ -25,27 +26,7 @@ pub type PolicyStoreRef = Arc>; pub struct PolicyInfo { pub name: String, pub policy: PolicySpec, - pub ref_info: PolicyObjectReference, -} - -#[derive(Debug, Clone, PartialEq, Eq, Hash, Default)] -pub struct PolicyObjectReference { - pub api_version: Option, - pub kind: Option, - pub name: Option, - pub uid: Option, -} - -impl PolicyObjectReference { - pub fn to_object_reference(&self) -> KubeObjectReference { - KubeObjectReference { - api_version: self.api_version.clone(), - kind: self.kind.clone(), - name: self.name.clone(), - uid: self.uid.clone(), - ..Default::default() - } - } + pub ref_info: ObjectReference, } impl PolicyStore { @@ -57,8 +38,8 @@ impl PolicyStore { } } -fn create_object_reference(obj: &Policy) -> PolicyObjectReference { - PolicyObjectReference { +fn create_object_reference(obj: &Policy) -> ObjectReference { + ObjectReference { api_version: Some(Policy::api_version(&()).to_string()), kind: Some(Policy::kind(&()).to_string()), name: obj.metadata.name.clone(), @@ -67,7 +48,7 @@ fn create_object_reference(obj: &Policy) -> PolicyObjectReference { } impl PolicyInfo { - pub fn new(name: String, policy: PolicySpec, ref_info: PolicyObjectReference) -> PolicyInfo { + pub fn new(name: String, policy: PolicySpec, ref_info: ObjectReference) -> PolicyInfo { PolicyInfo { name, policy, @@ -113,8 +94,8 @@ impl PolicyInfo { } } -impl PolicyStore { - pub fn add_policy(&mut self, policy: Policy) -> Option { +impl ObjectStore for PolicyStore { + fn add_object(&mut self, policy: Policy) -> Option { let ref_info = create_object_reference(&policy); let name = policy.metadata.name.expect("name is always set"); if let Some(existing_policy_info) = self.policies.get(&name) { @@ -135,7 +116,7 @@ impl PolicyStore { } } - pub fn remove_policy(&mut self, policy: Policy) { + fn remove_object(&mut self, policy: Policy) { let name = policy.metadata.name.expect("name is always set"); log::info!("Policy '{}' removed", name); self.policies.remove(&name); @@ -150,7 +131,7 @@ pub fn load_policies_from_file(policies: PolicyStoreRef, filename: &str) -> Resu let mut count = 0; for document in serde_yaml::Deserializer::from_str(&data) { let policy = Policy::deserialize(document).map_err(load_err)?; - policies.add_policy(policy); + policies.add_object(policy); count += 1; } Ok(count) diff --git a/src/server.rs b/src/server.rs index 84a9d20..7201c1f 100644 --- a/src/server.rs +++ b/src/server.rs @@ -4,7 +4,7 @@ use crate::constants::POD_CERTS_DIR; use crate::evaluator::PolicyEvaluator; use crate::events::init_event_watcher; use crate::manager::Manager; -use crate::policy::PolicyStore; +use crate::{module::ModuleStore, policy::PolicyStore}; #[derive(FromArgs, PartialEq, Eq, Debug)] #[argh(subcommand, name = "server")] @@ -50,17 +50,22 @@ pub async fn run(args: Args) { // Initiate services let policies = PolicyStore::new(); + let modules = ModuleStore::new(); let event_sender = init_event_watcher(&client); - let mut manager = Manager::new(client.clone(), policies.clone(), event_sender.clone()); - let evaluator = PolicyEvaluator::new(policies.clone(), event_sender.clone()); + let mut manager = Manager::new(client.clone(), policies.clone(), modules.clone(), event_sender.clone()); + let evaluator = PolicyEvaluator::new(policies.clone(), modules.clone(), event_sender.clone()); manager.start().await; manager .load_existing_policies() .await .expect("Could not load existing policies"); + manager + .load_existing_modules() + .await + .expect("Could not load existing policies"); if args.audit { - crate::audit::launch_loop(client, policies, args.audit_interval.unwrap_or(600)).await; + crate::audit::launch_loop(client, policies, modules, args.audit_interval.unwrap_or(600)).await; } // Launch API with webhook endpoint diff --git a/src/util/mod.rs b/src/util/mod.rs index b8e412d..63040d3 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -2,3 +2,5 @@ pub mod cert; pub mod error; pub mod k8s_client; pub mod webhook; +pub mod types; +pub mod traits; \ No newline at end of file diff --git a/src/util/traits.rs b/src/util/traits.rs new file mode 100644 index 0000000..4bab7cd --- /dev/null +++ b/src/util/traits.rs @@ -0,0 +1,6 @@ +use crate::util::types::ObjectReference; + +pub trait ObjectStore { + fn add_object(&mut self, object: T) -> Option; + fn remove_object(&mut self, object: T); +} \ No newline at end of file diff --git a/src/util/types.rs b/src/util/types.rs new file mode 100644 index 0000000..723095f --- /dev/null +++ b/src/util/types.rs @@ -0,0 +1,21 @@ +use k8s_openapi::api::core::v1::ObjectReference as KubeObjectReference; + +#[derive(Debug, Clone, PartialEq, Eq, Hash, Default)] +pub struct ObjectReference { + pub api_version: Option, + pub kind: Option, + pub name: Option, + pub uid: Option, +} + +impl ObjectReference { + pub fn to_object_reference(&self) -> KubeObjectReference { + KubeObjectReference { + api_version: self.api_version.clone(), + kind: self.kind.clone(), + name: self.name.clone(), + uid: self.uid.clone(), + ..Default::default() + } + } +} \ No newline at end of file From 4bd5feaa8cdc1ae177e121ca3b994ac16d53d661 Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Fri, 30 Dec 2022 13:47:40 +0100 Subject: [PATCH 2/8] Adding module code to validating webhook and auditing --- src/api.rs | 27 ++++++++++++++++++++++++++- src/audit.rs | 4 ++-- src/evaluator.rs | 19 ++++++++++++++----- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/api.rs b/src/api.rs index 84ea3f6..59e4d6b 100644 --- a/src/api.rs +++ b/src/api.rs @@ -83,6 +83,7 @@ async fn admission_mutate( #[rocket::post("/validate-policy", data = "")] async fn api_validate_policy( data: Json>, + evaluator: &State, ) -> Result>, ApiError> { HTTP_REQUEST_COUNTER .with_label_values(&["/validate-policy"]) @@ -93,7 +94,31 @@ async fn api_validate_policy( })?; let mut response: AdmissionResponse = AdmissionResponse::from(&admission_request); - let (allowed, reason) = validate_policy_admission(&admission_request); + let mut module_code = String::new(); + + if let Some(policy) = &admission_request.object { + if let Some(used_modules) = policy.spec.modules.clone() { + let evaluator = evaluator.inner().clone(); + let modules = evaluator.get_available_modules(); + + for module_name in used_modules.iter() { + match modules.get(module_name) { + Some(module_info) => { + module_code.push_str(&module_info.module.python); + module_code.push_str("\n"); + }, + None => { + response.allowed = false; + response.result.code = Some(403); + response.result.message = Some(format!("Could not find module '{}'", module_name)); + return Ok(Json(response.into_review())) + } + }; + } + } + } + + let (allowed, reason) = validate_policy_admission(&admission_request, &module_code); response.allowed = allowed; if !allowed { response.result.message = reason; diff --git a/src/audit.rs b/src/audit.rs index 65985a5..2e74999 100644 --- a/src/audit.rs +++ b/src/audit.rs @@ -156,13 +156,13 @@ impl Auditor { module_code.push_str("\n"); }, None => { - log::warn!("Could not find module '{}'", module_name) + log::warn!("Could not find module '{}'", module_name); } }; } } - let (valid, reason) = crate::evaluator::validate_policy(&policy.name, &policy.policy); + let (valid, reason) = crate::evaluator::validate_policy(&policy.name, &policy.policy, &module_code); if !valid { println!( "Failed to validate policy: {}", diff --git a/src/evaluator.rs b/src/evaluator.rs index 4bb206d..0b5bf64 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -2,7 +2,7 @@ use crate::{ crd::{Policy, PolicySpec}, events::{EventSender, Event, PolicyEventData, EventType}, policy::{PolicyInfo, PolicyStoreRef}, - module::{ModuleStoreRef}, + module::{ModuleStoreRef, ModuleInfo}, }; use kube::core::{ admission::{self, Operation}, @@ -12,7 +12,7 @@ use lazy_static::lazy_static; use prometheus::{register_counter_vec, CounterVec}; use pyo3::prelude::*; use serde_derive::Serialize; -use std::sync::Arc; +use std::{sync::Arc, collections::HashMap}; lazy_static! { static ref MATCHED_POLICIES: CounterVec = register_counter_vec!( @@ -225,24 +225,33 @@ impl PolicyEvaluator { patch: patches, } } + + pub fn get_available_modules(&self) -> HashMap { + let module_store = self.modules.lock().expect("lock failed. Cannot continue"); + + module_store.modules.clone() + } } pub fn validate_policy_admission( request: &admission::AdmissionRequest, + module_code: &str ) -> (bool, Option) { if let Some(policy) = request.object.as_ref() { let name = match policy.metadata.name.as_ref() { Some(name) => name.as_str(), None => "-invalidname-", }; - validate_policy(name, &policy.spec) + validate_policy(name, &policy.spec, module_code) } else { (false, Some("No rule found".to_string())) } } -pub fn validate_policy(name: &str, policy: &PolicySpec) -> (bool, Option) { - let python_code = policy.rule.python.clone(); +pub fn validate_policy(name: &str, policy: &PolicySpec, module_code: &str) -> (bool, Option) { + let mut python_code = String::from(module_code); + python_code.push_str(&policy.rule.python.clone()); + Python::with_gil(|py| { if let Err(err) = PyModule::from_code(py, &python_code, "rule.py", "bridgekeeper") { POLICY_VALIDATIONS_FAIL.with_label_values(&[name]).inc(); From 350e1396e35545e85cfc22c62216d6e90a8d107d Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Fri, 30 Dec 2022 13:58:39 +0100 Subject: [PATCH 3/8] Using loaded module code in audit --- src/audit.rs | 4 ++-- src/evaluator.rs | 3 ++- src/module.rs | 15 +-------------- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/audit.rs b/src/audit.rs index 2e74999..4e0f12d 100644 --- a/src/audit.rs +++ b/src/audit.rs @@ -209,7 +209,7 @@ impl Auditor { let target_identifier = gen_target_identifier(resource_description, &object); let (result, message, _patch) = - crate::evaluator::evaluate_policy_audit(policy, object); + crate::evaluator::evaluate_policy_audit(policy, object, &module_code); NUM_CHECKED_OBJECTS .with_label_values(&[policy.name.as_str(), namespace.as_str()]) .inc(); @@ -237,7 +237,7 @@ impl Auditor { for object in objects { let target_identifier = gen_target_identifier(resource_description, &object); let (result, message, _patch) = - crate::evaluator::evaluate_policy_audit(policy, object); + crate::evaluator::evaluate_policy_audit(policy, object, &module_code); NUM_CHECKED_OBJECTS .with_label_values(&[policy.name.as_str(), ""]) .inc(); diff --git a/src/evaluator.rs b/src/evaluator.rs index 0b5bf64..ec26439 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -299,12 +299,13 @@ fn evaluate_policy( pub fn evaluate_policy_audit( policy: &PolicyInfo, object: DynamicObject, + module_code: &str, ) -> (bool, Option, Option) { let request = ValidationRequest { object, operation: Operation::Update, }; - evaluate_policy(policy, &request, "") // TODO: load module_code + evaluate_policy(policy, &request, module_code) } fn extract_result( diff --git a/src/module.rs b/src/module.rs index abea4ec..86b1645 100644 --- a/src/module.rs +++ b/src/module.rs @@ -82,17 +82,4 @@ impl ObjectStore for ModuleStore { self.modules.remove(&name); ACTIVE_MODULES.dec(); } -} - -// pub fn load_modules_from_file(modules: ModuleStoreRef, filename: &str) -> Result { -// let mut modules = modules.lock().expect("Lock failed"); -// let data = std::fs::read_to_string(filename).map_err(load_err)?; - -// let mut count = 0; -// for document in serde_yaml::Deserializer::from_str(&data) { -// let module = Module::deserialize(document).map_err(load_err)?; -// modules.add_object(module); -// count += 1; -// } -// Ok(count) -// } +} \ No newline at end of file From b06776f31c82a0cc0d18466f10556548429841ce Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Fri, 30 Dec 2022 14:49:00 +0100 Subject: [PATCH 4/8] Adding unit test for module usage --- src/evaluator.rs | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/evaluator.rs b/src/evaluator.rs index ec26439..0ec7ef4 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -477,5 +477,35 @@ def validate(request): ); } - // TODO: Test modules + #[test] + fn test_use_of_module() { + pyo3::prepare_freethreaded_python(); + let python = r#" +def validate(request): + return bool_from_module() + "#; + + let module_code = r#" +def bool_from_module(): + return True + "#; + + let policy_spec = PolicySpec::from_python(python.to_string()); + let policy = PolicyInfo::new("test".to_string(), policy_spec, Default::default()); + + let object = DynamicObject { + types: None, + metadata: ObjectMeta::default(), + data: serde_json::Value::Null, + }; + let request = ValidationRequest { + object, + operation: Operation::Create, + }; + + let (res, reason, patch) = evaluate_policy(&policy, &request, &module_code); + assert!(res, "validate function failed: {}", reason.unwrap()); + assert!(reason.is_none()); + assert!(patch.is_none()); + } } From 8832169d4d044abd6e3d6ec4e21202ae1d3652b8 Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Fri, 30 Dec 2022 19:58:16 +0100 Subject: [PATCH 5/8] Using ObjectStore trait across the project --- src/audit.rs | 4 ++-- src/evaluator.rs | 6 +++--- src/manager.rs | 1 - src/module.rs | 8 ++++++-- src/policy.rs | 8 ++++++-- src/util/traits.rs | 3 ++- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/audit.rs b/src/audit.rs index 4e0f12d..abd2b97 100644 --- a/src/audit.rs +++ b/src/audit.rs @@ -110,7 +110,7 @@ impl Auditor { // While holding the lock only collect the policies or modules, directly auditing them would make the future of the method not implement Send which breaks the task spawn { let policy_store = self.policies.lock().expect("lock failed. Cannot continue"); - for policy in policy_store.policies.values() { + for policy in policy_store.get_objects().values() { if all || policy.policy.audit.unwrap_or(false) { policies.push(policy.clone()); } @@ -119,7 +119,7 @@ impl Auditor { { let module_store = self.modules.lock().expect("lock failed. Cannot continue"); - modules.extend(module_store.modules.clone()); + modules.extend(module_store.get_objects()); } for policy in policies.iter() { diff --git a/src/evaluator.rs b/src/evaluator.rs index 0ec7ef4..b019661 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -120,7 +120,7 @@ impl PolicyEvaluator { let mut matching_policies = Vec::new(); // Collect all matching policies - for value in policies.policies.values() { + for value in policies.get_objects().values() { if value.is_match(&gvk, &namespace) { MATCHED_POLICIES .with_label_values(&[value.name.as_str()]) @@ -154,7 +154,7 @@ impl PolicyEvaluator { if let Some(used_modules) = &value.policy.modules { for module_name in used_modules.iter() { - match modules.modules.get(module_name) { + match modules.get_objects().get(module_name) { Some(module_info) => { module_code.push_str(&module_info.module.python); module_code.push_str("\n"); @@ -229,7 +229,7 @@ impl PolicyEvaluator { pub fn get_available_modules(&self) -> HashMap { let module_store = self.modules.lock().expect("lock failed. Cannot continue"); - module_store.modules.clone() + return module_store.get_objects(); } } diff --git a/src/manager.rs b/src/manager.rs index d952224..e967dc7 100644 --- a/src/manager.rs +++ b/src/manager.rs @@ -1,6 +1,5 @@ use crate::events::{EventType}; use crate::util::error::{kube_err, Result}; -use crate::util::traits::ObjectStore; use crate::{ crd::Policy, crd::Module, diff --git a/src/module.rs b/src/module.rs index 86b1645..5acfc3f 100644 --- a/src/module.rs +++ b/src/module.rs @@ -17,7 +17,7 @@ pub struct ModuleStore { pub modules: HashMap, } -pub type ModuleStoreRef = Arc>; +pub type ModuleStoreRef = Arc> + Send>>; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct ModuleInfo { @@ -54,7 +54,7 @@ impl ModuleInfo { } } -impl ObjectStore for ModuleStore { +impl ObjectStore> for ModuleStore { fn add_object(&mut self, module: Module) -> Option { let ref_info = create_object_reference(&module); let name = module.metadata.name.expect("name is always set"); @@ -82,4 +82,8 @@ impl ObjectStore for ModuleStore { self.modules.remove(&name); ACTIVE_MODULES.dec(); } + + fn get_objects(&self) -> HashMap { + return self.modules.clone(); + } } \ No newline at end of file diff --git a/src/policy.rs b/src/policy.rs index ba6cead..9fb944a 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -20,7 +20,7 @@ pub struct PolicyStore { pub policies: HashMap, } -pub type PolicyStoreRef = Arc>; +pub type PolicyStoreRef = Arc> + Send>>; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct PolicyInfo { @@ -94,7 +94,7 @@ impl PolicyInfo { } } -impl ObjectStore for PolicyStore { +impl ObjectStore> for PolicyStore { fn add_object(&mut self, policy: Policy) -> Option { let ref_info = create_object_reference(&policy); let name = policy.metadata.name.expect("name is always set"); @@ -122,6 +122,10 @@ impl ObjectStore for PolicyStore { self.policies.remove(&name); ACTIVE_POLICIES.dec(); } + + fn get_objects(&self) -> HashMap { + return self.policies.clone(); + } } pub fn load_policies_from_file(policies: PolicyStoreRef, filename: &str) -> Result { diff --git a/src/util/traits.rs b/src/util/traits.rs index 4bab7cd..8a27795 100644 --- a/src/util/traits.rs +++ b/src/util/traits.rs @@ -1,6 +1,7 @@ use crate::util::types::ObjectReference; -pub trait ObjectStore { +pub trait ObjectStore { fn add_object(&mut self, object: T) -> Option; fn remove_object(&mut self, object: T); + fn get_objects(&self) -> V; } \ No newline at end of file From ce165df79d4b5821d068862b3dc673f3534443c9 Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Fri, 30 Dec 2022 20:07:27 +0100 Subject: [PATCH 6/8] Properly handling module events in event watcher --- src/events.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/events.rs b/src/events.rs index 2ff9244..51b76ee 100644 --- a/src/events.rs +++ b/src/events.rs @@ -77,17 +77,6 @@ pub fn init_event_watcher(client: &Client) -> EventSender { )); } } - if let Err(err) = events_api.create(&PostParams::default(), &kube_event).await { - log::warn!( - "Could not create event for policy {}. Reason: {}", - event - .object_reference - .name - .clone() - .unwrap_or_else(|| "-".to_string()), - err - ); - } }, EventType::Module(event_data) => { match event_data { @@ -98,6 +87,18 @@ pub fn init_event_watcher(client: &Client) -> EventSender { } } } + + if let Err(err) = events_api.create(&PostParams::default(), &kube_event).await { + log::warn!( + "Could not create event for policy {}. Reason: {}", + event + .object_reference + .name + .clone() + .unwrap_or_else(|| "-".to_string()), + err + ); + } } }); sender From 541b8a159c3ceb215d0d1a1b37593f17e8a5a7ee Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Thu, 16 Feb 2023 12:10:55 +0100 Subject: [PATCH 7/8] Changes from Code Review --- charts/bridgekeeper/templates/rbac.yaml | 4 +- src/api.rs | 28 +--------- src/audit.rs | 2 +- src/crd.rs | 6 +-- src/evaluator.rs | 62 +++++++++++++++------- src/events.rs | 2 +- src/module.rs | 51 +++++++++--------- src/policy.rs | 69 ++++++++++++------------- src/util/mod.rs | 3 +- src/util/traits.rs | 7 --- src/util/types.rs | 2 +- 11 files changed, 111 insertions(+), 125 deletions(-) delete mode 100644 src/util/traits.rs diff --git a/charts/bridgekeeper/templates/rbac.yaml b/charts/bridgekeeper/templates/rbac.yaml index 5ee2d5f..4bfc286 100644 --- a/charts/bridgekeeper/templates/rbac.yaml +++ b/charts/bridgekeeper/templates/rbac.yaml @@ -5,7 +5,7 @@ metadata: {{- include "bridgekeeper.labels" . | nindent 4 }} name: bridgekeeper-role rules: -# During audit runs bridgekeeper needs to potentially access all resources in the cluster +# During audit runs bridgekeeper needs to potentially access all resources in the cluster - apiGroups: - '*' resources: @@ -42,6 +42,8 @@ rules: resources: - policies - policies/status + - modules + - modules/status verbs: - patch - update diff --git a/src/api.rs b/src/api.rs index 59e4d6b..23d0e14 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1,5 +1,5 @@ use crate::crd::Policy; -use crate::evaluator::{validate_policy_admission, EvaluationResult, PolicyEvaluatorRef}; +use crate::evaluator::{EvaluationResult, PolicyEvaluatorRef}; use crate::util::cert::CertKeyPair; use kube::{ api::DynamicObject, @@ -94,31 +94,7 @@ async fn api_validate_policy( })?; let mut response: AdmissionResponse = AdmissionResponse::from(&admission_request); - let mut module_code = String::new(); - - if let Some(policy) = &admission_request.object { - if let Some(used_modules) = policy.spec.modules.clone() { - let evaluator = evaluator.inner().clone(); - let modules = evaluator.get_available_modules(); - - for module_name in used_modules.iter() { - match modules.get(module_name) { - Some(module_info) => { - module_code.push_str(&module_info.module.python); - module_code.push_str("\n"); - }, - None => { - response.allowed = false; - response.result.code = Some(403); - response.result.message = Some(format!("Could not find module '{}'", module_name)); - return Ok(Json(response.into_review())) - } - }; - } - } - } - - let (allowed, reason) = validate_policy_admission(&admission_request, &module_code); + let (allowed, reason) = evaluator.validate_policy_admission(&admission_request); response.allowed = allowed; if !allowed { response.result.message = reason; diff --git a/src/audit.rs b/src/audit.rs index abd2b97..7979bad 100644 --- a/src/audit.rs +++ b/src/audit.rs @@ -148,7 +148,7 @@ impl Auditor { let mut module_code = String::new(); - if let Some(used_modules) = &policy.policy.modules { + if let Some(used_modules) = &policy.policy.rule.modules { for module_name in used_modules.iter() { match modules.get(module_name) { Some(module_info) => { diff --git a/src/crd.rs b/src/crd.rs index b8d9f3e..014eb27 100644 --- a/src/crd.rs +++ b/src/crd.rs @@ -16,7 +16,6 @@ pub struct PolicySpec { pub rule: Rule, pub audit: Option, pub enforce: Option, - pub modules: Option>, } #[derive( @@ -50,6 +49,7 @@ pub struct Match { #[serde(rename_all = "camelCase")] pub struct Rule { pub python: String, + pub modules: Option>, } #[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)] @@ -118,7 +118,6 @@ impl PolicySpec { enforce: Some(true), target, rule: Default::default(), - modules: None, } } @@ -127,8 +126,7 @@ impl PolicySpec { audit: Some(false), enforce: Some(true), target: Default::default(), - rule: Rule { python }, - modules: None, + rule: Rule { python, modules: None }, } } } diff --git a/src/evaluator.rs b/src/evaluator.rs index b019661..90e6d2d 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -152,7 +152,7 @@ impl PolicyEvaluator { let mut module_code = String::new(); - if let Some(used_modules) = &value.policy.modules { + if let Some(used_modules) = &value.policy.rule.modules { for module_name in used_modules.iter() { match modules.get_objects().get(module_name) { Some(module_info) => { @@ -160,7 +160,12 @@ impl PolicyEvaluator { module_code.push_str("\n"); }, None => { - log::warn!("Could not find module '{}'", module_name) + return EvaluationResult { + allowed: false, + patch: None, + reason: Some(format!("Could not find module '{}'", module_name)), + warnings + } } }; } @@ -179,9 +184,9 @@ impl PolicyEvaluator { object_reference: value.ref_info.clone(), event_data: EventType::Policy( PolicyEventData::Evaluated { - target_identifier, - result: res.0, - reason: res.1.clone(), + target_identifier, + result: res.0, + reason: res.1.clone(), } ), }) @@ -228,23 +233,42 @@ impl PolicyEvaluator { pub fn get_available_modules(&self) -> HashMap { let module_store = self.modules.lock().expect("lock failed. Cannot continue"); - + return module_store.get_objects(); } -} -pub fn validate_policy_admission( - request: &admission::AdmissionRequest, - module_code: &str -) -> (bool, Option) { - if let Some(policy) = request.object.as_ref() { - let name = match policy.metadata.name.as_ref() { - Some(name) => name.as_str(), - None => "-invalidname-", - }; - validate_policy(name, &policy.spec, module_code) - } else { - (false, Some("No rule found".to_string())) + pub fn validate_policy_admission( + &self, + request: &admission::AdmissionRequest + ) -> (bool, Option) { + if let Some(policy) = request.object.as_ref() { + let name = match policy.metadata.name.as_ref() { + Some(name) => name.as_str(), + None => "-invalidname-", + }; + + let mut module_code = String::new(); + + if let Some(used_modules) = policy.spec.rule.modules.clone() { + let modules = self.get_available_modules(); + + for module_name in used_modules.iter() { + match modules.get(module_name) { + Some(module_info) => { + module_code.push_str(&module_info.module.python); + module_code.push_str("\n"); + }, + None => { + log::warn!("Could not find module '{}'", module_name) + } + }; + } + } + + validate_policy(name, &policy.spec, &module_code) + } else { + (false, Some("No rule found".to_string())) + } } } diff --git a/src/events.rs b/src/events.rs index 51b76ee..5350242 100644 --- a/src/events.rs +++ b/src/events.rs @@ -48,7 +48,7 @@ pub fn init_event_watcher(client: &Client) -> EventSender { let mut kube_event = KubeEvent::default(); kube_event.metadata.generate_name = event.object_reference.name.clone(); - kube_event.involved_object = event.object_reference.to_object_reference(); + kube_event.involved_object = event.object_reference.to_k8s_object_reference(); kube_event.type_ = Some("Normal".to_string()); kube_event.first_timestamp = Some(Time(Utc::now())); kube_event.source = Some(KubeEventSource { diff --git a/src/module.rs b/src/module.rs index 5acfc3f..979efb0 100644 --- a/src/module.rs +++ b/src/module.rs @@ -1,5 +1,4 @@ use crate::crd::{Module, ModuleSpec}; -use crate::util::traits::ObjectStore; use crate::util::types::ObjectReference; use kube::core::Resource; use lazy_static::lazy_static; @@ -17,7 +16,7 @@ pub struct ModuleStore { pub modules: HashMap, } -pub type ModuleStoreRef = Arc> + Send>>; +pub type ModuleStoreRef = Arc>; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct ModuleInfo { @@ -33,29 +32,8 @@ impl ModuleStore { }; Arc::new(Mutex::new(store)) } -} - -fn create_object_reference(obj: &Module) -> ObjectReference { - ObjectReference { - api_version: Some(Module::api_version(&()).to_string()), - kind: Some(Module::kind(&()).to_string()), - name: obj.metadata.name.clone(), - uid: obj.metadata.uid.clone(), - } -} - -impl ModuleInfo { - pub fn new(name: String, module: ModuleSpec, ref_info: ObjectReference) -> ModuleInfo { - ModuleInfo { - name, - module, - ref_info, - } - } -} -impl ObjectStore> for ModuleStore { - fn add_object(&mut self, module: Module) -> Option { + pub fn add_object(&mut self, module: Module) -> Option { let ref_info = create_object_reference(&module); let name = module.metadata.name.expect("name is always set"); if let Some(existing_module_info) = self.modules.get(&name) { @@ -76,14 +54,33 @@ impl ObjectStore> for ModuleStore { } } - fn remove_object(&mut self, module: Module) { + pub fn remove_object(&mut self, module: Module) { let name = module.metadata.name.expect("name is always set"); log::info!("Module '{}' removed", name); self.modules.remove(&name); ACTIVE_MODULES.dec(); } - fn get_objects(&self) -> HashMap { + pub fn get_objects(&self) -> HashMap { return self.modules.clone(); } -} \ No newline at end of file +} + +fn create_object_reference(obj: &Module) -> ObjectReference { + ObjectReference { + api_version: Some(Module::api_version(&()).to_string()), + kind: Some(Module::kind(&()).to_string()), + name: obj.metadata.name.clone(), + uid: obj.metadata.uid.clone(), + } +} + +impl ModuleInfo { + pub fn new(name: String, module: ModuleSpec, ref_info: ObjectReference) -> ModuleInfo { + ModuleInfo { + name, + module, + ref_info, + } + } +} diff --git a/src/policy.rs b/src/policy.rs index 9fb944a..f5091be 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -1,5 +1,4 @@ use crate::crd::{Policy, PolicySpec}; -use crate::util::traits::ObjectStore; use crate::util::error::{load_err, Result}; use crate::util::types::ObjectReference; use kube::api::GroupVersionKind; @@ -20,7 +19,7 @@ pub struct PolicyStore { pub policies: HashMap, } -pub type PolicyStoreRef = Arc> + Send>>; +pub type PolicyStoreRef = Arc>; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct PolicyInfo { @@ -36,6 +35,38 @@ impl PolicyStore { }; Arc::new(Mutex::new(store)) } + + pub fn add_object(&mut self, policy: Policy) -> Option { + let ref_info = create_object_reference(&policy); + let name = policy.metadata.name.expect("name is always set"); + if let Some(existing_policy_info) = self.policies.get(&name) { + if existing_policy_info.policy != policy.spec { + let policy_info = PolicyInfo::new(name.clone(), policy.spec, ref_info.clone()); + log::info!("Policy '{}' updated", name); + self.policies.insert(name, policy_info); + Some(ref_info) + } else { + None + } + } else { + let policy_info = PolicyInfo::new(name.clone(), policy.spec, ref_info.clone()); + log::info!("Policy '{}' added", name); + self.policies.insert(name, policy_info); + ACTIVE_POLICIES.inc(); + Some(ref_info) + } + } + + pub fn remove_object(&mut self, policy: Policy) { + let name = policy.metadata.name.expect("name is always set"); + log::info!("Policy '{}' removed", name); + self.policies.remove(&name); + ACTIVE_POLICIES.dec(); + } + + pub fn get_objects(&self) -> HashMap { + return self.policies.clone(); + } } fn create_object_reference(obj: &Policy) -> ObjectReference { @@ -94,40 +125,6 @@ impl PolicyInfo { } } -impl ObjectStore> for PolicyStore { - fn add_object(&mut self, policy: Policy) -> Option { - let ref_info = create_object_reference(&policy); - let name = policy.metadata.name.expect("name is always set"); - if let Some(existing_policy_info) = self.policies.get(&name) { - if existing_policy_info.policy != policy.spec { - let policy_info = PolicyInfo::new(name.clone(), policy.spec, ref_info.clone()); - log::info!("Policy '{}' updated", name); - self.policies.insert(name, policy_info); - Some(ref_info) - } else { - None - } - } else { - let policy_info = PolicyInfo::new(name.clone(), policy.spec, ref_info.clone()); - log::info!("Policy '{}' added", name); - self.policies.insert(name, policy_info); - ACTIVE_POLICIES.inc(); - Some(ref_info) - } - } - - fn remove_object(&mut self, policy: Policy) { - let name = policy.metadata.name.expect("name is always set"); - log::info!("Policy '{}' removed", name); - self.policies.remove(&name); - ACTIVE_POLICIES.dec(); - } - - fn get_objects(&self) -> HashMap { - return self.policies.clone(); - } -} - pub fn load_policies_from_file(policies: PolicyStoreRef, filename: &str) -> Result { let mut policies = policies.lock().expect("Lock failed"); let data = std::fs::read_to_string(filename).map_err(load_err)?; diff --git a/src/util/mod.rs b/src/util/mod.rs index 63040d3..40b11c6 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -2,5 +2,4 @@ pub mod cert; pub mod error; pub mod k8s_client; pub mod webhook; -pub mod types; -pub mod traits; \ No newline at end of file +pub mod types; \ No newline at end of file diff --git a/src/util/traits.rs b/src/util/traits.rs deleted file mode 100644 index 8a27795..0000000 --- a/src/util/traits.rs +++ /dev/null @@ -1,7 +0,0 @@ -use crate::util::types::ObjectReference; - -pub trait ObjectStore { - fn add_object(&mut self, object: T) -> Option; - fn remove_object(&mut self, object: T); - fn get_objects(&self) -> V; -} \ No newline at end of file diff --git a/src/util/types.rs b/src/util/types.rs index 723095f..7e81f5a 100644 --- a/src/util/types.rs +++ b/src/util/types.rs @@ -9,7 +9,7 @@ pub struct ObjectReference { } impl ObjectReference { - pub fn to_object_reference(&self) -> KubeObjectReference { + pub fn to_k8s_object_reference(&self) -> KubeObjectReference { KubeObjectReference { api_version: self.api_version.clone(), kind: self.kind.clone(), From cde6a42c5b54e79919b4cffb570b8846d4fd91b8 Mon Sep 17 00:00:00 2001 From: Luis Schweigard Date: Thu, 16 Feb 2023 12:11:30 +0100 Subject: [PATCH 8/8] Updating CRD --- charts/bridgekeeper/templates/crds.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/charts/bridgekeeper/templates/crds.yaml b/charts/bridgekeeper/templates/crds.yaml index a38bc00..658343e 100644 --- a/charts/bridgekeeper/templates/crds.yaml +++ b/charts/bridgekeeper/templates/crds.yaml @@ -27,13 +27,13 @@ spec: enforce: nullable: true type: boolean - modules: - items: - type: string - nullable: true - type: array rule: properties: + modules: + items: + type: string + nullable: true + type: array python: type: string required: