diff --git a/examples/v1alpha/KnownErrorMultiPattern.yaml b/examples/v1alpha/KnownErrorMultiPattern.yaml new file mode 100644 index 0000000..5e08abe --- /dev/null +++ b/examples/v1alpha/KnownErrorMultiPattern.yaml @@ -0,0 +1,10 @@ +apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: disk-full-or-oom + description: Check if the output contains a disk-full or out-of-memory error +spec: + pattern: + - "No space left on device" + - "Out of memory" + help: "The system ran out of disk space or memory. Free up resources and try again." diff --git a/schema/merged.json b/schema/merged.json index c320a59..d6a4989 100644 --- a/schema/merged.json +++ b/schema/merged.json @@ -216,6 +216,23 @@ "ScopeKnownError" ] }, + "KnownErrorPattern": { + "description": "One or more regexes that determine whether a log line matches this known error.\nAccepts either a single pattern string or a list of pattern strings; a line matching\nany one of the patterns triggers the error.", + "anyOf": [ + { + "description": "A single regex pattern.", + "type": "string" + }, + { + "description": "A list of regex patterns (at least one); the known error fires when any one matches.", + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1 + } + ] + }, "KnownErrorSpec": { "description": "Definition of the known error", "type": "object", @@ -236,8 +253,8 @@ "type": "string" }, "pattern": { - "description": "A Regex used to determine if the line is an error.", - "type": "string" + "description": "A regex (or list of regexes) used to determine if the line is an error.\nA single string or a list of strings are both accepted; the known error fires when any\npattern matches.", + "$ref": "#/$defs/KnownErrorPattern" } }, "additionalProperties": false, diff --git a/schema/v1alpha.com.github.scope.ScopeDoctorGroup.json b/schema/v1alpha.com.github.scope.ScopeDoctorGroup.json index 74f5deb..94c0c02 100644 --- a/schema/v1alpha.com.github.scope.ScopeDoctorGroup.json +++ b/schema/v1alpha.com.github.scope.ScopeDoctorGroup.json @@ -232,6 +232,23 @@ "ScopeKnownError" ] }, + "KnownErrorPattern": { + "description": "One or more regexes that determine whether a log line matches this known error.\nAccepts either a single pattern string or a list of pattern strings; a line matching\nany one of the patterns triggers the error.", + "anyOf": [ + { + "description": "A single regex pattern.", + "type": "string" + }, + { + "description": "A list of regex patterns (at least one); the known error fires when any one matches.", + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1 + } + ] + }, "KnownErrorSpec": { "description": "Definition of the known error", "type": "object", @@ -252,8 +269,8 @@ "type": "string" }, "pattern": { - "description": "A Regex used to determine if the line is an error.", - "type": "string" + "description": "A regex (or list of regexes) used to determine if the line is an error.\nA single string or a list of strings are both accepted; the known error fires when any\npattern matches.", + "$ref": "#/$defs/KnownErrorPattern" } }, "additionalProperties": false, diff --git a/schema/v1alpha.com.github.scope.ScopeKnownError.json b/schema/v1alpha.com.github.scope.ScopeKnownError.json index 27cb966..c93bec3 100644 --- a/schema/v1alpha.com.github.scope.ScopeKnownError.json +++ b/schema/v1alpha.com.github.scope.ScopeKnownError.json @@ -232,6 +232,23 @@ "ScopeKnownError" ] }, + "KnownErrorPattern": { + "description": "One or more regexes that determine whether a log line matches this known error.\nAccepts either a single pattern string or a list of pattern strings; a line matching\nany one of the patterns triggers the error.", + "anyOf": [ + { + "description": "A single regex pattern.", + "type": "string" + }, + { + "description": "A list of regex patterns (at least one); the known error fires when any one matches.", + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1 + } + ] + }, "KnownErrorSpec": { "description": "Definition of the known error", "type": "object", @@ -252,8 +269,8 @@ "type": "string" }, "pattern": { - "description": "A Regex used to determine if the line is an error.", - "type": "string" + "description": "A regex (or list of regexes) used to determine if the line is an error.\nA single string or a list of strings are both accepted; the known error fires when any\npattern matches.", + "$ref": "#/$defs/KnownErrorPattern" } }, "additionalProperties": false, diff --git a/schema/v1alpha.com.github.scope.ScopeReportLocation.json b/schema/v1alpha.com.github.scope.ScopeReportLocation.json index e36be27..42a97d5 100644 --- a/schema/v1alpha.com.github.scope.ScopeReportLocation.json +++ b/schema/v1alpha.com.github.scope.ScopeReportLocation.json @@ -232,6 +232,23 @@ "ScopeKnownError" ] }, + "KnownErrorPattern": { + "description": "One or more regexes that determine whether a log line matches this known error.\nAccepts either a single pattern string or a list of pattern strings; a line matching\nany one of the patterns triggers the error.", + "anyOf": [ + { + "description": "A single regex pattern.", + "type": "string" + }, + { + "description": "A list of regex patterns (at least one); the known error fires when any one matches.", + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1 + } + ] + }, "KnownErrorSpec": { "description": "Definition of the known error", "type": "object", @@ -252,8 +269,8 @@ "type": "string" }, "pattern": { - "description": "A Regex used to determine if the line is an error.", - "type": "string" + "description": "A regex (or list of regexes) used to determine if the line is an error.\nA single string or a list of strings are both accepted; the known error fires when any\npattern matches.", + "$ref": "#/$defs/KnownErrorPattern" } }, "additionalProperties": false, diff --git a/src/doctor/check.rs b/src/doctor/check.rs index 94b58b3..5512ca6 100644 --- a/src/doctor/check.rs +++ b/src/doctor/check.rs @@ -1573,7 +1573,7 @@ pub(crate) mod tests { use super::*; use crate::models::prelude::{ModelMetadata, ModelMetadataAnnotations}; use crate::shared::analyze::AnalyzeStatus; - use regex::Regex; + use regex::RegexSet; fn make_known_error(pattern: &str, with_fix: bool) -> KnownError { let fix = if with_fix { @@ -1601,8 +1601,7 @@ pub(crate) mod tests { }, labels: BTreeMap::new(), }, - pattern: pattern.to_string(), - regex: Regex::new(pattern).unwrap(), + regexes: RegexSet::new([pattern]).unwrap(), help_text: "test help".to_string(), fix, } diff --git a/src/models/v1alpha/known_error.rs b/src/models/v1alpha/known_error.rs index b80fd0b..b3b2036 100644 --- a/src/models/v1alpha/known_error.rs +++ b/src/models/v1alpha/known_error.rs @@ -7,6 +7,40 @@ use serde::{Deserialize, Serialize}; use super::prelude::DoctorFixSpec; +/// Schema helper: the list variant must have at least one element. +/// Using `schema_with` (rather than `#[schemars(length(min = 1))]`) avoids a stray +/// `minLength: 1` keyword that `length` emits unconditionally alongside `minItems`. +fn non_empty_string_list(generator: &mut schemars::generate::SchemaGenerator) -> schemars::Schema { + let item = generator.subschema_for::(); + schemars::json_schema!({ + "type": "array", + "items": item, + "minItems": 1_u64 + }) +} + +/// One or more regexes that determine whether a log line matches this known error. +/// Accepts either a single pattern string or a list of pattern strings; a line matching +/// any one of the patterns triggers the error. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)] +#[serde(untagged)] +pub enum KnownErrorPattern { + /// A single regex pattern. + Single(String), + /// A list of regex patterns (at least one); the known error fires when any one matches. + Many(#[schemars(schema_with = "non_empty_string_list")] Vec), +} + +impl KnownErrorPattern { + /// Returns the list of pattern strings (a single value becomes a one-element vec). + pub fn patterns(&self) -> Vec { + match self { + KnownErrorPattern::Single(p) => vec![p.clone()], + KnownErrorPattern::Many(p) => p.clone(), + } + } +} + /// Definition of the known error #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)] #[serde(rename_all = "camelCase")] @@ -15,8 +49,10 @@ pub struct KnownErrorSpec { /// Text that the user can use to fix the issue pub help: String, - /// A Regex used to determine if the line is an error. - pub pattern: String, + /// A regex (or list of regexes) used to determine if the line is an error. + /// A single string or a list of strings are both accepted; the known error fires when any + /// pattern matches. + pub pattern: KnownErrorPattern, /// An optional fix the user will be prompted to run. pub fix: Option, @@ -82,6 +118,35 @@ impl InternalScopeModel for V1AlphaKnownError #[cfg(test)] fn examples() -> Vec { - vec!["v1alpha/KnownError.yaml".to_string()] + vec![ + "v1alpha/KnownError.yaml".to_string(), + "v1alpha/KnownErrorMultiPattern.yaml".to_string(), + ] + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::InternalScopeModel; + + #[test] + fn schema_rejects_empty_pattern_list() { + let value = serde_json::json!({ + "apiVersion": "scope.github.com/v1alpha", + "kind": "ScopeKnownError", + "metadata": { + "name": "test", + "description": "test" + }, + "spec": { + "pattern": [], + "help": "some help" + } + }); + assert!( + V1AlphaKnownError::validate_resource(&value).is_err(), + "schema should reject an empty pattern list" + ); } } diff --git a/src/shared/analyze/mod.rs b/src/shared/analyze/mod.rs index 04f2d70..7e87072 100644 --- a/src/shared/analyze/mod.rs +++ b/src/shared/analyze/mod.rs @@ -34,7 +34,7 @@ where let mut known_errors_to_remove = Vec::new(); for (name, ke) in &known_errors { debug!("Checking known error {}", ke.name()); - if ke.regex.is_match(&line) { + if ke.regexes.is_match(&line) { warn!(target: "always", "Known error '{}' found on line {}", ke.name(), line_number); info!(target: "always", "\t==> {}", ke.help_text); diff --git a/src/shared/config_load.rs b/src/shared/config_load.rs index a9592df..4274dbd 100644 --- a/src/shared/config_load.rs +++ b/src/shared/config_load.rs @@ -368,15 +368,15 @@ mod tests { let canonical_nested = fs::canonicalize(&nested_dir).unwrap(); // Should include .scope directories for all ancestors - let expected_paths = vec![ + let expected_paths = [ canonical_nested.join(".scope"), - canonical_nested.parent().unwrap().join(".scope"), // child + canonical_nested.parent().unwrap().join(".scope"), canonical_nested .parent() .unwrap() .parent() .unwrap() - .join(".scope"), // parent + .join(".scope"), canonical_nested .parent() .unwrap() @@ -384,7 +384,7 @@ mod tests { .unwrap() .parent() .unwrap() - .join(".scope"), // temp_dir + .join(".scope"), ]; // Check that all expected ancestor paths are present (in order) diff --git a/src/shared/models/internal/known_error.rs b/src/shared/models/internal/known_error.rs index ae60637..19a3e82 100644 --- a/src/shared/models/internal/known_error.rs +++ b/src/shared/models/internal/known_error.rs @@ -3,7 +3,7 @@ use std::path::Path; use crate::models::HelpMetadata; use crate::models::prelude::{ModelMetadata, V1AlphaKnownError}; use derivative::Derivative; -use regex::Regex; +use regex::RegexSet; use super::fix::DoctorFix; @@ -13,13 +13,19 @@ use super::fix::DoctorFix; pub struct KnownError { pub full_name: String, pub metadata: ModelMetadata, - pub pattern: String, #[derivative(PartialEq = "ignore")] - pub regex: Regex, + pub regexes: RegexSet, pub help_text: String, pub fix: Option, } +impl KnownError { + /// Returns the original pattern strings in the order they were defined. + pub fn patterns(&self) -> &[String] { + self.regexes.patterns() + } +} + impl HelpMetadata for KnownError { fn metadata(&self) -> &ModelMetadata { &self.metadata @@ -34,7 +40,15 @@ impl TryFrom for KnownError { type Error = anyhow::Error; fn try_from(value: V1AlphaKnownError) -> Result { - let regex = Regex::new(&value.spec.pattern)?; + let patterns = value.spec.pattern.patterns(); + if patterns.is_empty() { + anyhow::bail!( + "known error '{}' must have at least one pattern", + value.full_name() + ); + } + // RegexSet::new accepts empty iterators, so the guard above is required. + let regexes = RegexSet::new(&patterns)?; let binding = value.metadata.containing_dir(); let containing_dir = Path::new(&binding); @@ -58,8 +72,7 @@ impl TryFrom for KnownError { Ok(KnownError { full_name: value.full_name(), metadata: value.metadata, - pattern: value.spec.pattern, - regex, + regexes, help_text: value.spec.help, fix: maybe_fix, }) @@ -70,16 +83,36 @@ impl TryFrom for KnownError { mod tests { use super::*; use crate::prelude::{ - DoctorFixSpec, KnownErrorKind, KnownErrorSpec, ModelMetadataAnnotations, V1AlphaApiVersion, - V1AlphaKnownError, + DoctorFixSpec, KnownErrorKind, KnownErrorPattern, KnownErrorSpec, ModelMetadataAnnotations, + V1AlphaApiVersion, V1AlphaKnownError, }; use crate::shared::models::parse_models_from_string; use std::collections::BTreeMap; use std::path::Path; + fn make_metadata( + name: &str, + file_path: &str, + file_dir: &str, + working_dir: &str, + ) -> ModelMetadata { + ModelMetadata { + name: name.to_string(), + description: "some description".to_string(), + annotations: ModelMetadataAnnotations { + file_path: Some(file_path.to_string()), + file_dir: Some(file_dir.to_string()), + working_dir: Some(working_dir.to_string()), + bin_path: None, + extra: BTreeMap::new(), + }, + labels: BTreeMap::new(), + } + } + #[test] - fn test_parse_scope_known_error() { + fn parses_single_pattern_string() { let text = "apiVersion: scope.github.com/v1alpha kind: ScopeKnownError metadata: @@ -101,31 +134,72 @@ spec: "The command had an error, try reading the logs around there to find out what happened.", model.help_text ); - assert_eq!("error", model.pattern); + assert_eq!(["error"], model.patterns()); } #[test] - fn try_from_spec() { - let model_metadata = ModelMetadata { - name: "some test error".to_string(), - description: "some description".to_string(), - annotations: ModelMetadataAnnotations { - file_path: Some("/foo/bar/file.yaml".to_string()), - file_dir: Some("/foo/bar".to_string()), - working_dir: Some("/some/work/dir".to_string()), - bin_path: None, - extra: BTreeMap::new(), + fn parses_list_of_patterns() { + let text = "apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: multi-error +spec: + pattern: + - first error + - second error + help: Multiple patterns matched."; + + let path = Path::new("/foo/bar/file.yaml"); + let work_dir = Path::new("/foo/bar"); + let configs = parse_models_from_string(work_dir, path, text).unwrap(); + assert_eq!(1, configs.len()); + let model = configs[0].get_known_error_spec().unwrap(); + + assert_eq!("multi-error", model.metadata.name); + assert_eq!(["first error", "second error"], model.patterns()); + assert_eq!(2, model.regexes.len()); + } + + #[test] + fn empty_pattern_list_is_rejected() { + let metadata = make_metadata("bad-error", "/foo/bar/file.yaml", "/foo/bar", "/foo/bar"); + + let input = V1AlphaKnownError { + api_version: V1AlphaApiVersion::ScopeV1Alpha, + kind: KnownErrorKind::ScopeKnownError, + metadata, + spec: KnownErrorSpec { + help: "some help".to_string(), + pattern: KnownErrorPattern::Many(vec![]), + fix: None, }, - labels: BTreeMap::new(), }; + let result = KnownError::try_from(input); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("at least one pattern"), + "unexpected error: {msg}" + ); + } + + #[test] + fn try_from_single_pattern_spec() { + let model_metadata = make_metadata( + "some test error", + "/foo/bar/file.yaml", + "/foo/bar", + "/some/work/dir", + ); + let input = V1AlphaKnownError { api_version: V1AlphaApiVersion::ScopeV1Alpha, kind: KnownErrorKind::ScopeKnownError, metadata: model_metadata.clone(), spec: KnownErrorSpec { help: "some help text".to_string(), - pattern: "some regex pattern".to_string(), + pattern: KnownErrorPattern::Single("some regex pattern".to_string()), fix: Some(DoctorFixSpec { commands: vec!["echo 'fix it!'".to_string()], help_text: None, @@ -137,12 +211,13 @@ spec: let actual = KnownError::try_from(input.clone()).unwrap(); + // regexes is PartialEq-ignored; check patterns explicitly. + assert_eq!(["some regex pattern"], actual.patterns()); assert_eq!( KnownError { full_name: "ScopeKnownError/some test error".to_string(), metadata: input.metadata, - pattern: input.spec.pattern.clone(), - regex: Regex::new(&input.spec.pattern).unwrap(), + regexes: RegexSet::new(["some regex pattern"]).unwrap(), help_text: input.spec.help, fix: Some( DoctorFix::from_spec( @@ -156,4 +231,36 @@ spec: actual ) } + + #[test] + fn try_from_multi_pattern_spec() { + let model_metadata = make_metadata( + "multi-error", + "/foo/bar/file.yaml", + "/foo/bar", + "/some/work/dir", + ); + + let input = V1AlphaKnownError { + api_version: V1AlphaApiVersion::ScopeV1Alpha, + kind: KnownErrorKind::ScopeKnownError, + metadata: model_metadata.clone(), + spec: KnownErrorSpec { + help: "some help text".to_string(), + pattern: KnownErrorPattern::Many(vec![ + "alpha error".to_string(), + "beta error".to_string(), + ]), + fix: None, + }, + }; + + let actual = KnownError::try_from(input).unwrap(); + + assert_eq!(["alpha error", "beta error"], actual.patterns()); + assert_eq!(2, actual.regexes.len()); + assert!(actual.regexes.is_match("alpha error in output")); + assert!(actual.regexes.is_match("beta error in output")); + assert!(!actual.regexes.is_match("unrelated line")); + } }