Fix registry resource what-if for non-existing key (#1692)

* Fix registry resource what-if for non-existing key

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Update how exit code is returned to work with code cov tools

* Allow coverage threshold override label

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Steve Lee (POWERSHELL HE/HIM) (from Dev Box) <slee@ntdev.microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: 4 people

.github/workflows/rust.yml

@@ -487,6 +487,7 @@ jobs:
           steps.coverage.outputs.has_rust_changes == 'true'
           && steps.coverage.outputs.coverage_failed != 'true'
           && steps.coverage.outputs.percentage < 70
+          && !contains(github.event.pull_request.labels.*.name, 'Ok-CodeCoverage')
         run: |
           Write-Error "Code coverage is ${{ steps.coverage.outputs.percentage }}%, which is below the 70% minimum threshold."
           exit 1

lib/dsc-lib-registry/locales/en-us.toml

@@ -18,6 +18,7 @@ unsupportedValueDataType = "Unsupported registry value data type"
 whatIfCreateKey = "Key '%{subkey}' not found, would create it"
 whatIfDeleteValue = "Would delete value '%{value_name}'"
 whatIfDeleteSubkey = "Would delete subkey '%{subkey_name}'"
+whatIfDeleteNonexistingKey = "Key '%{subkey}' not found, would do nothing"
 
 [offreg]
 loadFailed = "Failed to load offreg.dll"

lib/dsc-lib-registry/src/lib.rs

@@ -316,13 +316,25 @@ impl RegistryHelper {
             return self.remove_offline();
         }
 
+        // Accumulate what-if metadata like set()
+        let mut what_if_metadata: Vec<String> = Vec::new();
+
         // For deleting a value, we need SetValue permission (KEY_SET_VALUE).
         // Try to open with the minimal required permission.
         // If that fails due to permission, try with AllAccess as a fallback.
         let (reg_key, _subkey) = match self.open(Security::SetValue) {
             Ok(reg_key) => reg_key,
             // handle NotFound error
             Err(RegistryError::RegistryKeyNotFound(_)) => {
+                if self.what_if {
+                    what_if_metadata.push(t!("registry_helper.whatIfDeleteNonexistingKey", subkey = &self.config.key_path).to_string());
+                    return Ok(Some(Registry {
+                        key_path: self.config.key_path.clone(),
+                        value_name: self.config.value_name.clone(),
+                        metadata: Some(Metadata { what_if: Some(what_if_metadata) }),
+                        ..Default::default()
+                    }));
+                }
                 return Ok(None);
             },
             Err(RegistryError::RegistryKey(key::Error::PermissionDenied(_, _))) => {
@@ -334,9 +346,6 @@ impl RegistryHelper {
             Err(e) => return self.handle_error_or_what_if(e),
         };
 
-        // Accumulate what-if metadata like set()
-        let mut what_if_metadata: Vec<String> = Vec::new();
-
         if let Some(value_name) = &self.config.value_name {
             if self.what_if {
                 what_if_metadata.push(t!("registry_helper.whatIfDeleteValue", value_name = value_name).to_string());
@@ -385,6 +394,7 @@ impl RegistryHelper {
                 Err(e) => return self.handle_error_or_what_if(RegistryError::RegistryKey(e)),
             }
         }
+
         Ok(None)
     }
 

lib/dsc-lib/src/dscresources/invoke_result.rs

@@ -184,7 +184,6 @@ pub struct ResolveResult {
 }
 
 #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)]
-#[serde(deny_unknown_fields)]
 #[dsc_repo_schema(base_name = "delete", folder_path = "outputs/resource")]
 pub struct DeleteResult {
     /// The return from the resource by the Delete method with what-if simulation.

resources/registry/src/main.rs

@@ -12,7 +12,7 @@ use clap::Parser;
 use dsc_lib_registry::{config::Registry, RegistryHelper};
 use rust_i18n::t;
 use schemars::schema_for;
-use std::process::exit;
+use std::process::ExitCode;
 use tracing::{error, trace};
 use tracing_subscriber::{filter::LevelFilter, prelude::__tracing_subscriber_SubscriberExt, EnvFilter, Layer};
 use types::RegistryList;
@@ -24,12 +24,12 @@ mod types;
 
 rust_i18n::i18n!("locales", fallback = "en-us");
 
-const EXIT_SUCCESS: i32 = 0;
-const EXIT_INVALID_INPUT: i32 = 2;
-const EXIT_REGISTRY_ERROR: i32 = 3;
+const EXIT_SUCCESS: u8 = 0;
+const EXIT_INVALID_INPUT: u8 = 2;
+const EXIT_REGISTRY_ERROR: u8 = 3;
 
 #[allow(clippy::too_many_lines)]
-fn main() {
+fn main() -> ExitCode {
     #[cfg(debug_assertions)]
     check_debug();
 
@@ -45,17 +45,17 @@ fn main() {
                 AdapterSubCommand::Set { input, adapted_resource } => {
                     if let Err(e) = adapter_set(&input, &adapted_resource) {
                         error!("{e}");
-                        exit(EXIT_REGISTRY_ERROR);
+                        return ExitCode::from(EXIT_REGISTRY_ERROR);
                     }
-                    exit(EXIT_SUCCESS);
+                    return ExitCode::from(EXIT_SUCCESS);
                 },
                 AdapterSubCommand::Export { input, adapted_resource } => {
                     adapter_export(&input, &adapted_resource)
                 },
                 AdapterSubCommand::Schema => {
                     let schema = schema_for!(AdaptedRegistryValue);
                     println!("{}", serde_json::to_string(&schema).unwrap());
-                    exit(EXIT_SUCCESS);
+                    return ExitCode::from(EXIT_SUCCESS);
                 }
             };
             match result {
@@ -64,7 +64,7 @@ fn main() {
                 },
                 Err(err) => {
                     error!("{err}");
-                    exit(EXIT_INVALID_INPUT);
+                    return ExitCode::from(EXIT_INVALID_INPUT);
                 }
             }
         },
@@ -85,13 +85,15 @@ fn main() {
                 ConfigSubCommand::Get{input, list} => {
                     trace!("Get input: {input}");
                     let mut output = RegistryList { registry_entries: vec![], registry_file_path: None };
-                    let reg_list = import_input(&input, list);
+                    let Ok(reg_list) = import_input(&input, list) else {
+                        return ExitCode::from(EXIT_INVALID_INPUT);
+                    };
                     for reg in reg_list.registry_entries {
                         let reg_helper = match RegistryHelper::new_from_registry(&reg) {
                             Ok(helper) => helper,
                             Err(err) => {
                                 error!("{err}");
-                                exit(EXIT_INVALID_INPUT);
+                                return ExitCode::from(EXIT_INVALID_INPUT);
                             }
                         };
                         match reg_helper.get() {
@@ -101,29 +103,31 @@ fn main() {
                                 } else {
                                     let json = serde_json::to_string(&reg_config).unwrap();
                                     println!("{json}");
-                                    exit(EXIT_SUCCESS);
+                                    return ExitCode::from(EXIT_SUCCESS);
                                 }
                             },
                             Err(err) => {
                                 error!("{err}");
-                                exit(EXIT_REGISTRY_ERROR);
+                                return ExitCode::from(EXIT_REGISTRY_ERROR);
                             }
                         }
                     }
                     let json = serde_json::to_string(&output).unwrap();
                     println!("{json}");
-                    exit(EXIT_SUCCESS);
+                    return ExitCode::from(EXIT_SUCCESS);
                 },
                 ConfigSubCommand::Set{input, list, what_if} => {
                     trace!("Set input: {input}, what_if: {what_if}");
                     let mut output = RegistryList { registry_entries: vec![], registry_file_path: None };
-                    let reg_list = import_input(&input, list);
+                    let Ok(reg_list) = import_input(&input, list) else {
+                        return ExitCode::from(EXIT_INVALID_INPUT);
+                    };
                     for reg in reg_list.registry_entries {
                         let mut reg_helper = match RegistryHelper::new_from_registry(&reg) {
                             Ok(helper) => helper,
                             Err(err) => {
                                 error!("{err}");
-                                exit(EXIT_INVALID_INPUT);
+                                return ExitCode::from(EXIT_INVALID_INPUT);
                             }
                         };
                         if what_if { reg_helper.enable_what_if(); }
@@ -136,14 +140,14 @@ fn main() {
                                             } else {
                                                 let json = serde_json::to_string(&reg_config).unwrap();
                                                 println!("{json}");
-                                                exit(EXIT_SUCCESS);
+                                                return ExitCode::from(EXIT_SUCCESS);
                                             }
                                         }
                                     },
                                     Ok(None) => {},
                                     Err(err) => {
                                         error!("{err}");
-                                        exit(EXIT_REGISTRY_ERROR);
+                                        return ExitCode::from(EXIT_REGISTRY_ERROR);
                                     }
                                 }
                                 continue;
@@ -156,32 +160,32 @@ fn main() {
                                     } else {
                                         let json = serde_json::to_string(&config).unwrap();
                                         println!("{json}");
-                                        exit(EXIT_SUCCESS);
+                                        return ExitCode::from(EXIT_SUCCESS);
                                     }
                                 }
                                 if !list {
-                                    exit(EXIT_SUCCESS);
+                                    return ExitCode::from(EXIT_SUCCESS);
                                 }
                             },
                             Err(err) => {
                                 error!("{err}");
-                                exit(EXIT_REGISTRY_ERROR);
+                                return ExitCode::from(EXIT_REGISTRY_ERROR);
                             }
                         }
                     }
                     if what_if {
                         let json = serde_json::to_string(&output).unwrap();
                         println!("{json}");
                     }
-                    exit(EXIT_SUCCESS);
+                    return ExitCode::from(EXIT_SUCCESS);
                 },
                 ConfigSubCommand::Delete{input, what_if} => {
                     trace!("Delete input: {input}, what_if: {what_if}");
                     let mut reg_helper = match RegistryHelper::new_from_json(&input) {
                         Ok(reg_helper) => reg_helper,
                         Err(err) => {
                             error!("{err}");
-                            exit(EXIT_INVALID_INPUT);
+                            return ExitCode::from(EXIT_INVALID_INPUT);
                         }
                     };
                     if what_if { reg_helper.enable_what_if(); }
@@ -193,7 +197,7 @@ fn main() {
                         Ok(None) => {},
                         Err(err) => {
                             error!("{err}");
-                            exit(EXIT_REGISTRY_ERROR);
+                            return ExitCode::from(EXIT_REGISTRY_ERROR);
                         }
                     }
                 },
@@ -210,10 +214,10 @@ fn main() {
         },
     }
 
-    exit(EXIT_SUCCESS);
+    ExitCode::from(EXIT_SUCCESS)
 }
 
-fn import_input(input: &str, list: bool) -> RegistryList {
+fn import_input(input: &str, list: bool) -> Result<RegistryList, ExitCode> {
     if list {
         match serde_json::from_str::<RegistryList>(input) {
             Ok(mut reg_list) => {
@@ -225,19 +229,19 @@ fn import_input(input: &str, list: bool) -> RegistryList {
                         }
                     }
                 }
-                reg_list
+                Ok(reg_list)
             },
             Err(err) => {
                 error!("{err}");
-                exit(EXIT_INVALID_INPUT);
+                Err(ExitCode::from(EXIT_INVALID_INPUT))
             }
         }
     } else {
         match serde_json::from_str::<Registry>(input) {
-            Ok(reg) => RegistryList { registry_entries: vec![reg], registry_file_path: None },
+            Ok(reg) => Ok(RegistryList { registry_entries: vec![reg], registry_file_path: None }),
             Err(err) => {
                 error!("{err}");
-                exit(EXIT_INVALID_INPUT);
+                Err(ExitCode::from(EXIT_INVALID_INPUT))
             }
         }
     }

resources/registry/tests/registry.config.whatif.tests.ps1

@@ -200,4 +200,21 @@ Describe 'registry config whatif tests' {
         # For delete what-if, payload should only include keyPath (and optionally valueName when deleting a value)
         ($result.psobject.properties | Where-Object { $_.Name -ne '_metadata' } | Measure-Object).Count | Should -Be 1
     }
+
+
+    It 'Removing non-existing key' -Skip:(!$IsWindows) {
+        $after_config_yaml = @'
+            $schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
+            resources:
+            - name: Reg 1
+              type: Microsoft.Windows/Registry
+              properties:
+                keyPath: HKCU\1\2\NonExisting
+                _exist: false
+'@
+        $out = dsc -l trace config set --what-if --input $after_config_yaml 2>$TestDrive/error.log | ConvertFrom-Json
+        $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Path $TestDrive/error.log -Raw)
+        $out.results.result[0].afterState.keyPath | Should -BeExactly 'HKCU\1\2\NonExisting'
+        $out.results.executionInformation.whatIf[0] | Should -Match "Key 'HKCU\\1\\2\\NonExisting' not found, would do nothing"
+    }    
 }