From a333b425177e4a99c752c5319f9b038661dbaf2e Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 4 Oct 2023 13:43:56 -0700 Subject: [PATCH 01/20] Updated regex in DependsOn --- dsc_lib/src/configure/depends_on.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc_lib/src/configure/depends_on.rs b/dsc_lib/src/configure/depends_on.rs index 41efc655..2f0e3ad3 100644 --- a/dsc_lib/src/configure/depends_on.rs +++ b/dsc_lib/src/configure/depends_on.rs @@ -22,7 +22,7 @@ use crate::DscError; /// * `DscError::Validation` - The configuration is invalid pub fn get_resource_invocation_order(config: &Configuration) -> Result, DscError> { let mut order: Vec = Vec::new(); - let depends_on_regex = Regex::new(r"^\[resourceId\(\s*'(?[a-zA-Z0-9\.]+/[a-zA-Z0-9]+)'\s*,\s*'(?[a-zA-Z0-9 ]+)'\s*\)]$")?; + let depends_on_regex = Regex::new(r"^\[resourceId\(\s*'([a-zA-Z0-9\.]+/[a-zA-Z0-9]+)'\s*,\s*'([a-zA-Z0-9 ]+)'\s*\)]$")?; for resource in &config.resources { // validate that the resource isn't specified more than once in the config if config.resources.iter().filter(|r| r.name == resource.name && r.resource_type == resource.resource_type).count() > 1 { From feef2da1711081baf0b0a958fc9f5a9b5d013207 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 4 Oct 2023 17:21:21 -0700 Subject: [PATCH 02/20] Removed discovery resource list --- dsc/src/main.rs | 4 +- dsc/src/resource_command.rs | 72 ++++++++----------- dsc/src/subcommand.rs | 77 +++++++++++--------- dsc_lib/src/configure/mod.rs | 14 ++-- dsc_lib/src/discovery/command_discovery.rs | 71 +++++-------------- dsc_lib/src/discovery/discovery_trait.rs | 7 +- dsc_lib/src/discovery/mod.rs | 82 ++++++++-------------- dsc_lib/src/lib.rs | 15 ++-- 8 files changed, 140 insertions(+), 202 deletions(-) diff --git a/dsc/src/main.rs b/dsc/src/main.rs index 14570e8c..c75744e9 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -8,7 +8,7 @@ use clap_complete::generate; use std::io::{self, Read}; use std::process::exit; use sysinfo::{Process, ProcessExt, RefreshKind, System, SystemExt, get_current_pid, ProcessRefreshKind}; -use tracing::{error, info, warn}; +use tracing::{Level, error, info, warn}; #[cfg(debug_assertions)] use crossterm::event; @@ -26,7 +26,7 @@ fn main() { check_debug(); // create subscriber that writes all events to stderr - let subscriber = tracing_subscriber::fmt().pretty().with_writer(std::io::stderr).finish(); + let subscriber = tracing_subscriber::fmt().pretty().with_max_level(Level::DEBUG).with_writer(std::io::stderr).finish(); if tracing::subscriber::set_global_default(subscriber).is_err() { eprintln!("Unable to set global default subscriber"); } diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 5b8fe6eb..ffe97435 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -14,14 +14,14 @@ use dsc_lib::{ }; use std::process::exit; -pub fn get(dsc: &mut DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn get(dsc: &DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { // TODO: support streaming stdin which includes resource and input let mut input = get_input(input, stdin); - let mut resource = get_resource(dsc, resource); + let mut resource = get_resource(dsc, resource).unwrap(); debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); - if let Some(requires) = resource.requires { - input = add_type_name_to_json(input, resource.type_name); - resource = get_resource(dsc, &requires); + if let Some(requires) = &resource.requires { + input = add_type_name_to_json(input, resource.type_name.clone()); + resource = get_resource(dsc, &requires).unwrap(); } match resource.get(input.as_str()) { @@ -43,8 +43,8 @@ pub fn get(dsc: &mut DscManager, resource: &str, input: &Option, stdin: } } -pub fn get_all(dsc: &mut DscManager, resource: &str, _input: &Option, _stdin: &Option, format: &Option) { - let resource = get_resource(dsc, resource); +pub fn get_all(dsc: &DscManager, resource: &str, _input: &Option, _stdin: &Option, format: &Option) { + let resource = get_resource(dsc, resource).unwrap(); debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); let export_result = match resource.export() { Ok(export) => { export } @@ -71,20 +71,20 @@ pub fn get_all(dsc: &mut DscManager, resource: &str, _input: &Option, _s } } -pub fn set(dsc: &mut DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn set(dsc: &DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); if input.is_empty() { error!("Error: Input is empty"); exit(EXIT_INVALID_ARGS); } - let mut resource = get_resource(dsc, resource); + let mut resource = get_resource(dsc, resource).unwrap(); debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); - if let Some(requires) = resource.requires { - input = add_type_name_to_json(input, resource.type_name); - resource = get_resource(dsc, &requires); + if let Some(requires) = &resource.requires { + input = add_type_name_to_json(input, resource.type_name.clone()); + resource = get_resource(dsc, &requires).unwrap(); } match resource.set(input.as_str(), true) { @@ -106,15 +106,15 @@ pub fn set(dsc: &mut DscManager, resource: &str, input: &Option, stdin: } } -pub fn test(dsc: &mut DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn test(dsc: &DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); - let mut resource = get_resource(dsc, resource); + let mut resource = get_resource(dsc, resource).unwrap(); debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); - if let Some(requires) = resource.requires { - input = add_type_name_to_json(input, resource.type_name); - resource = get_resource(dsc, &requires); + if let Some(requires) = &resource.requires { + input = add_type_name_to_json(input, resource.type_name.clone()); + resource = get_resource(dsc, &requires).unwrap(); } match resource.test(input.as_str()) { @@ -136,8 +136,8 @@ pub fn test(dsc: &mut DscManager, resource: &str, input: &Option, stdin: } } -pub fn schema(dsc: &mut DscManager, resource: &str, format: &Option) { - let resource = get_resource(dsc, resource); +pub fn schema(dsc: &DscManager, resource: &str, format: &Option) { + let resource = get_resource(dsc, resource).unwrap(); match resource.schema() { Ok(json) => { // verify is json @@ -158,7 +158,7 @@ pub fn schema(dsc: &mut DscManager, resource: &str, format: &Option) { - let dsc_resource = get_resource(dsc, resource); + let dsc_resource = get_resource(dsc, resource).unwrap(); let mut conf = Configuration::new(); @@ -177,34 +177,24 @@ pub fn export(dsc: &mut DscManager, resource: &str, format: &Option DscResource { +pub fn get_resource<'a>(dsc: &'a DscManager, resource: &str) -> Option<&'a DscResource> { + //TODO: add dinamically generated resource to dsc // check if resource is JSON or just a name - match serde_json::from_str(resource) { - Ok(resource) => resource, + /*match serde_json::from_str(resource) { + Ok(resource) => + { + + resource + }, Err(err) => { if resource.contains('{') { error!("Not valid resource JSON: {err}\nInput was: {resource}"); exit(EXIT_INVALID_ARGS); } - - if let Err(err) = dsc.initialize_discovery() { - error!("Error: {err}"); - exit(EXIT_DSC_ERROR); - } - let resources: Vec = dsc.find_resource(resource).collect(); - match resources.len() { - 0 => { - error!("Error: Resource not found: '{resource}'"); - exit(EXIT_INVALID_ARGS); - } - 1 => resources[0].clone(), - _ => { - error!("Error: Multiple resources found"); - exit(EXIT_INVALID_ARGS); - } - } } - } + }*/ + + dsc.find_resource(resource) } fn get_input(input: &Option, stdin: &Option) -> String { diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index bef654dd..e9573da8 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -14,11 +14,11 @@ use dsc_lib::{ dscresources::dscresource::{ImplementedAs, Invoke}, dscresources::resource_manifest::{import_manifest, ResourceManifest}, }; -use jsonschema::JSONSchema; +use jsonschema::{JSONSchema, ValidationError}; use serde_yaml::Value; use std::process::exit; -pub fn config_get(configurator: &Configurator, format: &Option) +pub fn config_get(configurator: &mut Configurator, format: &Option) { match configurator.invoke_get(ErrorAction::Continue, || { /* code */ }) { Ok(result) => { @@ -41,7 +41,7 @@ pub fn config_get(configurator: &Configurator, format: &Option) } } -pub fn config_set(configurator: &Configurator, format: &Option) +pub fn config_set(configurator: &mut Configurator, format: &Option) { match configurator.invoke_set(false, ErrorAction::Continue, || { /* code */ }) { Ok(result) => { @@ -64,7 +64,7 @@ pub fn config_set(configurator: &Configurator, format: &Option) } } -pub fn config_test(configurator: &Configurator, format: &Option) +pub fn config_test(configurator: &mut Configurator, format: &Option) { match configurator.invoke_test(ErrorAction::Continue, || { /* code */ }) { Ok(result) => { @@ -87,7 +87,7 @@ pub fn config_test(configurator: &Configurator, format: &Option) } } -pub fn config_export(configurator: &Configurator, format: &Option) +pub fn config_export(configurator: &mut Configurator, format: &Option) { match configurator.invoke_export(ErrorAction::Continue, || { /* code */ }) { Ok(result) => { @@ -144,7 +144,7 @@ pub fn config(subcommand: &ConfigSubCommand, format: &Option, stdi }; let json_string = serde_json_value_to_string(&json); - let configurator = match Configurator::new(&json_string) { + let mut configurator = match Configurator::new(&json_string) { Ok(configurator) => configurator, Err(err) => { error!("Error: {err}"); @@ -154,23 +154,29 @@ pub fn config(subcommand: &ConfigSubCommand, format: &Option, stdi match subcommand { ConfigSubCommand::Get => { - config_get(&configurator, format); + config_get(&mut configurator, format); }, ConfigSubCommand::Set => { - config_set(&configurator, format); + config_set(&mut configurator, format); }, ConfigSubCommand::Test => { - config_test(&configurator, format); + config_test(&mut configurator, format); }, ConfigSubCommand::Validate => { validate_config(&json_string); }, ConfigSubCommand::Export => { - config_export(&configurator, format); + config_export(&mut configurator, format); } } } +/// Validate configuration. +/// +/// # Panics +/// +/// Will panic if resource is not found. +/// #[allow(clippy::too_many_lines)] pub fn validate_config(config: &str) { // first validate against the config schema @@ -204,7 +210,7 @@ pub fn validate_config(config: &str) { exit(EXIT_INVALID_INPUT); }; - let mut dsc = match DscManager::new() { + let dsc = match DscManager::new() { Ok(dsc) => dsc, Err(err) => { error!("Error: {err}"); @@ -223,7 +229,7 @@ pub fn validate_config(config: &str) { exit(EXIT_INVALID_INPUT); }); // get the actual resource - let resource = get_resource(&mut dsc, type_name); + let resource = get_resource(&dsc, type_name).unwrap(); // see if the resource is command based if resource.implemented_as == ImplementedAs::Command { // if so, see if it implements validate via the resource manifest @@ -246,7 +252,7 @@ pub fn validate_config(config: &str) { }; if !result.valid { let reason = result.reason.unwrap_or("No reason provided".to_string()); - let type_name = resource.type_name; + let type_name = resource.type_name.clone(); error!("Resource {type_name} failed validation: {reason}"); exit(EXIT_VALIDATION_FAILED); } @@ -272,15 +278,18 @@ pub fn validate_config(config: &str) { }, }; let properties = resource_block["properties"].clone(); - let validation = compiled_schema.validate(&properties); - if let Err(err) = validation { - let mut error = String::new(); - for e in err { - error.push_str(&format!("{e} ")); - } - error!("Error: Resource {type_name} failed validation: {error}"); - exit(EXIT_VALIDATION_FAILED); - } + let _result: Result<(), ValidationError> = match compiled_schema.validate(&properties) { + Ok(()) => Ok(()), + Err(err) => { + let mut error = String::new(); + for e in err { + error.push_str(&format!("{e} ")); + } + + error!("Error: Resource {type_name} failed validation: {error}"); + exit(EXIT_VALIDATION_FAILED); + }, + }; } } } @@ -300,17 +309,14 @@ pub fn resource(subcommand: &ResourceSubCommand, format: &Option, match subcommand { ResourceSubCommand::List { resource_name, description, tags } => { - if let Err(err) = dsc.initialize_discovery() { - error!("Error: {err}"); - exit(EXIT_DSC_ERROR); - } + let mut write_table = false; let mut table = Table::new(&["Type", "Version", "Requires", "Description"]); if format.is_none() && atty::is(Stream::Stdout) { // write as table if fornat is not specified and interactive write_table = true; } - for resource in dsc.find_resource(&resource_name.clone().unwrap_or_default()) { + for resource in dsc.list_available_resources(&resource_name.clone().unwrap_or_default()) { // if description is specified, skip if resource description does not contain it if description.is_some() || tags.is_some() { let Some(ref resource_manifest) = resource.manifest else { @@ -380,19 +386,26 @@ pub fn resource(subcommand: &ResourceSubCommand, format: &Option, } }, ResourceSubCommand::Get { resource, input, all } => { - if *all { resource_command::get_all(&mut dsc, resource, input, stdin, format); } - else { resource_command::get(&mut dsc, resource, input, stdin, format); }; + dsc.discover_resources(&[resource.to_string()]); + if *all { resource_command::get_all(&dsc, resource, input, stdin, format); } + else { + resource_command::get(&dsc, resource, input, stdin, format); + }; }, ResourceSubCommand::Set { resource, input } => { - resource_command::set(&mut dsc, resource, input, stdin, format); + dsc.discover_resources(&[resource.to_string()]); + resource_command::set(&dsc, resource, input, stdin, format); }, ResourceSubCommand::Test { resource, input } => { - resource_command::test(&mut dsc, resource, input, stdin, format); + dsc.discover_resources(&[resource.to_string()]); + resource_command::test(&dsc, resource, input, stdin, format); }, ResourceSubCommand::Schema { resource } => { - resource_command::schema(&mut dsc, resource, format); + dsc.discover_resources(&[resource.to_string()]); + resource_command::schema(&dsc, resource, format); }, ResourceSubCommand::Export { resource} => { + dsc.discover_resources(&[resource.to_string()]); resource_command::export(&mut dsc, resource, format); }, } diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index abc624e5..0a364bc9 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -66,9 +66,7 @@ impl Configurator { /// /// This function will return an error if the configuration is invalid or the underlying discovery fails. pub fn new(config: &str) -> Result { - let mut discovery = Discovery::new()?; - discovery.initialize()?; - + let discovery = Discovery::new()?; Ok(Configurator { config: config.to_owned(), discovery, @@ -94,7 +92,7 @@ impl Configurator { return Ok(result); } for resource in get_resource_invocation_order(&config)? { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type).next() else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; debug!("resource_type {}", &resource.resource_type); @@ -130,7 +128,7 @@ impl Configurator { return Ok(result); } for resource in get_resource_invocation_order(&config)? { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type).next() else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; debug!("resource_type {}", &resource.resource_type); @@ -166,7 +164,7 @@ impl Configurator { return Ok(result); } for resource in get_resource_invocation_order(&config)? { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type).next() else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; debug!("resource_type {}", &resource.resource_type); @@ -240,7 +238,7 @@ impl Configurator { let mut conf = config_doc::Configuration::new(); for resource in &config.resources { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type).next() else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; add_resource_export_results_to_configuration(&dsc_resource, &mut conf)?; @@ -256,7 +254,7 @@ impl Configurator { let mut messages: Vec = Vec::new(); let mut has_errors = false; for resource in &config.resources { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type).next() else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index c51a6df9..67c65157 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -11,21 +11,14 @@ use std::env; use std::fs::File; use std::io::BufReader; use std::path::Path; +use tracing::{debug, error}; pub struct CommandDiscovery { - pub resources: BTreeMap, - provider_resources: Vec, - discovery_messages: Vec, // this will later be used to display only messages for resources used in specific operation - initialized: bool, } impl CommandDiscovery { pub fn new() -> CommandDiscovery { CommandDiscovery { - resources: BTreeMap::new(), - provider_resources: Vec::new(), - discovery_messages: Vec::new(), - initialized: false, } } } @@ -37,19 +30,11 @@ impl Default for CommandDiscovery { } impl ResourceDiscovery for CommandDiscovery { - fn discover(&self) -> Box> { - if self.initialized { - Box::new(self.resources.values().cloned().collect::>().into_iter()) - } else { - Box::new(vec![].into_iter()) - } - } - fn initialize(&mut self) -> Result<(), DscError>{ - if self.initialized { - return Ok(()); - } + fn list_available_resources(&mut self) -> Result, DscError> { + let mut resources: BTreeMap = BTreeMap::new(); + let mut provider_resources: Vec = Vec::new(); // try DSC_RESOURCE_PATH env var first otherwise use PATH let path_env = match env::var_os("DSC_RESOURCE_PATH") { Some(value) => value, @@ -75,79 +60,61 @@ impl ResourceDiscovery for CommandDiscovery { if resource.manifest.is_some() { let manifest = import_manifest(resource.manifest.clone().unwrap())?; if manifest.provider.is_some() { - self.provider_resources.push(resource.type_name.clone()); + provider_resources.push(resource.type_name.clone()); } } - self.resources.insert(resource.type_name.clone(), resource.clone()); + resources.insert(resource.type_name.clone(), resource); } } } } } + debug!("Found {} non-provider resources", resources.len()); + // now go through the provider resources and add them to the list of resources - for provider in &self.provider_resources { - let provider_resource = self.resources.get(provider).unwrap(); + for provider in provider_resources { + let provider_resource = resources.get(&provider).unwrap(); let provider_type_name = provider_resource.type_name.clone(); let provider_path = provider_resource.path.clone(); let manifest = import_manifest(provider_resource.manifest.clone().unwrap())?; + let mut provider_resources_count = 0; // invoke the list command let list_command = manifest.provider.unwrap().list; let (exit_code, stdout, stderr) = match invoke_command(&list_command.executable, list_command.args, None, Some(&provider_resource.directory), None) { Ok((exit_code, stdout, stderr)) => (exit_code, stdout, stderr), Err(e) => { - self.discovery_messages.push(StreamMessage::new_error( - format!("Could not start {}: {}", list_command.executable, e), - Some(provider_type_name.clone()), - Some(provider_path.clone()))); - + error!("Could not start {}: {}", list_command.executable, e); continue; }, }; if exit_code != 0 { - self.discovery_messages.push(StreamMessage::new_error( - format!("Provider failed to list resources with exit code {exit_code}: {stderr}"), - Some(provider_type_name.clone()), - Some(provider_path.clone()))); + error!("Provider failed to list resources with exit code {exit_code}: {stderr}"); } for line in stdout.lines() { match serde_json::from_str::(line){ Result::Ok(resource) => { if resource.requires.is_none() { - self.discovery_messages.push(StreamMessage::new_error( - DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string(), - Some(resource.type_name.clone()), - Some(resource.path.clone()))); - + error!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); continue; } - self.resources.insert(resource.type_name.clone(), resource); + resources.insert(resource.type_name.clone(), resource); + provider_resources_count += 1; }, Result::Err(err) => { - self.discovery_messages.push(StreamMessage::new_error( - format!("Failed to parse resource: {line} -> {err}"), - Some(provider_type_name.clone()), - Some(provider_path.clone()))); - + error!("Failed to parse resource: {line} -> {err}"); continue; } }; } - } - - self.initialized = true; - Ok(()) - } - fn print_initialization_messages(&mut self, error_format:StreamMessageType, warning_format:StreamMessageType) -> Result<(), DscError>{ - for msg in &self.discovery_messages { - msg.print(&error_format, &warning_format)?; + debug!("Provider {} listed {} resources", provider_type_name, provider_resources_count); } - Ok(()) + Ok(resources) } } diff --git a/dsc_lib/src/discovery/discovery_trait.rs b/dsc_lib/src/discovery/discovery_trait.rs index 78848dc1..2fc9fb40 100644 --- a/dsc_lib/src/discovery/discovery_trait.rs +++ b/dsc_lib/src/discovery/discovery_trait.rs @@ -1,10 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::{dscresources::dscresource::DscResource, dscerror::DscError, dscerror::StreamMessageType}; +use crate::{dscresources::dscresource::DscResource, dscerror::DscError}; +use std::collections::BTreeMap; pub trait ResourceDiscovery { - fn discover(&self) -> Box>; - fn initialize(&mut self) -> Result<(), DscError>; - fn print_initialization_messages(&mut self, error_format:StreamMessageType, warning_format:StreamMessageType) -> Result<(), DscError>; + fn list_available_resources(&mut self) -> Result, DscError>; } diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index a01f40e9..1769dae2 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -7,80 +7,58 @@ mod discovery_trait; use crate::discovery::discovery_trait::ResourceDiscovery; use crate::{dscresources::dscresource::DscResource, dscerror::DscError, dscerror::StreamMessageType}; use regex::RegexBuilder; +use std::collections::BTreeMap; +use tracing::{debug, error}; pub struct Discovery { - resources: Vec, - initialized: bool, + resources: BTreeMap, } impl Discovery { - /// Create a new `Discovery` instance. - /// - /// # Errors - /// - /// This function will return an error if the underlying discovery fails. + pub fn new() -> Result { Ok(Self { - resources: Vec::new(), - initialized: false, + resources: BTreeMap::new(), }) } - /// Initialize the discovery process. - /// - /// # Errors - /// - /// This function will return an error if the underlying discovery fails. - pub fn initialize(&mut self) -> Result<(), DscError> { - // TODO: this vec is leftover when we had multiple discovery types, we should - // probably just have a single command discovery type + pub fn list_available_resources(&mut self, type_name_filter: &str) -> Vec { let discovery_types: Vec> = vec![ Box::new(command_discovery::CommandDiscovery::new()), ]; + let regex_str = convert_wildcard_to_regex(type_name_filter); + let mut regex_builder = RegexBuilder::new(regex_str.as_str()); + debug!("Using regex {regex_str} as filter for resource type"); + regex_builder.case_insensitive(true); + let Ok(regex) = regex_builder.build() else { + return vec![]; + }; + let mut resources: Vec = Vec::new(); for mut discovery_type in discovery_types { - discovery_type.initialize()?; - discovery_type.print_initialization_messages(StreamMessageType::Warning, StreamMessageType::Warning)?; - - let discovered_resources = discovery_type.discover(); - for resource in discovered_resources { - resources.push(resource.clone()); - } - } - self.resources = resources; - self.initialized = true; - Ok(()) - } + let discovered_resources = match discovery_type.list_available_resources() { + Ok(value) => value, + Err(err) => { + error!("{err}"); + continue; + } + }; - // TODO: Need to support version? - /// Find a resource by name. - /// - /// # Arguments - /// - /// * `type_name` - The name of the resource to find, can have wildcards. - #[must_use] - pub fn find_resource(&self, type_name: &str) -> ResourceIterator { - if !self.initialized { - return ResourceIterator::new(vec![]); + for resource in discovered_resources { + if type_name_filter.is_empty() | regex.is_match(resource.0.as_str()) { + resources.push(resource.1); + } + }; } - let mut regex_builder = RegexBuilder::new(convert_wildcard_to_regex(type_name).as_str()); - regex_builder.case_insensitive(true); - let Ok(regex) = regex_builder.build() else { - return ResourceIterator::new(vec![]); - }; - - let mut resources: Vec = Vec::new(); - for resource in &self.resources { - if type_name.is_empty() | regex.is_match(resource.type_name.as_str()) { - resources.push(resource.clone()); - } - } + resources + } - ResourceIterator::new(resources) + pub fn find_resource(&self, type_name: &str) -> Option<&DscResource> { + self.resources.get(type_name) } } diff --git a/dsc_lib/src/lib.rs b/dsc_lib/src/lib.rs index 04c8c12d..76e1c5aa 100644 --- a/dsc_lib/src/lib.rs +++ b/dsc_lib/src/lib.rs @@ -29,16 +29,6 @@ impl DscManager { }) } - /// Initialize the discovery process. - /// - /// # Errors - /// - /// This function will return an error if the underlying discovery fails. - /// - pub fn initialize_discovery(&mut self) -> Result<(), DscError> { - self.discovery.initialize() - } - /// Find a resource by name. /// /// # Arguments @@ -46,10 +36,13 @@ impl DscManager { /// * `name` - The name of the resource to find, can have wildcards. /// #[must_use] - pub fn find_resource(&self, name: &str) -> ResourceIterator { + pub fn find_resource(&self, name: &str) -> Option<&DscResource> { self.discovery.find_resource(name) } + pub fn list_available_resources(&mut self, type_name_filter: &str) -> Vec { + self.discovery.list_available_resources(type_name_filter) + } /// Invoke the get operation on a resource. /// /// # Arguments From 63d27a56231718922d020af0ad3273fd32360e34 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 10 Oct 2023 20:18:04 -0700 Subject: [PATCH 03/20] Optimized search_for_resources --- dsc_lib/src/discovery/command_discovery.rs | 78 +++++++++++++++++----- dsc_lib/src/discovery/discovery_trait.rs | 1 + dsc_lib/src/discovery/mod.rs | 55 +++++++++++---- dsc_lib/src/lib.rs | 7 ++ 4 files changed, 113 insertions(+), 28 deletions(-) diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 67c65157..ec4ab70d 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -5,7 +5,7 @@ use crate::discovery::discovery_trait::ResourceDiscovery; use crate::dscresources::dscresource::{DscResource, ImplementedAs}; use crate::dscresources::resource_manifest::{ResourceManifest, import_manifest}; use crate::dscresources::command_resource::invoke_command; -use crate::dscerror::{DscError, StreamMessage, StreamMessageType}; +use crate::dscerror::DscError; use std::collections::BTreeMap; use std::env; use std::fs::File; @@ -21,20 +21,14 @@ impl CommandDiscovery { CommandDiscovery { } } -} - -impl Default for CommandDiscovery { - fn default() -> Self { - Self::new() - } -} - -impl ResourceDiscovery for CommandDiscovery { - fn list_available_resources(&mut self) -> Result, DscError> { + fn search_for_resources(&mut self, required_resource_types: &Vec) -> Result, DscError> + { + let return_all_resources = required_resource_types.len() == 1 && required_resource_types[0] == "*"; let mut resources: BTreeMap = BTreeMap::new(); let mut provider_resources: Vec = Vec::new(); + let mut remaining_required_resource_types = required_resource_types.clone(); // try DSC_RESOURCE_PATH env var first otherwise use PATH let path_env = match env::var_os("DSC_RESOURCE_PATH") { Some(value) => value, @@ -61,22 +55,38 @@ impl ResourceDiscovery for CommandDiscovery { let manifest = import_manifest(resource.manifest.clone().unwrap())?; if manifest.provider.is_some() { provider_resources.push(resource.type_name.clone()); + resources.insert(resource.type_name.clone(), resource.clone()); + } + } + if return_all_resources + { + resources.insert(resource.type_name.clone(), resource); + } + else { + if remaining_required_resource_types.contains(&resource.type_name) + { + remaining_required_resource_types.retain(|x| *x != resource.type_name); + debug!("Found {} in {}", &resource.type_name, path.display()); + resources.insert(resource.type_name.clone(), resource); + if remaining_required_resource_types.len() == 0 + { + return Ok(resources); + } } } - resources.insert(resource.type_name.clone(), resource); } } } } } - debug!("Found {} non-provider resources", resources.len()); + debug!("Found {} non-provider resources", resources.len() - provider_resources.len()); // now go through the provider resources and add them to the list of resources for provider in provider_resources { + debug!("Enumerating resources for provider {}", provider); let provider_resource = resources.get(&provider).unwrap(); let provider_type_name = provider_resource.type_name.clone(); - let provider_path = provider_resource.path.clone(); let manifest = import_manifest(provider_resource.manifest.clone().unwrap())?; let mut provider_resources_count = 0; // invoke the list command @@ -101,8 +111,23 @@ impl ResourceDiscovery for CommandDiscovery { error!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); continue; } - resources.insert(resource.type_name.clone(), resource); - provider_resources_count += 1; + if return_all_resources + { + resources.insert(resource.type_name.clone(), resource); + provider_resources_count += 1; + } + else { + if remaining_required_resource_types.contains(&resource.type_name) + { + remaining_required_resource_types.retain(|x| *x != resource.type_name); + debug!("Found {} in {}", &resource.type_name, &resource.path); + resources.insert(resource.type_name.clone(), resource); + if remaining_required_resource_types.len() == 0 + { + return Ok(resources); + } + } + } }, Result::Err(err) => { error!("Failed to parse resource: {line} -> {err}"); @@ -118,6 +143,27 @@ impl ResourceDiscovery for CommandDiscovery { } } +impl Default for CommandDiscovery { + fn default() -> Self { + Self::new() + } +} + +impl ResourceDiscovery for CommandDiscovery { + + fn list_available_resources(&mut self) -> Result, DscError> { + + let required_resource_types = vec!["*".to_string()]; + self.search_for_resources(&required_resource_types) + } + + + fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError> + { + self.search_for_resources(required_resource_types) + } +} + fn load_manifest(path: &Path) -> Result { let file = File::open(path)?; let reader = BufReader::new(file); diff --git a/dsc_lib/src/discovery/discovery_trait.rs b/dsc_lib/src/discovery/discovery_trait.rs index 2fc9fb40..25208330 100644 --- a/dsc_lib/src/discovery/discovery_trait.rs +++ b/dsc_lib/src/discovery/discovery_trait.rs @@ -6,4 +6,5 @@ use std::collections::BTreeMap; pub trait ResourceDiscovery { fn list_available_resources(&mut self) -> Result, DscError>; + fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError>; } diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index 1769dae2..2ffc3bb5 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -5,13 +5,13 @@ mod command_discovery; mod discovery_trait; use crate::discovery::discovery_trait::ResourceDiscovery; -use crate::{dscresources::dscresource::DscResource, dscerror::DscError, dscerror::StreamMessageType}; -use regex::RegexBuilder; +use crate::{dscresources::dscresource::DscResource, dscerror::DscError}; +use regex::{RegexBuilder, Regex}; use std::collections::BTreeMap; use tracing::{debug, error}; pub struct Discovery { - resources: BTreeMap, + pub resources: BTreeMap, } impl Discovery { @@ -27,13 +27,16 @@ impl Discovery { Box::new(command_discovery::CommandDiscovery::new()), ]; - let regex_str = convert_wildcard_to_regex(type_name_filter); - let mut regex_builder = RegexBuilder::new(regex_str.as_str()); - debug!("Using regex {regex_str} as filter for resource type"); - regex_builder.case_insensitive(true); - let Ok(regex) = regex_builder.build() else { - return vec![]; - }; + let mut regex: Option> = None; + if !type_name_filter.is_empty() + { + let regex_str = convert_wildcard_to_regex(type_name_filter); + let mut regex_builder = RegexBuilder::new(regex_str.as_str()); + debug!("Using regex {regex_str} as filter for resource type"); + regex_builder.case_insensitive(true); + let reg_v = regex_builder.build().unwrap(); + regex = Some(Box::new(reg_v)); + } let mut resources: Vec = Vec::new(); @@ -45,11 +48,15 @@ impl Discovery { error!("{err}"); continue; } - }; + }; for resource in discovered_resources { - if type_name_filter.is_empty() | regex.is_match(resource.0.as_str()) { + if type_name_filter.is_empty() { resources.push(resource.1); + } else { + if regex.as_ref().unwrap().is_match(resource.0.as_str()) { + resources.push(resource.1); + } } }; } @@ -60,6 +67,30 @@ impl Discovery { pub fn find_resource(&self, type_name: &str) -> Option<&DscResource> { self.resources.get(type_name) } + + pub fn discover_resources(&mut self, required_resource_types: Vec) { + + let discovery_types: Vec> = vec![ + Box::new(command_discovery::CommandDiscovery::new()), + ]; + + let mut remaining_required_resource_types = required_resource_types.clone(); + for mut discovery_type in discovery_types { + + let discovered_resources = match discovery_type.discover_resources(&remaining_required_resource_types) { + Ok(value) => value, + Err(err) => { + error!("{err}"); + continue; + } + }; + + for resource in discovered_resources { + self.resources.insert(resource.0.clone(), resource.1); + remaining_required_resource_types.retain(|x| *x != resource.0); + }; + } + } } fn convert_wildcard_to_regex(wildcard: &str) -> String { diff --git a/dsc_lib/src/lib.rs b/dsc_lib/src/lib.rs index 76e1c5aa..2e74f347 100644 --- a/dsc_lib/src/lib.rs +++ b/dsc_lib/src/lib.rs @@ -12,6 +12,9 @@ pub mod dscresources; pub mod functions; pub mod parser; +use dscerror::DscError; +use dscresources::{dscresource::{DscResource, Invoke}, invoke_result::{GetResult, SetResult, TestResult}}; + pub struct DscManager { discovery: discovery::Discovery, } @@ -43,6 +46,10 @@ impl DscManager { pub fn list_available_resources(&mut self, type_name_filter: &str) -> Vec { self.discovery.list_available_resources(type_name_filter) } + + pub fn discover_resources(&mut self, required_resource_types: Vec) { + self.discovery.discover_resources(required_resource_types) + } /// Invoke the get operation on a resource. /// /// # Arguments From c8db1b49746786d856b657ada74a09df4a44754b Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 10 Oct 2023 20:21:23 -0700 Subject: [PATCH 04/20] Removed old tracing APIs --- dsc_lib/src/dscerror.rs | 127 ---------------------------------------- 1 file changed, 127 deletions(-) diff --git a/dsc_lib/src/dscerror.rs b/dsc_lib/src/dscerror.rs index 48097fe3..76471710 100644 --- a/dsc_lib/src/dscerror.rs +++ b/dsc_lib/src/dscerror.rs @@ -92,130 +92,3 @@ pub enum DscError { #[error("Validation: {0}")] Validation(String), } - -//TODO: remove this and use Tracing APIs instead -#[derive(Debug)] -#[derive(PartialEq)] -pub enum StreamMessageType { - None = 0, - Data = 1, - Error = 2, - Warning = 3, - Verbose = 4, - Custom = 5 -} - -//TODO: remove this and use Tracing APIs instead -pub struct StreamMessage { - pub message: String, - pub message_type: StreamMessageType, - pub time: DateTime, - pub resource_type_name: String, - pub resource_path: String -} - -//TODO: remove this and use Tracing APIs instead -impl Default for StreamMessage { - fn default() -> Self { - Self::new() - } -} - -//TODO: remove this and use Tracing APIs instead -impl StreamMessage { - /// Create a new message - #[must_use] - pub fn new() -> Self { - Self { - message: String::new(), - message_type: StreamMessageType::None, - time: Local::now(), - resource_type_name: String::new(), - resource_path: String::new(), - } - } - - //TODO: remove this and use Tracing APIs instead - /// Create a new error message - /// - /// # Arguments - /// - /// * `message` - The message to display - /// * `resource_type_name` - The name of the resource type - /// * `resource_path` - The path to the resource - /// - /// # Returns - /// - /// * `StreamMessage` - The new message - #[must_use] - pub fn new_error(message: String, resource_type_name: Option, resource_path: Option) -> StreamMessage { - StreamMessage { - message, - message_type: StreamMessageType::Error, - time: Local::now(), - resource_type_name: resource_type_name.unwrap_or("None".to_string()), - resource_path: resource_path.unwrap_or("None".to_string()) - } - } - - //TODO: remove this and use Tracing APIs instead - /// Create a new warning message - /// - /// # Arguments - /// - /// * `message` - The message to display - /// * `resource_type_name` - The name of the resource type - /// * `resource_path` - The path to the resource - /// - /// # Returns - /// - /// * `StreamMessage` - The new message - #[must_use] - pub fn new_warning(message: String, resource_type_name: Option, resource_path: Option) -> StreamMessage { - StreamMessage { - message, - message_type: StreamMessageType::Warning, - time: Local::now(), - resource_type_name: resource_type_name.unwrap_or("None".to_string()), - resource_path: resource_path.unwrap_or("None".to_string()) - } - } - - //TODO: remove this and use Tracing APIs instead - /// Print the message to the console - /// - /// # Arguments - /// - /// * `error_format` - The format to use for error messages - /// * `warning_format` - The format to use for warning messages - /// - /// # Errors - /// - /// * `DscError` - If there is an error writing to the console - pub fn print(&self, error_format:&StreamMessageType, warning_format:&StreamMessageType) -> Result<(), DscError>{ - if self.message_type == StreamMessageType::Error - { - if error_format == &StreamMessageType::Error - { - error!("{:?} -> {} {}", error_format, self.resource_type_name, self.message); - } - else - { - warn!("{:?} -> {} {}", warning_format, self.resource_type_name, self.message); - } - } - if self.message_type == StreamMessageType::Warning - { - if warning_format == &StreamMessageType::Error - { - error!("{:?} -> {} {}", warning_format, self.resource_type_name, self.message); - } - else - { - warn!("{:?} -> {} {}", warning_format, self.resource_type_name, self.message); - } - } - - Ok(()) - } -} From 870609927af3f98fb497dab11b3e7bb9f0e9b7fb Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 18 Oct 2023 21:14:37 -0700 Subject: [PATCH 05/20] Updated error handling --- dsc/src/main.rs | 4 +- dsc/src/resource_command.rs | 45 ++++++++++++++------ dsc_lib/src/configure/mod.rs | 16 ++++--- dsc_lib/src/discovery/command_discovery.rs | 39 +++++++++++++---- powershellgroup/powershellgroup.resource.ps1 | 4 +- 5 files changed, 79 insertions(+), 29 deletions(-) diff --git a/dsc/src/main.rs b/dsc/src/main.rs index c75744e9..3fb2fe9d 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -8,7 +8,7 @@ use clap_complete::generate; use std::io::{self, Read}; use std::process::exit; use sysinfo::{Process, ProcessExt, RefreshKind, System, SystemExt, get_current_pid, ProcessRefreshKind}; -use tracing::{Level, error, info, warn}; +use tracing::{error, info, warn}; #[cfg(debug_assertions)] use crossterm::event; @@ -26,7 +26,7 @@ fn main() { check_debug(); // create subscriber that writes all events to stderr - let subscriber = tracing_subscriber::fmt().pretty().with_max_level(Level::DEBUG).with_writer(std::io::stderr).finish(); + let subscriber = tracing_subscriber::fmt().pretty()/*.with_max_level(Level::DEBUG)*/.with_writer(std::io::stderr).finish(); if tracing::subscriber::set_global_default(subscriber).is_err() { eprintln!("Unable to set global default subscriber"); } diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index ffe97435..24860a27 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -6,6 +6,7 @@ use crate::util::{EXIT_DSC_ERROR, EXIT_INVALID_ARGS, EXIT_JSON_ERROR, add_type_n use dsc_lib::configure::config_doc::Configuration; use dsc_lib::configure::add_resource_export_results_to_configuration; use dsc_lib::dscresources::invoke_result::GetResult; +use dsc_lib::dscerror::DscError; use tracing::{error, debug}; use dsc_lib::{ @@ -14,10 +15,15 @@ use dsc_lib::{ }; use std::process::exit; -pub fn get(dsc: &DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { // TODO: support streaming stdin which includes resource and input let mut input = get_input(input, stdin); - let mut resource = get_resource(dsc, resource).unwrap(); + + let Some(mut resource) = get_resource(dsc, resource_str) else { + error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + return + }; + debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); @@ -43,8 +49,11 @@ pub fn get(dsc: &DscManager, resource: &str, input: &Option, stdin: &Opt } } -pub fn get_all(dsc: &DscManager, resource: &str, _input: &Option, _stdin: &Option, format: &Option) { - let resource = get_resource(dsc, resource).unwrap(); +pub fn get_all(dsc: &DscManager, resource_str: &str, _input: &Option, _stdin: &Option, format: &Option) { + let Some(resource) = get_resource(dsc, resource_str) else { + error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + return + }; debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); let export_result = match resource.export() { Ok(export) => { export } @@ -71,14 +80,17 @@ pub fn get_all(dsc: &DscManager, resource: &str, _input: &Option, _stdin } } -pub fn set(dsc: &DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); if input.is_empty() { error!("Error: Input is empty"); exit(EXIT_INVALID_ARGS); } - let mut resource = get_resource(dsc, resource).unwrap(); + let Some(mut resource) = get_resource(dsc, resource_str) else { + error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + return + }; debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); @@ -106,9 +118,12 @@ pub fn set(dsc: &DscManager, resource: &str, input: &Option, stdin: &Opt } } -pub fn test(dsc: &DscManager, resource: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn test(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); - let mut resource = get_resource(dsc, resource).unwrap(); + let Some(mut resource) = get_resource(dsc, resource_str) else { + error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + return + }; debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); @@ -136,8 +151,11 @@ pub fn test(dsc: &DscManager, resource: &str, input: &Option, stdin: &Op } } -pub fn schema(dsc: &DscManager, resource: &str, format: &Option) { - let resource = get_resource(dsc, resource).unwrap(); +pub fn schema(dsc: &DscManager, resource_str: &str, format: &Option) { + let Some(resource) = get_resource(dsc, resource_str) else { + error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + return + }; match resource.schema() { Ok(json) => { // verify is json @@ -157,8 +175,11 @@ pub fn schema(dsc: &DscManager, resource: &str, format: &Option) { } } -pub fn export(dsc: &mut DscManager, resource: &str, format: &Option) { - let dsc_resource = get_resource(dsc, resource).unwrap(); +pub fn export(dsc: &mut DscManager, resource_str: &str, format: &Option) { + let Some(resource) = get_resource(dsc, resource_str) else { + error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + return + }; let mut conf = Configuration::new(); diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 0a364bc9..7b94d6ca 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -83,7 +83,7 @@ impl Configurator { /// # Errors /// /// This function will return an error if the underlying resource fails. - pub fn invoke_get(&self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { + pub fn invoke_get(&mut self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { let (config, messages, had_errors) = self.validate_config()?; let mut result = ConfigurationGetResult::new(); result.messages = messages; @@ -119,7 +119,7 @@ impl Configurator { /// # Errors /// /// This function will return an error if the underlying resource fails. - pub fn invoke_set(&self, skip_test: bool, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { + pub fn invoke_set(&mut self, skip_test: bool, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { let (config, messages, had_errors) = self.validate_config()?; let mut result = ConfigurationSetResult::new(); result.messages = messages; @@ -155,7 +155,7 @@ impl Configurator { /// # Errors /// /// This function will return an error if the underlying resource fails. - pub fn invoke_test(&self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { + pub fn invoke_test(&mut self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { let (config, messages, had_errors) = self.validate_config()?; let mut result = ConfigurationTestResult::new(); result.messages = messages; @@ -216,7 +216,7 @@ impl Configurator { /// # Errors /// /// This function will return an error if the underlying resource fails. - pub fn invoke_export(&self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { + pub fn invoke_export(&mut self, _error_action: ErrorAction, _progress_callback: impl Fn() + 'static) -> Result { let (config, messages, had_errors) = self.validate_config()?; let duplicates = Self::find_duplicate_resource_types(&config); @@ -249,10 +249,16 @@ impl Configurator { Ok(result) } - fn validate_config(&self) -> Result<(Configuration, Vec, bool), DscError> { + fn validate_config(&mut self) -> Result<(Configuration, Vec, bool), DscError> { let config: Configuration = serde_json::from_str(self.config.as_str())?; let mut messages: Vec = Vec::new(); let mut has_errors = false; + + // Perform discovery of resources used in config + let required_resources = (&config.resources).into_iter().map(|p| p.resource_type.clone()).collect(); + self.discovery.discover_resources(required_resources); + + // Now perform the validation for resource in &config.resources { let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index ec4ab70d..26ff6acf 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -11,7 +11,7 @@ use std::env; use std::fs::File; use std::io::BufReader; use std::path::Path; -use tracing::{debug, error}; +use tracing::{debug, error, warn}; pub struct CommandDiscovery { } @@ -50,7 +50,24 @@ impl CommandDiscovery { if path.is_file() { let file_name = path.file_name().unwrap().to_str().unwrap(); if file_name.to_lowercase().ends_with(".dsc.resource.json") { - let resource = load_manifest(&path)?; + let resource = match load_manifest(&path) + { + Ok(r) => r, + Err(e) => { + if return_all_resources { + // In case of "resource list" operation - print all failures to read manifests as warnings + warn!("{}", e); + } else { + // In case of other resource/config operations: + // At this point we can't determine whether or not the bad manifest contains resource that is requested by resource/config operation + // if it is, then "ResouceNotFound" error will be issued later + // and here we just record the error into debug stream. + debug!("{}", e); + } + continue; + }, + }; + if resource.manifest.is_some() { let manifest = import_manifest(resource.manifest.clone().unwrap())?; if manifest.provider.is_some() { @@ -80,7 +97,7 @@ impl CommandDiscovery { } } - debug!("Found {} non-provider resources", resources.len() - provider_resources.len()); + debug!("Found {} matching non-provider resources", resources.len() - provider_resources.len()); // now go through the provider resources and add them to the list of resources for provider in provider_resources { @@ -95,20 +112,26 @@ impl CommandDiscovery { { Ok((exit_code, stdout, stderr)) => (exit_code, stdout, stderr), Err(e) => { - error!("Could not start {}: {}", list_command.executable, e); + // In case of "resource list" operation - print failure from provider as warning + // In case of other resource/config operations: + // print failure from provider as error because this provider was specifically requested by current resource/config operation + if return_all_resources { warn!("Could not start {}: {}", list_command.executable, e); } else { error!("Could not start {}: {}", list_command.executable, e); } continue; }, }; if exit_code != 0 { - error!("Provider failed to list resources with exit code {exit_code}: {stderr}"); + // In case of "resource list" operation - print failure from provider as warning + // In case of other resource/config operations: + // print failure from provider as error because this provider was specifically requested by current resource/config operation + if return_all_resources { warn!("Provider failed to list resources with exit code {exit_code}: {stderr}"); } else { error!("Provider failed to list resources with exit code {exit_code}: {stderr}"); } } for line in stdout.lines() { match serde_json::from_str::(line){ Result::Ok(resource) => { if resource.requires.is_none() { - error!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); + if return_all_resources { warn!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); } else { error!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); } continue; } if return_all_resources @@ -130,13 +153,13 @@ impl CommandDiscovery { } }, Result::Err(err) => { - error!("Failed to parse resource: {line} -> {err}"); + if return_all_resources { warn!("Failed to parse resource: {line} -> {err}"); } else { error!("Failed to parse resource: {line} -> {err}"); } continue; } }; } - debug!("Provider {} listed {} resources", provider_type_name, provider_resources_count); + debug!("Provider {} listed {} matching resources", provider_type_name, provider_resources_count); } Ok(resources) diff --git a/powershellgroup/powershellgroup.resource.ps1 b/powershellgroup/powershellgroup.resource.ps1 index 838950af..f9765cac 100644 --- a/powershellgroup/powershellgroup.resource.ps1 +++ b/powershellgroup/powershellgroup.resource.ps1 @@ -51,7 +51,7 @@ if ($null -eq $DscModule) exit 1 } -Import-Module $DscModule +Import-Module $DscModule -DisableNameChecking if ($Operation -eq 'List') { @@ -159,7 +159,7 @@ elseif ($Operation -eq 'Get') } } $e = $null - $op_result = Invoke-DscResource -Method Get -Name $ResourceTypeName -Property $inputht -ErrorVariable e + $op_result = Invoke-DscResource -Method Get -Name $ResourceTypeName -Property $inputht -ErrorVariable e -WarningAction SilentlyContinue if ($e) { # By this point Invoke-DscResource already wrote error message to stderr stream, From 905cbfdf8e8ea6f3585820904cbddb826a436d28 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 19 Oct 2023 11:38:42 -0700 Subject: [PATCH 06/20] clippy errors 1 --- dsc_lib/src/configure/mod.rs | 4 +-- dsc_lib/src/discovery/command_discovery.rs | 39 ++++++++++------------ dsc_lib/src/discovery/mod.rs | 6 ++-- dsc_lib/src/lib.rs | 2 +- 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 7b94d6ca..ab877549 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -241,7 +241,7 @@ impl Configurator { let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; - add_resource_export_results_to_configuration(&dsc_resource, &mut conf)?; + add_resource_export_results_to_configuration(dsc_resource, &mut conf)?; } result.result = Some(conf); @@ -255,7 +255,7 @@ impl Configurator { let mut has_errors = false; // Perform discovery of resources used in config - let required_resources = (&config.resources).into_iter().map(|p| p.resource_type.clone()).collect(); + let required_resources = (&config.resources).iter().map(|p| p.resource_type.clone()).collect(); self.discovery.discover_resources(required_resources); // Now perform the validation diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 26ff6acf..6abdd742 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -22,7 +22,8 @@ impl CommandDiscovery { } } - fn search_for_resources(&mut self, required_resource_types: &Vec) -> Result, DscError> + #[allow(clippy::too_many_lines)] + fn search_for_resources(required_resource_types: &Vec) -> Result, DscError> { let return_all_resources = required_resource_types.len() == 1 && required_resource_types[0] == "*"; @@ -79,16 +80,14 @@ impl CommandDiscovery { { resources.insert(resource.type_name.clone(), resource); } - else { - if remaining_required_resource_types.contains(&resource.type_name) + else if remaining_required_resource_types.contains(&resource.type_name) + { + remaining_required_resource_types.retain(|x| *x != resource.type_name); + debug!("Found {} in {}", &resource.type_name, path.display()); + resources.insert(resource.type_name.clone(), resource); + if remaining_required_resource_types.is_empty() { - remaining_required_resource_types.retain(|x| *x != resource.type_name); - debug!("Found {} in {}", &resource.type_name, path.display()); - resources.insert(resource.type_name.clone(), resource); - if remaining_required_resource_types.len() == 0 - { - return Ok(resources); - } + return Ok(resources); } } } @@ -139,16 +138,14 @@ impl CommandDiscovery { resources.insert(resource.type_name.clone(), resource); provider_resources_count += 1; } - else { - if remaining_required_resource_types.contains(&resource.type_name) + else if remaining_required_resource_types.contains(&resource.type_name) + { + remaining_required_resource_types.retain(|x| *x != resource.type_name); + debug!("Found {} in {}", &resource.type_name, &resource.path); + resources.insert(resource.type_name.clone(), resource); + if remaining_required_resource_types.is_empty() { - remaining_required_resource_types.retain(|x| *x != resource.type_name); - debug!("Found {} in {}", &resource.type_name, &resource.path); - resources.insert(resource.type_name.clone(), resource); - if remaining_required_resource_types.len() == 0 - { - return Ok(resources); - } + return Ok(resources); } } }, @@ -177,13 +174,13 @@ impl ResourceDiscovery for CommandDiscovery { fn list_available_resources(&mut self) -> Result, DscError> { let required_resource_types = vec!["*".to_string()]; - self.search_for_resources(&required_resource_types) + CommandDiscovery::search_for_resources(&required_resource_types) } fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError> { - self.search_for_resources(required_resource_types) + CommandDiscovery::search_for_resources(required_resource_types) } } diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index 2ffc3bb5..85086c94 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -53,10 +53,8 @@ impl Discovery { for resource in discovered_resources { if type_name_filter.is_empty() { resources.push(resource.1); - } else { - if regex.as_ref().unwrap().is_match(resource.0.as_str()) { - resources.push(resource.1); - } + } else if regex.as_ref().unwrap().is_match(resource.0.as_str()) { + resources.push(resource.1); } }; } diff --git a/dsc_lib/src/lib.rs b/dsc_lib/src/lib.rs index 2e74f347..4b5af28b 100644 --- a/dsc_lib/src/lib.rs +++ b/dsc_lib/src/lib.rs @@ -48,7 +48,7 @@ impl DscManager { } pub fn discover_resources(&mut self, required_resource_types: Vec) { - self.discovery.discover_resources(required_resource_types) + self.discovery.discover_resources(required_resource_types); } /// Invoke the get operation on a resource. /// From bcf484551ba5d9d10970483013f5768137354efc Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 19 Oct 2023 12:29:14 -0700 Subject: [PATCH 07/20] clippy errors 2 --- dsc_lib/src/configure/mod.rs | 2 +- dsc_lib/src/discovery/command_discovery.rs | 6 +++--- dsc_lib/src/discovery/discovery_trait.rs | 2 +- dsc_lib/src/discovery/mod.rs | 20 +++++++++++++++----- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index ab877549..43c5ec08 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -255,7 +255,7 @@ impl Configurator { let mut has_errors = false; // Perform discovery of resources used in config - let required_resources = (&config.resources).iter().map(|p| p.resource_type.clone()).collect(); + let required_resources = config.resources.iter().map(|p| p.resource_type.clone()).collect(); self.discovery.discover_resources(required_resources); // Now perform the validation diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 6abdd742..85efd5ca 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -23,7 +23,7 @@ impl CommandDiscovery { } #[allow(clippy::too_many_lines)] - fn search_for_resources(required_resource_types: &Vec) -> Result, DscError> + fn search_for_resources(required_resource_types: Vec) -> Result, DscError> { let return_all_resources = required_resource_types.len() == 1 && required_resource_types[0] == "*"; @@ -174,11 +174,11 @@ impl ResourceDiscovery for CommandDiscovery { fn list_available_resources(&mut self) -> Result, DscError> { let required_resource_types = vec!["*".to_string()]; - CommandDiscovery::search_for_resources(&required_resource_types) + CommandDiscovery::search_for_resources(required_resource_types) } - fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError> + fn discover_resources(&mut self, required_resource_types: Vec) -> Result, DscError> { CommandDiscovery::search_for_resources(required_resource_types) } diff --git a/dsc_lib/src/discovery/discovery_trait.rs b/dsc_lib/src/discovery/discovery_trait.rs index 25208330..3898eefd 100644 --- a/dsc_lib/src/discovery/discovery_trait.rs +++ b/dsc_lib/src/discovery/discovery_trait.rs @@ -6,5 +6,5 @@ use std::collections::BTreeMap; pub trait ResourceDiscovery { fn list_available_resources(&mut self) -> Result, DscError>; - fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError>; + fn discover_resources(&mut self, required_resource_types: Vec) -> Result, DscError>; } diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index 85086c94..91725f1d 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -15,13 +15,24 @@ pub struct Discovery { } impl Discovery { - + /// Create a new `Discovery` instance. + /// + /// # Errors + /// + /// This function will return an error if the underlying instance creation fails. + /// pub fn new() -> Result { Ok(Self { resources: BTreeMap::new(), }) } + /// List operation. + /// + /// # Panics + /// + /// Will panic if RegEx creation fails. + /// pub fn list_available_resources(&mut self, type_name_filter: &str) -> Vec { let discovery_types: Vec> = vec![ Box::new(command_discovery::CommandDiscovery::new()), @@ -51,9 +62,7 @@ impl Discovery { }; for resource in discovered_resources { - if type_name_filter.is_empty() { - resources.push(resource.1); - } else if regex.as_ref().unwrap().is_match(resource.0.as_str()) { + if type_name_filter.is_empty() || regex.as_ref().unwrap().is_match(resource.0.as_str()) { resources.push(resource.1); } }; @@ -62,6 +71,7 @@ impl Discovery { resources } + #[must_use] pub fn find_resource(&self, type_name: &str) -> Option<&DscResource> { self.resources.get(type_name) } @@ -75,7 +85,7 @@ impl Discovery { let mut remaining_required_resource_types = required_resource_types.clone(); for mut discovery_type in discovery_types { - let discovered_resources = match discovery_type.discover_resources(&remaining_required_resource_types) { + let discovered_resources = match discovery_type.discover_resources(remaining_required_resource_types.clone()) { Ok(value) => value, Err(err) => { error!("{err}"); From a2dfc9bed4bc782323f81b9857e04561a3045708 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 19 Oct 2023 12:48:23 -0700 Subject: [PATCH 08/20] clippy errors 3 --- dsc_lib/src/configure/mod.rs | 4 ++-- dsc_lib/src/discovery/command_discovery.rs | 8 ++++---- dsc_lib/src/discovery/mod.rs | 6 +++--- dsc_lib/src/lib.rs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 43c5ec08..6ce3503a 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -255,8 +255,8 @@ impl Configurator { let mut has_errors = false; // Perform discovery of resources used in config - let required_resources = config.resources.iter().map(|p| p.resource_type.clone()).collect(); - self.discovery.discover_resources(required_resources); + let required_resources = config.resources.iter().map(|p| p.resource_type.clone()).collect::>(); + self.discovery.discover_resources(&required_resources); // Now perform the validation for resource in &config.resources { diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 85efd5ca..a374f6c4 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -23,13 +23,13 @@ impl CommandDiscovery { } #[allow(clippy::too_many_lines)] - fn search_for_resources(required_resource_types: Vec) -> Result, DscError> + fn search_for_resources(required_resource_types: &[String]) -> Result, DscError> { let return_all_resources = required_resource_types.len() == 1 && required_resource_types[0] == "*"; let mut resources: BTreeMap = BTreeMap::new(); let mut provider_resources: Vec = Vec::new(); - let mut remaining_required_resource_types = required_resource_types.clone(); + let mut remaining_required_resource_types = required_resource_types.to_owned(); // try DSC_RESOURCE_PATH env var first otherwise use PATH let path_env = match env::var_os("DSC_RESOURCE_PATH") { Some(value) => value, @@ -174,13 +174,13 @@ impl ResourceDiscovery for CommandDiscovery { fn list_available_resources(&mut self) -> Result, DscError> { let required_resource_types = vec!["*".to_string()]; - CommandDiscovery::search_for_resources(required_resource_types) + CommandDiscovery::search_for_resources(&required_resource_types) } fn discover_resources(&mut self, required_resource_types: Vec) -> Result, DscError> { - CommandDiscovery::search_for_resources(required_resource_types) + CommandDiscovery::search_for_resources(&required_resource_types) } } diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index 91725f1d..5436ab37 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -31,7 +31,7 @@ impl Discovery { /// /// # Panics /// - /// Will panic if RegEx creation fails. + /// Will panic if `RegEx` creation fails. /// pub fn list_available_resources(&mut self, type_name_filter: &str) -> Vec { let discovery_types: Vec> = vec![ @@ -76,13 +76,13 @@ impl Discovery { self.resources.get(type_name) } - pub fn discover_resources(&mut self, required_resource_types: Vec) { + pub fn discover_resources(&mut self, required_resource_types: &[String]) { let discovery_types: Vec> = vec![ Box::new(command_discovery::CommandDiscovery::new()), ]; - let mut remaining_required_resource_types = required_resource_types.clone(); + let mut remaining_required_resource_types = required_resource_types.to_owned(); for mut discovery_type in discovery_types { let discovered_resources = match discovery_type.discover_resources(remaining_required_resource_types.clone()) { diff --git a/dsc_lib/src/lib.rs b/dsc_lib/src/lib.rs index 4b5af28b..87d596b0 100644 --- a/dsc_lib/src/lib.rs +++ b/dsc_lib/src/lib.rs @@ -48,7 +48,7 @@ impl DscManager { } pub fn discover_resources(&mut self, required_resource_types: Vec) { - self.discovery.discover_resources(required_resource_types); + self.discovery.discover_resources(&required_resource_types); } /// Invoke the get operation on a resource. /// From e50e095f4f09e647f649ad453f82d835f62dc60c Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 19 Oct 2023 13:22:04 -0700 Subject: [PATCH 09/20] clippy errors 4 --- dsc/src/resource_command.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 24860a27..48303a20 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -15,6 +15,12 @@ use dsc_lib::{ }; use std::process::exit; +/// Get operation. +/// +/// # Panics +/// +/// Will panic if provider-based resource is not found. +/// pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { // TODO: support streaming stdin which includes resource and input let mut input = get_input(input, stdin); @@ -27,7 +33,7 @@ pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - resource = get_resource(dsc, &requires).unwrap(); + resource = get_resource(dsc, requires).unwrap(); } match resource.get(input.as_str()) { @@ -80,6 +86,12 @@ pub fn get_all(dsc: &DscManager, resource_str: &str, _input: &Option, _s } } +/// Set operation. +/// +/// # Panics +/// +/// Will panic if provider-based resource is not found. +/// pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); if input.is_empty() { @@ -96,7 +108,7 @@ pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - resource = get_resource(dsc, &requires).unwrap(); + resource = get_resource(dsc, requires).unwrap(); } match resource.set(input.as_str(), true) { @@ -118,6 +130,12 @@ pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: } } +/// Test operation. +/// +/// # Panics +/// +/// Will panic if provider-based resource is not found. +/// pub fn test(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); let Some(mut resource) = get_resource(dsc, resource_str) else { @@ -129,7 +147,7 @@ pub fn test(dsc: &DscManager, resource_str: &str, input: &Option, stdin: if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - resource = get_resource(dsc, &requires).unwrap(); + resource = get_resource(dsc, requires).unwrap(); } match resource.test(input.as_str()) { @@ -198,6 +216,7 @@ pub fn export(dsc: &mut DscManager, resource_str: &str, format: &Option(dsc: &'a DscManager, resource: &str) -> Option<&'a DscResource> { //TODO: add dinamically generated resource to dsc // check if resource is JSON or just a name From e8f2abe307e5dbba0b2e5ad6e53da42885de649c Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 19 Oct 2023 14:05:32 -0700 Subject: [PATCH 10/20] clippy errors 6 --- dsc_lib/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsc_lib/src/lib.rs b/dsc_lib/src/lib.rs index 87d596b0..44d617b4 100644 --- a/dsc_lib/src/lib.rs +++ b/dsc_lib/src/lib.rs @@ -47,8 +47,8 @@ impl DscManager { self.discovery.list_available_resources(type_name_filter) } - pub fn discover_resources(&mut self, required_resource_types: Vec) { - self.discovery.discover_resources(&required_resource_types); + pub fn discover_resources(&mut self, required_resource_types: &[String]) { + self.discovery.discover_resources(required_resource_types); } /// Invoke the get operation on a resource. /// From 1c308db4acca5140782ba0c297355c66722823d0 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 19 Oct 2023 14:56:27 -0700 Subject: [PATCH 11/20] clippy errors 7 --- dsc_lib/src/configure/depends_on.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc_lib/src/configure/depends_on.rs b/dsc_lib/src/configure/depends_on.rs index 2f0e3ad3..41efc655 100644 --- a/dsc_lib/src/configure/depends_on.rs +++ b/dsc_lib/src/configure/depends_on.rs @@ -22,7 +22,7 @@ use crate::DscError; /// * `DscError::Validation` - The configuration is invalid pub fn get_resource_invocation_order(config: &Configuration) -> Result, DscError> { let mut order: Vec = Vec::new(); - let depends_on_regex = Regex::new(r"^\[resourceId\(\s*'([a-zA-Z0-9\.]+/[a-zA-Z0-9]+)'\s*,\s*'([a-zA-Z0-9 ]+)'\s*\)]$")?; + let depends_on_regex = Regex::new(r"^\[resourceId\(\s*'(?[a-zA-Z0-9\.]+/[a-zA-Z0-9]+)'\s*,\s*'(?[a-zA-Z0-9 ]+)'\s*\)]$")?; for resource in &config.resources { // validate that the resource isn't specified more than once in the config if config.resources.iter().filter(|r| r.name == resource.name && r.resource_type == resource.resource_type).count() > 1 { From c4da340b625cc1975a9ba5b26d02ab20c1c40835 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 26 Oct 2023 14:30:46 -0700 Subject: [PATCH 12/20] Fixed rebase leftovers --- dsc/src/resource_command.rs | 2 +- dsc_lib/src/dscerror.rs | 3 +-- dsc_lib/src/lib.rs | 4 ---- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 48303a20..7a4ba3f4 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -194,7 +194,7 @@ pub fn schema(dsc: &DscManager, resource_str: &str, format: &Option) { - let Some(resource) = get_resource(dsc, resource_str) else { + let Some(dsc_resource) = get_resource(dsc, resource_str) else { error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); return }; diff --git a/dsc_lib/src/dscerror.rs b/dsc_lib/src/dscerror.rs index 76471710..72af079d 100644 --- a/dsc_lib/src/dscerror.rs +++ b/dsc_lib/src/dscerror.rs @@ -5,8 +5,7 @@ use std::str::Utf8Error; use reqwest::StatusCode; use thiserror::Error; -use chrono::{Local, DateTime}; -use tracing::{error, warn}; +use tracing::error; use tree_sitter::LanguageError; #[derive(Error, Debug)] diff --git a/dsc_lib/src/lib.rs b/dsc_lib/src/lib.rs index 44d617b4..56734226 100644 --- a/dsc_lib/src/lib.rs +++ b/dsc_lib/src/lib.rs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use discovery::ResourceIterator; use dscerror::DscError; use dscresources::{dscresource::{DscResource, Invoke}, invoke_result::{GetResult, SetResult, TestResult}}; @@ -12,9 +11,6 @@ pub mod dscresources; pub mod functions; pub mod parser; -use dscerror::DscError; -use dscresources::{dscresource::{DscResource, Invoke}, invoke_result::{GetResult, SetResult, TestResult}}; - pub struct DscManager { discovery: discovery::Discovery, } From 72e3e439c0d77dbf681c32d1d6f3408c3e5e06a1 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 30 Oct 2023 15:53:05 -0700 Subject: [PATCH 13/20] Feedback 1 --- dsc/src/resource_command.rs | 45 ++++++++++------------ dsc/src/subcommand.rs | 31 +++++++-------- dsc_lib/src/discovery/command_discovery.rs | 44 ++++++++++++++------- 3 files changed, 64 insertions(+), 56 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 7a4ba3f4..28572a70 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -15,12 +15,6 @@ use dsc_lib::{ }; use std::process::exit; -/// Get operation. -/// -/// # Panics -/// -/// Will panic if provider-based resource is not found. -/// pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { // TODO: support streaming stdin which includes resource and input let mut input = get_input(input, stdin); @@ -33,7 +27,13 @@ pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - resource = get_resource(dsc, requires).unwrap(); + let provider_resource = get_resource(dsc, requires); + if provider_resource.is_none() { + error!("Provider {} not found", requires); + return; + } else { + resource = provider_resource.unwrap(); + } } match resource.get(input.as_str()) { @@ -108,7 +108,13 @@ pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - resource = get_resource(dsc, requires).unwrap(); + let provider_resource = get_resource(dsc, requires); + if provider_resource.is_none() { + error!("Provider {} not found", requires); + return; + } else { + resource = provider_resource.unwrap(); + } } match resource.set(input.as_str(), true) { @@ -147,7 +153,13 @@ pub fn test(dsc: &DscManager, resource_str: &str, input: &Option, stdin: if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - resource = get_resource(dsc, requires).unwrap(); + let provider_resource = get_resource(dsc, requires); + if provider_resource.is_none() { + error!("Provider {} not found", requires); + return; + } else { + resource = provider_resource.unwrap(); + } } match resource.test(input.as_str()) { @@ -219,21 +231,6 @@ pub fn export(dsc: &mut DscManager, resource_str: &str, format: &Option(dsc: &'a DscManager, resource: &str) -> Option<&'a DscResource> { //TODO: add dinamically generated resource to dsc - // check if resource is JSON or just a name - /*match serde_json::from_str(resource) { - Ok(resource) => - { - - resource - }, - Err(err) => { - if resource.contains('{') { - error!("Not valid resource JSON: {err}\nInput was: {resource}"); - exit(EXIT_INVALID_ARGS); - } - } - }*/ - dsc.find_resource(resource) } diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index e9573da8..2c68381c 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -14,7 +14,7 @@ use dsc_lib::{ dscresources::dscresource::{ImplementedAs, Invoke}, dscresources::resource_manifest::{import_manifest, ResourceManifest}, }; -use jsonschema::{JSONSchema, ValidationError}; +use jsonschema::JSONSchema; use serde_yaml::Value; use std::process::exit; @@ -172,11 +172,6 @@ pub fn config(subcommand: &ConfigSubCommand, format: &Option, stdi } /// Validate configuration. -/// -/// # Panics -/// -/// Will panic if resource is not found. -/// #[allow(clippy::too_many_lines)] pub fn validate_config(config: &str) { // first validate against the config schema @@ -229,7 +224,10 @@ pub fn validate_config(config: &str) { exit(EXIT_INVALID_INPUT); }); // get the actual resource - let resource = get_resource(&dsc, type_name).unwrap(); + let Some(resource) = get_resource(&dsc, type_name) else { + error!("Error: Resource type not found"); + exit(EXIT_DSC_ERROR); + }; // see if the resource is command based if resource.implemented_as == ImplementedAs::Command { // if so, see if it implements validate via the resource manifest @@ -278,17 +276,14 @@ pub fn validate_config(config: &str) { }, }; let properties = resource_block["properties"].clone(); - let _result: Result<(), ValidationError> = match compiled_schema.validate(&properties) { - Ok(()) => Ok(()), - Err(err) => { - let mut error = String::new(); - for e in err { - error.push_str(&format!("{e} ")); - } - - error!("Error: Resource {type_name} failed validation: {error}"); - exit(EXIT_VALIDATION_FAILED); - }, + let validation = compiled_schema.validate(&properties); + if let Err(err) = validation { + let mut error = String::new(); + for e in err { + error.push_str(&format!("{e} ")); + } + error!("Error: Resource {type_name} failed validation: {error}"); + exit(EXIT_VALIDATION_FAILED); }; } } diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index a374f6c4..6e851b67 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -59,10 +59,10 @@ impl CommandDiscovery { // In case of "resource list" operation - print all failures to read manifests as warnings warn!("{}", e); } else { - // In case of other resource/config operations: - // At this point we can't determine whether or not the bad manifest contains resource that is requested by resource/config operation - // if it is, then "ResouceNotFound" error will be issued later - // and here we just record the error into debug stream. + /* In case of other resource/config operations: + At this point we can't determine whether or not the bad manifest contains resource that is requested by resource/config operation + if it is, then "ResouceNotFound" error will be issued later + and here we just record the error into debug stream.*/ debug!("{}", e); } continue; @@ -111,26 +111,38 @@ impl CommandDiscovery { { Ok((exit_code, stdout, stderr)) => (exit_code, stdout, stderr), Err(e) => { - // In case of "resource list" operation - print failure from provider as warning - // In case of other resource/config operations: - // print failure from provider as error because this provider was specifically requested by current resource/config operation - if return_all_resources { warn!("Could not start {}: {}", list_command.executable, e); } else { error!("Could not start {}: {}", list_command.executable, e); } + /* In case of "resource list" operation - print failure from provider as warning + In case of other resource/config operations: + print failure from provider as error because this provider was specifically requested by current resource/config operation*/ + if return_all_resources { + warn!("Could not start {}: {}", list_command.executable, e); + } else { + error!("Could not start {}: {}", list_command.executable, e); + } continue; }, }; if exit_code != 0 { - // In case of "resource list" operation - print failure from provider as warning - // In case of other resource/config operations: - // print failure from provider as error because this provider was specifically requested by current resource/config operation - if return_all_resources { warn!("Provider failed to list resources with exit code {exit_code}: {stderr}"); } else { error!("Provider failed to list resources with exit code {exit_code}: {stderr}"); } + /* In case of "resource list" operation - print failure from provider as warning + In case of other resource/config operations: + print failure from provider as error because this provider was specifically requested by current resource/config operation*/ + if return_all_resources { + warn!("Provider failed to list resources with exit code {exit_code}: {stderr}"); + } else { + error!("Provider failed to list resources with exit code {exit_code}: {stderr}"); + } } for line in stdout.lines() { match serde_json::from_str::(line){ Result::Ok(resource) => { if resource.requires.is_none() { - if return_all_resources { warn!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); } else { error!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); } + if return_all_resources { + warn!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); + } else { + error!("{}", DscError::MissingRequires(provider.clone(), resource.type_name.clone()).to_string()); + } continue; } if return_all_resources @@ -150,7 +162,11 @@ impl CommandDiscovery { } }, Result::Err(err) => { - if return_all_resources { warn!("Failed to parse resource: {line} -> {err}"); } else { error!("Failed to parse resource: {line} -> {err}"); } + if return_all_resources { + warn!("Failed to parse resource: {line} -> {err}"); + } else { + error!("Failed to parse resource: {line} -> {err}"); + } continue; } }; From 627529e4a031a0392be86e70218e49807f8852e8 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 30 Oct 2023 19:38:03 -0700 Subject: [PATCH 14/20] Feedback 2 --- dsc/src/main.rs | 2 +- dsc_lib/src/discovery/command_discovery.rs | 4 ++-- dsc_lib/src/discovery/discovery_trait.rs | 2 +- dsc_lib/src/discovery/mod.rs | 27 +++++++++++----------- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/dsc/src/main.rs b/dsc/src/main.rs index 3fb2fe9d..14570e8c 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -26,7 +26,7 @@ fn main() { check_debug(); // create subscriber that writes all events to stderr - let subscriber = tracing_subscriber::fmt().pretty()/*.with_max_level(Level::DEBUG)*/.with_writer(std::io::stderr).finish(); + let subscriber = tracing_subscriber::fmt().pretty().with_writer(std::io::stderr).finish(); if tracing::subscriber::set_global_default(subscriber).is_err() { eprintln!("Unable to set global default subscriber"); } diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 6e851b67..d6b457ea 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -194,9 +194,9 @@ impl ResourceDiscovery for CommandDiscovery { } - fn discover_resources(&mut self, required_resource_types: Vec) -> Result, DscError> + fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError> { - CommandDiscovery::search_for_resources(&required_resource_types) + CommandDiscovery::search_for_resources(required_resource_types) } } diff --git a/dsc_lib/src/discovery/discovery_trait.rs b/dsc_lib/src/discovery/discovery_trait.rs index 3898eefd..25208330 100644 --- a/dsc_lib/src/discovery/discovery_trait.rs +++ b/dsc_lib/src/discovery/discovery_trait.rs @@ -6,5 +6,5 @@ use std::collections::BTreeMap; pub trait ResourceDiscovery { fn list_available_resources(&mut self) -> Result, DscError>; - fn discover_resources(&mut self, required_resource_types: Vec) -> Result, DscError>; + fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError>; } diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index 5436ab37..38049f8e 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -28,37 +28,38 @@ impl Discovery { } /// List operation. - /// - /// # Panics - /// - /// Will panic if `RegEx` creation fails. - /// pub fn list_available_resources(&mut self, type_name_filter: &str) -> Vec { let discovery_types: Vec> = vec![ Box::new(command_discovery::CommandDiscovery::new()), ]; let mut regex: Option> = None; + let mut resources: Vec = Vec::new(); if !type_name_filter.is_empty() { let regex_str = convert_wildcard_to_regex(type_name_filter); let mut regex_builder = RegexBuilder::new(regex_str.as_str()); debug!("Using regex {regex_str} as filter for resource type"); regex_builder.case_insensitive(true); - let reg_v = regex_builder.build().unwrap(); - regex = Some(Box::new(reg_v)); + match regex_builder.build() { + Ok(reg_v) => { + regex = Some(Box::new(reg_v)); + }, + Err(_) => { + error!("Could not build Regex"); + return resources; + } + } } - let mut resources: Vec = Vec::new(); - for mut discovery_type in discovery_types { let discovered_resources = match discovery_type.list_available_resources() { Ok(value) => value, Err(err) => { - error!("{err}"); - continue; - } + error!("{err}"); + continue; + } }; for resource in discovered_resources { @@ -85,7 +86,7 @@ impl Discovery { let mut remaining_required_resource_types = required_resource_types.to_owned(); for mut discovery_type in discovery_types { - let discovered_resources = match discovery_type.discover_resources(remaining_required_resource_types.clone()) { + let discovered_resources = match discovery_type.discover_resources(&remaining_required_resource_types) { Ok(value) => value, Err(err) => { error!("{err}"); From a5fcae80aeffd05a1171d01431d56d943e912efa Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 30 Oct 2023 21:35:41 -0700 Subject: [PATCH 15/20] clippy fixes 1 --- dsc/src/resource_command.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 28572a70..9198aad0 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -27,13 +27,12 @@ pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - let provider_resource = get_resource(dsc, requires); - if provider_resource.is_none() { + if let Some(pr) = get_resource(dsc, requires) { + resource = pr; + } else { error!("Provider {} not found", requires); return; - } else { - resource = provider_resource.unwrap(); - } + }; } match resource.get(input.as_str()) { @@ -108,13 +107,12 @@ pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - let provider_resource = get_resource(dsc, requires); - if provider_resource.is_none() { + if let Some(pr) = get_resource(dsc, requires) { + resource = pr; + } else { error!("Provider {} not found", requires); return; - } else { - resource = provider_resource.unwrap(); - } + }; } match resource.set(input.as_str(), true) { @@ -153,13 +151,12 @@ pub fn test(dsc: &DscManager, resource_str: &str, input: &Option, stdin: if let Some(requires) = &resource.requires { input = add_type_name_to_json(input, resource.type_name.clone()); - let provider_resource = get_resource(dsc, requires); - if provider_resource.is_none() { + if let Some(pr) = get_resource(dsc, requires) { + resource = pr; + } else { error!("Provider {} not found", requires); return; - } else { - resource = provider_resource.unwrap(); - } + }; } match resource.test(input.as_str()) { @@ -213,7 +210,7 @@ pub fn export(dsc: &mut DscManager, resource_str: &str, format: &Option Date: Mon, 30 Oct 2023 22:23:10 -0700 Subject: [PATCH 16/20] clippy fixes 2 --- dsc_lib/src/discovery/command_discovery.rs | 2 +- dsc_lib/src/discovery/discovery_trait.rs | 2 +- dsc_lib/src/discovery/mod.rs | 14 ++++++-------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index d6b457ea..2530784d 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -194,7 +194,7 @@ impl ResourceDiscovery for CommandDiscovery { } - fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError> + fn discover_resources(&mut self, required_resource_types: &[String]) -> Result, DscError> { CommandDiscovery::search_for_resources(required_resource_types) } diff --git a/dsc_lib/src/discovery/discovery_trait.rs b/dsc_lib/src/discovery/discovery_trait.rs index 25208330..cfd7e039 100644 --- a/dsc_lib/src/discovery/discovery_trait.rs +++ b/dsc_lib/src/discovery/discovery_trait.rs @@ -6,5 +6,5 @@ use std::collections::BTreeMap; pub trait ResourceDiscovery { fn list_available_resources(&mut self) -> Result, DscError>; - fn discover_resources(&mut self, required_resource_types: &Vec) -> Result, DscError>; + fn discover_resources(&mut self, required_resource_types: &[String]) -> Result, DscError>; } diff --git a/dsc_lib/src/discovery/mod.rs b/dsc_lib/src/discovery/mod.rs index 38049f8e..2ee14576 100644 --- a/dsc_lib/src/discovery/mod.rs +++ b/dsc_lib/src/discovery/mod.rs @@ -28,6 +28,7 @@ impl Discovery { } /// List operation. + #[allow(clippy::missing_panics_doc)] // false positive in clippy; this function will never panic pub fn list_available_resources(&mut self, type_name_filter: &str) -> Vec { let discovery_types: Vec> = vec![ Box::new(command_discovery::CommandDiscovery::new()), @@ -41,14 +42,11 @@ impl Discovery { let mut regex_builder = RegexBuilder::new(regex_str.as_str()); debug!("Using regex {regex_str} as filter for resource type"); regex_builder.case_insensitive(true); - match regex_builder.build() { - Ok(reg_v) => { - regex = Some(Box::new(reg_v)); - }, - Err(_) => { - error!("Could not build Regex"); - return resources; - } + if let Ok(reg_v) = regex_builder.build() { + regex = Some(Box::new(reg_v)); + } else { + error!("Could not build Regex"); + return resources; } } From a6ca7974f56dcc35771a7a5d50d16c1be47d9a87 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 30 Oct 2023 23:03:55 -0700 Subject: [PATCH 17/20] Test fixes --- dsc/src/resource_command.rs | 2 +- dsc/src/subcommand.rs | 10 +++++----- dsc_lib/src/configure/mod.rs | 2 +- dsc_lib/src/discovery/command_discovery.rs | 20 ++++++++++---------- osinfo/tests/osinfo.tests.ps1 | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 9198aad0..0dc6c2c8 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -228,7 +228,7 @@ pub fn export(dsc: &mut DscManager, resource_str: &str, format: &Option(dsc: &'a DscManager, resource: &str) -> Option<&'a DscResource> { //TODO: add dinamically generated resource to dsc - dsc.find_resource(resource) + dsc.find_resource(String::from(resource).to_lowercase().as_str()) } fn get_input(input: &Option, stdin: &Option) -> String { diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index 2c68381c..8c0f037c 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -381,26 +381,26 @@ pub fn resource(subcommand: &ResourceSubCommand, format: &Option, } }, ResourceSubCommand::Get { resource, input, all } => { - dsc.discover_resources(&[resource.to_string()]); + dsc.discover_resources(&[resource.to_lowercase().to_string()]); if *all { resource_command::get_all(&dsc, resource, input, stdin, format); } else { resource_command::get(&dsc, resource, input, stdin, format); }; }, ResourceSubCommand::Set { resource, input } => { - dsc.discover_resources(&[resource.to_string()]); + dsc.discover_resources(&[resource.to_lowercase().to_string()]); resource_command::set(&dsc, resource, input, stdin, format); }, ResourceSubCommand::Test { resource, input } => { - dsc.discover_resources(&[resource.to_string()]); + dsc.discover_resources(&[resource.to_lowercase().to_string()]); resource_command::test(&dsc, resource, input, stdin, format); }, ResourceSubCommand::Schema { resource } => { - dsc.discover_resources(&[resource.to_string()]); + dsc.discover_resources(&[resource.to_lowercase().to_string()]); resource_command::schema(&dsc, resource, format); }, ResourceSubCommand::Export { resource} => { - dsc.discover_resources(&[resource.to_string()]); + dsc.discover_resources(&[resource.to_lowercase().to_string()]); resource_command::export(&mut dsc, resource, format); }, } diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 6ce3503a..3c61bdda 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -255,7 +255,7 @@ impl Configurator { let mut has_errors = false; // Perform discovery of resources used in config - let required_resources = config.resources.iter().map(|p| p.resource_type.clone()).collect::>(); + let required_resources = config.resources.iter().map(|p| p.resource_type.to_lowercase()).collect::>(); self.discovery.discover_resources(&required_resources); // Now perform the validation diff --git a/dsc_lib/src/discovery/command_discovery.rs b/dsc_lib/src/discovery/command_discovery.rs index 2530784d..a7a939c0 100644 --- a/dsc_lib/src/discovery/command_discovery.rs +++ b/dsc_lib/src/discovery/command_discovery.rs @@ -72,19 +72,19 @@ impl CommandDiscovery { if resource.manifest.is_some() { let manifest = import_manifest(resource.manifest.clone().unwrap())?; if manifest.provider.is_some() { - provider_resources.push(resource.type_name.clone()); - resources.insert(resource.type_name.clone(), resource.clone()); + provider_resources.push(resource.type_name.to_lowercase()); + resources.insert(resource.type_name.to_lowercase(), resource.clone()); } } if return_all_resources { - resources.insert(resource.type_name.clone(), resource); + resources.insert(resource.type_name.to_lowercase(), resource); } - else if remaining_required_resource_types.contains(&resource.type_name) + else if remaining_required_resource_types.contains(&resource.type_name.to_lowercase()) { - remaining_required_resource_types.retain(|x| *x != resource.type_name); + remaining_required_resource_types.retain(|x| *x != resource.type_name.to_lowercase()); debug!("Found {} in {}", &resource.type_name, path.display()); - resources.insert(resource.type_name.clone(), resource); + resources.insert(resource.type_name.to_lowercase(), resource); if remaining_required_resource_types.is_empty() { return Ok(resources); @@ -147,14 +147,14 @@ impl CommandDiscovery { } if return_all_resources { - resources.insert(resource.type_name.clone(), resource); + resources.insert(resource.type_name.to_lowercase(), resource); provider_resources_count += 1; } - else if remaining_required_resource_types.contains(&resource.type_name) + else if remaining_required_resource_types.contains(&resource.type_name.to_lowercase()) { - remaining_required_resource_types.retain(|x| *x != resource.type_name); + remaining_required_resource_types.retain(|x| *x != resource.type_name.to_lowercase()); debug!("Found {} in {}", &resource.type_name, &resource.path); - resources.insert(resource.type_name.clone(), resource); + resources.insert(resource.type_name.to_lowercase(), resource); if remaining_required_resource_types.is_empty() { return Ok(resources); diff --git a/osinfo/tests/osinfo.tests.ps1 b/osinfo/tests/osinfo.tests.ps1 index b88ed02c..6b9e8301 100644 --- a/osinfo/tests/osinfo.tests.ps1 +++ b/osinfo/tests/osinfo.tests.ps1 @@ -31,7 +31,7 @@ Describe 'osinfo resource tests' { else { $invalid = 'Windows' } - $out = "{`"family`": `"$invalid`"}" | dsc resource test -r '*osinfo' | ConvertFrom-Json + $out = "{`"family`": `"$invalid`"}" | dsc resource test -r 'Microsoft/osinfo' | ConvertFrom-Json $actual = dsc resource get -r Microsoft/OSInfo | ConvertFrom-Json $out.actualState.family | Should -BeExactly $actual.actualState.family $out.actualState.version | Should -BeExactly $actual.actualState.version From 726925b5359262c2f64d06dc435c09914aeeb604 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 30 Oct 2023 23:26:38 -0700 Subject: [PATCH 18/20] Test fixes 2 --- dsc/tests/dsc_args.tests.ps1 | 2 +- dsc_lib/src/configure/mod.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dsc/tests/dsc_args.tests.ps1 b/dsc/tests/dsc_args.tests.ps1 index 586f7da3..531716e3 100644 --- a/dsc/tests/dsc_args.tests.ps1 +++ b/dsc/tests/dsc_args.tests.ps1 @@ -61,7 +61,7 @@ Describe 'config argument tests' { '@ } ) { param($text) - $output = $text | dsc resource get -r *registry + $output = $text | dsc resource get -r Microsoft.Windows/Registry $output = $output | ConvertFrom-Json $output.actualState.'$id' | Should -BeExactly 'https://developer.microsoft.com/json-schemas/windows/registry/20230303/Microsoft.Windows.Registry.schema.json' $output.actualState.keyPath | Should -BeExactly 'HKLM\Software\Microsoft\Windows NT\CurrentVersion' diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index 3c61bdda..a4267b78 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -92,7 +92,7 @@ impl Configurator { return Ok(result); } for resource in get_resource_invocation_order(&config)? { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; debug!("resource_type {}", &resource.resource_type); @@ -128,7 +128,7 @@ impl Configurator { return Ok(result); } for resource in get_resource_invocation_order(&config)? { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; debug!("resource_type {}", &resource.resource_type); @@ -164,7 +164,7 @@ impl Configurator { return Ok(result); } for resource in get_resource_invocation_order(&config)? { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else { return Err(DscError::ResourceNotFound(resource.resource_type)); }; debug!("resource_type {}", &resource.resource_type); @@ -238,7 +238,7 @@ impl Configurator { let mut conf = config_doc::Configuration::new(); for resource in &config.resources { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; add_resource_export_results_to_configuration(dsc_resource, &mut conf)?; @@ -260,7 +260,7 @@ impl Configurator { // Now perform the validation for resource in &config.resources { - let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else { + let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type.to_lowercase()) else { return Err(DscError::ResourceNotFound(resource.resource_type.clone())); }; From 121467f2e8fe964ca11cc549cafb298b3fb489ad Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 31 Oct 2023 00:08:49 -0700 Subject: [PATCH 19/20] Test fixes 3 --- dsc/tests/dsc_get.tests.ps1 | 3 +-- dsc/tests/dsc_schema.tests.ps1 | 2 +- dsc/tests/dsc_set.tests.ps1 | 4 ++-- dsc/tests/dsc_test.tests.ps1 | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dsc/tests/dsc_get.tests.ps1 b/dsc/tests/dsc_get.tests.ps1 index 4721aa75..bc2f8742 100644 --- a/dsc/tests/dsc_get.tests.ps1 +++ b/dsc/tests/dsc_get.tests.ps1 @@ -4,7 +4,6 @@ Describe 'config get tests' { It 'should get from registry using resource' -Skip:(!$IsWindows) -TestCases @( @{ type = 'string' } - @{ type = 'json' } ) { param($type) @@ -46,7 +45,7 @@ Describe 'config get tests' { "Name": "ProductName" } '@ - $json | dsc resource get -r *registry + $json | dsc resource get -r Microsoft.Windows/registry $LASTEXITCODE | Should -Be 2 } } diff --git a/dsc/tests/dsc_schema.tests.ps1 b/dsc/tests/dsc_schema.tests.ps1 index 320cccdc..7c07771d 100644 --- a/dsc/tests/dsc_schema.tests.ps1 +++ b/dsc/tests/dsc_schema.tests.ps1 @@ -3,7 +3,7 @@ Describe 'config schema tests' { It 'return resource schema' -Skip:(!$IsWindows) { - $schema = dsc resource schema -r *registry + $schema = dsc resource schema -r Microsoft.Windows/registry $LASTEXITCODE | Should -Be 0 $schema | Should -Not -BeNullOrEmpty $schema = $schema | ConvertFrom-Json diff --git a/dsc/tests/dsc_set.tests.ps1 b/dsc/tests/dsc_set.tests.ps1 index 11363909..8be4d4d3 100644 --- a/dsc/tests/dsc_set.tests.ps1 +++ b/dsc/tests/dsc_set.tests.ps1 @@ -36,7 +36,7 @@ Describe 'config set tests' { } } '@ - $out = $json | dsc resource set -r *registry + $out = $json | dsc resource set -r Microsoft.Windows/registry $LASTEXITCODE | Should -Be 0 $result = $out | ConvertFrom-Json $result.afterState.keyPath | Should -Be 'HKCU\1\2\3' @@ -45,7 +45,7 @@ Describe 'config set tests' { $result.changedProperties | Should -Be @('valueName', 'valueData', '_exist') ($result.psobject.properties | Measure-Object).Count | Should -Be 3 - $out = $json | dsc resource get -r *registry + $out = $json | dsc resource get -r Microsoft.Windows/registry $LASTEXITCODE | Should -Be 0 $result = $out | ConvertFrom-Json $result.actualState.keyPath | Should -Be 'HKCU\1\2\3' diff --git a/dsc/tests/dsc_test.tests.ps1 b/dsc/tests/dsc_test.tests.ps1 index bbd45d47..8e602b92 100644 --- a/dsc/tests/dsc_test.tests.ps1 +++ b/dsc/tests/dsc_test.tests.ps1 @@ -10,7 +10,7 @@ Describe 'config test tests' { } '@ $current = $json | registry config get - $out = $current | dsc resource test -r *registry + $out = $current | dsc resource test -r Microsoft.Windows/registry $LASTEXITCODE | Should -Be 0 $out = $out | ConvertFrom-Json $out.inDesiredState | Should -BeTrue @@ -45,7 +45,7 @@ Describe 'config test tests' { } } '@ - $out = $json | dsc resource test -r *registry + $out = $json | dsc resource test -r Microsoft.Windows/registry $LASTEXITCODE | Should -Be 0 $out = $out | ConvertFrom-Json $out.inDesiredState | Should -BeFalse From a5dbff07b106d8a6258d4036b8a161cf0caf29f3 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 31 Oct 2023 13:37:59 -0700 Subject: [PATCH 20/20] Feedback 3 --- dsc/src/resource_command.rs | 36 ++++++++++++++++++------------------ dsc_lib/src/configure/mod.rs | 4 +++- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index 0dc6c2c8..bb18008c 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -15,12 +15,12 @@ use dsc_lib::{ }; use std::process::exit; -pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn get(dsc: &DscManager, resource_type: &str, input: &Option, stdin: &Option, format: &Option) { // TODO: support streaming stdin which includes resource and input let mut input = get_input(input, stdin); - let Some(mut resource) = get_resource(dsc, resource_str) else { - error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + let Some(mut resource) = get_resource(dsc, resource_type) else { + error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return }; @@ -54,9 +54,9 @@ pub fn get(dsc: &DscManager, resource_str: &str, input: &Option, stdin: } } -pub fn get_all(dsc: &DscManager, resource_str: &str, _input: &Option, _stdin: &Option, format: &Option) { - let Some(resource) = get_resource(dsc, resource_str) else { - error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); +pub fn get_all(dsc: &DscManager, resource_type: &str, _input: &Option, _stdin: &Option, format: &Option) { + let Some(resource) = get_resource(dsc, resource_type) else { + error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return }; debug!("resource.type_name - {} implemented_as - {:?}", resource.type_name, resource.implemented_as); @@ -91,15 +91,15 @@ pub fn get_all(dsc: &DscManager, resource_str: &str, _input: &Option, _s /// /// Will panic if provider-based resource is not found. /// -pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn set(dsc: &DscManager, resource_type: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); if input.is_empty() { error!("Error: Input is empty"); exit(EXIT_INVALID_ARGS); } - let Some(mut resource) = get_resource(dsc, resource_str) else { - error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + let Some(mut resource) = get_resource(dsc, resource_type) else { + error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return }; @@ -140,10 +140,10 @@ pub fn set(dsc: &DscManager, resource_str: &str, input: &Option, stdin: /// /// Will panic if provider-based resource is not found. /// -pub fn test(dsc: &DscManager, resource_str: &str, input: &Option, stdin: &Option, format: &Option) { +pub fn test(dsc: &DscManager, resource_type: &str, input: &Option, stdin: &Option, format: &Option) { let mut input = get_input(input, stdin); - let Some(mut resource) = get_resource(dsc, resource_str) else { - error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); + let Some(mut resource) = get_resource(dsc, resource_type) else { + error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return }; @@ -178,9 +178,9 @@ pub fn test(dsc: &DscManager, resource_str: &str, input: &Option, stdin: } } -pub fn schema(dsc: &DscManager, resource_str: &str, format: &Option) { - let Some(resource) = get_resource(dsc, resource_str) else { - error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); +pub fn schema(dsc: &DscManager, resource_type: &str, format: &Option) { + let Some(resource) = get_resource(dsc, resource_type) else { + error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return }; match resource.schema() { @@ -202,9 +202,9 @@ pub fn schema(dsc: &DscManager, resource_str: &str, format: &Option) { - let Some(dsc_resource) = get_resource(dsc, resource_str) else { - error!("{}", DscError::ResourceNotFound(resource_str.to_string()).to_string()); +pub fn export(dsc: &mut DscManager, resource_type: &str, format: &Option) { + let Some(dsc_resource) = get_resource(dsc, resource_type) else { + error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return }; diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs index a4267b78..27f0b412 100644 --- a/dsc_lib/src/configure/mod.rs +++ b/dsc_lib/src/configure/mod.rs @@ -255,7 +255,9 @@ impl Configurator { let mut has_errors = false; // Perform discovery of resources used in config - let required_resources = config.resources.iter().map(|p| p.resource_type.to_lowercase()).collect::>(); + let mut required_resources = config.resources.iter().map(|p| p.resource_type.to_lowercase()).collect::>(); + required_resources.sort_unstable(); + required_resources.dedup(); self.discovery.discover_resources(&required_resources); // Now perform the validation