From b52b29ed0472c6afe16bbf0647b89fa754e8407f Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 13 Mar 2025 20:16:33 -0700 Subject: [PATCH 1/3] Fix not parsing expressions for Group resources --- dsc/tests/dsc_expressions.tests.ps1 | 28 ++++++++++++++++ dsc_lib/src/configure/mod.rs | 52 ++++++++++++++++++++++++----- dsc_lib/src/discovery/mod.rs | 1 + 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/dsc/tests/dsc_expressions.tests.ps1 b/dsc/tests/dsc_expressions.tests.ps1 index 11907a49..7b33366d 100644 --- a/dsc/tests/dsc_expressions.tests.ps1 +++ b/dsc/tests/dsc_expressions.tests.ps1 @@ -79,4 +79,32 @@ string. "@.Replace("`r", "") $out.results[1].result.actualState.output | Should -BeExactly "This is a single-quote: '" } + + It 'Nested Group resource does not invoke expressions' { + $yaml = @' +$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +resources: +- name: Nested Group + type: Microsoft.DSC/Group + properties: + $schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json + resources: + - name: Deeply nested OSInfo + type: Microsoft/OSInfo + properties: {} + - name: Deeply nested echo + type: Microsoft.DSC.Debug/Echo + properties: + output: >- + [reference( + resourceId('Microsoft/OSInfo', 'Deeply nested OSInfo') + )] + dependsOn: + - "[resourceId('Microsoft/OSInfo', 'Deeply nested OSInfo')]" +'@ + + $out = dsc config get -i $yaml | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $out.results[0].result[1].result.actualState.output.family | Should -BeExactly $out.results[0].result[0].result.actualState.family + } } diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 91fd5bc4..d6371901 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -248,10 +248,19 @@ impl Configurator { for resource in resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Get '{}'", resource.name).as_str()); - let properties = self.invoke_property_expressions(resource.properties.as_ref())?; - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let discovery = &self.discovery.clone(); + let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; + let properties = match &dsc_resource.kind { + Kind::Group => { + // if Group resource, we leave it to the resource to handle expressions + resource.properties.clone() + }, + _ => { + self.invoke_property_expressions(resource.properties.as_ref())? + }, + }; debug!("resource_type {}", &resource.resource_type); let filter = add_metadata(&dsc_resource.kind, properties)?; trace!("filter: {filter}"); @@ -325,10 +334,19 @@ impl Configurator { for resource in resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Set '{}'", resource.name).as_str()); - let properties = self.invoke_property_expressions(resource.properties.as_ref())?; - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let discovery = &self.discovery.clone(); + let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; + let properties = match &dsc_resource.kind { + Kind::Group => { + // if Group resource, we leave it to the resource to handle expressions + resource.properties.clone() + }, + _ => { + self.invoke_property_expressions(resource.properties.as_ref())? + }, + }; debug!("resource_type {}", &resource.resource_type); // see if the properties contains `_exist` and is false @@ -469,10 +487,19 @@ impl Configurator { for resource in resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Test '{}'", resource.name).as_str()); - let properties = self.invoke_property_expressions(resource.properties.as_ref())?; - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let discovery = &self.discovery.clone(); + let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; + let properties = match &dsc_resource.kind { + Kind::Group => { + // if Group resource, we leave it to the resource to handle expressions + resource.properties.clone() + }, + _ => { + self.invoke_property_expressions(resource.properties.as_ref())? + }, + }; debug!("resource_type {}", &resource.resource_type); let expected = add_metadata(&dsc_resource.kind, properties)?; trace!("{}", t!("configure.mod.expectedState", state = expected)); @@ -544,10 +571,19 @@ impl Configurator { for resource in &resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Export '{}'", resource.name).as_str()); - let properties = self.invoke_property_expressions(resource.properties.as_ref())?; - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let discovery = &self.discovery.clone(); + let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; + let properties = match &dsc_resource.kind { + Kind::Group => { + // if Group resource, we leave it to the resource to handle expressions + resource.properties.clone() + }, + _ => { + self.invoke_property_expressions(resource.properties.as_ref())? + }, + }; let input = add_metadata(&dsc_resource.kind, properties)?; trace!("{}", t!("configure.mod.exportInput", input = input)); let export_result = match add_resource_export_results_to_configuration(dsc_resource, Some(dsc_resource), &mut conf, input.as_str()) { diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index 17796999..edcc29ba 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -9,6 +9,7 @@ use crate::{dscresources::dscresource::DscResource, dscerror::DscError, progress use std::collections::BTreeMap; use tracing::error; +#[derive(Clone)] pub struct Discovery { pub resources: BTreeMap, } From 5e9c4971c3342bd02196b8d1c22af5b6390a7509 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 13 Mar 2025 20:41:22 -0700 Subject: [PATCH 2/3] move clone outside of loop --- dsc_lib/src/configure/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index d6371901..3a921e27 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -245,10 +245,10 @@ impl Configurator { let mut result = ConfigurationGetResult::new(); let resources = get_resource_invocation_order(&self.config, &mut self.statement_parser, &self.context)?; let mut progress = ProgressBar::new(resources.len() as u64, self.progress_format)?; + let discovery = &self.discovery.clone(); for resource in resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Get '{}'", resource.name).as_str()); - let discovery = &self.discovery.clone(); let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; @@ -331,10 +331,10 @@ impl Configurator { let mut result = ConfigurationSetResult::new(); let resources = get_resource_invocation_order(&self.config, &mut self.statement_parser, &self.context)?; let mut progress = ProgressBar::new(resources.len() as u64, self.progress_format)?; + let discovery = &self.discovery.clone(); for resource in resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Set '{}'", resource.name).as_str()); - let discovery = &self.discovery.clone(); let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; @@ -484,10 +484,10 @@ impl Configurator { let mut result = ConfigurationTestResult::new(); let resources = get_resource_invocation_order(&self.config, &mut self.statement_parser, &self.context)?; let mut progress = ProgressBar::new(resources.len() as u64, self.progress_format)?; + let discovery = &self.discovery.clone(); for resource in resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Test '{}'", resource.name).as_str()); - let discovery = &self.discovery.clone(); let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; @@ -568,10 +568,10 @@ impl Configurator { let mut progress = ProgressBar::new(self.config.resources.len() as u64, self.progress_format)?; let resources = self.config.resources.clone(); + let discovery = &self.discovery.clone(); for resource in &resources { progress.set_resource(&resource.name, &resource.resource_type); progress.write_activity(format!("Export '{}'", resource.name).as_str()); - let discovery = &self.discovery.clone(); let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; From f3d2a5a5aba892b061b863c9855a08659f3ca71a Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 14 Mar 2025 19:17:35 -0700 Subject: [PATCH 3/3] move common code to helper --- dsc_lib/src/configure/mod.rs | 52 +++++++++++------------------------- 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 3a921e27..d2004873 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -232,6 +232,18 @@ impl Configurator { &self.config } + fn get_properties(&mut self, resource: &Resource, resource_kind: &Kind) -> Result>, DscError> { + match resource_kind { + Kind::Group => { + // if Group resource, we leave it to the resource to handle expressions + Ok(resource.properties.clone()) + }, + _ => { + Ok(self.invoke_property_expressions(resource.properties.as_ref())?) + }, + } + } + /// Invoke the get operation on a resource. /// /// # Returns @@ -252,15 +264,7 @@ impl Configurator { let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; - let properties = match &dsc_resource.kind { - Kind::Group => { - // if Group resource, we leave it to the resource to handle expressions - resource.properties.clone() - }, - _ => { - self.invoke_property_expressions(resource.properties.as_ref())? - }, - }; + let properties = self.get_properties(&resource, &dsc_resource.kind)?; debug!("resource_type {}", &resource.resource_type); let filter = add_metadata(&dsc_resource.kind, properties)?; trace!("filter: {filter}"); @@ -338,15 +342,7 @@ impl Configurator { let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; - let properties = match &dsc_resource.kind { - Kind::Group => { - // if Group resource, we leave it to the resource to handle expressions - resource.properties.clone() - }, - _ => { - self.invoke_property_expressions(resource.properties.as_ref())? - }, - }; + let properties = self.get_properties(&resource, &dsc_resource.kind)?; debug!("resource_type {}", &resource.resource_type); // see if the properties contains `_exist` and is false @@ -491,15 +487,7 @@ impl Configurator { let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; - let properties = match &dsc_resource.kind { - Kind::Group => { - // if Group resource, we leave it to the resource to handle expressions - resource.properties.clone() - }, - _ => { - self.invoke_property_expressions(resource.properties.as_ref())? - }, - }; + let properties = self.get_properties(&resource, &dsc_resource.kind)?; debug!("resource_type {}", &resource.resource_type); let expected = add_metadata(&dsc_resource.kind, properties)?; trace!("{}", t!("configure.mod.expectedState", state = expected)); @@ -575,15 +563,7 @@ impl Configurator { let Some(dsc_resource) = discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; - let properties = match &dsc_resource.kind { - Kind::Group => { - // if Group resource, we leave it to the resource to handle expressions - resource.properties.clone() - }, - _ => { - self.invoke_property_expressions(resource.properties.as_ref())? - }, - }; + let properties = self.get_properties(resource, &dsc_resource.kind)?; let input = add_metadata(&dsc_resource.kind, properties)?; trace!("{}", t!("configure.mod.exportInput", input = input)); let export_result = match add_resource_export_results_to_configuration(dsc_resource, Some(dsc_resource), &mut conf, input.as_str()) {