Skip to content

Modularize the generators#702

Merged
shaobo-he-aws merged 17 commits into
mainfrom
feature/shaobo/modular-generators
Nov 5, 2025
Merged

Modularize the generators#702
shaobo-he-aws merged 17 commits into
mainfrom
feature/shaobo/modular-generators

Conversation

@shaobo-he-aws

@shaobo-he-aws shaobo-he-aws commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

This PR implements a schema generator trait used by other generators like the request generator. ValidatorSchema and the crate's Schema both implement it.
So it allows the generator to generate policies/requests/hierarchies based on existing schema.

Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
…feature/shaobo/modular-generators

Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
…feature/shaobo/modular-generators

Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
…r-generators

Signed-off-by: Shaobo He <shaobohe@amazon.com>
@shaobo-he-aws shaobo-he-aws marked this pull request as ready for review November 4, 2025 18:35
Signed-off-by: Shaobo He <shaobohe@amazon.com>

@cdisselkoen cdisselkoen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like a reasonable refactor to me

Comment thread cedar-policy-generators/src/hierarchy.rs
Comment on lines +1587 to +1602
Ok(u.choose(
&self
.actions_eids
.iter()
.map(|eid| {
EntityUID::from_components(
EntityType::from_normalized_str("Action")
.unwrap()
.qualify_with(self.namespace()),
eid.clone(),
None,
)
})
.collect::<Vec<_>>(),
)?
.clone())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't have to be addressed in this PR, but this is pretty inefficient, it builds all Action EntityUIDs and collects them all into a Vec only to clone one of its choice and throw away the whole Vec. More efficient would be to first u.choose() an EID and then only build the EntityUID for that particular chosen EID

Comment thread cedar-policy-generators/src/schema_gen.rs
Comment thread cedar-policy-generators/src/schema_gen.rs Outdated
Comment on lines +370 to +380
Ok(u.choose(
&self
.core_schema
.get_entity_type(entity_type)
.expect("entity type should exist")
.attributes()
.iter()
.map(|a| a.0.clone())
.collect::<Vec<_>>(),
)?
.clone())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: similar inefficiency here to one of my above comments

@chaluli chaluli left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks reasonable to me.

Comment thread cedar-drt/fuzz/fuzz_targets/input-generation.rs Outdated
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
@shaobo-he-aws shaobo-he-aws merged commit 7f31881 into main Nov 5, 2025
13 checks passed
@shaobo-he-aws shaobo-he-aws deleted the feature/shaobo/modular-generators branch November 5, 2025 19:48
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