diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index 84a6480b7..29eda6a08 100644 --- a/lib/dsc-lib/src/dscresources/dscresource.rs +++ b/lib/dsc-lib/src/dscresources/dscresource.rs @@ -11,7 +11,7 @@ use rust_i18n::t; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use tracing::{debug, info, trace, warn}; @@ -657,14 +657,15 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { #[must_use] /// Performs a comparison of two JSON Values using an optional JSON Schema. -/// If a property exists in `expected` but not in `actual`, the schema's `default` value -/// for that property is used for comparison when available. +/// Properties whose schema sets `writeOnly` to `true` are ignored. If a property exists +/// in `expected` but not in `actual`, the schema's `default` value for that property is +/// used for comparison when available. /// /// # Arguments /// /// * `expected` - The expected value /// * `actual` - The actual value -/// * `schema` - Optional JSON Schema to look up default values for missing properties +/// * `schema` - Optional JSON Schema to identify write-only properties and default values /// /// # Returns /// @@ -691,6 +692,10 @@ pub(crate) fn get_diff_with_schema(expected: &Value, actual: &Value, schema: Opt } for (key, value) in &*map { + if is_schema_write_only(schema, key) { + continue; + } + if is_secure_value(value) { // skip secure values as they are not comparable continue; @@ -726,7 +731,7 @@ pub(crate) fn get_diff_with_schema(expected: &Value, actual: &Value, schema: Opt } } else { // Property not in actual - check schema for a default value - if let Some(default_value) = get_schema_default(schema, key) { + if let Some(default_value) = get_schema_default(schema, key) { if value != &default_value { info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key)); diff_properties.push(key.to_string()); @@ -764,6 +769,42 @@ fn get_schema_default(schema: Option<&Value>, property_name: &str) -> Option, property_name: &str) -> bool { + let Some(schema) = schema else { + return false; + }; + let Some(mut property_schema) = schema + .get("properties") + .and_then(Value::as_object) + .and_then(|properties| properties.get(property_name)) + else { + return false; + }; + let mut visited_references = HashSet::new(); + + loop { + if property_schema.get("writeOnly").and_then(Value::as_bool) == Some(true) { + return true; + } + + let Some(reference) = property_schema.get("$ref").and_then(Value::as_str) else { + return false; + }; + let Some(pointer) = reference.strip_prefix('#') else { + return false; + }; + if !visited_references.insert(pointer) { + return false; + } + let Some(resolved_schema) = schema.pointer(pointer) else { + return false; + }; + property_schema = resolved_schema; + } +} + /// Validates the properties of a resource against its schema. /// /// # Arguments @@ -1023,6 +1064,73 @@ fn diff_with_schema_no_default_reports_missing_property() { assert_eq!(diff, vec!["enabled".to_string()]); } +#[test] +fn diff_with_schema_write_only_ignores_differing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test", "action": "ignore"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "type": "string", "writeOnly": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected write-only property to be ignored, got: {diff:?}"); +} + +#[test] +fn diff_with_schema_write_only_ignores_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "type": "string", "writeOnly": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected write-only property to be ignored, got: {diff:?}"); +} + +#[test] +fn diff_with_schema_write_only_local_ref_ignores_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "$ref": "#/$defs/action" } + }, + "$defs": { + "action": { "type": "string", "writeOnly": true } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert!(diff.is_empty(), "Expected referenced write-only property to be ignored, got: {diff:?}"); +} + +#[test] +fn diff_with_schema_write_only_false_reports_missing_property() { + use serde_json::json; + let expected = json!({"name": "test", "action": "remove"}); + let actual = json!({"name": "test"}); + let schema = json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "action": { "type": "string", "writeOnly": false } + } + }); + let diff = get_diff_with_schema(&expected, &actual, Some(&schema)); + assert_eq!(diff, vec!["action".to_string()]); +} + #[test] fn diff_without_schema_reports_missing_property() { use serde_json::json; diff --git a/resources/windows_firewall/src/firewall.rs b/resources/windows_firewall/src/firewall.rs index d825d8ffd..4815e0e3c 100644 --- a/resources/windows_firewall/src/firewall.rs +++ b/resources/windows_firewall/src/firewall.rs @@ -587,7 +587,7 @@ pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result {} // None or Ignore: no additional action. } - Ok(FirewallRuleList { rules: results, unspecified_rules: input.unspecified_rules.clone() }) + Ok(FirewallRuleList { rules: results, unspecified_rules: None }) } pub fn export_rules() -> Result { diff --git a/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 index fa0d6aa46..9c2279cb2 100644 --- a/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 +++ b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 @@ -26,9 +26,11 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul Remove-NetFirewallRule -Name $testRuleName -ErrorAction Ignore } - It 'unspecifiedRulesAction set to default "ignore" does not report as differing' { + It 'unspecifiedRules action "ignore" does not report as differing' { $json = @{ - unspecifiedRulesAction = 'ignore' + unspecifiedRules = @{ + action = 'ignore' + } rules = @(@{ name = $testRuleName direction = 'Inbound' @@ -43,10 +45,10 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul $result = $out | ConvertFrom-Json $result.inDesiredState | Should -Be $true - $result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction' + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' } - It 'unspecifiedRulesAction omitted does not report as differing' { + It 'unspecifiedRules omitted does not report as differing' { $json = @{ rules = @(@{ name = $testRuleName @@ -62,12 +64,14 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul $result = $out | ConvertFrom-Json $result.inDesiredState | Should -Be $true - $result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction' + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' } - It 'non-default unspecifiedRulesAction "disable" is reported as differing' { + It 'unspecifiedRules action "disable" is ignored for comparison' { $json = @{ - unspecifiedRulesAction = 'disable' + unspecifiedRules = @{ + action = 'disable' + } rules = @(@{ name = $testRuleName direction = 'Inbound' @@ -81,12 +85,15 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = $out | ConvertFrom-Json - $result.differingProperties | Should -Contain 'unspecifiedRulesAction' + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' } - It 'non-default unspecifiedRulesAction "remove" is reported as differing' { + It 'unspecifiedRules action "remove" is ignored for comparison' { $json = @{ - unspecifiedRulesAction = 'remove' + unspecifiedRules = @{ + action = 'remove' + } rules = @(@{ name = $testRuleName direction = 'Inbound' @@ -100,6 +107,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaul $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = $out | ConvertFrom-Json - $result.differingProperties | Should -Contain 'unspecifiedRulesAction' + $result.inDesiredState | Should -Be $true + $result.differingProperties | Should -Not -Contain 'unspecifiedRules' } } diff --git a/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 index dfa348901..453cd30c1 100644 --- a/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 +++ b/resources/windows_firewall/tests/windows_firewall_set.tests.ps1 @@ -59,6 +59,19 @@ Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$isElevate ($out | ConvertFrom-Json).afterState.rules | Should -BeNullOrEmpty } + It 'does not return unspecifiedRules in the after state' -Skip:(!$isElevated) { + $json = @{ + rules = @() + unspecifiedRules = @{ + action = 'ignore' + } + } | ConvertTo-Json -Compress -Depth 5 + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + + ($out | ConvertFrom-Json).afterState.PSObject.Properties.Name | Should -Not -Contain 'unspecifiedRules' + } + It 'updates an existing rule' -Skip:(!$isElevated) { Initialize-TestFirewallRule $json = @{ rules = @(@{ name = $testRuleName; description = 'Updated by DSC test'; enabled = $false }) } | ConvertTo-Json -Compress -Depth 5 diff --git a/resources/windows_firewall/windows_firewall.dsc.resource.json b/resources/windows_firewall/windows_firewall.dsc.resource.json index 58c6f7dc5..d24b1eea9 100644 --- a/resources/windows_firewall/windows_firewall.dsc.resource.json +++ b/resources/windows_firewall/windows_firewall.dsc.resource.json @@ -63,6 +63,7 @@ "type": "object", "title": "Unspecified rules", "description": "Defines the action and optional scope for firewall rules not explicitly listed in the rules array. When both direction and profiles are specified, a rule must match both filters.", + "writeOnly": true, "additionalProperties": false, "required": [ "action"