Skip to content

Commit 34be0ae

Browse files
SteveL-MSFTSteveL-MSFT
authored
Merge pull request #1215 from Gijsreyn/gh-1213/main/add-inline-parameter-support
Add parameter merging functionality with inline precedence
2 parents aa53e4f + 61ff574 commit 34be0ae

File tree

6 files changed

+448
-38
lines changed

6 files changed

+448
-38
lines changed

‎dsc/locales/en-us.toml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ failedReadingParametersFile = "Failed to read parameters file"
4444
readingParametersFromStdin = "Reading parameters from STDIN"
4545
generatingCompleter = "Generating completion script for"
4646
readingParametersFile = "Reading parameters from file"
47+
mergingParameters = "Merging inline parameters with parameters file (inline takes precedence)"
48+
failedMergingParameters = "Failed to merge parameters"
4749
usingDscVersion = "Running DSC version"
4850
foundProcesses = "Found processes"
4951
failedToGetPid = "Could not get current process id"
@@ -165,5 +167,5 @@ failedToAbsolutizePath = "Error making config path absolute"
165167
failedToGetParentPath = "Error reading config path parent"
166168
dscConfigRootAlreadySet = "The current value of DSC_CONFIG_ROOT env var will be overridden"
167169
settingDscConfigRoot = "Setting DSC_CONFIG_ROOT env var as"
168-
stdinNotAllowedForBothParametersAndInput = "Cannot read from STDIN for both parameters and input."
169170
removingUtf8Bom = "Removing UTF-8 BOM from input"
171+
parametersNotObject = "Parameters must be an object"

‎dsc/src/args.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,9 @@ pub enum SubCommand {
6666
Config {
6767
#[clap(subcommand)]
6868
subcommand: ConfigSubCommand,
69-
#[clap(short, long, help = t!("args.parameters").to_string(), conflicts_with = "parameters_file")]
69+
#[clap(short, long, help = t!("args.parameters").to_string())]
7070
parameters: Option<String>,
71-
#[clap(short = 'f', long, help = t!("args.parametersFile").to_string(), conflicts_with = "parameters")]
71+
#[clap(short = 'f', long, help = t!("args.parametersFile").to_string())]
7272
parameters_file: Option<String>,
7373
#[clap(short = 'r', long, help = t!("args.systemRoot").to_string())]
7474
system_root: Option<String>,

‎dsc/src/main.rs‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,15 @@ fn main() {
5454
generate(shell, &mut cmd, "dsc", &mut io::stdout());
5555
},
5656
SubCommand::Config { subcommand, parameters, parameters_file, system_root, as_group, as_assert, as_include } => {
57-
if let Some(file_name) = parameters_file {
57+
// Read parameters from file if provided
58+
let file_params = if let Some(file_name) = &parameters_file {
5859
if file_name == "-" {
5960
info!("{}", t!("main.readingParametersFromStdin"));
6061
let mut stdin = Vec::<u8>::new();
61-
let parameters = match io::stdin().read_to_end(&mut stdin) {
62+
match io::stdin().read_to_end(&mut stdin) {
6263
Ok(_) => {
6364
match String::from_utf8(stdin) {
64-
Ok(input) => {
65-
input
66-
},
65+
Ok(input) => Some(input),
6766
Err(err) => {
6867
error!("{}: {err}", t!("util.invalidUtf8"));
6968
exit(EXIT_INVALID_INPUT);
@@ -74,22 +73,38 @@ fn main() {
7473
error!("{}: {err}", t!("util.failedToReadStdin"));
7574
exit(EXIT_INVALID_INPUT);
7675
}
77-
};
78-
subcommand::config(&subcommand, &Some(parameters), true, system_root.as_ref(), &as_group, &as_assert, &as_include, progress_format);
79-
return;
80-
}
81-
info!("{}: {file_name}", t!("main.readingParametersFile"));
82-
match std::fs::read_to_string(&file_name) {
83-
Ok(parameters) => subcommand::config(&subcommand, &Some(parameters), false, system_root.as_ref(), &as_group, &as_assert, &as_include, progress_format),
84-
Err(err) => {
85-
error!("{} '{file_name}': {err}", t!("main.failedReadingParametersFile"));
86-
exit(util::EXIT_INVALID_INPUT);
76+
}
77+
} else {
78+
info!("{}: {file_name}", t!("main.readingParametersFile"));
79+
match std::fs::read_to_string(file_name) {
80+
Ok(content) => Some(content),
81+
Err(err) => {
82+
error!("{} '{file_name}': {err}", t!("main.failedReadingParametersFile"));
83+
exit(util::EXIT_INVALID_INPUT);
84+
}
8785
}
8886
}
89-
}
90-
else {
91-
subcommand::config(&subcommand, &parameters, false, system_root.as_ref(), &as_group, &as_assert, &as_include, progress_format);
92-
}
87+
} else {
88+
None
89+
};
90+
91+
let merged_parameters = match (file_params, parameters) {
92+
(Some(file_content), Some(inline_content)) => {
93+
info!("{}", t!("main.mergingParameters"));
94+
match util::merge_parameters(&file_content, &inline_content) {
95+
Ok(merged) => Some(merged),
96+
Err(err) => {
97+
error!("{}: {err}", t!("main.failedMergingParameters"));
98+
exit(EXIT_INVALID_INPUT);
99+
}
100+
}
101+
},
102+
(Some(file_content), None) => Some(file_content),
103+
(None, Some(inline_content)) => Some(inline_content),
104+
(None, None) => None,
105+
};
106+
107+
subcommand::config(&subcommand, &merged_parameters, system_root.as_ref(), &as_group, &as_assert, &as_include, progress_format);
93108
},
94109
SubCommand::Extension { subcommand } => {
95110
subcommand::extension(&subcommand, progress_format);

‎dsc/src/subcommand.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -276,15 +276,15 @@ fn initialize_config_root(path: Option<&String>) -> Option<String> {
276276

277277
#[allow(clippy::too_many_lines)]
278278
#[allow(clippy::too_many_arguments)]
279-
pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, parameters_from_stdin: bool, mounted_path: Option<&String>, as_group: &bool, as_assert: &bool, as_include: &bool, progress_format: ProgressFormat) {
279+
pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, mounted_path: Option<&String>, as_group: &bool, as_assert: &bool, as_include: &bool, progress_format: ProgressFormat) {
280280
let (new_parameters, json_string) = match subcommand {
281281
ConfigSubCommand::Get { input, file, .. } |
282282
ConfigSubCommand::Set { input, file, .. } |
283283
ConfigSubCommand::Test { input, file, .. } |
284284
ConfigSubCommand::Validate { input, file, .. } |
285285
ConfigSubCommand::Export { input, file, .. } => {
286286
let new_path = initialize_config_root(file.as_ref());
287-
let document = get_input(input.as_ref(), new_path.as_ref(), parameters_from_stdin);
287+
let document = get_input(input.as_ref(), new_path.as_ref());
288288
if *as_include {
289289
let (new_parameters, config_json) = match get_contents(&document) {
290290
Ok((parameters, config_json)) => (parameters, config_json),
@@ -300,7 +300,7 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, parame
300300
},
301301
ConfigSubCommand::Resolve { input, file, .. } => {
302302
let new_path = initialize_config_root(file.as_ref());
303-
let document = get_input(input.as_ref(), new_path.as_ref(), parameters_from_stdin);
303+
let document = get_input(input.as_ref(), new_path.as_ref());
304304
let (new_parameters, config_json) = match get_contents(&document) {
305305
Ok((parameters, config_json)) => (parameters, config_json),
306306
Err(err) => {
@@ -398,7 +398,7 @@ pub fn config(subcommand: &ConfigSubCommand, parameters: &Option<String>, parame
398398
};
399399
if *as_include {
400400
let new_path = initialize_config_root(file.as_ref());
401-
let input = get_input(input.as_ref(), new_path.as_ref(), parameters_from_stdin);
401+
let input = get_input(input.as_ref(), new_path.as_ref());
402402
match serde_json::from_str::<Include>(&input) {
403403
Ok(_) => {
404404
// valid, so do nothing
@@ -554,7 +554,7 @@ pub fn resource(subcommand: &ResourceSubCommand, progress_format: ProgressFormat
554554
},
555555
ResourceSubCommand::Export { resource, version, input, file, output_format } => {
556556
dsc.find_resources(&[DiscoveryFilter::new(resource, version.clone())], progress_format);
557-
let parsed_input = get_input(input.as_ref(), file.as_ref(), false);
557+
let parsed_input = get_input(input.as_ref(), file.as_ref());
558558
resource_command::export(&mut dsc, resource, version.as_deref(), &parsed_input, output_format.as_ref());
559559
},
560560
ResourceSubCommand::Get { resource, version, input, file: path, all, output_format } => {
@@ -567,23 +567,23 @@ pub fn resource(subcommand: &ResourceSubCommand, progress_format: ProgressFormat
567567
error!("{}", t!("subcommand.jsonArrayNotSupported"));
568568
exit(EXIT_INVALID_ARGS);
569569
}
570-
let parsed_input = get_input(input.as_ref(), path.as_ref(), false);
570+
let parsed_input = get_input(input.as_ref(), path.as_ref());
571571
resource_command::get(&mut dsc, resource, version.as_deref(), &parsed_input, output_format.as_ref());
572572
}
573573
},
574574
ResourceSubCommand::Set { resource, version, input, file: path, output_format } => {
575575
dsc.find_resources(&[DiscoveryFilter::new(resource, version.clone())], progress_format);
576-
let parsed_input = get_input(input.as_ref(), path.as_ref(), false);
576+
let parsed_input = get_input(input.as_ref(), path.as_ref());
577577
resource_command::set(&mut dsc, resource, version.as_deref(), &parsed_input, output_format.as_ref());
578578
},
579579
ResourceSubCommand::Test { resource, version, input, file: path, output_format } => {
580580
dsc.find_resources(&[DiscoveryFilter::new(resource, version.clone())], progress_format);
581-
let parsed_input = get_input(input.as_ref(), path.as_ref(), false);
581+
let parsed_input = get_input(input.as_ref(), path.as_ref());
582582
resource_command::test(&mut dsc, resource, version.as_deref(), &parsed_input, output_format.as_ref());
583583
},
584584
ResourceSubCommand::Delete { resource, version, input, file: path } => {
585585
dsc.find_resources(&[DiscoveryFilter::new(resource, version.clone())], progress_format);
586-
let parsed_input = get_input(input.as_ref(), path.as_ref(), false);
586+
let parsed_input = get_input(input.as_ref(), path.as_ref());
587587
resource_command::delete(&mut dsc, resource, version.as_deref(), &parsed_input);
588588
},
589589
}

‎dsc/src/util.rs‎

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Op
432432
info!("Trace-level is {:?}", tracing_setting.level);
433433
}
434434

435-
pub fn get_input(input: Option<&String>, file: Option<&String>, parameters_from_stdin: bool) -> String {
435+
pub fn get_input(input: Option<&String>, file: Option<&String>) -> String {
436436
trace!("Input: {input:?}, File: {file:?}");
437437
let value = if let Some(input) = input {
438438
debug!("{}", t!("util.readingInput"));
@@ -448,10 +448,6 @@ pub fn get_input(input: Option<&String>, file: Option<&String>, parameters_from_
448448
// check if need to read from STDIN
449449
if path == "-" {
450450
info!("{}", t!("util.readingInputFromStdin"));
451-
if parameters_from_stdin {
452-
error!("{}", t!("util.stdinNotAllowedForBothParametersAndInput"));
453-
exit(EXIT_INVALID_INPUT);
454-
}
455451
let mut stdin = Vec::<u8>::new();
456452
match std::io::stdin().read_to_end(&mut stdin) {
457453
Ok(_) => {
@@ -582,3 +578,109 @@ pub fn in_desired_state(test_result: &ResourceTestResult) -> bool {
582578
}
583579
}
584580
}
581+
582+
/// Parse input string as JSON or YAML and return a serde_json::Value.
583+
///
584+
/// # Arguments
585+
///
586+
/// * `input` - The input string to parse (JSON or YAML format)
587+
/// * `context` - Context string for error messages (e.g., "file parameters", "inline parameters")
588+
///
589+
/// # Returns
590+
///
591+
/// * `Result<serde_json::Value, DscError>` - Parsed JSON value
592+
///
593+
/// # Errors
594+
///
595+
/// This function will return an error if the input cannot be parsed as valid JSON or YAML
596+
fn parse_input_to_json_value(input: &str, context: &str) -> Result<serde_json::Value, DscError> {
597+
match serde_json::from_str(input) {
598+
Ok(json) => Ok(json),
599+
Err(_) => {
600+
match serde_yaml::from_str::<serde_yaml::Value>(input) {
601+
Ok(yaml) => Ok(serde_json::to_value(yaml)?),
602+
Err(err) => {
603+
Err(DscError::Parser(t!(&format!("util.failedToParse{context}"), error = err.to_string()).to_string()))
604+
}
605+
}
606+
}
607+
}
608+
}
609+
610+
/// Convert parameter input to a map, handling different formats.
611+
///
612+
/// # Arguments
613+
///
614+
/// * `params` - Parameter string to convert (JSON or YAML format)
615+
/// * `context` - Context string for error messages
616+
///
617+
/// # Returns
618+
///
619+
/// * `Result<serde_json::Map<String, serde_json::Value>, DscError>` - Parameter map
620+
///
621+
/// # Errors
622+
///
623+
/// Returns an error if the input cannot be parsed or is not an object
624+
fn params_to_map(params: &str, context: &str) -> Result<serde_json::Map<String, serde_json::Value>, DscError> {
625+
let value = parse_input_to_json_value(params, context)?;
626+
627+
let Some(map) = value.as_object().cloned() else {
628+
return Err(DscError::Parser(t!("util.parametersNotObject").to_string()));
629+
};
630+
631+
Ok(map)
632+
}
633+
634+
/// Merge two parameter sets, with inline parameters taking precedence over file parameters.
635+
/// Top-level keys (like "parameters") are merged recursively, but parameter values themselves
636+
/// are replaced (not merged) when specified inline.
637+
///
638+
/// # Arguments
639+
///
640+
/// * `file_params` - Parameters from file (JSON or YAML format)
641+
/// * `inline_params` - Inline parameters (JSON or YAML format) that take precedence
642+
///
643+
/// # Returns
644+
///
645+
/// * `Result<String, DscError>` - Merged parameters as JSON string
646+
///
647+
/// # Errors
648+
///
649+
/// This function will return an error if:
650+
/// - Either parameter set cannot be parsed as valid JSON or YAML
651+
/// - The merged result cannot be serialized to JSON
652+
pub fn merge_parameters(file_params: &str, inline_params: &str) -> Result<String, DscError> {
653+
use serde_json::Value;
654+
655+
// Convert both parameter inputs to maps
656+
let mut file_map = params_to_map(file_params, "FileParameters")?;
657+
let inline_map = params_to_map(inline_params, "InlineParameters")?;
658+
659+
// Merge top-level keys
660+
for (key, inline_value) in &inline_map {
661+
if key == "parameters" {
662+
// Special handling for "parameters" key - merge at parameter name level only
663+
// Within each parameter name, inline replaces (not merges)
664+
if let Some(file_params_value) = file_map.get_mut("parameters") {
665+
if let (Some(file_params_obj), Some(inline_params_obj)) = (file_params_value.as_object_mut(), inline_value.as_object()) {
666+
// For each parameter in inline, replace (not merge) in file
667+
for (param_name, param_value) in inline_params_obj {
668+
file_params_obj.insert(param_name.clone(), param_value.clone());
669+
}
670+
} else {
671+
// If one is not an object, inline replaces completely
672+
file_map.insert(key.clone(), inline_value.clone());
673+
}
674+
} else {
675+
// "parameters" key doesn't exist in file, add it
676+
file_map.insert(key.clone(), inline_value.clone());
677+
}
678+
} else {
679+
// For other top-level keys, inline value replaces file value
680+
file_map.insert(key.clone(), inline_value.clone());
681+
}
682+
}
683+
684+
let merged = Value::Object(file_map);
685+
Ok(serde_json::to_string(&merged)?)
686+
}

0 commit comments

Comments
 (0)