Skip to content

Commit 79fd2ab

Browse files
SteveL-MSFTSteve Lee (POWERSHELL HE/HIM) (from Dev Box)Copilotmichaeltlombardi
authored
Use JSON Schema defaults in synthetic test get_diff (#1670)
* Use JSON Schema defaults in synthetic test get_diff Update get_diff() to accept an optional JSON Schema parameter via the new get_diff_with_schema() function. When a property exists in the expected (desired) state but is missing from the actual state, the function now checks the schema for a 'default' value for that property. If the expected value matches the schema default, it is not reported as differing. This improves synthetic test accuracy for resources that don't return properties whose values match the schema-defined defaults. - Add get_diff_with_schema() with optional schema parameter - Keep get_diff() as a convenience wrapper (no schema) - Update invoke_synthetic_test to retrieve and pass the resource schema - Update DscResource synthetic test path for adapted resources - Add get_schema_default() helper to extract defaults from JSON Schema - Add Test/SchemaDefault test resource and dsctest subcommand - Add Rust unit tests for schema default comparison logic - Add Pester integration tests for end-to-end validation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address PR feedback: restrict visibility and avoid redundant serialization - Change get_diff_with_schema from pub to pub(crate) since it is only used within the dsc-lib crate - Read schema from RESOURCE_SCHEMAS cache directly (returns Value) instead of round-tripping through get_schema -> String -> from_str. Only calls get_schema to populate the cache on a miss. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Add FirewallRuleList Pester tests for schema default fix (#1666) Add tests verifying that unspecifiedRulesAction set to the schema default value 'ignore' is no longer reported as drift in synthetic test. Non-default values ('disable', 'remove') are still correctly flagged. Tests require elevation to create/remove firewall rules and are skipped when not running as Administrator. Fixes #1666 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix CI: skip firewall schema default tests when NetSecurity module unavailable Move -Skip to Describe block and check for Get-NetFirewallRule cmdlet availability in BeforeDiscovery. This prevents BeforeAll/AfterAll from running on CI runners without the NetSecurity module. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Mikey Lombardi (He/Him) <michael.t.lombardi@gmail.com> --------- Co-authored-by: Steve Lee (POWERSHELL HE/HIM) (from Dev Box) <slee@ntdev.microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Mikey Lombardi (He/Him) <michael.t.lombardi@gmail.com>
1 parent 01bfaaa commit 79fd2ab

8 files changed

Lines changed: 369 additions & 5 deletions

File tree

‎dsc/tests/dsc_schema_default.tests.ps1‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
Describe 'Synthetic test uses schema defaults' {
5+
It 'Property matching schema default is not reported as differing' {
6+
$out = '{"name":"test","enabled":true}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
7+
$LASTEXITCODE | Should -Be 0
8+
$out.inDesiredState | Should -Be $true
9+
$out.differingProperties | Should -BeNullOrEmpty
10+
}
11+
12+
It 'Property differing from schema default is reported as differing' {
13+
$out = '{"name":"test","enabled":false}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
14+
$LASTEXITCODE | Should -Be 0
15+
$out.inDesiredState | Should -Be $false
16+
$out.differingProperties | Should -Contain 'enabled'
17+
}
18+
19+
It 'Integer property matching schema default is not reported as differing' {
20+
$out = '{"name":"test","count":5}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
21+
$LASTEXITCODE | Should -Be 0
22+
$out.inDesiredState | Should -Be $true
23+
$out.differingProperties | Should -BeNullOrEmpty
24+
}
25+
26+
It 'Integer property differing from schema default is reported as differing' {
27+
$out = '{"name":"test","count":10}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
28+
$LASTEXITCODE | Should -Be 0
29+
$out.inDesiredState | Should -Be $false
30+
$out.differingProperties | Should -Contain 'count'
31+
}
32+
33+
It 'Multiple properties matching schema defaults are not reported as differing' {
34+
$out = '{"name":"test","enabled":true,"count":5}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
35+
$LASTEXITCODE | Should -Be 0
36+
$out.inDesiredState | Should -Be $true
37+
$out.differingProperties | Should -BeNullOrEmpty
38+
}
39+
40+
It 'Mix of matching and non-matching defaults reports only non-matching' {
41+
$out = '{"name":"test","enabled":true,"count":10}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
42+
$LASTEXITCODE | Should -Be 0
43+
$out.inDesiredState | Should -Be $false
44+
$out.differingProperties | Should -Contain 'count'
45+
$out.differingProperties | Should -Not -Contain 'enabled'
46+
}
47+
48+
It 'Property present in both expected and actual is compared normally' {
49+
$out = '{"name":"test"}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
50+
$LASTEXITCODE | Should -Be 0
51+
$out.inDesiredState | Should -Be $true
52+
$out.differingProperties | Should -BeNullOrEmpty
53+
}
54+
}

‎lib/dsc-lib/src/dscresources/command_resource.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::{configure::{config_doc::{ExecutionKind, SecurityContextKind}, config
1212
use crate::dscerror::DscError;
1313
use crate::locked_insert;
1414
use super::{
15-
dscresource::{get_diff, redact, DscResource},
15+
dscresource::{get_diff, get_diff_with_schema, redact, DscResource},
1616
invoke_result::{
1717
DeleteResult, DeleteResultKind, ExportResult,
1818
GetResult, ResolveResult, SetResult, TestResult, ValidateResult,
@@ -454,7 +454,14 @@ fn invoke_synthetic_test(resource: &DscResource, expected: &str, target_resource
454454
}
455455
};
456456
let expected_value: Value = serde_json::from_str(expected)?;
457-
let diff_properties = get_diff(&expected_value, &actual_state);
457+
let cached_resource = target_resource.unwrap_or(resource);
458+
let schema: Option<Value> = get_resource_schema(&cached_resource.type_name, &cached_resource.version)
459+
.or_else(|| {
460+
// Populate the cache on a miss, then read from cache
461+
get_schema(resource, target_resource).ok();
462+
get_resource_schema(&cached_resource.type_name, &cached_resource.version)
463+
});
464+
let diff_properties = get_diff_with_schema(&expected_value, &actual_state, schema.as_ref());
458465
Ok(TestResult::Resource(ResourceTestResponse {
459466
desired_state: expected_value,
460467
actual_state,

‎lib/dsc-lib/src/dscresources/dscresource.rs‎

Lines changed: 124 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,12 @@ impl Invoke for DscResource {
470470
response.actual_state
471471
}
472472
};
473-
let diff_properties = get_diff( &desired_state, &actual_state);
473+
let schema: Option<Value> = if let Some(s) = &self.schema {
474+
serde_json::to_value(s).ok()
475+
} else {
476+
self.schema().ok().and_then(|s| serde_json::from_str(&s).ok())
477+
};
478+
let diff_properties = get_diff_with_schema( &desired_state, &actual_state, schema.as_ref());
474479
desired_state = redact(&desired_state);
475480
let test_result = TestResult::Resource(ResourceTestResponse {
476481
desired_state,
@@ -647,6 +652,24 @@ pub fn get_adapter_input_kind(adapter: &DscResource) -> Result<AdapterInputKind,
647652
///
648653
/// An array of top level properties that differ, if any
649654
pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
655+
get_diff_with_schema(expected, actual, None)
656+
}
657+
658+
#[must_use]
659+
/// Performs a comparison of two JSON Values using an optional JSON Schema.
660+
/// If a property exists in `expected` but not in `actual`, the schema's `default` value
661+
/// for that property is used for comparison when available.
662+
///
663+
/// # Arguments
664+
///
665+
/// * `expected` - The expected value
666+
/// * `actual` - The actual value
667+
/// * `schema` - Optional JSON Schema to look up default values for missing properties
668+
///
669+
/// # Returns
670+
///
671+
/// An array of top level properties that differ, if any
672+
pub(crate) fn get_diff_with_schema(expected: &Value, actual: &Value, schema: Option<&Value>) -> Vec<String> {
650673
let mut diff_properties: Vec<String> = Vec::new();
651674
if expected.is_null() {
652675
return diff_properties;
@@ -702,8 +725,16 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
702725
diff_properties.push(key.to_string());
703726
}
704727
} else {
705-
info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key));
706-
diff_properties.push(key.to_string());
728+
// Property not in actual - check schema for a default value
729+
if let Some(default_value) = get_schema_default(schema, key) {
730+
if value != &default_value {
731+
info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key));
732+
diff_properties.push(key.to_string());
733+
}
734+
} else {
735+
info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key));
736+
diff_properties.push(key.to_string());
737+
}
707738
}
708739
} else {
709740
info!("{}", t!("dscresources.dscresource.diffKeyNotObject", key = key));
@@ -716,6 +747,23 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
716747
diff_properties
717748
}
718749

750+
/// Looks up the default value for a property from a JSON Schema.
751+
///
752+
/// # Arguments
753+
///
754+
/// * `schema` - Optional JSON Schema value
755+
/// * `property_name` - The property name to look up
756+
///
757+
/// # Returns
758+
///
759+
/// The default value if found in the schema's properties definition, otherwise None
760+
fn get_schema_default(schema: Option<&Value>, property_name: &str) -> Option<Value> {
761+
let schema = schema?;
762+
let properties = schema.get("properties")?.as_object()?;
763+
let property_schema = properties.get(property_name)?.as_object()?;
764+
property_schema.get("default").cloned()
765+
}
766+
719767
/// Validates the properties of a resource against its schema.
720768
///
721769
/// # Arguments
@@ -926,3 +974,76 @@ fn different_array_with_nested_array() {
926974
let array_two = vec![json!("a"), json!(1), json!({"a":"b"}), json!(vec![json!("a"), json!(2)])];
927975
assert_eq!(is_same_array(&array_one, &array_two), false);
928976
}
977+
978+
#[test]
979+
fn diff_with_schema_default_matches_expected() {
980+
use serde_json::json;
981+
let expected = json!({"name": "test", "enabled": true});
982+
let actual = json!({"name": "test"});
983+
let schema = json!({
984+
"type": "object",
985+
"properties": {
986+
"name": { "type": "string" },
987+
"enabled": { "type": "boolean", "default": true }
988+
}
989+
});
990+
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
991+
assert!(diff.is_empty(), "Expected no diff when expected matches schema default, got: {diff:?}");
992+
}
993+
994+
#[test]
995+
fn diff_with_schema_default_differs_from_expected() {
996+
use serde_json::json;
997+
let expected = json!({"name": "test", "enabled": false});
998+
let actual = json!({"name": "test"});
999+
let schema = json!({
1000+
"type": "object",
1001+
"properties": {
1002+
"name": { "type": "string" },
1003+
"enabled": { "type": "boolean", "default": true }
1004+
}
1005+
});
1006+
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
1007+
assert_eq!(diff, vec!["enabled".to_string()]);
1008+
}
1009+
1010+
#[test]
1011+
fn diff_with_schema_no_default_reports_missing_property() {
1012+
use serde_json::json;
1013+
let expected = json!({"name": "test", "enabled": true});
1014+
let actual = json!({"name": "test"});
1015+
let schema = json!({
1016+
"type": "object",
1017+
"properties": {
1018+
"name": { "type": "string" },
1019+
"enabled": { "type": "boolean" }
1020+
}
1021+
});
1022+
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
1023+
assert_eq!(diff, vec!["enabled".to_string()]);
1024+
}
1025+
1026+
#[test]
1027+
fn diff_without_schema_reports_missing_property() {
1028+
use serde_json::json;
1029+
let expected = json!({"name": "test", "enabled": true});
1030+
let actual = json!({"name": "test"});
1031+
let diff = get_diff_with_schema(&expected, &actual, None);
1032+
assert_eq!(diff, vec!["enabled".to_string()]);
1033+
}
1034+
1035+
#[test]
1036+
fn diff_with_schema_default_integer() {
1037+
use serde_json::json;
1038+
let expected = json!({"name": "test", "count": 5});
1039+
let actual = json!({"name": "test"});
1040+
let schema = json!({
1041+
"type": "object",
1042+
"properties": {
1043+
"name": { "type": "string" },
1044+
"count": { "type": "integer", "default": 5 }
1045+
}
1046+
});
1047+
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
1048+
assert!(diff.is_empty(), "Expected no diff when expected matches schema default integer, got: {diff:?}");
1049+
}

‎resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaults' -Skip:(!$canRunFirewallTests) {
5+
BeforeDiscovery {
6+
$canRunFirewallTests = $IsWindows -and
7+
(Get-Command Get-NetFirewallRule -ErrorAction Ignore) -and
8+
([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(
9+
[Security.Principal.WindowsBuiltInRole]::Administrator)
10+
}
11+
12+
BeforeAll {
13+
$resourceType = 'Microsoft.Windows/FirewallRuleList'
14+
$testRuleName = 'DSC-WindowsFirewall-SchemaDefault-Test'
15+
16+
# Ensure a known rule exists for testing
17+
$existing = Get-NetFirewallRule -Name $testRuleName -ErrorAction Ignore
18+
if (-not $existing) {
19+
New-NetFirewallRule -Name $testRuleName -DisplayName $testRuleName `
20+
-Direction Inbound -Action Allow -Protocol TCP -LocalPort 32921 `
21+
-Enabled True -PolicyStore PersistentStore | Out-Null
22+
}
23+
}
24+
25+
AfterAll {
26+
Remove-NetFirewallRule -Name $testRuleName -ErrorAction Ignore
27+
}
28+
29+
It 'unspecifiedRulesAction set to default "ignore" does not report as differing' {
30+
$json = @{
31+
unspecifiedRulesAction = 'ignore'
32+
rules = @(@{
33+
name = $testRuleName
34+
direction = 'Inbound'
35+
action = 'Allow'
36+
protocol = 6
37+
localPorts = '32921'
38+
enabled = $true
39+
})
40+
} | ConvertTo-Json -Compress -Depth 5
41+
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
42+
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)
43+
44+
$result = $out | ConvertFrom-Json
45+
$result.inDesiredState | Should -Be $true
46+
$result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction'
47+
}
48+
49+
It 'unspecifiedRulesAction omitted does not report as differing' {
50+
$json = @{
51+
rules = @(@{
52+
name = $testRuleName
53+
direction = 'Inbound'
54+
action = 'Allow'
55+
protocol = 6
56+
localPorts = '32921'
57+
enabled = $true
58+
})
59+
} | ConvertTo-Json -Compress -Depth 5
60+
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
61+
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)
62+
63+
$result = $out | ConvertFrom-Json
64+
$result.inDesiredState | Should -Be $true
65+
$result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction'
66+
}
67+
68+
It 'non-default unspecifiedRulesAction "disable" is reported as differing' {
69+
$json = @{
70+
unspecifiedRulesAction = 'disable'
71+
rules = @(@{
72+
name = $testRuleName
73+
direction = 'Inbound'
74+
action = 'Allow'
75+
protocol = 6
76+
localPorts = '32921'
77+
enabled = $true
78+
})
79+
} | ConvertTo-Json -Compress -Depth 5
80+
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
81+
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)
82+
83+
$result = $out | ConvertFrom-Json
84+
$result.differingProperties | Should -Contain 'unspecifiedRulesAction'
85+
}
86+
87+
It 'non-default unspecifiedRulesAction "remove" is reported as differing' {
88+
$json = @{
89+
unspecifiedRulesAction = 'remove'
90+
rules = @(@{
91+
name = $testRuleName
92+
direction = 'Inbound'
93+
action = 'Allow'
94+
protocol = 6
95+
localPorts = '32921'
96+
enabled = $true
97+
})
98+
} | ConvertTo-Json -Compress -Depth 5
99+
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
100+
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)
101+
102+
$result = $out | ConvertFrom-Json
103+
$result.differingProperties | Should -Contain 'unspecifiedRulesAction'
104+
}
105+
}

‎tools/dsctest/dsctest.dsc.manifests.json‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,45 @@
280280
}
281281
}
282282
},
283+
{
284+
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
285+
"type": "Test/SchemaDefault",
286+
"version": "0.1.0",
287+
"get": {
288+
"executable": "dsctest",
289+
"args": [
290+
"schema-default",
291+
{
292+
"jsonInputArg": "--input",
293+
"mandatory": true
294+
}
295+
]
296+
},
297+
"schema": {
298+
"embedded": {
299+
"$schema": "http://json-schema.org/draft-07/schema#",
300+
"type": "object",
301+
"required": ["name"],
302+
"additionalProperties": false,
303+
"properties": {
304+
"name": {
305+
"type": "string",
306+
"description": "The name of the resource instance."
307+
},
308+
"enabled": {
309+
"type": "boolean",
310+
"description": "Whether the resource is enabled.",
311+
"default": true
312+
},
313+
"count": {
314+
"type": "integer",
315+
"description": "The count value.",
316+
"default": 5
317+
}
318+
}
319+
}
320+
}
321+
},
283322
{
284323
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
285324
"type": "Test/InDesiredState",

0 commit comments

Comments
 (0)