From b13e60651d5e625cbef0babb737d6d6b616acd8c Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:27:47 +0200 Subject: [PATCH 1/5] Refactor settings management and tracing configuration --- dsc/locales/en-us.toml | 1 - dsc/src/args.rs | 9 +- dsc/src/util.rs | 56 +- dsc/tests/dsc_settings.tests.ps1 | 88 +++ lib/dsc-lib/locales/en-us.toml | 11 +- .../src/discovery/command_discovery.rs | 59 +- .../src/dscresources/command_resource.rs | 4 +- lib/dsc-lib/src/lib.rs | 1 + lib/dsc-lib/src/settings/mod.rs | 569 ++++++++++++++++++ lib/dsc-lib/src/util.rs | 117 +--- 10 files changed, 677 insertions(+), 238 deletions(-) create mode 100644 lib/dsc-lib/src/settings/mod.rs diff --git a/dsc/locales/en-us.toml b/dsc/locales/en-us.toml index 49e9b50dd..4bcf48070 100644 --- a/dsc/locales/en-us.toml +++ b/dsc/locales/en-us.toml @@ -158,7 +158,6 @@ jsonArrayNotSupported = "JSON array output format is only supported for `--all'" [util] failedToConvertJsonToString = "Failed to convert JSON to string" -failedToReadTracingSetting = "Could not read 'tracing' setting" invalidTraceLevel = "Default to 'warn', invalid DSC_TRACE_LEVEL value" failedToSetTracing = "Unable to set global default tracing subscriber. Tracing is disabled." readingInput = "Reading input from command line parameter" diff --git a/dsc/src/args.rs b/dsc/src/args.rs index 4d4622287..a778b92cd 100644 --- a/dsc/src/args.rs +++ b/dsc/src/args.rs @@ -34,14 +34,7 @@ pub enum ListOutputFormat { TableNoTruncate, } -#[derive(Debug, Clone, PartialEq, Eq, ValueEnum, Deserialize)] -pub enum TraceFormat { - Default, - Plaintext, - Json, - #[clap(hide = true)] - PassThrough, -} +pub use dsc_lib::settings::TraceFormat; #[derive(Debug, Parser)] #[clap(name = "dsc", version = env!("CARGO_PKG_VERSION"), about = t!("args.about").to_string(), long_about = None)] diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 51eb4596a..f0c2b7fcc 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -41,15 +41,12 @@ use dsc_lib::{ extension_manifest::ExtensionManifest, }, functions::FunctionDefinition, - util::{ - get_setting, - parse_input_to_json, - }, + settings::get_settings, + util::parse_input_to_json, }; use path_absolutize::Absolutize; use rust_i18n::t; use schemars::{Schema, schema_for}; -use serde::Deserialize; use std::collections::HashMap; use std::env; use std::io::{IsTerminal, Read, stdout, Write}; @@ -80,27 +77,6 @@ pub const EXIT_BICEP_FAILED: i32 = 10; pub const DSC_CONFIG_ROOT: &str = "DSC_CONFIG_ROOT"; 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 -} - -impl Default for TracingSetting { - fn default() -> TracingSetting { - TracingSetting { - level: TraceLevel::Warn, - format: TraceFormat::Default, - allow_override: true, - } - } -} - /// Get string representation of JSON value. /// /// # Arguments @@ -328,9 +304,6 @@ pub fn write_object(json: &str, format: Option<&OutputFormat>, include_separator #[allow(clippy::too_many_lines)] pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Option<&TraceFormat>) { - 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("warn")) .unwrap_or_default() @@ -344,27 +317,10 @@ pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Op 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 { - match serde_json::from_value::(v.policy) { - Ok(v) => { - tracing_setting = v; - policy_is_used = true; - }, - 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) => { error!("{e}"); } - } - } - } else { - error!("{}", t!("util.failedToReadTracingSetting")); - } + // read resolved settings/policy from files + let resolved_settings = get_settings(); + let mut tracing_setting = resolved_settings.tracing.value.clone(); + let policy_is_used = resolved_settings.tracing.is_policy(); // override with DSC_TRACE_LEVEL env var if permitted if tracing_setting.allow_override && let Ok(level) = env::var(DSC_TRACE_LEVEL) { diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index c4f6b7c56..ef1f850db 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -24,6 +24,8 @@ Describe 'tests for dsc settings' { $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:dscDefaultSettingsFilePath -Destination $script:dscDefaultSettingsFilePath_backup + + $script:originalXdgConfigHome = $env:XDG_CONFIG_HOME } AfterAll { @@ -45,6 +47,7 @@ Describe 'tests for dsc settings' { if ($IsWindows) { #"Setting policy on Linux requires sudo" Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue } + $env:XDG_CONFIG_HOME = $script:originalXdgConfigHome } It 'ensure a new tracing value in settings has effect' { @@ -106,4 +109,89 @@ Describe 'tests for dsc settings' { "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Trace" "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: Defaultv1SettingsDir' } + + It 'ensure a user settings file via XDG_CONFIG_HOME has effect' { + + $env:XDG_CONFIG_HOME = Join-Path $TestDrive 'xdg' + $userSettingsDir = Join-Path $env:XDG_CONFIG_HOME 'dsc' + New-Item -ItemType Directory -Path $userSettingsDir -Force | Out-Null + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("UserDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $userSettingsDir 'dsc.settings.json') + + dsc -l debug resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: UserDir' + } + + It 'ensure a workspace settings file has effect' { + + $workspaceDir = Join-Path $TestDrive 'workspace' + New-Item -ItemType Directory -Path $workspaceDir -Force | Out-Null + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("WorkspaceDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $workspaceDir 'dsc.settings.json') + + try { + Push-Location $workspaceDir + dsc -l debug resource list 2> $TestDrive/tracing.txt + } finally { + Pop-Location + } + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: WorkspaceDir' + } + + It 'ensure workspace settings override user and install settings' { + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("InstallDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:dscSettingsFilePath + + $env:XDG_CONFIG_HOME = Join-Path $TestDrive 'xdg' + $userSettingsDir = Join-Path $env:XDG_CONFIG_HOME 'dsc' + New-Item -ItemType Directory -Path $userSettingsDir -Force | Out-Null + $script:dscDefaultv1Settings."resourcePath"."directories" = @("UserDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $userSettingsDir 'dsc.settings.json') + + $workspaceDir = Join-Path $TestDrive 'workspace' + New-Item -ItemType Directory -Path $workspaceDir -Force | Out-Null + $script:dscDefaultv1Settings."resourcePath"."directories" = @("WorkspaceDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $workspaceDir 'dsc.settings.json') + + # user settings override install settings + dsc -l debug resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: UserDir' + + # workspace settings override user settings + try { + Push-Location $workspaceDir + dsc -l debug resource list 2> $TestDrive/tracing.txt + } finally { + Pop-Location + } + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: WorkspaceDir' + } + + It 'ensure policy overrides workspace settings' { + + if (! $IsWindows) { + Set-ItResult -Skip -Because "Setting policy requires sudo" + return + } + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("PolicyDir") + # only define resourcePath as policy so the tracing field stays overridable by '-l debug' + @{ resourcePath = $script:dscDefaultv1Settings."resourcePath" } | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:policyFilePath + + $workspaceDir = Join-Path $TestDrive 'workspace' + New-Item -ItemType Directory -Path $workspaceDir -Force | Out-Null + $script:dscDefaultv1Settings."resourcePath"."directories" = @("WorkspaceDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $workspaceDir 'dsc.settings.json') + + try { + Push-Location $workspaceDir + dsc -l debug resource list 2> $TestDrive/tracing.txt + } finally { + Pop-Location + } + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: PolicyDir' + } } diff --git a/lib/dsc-lib/locales/en-us.toml b/lib/dsc-lib/locales/en-us.toml index 968c2c674..e9273f11b 100644 --- a/lib/dsc-lib/locales/en-us.toml +++ b/lib/dsc-lib/locales/en-us.toml @@ -106,7 +106,6 @@ importingParametersFromInput = "Importing parameters from simple input" invalidParamsFormat = "Invalid parameters format: %{error}" [discovery.commandDiscovery] -couldNotReadSetting = "Could not read 'resourcePath' setting" appendingEnvPath = "Appending PATH to resourcePath" originalPath = "Original PATH: %{path}" failedGetEnvPath = "Failed to get PATH environment variable" @@ -923,11 +922,15 @@ utf16Conversion = "Failed to convert UTF-16 bytes to string" [progress] failedToSerialize = "Failed to serialize progress JSON: %{json}" +[settings] +failedToGetExePath = "Can't get 'dsc' executable path to locate settings files" +notFoundSettingsFile = "Settings file not found: %{path}" +loadedSettingsFile = "Loaded settings file: %{path}" +invalidSettingsFile = "Invalid settings file '%{path}': %{error}" +missingSettingsSchemaVersion = "Settings file '%{path}' is missing schema version key '%{version}'" + [util] -foundSetting = "Found setting '%{name}' in %{path}" -notFoundSetting = "Setting '%{name}' not found in %{path}" failedToGetExePath = "Can't get 'dsc' executable path" -settingNotFound = "Setting '%{name}' not found" failedToAbsolutizePath = "Failed to absolutize path '%{path}'" executableNotFoundInWorkingDirectory = "Executable '%{executable}' not found with working directory '%{cwd}'" executableNotFound = "Executable '%{executable}' not found" diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index 2fb07fd01..63a134282 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -13,6 +13,7 @@ use crate::extensions::dscextension::{self, DscExtension, Capability as Extensio use crate::extensions::extension_manifest::ExtensionManifest; use crate::progress::{ProgressBar, ProgressFormat}; use crate::schemas::transforms::idiomaticize_externally_tagged_enum; +use crate::settings::get_settings; use rust_i18n::t; use schemars::JsonSchema; use serde::Deserialize; @@ -25,7 +26,6 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use tracing::{debug, info, trace, warn}; -use crate::util::get_setting; use crate::util::{canonicalize_which, get_exe_path}; // NOTE: if new types of file extensions are added, ensure they are added to `process_discover_args` in `lib/dsc-lib/src/extensions/discover.rs` @@ -62,28 +62,6 @@ pub struct CommandDiscovery { discovery_mode: ResourceDiscoveryMode, } -#[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 -} - -impl Default for ResourcePathSetting { - fn default() -> ResourcePathSetting { - ResourcePathSetting { - allow_env_override: true, - append_env_path: true, - directories: vec![], - } - } -} - impl CommandDiscovery { #[must_use] pub fn new(progress_format: ProgressFormat) -> CommandDiscovery { @@ -96,42 +74,9 @@ impl CommandDiscovery { #[must_use] pub fn get_extensions(&self) -> DiscoveryExtensionCache { locked_clone!(EXTENSIONS) } - fn get_resource_path_setting() -> Result - { - 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::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::Setting(format!("{e}"))); } - } - } - } - - Err(DscError::Setting(t!("discovery.commandDiscovery.couldNotReadSetting").to_string())) - } - fn get_resource_paths() -> Result, DscError> { - let mut resource_path_setting = ResourcePathSetting::default(); - - match Self::get_resource_path_setting() { - Ok(v) => { - resource_path_setting = v; - }, - Err(e) => { - debug!("{e}"); - } - } + let resource_path_setting = get_settings().resource_path.value.clone(); let mut using_custom_path = false; let mut paths: Vec = vec![]; diff --git a/lib/dsc-lib/src/dscresources/command_resource.rs b/lib/dsc-lib/src/dscresources/command_resource.rs index 206069cfe..eafdda1cb 100644 --- a/lib/dsc-lib/src/dscresources/command_resource.rs +++ b/lib/dsc-lib/src/dscresources/command_resource.rs @@ -5,7 +5,7 @@ use clap::ValueEnum; use dsc_lib_security_context::{SecurityContext, get_security_context}; use jsonschema::Validator; use rust_i18n::t; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use std::{collections::HashMap, env, path::Path, process::Stdio}; use crate::{configure::{config_doc::{ExecutionKind, SecurityContextKind}, config_result::{ResourceGetResult, ResourceTestResult}}, dscresources::resource_manifest::{ExportSchemaKind, ExportSchemaOrFiltering, SchemaArgKind}, types::{ExitCodesMap}, util::canonicalize_which}; @@ -1305,7 +1305,7 @@ fn validate_security_context(required_security_context: &Option>>> = LazyLock::new(|| RwLock::new(None)); + +/// The format to use for trace output. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema, ValueEnum)] +pub enum TraceFormat { + /// Human-readable output with ANSI colors. + #[default] + Default, + /// Human-readable output without ANSI colors. + Plaintext, + /// Newline-delimited JSON objects. + Json, + /// Pass through trace messages from resources unmodified. + #[clap(hide = true)] + PassThrough, +} + +/// Setting controlling trace output for DSC. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct TracingSetting { + /// Trace level to use. + pub level: TraceLevel, + /// Trace format to use. + pub format: TraceFormat, + /// Whether `level` can be overridden by the `DSC_TRACE_LEVEL` environment variable + /// or the `--trace-level` CLI option. + pub allow_override: bool, +} + +impl Default for TracingSetting { + fn default() -> Self { + Self { + level: TraceLevel::Warn, + format: TraceFormat::Default, + allow_override: true, + } + } +} + +/// Setting controlling where DSC discovers resources. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ResourcePathSetting { + /// Whether to allow overriding with the `DSC_RESOURCE_PATH` environment variable. + pub allow_env_override: bool, + /// Whether to append the `PATH` environment variable to the list of resource directories. + pub append_env_path: bool, + /// Directories that DSC should search for non-built-in resources. + pub directories: Vec, +} + +impl Default for ResourcePathSetting { + fn default() -> Self { + Self { + allow_env_override: true, + append_env_path: true, + directories: vec![], + } + } +} + +/// The settings document a user or administrator can define in a settings file. +/// +/// Every field is optional; undefined fields fall back to lower-precedence scopes +/// or the code default. +#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct DscSettings { + /// Controls trace output. + #[serde(skip_serializing_if = "Option::is_none")] + pub tracing: Option, + /// Controls resource discovery paths. + #[serde(skip_serializing_if = "Option::is_none")] + pub resource_path: Option, +} + +/// The scope a resolved setting field was defined in. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub enum SettingsScope { + /// The code default; the field isn't defined in any settings file. + Default, + /// The built-in defaults file next to the executable. + BuiltIn, + /// The install settings file next to the executable. + Install, + /// The user settings file. + User, + /// The workspace settings file in the current directory. + Workspace, + /// The machine policy file. Fields defined as policy cannot be overridden. + Policy, +} + +/// A resolved setting field value with the scope it was defined in. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ResolvedField { + /// The resolved value for the field. + pub value: T, + /// The scope the value was defined in. + pub scope: SettingsScope, +} + +impl ResolvedField { + /// Returns true if the field is enforced by policy and must not be overridden + /// by environment variables or CLI options. + #[must_use] + pub fn is_policy(&self) -> bool { + self.scope == SettingsScope::Policy + } +} + +/// The fully-resolved settings for the current invocation. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ResolvedSettings { + /// The resolved tracing setting. + pub tracing: ResolvedField, + /// The resolved resource path setting. + pub resource_path: ResolvedField, +} + +impl Default for ResolvedSettings { + fn default() -> Self { + Self { + tracing: ResolvedField { + value: TracingSetting::default(), + scope: SettingsScope::Default, + }, + resource_path: ResolvedField { + value: ResourcePathSetting::default(), + scope: SettingsScope::Default, + }, + } + } +} + +impl ResolvedSettings { + /// Resolve settings from the given sources. + /// + /// Sources must be ordered from lowest to highest precedence. Each defined + /// top-level field in a higher-precedence source replaces the value from + /// lower-precedence sources. + #[must_use] + pub fn resolve(sources: &[(SettingsScope, DscSettings)]) -> Self { + let mut resolved = Self::default(); + for (scope, settings) in sources { + if let Some(tracing) = &settings.tracing { + resolved.tracing = ResolvedField { + value: tracing.clone(), + scope: *scope, + }; + } + if let Some(resource_path) = &settings.resource_path { + resolved.resource_path = ResolvedField { + value: resource_path.clone(), + scope: *scope, + }; + } + } + resolved + } +} + +/// Get the resolved settings for the current invocation. +/// +/// The settings files are read and resolved once; subsequent calls return the +/// cached result. +/// +/// # Panics +/// +/// Panics if the settings cache lock is poisoned. +#[must_use] +pub fn get_settings() -> Arc { + if let Some(settings) = RESOLVED_SETTINGS.read().unwrap().as_ref() { + return Arc::clone(settings); + } + + let resolved = Arc::new(ResolvedSettings::resolve(&gather_sources())); + *RESOLVED_SETTINGS.write().unwrap() = Some(Arc::clone(&resolved)); + resolved +} + +/// Gather the settings documents from all file scopes, ordered from lowest to +/// highest precedence. +fn gather_sources() -> Vec<(SettingsScope, DscSettings)> { + let mut sources = Vec::new(); + + if let Ok(exe_path) = get_exe_path() { + if let Some(exe_home) = exe_path.parent() { + if let Some(settings) = load_default_settings_file(&exe_home.join(DEFAULT_SETTINGS_FILE_NAME)) { + sources.push((SettingsScope::BuiltIn, settings)); + } + if let Some(settings) = load_settings_file(&exe_home.join(SETTINGS_FILE_NAME)) { + sources.push((SettingsScope::Install, settings)); + } + } + } else { + debug!("{}", t!("settings.failedToGetExePath")); + } + + if let Some(path) = user_settings_path() + && let Some(settings) = load_settings_file(&path) { + sources.push((SettingsScope::User, settings)); + } + + if let Some(settings) = load_settings_file(&workspace_settings_path()) { + sources.push((SettingsScope::Workspace, settings)); + } + + if let Some(path) = policy_settings_path() + && let Some(settings) = load_settings_file(&path) { + sources.push((SettingsScope::Policy, settings)); + } + + sources +} + +/// Load a settings document from a flat settings file. +/// +/// Returns `None` if the file doesn't exist or can't be parsed. Parse failures +/// are logged as warnings so a corrupt settings file doesn't break DSC. +fn load_settings_file(path: &Path) -> Option { + let Ok(file) = File::open(path) else { + debug!("{}", t!("settings.notFoundSettingsFile", path = path.to_string_lossy())); + return None; + }; + match serde_json::from_reader::<_, DscSettings>(BufReader::new(file)) { + Ok(settings) => { + debug!("{}", t!("settings.loadedSettingsFile", path = path.to_string_lossy())); + Some(settings) + }, + Err(err) => { + warn!("{}", t!("settings.invalidSettingsFile", path = path.to_string_lossy(), error = err)); + None + } + } +} + +/// Load the built-in defaults file, which nests the settings document under a +/// root key identifying the settings schema version. +fn load_default_settings_file(path: &Path) -> Option { + let Ok(file) = File::open(path) else { + debug!("{}", t!("settings.notFoundSettingsFile", path = path.to_string_lossy())); + return None; + }; + let root: serde_json::Value = match serde_json::from_reader(BufReader::new(file)) { + Ok(root) => root, + Err(err) => { + warn!("{}", t!("settings.invalidSettingsFile", path = path.to_string_lossy(), error = err)); + return None; + } + }; + let Some(document) = root.get(DEFAULT_SETTINGS_SCHEMA_VERSION) else { + warn!("{}", t!("settings.missingSettingsSchemaVersion", path = path.to_string_lossy(), version = DEFAULT_SETTINGS_SCHEMA_VERSION)); + return None; + }; + match serde_json::from_value::(document.clone()) { + Ok(settings) => { + debug!("{}", t!("settings.loadedSettingsFile", path = path.to_string_lossy())); + Some(settings) + }, + Err(err) => { + warn!("{}", t!("settings.invalidSettingsFile", path = path.to_string_lossy(), error = err)); + None + } + } +} + +/// Get the path to the machine policy settings file. +/// +/// The policy file location is writable only by administrators but readable by +/// all users: +/// +/// - **Windows**: `%PROGRAMDATA%\dsc\dsc.settings.json` +/// - **macOS**: `/Library/dsc/dsc.settings.json` +/// - **Linux**: `/etc/dsc/dsc.settings.json` +#[must_use] +pub fn policy_settings_path() -> Option { + #[cfg(target_os = "windows")] + { + let program_data = env::var_os("ProgramData")?; + return Some(Path::new(&program_data).join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(target_os = "macos")] + { + Some(Path::new("/Library").join("dsc").join(SETTINGS_FILE_NAME)) + } + #[cfg(not(any(target_os = "windows", target_os = "macos")))] + { + Some(Path::new("/etc").join("dsc").join(SETTINGS_FILE_NAME)) + } +} + +/// Get the path to the user settings file. +/// +/// If `XDG_CONFIG_HOME` is defined it's respected on every platform, otherwise +/// the platform convention is used: +/// +/// - **Windows**: `%APPDATA%\dsc\dsc.settings.json` +/// - **macOS**: `$HOME/Library/Application Support/dsc/dsc.settings.json` +/// - **Linux**: `$HOME/.config/dsc/dsc.settings.json` +#[must_use] +pub fn user_settings_path() -> Option { + if let Some(xdg_config_home) = env::var_os("XDG_CONFIG_HOME") { + return Some(Path::new(&xdg_config_home).join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(target_os = "windows")] + { + let app_data = env::var_os("APPDATA")?; + return Some(Path::new(&app_data).join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(target_os = "macos")] + { + let home = env::var_os("HOME")?; + return Some(Path::new(&home).join("Library").join("Application Support").join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(not(any(target_os = "windows", target_os = "macos")))] + { + let home = env::var_os("HOME")?; + return Some(Path::new(&home).join(".config").join("dsc").join(SETTINGS_FILE_NAME)); + } + #[allow(unreachable_code)] + None +} + +/// Get the path to the workspace settings file in the current directory. +#[must_use] +pub fn workspace_settings_path() -> PathBuf { + Path::new(".").join(SETTINGS_FILE_NAME) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + struct TempFile(PathBuf); + + impl TempFile { + fn new(name: &str, content: &str) -> Self { + let path = env::temp_dir().join(format!("dsc-settings-test-{}-{name}", std::process::id())); + fs::write(&path, content).unwrap(); + Self(path) + } + } + + impl Drop for TempFile { + fn drop(&mut self) { + let _ = fs::remove_file(&self.0); + } + } + + #[test] + fn resolve_with_no_sources_uses_defaults() { + let resolved = ResolvedSettings::resolve(&[]); + assert_eq!(resolved.tracing.scope, SettingsScope::Default); + assert_eq!(resolved.tracing.value, TracingSetting::default()); + assert_eq!(resolved.resource_path.scope, SettingsScope::Default); + assert_eq!(resolved.resource_path.value, ResourcePathSetting::default()); + } + + #[test] + fn lower_scope_setting_overrides_higher_scope() { + let install = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Info, + format: TraceFormat::Default, + allow_override: true, + }), + resource_path: None, + }; + let workspace = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Debug, + format: TraceFormat::Json, + allow_override: false, + }), + resource_path: None, + }; + let resolved = ResolvedSettings::resolve(&[ + (SettingsScope::Install, install), + (SettingsScope::Workspace, workspace), + ]); + assert_eq!(resolved.tracing.scope, SettingsScope::Workspace); + assert_eq!(resolved.tracing.value.level, TraceLevel::Debug); + assert_eq!(resolved.tracing.value.format, TraceFormat::Json); + assert!(!resolved.tracing.value.allow_override); + // undefined field falls back to the default + assert_eq!(resolved.resource_path.scope, SettingsScope::Default); + } + + #[test] + fn policy_overrides_all_settings() { + let workspace = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Debug, + format: TraceFormat::Json, + allow_override: true, + }), + resource_path: None, + }; + let policy = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Error, + format: TraceFormat::Plaintext, + allow_override: false, + }), + resource_path: None, + }; + let resolved = ResolvedSettings::resolve(&[ + (SettingsScope::Workspace, workspace), + (SettingsScope::Policy, policy), + ]); + assert!(resolved.tracing.is_policy()); + assert_eq!(resolved.tracing.value.level, TraceLevel::Error); + assert_eq!(resolved.tracing.value.format, TraceFormat::Plaintext); + } + + #[test] + fn fields_resolve_independently() { + let user = DscSettings { + tracing: Some(TracingSetting::default()), + resource_path: None, + }; + let workspace = DscSettings { + tracing: None, + resource_path: Some(ResourcePathSetting { + allow_env_override: false, + append_env_path: false, + directories: vec!["/custom".to_string()], + }), + }; + let resolved = ResolvedSettings::resolve(&[ + (SettingsScope::User, user), + (SettingsScope::Workspace, workspace), + ]); + assert_eq!(resolved.tracing.scope, SettingsScope::User); + assert_eq!(resolved.resource_path.scope, SettingsScope::Workspace); + assert_eq!(resolved.resource_path.value.directories, vec!["/custom".to_string()]); + } + + #[test] + fn settings_document_deserializes_camel_case() { + let json = r#"{ + "resourcePath": { + "allowEnvOverride": true, + "appendEnvPath": true, + "directories": [] + }, + "tracing": { + "level": "WARN", + "format": "Default", + "allowOverride": true + } + }"#; + let settings: DscSettings = serde_json::from_str(json).unwrap(); + assert_eq!(settings.tracing, Some(TracingSetting::default())); + assert_eq!(settings.resource_path, Some(ResourcePathSetting::default())); + } + + #[test] + fn settings_document_roundtrips() { + let settings = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Debug, + format: TraceFormat::Json, + allow_override: false, + }), + resource_path: Some(ResourcePathSetting { + allow_env_override: false, + append_env_path: false, + directories: vec!["/one".to_string(), "/two".to_string()], + }), + }; + let json = serde_json::to_value(&settings).unwrap(); + assert_eq!(json["tracing"]["level"], "DEBUG"); + assert_eq!(json["tracing"]["format"], "Json"); + assert_eq!(json["tracing"]["allowOverride"], false); + assert_eq!(json["resourcePath"]["allowEnvOverride"], false); + assert_eq!(json["resourcePath"]["appendEnvPath"], false); + assert_eq!(json["resourcePath"]["directories"][0], "/one"); + let roundtripped: DscSettings = serde_json::from_value(json).unwrap(); + assert_eq!(roundtripped, settings); + } + + #[test] + fn undefined_fields_deserialize_as_none() { + let settings: DscSettings = serde_json::from_str("{}").unwrap(); + assert_eq!(settings, DscSettings::default()); + // unknown fields are ignored so future settings don't break older versions + let settings: DscSettings = serde_json::from_str(r#"{"futureSetting": true}"#).unwrap(); + assert_eq!(settings, DscSettings::default()); + } + + #[test] + fn load_settings_file_reads_flat_document() { + let file = TempFile::new( + "flat.json", + r#"{"tracing": {"level": "INFO", "format": "Plaintext", "allowOverride": false}}"#, + ); + let settings = load_settings_file(&file.0).unwrap(); + let tracing = settings.tracing.unwrap(); + assert_eq!(tracing.level, TraceLevel::Info); + assert_eq!(tracing.format, TraceFormat::Plaintext); + assert!(!tracing.allow_override); + assert_eq!(settings.resource_path, None); + } + + #[test] + fn load_settings_file_returns_none_for_missing_or_invalid() { + assert!(load_settings_file(Path::new("nonexistent-dsc-settings.json")).is_none()); + let file = TempFile::new("invalid.json", "not json"); + assert!(load_settings_file(&file.0).is_none()); + // valid JSON but wrong shape is also rejected instead of panicking + let file = TempFile::new("wrong-shape.json", r#"{"tracing": "yes"}"#); + assert!(load_settings_file(&file.0).is_none()); + } + + #[test] + fn load_default_settings_file_requires_version_root() { + let file = TempFile::new( + "versioned.json", + r#"{"1": {"resourcePath": {"allowEnvOverride": false, "appendEnvPath": false, "directories": ["/default"]}}}"#, + ); + let settings = load_default_settings_file(&file.0).unwrap(); + let resource_path = settings.resource_path.unwrap(); + assert_eq!(resource_path.directories, vec!["/default".to_string()]); + + // a document without the expected version root is rejected + let file = TempFile::new("unversioned.json", r#"{"resourcePath": {}}"#); + assert!(load_default_settings_file(&file.0).is_none()); + } + + #[test] + fn get_settings_returns_cached_instance() { + let first = get_settings(); + let second = get_settings(); + assert!(Arc::ptr_eq(&first, &second)); + } +} diff --git a/lib/dsc-lib/src/util.rs b/lib/dsc-lib/src/util.rs index ba68e031c..0131b53cf 100644 --- a/lib/dsc-lib/src/util.rs +++ b/lib/dsc-lib/src/util.rs @@ -6,28 +6,12 @@ use rust_i18n::t; use serde_json::Value; use std::{ fs, - fs::{canonicalize, File}, - io::BufReader, + fs::canonicalize, path::{Path, PathBuf}, env, }; -use tracing::debug; use which::which; -pub struct DscSettingValue { - pub setting: Value, - pub policy: Value, -} - -impl Default for DscSettingValue { - fn default() -> DscSettingValue { - DscSettingValue { - setting: Value::Null, - policy: Value::Null, - } - } -} - /// Return JSON string whether the input is JSON or YAML /// /// # Arguments @@ -98,88 +82,6 @@ mod tests { } } -/// 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 -/// -/// * `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 = "dsc.settings.json"; - // 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; - - if let Some(exe_home) = get_exe_path()?.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, DEFAULT_SETTINGS_SCHEMA_VERSION) { - if let Some(n) = v.get(value_name) { - result.setting = n.clone(); - debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } - } else { - debug!("{}", t!("util.notFoundSetting", name = value_name, path = 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.setting = v; - debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } else { - debug!("{}", t!("util.notFoundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } - } else { - debug!("{}", t!("util.failedToGetExePath")); - } - - // 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.policy = v; - debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } else { - debug!("{}", t!("util.notFoundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } - - if (result.setting == serde_json::Value::Null) && (result.policy == serde_json::Value::Null) { - return Err(DscError::NotSupported(t!("util.settingNotFound", name = value_name).to_string())); - } - - Ok(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) { - Ok(j) => j, - Err(err) => { - return Err(DscError::Json(err)); - } - }; - - if let Some(r) = root.as_object() { - for (key, value) in r { - if *key == value_name { - return Ok(value.clone()) - } - } - } - - Err(DscError::NotSupported(value_name.to_string())) -} - /// Gets path to the current dsc process. /// If dsc is started using a symlink, this functon returns target of the symlink. /// @@ -198,23 +100,6 @@ pub fn get_exe_path() -> Result { Err(DscError::NotSupported(t!("util.failedToGetExePath").to_string())) } -#[cfg(target_os = "windows")] -fn get_settings_policy_file_path() -> String -{ - // $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("dsc.settings.json").display().to_string() -} - -#[cfg(not(target_os = "windows"))] -fn get_settings_policy_file_path() -> String -{ - // "/etc/dsc/dsc.settings.json" - // This location is writable only by admins, but readable by all users - Path::new("/etc").join("dsc").join("dsc.settings.json").display().to_string() -} - /// Generates a resource ID from the specified type and name. /// /// # Arguments From 94a46429385fff224e77726e40c2acb50e39fb97 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:23:44 +0200 Subject: [PATCH 2/5] Resolve Copilot remarks --- dsc/src/util.rs | 5 +++-- lib/dsc-lib/src/discovery/command_discovery.rs | 7 +++++-- lib/dsc-lib/src/settings/mod.rs | 5 ++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index f0c2b7fcc..de951e75d 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -322,8 +322,9 @@ pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Op let mut tracing_setting = resolved_settings.tracing.value.clone(); let policy_is_used = resolved_settings.tracing.is_policy(); - // override with DSC_TRACE_LEVEL env var if permitted - if tracing_setting.allow_override && let Ok(level) = env::var(DSC_TRACE_LEVEL) { + // override with DSC_TRACE_LEVEL env var if permitted; a policy-scoped tracing + // setting cannot be overridden by the environment variable + if !policy_is_used && tracing_setting.allow_override && let Ok(level) = env::var(DSC_TRACE_LEVEL) { tracing_setting.level = match level.to_ascii_uppercase().as_str() { "ERROR" => TraceLevel::Error, "WARN" => TraceLevel::Warn, diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index 63a134282..e3d2decd8 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -76,13 +76,16 @@ impl CommandDiscovery { fn get_resource_paths() -> Result, DscError> { - let resource_path_setting = get_settings().resource_path.value.clone(); + let resolved_resource_path = get_settings().resource_path.clone(); + // a policy-scoped resourcePath cannot be overridden by the environment variable + let allow_env_override = resolved_resource_path.value.allow_env_override && !resolved_resource_path.is_policy(); + let resource_path_setting = resolved_resource_path.value; let mut using_custom_path = false; let mut paths: Vec = vec![]; let dsc_resource_path = env::var_os("DSC_RESOURCE_PATH"); - if resource_path_setting.allow_env_override && dsc_resource_path.is_some() { + if allow_env_override && dsc_resource_path.is_some() { if let Some(value) = dsc_resource_path { debug!("DSC_RESOURCE_PATH: {:?}", value.to_string_lossy()); using_custom_path = true; diff --git a/lib/dsc-lib/src/settings/mod.rs b/lib/dsc-lib/src/settings/mod.rs index b49d46102..3b2046e34 100644 --- a/lib/dsc-lib/src/settings/mod.rs +++ b/lib/dsc-lib/src/settings/mod.rs @@ -49,8 +49,7 @@ pub struct TracingSetting { pub level: TraceLevel, /// Trace format to use. pub format: TraceFormat, - /// Whether `level` can be overridden by the `DSC_TRACE_LEVEL` environment variable - /// or the `--trace-level` CLI option. + /// Whether `level` can be overridden by the `DSC_TRACE_LEVEL` environment variable. pub allow_override: bool, } @@ -389,7 +388,7 @@ mod tests { } #[test] - fn lower_scope_setting_overrides_higher_scope() { + fn higher_scope_setting_overrides_lower_scope() { let install = DscSettings { tracing: Some(TracingSetting { level: TraceLevel::Info, From 2396fa681c7108d1dd135f64c851a9db11f357c1 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:27:47 +0200 Subject: [PATCH 3/5] Refactor settings management and tracing configuration --- dsc/locales/en-us.toml | 1 - dsc/src/args.rs | 9 +- dsc/src/util.rs | 56 +- dsc/tests/dsc_settings.tests.ps1 | 88 +++ lib/dsc-lib/locales/en-us.toml | 11 +- .../src/discovery/command_discovery.rs | 59 +- .../src/dscresources/command_resource.rs | 4 +- lib/dsc-lib/src/lib.rs | 1 + lib/dsc-lib/src/settings/mod.rs | 569 ++++++++++++++++++ lib/dsc-lib/src/util.rs | 117 +--- 10 files changed, 677 insertions(+), 238 deletions(-) create mode 100644 lib/dsc-lib/src/settings/mod.rs diff --git a/dsc/locales/en-us.toml b/dsc/locales/en-us.toml index 49e9b50dd..4bcf48070 100644 --- a/dsc/locales/en-us.toml +++ b/dsc/locales/en-us.toml @@ -158,7 +158,6 @@ jsonArrayNotSupported = "JSON array output format is only supported for `--all'" [util] failedToConvertJsonToString = "Failed to convert JSON to string" -failedToReadTracingSetting = "Could not read 'tracing' setting" invalidTraceLevel = "Default to 'warn', invalid DSC_TRACE_LEVEL value" failedToSetTracing = "Unable to set global default tracing subscriber. Tracing is disabled." readingInput = "Reading input from command line parameter" diff --git a/dsc/src/args.rs b/dsc/src/args.rs index 4d4622287..a778b92cd 100644 --- a/dsc/src/args.rs +++ b/dsc/src/args.rs @@ -34,14 +34,7 @@ pub enum ListOutputFormat { TableNoTruncate, } -#[derive(Debug, Clone, PartialEq, Eq, ValueEnum, Deserialize)] -pub enum TraceFormat { - Default, - Plaintext, - Json, - #[clap(hide = true)] - PassThrough, -} +pub use dsc_lib::settings::TraceFormat; #[derive(Debug, Parser)] #[clap(name = "dsc", version = env!("CARGO_PKG_VERSION"), about = t!("args.about").to_string(), long_about = None)] diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 51eb4596a..f0c2b7fcc 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -41,15 +41,12 @@ use dsc_lib::{ extension_manifest::ExtensionManifest, }, functions::FunctionDefinition, - util::{ - get_setting, - parse_input_to_json, - }, + settings::get_settings, + util::parse_input_to_json, }; use path_absolutize::Absolutize; use rust_i18n::t; use schemars::{Schema, schema_for}; -use serde::Deserialize; use std::collections::HashMap; use std::env; use std::io::{IsTerminal, Read, stdout, Write}; @@ -80,27 +77,6 @@ pub const EXIT_BICEP_FAILED: i32 = 10; pub const DSC_CONFIG_ROOT: &str = "DSC_CONFIG_ROOT"; 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 -} - -impl Default for TracingSetting { - fn default() -> TracingSetting { - TracingSetting { - level: TraceLevel::Warn, - format: TraceFormat::Default, - allow_override: true, - } - } -} - /// Get string representation of JSON value. /// /// # Arguments @@ -328,9 +304,6 @@ pub fn write_object(json: &str, format: Option<&OutputFormat>, include_separator #[allow(clippy::too_many_lines)] pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Option<&TraceFormat>) { - 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("warn")) .unwrap_or_default() @@ -344,27 +317,10 @@ pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Op 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 { - match serde_json::from_value::(v.policy) { - Ok(v) => { - tracing_setting = v; - policy_is_used = true; - }, - 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) => { error!("{e}"); } - } - } - } else { - error!("{}", t!("util.failedToReadTracingSetting")); - } + // read resolved settings/policy from files + let resolved_settings = get_settings(); + let mut tracing_setting = resolved_settings.tracing.value.clone(); + let policy_is_used = resolved_settings.tracing.is_policy(); // override with DSC_TRACE_LEVEL env var if permitted if tracing_setting.allow_override && let Ok(level) = env::var(DSC_TRACE_LEVEL) { diff --git a/dsc/tests/dsc_settings.tests.ps1 b/dsc/tests/dsc_settings.tests.ps1 index c4f6b7c56..ef1f850db 100644 --- a/dsc/tests/dsc_settings.tests.ps1 +++ b/dsc/tests/dsc_settings.tests.ps1 @@ -24,6 +24,8 @@ Describe 'tests for dsc settings' { $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:dscDefaultSettingsFilePath -Destination $script:dscDefaultSettingsFilePath_backup + + $script:originalXdgConfigHome = $env:XDG_CONFIG_HOME } AfterAll { @@ -45,6 +47,7 @@ Describe 'tests for dsc settings' { if ($IsWindows) { #"Setting policy on Linux requires sudo" Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue } + $env:XDG_CONFIG_HOME = $script:originalXdgConfigHome } It 'ensure a new tracing value in settings has effect' { @@ -106,4 +109,89 @@ Describe 'tests for dsc settings' { "$TestDrive/tracing.txt" | Should -FileContentMatchExactly "Trace-level is Trace" "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: Defaultv1SettingsDir' } + + It 'ensure a user settings file via XDG_CONFIG_HOME has effect' { + + $env:XDG_CONFIG_HOME = Join-Path $TestDrive 'xdg' + $userSettingsDir = Join-Path $env:XDG_CONFIG_HOME 'dsc' + New-Item -ItemType Directory -Path $userSettingsDir -Force | Out-Null + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("UserDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $userSettingsDir 'dsc.settings.json') + + dsc -l debug resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: UserDir' + } + + It 'ensure a workspace settings file has effect' { + + $workspaceDir = Join-Path $TestDrive 'workspace' + New-Item -ItemType Directory -Path $workspaceDir -Force | Out-Null + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("WorkspaceDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $workspaceDir 'dsc.settings.json') + + try { + Push-Location $workspaceDir + dsc -l debug resource list 2> $TestDrive/tracing.txt + } finally { + Pop-Location + } + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: WorkspaceDir' + } + + It 'ensure workspace settings override user and install settings' { + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("InstallDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:dscSettingsFilePath + + $env:XDG_CONFIG_HOME = Join-Path $TestDrive 'xdg' + $userSettingsDir = Join-Path $env:XDG_CONFIG_HOME 'dsc' + New-Item -ItemType Directory -Path $userSettingsDir -Force | Out-Null + $script:dscDefaultv1Settings."resourcePath"."directories" = @("UserDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $userSettingsDir 'dsc.settings.json') + + $workspaceDir = Join-Path $TestDrive 'workspace' + New-Item -ItemType Directory -Path $workspaceDir -Force | Out-Null + $script:dscDefaultv1Settings."resourcePath"."directories" = @("WorkspaceDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $workspaceDir 'dsc.settings.json') + + # user settings override install settings + dsc -l debug resource list 2> $TestDrive/tracing.txt + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: UserDir' + + # workspace settings override user settings + try { + Push-Location $workspaceDir + dsc -l debug resource list 2> $TestDrive/tracing.txt + } finally { + Pop-Location + } + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: WorkspaceDir' + } + + It 'ensure policy overrides workspace settings' { + + if (! $IsWindows) { + Set-ItResult -Skip -Because "Setting policy requires sudo" + return + } + + $script:dscDefaultv1Settings."resourcePath"."directories" = @("PolicyDir") + # only define resourcePath as policy so the tracing field stays overridable by '-l debug' + @{ resourcePath = $script:dscDefaultv1Settings."resourcePath" } | ConvertTo-Json -Depth 90 | Set-Content -Force -Path $script:policyFilePath + + $workspaceDir = Join-Path $TestDrive 'workspace' + New-Item -ItemType Directory -Path $workspaceDir -Force | Out-Null + $script:dscDefaultv1Settings."resourcePath"."directories" = @("WorkspaceDir") + $script:dscDefaultv1Settings | ConvertTo-Json -Depth 90 | Set-Content -Force -Path (Join-Path $workspaceDir 'dsc.settings.json') + + try { + Push-Location $workspaceDir + dsc -l debug resource list 2> $TestDrive/tracing.txt + } finally { + Pop-Location + } + "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Using Resource Path: PolicyDir' + } } diff --git a/lib/dsc-lib/locales/en-us.toml b/lib/dsc-lib/locales/en-us.toml index 968c2c674..e9273f11b 100644 --- a/lib/dsc-lib/locales/en-us.toml +++ b/lib/dsc-lib/locales/en-us.toml @@ -106,7 +106,6 @@ importingParametersFromInput = "Importing parameters from simple input" invalidParamsFormat = "Invalid parameters format: %{error}" [discovery.commandDiscovery] -couldNotReadSetting = "Could not read 'resourcePath' setting" appendingEnvPath = "Appending PATH to resourcePath" originalPath = "Original PATH: %{path}" failedGetEnvPath = "Failed to get PATH environment variable" @@ -923,11 +922,15 @@ utf16Conversion = "Failed to convert UTF-16 bytes to string" [progress] failedToSerialize = "Failed to serialize progress JSON: %{json}" +[settings] +failedToGetExePath = "Can't get 'dsc' executable path to locate settings files" +notFoundSettingsFile = "Settings file not found: %{path}" +loadedSettingsFile = "Loaded settings file: %{path}" +invalidSettingsFile = "Invalid settings file '%{path}': %{error}" +missingSettingsSchemaVersion = "Settings file '%{path}' is missing schema version key '%{version}'" + [util] -foundSetting = "Found setting '%{name}' in %{path}" -notFoundSetting = "Setting '%{name}' not found in %{path}" failedToGetExePath = "Can't get 'dsc' executable path" -settingNotFound = "Setting '%{name}' not found" failedToAbsolutizePath = "Failed to absolutize path '%{path}'" executableNotFoundInWorkingDirectory = "Executable '%{executable}' not found with working directory '%{cwd}'" executableNotFound = "Executable '%{executable}' not found" diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index 2fb07fd01..63a134282 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -13,6 +13,7 @@ use crate::extensions::dscextension::{self, DscExtension, Capability as Extensio use crate::extensions::extension_manifest::ExtensionManifest; use crate::progress::{ProgressBar, ProgressFormat}; use crate::schemas::transforms::idiomaticize_externally_tagged_enum; +use crate::settings::get_settings; use rust_i18n::t; use schemars::JsonSchema; use serde::Deserialize; @@ -25,7 +26,6 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use tracing::{debug, info, trace, warn}; -use crate::util::get_setting; use crate::util::{canonicalize_which, get_exe_path}; // NOTE: if new types of file extensions are added, ensure they are added to `process_discover_args` in `lib/dsc-lib/src/extensions/discover.rs` @@ -62,28 +62,6 @@ pub struct CommandDiscovery { discovery_mode: ResourceDiscoveryMode, } -#[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 -} - -impl Default for ResourcePathSetting { - fn default() -> ResourcePathSetting { - ResourcePathSetting { - allow_env_override: true, - append_env_path: true, - directories: vec![], - } - } -} - impl CommandDiscovery { #[must_use] pub fn new(progress_format: ProgressFormat) -> CommandDiscovery { @@ -96,42 +74,9 @@ impl CommandDiscovery { #[must_use] pub fn get_extensions(&self) -> DiscoveryExtensionCache { locked_clone!(EXTENSIONS) } - fn get_resource_path_setting() -> Result - { - 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::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::Setting(format!("{e}"))); } - } - } - } - - Err(DscError::Setting(t!("discovery.commandDiscovery.couldNotReadSetting").to_string())) - } - fn get_resource_paths() -> Result, DscError> { - let mut resource_path_setting = ResourcePathSetting::default(); - - match Self::get_resource_path_setting() { - Ok(v) => { - resource_path_setting = v; - }, - Err(e) => { - debug!("{e}"); - } - } + let resource_path_setting = get_settings().resource_path.value.clone(); let mut using_custom_path = false; let mut paths: Vec = vec![]; diff --git a/lib/dsc-lib/src/dscresources/command_resource.rs b/lib/dsc-lib/src/dscresources/command_resource.rs index 206069cfe..eafdda1cb 100644 --- a/lib/dsc-lib/src/dscresources/command_resource.rs +++ b/lib/dsc-lib/src/dscresources/command_resource.rs @@ -5,7 +5,7 @@ use clap::ValueEnum; use dsc_lib_security_context::{SecurityContext, get_security_context}; use jsonschema::Validator; use rust_i18n::t; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use std::{collections::HashMap, env, path::Path, process::Stdio}; use crate::{configure::{config_doc::{ExecutionKind, SecurityContextKind}, config_result::{ResourceGetResult, ResourceTestResult}}, dscresources::resource_manifest::{ExportSchemaKind, ExportSchemaOrFiltering, SchemaArgKind}, types::{ExitCodesMap}, util::canonicalize_which}; @@ -1305,7 +1305,7 @@ fn validate_security_context(required_security_context: &Option>>> = LazyLock::new(|| RwLock::new(None)); + +/// The format to use for trace output. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema, ValueEnum)] +pub enum TraceFormat { + /// Human-readable output with ANSI colors. + #[default] + Default, + /// Human-readable output without ANSI colors. + Plaintext, + /// Newline-delimited JSON objects. + Json, + /// Pass through trace messages from resources unmodified. + #[clap(hide = true)] + PassThrough, +} + +/// Setting controlling trace output for DSC. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct TracingSetting { + /// Trace level to use. + pub level: TraceLevel, + /// Trace format to use. + pub format: TraceFormat, + /// Whether `level` can be overridden by the `DSC_TRACE_LEVEL` environment variable + /// or the `--trace-level` CLI option. + pub allow_override: bool, +} + +impl Default for TracingSetting { + fn default() -> Self { + Self { + level: TraceLevel::Warn, + format: TraceFormat::Default, + allow_override: true, + } + } +} + +/// Setting controlling where DSC discovers resources. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ResourcePathSetting { + /// Whether to allow overriding with the `DSC_RESOURCE_PATH` environment variable. + pub allow_env_override: bool, + /// Whether to append the `PATH` environment variable to the list of resource directories. + pub append_env_path: bool, + /// Directories that DSC should search for non-built-in resources. + pub directories: Vec, +} + +impl Default for ResourcePathSetting { + fn default() -> Self { + Self { + allow_env_override: true, + append_env_path: true, + directories: vec![], + } + } +} + +/// The settings document a user or administrator can define in a settings file. +/// +/// Every field is optional; undefined fields fall back to lower-precedence scopes +/// or the code default. +#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct DscSettings { + /// Controls trace output. + #[serde(skip_serializing_if = "Option::is_none")] + pub tracing: Option, + /// Controls resource discovery paths. + #[serde(skip_serializing_if = "Option::is_none")] + pub resource_path: Option, +} + +/// The scope a resolved setting field was defined in. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub enum SettingsScope { + /// The code default; the field isn't defined in any settings file. + Default, + /// The built-in defaults file next to the executable. + BuiltIn, + /// The install settings file next to the executable. + Install, + /// The user settings file. + User, + /// The workspace settings file in the current directory. + Workspace, + /// The machine policy file. Fields defined as policy cannot be overridden. + Policy, +} + +/// A resolved setting field value with the scope it was defined in. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ResolvedField { + /// The resolved value for the field. + pub value: T, + /// The scope the value was defined in. + pub scope: SettingsScope, +} + +impl ResolvedField { + /// Returns true if the field is enforced by policy and must not be overridden + /// by environment variables or CLI options. + #[must_use] + pub fn is_policy(&self) -> bool { + self.scope == SettingsScope::Policy + } +} + +/// The fully-resolved settings for the current invocation. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ResolvedSettings { + /// The resolved tracing setting. + pub tracing: ResolvedField, + /// The resolved resource path setting. + pub resource_path: ResolvedField, +} + +impl Default for ResolvedSettings { + fn default() -> Self { + Self { + tracing: ResolvedField { + value: TracingSetting::default(), + scope: SettingsScope::Default, + }, + resource_path: ResolvedField { + value: ResourcePathSetting::default(), + scope: SettingsScope::Default, + }, + } + } +} + +impl ResolvedSettings { + /// Resolve settings from the given sources. + /// + /// Sources must be ordered from lowest to highest precedence. Each defined + /// top-level field in a higher-precedence source replaces the value from + /// lower-precedence sources. + #[must_use] + pub fn resolve(sources: &[(SettingsScope, DscSettings)]) -> Self { + let mut resolved = Self::default(); + for (scope, settings) in sources { + if let Some(tracing) = &settings.tracing { + resolved.tracing = ResolvedField { + value: tracing.clone(), + scope: *scope, + }; + } + if let Some(resource_path) = &settings.resource_path { + resolved.resource_path = ResolvedField { + value: resource_path.clone(), + scope: *scope, + }; + } + } + resolved + } +} + +/// Get the resolved settings for the current invocation. +/// +/// The settings files are read and resolved once; subsequent calls return the +/// cached result. +/// +/// # Panics +/// +/// Panics if the settings cache lock is poisoned. +#[must_use] +pub fn get_settings() -> Arc { + if let Some(settings) = RESOLVED_SETTINGS.read().unwrap().as_ref() { + return Arc::clone(settings); + } + + let resolved = Arc::new(ResolvedSettings::resolve(&gather_sources())); + *RESOLVED_SETTINGS.write().unwrap() = Some(Arc::clone(&resolved)); + resolved +} + +/// Gather the settings documents from all file scopes, ordered from lowest to +/// highest precedence. +fn gather_sources() -> Vec<(SettingsScope, DscSettings)> { + let mut sources = Vec::new(); + + if let Ok(exe_path) = get_exe_path() { + if let Some(exe_home) = exe_path.parent() { + if let Some(settings) = load_default_settings_file(&exe_home.join(DEFAULT_SETTINGS_FILE_NAME)) { + sources.push((SettingsScope::BuiltIn, settings)); + } + if let Some(settings) = load_settings_file(&exe_home.join(SETTINGS_FILE_NAME)) { + sources.push((SettingsScope::Install, settings)); + } + } + } else { + debug!("{}", t!("settings.failedToGetExePath")); + } + + if let Some(path) = user_settings_path() + && let Some(settings) = load_settings_file(&path) { + sources.push((SettingsScope::User, settings)); + } + + if let Some(settings) = load_settings_file(&workspace_settings_path()) { + sources.push((SettingsScope::Workspace, settings)); + } + + if let Some(path) = policy_settings_path() + && let Some(settings) = load_settings_file(&path) { + sources.push((SettingsScope::Policy, settings)); + } + + sources +} + +/// Load a settings document from a flat settings file. +/// +/// Returns `None` if the file doesn't exist or can't be parsed. Parse failures +/// are logged as warnings so a corrupt settings file doesn't break DSC. +fn load_settings_file(path: &Path) -> Option { + let Ok(file) = File::open(path) else { + debug!("{}", t!("settings.notFoundSettingsFile", path = path.to_string_lossy())); + return None; + }; + match serde_json::from_reader::<_, DscSettings>(BufReader::new(file)) { + Ok(settings) => { + debug!("{}", t!("settings.loadedSettingsFile", path = path.to_string_lossy())); + Some(settings) + }, + Err(err) => { + warn!("{}", t!("settings.invalidSettingsFile", path = path.to_string_lossy(), error = err)); + None + } + } +} + +/// Load the built-in defaults file, which nests the settings document under a +/// root key identifying the settings schema version. +fn load_default_settings_file(path: &Path) -> Option { + let Ok(file) = File::open(path) else { + debug!("{}", t!("settings.notFoundSettingsFile", path = path.to_string_lossy())); + return None; + }; + let root: serde_json::Value = match serde_json::from_reader(BufReader::new(file)) { + Ok(root) => root, + Err(err) => { + warn!("{}", t!("settings.invalidSettingsFile", path = path.to_string_lossy(), error = err)); + return None; + } + }; + let Some(document) = root.get(DEFAULT_SETTINGS_SCHEMA_VERSION) else { + warn!("{}", t!("settings.missingSettingsSchemaVersion", path = path.to_string_lossy(), version = DEFAULT_SETTINGS_SCHEMA_VERSION)); + return None; + }; + match serde_json::from_value::(document.clone()) { + Ok(settings) => { + debug!("{}", t!("settings.loadedSettingsFile", path = path.to_string_lossy())); + Some(settings) + }, + Err(err) => { + warn!("{}", t!("settings.invalidSettingsFile", path = path.to_string_lossy(), error = err)); + None + } + } +} + +/// Get the path to the machine policy settings file. +/// +/// The policy file location is writable only by administrators but readable by +/// all users: +/// +/// - **Windows**: `%PROGRAMDATA%\dsc\dsc.settings.json` +/// - **macOS**: `/Library/dsc/dsc.settings.json` +/// - **Linux**: `/etc/dsc/dsc.settings.json` +#[must_use] +pub fn policy_settings_path() -> Option { + #[cfg(target_os = "windows")] + { + let program_data = env::var_os("ProgramData")?; + return Some(Path::new(&program_data).join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(target_os = "macos")] + { + Some(Path::new("/Library").join("dsc").join(SETTINGS_FILE_NAME)) + } + #[cfg(not(any(target_os = "windows", target_os = "macos")))] + { + Some(Path::new("/etc").join("dsc").join(SETTINGS_FILE_NAME)) + } +} + +/// Get the path to the user settings file. +/// +/// If `XDG_CONFIG_HOME` is defined it's respected on every platform, otherwise +/// the platform convention is used: +/// +/// - **Windows**: `%APPDATA%\dsc\dsc.settings.json` +/// - **macOS**: `$HOME/Library/Application Support/dsc/dsc.settings.json` +/// - **Linux**: `$HOME/.config/dsc/dsc.settings.json` +#[must_use] +pub fn user_settings_path() -> Option { + if let Some(xdg_config_home) = env::var_os("XDG_CONFIG_HOME") { + return Some(Path::new(&xdg_config_home).join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(target_os = "windows")] + { + let app_data = env::var_os("APPDATA")?; + return Some(Path::new(&app_data).join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(target_os = "macos")] + { + let home = env::var_os("HOME")?; + return Some(Path::new(&home).join("Library").join("Application Support").join("dsc").join(SETTINGS_FILE_NAME)); + } + #[cfg(not(any(target_os = "windows", target_os = "macos")))] + { + let home = env::var_os("HOME")?; + return Some(Path::new(&home).join(".config").join("dsc").join(SETTINGS_FILE_NAME)); + } + #[allow(unreachable_code)] + None +} + +/// Get the path to the workspace settings file in the current directory. +#[must_use] +pub fn workspace_settings_path() -> PathBuf { + Path::new(".").join(SETTINGS_FILE_NAME) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + struct TempFile(PathBuf); + + impl TempFile { + fn new(name: &str, content: &str) -> Self { + let path = env::temp_dir().join(format!("dsc-settings-test-{}-{name}", std::process::id())); + fs::write(&path, content).unwrap(); + Self(path) + } + } + + impl Drop for TempFile { + fn drop(&mut self) { + let _ = fs::remove_file(&self.0); + } + } + + #[test] + fn resolve_with_no_sources_uses_defaults() { + let resolved = ResolvedSettings::resolve(&[]); + assert_eq!(resolved.tracing.scope, SettingsScope::Default); + assert_eq!(resolved.tracing.value, TracingSetting::default()); + assert_eq!(resolved.resource_path.scope, SettingsScope::Default); + assert_eq!(resolved.resource_path.value, ResourcePathSetting::default()); + } + + #[test] + fn lower_scope_setting_overrides_higher_scope() { + let install = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Info, + format: TraceFormat::Default, + allow_override: true, + }), + resource_path: None, + }; + let workspace = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Debug, + format: TraceFormat::Json, + allow_override: false, + }), + resource_path: None, + }; + let resolved = ResolvedSettings::resolve(&[ + (SettingsScope::Install, install), + (SettingsScope::Workspace, workspace), + ]); + assert_eq!(resolved.tracing.scope, SettingsScope::Workspace); + assert_eq!(resolved.tracing.value.level, TraceLevel::Debug); + assert_eq!(resolved.tracing.value.format, TraceFormat::Json); + assert!(!resolved.tracing.value.allow_override); + // undefined field falls back to the default + assert_eq!(resolved.resource_path.scope, SettingsScope::Default); + } + + #[test] + fn policy_overrides_all_settings() { + let workspace = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Debug, + format: TraceFormat::Json, + allow_override: true, + }), + resource_path: None, + }; + let policy = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Error, + format: TraceFormat::Plaintext, + allow_override: false, + }), + resource_path: None, + }; + let resolved = ResolvedSettings::resolve(&[ + (SettingsScope::Workspace, workspace), + (SettingsScope::Policy, policy), + ]); + assert!(resolved.tracing.is_policy()); + assert_eq!(resolved.tracing.value.level, TraceLevel::Error); + assert_eq!(resolved.tracing.value.format, TraceFormat::Plaintext); + } + + #[test] + fn fields_resolve_independently() { + let user = DscSettings { + tracing: Some(TracingSetting::default()), + resource_path: None, + }; + let workspace = DscSettings { + tracing: None, + resource_path: Some(ResourcePathSetting { + allow_env_override: false, + append_env_path: false, + directories: vec!["/custom".to_string()], + }), + }; + let resolved = ResolvedSettings::resolve(&[ + (SettingsScope::User, user), + (SettingsScope::Workspace, workspace), + ]); + assert_eq!(resolved.tracing.scope, SettingsScope::User); + assert_eq!(resolved.resource_path.scope, SettingsScope::Workspace); + assert_eq!(resolved.resource_path.value.directories, vec!["/custom".to_string()]); + } + + #[test] + fn settings_document_deserializes_camel_case() { + let json = r#"{ + "resourcePath": { + "allowEnvOverride": true, + "appendEnvPath": true, + "directories": [] + }, + "tracing": { + "level": "WARN", + "format": "Default", + "allowOverride": true + } + }"#; + let settings: DscSettings = serde_json::from_str(json).unwrap(); + assert_eq!(settings.tracing, Some(TracingSetting::default())); + assert_eq!(settings.resource_path, Some(ResourcePathSetting::default())); + } + + #[test] + fn settings_document_roundtrips() { + let settings = DscSettings { + tracing: Some(TracingSetting { + level: TraceLevel::Debug, + format: TraceFormat::Json, + allow_override: false, + }), + resource_path: Some(ResourcePathSetting { + allow_env_override: false, + append_env_path: false, + directories: vec!["/one".to_string(), "/two".to_string()], + }), + }; + let json = serde_json::to_value(&settings).unwrap(); + assert_eq!(json["tracing"]["level"], "DEBUG"); + assert_eq!(json["tracing"]["format"], "Json"); + assert_eq!(json["tracing"]["allowOverride"], false); + assert_eq!(json["resourcePath"]["allowEnvOverride"], false); + assert_eq!(json["resourcePath"]["appendEnvPath"], false); + assert_eq!(json["resourcePath"]["directories"][0], "/one"); + let roundtripped: DscSettings = serde_json::from_value(json).unwrap(); + assert_eq!(roundtripped, settings); + } + + #[test] + fn undefined_fields_deserialize_as_none() { + let settings: DscSettings = serde_json::from_str("{}").unwrap(); + assert_eq!(settings, DscSettings::default()); + // unknown fields are ignored so future settings don't break older versions + let settings: DscSettings = serde_json::from_str(r#"{"futureSetting": true}"#).unwrap(); + assert_eq!(settings, DscSettings::default()); + } + + #[test] + fn load_settings_file_reads_flat_document() { + let file = TempFile::new( + "flat.json", + r#"{"tracing": {"level": "INFO", "format": "Plaintext", "allowOverride": false}}"#, + ); + let settings = load_settings_file(&file.0).unwrap(); + let tracing = settings.tracing.unwrap(); + assert_eq!(tracing.level, TraceLevel::Info); + assert_eq!(tracing.format, TraceFormat::Plaintext); + assert!(!tracing.allow_override); + assert_eq!(settings.resource_path, None); + } + + #[test] + fn load_settings_file_returns_none_for_missing_or_invalid() { + assert!(load_settings_file(Path::new("nonexistent-dsc-settings.json")).is_none()); + let file = TempFile::new("invalid.json", "not json"); + assert!(load_settings_file(&file.0).is_none()); + // valid JSON but wrong shape is also rejected instead of panicking + let file = TempFile::new("wrong-shape.json", r#"{"tracing": "yes"}"#); + assert!(load_settings_file(&file.0).is_none()); + } + + #[test] + fn load_default_settings_file_requires_version_root() { + let file = TempFile::new( + "versioned.json", + r#"{"1": {"resourcePath": {"allowEnvOverride": false, "appendEnvPath": false, "directories": ["/default"]}}}"#, + ); + let settings = load_default_settings_file(&file.0).unwrap(); + let resource_path = settings.resource_path.unwrap(); + assert_eq!(resource_path.directories, vec!["/default".to_string()]); + + // a document without the expected version root is rejected + let file = TempFile::new("unversioned.json", r#"{"resourcePath": {}}"#); + assert!(load_default_settings_file(&file.0).is_none()); + } + + #[test] + fn get_settings_returns_cached_instance() { + let first = get_settings(); + let second = get_settings(); + assert!(Arc::ptr_eq(&first, &second)); + } +} diff --git a/lib/dsc-lib/src/util.rs b/lib/dsc-lib/src/util.rs index ba68e031c..0131b53cf 100644 --- a/lib/dsc-lib/src/util.rs +++ b/lib/dsc-lib/src/util.rs @@ -6,28 +6,12 @@ use rust_i18n::t; use serde_json::Value; use std::{ fs, - fs::{canonicalize, File}, - io::BufReader, + fs::canonicalize, path::{Path, PathBuf}, env, }; -use tracing::debug; use which::which; -pub struct DscSettingValue { - pub setting: Value, - pub policy: Value, -} - -impl Default for DscSettingValue { - fn default() -> DscSettingValue { - DscSettingValue { - setting: Value::Null, - policy: Value::Null, - } - } -} - /// Return JSON string whether the input is JSON or YAML /// /// # Arguments @@ -98,88 +82,6 @@ mod tests { } } -/// 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 -/// -/// * `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 = "dsc.settings.json"; - // 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; - - if let Some(exe_home) = get_exe_path()?.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, DEFAULT_SETTINGS_SCHEMA_VERSION) { - if let Some(n) = v.get(value_name) { - result.setting = n.clone(); - debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } - } else { - debug!("{}", t!("util.notFoundSetting", name = value_name, path = 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.setting = v; - debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } else { - debug!("{}", t!("util.notFoundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } - } else { - debug!("{}", t!("util.failedToGetExePath")); - } - - // 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.policy = v; - debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } else { - debug!("{}", t!("util.notFoundSetting", name = value_name, path = settings_file_path.to_string_lossy())); - } - - if (result.setting == serde_json::Value::Null) && (result.policy == serde_json::Value::Null) { - return Err(DscError::NotSupported(t!("util.settingNotFound", name = value_name).to_string())); - } - - Ok(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) { - Ok(j) => j, - Err(err) => { - return Err(DscError::Json(err)); - } - }; - - if let Some(r) = root.as_object() { - for (key, value) in r { - if *key == value_name { - return Ok(value.clone()) - } - } - } - - Err(DscError::NotSupported(value_name.to_string())) -} - /// Gets path to the current dsc process. /// If dsc is started using a symlink, this functon returns target of the symlink. /// @@ -198,23 +100,6 @@ pub fn get_exe_path() -> Result { Err(DscError::NotSupported(t!("util.failedToGetExePath").to_string())) } -#[cfg(target_os = "windows")] -fn get_settings_policy_file_path() -> String -{ - // $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("dsc.settings.json").display().to_string() -} - -#[cfg(not(target_os = "windows"))] -fn get_settings_policy_file_path() -> String -{ - // "/etc/dsc/dsc.settings.json" - // This location is writable only by admins, but readable by all users - Path::new("/etc").join("dsc").join("dsc.settings.json").display().to_string() -} - /// Generates a resource ID from the specified type and name. /// /// # Arguments From 5ce9a972e8ff46408da006c2e2380b2c961cfa85 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:23:44 +0200 Subject: [PATCH 4/5] Resolve Copilot remarks --- dsc/src/util.rs | 5 +++-- lib/dsc-lib/src/discovery/command_discovery.rs | 7 +++++-- lib/dsc-lib/src/settings/mod.rs | 5 ++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/dsc/src/util.rs b/dsc/src/util.rs index f0c2b7fcc..de951e75d 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -322,8 +322,9 @@ pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Op let mut tracing_setting = resolved_settings.tracing.value.clone(); let policy_is_used = resolved_settings.tracing.is_policy(); - // override with DSC_TRACE_LEVEL env var if permitted - if tracing_setting.allow_override && let Ok(level) = env::var(DSC_TRACE_LEVEL) { + // override with DSC_TRACE_LEVEL env var if permitted; a policy-scoped tracing + // setting cannot be overridden by the environment variable + if !policy_is_used && tracing_setting.allow_override && let Ok(level) = env::var(DSC_TRACE_LEVEL) { tracing_setting.level = match level.to_ascii_uppercase().as_str() { "ERROR" => TraceLevel::Error, "WARN" => TraceLevel::Warn, diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index 63a134282..e3d2decd8 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -76,13 +76,16 @@ impl CommandDiscovery { fn get_resource_paths() -> Result, DscError> { - let resource_path_setting = get_settings().resource_path.value.clone(); + let resolved_resource_path = get_settings().resource_path.clone(); + // a policy-scoped resourcePath cannot be overridden by the environment variable + let allow_env_override = resolved_resource_path.value.allow_env_override && !resolved_resource_path.is_policy(); + let resource_path_setting = resolved_resource_path.value; let mut using_custom_path = false; let mut paths: Vec = vec![]; let dsc_resource_path = env::var_os("DSC_RESOURCE_PATH"); - if resource_path_setting.allow_env_override && dsc_resource_path.is_some() { + if allow_env_override && dsc_resource_path.is_some() { if let Some(value) = dsc_resource_path { debug!("DSC_RESOURCE_PATH: {:?}", value.to_string_lossy()); using_custom_path = true; diff --git a/lib/dsc-lib/src/settings/mod.rs b/lib/dsc-lib/src/settings/mod.rs index b49d46102..3b2046e34 100644 --- a/lib/dsc-lib/src/settings/mod.rs +++ b/lib/dsc-lib/src/settings/mod.rs @@ -49,8 +49,7 @@ pub struct TracingSetting { pub level: TraceLevel, /// Trace format to use. pub format: TraceFormat, - /// Whether `level` can be overridden by the `DSC_TRACE_LEVEL` environment variable - /// or the `--trace-level` CLI option. + /// Whether `level` can be overridden by the `DSC_TRACE_LEVEL` environment variable. pub allow_override: bool, } @@ -389,7 +388,7 @@ mod tests { } #[test] - fn lower_scope_setting_overrides_higher_scope() { + fn higher_scope_setting_overrides_lower_scope() { let install = DscSettings { tracing: Some(TracingSetting { level: TraceLevel::Info, From 7894131d50d3f1f98c64ff5e577d8590c6621d51 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Sat, 11 Jul 2026 07:21:56 +0200 Subject: [PATCH 5/5] Fix path --- lib/dsc-lib/src/settings/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dsc-lib/src/settings/mod.rs b/lib/dsc-lib/src/settings/mod.rs index 3b2046e34..0bcddb9d4 100644 --- a/lib/dsc-lib/src/settings/mod.rs +++ b/lib/dsc-lib/src/settings/mod.rs @@ -307,7 +307,7 @@ pub fn policy_settings_path() -> Option { #[cfg(target_os = "windows")] { let program_data = env::var_os("ProgramData")?; - return Some(Path::new(&program_data).join("dsc").join(SETTINGS_FILE_NAME)); + Some(Path::new(&program_data).join("dsc").join(SETTINGS_FILE_NAME)) } #[cfg(target_os = "macos")] {