diff --git a/dsc/src/args.rs b/dsc/src/args.rs index 58bff0b4..ca74549b 100644 --- a/dsc/src/args.rs +++ b/dsc/src/args.rs @@ -33,10 +33,6 @@ pub struct Args { /// The subcommand to run #[clap(subcommand)] pub subcommand: SubCommand, - #[clap(short = 'i', long, help = "The input to pass to the configuration or resource", conflicts_with = "input_file")] - pub input: Option, - #[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource")] - pub input_file: Option, #[clap(short = 'l', long, help = "Trace level to use", value_enum, default_value = "warning")] pub trace_level: TraceLevel, #[clap(short = 'f', long, help = "Trace format to use", value_enum, default_value = "default")] @@ -77,23 +73,44 @@ pub enum SubCommand { pub enum ConfigSubCommand { #[clap(name = "get", about = "Retrieve the current configuration")] Get { + #[clap(short = 'd', long, help = "The document to pass to the configuration or resource", conflicts_with = "path")] + document: Option, + #[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")] + path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, }, #[clap(name = "set", about = "Set the current configuration")] Set { + #[clap(short = 'd', long, help = "The document to pass to the configuration or resource", conflicts_with = "path")] + document: Option, + #[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")] + path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, }, #[clap(name = "test", about = "Test the current configuration")] Test { + #[clap(short = 'd', long, help = "The document to pass to the configuration or resource", conflicts_with = "path")] + document: Option, + #[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")] + path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, }, #[clap(name = "validate", about = "Validate the current configuration", hide = true)] - Validate, + Validate { + #[clap(short = 'd', long, help = "The document to pass to the configuration or resource", conflicts_with = "path")] + document: Option, + #[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")] + path: Option, + }, #[clap(name = "export", about = "Export the current configuration")] Export { + #[clap(short = 'd', long, help = "The document to pass to the configuration or resource", conflicts_with = "path")] + document: Option, + #[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")] + path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, } @@ -118,8 +135,10 @@ pub enum ResourceSubCommand { all: bool, #[clap(short, long, help = "The name or DscResource JSON of the resource to invoke `get` on")] resource: String, - #[clap(short, long, help = "The input to pass to the resource as JSON")] + #[clap(short, long, help = "The input to pass to the resource as JSON or YAML", conflicts_with = "path")] input: Option, + #[clap(short = 'p', long, help = "The path to a JSON or YAML file used as input to the configuration or resource", conflicts_with = "input")] + path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, }, @@ -127,8 +146,10 @@ pub enum ResourceSubCommand { Set { #[clap(short, long, help = "The name or DscResource JSON of the resource to invoke `set` on")] resource: String, - #[clap(short, long, help = "The input to pass to the resource as JSON")] + #[clap(short, long, help = "The input to pass to the resource as JSON or YAML", conflicts_with = "path")] input: Option, + #[clap(short = 'p', long, help = "The path to a JSON or YAML file used as input to the configuration or resource", conflicts_with = "input")] + path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, }, @@ -136,8 +157,10 @@ pub enum ResourceSubCommand { Test { #[clap(short, long, help = "The name or DscResource JSON of the resource to invoke `test` on")] resource: String, - #[clap(short, long, help = "The input to pass to the resource as JSON")] + #[clap(short, long, help = "The input to pass to the resource as JSON or YAML", conflicts_with = "path")] input: Option, + #[clap(short = 'p', long, help = "The path to a JSON or YAML file used as input to the configuration or resource", conflicts_with = "input")] + path: Option, #[clap(short = 'f', long, help = "The output format to use")] format: Option, }, diff --git a/dsc/src/main.rs b/dsc/src/main.rs index db1e5335..4e3a5e01 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -35,19 +35,7 @@ fn main() { debug!("Running dsc {}", env!("CARGO_PKG_VERSION")); - let input = if args.input.is_some() { - args.input - } else if args.input_file.is_some() { - info!("Reading input from file {}", args.input_file.as_ref().unwrap()); - let input_file = args.input_file.unwrap(); - match std::fs::read_to_string(input_file) { - Ok(input) => Some(input), - Err(err) => { - error!("Error: Failed to read input file: {err}"); - exit(util::EXIT_INVALID_INPUT); - } - } - } else if atty::is(Stream::Stdin) { + let input = if atty::is(Stream::Stdin) { None } else { info!("Reading input from STDIN"); @@ -60,7 +48,15 @@ fn main() { exit(util::EXIT_INVALID_ARGS); }, }; - Some(input) + // get_input call expects at most 1 input, so wrapping Some(empty input) would throw it off + // have only seen this happen with dsc_args.test.ps1 running on the CI pipeline + if input.is_empty() { + debug!("Input from STDIN is empty"); + None + } + else { + Some(input) + } }; match args.subcommand { diff --git a/dsc/src/resource_command.rs b/dsc/src/resource_command.rs index d599e3c8..2b75a09e 100644 --- a/dsc/src/resource_command.rs +++ b/dsc/src/resource_command.rs @@ -15,10 +15,7 @@ use dsc_lib::{ }; use std::process::exit; -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); - +pub fn get(dsc: &DscManager, resource_type: &str, mut input: String, format: &Option) { let Some(mut resource) = get_resource(dsc, resource_type) else { error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return @@ -54,8 +51,8 @@ pub fn get(dsc: &DscManager, resource_type: &str, input: &Option, stdin: } } -pub fn get_all(dsc: &DscManager, resource_type: &str, _input: &Option, _stdin: &Option, format: &Option) { - let mut input = String::new() ; +pub fn get_all(dsc: &DscManager, resource_type: &str, format: &Option) { + let mut input = String::new(); let Some(mut resource) = get_resource(dsc, resource_type) else { error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return @@ -103,8 +100,7 @@ pub fn get_all(dsc: &DscManager, resource_type: &str, _input: &Option, _ /// /// Will panic if provider-based resource is not found. /// -pub fn set(dsc: &DscManager, resource_type: &str, input: &Option, stdin: &Option, format: &Option) { - let mut input = get_input(input, stdin); +pub fn set(dsc: &DscManager, resource_type: &str, mut input: String, format: &Option) { if input.is_empty() { error!("Error: Input is empty"); exit(EXIT_INVALID_ARGS); @@ -152,8 +148,7 @@ pub fn set(dsc: &DscManager, resource_type: &str, input: &Option, stdin: /// /// Will panic if provider-based resource is not found. /// -pub fn test(dsc: &DscManager, resource_type: &str, input: &Option, stdin: &Option, format: &Option) { - let mut input = get_input(input, stdin); +pub fn test(dsc: &DscManager, resource_type: &str, mut input: String, format: &Option) { let Some(mut resource) = get_resource(dsc, resource_type) else { error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string()); return @@ -254,47 +249,3 @@ pub fn get_resource<'a>(dsc: &'a DscManager, resource: &str) -> Option<&'a DscRe //TODO: add dinamically generated resource to dsc dsc.find_resource(String::from(resource).to_lowercase().as_str()) } - -fn get_input(input: &Option, stdin: &Option) -> String { - let input = match (input, stdin) { - (Some(_input), Some(_stdin)) => { - error!("Error: Cannot specify both --input and stdin"); - exit(EXIT_INVALID_ARGS); - } - (Some(input), None) => input.clone(), - (None, Some(stdin)) => stdin.clone(), - (None, None) => { - return String::new(); - }, - }; - - if input.is_empty() { - return String::new(); - } - - match serde_json::from_str::(&input) { - Ok(_) => input, - Err(json_err) => { - match serde_yaml::from_str::(&input) { - Ok(yaml) => { - match serde_json::to_string(&yaml) { - Ok(json) => json, - Err(err) => { - error!("Error: Cannot convert YAML to JSON: {err}"); - exit(EXIT_INVALID_ARGS); - } - } - }, - Err(err) => { - if input.contains('{') { - error!("Error: Input is not valid JSON: {json_err}"); - } - else { - error!("Error: Input is not valid YAML: {err}"); - } - exit(EXIT_INVALID_ARGS); - } - } - } - } -} diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index 376a50ad..c4fe82b1 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -4,7 +4,7 @@ use crate::args::{ConfigSubCommand, DscType, OutputFormat, ResourceSubCommand}; use crate::resource_command::{get_resource, self}; use crate::tablewriter::Table; -use crate::util::{EXIT_DSC_ERROR, EXIT_INVALID_ARGS, EXIT_INVALID_INPUT, EXIT_JSON_ERROR, EXIT_SUCCESS, EXIT_VALIDATION_FAILED, get_schema, serde_json_value_to_string, write_output}; +use crate::util::{EXIT_DSC_ERROR, EXIT_INVALID_INPUT, EXIT_JSON_ERROR, EXIT_SUCCESS, EXIT_VALIDATION_FAILED, get_schema, write_output, get_input}; use tracing::error; use atty::Stream; @@ -117,33 +117,16 @@ pub fn config_export(configurator: &mut Configurator, format: &Option, stdin: &Option) { - let Some(stdin) = stdin else { - error!("Configuration must be piped to STDIN"); - exit(EXIT_INVALID_ARGS); - }; - - let json: serde_json::Value = match serde_json::from_str(stdin.as_ref()) { - Ok(json) => json, - Err(_) => { - match serde_yaml::from_str::(stdin.as_ref()) { - Ok(yaml) => { - match serde_json::to_value(yaml) { - Ok(json) => json, - Err(err) => { - error!("Error: Failed to convert YAML to JSON: {err}"); - exit(EXIT_DSC_ERROR); - } - } - }, - Err(err) => { - error!("Error: Input is not valid JSON or YAML: {err}"); - exit(EXIT_INVALID_INPUT); - } - } + let json_string = match subcommand { + ConfigSubCommand::Get { document, path, .. } | + ConfigSubCommand::Set { document, path, .. } | + ConfigSubCommand::Test { document, path, .. } | + ConfigSubCommand::Validate { document, path, .. } | + ConfigSubCommand::Export { document, path, .. } => { + get_input(document, stdin, path) } }; - let json_string = serde_json_value_to_string(&json); let mut configurator = match Configurator::new(&json_string) { Ok(configurator) => configurator, Err(err) => { @@ -184,19 +167,19 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option, stdin: } match subcommand { - ConfigSubCommand::Get { format } => { + ConfigSubCommand::Get { format, .. } => { config_get(&mut configurator, format); }, - ConfigSubCommand::Set { format } => { + ConfigSubCommand::Set { format, .. } => { config_set(&mut configurator, format); }, - ConfigSubCommand::Test { format } => { + ConfigSubCommand::Test { format, .. } => { config_test(&mut configurator, format); }, - ConfigSubCommand::Validate => { + ConfigSubCommand::Validate { .. } => { validate_config(&json_string); }, - ConfigSubCommand::Export { format } => { + ConfigSubCommand::Export { format, .. } => { config_export(&mut configurator, format); } } @@ -407,32 +390,33 @@ pub fn resource(subcommand: &ResourceSubCommand, stdin: &Option) { } } - if write_table { - table.print(); - } + if write_table { table.print(); } }, - ResourceSubCommand::Get { resource, input, all, format } => { + ResourceSubCommand::Schema { resource , format } => { 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); - }; + resource_command::schema(&dsc, resource, format); }, - ResourceSubCommand::Set { resource, input, format } => { + ResourceSubCommand::Export { resource, format } => { dsc.discover_resources(&[resource.to_lowercase().to_string()]); - resource_command::set(&dsc, resource, input, stdin, format); + resource_command::export(&mut dsc, resource, format); }, - ResourceSubCommand::Test { resource, input, format } => { + ResourceSubCommand::Get { resource, input, path, all, format } => { dsc.discover_resources(&[resource.to_lowercase().to_string()]); - resource_command::test(&dsc, resource, input, stdin, format); + if *all { resource_command::get_all(&dsc, resource, format); } + else { + let parsed_input = get_input(input, stdin, path); + resource_command::get(&dsc, resource, parsed_input, format); + } }, - ResourceSubCommand::Schema { resource , format } => { + ResourceSubCommand::Set { resource, input, path, format } => { dsc.discover_resources(&[resource.to_lowercase().to_string()]); - resource_command::schema(&dsc, resource, format); + let parsed_input = get_input(input, stdin, path); + resource_command::set(&dsc, resource, parsed_input, format); }, - ResourceSubCommand::Export { resource, format } => { + ResourceSubCommand::Test { resource, input, path, format } => { dsc.discover_resources(&[resource.to_lowercase().to_string()]); - resource_command::export(&mut dsc, resource, format); + let parsed_input = get_input(input, stdin, path); + resource_command::test(&dsc, resource, parsed_input, format); }, } } diff --git a/dsc/src/util.rs b/dsc/src/util.rs index be9106df..fbaa15ea 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -17,6 +17,7 @@ use dsc_lib::{ } }; use schemars::{schema_for, schema::RootSchema}; +use serde_yaml::Value; use std::collections::HashMap; use std::process::exit; use syntect::{ @@ -25,7 +26,7 @@ use syntect::{ parsing::SyntaxSet, util::{as_24_bit_terminal_escaped, LinesWithEndings} }; -use tracing::{Level, error}; +use tracing::{Level, debug, error}; use tracing_subscriber::{filter::EnvFilter, layer::SubscriberExt, Layer}; pub const EXIT_SUCCESS: i32 = 0; @@ -288,3 +289,71 @@ pub fn enable_tracing(trace_level: &TraceLevel, trace_format: &TraceFormat) { eprintln!("Unable to set global default tracing subscriber. Tracing is diabled."); } } + +pub fn parse_input_to_json(value: &str) -> String { + match serde_json::from_str(value) { + Ok(json) => json, + Err(_) => { + match serde_yaml::from_str::(value) { + Ok(yaml) => { + match serde_json::to_value(yaml) { + Ok(json) => json.to_string(), + Err(err) => { + error!("Error: Failed to convert YAML to JSON: {err}"); + exit(EXIT_DSC_ERROR); + } + } + }, + Err(err) => { + error!("Error: Input is not valid JSON or YAML: {err}"); + exit(EXIT_INVALID_INPUT); + } + } + } + } +} + +pub fn get_input(input: &Option, stdin: &Option, path: &Option) -> String { + let value = match (input, stdin, path) { + (Some(_), Some(_), None) | (None, Some(_), Some(_)) => { + error!("Error: Cannot specify both stdin and --input or --path"); + exit(EXIT_INVALID_ARGS); + }, + (Some(input), None, None) => { + debug!("Reading input from command line parameter"); + input.clone() + }, + (None, Some(stdin), None) => { + debug!("Reading input from stdin"); + stdin.clone() + }, + (None, None, Some(path)) => { + debug!("Reading input from file {}", path); + match std::fs::read_to_string(path) { + Ok(input) => { + input.clone() + }, + Err(err) => { + error!("Error: Failed to read input file: {err}"); + exit(EXIT_INVALID_INPUT); + } + } + }, + (None, None, None) => { + debug!("No input provided via stdin, file, or command line"); + return String::new(); + }, + _default => { + /* clap should handle these cases via conflicts_with so this should not get reached */ + error!("Error: Invalid input"); + exit(EXIT_INVALID_ARGS); + } + }; + + if value.trim().is_empty() { + debug!("Provided input is empty"); + return String::new(); + } + + parse_input_to_json(&value) +} diff --git a/dsc/tests/dsc_args.tests.ps1 b/dsc/tests/dsc_args.tests.ps1 index 94db1a92..25bfd24e 100644 --- a/dsc/tests/dsc_args.tests.ps1 +++ b/dsc/tests/dsc_args.tests.ps1 @@ -44,6 +44,12 @@ Describe 'config argument tests' { $env:DSC_RESOURCE_PATH = $env:PATH + $sep + $TestDrive } + AfterEach { + if (Test-Path $TestDrive/error.txt) { + Remove-Item -Path $TestDrive/error.txt + } + } + AfterAll { $env:DSC_RESOURCE_PATH = $oldPath } @@ -100,8 +106,8 @@ actualState: } It 'input can be passed using ' -TestCases @( - @{ parameter = '-i' } - @{ parameter = '--input' } + @{ parameter = '-d' } + @{ parameter = '--document' } ) { param($parameter) @@ -114,14 +120,14 @@ resources: family: Windows '@ - $out = dsc $parameter "$yaml" config get | ConvertFrom-Json + $out = dsc config get $parameter "$yaml" | ConvertFrom-Json $LASTEXITCODE | Should -Be 0 $out.results[0].type | Should -BeExactly 'Microsoft/OSInfo' } It 'input can be passed using ' -TestCases @( @{ parameter = '-p' } - @{ parameter = '--input-file' } + @{ parameter = '--path' } ) { param($parameter) @@ -135,21 +141,71 @@ resources: '@ Set-Content -Path $TestDrive/foo.yaml -Value $yaml - $out = dsc $parameter "$TestDrive/foo.yaml" config get | ConvertFrom-Json + $out = dsc config get $parameter "$TestDrive/foo.yaml" | ConvertFrom-Json $LASTEXITCODE | Should -Be 0 $out.results[0].type | Should -BeExactly 'Microsoft/OSInfo' } - It '--input and --input-file cannot be used together' { - dsc --input 1 --input-file foo.json config get 2> $TestDrive/error.txt + It '--document and --path cannot be used together' { + dsc config get --document 1 --path foo.json 2> $TestDrive/error.txt + $err = Get-Content $testdrive/error.txt -Raw + $err.Length | Should -Not -Be 0 + $LASTEXITCODE | Should -Be 2 + } + + It 'stdin and --document cannot be used together' { + '{ "foo": true }' | dsc config get --document 1 2> $TestDrive/error.txt + $err = Get-Content $testdrive/error.txt -Raw + $err.Length | Should -Not -Be 0 + $LASTEXITCODE | Should -Be 1 + } + + It 'stdin and --path cannot be used together' { + '{ "foo": true }' | dsc config get --path foo.json 2> $TestDrive/error.txt + $err = Get-Content $testdrive/error.txt -Raw + $err.Length | Should -Not -Be 0 + $LASTEXITCODE | Should -Be 1 + } + + It 'stdin, --document and --path cannot be used together' { + '{ "foo": true }' | dsc config get --document 1 --path foo.json 2> $TestDrive/error.txt $err = Get-Content $testdrive/error.txt -Raw $err.Length | Should -Not -Be 0 $LASTEXITCODE | Should -Be 2 } - It '--logging-level has effect' { + It '--trace-level has effect' { dsc -l debug resource get -r Microsoft/OSInfo 2> $TestDrive/tracing.txt "$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'DEBUG' $LASTEXITCODE | Should -Be 0 } + + It 'stdin cannot be empty if neither input or path is provided' { + '' | dsc resource set -r Microsoft/OSInfo 2> $TestDrive/error.txt + $err = Get-Content $testdrive/error.txt -Raw + $err.Length | Should -Not -Be 0 + $LASTEXITCODE | Should -Be 1 + } + + It 'input cannot be empty if neither stdin or path is provided' { + dsc resource set -r Microsoft/OSInfo --input " " 2> $TestDrive/error.txt + $err = Get-Content $testdrive/error.txt -Raw + $err.Length | Should -Not -Be 0 + $LASTEXITCODE | Should -Be 1 + } + + It 'path contents cannot be empty if neither stdin or input is provided' { + Set-Content -Path $TestDrive/empty.yaml -Value " " + dsc resource set -r Microsoft/OSInfo --path $TestDrive/empty.yaml 2> $TestDrive/error.txt + $err = Get-Content $testdrive/error.txt -Raw + $err.Length | Should -Not -Be 0 + $LASTEXITCODE | Should -Be 1 + } + + It 'document cannot be empty if neither stdin or path is provided' { + dsc config set --document " " 2> $TestDrive/error.txt + $err = Get-Content $testdrive/error.txt -Raw + $err.Length | Should -Not -Be 0 + $LASTEXITCODE | Should -Be 4 + } } diff --git a/dsc/tests/dsc_parameters.tests.ps1 b/dsc/tests/dsc_parameters.tests.ps1 index 7a77866d..29829aac 100644 --- a/dsc/tests/dsc_parameters.tests.ps1 +++ b/dsc/tests/dsc_parameters.tests.ps1 @@ -248,7 +248,7 @@ Describe 'Parameters tests' { family: '[parameters(''osFamily'')]' '@ - $out = dsc -i $config_yaml config -p $params test | ConvertFrom-Json + $out = dsc config -p $params test -d $config_yaml | ConvertFrom-Json $LASTEXITCODE | Should -Be 0 $out.results[0].result.actualState.family | Should -BeExactly $os $out.results[0].result.inDesiredState | Should -BeTrue