Skip to content
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.

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

Rewrite registry resource to support delete operation #379

Merged
merged 10 commits into from
Apr 3, 2024

Conversation

SteveL-MSFT
Copy link
Member

PR Summary

  • Moved old registry resource to archive folder and excluded from build (no need to review, files moved without changes)
  • Wrote new registry resource using Registry crate simplifying the code, important behaviors to doc for resources:
    • When performing a get to a resource that doesn't exist, don't error. Instead return keys and _exist = false
    • When performing a delete to a resource that already doesn't exist, don't error. Instead return keys and _exist = false
  • Add delete as valid dsc resource operation
  • New property for set method handlesExist indicating that the resource will handle _exist directly so dsc will pass it as-is to the resource with test resource to validate. The engine work to change _exist to delete will come later after this is merged.
  • Updated table writer for dsc resource list to show SetHandlesExist and Delete capabilities
  • Updated existing tests using registry resource as it takes JSON via argument instead of stdin

PR Context

Fix #283
Partially address #290

Copy link
Collaborator

@anmenaga anmenaga left a comment

Choose a reason for hiding this comment

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

LGTM.

What was causing #283 ?

@@ -117,7 +117,7 @@ if (!$SkipBuild) {
New-Item -ItemType Directory $target > $null

# make sure dependencies are built first so clippy runs correctly
$windows_projects = @("pal", "ntreg", "ntstatuserror", "ntuserinfo", "registry", "reboot_pending", "wmi-adapter")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why "ntstatuserror" and "ntuserinfo" are removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those libs were used by the old registry resource for handling NT API errors (translating to human readable errors) and for getting the current user for HKCU (it's not a real hive). None of these are needed as the new crate uses win32 apis that abstract all of this.

@SteveL-MSFT
Copy link
Member Author

#283 was probably due to the old resource using libraries that called the NT APIs using lots of unsafe code that wasn't written correctly.

@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Apr 3, 2024
Merged via the queue into PowerShell:main with commit 80a60aa Apr 3, 2024
4 checks passed
@SteveL-MSFT SteveL-MSFT deleted the reg-delete branch April 3, 2024 21:47
michaeltlombardi added a commit to michaeltlombardi/DSC that referenced this pull request Apr 12, 2024
This change adds the `delete` method-defining property to the resource manifest
schema and updates the `capabilities` enumeration to include `SetHandlesExist`
and `Delete`, as implemented in PowerShell#379.

It updates both the source and composed schemas.
michaeltlombardi added a commit to michaeltlombardi/DSC that referenced this pull request Apr 15, 2024
This change adds the `delete` method-defining property to the resource manifest
schema and updates the `capabilities` enumeration to include `SetHandlesExist`
and `Delete`, as implemented in PowerShell#379.

It updates both the source and composed schemas.
michaeltlombardi added a commit to michaeltlombardi/DSC that referenced this pull request Apr 16, 2024
This change adds the `delete` method-defining property to the resource manifest
schema and updates the `capabilities` enumeration to include `SetHandlesExist`
and `Delete`, as implemented in PowerShell#379.

It updates both the source and composed schemas.
michaeltlombardi added a commit to michaeltlombardi/DSC that referenced this pull request Apr 18, 2024
This change adds the `delete` method-defining property to the resource manifest
schema and updates the `capabilities` enumeration to include `SetHandlesExist`
and `Delete`, as implemented in PowerShell#379.

It updates both the source and composed schemas.
michaeltlombardi added a commit to michaeltlombardi/DSC that referenced this pull request Apr 18, 2024
This change adds the `delete` method-defining property to the resource manifest
schema and updates the `capabilities` enumeration to include `SetHandlesExist`
and `Delete`, as implemented in PowerShell#379.

It updates both the source and composed schemas.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microsoft.Windows/Registry fails to open key in alpha 4 release
2 participants