From 502b0965111226b2ca2a33bb6d567c271a5103a3 Mon Sep 17 00:00:00 2001 From: David Morrison Date: Sat, 26 Oct 2024 14:48:08 -0700 Subject: [PATCH] feat: service_account_missing validation check --- sk-cli/src/validation/annotated_trace.rs | 6 +- sk-cli/src/validation/mod.rs | 2 +- sk-cli/src/validation/rules/mod.rs | 5 ++ .../rules/service_account_missing.rs | 90 +++++++++++++++++++ .../{ => rules}/status_field_populated.rs | 15 ++-- sk-cli/src/validation/rules/tests/mod.rs | 7 ++ .../tests/status_field_populated_test.rs | 0 sk-cli/src/validation/tests/mod.rs | 1 - sk-cli/src/validation/validation_store.rs | 5 +- sk-cli/src/validation/validator.rs | 15 +++- sk-cli/src/xray/view/mod.rs | 10 +++ 11 files changed, 139 insertions(+), 17 deletions(-) create mode 100644 sk-cli/src/validation/rules/mod.rs create mode 100644 sk-cli/src/validation/rules/service_account_missing.rs rename sk-cli/src/validation/{ => rules}/status_field_populated.rs (82%) create mode 100644 sk-cli/src/validation/rules/tests/mod.rs rename sk-cli/src/validation/{ => rules}/tests/status_field_populated_test.rs (100%) diff --git a/sk-cli/src/validation/annotated_trace.rs b/sk-cli/src/validation/annotated_trace.rs index 1aa10931..2aa04700 100644 --- a/sk-cli/src/validation/annotated_trace.rs +++ b/sk-cli/src/validation/annotated_trace.rs @@ -110,12 +110,12 @@ impl AnnotatedTrace { trace.export_all() } - pub fn validate(&mut self, validators: &BTreeMap) -> AnnotationSummary { + pub fn validate(&mut self, validators: &BTreeMap) -> anyhow::Result { let mut summary = BTreeMap::new(); for event in self.events.iter_mut() { event.clear_annotations(); for (code, validator) in validators.iter() { - let affected_indices = validator.check_next_event(event); + let affected_indices = validator.check_next_event(event, self.base.config())?; let count = affected_indices.len(); summary.entry(*code).and_modify(|e| *e += count).or_insert(count); @@ -124,7 +124,7 @@ impl AnnotatedTrace { } } } - summary + Ok(summary) } pub fn get_event(&self, idx: usize) -> Option<&AnnotatedTraceEvent> { diff --git a/sk-cli/src/validation/mod.rs b/sk-cli/src/validation/mod.rs index 28b29460..730cf23c 100644 --- a/sk-cli/src/validation/mod.rs +++ b/sk-cli/src/validation/mod.rs @@ -1,5 +1,5 @@ mod annotated_trace; -mod status_field_populated; +mod rules; mod summary; mod validation_store; mod validator; diff --git a/sk-cli/src/validation/rules/mod.rs b/sk-cli/src/validation/rules/mod.rs new file mode 100644 index 00000000..991266c5 --- /dev/null +++ b/sk-cli/src/validation/rules/mod.rs @@ -0,0 +1,5 @@ +pub mod service_account_missing; +pub mod status_field_populated; + +#[cfg(test)] +mod tests; diff --git a/sk-cli/src/validation/rules/service_account_missing.rs b/sk-cli/src/validation/rules/service_account_missing.rs new file mode 100644 index 00000000..43df1867 --- /dev/null +++ b/sk-cli/src/validation/rules/service_account_missing.rs @@ -0,0 +1,90 @@ +use std::collections::HashSet; +use std::sync::{ + Arc, + RwLock, +}; + +use json_patch_ext::prelude::*; +use sk_core::k8s::GVK; +use sk_core::prelude::*; +use sk_store::TracerConfig; + +use crate::validation::annotated_trace::AnnotatedTraceEvent; +use crate::validation::validator::{ + Diagnostic, + Validator, + ValidatorType, +}; + +const HELP: &str = r#"A pod needs a service account that is not present in +the trace file. The simulation will fail because pods cannot be created +if their service account does not exist."#; + +#[derive(Default)] +pub struct ServiceAccountMissing { + seen_service_accounts: HashSet, +} + +fn get_service_account(obj: &DynamicObject, config: &TracerConfig) -> anyhow::Result> { + let gvk = GVK::from_dynamic_obj(obj)?; + if let Some(pod_spec_template_path) = config.pod_spec_template_path(&gvk) { + let sa_ptr = format_ptr!("{pod_spec_template_path}/spec/serviceAccount"); + let sa_name_ptr = format_ptr!("{pod_spec_template_path}/spec/serviceAccountName"); + if let Ok(sa) = sa_ptr.resolve(&obj.data) { + return Ok(Some(sa.to_string())); + } else if let Ok(sa) = sa_name_ptr.resolve(&obj.data) { + return Ok(Some(sa.to_string())); + } + } + Ok(None) +} + +impl Diagnostic for ServiceAccountMissing { + fn check_next_event( + &mut self, + event: &mut AnnotatedTraceEvent, + config: &TracerConfig, + ) -> anyhow::Result> { + for obj in &event.data.applied_objs { + if let Some(ref type_meta) = obj.types { + if &type_meta.kind == "ServiceAccount" { + self.seen_service_accounts.insert(obj.namespaced_name()); + } + } + } + for obj in &event.data.deleted_objs { + if let Some(ref type_meta) = obj.types { + if &type_meta.kind == "ServiceAccount" { + self.seen_service_accounts.remove(&obj.namespaced_name()); + } + } + } + + let mut indices = vec![]; + for (i, obj) in event.data.applied_objs.iter().enumerate() { + let maybe_sa = get_service_account(obj, config)?; + if let Some(sa) = maybe_sa { + if !self.seen_service_accounts.contains(&sa) { + indices.push(i); + } + } + } + + Ok(indices) + } + + fn fixes(&self) -> Vec { + vec![remove_operation(format_ptr!("/spec/serviceAccount"))] + } + + fn reset(&mut self) {} +} + +pub fn validator() -> Validator { + Validator { + type_: ValidatorType::Error, + name: "service_account_missing", + help: HELP, + diagnostic: Arc::new(RwLock::new(ServiceAccountMissing::default())), + } +} diff --git a/sk-cli/src/validation/status_field_populated.rs b/sk-cli/src/validation/rules/status_field_populated.rs similarity index 82% rename from sk-cli/src/validation/status_field_populated.rs rename to sk-cli/src/validation/rules/status_field_populated.rs index eaabf8db..b82a8677 100644 --- a/sk-cli/src/validation/status_field_populated.rs +++ b/sk-cli/src/validation/rules/status_field_populated.rs @@ -4,9 +4,10 @@ use std::sync::{ }; use json_patch_ext::prelude::*; +use sk_store::TracerConfig; -use super::annotated_trace::AnnotatedTraceEvent; -use super::validator::{ +use crate::validation::annotated_trace::AnnotatedTraceEvent; +use crate::validation::validator::{ Diagnostic, Validator, ValidatorType, @@ -18,11 +19,11 @@ and shouldn't be applied "by hand". This is probably "fine" but it would be better to clean them up (and also they take up a lot of space."#; #[derive(Default)] -pub(super) struct StatusFieldPopulated {} +pub struct StatusFieldPopulated {} impl Diagnostic for StatusFieldPopulated { - fn check_next_event(&mut self, event: &mut AnnotatedTraceEvent) -> Vec { - event + fn check_next_event(&mut self, event: &mut AnnotatedTraceEvent, _: &TracerConfig) -> anyhow::Result> { + Ok(event .data .applied_objs .iter() @@ -39,7 +40,7 @@ impl Diagnostic for StatusFieldPopulated { } None }) - .collect() + .collect()) } fn fixes(&self) -> Vec { @@ -49,7 +50,7 @@ impl Diagnostic for StatusFieldPopulated { fn reset(&mut self) {} } -pub(super) fn validator() -> Validator { +pub fn validator() -> Validator { Validator { type_: ValidatorType::Warning, name: "status_field_populated", diff --git a/sk-cli/src/validation/rules/tests/mod.rs b/sk-cli/src/validation/rules/tests/mod.rs new file mode 100644 index 00000000..2a4b9609 --- /dev/null +++ b/sk-cli/src/validation/rules/tests/mod.rs @@ -0,0 +1,7 @@ +mod status_field_populated_test; + +use rstest::*; +use sk_core::prelude::*; + +use super::*; +use crate::validation::AnnotatedTraceEvent; diff --git a/sk-cli/src/validation/tests/status_field_populated_test.rs b/sk-cli/src/validation/rules/tests/status_field_populated_test.rs similarity index 100% rename from sk-cli/src/validation/tests/status_field_populated_test.rs rename to sk-cli/src/validation/rules/tests/status_field_populated_test.rs diff --git a/sk-cli/src/validation/tests/mod.rs b/sk-cli/src/validation/tests/mod.rs index 00fed317..b2fbcb84 100644 --- a/sk-cli/src/validation/tests/mod.rs +++ b/sk-cli/src/validation/tests/mod.rs @@ -1,5 +1,4 @@ mod annotated_trace_test; -mod status_field_populated_test; mod validation_store_test; use std::collections::BTreeMap; diff --git a/sk-cli/src/validation/validation_store.rs b/sk-cli/src/validation/validation_store.rs index fffe50c4..d97d459c 100644 --- a/sk-cli/src/validation/validation_store.rs +++ b/sk-cli/src/validation/validation_store.rs @@ -5,13 +5,13 @@ use lazy_static::lazy_static; use serde::Serialize; use sk_core::prelude::*; +use super::rules::*; use super::summary::ValidationSummary; use super::validator::{ Validator, ValidatorCode, }; use super::{ - status_field_populated, AnnotatedTrace, AnnotatedTracePatch, PatchLocations, @@ -32,7 +32,7 @@ impl ValidationStore { let mut summary = ValidationSummary::default(); let mut summary_populated = false; loop { - let s = trace.validate(&self.validators); + let s = trace.validate(&self.validators)?; if !summary_populated { summary.annotations = s; summary_populated = true; @@ -109,6 +109,7 @@ impl ValidationStore { let mut store = ValidationStore { validators: BTreeMap::new() }; store.register(status_field_populated::validator()); + store.register(service_account_missing::validator()); store } diff --git a/sk-cli/src/validation/validator.rs b/sk-cli/src/validation/validator.rs index 3c403980..d0bbb4af 100644 --- a/sk-cli/src/validation/validator.rs +++ b/sk-cli/src/validation/validator.rs @@ -11,6 +11,7 @@ use serde::{ Serialize, Serializer, }; +use sk_store::TracerConfig; use super::annotated_trace::AnnotatedTraceEvent; @@ -55,7 +56,11 @@ impl fmt::Display for ValidatorCode { } pub trait Diagnostic { - fn check_next_event(&mut self, evt: &mut AnnotatedTraceEvent) -> Vec; + fn check_next_event( + &mut self, + event: &mut AnnotatedTraceEvent, + config: &TracerConfig, + ) -> anyhow::Result>; fn fixes(&self) -> Vec; fn reset(&mut self); } @@ -74,8 +79,12 @@ pub struct Validator { } impl Validator { - pub fn check_next_event(&self, a_event: &mut AnnotatedTraceEvent) -> Vec { - self.diagnostic.write().unwrap().check_next_event(a_event) + pub fn check_next_event( + &self, + event: &mut AnnotatedTraceEvent, + config: &TracerConfig, + ) -> anyhow::Result> { + self.diagnostic.write().unwrap().check_next_event(event, config) } pub fn fixes(&self) -> Vec { diff --git a/sk-cli/src/xray/view/mod.rs b/sk-cli/src/xray/view/mod.rs index 25354b49..a8480540 100644 --- a/sk-cli/src/xray/view/mod.rs +++ b/sk-cli/src/xray/view/mod.rs @@ -194,6 +194,16 @@ fn render_object(app: &mut App, frame: &mut Frame, layout: Rect) { } } + // let contents = List::new(obj_str.split('\n').map(|s| { + // let mut span = Span::from(s); + // if s.contains("status") { + // span = span.style(Style::new().white().on_yellow()); + // } else if s.contains("serviceAccount") { + // span = span.style(Style::new().white().on_red()); + // } + // span + // })) + // .highlight_style(Style::new().bg(Color::Blue)); frame.render_stateful_widget(contents, layout, &mut app.object_contents_list_state); }