diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index 57d85cdc0..fab5f0d1a 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -263,7 +263,7 @@ fn initialize_config_root(path: Option<&String>) -> Option { } else { let current_directory = std::env::current_dir().unwrap_or_default(); debug!("DSC_CONFIG_ROOT = {} '{current_directory:?}'", t!("subcommand.currentDirectory")); - set_dscconfigroot(¤t_directory.to_string_lossy()); + set_dscconfigroot(current_directory.to_str().unwrap_or_default()); } // if the path is "-", we need to return it so later processing can handle it correctly diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 7d2d07d1e..d7f8f26fb 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -473,8 +473,9 @@ pub fn get_input(input: Option<&String>, file: Option<&String>, parameters_from_ } else { // see if an extension should handle this file let mut discovery = Discovery::new(); + let path_buf = Path::new(path); for extension in discovery.get_extensions(&Capability::Import) { - if let Ok(content) = extension.import(path) { + if let Ok(content) = extension.import(path_buf) { return content; } } diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index cc8101d57..1c6f9d845 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -751,8 +751,8 @@ fn load_resource_manifest(path: &Path, manifest: &ResourceManifest) -> Result Result< version: manifest.version.clone(), capabilities, import_file_extensions: import_extensions, - path: path.to_str().unwrap().to_string(), - directory: path.parent().unwrap().to_str().unwrap().to_string(), + path: path.to_path_buf(), + directory: path.parent().unwrap().to_path_buf(), manifest: serde_json::to_value(manifest)?, ..Default::default() }; @@ -803,7 +803,7 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result< } fn verify_executable(resource: &str, operation: &str, executable: &str, directory: &Path) { - if canonicalize_which(executable, Some(directory.to_string_lossy().as_ref())).is_err() { + if canonicalize_which(executable, Some(directory)).is_err() { info!("{}", t!("discovery.commandDiscovery.executableNotFound", resource = resource, operation = operation, executable = executable)); } } diff --git a/lib/dsc-lib/src/dscresources/command_resource.rs b/lib/dsc-lib/src/dscresources/command_resource.rs index 67e4f104e..8d7001c03 100644 --- a/lib/dsc-lib/src/dscresources/command_resource.rs +++ b/lib/dsc-lib/src/dscresources/command_resource.rs @@ -6,7 +6,7 @@ use jsonschema::Validator; use rust_i18n::t; use serde::Deserialize; use serde_json::{Map, Value}; -use std::{collections::HashMap, env, process::Stdio}; +use std::{collections::HashMap, env, path::Path, process::Stdio}; use crate::{configure::{config_doc::ExecutionKind, config_result::{ResourceGetResult, ResourceTestResult}}, util::canonicalize_which}; use crate::dscerror::DscError; use super::{dscresource::{get_diff, redact}, invoke_result::{ExportResult, GetResult, ResolveResult, SetResult, TestResult, ValidateResult, ResourceGetResponse, ResourceSetResponse, ResourceTestResponse, get_in_desired_state}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}}; @@ -25,7 +25,7 @@ pub const EXIT_PROCESS_TERMINATED: i32 = 0x102; /// # Errors /// /// Error returned if the resource does not successfully get the current state -pub fn invoke_get(resource: &ResourceManifest, cwd: &str, filter: &str, target_resource: Option<&str>) -> Result { +pub fn invoke_get(resource: &ResourceManifest, cwd: &Path, filter: &str, target_resource: Option<&str>) -> Result { debug!("{}", t!("dscresources.commandResource.invokeGet", resource = &resource.resource_type)); let mut command_input = CommandInput { env: None, stdin: None }; let Some(get) = &resource.get else { @@ -78,7 +78,7 @@ pub fn invoke_get(resource: &ResourceManifest, cwd: &str, filter: &str, target_r /// /// Error returned if the resource does not successfully set the desired state #[allow(clippy::too_many_lines)] -pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_test: bool, execution_type: &ExecutionKind, target_resource: Option<&str>) -> Result { +pub fn invoke_set(resource: &ResourceManifest, cwd: &Path, desired: &str, skip_test: bool, execution_type: &ExecutionKind, target_resource: Option<&str>) -> Result { debug!("{}", t!("dscresources.commandResource.invokeSet", resource = &resource.resource_type)); let operation_type: String; let mut is_synthetic_what_if = false; @@ -271,7 +271,7 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te /// # Errors /// /// Error is returned if the underlying command returns a non-zero exit code. -pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str, target_resource: Option<&str>) -> Result { +pub fn invoke_test(resource: &ResourceManifest, cwd: &Path, expected: &str, target_resource: Option<&str>) -> Result { debug!("{}", t!("dscresources.commandResource.invokeTest", resource = &resource.resource_type)); let Some(test) = &resource.test else { info!("{}", t!("dscresources.commandResource.testSyntheticTest", resource = &resource.resource_type)); @@ -380,7 +380,7 @@ fn get_desired_state(actual: &Value) -> Result, DscError> { Ok(in_desired_state) } -fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &str, expected: &str, target_resource: Option<&str>) -> Result { +fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &Path, expected: &str, target_resource: Option<&str>) -> Result { let get_result = invoke_get(resource, cwd, expected, target_resource)?; let actual_state = match get_result { GetResult::Group(results) => { @@ -415,7 +415,7 @@ fn invoke_synthetic_test(resource: &ResourceManifest, cwd: &str, expected: &str, /// # Errors /// /// Error is returned if the underlying command returns a non-zero exit code. -pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, target_resource: Option<&str>) -> Result<(), DscError> { +pub fn invoke_delete(resource: &ResourceManifest, cwd: &Path, filter: &str, target_resource: Option<&str>) -> Result<(), DscError> { let Some(delete) = &resource.delete else { return Err(DscError::NotImplemented("delete".to_string())); }; @@ -450,7 +450,7 @@ pub fn invoke_delete(resource: &ResourceManifest, cwd: &str, filter: &str, targe /// # Errors /// /// Error is returned if the underlying command returns a non-zero exit code. -pub fn invoke_validate(resource: &ResourceManifest, cwd: &str, config: &str, target_resource: Option<&str>) -> Result { +pub fn invoke_validate(resource: &ResourceManifest, cwd: &Path, config: &str, target_resource: Option<&str>) -> Result { trace!("{}", t!("dscresources.commandResource.invokeValidateConfig", resource = &resource.resource_type, config = &config)); // TODO: use schema to validate config if validate is not implemented let Some(validate) = resource.validate.as_ref() else { @@ -479,7 +479,7 @@ pub fn invoke_validate(resource: &ResourceManifest, cwd: &str, config: &str, tar /// # Errors /// /// Error if schema is not available or if there is an error getting the schema -pub fn get_schema(resource: &ResourceManifest, cwd: &str) -> Result { +pub fn get_schema(resource: &ResourceManifest, cwd: &Path) -> Result { let Some(schema_kind) = resource.schema.as_ref() else { return Err(DscError::SchemaNotAvailable(resource.resource_type.clone())); }; @@ -511,7 +511,7 @@ pub fn get_schema(resource: &ResourceManifest, cwd: &str) -> Result, target_resource: Option<&str>) -> Result { +pub fn invoke_export(resource: &ResourceManifest, cwd: &Path, input: Option<&str>, target_resource: Option<&str>) -> Result { let Some(export) = resource.export.as_ref() else { // see if get is supported and use that instead if resource.get.is_some() { @@ -591,7 +591,7 @@ pub fn invoke_export(resource: &ResourceManifest, cwd: &str, input: Option<&str> /// # Errors /// /// Error returned if the resource does not successfully resolve the input -pub fn invoke_resolve(resource: &ResourceManifest, cwd: &str, input: &str) -> Result { +pub fn invoke_resolve(resource: &ResourceManifest, cwd: &Path, input: &str) -> Result { let Some(resolve) = &resource.resolve else { return Err(DscError::Operation(t!("dscresources.commandResource.resolveNotSupported", resource = &resource.resource_type).to_string())); }; @@ -620,7 +620,7 @@ pub fn invoke_resolve(resource: &ResourceManifest, cwd: &str, input: &str) -> Re /// /// Error is returned if the command fails to execute or stdin/stdout/stderr cannot be opened. /// -async fn run_process_async(executable: &str, args: Option>, input: Option<&str>, cwd: Option<&str>, env: Option>, exit_codes: Option<&HashMap>) -> Result<(i32, String, String), DscError> { +async fn run_process_async(executable: &str, args: Option>, input: Option<&str>, cwd: Option<&Path>, env: Option>, exit_codes: Option<&HashMap>) -> Result<(i32, String, String), DscError> { // use somewhat large initial buffer to avoid early string reallocations; // the value is based on list result of largest of built-in adapters - WMI adapter ~500KB @@ -761,7 +761,7 @@ fn convert_hashmap_string_keys_to_i32(input: Option<&HashMap>) - /// Will panic if tokio runtime can't be created. /// #[allow(clippy::implicit_hasher)] -pub fn invoke_command(executable: &str, args: Option>, input: Option<&str>, cwd: Option<&str>, env: Option>, exit_codes: Option<&HashMap>) -> Result<(i32, String, String), DscError> { +pub fn invoke_command(executable: &str, args: Option>, input: Option<&str>, cwd: Option<&Path>, env: Option>, exit_codes: Option<&HashMap>) -> Result<(i32, String, String), DscError> { let exit_codes = convert_hashmap_string_keys_to_i32(exit_codes)?; let executable = canonicalize_which(executable, cwd)?; @@ -769,7 +769,7 @@ pub fn invoke_command(executable: &str, args: Option>, input: Option async { trace!("{}", t!("dscresources.commandResource.commandInvoke", executable = executable, args = args : {:?})); if let Some(cwd) = cwd { - trace!("{}", t!("dscresources.commandResource.commandCwd", cwd = cwd)); + trace!("{}", t!("dscresources.commandResource.commandCwd", cwd = cwd.display())); } match run_process_async(&executable, args, input, cwd, env, exit_codes.as_ref()).await { @@ -854,7 +854,7 @@ fn get_command_input(input_kind: Option<&InputKind>, input: &str) -> Result Result<(), DscError> { +fn verify_json(resource: &ResourceManifest, cwd: &Path, json: &str) -> Result<(), DscError> { debug!("{}", t!("dscresources.commandResource.verifyJson", resource = resource.resource_type)); diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index e38443b8d..55f00f7e3 100644 --- a/lib/dsc-lib/src/dscresources/dscresource.rs +++ b/lib/dsc-lib/src/dscresources/dscresource.rs @@ -11,6 +11,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use std::collections::HashMap; +use std::path::PathBuf; use tracing::{debug, info, trace}; use super::{ @@ -37,11 +38,11 @@ pub struct DscResource { /// The capabilities of the resource. pub capabilities: Vec, /// The file path to the resource. - pub path: String, + pub path: PathBuf, /// The description of the resource. pub description: Option, // The directory path to the resource. - pub directory: String, + pub directory: PathBuf, /// The implementation of the resource. #[serde(rename="implementedAs")] pub implemented_as: ImplementedAs, @@ -98,8 +99,8 @@ impl DscResource { version: String::new(), capabilities: Vec::new(), description: None, - path: String::new(), - directory: String::new(), + path: PathBuf::new(), + directory: PathBuf::new(), implemented_as: ImplementedAs::Command, author: None, properties: Vec::new(), diff --git a/lib/dsc-lib/src/dscresources/include.rs b/lib/dsc-lib/src/dscresources/include.rs index 125d7235e..970ce2d44 100644 --- a/lib/dsc-lib/src/dscresources/include.rs +++ b/lib/dsc-lib/src/dscresources/include.rs @@ -3,11 +3,12 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use std::path::PathBuf; #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] pub struct Include { /// The path to the file to include. Relative paths are relative to the file containing the include /// and not allowed to reference parent directories. #[serde(rename = "configurationFile")] - pub configuration_file: String, + pub configuration_file: PathBuf, } diff --git a/lib/dsc-lib/src/extensions/discover.rs b/lib/dsc-lib/src/extensions/discover.rs index 813b6578a..6efb91abb 100644 --- a/lib/dsc-lib/src/extensions/discover.rs +++ b/lib/dsc-lib/src/extensions/discover.rs @@ -24,7 +24,7 @@ use crate::{ use rust_i18n::t; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::path::Path; +use std::path::PathBuf; use tracing::{info, trace}; #[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] @@ -39,7 +39,7 @@ pub struct DiscoverMethod { pub struct DiscoverResult { /// The path to the resource manifest, must be absolute. #[serde(rename = "manifestPath")] - pub manifest_path: String, + pub manifest_path: PathBuf, } impl DscExtension { @@ -70,7 +70,7 @@ impl DscExtension { &discover.executable, args, None, - Some(self.directory.as_str()), + Some(&self.directory), None, extension.exit_codes.as_ref(), )?; @@ -85,12 +85,11 @@ impl DscExtension { return Err(DscError::Json(err)); } }; - if !Path::new(&discover_result.manifest_path).is_absolute() { - return Err(DscError::Extension(t!("extensions.dscextension.discoverNotAbsolutePath", extension = self.type_name.clone(), path = discover_result.manifest_path.clone()).to_string())); + if !discover_result.manifest_path.is_absolute() { + return Err(DscError::Extension(t!("extensions.dscextension.discoverNotAbsolutePath", extension = self.type_name.clone(), path = discover_result.manifest_path.display()).to_string())); } - let manifest_path = Path::new(&discover_result.manifest_path); // Currently we don't support extensions discovering other extensions - for imported_manifest in load_manifest(manifest_path)? { + for imported_manifest in load_manifest(&discover_result.manifest_path)? { if let ImportedManifest::Resource(resource) = imported_manifest { resources.push(resource); } diff --git a/lib/dsc-lib/src/extensions/dscextension.rs b/lib/dsc-lib/src/extensions/dscextension.rs index a25389644..6bb1daedf 100644 --- a/lib/dsc-lib/src/extensions/dscextension.rs +++ b/lib/dsc-lib/src/extensions/dscextension.rs @@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use schemars::JsonSchema; use std::fmt::Display; +use std::path::PathBuf; #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] @@ -21,11 +22,11 @@ pub struct DscExtension { #[serde(rename = "importFileExtensions")] pub import_file_extensions: Option>, /// The file path to the resource. - pub path: String, + pub path: PathBuf, /// The description of the resource. pub description: Option, // The directory path to the resource. - pub directory: String, + pub directory: PathBuf, /// The author of the resource. pub author: Option, /// The manifest of the resource. @@ -63,8 +64,8 @@ impl DscExtension { capabilities: Vec::new(), import_file_extensions: None, description: None, - path: String::new(), - directory: String::new(), + path: PathBuf::new(), + directory: PathBuf::new(), author: None, manifest: Value::Null, } diff --git a/lib/dsc-lib/src/extensions/import.rs b/lib/dsc-lib/src/extensions/import.rs index 79e83dd94..e38ffa743 100644 --- a/lib/dsc-lib/src/extensions/import.rs +++ b/lib/dsc-lib/src/extensions/import.rs @@ -61,16 +61,15 @@ impl DscExtension { /// # Errors /// /// This function will return an error if the import fails or if the extension does not support the import capability. - pub fn import(&self, file: &str) -> Result { + pub fn import(&self, file: &Path) -> Result { if self.capabilities.contains(&Capability::Import) { - let file_path = Path::new(file); - let file_extension = file_path.extension().and_then(|s| s.to_str()).unwrap_or_default().to_string(); + let file_extension = file.extension().and_then(|s| s.to_str()).unwrap_or_default().to_string(); if self.import_file_extensions.as_ref().is_some_and(|exts| exts.contains(&file_extension)) { - debug!("{}", t!("extensions.dscextension.importingFile", file = file, extension = self.type_name)); + debug!("{}", t!("extensions.dscextension.importingFile", file = file.display(), extension = self.type_name)); } else { - debug!("{}", t!("extensions.dscextension.importNotSupported", file = file, extension = self.type_name)); + debug!("{}", t!("extensions.dscextension.importNotSupported", file = file.display(), extension = self.type_name)); return Err(DscError::NotSupported( - t!("extensions.dscextension.importNotSupported", file = file, extension = self.type_name).to_string(), + t!("extensions.dscextension.importNotSupported", file = file.display(), extension = self.type_name).to_string(), )); } @@ -88,7 +87,7 @@ impl DscExtension { &import.executable, args, None, - Some(self.directory.as_str()), + Some(&self.directory), None, extension.exit_codes.as_ref(), )?; @@ -105,16 +104,15 @@ impl DscExtension { } } -fn process_import_args(args: Option<&Vec>, file: &str) -> Result>, DscError> { +fn process_import_args(args: Option<&Vec>, file: &Path) -> Result>, DscError> { let Some(arg_values) = args else { debug!("{}", t!("dscresources.commandResource.noArgs")); return Ok(None); }; // make path absolute - let path = Path::new(file); - let Ok(full_path) = path.absolutize() else { - return Err(DscError::Extension(t!("util.failedToAbsolutizePath", path = path : {:?}).to_string())); + let Ok(full_path) = file.absolutize() else { + return Err(DscError::Extension(t!("util.failedToAbsolutizePath", path = file.display()).to_string())); }; let mut processed_args = Vec::::new(); diff --git a/lib/dsc-lib/src/extensions/secret.rs b/lib/dsc-lib/src/extensions/secret.rs index 01d7fe3b2..5af7caa48 100644 --- a/lib/dsc-lib/src/extensions/secret.rs +++ b/lib/dsc-lib/src/extensions/secret.rs @@ -78,7 +78,7 @@ impl DscExtension { &secret.executable, args, vault, - Some(self.directory.as_str()), + Some(&self.directory), None, extension.exit_codes.as_ref(), )?; diff --git a/lib/dsc-lib/src/util.rs b/lib/dsc-lib/src/util.rs index 35f010891..086076cd6 100644 --- a/lib/dsc-lib/src/util.rs +++ b/lib/dsc-lib/src/util.rs @@ -233,19 +233,18 @@ pub fn resource_id(type_name: &str, name: &str) -> String { result } -pub fn canonicalize_which(executable: &str, cwd: Option<&str>) -> Result { +pub fn canonicalize_which(executable: &str, cwd: Option<&Path>) -> Result { // Use PathBuf to handle path separators robustly let mut executable_path = PathBuf::from(executable); if cfg!(target_os = "windows") && executable_path.extension().is_none() { executable_path.set_extension("exe"); } if which(executable).is_err() { - if let Some(cwd) = cwd { - let cwd_path = Path::new(cwd); + if let Some(cwd_path) = cwd { if let Ok(canonical_path) = canonicalize(cwd_path.join(&executable_path)) { return Ok(canonical_path.to_string_lossy().to_string()); } - return Err(DscError::CommandOperation(t!("util.executableNotFoundInWorkingDirectory", executable = &executable, cwd = cwd_path.to_string_lossy()).to_string(), executable_path.to_string_lossy().to_string())); + return Err(DscError::CommandOperation(t!("util.executableNotFoundInWorkingDirectory", executable = &executable, cwd = cwd_path.display()).to_string(), executable_path.to_string_lossy().to_string())); } return Err(DscError::CommandOperation(t!("util.executableNotFound", executable = &executable).to_string(), executable.to_string())); } diff --git a/tools/test_group_resource/src/main.rs b/tools/test_group_resource/src/main.rs index 04f753ffc..1536aab27 100644 --- a/tools/test_group_resource/src/main.rs +++ b/tools/test_group_resource/src/main.rs @@ -8,6 +8,7 @@ use clap::Parser; use dsc_lib::dscresources::resource_manifest::{ResourceManifest, GetMethod, Kind}; use dsc_lib::dscresources::dscresource::{Capability, DscResource, ImplementedAs}; use dsc_lib::schemas::DscRepoSchema; +use std::path::PathBuf; fn main() { let args = Args::parse(); @@ -20,8 +21,8 @@ fn main() { capabilities: vec![Capability::Get, Capability::Set], description: Some("This is a test resource.".to_string()), implemented_as: ImplementedAs::Custom("TestResource".to_string()), - path: "test_resource1".to_string(), - directory: "test_directory".to_string(), + path: PathBuf::from("test_resource1"), + directory: PathBuf::from("test_directory"), author: Some("Microsoft".to_string()), properties: vec!["Property1".to_string(), "Property2".to_string()], require_adapter: Some("Test/TestGroup".to_string()), @@ -46,8 +47,8 @@ fn main() { capabilities: vec![Capability::Get, Capability::Set], description: Some("This is a test resource.".to_string()), implemented_as: ImplementedAs::Custom("TestResource".to_string()), - path: "test_resource2".to_string(), - directory: "test_directory".to_string(), + path: PathBuf::from("test_resource2"), + directory: PathBuf::from("test_directory"), author: Some("Microsoft".to_string()), properties: vec!["Property1".to_string(), "Property2".to_string()], require_adapter: Some("Test/TestGroup".to_string()), @@ -76,8 +77,8 @@ fn main() { capabilities: vec![Capability::Get], description: Some("This is a test resource.".to_string()), implemented_as: ImplementedAs::Custom("TestResource".to_string()), - path: "test_resource1".to_string(), - directory: "test_directory".to_string(), + path: PathBuf::from("test_resource1"), + directory: PathBuf::from("test_directory"), author: Some("Microsoft".to_string()), properties: vec!["Property1".to_string(), "Property2".to_string()], require_adapter: None,