PowerShell / DSC

This repo is for the DSC v3 project
MIT License
200 stars 28 forks source link

Add ability to update PATH env var for current DSC process #430

Open SteveL-MSFT opened 4 months ago

SteveL-MSFT commented 4 months ago

Summary of the new feature / enhancement

With package installation (particularly on Windows), the PATH env var may be updated from install of a new app, but won't be found until the process is restarted. However, subsequent resources in the config may depend on that just installed app. We need a way to inform DSC to refresh it's PATH env var (and possibly other env vars, but currently out of scope without clear scenario).

This can't be a setenv() function as functions are resolved as part of properties. Instead, just like _exist, we can have specially defined properties that enable this behavior such as _updatePath which would be returned by a resource requesting to have PATH updated for new child processes from current DSC process (wouldn't update the actual current process PATH).

Proposed technical implementation details (optional)

Hypothetical example of use:

$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2024/04/config/document.json
resources:
- name: install foo
  type: Microsoft.Windows/WinGet
  properties:
    packageName: foo
    packageVersion: 1.0
    # `_updatePath` isn't specified here as it's not sent to the resource, but the resource returns it as part of its response
- name: this assumes foo is found in PATH
  type: Test/ConfigFoo
  properties:
    bar: hello
  dependsOn: "[resourceId('Microsoft.Windows/WinGet','install foo')]"
anmenaga commented 4 months ago

we can have specially defined properties that enable this behavior such as _updatePath which would be returned by a resource requesting to have PATH updated for new child processes from current DSC process (wouldn't update the actual current process PATH).

This seems reasonable; except that I think it's better to update for the current process as well.

SteveL-MSFT commented 4 months ago

except that I think it's better to update for the current process as well.

I don't think we want to update the current process as that could have unintended side effects like resource discovery or calling executables that were in the PATH but some resource updated it and now can't be found

michaeltlombardi commented 4 months ago

Proposed JSON schema definition (in YAML):

$schema: https://json-schema.org/draft/2020-12/schema
$id:     <HOST>/<PREFIX>/<VERSION>/resource/properties/updatePath.yaml

title: Update Path Environment Variable
description: >-
  Indicates that DSC should refresh the PATH environment variable after executing the set operation
  against this resource.

type:
  - boolean
  - 'null'
readOnly: true

# VS Code only
markdownDescription: |
  ***
  [_Online Documentation_][01]
  ***

  Indicates whether DSC should refresh the `PATH` environment variable after executing the `set`
  operation against this resource. When this value is `true`, DSC refreshes the `PATH` environment
  variable immediately after resolving the resource result. The updated `PATH` is available to
  the remaining resources in the operation.

  To be sure that DSC only invokes the other resources after one that updates the `PATH`, use the
  [dependsOn][02] property when defining those resource instances in your configuration.

  This property is typically used by package resources when use of their installed package requires
  updating the `PATH` environment variable. This ensures that other resources in a configuration
  depending on the newly available resource can find it in the `PATH`, even if it wasn't
  installed or available before the configuration operation.

  [01]: <DOCS_BASE_URL>/reference/schemas/resource/properties/updatePath?<DOCS_VERSION_PIN>
  [02]: <DOCS_BASE_URL>/reference/schemas/config/resource?<DOCS_VERSION_PIN>#dependson
SteveL-MSFT commented 2 months ago

Since we will have support for resources to return metadata, I'm now wondering if this should be in the metadata instead of a property. @michaeltlombardi ? We would want to do the same for restart required