Skip to content

Test2040 - #2069

Open
Aditya Patwardhan (adityapatwardhan) wants to merge 4 commits into
masterfrom
test2040
Open

Aditya Patwardhan (adityapatwardhan) wants to merge 4 commits into
masterfrom
test2040

Conversation

@adityapatwardhan

Copy link
Copy Markdown
Member

PR Summary

PR Context

PR Checklist

Prior to this change, there were a few issues with the DSC resource
schemas regarding the idiomatic definitions and semantics of JSON
schema:

1. Optional properties should not define the `type` keyword with `null`
   as a valid data type unless `null` is an explicitly modeled state
   for the property.

   In JSON Schema, the absence of a property in an object is
   semantically distinct from a property explicitly set to `null`.

   Instead, we control the optionality of properties through the
   `required` or `dependentRequired` keywords.
1. The schema for `Microsoft.PowerShell.PSResourceGet/Repository`
   didn't define `required` at the top-level. Instead, it used the
   `allOf` keyword to indicate whether the `name` and `uri` properties
   are required or just `name`.

   When a property is always required, we should hoist it to the
   top-level `required` keyword array. We can still use the `allOf` to
   conditionally extend the schema as needed.

   Parsing a JSON Schema is difficult for consumers, moreso when it
   requires unrolling schema composition. We should surface as much
   information to the consumer in the simplest form possible.

To address these issues, this change:

1. Removes `null` from the `type` keyword for optional properties that
   don't explicitly model `null` as a valid state.
1. Updates the `Microsoft.PowerShell.PSResourceGet/Repository` schema
   to define `required` at the top-level to indicate that `name` is
   always required and simplifies the `allOf` keyword for the remaining
   case (when `_exist` is `true` then `uri` is required).
Prior to this change, the resource script serialized the data by just
converting the instances to JSON (stripping `_inDesiredState` for
non-test operations).

This caused the resource to serialize `null` for properties that
weren't defined on the instance, causing a mismatch with the idiomatic
schema.

This change updates the serialization logic to omit undefined optional
fields from the serialized output by:

1. Defining the `ToData` method to convert instances to a data
   representation that omits undefined optional fields while preserving
   property order for predictable serialization.

   The method defines two overloads:

   - `ToData([bool$forTest)`: Converts the instance to a data
      representation, omitting undefined optional fields. If `$forTest`
      is `$false`, the method doesn't insert `_inDesiredState`.
   - `ToData()`: Convenience overload to call `ToData($false)`.
1. Standardizing the serialization trace messaging and extracting into
   reusable methods.
Prior to this change, the `PSResourceList` resource would fail during
`export` operations when a scope had no installed resources because
it would emit an empty/invalid item. The non-terminating error for
no resources in a scope was also erroneously emitted.

This change:

1. Defines the `GetAllPsResourcesInScope` function as a wrapper around
   `Get-PsResource` to retrieve all resources in a given scope,
   returning an empty array if no resources are found and erroring if
   the command fails for any other reason.
1. Updates `PopulatePSResourceListObject` to handle empty resource
   arrays gracefully, emitting an info message instead of invalid data.
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@adityapatwardhan

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Serialization truncates what-if metadata, and unexpected retrieval failures can produce successful but incomplete exports.

2 open issues
What changed in this PR

Updates PSResourceGet’s DSC serialization, schemas, and resource export handling.

Changes:

  • Introduces dictionary-based serialization and shared tracing.
  • Tightens string types and repository field requirements.
  • Adds scope-specific resource retrieval for exports.
File Description
src/​dsc/​repository.dsc.resource.json Revises required fields and disallows null URIs.
src/​dsc/​psresourcelist.dsc.resource.json Disallows null values for string properties.
src/​dsc/​psresourceget.ps1 Refactors serialization and scoped export retrieval.

Review effort: Balanced

Comment thread src/dsc/psresourceget.ps1
}

[string] ToJson() {
$retVal = $this.ToData() | ConvertTo-Json -Compress -EnumsAsStrings
Comment thread src/dsc/psresourceget.ps1
@()
} catch {
Write-Trace -level error -message "Failed to get PSResources for '$Scope' scope: $_"
@()

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants