From 565c3ef33b0e64ccb8de9e61e02ca4d97e7cecbc Mon Sep 17 00:00:00 2001 From: Mikey Lombardi Date: Tue, 17 Feb 2026 09:35:39 -0600 Subject: [PATCH 1/4] (GH-538) Use `SemanticVersion` for extensions Prior to this change, the `ExtensionManifest` and `DscExtension` structs defined the type for the `version` field as `String`. This change: - Updates those structs to use the `SemanticVersion` type, which wraps `semver::Version` and provides specific JSON Schema validation. - Updates the rest of the code as required for the new field type. This includes removing the `validate_semver()` function, as it's no longer needed---parsing a valid semantic version for the extension happens during deserialization with the new type. --- dsc/src/subcommand.rs | 2 +- .../src/discovery/command_discovery.rs | 12 +-------- lib/dsc-lib/src/extensions/dscextension.rs | 6 ++--- .../src/extensions/extension_manifest.rs | 27 +++---------------- 4 files changed, 9 insertions(+), 38 deletions(-) diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index 53700b2cd..9ad6a9a45 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -637,7 +637,7 @@ fn list_extensions(dsc: &mut DscManager, extension_name: Option<&String>, format if write_table { table.add_row(vec![ extension.type_name.to_string(), - extension.version, + extension.version.to_string(), capabilities, extension.description.unwrap_or_default() ]); diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index 50eb15991..9ca00592c 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -286,13 +286,7 @@ impl ResourceDiscovery for CommandDiscovery { trace!("{}", t!("discovery.commandDiscovery.extensionFound", extension = extension.type_name, version = extension.version)); // we only keep newest version of the extension so compare the version and only keep the newest if let Some(existing_extension) = extensions.get_mut(extension.type_name.as_ref()) { - let Ok(existing_version) = Version::parse(&existing_extension.version) else { - return Err(DscError::Operation(t!("discovery.commandDiscovery.extensionInvalidVersion", extension = existing_extension.type_name, version = existing_extension.version).to_string())); - }; - let Ok(new_version) = Version::parse(&extension.version) else { - return Err(DscError::Operation(t!("discovery.commandDiscovery.extensionInvalidVersion", extension = extension.type_name, version = extension.version).to_string())); - }; - if new_version > existing_version { + if extension.version > existing_extension.version { extensions.insert(extension.type_name.to_string(), extension.clone()); } } else { @@ -871,10 +865,6 @@ fn load_resource_manifest(path: &Path, manifest: &ResourceManifest) -> Result Result { - if let Err(err) = validate_semver(&manifest.version) { - warn!("{}", t!("discovery.commandDiscovery.invalidManifestVersion", path = path.to_string_lossy(), err = err).to_string()); - } - let mut capabilities: Vec = vec![]; if let Some(discover) = &manifest.discover { verify_executable(&manifest.r#type, "discover", &discover.executable, path.parent().unwrap()); diff --git a/lib/dsc-lib/src/extensions/dscextension.rs b/lib/dsc-lib/src/extensions/dscextension.rs index ad57a9762..4a6887068 100644 --- a/lib/dsc-lib/src/extensions/dscextension.rs +++ b/lib/dsc-lib/src/extensions/dscextension.rs @@ -3,7 +3,7 @@ use crate::extensions::import::ImportMethod; use crate::schemas::{dsc_repo::DscRepoSchema, transforms::idiomaticize_string_enum}; -use crate::types::FullyQualifiedTypeName; +use crate::types::{FullyQualifiedTypeName, SemanticVersion}; use serde::{Deserialize, Serialize}; use serde_json::Value; use schemars::JsonSchema; @@ -18,7 +18,7 @@ pub struct DscExtension { #[serde(rename="type")] pub type_name: FullyQualifiedTypeName, /// The version of the resource. - pub version: String, + pub version: SemanticVersion, /// The capabilities of the resource. pub capabilities: Vec, /// The import specifics. @@ -63,7 +63,7 @@ impl DscExtension { pub fn new() -> Self { Self { type_name: FullyQualifiedTypeName::default(), - version: String::new(), + version: SemanticVersion::default(), capabilities: Vec::new(), import: None, description: None, diff --git a/lib/dsc-lib/src/extensions/extension_manifest.rs b/lib/dsc-lib/src/extensions/extension_manifest.rs index c0bad512f..e574722d4 100644 --- a/lib/dsc-lib/src/extensions/extension_manifest.rs +++ b/lib/dsc-lib/src/extensions/extension_manifest.rs @@ -3,7 +3,6 @@ use rust_i18n::t; use schemars::JsonSchema; -use semver::Version; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use std::collections::HashMap; @@ -11,7 +10,7 @@ use std::collections::HashMap; use crate::dscerror::DscError; use crate::extensions::{discover::DiscoverMethod, import::ImportMethod, secret::SecretMethod}; use crate::schemas::dsc_repo::DscRepoSchema; -use crate::types::FullyQualifiedTypeName; +use crate::types::{FullyQualifiedTypeName, SemanticVersion}; #[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] #[serde(deny_unknown_fields)] @@ -34,7 +33,7 @@ pub struct ExtensionManifest { #[serde(rename = "type")] pub r#type: FullyQualifiedTypeName, /// The version of the extension using semantic versioning. - pub version: String, + pub version: SemanticVersion, /// An optional condition for the extension to be active. If the condition evaluates to false, the extension is skipped. #[serde(skip_serializing_if = "Option::is_none")] pub condition: Option, @@ -77,24 +76,6 @@ pub fn import_manifest(manifest: Value) -> Result { Ok(manifest) } -/// Validate a semantic version string. -/// -/// # Arguments -/// -/// * `version` - The semantic version string to validate. -/// -/// # Returns -/// -/// * `Result<(), Error>` - The result of the validation. -/// -/// # Errors -/// -/// * `Error` - The version string is not a valid semantic version. -pub fn validate_semver(version: &str) -> Result<(), semver::Error> { - Version::parse(version)?; - Ok(()) -} - #[cfg(test)] mod test { use crate::schemas::dsc_repo::{DscRepoSchema, UnrecognizedSchemaUri}; @@ -108,7 +89,7 @@ mod test { let manifest = ExtensionManifest{ schema_version: invalid_uri.clone(), r#type: "Microsoft.Dsc.Test/InvalidSchemaUri".parse().unwrap(), - version: "0.1.0".to_string(), + version: "0.1.0".parse().unwrap(), ..Default::default() }; @@ -129,7 +110,7 @@ mod test { let manifest = ExtensionManifest{ schema_version: ExtensionManifest::default_schema_id_uri(), r#type: "Microsoft.Dsc.Test/ValidSchemaUri".parse().unwrap(), - version: "0.1.0".to_string(), + version: "0.1.0".parse().unwrap(), ..Default::default() }; From 0f66fea730dd6e02a3c83dad7bb06309ccf603a5 Mon Sep 17 00:00:00 2001 From: Mikey Lombardi Date: Tue, 17 Feb 2026 11:07:46 -0600 Subject: [PATCH 2/4] (GH-538) Use `ResourceVersion` for DSC resources Prior to this change, the `DscResource`, `ResourceManifest`, and `AdaptedDscResourceManifest` structs defined the `version` field as type `String`. This change: - Updates the `version` fields to use `ResourceVersion`, which encapsulates both semantic versions and arbitrary string versions. It also provides specific JSON Schema validation and documentation. - Updates the `version` field for the discovery filter to rename the field to `version_req` and change the type from `Option` to `Option`. The existing code always referenced the `version` field as a version requirement. - Updates code throughout the repository to account for the changes in field types. This enabled removing several `validate_semver()` and `fix_semver()` functions, as they're no longer needed---parsing a valid semantic version for the resource happens during deserialization with the new type. > [!IMPORTANT] > This change replaces the repeated parsing of a version requirement > string by deserializing into a `ResourceVersionReq`, which contains > _either_ a `Semantic` req (wrapping `semver::VersionReq`) or an > arbitrary string (which only matches an _identical_ version string). > > This breaks the previously implemented default behavior where the > code treats a value that parses as a semantic version as an exact > match. We can update the type to account for the previous behavior > by either: > > 1. Creating an instance of `ResourceVersionReq::Semantic` that > intercepts the given string to parse as `=`. > 1. Adding another variant, `Exact`, which contains a semantic version > and operates as an exact match for a semantic version. > > This should be addressed before we merge this change. --- dsc/src/mcp/show_dsc_resource.rs | 4 +- dsc/src/subcommand.rs | 2 +- .../src/discovery/command_discovery.rs | 52 ++++--------------- lib/dsc-lib/src/discovery/discovery_trait.rs | 15 +++--- lib/dsc-lib/src/discovery/mod.rs | 40 ++------------ .../dscresources/adapted_resource_manifest.rs | 9 ++-- lib/dsc-lib/src/dscresources/dscresource.rs | 6 +-- .../src/dscresources/resource_manifest.rs | 27 ++-------- lib/dsc-lib/src/types/resource_version_req.rs | 2 +- tools/test_group_resource/src/main.rs | 10 ++-- 10 files changed, 41 insertions(+), 126 deletions(-) diff --git a/dsc/src/mcp/show_dsc_resource.rs b/dsc/src/mcp/show_dsc_resource.rs index ae9dc50d4..d41d3b12f 100644 --- a/dsc/src/mcp/show_dsc_resource.rs +++ b/dsc/src/mcp/show_dsc_resource.rs @@ -8,7 +8,7 @@ use dsc_lib::{ dscresources::{ dscresource::{Capability, Invoke}, resource_manifest::Kind - }, types::FullyQualifiedTypeName, + }, types::{FullyQualifiedTypeName, ResourceVersion}, }; use rmcp::{ErrorData as McpError, Json, tool, tool_router, handler::server::wrapper::Parameters}; use rust_i18n::t; @@ -25,7 +25,7 @@ pub struct DscResource { /// The kind of resource. pub kind: Kind, /// The version of the resource. - pub version: String, + pub version: ResourceVersion, /// The capabilities of the resource. pub capabilities: Vec, /// The description of the resource. diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index 9ad6a9a45..507c4b35c 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -829,7 +829,7 @@ pub fn list_resources(dsc: &mut DscManager, resource_name: Option<&String>, adap table.add_row(vec![ resource.type_name.to_string(), format!("{:?}", resource.kind), - resource.version, + resource.version.to_string(), capabilities, resource.require_adapter.unwrap_or_default().to_string(), resource.description.unwrap_or_default() diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index 9ca00592c..1b97646a7 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -5,7 +5,7 @@ use crate::{discovery::{discovery_trait::{DiscoveryFilter, DiscoveryKind, Resour use crate::{locked_clear, locked_is_empty, locked_extend, locked_clone, locked_get}; use crate::configure::{config_doc::ResourceDiscoveryMode, context::Context}; use crate::dscresources::dscresource::{Capability, DscResource, ImplementedAs}; -use crate::dscresources::resource_manifest::{validate_semver, Kind, ResourceManifest, SchemaKind}; +use crate::dscresources::resource_manifest::{Kind, ResourceManifest, SchemaKind}; use crate::dscresources::command_resource::invoke_command; use crate::dscerror::DscError; use crate::extensions::dscextension::{self, DscExtension, Capability as ExtensionCapability}; @@ -15,7 +15,6 @@ use crate::util::convert_wildcard_to_regex; use crate::schemas::transforms::idiomaticize_externally_tagged_enum; use regex::RegexBuilder; use rust_i18n::t; -use semver::{Version, VersionReq}; use schemars::JsonSchema; use serde::Deserialize; use std::{collections::{BTreeMap, HashMap, HashSet}, sync::{LazyLock, RwLock}}; @@ -560,24 +559,12 @@ impl ResourceDiscovery for CommandDiscovery { fn filter_resources(found_resources: &mut BTreeMap>, required_resources: &mut HashMap, resources: &[DscResource], filter: &DiscoveryFilter) { for resource in resources { - if let Some(required_version) = filter.version() { - if let Ok(resource_version) = Version::parse(&resource.version) { - if let Ok(version_req) = VersionReq::parse(required_version) { - if version_req.matches(&resource_version) && matches_adapter_requirement(resource, filter) { - found_resources.entry(filter.resource_type().to_string()).or_default().push(resource.clone()); - required_resources.insert(filter.clone(), true); - debug!("{}", t!("discovery.commandDiscovery.foundResourceWithVersion", resource = resource.type_name, version = resource.version)); - break; - } - } - } else { - // if not semver, we do a string comparison - if resource.version == *required_version && matches_adapter_requirement(resource, filter) { - found_resources.entry(filter.resource_type().to_string()).or_default().push(resource.clone()); - required_resources.insert(filter.clone(), true); - debug!("{}", t!("discovery.commandDiscovery.foundResourceWithVersion", resource = resource.type_name, version = resource.version)); - break; - } + if let Some(version_req) = filter.version_req() { + if version_req.matches(&resource.version) && matches_adapter_requirement(resource, filter) { + found_resources.entry(filter.resource_type().to_string()).or_default().push(resource.clone()); + required_resources.insert(filter.clone(), true); + debug!("{}", t!("discovery.commandDiscovery.foundResourceWithVersion", resource = resource.type_name, version = resource.version)); + break; } } else { if matches_adapter_requirement(resource, filter) { @@ -595,23 +582,10 @@ fn filter_resources(found_resources: &mut BTreeMap>, re /// Inserts a resource into tree adding to vector if already exists fn insert_resource(resources: &mut BTreeMap>, resource: &DscResource) { if let Some(resource_versions) = resources.get_mut(&resource.type_name.to_lowercase()) { - // compare the resource versions and insert newest to oldest using semver + // compare the resource versions and insert newest to oldest let mut insert_index = resource_versions.len(); for (index, resource_instance) in resource_versions.iter().enumerate() { - let resource_instance_version = match Version::parse(&resource_instance.version) { - Ok(v) => v, - Err(_err) => { - continue; - }, - }; - let resource_version = match Version::parse(&resource.version) { - Ok(v) => v, - Err(_err) => { - continue; - }, - }; - - if resource_instance_version < resource_version { + if &resource_instance.version < &resource.version { insert_index = index; break; } @@ -775,10 +749,6 @@ pub fn load_manifest(path: &Path) -> Result, DscError> { } fn load_adapted_resource_manifest(path: &Path, manifest: &AdaptedDscResourceManifest) -> Result { - if let Err(err) = validate_semver(&manifest.version) { - warn!("{}", t!("discovery.commandDiscovery.invalidManifestVersion", path = path.to_string_lossy(), err = err).to_string()); - } - let directory = path.parent().unwrap(); let resource_path = directory.join(&manifest.path); if !resource_path.exists() { @@ -804,10 +774,6 @@ fn load_adapted_resource_manifest(path: &Path, manifest: &AdaptedDscResourceMani } fn load_resource_manifest(path: &Path, manifest: &ResourceManifest) -> Result { - if let Err(err) = validate_semver(&manifest.version) { - warn!("{}", t!("discovery.commandDiscovery.invalidManifestVersion", path = path.to_string_lossy(), err = err).to_string()); - } - let kind = if let Some(kind) = manifest.kind.clone() { kind } else if manifest.adapter.is_some() { diff --git a/lib/dsc-lib/src/discovery/discovery_trait.rs b/lib/dsc-lib/src/discovery/discovery_trait.rs index ddc007198..fc20a6bc3 100644 --- a/lib/dsc-lib/src/discovery/discovery_trait.rs +++ b/lib/dsc-lib/src/discovery/discovery_trait.rs @@ -1,9 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::{configure::config_doc::ResourceDiscoveryMode, dscerror::DscError, dscresources::dscresource::DscResource, extensions::dscextension::DscExtension}; +use crate::{configure::config_doc::ResourceDiscoveryMode, dscerror::DscError, dscresources::dscresource::DscResource, extensions::dscextension::DscExtension, types::ResourceVersionReq}; use std::collections::BTreeMap; -use super::{command_discovery::ImportedManifest, fix_semver}; +use super::command_discovery::ImportedManifest; #[derive(Debug, PartialEq)] pub enum DiscoveryKind { @@ -15,17 +15,16 @@ pub enum DiscoveryKind { pub struct DiscoveryFilter { require_adapter: Option, r#type: String, - version: Option, + version_req: Option, } impl DiscoveryFilter { #[must_use] - pub fn new(resource_type: &str, version: Option<&str>, adapter: Option<&str>) -> Self { - let version = version.map(|v| fix_semver(&v)); + pub fn new(resource_type: &str, version_req: Option<&str>, adapter: Option<&str>) -> Self { Self { require_adapter: adapter.map(|a| a.to_lowercase()), r#type: resource_type.to_lowercase(), - version, + version_req: version_req.map(|r| ResourceVersionReq::new(r)), } } @@ -40,8 +39,8 @@ impl DiscoveryFilter { } #[must_use] - pub fn version(&self) -> Option<&String> { - self.version.as_ref() + pub fn version_req(&self) -> Option<&ResourceVersionReq> { + self.version_req.as_ref() } } diff --git a/lib/dsc-lib/src/discovery/mod.rs b/lib/dsc-lib/src/discovery/mod.rs index 38c76113f..87713fb8f 100644 --- a/lib/dsc-lib/src/discovery/mod.rs +++ b/lib/dsc-lib/src/discovery/mod.rs @@ -10,7 +10,6 @@ use crate::dscerror::DscError; use crate::extensions::dscextension::{Capability, DscExtension}; use crate::{dscresources::dscresource::DscResource, progress::ProgressFormat}; use core::result::Result::Ok; -use semver::{Version, VersionReq}; use std::collections::BTreeMap; use command_discovery::{CommandDiscovery, ImportedManifest}; use tracing::error; @@ -98,25 +97,13 @@ impl Discovery { let type_name = filter.resource_type().to_lowercase(); if let Some(resources) = self.resources.get(&type_name) { - if let Some(version) = filter.version() { - let version = fix_semver(version); - if let Ok(version_req) = VersionReq::parse(&version) { - for resource in resources { - if let Ok(resource_version) = Version::parse(&resource.version) { - if version_req.matches(&resource_version) && matches_adapter_requirement(resource, filter) { - return Ok(Some(resource)); - } - } - } - Ok(None) - } else { - for resource in resources { - if resource.version == version && matches_adapter_requirement(resource, filter) { - return Ok(Some(resource)); - } + if let Some(version_req) = filter.version_req() { + for resource in resources { + if version_req.matches(&resource.version) && matches_adapter_requirement(resource, filter) { + return Ok(Some(resource)); } - Ok(None) } + Ok(None) } else { for resource in resources { if matches_adapter_requirement(resource, filter) { @@ -185,23 +172,6 @@ pub fn matches_adapter_requirement(resource: &DscResource, filter: &DiscoveryFil } } -/// Fix the semantic versioning requirements of a given version requirements string. -/// The `semver` crate uses caret (meaning compatible) by default instead of exact if not specified -/// -/// # Parameters -/// * `version` - The version requirements string to fix. -/// -/// # Returns -/// The fixed version requirements string. -#[must_use] -pub fn fix_semver(version: &str) -> String { - // Check if is semver, then if the first character is a number, then we prefix with = - if Version::parse(version).is_ok() && version.chars().next().is_some_and(|c| c.is_ascii_digit()) { - return format!("={version}"); - } - version.to_string() -} - impl Default for Discovery { fn default() -> Self { Self::new() diff --git a/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs b/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs index a93357f4a..b4034e73d 100644 --- a/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs +++ b/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs @@ -1,10 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::dscresources::{ - resource_manifest::{Kind, ResourceManifest}, - dscresource::Capability, -}; +use crate::{dscresources::{ + dscresource::Capability, resource_manifest::{Kind, ResourceManifest} +}, types::ResourceVersion}; use crate::{ schemas::dsc_repo::DscRepoSchema, types::FullyQualifiedTypeName, @@ -38,7 +37,7 @@ pub struct AdaptedDscResourceManifest { /// The kind of resource. pub kind: Kind, /// The version of the resource. - pub version: String, + pub version: ResourceVersion, /// The capabilities of the resource. pub capabilities: Vec, /// An optional condition for the resource to be active. diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index d772ad1ff..964adea61 100644 --- a/lib/dsc-lib/src/dscresources/dscresource.rs +++ b/lib/dsc-lib/src/dscresources/dscresource.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::{configure::{Configurator, config_doc::{Configuration, ExecutionKind, Resource}, context::ProcessMode, parameters::{SECURE_VALUE_REDACTED, is_secure_value}}, dscresources::resource_manifest::{AdapterInputKind, Kind}, types::FullyQualifiedTypeName}; +use crate::{configure::{Configurator, config_doc::{Configuration, ExecutionKind, Resource}, context::ProcessMode, parameters::{SECURE_VALUE_REDACTED, is_secure_value}}, dscresources::resource_manifest::{AdapterInputKind, Kind}, types::{FullyQualifiedTypeName, ResourceVersion}}; use crate::discovery::discovery_trait::DiscoveryFilter; use crate::dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse}; use crate::schemas::transforms::idiomaticize_string_enum; @@ -36,7 +36,7 @@ pub struct DscResource { /// The kind of resource. pub kind: Kind, /// The version of the resource. - pub version: String, + pub version: ResourceVersion, /// The capabilities of the resource. pub capabilities: Vec, /// The file path to the resource. @@ -101,7 +101,7 @@ impl DscResource { Self { type_name: FullyQualifiedTypeName::default(), kind: Kind::Resource, - version: String::new(), + version: ResourceVersion::default(), capabilities: Vec::new(), description: None, path: PathBuf::new(), diff --git a/lib/dsc-lib/src/dscresources/resource_manifest.rs b/lib/dsc-lib/src/dscresources/resource_manifest.rs index aeb35b814..cd0d8c381 100644 --- a/lib/dsc-lib/src/dscresources/resource_manifest.rs +++ b/lib/dsc-lib/src/dscresources/resource_manifest.rs @@ -3,14 +3,13 @@ use rust_i18n::t; use schemars::JsonSchema; -use semver::Version; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use std::collections::HashMap; use crate::{ schemas::{dsc_repo::DscRepoSchema, transforms::idiomaticize_string_enum}, - types::FullyQualifiedTypeName, + types::{FullyQualifiedTypeName, ResourceVersion}, }; #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] @@ -52,7 +51,7 @@ pub struct ResourceManifest { #[serde(skip_serializing_if = "Option::is_none")] pub kind: Option, /// The version of the resource using semantic versioning. - pub version: String, + pub version: ResourceVersion, /// The description of the resource. pub description: Option, /// Tags for the resource. @@ -316,24 +315,6 @@ pub struct ListMethod { pub args: Option>, } -/// Validate a semantic version string. -/// -/// # Arguments -/// -/// * `version` - The semantic version string to validate. -/// -/// # Returns -/// -/// * `Result<(), Error>` - The result of the validation. -/// -/// # Errors -/// -/// * `Error` - The version string is not a valid semantic version. -pub fn validate_semver(version: &str) -> Result<(), semver::Error> { - Version::parse(version)?; - Ok(()) -} - #[cfg(test)] mod test { use crate::schemas::dsc_repo::{DscRepoSchema, UnrecognizedSchemaUri}; @@ -347,7 +328,7 @@ mod test { let manifest = ResourceManifest{ schema_version: invalid_uri.clone(), resource_type: "Microsoft.Dsc.Test/InvalidSchemaUri".parse().unwrap(), - version: "0.1.0".to_string(), + version: "0.1.0".parse().unwrap(), ..Default::default() }; @@ -368,7 +349,7 @@ mod test { let manifest = ResourceManifest{ schema_version: ResourceManifest::default_schema_id_uri(), resource_type: "Microsoft.Dsc.Test/ValidSchemaUri".parse().unwrap(), - version: "0.1.0".to_string(), + version: "0.1.0".parse().unwrap(), ..Default::default() }; diff --git a/lib/dsc-lib/src/types/resource_version_req.rs b/lib/dsc-lib/src/types/resource_version_req.rs index 15025771a..fed8077e9 100644 --- a/lib/dsc-lib/src/types/resource_version_req.rs +++ b/lib/dsc-lib/src/types/resource_version_req.rs @@ -66,7 +66,7 @@ use crate::{dscerror::DscError, schemas::dsc_repo::DscRepoSchema, types::{Resour /// ``` /// /// [01]: SemanticVersionReq -#[derive(Debug, Clone, Eq, Serialize, Deserialize, JsonSchema, DscRepoSchema)] +#[derive(Debug, Clone, Hash, Eq, Serialize, Deserialize, JsonSchema, DscRepoSchema)] #[dsc_repo_schema(base_name = "resourceVersionReq", folder_path = "definitions")] #[serde(untagged)] #[schemars( diff --git a/tools/test_group_resource/src/main.rs b/tools/test_group_resource/src/main.rs index 219357038..733b4c487 100644 --- a/tools/test_group_resource/src/main.rs +++ b/tools/test_group_resource/src/main.rs @@ -17,7 +17,7 @@ fn main() { let resource1 = DscResource { type_name: "Test/TestResource1".parse().unwrap(), kind: Kind::Resource, - version: "1.0.0".to_string(), + version: "1.0.0".parse().unwrap(), capabilities: vec![Capability::Get, Capability::Set], description: Some("This is a test resource.".to_string()), implemented_as: Some(ImplementedAs::Custom("TestResource".to_string())), @@ -33,7 +33,7 @@ fn main() { schema_version: dsc_lib::dscresources::resource_manifest::ResourceManifest::default_schema_id_uri(), resource_type: "Test/TestResource1".parse().unwrap(), kind: Some(Kind::Resource), - version: "1.0.0".to_string(), + version: "1.0.0".parse().unwrap(), get: Some(GetMethod { executable: String::new(), ..Default::default() @@ -44,7 +44,7 @@ fn main() { let resource2 = DscResource { type_name: "Test/TestResource2".parse().unwrap(), kind: Kind::Resource, - version: "1.0.1".to_string(), + version: "1.0.1".parse().unwrap(), capabilities: vec![Capability::Get, Capability::Set], description: Some("This is a test resource.".to_string()), implemented_as: Some(ImplementedAs::Custom("TestResource".to_string())), @@ -60,7 +60,7 @@ fn main() { schema_version: dsc_lib::dscresources::resource_manifest::ResourceManifest::default_schema_id_uri(), resource_type: "Test/TestResource2".parse().unwrap(), kind: Some(Kind::Resource), - version: "1.0.1".to_string(), + version: "1.0.1".parse().unwrap(), get: Some(GetMethod { executable: String::new(), ..Default::default() @@ -75,7 +75,7 @@ fn main() { let resource1 = DscResource { type_name: "Test/InvalidResource".parse().unwrap(), kind: Kind::Resource, - version: "1.0.0".to_string(), + version: "1.0.0".parse().unwrap(), capabilities: vec![Capability::Get], description: Some("This is a test resource.".to_string()), implemented_as: Some(ImplementedAs::Custom("TestResource".to_string())), From 457f147a3d624b845deed700a771f70049615c0e Mon Sep 17 00:00:00 2001 From: Mikey Lombardi Date: Tue, 17 Feb 2026 14:36:12 -0600 Subject: [PATCH 3/4] (GH-538) Use `ResourceVersionReq` for `api_version` Prior to this change, the `api_version` field for a configuration document was defined as type `Option`. This change: - Updates the type to `Option` to provide correct data validation and remove the need to parse the field when accessing it after deserialization. - Updates the code to account for the changed data type. - Defines the `From` trait to convert an instance of `Resource` in a configuration document into a `DiscoveryFilter` to simplify the code. - Defines the `fail_resource_not_found()` private function for creating an instance of `DscError::ResourceNotFound` from an instance of `Resource` in a configuration document. The new trait implementation and helper function are used to simplify the code that needed updating when the type for `api_version` moved from a string to the resource version requirement. --- lib/dsc-lib/src/configure/config_doc.rs | 22 ++++++++++---- lib/dsc-lib/src/configure/mod.rs | 39 ++++++++++++++----------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/lib/dsc-lib/src/configure/config_doc.rs b/lib/dsc-lib/src/configure/config_doc.rs index fd3eb680e..427243a05 100644 --- a/lib/dsc-lib/src/configure/config_doc.rs +++ b/lib/dsc-lib/src/configure/config_doc.rs @@ -8,10 +8,10 @@ use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use std::{collections::HashMap, fmt::Display}; -use crate::{schemas::{ +use crate::{configure::get_require_adapter_from_metadata, discovery::discovery_trait::DiscoveryFilter, schemas::{ dsc_repo::DscRepoSchema, transforms::{idiomaticize_externally_tagged_enum, idiomaticize_string_enum} -}, types::FullyQualifiedTypeName}; +}, types::{FullyQualifiedTypeName, ResourceVersionReq}}; #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] #[serde(rename_all = "camelCase")] @@ -354,7 +354,7 @@ pub struct Resource { #[serde(rename = "type")] pub resource_type: FullyQualifiedTypeName, #[serde(skip_serializing_if = "Option::is_none", rename = "apiVersion")] - pub api_version: Option, + pub api_version: Option, /// A friendly name for the resource instance #[serde(default)] pub name: String, // friendly unique instance name @@ -440,11 +440,23 @@ impl Default for Resource { } } +// Enable converting a resource instance into a discovery filter +impl From<&Resource> for DiscoveryFilter { + fn from(value: &Resource) -> Self { + Self::new( + &value.resource_type.clone(), + value.api_version.clone().map(|r| r.to_string()).as_deref(), + get_require_adapter_from_metadata(&value.metadata).as_deref() + ) + } +} + #[cfg(test)] mod test { use crate::schemas::dsc_repo::{DscRepoSchema, UnrecognizedSchemaUri}; use crate::configure::config_doc::Configuration; + use crate::types::ResourceVersionReq; #[test] fn test_validate_schema_uri_with_invalid_uri() { @@ -532,11 +544,11 @@ mod test { assert_eq!(config.resources.len(), 2); assert_eq!(config.resources[0].name, "echoResource"); assert_eq!(config.resources[0].resource_type, "Microsoft.DSC.Debug/Echo"); - assert_eq!(config.resources[0].api_version.as_deref(), Some("1.0.0")); + assert_eq!(config.resources[0].api_version, Some(ResourceVersionReq::new("1.0.0"))); assert_eq!(config.resources[1].name, "processResource"); assert_eq!(config.resources[1].resource_type, "Microsoft/Process"); - assert_eq!(config.resources[1].api_version.as_deref(), Some("0.1.0")); + assert_eq!(config.resources[1].api_version, Some(ResourceVersionReq::new("0.1.0"))); } } diff --git a/lib/dsc-lib/src/configure/mod.rs b/lib/dsc-lib/src/configure/mod.rs index 47816ec06..ba9ae71bf 100644 --- a/lib/dsc-lib/src/configure/mod.rs +++ b/lib/dsc-lib/src/configure/mod.rs @@ -293,6 +293,17 @@ fn get_metadata_from_result(mut context: Option<&mut Context>, result: &mut Valu Ok(()) } +/// Returns an instance of [`DscError::ResourceNotFound`] from the given resource instance. +/// +/// This helper function simplifies the generation of the error instead of having to repeatedly +/// handling the optional version requirement everywhere this error needs to be raised. +fn fail_resource_not_found(resource: &Resource) -> DscError { + DscError::ResourceNotFound( + resource.resource_type.to_string(), + resource.api_version.clone().map(|r| r.to_string()).unwrap_or_default() + ) +} + impl Configurator { /// Create a new `Configurator` instance. /// @@ -394,9 +405,8 @@ impl Configurator { progress.write_increment(1); continue; } - let adapter = get_require_adapter_from_metadata(&resource.metadata); - let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::new(&resource.resource_type, resource.api_version.as_deref(), adapter.as_deref()))? else { - return Err(DscError::ResourceNotFound(resource.resource_type.to_string(), resource.api_version.as_deref().unwrap_or("").to_string())); + let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::from(&resource))? else { + return Err(fail_resource_not_found(&resource)); }; let properties = self.get_properties(&resource, &dsc_resource.kind)?; let filter = add_metadata(&dsc_resource, properties, resource.metadata.clone())?; @@ -479,9 +489,8 @@ impl Configurator { progress.write_increment(1); continue; } - let adapter = get_require_adapter_from_metadata(&resource.metadata); - let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::new(&resource.resource_type, resource.api_version.as_deref(), adapter.as_deref()))? else { - return Err(DscError::ResourceNotFound(resource.resource_type.to_string(), resource.api_version.as_deref().unwrap_or("").to_string())); + let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::from(&resource))? else { + return Err(fail_resource_not_found(&resource)); }; let properties = self.get_properties(&resource, &dsc_resource.kind)?; debug!("resource_type {}", &resource.resource_type); @@ -664,9 +673,8 @@ impl Configurator { progress.write_increment(1); continue; } - let adapter = get_require_adapter_from_metadata(&resource.metadata); - let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::new(&resource.resource_type, resource.api_version.as_deref(), adapter.as_deref()))? else { - return Err(DscError::ResourceNotFound(resource.resource_type.to_string(), resource.api_version.as_deref().unwrap_or("").to_string())); + let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::from(&resource))? else { + return Err(fail_resource_not_found(&resource)); }; let properties = self.get_properties(&resource, &dsc_resource.kind)?; debug!("resource_type {}", &resource.resource_type); @@ -748,9 +756,8 @@ impl Configurator { progress.write_increment(1); continue; } - let adapter = get_require_adapter_from_metadata(&resource.metadata); - let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::new(&resource.resource_type, resource.api_version.as_deref(), adapter.as_deref()))? else { - return Err(DscError::ResourceNotFound(resource.resource_type.to_string(), resource.api_version.as_deref().unwrap_or("").to_string())); + let Some(dsc_resource) = discovery.find_resource(&DiscoveryFilter::from(resource))? else { + return Err(fail_resource_not_found(resource)); }; let properties = self.get_properties(resource, &dsc_resource.kind)?; debug!("resource_type {}", &resource.resource_type); @@ -1049,8 +1056,7 @@ impl Configurator { let mut discovery_filter: Vec = Vec::new(); let config_copy = config.clone(); for resource in config_copy.resources { - let adapter = get_require_adapter_from_metadata(&resource.metadata); - let filter = DiscoveryFilter::new(&resource.resource_type, resource.api_version.as_deref(), adapter.as_deref()); + let filter = DiscoveryFilter::from(&resource); if !discovery_filter.contains(&filter) { discovery_filter.push(filter); } @@ -1069,9 +1075,8 @@ impl Configurator { // now check that each resource in the config was found for resource in config.resources.iter() { - let adapter = get_require_adapter_from_metadata(&resource.metadata); - let Some(_dsc_resource) = self.discovery.find_resource(&DiscoveryFilter::new(&resource.resource_type, resource.api_version.as_deref(), adapter.as_deref()))? else { - return Err(DscError::ResourceNotFound(resource.resource_type.to_string(), resource.api_version.as_deref().unwrap_or("").to_string())); + let Some(_dsc_resource) = self.discovery.find_resource(&DiscoveryFilter::from(resource))? else { + return Err(fail_resource_not_found(resource)); }; } } From 9659c74b1cfa18e4497f5b336c9402662d1abbe1 Mon Sep 17 00:00:00 2001 From: Mikey Lombardi Date: Tue, 17 Feb 2026 16:44:11 -0600 Subject: [PATCH 4/4] (MAINT) Address copilot review --- lib/dsc-lib/src/configure/mod.rs | 4 ++-- .../src/dscresources/adapted_resource_manifest.rs | 15 +++++++++++---- tools/test_group_resource/src/main.rs | 6 +++--- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/dsc-lib/src/configure/mod.rs b/lib/dsc-lib/src/configure/mod.rs index ba9ae71bf..85f7fcfd1 100644 --- a/lib/dsc-lib/src/configure/mod.rs +++ b/lib/dsc-lib/src/configure/mod.rs @@ -296,11 +296,11 @@ fn get_metadata_from_result(mut context: Option<&mut Context>, result: &mut Valu /// Returns an instance of [`DscError::ResourceNotFound`] from the given resource instance. /// /// This helper function simplifies the generation of the error instead of having to repeatedly -/// handling the optional version requirement everywhere this error needs to be raised. +/// handle the optional version requirement everywhere this error needs to be raised. fn fail_resource_not_found(resource: &Resource) -> DscError { DscError::ResourceNotFound( resource.resource_type.to_string(), - resource.api_version.clone().map(|r| r.to_string()).unwrap_or_default() + resource.api_version.as_ref().map(|r| r.to_string()).unwrap_or_default() ) } diff --git a/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs b/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs index b4034e73d..9d348447b 100644 --- a/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs +++ b/lib/dsc-lib/src/dscresources/adapted_resource_manifest.rs @@ -1,12 +1,19 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::{dscresources::{ - dscresource::Capability, resource_manifest::{Kind, ResourceManifest} -}, types::ResourceVersion}; use crate::{ + dscresources::{ + dscresource::Capability, + resource_manifest::{ + Kind, + ResourceManifest, + }, + }, schemas::dsc_repo::DscRepoSchema, - types::FullyQualifiedTypeName, + types::{ + FullyQualifiedTypeName, + ResourceVersion, + }, }; use rust_i18n::t; use schemars::JsonSchema; diff --git a/tools/test_group_resource/src/main.rs b/tools/test_group_resource/src/main.rs index 733b4c487..2facf1206 100644 --- a/tools/test_group_resource/src/main.rs +++ b/tools/test_group_resource/src/main.rs @@ -17,7 +17,7 @@ fn main() { let resource1 = DscResource { type_name: "Test/TestResource1".parse().unwrap(), kind: Kind::Resource, - version: "1.0.0".parse().unwrap(), + version: "1.0.0".into(), capabilities: vec![Capability::Get, Capability::Set], description: Some("This is a test resource.".to_string()), implemented_as: Some(ImplementedAs::Custom("TestResource".to_string())), @@ -44,7 +44,7 @@ fn main() { let resource2 = DscResource { type_name: "Test/TestResource2".parse().unwrap(), kind: Kind::Resource, - version: "1.0.1".parse().unwrap(), + version: "1.0.1".into(), capabilities: vec![Capability::Get, Capability::Set], description: Some("This is a test resource.".to_string()), implemented_as: Some(ImplementedAs::Custom("TestResource".to_string())), @@ -75,7 +75,7 @@ fn main() { let resource1 = DscResource { type_name: "Test/InvalidResource".parse().unwrap(), kind: Kind::Resource, - version: "1.0.0".parse().unwrap(), + version: "1.0.0".into(), capabilities: vec![Capability::Get], description: Some("This is a test resource.".to_string()), implemented_as: Some(ImplementedAs::Custom("TestResource".to_string())),