Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parameters to work with property values #294

Merged
merged 10 commits into from
Jan 17, 2024
2 changes: 1 addition & 1 deletion dsc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 1 addition & 6 deletions dsc/tests/dsc_config_get.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}
44 changes: 41 additions & 3 deletions dsc/tests/dsc_parameters.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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"]' }
Expand Down Expand Up @@ -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
}
}
2 changes: 1 addition & 1 deletion dsc_lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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="*"
102 changes: 8 additions & 94 deletions dsc_lib/src/configure/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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 {
Expand Down Expand Up @@ -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> {
Expand Down
44 changes: 44 additions & 0 deletions dsc_lib/src/dscresources/dscresource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()))
Expand All @@ -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()))
Expand All @@ -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()))
Expand Down
8 changes: 8 additions & 0 deletions dsc_lib/src/functions/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion osinfo/osinfo.dsc.resource.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"type": "string",
"enum": [
"Linux",
"MacOS",
"macOS",
"Windows"
],
"title": "Operating system family",
Expand Down
1 change: 1 addition & 0 deletions osinfo/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub enum Bitness {
#[derive(Debug, Clone, PartialEq, Serialize)]
pub enum Family {
Linux,
#[serde(rename = "macOS")]
MacOS,
Windows,
}
Expand Down
4 changes: 2 additions & 2 deletions osinfo/tests/osinfo.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
}
}
}
10 changes: 10 additions & 0 deletions powershellgroup/powershellgroup.dsc.resource.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading