Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor input options #305

Merged
merged 27 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
048088a
refactor input and input file commands as subcommands
tgauth Feb 1, 2024
a0aa9e8
refactor config and resource methods with input subcommands
tgauth Feb 1, 2024
2387e55
fix typo
tgauth Feb 1, 2024
bb88996
fix formatting
tgauth Feb 1, 2024
69ab07b
fix tests
tgauth Feb 2, 2024
efff8dc
fix tests
tgauth Feb 2, 2024
2e5e1bd
add tests for multiple input sources
tgauth Feb 2, 2024
cff8525
fix typo
tgauth Feb 2, 2024
73a9086
add debug statements for CI test, not reproing locally
tgauth Feb 2, 2024
ac410ac
more debug statements
tgauth Feb 2, 2024
f506368
remove debug statements from pester tests
tgauth Feb 2, 2024
314b59c
Merge branch 'main' into refactor-input-options
tgauth Feb 6, 2024
bf81de0
update document help description
tgauth Feb 6, 2024
a8f46f0
call get_inputs earlier from get/set/test
tgauth Feb 6, 2024
9605972
add comment around stdin empty input
tgauth Feb 6, 2024
cfdc229
add tests for empty input for set
tgauth Feb 6, 2024
fb787b3
Merge branch 'refactor-input-options' of https://github.com/tgauth/ds…
tgauth Feb 6, 2024
a0ba478
Merge branch 'main' into refactor-input-options
SteveL-MSFT Feb 6, 2024
5a87b6c
Merge branch 'main' into refactor-input-options
tgauth Feb 6, 2024
62be592
fix merge conflicts
tgauth Feb 6, 2024
038e268
add logging around inputs
tgauth Feb 6, 2024
17a941e
refactor empty input tests and debug mac ci failure
tgauth Feb 7, 2024
9b21daa
remove debug statements from pester tests
tgauth Feb 7, 2024
3922c47
much confusion
tgauth Feb 7, 2024
89ad1c9
try removing log file after each test
tgauth Feb 7, 2024
bc7f14e
try using explicit whitespace in test
tgauth Feb 7, 2024
f188b40
change from info to debug logging level
tgauth Feb 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions dsc/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
#[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource")]
pub input_file: Option<String>,
#[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")]
Expand Down Expand Up @@ -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<String>,
#[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")]
path: Option<String>,
#[clap(short = 'f', long, help = "The output format to use")]
format: Option<OutputFormat>,
},
#[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<String>,
#[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")]
path: Option<String>,
#[clap(short = 'f', long, help = "The output format to use")]
format: Option<OutputFormat>,
},
#[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<String>,
#[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")]
path: Option<String>,
#[clap(short = 'f', long, help = "The output format to use")]
format: Option<OutputFormat>,
},
#[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<String>,
#[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")]
path: Option<String>,
},
#[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<String>,
#[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource", conflicts_with = "document")]
path: Option<String>,
#[clap(short = 'f', long, help = "The output format to use")]
format: Option<OutputFormat>,
}
Expand All @@ -118,26 +135,32 @@ 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<String>,
#[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<String>,
#[clap(short = 'f', long, help = "The output format to use")]
format: Option<OutputFormat>,
},
#[clap(name = "set", about = "Invoke the set operation to a resource", arg_required_else_help = true)]
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<String>,
#[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<String>,
#[clap(short = 'f', long, help = "The output format to use")]
format: Option<OutputFormat>,
},
#[clap(name = "test", about = "Invoke the test operation to a resource", arg_required_else_help = true)]
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<String>,
#[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<String>,
#[clap(short = 'f', long, help = "The output format to use")]
format: Option<OutputFormat>,
},
Expand Down
24 changes: 10 additions & 14 deletions dsc/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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 {
Expand Down
59 changes: 5 additions & 54 deletions dsc/src/resource_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ use dsc_lib::{
};
use std::process::exit;

pub fn get(dsc: &DscManager, resource_type: &str, input: &Option<String>, stdin: &Option<String>, format: &Option<OutputFormat>) {
// 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<OutputFormat>) {
let Some(mut resource) = get_resource(dsc, resource_type) else {
error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string());
return
Expand Down Expand Up @@ -54,8 +51,8 @@ pub fn get(dsc: &DscManager, resource_type: &str, input: &Option<String>, stdin:
}
}

pub fn get_all(dsc: &DscManager, resource_type: &str, _input: &Option<String>, _stdin: &Option<String>, format: &Option<OutputFormat>) {
let mut input = String::new() ;
pub fn get_all(dsc: &DscManager, resource_type: &str, format: &Option<OutputFormat>) {
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
Expand Down Expand Up @@ -103,8 +100,7 @@ pub fn get_all(dsc: &DscManager, resource_type: &str, _input: &Option<String>, _
///
/// Will panic if provider-based resource is not found.
///
pub fn set(dsc: &DscManager, resource_type: &str, input: &Option<String>, stdin: &Option<String>, format: &Option<OutputFormat>) {
let mut input = get_input(input, stdin);
pub fn set(dsc: &DscManager, resource_type: &str, mut input: String, format: &Option<OutputFormat>) {
if input.is_empty() {
error!("Error: Input is empty");
exit(EXIT_INVALID_ARGS);
Expand Down Expand Up @@ -152,8 +148,7 @@ pub fn set(dsc: &DscManager, resource_type: &str, input: &Option<String>, stdin:
///
/// Will panic if provider-based resource is not found.
///
pub fn test(dsc: &DscManager, resource_type: &str, input: &Option<String>, stdin: &Option<String>, format: &Option<OutputFormat>) {
let mut input = get_input(input, stdin);
pub fn test(dsc: &DscManager, resource_type: &str, mut input: String, format: &Option<OutputFormat>) {
let Some(mut resource) = get_resource(dsc, resource_type) else {
error!("{}", DscError::ResourceNotFound(resource_type.to_string()).to_string());
return
Expand Down Expand Up @@ -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<String>, stdin: &Option<String>) -> 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::<serde_json::Value>(&input) {
Ok(_) => input,
Err(json_err) => {
match serde_yaml::from_str::<serde_yaml::Value>(&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);
}
}
}
}
}
76 changes: 30 additions & 46 deletions dsc/src/subcommand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -117,33 +117,16 @@ pub fn config_export(configurator: &mut Configurator, format: &Option<OutputForm
}

pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, stdin: &Option<String>) {
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::<Value>(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) => {
Expand Down Expand Up @@ -184,19 +167,19 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, 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);
}
}
Expand Down Expand Up @@ -407,32 +390,33 @@ pub fn resource(subcommand: &ResourceSubCommand, stdin: &Option<String>) {
}
}

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);
},
}
}
Loading