Merge pull request #206 from SteveL-MSFT/exists

Change `_ensure` to `_exist`
PowerShell · Sep 29, 2023 · cc22e11 · cc22e11
2 parents ac8bc2f + 93c3a4d
commit cc22e11
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 73 deletions.
diff --git a/dsc/tests/dsc_set.tests.ps1 b/dsc/tests/dsc_set.tests.ps1
@@ -7,7 +7,7 @@ Describe 'config set tests' {
             $json = @'
             {
                 "keyPath": "HKCU\\1\\2\\3",
-                "_ensure": "Absent"
+                "_exist": false
             }
 '@
             $json | registry config set
@@ -19,7 +19,7 @@ Describe 'config set tests' {
             $json = @'
             {
                 "keyPath": "HKCU\\1",
-                "_ensure": "Absent"
+                "_exist": false
             }
 '@
             $json | registry config set
@@ -42,7 +42,7 @@ Describe 'config set tests' {
         $result.afterState.keyPath | Should -Be 'HKCU\1\2\3'
         $result.afterState.valueName | Should -Be 'Hello'
         $result.afterState.valueData.String | Should -Be 'World'
-        $result.changedProperties | Should -Be @('keyPath', 'valueName', 'valueData')
+        $result.changedProperties | Should -Be @('valueName', 'valueData', '_exist')
         ($result.psobject.properties | Measure-Object).Count | Should -Be 3
 
         $out = $json | dsc resource get -r *registry
@@ -56,14 +56,14 @@ Describe 'config set tests' {
         $json = @'
         {
             "keyPath": "HKCU\\1",
-            "_ensure": "Absent"
+            "_exist": false
         }
 '@
         $out = $json | dsc resource set -r Microsoft.Windows/registry
         $LASTEXITCODE | Should -Be 0
         $result = $out | ConvertFrom-Json
-        $result.afterState.keyPath | Should -BeNullOrEmpty
-        $result.changedProperties | Should -Be @('keyPath')
+        $result.afterState.keyPath | Should -BeExactly 'HKCU\1'
+        $result.changedProperties | Should -Be @('_exist')
         ($result.psobject.properties | Measure-Object).Count | Should -Be 3
     }
 

diff --git a/dsc/tests/dsc_test.tests.ps1 b/dsc/tests/dsc_test.tests.ps1
@@ -49,8 +49,9 @@ Describe 'config test tests' {
         $LASTEXITCODE | Should -Be 0
         $out = $out | ConvertFrom-Json
         $out.inDesiredState | Should -BeFalse
-        $out.differingProperties.Count | Should -Be 2
+        $out.differingProperties.Count | Should -Be 3
         $out.differingProperties[0] | Should -BeExactly 'valueName'
         $out.differingProperties[1] | Should -BeExactly 'valueData'
+        $out.differingProperties[2] | Should -BeExactly '_exist'
     }
 }
diff --git a/dsc_lib/src/dscresources/dscresource.rs b/dsc_lib/src/dscresources/dscresource.rs
@@ -5,6 +5,7 @@ use dscerror::DscError;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
+use std::collections::HashMap;
 use super::{command_resource, dscerror, resource_manifest::import_manifest, invoke_result::{GetResult, SetResult, TestResult, ValidateResult, ExportResult}};
 
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
@@ -237,20 +238,36 @@ impl Invoke for DscResource {
     }
 }
 
+#[must_use]
+pub fn get_well_known_properties() -> HashMap<String, Value> {
+    HashMap::<String, Value>::from([
+        ("_exist".to_string(), Value::Bool(true)),
+    ])
+}
+
 #[must_use]
 pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
     let mut diff_properties: Vec<String> = Vec::new();
     if expected.is_null() {
         return diff_properties;
     }
 
-    if let Some(map) = expected.as_object() {
-        for (key, value) in map {
-            // skip meta properties
-            if key.starts_with('_') || key.starts_with('$') {
-                continue;
+    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 {
             if value.is_object() {
                 let sub_diff = get_diff(value, &actual[key]);
                 if !sub_diff.is_empty() {

diff --git a/registry/src/config.rs b/registry/src/config.rs
@@ -4,14 +4,6 @@
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)]
-pub enum EnsureKind {
-    /// The registry key and value should be present.
-    Present,
-    /// The registry key and value should be absent.
-    Absent,
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)]
 pub enum RegistryValueData {
     String(String),
@@ -40,9 +32,9 @@ pub struct Registry {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub value_data: Option<RegistryValueData>,
     /// Flag indicating whether the registry value should be present or absent.
-    #[serde(rename = "_ensure")]
+    #[serde(rename = "_exist")]
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub ensure: Option<EnsureKind>,
+    pub exist: Option<bool>,
     /// Flag indicating whether the registry value should be overwritten if it already exists.
     #[serde(rename = "_clobber")]
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -74,7 +66,7 @@ impl Default for Registry {
             key_path: String::new(),
             value_name: None,
             value_data: None,
-            ensure: None,
+            exist: None,
             clobber: None,
             in_desired_state: None,
         }

diff --git a/registry/src/main.rs b/registry/src/main.rs
@@ -98,8 +98,8 @@ fn main() {
                 }
             }
 
-            if config.ensure.is_none() {
-                config.ensure = Some(config::EnsureKind::Present);
+            if config.exist.is_none() {
+                config.exist = Some(true);
             }
 
             match subcommand {

diff --git a/registry/src/regconfighelper.rs b/registry/src/regconfighelper.rs
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use crate::config::{EnsureKind, Registry, RegistryValueData};
+use crate::config::{Registry, RegistryValueData};
 use ntreg::{registry_key::RegistryKey, registry_value::RegistryValueData as NtRegistryValueData, registry_value::RegistryValue};
 use ntstatuserror::{NtStatusError, NtStatusErrorKind};
 use std::fmt::{Display, Formatter};
@@ -29,15 +29,14 @@ impl Display for RegistryError {
 }
 
 pub fn config_get(config: &Registry) -> Result<String, RegistryError> {
-    let mut reg_result = Registry::default();
+    let mut reg_result = Registry { key_path: config.key_path.clone(), ..Default::default() };
 
     let reg_key = match RegistryKey::new(config.key_path.as_str()) {
         Ok(reg_key) => reg_key,
         Err(NtStatusError { status: NtStatusErrorKind::ObjectNameNotFound, ..}) => {
+            reg_result.exist = Some(false);
             match serde_json::to_string(&reg_result) {
                 Ok(reg_json) => {
-                    // TODO: current design is to return an empty JSON if the key is not found instead of an error
-                    // this makes it consistent with result for _ensure = absent so that a result is returned
                     return Ok(reg_json);
                 },
                 Err(err) => {
@@ -50,8 +49,6 @@ pub fn config_get(config: &Registry) -> Result<String, RegistryError> {
         }
     };
 
-    reg_result.key_path = config.key_path.clone();
-
     if config.value_name.is_some() {
         let reg_value = match reg_key.get_value(config.value_name.as_ref().unwrap().as_str()) {
             Ok(reg_value) => reg_value,
@@ -75,25 +72,24 @@ pub fn config_get(config: &Registry) -> Result<String, RegistryError> {
 }
 
 pub fn config_set(config: &Registry) -> Result<String, RegistryError> {
-    let mut reg_result: Registry = Registry::default();
+    let mut reg_result = Registry { key_path: config.key_path.clone(), ..Default::default() };
     let reg_key: RegistryKey;
     match &config.value_name {
         None => {
-            match config.ensure.as_ref().unwrap() {
-                EnsureKind::Present => {
+            match config.exist {
+                Some(true) | None => {
                     open_or_create_key(&config.key_path)?;
-                    reg_result.key_path = config.key_path.clone();
                 },
-                EnsureKind::Absent => {
+                Some(false) => {
+                    reg_result.exist = Some(false);
                     remove_key(&config.key_path)?;
                 },
             }
         },
         Some(value_name) => {
-            reg_result.key_path = config.key_path.clone();
             reg_result.value_name = Some(value_name.clone());
-            match &config.ensure {
-                Some(EnsureKind::Present) | None => {
+            match &config.exist {
+                Some(true) | None => {
                     reg_key = open_or_create_key(&config.key_path)?;
                     match config.value_data.as_ref() {
                         Some(value_data) => {
@@ -114,8 +110,9 @@ pub fn config_set(config: &Registry) -> Result<String, RegistryError> {
                         }
                     }
                 },
-                Some(EnsureKind::Absent) => {
+                Some(false) => {
                     reg_key = open_or_create_key(&config.key_path)?;
+                    reg_result.exist = Some(false);
                     match reg_key.delete_value(value_name) {
                         Ok(_) | Err(NtStatusError { status: NtStatusErrorKind::ObjectNameNotFound, ..}) => {},
                         Err(err) => {
@@ -232,7 +229,7 @@ pub fn config_test(config: &Registry) -> Result<String, RegistryError> {
 }
 
 fn test_value(config: &Registry) -> Result<String, RegistryError> {
-    let mut reg_result: Registry = Registry::default();
+    let mut reg_result: Registry = Registry { key_path: config.key_path.clone(), ..Default::default()};
     let mut in_desired_state = true;
 
     let reg_key = match RegistryKey::new(config.key_path.as_str()) {
@@ -249,8 +246,6 @@ fn test_value(config: &Registry) -> Result<String, RegistryError> {
         }
     };
 
-    reg_result.key_path = config.key_path.clone();
-
     let value_name = config.value_name.as_ref().unwrap();
     let mut value_exists = false;
     let reg_value: RegistryValue = match reg_key.get_value(value_name) {
@@ -272,22 +267,23 @@ fn test_value(config: &Registry) -> Result<String, RegistryError> {
         }
     };
 
-    match &config.ensure.as_ref().unwrap() {
-        EnsureKind::Present => {
+    match &config.exist {
+        Some(true) | None => {
             if value_exists {
                 in_desired_state = reg_values_are_eq(config, &reg_value)?;
             }
             else {
                 in_desired_state = false;
             }
         },
-        EnsureKind::Absent => {
+        Some(false) => {
             if value_exists {
                 in_desired_state = false;
             }
         }
     }
 
+    reg_result.exist = Some(value_exists);
     reg_result.in_desired_state = Some(in_desired_state);
     Ok(reg_result.to_json())
 }
@@ -316,7 +312,7 @@ fn reg_values_are_eq(config: &Registry, reg_value: &RegistryValue) -> Result<boo
 }
 
 fn test_key(config: &Registry) -> Result<String, RegistryError> {
-    let mut reg_result: Registry = Registry::default();
+    let mut reg_result: Registry = Registry { key_path: config.key_path.clone(), ..Default::default()};
 
     let key_exists = match RegistryKey::new(config.key_path.as_str()) {
         Ok( _ ) => {
@@ -331,21 +327,20 @@ fn test_key(config: &Registry) -> Result<String, RegistryError> {
     };
 
     let mut in_desired_state = true;
-    match &config.ensure.as_ref().unwrap() {
-        EnsureKind::Present => {
+    match &config.exist {
+        Some(true) | None => {
             if !key_exists {
-                reg_result.key_path = String::new();
                 in_desired_state = false;
             }
         },
-        EnsureKind::Absent => {
+        Some(false) => {
             if key_exists {
-                reg_result.key_path = config.key_path.clone();
                 in_desired_state = false;
             }
         }
     }
 
+    reg_result.exist = Some(key_exists);
     reg_result.in_desired_state = Some(in_desired_state);
     Ok(reg_result.to_json())
 }
@@ -381,13 +376,13 @@ fn test_registry_value_present() {
     {
         "keyPath": "HKLM\\Software\\Microsoft\\Windows\\CurrentVersion",
         "valueName": "ProgramFilesPath",
-        "_ensure": "Present"
+        "_exist": true
     }
     "#;
 
     let config: Registry = serde_json::from_str(input_json).unwrap();
     let json = config_test(&config).unwrap();
-    assert_eq!(json, r#"{"$id":"https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json","keyPath":"HKLM\\Software\\Microsoft\\Windows\\CurrentVersion","valueName":"ProgramFilesPath","valueData":{"ExpandString":"%ProgramFiles%"},"_inDesiredState":true}"#);
+    assert_eq!(json, r#"{"$id":"https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json","keyPath":"HKLM\\Software\\Microsoft\\Windows\\CurrentVersion","valueName":"ProgramFilesPath","valueData":{"ExpandString":"%ProgramFiles%"},"_exist":true,"_inDesiredState":true}"#);
 }
 
 #[test]
@@ -396,11 +391,11 @@ fn test_registry_value_absent() {
     {
         "keyPath": "HKLM\\Software\\Microsoft\\Windows\\CurrentVersion",
         "valueName": "DoesNotExist",
-        "_ensure": "Absent"
+        "_exist": false
     }
     "#;
 
     let config: Registry = serde_json::from_str(input_json).unwrap();
     let json = config_test(&config).unwrap();
-    assert_eq!(json, r#"{"$id":"https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json","keyPath":"HKLM\\Software\\Microsoft\\Windows\\CurrentVersion","_inDesiredState":true}"#);
+    assert_eq!(json, r#"{"$id":"https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json","keyPath":"HKLM\\Software\\Microsoft\\Windows\\CurrentVersion","_exist":false,"_inDesiredState":true}"#);
 }
diff --git a/registry/tests/registry.config.set.tests.ps1 b/registry/tests/registry.config.set.tests.ps1
@@ -33,13 +33,14 @@ Describe 'registry config set tests' {
         $json = @'
         {
             "keyPath": "HKCU\\1",
-            "_ensure": "Absent"
+            "_exist": false
         }
 '@
         $out = $json | registry config set
         $LASTEXITCODE | Should -Be 0
         $result = $out | ConvertFrom-Json
-        $result.keyPath | Should -BeNullOrEmpty
-        ($result.psobject.properties | Measure-Object).Count | Should -Be 2
+        $result.keyPath | Should -BeExactly 'HKCU\1'
+        $result._exist | Should -Be $false
+        ($result.psobject.properties | Measure-Object).Count | Should -Be 3
     }
 }