Skip to content

Commit 29382d6

Browse files
shaobo-he-awsadpaco-aws
shaobo-he-aws
and
adpaco-aws
authored
Enable generation of arbitrary annotations and test their equivalence (#501)
Signed-off-by: Shaobo He <[email protected]> Co-authored-by: Adrian Palacios <[email protected]>
1 parent 1036c96 commit 29382d6

File tree

3 files changed

+120
-28
lines changed

3 files changed

+120
-28
lines changed

‎cedar-drt/fuzz/src/schemas.rs‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,40 @@ impl<'a, T: Equiv> Equiv for &'a T {
8888
}
8989
}
9090

91+
impl Equiv for cedar_policy_core::est::Annotations {
92+
fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> {
93+
Equiv::equiv(&lhs.0, &rhs.0)
94+
}
95+
}
96+
97+
impl Equiv for Option<cedar_policy_core::ast::Annotation> {
98+
fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> {
99+
match (lhs, rhs) {
100+
(Some(a), Some(b)) => {
101+
if a == b {
102+
return Ok(());
103+
}
104+
}
105+
(Some(a), None) | (None, Some(a)) => {
106+
if a.val.is_empty() {
107+
return Ok(());
108+
}
109+
}
110+
(None, None) => return Ok(()),
111+
};
112+
Err(format!("{lhs:?} and {rhs:?} are not equivalent"))
113+
}
114+
}
115+
91116
impl<N: Clone + PartialEq + Debug + Display + TypeName + Ord> Equiv
92117
for json_schema::NamespaceDefinition<N>
93118
{
94119
fn equiv(
95120
lhs: &json_schema::NamespaceDefinition<N>,
96121
rhs: &json_schema::NamespaceDefinition<N>,
97122
) -> Result<(), String> {
123+
Equiv::equiv(&lhs.annotations, &rhs.annotations)
124+
.map_err(|e| format!("mismatch in namespace annotations: {e}"))?;
98125
Equiv::equiv(&lhs.entity_types, &rhs.entity_types)
99126
.map_err(|e| format!("mismatch in entity type declarations: {e}"))?;
100127
Equiv::equiv(&lhs.common_types, &rhs.common_types)
@@ -191,12 +218,16 @@ impl<K: Eq + Ord + Display, V: Equiv> Equiv for BTreeMap<K, V> {
191218

192219
impl<N: Clone + PartialEq + Debug + Display + TypeName + Ord> Equiv for json_schema::CommonType<N> {
193220
fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> {
221+
Equiv::equiv(&lhs.annotations, &rhs.annotations)
222+
.map_err(|e| format!("mismatch in common type annotations: {e}"))?;
194223
Equiv::equiv(&lhs.ty, &rhs.ty)
195224
}
196225
}
197226

198227
impl<N: Clone + PartialEq + Debug + Display + TypeName + Ord> Equiv for json_schema::EntityType<N> {
199228
fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> {
229+
Equiv::equiv(&lhs.annotations, &rhs.annotations)
230+
.map_err(|e| format!("mismatch in entity annotations: {e}"))?;
200231
Equiv::equiv(
201232
&lhs.member_of_types.iter().collect::<BTreeSet<_>>(),
202233
&rhs.member_of_types.iter().collect::<BTreeSet<_>>(),
@@ -226,6 +257,8 @@ impl Equiv for cedar_policy_validator::ValidatorEntityType {
226257

227258
impl<N: Clone + PartialEq + TypeName + Debug + Display> Equiv for json_schema::TypeOfAttribute<N> {
228259
fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> {
260+
Equiv::equiv(&lhs.annotations, &rhs.annotations)
261+
.map_err(|e| format!("mismatch in type of attribute annotations: {e}"))?;
229262
if lhs.required != rhs.required {
230263
return Err("attributes differ in required flag".into());
231264
}
@@ -434,6 +467,8 @@ impl TypeName for InternalName {
434467

435468
impl<N: PartialEq + Debug + Display + Clone + TypeName + Ord> Equiv for json_schema::ActionType<N> {
436469
fn equiv(lhs: &Self, rhs: &Self) -> Result<(), String> {
470+
Equiv::equiv(&lhs.annotations, &rhs.annotations)
471+
.map_err(|e| format!("mismatch in action annotations: {e}"))?;
437472
if &lhs.attributes != &rhs.attributes
438473
&& !(lhs.attributes.as_ref().is_none_or(HashMap::is_empty)
439474
&& rhs.attributes.as_ref().is_none_or(HashMap::is_empty))
@@ -517,3 +552,70 @@ impl Equiv for cedar_policy_validator::ValidatorSchema {
517552
Ok(())
518553
}
519554
}
555+
556+
#[cfg(test)]
557+
mod tests {
558+
use cedar_drt::est::Annotations;
559+
560+
use crate::schemas::Equiv;
561+
562+
#[test]
563+
fn annotations() {
564+
// positive cases
565+
let pairs: [(Annotations, Annotations); 5] = [
566+
// value being null is equivalent to an empty string
567+
(
568+
serde_json::from_value(serde_json::json!({"a": null})).unwrap(),
569+
serde_json::from_value(serde_json::json!({"a": ""})).unwrap(),
570+
),
571+
(
572+
serde_json::from_value(serde_json::json!({"a": ""})).unwrap(),
573+
serde_json::from_value(serde_json::json!({"a": null})).unwrap(),
574+
),
575+
// both values being null is also equivalent
576+
(
577+
serde_json::from_value(serde_json::json!({"a": null})).unwrap(),
578+
serde_json::from_value(serde_json::json!({"a": null})).unwrap(),
579+
),
580+
// otherwise compare non-null values
581+
(
582+
serde_json::from_value(serde_json::json!({"a": "🥨"})).unwrap(),
583+
serde_json::from_value(serde_json::json!({"a": "🥨"})).unwrap(),
584+
),
585+
(
586+
serde_json::from_value(serde_json::json!({"a": "🥨", "b": "🥯🍩"})).unwrap(),
587+
serde_json::from_value(serde_json::json!({"b": "🥯🍩", "a": "🥨"})).unwrap(),
588+
),
589+
];
590+
pairs
591+
.iter()
592+
.for_each(|(a, b)| assert!(Equiv::equiv(a, b).is_ok()));
593+
594+
// negative cases
595+
let pairs: [(Annotations, Annotations); 5] = [
596+
(
597+
serde_json::from_value(serde_json::json!({"a": null})).unwrap(),
598+
serde_json::from_value(serde_json::json!({"a": "🍪"})).unwrap(),
599+
),
600+
(
601+
serde_json::from_value(serde_json::json!({"a": ""})).unwrap(),
602+
serde_json::from_value(serde_json::json!({"b": null})).unwrap(),
603+
),
604+
(
605+
serde_json::from_value(serde_json::json!({"a": null})).unwrap(),
606+
serde_json::from_value(serde_json::json!({"b": null})).unwrap(),
607+
),
608+
(
609+
serde_json::from_value(serde_json::json!({"a": "🥨"})).unwrap(),
610+
serde_json::from_value(serde_json::json!({"a": "🍪"})).unwrap(),
611+
),
612+
(
613+
serde_json::from_value(serde_json::json!({"a": "🥨", "b": "🥯🍪"})).unwrap(),
614+
serde_json::from_value(serde_json::json!({"b": "🥯🍩", "a": "🥨"})).unwrap(),
615+
),
616+
];
617+
pairs
618+
.iter()
619+
.for_each(|(a, b)| assert!(Equiv::equiv(a, b).is_err()));
620+
}
621+
}

‎cedar-policy-generators/src/policy.rs‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,10 @@ use crate::hierarchy::Hierarchy;
2020
use crate::size_hint_utils::size_hint_for_ratio;
2121
use arbitrary::{Arbitrary, Unstructured};
2222
use cedar_policy_core::ast::{
23-
Annotation, Annotations, AnyId, Effect, EntityUID, Expr, Policy, PolicyID, PolicySet,
24-
StaticPolicy, Template,
23+
Effect, EntityUID, Expr, Policy, PolicyID, PolicySet, StaticPolicy, Template,
2524
};
2625
use cedar_policy_core::{ast, est};
2726
use serde::Serialize;
28-
use smol_str::SmolStr;
2927
use std::fmt::Display;
3028
use std::sync::Arc;
3129

@@ -39,7 +37,7 @@ use std::sync::Arc;
3937
#[serde(into = "est::Policy")]
4038
pub struct GeneratedPolicy {
4139
id: PolicyID,
42-
annotations: HashMap<AnyId, SmolStr>,
40+
annotations: cedar_policy_core::est::Annotations,
4341
effect: Effect,
4442
principal_constraint: PrincipalOrResourceConstraint,
4543
action_constraint: ActionConstraint,
@@ -59,7 +57,7 @@ impl GeneratedPolicy {
5957
/// Create a new `GeneratedPolicy` with these fields
6058
pub fn new(
6159
id: PolicyID,
62-
annotations: impl IntoIterator<Item = (AnyId, SmolStr)>,
60+
annotations: cedar_policy_core::est::Annotations,
6361
effect: Effect,
6462
principal_constraint: PrincipalOrResourceConstraint,
6563
action_constraint: ActionConstraint,
@@ -68,7 +66,7 @@ impl GeneratedPolicy {
6866
) -> Self {
6967
Self {
7068
id,
71-
annotations: annotations.into_iter().collect(),
69+
annotations,
7270
effect,
7371
principal_constraint,
7472
action_constraint,
@@ -141,19 +139,12 @@ impl GeneratedPolicy {
141139
}
142140
}
143141

144-
fn convert_annotations(annotations: HashMap<AnyId, SmolStr>) -> Annotations {
145-
annotations
146-
.into_iter()
147-
.map(|(k, v)| (k, Annotation { val: v, loc: None }))
148-
.collect()
149-
}
150-
151142
impl From<GeneratedPolicy> for StaticPolicy {
152143
fn from(gen: GeneratedPolicy) -> StaticPolicy {
153144
StaticPolicy::new(
154145
gen.id,
155146
None,
156-
convert_annotations(gen.annotations),
147+
gen.annotations.into(),
157148
gen.effect,
158149
gen.principal_constraint.into(),
159150
gen.action_constraint.into(),
@@ -169,7 +160,7 @@ impl From<GeneratedPolicy> for Template {
169160
Template::new(
170161
gen.id,
171162
None,
172-
convert_annotations(gen.annotations),
163+
gen.annotations.into(),
173164
gen.effect,
174165
gen.principal_constraint.into(),
175166
gen.action_constraint.into(),

‎cedar-policy-generators/src/schema.rs‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ use crate::size_hint_utils::{size_hint_for_choose, size_hint_for_range, size_hin
3030
use crate::{accum, gen, gen_inner, uniform};
3131
use arbitrary::{self, Arbitrary, MaxRecursionReached, Unstructured};
3232
use cedar_policy_core::ast::{self, Effect, PolicyID, UnreservedId};
33-
use cedar_policy_core::est::Annotations;
3433
use cedar_policy_core::extensions::Extensions;
3534
use cedar_policy_validator::json_schema::{CommonType, CommonTypeId};
3635
use cedar_policy_validator::{
@@ -153,15 +152,16 @@ fn arbitrary_typeofattribute_with_bounded_depth<N: From<ast::Name>>(
153152
Ok(json_schema::TypeOfAttribute {
154153
ty: arbitrary_schematype_with_bounded_depth::<N>(settings, entity_types, max_depth, u)?,
155154
required: u.arbitrary()?,
156-
annotations: Annotations::new(),
155+
annotations: u.arbitrary()?,
157156
})
158157
}
159158
/// size hint for arbitrary_typeofattribute_with_bounded_depth
160159
fn arbitrary_typeofattribute_size_hint(depth: usize) -> (usize, Option<usize>) {
161-
arbitrary::size_hint::and(
160+
arbitrary::size_hint::and_all(&[
162161
arbitrary_schematype_size_hint(depth),
163162
<bool as Arbitrary>::size_hint(depth),
164-
)
163+
<cedar_policy_core::est::Annotations as Arbitrary>::size_hint(depth),
164+
])
165165
}
166166

167167
/// internal helper function, an alternative to the `Arbitrary` impl for
@@ -710,15 +710,15 @@ impl Schema {
710710
common_types: common_types
711711
.into_iter()
712712
.map(|(id, ty)| {
713-
(
713+
Ok((
714714
id,
715715
CommonType {
716716
ty,
717-
annotations: Annotations::new(),
717+
annotations: u.arbitrary()?,
718718
},
719-
)
719+
))
720720
})
721-
.collect(),
721+
.collect::<Result<BTreeMap<_, _>>>()?,
722722
entity_types: entity_types.into(),
723723
actions: actions.into(),
724724
annotations: self.schema.annotations.clone(),
@@ -931,7 +931,7 @@ impl Schema {
931931
u
932932
)?)
933933
),
934-
annotations: Annotations::new(),
934+
annotations: u.arbitrary()?,
935935
},
936936
))
937937
})
@@ -1034,7 +1034,7 @@ impl Schema {
10341034
},
10351035
//TODO: Fuzz arbitrary attribute names and values.
10361036
attributes: None,
1037-
annotations: Annotations::new(),
1037+
annotations: u.arbitrary()?,
10381038
},
10391039
))
10401040
})
@@ -1064,7 +1064,7 @@ impl Schema {
10641064
common_types: BTreeMap::new().into(),
10651065
entity_types: entity_types.into_iter().collect(),
10661066
actions: actions.into_iter().collect(),
1067-
annotations: Annotations::new(),
1067+
annotations: u.arbitrary()?,
10681068
};
10691069
let attrsorcontexts /* : impl Iterator<Item = &AttributesOrContext> */ = nsdef.entity_types.values().map(|et| attrs_from_attrs_or_context(&nsdef, &et.shape))
10701070
.chain(nsdef.actions.iter().filter_map(|(_, action)| action.applies_to.as_ref()).map(|a| attrs_from_attrs_or_context(&nsdef, &a.context)));
@@ -1361,7 +1361,6 @@ impl Schema {
13611361
u: &mut Unstructured<'_>,
13621362
) -> Result<ABACPolicy> {
13631363
let id = u.arbitrary()?;
1364-
let annotations: HashMap<ast::AnyId, SmolStr> = u.arbitrary()?;
13651364
let effect = u.arbitrary()?;
13661365
let principal_constraint = self.arbitrary_principal_constraint(hierarchy, u)?;
13671366
let action_constraint = self.arbitrary_action_constraint(u, Some(3))?;
@@ -1386,7 +1385,7 @@ impl Schema {
13861385
}
13871386
Ok(ABACPolicy(GeneratedPolicy::new(
13881387
id,
1389-
annotations,
1388+
u.arbitrary()?,
13901389
effect,
13911390
principal_constraint,
13921391
action_constraint,

0 commit comments

Comments
 (0)