Skip to content
Open
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
54 changes: 54 additions & 0 deletions dsc/tests/dsc_schema_default.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

Describe 'Synthetic test uses schema defaults' {
It 'Property matching schema default is not reported as differing' {
$out = '{"name":"test","enabled":true}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.inDesiredState | Should -Be $true
$out.differingProperties | Should -BeNullOrEmpty
}

It 'Property differing from schema default is reported as differing' {
$out = '{"name":"test","enabled":false}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.inDesiredState | Should -Be $false
$out.differingProperties | Should -Contain 'enabled'
}

It 'Integer property matching schema default is not reported as differing' {
$out = '{"name":"test","count":5}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.inDesiredState | Should -Be $true
$out.differingProperties | Should -BeNullOrEmpty
}

It 'Integer property differing from schema default is reported as differing' {
$out = '{"name":"test","count":10}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.inDesiredState | Should -Be $false
$out.differingProperties | Should -Contain 'count'
}

It 'Multiple properties matching schema defaults are not reported as differing' {
$out = '{"name":"test","enabled":true,"count":5}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.inDesiredState | Should -Be $true
$out.differingProperties | Should -BeNullOrEmpty
}

It 'Mix of matching and non-matching defaults reports only non-matching' {
$out = '{"name":"test","enabled":true,"count":10}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.inDesiredState | Should -Be $false
$out.differingProperties | Should -Contain 'count'
$out.differingProperties | Should -Not -Contain 'enabled'
}

It 'Property present in both expected and actual is compared normally' {
$out = '{"name":"test"}' | dsc resource test -r Test/SchemaDefault -f - | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.inDesiredState | Should -Be $true
$out.differingProperties | Should -BeNullOrEmpty
}
}
11 changes: 9 additions & 2 deletions lib/dsc-lib/src/dscresources/command_resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{configure::{config_doc::{ExecutionKind, SecurityContextKind}, config
use crate::dscerror::DscError;
use crate::locked_insert;
use super::{
dscresource::{get_diff, redact, DscResource},
dscresource::{get_diff, get_diff_with_schema, redact, DscResource},
invoke_result::{
DeleteResult, DeleteResultKind, ExportResult,
GetResult, ResolveResult, SetResult, TestResult, ValidateResult,
Expand Down Expand Up @@ -454,7 +454,14 @@ fn invoke_synthetic_test(resource: &DscResource, expected: &str, target_resource
}
};
let expected_value: Value = serde_json::from_str(expected)?;
let diff_properties = get_diff(&expected_value, &actual_state);
let cached_resource = target_resource.unwrap_or(resource);
let schema: Option<Value> = get_resource_schema(&cached_resource.type_name, &cached_resource.version)
.or_else(|| {
// Populate the cache on a miss, then read from cache
get_schema(resource, target_resource).ok();
get_resource_schema(&cached_resource.type_name, &cached_resource.version)
});
let diff_properties = get_diff_with_schema(&expected_value, &actual_state, schema.as_ref());
Ok(TestResult::Resource(ResourceTestResponse {
desired_state: expected_value,
actual_state,
Expand Down
127 changes: 124 additions & 3 deletions lib/dsc-lib/src/dscresources/dscresource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,12 @@ impl Invoke for DscResource {
response.actual_state
}
};
let diff_properties = get_diff( &desired_state, &actual_state);
let schema: Option<Value> = if let Some(s) = &self.schema {
serde_json::to_value(s).ok()
} else {
self.schema().ok().and_then(|s| serde_json::from_str(&s).ok())
};
let diff_properties = get_diff_with_schema( &desired_state, &actual_state, schema.as_ref());
desired_state = redact(&desired_state);
let test_result = TestResult::Resource(ResourceTestResponse {
desired_state,
Expand Down Expand Up @@ -647,6 +652,24 @@ pub fn get_adapter_input_kind(adapter: &DscResource) -> Result<AdapterInputKind,
///
/// An array of top level properties that differ, if any
pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
get_diff_with_schema(expected, actual, None)
}

#[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.
///
/// # Arguments
///
/// * `expected` - The expected value
/// * `actual` - The actual value
/// * `schema` - Optional JSON Schema to look up default values for missing properties
///
/// # Returns
///
/// An array of top level properties that differ, if any
pub(crate) fn get_diff_with_schema(expected: &Value, actual: &Value, schema: Option<&Value>) -> Vec<String> {
let mut diff_properties: Vec<String> = Vec::new();
if expected.is_null() {
return diff_properties;
Expand Down Expand Up @@ -702,8 +725,16 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
diff_properties.push(key.to_string());
}
} else {
info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key));
diff_properties.push(key.to_string());
// Property not in actual - check schema for a default value
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());
}
} else {
info!("{}", t!("dscresources.dscresource.diffKeyMissing", key = key));
diff_properties.push(key.to_string());
}
}
} else {
info!("{}", t!("dscresources.dscresource.diffKeyNotObject", key = key));
Expand All @@ -716,6 +747,23 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
diff_properties
}

/// Looks up the default value for a property from a JSON Schema.
///
/// # Arguments
///
/// * `schema` - Optional JSON Schema value
/// * `property_name` - The property name to look up
///
/// # Returns
///
/// The default value if found in the schema's properties definition, otherwise None
fn get_schema_default(schema: Option<&Value>, property_name: &str) -> Option<Value> {
let schema = schema?;
let properties = schema.get("properties")?.as_object()?;
let property_schema = properties.get(property_name)?.as_object()?;
property_schema.get("default").cloned()
}
Comment on lines +750 to +765

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: Out of scope for this PR, but I think this fits better as an extension method in dsc-lib-jsonschema - probably something like the following signature:

pub get_default_property_value(
  &Self,
  property_name: &str,
) -> Option<&Value> {}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agree we can improve this outside of this PR


/// Validates the properties of a resource against its schema.
///
/// # Arguments
Expand Down Expand Up @@ -926,3 +974,76 @@ fn different_array_with_nested_array() {
let array_two = vec![json!("a"), json!(1), json!({"a":"b"}), json!(vec![json!("a"), json!(2)])];
assert_eq!(is_same_array(&array_one, &array_two), false);
}

#[test]
fn diff_with_schema_default_matches_expected() {
use serde_json::json;
let expected = json!({"name": "test", "enabled": true});
let actual = json!({"name": "test"});
let schema = json!({
"type": "object",
"properties": {
"name": { "type": "string" },
"enabled": { "type": "boolean", "default": true }
}
});
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
assert!(diff.is_empty(), "Expected no diff when expected matches schema default, got: {diff:?}");
}

#[test]
fn diff_with_schema_default_differs_from_expected() {
use serde_json::json;
let expected = json!({"name": "test", "enabled": false});
let actual = json!({"name": "test"});
let schema = json!({
"type": "object",
"properties": {
"name": { "type": "string" },
"enabled": { "type": "boolean", "default": true }
}
});
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
assert_eq!(diff, vec!["enabled".to_string()]);
}

#[test]
fn diff_with_schema_no_default_reports_missing_property() {
use serde_json::json;
let expected = json!({"name": "test", "enabled": true});
let actual = json!({"name": "test"});
let schema = json!({
"type": "object",
"properties": {
"name": { "type": "string" },
"enabled": { "type": "boolean" }
}
});
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
assert_eq!(diff, vec!["enabled".to_string()]);
}

#[test]
fn diff_without_schema_reports_missing_property() {
use serde_json::json;
let expected = json!({"name": "test", "enabled": true});
let actual = json!({"name": "test"});
let diff = get_diff_with_schema(&expected, &actual, None);
assert_eq!(diff, vec!["enabled".to_string()]);
}

#[test]
fn diff_with_schema_default_integer() {
use serde_json::json;
let expected = json!({"name": "test", "count": 5});
let actual = json!({"name": "test"});
let schema = json!({
"type": "object",
"properties": {
"name": { "type": "string" },
"count": { "type": "integer", "default": 5 }
}
});
let diff = get_diff_with_schema(&expected, &actual, Some(&schema));
assert!(diff.is_empty(), "Expected no diff when expected matches schema default integer, got: {diff:?}");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

Describe 'Microsoft.Windows/FirewallRuleList - synthetic test with schema defaults' -Skip:(!$canRunFirewallTests) {
BeforeDiscovery {
$canRunFirewallTests = $IsWindows -and
(Get-Command Get-NetFirewallRule -ErrorAction Ignore) -and
([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(
[Security.Principal.WindowsBuiltInRole]::Administrator)
}

BeforeAll {
$resourceType = 'Microsoft.Windows/FirewallRuleList'
$testRuleName = 'DSC-WindowsFirewall-SchemaDefault-Test'

# Ensure a known rule exists for testing
$existing = Get-NetFirewallRule -Name $testRuleName -ErrorAction Ignore
if (-not $existing) {
New-NetFirewallRule -Name $testRuleName -DisplayName $testRuleName `
-Direction Inbound -Action Allow -Protocol TCP -LocalPort 32921 `
-Enabled True -PolicyStore PersistentStore | Out-Null
}
}

AfterAll {
Remove-NetFirewallRule -Name $testRuleName -ErrorAction Ignore
}

It 'unspecifiedRulesAction set to default "ignore" does not report as differing' {
$json = @{
unspecifiedRulesAction = 'ignore'
rules = @(@{
name = $testRuleName
direction = 'Inbound'
action = 'Allow'
protocol = 6
localPorts = '32921'
enabled = $true
})
} | ConvertTo-Json -Compress -Depth 5
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)

$result = $out | ConvertFrom-Json
$result.inDesiredState | Should -Be $true
$result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction'
}

It 'unspecifiedRulesAction omitted does not report as differing' {
$json = @{
rules = @(@{
name = $testRuleName
direction = 'Inbound'
action = 'Allow'
protocol = 6
localPorts = '32921'
enabled = $true
})
} | ConvertTo-Json -Compress -Depth 5
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)

$result = $out | ConvertFrom-Json
$result.inDesiredState | Should -Be $true
$result.differingProperties | Should -Not -Contain 'unspecifiedRulesAction'
}

It 'non-default unspecifiedRulesAction "disable" is reported as differing' {
$json = @{
unspecifiedRulesAction = 'disable'
rules = @(@{
name = $testRuleName
direction = 'Inbound'
action = 'Allow'
protocol = 6
localPorts = '32921'
enabled = $true
})
} | ConvertTo-Json -Compress -Depth 5
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)

$result = $out | ConvertFrom-Json
$result.differingProperties | Should -Contain 'unspecifiedRulesAction'
}

It 'non-default unspecifiedRulesAction "remove" is reported as differing' {
$json = @{
unspecifiedRulesAction = 'remove'
rules = @(@{
name = $testRuleName
direction = 'Inbound'
action = 'Allow'
protocol = 6
localPorts = '32921'
enabled = $true
})
} | ConvertTo-Json -Compress -Depth 5
$out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)

$result = $out | ConvertFrom-Json
$result.differingProperties | Should -Contain 'unspecifiedRulesAction'
}
}
39 changes: 39 additions & 0 deletions tools/dsctest/dsctest.dsc.manifests.json
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,45 @@
}
}
},
{
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
"type": "Test/SchemaDefault",
"version": "0.1.0",
"get": {
"executable": "dsctest",
"args": [
"schema-default",
{
"jsonInputArg": "--input",
"mandatory": true
}
]
},
"schema": {
"embedded": {
"$schema": "http://json-schema.org/draft-07/schema#",
"type": "object",
"required": ["name"],
"additionalProperties": false,
"properties": {
"name": {
"type": "string",
"description": "The name of the resource instance."
},
"enabled": {
"type": "boolean",
"description": "Whether the resource is enabled.",
"default": true
},
"count": {
"type": "integer",
"description": "The count value.",
"default": 5
}
}
}
}
},
{
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
"type": "Test/InDesiredState",
Expand Down
Loading
Loading