Skip to content

update property names for JSON camel casing, add inDesiredState property #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SteveL-MSFT merged 6 commits into from
Aug 1, 2023
Merged

update property names for JSON camel casing, add inDesiredState property #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b35911a
update property names for JSON camel casing, add `inDesiredState` pro…
SteveL-MSFT Jul 29, 2023
1353414
fix osinfo tests
SteveL-MSFT Jul 29, 2023
18fe472
fix renaming get result
SteveL-MSFT Jul 29, 2023
d814979
rename remaining actual_state
SteveL-MSFT Jul 29, 2023
0a6cbad
rename after_state
SteveL-MSFT Jul 29, 2023
8841d5a
rename expectedState to desiredState
SteveL-MSFT Jul 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Cargo.lock
target
bin/
.DS_Store
8 changes: 4 additions & 4 deletions dsc/tests/dsc_args.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ Describe 'config argument tests' {
param($text)
$output = $text | dsc resource get -r *registry
$output = $output | ConvertFrom-Json
$output.actual_state.'$id' | Should -BeExactly 'https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json'
$output.actual_state.keyPath | Should -BeExactly 'HKLM\Software\Microsoft\Windows NT\CurrentVersion'
$output.actual_state.valueName | Should -BeExactly 'ProductName'
$output.actual_state.valueData.String | Should -Match 'Windows .*'
$output.actualState.'$id' | Should -BeExactly 'https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json'
$output.actualState.keyPath | Should -BeExactly 'HKLM\Software\Microsoft\Windows NT\CurrentVersion'
$output.actualState.valueName | Should -BeExactly 'ProductName'
$output.actualState.valueData.String | Should -Match 'Windows .*'
}
}
6 changes: 3 additions & 3 deletions dsc/tests/dsc_config_get.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ Describe 'dsc config get tests' {
$out.results.Count | Should -Be 3
$out.results[0].Name | Should -Be 'os'
$out.results[0].type | Should -BeExactly 'Microsoft/OSInfo'
$out.results[0].result.actual_state.family | Should -BeExactly 'Windows'
$out.results[0].result.actualState.family | Should -BeExactly 'Windows'
$out.results[1].Name | Should -Be 'windows product name'
$out.results[1].type | Should -BeExactly 'Microsoft.Windows/Registry'
$out.results[1].result.actual_state.valueData.String | Should -BeLike 'Windows*'
$out.results[1].result.actualState.valueData.String | Should -BeLike 'Windows*'
$out.results[2].Name | Should -Be 'system root'
$out.results[2].type | Should -BeExactly 'Microsoft.Windows/Registry'
$out.results[2].result.actual_state.valueData.String | Should -BeExactly $env:SystemRoot
$out.results[2].result.actualState.valueData.String | Should -BeExactly $env:SystemRoot
}

It 'will fail if resource schema does not match' -Skip:(!$IsWindows) {
Expand Down
8 changes: 4 additions & 4 deletions dsc/tests/dsc_get.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ Describe 'config get tests' {
$output = $json | dsc resource get -r $resource
$LASTEXITCODE | Should -Be 0
$output = $output | ConvertFrom-Json
$output.actual_state.'$id' | Should -BeExactly 'https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json'
$output.actual_state.keyPath | Should -BeExactly 'HKLM\Software\Microsoft\Windows NT\CurrentVersion'
$output.actual_state.valueName | Should -BeExactly 'ProductName'
$output.actual_state.valueData.String | Should -Match 'Windows .*'
$output.actualState.'$id' | Should -BeExactly 'https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json'
$output.actualState.keyPath | Should -BeExactly 'HKLM\Software\Microsoft\Windows NT\CurrentVersion'
$output.actualState.valueName | Should -BeExactly 'ProductName'
$output.actualState.valueData.String | Should -Match 'Windows .*'
}

It 'invalid input is validated against schema' -Skip:(!$IsWindows) {
Expand Down
18 changes: 9 additions & 9 deletions dsc/tests/dsc_set.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,18 @@ Describe 'config set tests' {
$out = $json | dsc resource set -r *registry
$LASTEXITCODE | Should -Be 0
$result = $out | ConvertFrom-Json
$result.after_state.keyPath | Should -Be 'HKCU\1\2\3'
$result.after_state.valueName | Should -Be 'Hello'
$result.after_state.valueData.String | Should -Be 'World'
$result.changed_properties | Should -Be @('keyPath', 'valueName', 'valueData')
$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.psobject.properties | Measure-Object).Count | Should -Be 3

$out = $json | dsc resource get -r *registry
$LASTEXITCODE | Should -Be 0
$result = $out | ConvertFrom-Json
$result.actual_state.keyPath | Should -Be 'HKCU\1\2\3'
$result.actual_state.valueName | Should -Be 'Hello'
$result.actual_state.valueData.String | Should -Be 'World'
$result.actualState.keyPath | Should -Be 'HKCU\1\2\3'
$result.actualState.valueName | Should -Be 'Hello'
$result.actualState.valueData.String | Should -Be 'World'
($result.psobject.properties | Measure-Object).Count | Should -Be 1

$json = @'
Expand All @@ -58,8 +58,8 @@ Describe 'config set tests' {
$out = $json | dsc resource set -r Microsoft.Windows/registry
$LASTEXITCODE | Should -Be 0
$result = $out | ConvertFrom-Json
$result.after_state.keyPath | Should -BeNullOrEmpty
$result.changed_properties | Should -Be @('keyPath')
$result.afterState.keyPath | Should -BeNullOrEmpty
$result.changedProperties | Should -Be @('keyPath')
($result.psobject.properties | Measure-Object).Count | Should -Be 3
}
}
18 changes: 9 additions & 9 deletions dsc/tests/dsc_test.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ Describe 'config test tests' {
$out = $current | dsc resource test -r *registry
$LASTEXITCODE | Should -Be 0
$out = $out | ConvertFrom-Json
$out.actual_state._inDesiredState | Should -BeTrue
$out.diff_properties | Should -BeNullOrEmpty
$out.inDesiredState | Should -BeTrue
$out.differingProperties | Should -BeNullOrEmpty
}

It 'should confirm non-matching state' -Skip:(!$IsWindows) {
Expand All @@ -30,9 +30,9 @@ Describe 'config test tests' {
$out = $json | dsc resource test -r Microsoft.Windows/registry
$LASTEXITCODE | Should -Be 0
$out = $out | ConvertFrom-Json
$out.actual_state._inDesiredState | Should -BeFalse
$out.diff_properties.Count | Should -Be 1
$out.diff_properties[0] | Should -BeExactly 'valueData'
$out.inDesiredState | Should -BeFalse
$out.differingProperties.Count | Should -Be 1
$out.differingProperties[0] | Should -BeExactly 'valueData'
}

It 'should confirm non-matching multiple state' -Skip:(!$IsWindows) {
Expand All @@ -48,9 +48,9 @@ Describe 'config test tests' {
$out = $json | dsc resource test -r *registry
$LASTEXITCODE | Should -Be 0
$out = $out | ConvertFrom-Json
$out.actual_state._inDesiredState | Should -BeFalse
$out.diff_properties.Count | Should -Be 2
$out.diff_properties[0] | Should -BeExactly 'valueName'
$out.diff_properties[1] | Should -BeExactly 'valueData'
$out.inDesiredState | Should -BeFalse
$out.differingProperties.Count | Should -Be 2
$out.differingProperties[0] | Should -BeExactly 'valueName'
$out.differingProperties[1] | Should -BeExactly 'valueData'
}
}
21 changes: 12 additions & 9 deletions dsc_lib/src/dscresources/command_resource.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str) -> Resu
// if resource doesn't implement a pre-test, we execute test first to see if a set is needed
if !set.pre_test.unwrap_or_default() {
let test_result = invoke_test(resource, cwd, desired)?;
if test_result.diff_properties.is_none() {
if test_result.in_desired_state {
return Ok(SetResult {
before_state: test_result.expected_state,
before_state: test_result.desired_state,
after_state: test_result.actual_state,
changed_properties: None,
});
Expand Down Expand Up @@ -162,9 +162,10 @@ pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str) -> Re
};
let diff_properties = get_diff(&expected_value, &actual_value);
Ok(TestResult {
expected_state: expected_value,
desired_state: expected_value,
actual_state: actual_value,
diff_properties: Some(diff_properties),
in_desired_state: diff_properties.is_empty(),
diff_properties,
})
},
Some(ReturnKind::StateAndDiff) => {
Expand All @@ -179,19 +180,21 @@ pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str) -> Re
};
let diff_properties: Vec<String> = serde_json::from_str(diff_properties)?;
Ok(TestResult {
expected_state: expected_value,
desired_state: expected_value,
actual_state: actual_value,
diff_properties: Some(diff_properties),
in_desired_state: diff_properties.is_empty(),
diff_properties,
})
},
None => {
// perform a get and compare the result to the expected state
let get_result = invoke_get(resource, cwd, expected)?;
let diff_properties = get_diff(&expected_value, &get_result.actual_state);
Ok(TestResult {
expected_state: expected_value,
desired_state: expected_value,
actual_state: get_result.actual_state,
diff_properties: Some(diff_properties),
in_desired_state: diff_properties.is_empty(),
diff_properties,
})
},
}
Expand Down Expand Up @@ -310,7 +313,7 @@ pub fn invoke_command(executable: &str, args: Option<Vec<String>>, input: Option
}

fn verify_json(resource: &ResourceManifest, cwd: &str, json: &str) -> Result<(), DscError> {

//TODO: add to debug stream:
//println!("verify_json - resource_type - {}", resource.resource_type);

Expand Down
9 changes: 5 additions & 4 deletions dsc_lib/src/dscresources/dscresource.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,13 @@ impl Invoke for DscResource {
let resource_manifest = serde_json::from_value::<ResourceManifest>(manifest.clone())?;
if resource_manifest.test.is_none() {
let get_result = self.get(expected)?;
let expected_state = serde_json::from_str(expected)?;
let diff_properties = get_diff(&expected_state, &get_result.actual_state);
let desired_state = serde_json::from_str(expected)?;
let diff_properties = get_diff(&desired_state, &get_result.actual_state);
let test_result = TestResult {
expected_state: serde_json::from_str(expected)?,
desired_state: serde_json::from_str(expected)?,
actual_state: get_result.actual_state,
diff_properties: Some(diff_properties),
in_desired_state: diff_properties.is_empty(),
diff_properties,
};
Ok(test_result)
}
Expand Down
14 changes: 12 additions & 2 deletions dsc_lib/src/dscresources/invoke_result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,39 @@ use serde_json::Value;
#[serde(deny_unknown_fields)]
pub struct GetResult {
/// The state of the resource as it was returned by the Get method.
#[serde(rename = "actualState")]
pub actual_state: Value,
}

#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct SetResult {
/// The state of the resource as it was before the Set method was called.
#[serde(rename = "beforeState")]
pub before_state: Value,
/// The state of the resource as it was after the Set method was called.
#[serde(rename = "afterState")]
pub after_state: Value,
/// The properties that were changed by the Set method from the before state.
#[serde(rename = "changedProperties")]
pub changed_properties: Option<Vec<String>>,
}

#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct TestResult {
/// The state of the resource as it was expected to be.
pub expected_state: Value,
#[serde(rename = "desiredState")]
pub desired_state: Value,
/// The state of the resource as it was returned by the Get method.
#[serde(rename = "actualState")]
pub actual_state: Value,
/// Whether the resource was in the desired state.
#[serde(rename = "inDesiredState")]
pub in_desired_state: bool,
/// The properties that were different from the expected state.
pub diff_properties: Option<Vec<String>>,
#[serde(rename = "differingProperties")]
pub diff_properties: Vec<String>,
Comment on lines +37 to +44
Copy link
Collaborator

@michaeltlombardi michaeltlombardi Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What effect, if any, does this have on resources that implement their own test operation? I think this change set only applies to how DSC returns the information about a test operation, but better to be sure.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although not implemented yet, the expectation is that resources that implement test (with state and/or diff) match the schema that dsc itself returns

Copy link
Collaborator

@michaeltlombardi michaeltlombardi Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in those cases, if I follow correctly:

  • For State, dsc generates the differingProperties array for the resource
  • For StateAndDiff, the resource is responsible for returning the array of differing properties itself

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the expectation

Copy link
Collaborator

@michaeltlombardi michaeltlombardi Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this change interact with the _inDesiredState well-known property? In the current schema/docs, we have resources that implement test using that property as part of the instance's schema, but this implies that resources which implement test return something other than the data blob for the resource instance.

In other words, given this resource's manifest snippet:

  "test": {
    // defines the command etc
    "return": "state"
  },
  "schema": {
    "embedded": {
      "$schema": "https://json-schema.org/draft/2020-12/schema",
      "type": "object",
      "properties": {
        "foo": { "type": "string" }
      }
    }
  }

The resource should return:

{"actualState": { "foo": "bar" }, "inDesiredState": false}

instead of the prior contract (which would've had _inDesiredState in the instance schema):

{ "foo": "bar", "_inDesiredState": false}

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Aug 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the change here is that resources don't emit _inDesiredState anymore unless they return state then it will look like the TestResult schema.

}

#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
Expand Down
22 changes: 11 additions & 11 deletions osinfo/tests/osinfo.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,31 @@ Describe 'osinfo resource tests' {
$out = dsc resource get -r Microsoft/osinfo | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
if ($IsWindows) {
$out.actual_state.family | Should -BeExactly 'Windows'
$out.actualState.family | Should -BeExactly 'Windows'
}
elseif ($IsLinux) {
$out.actual_state.family | Should -BeExactly 'Linux'
$out.actualState.family | Should -BeExactly 'Linux'
}
elseif ($IsMacOS) {
$out.actual_state.family | Should -BeExactly 'MacOS'
$out.actualState.family | Should -BeExactly 'MacOS'
}

$out.actual_state.version | Should -Not -BeNullOrEmpty
$out.actualState.version | Should -Not -BeNullOrEmpty
if ([Environment]::Is64BitProcess) {
$out.actual_state.bitness | Should -BeExactly '64'
$out.actualState.bitness | Should -BeExactly '64'
}
else {
$out.actual_state.bitness | Should -BeExactly '32'
$out.actualState.bitness | Should -BeExactly '32'
}
}

It 'should perform synthetic test' {
$out = '{"family": "does_not_exist"}' | dsc resource test -r '*osinfo' | ConvertFrom-Json
$actual = dsc resource get -r Microsoft/OSInfo | ConvertFrom-Json
$out.actual_state.family | Should -BeExactly $actual.actual_state.family
$out.actual_state.version | Should -BeExactly $actual.actual_state.version
$out.actual_state.bitness | Should -BeExactly $actual.actual_state.bitness
$out.actual_state.edition | Should -BeExactly $actual.actual_state.edition
$out.diff_properties | Should -Be @('family')
$out.actualState.family | Should -BeExactly $actual.actualState.family
$out.actualState.version | Should -BeExactly $actual.actualState.version
$out.actualState.bitness | Should -BeExactly $actual.actualState.bitness
$out.actualState.edition | Should -BeExactly $actual.actualState.edition
$out.differingproperties | Should -Be @('family')
}
}
18 changes: 9 additions & 9 deletions powershellgroup/Tests/powershellgroup.config.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,29 @@ Describe 'PowerShellGroup resource tests' {
}

It 'Get works on config with class-based and script-based resources' -Skip:(!$IsWindows){

$r = Get-Content -Raw $configPath | dsc config get
$LASTEXITCODE | Should -Be 0
$res = $r | ConvertFrom-Json
$res.results[0].result.actual_state[0].PublishLocation | Should -BeExactly 'https://www.powershellgallery.com/api/v2/package/'
$res.results[0].result.actual_state[1].Prop1 | Should -BeExactly 'ValueForProp1'
$res.results[0].result.actualState[0].PublishLocation | Should -BeExactly 'https://www.powershellgallery.com/api/v2/package/'
$res.results[0].result.actualState[1].Prop1 | Should -BeExactly 'ValueForProp1'
}

It 'Test works on config with class-based and script-based resources' -Skip:(!$IsWindows){

$r = Get-Content -Raw $configPath | dsc config test
$LASTEXITCODE | Should -Be 0
$res = $r | ConvertFrom-Json
$res.results[0].result.actual_state[0] | Should -Not -BeNull
$res.results[0].result.actual_state[1] | Should -Not -BeNull
$res.results[0].result.actualState[0] | Should -Not -BeNull
$res.results[0].result.actualState[1] | Should -Not -BeNull
}

It 'Set works on config with class-based and script-based resources' -Skip:(!$IsWindows){

$r = Get-Content -Raw $configPath | dsc config set
$LASTEXITCODE | Should -Be 0
$res = $r | ConvertFrom-Json
$res.results.result.after_state[0].RebootRequired | Should -Not -BeNull
$res.results.result.after_state[1].RebootRequired | Should -Not -BeNull
$res.results.result.afterState[0].RebootRequired | Should -Not -BeNull
$res.results.result.afterState[1].RebootRequired | Should -Not -BeNull
}
}
Loading