feat: service_account_missing validation check

.pre-commit-config.yaml

@@ -52,4 +52,4 @@ repos:
         language: system
         entry: bash -c 'make validation_rules && git diff --quiet'
         pass_filenames: false
-        files: "sk-store/src/validation/.*"
+        files: "sk-cli/src/validation/.*"

Makefile

@@ -88,7 +88,7 @@ crd: skctl
 pre-k8s:: crd
 
 .PHONY: validation_rules
-validation_rules: VALIDATION_FILE=sk-cli/src/validation/README.md
+validation_rules: VALIDATION_FILE=sk-cli/src/validation/rules/README.md
 validation_rules: skctl
 	printf "# SimKube Trace Validation Checks\n\n" > $(VALIDATION_FILE)
 	$(BUILD_DIR)/skctl validate print --format table >> $(VALIDATION_FILE)

sk-cli/src/validation/annotated_trace.rs

@@ -123,7 +123,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);
 

sk-cli/src/validation/rules/README.md

@@ -0,0 +1,8 @@
+# SimKube Trace Validation Checks
+
+| code | name | description |
+|---|---|---|
+| W0000 | status_field_populated | Indicates that the status field of a Kubernetes object in the trace is non-empty; status fields are updated by their controlling objects 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. |
+| E0001 | service_account_missing | 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. |
+
+This file is auto-generated; to rebuild, run `make validation_rules`.

sk-cli/src/validation/rules/mod.rs

@@ -1,3 +1,4 @@
+pub mod service_account_missing;
 pub mod status_field_populated;
 
 #[cfg(test)]

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())),
+    }
+}

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

sk-cli/src/validation/rules/tests/mod.rs

@@ -1,7 +1,31 @@
+mod service_account_missing_test;
 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()),
+        ]),
+    }
+}

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);
+}

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

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,

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
     }

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) {

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");
     }
 }