Fix validation for action entitites in TPE

[luxas]

Description of changes

In the Cedar+Kubernetes integration, I noticed that I could not use TPE correctly. At first, I did not add the action entities to the entities list when running TPE, but then the residual contained things like action in Action::"foo", which I did not want, as this data is in fact known at this point.

However, adding the actions to the entity list manually here yielded an error, as PartialEntity.validate both errors if the action has None and Some attributes and tags, which I presume is a bug.

With this patch the k8s integration works as expected.

As a side point: it'd be nice to not have to do the extra work of adding the action entities at all, but let that be done automatically by TPE, as it anyways has the schema at hand.

FYI @shaobo-he-aws

I'll make a unit test once you've confirmed removing this if statement was desired.

Checklist for requesting a review

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

• A bug fix that only impacts unreleased or experimental code.

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

• Does not update the CHANGELOG because my change does not significantly impact released code.

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

• I'm not sure how my change impacts cedar-spec. (Post your PR anyways, and we'll discuss in the comments.)

Not sure if this is covered (or should be) by DRT.

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.

[github-actions]

Coverage Report

Head Commit: e5e390721abfa3b8f9832785674a25c22fbb9306

Base Commit: 22ec17872ca100485b235e6fe925b715619ad898

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 100.00%

Status: PASSED ✅

Details

File	Status	Covered	Coverage	Missed Lines

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 84.08%

Status: PASSED ✅

Details

Package	Status	Covered	Coverage	Base Coverage
cedar-language-server	🟢	4619/5120	90.21%	90.21%
cedar-policy	🟡	3513/4929	71.27%	71.27%
cedar-policy-cli	🔴	675/1054	64.04%	64.04%
cedar-policy-core	🟢	21448/25205	85.09%	85.08%
cedar-policy-formatter	🟢	930/1112	83.63%	83.63%
cedar-policy-symcc	🟢	4715/5249	89.83%	89.83%
cedar-wasm	🔴	0/28	0.00%	0.00%

[chaluli]

I believe here none is used to denote unknown attributes / tags. Which is different from no attributes which would be represented as Some(Empty BTreeMap).

@shaobo-he-aws can confirm.

[chaluli]

In the code you linked here, if you changed lines 425 and 434 to be Some(BTreeMap::new()) does it work as expected without this patch?

[shaobo-he-aws]

@chaluli is correct.

[luxas]

I'll check. Don't remember of the top of my head, it's a month or so since I looked at this, I just put a note to myself to check this later

[chaluli]

To your aside, I agree we should have a way to automatically add the action entities on a users behalf within TPE because the schema is known (perhaps as a separate API for constructing a partial entities).

[shaobo-he-aws]

@luxas sorry about the confusion. Like @chaluli said exactly, you will need to make empty attributes and empty tags in action entities. We should be able to add action entities automatically. I'll make a PR.

[shaobo-he-aws]

To your aside, I agree we should have a way to automatically add the action entities on a users behalf within TPE because the schema is known (perhaps as a separate API for constructing a partial entities).

As a matter of fact, actions are inserted in PartialEntities::from_json_value not in PartialEntities::from_entities. I'll update the latter.

[chaluli]

Awesome. Thanks @shaobo-he-aws

[luxas]

Awesome, #1844 addresses this then 💯
Thanks for the response, not sure why I got confused of the None vs Some when implementing that code a month ago, technically I did know None meant "unknown" and not "empty", as I had reviewed the TPE PR 😄
Now, let's move on to the next item I had on that list for small improvements 👍