(GH-538) Plumb defined types for versions and names through CLI#1449
Draft
michaeltlombardi wants to merge 15 commits intoPowerShell:mainfrom
Draft
(GH-538) Plumb defined types for versions and names through CLI#1449michaeltlombardi wants to merge 15 commits intoPowerShell:mainfrom
michaeltlombardi wants to merge 15 commits intoPowerShell:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces stringly-typed resource/extension type names and versions with strongly validated types across dsc-lib discovery/configuration and the dsc CLI, enabling earlier validation failures and more canonical schema representations.
Changes:
- Introduces
WildcardTypeNameandTypeNameFilterfor validated literal/wildcard type-name filtering. - Migrates resource/extension versions and version requirements to
ResourceVersion/SemanticVersionandResourceVersionReq/SemanticVersionReq. - Updates discovery and CLI argument parsing to use the new defined types instead of raw strings.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/test_group_resource/src/main.rs | Updates test resource construction to parse typed versions. |
| lib/dsc-lib/tests/integration/types/wildcard_type_name.rs | Adds integration tests for WildcardTypeName parsing/matching/serde. |
| lib/dsc-lib/tests/integration/types/type_name_filter.rs | Adds integration tests for TypeNameFilter parsing/matching/serde/traits. |
| lib/dsc-lib/tests/integration/types/mod.rs | Registers new integration test modules. |
| lib/dsc-lib/tests/integration/types/fully_qualified_type_name.rs | Expands tests to cover parsing errors, schema docs, serde, and traits. |
| lib/dsc-lib/src/types/wildcard_type_name.rs | Adds WildcardTypeName type + validation + regex matching + serde. |
| lib/dsc-lib/src/types/type_name_filter.rs | Adds TypeNameFilter enum to represent literal vs wildcard filters. |
| lib/dsc-lib/src/types/mod.rs | Re-exports new types and error types from the types module. |
| lib/dsc-lib/src/types/fully_qualified_type_name.rs | Reworks FQTN parsing into detailed error diagnostics and adds conversions. |
| lib/dsc-lib/src/lib.rs | Updates DscManager::list_available to accept typed name filters. |
| lib/dsc-lib/src/extensions/extension_manifest.rs | Changes extension manifest version from String to SemanticVersion. |
| lib/dsc-lib/src/extensions/dscextension.rs | Changes extension model version from String to SemanticVersion. |
| lib/dsc-lib/src/dscresources/resource_manifest.rs | Changes resource manifest version from String to ResourceVersion. |
| lib/dsc-lib/src/dscresources/dscresource.rs | Changes resource model version to ResourceVersion; updates filter construction. |
| lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs | Changes adapted manifest version from String to ResourceVersion. |
| lib/dsc-lib/src/dscerror.rs | Routes type-name and filter errors through typed, transparent error variants. |
| lib/dsc-lib/src/discovery/mod.rs | Switches discovery caches/filters to typed names and typed version requirements. |
| lib/dsc-lib/src/discovery/discovery_trait.rs | Updates discovery traits and DiscoveryFilter to typed fields and typed filters. |
| lib/dsc-lib/src/discovery/command_discovery.rs | Uses TypeNameFilter matching and typed caches; updates version comparisons. |
| lib/dsc-lib/src/configure/mod.rs | Uses typed adapter/type/version fields and introduces helper macro for lookups. |
| lib/dsc-lib/src/configure/config_doc.rs | Updates config schema structs to typed version reqs and typed adapter/type fields. |
| lib/dsc-lib/locales/schemas.definitions.yaml | Updates resourceType schema definition localization structure/content. |
| lib/dsc-lib/locales/en-us.toml | Adds i18n strings for new typed errors and removes old type-name strings. |
| dsc/src/subcommand.rs | Switches CLI logic to typed type names, filters, and version requirements. |
| dsc/src/resource_command.rs | Updates resource command functions to accept typed resource names/version reqs. |
| dsc/src/mcp/show_dsc_resource.rs | Updates MCP tool to accept typed FullyQualifiedTypeName. |
| dsc/src/mcp/list_dsc_resources.rs | Updates MCP listing to use TypeNameFilter and typed map keys. |
| dsc/src/args.rs | Updates clap arg types to FullyQualifiedTypeName, TypeNameFilter, and ResourceVersionReq. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
25
to
+31
| #[must_use] | ||
| pub fn new(resource_type: &str, version: Option<&str>, adapter: Option<&str>) -> Self { | ||
| let version = version.map(|v| fix_semver(&v)); | ||
| Self { | ||
| require_adapter: adapter.map(|a| a.to_lowercase()), | ||
| r#type: resource_type.to_lowercase(), | ||
| version, | ||
| require_adapter: adapter.map(|a| a.parse().unwrap()), | ||
| r#type: resource_type.parse().unwrap(), | ||
| require_version: version.map(|v| v.parse().unwrap()), | ||
| } |
There was a problem hiding this comment.
DiscoveryFilter::new() uses parse().unwrap() for resource_type, version, and adapter. This can panic on invalid input and is currently called from user-facing entrypoints (e.g., MCP/CLI/Bicep extension). Prefer making new() return Result<Self, DscError> (or removing it in favor of the typed new_for_resource/new_for_extension constructors) so invalid input becomes a normal error instead of crashing the process.
Collaborator
Author
There was a problem hiding this comment.
Leaving this to address in a follow-up PR targeting the bicep extension specifically, which is the last caller of this method.
Prior to this change, the `FullyQualifiedTypeName` struct relied entirely on the JSON Schema validation pattern for parsing an input string into a fully qualified type name. This change refactors the implementation to take advantage of the improved diagnostics and erroring model used for the version types. This change also updates the test suite for the type to mirror types that were developed later.
Prior to this change, the logic for filtering type names used in discovery relied on simple strings converted to regex patterns for wildcard matching. This approach prevented defining either the caching `BTreeMap`s or the filter itself from using `FullyQualifiedTypeName` instances, which are the canonically identifying fields for resources, extensions, and adapters. It also meant that the code couldn't take advantage of the "parse don't validate" approach where we move validation errors earlier in the process and guarantee the safety for any instance after parsing. This change defines a new `WildcardTypeName` struct that represents a type name with one or more wildcards to use for filtering. It provides parsing and validation logic to ensure that the created instance _can_ match a `FullyQualifiedTypeName`, and it also provides a method for checking if a given `FullyQualifiedTypeName` matches the filter.
This change defines the new `TypeNameFilter` enum which has variants for literal type names (`FullyQualifiedTypeName`) and wildcard type names (`WildcardTypeName`). This change is required for updating the discovery components to cache discovered types with their `FullyQualifiedTypeName` and incorporate both precise and wildcard lookups for those types. Updates to the discovery implementation follow in a subsequent change.
Prior to this change, the discovery implementation used `BTreeMap` directly for caching discovered resources and extensions. This made using those cache types verbose and difficult to update, requiring every output type and initialization to specify the full `BTreeMap` type signature and be modified everywhere. In preparation for updating the filtering logic work more idiomatically and safely on the version and type name structs, this change defines type aliases for each of the btree maps. This change _only_ affects the ergonomics, readability, and maintainability of the code. It doesn't affect the behavior at all.
This change updates the `DiscoveryFilter` to use the parsed and validated `FullyQualifiedTypeName` instead of a raw string for the resource type and optional required adapter type. This more accurately reflects the underlying data for the caches and lookups and enables us to take advantage of the builtin parsing, comparison, and validation logic of `FullyQualifiedTypeName` instead of re-implementing that logic in the discovery code and on every access to these fields.
This change updates the following struct fields to use the `ResourceVersion` type instead of a string: - `dsc_lib::dscresources::dscresource::DscResource.version` - `dsc_lib::dscresources::resource_manifest::ResourceManifest.version` - `dsc_lib::dscresources::adapted_resource_manifest::AdaptedDscResourceManifest.version` - `dsc::mcp::show_dsc_resource::DscResource.version` This change _minimally_ updates the code to use `ResourceVersion` type, especially in discovery, but it doesn't yet take advantage of the richer semantics. This change is limited to updating the struct fields and making the code continue to compile for ease of reviewing.
This change updates the following struct fields to use the `SemanticVersion` type instead of a string: - `dsc_lib::extensions::dscextension::DscExtension.version` - `dsc_lib::extensions::extension_manifest::ExtensionManifest.version` This change _minimally_ updates the code to use `SemanticVersion` type, especially in discovery, but it doesn't yet take advantage of the richer semantics. This change is limited to updating the struct fields and making the code continue to compile for ease of reviewing.
This change continues the canonicalization work by updating the the following types and fields: - `dsc_lib::configure::config_doc::ConfigDirective.version` is now an optional `SemanticVersionReq` instead of a string. - `dsc_lib::configure::config_doc::ResourceDirective.require_adapter` is now an optional `FullyQualifiedTypeName` instead of a string. - `dsc_lib::configure::config_doc::Resource.require_version` is now an optional `ResourceVersionReq` instead of a string. This change makes additional enhancements for maintainability: - Defines the `new_for_resource` and `new_for_extension` constructors for `DiscoveryFilter` to ensure that the filter can be correctly created from existing valid data instead of needing to convert to strings and reparse on every construction. - Defines the `find_resource_or_error!` macro for the configurator to simplify the common pattern of looking up a resource and returning an error if it is not found or does not match the filter. - Simplifies the version and type name matching logic in discovery to take advantage of the canonical types, making the logic more readable and keeping the parsing and matching implementation details encapsulated in the appropriate types.
Prior to this change, the `discover()`, `discover_adapted()`, and `list_available()` methods for discovery used string slices as input for type name filters. This change updates the signatures for those methods to use the new `TypeNameFilter` struct instead. This helped to simplify the implementation for the methods and removed the need to continually construct an appropriate regex, since that is now encapsulated within the `WildcardTypeName` struct as a variant of `TypeNameFilter`. This change updates the callers of these methods to construct a `TypeNameFilter` from the input strings. This is temporary to enable updating the callers without modifying the CLI arguments, which will be addressed in a following change.
This change finishes the transition to using `TypeNameFilter` for wildcard searching resources and extensions by updating the CLI argument types and updating the implementation to match. This change enables DSC to parse the filter _once_ as an argument to the CLI and then use the parsed and validated `TypeNameFilter` instead of constructing the regular expression on demand every time it's used.
This change updates the CLI arguments for resource type names to use the `FullyQualifiedTypeName` type, and for resource version requirements to use the `ResourceVersionReq` type. This enables DSC to fail fast and clearly on invalid input instead of propagating invalid strings to parse and validate later. This change also updates the `get_resource()` function to accept the defined types instead of string slices and updates the callers as needed.
michaeltlombardi
force-pushed
the
gh-538/main/version-plumbing-redux
branch
from
bc69e9e to
8eaed82
Compare
michaeltlombardi
force-pushed
the
gh-538/main/version-plumbing-redux
branch
from
ae8b6eb to
fa340c2
Compare
Prior to this change, the PowerShell adapters returned non-semantic versions when discovering resources. This change updates the implementations to munge the `[Version]` type to a semantic version string. If the `Build` segment is defined, it is used as the patch segment. Otherwise the patch segment is set to `0`.
michaeltlombardi
force-pushed
the
gh-538/main/version-plumbing-redux
branch
from
fa340c2 to
a3097d3
Compare
This change addresses failing acceptance tests caused by the update to version parsing and handling.
michaeltlombardi
force-pushed
the
gh-538/main/version-plumbing-redux
branch
from
a3097d3 to
2b8f046
Compare
Prior to this change, the WMI adapter incorrectly set the version for the adapted resources to an empty string. This wasn't caught by prior implementations because invalid version fields were ignored, rather than raising a validation error. This change updates the adapter to set the version for the resources to `1.0.0`, mirroring the version of the adapter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
This set of changes:
WildcardTypeNamestruct andTypeNameFilterenum to enable strongly validated type name filters for use in discovery and in the CLI.ResourceVersionandSemanticVersionrespectively.SemanticVersionReqfor the required DSC version andResourceVersionReqfor the required version of resource instances.FullyQualifiedTypeName,ResourceVersion, andResourceVersionReq.FullyQualifiedTypeName,TypeNameFilter, andResourceVersionReqtypes instead of strings, enabling fast-failing on invalid input.PR Context
Prior to this change, the codebase in
dsc-libanddscused strings to represent type names, filters, versions, and version requirements. This change updates the library and binary to use defined types for these values to improve safety, error messaging, and ensure that the types are canonical for the JSON Schemas.