diff --git a/dsc/tests/dsc_schema_default.tests.ps1 b/dsc/tests/dsc_schema_default.tests.ps1 new file mode 100644 index 000000000..286e76eda --- /dev/null +++ b/dsc/tests/dsc_schema_default.tests.ps1 @@ -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 + } +} diff --git a/lib/dsc-lib/src/dscresources/command_resource.rs b/lib/dsc-lib/src/dscresources/command_resource.rs index 6674d4924..5b4479b72 100644 --- a/lib/dsc-lib/src/dscresources/command_resource.rs +++ b/lib/dsc-lib/src/dscresources/command_resource.rs @@ -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, @@ -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 = 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, diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index 8c2566610..84a6480b7 100644 --- a/lib/dsc-lib/src/dscresources/dscresource.rs +++ b/lib/dsc-lib/src/dscresources/dscresource.rs @@ -470,7 +470,12 @@ impl Invoke for DscResource { response.actual_state } }; - let diff_properties = get_diff( &desired_state, &actual_state); + let schema: Option = 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, @@ -647,6 +652,24 @@ pub fn get_adapter_input_kind(adapter: &DscResource) -> Result Vec { + 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 { let mut diff_properties: Vec = Vec::new(); if expected.is_null() { return diff_properties; @@ -702,8 +725,16 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { 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)); @@ -716,6 +747,23 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { 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 { + let schema = schema?; + let properties = schema.get("properties")?.as_object()?; + let property_schema = properties.get(property_name)?.as_object()?; + property_schema.get("default").cloned() +} + /// Validates the properties of a resource against its schema. /// /// # Arguments @@ -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:?}"); +} diff --git a/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 new file mode 100644 index 000000000..fa0d6aa46 --- /dev/null +++ b/resources/windows_firewall/tests/windows_firewall_schema_default.tests.ps1 @@ -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' + } +} diff --git a/tools/dsctest/dsctest.dsc.manifests.json b/tools/dsctest/dsctest.dsc.manifests.json index 258bf0d17..e5bcc8a3c 100644 --- a/tools/dsctest/dsctest.dsc.manifests.json +++ b/tools/dsctest/dsctest.dsc.manifests.json @@ -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", diff --git a/tools/dsctest/src/args.rs b/tools/dsctest/src/args.rs index 18287e764..135572bbe 100644 --- a/tools/dsctest/src/args.rs +++ b/tools/dsctest/src/args.rs @@ -20,6 +20,7 @@ pub enum Schemas { Operation, RefreshEnv, RestartRequired, + SchemaDefault, Set, Sleep, StateAndDiff, @@ -157,6 +158,12 @@ pub enum SubCommand { input: String, }, + #[clap(name = "schema-default", about = "Test resource for schema default values in synthetic test")] + SchemaDefault { + #[clap(name = "input", short, long, help = "The input to the schema-default command as JSON")] + input: String, + }, + #[clap(name = "schema", about = "Get the JSON schema for a subcommand")] Schema { #[clap(name = "subcommand", short, long, help = "The subcommand to get the schema for")] diff --git a/tools/dsctest/src/main.rs b/tools/dsctest/src/main.rs index dc5c56f7f..a7e8f4ebe 100644 --- a/tools/dsctest/src/main.rs +++ b/tools/dsctest/src/main.rs @@ -16,6 +16,7 @@ mod operation; mod adapter; mod refresh_env; mod restart_required; +mod schema_default; mod set; mod sleep; mod state_and_diff; @@ -41,6 +42,7 @@ use crate::metadata::Metadata; use crate::operation::Operation; use crate::refresh_env::RefreshEnv; use crate::restart_required::RestartRequired; +use crate::schema_default::SchemaDefault; use crate::set::{Set, invoke_set}; use crate::sleep::Sleep; use crate::state_and_diff::StateAndDiff; @@ -288,6 +290,18 @@ fn main() { }; serde_json::to_string(&restart_required).unwrap() }, + SubCommand::SchemaDefault { input } => { + let schema_default = match serde_json::from_str::(&input) { + Ok(sd) => sd, + Err(err) => { + eprintln!("Error JSON does not match schema: {err}"); + std::process::exit(1); + } + }; + // Only return 'name' in the output - omit 'enabled' and 'count' + // to test schema default comparison + serde_json::json!({"name": schema_default.name}).to_string() + }, SubCommand::Schema { subcommand } => { let schema = match subcommand { Schemas::Adapter => { @@ -335,6 +349,9 @@ fn main() { Schemas::RestartRequired => { schema_for!(RestartRequired) }, + Schemas::SchemaDefault => { + schema_for!(SchemaDefault) + }, Schemas::Set => { schema_for!(Set) }, diff --git a/tools/dsctest/src/schema_default.rs b/tools/dsctest/src/schema_default.rs new file mode 100644 index 000000000..6a661de48 --- /dev/null +++ b/tools/dsctest/src/schema_default.rs @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +pub struct SchemaDefault { + pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub enabled: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub count: Option, +}