-
Enums should emit as strings, not integers - ConvertTo-Json should always specify the -EnumsAsStrings parameter. Currently, returning any enum value serializes it to its integer representation, which is unlikely to be what the author intended.
-
The input property erroneously forbids the input type from being a non-integer number. Defining the instance as:
input: 1.2
getScript: |-
param($inputData)
"Input number is: $inputData"
Fails the JSON Schema validation. However, the following definition is considered valid:
input:
number: 1.2
getScript: |-
param($inputData)
"Input number is: $($inputData.number)"
-
We do not have a good way to indicate changes from within the resource. When there are any changes, the changedProperties field in the result is always ["output"]. To review the changes, users must carefully compare beforeState.output to afterState.output. The only way to clearly indicate changes is to use a Write-* cmdlet to emit a message to the user.
-
The testScript is only ever used for Test operations. For both dsc resource set and dsc config set commands the resource only invokes the getScript and setScript. There is no way to avoid actually invoking the setScript when the testScript would indicate that the resource is in the desired state.
Although the resource manifest specifies set.implementsPreTest as true, the resource doesn't actually do this - we would need the script to invoke the testScript if it's defined to determine whether to invoke setScript.
-
The output for the Test operation is unreadable because it automatically includes the full definition for desiredState and actualState is always {"_inDesiredState":<true|false>}. This tells the user almost nothing about the instance and just adds noise to the result, especially when the scripts are longer than a few lines.
The differingProperties array then always shows ["input", "getScript", "setScript", "testScript"] (assuming all properties are defined). This is technically true but useless noise for the user.
We should consider a way to more usefully report the difference between desired state and actual state. It also makes me think about whether we should eventually introduce handling for read-only and write-only properties in the result objects, or perhaps a custom keyword like x-dsc-hideFromDesiredState.
-
Currently, output is always emitted as an array of JSON values. Instead, when output is a single item, we should hoist that value out of the array. Then the user could compare output.foo to output.foo instead of indexing into the array.
I think we do want to support emitting multiple objects but that we should encourage (and make readable) returning a single object to represent the state of the instance. I think that, most of the time, users are going to check a specific piece of data and report on it, which maps best to emitting a single object.
Under this proposal, the behavior for the resource itself would be:
- If
$outputObjects is empty, do not insert output into the return data for the operation.
- If
$outputObjects.Length is 1, emit that item as the value for output.
- If
$outputObjects.Length is greater than 1, emit the array as the value for output.
-
The JSON Schema for the resource needs some cleanup. In particular, the existing properties all need to be marked as writeOnly (input, *Script) and readOnly (output and _inDesiredState).
The only property that can validly be defined as null is output (if we hoist the data for scripts that only emit a single item). If you define input as null, the resource fails.
When we set type to include null we are indicating that the actual JSON value null is valid for that property. To represent whether the property is mandatory we need to use the required keyword. From a validation perspective, none of the properties are required, so long as any one of the *Script properties is defined. We can represent that in the JSON Schema as (using YAML for readability):
type: object
properties:
input: # elided for brevity
getScript: # elided for brevity
testScript: # elided for brevity
setScript: # elided for brevity
_inDesiredState: # elided for brevity
anyOf: # if the resource is defined without any `*Script` property it's invalid.
- required: [getScript]
- required: [setScript]
- required: [testScript]
We should also consider defining the contentMediaType keyword for the script properties as text/vnd.microsoft.powershell.script - those properties are expecting PowerShell scripts. While our validation engine can't operate on this keyword, it's still useful to indicate it as part of the schema.
An updated recommendation for the schema (including previous suggestions) follows:
$schema: https://json-schema.org/draft/2020-12/schema
type: object
title: Transitional PowerShell script resource
description: >-
Defines an instance of the `Microsoft.DSC.Transitional/PowerShellScript`
resource, which enables users to define inline scripts for the get, set, and
test resource operations.
anyOf:
- required: [getScript]
- required: [testScript]
- required: [setScript]
properties:
getScript:
title: Get operation script
description: >-
A PowerShell script that retrieves the current state of the instance.
$ref: "#/$defs/powershellScript"
setScript:
title: Set operation script
description: >-
A PowerShell script that enforces the desired state for the instance.
$ref: "#/$defs/powershellScript"
testScript:
title: Test operation script
description: >-
A PowerShell script that checks whether the instance is in the desired
state.
$ref: "#/$defs/powershellScript"
input:
title: Script input data
description: >-
Defines the data to pass to every script as a parameter. When defined,
every script property must include a `param()` statement with a single
parameter.
writeOnly: true
type: [string, boolean, integer, number, object, array]
output:
title: Script output data
description: >-
Defines output emitted to the success stream from the invoked script.
The resource result for an operation only includes this property when
the script emits one or more items to the success stream.
readOnly: true
type: [string, boolean, integer, number, object, array, 'null']
_inDesiredState:
$ref: "https://aka.ms/dsc/schemas/v3/resource/properties/inDesiredState.json"
$defs:
powershellScript:
writeOnly: true
type: string
minLength: 1
contentMediaType: text/vnd.microsoft.powershell
"https://aka.ms/dsc/schemas/v3/resource/properties/inDesiredState.json":
$schema: https://json-schema.org/draft/2020-12/schema
$id: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/v3/resource/properties/inDesiredState.json
title: Instance is in the Desired State
description: >-
Indicates whether the instance is in the desired state.
This property is only returned by the `test` method.
type: [boolean, 'null']
readOnly: true
As I was working on the documentation for
Microsoft.DSC.Transitional/PowerShellScriptandWindowsPowerShellScript, I noticed a few issues that I wanted to be sure to capture.Enums should emit as strings, not integers -
ConvertTo-Jsonshould always specify the-EnumsAsStringsparameter. Currently, returning any enum value serializes it to its integer representation, which is unlikely to be what the author intended.The
inputproperty erroneously forbids the input type from being a non-integer number. Defining the instance as:Fails the JSON Schema validation. However, the following definition is considered valid:
We do not have a good way to indicate changes from within the resource. When there are any changes, the
changedPropertiesfield in the result is always["output"]. To review the changes, users must carefully comparebeforeState.outputtoafterState.output. The only way to clearly indicate changes is to use aWrite-*cmdlet to emit a message to the user.The
testScriptis only ever used for Test operations. For bothdsc resource setanddsc config setcommands the resource only invokes thegetScriptandsetScript. There is no way to avoid actually invoking thesetScriptwhen thetestScriptwould indicate that the resource is in the desired state.Although the resource manifest specifies
set.implementsPreTestastrue, the resource doesn't actually do this - we would need the script to invoke thetestScriptif it's defined to determine whether to invokesetScript.The output for the Test operation is unreadable because it automatically includes the full definition for
desiredStateandactualStateis always{"_inDesiredState":<true|false>}. This tells the user almost nothing about the instance and just adds noise to the result, especially when the scripts are longer than a few lines.The
differingPropertiesarray then always shows["input", "getScript", "setScript", "testScript"](assuming all properties are defined). This is technically true but useless noise for the user.We should consider a way to more usefully report the difference between desired state and actual state. It also makes me think about whether we should eventually introduce handling for read-only and write-only properties in the result objects, or perhaps a custom keyword like
x-dsc-hideFromDesiredState.Currently,
outputis always emitted as an array of JSON values. Instead, whenoutputis a single item, we should hoist that value out of the array. Then the user could compareoutput.footooutput.fooinstead of indexing into the array.I think we do want to support emitting multiple objects but that we should encourage (and make readable) returning a single object to represent the state of the instance. I think that, most of the time, users are going to check a specific piece of data and report on it, which maps best to emitting a single object.
Under this proposal, the behavior for the resource itself would be:
$outputObjectsis empty, do not insertoutputinto the return data for the operation.$outputObjects.Lengthis1, emit that item as the value foroutput.$outputObjects.Lengthis greater than1, emit the array as the value foroutput.The JSON Schema for the resource needs some cleanup. In particular, the existing properties all need to be marked as
writeOnly(input,*Script) andreadOnly(outputand_inDesiredState).The only property that can validly be defined as
nullisoutput(if we hoist the data for scripts that only emit a single item). If you defineinputasnull, the resource fails.When we set
typeto includenullwe are indicating that the actual JSON valuenullis valid for that property. To represent whether the property is mandatory we need to use therequiredkeyword. From a validation perspective, none of the properties are required, so long as any one of the*Scriptproperties is defined. We can represent that in the JSON Schema as (using YAML for readability):We should also consider defining the
contentMediaTypekeyword for the script properties astext/vnd.microsoft.powershell.script- those properties are expecting PowerShell scripts. While our validation engine can't operate on this keyword, it's still useful to indicate it as part of the schema.An updated recommendation for the schema (including previous suggestions) follows: