Merge pull request #294 from SteveL-MSFT/validate-after-expression

Fix parameters to work with property values

dsc/Cargo.toml

@@ -21,7 +21,7 @@ jsonschema = "0.17"
 schemars = { version = "0.8.12" }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = { version = "1.0", features = ["preserve_order"] }
-serde_yaml = { version = "0.9" }
+serde_yaml = { version = "0.9.3" }
 syntect = { version = "5.0", features = ["default-fancy"], default-features = false }
 sysinfo = { version = "0.29.10" }
 thiserror = "1.0"

dsc/tests/dsc_config_get.tests.ps1

@@ -27,12 +27,7 @@ Describe 'dsc config get tests' {
     It 'will fail if resource schema does not match' -Skip:(!$IsWindows) {
         $jsonPath = Join-Path $PSScriptRoot '../examples/invalid_schema.dsc.yaml'
         $config = Get-Content $jsonPath -Raw
-        $out = $config | dsc config get | ConvertFrom-Json
+        $null = $config | dsc config get | ConvertFrom-Json
         $LASTEXITCODE | Should -Be 2
-        $out.hadErrors | Should -BeTrue
-        $out.results.Count | Should -Be 0
-        $out.messages.Count | Should -Be 2
-        $out.messages[0].level | Should -BeExactly 'Error'
-        $out.messages[1].level | Should -BeExactly 'Error'
     }
 }

dsc/tests/dsc_parameters.tests.ps1

@@ -31,11 +31,11 @@ Describe 'Parameters tests' {
         }
 
         $LASTEXITCODE | Should -Be 0
-        $out.results[0].result.actualState.text | Should -BeExactly '"hello"'
+        $out.results[0].result.actualState.text | Should -BeExactly 'hello'
     }
 
     It 'Input is <type>' -TestCases @(
-        @{ type = 'string'; value = 'hello'; expected = '"hello"' }
+        @{ type = 'string'; value = 'hello'; expected = 'hello' }
         @{ type = 'int'; value = 42; expected = 42 }
         @{ type = 'bool'; value = $true; expected = $true }
         @{ type = 'array'; value = @('hello', 'world'); expected = '["hello","world"]' }
@@ -213,6 +213,44 @@ Describe 'Parameters tests' {
 
         $out = $config_yaml | dsc config get | ConvertFrom-Json
         $LASTEXITCODE | Should -Be 0
-        $out.results[0].result.actualState.text | Should -BeExactly '"hello",7,false,["hello","world"]'
+        $out.results[0].result.actualState.text | Should -BeExactly 'hello,7,false,["hello","world"]'
+    }
+
+    It 'property value uses parameter value' {
+      $os = 'Windows'
+      if ($IsLinux) {
+        $os = 'Linux'
+      }
+      elseif ($IsMacOS) {
+        $os = 'macOS'
+      }
+
+      $params = @{
+        parameters = @{
+          osFamily = $os
+        }
+      } | ConvertTo-Json
+
+      $config_yaml = @'
+        $schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/08/config/document.json
+        parameters:
+          osFamily:
+            type: string
+            defaultValue: Windows
+            allowedValues:
+              - Windows
+              - Linux
+              - macOS
+        resources:
+        - name: os
+          type: Microsoft/OSInfo
+          properties:
+            family: '[parameters(''osFamily'')]'
+'@
+
+      $out = dsc -i $config_yaml config -p $params test | ConvertFrom-Json
+      $LASTEXITCODE | Should -Be 0
+      $out.results[0].result.actualState.family | Should -BeExactly $os
+      $out.results[0].result.inDesiredState | Should -BeTrue
     }
 }

dsc_lib/Cargo.toml

@@ -19,7 +19,7 @@ tree-sitter = "~0.20.10"
 tree-sitter-dscexpression = { path = "../tree-sitter-dscexpression" }
 
 [dev-dependencies]
-serde_yaml = "0.9"
+serde_yaml = "0.9.3"
 
 [build-dependencies]
 cc="*"

dsc_lib/src/configure/mod.rs

@@ -1,8 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use jsonschema::JSONSchema;
-
 use crate::configure::parameters::Input;
 use crate::dscerror::DscError;
 use crate::dscresources::dscresource::Invoke;
@@ -12,7 +10,7 @@ use crate::parser::Statement;
 use self::context::Context;
 use self::config_doc::{Configuration, DataType};
 use self::depends_on::get_resource_invocation_order;
-use self::config_result::{ConfigurationGetResult, ConfigurationSetResult, ConfigurationTestResult, ConfigurationExportResult, ResourceMessage, MessageLevel};
+use self::config_result::{ConfigurationGetResult, ConfigurationSetResult, ConfigurationTestResult, ConfigurationExportResult};
 use self::contraints::{check_length, check_number_limits, check_allowed_values};
 use serde_json::{Map, Value};
 use std::collections::{HashMap, HashSet};
@@ -155,13 +153,8 @@ impl Configurator {
     ///
     /// This function will return an error if the underlying resource fails.
     pub fn invoke_get(&mut self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result<ConfigurationGetResult, DscError> {
-        let (config, messages, had_errors) = self.validate_config()?;
+        let config = self.validate_config()?;
         let mut result = ConfigurationGetResult::new();
-        result.messages = messages;
-        result.had_errors = had_errors;
-        if had_errors {
-            return Ok(result);
-        }
         for resource in get_resource_invocation_order(&config, &mut self.statement_parser, &self.context)? {
             let properties = self.invoke_property_expressions(&resource.properties)?;
             let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else {
@@ -192,13 +185,8 @@ impl Configurator {
     ///
     /// This function will return an error if the underlying resource fails.
     pub fn invoke_set(&mut self, skip_test: bool, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result<ConfigurationSetResult, DscError> {
-        let (config, messages, had_errors) = self.validate_config()?;
+        let config = self.validate_config()?;
         let mut result = ConfigurationSetResult::new();
-        result.messages = messages;
-        result.had_errors = had_errors;
-        if had_errors {
-            return Ok(result);
-        }
         for resource in get_resource_invocation_order(&config, &mut self.statement_parser, &self.context)? {
             let properties = self.invoke_property_expressions(&resource.properties)?;
             let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else {
@@ -229,13 +217,8 @@ impl Configurator {
     ///
     /// This function will return an error if the underlying resource fails.
     pub fn invoke_test(&mut self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result<ConfigurationTestResult, DscError> {
-        let (config, messages, had_errors) = self.validate_config()?;
+        let config = self.validate_config()?;
         let mut result = ConfigurationTestResult::new();
-        result.messages = messages;
-        result.had_errors = had_errors;
-        if had_errors {
-            return Ok(result);
-        }
         for resource in get_resource_invocation_order(&config, &mut self.statement_parser, &self.context)? {
             let properties = self.invoke_property_expressions(&resource.properties)?;
             let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else {
@@ -270,7 +253,7 @@ impl Configurator {
     ///
     /// This function will return an error if the underlying resource fails.
     pub fn invoke_export(&mut self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result<ConfigurationExportResult, DscError> {
-        let (config, messages, had_errors) = self.validate_config()?;
+        let config = self.validate_config()?;
 
         let duplicates = Self::find_duplicate_resource_types(&config);
         if !duplicates.is_empty()
@@ -279,15 +262,7 @@ impl Configurator {
             return Err(DscError::Validation(format!("Resource(s) {duplicates_string} specified multiple times")));
         }
 
-        let mut result = ConfigurationExportResult {
-            result: None,
-            messages,
-            had_errors
-        };
-
-        if had_errors {
-            return Ok(result);
-        };
+        let mut result = ConfigurationExportResult::new();
         let mut conf = config_doc::Configuration::new();
 
         for resource in &config.resources {
@@ -403,76 +378,15 @@ impl Configurator {
         result.into_iter().collect()
     }
 
-    fn validate_config(&mut self) -> Result<(Configuration, Vec<ResourceMessage>, bool), DscError> {
+    fn validate_config(&mut self) -> Result<Configuration, DscError> {
         let config: Configuration = serde_json::from_str(self.config.as_str())?;
-        let mut messages: Vec<ResourceMessage> = Vec::new();
-        let mut has_errors = false;
 
         // Perform discovery of resources used in config
         let mut required_resources = config.resources.iter().map(|p| p.resource_type.to_lowercase()).collect::<Vec<String>>();
         required_resources.sort_unstable();
         required_resources.dedup();
         self.discovery.discover_resources(&required_resources);
-
-        // Now perform the validation
-        for resource in &config.resources {
-            let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else {
-                return Err(DscError::ResourceNotFound(resource.resource_type.clone()));
-            };
-
-            debug!("resource_type {}", &resource.resource_type);
-            //TODO: remove this after schema validation for classic PS resources is implemented
-            if (resource.resource_type == "DSC/PowerShellGroup")
-            || (resource.resource_type == "DSC/WMIGroup") {continue;}
-
-            let input = serde_json::to_string(&resource.properties)?;
-            let schema = match dsc_resource.schema() {
-                Ok(schema) => schema,
-                Err(DscError::SchemaNotAvailable(_) ) => {
-                    messages.push(ResourceMessage {
-                        name: resource.name.clone(),
-                        resource_type: resource.resource_type.clone(),
-                        message: "Schema not available".to_string(),
-                        level: MessageLevel::Warning,
-                    });
-                    continue;
-                },
-                Err(e) => {
-                    return Err(e);
-                },
-            };
-            let schema = serde_json::from_str(&schema)?;
-            let compiled_schema = match JSONSchema::compile(&schema) {
-                Ok(schema) => schema,
-                Err(e) => {
-                    messages.push(ResourceMessage {
-                        name: resource.name.clone(),
-                        resource_type: resource.resource_type.clone(),
-                        message: format!("Failed to compile schema: {e}"),
-                        level: MessageLevel::Error,
-                    });
-                    has_errors = true;
-                    continue;
-                },
-            };
-            let input = serde_json::from_str(&input)?;
-            if let Err(err) = compiled_schema.validate(&input) {
-                let mut error = format!("Resource '{}' failed validation: ", resource.name);
-                for e in err {
-                    error.push_str(&format!("\n{e} "));
-                }
-                messages.push(ResourceMessage {
-                    name: resource.name.clone(),
-                    resource_type: resource.resource_type.clone(),
-                    message: error,
-                    level: MessageLevel::Error,
-                });
-                has_errors = true;
-                continue;
-            };
-        }
-
-        Ok((config, messages, has_errors))
+        Ok(config)
     }
 
     fn invoke_property_expressions(&mut self, properties: &Option<Map<String, Value>>) -> Result<Option<Map<String, Value>>, DscError> {

dsc_lib/src/dscresources/dscresource.rs

@@ -60,6 +60,47 @@ impl DscResource {
             manifest: None,
         }
     }
+
+    fn validate_input(&self, input: &str) -> Result<(), DscError> {
+        if input.is_empty() {
+            return Ok(());
+        }
+        let Some(manifest) = &self.manifest else {
+            return Err(DscError::MissingManifest(self.type_name.clone()));
+        };
+        let resource_manifest = import_manifest(manifest.clone())?;
+
+        if resource_manifest.validate.is_some() {
+            let Ok(validation_result) = self.validate(input) else {
+                return Err(DscError::Validation("Validation invocation failed".to_string()));
+            };
+            if !validation_result.valid {
+                return Err(DscError::Validation("Validation failed".to_string()));
+            }
+        }
+        else {
+            let Ok(schema) = self.schema() else {
+                return Err(DscError::Validation("Schema not available".to_string()));
+            };
+
+            let schema = serde_json::from_str::<Value>(&schema)?;
+
+            let Ok(compiled_schema) = jsonschema::JSONSchema::compile(&schema) else {
+                return Err(DscError::Validation("Schema compilation failed".to_string()));
+            };
+
+            let input = serde_json::from_str::<Value>(input)?;
+            if let Err(err) = compiled_schema.validate(&input) {
+                let mut error = format!("Resource '{}' failed validation: ", self.type_name);
+                for e in err {
+                    error.push_str(&format!("\n{e} "));
+                }
+                return Err(DscError::Validation(error));
+            };
+        }
+
+        Ok(())
+    }
 }
 
 impl Default for DscResource {
@@ -132,6 +173,7 @@ pub trait Invoke {
 
 impl Invoke for DscResource {
     fn get(&self, filter: &str) -> Result<GetResult, DscError> {
+        self.validate_input(filter)?;
         match &self.implemented_as {
             ImplementedAs::Custom(_custom) => {
                 Err(DscError::NotImplemented("get custom resources".to_string()))
@@ -147,6 +189,7 @@ impl Invoke for DscResource {
     }
 
     fn set(&self, desired: &str, skip_test: bool) -> Result<SetResult, DscError> {
+        self.validate_input(desired)?;
         match &self.implemented_as {
             ImplementedAs::Custom(_custom) => {
                 Err(DscError::NotImplemented("set custom resources".to_string()))
@@ -162,6 +205,7 @@ impl Invoke for DscResource {
     }
 
     fn test(&self, expected: &str) -> Result<TestResult, DscError> {
+        self.validate_input(expected)?;
         match &self.implemented_as {
             ImplementedAs::Custom(_custom) => {
                 Err(DscError::NotImplemented("test custom resources".to_string()))

dsc_lib/src/functions/parameters.rs

@@ -28,6 +28,14 @@ impl Function for Parameters {
         };
         debug!("parameters key: {key}");
         if context.parameters.contains_key(key) {
+            let value = &context.parameters[key];
+            // we have to check if it's a string as a to_string() will put the string in quotes as part of the value
+            if value.is_string() {
+                if let Some(value) = value.as_str() {
+                    return Ok(FunctionResult::String(value.to_string()));
+                }
+            }
+
             Ok(FunctionResult::Object(context.parameters[key].clone()))
         }
         else {

osinfo/osinfo.dsc.resource.json

@@ -66,7 +66,7 @@
                     "type": "string",
                     "enum": [
                         "Linux",
-                        "MacOS",
+                        "macOS",
                         "Windows"
                     ],
                     "title": "Operating system family",

osinfo/src/config.rs

@@ -41,6 +41,7 @@ pub enum Bitness {
 #[derive(Debug, Clone, PartialEq, Serialize)]
 pub enum Family {
     Linux,
+    #[serde(rename = "macOS")]
     MacOS,
     Windows,
 }

osinfo/tests/osinfo.tests.ps1

@@ -12,7 +12,7 @@ Describe 'osinfo resource tests' {
             $out.actualState.family | Should -BeExactly 'Linux'
         }
         elseif ($IsMacOS) {
-            $out.actualState.family | Should -BeExactly 'MacOS'
+            $out.actualState.family | Should -BeExactly 'macOS'
         }
 
         $out.actualState.version | Should -Not -BeNullOrEmpty
@@ -50,7 +50,7 @@ Describe 'osinfo resource tests' {
             $out.resources[0].properties.family | Should -BeExactly 'Linux'
         }
         elseif ($IsMacOS) {
-            $out.resources[0].properties.family | Should -BeExactly 'MacOS'
+            $out.resources[0].properties.family | Should -BeExactly 'macOS'
         }
     }
 }

powershellgroup/powershellgroup.dsc.resource.json

@@ -55,6 +55,16 @@
       "input": "stdin",
       "return": "state"
       },
+    "validate": {
+        "executable": "pwsh",
+        "args": [
+          "-NoLogo",
+          "-NonInteractive",
+          "-NoProfile",
+          "-Command",
+          "$Input | ./powershellgroup.resource.ps1 Validate"
+          ]
+      },  
     "exitCodes": {
       "0": "Success",
       "1": "Error"