feat(azure-policy): add policy rule and policy definition parsers#660
feat(azure-policy): add policy rule and policy definition parsers#660anakrish wants to merge 1 commit intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds new Azure Policy parsing entry points for full policyRule and full policy definition envelopes, extending the existing constraint-only parser and broadening the YAML-driven test harness to run at different parse “levels”.
Changes:
- Introduce
parse_policy_rule()(top-level"if"+"then", effect handling, existenceCondition extraction). - Introduce
parse_policy_definition()(wrapped/unwrapped envelope handling, parameter definitions,policyRuleparsing). - Extend YAML test harness with
parse_leveland add new test suites for policy rules/definitions.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/azure_policy/parser_tests/mod.rs | Adds parse_level switching to run tests against constraint/policy_rule/policy_definition parsers. |
| tests/azure_policy/parser_tests/cases/policy_rule.yaml | New YAML cases exercising full policyRule parsing. |
| tests/azure_policy/parser_tests/cases/policy_definition.yaml | New YAML cases exercising full policy definition envelope parsing. |
| tests/azure_policy/parser_tests/cases/parse_errors.yaml | Un-skips policy_rule-level error cases by switching them to parse_level: policy_rule. |
| src/languages/azure_policy/parser/policy_rule.rs | New policy rule + then-block parser and existenceCondition extraction. |
| src/languages/azure_policy/parser/policy_definition.rs | New policy definition parser plus JSON re-serialization helpers for reparsing policyRule. |
| src/languages/azure_policy/parser/mod.rs | Exposes new public APIs and updates module docs. |
| src/languages/azure_policy/ast/mod.rs | Updates EffectKind::Other to carry a string payload; updates parameter doc comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a20db77 to
270f8b6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
270f8b6 to
d69f24c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d69f24c to
77cca59
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
77cca59 to
7c5eddd
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7c5eddd to
99c9eaf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
99c9eaf to
2afcc6e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4b4118e to
949ca8a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
949ca8a to
f36aab3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f36aab3 to
00f00d6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let constraint_json = | ||
| extract_if_json(&policy_rule_json).unwrap_or_else(|| policy_rule_json.clone()); |
There was a problem hiding this comment.
In the constraint parse_level branch, unwrap_or_else(|| policy_rule_json.clone()) clones the entire JSON string on the fallback path. This can be avoided by restructuring to move policy_rule_json into constraint_json when extraction fails (e.g., using a match on extract_if_json(...)) so the error-path tests don’t pay an extra allocation for large inputs.
| let constraint_json = | |
| extract_if_json(&policy_rule_json).unwrap_or_else(|| policy_rule_json.clone()); | |
| let constraint_json = match extract_if_json(&policy_rule_json) { | |
| Some(constraint_json) => constraint_json, | |
| None => policy_rule_json, | |
| }; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.expect_symbol("{")?; | ||
|
|
||
| let mut seen_keys: Vec<String> = Vec::new(); | ||
|
|
There was a problem hiding this comment.
parse_properties_fields creates a fresh seen_keys list, so duplicate detection for recognized properties (e.g., displayName, mode, policyRule, parameters) does not apply across the outer object and the inner properties object. If a wrapped ARM envelope contains a recognized key before properties (or inside extra) and the same key appears inside properties, the later parse will overwrite the earlier typed field without producing ParseError::DuplicateKey, contradicting “duplicate key detection throughout”. Consider sharing a single case-insensitive seen_keys set between the outer loop and parse_properties_fields (or explicitly rejecting/extra-ifying recognized keys once properties is detected) so cross-scope duplicates can’t silently overwrite.
00f00d6 to
4a25982
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extend the Azure Policy parser to handle complete policyRule and
policyDefinition JSON structures, not just standalone constraints.
Policy rule parser (policy_rule.rs):
- Parse top-level { "if": ..., "then": ... } objects
- Extract effect kind (deny, audit, append, modify, etc.) into typed AST
- Parse "details" structurally when it is an object to pull out
existenceCondition as a first-class Constraint; fall back to opaque
JSON for non-object details (e.g. append array form)
- Detect duplicate/missing keys for "if", "then", "effect", "details"
Policy definition parser (policy_definition.rs):
- Handle both wrapped ARM envelope ({ "properties": { ... } }) and
unwrapped (properties-level keys at top level) forms
- Type-extract displayName, description, mode, metadata, parameters,
and policyRule; everything else goes into extra
- Parse parameter definitions with type, defaultValue, allowedValues,
and metadata; detect duplicate parameter names
- Duplicate key detection throughout
Grammar documentation (docs/azure-policy/azurepolicy.ebnf):
- Add formal EBNF grammar covering policy-rule, then-block,
constraints, conditions, all 19 operators, count expressions,
JSON values, and ARM template expressions
Test harness changes:
- Add parse_level field to YAML test cases: "constraint" (default),
"policy_rule", or "policy_definition"
- Un-skip three parse_errors cases that needed policy_rule-level parsing
- Add policy_rule.yaml with 12 cases covering all 9 effect kinds,
existenceCondition, parameterized effects, complex conditions, and
extra key handling
- Add policy_definition.yaml with wrapped, unwrapped, parameterized,
missing-policyRule, and duplicate-key error cases
4a25982 to
7219f55
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extend the Azure Policy parser to handle complete policyRule and
policyDefinition JSON structures, not just standalone constraints.
Policy rule parser (policy_rule.rs):
existenceCondition as a first-class Constraint; fall back to opaque
JSON for non-object details (e.g. append's array form)
Policy definition parser (policy_definition.rs):
unwrapped (properties-level keys at top level) forms
and policyRule; everything else goes into extra
and metadata; detect duplicate parameter names
Grammar documentation (docs/azure-policy/azurepolicy.ebnf):
constraints, conditions, all 20 operators, count expressions,
JSON values, and ARM template expressions
Test harness changes:
"policy_rule", or "policy_definition"
existenceCondition, parameterized effects, complex conditions, and
extra key handling
and missing-policyRule error cases