Add support for adapted resource manifests to indicate required security context - #1701
Add support for adapted resource manifests to indicate required security context#1701Steve Lee (SteveL-MSFT) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The current security-context validation logic can return early and skip adapter-level enforcement, and the dsctest manifests/schemas have inconsistencies that can invalidate the intended tests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends adapted resource manifests to support per-operation security-context requirements (get, set, test, delete, export) and wires runtime validation into resource invocation so operations fail fast when the current context is not permitted. It also updates the dsctest adapter/test fixtures and adds a Pester test validating the new behavior end-to-end.
Changes:
- Add optional per-operation
requireSecurityContextfields to adapted resource manifests and plumb the parsed adapted manifest into discoveredDscResourceinstances. - Enforce security-context requirements during command-resource invocation, including adapted-manifest overrides per operation.
- Extend dsctest adapter/fixtures for
deleteand add Pester coverage for elevated/restricted/current contexts across operations.
File summaries
| File | Description |
|---|---|
| tools/test_group_resource/src/main.rs | Initializes the new adapted_manifest field when emitting test resources. |
| tools/dsctest/src/args.rs | Adds Delete to adapter operation CLI enum. |
| tools/dsctest/src/adapter.rs | Adds security-context adapted test resources and implements adapter Delete; adjusts schema dispatch. |
| tools/dsctest/dsctest.dsc.manifests.json | Adds adapted test manifests with per-operation requireSecurityContext and adapter delete wiring. |
| lib/dsc-lib/src/dscresources/dscresource.rs | Adds adapted_manifest field to DscResource plus an internal operation enum used for validation. |
| lib/dsc-lib/src/dscresources/command_resource.rs | Validates security context per operation, including adapted-manifest overrides. |
| lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs | Defines per-operation sections (get/set/delete/test/export) with requireSecurityContext. |
| lib/dsc-lib/src/discovery/command_discovery.rs | Stores the loaded adapted manifest onto the discovered resource. |
| dsc/tests/dsc_adaptedResource.tests.ps1 | Adds Pester coverage validating security-context enforcement across operations. |
Review details
Suppressed comments (6)
lib/dsc-lib/src/dscresources/command_resource.rs:1359
- Same early-return issue for the adapted manifest’s
delete/testoperation blocks:return Ok(())bypasses validating the adapter/command manifest’srequire_security_context.
if let Some(delete) = &adapted_manifest.delete {
&delete.require_security_context
} else {
return Ok(()); // if delete is not defined, no security context validation needed
}
lib/dsc-lib/src/dscresources/command_resource.rs:1373
- Same early-return issue for the adapted manifest’s
exportoperation block:return Ok(())bypasses validating the adapter/command manifest’srequire_security_context.
if let Some(export) = &adapted_manifest.export {
&export.require_security_context
} else {
return Ok(()); // if export is not defined, no security context validation needed
}
tools/dsctest/dsctest.dsc.manifests.json:143
- This manifest defines a
deleteoperation section, butcapabilitiesdoesn’t includedelete. If capability gating is enforced,dsc resource deletemay be rejected even thoughdeleteis configured.
"capabilities": [
"get",
"set",
"test",
"export"
tools/dsctest/dsctest.dsc.manifests.json:197
- This manifest defines a
deleteoperation section, butcapabilitiesdoesn’t includedelete. If capability gating is enforced,dsc resource deletemay be rejected even thoughdeleteis configured.
"capabilities": [
"get",
"set",
"test",
"export"
tools/dsctest/dsctest.dsc.manifests.json:178
- Same schema mismatch here: schema defines
twobut adapter/tests useone. WithadditionalProperties: false, schema validation can fail for valid inputs.
"two": {
"type": "string",
"title": "Property Two",
"description": "This is property two of the adapted resource."
},
tools/dsctest/dsctest.dsc.manifests.json:232
- Same schema mismatch here: schema defines
twobut adapter/tests useone. WithadditionalProperties: false, schema validation can fail for valid inputs.
"two": {
"type": "string",
"title": "Property Two",
"description": "This is property two of the adapted resource."
},
- Files reviewed: 9/9 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
PR Summary
The adapted resource manifest now has separate sections for
get,set,delete,test, andexportwith an optional propertyrequireSecurityContextthat is eitherelevated,restricted, orcurrent. The current security context is validated against this at runtime and will cause failure if not acceptable.Note that if the adapted resource and the adapter specify a security context, the adapted resource manifest overrides the adapter. If the adapted resource manifest doesn't explicitly define a security context, then it fallsback to the adapter.
Also noticed some function inline docs were misssing parameters, so fixed those.
Added separate adapted test resources for
elevated,restricted, andcurrentwith appropriate tests.PR Context
Fix #1543