Skip to content

Conversation

cdisselkoen
Copy link
Contributor

Description of changes

More simplifications of the Protobuf format for Schema, some suggested in #1488 and some based on conversations with @john-h-kastner-aws offline.

  • resolving the TODO: Schema is just a list of EntityDecl and a list of ActionDecl, not a map, as this is simpler and sufficient
  • removing the ability to have open attributes, as this is not a stable feature (in both entities and records)
  • the context for an Action must be a record, so instead of Type, we require it to be a map<string, AttributeType>
  • rename Type.Ty to Type.Prim which is more clear
  • remove the Never, True, False, EmptySet, and AnyEntity types, as these should never appear in schemas
  • remove the ActionEntity type as action entities can just be encoded as Entity type (we do not support action attributes in Protobuf, as that is not a stable feature)
  • remove EntityRecordKind in favor of inlining it into Type
  • a few field renumberings, which is a breaking change we can make now but not after stabilization

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

  • A change (breaking or otherwise) that only impacts unreleased or experimental code.

I confirm that this PR (choose one, and delete the other options):

  • Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that cedar-spec (choose one, and delete the other options):

  • Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in cedar-spec, and how you have tested that your updates are correct.)

I confirm that docs.cedarpolicy.com (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar language specification.

Signed-off-by: Craig Disselkoen <[email protected]>
Copy link

Coverage Report

Head Commit: e47d8ac1b9dffb111c6b2d09aab66059f5bb400f

Base Commit: 25f7d950cadcfecddaecc2f3ba5af99a0adde0bf

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 0.00%

Status: FAILED ❌

Details
File Status Covered Coverage Missed Lines
cedar-policy/src/proto/validator.rs 🔴 0/53 0.00% 29-30, 42, 45, 92, 94-97, 104, 118-121, 167, 186-187, 189-191, 194-195, 197-200, 202-206, 208, 222-223, 226, 229, 232, 236, 239, 243-244, 246-250, 252-256, 258, 260

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 85.50%

Status: PASSED ✅

Details
Package Status Covered Coverage Base Coverage
cedar-policy 🟢 10129/11618 87.18% 86.66%
cedar-policy-cli 🔴 526/912 57.68% 57.68%
cedar-policy-core 🟢 11947/14443 82.72% 82.72%
cedar-policy-formatter 🟢 907/1035 87.63% 87.63%
cedar-policy-validator 🟢 14290/15744 90.76% 93.15%
cedar-testing 🔴 0/426 0.00% 0.00%
cedar-wasm 🔴 0/29 0.00% 0.00%

Copy link
Contributor

@john-h-kastner-aws john-h-kastner-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like our new coverage check is correctly complaining about the lack of tests for validator protobuf. should ofc add tests before stabilizing, but we can bypass for now

@cdisselkoen cdisselkoen merged commit 44b4e70 into main Feb 26, 2025
18 of 20 checks passed
@cdisselkoen cdisselkoen deleted the cdisselkoen/1488-followups branch February 26, 2025 18:54
jv-garcia pushed a commit to jv-garcia/cedar that referenced this pull request May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants