remove LiteralPolicy for proto and merge validation steps#2475
remove LiteralPolicy for proto and merge validation steps#2475victornicolet wants to merge 4 commits into
LiteralPolicy for proto and merge validation steps#2475Conversation
Signed-off-by: Victor Nicolet <victornl@amazon.com>
| /// Convert a [`models::Policy`] to an [`ast::Policy`] given a set of `templates`. | ||
| /// Returns an error when any of the invariants of [`ast::Policy`] would be violated, | ||
| /// or the template references in the model doesn't exist in the set of templates. | ||
| fn reify( |
There was a problem hiding this comment.
This is similar to what LiteralPolicy::reify(self,... was doing.
Signed-off-by: Victor Nicolet <victornl@amazon.com>
34cb0d1 to
bd8a76e
Compare
| make_linked_policy("template\0\n \' \"+-$^!", "link\0\n \' \"+-$^!", "A", "eid"); | ||
| let model2 = models::Policy::from(&linked_policy2); | ||
| let roundtripped2 = reify(model2, &templates2).unwrap(); | ||
| assert_eq!(linked_policy2, roundtripped2); |
There was a problem hiding this comment.
The test mostly change the input type and tests reify as opposed to try_from.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Pull request overview
This PR completes the protobuf validation/refactor work started in #2469 by removing the LiteralPolicy / LiteralPolicySet intermediate representation and consolidating policy/policyset validation and reification directly into ast::Policy/ast::PolicySet conversion paths.
Changes:
- Introduces a
reify(models::Policy, templates)conversion flow to produceast::Policydirectly (removingast::LiteralPolicyusage in protobuf code). - Refactors protobuf
models::PolicySet -> ast::PolicySetconversion to build template/link maps in one pass with duplicate detection. - Removes
LiteralPolicy(and associated reification error type) fromcedar-policy-core, replacing static-policy creation withTemplate::try_as_policy.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cedar-policy/src/proto/policy.rs | Switches protobuf policy conversion to direct ast::Policy reification and refactors PolicySet conversion + tests accordingly. |
| cedar-policy-core/src/ast/policy.rs | Adds Template::try_as_policy and removes the LiteralPolicy IR and related reification logic/types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment was marked as outdated.
This comment was marked as outdated.
25727ce to
b520054
Compare
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 92.86% Status: PASSED ✅ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 88.27% Status: PASSED ✅ Details
|
LiteralPolicy for proto and merge validation steps
Description of changes
This part 2 of #2469:
LiteralPolicySetstruct that was used as an intermediate betweenmodels::PolicySetandast::PolicySet.PolicySetthat were copy-pasted in the previous PR,Policythat were separated byLiteralPolicy.The
Policyinvariants on slot well formedness is now entirely in the Template to Policy conversion (try_as_policyenforces the same aslinkofast::Template).Issue #, if available
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., changes to the signature of an existing API).cedar-policy(e.g., addition of a new API).cedar-policy.cedar-policy-core,cedar-validator, etc.)I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):cedar-spec, and how you have tested that your updates are correct.)cedar-spec. (Post your PR anyways, and we'll discuss in the comments.)I confirm that
docs.cedarpolicy.com(choose one, and delete the other options):cedar-docs. PRs should be targeted at astaging-X.Ybranch, notmain.)