Skip to content

Commit

Permalink
feat: fix issues discovered by validation
Browse files Browse the repository at this point in the history
  • Loading branch information
drmorr0 committed Nov 21, 2024
1 parent 21b5fe5 commit 2ebdc19
Show file tree
Hide file tree
Showing 30 changed files with 176 additions and 41 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions sk-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ testutils = []

[dependencies]
anyhow = { workspace = true }
bytes = { workspace = true }
chrono = { workspace = true }
clockabilly = { workspace = true }
clap = { workspace = true }
Expand All @@ -25,6 +26,7 @@ k8s-openapi = { workspace = true }
lazy_static = { workspace = true }
ratatui = { workspace = true }
reqwest = { workspace = true }
rmp-serde = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
Expand Down
65 changes: 63 additions & 2 deletions sk-cli/src/validation/annotated_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ use std::collections::{
};
use std::slice;

use kube::api::DynamicObject;
use json_patch_ext::{
patch_ext,
PatchOperation,
};
use sk_core::external_storage::{
ObjectStoreWrapper,
SkObjectStore,
};
use sk_core::prelude::*;
use sk_store::{
TraceEvent,
TraceStorable,
Expand All @@ -32,13 +36,30 @@ impl AnnotatedTraceEvent {

AnnotatedTraceEvent { data, annotations }
}

pub fn clear_annotations(&mut self) {
self.annotations = vec![vec![]; self.data.len()];
}
}

pub enum PatchLocations {
Everywhere,
#[allow(dead_code)]
ObjectReference(TypeMeta, String),
}

pub struct AnnotatedTracePatch {
pub locations: PatchLocations,
pub op: PatchOperation,
}

#[derive(Default)]
pub struct AnnotatedTrace {
path: String,
#[allow(dead_code)]
base: TraceStore,
path: String,
patches: Vec<AnnotatedTracePatch>,

events: Vec<AnnotatedTraceEvent>,
summary: BTreeMap<ValidatorCode, usize>,
}
Expand All @@ -57,8 +78,37 @@ impl AnnotatedTrace {
})
}

pub fn apply_patch(&mut self, patch: AnnotatedTracePatch) -> EmptyResult {
for event in self.events.iter_mut() {
for obj in event.data.applied_objs.iter_mut() {
let should_apply_here = match patch.locations {
PatchLocations::Everywhere => true,
PatchLocations::ObjectReference(ref type_, ref ns_name) => {
obj.types.as_ref().is_some_and(|t| t == type_) && &obj.namespaced_name() == ns_name
},
};

if should_apply_here {
patch_ext(&mut obj.data, patch.op.clone())?;
}
}
}
self.patches.push(patch);

Ok(())
}

pub fn export(&self) -> anyhow::Result<Vec<u8>> {
let trace = self
.base
.clone_with_events(self.events.iter().map(|a_event| a_event.data.clone()).collect());
trace.export_all()
}

pub fn validate(&mut self, validators: &mut BTreeMap<ValidatorCode, Validator>) {
self.summary.clear();
for event in self.events.iter_mut() {
event.clear_annotations();
for (code, validator) in validators.iter_mut() {
let affected_indices = validator.check_next_event(event);
let count = affected_indices.len();
Expand All @@ -75,6 +125,17 @@ impl AnnotatedTrace {
self.events.get(idx)
}

pub fn get_next_error(&self) -> Option<ValidatorCode> {
for event in &self.events {
for annotation in &event.annotations {
if let Some(code) = annotation.first() {
return Some(*code);
}
}
}
None
}

pub fn get_object(&self, event_idx: usize, obj_idx: usize) -> Option<&DynamicObject> {
let event = self.events.get(event_idx)?;
let applied_len = event.data.applied_objs.len();
Expand Down
25 changes: 24 additions & 1 deletion sk-cli/src/validation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,23 @@ mod status_field_populated;
mod validation_store;
mod validator;

use bytes::Bytes;
use clap::{
value_parser,
Subcommand,
ValueEnum,
};
use sk_core::external_storage::{
ObjectStoreWrapper,
SkObjectStore,
};
use sk_core::prelude::*;

pub use self::annotated_trace::{
AnnotatedTrace,
AnnotatedTraceEvent,
AnnotatedTracePatch,
PatchLocations,
};
pub use self::validation_store::ValidationStore;
pub use self::validator::{
Expand All @@ -36,6 +43,17 @@ pub enum ValidateSubcommand {
pub struct CheckArgs {
#[arg(long_help = "location of the input trace file")]
pub trace_path: String,

#[arg(long, long_help = "fix all discovered issues")]
pub fix: bool,

#[arg(
short,
long,
long_help = "output path for modified trace (REQUIRED if --fix is set)",
required_if_eq("fix", "true")
)]
pub output: Option<String>,
}

#[derive(clap::Args)]
Expand Down Expand Up @@ -69,7 +87,12 @@ pub async fn cmd(subcommand: &ValidateSubcommand) -> EmptyResult {
match subcommand {
ValidateSubcommand::Check(args) => {
let mut trace = AnnotatedTrace::new(&args.trace_path).await?;
validators.validate_trace(&mut trace);
validators.validate_trace(&mut trace, args.fix)?;
if let Some(output_path) = &args.output {
let trace_data = trace.export()?;
let object_store = SkObjectStore::new(output_path)?;
object_store.put(Bytes::from(trace_data)).await?;
}
print_summary(&trace, &validators)?;
},
ValidateSubcommand::Explain(args) => validators.explain(&args.code)?,
Expand Down
10 changes: 10 additions & 0 deletions sk-cli/src/validation/status_field_populated.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
use json_patch_ext::{
format_ptr,
remove_operation,
PatchOperation,
};

use super::annotated_trace::AnnotatedTraceEvent;
use super::validator::{
Diagnostic,
Expand Down Expand Up @@ -35,6 +41,10 @@ impl Diagnostic for StatusFieldPopulated {
.collect()
}

fn fixes(&self) -> Vec<PatchOperation> {
vec![remove_operation(format_ptr!("/status"))]
}

fn reset(&mut self) {}
}

Expand Down
5 changes: 5 additions & 0 deletions sk-cli/src/validation/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mod validation_store_test;

use std::collections::BTreeMap;

use json_patch_ext::PatchOperation;
use rstest::*;
use sk_core::prelude::*;
use sk_store::TraceEvent;
Expand Down Expand Up @@ -55,6 +56,10 @@ impl Diagnostic for TestDiagnostic {
}
}

fn fixes(&self) -> Vec<PatchOperation> {
vec![]
}

fn reset(&mut self) {}
}

Expand Down
1 change: 0 additions & 1 deletion sk-cli/src/validation/tests/status_field_populated_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use assertables::*;
use kube::api::DynamicObject;
use serde_json::json;
use sk_store::TraceEvent;

Expand Down
2 changes: 1 addition & 1 deletion sk-cli/src/validation/tests/validation_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::*;

#[rstest]
fn test_validate_trace(mut test_validation_store: ValidationStore, mut annotated_trace: AnnotatedTrace) {
test_validation_store.validate_trace(&mut annotated_trace);
test_validation_store.validate_trace(&mut annotated_trace, false).unwrap();

for evt in annotated_trace.iter() {
if evt.data.applied_objs.len() > 1 {
Expand Down
33 changes: 30 additions & 3 deletions sk-cli/src/validation/validation_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ use anyhow::anyhow;
use serde::Serialize;
use sk_core::prelude::*;

use super::annotated_trace::AnnotatedTrace;
use super::validator::{
Validator,
ValidatorCode,
};
use super::{
status_field_populated,
AnnotatedTrace,
AnnotatedTracePatch,
PatchLocations,
PrintFormat,
};

Expand All @@ -20,12 +22,37 @@ pub struct ValidationStore {
}

impl ValidationStore {
pub fn validate_trace(&mut self, trace: &mut AnnotatedTrace) {
pub fn validate_trace(&mut self, trace: &mut AnnotatedTrace, fix: bool) -> EmptyResult {
for validator in self.validators.values_mut() {
validator.reset();
}

trace.validate(&mut self.validators);
loop {
trace.validate(&mut self.validators);

if !fix {
break;
}

let Some(next_error) = trace.get_next_error() else {
break;
};

let Some(op) = self
.validators
.get(&next_error)
.ok_or(anyhow!("validation error"))?
.fixes()
.first()
.cloned()
else {
println!("no fix available for {next_error}; continuing");
break;
};
trace.apply_patch(AnnotatedTracePatch { locations: PatchLocations::Everywhere, op })?;
}

Ok(())
}

pub(super) fn explain(&self, code: &ValidatorCode) -> EmptyResult {
Expand Down
6 changes: 6 additions & 0 deletions sk-cli/src/validation/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::fmt;
use std::str::from_utf8;

use anyhow::bail;
use json_patch_ext::PatchOperation;
use serde::{
Serialize,
Serializer,
Expand Down Expand Up @@ -51,6 +52,7 @@ impl fmt::Display for ValidatorCode {

pub trait Diagnostic {
fn check_next_event(&mut self, evt: &mut AnnotatedTraceEvent) -> Vec<usize>;
fn fixes(&self) -> Vec<PatchOperation>;
fn reset(&mut self);
}

Expand All @@ -72,6 +74,10 @@ impl Validator {
self.diagnostic.check_next_event(a_event)
}

pub fn fixes(&self) -> Vec<PatchOperation> {
self.diagnostic.fixes()
}

pub fn reset(&mut self) {
self.diagnostic.reset()
}
Expand Down
4 changes: 3 additions & 1 deletion sk-cli/src/xray/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ impl App {
}

pub(super) fn rebuild_annotated_trace(&mut self) {
self.validation_store.validate_trace(&mut self.annotated_trace)
self.validation_store
.validate_trace(&mut self.annotated_trace, false)
.expect("validation failed");
}

pub(super) fn update_state(&mut self, msg: Message) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion sk-cli/src/xray/tests/view_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::validation::{

#[fixture]
fn test_app(mut test_validation_store: ValidationStore, mut annotated_trace: AnnotatedTrace) -> App {
test_validation_store.validate_trace(&mut annotated_trace);
test_validation_store.validate_trace(&mut annotated_trace, false).unwrap();
App {
annotated_trace,
event_list_state: ListState::default().with_selected(Some(0)),
Expand Down
1 change: 0 additions & 1 deletion sk-cli/src/xray/view/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use chrono::TimeDelta;
use kube::api::DynamicObject;
use lazy_static::lazy_static;
use ratatui::prelude::*;
use sk_core::prelude::*;
Expand Down
6 changes: 1 addition & 5 deletions sk-core/src/k8s/gvk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ use std::borrow::Cow;
use std::fmt;
use std::ops::Deref;

use kube::api::{
DynamicObject,
GroupVersionKind,
TypeMeta,
};
use kube::api::GroupVersionKind;
use serde::{
de,
Deserialize,
Expand Down
1 change: 0 additions & 1 deletion sk-core/src/k8s/tests/util_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use clockabilly::Utc;
use kube::api::DynamicObject;
use serde_json as json;

use super::*;
Expand Down
1 change: 0 additions & 1 deletion sk-core/src/k8s/testutils/objs.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use kube::api::DynamicObject;
use kube::discovery::ApiResource;
use rstest::*;
use serde_json::json;
Expand Down
6 changes: 1 addition & 5 deletions sk-core/src/k8s/util.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
use std::collections::BTreeMap;

use kube::api::{
DynamicObject,
Resource,
TypeMeta,
};
use kube::api::Resource;
use serde_json as json;

use super::*;
Expand Down
Loading

0 comments on commit 2ebdc19

Please sign in to comment.