diff --git a/src/languages/rego/compiler/core.rs b/src/languages/rego/compiler/core.rs index e9c1c32..11b1382 100644 --- a/src/languages/rego/compiler/core.rs +++ b/src/languages/rego/compiler/core.rs @@ -140,6 +140,14 @@ impl<'a> Compiler<'a> { } } + /// Returns true when a variable is already bound in the innermost scope + pub fn is_var_bound_in_current_scope(&self, var_name: &str) -> bool { + self.scopes + .last() + .map(|scope| scope.bound_vars.contains_key(var_name)) + .unwrap_or(false) + } + /// Look up a variable in all scopes starting from innermost (like interpreter's lookup_local_var) pub fn lookup_local_var(&self, var_name: &str) -> Option { self.scopes diff --git a/src/languages/rego/compiler/destructuring.rs b/src/languages/rego/compiler/destructuring.rs index edbc0dd..6601f3f 100644 --- a/src/languages/rego/compiler/destructuring.rs +++ b/src/languages/rego/compiler/destructuring.rs @@ -289,7 +289,7 @@ impl<'a> Compiler<'a> { return Ok(()); } - if self.lookup_local_var(var_name).is_some() { + if self.is_var_bound_in_current_scope(var_name) { bail!("Variable '{var_name}' already defined in current scope"); } diff --git a/src/rvm/vm/dispatch.rs b/src/rvm/vm/dispatch.rs index 464c05c..82aae74 100644 --- a/src/rvm/vm/dispatch.rs +++ b/src/rvm/vm/dispatch.rs @@ -299,7 +299,10 @@ impl RegoVM { let operand_value = &self.registers[operand as usize]; if operand_value == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + // In Rego, `not expr` succeeds when `expr` has no results. + // When the operand evaluates to undefined we should treat it as + // a successful negation instead of propagating undefined. + self.registers[dest as usize] = Value::Bool(true); return Ok(InstructionOutcome::Continue); } diff --git a/tests/aci/aci.yaml b/tests/aci/aci.yaml index b16e64c..57128e0 100644 --- a/tests/aci/aci.yaml +++ b/tests/aci/aci.yaml @@ -11,7 +11,7 @@ cases: target: /run/layers/p0-layer0 data: metadata: {} - query: data.policy.mount_device=x + query: data.policy.mount_device want_result: - x: allowed: true @@ -44,7 +44,7 @@ cases: "/run/layers/p0-layer3": 41d64cdeb347bf236b4c13b7403b633ff11f1cf94dbc7cf881a44d6da88c5156 "/run/layers/p0-layer4": 4dedae42847c704da891a28c25d32201a1ae440bce2aecccfa8e6f03b97a6a6c "/run/layers/p0-layer5": fe84c9d5bfddd07a2624d00333cf13c1a9c941f3a261f13ead44fc6a93bc0e7a - query: data.policy.mount_overlay=x + query: data.policy.mount_overlay want_result: - x: allowed: true @@ -68,7 +68,7 @@ cases: target: /mnt/layer6 data: metadata: {} - query: data.policy.scratch_mount=x + query: data.policy.scratch_mount want_result: - x: allowed: true @@ -201,7 +201,7 @@ cases: scratch_mounts: "/mnt/layer6": encrypted: true - query: data.policy.create_container=x + query: data.policy.create_container want_result: - x: allow_stdio_access: false @@ -257,7 +257,7 @@ cases: started: container0: - {"privileged": false} - query: data.policy.shutdown_container=x + query: data.policy.shutdown_container want_result: - x: allowed: true @@ -286,7 +286,7 @@ cases: scratch_mounts: "/mnt/layer6": encrypted: true - query: data.policy.scratch_unmount=x + query: data.policy.scratch_unmount want_result: - x: allowed: true @@ -313,7 +313,7 @@ cases: overlayTargets: "/run/gcs/c/container0/rootfs": true scratch_mounts: [] - query: data.policy.unmount_overlay=x + query: data.policy.unmount_overlay want_result: - x: allowed: true @@ -332,7 +332,7 @@ cases: metadata: devices: "/run/layers/p0-layer0": 1b80f120dbd88e4355d6241b519c3e25290215c469516b49dece9cf07175a766 - query: data.policy.unmount_device=x + query: data.policy.unmount_device want_result: - x: allowed: true @@ -378,7 +378,7 @@ cases: namespace: fragment data: metadata: {} - query: data.policy.load_fragment=x + query: data.policy.load_fragment want_result: - x: add_module: false diff --git a/tests/aci/main.rs b/tests/aci/main.rs index 86d6bd9..8b79b86 100644 --- a/tests/aci/main.rs +++ b/tests/aci/main.rs @@ -1,9 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +use regorus::languages::rego::compiler::Compiler; +use regorus::rvm::tests::test_utils::test_round_trip_serialization; +use regorus::rvm::vm::RegoVM; use regorus::*; +use std::collections::BTreeMap; use std::path::Path; -use std::time::Instant; +use std::time::{Duration, Instant}; use anyhow::Result; use clap::Parser; @@ -18,6 +22,8 @@ struct TestCase { modules: Vec, query: String, want_result: Value, + #[serde(default)] + skip_rvm: bool, } #[derive(Serialize, Deserialize, PartialEq, Debug)] @@ -25,7 +31,46 @@ struct YamlTest { cases: Vec, } -fn eval_test_case(dir: &Path, case: &TestCase) -> Result { +fn format_duration(duration: Duration) -> String { + if duration.as_secs_f64() >= 1.0 { + format!("{:.3}s", duration.as_secs_f64()) + } else if duration.as_millis() >= 1 { + format!("{:.3}ms", duration.as_secs_f64() * 1_000.0) + } else if duration.as_micros() >= 1 { + format!("{:.0}us", duration.as_secs_f64() * 1_000_000.0) + } else { + format!("{}ns", duration.as_nanos()) + } +} + +fn is_binding_array(value: &Value) -> bool { + match value { + Value::Array(entries) => { + if entries.is_empty() { + return true; + } + + let binding_key = Value::from("x"); + entries.iter().all(|entry| match entry { + Value::Object(fields) => fields.contains_key(&binding_key), + _ => false, + }) + } + _ => false, + } +} + +fn align_result_with_expected(result: Value, expected: &Value) -> Value { + if is_binding_array(expected) && !is_binding_array(&result) { + let mut binding = BTreeMap::new(); + binding.insert(Value::from("x"), result); + return Value::from(vec![Value::from(binding)]); + } + + result +} + +fn setup_engine(dir: &Path, case: &TestCase) -> Result { let mut engine = Engine::new(); engine.set_rego_v0(true); @@ -42,29 +87,76 @@ fn eval_test_case(dir: &Path, case: &TestCase) -> Result { } } - let mut engine_full = engine.clone(); - let query_results = engine.eval_query(case.query.clone(), true)?; - - // Ensure that full evaluation produces the same results. - let query_results_full = engine_full.eval_query_and_all_rules(case.query.clone(), true)?; - assert_eq!(query_results, query_results_full); - - let mut values = vec![]; - for qr in query_results.result { - values.push(if !qr.bindings.as_object()?.is_empty() { - qr.bindings.clone() - } else if let Some(v) = qr.expressions.last() { - v.value.clone() - } else { - Value::Undefined - }); - } - let result = Value::from(values); - // Make result json compatible. (E.g: avoid sets). - Value::from_json_str(&result.to_string()) + Ok(engine) } -fn run_aci_tests(dir: &Path) -> Result<()> { +fn eval_test_case_interpreter(dir: &Path, case: &TestCase) -> Result<(Value, Duration)> { + let setup_start = Instant::now(); + let mut engine = setup_engine(dir, case)?; + let setup_duration = setup_start.elapsed(); + + // Use eval_rule instead of eval_query since we're evaluating specific rules + let result = engine.eval_rule(case.query.clone())?; + + // Make result json compatible. (E.g: avoid sets). + let value = Value::from_json_str(&result.to_string())?; + Ok((value, setup_duration)) +} + +fn eval_test_case_rvm( + dir: &Path, + case: &TestCase, +) -> Result<(Value, Duration, Duration, Duration)> { + let setup_start = Instant::now(); + let mut engine = setup_engine(dir, case)?; + + // Convert input and data for RVM + let input = case.input.clone(); + let data = case.data.clone(); + + // Create CompiledPolicy first (needed for RVM compiler) + let rule = Rc::from(case.query.as_str()); + let compiled_policy = engine.compile_with_entrypoint(&rule)?; + let setup_and_compile_duration = setup_start.elapsed(); + + // Use RVM compiler to create a program + let compile_start = Instant::now(); + let program = Compiler::compile_from_policy(&compiled_policy, &[&case.query])?; + + let compile_duration = compile_start.elapsed(); + + // Test round-trip serialization + test_round_trip_serialization(program.as_ref()).map_err(|e| { + anyhow::anyhow!( + "Round-trip serialization test failed for case '{}': {}", + case.note, + e + ) + })?; + + // Create RVM and load the program + let mut vm = RegoVM::new(); + vm.load_program(program); + vm.set_data(data)?; + vm.set_input(input); + + // Execute on RVM + let execution_start = Instant::now(); + let result = vm.execute()?; + let execution_duration = execution_start.elapsed(); + + // Make result json compatible. (E.g: avoid sets). + let value = Value::from_json_str(&result.to_string())?; + + Ok(( + value, + setup_and_compile_duration, + compile_duration, + execution_duration, + )) +} + +fn run_aci_tests(dir: &Path, filter: Option<&str>) -> Result<()> { let mut nfailures = 0; for entry in WalkDir::new(dir) .sort_by_file_name() @@ -81,32 +173,79 @@ fn run_aci_tests(dir: &Path) -> Result<()> { let test: YamlTest = serde_yaml::from_str(&yaml)?; for case in &test.cases { - print!("{:50}", case.note); - let start = Instant::now(); - let results = eval_test_case(dir, case); - let duration = start.elapsed(); - - match results { - Ok(actual) if actual == case.want_result => { - println!("passed {:?}", duration); - } - Ok(actual) => { - println!( - "DIFF {}", - prettydiff::diff_chars( - &serde_yaml::to_string(&case.want_result)?, - &serde_yaml::to_string(&actual)? - ) - ); - - nfailures += 1; - } - Err(e) => { - println!("failed {:?}", duration); - println!("{e}"); - nfailures += 1; + // Apply filter if specified + if let Some(filter_str) = filter { + if !case.note.contains(filter_str) { + continue; } } + + print!("{:50}", case.note); + + // Test with interpreter + let start = Instant::now(); + let (interpreter_raw, interpreter_setup_duration) = + eval_test_case_interpreter(dir, case)?; + let interpreter_results = + align_result_with_expected(interpreter_raw, &case.want_result); + let interpreter_duration = start.elapsed(); + + if interpreter_results != case.want_result { + println!( + "INTERPRETER DIFF {}", + prettydiff::diff_chars( + &serde_yaml::to_string(&case.want_result)?, + &serde_yaml::to_string(&interpreter_results)? + ) + ); + nfailures += 1; + continue; + } + if case.skip_rvm { + println!("skipped rvm"); + continue; + } + + // Test with RVM + let start = Instant::now(); + let (rvm_results, setup_and_compile_duration, rvm_compile_duration, execution_duration) = + eval_test_case_rvm(dir, case)?; + let rvm_results = align_result_with_expected(rvm_results, &case.want_result); + let rvm_duration = start.elapsed(); + + if interpreter_results != rvm_results { + println!("INTERPRETER RESULT:"); + println!("{}", serde_yaml::to_string(&interpreter_results)?); + println!("RVM RESULT:"); + println!("{}", serde_yaml::to_string(&rvm_results)?); + println!( + "INTERPRETER vs RVM DIFF {}", + prettydiff::diff_chars( + &serde_yaml::to_string(&interpreter_results)?, + &serde_yaml::to_string(&rvm_results)? + ) + ); + nfailures += 1; + continue; + } + + println!("ok"); + + let interp_total = format_duration(interpreter_duration); + let interp_setup = format_duration(interpreter_setup_duration); + let rvm_total = format_duration(rvm_duration); + let rvm_prep = format_duration(setup_and_compile_duration); + let rvm_compile = format_duration(rvm_compile_duration); + let rvm_run = format_duration(execution_duration); + + println!( + " Interp total {:>10} (setup {:>10})", + interp_total, interp_setup, + ); + println!( + " RVM total {:>10} (prep {:>10}, compile {:>10}, run {:>10})", + rvm_total, rvm_prep, rvm_compile, rvm_run, + ); } } assert!(nfailures == 0); @@ -158,8 +297,8 @@ fn run_aci_tests_coverage(dir: &Path) -> Result<()> { } } - let report = engine.get_coverage_report()?; - println!("{}", report.to_string_pretty()?); + //let report = engine.get_coverage_report()?; + //println!("{}", report.to_string_pretty()?); Ok(()) } @@ -171,6 +310,10 @@ struct Cli { #[arg(long, short)] #[clap(default_value = "tests/aci")] test_dir: String, + + /// Filter to run only specific test cases (by note field, e.g., "create_container") + #[arg(long, short)] + filter: Option, } fn main() -> Result<()> { @@ -179,5 +322,5 @@ fn main() -> Result<()> { #[cfg(feature = "coverage")] run_aci_tests_coverage(&Path::new(&cli.test_dir))?; - run_aci_tests(&Path::new(&cli.test_dir)) + run_aci_tests(&Path::new(&cli.test_dir), cli.filter.as_deref()) }