From 230842652d6ecd55f86ef677cc206ebef052eb82 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 16 Apr 2024 16:53:32 -0400 Subject: [PATCH 01/11] add whatif to metadata changes --- dsc/src/args.rs | 2 + dsc/src/resource_command.rs | 4 +- dsc/src/subcommand.rs | 8 ++- dsc_lib/src/configure/context.rs | 1 - dsc_lib/src/configure/mod.rs | 10 +-- dsc_lib/src/dscresources/command_resource.rs | 32 +++++++--- dsc_lib/src/dscresources/dscresource.rs | 67 ++++++++++++++++++-- dsc_lib/src/dscresources/invoke_result.rs | 8 +++ dsc_lib/src/lib.rs | 3 +- 9 files changed, 113 insertions(+), 22 deletions(-) diff --git a/dsc/src/args.rs b/dsc/src/args.rs index 83cc6771..de2745b5 100644 --- a/dsc/src/args.rs +++ b/dsc/src/args.rs @@ -90,6 +90,8 @@ pub enum ConfigSubCommand { path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, + #[clap(short = 'w', long, help = "Run as a what-if operation instead of executing the configuration or resource")] + what_if: bool, }, #[clap(name = "test", about = "Test the current configuration")] Test { diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index ba40fcac..39ff6fa9 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -3,7 +3,7 @@ use crate::args::OutputFormat; use crate::util::{EXIT_DSC_ERROR, EXIT_INVALID_ARGS, EXIT_JSON_ERROR, add_type_name_to_json, write_output}; -use dsc_lib::configure::config_doc::Configuration; +use dsc_lib::configure::config_doc::{Configuration, ExecutionKind}; use dsc_lib::configure::add_resource_export_results_to_configuration; use dsc_lib::dscresources::invoke_result::{GetResult, ResourceGetResponse}; use dsc_lib::dscerror::DscError; @@ -117,7 +117,7 @@ pub fn set(dsc: &DscManager, resource_type: &str, mut input: String, format: &Op }; } - match resource.set(input.as_str(), true) { + match resource.set(input.as_str(), true, &ExecutionKind::Actual) { Ok(result) => { // convert to json let json = match serde_json::to_string(&result) { diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index 2d153c39..3d635a0b 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -6,7 +6,7 @@ use crate::resource_command::{get_resource, self}; use crate::Stream; use crate::tablewriter::Table; use crate::util::{EXIT_DSC_ERROR, EXIT_INVALID_INPUT, EXIT_JSON_ERROR, EXIT_VALIDATION_FAILED, get_schema, write_output, get_input, set_dscconfigroot, validate_json}; -use dsc_lib::configure::{Configurator, ErrorAction, config_result::ResourceGetResult}; +use dsc_lib::configure::{Configurator, ErrorAction, config_doc::ExecutionKind, config_result::ResourceGetResult}; use dsc_lib::dscerror::DscError; use dsc_lib::dscresources::invoke_result::{ GroupResourceSetResponse, GroupResourceTestResponse, TestResult @@ -225,6 +225,12 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option, stdin: } }; + if let ConfigSubCommand::Set { what_if , .. } = subcommand { + if *what_if { + configurator.context.execution_type = ExecutionKind::WhatIf; + } + }; + let parameters: Option = match parameters { None => None, Some(parameters) => { diff --git a/dsc_lib/src/configure/context.rs b/dsc_lib/src/configure/context.rs index 93823ebe..6fd52f72 100644 --- a/dsc_lib/src/configure/context.rs +++ b/dsc_lib/src/configure/context.rs @@ -15,7 +15,6 @@ pub struct Context { pub parameters: HashMap, pub security_context: SecurityContextKind, _variables: HashMap, - pub start_datetime: DateTime, } diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index c2132faf..35d957de 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -31,7 +31,7 @@ pub mod parameters; pub struct Configurator { config: String, - context: Context, + pub context: Context, discovery: Discovery, statement_parser: Statement, } @@ -317,7 +317,7 @@ impl Configurator { if exist || dsc_resource.capabilities.contains(&Capability::SetHandlesExist) { debug!("Resource handles _exist or _exist is true"); let start_datetime = chrono::Local::now(); - let set_result = dsc_resource.set(&desired, skip_test)?; + let set_result = dsc_resource.set(&desired, skip_test, &self.context.execution_type)?; let end_datetime = chrono::Local::now(); self.context.outputs.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&set_result)?); let resource_result = config_result::ResourceSetResult { @@ -335,7 +335,7 @@ impl Configurator { resource_type: resource.resource_type.clone(), result: set_result, }; - result.results.push(resource_result); + result.results.push(resource_result); } else if dsc_resource.capabilities.contains(&Capability::Delete) { debug!("Resource implements delete and _exist is false"); let before_result = dsc_resource.get(&desired)?; @@ -343,10 +343,10 @@ impl Configurator { dsc_resource.delete(&desired)?; let end_datetime = chrono::Local::now(); let after_result = dsc_resource.get(&desired)?; - // convert get result to set result + // convert get result to set result let set_result = match before_result { GetResult::Resource(before_response) => { - let GetResult::Resource(after_result) = after_result else { + let GetResult::Resource(after_result) = after_result else { return Err(DscError::NotSupported("Group resources not supported for delete".to_string())) }; let before_value = serde_json::to_value(&before_response.actual_state)?; diff --git a/dsc_lib/src/dscresources/command_resource.rs b/dsc_lib/src/dscresources/command_resource.rs index c0718bc0..4276591f 100644 --- a/dsc_lib/src/dscresources/command_resource.rs +++ b/dsc_lib/src/dscresources/command_resource.rs @@ -4,9 +4,9 @@ use jsonschema::JSONSchema; use serde_json::Value; use std::{collections::HashMap, env, io::{Read, Write}, process::{Command, Stdio}}; -use crate::{dscerror::DscError, dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse, ResourceTestResponse}}; -use crate::configure::config_result::ResourceGetResult; -use super::{dscresource::get_diff, invoke_result::{ExportResult, GetResult, SetResult, TestResult, ValidateResult}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}}; +use crate::{dscerror::DscError, dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse, ResourceSetWhatIfResponse, ResourceTestResponse}}; +use crate::configure::{config_doc::ExecutionKind, config_result::ResourceGetResult}; +use super::{dscresource::{get_diff, get_diff_what_if}, invoke_result::{ExportResult, GetResult, SetResult, TestResult, ValidateResult}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}}; use tracing::{error, warn, info, debug, trace}; pub const EXIT_PROCESS_TERMINATED: i32 = 0x102; @@ -95,7 +95,7 @@ pub fn invoke_get(resource: &ResourceManifest, cwd: &str, filter: &str) -> Resul /// /// Error returned if the resource does not successfully set the desired state #[allow(clippy::too_many_lines)] -pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_test: bool) -> Result { +pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_test: bool, execution_type: &ExecutionKind) -> Result { let Some(set) = &resource.set else { return Err(DscError::NotImplemented("set".to_string())); }; @@ -104,19 +104,35 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te // if resource doesn't implement a pre-test, we execute test first to see if a set is needed if !skip_test && set.pre_test != Some(true) { info!("No pretest, invoking test {}", &resource.resource_type); - let (in_desired_state, actual_state) = match invoke_test(resource, cwd, desired)? { + let (in_desired_state, actual_state, desired_state) = match invoke_test(resource, cwd, desired)? { TestResult::Group(group_response) => { let mut result_array: Vec = Vec::new(); for result in group_response.results { result_array.push(serde_json::to_value(result)?); } - (group_response.in_desired_state, Value::from(result_array)) + (group_response.in_desired_state, Value::from(result_array), Value::Null) }, TestResult::Resource(response) => { - (response.in_desired_state, response.actual_state) + (response.in_desired_state, response.actual_state, response.desired_state) } }; + if execution_type == &ExecutionKind::WhatIf { + if in_desired_state { + debug!("what-if: resource is already in desired state, returning null ResourceWhatIf response"); + return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ + what_if_changes: Value::String("none".to_string()) + })); + } + debug!("what-if: resource is not in desired state, returning diff ResourceWhatIf response"); + let diff_properties = get_diff_what_if( &desired_state, &actual_state); + println!("desired_state: {:?}", &desired_state); + println!("actual_state: {:?}", &actual_state); + println!("diff_properties: {:?}", &diff_properties); + return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ + what_if_changes: Value::from_iter(diff_properties) + })); + } if in_desired_state { return Ok(SetResult::Resource(ResourceSetResponse{ before_state: serde_json::from_str(desired)?, @@ -490,7 +506,7 @@ pub fn invoke_export(resource: &ResourceManifest, cwd: &str, input: Option<&str> if let Some(input) = input { if !input.is_empty() { verify_json(resource, cwd, input)?; - + command_input = get_command_input(&export.input, input)?; } diff --git a/dsc_lib/src/dscresources/dscresource.rs b/dsc_lib/src/dscresources/dscresource.rs index 37fa8317..464af920 100644 --- a/dsc_lib/src/dscresources/dscresource.rs +++ b/dsc_lib/src/dscresources/dscresource.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::dscresources::resource_manifest::Kind; +use crate::{configure::config_doc::ExecutionKind, dscresources::resource_manifest::Kind}; use dscerror::DscError; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -116,7 +116,7 @@ pub trait Invoke { /// # Errors /// /// This function will return an error if the underlying resource fails. - fn set(&self, desired: &str, skip_test: bool) -> Result; + fn set(&self, desired: &str, skip_test: bool, execution_type: &ExecutionKind) -> Result; /// Invoke the test operation on the resource. /// @@ -186,7 +186,7 @@ impl Invoke for DscResource { } } - fn set(&self, desired: &str, skip_test: bool) -> Result { + fn set(&self, desired: &str, skip_test: bool, execution_type: &ExecutionKind) -> Result { match &self.implemented_as { ImplementedAs::Custom(_custom) => { Err(DscError::NotImplemented("set custom resources".to_string())) @@ -196,7 +196,7 @@ impl Invoke for DscResource { return Err(DscError::MissingManifest(self.type_name.clone())); }; let resource_manifest = import_manifest(manifest.clone())?; - command_resource::invoke_set(&resource_manifest, &self.directory, desired, skip_test) + command_resource::invoke_set(&resource_manifest, &self.directory, desired, skip_test, execution_type) }, } } @@ -356,3 +356,62 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { diff_properties } + +#[must_use] +pub fn get_diff_what_if(expected: &Value, actual: &Value) -> HashMap { + let mut diff_properties: HashMap = HashMap::new(); + if expected.is_null() { + return diff_properties; + } + + let mut expected = expected.clone(); + let mut actual = actual.clone(); + + if let Some(map) = expected.as_object_mut() { + // handle well-known optional properties with default values by adding them + for (key, value) in get_well_known_properties() { + if !map.contains_key(&key) { + map.insert(key.clone(), value.clone()); + } + + if actual.is_object() && actual[&key].is_null() { + actual[key.clone()] = value.clone(); + } + } + + for (key, value) in &*map { + let mut is_diff = false; + if value.is_object() { + let sub_diff = get_diff_what_if(value, &actual[key]); + if !sub_diff.is_empty() { + is_diff = true; + } + } + else { + match actual.as_object() { + Some(actual_object) => { + if actual_object.contains_key(key) { + if value != &actual[key] { + is_diff = true; + } + } + else { + is_diff = true; + } + }, + None => { + is_diff = true; + }, + } + } + if is_diff { + let mut temp_hashmap = HashMap::new(); + temp_hashmap.insert("to".to_string(), value.clone()); + temp_hashmap.insert("from".to_string(), actual[key].clone()); + diff_properties.insert(key.to_string(), Value::from_iter(temp_hashmap)); + } + } + } + + diff_properties +} \ No newline at end of file diff --git a/dsc_lib/src/dscresources/invoke_result.rs b/dsc_lib/src/dscresources/invoke_result.rs index 5df73e37..5f5293d4 100644 --- a/dsc_lib/src/dscresources/invoke_result.rs +++ b/dsc_lib/src/dscresources/invoke_result.rs @@ -66,6 +66,7 @@ impl Default for GroupResourceSetResponse { #[serde(untagged)] pub enum SetResult { Resource(ResourceSetResponse), + ResourceWhatIf(ResourceSetWhatIfResponse), Group(GroupResourceSetResponse), } @@ -83,6 +84,13 @@ pub struct ResourceSetResponse { pub changed_properties: Option>, } +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct ResourceSetWhatIfResponse { + #[serde(rename = "whatIfChanges")] + pub what_if_changes: Value +} + #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct GroupResourceTestResponse { diff --git a/dsc_lib/src/lib.rs b/dsc_lib/src/lib.rs index d871952f..ac880846 100644 --- a/dsc_lib/src/lib.rs +++ b/dsc_lib/src/lib.rs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +use configure::config_doc::ExecutionKind; use dscerror::DscError; use dscresources::{dscresource::{DscResource, Invoke}, invoke_result::{GetResult, SetResult, TestResult}}; @@ -74,7 +75,7 @@ impl DscManager { /// This function will return an error if the underlying resource fails. /// pub fn resource_set(&self, resource: &DscResource, input: &str, skip_test: bool) -> Result { - resource.set(input, skip_test) + resource.set(input, skip_test, &ExecutionKind::Actual) } /// Invoke the test operation on a resource. From b6eb1f7ed379406cc14453fe0fe8221e1bbb8316 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 18 Apr 2024 15:34:42 -0400 Subject: [PATCH 02/11] add tests --- dsc/tests/dsc_whatif.tests.ps1 | 67 ++++++++++++++++++++ dsc_lib/src/configure/mod.rs | 6 +- dsc_lib/src/dscresources/command_resource.rs | 11 ++-- 3 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 dsc/tests/dsc_whatif.tests.ps1 diff --git a/dsc/tests/dsc_whatif.tests.ps1 b/dsc/tests/dsc_whatif.tests.ps1 new file mode 100644 index 00000000..31cc2c50 --- /dev/null +++ b/dsc/tests/dsc_whatif.tests.ps1 @@ -0,0 +1,67 @@ +Describe 'whatif tests' { + AfterEach { + if ($IsWindows) { + Remove-Item -Path 'HKCU:\1' -Recurse -ErrorAction Ignore + } + } + + It 'config set whatif when actual state matches desired state' { + $config_yaml = @" + `$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/10/config/document.json + resources: + - name: Hello + type: Test/Echo + properties: + output: hello +"@ + $result = $config_yaml | dsc config set -w --format pretty-json | ConvertFrom-Json + $result.hadErrors | Should -BeFalse + $result.results.Count | Should -Be 1 + $result.results[0].Name | Should -Be 'Hello' + $result.results[0].type | Should -BeExactly 'Test/Echo' + $result.results[0].result.whatIfChanges | Should -Be 'none' + $result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' + $LASTEXITCODE | Should -Be 0 + } + + It 'config set whatif when actual state does not match desired state' -Skip:(!$IsWindows) { + # TODO: change/create cross-plat resource that implements set without just matching desired state + $config_yaml = @" + `$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/10/config/document.json + resources: + - name: Registry + type: Microsoft.Windows/Registry + properties: + keyPath: 'HKCU\1\2' +"@ + $result = $config_yaml | dsc config set -w --format pretty-json | ConvertFrom-Json + $result.hadErrors | Should -BeFalse + $result.results.Count | Should -Be 1 + $result.results[0].Name | Should -Be 'Registry' + $result.results[0].type | Should -BeExactly 'Microsoft.Windows/Registry' + $result.results[0].result.whatIfChanges._exist.from | Should -Be 'false' + $result.results[0].result.whatIfChanges._exist.to | Should -Be 'true' + $result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' + $LASTEXITCODE | Should -Be 0 + } + + It 'config set whatif for delete is not supported' { + $config_yaml = @" + `$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/10/config/document.json + resources: + - name: Delete + type: Test/Delete + properties: + _exist: false +"@ + $result = $config_yaml | dsc config set -w 2>&1 + $result | Should -Match 'ERROR.*?Not implemented.*?what-if' + $LASTEXITCODE | Should -Be 2 + } + + It 'config set whatif when there is no pre-test is not supported' { + $result = dsc config set -p c:\dsc\dsc\examples\groups.dsc.yaml -w 2>&1 + $result | Should -Match 'ERROR.*?Not implemented.*?what-if' + $LASTEXITCODE | Should -Be 2 + } +} diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 35d957de..7554a4a0 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::configure::config_doc::Metadata; +use crate::configure::config_doc::{ExecutionKind, Metadata}; use crate::configure::parameters::Input; use crate::dscerror::DscError; use crate::dscresources::dscresource::get_diff; @@ -336,7 +336,7 @@ impl Configurator { result: set_result, }; result.results.push(resource_result); - } else if dsc_resource.capabilities.contains(&Capability::Delete) { + } else if dsc_resource.capabilities.contains(&Capability::Delete) && (self.context.execution_type != ExecutionKind::WhatIf) { debug!("Resource implements delete and _exist is false"); let before_result = dsc_resource.get(&desired)?; let start_datetime = chrono::Local::now(); @@ -378,6 +378,8 @@ impl Configurator { result: SetResult::Resource(set_result), }; result.results.push(resource_result); + } else if self.context.execution_type == ExecutionKind::WhatIf { + return Err(DscError::NotImplemented(format!("Resource '{}' does not support `what-if flag` with `delete`", resource.resource_type))); } else { return Err(DscError::NotImplemented(format!("Resource '{}' does not support `delete` and does not handle `_exist` as false", resource.resource_type))); } diff --git a/dsc_lib/src/dscresources/command_resource.rs b/dsc_lib/src/dscresources/command_resource.rs index 4276591f..ad3d8cac 100644 --- a/dsc_lib/src/dscresources/command_resource.rs +++ b/dsc_lib/src/dscresources/command_resource.rs @@ -110,7 +110,7 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te for result in group_response.results { result_array.push(serde_json::to_value(result)?); } - (group_response.in_desired_state, Value::from(result_array), Value::Null) + (group_response.in_desired_state, Value::from(result_array), Value::String("not implemented".to_string())) }, TestResult::Resource(response) => { (response.in_desired_state, response.actual_state, response.desired_state) @@ -118,6 +118,9 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te }; if execution_type == &ExecutionKind::WhatIf { + if desired_state == Value::String("not implemented".to_string()) { + return Err(DscError::NotImplemented(format!("Group '{}' does not currently support `what-if flag`", resource.resource_type))); + } if in_desired_state { debug!("what-if: resource is already in desired state, returning null ResourceWhatIf response"); return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ @@ -126,9 +129,6 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te } debug!("what-if: resource is not in desired state, returning diff ResourceWhatIf response"); let diff_properties = get_diff_what_if( &desired_state, &actual_state); - println!("desired_state: {:?}", &desired_state); - println!("actual_state: {:?}", &actual_state); - println!("diff_properties: {:?}", &diff_properties); return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ what_if_changes: Value::from_iter(diff_properties) })); @@ -140,6 +140,9 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te changed_properties: None, })); } + } else if execution_type == &ExecutionKind::WhatIf { + // until resources implement what-if, if we are in what-if mode and there is no pre-test, we can't determine what-if changes + return Err(DscError::NotImplemented(format!("Resource '{}' does not currently support `what-if flag` because there is no pre-test", resource.resource_type))); } let args = process_args(&resource.get.args, desired); From 1d234c1b7e6f14156bb72b732ca31ebdd564717c Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 18 Apr 2024 15:56:03 -0400 Subject: [PATCH 03/11] use relative path --- dsc/tests/dsc_whatif.tests.ps1 | 2 +- dsc_lib/src/dscresources/dscresource.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc/tests/dsc_whatif.tests.ps1 b/dsc/tests/dsc_whatif.tests.ps1 index 31cc2c50..5e7e4a13 100644 --- a/dsc/tests/dsc_whatif.tests.ps1 +++ b/dsc/tests/dsc_whatif.tests.ps1 @@ -60,7 +60,7 @@ Describe 'whatif tests' { } It 'config set whatif when there is no pre-test is not supported' { - $result = dsc config set -p c:\dsc\dsc\examples\groups.dsc.yaml -w 2>&1 + $result = dsc config -l trace set -p .\..\examples\groups.dsc.yaml -w 2>&1 $result | Should -Match 'ERROR.*?Not implemented.*?what-if' $LASTEXITCODE | Should -Be 2 } diff --git a/dsc_lib/src/dscresources/dscresource.rs b/dsc_lib/src/dscresources/dscresource.rs index 464af920..73ed421e 100644 --- a/dsc_lib/src/dscresources/dscresource.rs +++ b/dsc_lib/src/dscresources/dscresource.rs @@ -414,4 +414,4 @@ pub fn get_diff_what_if(expected: &Value, actual: &Value) -> HashMap Date: Thu, 18 Apr 2024 16:43:59 -0400 Subject: [PATCH 04/11] fix test typo --- dsc/tests/dsc_whatif.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc/tests/dsc_whatif.tests.ps1 b/dsc/tests/dsc_whatif.tests.ps1 index 5e7e4a13..36f22b7f 100644 --- a/dsc/tests/dsc_whatif.tests.ps1 +++ b/dsc/tests/dsc_whatif.tests.ps1 @@ -60,7 +60,7 @@ Describe 'whatif tests' { } It 'config set whatif when there is no pre-test is not supported' { - $result = dsc config -l trace set -p .\..\examples\groups.dsc.yaml -w 2>&1 + $result = dsc config set -p .\..\examples\groups.dsc.yaml -w 2>&1 $result | Should -Match 'ERROR.*?Not implemented.*?what-if' $LASTEXITCODE | Should -Be 2 } From 470d86f4ac0bca3151ad2f229b58553c2b51489d Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Wed, 24 Apr 2024 16:07:45 -0400 Subject: [PATCH 05/11] update delete what-if scenario --- dsc/src/resource_command.rs | 4 +- dsc/tests/dsc_whatif.tests.ps1 | 18 +-- dsc_lib/src/configure/mod.rs | 111 ++++++++----------- dsc_lib/src/dscresources/command_resource.rs | 13 ++- dsc_lib/src/dscresources/dscresource.rs | 6 +- dsc_lib/src/dscresources/invoke_result.rs | 4 +- 6 files changed, 75 insertions(+), 81 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 39ff6fa9..ef3028bc 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -191,8 +191,8 @@ pub fn delete(dsc: &DscManager, resource_type: &str, mut input: String) { }; } - match resource.delete(input.as_str()) { - Ok(()) => {} + match resource.delete(input.as_str(), &ExecutionKind::Actual) { + Ok(_) => {} Err(err) => { error!("Error: {err}"); exit(EXIT_DSC_ERROR); diff --git a/dsc/tests/dsc_whatif.tests.ps1 b/dsc/tests/dsc_whatif.tests.ps1 index 36f22b7f..de1ef741 100644 --- a/dsc/tests/dsc_whatif.tests.ps1 +++ b/dsc/tests/dsc_whatif.tests.ps1 @@ -19,7 +19,7 @@ Describe 'whatif tests' { $result.results.Count | Should -Be 1 $result.results[0].Name | Should -Be 'Hello' $result.results[0].type | Should -BeExactly 'Test/Echo' - $result.results[0].result.whatIfChanges | Should -Be 'none' + $result.results[0].result.changes | Should -Be $null $result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' $LASTEXITCODE | Should -Be 0 } @@ -39,13 +39,13 @@ Describe 'whatif tests' { $result.results.Count | Should -Be 1 $result.results[0].Name | Should -Be 'Registry' $result.results[0].type | Should -BeExactly 'Microsoft.Windows/Registry' - $result.results[0].result.whatIfChanges._exist.from | Should -Be 'false' - $result.results[0].result.whatIfChanges._exist.to | Should -Be 'true' + $result.results[0].result.changes[0]._exist.from | Should -Be 'false' + $result.results[0].result.changes[0]._exist.to | Should -Be 'true' $result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' $LASTEXITCODE | Should -Be 0 } - It 'config set whatif for delete is not supported' { + It 'config set whatif for delete' { $config_yaml = @" `$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/10/config/document.json resources: @@ -54,13 +54,15 @@ Describe 'whatif tests' { properties: _exist: false "@ - $result = $config_yaml | dsc config set -w 2>&1 - $result | Should -Match 'ERROR.*?Not implemented.*?what-if' - $LASTEXITCODE | Should -Be 2 + $result = $config_yaml | dsc config set -w --format pretty-json | ConvertFrom-Json + $result.hadErrors | Should -BeFalse + $result.results.Count | Should -Be 1 + $result.results[0].result.changes[0] | Should -Be "delete 'Test/Delete' using 'dsctest'" + $LASTEXITCODE | Should -Be 0 } It 'config set whatif when there is no pre-test is not supported' { - $result = dsc config set -p .\..\examples\groups.dsc.yaml -w 2>&1 + $result = dsc config set -p $PSScriptRoot/../examples/groups.dsc.yaml -w 2>&1 $result | Should -Match 'ERROR.*?Not implemented.*?what-if' $LASTEXITCODE | Should -Be 2 } diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 7554a4a0..73aa5009 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::configure::config_doc::{ExecutionKind, Metadata}; +use crate::configure::config_doc::Metadata; use crate::configure::parameters::Input; use crate::dscerror::DscError; use crate::dscresources::dscresource::get_diff; @@ -314,75 +314,62 @@ impl Configurator { let desired = add_metadata(&dsc_resource.kind, properties)?; trace!("desired: {desired}"); + let start_datetime; + let end_datetime; + let set_result; if exist || dsc_resource.capabilities.contains(&Capability::SetHandlesExist) { debug!("Resource handles _exist or _exist is true"); - let start_datetime = chrono::Local::now(); - let set_result = dsc_resource.set(&desired, skip_test, &self.context.execution_type)?; - let end_datetime = chrono::Local::now(); - self.context.outputs.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&set_result)?); - let resource_result = config_result::ResourceSetResult { - metadata: Some( - Metadata { - microsoft: Some( - MicrosoftDscMetadata { - duration: Some(end_datetime.signed_duration_since(start_datetime).to_string()), - ..Default::default() - } - ) - } - ), - name: resource.name.clone(), - resource_type: resource.resource_type.clone(), - result: set_result, - }; - result.results.push(resource_result); - } else if dsc_resource.capabilities.contains(&Capability::Delete) && (self.context.execution_type != ExecutionKind::WhatIf) { + start_datetime = chrono::Local::now(); + set_result = dsc_resource.set(&desired, skip_test, &self.context.execution_type)?; + end_datetime = chrono::Local::now(); + } else if dsc_resource.capabilities.contains(&Capability::Delete) { debug!("Resource implements delete and _exist is false"); let before_result = dsc_resource.get(&desired)?; - let start_datetime = chrono::Local::now(); - dsc_resource.delete(&desired)?; - let end_datetime = chrono::Local::now(); - let after_result = dsc_resource.get(&desired)?; - // convert get result to set result - let set_result = match before_result { - GetResult::Resource(before_response) => { - let GetResult::Resource(after_result) = after_result else { - return Err(DscError::NotSupported("Group resources not supported for delete".to_string())) - }; - let before_value = serde_json::to_value(&before_response.actual_state)?; - let after_value = serde_json::to_value(&after_result.actual_state)?; - ResourceSetResponse { - before_state: before_response.actual_state, - after_state: after_result.actual_state, - changed_properties: Some(get_diff(&before_value, &after_value)), - } - }, - GetResult::Group(_) => { - return Err(DscError::NotSupported("Group resources not supported for delete".to_string())); - }, - }; - self.context.outputs.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&set_result)?); - let resource_result = config_result::ResourceSetResult { - metadata: Some( - Metadata { - microsoft: Some( - MicrosoftDscMetadata { - duration: Some(end_datetime.signed_duration_since(start_datetime).to_string()), - ..Default::default() - } - ) - } - ), - name: resource.name.clone(), - resource_type: resource.resource_type.clone(), - result: SetResult::Resource(set_result), + start_datetime = chrono::Local::now(); + let delete_result = dsc_resource.delete(&desired, &self.context.execution_type)?; + set_result = if let Some(whatif_result) = delete_result { whatif_result } else { + let after_result = dsc_resource.get(&desired)?; + // convert get result to set result + match before_result { + GetResult::Resource(before_response) => { + let GetResult::Resource(after_result) = after_result else { + return Err(DscError::NotSupported("Group resources not supported for delete".to_string())) + }; + let before_value = serde_json::to_value(&before_response.actual_state)?; + let after_value = serde_json::to_value(&after_result.actual_state)?; + SetResult::Resource(ResourceSetResponse { + before_state: before_response.actual_state, + after_state: after_result.actual_state, + changed_properties: Some(get_diff(&before_value, &after_value)), + }) + }, + GetResult::Group(_) => { + return Err(DscError::NotSupported("Group resources not supported for delete".to_string())); + }, + } }; - result.results.push(resource_result); - } else if self.context.execution_type == ExecutionKind::WhatIf { - return Err(DscError::NotImplemented(format!("Resource '{}' does not support `what-if flag` with `delete`", resource.resource_type))); + end_datetime = chrono::Local::now(); } else { return Err(DscError::NotImplemented(format!("Resource '{}' does not support `delete` and does not handle `_exist` as false", resource.resource_type))); } + + self.context.outputs.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&set_result)?); + let resource_result = config_result::ResourceSetResult { + metadata: Some( + Metadata { + microsoft: Some( + MicrosoftDscMetadata { + duration: Some(end_datetime.signed_duration_since(start_datetime).to_string()), + ..Default::default() + } + ) + } + ), + name: resource.name.clone(), + resource_type: resource.resource_type.clone(), + result: set_result, + }; + result.results.push(resource_result); } result.metadata = Some( diff --git a/dsc_lib/src/dscresources/command_resource.rs b/dsc_lib/src/dscresources/command_resource.rs index ad3d8cac..2923dd19 100644 --- a/dsc_lib/src/dscresources/command_resource.rs +++ b/dsc_lib/src/dscresources/command_resource.rs @@ -124,13 +124,13 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te if in_desired_state { debug!("what-if: resource is already in desired state, returning null ResourceWhatIf response"); return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: Value::String("none".to_string()) + what_if_changes: Vec::new() })); } debug!("what-if: resource is not in desired state, returning diff ResourceWhatIf response"); let diff_properties = get_diff_what_if( &desired_state, &actual_state); return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: Value::from_iter(diff_properties) + what_if_changes: vec![Value::from_iter(diff_properties)] })); } if in_desired_state { @@ -385,7 +385,7 @@ fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &str, expected: &str) /// # Errors /// /// Error is returned if the underlying command returns a non-zero exit code. -pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str) -> Result<(), DscError> { +pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, execution_type: &ExecutionKind) -> Result, DscError> { let Some(delete) = &resource.delete else { return Err(DscError::NotImplemented("delete".to_string())); }; @@ -395,6 +395,11 @@ pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str) -> Re let args = process_args(&delete.args, filter); let command_input = get_command_input(&delete.input, filter)?; + if execution_type == &ExecutionKind::WhatIf { + return Ok(Some(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ + what_if_changes: vec![Value::String(format!("delete '{}' using '{}'", &resource.resource_type, &delete.executable))] + }))); + } info!("Invoking delete '{}' using '{}'", &resource.resource_type, &delete.executable); let (exit_code, _stdout, stderr) = invoke_command(&delete.executable, args, command_input.stdin.as_deref(), Some(cwd), command_input.env)?; log_resource_traces(&stderr); @@ -402,7 +407,7 @@ pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str) -> Re return Err(DscError::Command(resource.resource_type.clone(), exit_code, stderr)); } - Ok(()) + Ok(None) } /// Invoke the validate operation against a command resource. diff --git a/dsc_lib/src/dscresources/dscresource.rs b/dsc_lib/src/dscresources/dscresource.rs index 73ed421e..11bd4d80 100644 --- a/dsc_lib/src/dscresources/dscresource.rs +++ b/dsc_lib/src/dscresources/dscresource.rs @@ -138,7 +138,7 @@ pub trait Invoke { /// # Errors /// /// This function will return an error if the underlying resource fails. - fn delete(&self, filter: &str) -> Result<(), DscError>; + fn delete(&self, filter: &str, execution_type: &ExecutionKind) -> Result, DscError>; /// Invoke the validate operation on the resource. /// @@ -244,7 +244,7 @@ impl Invoke for DscResource { } } - fn delete(&self, filter: &str) -> Result<(), DscError> { + fn delete(&self, filter: &str, execution_type: &ExecutionKind) -> Result, DscError> { match &self.implemented_as { ImplementedAs::Custom(_custom) => { Err(DscError::NotImplemented("set custom resources".to_string())) @@ -254,7 +254,7 @@ impl Invoke for DscResource { return Err(DscError::MissingManifest(self.type_name.clone())); }; let resource_manifest = import_manifest(manifest.clone())?; - command_resource::invoke_delete(&resource_manifest, &self.directory, filter) + command_resource::invoke_delete(&resource_manifest, &self.directory, filter, execution_type) }, } } diff --git a/dsc_lib/src/dscresources/invoke_result.rs b/dsc_lib/src/dscresources/invoke_result.rs index 5f5293d4..46109d84 100644 --- a/dsc_lib/src/dscresources/invoke_result.rs +++ b/dsc_lib/src/dscresources/invoke_result.rs @@ -87,8 +87,8 @@ pub struct ResourceSetResponse { #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct ResourceSetWhatIfResponse { - #[serde(rename = "whatIfChanges")] - pub what_if_changes: Value + #[serde(rename = "changes")] + pub what_if_changes: Vec } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] From 884d86124387d48acab4c6bf1e46025195576a52 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 2 May 2024 17:31:08 -0400 Subject: [PATCH 06/11] use struct for from/to changes --- dsc/tests/dsc_whatif.tests.ps1 | 7 ++++--- dsc_lib/src/dscresources/command_resource.rs | 8 ++++---- dsc_lib/src/dscresources/dscresource.rs | 17 ++++++++++------- dsc_lib/src/dscresources/invoke_result.rs | 17 ++++++++++++++++- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/dsc/tests/dsc_whatif.tests.ps1 b/dsc/tests/dsc_whatif.tests.ps1 index de1ef741..4ce3999b 100644 --- a/dsc/tests/dsc_whatif.tests.ps1 +++ b/dsc/tests/dsc_whatif.tests.ps1 @@ -39,8 +39,9 @@ Describe 'whatif tests' { $result.results.Count | Should -Be 1 $result.results[0].Name | Should -Be 'Registry' $result.results[0].type | Should -BeExactly 'Microsoft.Windows/Registry' - $result.results[0].result.changes[0]._exist.from | Should -Be 'false' - $result.results[0].result.changes[0]._exist.to | Should -Be 'true' + $result.results[0].result.changes[0].name | Should -Be '_exist' + $result.results[0].result.changes[0].from | Should -Be 'false' + $result.results[0].result.changes[0].to | Should -Be 'true' $result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' $LASTEXITCODE | Should -Be 0 } @@ -57,7 +58,7 @@ Describe 'whatif tests' { $result = $config_yaml | dsc config set -w --format pretty-json | ConvertFrom-Json $result.hadErrors | Should -BeFalse $result.results.Count | Should -Be 1 - $result.results[0].result.changes[0] | Should -Be "delete 'Test/Delete' using 'dsctest'" + $result.results[0].result.changes | Should -Be "delete 'Test/Delete' using 'dsctest'" $LASTEXITCODE | Should -Be 0 } diff --git a/dsc_lib/src/dscresources/command_resource.rs b/dsc_lib/src/dscresources/command_resource.rs index 7ce917a5..87f88b71 100644 --- a/dsc_lib/src/dscresources/command_resource.rs +++ b/dsc_lib/src/dscresources/command_resource.rs @@ -4,7 +4,7 @@ use jsonschema::JSONSchema; use serde_json::Value; use std::{collections::HashMap, env, io::{Read, Write}, process::{Command, Stdio}}; -use crate::{dscerror::DscError, dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse, ResourceSetWhatIfResponse, ResourceTestResponse}}; +use crate::{dscerror::DscError, dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse, ResourceSetWhatIfResponse, ResourceTestResponse, WhatIfResult}}; use crate::configure::{config_doc::ExecutionKind, config_result::ResourceGetResult}; use super::{dscresource::{get_diff, get_diff_what_if}, invoke_result::{ExportResult, GetResult, SetResult, TestResult, ValidateResult}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}}; use tracing::{error, warn, info, debug, trace}; @@ -119,13 +119,13 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te if in_desired_state { debug!("what-if: resource is already in desired state, returning null ResourceWhatIf response"); return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: Vec::new() + what_if_changes: WhatIfResult::Diff(Vec::new()) })); } debug!("what-if: resource is not in desired state, returning diff ResourceWhatIf response"); let diff_properties = get_diff_what_if( &desired_state, &actual_state); return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: vec![Value::from_iter(diff_properties)] + what_if_changes: WhatIfResult::Diff(diff_properties) })); } if in_desired_state { @@ -380,7 +380,7 @@ pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, execu if execution_type == &ExecutionKind::WhatIf { return Ok(Some(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: vec![Value::String(format!("delete '{}' using '{}'", &resource.resource_type, &delete.executable))] + what_if_changes: WhatIfResult::String(format!("delete '{}' using '{}'", &resource.resource_type, &delete.executable)) }))); } info!("Invoking delete '{}' using '{}'", &resource.resource_type, &delete.executable); diff --git a/dsc_lib/src/dscresources/dscresource.rs b/dsc_lib/src/dscresources/dscresource.rs index 11bd4d80..75ec09dd 100644 --- a/dsc_lib/src/dscresources/dscresource.rs +++ b/dsc_lib/src/dscresources/dscresource.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::HashMap; -use super::{command_resource, dscerror, invoke_result::{ExportResult, GetResult, ResourceTestResponse, SetResult, TestResult, ValidateResult}, resource_manifest::import_manifest}; +use super::{command_resource, dscerror, invoke_result::{ExportResult, GetResult, ResourceTestResponse, SetResult, TestResult, ValidateResult, WhatIfChanges}, resource_manifest::import_manifest}; #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] @@ -358,8 +358,8 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { } #[must_use] -pub fn get_diff_what_if(expected: &Value, actual: &Value) -> HashMap { - let mut diff_properties: HashMap = HashMap::new(); +pub fn get_diff_what_if(expected: &Value, actual: &Value) -> Vec { + let mut diff_properties: Vec = Vec::new(); if expected.is_null() { return diff_properties; } @@ -405,10 +405,13 @@ pub fn get_diff_what_if(expected: &Value, actual: &Value) -> HashMap + pub what_if_changes: WhatIfResult +} + +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +#[serde(untagged)] +pub enum WhatIfResult { + Diff(Vec), + String(String) +} + +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct WhatIfChanges { + pub name: String, + pub from: Value, + pub to: Value } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] From 5bab14f9f2bba0155180178401f9f33f1758bf31 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Fri, 10 May 2024 11:50:51 -0400 Subject: [PATCH 07/11] implement SetResult from TestResult --- dsc/src/resource_command.rs | 4 +- dsc_lib/src/configure/config_result.rs | 11 +++ dsc_lib/src/configure/mod.rs | 45 +++++++------ dsc_lib/src/dscresources/command_resource.rs | 47 +++++-------- dsc_lib/src/dscresources/dscresource.rs | 70 ++------------------ dsc_lib/src/dscresources/invoke_result.rs | 44 ++++++------ 6 files changed, 77 insertions(+), 144 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index a4a56850..300cda78 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -191,8 +191,8 @@ pub fn delete(dsc: &DscManager, resource_type: &str, mut input: String) { }; } - match resource.delete(input.as_str(), &ExecutionKind::Actual) { - Ok(_) => {} + match resource.delete(input.as_str()) { + Ok(()) => {} Err(err) => { error!("Error: {err}"); exit(EXIT_DSC_ERROR); diff --git a/dsc_lib/src/configure/config_result.rs b/dsc_lib/src/configure/config_result.rs index 7a622666..cced4b0d 100644 --- a/dsc_lib/src/configure/config_result.rs +++ b/dsc_lib/src/configure/config_result.rs @@ -99,6 +99,17 @@ pub struct ResourceSetResult { pub result: SetResult, } +impl From for ResourceSetResult { + fn from(test_result: ResourceTestResult) -> Self { + Self { + metadata: None, + name: test_result.name, + resource_type: test_result.resource_type, + result: test_result.result.into(), + } + } +} + #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct GroupResourceSetResult { diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 3b9715e4..93556d20 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::configure::config_doc::Metadata; +use crate::configure::config_doc::{ExecutionKind, Metadata}; use crate::configure::parameters::Input; use crate::dscerror::DscError; use crate::dscresources::dscresource::get_diff; @@ -323,30 +323,31 @@ impl Configurator { set_result = dsc_resource.set(&desired, skip_test, &self.context.execution_type)?; end_datetime = chrono::Local::now(); } else if dsc_resource.capabilities.contains(&Capability::Delete) { + if self.context.execution_type == ExecutionKind::WhatIf { + return Err(DscError::NotSupported("What-if execution not supported for delete".to_string())); + } debug!("Resource implements delete and _exist is false"); let before_result = dsc_resource.get(&desired)?; start_datetime = chrono::Local::now(); - let delete_result = dsc_resource.delete(&desired, &self.context.execution_type)?; - set_result = if let Some(whatif_result) = delete_result { whatif_result } else { - let after_result = dsc_resource.get(&desired)?; - // convert get result to set result - match before_result { - GetResult::Resource(before_response) => { - let GetResult::Resource(after_result) = after_result else { - return Err(DscError::NotSupported("Group resources not supported for delete".to_string())) - }; - let before_value = serde_json::to_value(&before_response.actual_state)?; - let after_value = serde_json::to_value(&after_result.actual_state)?; - SetResult::Resource(ResourceSetResponse { - before_state: before_response.actual_state, - after_state: after_result.actual_state, - changed_properties: Some(get_diff(&before_value, &after_value)), - }) - }, - GetResult::Group(_) => { - return Err(DscError::NotSupported("Group resources not supported for delete".to_string())); - }, - } + dsc_resource.delete(&desired)?; + let after_result = dsc_resource.get(&desired)?; + // convert get result to set result + set_result = match before_result { + GetResult::Resource(before_response) => { + let GetResult::Resource(after_result) = after_result else { + return Err(DscError::NotSupported("Group resources not supported for delete".to_string())) + }; + let before_value = serde_json::to_value(&before_response.actual_state)?; + let after_value = serde_json::to_value(&after_result.actual_state)?; + SetResult::Resource(ResourceSetResponse { + before_state: before_response.actual_state, + after_state: after_result.actual_state, + changed_properties: Some(get_diff(&before_value, &after_value)), + }) + }, + GetResult::Group(_) => { + return Err(DscError::NotSupported("Group resources not supported for delete".to_string())); + }, }; end_datetime = chrono::Local::now(); } else { diff --git a/dsc_lib/src/dscresources/command_resource.rs b/dsc_lib/src/dscresources/command_resource.rs index 87f88b71..1820be91 100644 --- a/dsc_lib/src/dscresources/command_resource.rs +++ b/dsc_lib/src/dscresources/command_resource.rs @@ -4,9 +4,9 @@ use jsonschema::JSONSchema; use serde_json::Value; use std::{collections::HashMap, env, io::{Read, Write}, process::{Command, Stdio}}; -use crate::{dscerror::DscError, dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse, ResourceSetWhatIfResponse, ResourceTestResponse, WhatIfResult}}; +use crate::{dscerror::DscError, dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse, ResourceTestResponse}}; use crate::configure::{config_doc::ExecutionKind, config_result::ResourceGetResult}; -use super::{dscresource::{get_diff, get_diff_what_if}, invoke_result::{ExportResult, GetResult, SetResult, TestResult, ValidateResult}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}}; +use super::{dscresource::get_diff, invoke_result::{ExportResult, GetResult, SetResult, TestResult, ValidateResult}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}}; use tracing::{error, warn, info, debug, trace}; pub const EXIT_PROCESS_TERMINATED: i32 = 0x102; @@ -99,35 +99,23 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te // if resource doesn't implement a pre-test, we execute test first to see if a set is needed if !skip_test && set.pre_test != Some(true) { info!("No pretest, invoking test {}", &resource.resource_type); - let (in_desired_state, actual_state, desired_state) = match invoke_test(resource, cwd, desired)? { + let test_result = invoke_test(resource, cwd, desired)?; + if execution_type == &ExecutionKind::WhatIf { + return Ok(test_result.into()); + } + let (in_desired_state, actual_state) = match test_result { TestResult::Group(group_response) => { let mut result_array: Vec = Vec::new(); for result in group_response.results { result_array.push(serde_json::to_value(result)?); } - (group_response.in_desired_state, Value::from(result_array), Value::String("not implemented".to_string())) + (group_response.in_desired_state, Value::from(result_array)) }, TestResult::Resource(response) => { - (response.in_desired_state, response.actual_state, response.desired_state) + (response.in_desired_state, response.actual_state) } }; - if execution_type == &ExecutionKind::WhatIf { - if desired_state == Value::String("not implemented".to_string()) { - return Err(DscError::NotImplemented(format!("Group '{}' does not currently support `what-if flag`", resource.resource_type))); - } - if in_desired_state { - debug!("what-if: resource is already in desired state, returning null ResourceWhatIf response"); - return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: WhatIfResult::Diff(Vec::new()) - })); - } - debug!("what-if: resource is not in desired state, returning diff ResourceWhatIf response"); - let diff_properties = get_diff_what_if( &desired_state, &actual_state); - return Ok(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: WhatIfResult::Diff(diff_properties) - })); - } if in_desired_state { return Ok(SetResult::Resource(ResourceSetResponse{ before_state: serde_json::from_str(desired)?, @@ -135,9 +123,11 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te changed_properties: None, })); } - } else if execution_type == &ExecutionKind::WhatIf { - // until resources implement what-if, if we are in what-if mode and there is no pre-test, we can't determine what-if changes - return Err(DscError::NotImplemented(format!("Resource '{}' does not currently support `what-if flag` because there is no pre-test", resource.resource_type))); + } + + if ExecutionKind::WhatIf == *execution_type { + // until resources implement their own what-if, return an error here + return Err(DscError::NotImplemented("what-if not yet supported for resources that implement pre-test".to_string())); } let args = process_args(&resource.get.args, desired); @@ -368,7 +358,7 @@ fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &str, expected: &str) /// # Errors /// /// Error is returned if the underlying command returns a non-zero exit code. -pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, execution_type: &ExecutionKind) -> Result, DscError> { +pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str) -> Result<(), DscError> { let Some(delete) = &resource.delete else { return Err(DscError::NotImplemented("delete".to_string())); }; @@ -378,15 +368,10 @@ pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, execu let args = process_args(&delete.args, filter); let command_input = get_command_input(&delete.input, filter)?; - if execution_type == &ExecutionKind::WhatIf { - return Ok(Some(SetResult::ResourceWhatIf(ResourceSetWhatIfResponse{ - what_if_changes: WhatIfResult::String(format!("delete '{}' using '{}'", &resource.resource_type, &delete.executable)) - }))); - } info!("Invoking delete '{}' using '{}'", &resource.resource_type, &delete.executable); let (_exit_code, _stdout, _stderr) = invoke_command(&delete.executable, args, command_input.stdin.as_deref(), Some(cwd), command_input.env)?; - Ok(None) + Ok(()) } /// Invoke the validate operation against a command resource. diff --git a/dsc_lib/src/dscresources/dscresource.rs b/dsc_lib/src/dscresources/dscresource.rs index 75ec09dd..fad9a09b 100644 --- a/dsc_lib/src/dscresources/dscresource.rs +++ b/dsc_lib/src/dscresources/dscresource.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::HashMap; -use super::{command_resource, dscerror, invoke_result::{ExportResult, GetResult, ResourceTestResponse, SetResult, TestResult, ValidateResult, WhatIfChanges}, resource_manifest::import_manifest}; +use super::{command_resource, dscerror, invoke_result::{ExportResult, GetResult, ResourceTestResponse, SetResult, TestResult, ValidateResult}, resource_manifest::import_manifest}; #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] @@ -138,7 +138,7 @@ pub trait Invoke { /// # Errors /// /// This function will return an error if the underlying resource fails. - fn delete(&self, filter: &str, execution_type: &ExecutionKind) -> Result, DscError>; + fn delete(&self, filter: &str) -> Result<(), DscError>; /// Invoke the validate operation on the resource. /// @@ -244,7 +244,7 @@ impl Invoke for DscResource { } } - fn delete(&self, filter: &str, execution_type: &ExecutionKind) -> Result, DscError> { + fn delete(&self, filter: &str) -> Result<(), DscError> { match &self.implemented_as { ImplementedAs::Custom(_custom) => { Err(DscError::NotImplemented("set custom resources".to_string())) @@ -254,7 +254,7 @@ impl Invoke for DscResource { return Err(DscError::MissingManifest(self.type_name.clone())); }; let resource_manifest = import_manifest(manifest.clone())?; - command_resource::invoke_delete(&resource_manifest, &self.directory, filter, execution_type) + command_resource::invoke_delete(&resource_manifest, &self.directory, filter) }, } } @@ -356,65 +356,3 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec { diff_properties } - -#[must_use] -pub fn get_diff_what_if(expected: &Value, actual: &Value) -> Vec { - let mut diff_properties: Vec = Vec::new(); - if expected.is_null() { - return diff_properties; - } - - let mut expected = expected.clone(); - let mut actual = actual.clone(); - - if let Some(map) = expected.as_object_mut() { - // handle well-known optional properties with default values by adding them - for (key, value) in get_well_known_properties() { - if !map.contains_key(&key) { - map.insert(key.clone(), value.clone()); - } - - if actual.is_object() && actual[&key].is_null() { - actual[key.clone()] = value.clone(); - } - } - - for (key, value) in &*map { - let mut is_diff = false; - if value.is_object() { - let sub_diff = get_diff_what_if(value, &actual[key]); - if !sub_diff.is_empty() { - is_diff = true; - } - } - else { - match actual.as_object() { - Some(actual_object) => { - if actual_object.contains_key(key) { - if value != &actual[key] { - is_diff = true; - } - } - else { - is_diff = true; - } - }, - None => { - is_diff = true; - }, - } - } - if is_diff { - diff_properties.push( - WhatIfChanges { - name: key.to_string(), - from: actual[key].clone(), - to: value.clone(), - } - ); - } - } - } - - diff_properties -} diff --git a/dsc_lib/src/dscresources/invoke_result.rs b/dsc_lib/src/dscresources/invoke_result.rs index 319be7f1..e5c567ce 100644 --- a/dsc_lib/src/dscresources/invoke_result.rs +++ b/dsc_lib/src/dscresources/invoke_result.rs @@ -66,10 +66,30 @@ impl Default for GroupResourceSetResponse { #[serde(untagged)] pub enum SetResult { Resource(ResourceSetResponse), - ResourceWhatIf(ResourceSetWhatIfResponse), Group(GroupResourceSetResponse), } +impl From for SetResult { + fn from(value: TestResult) -> Self { + match value { + TestResult::Group(group) => { + let mut results = Vec::::new(); + for result in group.results { + results.push(result.into()); + } + SetResult::Group(GroupResourceSetResponse { results }) + }, + TestResult::Resource(resource) => { + SetResult::Resource(ResourceSetResponse { + before_state: resource.actual_state, + after_state: resource.desired_state, + changed_properties: if resource.diff_properties.is_empty() { None } else { Some(resource.diff_properties) }, + }) + } + } + } +} + #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct ResourceSetResponse { @@ -84,28 +104,6 @@ pub struct ResourceSetResponse { pub changed_properties: Option>, } -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] -#[serde(deny_unknown_fields)] -pub struct ResourceSetWhatIfResponse { - #[serde(rename = "changes")] - pub what_if_changes: WhatIfResult -} - -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] -#[serde(untagged)] -pub enum WhatIfResult { - Diff(Vec), - String(String) -} - -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] -#[serde(deny_unknown_fields)] -pub struct WhatIfChanges { - pub name: String, - pub from: Value, - pub to: Value -} - #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct GroupResourceTestResponse { From 0687281f82ee8e1238878c61722101ed4616305f Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Fri, 10 May 2024 11:51:08 -0400 Subject: [PATCH 08/11] update tests --- dsc/tests/dsc_whatif.tests.ps1 | 46 +++++++++++++++++----------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/dsc/tests/dsc_whatif.tests.ps1 b/dsc/tests/dsc_whatif.tests.ps1 index 4ce3999b..ff0f979c 100644 --- a/dsc/tests/dsc_whatif.tests.ps1 +++ b/dsc/tests/dsc_whatif.tests.ps1 @@ -14,13 +14,14 @@ Describe 'whatif tests' { properties: output: hello "@ - $result = $config_yaml | dsc config set -w --format pretty-json | ConvertFrom-Json - $result.hadErrors | Should -BeFalse - $result.results.Count | Should -Be 1 - $result.results[0].Name | Should -Be 'Hello' - $result.results[0].type | Should -BeExactly 'Test/Echo' - $result.results[0].result.changes | Should -Be $null - $result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' + $what_if_result = $config_yaml | dsc config set -w | ConvertFrom-Json + $set_result = $config_yaml | dsc config set | ConvertFrom-Json + $what_if_result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' + $what_if_result.results.result.beforeState.output | Should -Be $set_result.results.result.beforeState.output + $what_if_result.results.result.afterState.output | Should -Be $set_result.results.result.afterState.output + $what_if_result.results.result.changedProperties | Should -Be $set_result.results.result.changedProperties + $what_if_result.hadErrors | Should -BeFalse + $what_if_result.results.Count | Should -Be 1 $LASTEXITCODE | Should -Be 0 } @@ -34,19 +35,20 @@ Describe 'whatif tests' { properties: keyPath: 'HKCU\1\2' "@ - $result = $config_yaml | dsc config set -w --format pretty-json | ConvertFrom-Json - $result.hadErrors | Should -BeFalse - $result.results.Count | Should -Be 1 - $result.results[0].Name | Should -Be 'Registry' - $result.results[0].type | Should -BeExactly 'Microsoft.Windows/Registry' - $result.results[0].result.changes[0].name | Should -Be '_exist' - $result.results[0].result.changes[0].from | Should -Be 'false' - $result.results[0].result.changes[0].to | Should -Be 'true' - $result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' + $what_if_result = $config_yaml | dsc config set -w | ConvertFrom-Json + $set_result = $config_yaml | dsc config set | ConvertFrom-Json + $what_if_result.metadata.'Microsoft.DSC'.executionType | Should -BeExactly 'WhatIf' + $what_if_result.results.result.beforeState._exist | Should -Be $set_result.results.result.beforeState._exist + $what_if_result.results.result.beforeState.keyPath | Should -Be $set_result.results.result.beforeState.keyPath + $what_if_result.results.result.afterState.KeyPath | Should -Be $set_result.results.result.afterState.keyPath + $what_if_result.results.result.changedProperties | Should -Be $set_result.results.result.changedProperties + $what_if_result.hadErrors | Should -BeFalse + $what_if_result.results.Count | Should -Be 1 $LASTEXITCODE | Should -Be 0 + } - It 'config set whatif for delete' { + It 'config set whatif for delete is not supported' { $config_yaml = @" `$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/10/config/document.json resources: @@ -55,14 +57,12 @@ Describe 'whatif tests' { properties: _exist: false "@ - $result = $config_yaml | dsc config set -w --format pretty-json | ConvertFrom-Json - $result.hadErrors | Should -BeFalse - $result.results.Count | Should -Be 1 - $result.results[0].result.changes | Should -Be "delete 'Test/Delete' using 'dsctest'" - $LASTEXITCODE | Should -Be 0 + $result = $config_yaml | dsc config set -w 2>&1 + $result | Should -Match 'ERROR.*?Not supported.*?what-if' + $LASTEXITCODE | Should -Be 2 } - It 'config set whatif when there is no pre-test is not supported' { + It 'config set whatif for group resource' { $result = dsc config set -p $PSScriptRoot/../examples/groups.dsc.yaml -w 2>&1 $result | Should -Match 'ERROR.*?Not implemented.*?what-if' $LASTEXITCODE | Should -Be 2 From dec10d8e1207d73ef58c12f3c0b5e2ae96384a57 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 16 May 2024 10:38:33 -0400 Subject: [PATCH 09/11] update comments with TODOs --- dsc_lib/src/configure/mod.rs | 1 + dsc_lib/src/dscresources/command_resource.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 06da6f10..53275c99 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -319,6 +319,7 @@ impl Configurator { end_datetime = chrono::Local::now(); } else if dsc_resource.capabilities.contains(&Capability::Delete) { if self.context.execution_type == ExecutionKind::WhatIf { + // TODO: add delete what-if support return Err(DscError::NotSupported("What-if execution not supported for delete".to_string())); } debug!("Resource implements delete and _exist is false"); diff --git a/dsc_lib/src/dscresources/command_resource.rs b/dsc_lib/src/dscresources/command_resource.rs index c07ca393..ce319613 100644 --- a/dsc_lib/src/dscresources/command_resource.rs +++ b/dsc_lib/src/dscresources/command_resource.rs @@ -131,7 +131,7 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te } if ExecutionKind::WhatIf == *execution_type { - // until resources implement their own what-if, return an error here + // TODO: continue execution when resources can implement what-if; only return an error here temporarily return Err(DscError::NotImplemented("what-if not yet supported for resources that implement pre-test".to_string())); } From 6a3489ca53e38c93c67158ae73770f2a427442cf Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 16 May 2024 10:46:50 -0400 Subject: [PATCH 10/11] fix clippy --- dsc_lib/src/configure/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 53275c99..6e99670e 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -59,7 +59,7 @@ pub fn add_resource_export_results_to_configuration(resource: &DscResource, adap for (i, instance) in export_result.actual_state.iter().enumerate() { let mut r = config_doc::Resource::new(); - r.resource_type = resource.type_name.clone(); + r.resource_type.clone_from(&resource.type_name); r.name = format!("{}-{i}", r.resource_type); let props: Map = serde_json::from_value(instance.clone())?; r.properties = escape_property_values(&props)?; From 144ac00c3dbe331d0eb2611c94e1e37d06c90170 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 16 May 2024 11:05:37 -0400 Subject: [PATCH 11/11] fix clippy 2 --- osinfo/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osinfo/src/config.rs b/osinfo/src/config.rs index 1484ddc5..2e879294 100644 --- a/osinfo/src/config.rs +++ b/osinfo/src/config.rs @@ -8,7 +8,7 @@ use std::string::ToString; #[derive(Debug, Clone, PartialEq, Serialize)] #[serde(deny_unknown_fields)] pub struct OsInfo { - /// Returns the unique ID for the OSInfo instance data type. + /// Returns the unique ID for the `OSInfo` instance data type. #[serde(rename = "$id")] pub id: String, family: Family,