diff --git a/resources/environment_variable/environment_variable.dsc.manifests.json b/resources/environment_variable/environment_variable.dsc.manifests.json index f611832e8..36f695425 100644 --- a/resources/environment_variable/environment_variable.dsc.manifests.json +++ b/resources/environment_variable/environment_variable.dsc.manifests.json @@ -3,12 +3,12 @@ { "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", "type": "Microsoft.Windows/EnvironmentVariable", - "description": "Manage a user or machine environment variable stored in the Windows registry.", + "description": "Manage a scalar user or machine environment variable stored in the Windows registry.", "tags": [ "Windows", "Environment" ], - "version": "0.1.0", + "version": "0.2.0", "get": { "executable": "environment_variable", "args": [ @@ -54,33 +54,12 @@ "embedded": { "$schema": "https://json-schema.org/draft/2020-12/schema", "title": "Windows Environment Variable", - "description": "Manage a user or machine environment variable stored in the Windows registry.", + "description": "Manage a scalar user or machine environment variable stored in the Windows registry.", "type": "object", "additionalProperties": false, "required": [ "name" ], - "dependentSchemas": { - "value": { - "not": { - "required": [ - "pathValue" - ] - } - }, - "pathValue": { - "not": { - "required": [ - "value" - ] - } - } - }, - "dependentRequired": { - "pathAction": [ - "pathValue" - ] - }, "properties": { "scope": { "type": "string", @@ -103,20 +82,124 @@ "title": "Value", "description": "The environment variable value." }, - "pathValue": { + "_exist": { + "type": "boolean", + "title": "Exists", + "description": "Whether the environment variable should exist. Set to false to remove it.", + "default": true + }, + "_inDesiredState": { + "type": "boolean", + "title": "In desired state", + "description": "Whether the environment variable is in the desired state. Returned only by the test operation.", + "readOnly": true + } + } + } + } + }, + { + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", + "type": "Microsoft.Windows/EnvironmentPathVariable", + "description": "Manage a delimiter-separated user or machine environment variable as an ordered list.", + "tags": [ + "Windows", + "Environment", + "Path" + ], + "version": "0.1.0", + "get": { + "executable": "environment_variable", + "args": [ + "get", + "--path", + { + "jsonInputArg": "--input", + "mandatory": true + } + ] + }, + "set": { + "executable": "environment_variable", + "args": [ + "set", + "--path", + { + "jsonInputArg": "--input", + "mandatory": true + } + ], + "implementsPretest": false, + "handlesExist": true, + "return": "state" + }, + "test": { + "executable": "environment_variable", + "args": [ + "test", + "--path", + { + "jsonInputArg": "--input", + "mandatory": true + } + ], + "return": "state" + }, + "exitCodes": { + "0": "Success", + "1": "Invalid arguments", + "2": "Invalid input", + "3": "Environment variable resource error", + "4": "Elevation required: Setting or removing AllUsers environment variables requires an elevated process" + }, + "schema": { + "embedded": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "Windows Environment Path Variable", + "description": "Manage a delimiter-separated user or machine environment variable as an ordered list.", + "type": "object", + "additionalProperties": false, + "required": [ + "name" + ], + "properties": { + "scope": { + "type": "string", + "title": "Scope", + "description": "The registry scope for the environment variable.", + "default": "currentUser", + "enum": [ + "allUsers", + "currentUser" + ] + }, + "name": { + "type": "string", + "title": "Name", + "description": "The environment variable name.", + "minLength": 1 + }, + "value": { "type": "array", - "title": "Path value", - "description": "The semicolon-delimited environment variable value represented as path entries.", + "title": "Value", + "description": "The environment variable value represented as ordered path entries.", "items": { "type": "string", - "minLength": 1, - "pattern": "^[^;]+$" + "minLength": 1 } }, - "pathAction": { + "delimiter": { + "type": "string", + "title": "Delimiter", + "description": "The string separating entries in the stored environment variable.", + "writeOnly": true, + "default": ";", + "minLength": 1 + }, + "setAction": { "type": "string", - "title": "Path action", - "description": "How pathValue entries are combined with the current value.", + "title": "Set action", + "description": "How value entries are combined with the current value.", "writeOnly": true, "default": "clobber", "enum": [ @@ -144,12 +227,12 @@ { "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", "type": "Microsoft.Windows/EnvironmentVariableList", - "description": "Manage user and machine environment variables stored in the Windows registry.", + "description": "Manage and export user and machine environment variables stored in the Windows registry.", "tags": [ "Windows", "Environment" ], - "version": "0.1.0", + "version": "0.2.0", "get": { "executable": "environment_variable", "args": [ @@ -187,6 +270,17 @@ ], "return": "state" }, + "export": { + "executable": "environment_variable", + "args": [ + "export", + "--list", + { + "jsonInputArg": "--input" + } + ], + "supportsFiltering": true + }, "exitCodes": { "0": "Success", "1": "Invalid arguments", @@ -198,96 +292,23 @@ "embedded": { "$schema": "https://json-schema.org/draft/2020-12/schema", "title": "Windows Environment Variable List", - "description": "Manage user and machine environment variables stored in the Windows registry.", + "description": "Manage and export scalar and path environment variables.", "type": "object", "additionalProperties": false, - "required": [ - "environmentVariables" - ], "properties": { "environmentVariables": { "type": "array", "title": "Environment variables", - "description": "The environment variables to get or set.", - "minItems": 1, + "description": "The scalar and path environment variables to manage or filter.", "items": { - "type": "object", - "additionalProperties": false, - "required": [ - "name" - ], - "dependentSchemas": { - "value": { - "not": { - "required": [ - "pathValue" - ] - } + "anyOf": [ + { + "$ref": "#/$defs/scalar" }, - "pathValue": { - "not": { - "required": [ - "value" - ] - } + { + "$ref": "#/$defs/path" } - }, - "dependentRequired": { - "pathAction": [ - "pathValue" - ] - }, - "properties": { - "scope": { - "type": "string", - "title": "Scope", - "description": "The registry scope for the environment variable.", - "default": "currentUser", - "enum": [ - "allUsers", - "currentUser" - ] - }, - "name": { - "type": "string", - "title": "Name", - "description": "The environment variable name.", - "minLength": 1 - }, - "value": { - "type": "string", - "title": "Value", - "description": "The environment variable value." - }, - "pathValue": { - "type": "array", - "title": "Path value", - "description": "The semicolon-delimited environment variable value represented as path entries.", - "items": { - "type": "string", - "minLength": 1, - "pattern": "^[^;]+$" - } - }, - "pathAction": { - "type": "string", - "title": "Path action", - "description": "How pathValue entries are combined with the current value.", - "writeOnly": true, - "default": "clobber", - "enum": [ - "prepend", - "append", - "clobber" - ] - }, - "_exist": { - "type": "boolean", - "title": "Exists", - "description": "Whether the environment variable should exist. Set to false to remove it.", - "default": true - } - } + ] } }, "_inDesiredState": { @@ -296,6 +317,98 @@ "description": "Whether all environment variables are in the desired state. Returned only by the test operation.", "readOnly": true } + }, + "$defs": { + "commonProperties": { + "scope": { + "type": "string", + "title": "Scope", + "description": "The registry scope for the environment variable.", + "default": "currentUser", + "enum": [ + "allUsers", + "currentUser" + ] + }, + "name": { + "type": "string", + "title": "Name", + "description": "The environment variable name. Export filters support the asterisk wildcard.", + "minLength": 1 + }, + "_exist": { + "type": "boolean", + "title": "Exists", + "description": "Whether the environment variable should exist.", + "default": true + } + }, + "scalar": { + "type": "object", + "additionalProperties": false, + "required": [ + "name" + ], + "properties": { + "scope": { + "$ref": "#/$defs/commonProperties/scope" + }, + "name": { + "$ref": "#/$defs/commonProperties/name" + }, + "value": { + "type": "string", + "title": "Value" + }, + "_exist": { + "$ref": "#/$defs/commonProperties/_exist" + } + } + }, + "path": { + "type": "object", + "additionalProperties": false, + "required": [ + "name" + ], + "properties": { + "scope": { + "$ref": "#/$defs/commonProperties/scope" + }, + "name": { + "$ref": "#/$defs/commonProperties/name" + }, + "value": { + "type": "array", + "title": "Value", + "items": { + "type": "string", + "minLength": 1 + } + }, + "delimiter": { + "type": "string", + "title": "Delimiter", + "writeOnly": true, + "default": ";", + "minLength": 1 + }, + "setAction": { + "type": "string", + "title": "Set action", + "writeOnly": true, + "default": "clobber", + "enum": [ + "prepend", + "append", + "clobber" + ] + }, + "_exist": { + "$ref": "#/$defs/commonProperties/_exist" + } + } + } } } } diff --git a/resources/environment_variable/locales/en-us.toml b/resources/environment_variable/locales/en-us.toml index 557bfa321..bd5122703 100644 --- a/resources/environment_variable/locales/en-us.toml +++ b/resources/environment_variable/locales/en-us.toml @@ -1,8 +1,8 @@ _version = 1 [main] -missingOperation = "Missing operation. Usage: environment_variable get --input | set --input | test --input " -unknownOperation = "Unknown operation: '%{operation}'. Expected: get, set, or test" +missingOperation = "Missing operation. Usage: environment_variable get|set|test|export [--path|--list] --input " +unknownOperation = "Unknown operation: '%{operation}'. Expected: get, set, test, or export" missingInput = "Missing --input argument" missingInputValue = "Missing value for --input argument" invalidJson = "Invalid JSON input: %{error}" @@ -16,10 +16,9 @@ emptyList = "The environmentVariables array must contain at least one environmen emptyName = "Environment variable name must not be empty" invalidName = "Environment variable name '%{name}' contains an invalid null character" duplicate = "Environment variable '%{name}' is specified more than once in scope '%{scope}'" -valueConflict = "Environment variable '%{name}' cannot specify both value and pathValue" -pathActionWithoutValue = "Environment variable '%{name}' can only specify pathAction with pathValue" -invalidPathEntry = "Environment variable '%{name}' has a pathValue entry that is empty or contains a semicolon or null character" -missingValue = "Environment variable '%{name}' must specify value or pathValue when _exist is true" +invalidDelimiter = "Environment path variable '%{name}' must specify a non-empty delimiter without null characters" +invalidPathEntry = "Environment path variable '%{name}' has a value entry that is empty or contains the delimiter '%{delimiter}' or a null character" +missingValue = "Environment variable '%{name}' must specify value when _exist is true" [get] readError = "Failed to read environment variable '%{name}' in scope '%{scope}': %{error}" @@ -30,3 +29,7 @@ elevationRequired = "Setting or removing AllUsers environment variables requires readError = "Failed to read environment variable '%{name}' in scope '%{scope}' before setting it: %{error}" writeError = "Failed to set environment variable '%{name}' in scope '%{scope}': %{error}" removeError = "Failed to remove environment variable '%{name}' in scope '%{scope}': %{error}" + +[export] +registryError = "Failed to open the environment variables in scope '%{scope}': %{error}" +readError = "Failed to enumerate the environment variables in scope '%{scope}': %{error}" diff --git a/resources/environment_variable/src/environment.rs b/resources/environment_variable/src/environment.rs index cc32995c6..826fc27ee 100644 --- a/resources/environment_variable/src/environment.rs +++ b/resources/environment_variable/src/environment.rs @@ -1,7 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::types::{EnvironmentVariable, EnvironmentVariableList, PathAction, Scope}; +use crate::types::{ + EnvironmentPathVariable, EnvironmentPathVariableFilter, EnvironmentVariable, + EnvironmentVariableFilter, EnvironmentVariableFilterItem, EnvironmentVariableFilterList, + EnvironmentVariableItem, EnvironmentVariableList, Scope, SetAction, +}; use dsc_lib_registry::{RegistryHelper, config::RegistryValueData}; use dsc_lib_security_context::{SecurityContext, get_security_context}; use rust_i18n::t; @@ -18,7 +22,6 @@ pub enum EnvironmentError { #[derive(Clone, Copy)] enum OperationError { - Registry, GetRead, SetRead, SetWrite, @@ -64,11 +67,11 @@ pub fn test_variables( let mut environment_variables = Vec::with_capacity(input.environment_variables.len()); for variable in &input.environment_variables { - let state = registry_helper(variable, None)? + let state = registry_helper(variable.scope(), variable.name(), None)? .get() .map_err(|error| operation_error(OperationError::GetRead, variable, &error))?; let exists = state.exist != Some(false); - let should_exist = variable.exist.unwrap_or(true); + let should_exist = variable.exist().unwrap_or(true); if exists != should_exist { in_desired_state = false; @@ -79,7 +82,6 @@ pub fn test_variables( in_desired_state = false; } } - environment_variables.push(get_variable(variable)?); } @@ -95,7 +97,7 @@ pub fn set_variables( if input .environment_variables .iter() - .any(|variable| variable.scope == Scope::AllUsers) + .any(|variable| variable.scope() == Scope::AllUsers) && get_security_context() != SecurityContext::Admin { return Err(EnvironmentError::ElevationRequired); @@ -103,19 +105,12 @@ pub fn set_variables( let mut environment_variables = Vec::with_capacity(input.environment_variables.len()); for variable in &input.environment_variables { - let helper = registry_helper(variable, None)?; - if !variable.exist.unwrap_or(true) { + let helper = registry_helper(variable.scope(), variable.name(), None)?; + if !variable.exist().unwrap_or(true) { helper .remove() .map_err(|error| operation_error(OperationError::SetRemove, variable, &error))?; - environment_variables.push(EnvironmentVariable { - scope: variable.scope, - name: variable.name.clone(), - value: None, - path_value: None, - path_action: None, - exist: Some(false), - }); + environment_variables.push(missing_variable(variable)); continue; } @@ -125,9 +120,13 @@ pub fn set_variables( .value_data; let desired_value = desired_value(variable, current_data.as_ref()); let value_data = registry_data(&desired_value, current_data.as_ref()); - registry_helper(variable, Some(value_data))? - .set() - .map_err(|error| operation_error(OperationError::SetWrite, variable, &error))?; + registry_helper( + variable.scope(), + variable.name(), + Some(value_data), + )? + .set() + .map_err(|error| operation_error(OperationError::SetWrite, variable, &error))?; environment_variables.push(get_variable(variable)?); } @@ -137,63 +136,254 @@ pub fn set_variables( }) } -fn get_variable(variable: &EnvironmentVariable) -> Result { - let state = registry_helper(variable, None)? - .get() - .map_err(|error| operation_error(OperationError::GetRead, variable, &error))?; +pub fn export_variables( + input: &EnvironmentVariableFilterList, +) -> Result { + let mut environment_variables = Vec::new(); - if state.exist == Some(false) { - return Ok(EnvironmentVariable { - scope: variable.scope, - name: variable.name.clone(), - value: None, - path_value: None, - path_action: None, - exist: Some(false), - }); - } - - let value = match state.value_data { - Some(RegistryValueData::String(value) | RegistryValueData::ExpandString(value)) => value, - Some(_) => { - return Err(EnvironmentError::Resource( + for scope in [Scope::CurrentUser, Scope::AllUsers] { + let helper = RegistryHelper::new(key_path(scope), None, None).map_err(|error| { + EnvironmentError::Resource( + t!( + "export.registryError", + scope = scope.to_string(), + error = error.to_string() + ) + .to_string(), + ) + })?; + let values = helper.get_values().map_err(|error| { + EnvironmentError::Resource( t!( - "get.unsupportedType", - name = variable.name.as_str(), - scope = variable.scope.to_string() + "export.readError", + scope = scope.to_string(), + error = error.to_string() ) .to_string(), - )); + ) + })?; + + for (name, data) in values { + if name.is_empty() { + continue; + } + let value = match data { + RegistryValueData::String(value) | RegistryValueData::ExpandString(value) => value, + _ => continue, + }; + + if input.environment_variables.is_empty() { + environment_variables.push(EnvironmentVariableItem::Scalar( + exported_scalar(scope, name, value), + )); + continue; + } + + let matching_filter = input + .environment_variables + .iter() + .filter(|filter| filter_matches(filter, scope, &name, &value)) + .find(|filter| matches!(filter, EnvironmentVariableFilterItem::Path(_))) + .or_else(|| { + input + .environment_variables + .iter() + .find(|filter| filter_matches(filter, scope, &name, &value)) + }); + + match matching_filter { + Some(EnvironmentVariableFilterItem::Path(filter)) => { + let delimiter = filter.delimiter.as_deref().unwrap_or(";"); + environment_variables.push(EnvironmentVariableItem::Path( + EnvironmentPathVariable { + scope, + name, + value: Some(split_path(&value, delimiter)), + delimiter: delimiter.to_string(), + set_action: SetAction::Clobber, + exist: Some(true), + }, + )); + } + Some(EnvironmentVariableFilterItem::Scalar(_)) => { + environment_variables.push(EnvironmentVariableItem::Scalar( + exported_scalar(scope, name, value), + )); + } + None => {} + } } - None => String::new(), - }; + } - let (value, path_value) = if variable.path_value.is_some() { - (None, Some(split_path(&value))) + Ok(EnvironmentVariableList { + environment_variables, + in_desired_state: None, + }) +} + +fn exported_scalar(scope: Scope, name: String, value: String) -> EnvironmentVariable { + EnvironmentVariable { + scope, + name, + value: Some(value), + exist: Some(true), + } +} + +fn filter_matches( + filter: &EnvironmentVariableFilterItem, + scope: Scope, + name: &str, + value: &str, +) -> bool { + match filter { + EnvironmentVariableFilterItem::Scalar(filter) => { + scalar_filter_matches(filter, scope, name, value) + } + EnvironmentVariableFilterItem::Path(filter) => { + path_filter_matches(filter, scope, name, value) + } + } +} + +fn scalar_filter_matches( + filter: &EnvironmentVariableFilter, + scope: Scope, + name: &str, + value: &str, +) -> bool { + filter.scope.is_none_or(|expected| expected == scope) + && filter + .name + .as_deref() + .is_none_or(|pattern| matches_wildcard(name, pattern)) + && filter.value.as_deref().is_none_or(|expected| expected == value) + && filter.exist.is_none_or(|expected| expected) +} + +fn path_filter_matches( + filter: &EnvironmentPathVariableFilter, + scope: Scope, + name: &str, + value: &str, +) -> bool { + let delimiter = filter.delimiter.as_deref().unwrap_or(";"); + filter.scope.is_none_or(|expected| expected == scope) + && filter + .name + .as_deref() + .is_none_or(|pattern| matches_wildcard(name, pattern)) + && filter.value.as_deref().is_none_or(|expected| { + expected.is_empty() || path_values_equal(&split_path(value, delimiter), expected) + }) + && filter.exist.is_none_or(|expected| expected) +} + +fn matches_wildcard(text: &str, pattern: &str) -> bool { + let text = text.to_lowercase(); + let pattern = pattern.to_lowercase(); + if !pattern.contains('*') { + return text == pattern; + } + + let parts = pattern.split('*').collect::>(); + if !parts[0].is_empty() && !text.starts_with(parts[0]) { + return false; + } + let mut position = parts[0].len(); + let suffix = parts.last().copied().unwrap_or_default(); + let end = if suffix.is_empty() { + text.len() } else { - (Some(value), None) + if !text.ends_with(suffix) { + return false; + } + text.len() - suffix.len() }; + for part in &parts[1..parts.len().saturating_sub(1)] { + if part.is_empty() { + continue; + } + let Some(index) = text.get(position..end).and_then(|text| text.find(part)) else { + return false; + }; + position += index + part.len(); + } + position <= end +} - Ok(EnvironmentVariable { - scope: variable.scope, - name: variable.name.clone(), - value, - path_value, - path_action: None, - exist: Some(true), +fn get_variable( + variable: &EnvironmentVariableItem, +) -> Result { + let state = registry_helper(variable.scope(), variable.name(), None)? + .get() + .map_err(|error| operation_error(OperationError::GetRead, variable, &error))?; + if state.exist == Some(false) { + return Ok(missing_variable(variable)); + } + + let value = registry_string(variable, state.value_data.as_ref())?; + Ok(match variable { + EnvironmentVariableItem::Scalar(variable) => { + EnvironmentVariableItem::Scalar(EnvironmentVariable { + scope: variable.scope, + name: variable.name.clone(), + value: Some(value), + exist: Some(true), + }) + } + EnvironmentVariableItem::Path(variable) => { + EnvironmentVariableItem::Path(EnvironmentPathVariable { + scope: variable.scope, + name: variable.name.clone(), + value: Some(split_path(&value, &variable.delimiter)), + delimiter: variable.delimiter.clone(), + set_action: SetAction::Clobber, + exist: Some(true), + }) + } }) } +fn missing_variable(variable: &EnvironmentVariableItem) -> EnvironmentVariableItem { + match variable { + EnvironmentVariableItem::Scalar(variable) => { + EnvironmentVariableItem::Scalar(EnvironmentVariable { + scope: variable.scope, + name: variable.name.clone(), + value: None, + exist: Some(false), + }) + } + EnvironmentVariableItem::Path(variable) => { + EnvironmentVariableItem::Path(EnvironmentPathVariable { + scope: variable.scope, + name: variable.name.clone(), + value: None, + delimiter: variable.delimiter.clone(), + set_action: SetAction::Clobber, + exist: Some(false), + }) + } + } +} + fn registry_helper( - variable: &EnvironmentVariable, + scope: Scope, + name: &str, value_data: Option, ) -> Result { - RegistryHelper::new( - key_path(variable.scope), - Some(variable.name.clone()), - value_data, - ) - .map_err(|error| operation_error(OperationError::Registry, variable, &error)) + RegistryHelper::new(key_path(scope), Some(name.to_string()), value_data).map_err(|error| { + EnvironmentError::Resource( + t!( + "main.registryError", + name = name, + scope = scope.to_string(), + error = error.to_string() + ) + .to_string(), + ) + }) } fn key_path(scope: Scope) -> &'static str { @@ -204,22 +394,24 @@ fn key_path(scope: Scope) -> &'static str { } fn desired_value( - variable: &EnvironmentVariable, + variable: &EnvironmentVariableItem, current_data: Option<&RegistryValueData>, ) -> String { - if let Some(value) = &variable.value { - return value.clone(); - } - - let desired = variable.path_value.as_deref().unwrap_or_default(); - let existing = match current_data { - Some(RegistryValueData::String(value) | RegistryValueData::ExpandString(value)) => { - split_path(value) + match variable { + EnvironmentVariableItem::Scalar(variable) => { + variable.value.clone().unwrap_or_default() } - _ => Vec::new(), - }; - - merge_path(&existing, desired, variable.path_action.unwrap_or_default()).join(";") + EnvironmentVariableItem::Path(variable) => { + let desired = variable.value.as_deref().unwrap_or_default(); + let existing = match current_data { + Some(RegistryValueData::String(value) | RegistryValueData::ExpandString(value)) => { + split_path(value, &variable.delimiter) + } + _ => Vec::new(), + }; + merge_path(&existing, desired, variable.set_action).join(&variable.delimiter) + } + } } fn registry_data(value: &str, current_data: Option<&RegistryValueData>) -> RegistryValueData { @@ -231,7 +423,7 @@ fn registry_data(value: &str, current_data: Option<&RegistryValueData>) -> Regis } fn registry_string( - variable: &EnvironmentVariable, + variable: &EnvironmentVariableItem, current_data: Option<&RegistryValueData>, ) -> Result { match current_data { @@ -241,8 +433,8 @@ fn registry_string( Some(_) => Err(EnvironmentError::Resource( t!( "get.unsupportedType", - name = variable.name.as_str(), - scope = variable.scope.to_string() + name = variable.name(), + scope = variable.scope().to_string() ) .to_string(), )), @@ -251,35 +443,46 @@ fn registry_string( } fn value_in_desired_state( - variable: &EnvironmentVariable, + variable: &EnvironmentVariableItem, current_value: &str, current_data: Option<&RegistryValueData>, ) -> bool { - if let Some(value) = &variable.value { - return current_value == value; + match variable { + EnvironmentVariableItem::Scalar(variable) => variable + .value + .as_deref() + .is_some_and(|value| current_value == value), + EnvironmentVariableItem::Path(variable) => { + let projected = desired_value( + &EnvironmentVariableItem::Path(variable.clone()), + current_data, + ); + path_values_equal( + &split_path(current_value, &variable.delimiter), + &split_path(&projected, &variable.delimiter), + ) + } } +} - let projected = desired_value(variable, current_data); - split_path(current_value) - .iter() +fn path_values_equal(left: &[String], right: &[String]) -> bool { + left.iter() .map(|entry| entry.to_lowercase()) - .eq(split_path(&projected) - .iter() - .map(|entry| entry.to_lowercase())) + .eq(right.iter().map(|entry| entry.to_lowercase())) } -fn split_path(value: &str) -> Vec { +fn split_path(value: &str, delimiter: &str) -> Vec { value - .split(';') + .split(delimiter) .filter(|entry| !entry.is_empty()) .map(str::to_string) .collect() } -fn merge_path(existing: &[String], desired: &[String], action: PathAction) -> Vec { +fn merge_path(existing: &[String], desired: &[String], action: SetAction) -> Vec { let mut values = match action { - PathAction::Prepend => desired.iter().chain(existing).cloned().collect::>(), - PathAction::Append => { + SetAction::Prepend => desired.iter().chain(existing).cloned().collect::>(), + SetAction::Append => { let desired_keys = desired .iter() .map(|entry| entry.to_lowercase()) @@ -291,7 +494,7 @@ fn merge_path(existing: &[String], desired: &[String], action: PathAction) -> Ve .cloned() .collect::>() } - PathAction::Clobber => desired.to_vec(), + SetAction::Clobber => desired.to_vec(), }; let mut seen = HashSet::new(); @@ -301,19 +504,13 @@ fn merge_path(existing: &[String], desired: &[String], action: PathAction) -> Ve fn operation_error( operation: OperationError, - variable: &EnvironmentVariable, + variable: &EnvironmentVariableItem, error: &impl std::fmt::Display, ) -> EnvironmentError { - let name = variable.name.as_str(); - let scope = variable.scope.to_string(); + let name = variable.name(); + let scope = variable.scope().to_string(); let error = error.to_string(); let message = match operation { - OperationError::Registry => t!( - "main.registryError", - name = name, - scope = scope, - error = error - ), OperationError::GetRead => t!("get.readError", name = name, scope = scope, error = error), OperationError::SetRead => t!("set.readError", name = name, scope = scope, error = error), OperationError::SetWrite => t!("set.writeError", name = name, scope = scope, error = error), @@ -327,12 +524,16 @@ fn operation_error( #[cfg(test)] mod tests { use super::{ - CURRENT_USER_KEY, EnvironmentError, OperationError, get_variables, key_path, merge_path, - operation_error, set_variables, split_path, test_variables, value_in_desired_state, + CURRENT_USER_KEY, EnvironmentError, OperationError, export_variables, get_variables, + key_path, matches_wildcard, merge_path, operation_error, path_filter_matches, + scalar_filter_matches, set_variables, split_path, test_variables, value_in_desired_state, }; - use crate::types::{EnvironmentVariable, EnvironmentVariableList, PathAction, Scope}; - use dsc_lib_registry::RegistryHelper; - use dsc_lib_registry::config::RegistryValueData; + use crate::types::{ + EnvironmentPathVariable, EnvironmentPathVariableFilter, EnvironmentVariable, + EnvironmentVariableFilter, EnvironmentVariableFilterItem, EnvironmentVariableFilterList, + EnvironmentVariableItem, EnvironmentVariableList, Scope, SetAction, + }; + use dsc_lib_registry::{RegistryHelper, config::RegistryValueData}; use std::sync::atomic::{AtomicUsize, Ordering}; static TEST_ID: AtomicUsize = AtomicUsize::new(0); @@ -366,56 +567,125 @@ mod tests { } } - fn variable(name: &str) -> EnvironmentVariable { - EnvironmentVariable { + fn scalar(name: &str, value: Option<&str>) -> EnvironmentVariableItem { + EnvironmentVariableItem::Scalar(EnvironmentVariable { scope: Scope::CurrentUser, name: name.to_string(), - value: Some("expected".to_string()), - path_value: None, - path_action: None, + value: value.map(str::to_string), exist: None, - } + }) + } + + fn path(name: &str, values: &[&str], action: SetAction) -> EnvironmentVariableItem { + EnvironmentVariableItem::Path(EnvironmentPathVariable { + scope: Scope::CurrentUser, + name: name.to_string(), + value: Some(values.iter().map(|value| (*value).to_string()).collect()), + delimiter: ";".to_string(), + set_action: action, + exist: None, + }) } - fn list(variable: EnvironmentVariable) -> EnvironmentVariableList { + fn list(variable: EnvironmentVariableItem) -> EnvironmentVariableList { EnvironmentVariableList { environment_variables: vec![variable], in_desired_state: None, } } + #[test] + fn path_helpers_use_custom_delimiter_and_deduplicate() { + assert_eq!(split_path("one::two::::three", "::"), vec!["one", "two", "three"]); + assert_eq!( + merge_path( + &["One".to_string(), "Two".to_string()], + &["one".to_string(), "Three".to_string()], + SetAction::Append, + ), + vec!["Two", "one", "Three"] + ); + } + + #[test] + fn wildcard_matching_is_case_insensitive() { + assert!(matches_wildcard("PSModulePath", "ps*path")); + assert!(matches_wildcard("Path", "*")); + assert!(!matches_wildcard("TEMP", "path*")); + } + + #[test] + fn scalar_filter_ands_specified_properties() { + let filter = EnvironmentVariableFilter { + scope: Some(Scope::CurrentUser), + name: Some("DSC_*".to_string()), + value: Some("expected".to_string()), + exist: Some(true), + }; + assert!(scalar_filter_matches( + &filter, + Scope::CurrentUser, + "dsc_test", + "expected" + )); + assert!(!scalar_filter_matches( + &filter, + Scope::CurrentUser, + "dsc_test", + "different" + )); + } + + #[test] + fn path_filter_compares_split_values() { + let filter = EnvironmentPathVariableFilter { + scope: None, + name: Some("*Path".to_string()), + value: Some(vec!["C:\\One".to_string(), "C:\\Two".to_string()]), + delimiter: None, + _set_action: None, + exist: None, + }; + assert!(path_filter_matches( + &filter, + Scope::AllUsers, + "Path", + "c:\\one;C:\\TWO" + )); + } + #[test] fn registry_operations_round_trip_scalar_and_removal() { let guard = RegistryValueGuard::new(); - let input = list(variable(&guard.name)); + let input = list(scalar(&guard.name, Some("expected"))); let set = set_variables(&input).unwrap(); - assert_eq!( - set.environment_variables[0].value.as_deref(), - Some("expected") - ); + let EnvironmentVariableItem::Scalar(set) = &set.environment_variables[0] else { + panic!("expected scalar state"); + }; + assert_eq!(set.value.as_deref(), Some("expected")); let get = get_variables(&input).unwrap(); - assert_eq!( - get.environment_variables[0].value.as_deref(), - Some("expected") - ); - - let test = test_variables(&input).unwrap(); - assert_eq!(test.in_desired_state, Some(true)); + let EnvironmentVariableItem::Scalar(get) = &get.environment_variables[0] else { + panic!("expected scalar state"); + }; + assert_eq!(get.value.as_deref(), Some("expected")); + assert_eq!(test_variables(&input).unwrap().in_desired_state, Some(true)); - let mut different = variable(&guard.name); - different.value = Some("different".to_string()); assert_eq!( - test_variables(&list(different)).unwrap().in_desired_state, + test_variables(&list(scalar(&guard.name, Some("different")))) + .unwrap() + .in_desired_state, Some(false) ); - let mut remove = variable(&guard.name); - remove.value = None; - remove.exist = Some(false); + let mut remove = scalar(&guard.name, None); + let EnvironmentVariableItem::Scalar(variable) = &mut remove else { + unreachable!(); + }; + variable.exist = Some(false); let removed = set_variables(&list(remove.clone())).unwrap(); - assert_eq!(removed.environment_variables[0].exist, Some(false)); + assert_eq!(removed.environment_variables[0].exist(), Some(false)); assert_eq!( test_variables(&list(remove)).unwrap().in_desired_state, Some(true) @@ -428,18 +698,14 @@ mod tests { guard.set(RegistryValueData::ExpandString( r"%SystemRoot%\Existing".to_string(), )); - let input = list(EnvironmentVariable { - scope: Scope::CurrentUser, - name: guard.name.clone(), - value: None, - path_value: Some(vec![r"C:\New".to_string()]), - path_action: Some(PathAction::Append), - exist: None, - }); + let input = list(path(&guard.name, &[r"C:\New"], SetAction::Append)); let set = set_variables(&input).unwrap(); + let EnvironmentVariableItem::Path(set) = &set.environment_variables[0] else { + panic!("expected path state"); + }; assert_eq!( - set.environment_variables[0].path_value.as_deref(), + set.value.as_deref(), Some([r"%SystemRoot%\Existing".to_string(), r"C:\New".to_string()].as_slice()) ); assert_eq!(test_variables(&input).unwrap().in_desired_state, Some(true)); @@ -454,35 +720,81 @@ mod tests { )); } + #[test] + fn exports_matching_registry_value_as_requested_shape() { + let guard = RegistryValueGuard::new(); + guard.set(RegistryValueData::String("one::two".to_string())); + let filters = EnvironmentVariableFilterList { + environment_variables: vec![ + EnvironmentVariableFilterItem::Scalar(EnvironmentVariableFilter { + scope: Some(Scope::CurrentUser), + name: Some(guard.name.clone()), + value: None, + exist: None, + }), + EnvironmentVariableFilterItem::Path(EnvironmentPathVariableFilter { + scope: Some(Scope::CurrentUser), + name: Some(guard.name.clone()), + value: Some(Vec::new()), + delimiter: Some("::".to_string()), + _set_action: None, + exist: None, + }), + ], + }; + + let exported = export_variables(&filters).unwrap(); + assert_eq!(exported.environment_variables.len(), 1); + let EnvironmentVariableItem::Path(variable) = &exported.environment_variables[0] else { + panic!("path filter should control the exported shape"); + }; + assert_eq!( + variable.value.as_deref(), + Some(["one".to_string(), "two".to_string()].as_slice()) + ); + } + #[test] fn unsupported_registry_type_returns_resource_error() { let guard = RegistryValueGuard::new(); guard.set(RegistryValueData::DWord(42)); - let input = list(variable(&guard.name)); - - let get_error = get_variables(&input).unwrap_err(); - assert!(get_error.to_string().contains(&guard.name)); + let input = list(scalar(&guard.name, Some("expected"))); - let test_error = test_variables(&input).unwrap_err(); - assert!(test_error.to_string().contains(&guard.name)); + assert!( + get_variables(&input) + .unwrap_err() + .to_string() + .contains(&guard.name) + ); + assert!( + test_variables(&input) + .unwrap_err() + .to_string() + .contains(&guard.name) + ); } #[test] - fn formats_error_variants_and_scope_paths() { - let variable = variable("TestName"); - let elevation = EnvironmentError::ElevationRequired; - assert!(elevation.is_elevation_required()); - assert!(!elevation.to_string().is_empty()); - - let resource = EnvironmentError::Resource("message".to_string()); - assert!(!resource.is_elevation_required()); - assert_eq!(resource.to_string(), "message"); + fn covers_path_projection_and_error_variants() { + let prepend = path("Path", &[r"c:\shared", r"C:\New"], SetAction::Prepend); + let current = RegistryValueData::String(r"c:\shared;C:\New;C:\Existing".to_string()); + assert!(value_in_desired_state( + &prepend, + r"c:\shared;C:\New;C:\Existing", + Some(¤t) + )); + assert!(!value_in_desired_state( + &path("Path", &[r"C:\New"], SetAction::Prepend), + r"C:\Existing", + Some(&RegistryValueData::String(r"C:\Existing".to_string())) + )); + let variable = scalar("TestName", Some("value")); assert_eq!(key_path(Scope::CurrentUser), CURRENT_USER_KEY); assert!(key_path(Scope::AllUsers).starts_with("HKLM\\")); - + assert!(EnvironmentError::ElevationRequired.is_elevation_required()); + assert!(!EnvironmentError::Resource("message".to_string()).is_elevation_required()); for operation in [ - OperationError::Registry, OperationError::GetRead, OperationError::SetRead, OperationError::SetWrite, @@ -497,77 +809,24 @@ mod tests { } #[test] - fn prepends_and_deduplicates_case_insensitively() { - let existing = vec!["C:\\Existing".to_string(), "C:\\Shared".to_string()]; - let desired = vec!["c:\\shared".to_string(), "C:\\New".to_string()]; - + fn merge_path_covers_all_actions() { + let existing = vec![r"C:\Existing".to_string(), r"C:\Shared".to_string()]; + let desired = vec![r"c:\shared".to_string(), r"C:\New".to_string()]; assert_eq!( - merge_path(&existing, &desired, PathAction::Prepend), - vec!["c:\\shared", "C:\\New", "C:\\Existing"] + merge_path(&existing, &desired, SetAction::Prepend), + vec![r"c:\shared", r"C:\New", r"C:\Existing"] ); - } - - #[test] - fn appends_entries_at_the_end() { - let existing = vec!["C:\\Shared".to_string(), "C:\\Existing".to_string()]; - let desired = vec!["c:\\shared".to_string(), "C:\\New".to_string()]; - assert_eq!( - merge_path(&existing, &desired, PathAction::Append), - vec!["C:\\Existing", "c:\\shared", "C:\\New"] + merge_path(&existing, &desired, SetAction::Append), + vec![r"C:\Existing", r"c:\shared", r"C:\New"] ); - } - - #[test] - fn clobber_deduplicates_desired_entries() { - let desired = vec!["C:\\One".to_string(), "c:\\one".to_string()]; - assert_eq!( - merge_path(&[], &desired, PathAction::Clobber), - vec!["C:\\One"] + merge_path( + &[], + &[r"C:\One".to_string(), r"c:\one".to_string()], + SetAction::Clobber + ), + vec![r"C:\One"] ); } - - #[test] - fn splitting_omits_empty_path_segments() { - assert_eq!(split_path("C:\\One;;C:\\Two;"), vec!["C:\\One", "C:\\Two"]); - } - - #[test] - fn prepend_is_in_desired_state_after_projecting_same_value() { - let variable = EnvironmentVariable { - scope: Scope::CurrentUser, - name: "Path".to_string(), - value: None, - path_value: Some(vec!["c:\\shared".to_string(), "C:\\New".to_string()]), - path_action: Some(PathAction::Prepend), - exist: None, - }; - let current = RegistryValueData::String("c:\\shared;C:\\New;C:\\Existing".to_string()); - - assert!(value_in_desired_state( - &variable, - "c:\\shared;C:\\New;C:\\Existing", - Some(¤t) - )); - } - - #[test] - fn prepend_is_not_in_desired_state_before_projection() { - let variable = EnvironmentVariable { - scope: Scope::CurrentUser, - name: "Path".to_string(), - value: None, - path_value: Some(vec!["C:\\New".to_string()]), - path_action: Some(PathAction::Prepend), - exist: None, - }; - let current = RegistryValueData::String("C:\\Existing".to_string()); - - assert!(!value_in_desired_state( - &variable, - "C:\\Existing", - Some(¤t) - )); - } } diff --git a/resources/environment_variable/src/main.rs b/resources/environment_variable/src/main.rs index 434c9fcb8..9406cad99 100644 --- a/resources/environment_variable/src/main.rs +++ b/resources/environment_variable/src/main.rs @@ -8,7 +8,10 @@ mod environment; use rust_i18n::t; use std::process::exit; -use types::{EnvironmentVariable, EnvironmentVariableList, Operation}; +use types::{ + EnvironmentPathVariable, EnvironmentVariable, EnvironmentVariableFilterList, + EnvironmentVariableList, Operation, +}; rust_i18n::i18n!("locales", fallback = "en-us"); @@ -18,6 +21,13 @@ const EXIT_INVALID_INPUT: i32 = 2; const EXIT_RESOURCE_ERROR: i32 = 3; const EXIT_ELEVATION_REQUIRED: i32 = 4; +#[derive(Clone, Copy)] +enum InputKind { + Scalar, + Path, + List, +} + fn write_error(message: &str) { eprintln!("{}", serde_json::json!({ "error": message })); } @@ -32,57 +42,82 @@ fn print_json(value: &impl serde::Serialize) { } } -fn require_input( +fn parse_list( input_json: Option, operation: Operation, - is_list: bool, -) -> EnvironmentVariableList { - let Some(json) = input_json else { - write_error(&t!("main.missingInput")); - exit(EXIT_INVALID_ARGS); - }; + kind: InputKind, +) -> Result { + let json = + input_json.ok_or_else(|| (t!("main.missingInput").to_string(), EXIT_INVALID_ARGS))?; - let input = match if is_list { - serde_json::from_str::(&json) - } else { - serde_json::from_str::(&json).map(EnvironmentVariableList::from) - } { - Ok(value) => value, - Err(error) => { - write_error(&t!("main.invalidJson", error = error.to_string())); - exit(EXIT_INVALID_INPUT); + let input = match kind { + InputKind::List => serde_json::from_str::(&json), + InputKind::Scalar => { + serde_json::from_str::(&json).map(EnvironmentVariableList::from) } - }; - - if let Err(error) = input.validate(operation) { - write_error(&error); - exit(EXIT_INVALID_INPUT); + InputKind::Path => serde_json::from_str::(&json) + .map(EnvironmentVariableList::from), } + .map_err(|error| { + ( + t!("main.invalidJson", error = error.to_string()).to_string(), + EXIT_INVALID_INPUT, + ) + })?; input + .validate(operation) + .map_err(|error| (error, EXIT_INVALID_INPUT))?; + Ok(input) } -fn print_result(mut value: EnvironmentVariableList, is_list: bool) { - if is_list { - print_json(&value); - return; +fn parse_export_filters( + input_json: Option, +) -> Result { + let Some(json) = input_json else { + return Ok(EnvironmentVariableFilterList::default()); + }; + let input = serde_json::from_str::(&json).map_err(|error| { + ( + t!("main.invalidJson", error = error.to_string()).to_string(), + EXIT_INVALID_INPUT, + ) + })?; + input + .validate() + .map_err(|error| (error, EXIT_INVALID_INPUT))?; + Ok(input) +} + +fn serialize_result( + mut value: EnvironmentVariableList, + kind: InputKind, +) -> Result { + if matches!(kind, InputKind::List) { + return serde_json::to_value(value) + .map_err(|error| t!("main.serializeError", error = error.to_string()).to_string()); } - let Some(variable) = value.environment_variables.pop() else { - write_error(&t!("main.missingState")); - exit(EXIT_RESOURCE_ERROR); - }; - let mut output = match serde_json::to_value(variable) { - Ok(value) => value, + let variable = value + .environment_variables + .pop() + .ok_or_else(|| t!("main.missingState").to_string())?; + let mut output = serde_json::to_value(variable) + .map_err(|error| t!("main.serializeError", error = error.to_string()).to_string())?; + if let Some(in_desired_state) = value.in_desired_state { + output["_inDesiredState"] = serde_json::Value::Bool(in_desired_state); + } + Ok(output) +} + +fn print_result(value: EnvironmentVariableList, kind: InputKind) { + match serialize_result(value, kind) { + Ok(output) => print_json(&output), Err(error) => { - write_error(&t!("main.serializeError", error = error.to_string())); + write_error(&error); exit(EXIT_RESOURCE_ERROR); } - }; - if let Some(in_desired_state) = value.in_desired_state { - output["_inDesiredState"] = serde_json::Value::Bool(in_desired_state); } - print_json(&output); } #[cfg(not(windows))] @@ -101,21 +136,40 @@ fn main() { let operation = args[1].as_str(); let input_json = parse_input_arg(&args); - let is_list = args.iter().any(|arg| arg == "--list"); + let kind = if args.iter().any(|arg| arg == "--list") { + InputKind::List + } else if args.iter().any(|arg| arg == "--path") { + InputKind::Path + } else { + InputKind::Scalar + }; - let result = match operation { - "get" => environment::get_variables(&require_input(input_json, Operation::Get, is_list)), - "set" => environment::set_variables(&require_input(input_json, Operation::Set, is_list)), - "test" => environment::test_variables(&require_input(input_json, Operation::Test, is_list)), + let input = match operation { + "get" => parse_list(input_json, Operation::Get, kind) + .map(|input| environment::get_variables(&input)), + "set" => parse_list(input_json, Operation::Set, kind) + .map(|input| environment::set_variables(&input)), + "test" => parse_list(input_json, Operation::Test, kind) + .map(|input| environment::test_variables(&input)), + "export" if matches!(kind, InputKind::List) => { + parse_export_filters(input_json).map(|input| environment::export_variables(&input)) + } _ => { write_error(&t!("main.unknownOperation", operation = operation)); exit(EXIT_INVALID_ARGS); } }; + let result = match input { + Ok(result) => result, + Err((error, exit_code)) => { + write_error(&error); + exit(exit_code); + } + }; match result { Ok(value) => { - print_result(value, is_list); + print_result(value, kind); exit(EXIT_SUCCESS); } Err(error) => { @@ -143,3 +197,156 @@ fn parse_input_arg(args: &[String]) -> Option { } None } + +#[cfg(test)] +mod tests { + use super::{InputKind, parse_export_filters, parse_input_arg, parse_list, serialize_result}; + use crate::types::{EnvironmentVariableItem, Operation}; + + #[test] + fn parses_each_input_kind() { + let scalar = parse_list( + Some(r#"{"name":"Scalar","value":"text"}"#.to_string()), + Operation::Set, + InputKind::Scalar, + ) + .unwrap(); + assert!(matches!( + scalar.environment_variables[0], + EnvironmentVariableItem::Scalar(_) + )); + + let path = parse_list( + Some(r#"{"name":"Path","value":["one"],"delimiter":"::"}"#.to_string()), + Operation::Set, + InputKind::Path, + ) + .unwrap(); + assert!(matches!( + path.environment_variables[0], + EnvironmentVariableItem::Path(_) + )); + + let list = parse_list( + Some( + r#"{"environmentVariables":[{"name":"Scalar","value":"text"},{"name":"Path","value":["one"]}]}"# + .to_string(), + ), + Operation::Test, + InputKind::List, + ) + .unwrap(); + assert_eq!(list.environment_variables.len(), 2); + } + + #[test] + fn reports_missing_invalid_and_failed_validation() { + assert_eq!( + parse_list(None, Operation::Get, InputKind::Scalar) + .unwrap_err() + .1, + super::EXIT_INVALID_ARGS + ); + assert_eq!( + parse_list( + Some("{invalid".to_string()), + Operation::Get, + InputKind::Scalar + ) + .unwrap_err() + .1, + super::EXIT_INVALID_INPUT + ); + assert_eq!( + parse_list( + Some(r#"{"name":"MissingValue"}"#.to_string()), + Operation::Set, + InputKind::Scalar + ) + .unwrap_err() + .1, + super::EXIT_INVALID_INPUT + ); + } + + #[test] + fn parses_and_validates_export_filters() { + assert!( + parse_export_filters(None) + .unwrap() + .environment_variables + .is_empty() + ); + assert_eq!( + parse_export_filters(Some("{invalid".to_string())) + .unwrap_err() + .1, + super::EXIT_INVALID_INPUT + ); + assert!( + parse_export_filters(Some( + r#"{"environmentVariables":[{"name":"*Path","value":[]}]}"#.to_string() + )) + .is_ok() + ); + assert_eq!( + parse_export_filters(Some( + r#"{"environmentVariables":[{"name":"Path","delimiter":""}]}"#.to_string() + )) + .unwrap_err() + .1, + super::EXIT_INVALID_INPUT + ); + } + + #[test] + fn serializes_single_and_list_results() { + let mut scalar = parse_list( + Some(r#"{"name":"Scalar","value":"text"}"#.to_string()), + Operation::Get, + InputKind::Scalar, + ) + .unwrap(); + scalar.in_desired_state = Some(true); + let output = serialize_result(scalar, InputKind::Scalar).unwrap(); + assert_eq!(output["_inDesiredState"], true); + assert_eq!(output["value"], "text"); + + let list = parse_list( + Some(r#"{"environmentVariables":[{"name":"Scalar"}]}"#.to_string()), + Operation::Get, + InputKind::List, + ) + .unwrap(); + assert!( + serialize_result(list, InputKind::List) + .unwrap() + .get("environmentVariables") + .is_some() + ); + + assert!( + serialize_result( + crate::types::EnvironmentVariableList { + environment_variables: Vec::new(), + in_desired_state: None, + }, + InputKind::Path, + ) + .is_err() + ); + } + + #[test] + fn finds_long_and_short_input_arguments() { + assert_eq!( + parse_input_arg(&["exe".into(), "get".into(), "--input".into(), "{}".into()]), + Some("{}".to_string()) + ); + assert_eq!( + parse_input_arg(&["exe".into(), "get".into(), "-i".into(), "[]".into()]), + Some("[]".to_string()) + ); + assert_eq!(parse_input_arg(&["exe".into(), "get".into()]), None); + } +} diff --git a/resources/environment_variable/src/types.rs b/resources/environment_variable/src/types.rs index 988c2fd86..92b3eae0e 100644 --- a/resources/environment_variable/src/types.rs +++ b/resources/environment_variable/src/types.rs @@ -22,19 +22,15 @@ pub enum Scope { #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub enum PathAction { +pub enum SetAction { Prepend, Append, #[default] Clobber, } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase", deny_unknown_fields)] -pub struct EnvironmentVariableList { - pub environment_variables: Vec, - #[serde(rename = "_inDesiredState", skip_serializing_if = "Option::is_none")] - pub in_desired_state: Option, +fn default_delimiter() -> String { + ";".to_string() } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -45,18 +41,122 @@ pub struct EnvironmentVariable { pub name: String, #[serde(skip_serializing_if = "Option::is_none")] pub value: Option, + #[serde(rename = "_exist", skip_serializing_if = "Option::is_none")] + pub exist: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EnvironmentPathVariable { + #[serde(default)] + pub scope: Scope, + pub name: String, #[serde(skip_serializing_if = "Option::is_none")] - pub path_value: Option>, + pub value: Option>, + #[serde(default = "default_delimiter", skip_serializing)] + pub delimiter: String, #[serde(default, skip_serializing)] - pub path_action: Option, + pub set_action: SetAction, #[serde(rename = "_exist", skip_serializing_if = "Option::is_none")] pub exist: Option, } +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(untagged)] +pub enum EnvironmentVariableItem { + Scalar(EnvironmentVariable), + Path(EnvironmentPathVariable), +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EnvironmentVariableList { + #[serde(default)] + pub environment_variables: Vec, + #[serde(rename = "_inDesiredState", skip_serializing_if = "Option::is_none")] + pub in_desired_state: Option, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EnvironmentVariableFilter { + pub scope: Option, + pub name: Option, + pub value: Option, + #[serde(rename = "_exist")] + pub exist: Option, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EnvironmentPathVariableFilter { + pub scope: Option, + pub name: Option, + pub value: Option>, + pub delimiter: Option, + #[serde(rename = "setAction")] + pub _set_action: Option, + #[serde(rename = "_exist")] + pub exist: Option, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(untagged)] +pub enum EnvironmentVariableFilterItem { + Scalar(EnvironmentVariableFilter), + Path(EnvironmentPathVariableFilter), +} + +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EnvironmentVariableFilterList { + #[serde(default)] + pub environment_variables: Vec, +} + +impl EnvironmentVariableItem { + pub fn scope(&self) -> Scope { + match self { + Self::Scalar(variable) => variable.scope, + Self::Path(variable) => variable.scope, + } + } + + pub fn name(&self) -> &str { + match self { + Self::Scalar(variable) => &variable.name, + Self::Path(variable) => &variable.name, + } + } + + pub fn exist(&self) -> Option { + match self { + Self::Scalar(variable) => variable.exist, + Self::Path(variable) => variable.exist, + } + } + + pub fn validate(&self, operation: Operation) -> Result<(), String> { + match self { + Self::Scalar(variable) => variable.validate(operation), + Self::Path(variable) => variable.validate(operation), + } + } +} + impl From for EnvironmentVariableList { fn from(variable: EnvironmentVariable) -> Self { Self { - environment_variables: vec![variable], + environment_variables: vec![EnvironmentVariableItem::Scalar(variable)], + in_desired_state: None, + } + } +} + +impl From for EnvironmentVariableList { + fn from(variable: EnvironmentPathVariable) -> Self { + Self { + environment_variables: vec![EnvironmentVariableItem::Path(variable)], in_desired_state: None, } } @@ -71,12 +171,12 @@ impl EnvironmentVariableList { let mut identities = HashSet::new(); for variable in &self.environment_variables { variable.validate(operation)?; - let identity = (variable.scope, variable.name.to_lowercase()); + let identity = (variable.scope(), variable.name().to_lowercase()); if !identities.insert(identity) { return Err(t!( "validation.duplicate", - name = variable.name.as_str(), - scope = variable.scope.to_string() + name = variable.name(), + scope = variable.scope().to_string() ) .to_string()); } @@ -86,39 +186,110 @@ impl EnvironmentVariableList { } } -impl EnvironmentVariable { - fn validate(&self, operation: Operation) -> Result<(), String> { - if self.name.is_empty() { - return Err(t!("validation.emptyName").to_string()); - } - if self.name.contains('\0') { - return Err(t!("validation.invalidName", name = self.name.as_str()).to_string()); - } - if self.value.is_some() && self.path_value.is_some() { - return Err(t!("validation.valueConflict", name = self.name.as_str()).to_string()); +impl EnvironmentVariableFilterList { + pub fn validate(&self) -> Result<(), String> { + for filter in &self.environment_variables { + let (name, delimiter, values) = match filter { + EnvironmentVariableFilterItem::Scalar(filter) => { + (filter.name.as_deref(), None, None) + } + EnvironmentVariableFilterItem::Path(filter) => ( + filter.name.as_deref(), + filter.delimiter.as_deref(), + filter.value.as_deref(), + ), + }; + if name.is_some_and(|name| name.contains('\0')) { + return Err( + t!("validation.invalidName", name = name.unwrap_or_default()).to_string(), + ); + } + if let Some(delimiter) = delimiter + && (delimiter.is_empty() || delimiter.contains('\0')) + { + return Err(t!( + "validation.invalidDelimiter", + name = name.unwrap_or("*") + ) + .to_string()); + } + if let Some(values) = values { + let delimiter = delimiter.unwrap_or(";"); + if values.iter().any(|entry| { + entry.is_empty() || entry.contains(delimiter) || entry.contains('\0') + }) { + return Err(t!( + "validation.invalidPathEntry", + name = name.unwrap_or("*"), + delimiter = delimiter + ) + .to_string()); + } + } } - if self.path_action.is_some() && self.path_value.is_none() { + Ok(()) + } +} + +fn validate_common( + name: &str, + value_is_some: bool, + exist: Option, + operation: Operation, +) -> Result<(), String> { + if name.is_empty() { + return Err(t!("validation.emptyName").to_string()); + } + if name.contains('\0') { + return Err(t!("validation.invalidName", name = name).to_string()); + } + if matches!(operation, Operation::Set | Operation::Test) + && exist.unwrap_or(true) + && !value_is_some + { + return Err(t!("validation.missingValue", name = name).to_string()); + } + Ok(()) +} + +impl EnvironmentVariable { + pub fn validate(&self, operation: Operation) -> Result<(), String> { + validate_common( + &self.name, + self.value.is_some(), + self.exist, + operation, + ) + } +} + +impl EnvironmentPathVariable { + pub fn validate(&self, operation: Operation) -> Result<(), String> { + validate_common( + &self.name, + self.value.is_some(), + self.exist, + operation, + )?; + if self.delimiter.is_empty() || self.delimiter.contains('\0') { return Err(t!( - "validation.pathActionWithoutValue", + "validation.invalidDelimiter", name = self.name.as_str() ) .to_string()); } - if let Some(entries) = &self.path_value - && entries - .iter() - .any(|entry| entry.is_empty() || entry.contains(';') || entry.contains('\0')) + if let Some(entries) = &self.value + && entries.iter().any(|entry| { + entry.is_empty() || entry.contains(&self.delimiter) || entry.contains('\0') + }) { - return Err(t!("validation.invalidPathEntry", name = self.name.as_str()).to_string()); - } - if matches!(operation, Operation::Set | Operation::Test) - && self.exist.unwrap_or(true) - && self.value.is_none() - && self.path_value.is_none() - { - return Err(t!("validation.missingValue", name = self.name.as_str()).to_string()); + return Err(t!( + "validation.invalidPathEntry", + name = self.name.as_str(), + delimiter = self.delimiter.as_str() + ) + .to_string()); } - Ok(()) } } @@ -134,25 +305,34 @@ impl std::fmt::Display for Scope { #[cfg(test)] mod tests { - use super::{EnvironmentVariable, EnvironmentVariableList, Operation, PathAction, Scope}; + use super::{ + EnvironmentPathVariable, EnvironmentVariable, EnvironmentVariableItem, + EnvironmentVariableList, Operation, Scope, SetAction, + }; - fn variable(name: &str) -> EnvironmentVariable { - EnvironmentVariable { + fn scalar(name: &str) -> EnvironmentVariableItem { + EnvironmentVariableItem::Scalar(EnvironmentVariable { scope: Scope::CurrentUser, name: name.to_string(), value: Some("value".to_string()), - path_value: None, - path_action: None, exist: None, - } + }) } #[test] - fn rejects_duplicate_identity_case_insensitively() { - let mut second = variable("TEST_NAME"); - second.scope = Scope::CurrentUser; + fn rejects_duplicate_identity_across_variants() { let list = EnvironmentVariableList { - environment_variables: vec![variable("Test_Name"), second], + environment_variables: vec![ + scalar("Test_Name"), + EnvironmentVariableItem::Path(EnvironmentPathVariable { + scope: Scope::CurrentUser, + name: "TEST_NAME".to_string(), + value: Some(vec![r"C:\Path".to_string()]), + delimiter: ";".to_string(), + set_action: SetAction::Clobber, + exist: None, + }), + ], in_desired_state: None, }; @@ -160,96 +340,42 @@ mod tests { } #[test] - fn allows_same_name_in_different_scopes() { - let mut second = variable("Test_Name"); - second.scope = Scope::AllUsers; + fn allows_empty_export_filter() { let list = EnvironmentVariableList { - environment_variables: vec![variable("Test_Name"), second], - in_desired_state: None, - }; - - assert!(list.validate(Operation::Set).is_ok()); - } - - #[test] - fn rejects_path_action_without_path_value() { - let mut input = variable("Test_Name"); - input.path_action = Some(PathAction::Append); - let list = EnvironmentVariableList { - environment_variables: vec![input], - in_desired_state: None, - }; - - assert!(list.validate(Operation::Set).is_err()); - } - - #[test] - fn rejects_invalid_inputs() { - let empty = EnvironmentVariableList { environment_variables: Vec::new(), in_desired_state: None, }; - assert!(empty.validate(Operation::Get).is_err()); - - for name in ["", "invalid\0name"] { - assert!( - EnvironmentVariableList { - environment_variables: vec![variable(name)], - in_desired_state: None, - } - .validate(Operation::Get) - .is_err() - ); - } - let mut conflicting = variable("Test_Name"); - conflicting.path_value = Some(vec!["C:\\Path".to_string()]); - assert!( - EnvironmentVariableList { - environment_variables: vec![conflicting], - in_desired_state: None, - } - .validate(Operation::Set) - .is_err() - ); - - for entry in ["", "C:\\One;C:\\Two", "invalid\0path"] { - let mut invalid_path = variable("Test_Name"); - invalid_path.value = None; - invalid_path.path_value = Some(vec![entry.to_string()]); - assert!( - EnvironmentVariableList { - environment_variables: vec![invalid_path], - in_desired_state: None, - } - .validate(Operation::Set) - .is_err() - ); - } - - let mut missing_value = variable("Test_Name"); - missing_value.value = None; - let list = EnvironmentVariableList { - environment_variables: vec![missing_value], - in_desired_state: None, - }; - assert!(list.validate(Operation::Set).is_err()); - assert!(list.validate(Operation::Test).is_err()); - assert!(list.validate(Operation::Get).is_ok()); + assert!(list.validate(Operation::Get).is_err()); } #[test] - fn formats_scope_values_as_camel_case() { - assert_eq!(Scope::AllUsers.to_string(), "allUsers"); - assert_eq!(Scope::CurrentUser.to_string(), "currentUser"); + fn rejects_invalid_path_delimiter_and_entries() { + for (delimiter, value) in [ + ("", vec!["value"]), + (";", vec!["C:\\One;C:\\Two"]), + ("::", vec!["one::two"]), + ] { + let item = EnvironmentVariableItem::Path(EnvironmentPathVariable { + scope: Scope::CurrentUser, + name: "Path".to_string(), + value: Some(value.into_iter().map(str::to_string).collect()), + delimiter: delimiter.to_string(), + set_action: SetAction::Clobber, + exist: None, + }); + assert!(item.validate(Operation::Set).is_err()); + } } #[test] - fn wraps_single_variable_in_list() { - let list = EnvironmentVariableList::from(variable("Test_Name")); + fn untagged_items_select_value_shape() { + let scalar: EnvironmentVariableItem = + serde_json::from_str(r#"{"name":"One","value":"text"}"#).unwrap(); + let path: EnvironmentVariableItem = + serde_json::from_str(r#"{"name":"Path","value":["C:\\One"]}"#).unwrap(); - assert_eq!(list.environment_variables.len(), 1); - assert_eq!(list.environment_variables[0].name, "Test_Name"); - assert_eq!(list.in_desired_state, None); + assert!(matches!(scalar, EnvironmentVariableItem::Scalar(_))); + assert!(matches!(path, EnvironmentVariableItem::Path(_))); } } diff --git a/resources/environment_variable/tests/environment_path_variable.tests.ps1 b/resources/environment_variable/tests/environment_path_variable.tests.ps1 new file mode 100644 index 000000000..2d31edc16 --- /dev/null +++ b/resources/environment_variable/tests/environment_path_variable.tests.ps1 @@ -0,0 +1,71 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'Microsoft.Windows/EnvironmentPathVariable operations' -Skip:(!$IsWindows) { + BeforeAll { + $resourceType = 'Microsoft.Windows/EnvironmentPathVariable' + $testName = "DSC_Environment_Path_$([guid]::NewGuid().ToString('N'))" + } + + AfterEach { + Remove-ItemProperty -Path 'HKCU:\Environment' -Name $testName -ErrorAction Ignore + } + + It 'Gets entries using the default delimiter' { + Set-ItemProperty -Path 'HKCU:\Environment' -Name $testName ` + -Value 'C:\One;C:\Two' -Type String + $json = @{ name = $testName } | ConvertTo-Json -Compress + + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).actualState + + ($result.value | ConvertTo-Json -Compress) | + Should -BeExactly '["C:\\One","C:\\Two"]' + $result.PSObject.Properties.Name | Should -Not -Contain 'delimiter' + $result.PSObject.Properties.Name | Should -Not -Contain 'setAction' + } + + It 'Sets entries using a custom delimiter' { + $json = @{ + name = $testName + value = @('one', 'two') + delimiter = '::' + } | ConvertTo-Json -Compress + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).afterState + + ($result.value | ConvertTo-Json -Compress) | Should -BeExactly '["one","two"]' + (Get-ItemPropertyValue -Path 'HKCU:\Environment' -Name $testName) | + Should -BeExactly 'one::two' + } + + It 'Appends entries and tests the projected state' { + Set-ItemProperty -Path 'HKCU:\Environment' -Name $testName -Value 'C:\Existing' -Type String + $json = @{ + name = $testName + value = @('C:\New') + setAction = 'append' + } | ConvertTo-Json -Compress + + $set = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + (($set | ConvertFrom-Json).afterState.value | ConvertTo-Json -Compress) | + Should -BeExactly '["C:\\Existing","C:\\New"]' + + $test = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + ($test | ConvertFrom-Json).inDesiredState | Should -BeTrue + } + + It 'Removes a path variable when _exist is false' { + Set-ItemProperty -Path 'HKCU:\Environment' -Name $testName -Value 'remove me' -Type String + $json = @{ name = $testName; _exist = $false } | ConvertTo-Json -Compress + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + ($out | ConvertFrom-Json).afterState._exist | Should -BeFalse + } +} diff --git a/resources/environment_variable/tests/environment_variable_export.tests.ps1 b/resources/environment_variable/tests/environment_variable_export.tests.ps1 new file mode 100644 index 000000000..73712b8e5 --- /dev/null +++ b/resources/environment_variable/tests/environment_variable_export.tests.ps1 @@ -0,0 +1,82 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'Microsoft.Windows/EnvironmentVariableList export operation' -Skip:(!$IsWindows) { + BeforeAll { + $resourceType = 'Microsoft.Windows/EnvironmentVariableList' + $namePrefix = "DSC_Environment_Export_$([guid]::NewGuid().ToString('N'))" + $scalarName = "${namePrefix}_Scalar" + $pathName = "${namePrefix}_Path" + Set-ItemProperty -Path 'HKCU:\Environment' -Name $scalarName -Value 'scalar value' -Type String + Set-ItemProperty -Path 'HKCU:\Environment' -Name $pathName -Value 'C:\One;C:\Two' -Type String + } + + AfterAll { + Remove-ItemProperty -Path 'HKCU:\Environment' -Name $scalarName -ErrorAction Ignore + Remove-ItemProperty -Path 'HKCU:\Environment' -Name $pathName -ErrorAction Ignore + } + + It 'Exports all variables as scalar items by default' { + $out = dsc resource export -r $resourceType 2>$testdrive/error.log | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $items = $out.resources[0].properties.environmentVariables + $item = $items | Where-Object -Property name -EQ $pathName + + $item.value | Should -BeExactly 'C:\One;C:\Two' + $item.scope | Should -BeExactly 'currentUser' + } + + It 'Supports a case-insensitive asterisk wildcard name filter' { + $json = @{ + environmentVariables = @( + @{ name = "$($namePrefix.ToLower())*" } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource export -r $resourceType -f - 2>$testdrive/error.log | + ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $items = $out.resources[0].properties.environmentVariables + + $items.Count | Should -Be 2 + $items.name | Should -Contain $scalarName + $items.name | Should -Contain $pathName + } + + It 'Uses path representation for a path-shaped filter' { + $json = @{ + environmentVariables = @( + @{ + name = $pathName + value = @() + delimiter = ';' + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource export -r $resourceType -f - 2>$testdrive/error.log | + ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $item = $out.resources[0].properties.environmentVariables[0] + + ($item.value | ConvertTo-Json -Compress) | + Should -BeExactly '["C:\\One","C:\\Two"]' + } + + It 'Applies AND within filters and OR across filters' { + $json = @{ + environmentVariables = @( + @{ scope = 'allUsers'; name = $scalarName } + @{ scope = 'currentUser'; name = $pathName } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource export -r $resourceType -f - 2>$testdrive/error.log | + ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $items = $out.resources[0].properties.environmentVariables + + @($items).Count | Should -Be 1 + $items[0].name | Should -BeExactly $pathName + } +} diff --git a/resources/environment_variable/tests/environment_variable_get.tests.ps1 b/resources/environment_variable/tests/environment_variable_get.tests.ps1 index dbe51ccfc..3980ed88b 100644 --- a/resources/environment_variable/tests/environment_variable_get.tests.ps1 +++ b/resources/environment_variable/tests/environment_variable_get.tests.ps1 @@ -28,15 +28,15 @@ Describe 'Microsoft.Windows/EnvironmentVariableList get operation' -Skip:(!$IsWi $result.name | Should -BeExactly $testName $result.value | Should -BeExactly $testValue $result._exist | Should -BeTrue - $result.PSObject.Properties.Name | Should -Not -Contain 'pathAction' + $result.PSObject.Properties.Name | Should -Not -Contain 'setAction' } - It 'Gets a variable as pathValue when pathValue is requested' { + It 'Gets a variable as a path when an array value is requested' { $json = @{ environmentVariables = @( @{ name = $testName - pathValue = @() + value = @() } ) } | ConvertTo-Json -Compress -Depth 5 @@ -45,9 +45,9 @@ Describe 'Microsoft.Windows/EnvironmentVariableList get operation' -Skip:(!$IsWi $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = ($out | ConvertFrom-Json).actualState.environmentVariables[0] - ($result.pathValue | ConvertTo-Json -Compress) | + ($result.value | ConvertTo-Json -Compress) | Should -BeExactly '["C:\\DSC\\First","C:\\DSC\\Second"]' - $result.PSObject.Properties.Name | Should -Not -Contain 'value' + $result.PSObject.Properties.Name | Should -Not -Contain 'delimiter' } It 'Returns _exist false for a missing variable' { diff --git a/resources/environment_variable/tests/environment_variable_set.tests.ps1 b/resources/environment_variable/tests/environment_variable_set.tests.ps1 index 3e64d6135..6c06a343a 100644 --- a/resources/environment_variable/tests/environment_variable_set.tests.ps1 +++ b/resources/environment_variable/tests/environment_variable_set.tests.ps1 @@ -58,7 +58,7 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi environmentVariables = @( @{ name = $testNames[1] - pathValue = @('C:\One', 'c:\one', 'C:\Two') + value = @('C:\One', 'c:\one', 'C:\Two') } ) } | ConvertTo-Json -Compress -Depth 5 @@ -67,7 +67,7 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] - ($result.pathValue | ConvertTo-Json -Compress) | + ($result.value | ConvertTo-Json -Compress) | Should -BeExactly '["C:\\One","C:\\Two"]' [Environment]::GetEnvironmentVariable( $testNames[1], @@ -81,8 +81,8 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi environmentVariables = @( @{ name = $testNames[1] - pathValue = @('c:\shared', 'C:\New') - pathAction = 'prepend' + value = @('c:\shared', 'C:\New') + setAction = 'prepend' } ) } | ConvertTo-Json -Compress -Depth 5 @@ -91,7 +91,7 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] - ($result.pathValue | ConvertTo-Json -Compress) | + ($result.value | ConvertTo-Json -Compress) | Should -BeExactly '["c:\\shared","C:\\New","C:\\Existing"]' } @@ -102,8 +102,8 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi environmentVariables = @( @{ name = $testNames[1] - pathValue = @('c:\shared', 'C:\New') - pathAction = 'append' + value = @('c:\shared', 'C:\New') + setAction = 'append' } ) } | ConvertTo-Json -Compress -Depth 5 @@ -112,7 +112,7 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] - ($result.pathValue | ConvertTo-Json -Compress) | + ($result.value | ConvertTo-Json -Compress) | Should -BeExactly '["C:\\Existing","c:\\shared","C:\\New"]' } @@ -160,20 +160,20 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi $result[1].value | Should -BeExactly 'second' } - It 'Rejects value and pathValue together' { + It 'Rejects path entries containing the selected delimiter' { $json = @{ environmentVariables = @( @{ name = $testNames[0] - value = 'value' - pathValue = @('C:\Path') + value = @('one::two') + delimiter = '::' } ) } | ConvertTo-Json -Compress -Depth 5 - $out = $json | dsc resource set -r $resourceType -f - 2>&1 + $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log | Out-Null $LASTEXITCODE | Should -Not -Be 0 - $out | Should -Match 'value.*pathValue' + Get-Content -Raw $testdrive/error.log | Should -Match 'delimiter' } It 'Returns an actionable elevation error for AllUsers' -Skip:$isAdmin { @@ -188,10 +188,10 @@ Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWi ) } | ConvertTo-Json -Compress -Depth 5 - $out = $json | dsc resource set -r $resourceType -f - 2>&1 + $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log | Out-Null $LASTEXITCODE | Should -Not -Be 0 - $out | Should -Match 'elevation' + Get-Content -Raw $testdrive/error.log | Should -Match 'elevation' [Environment]::GetEnvironmentVariable( $machineName, [EnvironmentVariableTarget]::Machine) | Should -BeNullOrEmpty diff --git a/resources/environment_variable/tests/environment_variable_single_test.tests.ps1 b/resources/environment_variable/tests/environment_variable_single_test.tests.ps1 index b39e1816f..5c2a705bd 100644 --- a/resources/environment_variable/tests/environment_variable_single_test.tests.ps1 +++ b/resources/environment_variable/tests/environment_variable_single_test.tests.ps1 @@ -27,18 +27,16 @@ Describe 'Microsoft.Windows/EnvironmentVariable test operation' -Skip:(!$IsWindo $result.actualState.PSObject.Properties.Name | Should -Not -Contain 'environmentVariables' } - It 'Honors pathAction for one environment variable' { - Set-ItemProperty -Path 'HKCU:\Environment' -Name $testName ` - -Value 'C:\Existing;C:\New' -Type String + It 'Reports a different scalar value outside desired state' { + Set-ItemProperty -Path 'HKCU:\Environment' -Name $testName -Value 'actual' -Type String $json = @{ - name = $testName - pathValue = @('C:\New') - pathAction = 'append' + name = $testName + value = 'expected' } | ConvertTo-Json -Compress $out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) - ($out | ConvertFrom-Json).inDesiredState | Should -BeTrue + ($out | ConvertFrom-Json).inDesiredState | Should -BeFalse } } diff --git a/resources/environment_variable/tests/environment_variable_test.tests.ps1 b/resources/environment_variable/tests/environment_variable_test.tests.ps1 index faa8c8628..1b508dda4 100644 --- a/resources/environment_variable/tests/environment_variable_test.tests.ps1 +++ b/resources/environment_variable/tests/environment_variable_test.tests.ps1 @@ -58,8 +58,8 @@ Describe 'Microsoft.Windows/EnvironmentVariableList test operation' -Skip:(!$IsW environmentVariables = @( @{ name = $pathName - pathValue = @('c:\new') - pathAction = 'prepend' + value = @('c:\new') + setAction = 'prepend' } ) } | ConvertTo-Json -Compress -Depth 5 @@ -76,8 +76,8 @@ Describe 'Microsoft.Windows/EnvironmentVariableList test operation' -Skip:(!$IsW environmentVariables = @( @{ name = $pathName - pathValue = @('C:\New') - pathAction = 'prepend' + value = @('C:\New') + setAction = 'prepend' } ) } | ConvertTo-Json -Compress -Depth 5 @@ -95,8 +95,8 @@ Describe 'Microsoft.Windows/EnvironmentVariableList test operation' -Skip:(!$IsW environmentVariables = @( @{ name = $pathName - pathValue = @('C:\New') - pathAction = 'append' + value = @('C:\New') + setAction = 'append' } ) } | ConvertTo-Json -Compress -Depth 5 @@ -114,8 +114,8 @@ Describe 'Microsoft.Windows/EnvironmentVariableList test operation' -Skip:(!$IsW environmentVariables = @( @{ name = $pathName - pathValue = @('C:\Expected') - pathAction = 'clobber' + value = @('C:\Expected') + setAction = 'clobber' } ) } | ConvertTo-Json -Compress -Depth 5