feat: service_account_missing validation check

acrlabs · Dec 19, 2024 · ca1bbdf · ca1bbdf
1 parent e2d2644
commit ca1bbdf
Show file tree

Hide file tree

Showing 14 changed files with 263 additions and 11 deletions.
diff --git a/sk-cli/src/validation/annotated_trace.rs b/sk-cli/src/validation/annotated_trace.rs
@@ -124,7 +124,7 @@ impl AnnotatedTrace {
         for event in self.events.iter_mut() {
             event.clear_annotations();
             for (code, validator) in validators.iter() {
-                let event_patches = validator.check_next_event(event)?;
+                let event_patches = validator.check_next_event(event, self.base.config())?;
                 let count = event_patches.len();
                 summary.entry(*code).and_modify(|e| *e += count).or_insert(count);
 

diff --git a/sk-cli/src/validation/rules/mod.rs b/sk-cli/src/validation/rules/mod.rs
@@ -1,3 +1,4 @@
+pub mod service_account_missing;
 pub mod status_field_populated;
 
 #[cfg(test)]

diff --git a/sk-cli/src/validation/rules/service_account_missing.rs b/sk-cli/src/validation/rules/service_account_missing.rs
@@ -0,0 +1,92 @@
+use std::collections::{
+    BTreeMap,
+    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::validator::{
+    CheckResult,
+    Diagnostic,
+    Validator,
+    ValidatorType,
+};
+use crate::validation::{
+    AnnotatedTraceEvent,
+    AnnotatedTracePatch,
+    PatchLocations,
+};
+
+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 {
+    pub(crate) seen_service_accounts: HashSet<String>,
+}
+
+impl Diagnostic for ServiceAccountMissing {
+    fn check_next_event(&mut self, event: &mut AnnotatedTraceEvent, config: &TracerConfig) -> CheckResult {
+        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 patches = vec![];
+        for (i, obj) in event.data.applied_objs.iter().enumerate() {
+            let gvk = GVK::from_dynamic_obj(obj)?;
+            if let Some(pod_spec_template_path) = config.pod_spec_template_path(&gvk) {
+                let sa_ptrs = [
+                    // serviceAccount is deprecated but still supported (for now)
+                    format_ptr!("{pod_spec_template_path}/spec/serviceAccount"),
+                    format_ptr!("{pod_spec_template_path}/spec/serviceAccountName"),
+                ];
+                if let Some(sa) = sa_ptrs.iter().filter_map(|ptr| ptr.resolve(&obj.data).ok()).next() {
+                    if !self.seen_service_accounts.contains(sa.as_str().expect("expected string")) {
+                        let fix = AnnotatedTracePatch {
+                            locations: PatchLocations::ObjectReference(
+                                obj.types.clone().unwrap_or_default(),
+                                obj.namespaced_name(),
+                            ),
+                            ops: sa_ptrs.iter().map(|ptr| remove_operation(ptr.clone())).collect(),
+                        };
+                        patches.push((i, vec![fix]));
+                    }
+                }
+            }
+        }
+
+        Ok(BTreeMap::from_iter(patches))
+    }
+
+    fn reset(&mut self) {
+        self.seen_service_accounts.clear();
+    }
+}
+
+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/rules/status_field_populated.rs b/sk-cli/src/validation/rules/status_field_populated.rs
@@ -5,6 +5,7 @@ use std::sync::{
 
 use json_patch_ext::prelude::*;
 use lazy_static::lazy_static;
+use sk_store::TracerConfig;
 
 use crate::validation::validator::{
     CheckResult,
@@ -34,7 +35,7 @@ lazy_static! {
 }
 
 impl Diagnostic for StatusFieldPopulated {
-    fn check_next_event(&mut self, event: &mut AnnotatedTraceEvent) -> CheckResult {
+    fn check_next_event(&mut self, event: &mut AnnotatedTraceEvent, _: &TracerConfig) -> CheckResult {
         Ok(event
             .data
             .applied_objs

diff --git a/sk-cli/src/validation/rules/tests/mod.rs b/sk-cli/src/validation/rules/tests/mod.rs
@@ -1,7 +1,30 @@
 mod status_field_populated_test;
 
+use std::collections::HashMap;
+
 use rstest::*;
 use sk_core::prelude::*;
+use sk_store::{
+    TracerConfig,
+    TrackedObjectConfig,
+};
 
 use super::*;
+use crate::validation::validator::Diagnostic;
 use crate::validation::AnnotatedTraceEvent;
+
+#[fixture]
+fn test_trace_config() -> TracerConfig {
+    TracerConfig {
+        tracked_objects: HashMap::from([
+            (
+                DEPL_GVK.clone(),
+                TrackedObjectConfig {
+                    pod_spec_template_path: Some("/spec/template".into()),
+                    ..Default::default()
+                },
+            ),
+            (SA_GVK.clone(), Default::default()),
+        ]),
+    }
+}
diff --git a/sk-cli/src/validation/rules/tests/service_account_missing_test.rs b/sk-cli/src/validation/rules/tests/service_account_missing_test.rs
@@ -0,0 +1,113 @@
+use assertables::*;
+use serde_json::json;
+use sk_store::{
+    TraceEvent,
+    TracerConfig,
+};
+
+use super::service_account_missing::ServiceAccountMissing;
+use super::*;
+
+#[fixture]
+fn depl_event(test_deployment: DynamicObject, #[default("serviceAccount")] sa_key: &str) -> AnnotatedTraceEvent {
+    AnnotatedTraceEvent {
+        data: TraceEvent {
+            ts: 1,
+            applied_objs: vec![test_deployment.data(json!({"spec": {"template": {"spec": {sa_key: "foobar"}}}}))],
+            deleted_objs: vec![],
+        },
+        ..Default::default()
+    }
+}
+
+#[fixture]
+fn sa_event(test_service_account: DynamicObject) -> AnnotatedTraceEvent {
+    AnnotatedTraceEvent {
+        data: TraceEvent {
+            ts: 0,
+            applied_objs: vec![test_service_account],
+            deleted_objs: vec![],
+        },
+        ..Default::default()
+    }
+}
+
+#[rstest]
+#[case("serviceAccount")]
+#[case("serviceAccountName")]
+fn test_service_account_missing(test_deployment: DynamicObject, test_trace_config: TracerConfig, #[case] sa_key: &str) {
+    let mut v = ServiceAccountMissing::default();
+    let mut evt = depl_event(test_deployment, sa_key);
+    let annotations = v.check_next_event(&mut evt, &test_trace_config).unwrap();
+
+    assert_eq!(annotations.keys().collect::<Vec<_>>(), vec![&0]);
+}
+
+#[rstest]
+fn test_service_account_missing_deleted(
+    mut depl_event: AnnotatedTraceEvent,
+    mut sa_event: AnnotatedTraceEvent,
+    test_service_account: DynamicObject,
+    test_trace_config: TracerConfig,
+) {
+    let mut v = ServiceAccountMissing::default();
+    let mut sa_event_del = AnnotatedTraceEvent {
+        data: TraceEvent {
+            ts: 0,
+            applied_objs: vec![],
+            deleted_objs: vec![test_service_account],
+        },
+        ..Default::default()
+    };
+    v.check_next_event(&mut sa_event, &test_trace_config).unwrap();
+    v.check_next_event(&mut sa_event_del, &test_trace_config).unwrap();
+    let annotations = v.check_next_event(&mut depl_event, &test_trace_config).unwrap();
+
+    assert_eq!(annotations.keys().collect::<Vec<_>>(), vec![&0]);
+}
+
+#[rstest]
+fn test_service_account_not_missing(
+    mut depl_event: AnnotatedTraceEvent,
+    mut sa_event: AnnotatedTraceEvent,
+    test_trace_config: TracerConfig,
+) {
+    let mut v = ServiceAccountMissing::default();
+    v.check_next_event(&mut sa_event, &test_trace_config).unwrap();
+    let annotations = v.check_next_event(&mut depl_event, &test_trace_config).unwrap();
+
+    assert_eq!(annotations.keys().collect::<Vec<_>>(), vec![&0]);
+}
+
+#[rstest]
+fn test_service_account_not_missing_same_evt(
+    test_deployment: DynamicObject,
+    test_service_account: DynamicObject,
+    test_trace_config: TracerConfig,
+) {
+    let mut v = ServiceAccountMissing::default();
+    let mut depl_evt = AnnotatedTraceEvent {
+        data: TraceEvent {
+            ts: 1,
+            applied_objs: vec![
+                test_deployment
+                    .data(json!({"spec": {"template": {"spec": {"serviceAccountName": TEST_SERVICE_ACCOUNT}}}})),
+                test_service_account,
+            ],
+            deleted_objs: vec![],
+        },
+        ..Default::default()
+    };
+    let annotations = v.check_next_event(&mut depl_evt, &test_trace_config).unwrap();
+
+    assert_eq!(annotations.keys().collect::<Vec<_>>(), vec![&0]);
+}
+
+#[rstest]
+fn test_service_account_reset(mut depl_event: AnnotatedTraceEvent, test_trace_config: TracerConfig) {
+    let mut v = ServiceAccountMissing::default();
+    v.check_next_event(&mut depl_event, &test_trace_config).unwrap();
+    v.reset();
+
+    assert_is_empty!(v.seen_service_accounts);
+}
diff --git a/sk-cli/src/validation/rules/tests/status_field_populated_test.rs b/sk-cli/src/validation/rules/tests/status_field_populated_test.rs
@@ -15,7 +15,7 @@ fn test_status_field_populated(test_deployment: DynamicObject) {
         },
         ..Default::default()
     };
-    let annotations = v.check_next_event(&mut evt).unwrap();
+    let annotations = v.check_next_event(&mut evt, &Default::default()).unwrap();
     assert_eq!(annotations.keys().collect::<Vec<_>>(), vec![&0]);
 }
 
@@ -30,6 +30,6 @@ fn test_status_field_not_populated(test_deployment: DynamicObject) {
         },
         ..Default::default()
     };
-    let annotations = v.check_next_event(&mut evt).unwrap();
+    let annotations = v.check_next_event(&mut evt, &Default::default()).unwrap();
     assert_is_empty!(annotations);
 }
diff --git a/sk-cli/src/validation/tests/mod.rs b/sk-cli/src/validation/tests/mod.rs
@@ -10,7 +10,10 @@ use std::sync::{
 use json_patch_ext::prelude::*;
 use rstest::*;
 use sk_core::prelude::*;
-use sk_store::TraceEvent;
+use sk_store::{
+    TraceEvent,
+    TracerConfig,
+};
 
 use super::annotated_trace::AnnotatedTraceEvent;
 use super::validator::{
@@ -53,7 +56,7 @@ pub fn annotated_trace() -> AnnotatedTrace {
 struct TestDiagnostic {}
 
 impl Diagnostic for TestDiagnostic {
-    fn check_next_event(&mut self, evt: &mut AnnotatedTraceEvent) -> CheckResult {
+    fn check_next_event(&mut self, evt: &mut AnnotatedTraceEvent, _: &TracerConfig) -> CheckResult {
         if evt.data.applied_objs.len() > 1 && evt.data.applied_objs[1].data.get("foo").is_none() {
             Ok(BTreeMap::from([(
                 1,

diff --git a/sk-cli/src/validation/tests/validation_store_test.rs b/sk-cli/src/validation/tests/validation_store_test.rs
@@ -26,7 +26,6 @@ fn test_fix_trace(test_validation_store: ValidationStore, mut annotated_trace: A
     let summary = test_validation_store.validate_trace(&mut annotated_trace, true).unwrap();
 
     for event in annotated_trace.iter() {
-        println!("{:?}", event.data);
         if event.data.applied_objs.len() > 1 {
             assert_eq!(event.data.applied_objs[1].data.get("foo").unwrap(), "bar");
         }

diff --git a/sk-cli/src/validation/validation_store.rs b/sk-cli/src/validation/validation_store.rs
@@ -97,6 +97,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
@@ -11,6 +11,7 @@ use serde::{
     Serialize,
     Serializer,
 };
+use sk_store::TracerConfig;
 
 use super::annotated_trace::{
     AnnotatedTraceEvent,
@@ -60,7 +61,7 @@ impl fmt::Display for ValidatorCode {
 pub type CheckResult = anyhow::Result<BTreeMap<usize, Vec<AnnotatedTracePatch>>>;
 
 pub trait Diagnostic {
-    fn check_next_event(&mut self, event: &mut AnnotatedTraceEvent) -> CheckResult;
+    fn check_next_event(&mut self, event: &mut AnnotatedTraceEvent, config: &TracerConfig) -> CheckResult;
     fn reset(&mut self);
 }
 
@@ -78,8 +79,8 @@ pub struct Validator {
 }
 
 impl Validator {
-    pub fn check_next_event(&self, event: &mut AnnotatedTraceEvent) -> CheckResult {
-        self.diagnostic.write().unwrap().check_next_event(event)
+    pub fn check_next_event(&self, event: &mut AnnotatedTraceEvent, config: &TracerConfig) -> CheckResult {
+        self.diagnostic.write().unwrap().check_next_event(event, config)
     }
 
     pub fn reset(&self) {

diff --git 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);
 }
 

diff --git a/sk-core/src/constants.rs b/sk-core/src/constants.rs
@@ -36,6 +36,8 @@ mod test_constants {
 
     pub const EMPTY_POD_SPEC_HASH: u64 = 17506812802394981455;
     pub const TEST_DEPLOYMENT: &str = "the-deployment";
+    pub const TEST_DAEMONSET: &str = "the-daemonset";
+    pub const TEST_SERVICE_ACCOUNT: &str = "the-service-account";
     pub const TEST_NAMESPACE: &str = "test-namespace";
     pub const TEST_SIM_NAME: &str = "test-sim";
     pub const TEST_SIM_ROOT_NAME: &str = "test-sim-root";
@@ -47,6 +49,7 @@ mod test_constants {
     lazy_static! {
         pub static ref DEPL_GVK: GVK = GVK::new("apps", "v1", "Deployment");
         pub static ref DS_GVK: GVK = GVK::new("apps", "v1", "DaemonSet");
+        pub static ref SA_GVK: GVK = GVK::new("", "v1", "ServiceAccount");
     }
 }
 

diff --git a/sk-core/src/k8s/testutils/objs.rs b/sk-core/src/k8s/testutils/objs.rs
@@ -16,8 +16,13 @@ pub fn test_deployment(#[default(TEST_DEPLOYMENT)] name: &str) -> DynamicObject
 }
 
 #[fixture]
-pub fn test_daemonset(#[default(TEST_DEPLOYMENT)] name: &str) -> DynamicObject {
+pub fn test_daemonset(#[default(TEST_DAEMONSET)] name: &str) -> DynamicObject {
     DynamicObject::new(&name, &ApiResource::from_gvk(&DS_GVK))
         .within(TEST_NAMESPACE)
         .data(json!({"spec": {"updateStrategy": {"type": "onDelete"}}}))
 }
+
+#[fixture]
+pub fn test_service_account(#[default(TEST_SERVICE_ACCOUNT)] name: &str) -> DynamicObject {
+    DynamicObject::new(&name, &ApiResource::from_gvk(&SA_GVK)).within(TEST_NAMESPACE)
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -97,6 +97,7 @@ impl ValidationStore { @@
             let mut store = ValidationStore { validators: BTreeMap::new() };
             store.register(status_field_populated::validator());
+            store.register(service_account_missing::validator());
             store
         }
@@ Expand Down @@