From f2a4e21bee87861693c8085b2683f2daa5c10c7f Mon Sep 17 00:00:00 2001 From: Bryant Biggs Date: Mon, 20 Feb 2023 11:29:04 -0500 Subject: [PATCH] feat: Add check `K8S005` for pod topology distribution --- .github/RELEASE.md | 1 + docs/process/checks.md | 4 - eksup/src/analysis.rs | 1 + eksup/src/k8s/checks.rs | 170 +++++++++++++++++++++++++++++-------- eksup/src/k8s/findings.rs | 4 + eksup/src/k8s/resources.rs | 36 ++++++++ 6 files changed, 178 insertions(+), 38 deletions(-) diff --git a/.github/RELEASE.md b/.github/RELEASE.md index d1ee8d5..936cfcd 100644 --- a/.github/RELEASE.md +++ b/.github/RELEASE.md @@ -29,5 +29,6 @@ This document captures the steps to follow when releasing a new version of `eksu 5. Update package on crates.io. Update the version of `eksup` used throughout the project as well as within `Cargo.toml`. Commit the changes and push to `main` before publishing the new version to crates.io. ```sh + cd eksup cargo publish ``` diff --git a/docs/process/checks.md b/docs/process/checks.md index f7df6cc..8eac0f8 100644 --- a/docs/process/checks.md +++ b/docs/process/checks.md @@ -193,8 +193,6 @@ The Kubernetes eviction API is the preferred method for draining nodes for repla #### K8S005 -!!! info "🚧 _Not yet implemented_" - **❌ Remediation required** Either `.spec.affinity.podAntiAffinity` or `.spec.topologySpreadConstraints` is set to avoid multiple pods from the same workload from being scheduled on the same node. @@ -211,8 +209,6 @@ Either `.spec.affinity.podAntiAffinity` or `.spec.topologySpreadConstraints` is #### K8S006 -**❌ Remediation required** - A `readinessProbe` must be set to ensure traffic is not sent to pods before they are ready following their re-deployment from a node replacement. #### K8S007 diff --git a/eksup/src/analysis.rs b/eksup/src/analysis.rs index 12830d7..60d053e 100644 --- a/eksup/src/analysis.rs +++ b/eksup/src/analysis.rs @@ -34,6 +34,7 @@ impl Results { output.push_str(&self.data_plane.version_skew.to_stdout_table()?); output.push_str(&self.kubernetes.min_replicas.to_stdout_table()?); output.push_str(&self.kubernetes.min_ready_seconds.to_stdout_table()?); + output.push_str(&self.kubernetes.pod_topology_distribution.to_stdout_table()?); output.push_str(&self.kubernetes.readiness_probe.to_stdout_table()?); Ok(output) diff --git a/eksup/src/k8s/checks.rs b/eksup/src/k8s/checks.rs index 5c65736..b6d6547 100644 --- a/eksup/src/k8s/checks.rs +++ b/eksup/src/k8s/checks.rs @@ -226,24 +226,56 @@ impl Findings for Vec { } } -#[derive(Debug, Serialize, Deserialize)] -pub(crate) struct PodDisruptionBudgetFinding { - pub(crate) resource: Resource, - /// Has pod associated pod disruption budget - /// TODO - more relevant information than just present? - pub(crate) remediation: finding::Remediation, - pub(crate) fcode: finding::Code, +#[derive(Debug, Serialize, Deserialize, Tabled)] +#[tabled(rename_all = "UpperCase")] +pub struct PodDisruptionBudget { + #[tabled(inline)] + pub finding: finding::Finding, + #[tabled(inline)] + pub resource: Resource, + // Has pod associated pod disruption budget + // TODO - more relevant information than just present? +} + +#[derive(Debug, Serialize, Deserialize, Tabled)] +#[tabled(rename_all = "UpperCase")] +pub struct PodTopologyDistribution { + #[tabled(inline)] + pub finding: finding::Finding, + #[tabled(inline)] + pub resource: Resource, + + pub anti_affinity: bool, + pub topology_spread_constraints: bool, } -#[derive(Debug, Serialize, Deserialize)] -pub(crate) struct PodTopologyDistributionFinding { - pub(crate) resource: Resource, - /// - pub(crate) anti_affinity: Option, - /// - pub(crate) toplogy_spread_constraints: Option, - pub(crate) remediation: finding::Remediation, - pub(crate) fcode: finding::Code, +impl Findings for Vec { + fn to_markdown_table(&self, leading_whitespace: &str) -> Result { + if self.is_empty() { + return Ok(format!( + "{leading_whitespace}✅ - All relevant Kubernetes workloads have either podAntiAffinity or topologySpreadConstraints set" + )); + } + + let mut table = Table::new(self); + table + .with(Disable::column(ByColumnName::new("CHECK"))) + .with(Margin::new(1, 0, 0, 0).set_fill('\t', 'x', 'x', 'x')) + .with(Style::markdown()); + + Ok(format!("{table}\n")) + } + + fn to_stdout_table(&self) -> Result { + if self.is_empty() { + return Ok("".to_owned()); + } + + let mut table = Table::new(self); + table.with(Style::sharp()); + + Ok(format!("{table}\n")) + } } #[derive(Debug, Serialize, Deserialize, Tabled)] @@ -253,7 +285,9 @@ pub struct Probe { pub finding: finding::Finding, #[tabled(inline)] - pub(crate) resource: Resource, + pub resource: Resource, + #[tabled(rename = "READINESS PROBE")] + pub readiness_probe: bool, } impl Findings for Vec { @@ -285,36 +319,104 @@ impl Findings for Vec { } } -#[derive(Debug, Serialize, Deserialize)] -pub(crate) struct TerminationGracePeriodFinding { - pub(crate) resource: Resource, +#[derive(Debug, Serialize, Deserialize, Tabled)] +#[tabled(rename_all = "UpperCase")] +pub struct TerminationGracePeriod { + #[tabled(inline)] + pub finding: finding::Finding, + + #[tabled(inline)] + pub resource: Resource, /// Min ready seconds - pub(crate) seconds: i32, - pub(crate) remediation: finding::Remediation, - pub(crate) fcode: finding::Code, + pub seconds: i32, +} + +impl Findings for Vec { + fn to_markdown_table(&self, leading_whitespace: &str) -> Result { + if self.is_empty() { + return Ok(format!( + "{leading_whitespace}✅ - No StatefulSet workloads have a terminationGracePeriodSeconds set to more than 0" + )); + } + + let mut table = Table::new(self); + table + .with(Disable::column(ByColumnName::new("CHECK"))) + .with(Margin::new(1, 0, 0, 0).set_fill('\t', 'x', 'x', 'x')) + .with(Style::markdown()); + + Ok(format!("{table}\n")) + } + + fn to_stdout_table(&self) -> Result { + if self.is_empty() { + return Ok("".to_owned()); + } + + let mut table = Table::new(self); + table.with(Style::sharp()); + + Ok(format!("{table}\n")) + } +} + +#[derive(Debug, Serialize, Deserialize, Tabled)] +#[tabled(rename_all = "UpperCase")] +pub struct DockerSocket { + #[tabled(inline)] + pub finding: finding::Finding, + + #[tabled(inline)] + pub resource: Resource, } -#[derive(Debug, Serialize, Deserialize)] -pub(crate) struct DockerSocketFinding { - pub(crate) resource: Resource, - /// - pub(crate) volumes: Vec, - pub(crate) remediation: finding::Remediation, - pub(crate) fcode: finding::Code, +impl Findings for Vec { + fn to_markdown_table(&self, leading_whitespace: &str) -> Result { + if self.is_empty() { + return Ok(format!( + "{leading_whitespace}✅ - No relevant Kubernetes workloads are found to be utilizing the Docker socket" + )); + } + + let mut table = Table::new(self); + table + .with(Disable::column(ByColumnName::new("CHECK"))) + .with(Margin::new(1, 0, 0, 0).set_fill('\t', 'x', 'x', 'x')) + .with(Style::markdown()); + + Ok(format!("{table}\n")) + } + + fn to_stdout_table(&self) -> Result { + if self.is_empty() { + return Ok("".to_owned()); + } + + let mut table = Table::new(self); + table.with(Style::sharp()); + + Ok(format!("{table}\n")) + } } pub trait K8sFindings { fn get_resource(&self) -> Resource; + /// K8S002 - check if resources contain a minimum of 3 replicas fn min_replicas(&self) -> Option; + /// K8S003 - check if resources contain minReadySeconds > 0 fn min_ready_seconds(&self) -> Option; + // /// K8S004 - check if resources have associated podDisruptionBudgets - // fn pod_disruption_budget(&self) -> Result>; - // /// K8S005 - check if resources have podAntiAffinity or topologySpreadConstraints - // fn pod_topology_distribution(&self) -> Result>; + // fn pod_disruption_budget(&self) -> Option; + + /// K8S005 - check if resources have podAntiAffinity or topologySpreadConstraints + fn pod_topology_distribution(&self) -> Option; + /// K8S006 - check if resources have readinessProbe fn readiness_probe(&self) -> Option; + // /// K8S008 - check if resources use the Docker socket - // fn docker_socket(&self) -> Result>; + // fn docker_socket(&self) -> Option; } diff --git a/eksup/src/k8s/findings.rs b/eksup/src/k8s/findings.rs index 22cc487..9eaf81a 100644 --- a/eksup/src/k8s/findings.rs +++ b/eksup/src/k8s/findings.rs @@ -12,6 +12,7 @@ pub struct KubernetesFindings { pub min_replicas: Vec, pub min_ready_seconds: Vec, pub readiness_probe: Vec, + pub pod_topology_distribution: Vec, } pub async fn get_kubernetes_findings(k8s_client: &K8sClient) -> Result { @@ -21,10 +22,13 @@ pub async fn get_kubernetes_findings(k8s_client: &K8sClient) -> Result = resources.iter().filter_map(|s| s.min_ready_seconds()).collect(); let readiness_probe: Vec = resources.iter().filter_map(|s| s.readiness_probe()).collect(); + let pod_topology_distribution: Vec = + resources.iter().filter_map(|s| s.pod_topology_distribution()).collect(); Ok(KubernetesFindings { min_replicas, min_ready_seconds, readiness_probe, + pod_topology_distribution, }) } diff --git a/eksup/src/k8s/resources.rs b/eksup/src/k8s/resources.rs index a6de6a1..58f0d43 100644 --- a/eksup/src/k8s/resources.rs +++ b/eksup/src/k8s/resources.rs @@ -405,6 +405,7 @@ impl checks::K8sFindings for StdResource { return Some(checks::Probe { finding, resource: self.get_resource(), + readiness_probe: !container.readiness_probe.is_none(), }); } } @@ -413,6 +414,41 @@ impl checks::K8sFindings for StdResource { None => None, } } + + fn pod_topology_distribution(&self) -> Option { + let pod_template = self.spec.template.to_owned(); + + let resource = self.get_resource(); + match resource.kind { + Kind::DaemonSet | Kind::Job | Kind::CronJob => return None, + _ => (), + } + + match pod_template { + Some(pod_template) => { + let pod_spec = pod_template.spec.unwrap_or_default(); + if pod_spec.affinity.is_none() && pod_spec.topology_spread_constraints.is_none() { + let remediation = finding::Remediation::Required; + let finding = finding::Finding { + code: finding::Code::K8S005, + symbol: remediation.symbol(), + remediation, + }; + + // As soon as we find one container without a readiness probe, we return the finding + Some(checks::PodTopologyDistribution { + finding, + resource: self.get_resource(), + anti_affinity: !pod_spec.affinity.is_none(), + topology_spread_constraints: !pod_spec.topology_spread_constraints.is_none(), + }) + } else { + None + } + } + None => None, + } + } } pub async fn get_resources(client: &Client) -> Result> {