PowerShell / DSC

This repo is for the DSC v3 project
MIT License
133 stars 22 forks source link

remove support to retrieve schema from URL #457

Closed SteveL-MSFT closed 6 hours ago

SteveL-MSFT commented 2 weeks ago

PR Summary

The support for schema URL was initially intended for intellisense support. However, we can still resolve resources for intellisense using existing other means to get the json schema. We should not allow reaching out to internet during configuration deployment.

PR Context

michaeltlombardi commented 2 weeks ago

What about resolving references in the embedded-or-command-returned schema? Will we require bundling for resource JSON schemas?

SteveL-MSFT commented 2 weeks ago

@michaeltlombardi yeah, forgot about referenced schema. We should discuss this further.

SteveL-MSFT commented 2 weeks ago

@michaeltlombardi actually, I just remembered that the JsonSchema crate has its own built in way to retrieve web referenced schema so this PR is literally only removing it as an option the manifest so I think we're ok

michaeltlombardi commented 2 weeks ago

I mean that, from a guidance perspective, if the concern is either offline-functionality or no-web-calls-on-apply, we should advise authors on whether and when to bundle their schemas.

SteveL-MSFT commented 2 weeks ago

@michaeltlombardi with this change, we require schema to be shipped with the resource whether it's embedded or via command. We should recommend that those schema should not use web references as that would mean their resource won't work on some systems that limit outbound connections.

michaeltlombardi commented 2 weeks ago

@SteveL-MSFT One dangling question then:

In our docs, we recommend that users reference the well-known properties instead of redefining them, e.g.

"_rebootRequested": {
  "$ref": "https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2024/04/resource/properties/rebootRequested.json"
}

This ensures that users don't accidentally mis-defined the properties. I think we can tell users either that they can reference DSC schemas and subschemas, or that they need to bundle these into their full definition. I prefer the former, for simplicity - but some bundling tools will also find and pull those references in for them, too.

SteveL-MSFT commented 2 weeks ago

@SteveL-MSFT One dangling question then:

In our docs, we recommend that users reference the well-known properties instead of redefining them, e.g.

"_rebootRequested": {
  "$ref": "https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2024/04/resource/properties/rebootRequested.json"
}

This ensures that users don't accidentally mis-defined the properties. I think we can tell users either that they can reference DSC schemas and subschemas, or that they need to bundle these into their full definition. I prefer the former, for simplicity - but some bundling tools will also find and pull those references in for them, too.

Referencing schema should still be recommended and this change shouldn't affect that. This should only affect having the url type of schema in the manifest.