{{ message }}
Repository navigation
(GH-579) Add dsc lint command - #1768
Draft
Gijs Reijn (Gijsreyn) wants to merge 1 commit into
Draft
Gijs Reijn (Gijsreyn) wants to merge 1 commit into
Gijs Reijn (Gijsreyn) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Secure values can leak into diagnostics and logs, and validation mismatches produce incorrect lint results.
11 open issues
What changed in this PR
Adds dsc lint to detect configuration problems before execution, including duplicate resources and system-root usage checks requested in #842 and #591.
Changes:
- Adds configurable lint rules, structured diagnostics, discovery checks, and failure thresholds.
- Shares parsing and validation helpers with runtime code.
- Adds schemas, documentation, localization, and automated tests.
Review effort: Balanced
| /// * `DscError::Validation` - The schema failed to compile | ||
| pub fn validate_json_all(source: &str, schema: &Value, json: &Value) -> Result<Vec<SchemaViolation>, DscError> { | ||
| debug!("{}: {source}", t!("dscresources.dscresource.validatingSchema")); | ||
| trace!("{}", t!("dscresources.dscresource.json", json = json)); |
Comment on lines
+110
to
+111
| expression = site.text, | ||
| error = LintContext::error_message(err) |
Comment on lines
+100
to
+101
| #[clap(name = "lint", about = t!("args.lintAbout").to_string())] | ||
| Lint { |
Comment on lines
+1126
to
+1127
| secureValueInOutputDescription = "Outputs shouldn't expose secure values; DSC skips such outputs at runtime." | ||
| secureValueInOutput = "Output '%{name}' exposes the secure value from %{source}, so DSC skips it at runtime" |
Comment on lines
+124
to
+127
| let scope = ExpressionScope::Resource(position); | ||
| for key in ["name", "condition", "dependsOn", "properties"] { | ||
| if let Some(member) = resource.get(key) { | ||
| index.walk(member, &site.pointer.push(key), &scope, parser); |
Comment on lines
+190
to
+191
| for (pointer, key) in literal_dependencies(context, index).into_iter().chain(reference_targets(context, index)) { | ||
| match resolve(context, &group, &key) { |
| if value.as_str().is_some_and(LintContext::is_expression) { | ||
| continue; | ||
| } | ||
| if let Err(reason) = check_value(name, value, parameter) { |
Comment on lines
+440
to
+442
| if let Err(reason) = check_value(name, value, parameter) { | ||
| sink.report(¶meter_pointer(name), t!("lint.rules.parameter.parameterValueType", name = name, reason = reason).to_string()); | ||
| } |
Comment on lines
+203
to
+206
| let pointer = base.join(&violation.instance_path); | ||
| let in_expression = context.expressions.sites().iter().any(|expression| { | ||
| expression.scope == ExpressionScope::Resource(index) && pointer.starts_with(&expression.pointer) | ||
| }); |
Comment on lines
+204
to
+206
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
This change:
dsc lintcommand. It checks a configuration document for problems without applyingit, reads one or more files or an inline document, and always writes a result with the
diagnostics, a summary, whether the document is valid, and whether discovery ran. It exits with
code 5 when a diagnostic reaches the level set by the fail-on option, which defaults to error.
schema and reports every violation with its JSON pointer, then indexes every expression once and
runs 43 rules across seven categories plus four fixed rules for parse, schema, unsupported
document type, and discovery failures. Rule levels can be changed per run or in a new lint
section of the settings file, with the policy file taking precedence.
the expression parser, argument-count and argument-kind checks on the function dispatcher
extracted from invoke, a schema validator that returns every violation instead of the first, and
a raw-text input reader in the CLI so a syntax error becomes a diagnostic instead of an early
exit. Parameter merging and parameter parsing move into helpers shared by the config and lint
commands.
integration test, and adds reference docs, a rules catalog, a changelog entry, 68 Rust unit
tests, and 43 Pester tests including a sweep of the shipped examples.
PR Context
DSC only finds most document mistakes when it runs the document. The hidden validate subcommand
is only called by the Group, Assertion and Include resources, reports a bare boolean, exits zero
even when invalid, and discovers resources before validating anything. Mistakes such as a
dependency on a resource that doesn't exist, a reference to a resource that hasn't run, a
condition written as a plain string, or a secure parameter with a hard-coded default are reported
late, without a location, or not at all.
Remarks on the design
skip it. Rules that depend on discovery report warnings rather than errors, so a build agent
without the resources still passes. The linter never invokes a resource; property checks only
use the schema embedded in the manifest.
pipeline always has something to show. The valid flag doesn't depend on the fail-on level.
JSON parsers keep no positions for values. The location field is in the schema from the start so
adding positions later isn't a breaking change.
rule exists because DSC doesn't auto-order on references the way ARM does. Duplicate and
dependency rules skip copy loops, whose names are only known at runtime. Secure default values
are never echoed in messages.