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

Export scenario support #171

Merged
merged 18 commits into from
Sep 1, 2023
Merged

Conversation

anmenaga
Copy link
Collaborator

@anmenaga anmenaga commented Aug 25, 2023

PR Summary

Fix #73 , #174

This adds support for enumeration of all resource instances.
Export support is declared in a resource manifest similar to Get/Set/Test methods:

    "export": {
      "executable": "process",
      "args": [
        "list"
      ]
    },

In response to this call a resource is expected to return JSONLINES - one per resource instance.

Not implemented yet:

  1. Export for provider-based resources Export support for provider-based resources  #183
  2. Export for classic PS DSC resources (this will need Export support for provider-based resources  #183 plus new version of v2 PSDesiredStateConfiguration module)

PR Context

Following scenarios supported:

  1. Export of a single resource type.
    This returns a configuration document that can be piped to dsc config set
    For example:
dsc resource export -r Microsoft/Process | dsc config set
  1. Export of all types in a configuration.
    This returns a configuration document that can be piped to dsc config set
    For example:
Get-Content -raw C:\DSCv3\process\ExportTest.dsc.yaml | dsc config export | dsc config set
  1. "resource get --all" operation that returns dsc config get result for every instance of the specified resource type.
    For example:
dsc resource get --all -r Microsoft/Process

Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

Create issues for the two items you identified for providers and PS not supported and link to this PR

dsc/tests/dsc_export.tests.ps1 Outdated Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Outdated Show resolved Hide resolved
dsc_lib/src/configure/mod.rs Outdated Show resolved Hide resolved
@michaeltlombardi
Copy link
Collaborator

To clarify my understanding of the behavior here:

  1. The dsc resource export command is for returning the full list of instances for a resource without any special handling from dsc - just returning the JSONLINES the resource emits. No formatting or result object.
  2. The dsc config export command is for returning an immediately usable configuration document, where the resources key is a flat array of the instances returned by every defined instance in the input configuration.

Assuming that's correct, a few thoughts:

Regarding the semantics/output of dsc resource export

The dsc resource export output breaks from the way all other dsc commands behave, where they have a result schema that handles some behaviors/collection for you. I would expect a result that mirrors the output from dsc resource get to some degree. I think the resource should definitely return the list of instances as JSONLINES, but I'm not sure that's the right contract for the dsc resource export command. It would make this output unique for dsc in that it just passes the resource output back through to the caller.

I think the semantic UX of the dsc resource export command is confusing. If I was a naive user, I would expect to be able to call dsc resource export -r foo/bar to get a configuration document that only includes instances of that resource. Functionally, a shorter way for me to get the functionality of dsc config export when I only care about a single resource.

As discussed in #73 (comment link), the current functionality of dsc resource export more closely mirrors the proposal for dsc resource get --all, which is a definitively distinct operation from dsc resource export. There's valid reasons both to say "I want the current state of all instances of this resource" (dsc resource get --all) and "I want a configuration based on every defined instance of this resource" (dsc resource export). The underlying code for what the resource does to return the values is the same, but how DSC presents the operation option in UX and processes the output are different.

Regarding the schema/data model

In #166, I mentioned that we might want to have a better DevX for defining the manifest entry to indicate whether a resource supports the dsc resource get --all/dsc resource export/dsc config export commands. I suspect the most common model for implementing support for this will be one of the following:

  1. When you call the command defined for get without passing it any input JSON or argument flags, it returns every instance of the resouce.
  2. When you call the command defined for get with a single boolean flag, like --all, it ignores other filtering options and input JSON, and returns every instance of the resource.

I think a fully separate command structure is less likely, but should be enabled. However, it would be good DevX to allow manifest authors to opt-in for these operations without having to redefine the setting if the case is 1. It's also potentially less intuitive to define get and export separately than to define an all property in get to indicate alternate arguments. What the author is defining is whether their resource supports returning every instance of itself.

// 'process get --all' expects no input, returns all instances
{
    "get": {
        "executable": "process",
        "args": ["get"],
        "all": {
          // executable is optional
          "args": ["get", "--all"]
        }
    }
}
// 'process get' returns all instances when called without input
{
    "get": {
        "executable": "process",
        "args": ["get"],
        "all": true // indicates that calling without input returns all
    }
}
// 'process get' returns null/error when called without input
{
    "get": {
        "executable": "process",
        "args": ["get"],
        "all": false // indicates that the resource doesn't support this op
    }
}
// 'osinfo' always and only returns one instance
{
    // this doesn't exist yet, but worth noting we can't tell the difference now
    "isSingleInstance": true,
    "get": {
        "executable": "osinfo",
        "args": [],
        "all": true
    }
}

@anmenaga
Copy link
Collaborator Author

anmenaga commented Aug 25, 2023

  1. The dsc resource export command is for returning the full list of instances for a resource without any special handling from dsc - just returning the JSONLINES the resource emits. No formatting or result object.

"No formatting" part is not correct. Internally there is deserialization from json, emitted by resource, into internal representation and then serialization into output format. This means that dsc will catch errors like invalid json, issued by resource, and can report back json as well as yaml based on dsc --format flag.

I would expect to be able to call dsc resource export -r foo/bar to get a configuration document that only includes instances of that resource. Functionally, a shorter way for me to get the functionality of dsc config export when I only care about a single resource.

I tend to agree with this. This would make it consistent so that export command always (whether resource export or config export) returns a configuration document that can be used in config set.

dsc/src/resource_command.rs Outdated Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Show resolved Hide resolved
dsc_lib/src/configure/mod.rs Outdated Show resolved Hide resolved
dsc_lib/src/configure/mod.rs Outdated Show resolved Hide resolved
dsc_lib/src/configure/mod.rs Outdated Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Outdated Show resolved Hide resolved
dsc/tests/dsc_export.tests.ps1 Outdated Show resolved Hide resolved
@SteveL-MSFT SteveL-MSFT merged commit 415d60e into PowerShell:main Sep 1, 2023
4 checks passed
michaeltlombardi added a commit to michaeltlombardi/DSC that referenced this pull request Sep 6, 2023
In PowerShell#171, DSC was updated to support the `export` commands and `dsc resource get --all`
functionality.

This change:

- Updates the resource manifest schemas to include the `export` property.
- Updates the documentation for the manifest schemas to reflect the change.
- Adds documentation for the new `dsc config export` command.
- Adds documentation for the new `dsc resource export` command.
- Adds documentation for the new `--all` option for the `dsc resource get`
  command.
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.

Need a way to support export scenario
3 participants