Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ jobs:
run: |
cargo run -- --quiet registry json-schema -j semconv-definition-v2 -o /tmp/semconv.schema.v2.json
cargo run -- --quiet registry json-schema -j resolved-registry-v2 -o /tmp/semconv.resolved.v2.json
cargo run -- --quiet registry json-schema -j policy-finding -o /tmp/policy.finding.json
- name: Check generated files are up-to-date
run: |
failed=0
Expand All @@ -320,6 +321,12 @@ jobs:
failed=1
fi

if ! diff -q schemas/policy.finding.json /tmp/policy.finding.json > /dev/null 2>&1; then
echo "schemas/policy.finding.json is out of date. Run 'cargo run -- --quiet registry json-schema -j policy-finding -o ./schemas/policy.finding.json' to regenerate it."
diff schemas/policy.finding.json /tmp/policy.finding.json
failed=1
fi

exit $failed
coverage:
name: Coverage
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ cargo run -- --quiet markdown-help > docs/usage.md
```bash
cargo run -- --quiet registry json-schema -j semconv-definition-v2 -o ./schemas/semconv.schema.v2.json
cargo run -- --quiet registry json-schema -j resolved-registry-v2 -o ./schemas/semconv.resolved.v2.json
cargo run -- --quiet registry json-schema -j policy-finding -o ./schemas/policy.finding.json
```

**Run `just` before any push to pre-validate all the steps performed by CI.**
Expand Down
62 changes: 54 additions & 8 deletions crates/weaver_checker/src/finding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub struct PolicyFinding {
/// The context associated with the finding e.g. { "attribute_name": "foo.bar", "attribute_value": "bar" }
/// The context should contain all dynamic parts of the message
/// Context values may be used with custom templates and filters to customize reports.
pub context: Value,
pub context: Option<Value>,

/// The human-readable message of the finding e.g. "This attribute 'foo.bar' is deprecated, reason: 'use foo.baz'"
/// The message, along with signal_name and signal_type, should contain enough information to understand the advice and
Expand Down Expand Up @@ -55,7 +55,7 @@ impl PolicyFinding {
let message = format!("id={id}, category={category}, group={group}, attr={attr}");
PolicyFinding {
id: SEMCONV_ATTRIBUTE.to_owned(),
context: ctx,
context: Some(ctx),
message,
level: FindingLevel::Violation,
signal_type: None,
Expand Down Expand Up @@ -144,11 +144,9 @@ impl<'de> serde::de::Visitor<'de> for ViolationBuilder {
let message = opt_message.ok_or(serde::de::Error::missing_field("message"))?;
let signal_type = opt_signal_type;
let signal_name = opt_signal_name;
let advice_context =
opt_context.ok_or(serde::de::Error::missing_field("advice_context"))?;
Ok(PolicyFinding {
id,
context: advice_context,
context: opt_context,
message,
level,
signal_type,
Expand All @@ -162,10 +160,9 @@ impl<'de> serde::de::Visitor<'de> for ViolationBuilder {
let message = opt_message.ok_or(serde::de::Error::missing_field("message"))?;
let signal_type = opt_signal_type;
let signal_name = opt_signal_name;
let context = opt_context.ok_or(serde::de::Error::missing_field("context"))?;
Ok(PolicyFinding {
id,
context,
context: opt_context,
message,
level,
signal_type,
Expand All @@ -187,7 +184,14 @@ impl Display for PolicyFinding {
_ => write!(
f,
"id={}, context={}, message={}, level={:?}, signal_type={:?}, signal_name={:?}",
self.id, self.context, self.message, self.level, self.signal_type, self.signal_name,
self.id,
self.context
.as_ref()
.map_or("None".to_owned(), |c| c.to_string()),
self.message,
self.level,
self.signal_type,
self.signal_name,
),
}
}
Expand All @@ -201,6 +205,48 @@ impl PolicyFinding {
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_deserialize_unknown_type_returns_error() {
let json =
r#"{"type": "unknown_type", "id": "foo", "message": "bar", "level": "violation"}"#;
let result = serde_json::from_str::<PolicyFinding>(json);
assert!(
result.is_err(),
"Expected error for unknown type, got: {result:?}"
);
}

#[test]
fn test_display_with_no_context() {
let finding = PolicyFinding {
id: "some_id".to_owned(),
context: None,
message: "some message".to_owned(),
level: FindingLevel::Violation,
signal_type: None,
signal_name: None,
};
assert!(finding.to_string().contains("context=None"));
}

#[test]
fn test_display_with_context() {
let finding = PolicyFinding {
id: "some_id".to_owned(),
context: Some(serde_json::json!({"key": "value"})),
message: "some message".to_owned(),
level: FindingLevel::Violation,
signal_type: None,
signal_name: None,
};
assert!(finding.to_string().contains("context={\"key\":\"value\"}"));
}
}

/// The level of a finding.
#[derive(
Debug, Clone, PartialEq, Serialize, Deserialize, PartialOrd, Ord, Eq, Hash, JsonSchema,
Expand Down
12 changes: 9 additions & 3 deletions crates/weaver_checker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,11 @@ mod tests {
),
PolicyFinding {
id: "attr_removed".to_owned(),
context: serde_json::json!({
context: Some(serde_json::json!({
"id": "schema_evolution",
"group": "registry.network1".to_owned(),
"attr": "protocol.name.3".to_owned(),
}),
})),
message: "Schema evolution violation".to_owned(),
level: crate::finding::FindingLevel::Violation,
signal_type: None,
Expand Down Expand Up @@ -578,7 +578,13 @@ mod tests {
format!(
"{}-{}",
v.id,
v.context.as_object().unwrap().get("id").unwrap()
v.context
.as_ref()
.unwrap()
.as_object()
.unwrap()
.get("id")
.unwrap()
)
}
#[test]
Expand Down
6 changes: 3 additions & 3 deletions crates/weaver_live_check/src/advice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub trait Advisor {
/// Fluent builder for creating PolicyFinding instances with automatic emission
pub struct FindingBuilder {
id: String,
context: JsonValue,
context: Option<JsonValue>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - I noticed this but forgot to make a note/issue to fix later. Thanks for fixing this!

message: String,
level: FindingLevel,
signal_type: Option<String>,
Expand All @@ -52,7 +52,7 @@ impl FindingBuilder {
pub fn new(id: impl Into<String>) -> Self {
Self {
id: id.into(),
context: JsonValue::Null,
context: None,
message: String::new(),
level: FindingLevel::Information,
signal_type: None,
Expand All @@ -63,7 +63,7 @@ impl FindingBuilder {
/// Set the context JSON for this finding
#[must_use]
pub fn context(mut self, context: JsonValue) -> Self {
self.context = context;
self.context = Some(context);
self
}

Expand Down
4 changes: 2 additions & 2 deletions crates/weaver_live_check/src/advice/type_advisor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ fn check_attributes<T: CheckableAttribute>(
};
advice_list.push(PolicyFinding {
id: advice_type,
context: json!({
context: Some(json!({
ATTRIBUTE_NAME_ADVICE_CONTEXT_KEY: key.to_owned()
}),
})),
message,
level: advice_level,
signal_type: sample.signal_type(),
Expand Down
Loading