From d889da82b61c906eacfe3625465c4ed6f5d32a4d Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 16 Oct 2024 15:16:30 -0700 Subject: [PATCH 01/26] metaconfig 1 --- build.ps1 | 10 ++- dsc_lib/src/discovery/command_discovery.rs | 81 +++++++++++++++---- dsc_lib/src/util.rs | 90 ++++++++++++++++++++++ 3 files changed, 164 insertions(+), 17 deletions(-) diff --git a/build.ps1 b/build.ps1 index 09e848df..c684e652 100644 --- a/build.ps1 +++ b/build.ps1 @@ -47,6 +47,8 @@ $filesForWindowsPackage = @( 'wmi.resource.ps1', 'windows_baseline.dsc.yaml', 'windows_inventory.dsc.yaml' + 'default_settings.v1.dsc.json', + 'settings.dsc.json' ) $filesForLinuxPackage = @( @@ -61,7 +63,9 @@ $filesForLinuxPackage = @( 'powershell.dsc.resource.json', 'psDscAdapter/', 'RunCommandOnSet.dsc.resource.json', - 'runcommandonset' + 'runcommandonset', + 'default_settings.v1.dsc.json', + 'settings.dsc.json' ) $filesForMacPackage = @( @@ -76,7 +80,9 @@ $filesForMacPackage = @( 'powershell.dsc.resource.json', 'psDscAdapter/', 'RunCommandOnSet.dsc.resource.json', - 'runcommandonset' + 'runcommandonset', + 'default_settings.v1.dsc.json', + 'settings.dsc.json' ) # the list of files other than the binaries which need to be executable diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index c94abb7d..7d08b629 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -11,6 +11,7 @@ use indicatif::ProgressStyle; use linked_hash_map::LinkedHashMap; use regex::RegexBuilder; use semver::Version; +use serde::Deserialize; use std::collections::{BTreeMap, HashSet, HashMap}; use std::env; use std::ffi::OsStr; @@ -18,9 +19,12 @@ use std::fs; use std::fs::File; use std::io::BufReader; use std::path::{Path, PathBuf}; +use std::str::FromStr; use tracing::{debug, info, trace, warn, warn_span}; use tracing_indicatif::span_ext::IndicatifSpanExt; +use crate::util::get_setting; + pub struct CommandDiscovery { // use BTreeMap so that the results are sorted by the typename, the Vec is sorted by version resources: BTreeMap>, @@ -28,6 +32,23 @@ pub struct CommandDiscovery { adapted_resources: BTreeMap>, } +#[derive(Deserialize)] +pub struct ResourcePathSetting { + allow_env_override: bool, + append_env_path: bool, + directories: Vec +} + +impl Default for ResourcePathSetting { + fn default() -> ResourcePathSetting { + ResourcePathSetting { + allow_env_override: true, + append_env_path: true, + directories: vec![], + } + } +} + impl CommandDiscovery { pub fn new() -> CommandDiscovery { CommandDiscovery { @@ -37,31 +58,58 @@ impl CommandDiscovery { } } + fn get_resource_path_setting() -> Result + { + if let Ok(v) = get_setting("resource_path") { + if let Ok(resource_path_setting) = serde_json::from_value(v) { + return Ok(resource_path_setting); + } + } + + return Err(DscError::Operation("Could not read resource_path setting".to_string())); + } + fn get_resource_paths() -> Result, DscError> { + let mut resource_path_setting = ResourcePathSetting::default(); + if let Ok(v) = Self::get_resource_path_setting() { + resource_path_setting = v; + } else { + debug!("Could not read resource_path setting"); + } + let mut using_custom_path = false; + let mut paths: Vec = vec![]; - // try DSC_RESOURCE_PATH env var first otherwise use PATH - let path_env = if let Some(value) = env::var_os("DSC_RESOURCE_PATH") { + let dsc_resource_path = env::var_os("DSC_RESOURCE_PATH"); + if resource_path_setting.allow_env_override && dsc_resource_path.is_some(){ + let value = dsc_resource_path.unwrap(); debug!("Using DSC_RESOURCE_PATH: {:?}", value.to_string_lossy()); using_custom_path = true; - value + paths.append(&mut env::split_paths(&value).collect::>()); } else { - trace!("DSC_RESOURCE_PATH not set, trying PATH"); - match env::var_os("PATH") { - Some(value) => { - trace!("Original PATH: {:?}", value.to_string_lossy()); - value - }, - None => { - return Err(DscError::Operation("Failed to get PATH environment variable".to_string())); + for p in resource_path_setting.directories { + if let Ok(v) = PathBuf::from_str(&p) { + paths.push(v); } } - }; - let mut paths = env::split_paths(&path_env).collect::>(); + if resource_path_setting.append_env_path { + trace!("Appending PATH to resource_path"); + match env::var_os("PATH") { + Some(value) => { + trace!("Original PATH: {:?}", value.to_string_lossy()); + paths.append(&mut env::split_paths(&value).collect::>()); + }, + None => { + return Err(DscError::Operation("Failed to get PATH environment variable".to_string())); + } + } + } + } + // remove duplicate entries - let mut uniques = HashSet::new(); + let mut uniques: HashSet = HashSet::new(); paths.retain(|e|uniques.insert((*e).clone())); // if exe home is not already in PATH env var then add it to env var and list of searched paths @@ -75,13 +123,16 @@ impl CommandDiscovery { paths.push(exe_home_pb); if let Ok(new_path) = env::join_paths(paths.clone()) { - debug!("Using PATH: {:?}", new_path.to_string_lossy()); env::set_var("PATH", &new_path); } } } }; + if let Ok(final_resource_path) = env::join_paths(paths.clone()) { + debug!("Using Resource Path: {:?}", final_resource_path.to_string_lossy()); + } + Ok(paths) } } diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index a5bbcccf..bf190faf 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -3,6 +3,12 @@ use crate::dscerror::DscError; use serde_json::Value; +use std::fs::File; +use std::io::BufReader; +use std::path::PathBuf; +use std::path::Path; +use std::env; +use tracing::debug; /// Return JSON string whether the input is JSON or YAML /// @@ -37,3 +43,87 @@ pub fn parse_input_to_json(value: &str) -> Result { } } } + +pub fn get_setting(value_name: &str) -> Result { + + const SETTINGS_FILE_NAME: &str = "settings.dsc.json"; + // Note that default settings file name has a version that is specific to this version of dsc + const DEFAULT_SETTINGS_FILE_NAME: &str = "default_settings.v1.dsc.json"; + + let mut result:serde_json::Value = serde_json::Value::Null; + let mut settings_file_path : PathBuf; + + if let Some(exe_home) = env::current_exe()?.parent() { + + // First, get setting from the default settings file + settings_file_path = exe_home.join(DEFAULT_SETTINGS_FILE_NAME); + if let Ok(v) = load_value_from_json(&settings_file_path, &value_name) { + result = v; + debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + } else { + debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + } + + // Second, get setting from the active settings file overwriting previous value + settings_file_path = exe_home.join(SETTINGS_FILE_NAME); + if let Ok(v) = load_value_from_json(&settings_file_path, &value_name) { + result = v; + debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + } else { + debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + } + } else { + debug!("Can't get dsc executable path"); + } + + // Third, get setting from the policy settings file overwriting previous value + settings_file_path = PathBuf::from(get_settings_policy_file_path()); + if let Ok(v) = load_value_from_json(&settings_file_path, &value_name) { + result = v; + debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + } else { + debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + } + + if result == serde_json::Value::Null { + return Err(DscError::NotSupported(format!("Could not find '{}' in settings", value_name).to_string())); + } + + Ok(result) +} + +pub fn load_value_from_json(path: &PathBuf, value_name: &str) -> Result { + let file = File::open(path)?; + let reader = BufReader::new(file); + let root: serde_json::Value = match serde_json::from_reader(reader) { + Ok(j) => j, + Err(err) => { + return Err(DscError::Json(err)); + } + }; + + for (key, value) in root.as_object().unwrap() { + if *key == value_name { + return Ok(value.clone()) + } + } + + Err(DscError::NotSupported(value_name.to_string())) +} + +#[cfg(target_os = "windows")] +fn get_settings_policy_file_path() -> String +{ + // $env:ProgramData+"\dsc\settings.dsc.json" + // This location is writable only by admins, but readable by all users + let Ok(local_program_data_path) = std::env::var("ProgramData") else { return String::new(); }; + Path::new(&local_program_data_path).join("dsc").join("settings.dsc.json").display().to_string() +} + +#[cfg(not(target_os = "windows"))] +fn get_settings_policy_file_path() -> String +{ + // "/etc/.dsc/settings.dsc.json" + // This location is writable only by admins, but readable by all users + Path::new("/etc").join(".dsc").join("settings.dsc.json").display().to_string() +} From 304815ca1a17b44a9eac16fbd72a8a5c3601b6ad Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 16 Oct 2024 16:08:24 -0700 Subject: [PATCH 02/26] metaconfig 2 --- dsc/copy_files.txt | 2 + dsc/default_settings.v1.dsc.json | 7 ++++ dsc/settings.dsc.json | 7 ++++ dsc_lib/src/discovery/command_discovery.rs | 2 +- dsc_lib/src/util.rs | 47 +++++++++++++--------- 5 files changed, 46 insertions(+), 19 deletions(-) create mode 100644 dsc/copy_files.txt create mode 100644 dsc/default_settings.v1.dsc.json create mode 100644 dsc/settings.dsc.json diff --git a/dsc/copy_files.txt b/dsc/copy_files.txt new file mode 100644 index 00000000..6f2f0cce --- /dev/null +++ b/dsc/copy_files.txt @@ -0,0 +1,2 @@ +settings.dsc.json +default_settings.v1.dsc.json diff --git a/dsc/default_settings.v1.dsc.json b/dsc/default_settings.v1.dsc.json new file mode 100644 index 00000000..5bc4f186 --- /dev/null +++ b/dsc/default_settings.v1.dsc.json @@ -0,0 +1,7 @@ +{ + "resource_path": { + "allow_env_override": true, + "append_env_path": true, + "directories": [] + } +} diff --git a/dsc/settings.dsc.json b/dsc/settings.dsc.json new file mode 100644 index 00000000..5bc4f186 --- /dev/null +++ b/dsc/settings.dsc.json @@ -0,0 +1,7 @@ +{ + "resource_path": { + "allow_env_override": true, + "append_env_path": true, + "directories": [] + } +} diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 7d08b629..b2c216aa 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -66,7 +66,7 @@ impl CommandDiscovery { } } - return Err(DscError::Operation("Could not read resource_path setting".to_string())); + Err(DscError::Operation("Could not read resource_path setting".to_string())) } fn get_resource_paths() -> Result, DscError> diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index bf190faf..e1912ba9 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -44,6 +44,15 @@ pub fn parse_input_to_json(value: &str) -> Result { } } +/// Will search setting files for the specified setting. +/// +/// # Arguments +/// +/// * `value_name` - The name of the setting. +/// +/// # Errors +/// +/// Will return `Err` if could not find requested setting. pub fn get_setting(value_name: &str) -> Result { const SETTINGS_FILE_NAME: &str = "settings.dsc.json"; @@ -57,20 +66,20 @@ pub fn get_setting(value_name: &str) -> Result { // First, get setting from the default settings file settings_file_path = exe_home.join(DEFAULT_SETTINGS_FILE_NAME); - if let Ok(v) = load_value_from_json(&settings_file_path, &value_name) { + if let Ok(v) = load_value_from_json(&settings_file_path, value_name) { result = v; - debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } else { - debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } // Second, get setting from the active settings file overwriting previous value settings_file_path = exe_home.join(SETTINGS_FILE_NAME); - if let Ok(v) = load_value_from_json(&settings_file_path, &value_name) { + if let Ok(v) = load_value_from_json(&settings_file_path, value_name) { result = v; - debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } else { - debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } } else { debug!("Can't get dsc executable path"); @@ -78,15 +87,15 @@ pub fn get_setting(value_name: &str) -> Result { // Third, get setting from the policy settings file overwriting previous value settings_file_path = PathBuf::from(get_settings_policy_file_path()); - if let Ok(v) = load_value_from_json(&settings_file_path, &value_name) { + if let Ok(v) = load_value_from_json(&settings_file_path, value_name) { result = v; - debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } else { - debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()) + debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } if result == serde_json::Value::Null { - return Err(DscError::NotSupported(format!("Could not find '{}' in settings", value_name).to_string())); + return Err(DscError::NotSupported(format!("Could not find '{value_name}' in settings").to_string())); } Ok(result) @@ -96,15 +105,17 @@ pub fn load_value_from_json(path: &PathBuf, value_name: &str) -> Result j, - Err(err) => { - return Err(DscError::Json(err)); - } - }; + Ok(j) => j, + Err(err) => { + return Err(DscError::Json(err)); + } + }; - for (key, value) in root.as_object().unwrap() { - if *key == value_name { - return Ok(value.clone()) + if let Some(r) = root.as_object() { + for (key, value) in r { + if *key == value_name { + return Ok(value.clone()) + } } } From fa16c3ea5f4413f548aa3c4709190f785e69f4ba Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 16 Oct 2024 16:13:12 -0700 Subject: [PATCH 03/26] metaconfig 3 --- dsc_lib/src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index e1912ba9..72c41ba2 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -101,7 +101,7 @@ pub fn get_setting(value_name: &str) -> Result { Ok(result) } -pub fn load_value_from_json(path: &PathBuf, value_name: &str) -> Result { +fn load_value_from_json(path: &PathBuf, value_name: &str) -> Result { let file = File::open(path)?; let reader = BufReader::new(file); let root: serde_json::Value = match serde_json::from_reader(reader) { From 78ebef5a1ec6d9327b54402877d1892fe882e996 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 17 Oct 2024 23:20:22 -0700 Subject: [PATCH 04/26] metaconfig 4 --- dsc/default_settings.v1.dsc.json | 5 ++ dsc/settings.dsc.json | 5 ++ dsc/src/args.rs | 7 +- dsc/src/main.rs | 2 +- dsc/src/util.rs | 96 +++++++++++++++++----- dsc_lib/src/discovery/command_discovery.rs | 31 +++++-- dsc_lib/src/util.rs | 32 ++++++-- 7 files changed, 137 insertions(+), 41 deletions(-) diff --git a/dsc/default_settings.v1.dsc.json b/dsc/default_settings.v1.dsc.json index 5bc4f186..fea79674 100644 --- a/dsc/default_settings.v1.dsc.json +++ b/dsc/default_settings.v1.dsc.json @@ -3,5 +3,10 @@ "allow_env_override": true, "append_env_path": true, "directories": [] + }, + "tracing": { + "level": "WARN", + "format": "Default", + "allow_override": true } } diff --git a/dsc/settings.dsc.json b/dsc/settings.dsc.json index 5bc4f186..fea79674 100644 --- a/dsc/settings.dsc.json +++ b/dsc/settings.dsc.json @@ -3,5 +3,10 @@ "allow_env_override": true, "append_env_path": true, "directories": [] + }, + "tracing": { + "level": "WARN", + "format": "Default", + "allow_override": true } } diff --git a/dsc/src/args.rs b/dsc/src/args.rs index e572ca31..96ada198 100644 --- a/dsc/src/args.rs +++ b/dsc/src/args.rs @@ -4,6 +4,7 @@ use clap::{Parser, Subcommand, ValueEnum}; use clap_complete::Shell; use dsc_lib::dscresources::command_resource::TraceLevel; +use serde::Deserialize; #[derive(Debug, Clone, PartialEq, Eq, ValueEnum)] pub enum OutputFormat { @@ -12,7 +13,7 @@ pub enum OutputFormat { Yaml, } -#[derive(Debug, Clone, PartialEq, Eq, ValueEnum)] +#[derive(Debug, Clone, PartialEq, Eq, ValueEnum, Deserialize)] pub enum TraceFormat { Default, Plaintext, @@ -29,8 +30,8 @@ pub struct Args { pub subcommand: SubCommand, #[clap(short = 'l', long, help = "Trace level to use", value_enum)] pub trace_level: Option, - #[clap(short = 'f', long, help = "Trace format to use", value_enum, default_value = "default")] - pub trace_format: TraceFormat, + #[clap(short = 'f', long, help = "Trace format to use", value_enum)] + pub trace_format: Option, } #[derive(Debug, PartialEq, Eq, Subcommand)] diff --git a/dsc/src/main.rs b/dsc/src/main.rs index 77adaa4c..a4850f49 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -34,7 +34,7 @@ fn main() { let args = Args::parse(); - util::enable_tracing(&args.trace_level, &args.trace_format); + util::enable_tracing(args.trace_level, args.trace_format); debug!("Running dsc {}", env!("CARGO_PKG_VERSION")); diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 675b7113..812f2508 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -23,10 +23,12 @@ use dsc_lib::{ }, resource_manifest::ResourceManifest }, util::parse_input_to_json, + util::get_setting, }; use jsonschema::Validator; use path_absolutize::Absolutize; use schemars::{schema_for, schema::RootSchema}; +use serde::Deserialize; use serde_json::Value; use std::collections::HashMap; use std::env; @@ -55,6 +57,23 @@ pub const EXIT_DSC_RESOURCE_NOT_FOUND: i32 = 7; pub const DSC_CONFIG_ROOT: &str = "DSC_CONFIG_ROOT"; pub const DSC_TRACE_LEVEL: &str = "DSC_TRACE_LEVEL"; +#[derive(Deserialize)] +pub struct TracingSetting { + level: TraceLevel, + format: TraceFormat, + allow_override: bool +} + +impl Default for TracingSetting { + fn default() -> TracingSetting { + TracingSetting { + level: TraceLevel::Warn, + format: TraceFormat::Default, + allow_override: true, + } + } +} + /// Get string representation of JSON value. /// /// # Arguments @@ -268,31 +287,65 @@ pub fn write_output(json: &str, format: &Option) { } } -pub fn enable_tracing(trace_level: &Option, trace_format: &TraceFormat) { - let tracing_level = match trace_level { - Some(level) => level, - None => { - // use DSC_TRACE_LEVEL env var if set - match env::var(DSC_TRACE_LEVEL) { - Ok(level) => { - match level.to_ascii_uppercase().as_str() { - "ERROR" => &TraceLevel::Error, - "WARN" => &TraceLevel::Warn, - "INFO" => &TraceLevel::Info, - "DEBUG" => &TraceLevel::Debug, - "TRACE" => &TraceLevel::Trace, - _ => { - warn!("Invalid DSC_TRACE_LEVEL value '{level}', defaulting to 'warn'"); - &TraceLevel::Warn - }, - } +pub fn enable_tracing(trace_level_arg: Option, trace_format_arg: Option) { + + let mut policy_is_used = false; + let mut tracing_setting = TracingSetting::default(); + + // read setting/policy from files + if let Ok(v) = get_setting("tracing") { + if v.policy != serde_json::Value::Null { + match serde_json::from_value::(v.policy) { + Ok(v) => { + tracing_setting = v; + policy_is_used = true; }, - Err(_) => &TraceLevel::Warn, + Err(e) => { println!("{}", format!("{e}")); } + } + } else if v.setting != serde_json::Value::Null { + match serde_json::from_value::(v.setting) { + Ok(v) => { + tracing_setting = v; + }, + Err(e) => { println!("{}", format!("{e}")); } } } + } else { + println!("Could not read 'tracing' setting"); + } + + // override with DSC_TRACE_LEVEL env var if permitted + if tracing_setting.allow_override { + match env::var(DSC_TRACE_LEVEL) { + Ok(level) => { + tracing_setting.level = match level.to_ascii_uppercase().as_str() { + "ERROR" => TraceLevel::Error, + "WARN" => TraceLevel::Warn, + "INFO" => TraceLevel::Info, + "DEBUG" => TraceLevel::Debug, + "TRACE" => TraceLevel::Trace, + _ => { + warn!("Invalid DSC_TRACE_LEVEL value '{level}', defaulting to 'warn'"); + TraceLevel::Warn + } + } + }, + Err(_) => {}, + } + } + + // command-line args override setting value, but not policy + if !policy_is_used { + if let Some(v) = trace_level_arg { + tracing_setting.level = v.clone(); + }; + if let Some(v) = trace_format_arg { + tracing_setting.format = v.clone(); + }; }; - let tracing_level = match tracing_level { + // convert to 'tracing' crate type + let tracing_level = match tracing_setting.level { TraceLevel::Error => Level::ERROR, TraceLevel::Warn => Level::WARN, TraceLevel::Info => Level::INFO, @@ -300,6 +353,7 @@ pub fn enable_tracing(trace_level: &Option, trace_format: &TraceForm TraceLevel::Trace => Level::TRACE, }; + // enable tracing let filter = EnvFilter::try_from_default_env() .or_else(|_| EnvFilter::try_new("warning")) .unwrap_or_default() @@ -307,7 +361,7 @@ pub fn enable_tracing(trace_level: &Option, trace_format: &TraceForm let indicatif_layer = IndicatifLayer::new(); let layer = tracing_subscriber::fmt::Layer::default().with_writer(indicatif_layer.get_stderr_writer()); let with_source = tracing_level == Level::DEBUG || tracing_level == Level::TRACE; - let fmt = match trace_format { + let fmt = match tracing_setting.format { TraceFormat::Default => { layer .with_ansi(true) diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index b2c216aa..13980127 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -61,21 +61,38 @@ impl CommandDiscovery { fn get_resource_path_setting() -> Result { if let Ok(v) = get_setting("resource_path") { - if let Ok(resource_path_setting) = serde_json::from_value(v) { - return Ok(resource_path_setting); + // if there is a policy value defined - use it; otherwise use setting value + if v.policy != serde_json::Value::Null { + match serde_json::from_value::(v.policy) { + Ok(v) => { + return Ok(v); + }, + Err(e) => { return Err(DscError::Operation(format!("{}", e))); } + } + } else if v.setting != serde_json::Value::Null { + match serde_json::from_value::(v.setting) { + Ok(v) => { + return Ok(v); + }, + Err(e) => { return Err(DscError::Operation(format!("{}", e))); } + } } } - Err(DscError::Operation("Could not read resource_path setting".to_string())) + Err(DscError::Operation("Could not read 'resource_path' setting".to_string())) } fn get_resource_paths() -> Result, DscError> { let mut resource_path_setting = ResourcePathSetting::default(); - if let Ok(v) = Self::get_resource_path_setting() { - resource_path_setting = v; - } else { - debug!("Could not read resource_path setting"); + + match Self::get_resource_path_setting() { + Ok(v) => { + resource_path_setting = v; + }, + Err(e) => { + debug!("{e}"); + } } let mut using_custom_path = false; diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index 72c41ba2..02a6fec5 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -10,6 +10,20 @@ use std::path::Path; use std::env; use tracing::debug; +pub struct DscSettingValue { + pub setting: serde_json::Value, + pub policy: serde_json::Value, +} + +impl Default for DscSettingValue { + fn default() -> DscSettingValue { + DscSettingValue { + setting: serde_json::Value::Null, + policy: serde_json::Value::Null, + } + } +} + /// Return JSON string whether the input is JSON or YAML /// /// # Arguments @@ -53,30 +67,29 @@ pub fn parse_input_to_json(value: &str) -> Result { /// # Errors /// /// Will return `Err` if could not find requested setting. -pub fn get_setting(value_name: &str) -> Result { +pub fn get_setting(value_name: &str) -> Result { const SETTINGS_FILE_NAME: &str = "settings.dsc.json"; // Note that default settings file name has a version that is specific to this version of dsc const DEFAULT_SETTINGS_FILE_NAME: &str = "default_settings.v1.dsc.json"; - let mut result:serde_json::Value = serde_json::Value::Null; + let mut result: DscSettingValue = DscSettingValue::default(); let mut settings_file_path : PathBuf; if let Some(exe_home) = env::current_exe()?.parent() { - // First, get setting from the default settings file settings_file_path = exe_home.join(DEFAULT_SETTINGS_FILE_NAME); if let Ok(v) = load_value_from_json(&settings_file_path, value_name) { - result = v; + result.setting = v; debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } else { debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } - // Second, get setting from the active settings file overwriting previous value + // Second, get setting from the active settings file overwriting previous value settings_file_path = exe_home.join(SETTINGS_FILE_NAME); if let Ok(v) = load_value_from_json(&settings_file_path, value_name) { - result = v; + result.setting = v; debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } else { debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); @@ -85,16 +98,17 @@ pub fn get_setting(value_name: &str) -> Result { debug!("Can't get dsc executable path"); } - // Third, get setting from the policy settings file overwriting previous value + // Third, get setting from the policy settings_file_path = PathBuf::from(get_settings_policy_file_path()); if let Ok(v) = load_value_from_json(&settings_file_path, value_name) { - result = v; + result.policy = v; debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } else { debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } - if result == serde_json::Value::Null { + if (result.setting == serde_json::Value::Null) && + (result.policy == serde_json::Value::Null) { return Err(DscError::NotSupported(format!("Could not find '{value_name}' in settings").to_string())); } From 6255d7c68f6325c7853e5447f31e632cb51799ac Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 17 Oct 2024 23:24:03 -0700 Subject: [PATCH 05/26] metaconfig 5 --- dsc_lib/src/discovery/command_discovery.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 13980127..ef1e6a8c 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -67,14 +67,14 @@ impl CommandDiscovery { Ok(v) => { return Ok(v); }, - Err(e) => { return Err(DscError::Operation(format!("{}", e))); } + Err(e) => { return Err(DscError::Operation(format!("{e}"))); } } } else if v.setting != serde_json::Value::Null { match serde_json::from_value::(v.setting) { Ok(v) => { return Ok(v); }, - Err(e) => { return Err(DscError::Operation(format!("{}", e))); } + Err(e) => { return Err(DscError::Operation(format!("{e}"))); } } } } From f2479a168c8b4c3e64806ee14d7241cd76588872 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 17 Oct 2024 23:32:20 -0700 Subject: [PATCH 06/26] metaconfig 6 --- dsc/src/util.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 812f2508..1ee98464 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -300,14 +300,14 @@ pub fn enable_tracing(trace_level_arg: Option, trace_format_arg: Opt tracing_setting = v; policy_is_used = true; }, - Err(e) => { println!("{}", format!("{e}")); } + Err(e) => { println!("{}", e); } } } else if v.setting != serde_json::Value::Null { match serde_json::from_value::(v.setting) { Ok(v) => { tracing_setting = v; }, - Err(e) => { println!("{}", format!("{e}")); } + Err(e) => { println!("{}", e); } } } } else { @@ -316,21 +316,18 @@ pub fn enable_tracing(trace_level_arg: Option, trace_format_arg: Opt // override with DSC_TRACE_LEVEL env var if permitted if tracing_setting.allow_override { - match env::var(DSC_TRACE_LEVEL) { - Ok(level) => { - tracing_setting.level = match level.to_ascii_uppercase().as_str() { - "ERROR" => TraceLevel::Error, - "WARN" => TraceLevel::Warn, - "INFO" => TraceLevel::Info, - "DEBUG" => TraceLevel::Debug, - "TRACE" => TraceLevel::Trace, - _ => { - warn!("Invalid DSC_TRACE_LEVEL value '{level}', defaulting to 'warn'"); - TraceLevel::Warn - } + if let Ok(level) = env::var(DSC_TRACE_LEVEL) { + tracing_setting.level = match level.to_ascii_uppercase().as_str() { + "ERROR" => TraceLevel::Error, + "WARN" => TraceLevel::Warn, + "INFO" => TraceLevel::Info, + "DEBUG" => TraceLevel::Debug, + "TRACE" => TraceLevel::Trace, + _ => { + warn!("Invalid DSC_TRACE_LEVEL value '{level}', defaulting to 'warn'"); + TraceLevel::Warn } - }, - Err(_) => {}, + } } } From 2a02351181e16001f807f7b0c87b2e570c49996d Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 17 Oct 2024 23:38:51 -0700 Subject: [PATCH 07/26] metaconfig 7 --- dsc/src/util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 1ee98464..67bb0224 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -300,14 +300,14 @@ pub fn enable_tracing(trace_level_arg: Option, trace_format_arg: Opt tracing_setting = v; policy_is_used = true; }, - Err(e) => { println!("{}", e); } + Err(e) => { println!("{e}"); } } } else if v.setting != serde_json::Value::Null { match serde_json::from_value::(v.setting) { Ok(v) => { tracing_setting = v; }, - Err(e) => { println!("{}", e); } + Err(e) => { println!("{e}"); } } } } else { From 9ee49bdb74dd9eaa070694dd950e8315238d7a56 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 18 Oct 2024 10:30:45 -0700 Subject: [PATCH 08/26] metaconfig 8 --- dsc/src/util.rs | 3 +- dsc/tests/dsc_settings.tests.ps1 | 93 ++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 dsc/tests/dsc_settings.tests.ps1 diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 67bb0224..f1b748f2 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -41,7 +41,7 @@ use syntect::{ parsing::SyntaxSet, util::{as_24_bit_terminal_escaped, LinesWithEndings} }; -use tracing::{Level, debug, error, warn, trace}; +use tracing::{Level, info, debug, error, warn, trace}; use tracing_subscriber::{filter::EnvFilter, layer::SubscriberExt, Layer}; use tracing_indicatif::IndicatifLayer; @@ -394,6 +394,7 @@ pub fn enable_tracing(trace_level_arg: Option, trace_format_arg: Opt // set DSC_TRACE_LEVEL for child processes env::set_var(DSC_TRACE_LEVEL, tracing_level.to_string().to_ascii_lowercase()); + info!("Trace-level is {:?}", tracing_setting.level); } /// Validate the JSON against the schema. diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 new file mode 100644 index 00000000..9f72a59e --- /dev/null +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -0,0 +1,93 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'tests for dsc settings' { + BeforeAll { + $env:DSC_RESOURCE_PATH = $testdrive + + $script:policyFilePath = if ($IsWindows) { + Join-Path $env:ProgramData "dsc" "settings.dsc.json" + } else { + "/etc/.dsc/settings.dsc.json" + } + + $script:dscHome = (Get-Command dsc).Path | Split-Path + $script:dscSettingsFilePath = Join-Path $script:dscHome "settings.dsc.json" + $script:dscDefaultv1SettingsFilePath = Join-Path $script:dscHome "default_settings.v1.dsc.json" + $script:dscDefaultv1SettingsJson = Get-Content -Raw -Path $script:dscDefaultv1SettingsFilePath + + $script:policyDirPath = $script:policyFilePath | Split-Path + New-Item -ItemType Directory -Path $script:policyDirPath -ErrorAction SilentlyContinue | Out-Null + + #create backups of settings files + $script:dscSettingsFilePath_backup = Join-Path $script:dscHome "settings.dsc.json.backup" + $script:dscDefaultv1SettingsFilePath_backup = Join-Path $script:dscHome "default_settings.v1.dsc.json.backup" + Copy-Item -Force -Path $script:dscSettingsFilePath -Destination $script:dscSettingsFilePath_backup + Copy-Item -Force -Path $script:dscDefaultv1SettingsFilePath -Destination $script:dscDefaultv1SettingsFilePath_backup + } + + AfterAll { + Remove-Item -Force -Path $script:dscSettingsFilePath_backup + Remove-Item -Force -Path $script:dscDefaultv1SettingsFilePath_backup + Remove-Item -Recurse -Force -Path $script:policyDirPath + } + + AfterEach { + Copy-Item -Force -Path $script:dscSettingsFilePath_backup -Destination $script:dscSettingsFilePath + Copy-Item -Force -Path $script:dscDefaultv1SettingsFilePath_backup -Destination $script:dscDefaultv1SettingsFilePath + Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue + $env:DSC_RESOURCE_PATH = $null + } + + It 'ensure a new tracing value in settings has effect' { + + $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') | Set-Content -Force -Path $script:dscSettingsFilePath + + dsc resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Trace" + } + + It 'ensure a new resource_path value in settings has effect' { + + $script:dscDefaultv1SettingsJson.Replace('"directories": []', '"directories": ["TestDir"]') | Set-Content -Force -Path $script:dscSettingsFilePath + dsc -l debug resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: "TestDir' + } + + It 'Confirm settings override priorities' { + + $v = $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') + $v = $v.Replace('"directories": []', '"directories": ["PolicyDir"]') + $v | Set-Content -Force -Path $script:policyFilePath + + $v = $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') + $v = $v.Replace('"directories": []', '"directories": ["SettingsDir"]') + $v | Set-Content -Force -Path $script:dscSettingsFilePath + + $v = $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') + $v = $v.Replace('"directories": []', '"directories": ["Defaultv1SettingsDir"]') + $v | Set-Content -Force -Path $script:dscDefaultv1SettingsFilePath + + # ensure policy overrides everything + dsc -l debug resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Trace" + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: "PolicyDir' + + # without policy, command-line args have priority + Remove-Item -Path $script:policyFilePath + dsc -l debug resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Debug" + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: "SettingsDir' + + # without policy and command-line args, settings file is used + dsc resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Trace" + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: "SettingsDir' + + # without policy and command-line args and settings file, the default settings file is used + Remove-Item -Path $script:dscSettingsFilePath + dsc resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Trace" + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: "Defaultv1SettingsDir' + } +} From fbb0c8e676d270d45763e2ea129f3c41b5722321 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 18 Oct 2024 10:42:38 -0700 Subject: [PATCH 09/26] metaconfig 9 --- dsc/tests/dsc_settings.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index 9f72a59e..74584e16 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -17,7 +17,7 @@ Describe 'tests for dsc settings' { $script:dscDefaultv1SettingsJson = Get-Content -Raw -Path $script:dscDefaultv1SettingsFilePath $script:policyDirPath = $script:policyFilePath | Split-Path - New-Item -ItemType Directory -Path $script:policyDirPath -ErrorAction SilentlyContinue | Out-Null + New-Item -ItemType Directory -Path $script:policyDirPath | Out-Null #create backups of settings files $script:dscSettingsFilePath_backup = Join-Path $script:dscHome "settings.dsc.json.backup" From b32ac65382e0ae56818b39e2af98e9a43254d068 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 18 Oct 2024 11:25:18 -0700 Subject: [PATCH 10/26] metaconfig 10 --- dsc/tests/dsc_settings.tests.ps1 | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index 74584e16..7823a49d 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -3,7 +3,6 @@ Describe 'tests for dsc settings' { BeforeAll { - $env:DSC_RESOURCE_PATH = $testdrive $script:policyFilePath = if ($IsWindows) { Join-Path $env:ProgramData "dsc" "settings.dsc.json" @@ -16,8 +15,10 @@ Describe 'tests for dsc settings' { $script:dscDefaultv1SettingsFilePath = Join-Path $script:dscHome "default_settings.v1.dsc.json" $script:dscDefaultv1SettingsJson = Get-Content -Raw -Path $script:dscDefaultv1SettingsFilePath - $script:policyDirPath = $script:policyFilePath | Split-Path - New-Item -ItemType Directory -Path $script:policyDirPath | Out-Null + if ($IsWindows) { #"Setting policy on Linux requires sudo" + $script:policyDirPath = $script:policyFilePath | Split-Path + New-Item -ItemType Directory -Path $script:policyDirPath | Out-Null + } #create backups of settings files $script:dscSettingsFilePath_backup = Join-Path $script:dscHome "settings.dsc.json.backup" @@ -29,14 +30,17 @@ Describe 'tests for dsc settings' { AfterAll { Remove-Item -Force -Path $script:dscSettingsFilePath_backup Remove-Item -Force -Path $script:dscDefaultv1SettingsFilePath_backup - Remove-Item -Recurse -Force -Path $script:policyDirPath + if ($IsWindows) { #"Setting policy on Linux requires sudo" + Remove-Item -Recurse -Force -Path $script:policyDirPath + } } AfterEach { Copy-Item -Force -Path $script:dscSettingsFilePath_backup -Destination $script:dscSettingsFilePath Copy-Item -Force -Path $script:dscDefaultv1SettingsFilePath_backup -Destination $script:dscDefaultv1SettingsFilePath - Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue - $env:DSC_RESOURCE_PATH = $null + if ($IsWindows) { #"Setting policy on Linux requires sudo" + Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue + } } It 'ensure a new tracing value in settings has effect' { @@ -55,6 +59,11 @@ Describe 'tests for dsc settings' { } It 'Confirm settings override priorities' { + + if (! $IsWindows) { + Set-ItResult -Skip -Because "Setting policy requires sudo" + return + } $v = $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') $v = $v.Replace('"directories": []', '"directories": ["PolicyDir"]') From ff1ae6fcf566ec05f478936521d84a6a7d76c598 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 23 Oct 2024 12:09:23 -0700 Subject: [PATCH 11/26] feedback 1 --- dsc/default_settings.v1.dsc.json | 8 ++++---- dsc/settings.dsc.json | 8 ++++---- dsc/src/util.rs | 6 +++++- dsc/tests/dsc_settings.tests.ps1 | 9 ++++++--- dsc_lib/src/discovery/command_discovery.rs | 15 ++++++++++----- dsc_lib/src/dscerror.rs | 3 +++ dsc_lib/src/util.rs | 12 ++++++------ 7 files changed, 38 insertions(+), 23 deletions(-) diff --git a/dsc/default_settings.v1.dsc.json b/dsc/default_settings.v1.dsc.json index fea79674..19acd63e 100644 --- a/dsc/default_settings.v1.dsc.json +++ b/dsc/default_settings.v1.dsc.json @@ -1,12 +1,12 @@ { - "resource_path": { - "allow_env_override": true, - "append_env_path": true, + "resourcePath": { + "allowEnvOverride": true, + "appendEnvPath": true, "directories": [] }, "tracing": { "level": "WARN", "format": "Default", - "allow_override": true + "allowOverride": true } } diff --git a/dsc/settings.dsc.json b/dsc/settings.dsc.json index fea79674..19acd63e 100644 --- a/dsc/settings.dsc.json +++ b/dsc/settings.dsc.json @@ -1,12 +1,12 @@ { - "resource_path": { - "allow_env_override": true, - "append_env_path": true, + "resourcePath": { + "allowEnvOverride": true, + "appendEnvPath": true, "directories": [] }, "tracing": { "level": "WARN", "format": "Default", - "allow_override": true + "allowOverride": true } } diff --git a/dsc/src/util.rs b/dsc/src/util.rs index f1b748f2..76aa3e26 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -41,7 +41,7 @@ use syntect::{ parsing::SyntaxSet, util::{as_24_bit_terminal_escaped, LinesWithEndings} }; -use tracing::{Level, info, debug, error, warn, trace}; +use tracing::{Level, debug, error, info, warn, trace}; use tracing_subscriber::{filter::EnvFilter, layer::SubscriberExt, Layer}; use tracing_indicatif::IndicatifLayer; @@ -59,8 +59,12 @@ pub const DSC_TRACE_LEVEL: &str = "DSC_TRACE_LEVEL"; #[derive(Deserialize)] pub struct TracingSetting { + /// Trace level to use - see pub enum TraceLevel in dsc_lib\src\dscresources\command_resource.rs level: TraceLevel, + /// Trace format to use - see pub enum TraceFormat in dsc\src\args.rs format: TraceFormat, + /// Whether the 'level' can be overrridden by DSC_TRACE_LEVEL environment variable + #[serde(rename = "allowOverride")] allow_override: bool } diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index 7823a49d..1aa9a899 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -7,7 +7,7 @@ Describe 'tests for dsc settings' { $script:policyFilePath = if ($IsWindows) { Join-Path $env:ProgramData "dsc" "settings.dsc.json" } else { - "/etc/.dsc/settings.dsc.json" + "/etc/dsc/settings.dsc.json" } $script:dscHome = (Get-Command dsc).Path | Split-Path @@ -53,9 +53,12 @@ Describe 'tests for dsc settings' { It 'ensure a new resource_path value in settings has effect' { - $script:dscDefaultv1SettingsJson.Replace('"directories": []', '"directories": ["TestDir"]') | Set-Content -Force -Path $script:dscSettingsFilePath + $script:dscDefaultv1SettingsJson.Replace('"directories": []', '"directories": ["TestDir1","TestDir2"]') | Set-Content -Force -Path $script:dscSettingsFilePath + copy-Item -Recurse -Force -Path $script:dscSettingsFilePath -Destination "C:\Temp\" dsc -l debug resource list 2> $TestDrive/tracing.txt - "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: "TestDir' + copy-Item -Recurse -Force -Path $TestDrive/tracing.txt -Destination "C:\Temp\" + $expectedString = 'Using Resource Path: "TestDir1'+[System.IO.Path]::PathSeparator+'TestDir2' + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly $expectedString } It 'Confirm settings override priorities' { diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index ef1e6a8c..26d1175a 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -34,8 +34,13 @@ pub struct CommandDiscovery { #[derive(Deserialize)] pub struct ResourcePathSetting { + /// whether to allow overriding with the DSC_RESOURCE_PATH environment variable + #[serde(rename = "allowEnvOverride")] allow_env_override: bool, + /// whether to append the PATH environment variable to the list of resource directories + #[serde(rename = "appendEnvPath")] append_env_path: bool, + /// array of directories that DSC should search for non-built-in resources directories: Vec } @@ -60,26 +65,26 @@ impl CommandDiscovery { fn get_resource_path_setting() -> Result { - if let Ok(v) = get_setting("resource_path") { + if let Ok(v) = get_setting("resourcePath") { // if there is a policy value defined - use it; otherwise use setting value if v.policy != serde_json::Value::Null { match serde_json::from_value::(v.policy) { Ok(v) => { return Ok(v); }, - Err(e) => { return Err(DscError::Operation(format!("{e}"))); } + Err(e) => { return Err(DscError::Setting(format!("{e}"))); } } } else if v.setting != serde_json::Value::Null { match serde_json::from_value::(v.setting) { Ok(v) => { return Ok(v); }, - Err(e) => { return Err(DscError::Operation(format!("{e}"))); } + Err(e) => { return Err(DscError::Setting(format!("{e}"))); } } } } - Err(DscError::Operation("Could not read 'resource_path' setting".to_string())) + Err(DscError::Setting("Could not read 'resourcePath' setting".to_string())) } fn get_resource_paths() -> Result, DscError> @@ -112,7 +117,7 @@ impl CommandDiscovery { } if resource_path_setting.append_env_path { - trace!("Appending PATH to resource_path"); + debug!("Appending PATH to resourcePath"); match env::var_os("PATH") { Some(value) => { trace!("Original PATH: {:?}", value.to_string_lossy()); diff --git a/dsc_lib/src/dscerror.rs b/dsc_lib/src/dscerror.rs index f570988b..e2c3b631 100644 --- a/dsc_lib/src/dscerror.rs +++ b/dsc_lib/src/dscerror.rs @@ -117,4 +117,7 @@ pub enum DscError { #[error("YAML: {0}")] Yaml(#[from] serde_yaml::Error), + + #[error("Setting: {0}")] + Setting(String), } diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index 02a6fec5..ac39c3b9 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -11,15 +11,15 @@ use std::env; use tracing::debug; pub struct DscSettingValue { - pub setting: serde_json::Value, - pub policy: serde_json::Value, + pub setting: Value, + pub policy: Value, } impl Default for DscSettingValue { fn default() -> DscSettingValue { DscSettingValue { - setting: serde_json::Value::Null, - policy: serde_json::Value::Null, + setting: Value::Null, + policy: Value::Null, } } } @@ -148,7 +148,7 @@ fn get_settings_policy_file_path() -> String #[cfg(not(target_os = "windows"))] fn get_settings_policy_file_path() -> String { - // "/etc/.dsc/settings.dsc.json" + // "/etc/dsc/settings.dsc.json" // This location is writable only by admins, but readable by all users - Path::new("/etc").join(".dsc").join("settings.dsc.json").display().to_string() + Path::new("/etc").join("dsc").join("settings.dsc.json").display().to_string() } From 210a01032aaa600d352d3ff6c6defaf58720e8d5 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 23 Oct 2024 12:16:22 -0700 Subject: [PATCH 12/26] feedback 1_1 --- dsc_lib/src/discovery/command_discovery.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 26d1175a..cdc07adb 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -34,7 +34,7 @@ pub struct CommandDiscovery { #[derive(Deserialize)] pub struct ResourcePathSetting { - /// whether to allow overriding with the DSC_RESOURCE_PATH environment variable + /// whether to allow overriding with the `DSC_RESOURCE_PATH` environment variable #[serde(rename = "allowEnvOverride")] allow_env_override: bool, /// whether to append the PATH environment variable to the list of resource directories From 7bc620266d7f93f32b78f8da467a27221c3e8332 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 23 Oct 2024 14:19:11 -0700 Subject: [PATCH 13/26] feedback 1_2 --- dsc/src/util.rs | 6 +++--- dsc_lib/src/discovery/command_discovery.rs | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 76aa3e26..3adc8dd3 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -59,11 +59,11 @@ pub const DSC_TRACE_LEVEL: &str = "DSC_TRACE_LEVEL"; #[derive(Deserialize)] pub struct TracingSetting { - /// Trace level to use - see pub enum TraceLevel in dsc_lib\src\dscresources\command_resource.rs + /// Trace level to use - see pub enum `TraceLevel` in dsc_lib\src\dscresources\command_resource.rs level: TraceLevel, - /// Trace format to use - see pub enum TraceFormat in dsc\src\args.rs + /// Trace format to use - see pub enum `TraceFormat` in dsc\src\args.rs format: TraceFormat, - /// Whether the 'level' can be overrridden by DSC_TRACE_LEVEL environment variable + /// Whether the 'level' can be overrridden by `DSC_TRACE_LEVEL` environment variable #[serde(rename = "allowOverride")] allow_override: bool } diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index cdc07adb..6896fe59 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -113,7 +113,9 @@ impl CommandDiscovery { for p in resource_path_setting.directories { if let Ok(v) = PathBuf::from_str(&p) { paths.push(v); - } + } else { + trace!("Can't parse path: '{p}'"); + }; } if resource_path_setting.append_env_path { From 2d2edc3abb6db2c65c21428d0dc139c088d36a4c Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 23 Oct 2024 14:30:38 -0700 Subject: [PATCH 14/26] feedback 1_3 --- dsc/src/main.rs | 2 +- dsc/src/util.rs | 2 +- dsc_lib/src/util.rs | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dsc/src/main.rs b/dsc/src/main.rs index a4850f49..77adaa4c 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -34,7 +34,7 @@ fn main() { let args = Args::parse(); - util::enable_tracing(args.trace_level, args.trace_format); + util::enable_tracing(&args.trace_level, &args.trace_format); debug!("Running dsc {}", env!("CARGO_PKG_VERSION")); diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 3adc8dd3..a8bfc8ba 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -291,7 +291,7 @@ pub fn write_output(json: &str, format: &Option) { } } -pub fn enable_tracing(trace_level_arg: Option, trace_format_arg: Option) { +pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &Option) { let mut policy_is_used = false; let mut tracing_setting = TracingSetting::default(); diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index ac39c3b9..2ac0050d 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -59,6 +59,7 @@ pub fn parse_input_to_json(value: &str) -> Result { } /// Will search setting files for the specified setting. +/// Performance implication: Use this function economically as every call opens/reads several config files. /// /// # Arguments /// From f1c44c70d43fc57596b5fd7033702c341260a923 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 23 Oct 2024 14:37:59 -0700 Subject: [PATCH 15/26] feedback 1_4 --- dsc/src/util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index a8bfc8ba..8cfb0545 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -59,9 +59,9 @@ pub const DSC_TRACE_LEVEL: &str = "DSC_TRACE_LEVEL"; #[derive(Deserialize)] pub struct TracingSetting { - /// Trace level to use - see pub enum `TraceLevel` in dsc_lib\src\dscresources\command_resource.rs + /// Trace level to use - see pub enum `TraceLevel` in `dsc_lib\src\dscresources\command_resource.rs` level: TraceLevel, - /// Trace format to use - see pub enum `TraceFormat` in dsc\src\args.rs + /// Trace format to use - see pub enum `TraceFormat` in `dsc\src\args.rs` format: TraceFormat, /// Whether the 'level' can be overrridden by `DSC_TRACE_LEVEL` environment variable #[serde(rename = "allowOverride")] From 65702f4e86459a1ad2adc8457dee54da3744f25c Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 23 Oct 2024 18:35:37 -0700 Subject: [PATCH 16/26] feedback 1_5 --- dsc/{settings.dsc.json => dsc.settings.json} | 0 dsc/tests/dsc_settings.tests.ps1 | 4 ++-- dsc_lib/src/util.rs | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) rename dsc/{settings.dsc.json => dsc.settings.json} (100%) diff --git a/dsc/settings.dsc.json b/dsc/dsc.settings.json similarity index 100% rename from dsc/settings.dsc.json rename to dsc/dsc.settings.json diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index 1aa9a899..71b33469 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -5,13 +5,13 @@ Describe 'tests for dsc settings' { BeforeAll { $script:policyFilePath = if ($IsWindows) { - Join-Path $env:ProgramData "dsc" "settings.dsc.json" + Join-Path $env:ProgramData "dsc" "dsc.settings.json" } else { "/etc/dsc/settings.dsc.json" } $script:dscHome = (Get-Command dsc).Path | Split-Path - $script:dscSettingsFilePath = Join-Path $script:dscHome "settings.dsc.json" + $script:dscSettingsFilePath = Join-Path $script:dscHome "dsc.settings.json" $script:dscDefaultv1SettingsFilePath = Join-Path $script:dscHome "default_settings.v1.dsc.json" $script:dscDefaultv1SettingsJson = Get-Content -Raw -Path $script:dscDefaultv1SettingsFilePath diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index 2ac0050d..73fd1436 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -70,7 +70,7 @@ pub fn parse_input_to_json(value: &str) -> Result { /// Will return `Err` if could not find requested setting. pub fn get_setting(value_name: &str) -> Result { - const SETTINGS_FILE_NAME: &str = "settings.dsc.json"; + const SETTINGS_FILE_NAME: &str = "dsc.settings.json"; // Note that default settings file name has a version that is specific to this version of dsc const DEFAULT_SETTINGS_FILE_NAME: &str = "default_settings.v1.dsc.json"; @@ -140,16 +140,16 @@ fn load_value_from_json(path: &PathBuf, value_name: &str) -> Result String { - // $env:ProgramData+"\dsc\settings.dsc.json" + // $env:ProgramData+"\dsc\dsc.settings.json" // This location is writable only by admins, but readable by all users let Ok(local_program_data_path) = std::env::var("ProgramData") else { return String::new(); }; - Path::new(&local_program_data_path).join("dsc").join("settings.dsc.json").display().to_string() + Path::new(&local_program_data_path).join("dsc").join("dsc.settings.json").display().to_string() } #[cfg(not(target_os = "windows"))] fn get_settings_policy_file_path() -> String { - // "/etc/dsc/settings.dsc.json" + // "/etc/dsc/dsc.settings.json" // This location is writable only by admins, but readable by all users - Path::new("/etc").join("dsc").join("settings.dsc.json").display().to_string() + Path::new("/etc").join("dsc").join("dsc.settings.json").display().to_string() } From ebb1e96ebd5bd4245d6c1fc0616140ab15ea2bd2 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 09:55:49 -0700 Subject: [PATCH 17/26] renamed file to dsc_default.settings.json --- build.ps1 | 6 +++--- dsc/copy_files.txt | 2 +- ...fault_settings.v1.dsc.json => dsc_default.settings.json} | 0 dsc/tests/dsc_settings.tests.ps1 | 4 ++-- dsc_lib/src/util.rs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename dsc/{default_settings.v1.dsc.json => dsc_default.settings.json} (100%) diff --git a/build.ps1 b/build.ps1 index 303c2a0a..44900a98 100644 --- a/build.ps1 +++ b/build.ps1 @@ -48,7 +48,7 @@ $filesForWindowsPackage = @( 'wmi.resource.ps1', 'windows_baseline.dsc.yaml', 'windows_inventory.dsc.yaml' - 'default_settings.v1.dsc.json', + 'dsc_default.settings.json', 'settings.dsc.json' ) @@ -65,7 +65,7 @@ $filesForLinuxPackage = @( 'psDscAdapter/', 'RunCommandOnSet.dsc.resource.json', 'runcommandonset', - 'default_settings.v1.dsc.json', + 'dsc_default.settings.json', 'settings.dsc.json' ) @@ -82,7 +82,7 @@ $filesForMacPackage = @( 'psDscAdapter/', 'RunCommandOnSet.dsc.resource.json', 'runcommandonset', - 'default_settings.v1.dsc.json', + 'dsc_default.settings.json', 'settings.dsc.json' ) diff --git a/dsc/copy_files.txt b/dsc/copy_files.txt index 6f2f0cce..61ec9456 100644 --- a/dsc/copy_files.txt +++ b/dsc/copy_files.txt @@ -1,2 +1,2 @@ settings.dsc.json -default_settings.v1.dsc.json +dsc_default.settings.json diff --git a/dsc/default_settings.v1.dsc.json b/dsc/dsc_default.settings.json similarity index 100% rename from dsc/default_settings.v1.dsc.json rename to dsc/dsc_default.settings.json diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index 71b33469..78511d7f 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -12,7 +12,7 @@ Describe 'tests for dsc settings' { $script:dscHome = (Get-Command dsc).Path | Split-Path $script:dscSettingsFilePath = Join-Path $script:dscHome "dsc.settings.json" - $script:dscDefaultv1SettingsFilePath = Join-Path $script:dscHome "default_settings.v1.dsc.json" + $script:dscDefaultv1SettingsFilePath = Join-Path $script:dscHome "dsc_default.settings.json" $script:dscDefaultv1SettingsJson = Get-Content -Raw -Path $script:dscDefaultv1SettingsFilePath if ($IsWindows) { #"Setting policy on Linux requires sudo" @@ -22,7 +22,7 @@ Describe 'tests for dsc settings' { #create backups of settings files $script:dscSettingsFilePath_backup = Join-Path $script:dscHome "settings.dsc.json.backup" - $script:dscDefaultv1SettingsFilePath_backup = Join-Path $script:dscHome "default_settings.v1.dsc.json.backup" + $script:dscDefaultv1SettingsFilePath_backup = Join-Path $script:dscHome "dsc_default.settings.json.backup" Copy-Item -Force -Path $script:dscSettingsFilePath -Destination $script:dscSettingsFilePath_backup Copy-Item -Force -Path $script:dscDefaultv1SettingsFilePath -Destination $script:dscDefaultv1SettingsFilePath_backup } diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index 73fd1436..34120a71 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -72,7 +72,7 @@ pub fn get_setting(value_name: &str) -> Result { const SETTINGS_FILE_NAME: &str = "dsc.settings.json"; // Note that default settings file name has a version that is specific to this version of dsc - const DEFAULT_SETTINGS_FILE_NAME: &str = "default_settings.v1.dsc.json"; + const DEFAULT_SETTINGS_FILE_NAME: &str = "dsc_default.settings.json"; let mut result: DscSettingValue = DscSettingValue::default(); let mut settings_file_path : PathBuf; From 2b76fd1d498970f8b1b9c6e8a196beeba0a34ca0 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 10:13:50 -0700 Subject: [PATCH 18/26] updated schema in dsc_default.settings.json --- dsc/copy_files.txt | 2 +- dsc/dsc_default.settings.json | 20 +++++++++++--------- dsc_lib/src/util.rs | 11 +++++++---- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/dsc/copy_files.txt b/dsc/copy_files.txt index 61ec9456..ea3d9178 100644 --- a/dsc/copy_files.txt +++ b/dsc/copy_files.txt @@ -1,2 +1,2 @@ -settings.dsc.json +dsc.settings.json dsc_default.settings.json diff --git a/dsc/dsc_default.settings.json b/dsc/dsc_default.settings.json index 19acd63e..fe37d458 100644 --- a/dsc/dsc_default.settings.json +++ b/dsc/dsc_default.settings.json @@ -1,12 +1,14 @@ { - "resourcePath": { - "allowEnvOverride": true, - "appendEnvPath": true, - "directories": [] - }, - "tracing": { - "level": "WARN", - "format": "Default", - "allowOverride": true + "1": { + "resourcePath": { + "allowEnvOverride": true, + "appendEnvPath": true, + "directories": [] + }, + "tracing": { + "level": "WARN", + "format": "Default", + "allowOverride": true + } } } diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index 34120a71..95bfaafc 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -71,8 +71,9 @@ pub fn parse_input_to_json(value: &str) -> Result { pub fn get_setting(value_name: &str) -> Result { const SETTINGS_FILE_NAME: &str = "dsc.settings.json"; - // Note that default settings file name has a version that is specific to this version of dsc + // Note that default settings file has root nodes as settings schema version that is specific to this version of dsc const DEFAULT_SETTINGS_FILE_NAME: &str = "dsc_default.settings.json"; + const DEFAULT_SETTINGS_SCHEMA_VERSION: &str = "1"; let mut result: DscSettingValue = DscSettingValue::default(); let mut settings_file_path : PathBuf; @@ -80,9 +81,11 @@ pub fn get_setting(value_name: &str) -> Result { if let Some(exe_home) = env::current_exe()?.parent() { // First, get setting from the default settings file settings_file_path = exe_home.join(DEFAULT_SETTINGS_FILE_NAME); - if let Ok(v) = load_value_from_json(&settings_file_path, value_name) { - result.setting = v; - debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); + if let Ok(v) = load_value_from_json(&settings_file_path, DEFAULT_SETTINGS_SCHEMA_VERSION) { + if let Some(n) = v.get(value_name) { + result.setting = n.clone(); + debug!("Found setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); + } } else { debug!("Did not find setting '{}' in {}", &value_name, settings_file_path.to_string_lossy()); } From da94e7629cf8d212d055ef545cdce7612344bf7d Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 10:52:27 -0700 Subject: [PATCH 19/26] updated tests --- dsc/tests/dsc_settings.tests.ps1 | 46 +++++++++++++++++--------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index 78511d7f..d78d87fb 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -7,13 +7,12 @@ Describe 'tests for dsc settings' { $script:policyFilePath = if ($IsWindows) { Join-Path $env:ProgramData "dsc" "dsc.settings.json" } else { - "/etc/dsc/settings.dsc.json" + "/etc/dsc/dsc.settings.json" } $script:dscHome = (Get-Command dsc).Path | Split-Path $script:dscSettingsFilePath = Join-Path $script:dscHome "dsc.settings.json" - $script:dscDefaultv1SettingsFilePath = Join-Path $script:dscHome "dsc_default.settings.json" - $script:dscDefaultv1SettingsJson = Get-Content -Raw -Path $script:dscDefaultv1SettingsFilePath + $script:dscDefaultSettingsFilePath = Join-Path $script:dscHome "dsc_default.settings.json" if ($IsWindows) { #"Setting policy on Linux requires sudo" $script:policyDirPath = $script:policyFilePath | Split-Path @@ -21,23 +20,28 @@ Describe 'tests for dsc settings' { } #create backups of settings files - $script:dscSettingsFilePath_backup = Join-Path $script:dscHome "settings.dsc.json.backup" - $script:dscDefaultv1SettingsFilePath_backup = Join-Path $script:dscHome "dsc_default.settings.json.backup" + $script:dscSettingsFilePath_backup = Join-Path $script:dscHome "dsc.settings.json.backup" + $script:dscDefaultSettingsFilePath_backup = Join-Path $script:dscHome "dsc_default.settings.json.backup" Copy-Item -Force -Path $script:dscSettingsFilePath -Destination $script:dscSettingsFilePath_backup - Copy-Item -Force -Path $script:dscDefaultv1SettingsFilePath -Destination $script:dscDefaultv1SettingsFilePath_backup + Copy-Item -Force -Path $script:dscDefaultSettingsFilePath -Destination $script:dscDefaultSettingsFilePath_backup } AfterAll { Remove-Item -Force -Path $script:dscSettingsFilePath_backup - Remove-Item -Force -Path $script:dscDefaultv1SettingsFilePath_backup + Remove-Item -Force -Path $script:dscDefaultSettingsFilePath_backup if ($IsWindows) { #"Setting policy on Linux requires sudo" Remove-Item -Recurse -Force -Path $script:policyDirPath } } + BeforeEach { + $script:dscDefaultSettings = Get-Content -Raw -Path $script:dscDefaultSettingsFilePath_backup | ConvertFrom-Json + $script:dscDefaultv1Settings = (Get-Content -Raw -Path $script:dscDefaultSettingsFilePath_backup | ConvertFrom-Json)."1" + } + AfterEach { Copy-Item -Force -Path $script:dscSettingsFilePath_backup -Destination $script:dscSettingsFilePath - Copy-Item -Force -Path $script:dscDefaultv1SettingsFilePath_backup -Destination $script:dscDefaultv1SettingsFilePath + Copy-Item -Force -Path $script:dscDefaultSettingsFilePath_backup -Destination $script:dscDefaultSettingsFilePath if ($IsWindows) { #"Setting policy on Linux requires sudo" Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue } @@ -45,7 +49,8 @@ Describe 'tests for dsc settings' { It 'ensure a new tracing value in settings has effect' { - $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') | Set-Content -Force -Path $script:dscSettingsFilePath + $script:dscDefaultv1Settings."tracing"."level" = "TRACE" + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:dscSettingsFilePath dsc resource list 2> $TestDrive/tracing.txt "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Trace" @@ -53,10 +58,9 @@ Describe 'tests for dsc settings' { It 'ensure a new resource_path value in settings has effect' { - $script:dscDefaultv1SettingsJson.Replace('"directories": []', '"directories": ["TestDir1","TestDir2"]') | Set-Content -Force -Path $script:dscSettingsFilePath - copy-Item -Recurse -Force -Path $script:dscSettingsFilePath -Destination "C:\Temp\" + $script:dscDefaultv1Settings."resourcePath"."directories" = @("TestDir1","TestDir2") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:dscSettingsFilePath dsc -l debug resource list 2> $TestDrive/tracing.txt - copy-Item -Recurse -Force -Path $TestDrive/tracing.txt -Destination "C:\Temp\" $expectedString = 'Using Resource Path: "TestDir1'+[System.IO.Path]::PathSeparator+'TestDir2' "$TestDrive/tracing.txt" | Should -FileContentMatchExactly $expectedString } @@ -68,17 +72,17 @@ Describe 'tests for dsc settings' { return } - $v = $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') - $v = $v.Replace('"directories": []', '"directories": ["PolicyDir"]') - $v | Set-Content -Force -Path $script:policyFilePath + $script:dscDefaultv1Settings."tracing"."level" = "TRACE" + $script:dscDefaultv1Settings."resourcePath"."directories" = @("PolicyDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:policyFilePath - $v = $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') - $v = $v.Replace('"directories": []', '"directories": ["SettingsDir"]') - $v | Set-Content -Force -Path $script:dscSettingsFilePath + $script:dscDefaultv1Settings."tracing"."level" = "TRACE" + $script:dscDefaultv1Settings."resourcePath"."directories" = @("SettingsDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:dscSettingsFilePath - $v = $script:dscDefaultv1SettingsJson.Replace('"level": "WARN"', '"level": "TRACE"') - $v = $v.Replace('"directories": []', '"directories": ["Defaultv1SettingsDir"]') - $v | Set-Content -Force -Path $script:dscDefaultv1SettingsFilePath + $script:dscDefaultSettings."1"."tracing"."level" = "TRACE" + $script:dscDefaultSettings."1"."resourcePath"."directories" = @("Defaultv1SettingsDir") + $script:dscDefaultSettings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:dscDefaultSettingsFilePath # ensure policy overrides everything dsc -l debug resource list 2> $TestDrive/tracing.txt From 45399861a055d9d4a2027018418d40b2c1ece615 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 11:25:47 -0700 Subject: [PATCH 20/26] added default tracer --- dsc/src/util.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 8cfb0545..4abbf10f 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -296,6 +296,19 @@ pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &O let mut policy_is_used = false; let mut tracing_setting = TracingSetting::default(); + let default_filter = EnvFilter::try_from_default_env() + .or_else(|_| EnvFilter::try_new("warning")) + .unwrap_or_default() + .add_directive(Level::TRACE.into()); + let default_indicatif_layer = IndicatifLayer::new(); + let default_layer = tracing_subscriber::fmt::Layer::default().with_writer(default_indicatif_layer.get_stderr_writer()); + let default_fmt = default_layer + .with_ansi(true) + .with_level(true) + .boxed(); + let default_subscriber = tracing_subscriber::Registry::default().with(default_fmt).with(default_filter).with(default_indicatif_layer); + let default_guard = tracing::subscriber::set_default(default_subscriber); + // read setting/policy from files if let Ok(v) = get_setting("tracing") { if v.policy != serde_json::Value::Null { @@ -304,18 +317,18 @@ pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &O tracing_setting = v; policy_is_used = true; }, - Err(e) => { println!("{e}"); } + Err(e) => { error!("{e}"); } } } else if v.setting != serde_json::Value::Null { match serde_json::from_value::(v.setting) { Ok(v) => { tracing_setting = v; }, - Err(e) => { println!("{e}"); } + Err(e) => { error!("{e}"); } } } } else { - println!("Could not read 'tracing' setting"); + error!("Could not read 'tracing' setting"); } // override with DSC_TRACE_LEVEL env var if permitted @@ -392,6 +405,7 @@ pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &O let subscriber = tracing_subscriber::Registry::default().with(fmt).with(filter).with(indicatif_layer); + drop(default_guard); if tracing::subscriber::set_global_default(subscriber).is_err() { eprintln!("Unable to set global default tracing subscriber. Tracing is diabled."); } From 0fef98022729053079e346717b110bdc1a64f34d Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 11:36:22 -0700 Subject: [PATCH 21/26] style fix 1 --- dsc_lib/src/discovery/command_discovery.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 6896fe59..270128a2 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -111,11 +111,8 @@ impl CommandDiscovery { paths.append(&mut env::split_paths(&value).collect::>()); } else { for p in resource_path_setting.directories { - if let Ok(v) = PathBuf::from_str(&p) { - paths.push(v); - } else { - trace!("Can't parse path: '{p}'"); - }; + let Ok(v) = PathBuf::from_str(&p) else { return Err(DscError::Operation("Can't parse '{p}'".to_string()));}; + paths.push(v); } if resource_path_setting.append_env_path { From e98f65cd897ebd9d9f5db4a877b7a96290c78d4f Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 11:41:08 -0700 Subject: [PATCH 22/26] style fix 2 --- dsc/src/util.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 4abbf10f..85f27d4e 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -291,6 +291,7 @@ pub fn write_output(json: &str, format: &Option) { } } +#[allow(clippy::too_many_lines)] pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &Option) { let mut policy_is_used = false; From 35768c4dbd4b2f312c406038aa6366d6ae7ebd67 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 11:48:38 -0700 Subject: [PATCH 23/26] style fix 3 --- dsc_lib/src/discovery/command_discovery.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 270128a2..6d0e26a5 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -111,8 +111,8 @@ impl CommandDiscovery { paths.append(&mut env::split_paths(&value).collect::>()); } else { for p in resource_path_setting.directories { - let Ok(v) = PathBuf::from_str(&p) else { return Err(DscError::Operation("Can't parse '{p}'".to_string()));}; - paths.push(v); + let v = PathBuf::from_str(&p); + paths.push(v.unwrap_or_default()); } if resource_path_setting.append_env_path { From a590112fdb5a38d4f89bfd42d0483857619a09ac Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 12:15:30 -0700 Subject: [PATCH 24/26] style fix 4 --- dsc/src/util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 85f27d4e..e208d2be 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -298,9 +298,9 @@ pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &O let mut tracing_setting = TracingSetting::default(); let default_filter = EnvFilter::try_from_default_env() - .or_else(|_| EnvFilter::try_new("warning")) + .or_else(|_| EnvFilter::try_new("error")) .unwrap_or_default() - .add_directive(Level::TRACE.into()); + .add_directive(Level::ERROR.into()); let default_indicatif_layer = IndicatifLayer::new(); let default_layer = tracing_subscriber::fmt::Layer::default().with_writer(default_indicatif_layer.get_stderr_writer()); let default_fmt = default_layer From 7d5a4a056c7b4c47fda3df2c5efd17653dd33100 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 31 Oct 2024 13:52:55 -0700 Subject: [PATCH 25/26] new feedback --- build.ps1 | 6 +++--- dsc/src/util.rs | 4 ++-- dsc_lib/src/util.rs | 1 + 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/build.ps1 b/build.ps1 index 44900a98..9d1ad0a6 100644 --- a/build.ps1 +++ b/build.ps1 @@ -49,7 +49,7 @@ $filesForWindowsPackage = @( 'windows_baseline.dsc.yaml', 'windows_inventory.dsc.yaml' 'dsc_default.settings.json', - 'settings.dsc.json' + 'dsc.settings.json' ) $filesForLinuxPackage = @( @@ -66,7 +66,7 @@ $filesForLinuxPackage = @( 'RunCommandOnSet.dsc.resource.json', 'runcommandonset', 'dsc_default.settings.json', - 'settings.dsc.json' + 'dsc.settings.json' ) $filesForMacPackage = @( @@ -83,7 +83,7 @@ $filesForMacPackage = @( 'RunCommandOnSet.dsc.resource.json', 'runcommandonset', 'dsc_default.settings.json', - 'settings.dsc.json' + 'dsc.settings.json' ) # the list of files other than the binaries which need to be executable diff --git a/dsc/src/util.rs b/dsc/src/util.rs index e208d2be..cbdabcc1 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -298,9 +298,9 @@ pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &O let mut tracing_setting = TracingSetting::default(); let default_filter = EnvFilter::try_from_default_env() - .or_else(|_| EnvFilter::try_new("error")) + .or_else(|_| EnvFilter::try_new("warning")) .unwrap_or_default() - .add_directive(Level::ERROR.into()); + .add_directive(Level::WARN.into()); let default_indicatif_layer = IndicatifLayer::new(); let default_layer = tracing_subscriber::fmt::Layer::default().with_writer(default_indicatif_layer.get_stderr_writer()); let default_fmt = default_layer diff --git a/dsc_lib/src/util.rs b/dsc_lib/src/util.rs index 95bfaafc..177d9549 100644 --- a/dsc_lib/src/util.rs +++ b/dsc_lib/src/util.rs @@ -60,6 +60,7 @@ pub fn parse_input_to_json(value: &str) -> Result { /// Will search setting files for the specified setting. /// Performance implication: Use this function economically as every call opens/reads several config files. +/// TODO: cache the config /// /// # Arguments /// From 4085e32f58389c90c2c1a7f27e0ba144f0a4b181 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 1 Nov 2024 15:51:38 -0700 Subject: [PATCH 26/26] new feedback 2 --- dsc/src/util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index cbdabcc1..5a2d402f 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -298,7 +298,7 @@ pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &O let mut tracing_setting = TracingSetting::default(); let default_filter = EnvFilter::try_from_default_env() - .or_else(|_| EnvFilter::try_new("warning")) + .or_else(|_| EnvFilter::try_new("warn")) .unwrap_or_default() .add_directive(Level::WARN.into()); let default_indicatif_layer = IndicatifLayer::new(); @@ -370,7 +370,7 @@ pub fn enable_tracing(trace_level_arg: &Option, trace_format_arg: &O // enable tracing let filter = EnvFilter::try_from_default_env() - .or_else(|_| EnvFilter::try_new("warning")) + .or_else(|_| EnvFilter::try_new("warn")) .unwrap_or_default() .add_directive(tracing_level.into()); let indicatif_layer = IndicatifLayer::new();