diff --git a/Cargo.lock b/Cargo.lock index c5b331eb..9f02b0a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -139,9 +139,9 @@ dependencies = [ [[package]] name = "assertables" -version = "7.0.1" +version = "8.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c24e9d990669fbd16806bff449e4ac644fd9b1fca014760087732fe4102f131" +checksum = "857057651cdf1fe4bc1e8308493c752db559df0330f23b45f532f6b24c2b443d" [[package]] name = "async-channel" diff --git a/Cargo.toml b/Cargo.toml index 7f64c31f..e44eec52 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,7 +61,7 @@ tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } url = "2.4.1" # test dependencies -assertables = "7.0.1" +assertables = "8.18.0" http = "0.2.9" httpmock = "0.6.8" hyper = "0.14.27" diff --git a/k8s/kustomize/prod/sk-tracer.yml b/k8s/kustomize/prod/sk-tracer.yml index 63eaf333..2d0bef5b 100644 --- a/k8s/kustomize/prod/sk-tracer.yml +++ b/k8s/kustomize/prod/sk-tracer.yml @@ -40,6 +40,8 @@ data: trackedObjects: apps/v1.Deployment: podSpecTemplatePath: /spec/template + v1.ServiceAccount: {} + v1.ConfigMap: {} --- apiVersion: apps/v1 kind: Deployment diff --git a/k8s/sk_tracer.py b/k8s/sk_tracer.py index 7a1193c2..63526076 100644 --- a/k8s/sk_tracer.py +++ b/k8s/sk_tracer.py @@ -8,6 +8,8 @@ trackedObjects: apps/v1.Deployment: podSpecTemplatePath: /spec/template + v1.ServiceAccount: {} + v1.ConfigMap: {} """ CONFIGMAP_NAME = "tracer-config" diff --git a/sk-cli/src/export.rs b/sk-cli/src/export.rs index a91e085c..4d1126f0 100644 --- a/sk-cli/src/export.rs +++ b/sk-cli/src/export.rs @@ -38,7 +38,8 @@ pub struct Args { long, long_help = "namespaces to exclude from the trace", value_delimiter = ',', - default_value = "cert-manager,kube-system,local-path-storage,monitoring,simkube" + default_value = "cert-manager,default,kube-public,kube-node-lease,kube-system,\ + local-path-storage,monitoring,simkube" )] pub excluded_namespaces: Vec, diff --git a/sk-core/src/k8s/gvk.rs b/sk-core/src/k8s/gvk.rs index 1c334a07..12c46863 100644 --- a/sk-core/src/k8s/gvk.rs +++ b/sk-core/src/k8s/gvk.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::fmt; use std::ops::Deref; @@ -20,6 +21,8 @@ use crate::prelude::*; // custom serialization methods. We also add some handy helper/conversion functions. // // Specifically for serialization/deserialization, we convert to the format "group/version.kind". +// (unless the group is "core", and then we serialize to "version.kind", but can deserialize from +// either "version.kind" or "/version.kind" for backwards compatibility) #[derive(Clone, Debug, Hash, Eq, PartialEq)] pub struct GVK(GroupVersionKind); @@ -62,7 +65,12 @@ impl Serialize for GVK { where S: Serializer, { - let skey = format!("{}/{}.{}", self.0.group, self.0.version, self.0.kind); + let mut group = Cow::from(&self.0.group); + if !group.is_empty() { + group.to_mut().push('/'); + } + + let skey = format!("{group}{}.{}", self.0.version, self.0.kind); serializer.serialize_str(&skey) } } @@ -81,16 +89,18 @@ impl<'de> de::Visitor<'de> for GVKVisitor { E: de::Error, { let p1: Vec<_> = value.split('/').collect(); - if p1.len() != 2 { - return Err(E::custom(format!("invalid format for gvk: {value}"))); - } - let p2: Vec<_> = p1[1].split('.').collect(); - if p2.len() != 2 { - return Err(E::custom(format!("invalid format for gvk: {value}"))); - } - - let parts = [p1[0], p2[0], p2[1]]; - Ok(GVK(GroupVersionKind::gvk(parts[0], parts[1], parts[2]))) + let (group, rest) = match p1.len() { + 2 => (p1[0], p1[1]), + 1 => ("", p1[0]), + _ => return Err(E::custom(format!("invalid format for gvk: {value}"))), + }; + let p2: Vec<_> = rest.split('.').collect(); + let (version, kind) = match p2.len() { + 2 => (p2[0], p2[1]), + _ => return Err(E::custom(format!("invalid format for gvk: {value}"))), + }; + + Ok(GVK(GroupVersionKind::gvk(group, version, kind))) } } @@ -102,3 +112,42 @@ impl<'de> Deserialize<'de> for GVK { deserializer.deserialize_str(GVKVisitor) } } + +#[cfg(test)] +mod test { + use assertables::*; + use rstest::*; + use serde::de::value::{ + Error as SerdeError, + StrDeserializer, + }; + use serde::de::IntoDeserializer; + + use super::*; + + #[rstest] + fn test_serialize() { + // I had to think about this for a minute, but strings in JSON have to include quotes, + // which is why they're escaped out here. + assert_eq!(serde_json::to_string(&GVK::new("foo", "v1", "bar")).unwrap(), "\"foo/v1.bar\""); + assert_eq!(serde_json::to_string(&GVK::new("", "v1", "bar")).unwrap(), "\"v1.bar\""); + } + + #[rstest] + fn test_deserialize() { + let d1: StrDeserializer = "foo/v1.bar".into_deserializer(); + assert_eq!(GVK::deserialize(d1).unwrap(), GVK::new("foo", "v1", "bar")); + + let d2: StrDeserializer = "/v1.bar".into_deserializer(); + assert_eq!(GVK::deserialize(d2).unwrap(), GVK::new("", "v1", "bar")); + + let d3: StrDeserializer = "v1.bar".into_deserializer(); + assert_eq!(GVK::deserialize(d3).unwrap(), GVK::new("", "v1", "bar")); + + let d4: StrDeserializer = "asdf".into_deserializer(); + assert_err!(GVK::deserialize(d4)); + + let d5: StrDeserializer = "foo/asdf/asdf".into_deserializer(); + assert_err!(GVK::deserialize(d5)); + } +} diff --git a/sk-driver/src/runner.rs b/sk-driver/src/runner.rs index 211cbf28..16736a55 100644 --- a/sk-driver/src/runner.rs +++ b/sk-driver/src/runner.rs @@ -62,7 +62,7 @@ pub fn build_virtual_obj( original_ns: &str, virtual_ns: &str, obj: &DynamicObject, - pod_spec_template_path: &str, + maybe_pod_spec_template_path: Option<&str>, ) -> anyhow::Result { let owner = root; let mut vobj = obj.clone(); @@ -70,30 +70,36 @@ pub fn build_virtual_obj( vobj.metadata.namespace = Some(virtual_ns.into()); klabel_insert!(vobj, VIRTUAL_LABEL_KEY => "true"); - jsonutils::patch_ext::add(pod_spec_template_path, "metadata", &json!({}), &mut vobj.data, false)?; - jsonutils::patch_ext::add( - &format!("{}/metadata", pod_spec_template_path), - "annotations", - &json!({}), - &mut vobj.data, - false, - )?; - jsonutils::patch_ext::add( - &format!("{}/metadata/annotations", pod_spec_template_path), - ORIG_NAMESPACE_ANNOTATION_KEY, - &json!(original_ns), - &mut vobj.data, - true, - )?; - jsonutils::patch_ext::remove("", "status", &mut vobj.data)?; - - // We remove all container ports from the pod specification just before applying, because it is - // _possible_ to create a pod with duplicate container ports, but the apiserver will _reject_ a - // patch containing duplicate container ports. Since pods are mocked out _anyways_ there's no - // reason to expose the ports. We do this here because we still want the ports to be a part of - // the podspec when we're computing its hash, i.e., changes to the container ports will still - // result in changes to the pod in the trace/simulation - jsonutils::patch_ext::remove(&format!("{}/spec/containers/*", pod_spec_template_path), "ports", &mut vobj.data)?; + if let Some(pod_spec_template_path) = maybe_pod_spec_template_path { + jsonutils::patch_ext::add(pod_spec_template_path, "metadata", &json!({}), &mut vobj.data, false)?; + jsonutils::patch_ext::add( + &format!("{}/metadata", pod_spec_template_path), + "annotations", + &json!({}), + &mut vobj.data, + false, + )?; + jsonutils::patch_ext::add( + &format!("{}/metadata/annotations", pod_spec_template_path), + ORIG_NAMESPACE_ANNOTATION_KEY, + &json!(original_ns), + &mut vobj.data, + true, + )?; + jsonutils::patch_ext::remove("", "status", &mut vobj.data)?; + + // We remove all container ports from the pod specification just before applying, because it is + // _possible_ to create a pod with duplicate container ports, but the apiserver will _reject_ a + // patch containing duplicate container ports. Since pods are mocked out _anyways_ there's no + // reason to expose the ports. We do this here because we still want the ports to be a part of + // the podspec when we're computing its hash, i.e., changes to the container ports will still + // result in changes to the pod in the trace/simulation + jsonutils::patch_ext::remove( + &format!("{}/spec/containers/*", pod_spec_template_path), + "ports", + &mut vobj.data, + )?; + } Ok(vobj) } @@ -132,11 +138,7 @@ pub async fn run_trace(ctx: DriverContext, client: kube::Client) -> EmptyResult ns_api.create(&Default::default(), &vns).await?; } - let pod_spec_template_path = ctx - .store - .config() - .pod_spec_template_path(&gvk) - .ok_or(anyhow!("unknown simulated object: {:?}", gvk))?; + let pod_spec_template_path = ctx.store.config().pod_spec_template_path(&gvk); let vobj = build_virtual_obj(&ctx, &root_obj, &original_ns, &virtual_ns, obj, pod_spec_template_path)?; info!("applying object {}", vobj.namespaced_name()); @@ -149,8 +151,11 @@ pub async fn run_trace(ctx: DriverContext, client: kube::Client) -> EmptyResult for obj in &evt.deleted_objs { info!("deleting object {}", obj.namespaced_name()); + let virtual_ns = format!("{}-{}", ctx.virtual_ns_prefix, obj.namespace().unwrap()); + let mut vobj = obj.clone(); + vobj.metadata.namespace = Some(virtual_ns); apiset - .api_for_obj(obj) + .api_for_obj(&vobj) .await? .delete(&obj.name_any(), &Default::default()) .await?; diff --git a/sk-driver/src/tests/data/trace.json b/sk-driver/src/tests/data/trace.json index a0b58650..7aab755f 100644 --- a/sk-driver/src/tests/data/trace.json +++ b/sk-driver/src/tests/data/trace.json @@ -93,7 +93,17 @@ } } ], - "deleted_objs": [] + "deleted_objs": [ + { + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "nginx-deployment-2", + "namespace": "default" + }, + "spec": {} + } + ] } ], { diff --git a/sk-driver/src/tests/runner_test.rs b/sk-driver/src/tests/runner_test.rs index a6eedb16..fc95b363 100644 --- a/sk-driver/src/tests/runner_test.rs +++ b/sk-driver/src/tests/runner_test.rs @@ -152,6 +152,12 @@ async fn itest_run(#[case] has_start_marker: bool) { )); then.json_body(status_ok()); }) + .handle(|when, then| { + when.method(DELETE).path(format!( + "/apis/apps/v1/namespaces/{TEST_VIRT_NS_PREFIX}-{TEST_NS_NAME}/deployments/nginx-deployment-2" + )); + then.json_body(status_ok()); + }) .handle(|when, then| { when.path(format!("/apis/simkube.io/v1/simulationroots/{TEST_DRIVER_ROOT_NAME}")) .method(DELETE); diff --git a/sk-store/src/config.rs b/sk-store/src/config.rs index ce59efce..ecb87578 100644 --- a/sk-store/src/config.rs +++ b/sk-store/src/config.rs @@ -11,7 +11,7 @@ use sk_core::k8s::GVK; #[derive(Clone, Debug, Default, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct TrackedObjectConfig { - pub pod_spec_template_path: String, + pub pod_spec_template_path: Option, #[serde(default, skip_serializing_if = "<&bool>::not")] pub track_lifecycle: bool, @@ -29,7 +29,7 @@ impl TracerConfig { } pub fn pod_spec_template_path(&self, gvk: &GVK) -> Option<&str> { - Some(&self.tracked_objects.get(gvk)?.pod_spec_template_path) + self.tracked_objects.get(gvk)?.pod_spec_template_path.as_deref() } pub fn track_lifecycle_for(&self, gvk: &GVK) -> bool { diff --git a/sk-store/src/tests/trace_store_test.rs b/sk-store/src/tests/trace_store_test.rs index 54aa970f..c6aa05d0 100644 --- a/sk-store/src/tests/trace_store_test.rs +++ b/sk-store/src/tests/trace_store_test.rs @@ -19,7 +19,7 @@ fn tracer() -> TraceStore { GVK::new("apps", "v1", "Deployment"), TrackedObjectConfig { track_lifecycle: true, - pod_spec_template_path: "/spec/template".into(), + pod_spec_template_path: Some("/spec/template".into()), }, )]), })