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

Enable using adapter resources without the adapter wrapper #720

Merged
merged 10 commits into from
Apr 1, 2025

Conversation

SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Mar 21, 2025

PR Summary

This is a breaking change, but should not affect anyone as no one has implemented an adapter yet, but we should doc it.

  • The previous code had two problems:
    • the change was in dsc exe instead of the library so it was only written for direct resource invocation and not for configuration
    • it relied on passing a special property that the adapter would recognize

So all that code was removed. The old code relied on modifying the input JSON and injecting the special property, so parameters for functions were changed to no longer need the input to be mutable.

Instead, this PR removes all that old code (the breaking change) and instead when a resource is specified whether in a config or direct resource invocation and requires an adapter, then internally it dynamically generates a configuration with that adapter along with the resource and executes that instead. The resource part of the configuration invocation output is extracted to appear as though just the resource was executed.

In the PS Adapter, I removed the code that checked for the special property so this means adapters should ONLY expect to get configuration and doesn't need to handle whether the input it received was for a resource or a complete configuration. Also, to improve error messages, added -ErrorAction Stop to use of Invoke-DscResource so that the correct error surfaces rather than a later error.

WMI Adapter updated to also only expect configuration. TestAdapter fixed as export requires an array of instances of properties representing the resources.

PR Context

Fix #693

@SteveL-MSFT
Copy link
Member Author

One problem is that if both WinPS and PS7 adapters support the same resource, the first one found is used. This is generally not a problem, but when using export, it has to be the PS7 one since WinPS doesn't support export, but if the WinPS one is returned, then the operation fails.

@michaeltlombardi
Copy link
Collaborator

One problem is that if both WinPS and PS7 adapters support the same resource, the first one found is used. This is generally not a problem, but when using export, it has to be the PS7 one since WinPS doesn't support export, but if the WinPS one is returned, then the operation fails.

Maybe we can further filter the results by whether they support the current operation, or consider (eventually?) defining a default adapter or an adapter priority in the DSC settings?

For example, in the case of export where we filter results by operation:

  1. Find all resources with the given name.
  2. If there's more than one resource, filter out resources missing the appropriate capability.
  3. Use the default selection for multiple resources with the given name.

That could be implemented independently of any settings value that defines adapter priority.

@SteveL-MSFT
Copy link
Member Author

Rather than make this PR even bigger, I'm going to fix up the tests and defer the discovery to next PR. For that one, I'll prepare for supporting specifying versions as discovery should take into account version and capability rather than the simple approach right now which is just finding the first type that matches.

@SteveL-MSFT SteveL-MSFT marked this pull request as ready for review March 25, 2025 20:27
Steve Lee (POWERSHELL HE/HIM) (from Dev Box) added 2 commits March 25, 2025 15:30
@SteveL-MSFT SteveL-MSFT changed the title Implicit ps Enable using adapter resources without the adapter wrapper Mar 26, 2025
Co-authored-by: Tess Gauthier <tgauth@bu.edu>
@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Apr 1, 2025
Merged via the queue into PowerShell:main with commit 02906fc Apr 1, 2025
4 checks passed
@SteveL-MSFT SteveL-MSFT deleted the implicit-ps branch April 1, 2025 19:31
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.

Not implemented: Custom resource not supported - Managing a Windows Service
3 participants