Skip to content

Commit 548fa97

Browse files
SteveL-MSFTSteveL-MSFT
authored
Merge pull request #1067 from Gijsreyn/feature/add-delete-whatif
Feature/add delete whatif
2 parents 3f6986c + 89d042b commit 548fa97

File tree

7 files changed

+163
-72
lines changed

7 files changed

+163
-72
lines changed

‎dsc/tests/dsc_whatif.tests.ps1‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,6 @@ Describe 'whatif tests' {
5050

5151
}
5252

53-
It 'config set whatif for delete is not supported' {
54-
$config_yaml = @"
55-
`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
56-
resources:
57-
- name: Delete
58-
type: Test/Delete
59-
properties:
60-
_exist: false
61-
"@
62-
$result = $config_yaml | dsc config set -w -f - 2>&1
63-
$result | Should -Match 'ERROR.*?Not supported.*?what-if'
64-
$LASTEXITCODE | Should -Be 2
65-
}
66-
6753
It 'config set whatif for group resource' {
6854
$result = dsc config set -f $PSScriptRoot/../examples/groups.dsc.yaml -w 2>&1
6955
$result | Should -Match 'ERROR.*?Not implemented.*?what-if'

‎dsc_lib/locales/en-us.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ elevationRequired = "Elevated security context required"
4343
restrictedRequired = "Restricted security context required"
4444
desired = "Desired state: %{state}"
4545
handlesExist = "Resource handles _exist or _exist is true"
46-
whatIfNotSupportedForDelete = "What-if execution not supported for delete"
4746
implementsDelete = "Resource implements delete and _exist is false"
4847
groupNotSupportedForDelete = "Group resources not supported for delete"
4948
deleteNotSupported = "Resource '%{resource}' does not support `delete` and does not handle `_exist` as false"

‎dsc_lib/src/configure/mod.rs‎

Lines changed: 57 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -485,58 +485,68 @@ impl Configurator {
485485
};
486486
end_datetime = chrono::Local::now();
487487
} else if dsc_resource.capabilities.contains(&Capability::Delete) {
488-
if self.context.execution_type == ExecutionKind::WhatIf {
489-
// TODO: add delete what-if support
490-
return Err(DscError::NotSupported(t!("configure.mod.whatIfNotSupportedForDelete").to_string()));
491-
}
492488
debug!("{}", t!("configure.mod.implementsDelete"));
493-
let before_result = match dsc_resource.get(&desired) {
494-
Ok(result) => result,
495-
Err(e) => {
496-
progress.set_failure(get_failure_from_error(&e));
497-
progress.write_increment(1);
498-
return Err(e);
499-
},
500-
};
501-
start_datetime = chrono::Local::now();
502-
if let Err(e) = dsc_resource.delete(&desired) {
503-
progress.set_failure(get_failure_from_error(&e));
504-
progress.write_increment(1);
505-
return Err(e);
506-
}
507-
let after_result = match dsc_resource.get(&desired) {
508-
Ok(result) => result,
509-
Err(e) => {
489+
if self.context.execution_type == ExecutionKind::WhatIf {
490+
// Let the resource handle WhatIf via set (-w), which may route to delete
491+
start_datetime = chrono::Local::now();
492+
set_result = match dsc_resource.set(&desired, skip_test, &self.context.execution_type) {
493+
Ok(result) => result,
494+
Err(e) => {
495+
progress.set_failure(get_failure_from_error(&e));
496+
progress.write_increment(1);
497+
return Err(e);
498+
},
499+
};
500+
end_datetime = chrono::Local::now();
501+
} else {
502+
let before_result = match dsc_resource.get(&desired) {
503+
Ok(result) => result,
504+
Err(e) => {
505+
progress.set_failure(get_failure_from_error(&e));
506+
progress.write_increment(1);
507+
return Err(e);
508+
},
509+
};
510+
start_datetime = chrono::Local::now();
511+
if let Err(e) = dsc_resource.delete(&desired) {
510512
progress.set_failure(get_failure_from_error(&e));
511513
progress.write_increment(1);
512514
return Err(e);
513-
},
514-
};
515-
// convert get result to set result
516-
set_result = match before_result {
517-
GetResult::Resource(before_response) => {
518-
let GetResult::Resource(after_result) = after_result else {
515+
}
516+
let after_result = match dsc_resource.get(&desired) {
517+
Ok(result) => result,
518+
Err(e) => {
519+
progress.set_failure(get_failure_from_error(&e));
520+
progress.write_increment(1);
521+
return Err(e);
522+
},
523+
};
524+
// convert get result to set result
525+
set_result = match before_result {
526+
GetResult::Resource(before_response) => {
527+
let GetResult::Resource(after_result) = after_result else {
528+
return Err(DscError::NotSupported(t!("configure.mod.groupNotSupportedForDelete").to_string()))
529+
};
530+
let diff = get_diff(&before_response.actual_state, &after_result.actual_state);
531+
let mut before: Map<String, Value> = serde_json::from_value(before_response.actual_state)?;
532+
// a `get` will return a `result` property, but an actual `set` will have that as `resources`
533+
if before.contains_key("result") && !before.contains_key("resources") {
534+
before.insert("resources".to_string(), before["result"].clone());
535+
before.remove("result");
536+
}
537+
let before_value = serde_json::to_value(&before)?;
538+
SetResult::Resource(ResourceSetResponse {
539+
before_state: before_value.clone(),
540+
after_state: after_result.actual_state,
541+
changed_properties: Some(diff),
542+
})
543+
},
544+
GetResult::Group(_) => {
519545
return Err(DscError::NotSupported(t!("configure.mod.groupNotSupportedForDelete").to_string()))
520-
};
521-
let diff = get_diff(&before_response.actual_state, &after_result.actual_state);
522-
let mut before: Map<String, Value> = serde_json::from_value(before_response.actual_state)?;
523-
// a `get` will return a `result` property, but an actual `set` will have that as `resources`
524-
if before.contains_key("result") && !before.contains_key("resources") {
525-
before.insert("resources".to_string() ,before["result"].clone());
526-
before.remove("result");
527-
}
528-
let before_value = serde_json::to_value(&before)?;
529-
SetResult::Resource(ResourceSetResponse {
530-
before_state: before_value.clone(),
531-
after_state: after_result.actual_state,
532-
changed_properties: Some(diff),
533-
})
534-
},
535-
GetResult::Group(_) => {
536-
return Err(DscError::NotSupported(t!("configure.mod.groupNotSupportedForDelete").to_string()))
537-
},
538-
};
539-
end_datetime = chrono::Local::now();
546+
},
547+
};
548+
end_datetime = chrono::Local::now();
549+
}
540550
} else {
541551
return Err(DscError::NotImplemented(t!("configure.mod.deleteNotSupported", resource = resource.resource_type).to_string()));
542552
}

‎registry/src/main.rs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,26 @@ fn main() {
7575
exit(EXIT_INVALID_INPUT);
7676
}
7777
};
78+
if what_if { reg_helper.enable_what_if(); }
79+
80+
// In what-if, if the desired state is _exist: false, route to delete
7881
if what_if {
79-
reg_helper.enable_what_if();
82+
if let Ok(desired) = serde_json::from_str::<Registry>(&input) {
83+
if matches!(desired.exist, Some(false)) {
84+
match reg_helper.remove() {
85+
Ok(Some(reg_config)) => {
86+
let json = serde_json::to_string(&reg_config).unwrap();
87+
println!("{json}");
88+
},
89+
Ok(None) => {},
90+
Err(err) => {
91+
error!("{err}");
92+
exit(EXIT_REGISTRY_ERROR);
93+
}
94+
}
95+
return;
96+
}
97+
}
8098
}
8199
match reg_helper.set() {
82100
Ok(reg_config) => {
@@ -101,7 +119,11 @@ fn main() {
101119
}
102120
};
103121
match reg_helper.remove() {
104-
Ok(()) => {},
122+
Ok(Some(reg_config)) => {
123+
let json = serde_json::to_string(&reg_config).unwrap();
124+
println!("{json}");
125+
},
126+
Ok(None) => {},
105127
Err(err) => {
106128
error!("{err}");
107129
exit(EXIT_REGISTRY_ERROR);

‎registry/tests/registry.config.whatif.tests.ps1‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,52 @@ Describe 'registry config whatif tests' {
131131
$result.keyPath | Should -Be 'HKCU\1\2'
132132
($result.psobject.properties | Measure-Object).Count | Should -Be 1
133133
}
134+
135+
It 'Can whatif delete an existing value using _exist is false' -Skip:(!$IsWindows) {
136+
$set_json = @'
137+
{
138+
"keyPath": "HKCU\\1\\2\\3",
139+
"valueName": "Hello",
140+
"valueData": {
141+
"String": "World"
142+
}
143+
}
144+
'@
145+
registry config set --input $set_json | Out-Null
146+
147+
$whatif_delete_value = @'
148+
{
149+
"keyPath": "HKCU\\1\\2\\3",
150+
"valueName": "Hello",
151+
"_exist": false
152+
}
153+
'@
154+
$result = registry config set -w --input $whatif_delete_value | ConvertFrom-Json
155+
$LASTEXITCODE | Should -Be 0
156+
$result.keyPath | Should -Be 'HKCU\1\2\3'
157+
$result.valueName | Should -Be 'Hello'
158+
$result._metadata.whatIf | Should -Match "Would delete value 'Hello'"
159+
}
160+
161+
It 'Can whatif delete an existing subkey using _exist is false' -Skip:(!$IsWindows) {
162+
$set_key = @'
163+
{
164+
"keyPath": "HKCU\\1\\2\\3"
165+
}
166+
'@
167+
registry config set --input $set_key | Out-Null
168+
169+
$whatif_delete_key = @'
170+
{
171+
"keyPath": "HKCU\\1\\2\\3",
172+
"_exist": false
173+
}
174+
'@
175+
$result = registry config set -w --input $whatif_delete_key | ConvertFrom-Json
176+
$LASTEXITCODE | Should -Be 0
177+
$result.keyPath | Should -Be 'HKCU\1\2\3'
178+
$result._metadata.whatIf | Should -Match "Would delete subkey '3'"
179+
# For delete what-if, payload should only include keyPath (and optionally valueName when deleting a value)
180+
($result.psobject.properties | Where-Object { $_.Name -ne '_metadata' } | Measure-Object).Count | Should -Be 1
181+
}
134182
}

‎registry_lib/locales/en-us.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,7 @@ unsupportedValueDataType = "Unsupported registry value data type"
1515

1616
[registry_helper]
1717
whatIfCreateKey = "Key '%{subkey}' not found, would create it"
18+
whatIfDeleteValue = "Would delete value '%{value_name}'"
19+
whatIfDeleteSubkey = "Would delete subkey '%{subkey_name}'"
1820
removeErrorKeyNotExist = "Key already does not exist"
1921
removeDeletingSubKey = "Deleting subkey '%{name}' using %{parent}"

‎registry_lib/src/lib.rs‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -258,45 +258,69 @@ impl RegistryHelper {
258258
/// # Errors
259259
///
260260
/// * `RegistryError` - The error that occurred.
261-
pub fn remove(&self) -> Result<(), RegistryError> {
261+
pub fn remove(&self) -> Result<Option<Registry>, RegistryError> {
262262
let (reg_key, _subkey) = match self.open(Security::AllAccess) {
263263
Ok(reg_key) => reg_key,
264264
// handle NotFound error
265265
Err(RegistryError::RegistryKeyNotFound(_)) => {
266266
eprintln!("{}", t!("registry_helper.removeErrorKeyNotExist"));
267-
return Ok(());
267+
return Ok(None);
268268
},
269-
Err(e) => return Err(e),
269+
Err(e) => return self.handle_error_or_what_if(e),
270270
};
271+
272+
// Accumulate what-if metadata like set()
273+
let mut what_if_metadata: Vec<String> = Vec::new();
274+
271275
if let Some(value_name) = &self.config.value_name {
276+
if self.what_if {
277+
what_if_metadata.push(t!("registry_helper.whatIfDeleteValue", value_name = value_name).to_string());
278+
return Ok(Some(Registry {
279+
key_path: self.config.key_path.clone(),
280+
value_name: Some(value_name.clone()),
281+
metadata: Some(Metadata { what_if: Some(what_if_metadata) }),
282+
..Default::default()
283+
}));
284+
}
272285
match reg_key.delete_value(value_name) {
273286
Ok(()) | Err(value::Error::NotFound(_, _)) => {
274287
// if the value doesn't exist, we don't need to do anything
275288
},
276-
Err(e) => return Err(RegistryError::RegistryValue(e)),
289+
Err(e) => return self.handle_error_or_what_if(RegistryError::RegistryValue(e)),
277290
}
278291
} else {
279292
// to delete the key, we need to open the parent key first
280293
let parent_path = get_parent_key_path(&self.config.key_path);
281294
let (hive, parent_subkey) = get_hive_from_path(parent_path)?;
282-
let parent_reg_key = hive.open(parent_subkey, Security::AllAccess)?;
295+
let parent_reg_key = match hive.open(parent_subkey, Security::AllAccess) {
296+
Ok(k) => k,
297+
Err(e) => return self.handle_error_or_what_if(RegistryError::RegistryKey(e)),
298+
};
283299

284300
// get the subkey name
285301
let subkey_name = &self.config.key_path[parent_path.len() + 1..];
286302

303+
if self.what_if {
304+
what_if_metadata.push(t!("registry_helper.whatIfDeleteSubkey", subkey_name = subkey_name).to_string());
305+
return Ok(Some(Registry {
306+
key_path: self.config.key_path.clone(),
307+
metadata: Some(Metadata { what_if: Some(what_if_metadata) }),
308+
..Default::default()
309+
}));
310+
}
287311
eprintln!("{}", t!("registry_helper.removeDeletingSubKey", name = subkey_name, parent = parent_reg_key));
288312
let Ok(subkey_name) = UCString::<u16>::from_str(subkey_name) else {
289-
return Err(RegistryError::Utf16Conversion("subkey_name".to_string()));
313+
return self.handle_error_or_what_if(RegistryError::Utf16Conversion("subkey_name".to_string()));
290314
};
291315

292316
match parent_reg_key.delete(subkey_name, true) {
293317
Ok(()) | Err(key::Error::NotFound(_, _)) => {
294318
// if the subkey doesn't exist, we don't need to do anything
295319
},
296-
Err(e) => return Err(RegistryError::RegistryKey(e)),
320+
Err(e) => return self.handle_error_or_what_if(RegistryError::RegistryKey(e)),
297321
}
298322
}
299-
Ok(())
323+
Ok(None)
300324
}
301325

302326
fn open(&self, permission: Security) -> Result<(RegKey, &str), RegistryError> {

0 commit comments

Comments
 (0)