implement SetResult from TestResult

Author: tgauth

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);

dsc_lib/src/configure/config_result.rs

@@ -99,6 +99,17 @@ pub struct ResourceSetResult {
     pub result: SetResult,
 }
 
+impl From<ResourceTestResult> 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 {

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 {

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,45 +99,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, 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<Value> = 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)?,
                 after_state: actual_state,
                 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<Option<SetResult>, 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.

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<Option<SetResult>, 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<Option<SetResult>, 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<String> {
 
     diff_properties
 }
-
-#[must_use]
-pub fn get_diff_what_if(expected: &Value, actual: &Value) -> Vec<WhatIfChanges> {
-    let mut diff_properties: Vec<WhatIfChanges> = 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
-}

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<TestResult> for SetResult {
+    fn from(value: TestResult) -> Self {
+        match value {
+            TestResult::Group(group) => {
+                let mut results = Vec::<ResourceSetResult>::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<Vec<String>>,
 }
 
-#[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<WhatIfChanges>),
-    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 {