Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Followup to #145 #155

Merged
merged 2 commits into from
Nov 16, 2023
Merged

Followup to #145 #155

merged 2 commits into from
Nov 16, 2023

Conversation

khieta
Copy link
Contributor

@khieta khieta commented Nov 16, 2023

Issue #, if available:

Description of changes:

Followup to #145, which updated generators with support for the is operator. This PR adds support for generating is in scope constraints to schema.rs (which is used for most DRT targets). It mirrors the corresponding change made previously in policy.rs (which is used for the rbac target).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

1 => Ok(PrincipalOrResourceConstraint::IsType(ety)),
1 => Ok(PrincipalOrResourceConstraint::IsTypeIn(ety, uid))
)
}
}
fn arbitrary_principal_constraint_size_hint(depth: usize) -> (usize, Option<usize>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure whether/how the *_size_hint functions need to be updated. Suggestions welcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cdisselkoen may have written the original

Copy link
Contributor

Choose a reason for hiding this comment

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

They are mostly performance optimizations; see docs on Arbitrary::size_hint. If/when we migrate to Bolero, they will go away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it's fine to leave these functions as-is?

cedar-policy-generators/src/policy.rs Show resolved Hide resolved
Comment on lines +1042 to +1044
let uid = self
.exprgenerator(Some(hierarchy))
.arbitrary_principal_uid(u)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

generates the uid even in the IsType case where it isn't used. Probably ok though

@khieta khieta merged commit 8796300 into main Nov 16, 2023
3 checks passed
@khieta khieta deleted the khieta/fix/is-drt branch November 16, 2023 19:26
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