mirror of
https://github.com/microsoft/regorus.git
synced 2026-08-05 02:16:11 +00:00
refactor(azure_policy): reduce AliasRegistry allocations via Rc sharing (#725)
Replace the compiler's two cloned BTreeMap fields (alias_map and alias_modifiable) with a single Option<Rc<AliasRegistry>>. This eliminates cloning two 73K-entry maps on every compilation by sharing the registry through a reference-counted pointer. Additional improvements: - alias_map()/alias_modifiable_map() return &BTreeMap (zero-copy) - Safe .get() indexing in ingest_alias_entries and build_object_from_keys - Null-tolerant deserialization for AliasEntry.paths (~97% of real Azure catalog entries emit "paths": null) All changes are within the azure_policy feature gate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
dae3052781
commit
5467cd9e69
@@ -23,8 +23,7 @@ use regorus::languages::azure_policy::aliases::AliasRegistry;
|
||||
use regorus::languages::azure_policy::compiler;
|
||||
use regorus::languages::azure_policy::parser;
|
||||
use regorus::rvm::RegoVM;
|
||||
use regorus::Source;
|
||||
use regorus::Value;
|
||||
use regorus::{Rc, Source, Value};
|
||||
|
||||
/// Evaluate an Azure Policy definition against a resource.
|
||||
///
|
||||
@@ -60,11 +59,8 @@ pub fn azure_policy_eval(
|
||||
println!("Parsed policy definition from {policy_definition}");
|
||||
|
||||
// 3. Compile to RVM bytecode.
|
||||
let program = compiler::compile_policy_definition_with_aliases(
|
||||
&defn,
|
||||
registry.alias_map(),
|
||||
registry.alias_modifiable_map(),
|
||||
)?;
|
||||
let registry = Rc::new(registry);
|
||||
let program = compiler::compile_policy_definition_with_aliases(&defn, Rc::clone(®istry))?;
|
||||
println!("Compiled policy to RVM bytecode");
|
||||
|
||||
// 4. Build normalized input.
|
||||
@@ -138,7 +134,7 @@ pub fn azure_policy_aliases(aliases: String, resource_type: Option<String>) -> R
|
||||
if let Some(ref rt) = resource_type {
|
||||
let rt_lower = rt.to_lowercase();
|
||||
let mut found = false;
|
||||
for (alias_name, _) in registry.alias_map() {
|
||||
for alias_name in registry.alias_map().keys() {
|
||||
if alias_name.to_lowercase().starts_with(&rt_lower) {
|
||||
println!(" {alias_name}");
|
||||
found = true;
|
||||
@@ -148,7 +144,7 @@ pub fn azure_policy_aliases(aliases: String, resource_type: Option<String>) -> R
|
||||
bail!("no aliases found for resource type '{rt}'");
|
||||
}
|
||||
} else {
|
||||
for (alias_name, _) in registry.alias_map() {
|
||||
for alias_name in registry.alias_map().keys() {
|
||||
println!(" {alias_name}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -172,11 +172,31 @@ impl AliasRegistry {
|
||||
let prefix = alloc::format!("{}/", fq_type);
|
||||
|
||||
for alias in aliases {
|
||||
// Skip aliases without a default_path — the normalizer's
|
||||
// resolve_resource_type also skips these, so inserting them into
|
||||
// compiler maps would cause a divergence where the compiler
|
||||
// resolves the alias but normalized input never contains the field.
|
||||
if alias.default_path.is_none() {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Derive the short name by stripping the resource type prefix.
|
||||
let raw_short = if alias.name.len() > prefix.len()
|
||||
&& alias.name[..prefix.len()].eq_ignore_ascii_case(&prefix)
|
||||
&& alias
|
||||
.name
|
||||
.get(..prefix.len())
|
||||
.is_some_and(|s| s.eq_ignore_ascii_case(&prefix))
|
||||
{
|
||||
alias.name[prefix.len()..].to_string()
|
||||
// Both slice boundaries are valid: prefix is ASCII
|
||||
// (resource type + '/'), so if `..prefix.len()` succeeded
|
||||
// above, `prefix.len()..` is guaranteed to be on a char
|
||||
// boundary too. The `unwrap_or` is a defensive fallback
|
||||
// that can never trigger for well-formed Azure alias names.
|
||||
alias
|
||||
.name
|
||||
.get(prefix.len()..)
|
||||
.unwrap_or(&alias.name)
|
||||
.to_string()
|
||||
} else if let Some(rest) = alias
|
||||
.name
|
||||
.rfind('/')
|
||||
@@ -260,20 +280,19 @@ impl AliasRegistry {
|
||||
.map(String::as_str)
|
||||
}
|
||||
|
||||
/// Return a clone of the alias-to-short-name map for use by the compiler.
|
||||
/// Return a reference to the alias-to-short-name map.
|
||||
///
|
||||
/// The compiler stores this map internally so it can resolve fully-qualified
|
||||
/// alias names without holding a reference to the registry.
|
||||
pub fn alias_map(&self) -> BTreeMap<String, String> {
|
||||
self.alias_to_short.clone()
|
||||
/// Keys are lowercase fully-qualified alias names; values are short names.
|
||||
pub const fn alias_map(&self) -> &BTreeMap<String, String> {
|
||||
&self.alias_to_short
|
||||
}
|
||||
|
||||
/// Return a clone of the alias-to-modifiable map for use by the compiler.
|
||||
/// Return a reference to the alias-to-modifiable map.
|
||||
///
|
||||
/// Maps lowercase fully-qualified alias names to `true` when the alias
|
||||
/// has `defaultMetadata.attributes = "Modifiable"`.
|
||||
pub fn alias_modifiable_map(&self) -> BTreeMap<String, bool> {
|
||||
self.alias_modifiable.clone()
|
||||
/// Keys are lowercase fully-qualified alias names; values are `true` when
|
||||
/// the alias has `defaultMetadata.attributes = "Modifiable"`.
|
||||
pub const fn alias_modifiable_map(&self) -> &BTreeMap<String, bool> {
|
||||
&self.alias_modifiable
|
||||
}
|
||||
|
||||
/// Normalize a raw ARM resource and wrap it in the input envelope.
|
||||
|
||||
@@ -20,6 +20,20 @@ use serde::{Deserialize, Deserializer};
|
||||
|
||||
use crate::Rc;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Deserialization helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// Deserialize a `Vec<T>` that tolerates JSON `null` by mapping it to an
|
||||
/// empty vector.
|
||||
fn deserialize_null_as_empty_vec<'de, T, D>(deserializer: D) -> Result<Vec<T>, D::Error>
|
||||
where
|
||||
T: Deserialize<'de>,
|
||||
D: Deserializer<'de>,
|
||||
{
|
||||
Ok(Option::<Vec<T>>::deserialize(deserializer)?.unwrap_or_default())
|
||||
}
|
||||
|
||||
// ─── Top-level response wrappers ────────────────────────────────────────────
|
||||
|
||||
/// ARM API response envelope: `{ "value": [...] }`
|
||||
@@ -100,7 +114,10 @@ pub struct AliasEntry {
|
||||
|
||||
/// Versioned path entries. Empty for the vast majority of aliases that
|
||||
/// have only a `defaultPath`.
|
||||
#[serde(default)]
|
||||
///
|
||||
/// In real Azure catalog data (~97% of aliases), `az provider list` emits
|
||||
/// `"paths": null` rather than an empty array.
|
||||
#[serde(default, deserialize_with = "deserialize_null_as_empty_vec")]
|
||||
pub paths: Vec<AliasPath>,
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ use crate::rvm::program::{Program, SpanInfo};
|
||||
use crate::rvm::Instruction;
|
||||
use crate::{Rc, Value};
|
||||
|
||||
use crate::languages::azure_policy::aliases::AliasRegistry;
|
||||
use crate::languages::azure_policy::ast::PolicyRule;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -44,10 +45,9 @@ pub(super) struct Compiler {
|
||||
pub(super) cached_input_reg: Option<u8>,
|
||||
/// Cached register for `LoadContext` — allocated once on first use.
|
||||
pub(super) cached_context_reg: Option<u8>,
|
||||
/// Map from lowercase fully-qualified alias name → short name.
|
||||
pub(super) alias_map: BTreeMap<String, String>,
|
||||
/// Map from lowercase fully-qualified alias name → modifiable flag.
|
||||
pub(super) alias_modifiable: BTreeMap<String, bool>,
|
||||
/// Alias registry for resolving fully-qualified alias names.
|
||||
/// Shared via `Rc` to avoid cloning the 73K-entry alias maps.
|
||||
pub(super) alias_registry: Option<Rc<AliasRegistry>>,
|
||||
/// Default values for policy parameters.
|
||||
pub(super) parameter_defaults: Option<Value>,
|
||||
/// Cached literal-table index for `parameter_defaults` (or an empty object
|
||||
@@ -338,8 +338,13 @@ impl Compiler {
|
||||
path: &str,
|
||||
span: &crate::lexer::Span,
|
||||
) -> Result<String> {
|
||||
let alias_map = match &self.alias_registry {
|
||||
Some(reg) => reg.alias_map(),
|
||||
None => return Ok(path.to_string()),
|
||||
};
|
||||
|
||||
let lc = path.to_ascii_lowercase();
|
||||
if let Some(short) = self.alias_map.get(&lc) {
|
||||
if let Some(short) = alias_map.get(&lc) {
|
||||
let resolved = short.clone();
|
||||
let result = Self::strip_fq_prefix(&resolved).to_ascii_lowercase();
|
||||
return Ok(result);
|
||||
@@ -348,7 +353,7 @@ impl Compiler {
|
||||
// Fallback: derive array path from a corresponding `[*]` alias.
|
||||
if !lc.contains("[*]") {
|
||||
let wildcard_key = alloc::format!("{}[*]", lc);
|
||||
if let Some(short) = self.alias_map.get(&wildcard_key) {
|
||||
if let Some(short) = alias_map.get(&wildcard_key) {
|
||||
let resolved = Self::strip_fq_prefix(short).to_ascii_lowercase();
|
||||
if let Some(base) = resolved.strip_suffix("[*]") {
|
||||
return Ok(base.to_string());
|
||||
@@ -356,14 +361,14 @@ impl Compiler {
|
||||
}
|
||||
}
|
||||
|
||||
if !self.alias_map.is_empty() && !self.alias_fallback_to_raw {
|
||||
if !alias_map.is_empty() && !self.alias_fallback_to_raw {
|
||||
bail!(span.error(&alloc::format!(
|
||||
"unknown alias '{}': field references must use fully-qualified alias names when an alias catalog is loaded",
|
||||
path
|
||||
)));
|
||||
}
|
||||
|
||||
if self.alias_map.is_empty() {
|
||||
if alias_map.is_empty() {
|
||||
Ok(path.to_string())
|
||||
} else {
|
||||
let result = Self::strip_fq_prefix(path).to_ascii_lowercase();
|
||||
|
||||
@@ -998,7 +998,13 @@ impl Compiler {
|
||||
return Ok(result);
|
||||
}
|
||||
}
|
||||
Err(e) if !self.alias_map.is_empty() && !self.alias_fallback_to_raw => {
|
||||
Err(e)
|
||||
if self
|
||||
.alias_registry
|
||||
.as_ref()
|
||||
.is_some_and(|r| !r.alias_map().is_empty())
|
||||
&& !self.alias_fallback_to_raw =>
|
||||
{
|
||||
return Err(e);
|
||||
}
|
||||
_ => {}
|
||||
|
||||
@@ -692,13 +692,18 @@ impl Compiler {
|
||||
field_path: &str,
|
||||
span: &crate::lexer::Span,
|
||||
) -> Result<()> {
|
||||
if self.alias_modifiable.is_empty() {
|
||||
let modifiable_map = match &self.alias_registry {
|
||||
Some(reg) => reg.alias_modifiable_map(),
|
||||
None => return Ok(()),
|
||||
};
|
||||
|
||||
if modifiable_map.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let lc = field_path.to_lowercase();
|
||||
|
||||
if let Some(&modifiable) = self.alias_modifiable.get(&lc) {
|
||||
if let Some(&modifiable) = modifiable_map.get(&lc) {
|
||||
if !modifiable {
|
||||
bail!(span.error(&format!(
|
||||
"alias '{}' is not modifiable (defaultMetadata.attributes != 'Modifiable')",
|
||||
@@ -803,7 +808,6 @@ fn unescape_arm_literal(s: &str) -> alloc::string::String {
|
||||
/// 1. Build a template `BTreeMap` with `Value::Undefined` placeholders.
|
||||
/// 2. Sort keys by their literal value (BTreeMap order).
|
||||
/// 3. Emit `ObjectCreate`.
|
||||
#[allow(clippy::indexing_slicing)]
|
||||
pub(super) fn build_object_from_keys(
|
||||
compiler: &mut Compiler,
|
||||
mut keys: Vec<(u16, u8)>,
|
||||
@@ -812,17 +816,33 @@ pub(super) fn build_object_from_keys(
|
||||
// Build template: object with all keys set to Undefined.
|
||||
let mut template = BTreeMap::new();
|
||||
for &(key_idx, _) in &keys {
|
||||
// SAFETY: key_idx was just returned by `add_literal_u16`, so the
|
||||
// index is guaranteed to be in bounds.
|
||||
let key_val = compiler.program.literals[usize::from(key_idx)].clone();
|
||||
// key_idx was returned by `add_literal_u16` in the calling code,
|
||||
// so it is always in bounds. We use `.get()` + `?` instead of
|
||||
// direct indexing to satisfy the crate-wide `deny(indexing_slicing)`.
|
||||
let key_val = compiler
|
||||
.program
|
||||
.literals
|
||||
.get(usize::from(key_idx))
|
||||
.ok_or_else(|| {
|
||||
anyhow!(
|
||||
"internal error in build_object_from_keys: \
|
||||
literal index {} out of bounds (literals len = {})",
|
||||
key_idx,
|
||||
compiler.program.literals.len()
|
||||
)
|
||||
})?
|
||||
.clone();
|
||||
template.insert(key_val, Value::Undefined);
|
||||
}
|
||||
let template_idx = compiler.add_literal_u16(Value::Object(crate::Rc::new(template)))?;
|
||||
|
||||
// Sort keys by literal value (BTreeMap order).
|
||||
// Sort keys by literal value (BTreeMap order). All indices were
|
||||
// validated in the loop above (which returns Err for out-of-bounds),
|
||||
// so `.get()` always returns `Some` here — `None` is unreachable.
|
||||
keys.sort_by(|a, b| {
|
||||
compiler.program.literals[usize::from(a.0)]
|
||||
.cmp(&compiler.program.literals[usize::from(b.0)])
|
||||
let a_val = compiler.program.literals.get(usize::from(a.0));
|
||||
let b_val = compiler.program.literals.get(usize::from(b.0));
|
||||
a_val.cmp(&b_val)
|
||||
});
|
||||
|
||||
let dest = compiler.alloc_register()?;
|
||||
|
||||
@@ -30,11 +30,11 @@ mod metadata;
|
||||
mod template_dispatch;
|
||||
mod utils;
|
||||
|
||||
use alloc::collections::BTreeMap;
|
||||
use alloc::string::{String, ToString as _};
|
||||
use alloc::string::ToString as _;
|
||||
|
||||
use anyhow::Result;
|
||||
|
||||
use crate::languages::azure_policy::aliases::AliasRegistry;
|
||||
use crate::languages::azure_policy::ast::{PolicyDefinition, PolicyRule};
|
||||
use crate::rvm::program::Program;
|
||||
use crate::{Rc, Value};
|
||||
@@ -69,17 +69,14 @@ pub fn compile_policy_rule(rule: &PolicyRule) -> Result<Rc<Program>> {
|
||||
|
||||
/// Compile a parsed Azure Policy rule with alias resolution.
|
||||
///
|
||||
/// The `alias_map` maps lowercase fully-qualified alias names to their short
|
||||
/// names. Obtain it from
|
||||
/// [`AliasRegistry::alias_map()`](crate::languages::azure_policy::aliases::AliasRegistry::alias_map).
|
||||
/// The registry provides alias-to-short-name resolution and modifiability
|
||||
/// data. Pass it as an `Rc` to avoid cloning the internal alias maps.
|
||||
pub fn compile_policy_rule_with_aliases(
|
||||
rule: &PolicyRule,
|
||||
alias_map: BTreeMap<String, String>,
|
||||
alias_modifiable: BTreeMap<String, bool>,
|
||||
registry: Rc<AliasRegistry>,
|
||||
) -> Result<Rc<Program>> {
|
||||
let mut compiler = Compiler::new();
|
||||
compiler.alias_map = alias_map;
|
||||
compiler.alias_modifiable = alias_modifiable;
|
||||
compiler.alias_registry = Some(registry);
|
||||
init_effect_annotation(&mut compiler, rule);
|
||||
compiler.compile(rule)
|
||||
}
|
||||
@@ -100,12 +97,10 @@ pub fn compile_policy_definition(defn: &PolicyDefinition) -> Result<Rc<Program>>
|
||||
/// Compile a parsed Azure Policy definition with alias resolution.
|
||||
pub fn compile_policy_definition_with_aliases(
|
||||
defn: &PolicyDefinition,
|
||||
alias_map: BTreeMap<String, String>,
|
||||
alias_modifiable: BTreeMap<String, bool>,
|
||||
registry: Rc<AliasRegistry>,
|
||||
) -> Result<Rc<Program>> {
|
||||
let mut compiler = Compiler::new();
|
||||
compiler.alias_map = alias_map;
|
||||
compiler.alias_modifiable = alias_modifiable;
|
||||
compiler.alias_registry = Some(registry);
|
||||
compiler.parameter_defaults = Some(build_parameter_defaults(&defn.parameters)?);
|
||||
compiler.populate_definition_metadata(defn);
|
||||
init_effect_annotation(&mut compiler, &defn.policy_rule);
|
||||
@@ -119,13 +114,11 @@ pub fn compile_policy_definition_with_aliases(
|
||||
/// a known alias are silently treated as raw property paths.
|
||||
pub fn compile_policy_definition_with_aliases_opts(
|
||||
defn: &PolicyDefinition,
|
||||
alias_map: BTreeMap<String, String>,
|
||||
alias_modifiable: BTreeMap<String, bool>,
|
||||
registry: Rc<AliasRegistry>,
|
||||
alias_fallback_to_raw: bool,
|
||||
) -> Result<Rc<Program>> {
|
||||
let mut compiler = Compiler::new();
|
||||
compiler.alias_map = alias_map;
|
||||
compiler.alias_modifiable = alias_modifiable;
|
||||
compiler.alias_registry = Some(registry);
|
||||
compiler.alias_fallback_to_raw = alias_fallback_to_raw;
|
||||
compiler.parameter_defaults = Some(build_parameter_defaults(&defn.parameters)?);
|
||||
compiler.populate_definition_metadata(defn);
|
||||
|
||||
@@ -23,8 +23,7 @@ use regorus::languages::azure_policy::aliases::AliasRegistry;
|
||||
use regorus::languages::azure_policy::compiler;
|
||||
use regorus::languages::azure_policy::parser;
|
||||
use regorus::rvm::RegoVM;
|
||||
use regorus::Source;
|
||||
use regorus::Value;
|
||||
use regorus::{Rc, Source, Value};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::collections::BTreeMap;
|
||||
use std::fs;
|
||||
@@ -194,7 +193,7 @@ fn yaml_test_impl(file: &str) -> Result<()> {
|
||||
let test: YamlTest = serde_yaml::from_str(&yaml_str)?;
|
||||
|
||||
// Load alias registry if an aliases file is specified.
|
||||
let alias_registry = if let Some(ref aliases_file) = test.aliases {
|
||||
let alias_registry: Option<Rc<AliasRegistry>> = if let Some(ref aliases_file) = test.aliases {
|
||||
let aliases_dir = Path::new(file)
|
||||
.parent()
|
||||
.unwrap_or_else(|| Path::new("."))
|
||||
@@ -209,7 +208,7 @@ fn yaml_test_impl(file: &str) -> Result<()> {
|
||||
})?;
|
||||
let mut registry = AliasRegistry::new();
|
||||
registry.load_from_json(&aliases_json)?;
|
||||
Some(registry)
|
||||
Some(Rc::new(registry))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -281,11 +280,7 @@ fn yaml_test_impl(file: &str) -> Result<()> {
|
||||
);
|
||||
}
|
||||
if let Some(ref registry) = alias_registry {
|
||||
compiler::compile_policy_definition_with_aliases(
|
||||
&defn,
|
||||
registry.alias_map(),
|
||||
registry.alias_modifiable_map(),
|
||||
)
|
||||
compiler::compile_policy_definition_with_aliases(&defn, Rc::clone(registry))
|
||||
} else {
|
||||
compiler::compile_policy_definition(&defn)
|
||||
}
|
||||
@@ -308,11 +303,7 @@ fn yaml_test_impl(file: &str) -> Result<()> {
|
||||
);
|
||||
}
|
||||
if let Some(ref registry) = alias_registry {
|
||||
compiler::compile_policy_rule_with_aliases(
|
||||
&ast,
|
||||
registry.alias_map(),
|
||||
registry.alias_modifiable_map(),
|
||||
)
|
||||
compiler::compile_policy_rule_with_aliases(&ast, Rc::clone(registry))
|
||||
} else {
|
||||
compiler::compile_policy_rule(&ast)
|
||||
}
|
||||
@@ -366,7 +357,7 @@ fn yaml_test_impl(file: &str) -> Result<()> {
|
||||
|
||||
let mut vm = RegoVM::new();
|
||||
vm.load_program(program);
|
||||
vm.set_input(make_input(case, alias_registry.as_ref())?);
|
||||
vm.set_input(make_input(case, alias_registry.as_deref())?);
|
||||
vm.set_context(make_context(case)?);
|
||||
|
||||
// Load host-await responses (for auditIfNotExists / deployIfNotExists policies).
|
||||
@@ -391,7 +382,11 @@ fn yaml_test_impl(file: &str) -> Result<()> {
|
||||
if let Some(rt) = effective_type {
|
||||
inject_type_field(&mut raw, rt);
|
||||
}
|
||||
normalizer::normalize(&raw, Some(registry), case.api_version.as_deref())
|
||||
normalizer::normalize(
|
||||
&raw,
|
||||
Some(registry.as_ref()),
|
||||
case.api_version.as_deref(),
|
||||
)
|
||||
} else {
|
||||
raw
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user