diff --git a/.github/workflows/build-artifacts.yml b/.github/workflows/build-artifacts.yml index cbf7c372..d2ae89eb 100644 --- a/.github/workflows/build-artifacts.yml +++ b/.github/workflows/build-artifacts.yml @@ -4,27 +4,6 @@ run-name: Build Artifacts ${{ github.actor }} ${{ github.event_name }} (${{ inpu on: push: branches: [ main ] - paths-ignore: - - '.github/**' - - 'scripts/**' - - '.gitattributes' - - '.gitignore' - - '**/*.md' - - '**/*.txt' - - '**/*.jar' - - 'LICENSE' - - 'NOTICE' - - 'gradlew' - - 'gradlew.bat' - - 'src/bindings-*/**/bench/**' - - 'src/bindings-*/examples/**' - - 'src/bindings-*/tests/**' - - 'src/bindings-jvm/version.properties' - - 'src/bindings-wasm/dist/*' - - 'src/bindings-python/generated/**' - - 'src/bindings-go/go/internal/**' - - 'src/bindings-go/go/libs/**' - - 'release-bin/*' workflow_dispatch: inputs: branch: @@ -41,10 +20,25 @@ permissions: contents: read jobs: + relevant-changes: + uses: ./.github/workflows/relevant-source-changes.yml + permissions: + contents: read + pull-requests: read + + build-required: + needs: [ relevant-changes ] + if: ${{ needs.relevant-changes.outputs.relevant == 'true' }} + runs-on: ubuntu-latest + steps: + - run: echo "Artifact build required" + get-configs: + needs: [ build-required ] uses: ./.github/workflows/configs.yml resolve-ref: + needs: [ build-required ] runs-on: ubuntu-latest outputs: sha: ${{ steps.head.outputs.sha }} diff --git a/.github/workflows/performance-regression.yml b/.github/workflows/performance-regression.yml new file mode 100644 index 00000000..a1aebb9c --- /dev/null +++ b/.github/workflows/performance-regression.yml @@ -0,0 +1,91 @@ +name: Performance Regression + +on: + workflow_call: + inputs: + base_ref: + description: Git ref to use as the performance baseline + required: false + default: origin/main + type: string + head_ref: + description: Git ref to test + required: false + default: '' + type: string + workflow_dispatch: + inputs: + base_ref: + description: Git ref to use as the performance baseline + required: false + default: origin/main + type: string + head_ref: + description: Git ref to test + required: false + default: '' + type: string + +permissions: + contents: read + +concurrency: + group: performance-regression-${{ github.ref }} + cancel-in-progress: true + +jobs: + get-configs: + uses: ./.github/workflows/configs.yml + + compare: + name: Compare base and head + needs: [get-configs] + runs-on: ubuntu-latest + timeout-minutes: 35 + env: + BASE_REF: ${{ inputs.base_ref || 'origin/main' }} + HEAD_REF: ${{ inputs.head_ref || github.sha }} + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Install Rust ${{ needs.get-configs.outputs.rust-toolchain }} + uses: dtolnay/rust-toolchain@4360b52568e2003a75bf9bc1d59f33a8e3fc893c + with: + toolchain: ${{ needs.get-configs.outputs.rust-toolchain }} + + - name: Setup Python ${{ needs.get-configs.outputs.python-version }} + uses: actions/setup-python@v6 + with: + python-version: ${{ needs.get-configs.outputs.python-version }} + + - name: Test comparison logic + run: python3 scripts/check_performance_regression.py --self-test + + - name: Compare performance + run: | + set -euo pipefail + python3 scripts/check_performance_regression.py \ + --base-ref "$BASE_REF" \ + --head-ref "$HEAD_REF" \ + --output-dir "$RUNNER_TEMP/performance-regression" + + - name: Add comparison to job summary + if: always() + run: | + if [[ -f "$RUNNER_TEMP/performance-regression/performance-comparison.md" ]]; then + cat "$RUNNER_TEMP/performance-regression/performance-comparison.md" >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Upload performance evidence + if: always() + uses: actions/upload-artifact@v7 + with: + name: performance-regression-${{ github.event.pull_request.number || github.run_id }} + path: | + ${{ runner.temp }}/performance-regression/performance-comparison.json + ${{ runner.temp }}/performance-regression/performance-comparison.md + if-no-files-found: warn + retention-days: 7 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index ea1bdc6a..d7b9e737 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -13,6 +13,12 @@ concurrency: cancel-in-progress: true jobs: + relevant-changes: + uses: ./.github/workflows/relevant-source-changes.yml + permissions: + contents: read + pull-requests: read + validate: uses: ./.github/workflows/validate.yml permissions: @@ -21,3 +27,13 @@ jobs: with: ref: ${{ github.sha }} release-run: false + + performance-regression: + needs: [relevant-changes] + if: ${{ needs.relevant-changes.outputs.relevant == 'true' }} + uses: ./.github/workflows/performance-regression.yml + permissions: + contents: read + with: + base_ref: ${{ github.event.pull_request.base.sha }} + head_ref: ${{ github.event.pull_request.head.sha }} diff --git a/.github/workflows/relevant-source-changes.yml b/.github/workflows/relevant-source-changes.yml new file mode 100644 index 00000000..3bb9080d --- /dev/null +++ b/.github/workflows/relevant-source-changes.yml @@ -0,0 +1,64 @@ +name: Relevant Source Changes + +on: + workflow_call: + outputs: + relevant: + description: Whether at least one changed path requires artifact or performance validation + value: ${{ jobs.changes.outputs.relevant }} + +permissions: + contents: read + pull-requests: read + +jobs: + changes: + runs-on: ubuntu-latest + outputs: + relevant: ${{ steps.decision.outputs.relevant }} + steps: + - name: Checkout push history + if: ${{ github.event_name == 'push' }} + uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Detect relevant paths + if: ${{ github.event_name != 'workflow_dispatch' }} + id: filter + uses: dorny/paths-filter@v4.0.3 + with: + filters: | + relevant: + - '**' + - '!.github/**' + - '!scripts/**' + - '!.gitattributes' + - '!.gitignore' + - '!**/*.md' + - '!**/*.txt' + - '!**/*.jar' + - '!LICENSE' + - '!NOTICE' + - '!gradlew' + - '!gradlew.bat' + - '!src/bindings-*/**/bench/**' + - '!src/bindings-*/examples/**' + - '!src/bindings-*/tests/**' + - '!src/bindings-jvm/version.properties' + - '!src/bindings-wasm/dist/*' + - '!src/bindings-python/generated/**' + - '!src/bindings-go/go/internal/**' + - '!src/bindings-go/go/libs/**' + - '!release-bin/*' + + - name: Export decision + id: decision + env: + FILTER_RELEVANT: ${{ steps.filter.outputs.relevant }} + run: | + if [[ "$GITHUB_EVENT_NAME" == "workflow_dispatch" || "$FILTER_RELEVANT" == "true" ]]; then + echo "relevant=true" >> "$GITHUB_OUTPUT" + else + echo "relevant=false" >> "$GITHUB_OUTPUT" + fi diff --git a/scripts/check_performance_regression.py b/scripts/check_performance_regression.py new file mode 100644 index 00000000..ef073d0d --- /dev/null +++ b/scripts/check_performance_regression.py @@ -0,0 +1,716 @@ +#!/usr/bin/env python3 +"""Detect meaningful PR performance regressions with paired base/head runs. + +The gate builds one identical Rust harness against each revision, alternates run +order on one runner, and confirms apparent regressions before failing. Absolute +milliseconds are never compared across machines. +""" + +from __future__ import annotations + +import argparse +import json +import math +import os +import platform +import re +import shutil +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path +from statistics import median +from typing import Any, Callable + +SCRIPT_DIR = Path(__file__).resolve().parent +PROJECT_ROOT = SCRIPT_DIR.parent +HARNESS_SOURCE = PROJECT_ROOT / "src" / "performance-harness" +DEFAULT_OUTPUT_DIR = PROJECT_ROOT / "tmp" / "performance-regression" + + +@dataclass(frozen=True) +class Workload: + name: str + templates: tuple[Path, ...] + iterations: int + gate_process_lifecycle: bool = True + + +@dataclass(frozen=True) +class MetricGate: + name: str + accessor: Callable[[dict[str, Any]], float] + threshold: float + floor: float + minimum_delta: float + + +@dataclass(frozen=True) +class MetricEvaluation: + metric: str + ratio: float + median_delta: float + slower_pairs: int + pair_count: int + regression: bool + enforced: bool = True + + +METRIC_GATES = ( + MetricGate("initialization", lambda sample: sample["initTotalMs"], math.inf, 1.0, math.inf), + MetricGate( + "init + first", + lambda sample: sample["initTotalMs"] + sample["firstValidation"]["wallMs"], + 1.10, + 1.0, + 5.0, + ), + MetricGate("first validation", lambda sample: sample["firstValidation"]["wallMs"], 1.12, 0.5, 5.0), + MetricGate("warm total", lambda sample: sample["warm"]["perCallTotalMs"], 1.10, 0.2, 1.0), + MetricGate("warm model", lambda sample: sample["warm"]["modelMedianMs"], 1.15, 0.2, 1.0), + MetricGate("warm schema", lambda sample: sample["warm"]["schemaMedianMs"], 1.15, 0.2, 1.0), + MetricGate("warm rules", lambda sample: sample["warm"]["ruleMedianMs"], 1.15, 0.2, 1.0), + MetricGate("peak RSS", lambda sample: sample["peakRssBytes"], 1.15, 1024.0, 16 * 1024 * 1024), +) +AGGREGATE_WARM_THRESHOLD = 1.07 +CONSISTENT_SLOWER_FRACTION = 2 / 3 + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base-ref", default="origin/main", help="Git ref for the comparison baseline") + parser.add_argument("--head-ref", default="HEAD", help="Git ref for the candidate revision") + parser.add_argument("--output-dir", type=Path, default=DEFAULT_OUTPUT_DIR) + parser.add_argument("--initial-pairs", type=int, default=3) + parser.add_argument("--confirmation-pairs", type=int, default=3) + parser.add_argument("--quick", action="store_true", help="Use one pair and reduced iterations for local smoke tests") + parser.add_argument("--keep-worktrees", action="store_true") + parser.add_argument("--self-test", action="store_true", help="Test comparison logic without building Rust") + arguments = parser.parse_args() + if arguments.initial_pairs < 1 or arguments.confirmation_pairs < 1: + parser.error("pair counts must be positive") + return arguments + + +def run_command( + command: list[str], + *, + cwd: Path, + environment: dict[str, str] | None = None, + capture: bool = False, +) -> subprocess.CompletedProcess[str]: + print(f" $ {' '.join(command)}", file=sys.stderr) + completed = subprocess.run( + command, + cwd=cwd, + env=environment, + text=True, + stdout=subprocess.PIPE if capture else None, + stderr=subprocess.PIPE if capture else None, + ) + if completed.returncode != 0: + if capture: + print(completed.stdout, file=sys.stderr) + print(completed.stderr, file=sys.stderr) + raise RuntimeError(f"command failed with exit {completed.returncode}: {' '.join(command)}") + return completed + + +def resolve_ref(reference: str) -> str: + completed = run_command(["git", "rev-parse", reference], cwd=PROJECT_ROOT, capture=True) + return completed.stdout.strip() + + +def remove_worktree(path: Path) -> None: + if not path.exists(): + return + subprocess.run( + ["git", "worktree", "remove", "--force", str(path)], + cwd=PROJECT_ROOT, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + if path.exists(): + shutil.rmtree(path) + + +def prepare_worktree(reference: str, destination: Path, target_dir: Path) -> Path: + remove_worktree(destination) + run_command(["git", "worktree", "add", "--detach", str(destination), reference], cwd=PROJECT_ROOT) + + workspace_dir = destination / "src" + run_command(["cargo", "fetch", "--locked"], cwd=workspace_dir) + harness_destination = workspace_dir / "performance-harness" + if harness_destination.exists(): + shutil.rmtree(harness_destination) + shutil.copytree(HARNESS_SOURCE, harness_destination) + + manifest_path = workspace_dir / "Cargo.toml" + manifest = manifest_path.read_text() + if '"performance-harness"' not in manifest: + marker = ' "validation-engine",\n]' + if marker not in manifest: + raise RuntimeError(f"workspace member marker not found in {manifest_path}") + manifest = manifest.replace(marker, ' "validation-engine",\n "performance-harness",\n]', 1) + manifest_path.write_text(manifest) + + build_environment = os.environ.copy() + build_environment["CARGO_TARGET_DIR"] = str(target_dir) + run_command( + ["cargo", "build", "--release", "--offline", "-p", "performance-harness"], + cwd=workspace_dir, + environment=build_environment, + ) + binary_name = "performance-harness.exe" if platform.system() == "Windows" else "performance-harness" + binary = target_dir / "release" / binary_name + if not binary.is_file(): + raise RuntimeError(f"benchmark binary was not produced: {binary}") + return binary + + +def generate_fixtures(directory: Path) -> dict[str, Path]: + directory.mkdir(parents=True, exist_ok=True) + tiny = directory / "tiny.yaml" + tiny.write_text( + "AWSTemplateFormatVersion: '2010-09-09'\n" + "Resources:\n" + " Bucket:\n" + " Type: AWS::S3::Bucket\n" + " Properties:\n" + " BucketName: performance-regression-bucket\n" + ) + + def write_buckets(path: Path, count: int, duplicate: bool) -> None: + lines = ["AWSTemplateFormatVersion: '2010-09-09'", "Resources:"] + for index in range(count): + bucket_name = "shared-performance-id" if duplicate else f"unique-performance-id-{index}" + lines.extend( + [ + f" Bucket{index}:", + " Type: AWS::S3::Bucket", + " Properties:", + f" BucketName: {bucket_name}", + ] + ) + path.write_text("\n".join(lines) + "\n") + + unique = directory / "unique-500.yaml" + duplicate = directory / "duplicate-500.yaml" + write_buckets(unique, 500, False) + write_buckets(duplicate, 500, True) + + conditional = directory / "conditional-100.yaml" + conditional_lines = [ + "AWSTemplateFormatVersion: '2010-09-09'", + "Parameters:", + " Environment:", + " Type: String", + " AllowedValues: [a, b]", + "Conditions:", + " IsA: !Equals [!Ref Environment, a]", + " IsB: !Equals [!Ref Environment, b]", + "Resources:", + ] + for index in range(100): + condition = "IsA" if index % 2 == 0 else "IsB" + conditional_lines.extend( + [ + f" Bucket{index}:", + " Type: AWS::S3::Bucket", + f" Condition: {condition}", + " Properties:", + " BucketName: shared-conditional-id", + ] + ) + conditional.write_text("\n".join(conditional_lines) + "\n") + return {"tiny": tiny, "unique": unique, "duplicate": duplicate, "conditional": conditional} + + +def security_workloads(directory: Path, iteration_divisor: int) -> tuple[Workload, ...]: + iteration_counts = { + "condition_fusion.yaml": 2, + "cross_reference_fanout.yaml": 1, + "cross_resource_scale.yaml": 3, + "deep_intrinsic_resolution.yaml": 2, + "deep_nesting.json": 1, + "deep_yaml_nesting.yaml": 1, + "many_resources.yaml": 5, + "pathological_conditions.yaml": 3, + "scenario_assignment_budget.yaml": 2, + } + templates = sorted( + path + for path in directory.rglob("*") + if path.is_file() and path.suffix.lower() in {".json", ".yaml", ".yml"} + ) + workloads = [] + for template in templates: + relative = template.relative_to(directory).with_suffix("") + label = "-".join(relative.parts).replace("_", "-") + normal_iterations = iteration_counts.get(template.name, 2) + workloads.append( + Workload( + f"security-{label}", + (template,), + max(1, normal_iterations // iteration_divisor), + gate_process_lifecycle=template.name not in {"deep_nesting.json", "deep_yaml_nesting.yaml"}, + ) + ) + return tuple(workloads) + + +def workload_matrix(fixtures: dict[str, Path], quick: bool) -> tuple[Workload, ...]: + divisor = 5 if quick else 1 + + def iterations(normal: int) -> int: + return max(1, normal // divisor) + + templates = PROJECT_ROOT / "src" / "resources" / "templates" + security = PROJECT_ROOT / "src" / "resources" / "security" + return ( + Workload("tiny", (fixtures["tiny"],), iterations(151)), + Workload("unique-500", (fixtures["unique"],), iterations(9)), + Workload("duplicate-500", (fixtures["duplicate"],), iterations(7)), + Workload("conditional-100", (fixtures["conditional"],), iterations(15)), + Workload( + "mixed-real", + ( + templates / "cdk" / "codepipeline-build-deploy--CodepipelineBuildDeployStack.template.json", + templates / "quickstart" / "vpc.json", + ), + iterations(7), + ), + *security_workloads(security, divisor), + ) + + +def time_prefix() -> list[str]: + time_binary = Path("/usr/bin/time") + if not time_binary.exists(): + raise RuntimeError("/usr/bin/time is required for peak RSS measurement") + return [str(time_binary), "-v"] if platform.system() == "Linux" else [str(time_binary), "-l"] + + +def cpu_pin_prefix() -> list[str]: + taskset = shutil.which("taskset") + if taskset is None or not hasattr(os, "sched_getaffinity"): + return [] + allowed_cpus = os.sched_getaffinity(0) + return [taskset, "-c", str(min(allowed_cpus))] if allowed_cpus else [] + + +def parse_peak_rss(stderr: str) -> int: + linux_match = re.search(r"Maximum resident set size \(kbytes\):\s*(\d+)", stderr) + if linux_match: + return int(linux_match.group(1)) * 1024 + darwin_match = re.search(r"(\d+)\s+maximum resident set size", stderr) + if darwin_match: + return int(darwin_match.group(1)) + raise RuntimeError("maximum resident set size was not present in /usr/bin/time output") + + +def run_measurement( + binary: Path, + engine: str, + workload: Workload, + variant: str, + pair_index: int, +) -> dict[str, Any]: + command = [ + *time_prefix(), + *cpu_pin_prefix(), + str(binary), + engine, + str(workload.iterations), + "2", + workload.name, + *(str(path) for path in workload.templates), + ] + completed = run_command(command, cwd=PROJECT_ROOT, capture=True) + output_lines = [line for line in completed.stdout.splitlines() if line.strip()] + if not output_lines: + raise RuntimeError(f"benchmark emitted no JSON for {engine}/{workload.name}/{variant}") + measurement = json.loads(output_lines[-1]) + measurement["peakRssBytes"] = parse_peak_rss(completed.stderr) + measurement["variant"] = variant + measurement["pair"] = pair_index + measurement["gateProcessLifecycle"] = workload.gate_process_lifecycle + return measurement + + +def diagnostic_signature(measurement: dict[str, Any]) -> tuple[Any, ...]: + fingerprints = tuple( + (Path(item["path"]).name, item["fingerprint"], item["diagnostics"], item["status"]) + for item in measurement["fingerprints"] + ) + return (measurement["firstValidation"]["fingerprint"], fingerprints) + + +def metric_evaluation( + pairs: list[dict[str, dict[str, Any]]], + gate: MetricGate, +) -> MetricEvaluation: + ratios = [] + deltas = [] + for pair in pairs: + baseline = gate.accessor(pair["base"]) + candidate = gate.accessor(pair["head"]) + if baseline >= gate.floor: + ratios.append(candidate / baseline) + deltas.append(candidate - baseline) + if not ratios: + return MetricEvaluation(gate.name, 1.0, 0.0, 0, 0, False) + ratio = math.exp(sum(math.log(value) for value in ratios) / len(ratios)) + median_delta = median(deltas) + slower_pairs = sum(value > 1.0 for value in ratios) + required_slower = math.ceil(len(ratios) * CONSISTENT_SLOWER_FRACTION) + regression = ( + ratio > gate.threshold + and median_delta > gate.minimum_delta + and slower_pairs >= required_slower + ) + return MetricEvaluation(gate.name, ratio, median_delta, slower_pairs, len(ratios), regression) + + +def evaluate_cases( + measurements: dict[tuple[str, str], list[dict[str, dict[str, Any]]]], +) -> tuple[dict[tuple[str, str], list[MetricEvaluation]], list[str], bool]: + evaluations: dict[tuple[str, str], list[MetricEvaluation]] = {} + failures: list[str] = [] + diagnostic_mismatch = False + for case, pairs in sorted(measurements.items()): + engine, workload = case + for pair in pairs: + if diagnostic_signature(pair["base"]) != diagnostic_signature(pair["head"]): + diagnostic_mismatch = True + failures.append(f"{engine}/{workload}: diagnostics differ between base and head") + break + gate_process_lifecycle = all( + pair["base"].get("gateProcessLifecycle", True) + and pair["head"].get("gateProcessLifecycle", True) + for pair in pairs + ) + case_evaluations = [] + for gate in METRIC_GATES: + evaluation = metric_evaluation(pairs, gate) + if not gate_process_lifecycle and gate.name in {"init + first", "peak RSS"}: + evaluation = MetricEvaluation( + evaluation.metric, + evaluation.ratio, + evaluation.median_delta, + evaluation.slower_pairs, + evaluation.pair_count, + False, + False, + ) + case_evaluations.append(evaluation) + evaluations[case] = case_evaluations + for evaluation in case_evaluations: + if evaluation.regression: + failures.append( + f"{engine}/{workload}: {evaluation.metric} regressed by " + f"{(evaluation.ratio - 1) * 100:.1f}% " + f"(median delta {evaluation.median_delta:.3f}; " + f"{evaluation.slower_pairs}/{evaluation.pair_count} paired runs slower)" + ) + + warm_ratios = [] + for pairs in measurements.values(): + for pair in pairs: + baseline = pair["base"]["warm"]["perCallTotalMs"] + candidate = pair["head"]["warm"]["perCallTotalMs"] + if baseline >= 0.2: + warm_ratios.append(candidate / baseline) + aggregate_regression = False + if warm_ratios: + aggregate_ratio = math.exp(sum(math.log(value) for value in warm_ratios) / len(warm_ratios)) + slower_pairs = sum(value > 1 for value in warm_ratios) + required_slower = math.ceil(len(warm_ratios) * CONSISTENT_SLOWER_FRACTION) + aggregate_regression = aggregate_ratio > AGGREGATE_WARM_THRESHOLD and slower_pairs >= required_slower + if aggregate_regression: + failures.append( + f"aggregate warm validation regressed by {(aggregate_ratio - 1) * 100:.1f}% " + f"({slower_pairs}/{len(warm_ratios)} paired runs slower)" + ) + return evaluations, failures, diagnostic_mismatch + + +def collect_pairs( + base_binary: Path, + head_binary: Path, + workloads: tuple[Workload, ...], + engines: tuple[str, ...], + pair_start: int, + pair_count: int, + selected_cases: set[tuple[str, str]] | None, + measurements: dict[tuple[str, str], list[dict[str, dict[str, Any]]]], +) -> None: + for engine in engines: + for workload_index, workload in enumerate(workloads): + case = (engine, workload.name) + if selected_cases is not None and case not in selected_cases: + continue + case_pairs = measurements.setdefault(case, []) + for pair_index in range(pair_start, pair_start + pair_count): + base_first = (pair_index + workload_index + (0 if engine == "rego" else 1)) % 2 == 0 + order = (("base", base_binary), ("head", head_binary)) + if not base_first: + order = tuple(reversed(order)) + paired_measurement: dict[str, dict[str, Any]] = {} + for variant, binary in order: + paired_measurement[variant] = run_measurement(binary, engine, workload, variant, pair_index) + case_pairs.append(paired_measurement) + + +def render_markdown( + evaluations: dict[tuple[str, str], list[MetricEvaluation]], + failures: list[str], + base_sha: str, + head_sha: str, +) -> str: + lines = [ + "# PR performance regression check", + "", + f"Base: `{base_sha[:12]}` ", + f"Head: `{head_sha[:12]}`", + "", + "Ratios are paired head/base geometric means. Values below 1.0 are faster/smaller.", + "Ratios marked `(info)` are reported but not gated for parser-only robustness fixtures.", + "", + "| Engine | Workload | Init | Init + first | First validate | Warm total | Model | Schema | Rules | Peak RSS | Status |", + "|---|---|---:|---:|---:|---:|---:|---:|---:|---:|---|", + ] + metric_order = [ + "initialization", + "init + first", + "first validation", + "warm total", + "warm model", + "warm schema", + "warm rules", + "peak RSS", + ] + for (engine, workload), case_evaluations in sorted(evaluations.items()): + by_name = {evaluation.metric: evaluation for evaluation in case_evaluations} + values = [by_name[name] for name in metric_order] + status = "FAIL" if any(value.regression for value in values) else "pass" + ratios = [f"{value.ratio:.3f}×" + (" (info)" if not value.enforced else "") for value in values] + lines.append(f"| {engine} | {workload} | {' | '.join(ratios)} | {status} |") + lines.extend(["", "## Result", ""]) + if failures: + lines.extend([f"* ❌ {failure}" for failure in failures]) + else: + lines.append("✅ No meaningful performance or diagnostic regression detected.") + return "\n".join(lines) + "\n" + + +def run_self_test() -> None: + gate = MetricGate("test", lambda sample: sample["value"], 1.10, 0.1, 1.0) + + def pairs(ratios: list[float]) -> list[dict[str, dict[str, float]]]: + return [{"base": {"value": 10.0}, "head": {"value": 10.0 * ratio}} for ratio in ratios] + + assert not metric_evaluation(pairs([1.01, 0.99, 1.02]), gate).regression + assert not metric_evaluation(pairs([1.0, 1.0, 1.30]), gate).regression + assert metric_evaluation(pairs([1.15, 1.14, 1.16]), gate).regression + tiny_delta_pairs = [{"base": {"value": 1.0}, "head": {"value": 1.2}} for _ in range(3)] + assert not metric_evaluation(tiny_delta_pairs, gate).regression + assert not metric_evaluation([{"base": {"value": 0.01}, "head": {"value": 1.0}}], gate).regression + + memory_gate = MetricGate("memory", lambda sample: sample["value"], 1.15, 1.0, 1.0) + assert metric_evaluation(pairs([1.20, 1.19, 1.21]), memory_gate).regression + + def synthetic_sample(multiplier: float, fingerprint_value: str = "same") -> dict[str, Any]: + return { + "initTotalMs": 10.0 * multiplier, + "firstValidation": {"wallMs": 10.0 * multiplier, "fingerprint": fingerprint_value}, + "warm": { + "perCallTotalMs": 10.0 * multiplier, + "modelMedianMs": 2.0 * multiplier, + "schemaMedianMs": 2.0 * multiplier, + "ruleMedianMs": 5.0 * multiplier, + }, + "peakRssBytes": 100_000_000 * multiplier, + "fingerprints": [ + {"path": "fixture.yaml", "fingerprint": fingerprint_value, "diagnostics": 1, "status": "OK"} + ], + } + + stable_measurements = { + ("rego", "fixture"): [ + {"base": synthetic_sample(1.0), "head": synthetic_sample(1.02)} for _ in range(3) + ] + } + _, stable_failures, stable_diagnostic_mismatch = evaluate_cases(stable_measurements) + assert not stable_failures and not stable_diagnostic_mismatch + + tradeoff_base = synthetic_sample(1.0) + tradeoff_head = synthetic_sample(1.0) + tradeoff_head["initTotalMs"] = 15.0 + tradeoff_head["firstValidation"]["wallMs"] = 5.0 + tradeoff_measurements = { + ("rego", "tradeoff"): [{"base": tradeoff_base, "head": tradeoff_head} for _ in range(3)] + } + _, tradeoff_failures, _ = evaluate_cases(tradeoff_measurements) + assert not tradeoff_failures + + aggregate_measurements = { + (engine, workload): [ + {"base": synthetic_sample(1.0), "head": synthetic_sample(1.08)} for _ in range(3) + ] + for engine, workload in [("rego", "one"), ("cel", "two")] + } + _, aggregate_failures, aggregate_diagnostic_mismatch = evaluate_cases(aggregate_measurements) + assert any(failure.startswith("aggregate warm") for failure in aggregate_failures) + assert not aggregate_diagnostic_mismatch + + model_only_head = synthetic_sample(1.0) + model_only_head["warm"]["modelMedianMs"] = 4.0 + model_only_measurements = { + ("rego", "model-only"): [ + {"base": synthetic_sample(1.0), "head": model_only_head} for _ in range(3) + ] + } + model_only_evaluations, model_only_failures, _ = evaluate_cases(model_only_measurements) + model_only_markdown = render_markdown(model_only_evaluations, model_only_failures, "base", "head") + assert "| Model |" in model_only_markdown + assert "| rego | model-only |" in model_only_markdown and "| FAIL |" in model_only_markdown + + parser_only_base = synthetic_sample(1.0) + parser_only_head = synthetic_sample(1.0) + parser_only_head["initTotalMs"] = 25.0 + parser_only_head["peakRssBytes"] = 200_000_000 + parser_only_measurements = { + ("rego", "parser-only"): [ + { + "base": {**parser_only_base, "gateProcessLifecycle": False}, + "head": {**parser_only_head, "gateProcessLifecycle": False}, + } + for _ in range(3) + ] + } + parser_evaluations, parser_failures, _ = evaluate_cases(parser_only_measurements) + assert not parser_failures + parser_by_name = {evaluation.metric: evaluation for evaluation in parser_evaluations[("rego", "parser-only")]} + assert not parser_by_name["init + first"].enforced + assert not parser_by_name["peak RSS"].enforced + assert parser_by_name["first validation"].enforced + assert parser_by_name["warm total"].enforced + + mismatch_measurements = { + ("rego", "fixture"): [ + {"base": synthetic_sample(1.0, "base"), "head": synthetic_sample(1.0, "head")} + ] + } + _, mismatch_failures, mismatch_detected = evaluate_cases(mismatch_measurements) + assert mismatch_detected and any("diagnostics differ" in failure for failure in mismatch_failures) + security_directory = PROJECT_ROOT / "src" / "resources" / "security" + expected_security_templates = { + path.resolve() + for path in security_directory.rglob("*") + if path.is_file() and path.suffix.lower() in {".json", ".yaml", ".yml"} + } + discovered_security_workloads = security_workloads(security_directory, 5) + discovered_security_templates = { + workload.templates[0].resolve() for workload in discovered_security_workloads + } + assert expected_security_templates + assert discovered_security_templates == expected_security_templates + assert len({workload.name for workload in discovered_security_workloads}) == len(discovered_security_workloads) + + print("performance comparison self-tests passed") + + +def main() -> int: + arguments = parse_args() + if arguments.self_test: + run_self_test() + return 0 + + time_prefix() + base_sha = resolve_ref(arguments.base_ref) + head_sha = resolve_ref(arguments.head_ref) + if base_sha == head_sha: + raise RuntimeError("base and head resolve to the same commit") + + output_dir = arguments.output_dir.resolve() + output_dir.mkdir(parents=True, exist_ok=True) + worktree_root = output_dir / "worktrees" + target_root = output_dir / "targets" + fixtures = generate_fixtures(output_dir / "fixtures") + workloads = workload_matrix(fixtures, arguments.quick) + engines = ("rego", "cel") + initial_pairs = 1 if arguments.quick else arguments.initial_pairs + confirmation_pairs = 1 if arguments.quick else arguments.confirmation_pairs + + base_tree = worktree_root / "base" + head_tree = worktree_root / "head" + measurements: dict[tuple[str, str], list[dict[str, dict[str, Any]]]] = {} + try: + print(f"Building base {base_sha}", file=sys.stderr) + base_binary = prepare_worktree(base_sha, base_tree, target_root / "base") + print(f"Building head {head_sha}", file=sys.stderr) + head_binary = prepare_worktree(head_sha, head_tree, target_root / "head") + + collect_pairs(base_binary, head_binary, workloads, engines, 0, initial_pairs, None, measurements) + evaluations, failures, diagnostic_mismatch = evaluate_cases(measurements) + if failures and not diagnostic_mismatch: + failing_cases = { + case + for case, case_evaluations in evaluations.items() + if any(evaluation.regression for evaluation in case_evaluations) + } + if any(failure.startswith("aggregate ") for failure in failures): + failing_cases = set(evaluations) + print(f"Confirming {len(failing_cases)} apparent regression(s)", file=sys.stderr) + collect_pairs( + base_binary, + head_binary, + workloads, + engines, + initial_pairs, + confirmation_pairs, + failing_cases, + measurements, + ) + evaluations, failures, diagnostic_mismatch = evaluate_cases(measurements) + + serialized_measurements = { + f"{engine}/{workload}": pairs for (engine, workload), pairs in sorted(measurements.items()) + } + json_path = output_dir / "performance-comparison.json" + json_path.write_text( + json.dumps( + { + "base": base_sha, + "head": head_sha, + "measurements": serialized_measurements, + "failures": failures, + }, + indent=2, + sort_keys=True, + ) + + "\n" + ) + markdown = render_markdown(evaluations, failures, base_sha, head_sha) + markdown_path = output_dir / "performance-comparison.md" + markdown_path.write_text(markdown) + print(markdown) + if failures: + for failure in failures: + print(f"::error::{failure}") + return 1 + return 0 + finally: + if not arguments.keep_worktrees: + remove_worktree(base_tree) + remove_worktree(head_tree) + subprocess.run(["git", "worktree", "prune"], cwd=PROJECT_ROOT, check=False) + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except (OSError, RuntimeError, ValueError, json.JSONDecodeError) as error: + print(f"performance regression check failed: {error}", file=sys.stderr) + raise SystemExit(2) from error diff --git a/src/Cargo.lock b/src/Cargo.lock index bd7528a3..aec154cd 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -1480,6 +1480,19 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" +[[package]] +name = "performance-harness" +version = "1.9.0" +dependencies = [ + "cloudformation-validate-cel-engine", + "cloudformation-validate-diagnostics", + "cloudformation-validate-rego-engine", + "cloudformation-validate-rules", + "cloudformation-validate-schema-validator", + "cloudformation-validate-validation-engine", + "serde_json", +] + [[package]] name = "pin-project-lite" version = "0.2.17" diff --git a/src/Cargo.toml b/src/Cargo.toml index 4255a0f8..f14d9e6d 100644 --- a/src/Cargo.toml +++ b/src/Cargo.toml @@ -10,6 +10,7 @@ members = [ "data-source", "diagnostics", "guard-translator", + "performance-harness", "rego-engine", "resources", "rules", diff --git a/src/cel-engine/src/engine.rs b/src/cel-engine/src/engine.rs index a818ec11..3b6d9ae0 100644 --- a/src/cel-engine/src/engine.rs +++ b/src/cel-engine/src/engine.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use diagnostics::{Diagnostic, Entity, PhaseMetric, phase_metric}; use guard_translator::{ensure_translatable, pack_name_from_path, parse_guard}; use rules::{RuleInfo, RuleMetadataEntry, RuleOrigin, Severity, build_rule_metadata_map, is_valid_custom_rule_id}; -use schema_validator::{OverlayCatalog, SchemaValidator}; +use schema_validator::{OverlayCatalog, SchemaMetadataCatalog, SchemaValidator, schema_metadata_catalog_with_overlays}; use template_model::{SemanticModel, UNKNOWN_SPAN}; use validation_engine::{ EngineConfig, ValidateConfig, ValidationEngine, ValidationError, build_rule_list, semantic_model_to_input_json, @@ -67,7 +67,9 @@ impl CelEngine { pub fn new(config: EngineConfig) -> anyhow::Result { let catalog = config.build_overlay_catalog().map_err(|e| anyhow::anyhow!("Failed to build overlay catalog: {e}"))?; - Self::new_from_catalog(config, &catalog) + let start = web_time::Instant::now(); + let schema_metadata = schema_metadata_catalog_with_overlays(&catalog)?; + Self::new_from_parts(config, &catalog, schema_metadata, start) } /// Constructs the engine reusing metadata from an already-built @@ -75,16 +77,25 @@ impl CelEngine { /// authoritative - the engine does not re-resolve overlay schemas. /// /// This entry point is intended for language bindings and the CLI, which - /// construct a `SchemaValidator` once and share it with the engine. + /// construct a `SchemaValidator` once and share it with the engine. The + /// validator's shared schema-metadata catalog is reused rather than rebuilt, + /// so every engine built from the same validator shares one catalog rather + /// than rebuilding it. #[doc(hidden)] pub fn new_with_schema_validator(config: EngineConfig, validator: &SchemaValidator) -> anyhow::Result { - Self::new_from_catalog(config, validator.overlay_catalog()) - } - - /// Internal constructor that accepts a pre-built overlay catalog. - fn new_from_catalog(config: EngineConfig, overlay_catalog: &OverlayCatalog) -> anyhow::Result { let start = web_time::Instant::now(); - + let schema_metadata = validator.schema_metadata_catalog()?; + Self::new_from_parts(config, validator.overlay_catalog(), schema_metadata, start) + } + + /// Internal constructor that accepts a pre-built overlay catalog and the + /// shared schema-metadata catalog resolved for it. + fn new_from_parts( + config: EngineConfig, + overlay_catalog: &OverlayCatalog, + schema_metadata: Arc, + start: web_time::Instant, + ) -> anyhow::Result { let native_rules = NativeRuleRegistry::new(); let generated_rules = GeneratedRuleRegistry::new()?; @@ -158,6 +169,7 @@ impl CelEngine { if !overlay_catalog.is_empty() { cached_data.merge_overlay_catalog(overlay_catalog)?; } + cached_data.set_schema_metadata(schema_metadata); let init_metric = phase_metric(start); Ok(CelEngine { native_rules, diff --git a/src/cel-engine/src/rules/best_practices.rs b/src/cel-engine/src/rules/best_practices.rs index 526d8cea..a56caca3 100644 --- a/src/cel-engine/src/rules/best_practices.rs +++ b/src/cel-engine/src/rules/best_practices.rs @@ -510,16 +510,15 @@ None, } } - if let Some(sm) = ctx.cached_data.schema_metadata().get("schema_metadata") { + { + let schema_metadata = ctx.cached_data.schema_metadata_catalog(); for (name, res) in &m.resources { if res.resource_type.ends_with("::MODULE") { continue; } - let supports_tags = sm + let supports_tags = schema_metadata .get(&res.resource_type) - .and_then(|entry| entry.get("properties")) - .and_then(|p| p.as_array()) - .map(|arr| arr.iter().any(|v| v.as_str() == Some("Tags"))) + .map(|entry| entry.properties.iter().any(|p| p == "Tags")) .unwrap_or(false); let tags_missing = m.resolve_properties_scenarios(name).iter().any(|(properties, conditions)| { scenario_is_reachable(m, name, conditions) && !scenario_has_effective_property(properties, "Tags") diff --git a/src/cel-engine/src/rules/mod.rs b/src/cel-engine/src/rules/mod.rs index 49f0d072..208d6bb7 100644 --- a/src/cel-engine/src/rules/mod.rs +++ b/src/cel-engine/src/rules/mod.rs @@ -2,14 +2,14 @@ use data_source::embedded; use data_source::rule_data::{NormalizedRuleTablesDocument, RuleData, RuleTables}; use data_source::types::{ ArtifactCountEntry, CodepipelineArtifactCounts, DeprecatedResourceTypes, GetattData, IamActionResourcePatterns, - KnownResourceTypes, PrimaryIdentifiers, RetentionPeriodRequirements, SecretsManagerArnFields, SensitivePorts, - StatefulResourceTypes, + KnownResourceTypes, PrimaryIdentifiers, RetentionPeriodRequirements, SchemaMetadataCatalog, + SecretsManagerArnFields, SensitivePorts, StatefulResourceTypes, }; use diagnostics::Diagnostic; use rules::Category; use schema_validator::OverlayCatalog; use std::collections::{HashMap, HashSet}; -use std::sync::{Arc, LazyLock, OnceLock}; +use std::sync::{Arc, LazyLock}; use template_model::SemanticModel; pub mod best_practices; @@ -26,7 +26,7 @@ pub struct CachedData { pub known_types: HashSet, pub getatt_attrs: HashMap>, pub getatt_attr_types: HashMap>, - schema_metadata_lazy: OnceLock, + schema_metadata: Arc, pub iam_action_resource_patterns: HashMap>, pub enum_data: HashMap, pub stateful_resource_types: HashSet, @@ -263,7 +263,7 @@ impl CachedData { known_types, getatt_attrs, getatt_attr_types, - schema_metadata_lazy: OnceLock::new(), + schema_metadata: Arc::new(SchemaMetadataCatalog::new()), iam_action_resource_patterns, enum_data, stateful_resource_types, @@ -286,8 +286,10 @@ impl CachedData { /// Merges overlay catalog data into this cached data instance. /// - /// Called when overlays are non-empty so GetAtt attributes, attribute types, - /// primary identifiers, and schema metadata from overlays are visible to rules. + /// Called when overlays are non-empty so their resource types, GetAtt + /// attributes, attribute types, and primary identifiers are visible to rules. + /// Overlay-aware schema metadata is installed separately through + /// [`Self::set_schema_metadata`]. pub fn merge_overlay_catalog(&mut self, catalog: &OverlayCatalog) -> anyhow::Result<()> { if catalog.is_empty() { return Ok(()); @@ -320,40 +322,20 @@ impl CachedData { self.primary_identifiers.insert(type_name.clone(), pids.clone()); } - // Eagerly initialize schema metadata with merged overlay data. - let base_metadata: serde_json::Value = serde_json::from_slice(&embedded::SCHEMA_METADATA_BYTES) - .map_err(|e| anyhow::anyhow!("Failed to parse schema_metadata JSON: {}", e))?; - let mut metadata_obj = base_metadata - .as_object() - .cloned() - .ok_or_else(|| anyhow::anyhow!("Embedded schema_metadata data must be an object"))?; - let inner_map = metadata_obj - .get_mut("schema_metadata") - .and_then(|value| value.as_object_mut()) - .ok_or_else(|| anyhow::anyhow!("Embedded schema_metadata data must contain a schema_metadata object"))?; - anyhow::ensure!(!inner_map.is_empty(), "Embedded schema_metadata data must not be empty"); - for (type_name, entry) in &catalog.schema_metadata { - inner_map.insert( - type_name.clone(), - serde_json::to_value(entry) - .map_err(|e| anyhow::anyhow!("Failed to serialize SchemaMetadataEntry: {}", e))?, - ); - } - let merged = serde_json::Value::Object(metadata_obj); - // Construct the OnceLock with the merged value. The OnceLock is freshly - // created in `load()`, so this is the first and only set call. - self.schema_metadata_lazy = OnceLock::new(); - self.schema_metadata_lazy - .set(merged) - .map_err(|_| anyhow::anyhow!("schema_metadata OnceLock was unexpectedly already initialized"))?; Ok(()) } - /// Lazy accessor - parses the 14MB `schema_metadata` JSON on first call. - pub fn schema_metadata(&self) -> &serde_json::Value { - self.schema_metadata_lazy.get_or_init(|| { - serde_json::from_slice(&embedded::SCHEMA_METADATA_BYTES).expect("Failed to parse schema_metadata JSON") - }) + /// Installs the shared, overlay-aware schema-metadata catalog the engine + /// resolved. The same [`Arc`] is shared across engines and validators, so no + /// catalog is parsed or copied per engine. + pub fn set_schema_metadata(&mut self, catalog: Arc) { + self.schema_metadata = catalog; + } + + /// The shared, overlay-aware schema-metadata catalog: resource type name to + /// its typed metadata entry. + pub fn schema_metadata_catalog(&self) -> &SchemaMetadataCatalog { + &self.schema_metadata } } diff --git a/src/cel-engine/src/rules/resources_extra.rs b/src/cel-engine/src/rules/resources_extra.rs index b1abbf4b..5900b2fc 100644 --- a/src/cel-engine/src/rules/resources_extra.rs +++ b/src/cel-engine/src/rules/resources_extra.rs @@ -1414,13 +1414,12 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { // list whose schema requires uniqueItems is covered by the Fatal uniqueItems // check instead, so it is excluded here. The `Command` property of // run-command resources legitimately repeats values and is exempt. - if let Some(sm) = ctx.cached_data.schema_metadata().get("schema_metadata").and_then(|s| s.as_object()) { + { + let schema_metadata = ctx.cached_data.schema_metadata_catalog(); for (name, res) in &m.resources { - let Some(type_meta) = sm.get(&res.resource_type).and_then(|t| t.as_object()) else { + let Some(type_meta) = schema_metadata.get(&res.resource_type) else { continue; }; - let known_props = type_meta.get("property_types").and_then(|p| p.as_object()); - let constraints = type_meta.get("property_constraints").and_then(|p| p.as_object()); for prop in res.properties.keys() { if prop == "Command" { continue; @@ -1428,13 +1427,13 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { // Only a property the schema actually defines can be a "list that // permits duplicates"; an unknown property is a structural error // handled elsewhere. - if !known_props.is_some_and(|p| p.contains_key(prop)) { + if !type_meta.property_types.contains_key(prop) { continue; } - let requires_unique = constraints - .and_then(|c| c.get(prop)) - .and_then(|meta| meta.get("uniqueItems")) - .and_then(|v| v.as_bool()) + let requires_unique = type_meta + .property_constraints + .get(prop) + .and_then(|constraints| constraints.unique_items) .unwrap_or(false); if requires_unique { continue; @@ -2410,42 +2409,47 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { check_iam_action_resources(&mut out, m, name, &doc, &ctx.cached_data.iam_action_resource_patterns); } + let mut vpc_cidrs: HashMap<&str, (String, Ipv4Cidr)> = HashMap::new(); for vpc_name in m.resources_of_type("AWS::EC2::VPC") { - let vpc_cidr_str = match resolve_concrete(m, vpc_name, "Properties.CidrBlock") - .and_then(|v| if let serde_json::Value::String(s) = v { Some(s) } else { None }) - { - Some(s) => s, - None => continue, + let Some(serde_json::Value::String(vpc_cidr)) = resolve_concrete(m, vpc_name, "Properties.CidrBlock") else { + continue; }; - let vpc_net = match parse_ipv4_cidr(&vpc_cidr_str) { - Some(n) => n, - None => continue, + if let Some(vpc_network) = parse_ipv4_cidr(&vpc_cidr) { + vpc_cidrs.insert(vpc_name, (vpc_cidr, vpc_network)); + } + } + for subnet_name in m.resources_of_type("AWS::EC2::Subnet") { + let subnet_vpc = resolve_concrete(m, subnet_name, "Properties.VpcId"); + let referenced_vpc = m + .follow_ref(subnet_name, "Properties.VpcId") + .or_else(|| subnet_vpc.as_ref().and_then(|value| value.as_str())); + let Some((vpc_cidr, vpc_network)) = referenced_vpc.and_then(|vpc_name| vpc_cidrs.get(vpc_name)) else { + continue; }; - for subnet_name in m.resources_of_type("AWS::EC2::Subnet") { - let subnet_vpc = resolve_concrete(m, subnet_name, "Properties.VpcId"); - let refs_this_vpc = m.follow_ref(subnet_name, "Properties.VpcId").map(|t| t == vpc_name).unwrap_or(false) - || subnet_vpc.as_ref().and_then(|v| v.as_str()) == Some(vpc_name); - if !refs_this_vpc { - continue; - } - if let Some(serde_json::Value::String(sub_cidr)) = resolve_concrete(m, subnet_name, "Properties.CidrBlock") - && let Some(sub_net) = parse_ipv4_cidr(&sub_cidr) - && !is_subnet_of(sub_net, vpc_net) - { - out.push(make_resource_diagnostic( - "E3059", - &format!("Subnet CIDR '{}' is not within VPC CIDR '{}'", sub_cidr, vpc_cidr_str), - m, - subnet_name, - "Properties.CidrBlock", - None, - )); - } + if let Some(serde_json::Value::String(subnet_cidr)) = resolve_concrete(m, subnet_name, "Properties.CidrBlock") + && let Some(subnet_network) = parse_ipv4_cidr(&subnet_cidr) + && !is_subnet_of(subnet_network, *vpc_network) + { + out.push(make_resource_diagnostic( + "E3059", + &format!("Subnet CIDR '{}' is not within VPC CIDR '{}'", subnet_cidr, vpc_cidr), + m, + subnet_name, + "Properties.CidrBlock", + None, + )); } } { - for (resource_type, identifier_properties) in &ctx.cached_data.primary_identifiers { + let mut resource_types: Vec<&String> = m + .resources_by_type + .keys() + .filter(|resource_type| ctx.cached_data.primary_identifiers.contains_key(*resource_type)) + .collect(); + resource_types.sort_unstable(); + for resource_type in resource_types { + let identifier_properties = &ctx.cached_data.primary_identifiers[resource_type]; let conflicts = m.primary_identifier_conflicts(resource_type, identifier_properties); for (tuple, resources) in &conflicts { let instance_repr = render_primary_id_dict(identifier_properties, tuple); @@ -2485,63 +2489,52 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { } } - if let Some(sm) = ctx.cached_data.schema_metadata().get("schema_metadata").and_then(|s| s.as_object()) { + { + let schema_metadata = ctx.cached_data.schema_metadata_catalog(); for (name, res) in &m.resources { - if let Some(type_meta) = sm.get(&res.resource_type).and_then(|t| t.as_object()) { - let prop_types = type_meta.get("property_types").and_then(|p| p.as_object()); - if let Some(props_meta) = type_meta.get("property_constraints").and_then(|p| p.as_object()) { - for (prop, meta) in props_meta { - let is_string = - prop_types.and_then(|pt| pt.get(prop)).and_then(|v| v.as_str()) == Some("string"); - let max_len = meta - .get("maxLength") - .and_then(|v| v.as_u64()) - .or_else(|| if is_string { meta.get("maximum").and_then(|v| v.as_u64()) } else { None }); - let min_len = meta - .get("minLength") - .and_then(|v| v.as_u64()) - .or_else(|| if is_string { meta.get("minimum").and_then(|v| v.as_u64()) } else { None }); - if max_len.is_none() && min_len.is_none() { - continue; + if let Some(type_meta) = schema_metadata.get(&res.resource_type) { + for (prop, constraints) in &type_meta.property_constraints { + let is_string = type_meta.property_types.get(prop).map(|t| t == "string").unwrap_or(false); + let max_len = constraints.max_length.or_else(|| { + if is_string { constraints.maximum.as_ref().and_then(|v| v.as_u64()) } else { None } + }); + let min_len = constraints.min_length.or_else(|| { + if is_string { constraints.minimum.as_ref().and_then(|v| v.as_u64()) } else { None } + }); + if max_len.is_none() && min_len.is_none() { + continue; + } + let path = format!("Properties.{}", prop); + // Only reported when the constraint is broken whichever + // value the deployment picks: the shortest possibility + // still too long, or the longest still too short. A value + // the template states literally is checked against the + // constraint by schema validation instead, and a value with + // any unknown possibility yields no bounds at all. + if let Some((shortest, longest)) = m.estimated_string_length_bounds(name, &path) { + if let Some(max) = max_len + && shortest as u64 > max + { + out.push(make_resource_diagnostic( + "W9006", + &format!("String length {} exceeds maximum {} for property '{}'", shortest, max, prop), + m, + name, + &path, + None, + )); } - let path = format!("Properties.{}", prop); - // Only reported when the constraint is broken whichever - // value the deployment picks: the shortest possibility - // still too long, or the longest still too short. A value - // the template states literally is checked against the - // constraint by schema validation instead, and a value with - // any unknown possibility yields no bounds at all. - if let Some((shortest, longest)) = m.estimated_string_length_bounds(name, &path) { - if let Some(max) = max_len - && shortest as u64 > max - { - out.push(make_resource_diagnostic( - "W9006", - &format!( - "String length {} exceeds maximum {} for property '{}'", - shortest, max, prop - ), - m, - name, - &path, - None, - )); - } - if let Some(min) = min_len - && (longest as u64) < min - { - out.push(make_resource_diagnostic( - "W9006", - &format!( - "String length {} is below minimum {} for property '{}'", - longest, min, prop - ), - m, - name, - &path, - None, - )); - } + if let Some(min) = min_len + && (longest as u64) < min + { + out.push(make_resource_diagnostic( + "W9006", + &format!("String length {} is below minimum {} for property '{}'", longest, min, prop), + m, + name, + &path, + None, + )); } } } @@ -3050,10 +3043,14 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { ), ]; for &(rule_id, rtype, prop_path, enum_key) in enum_checks { + let resources = m.resources_of_type(rtype); + if resources.is_empty() { + continue; + } let Some(allowed) = region_flat_allowed(&ctx.cached_data.enum_data, enum_key, region) else { continue; }; - for name in m.resources_of_type(rtype) { + for name in resources { if let Some(val) = resolve_enum_string(m, name, prop_path) && !allowed.contains(val.as_str()) { @@ -3100,10 +3097,14 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { ), ]; for &(rule_id, rtype, wildcard_path, report_path, enum_key) in wildcard_enum_checks { + let resources = m.resources_of_type(rtype); + if resources.is_empty() { + continue; + } let Some(allowed) = region_flat_allowed(&ctx.cached_data.enum_data, enum_key, region) else { continue; }; - for name in m.resources_of_type(rtype) { + for name in resources { let mut reported = HashSet::new(); for val in resolve_concrete_strings(m, name, wildcard_path) { if allowed.contains(val.as_str()) || !reported.insert(val.clone()) { @@ -3184,10 +3185,12 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { } } - if let Some(allowed) = - region_flat_allowed(&ctx.cached_data.enum_data, "data/aws_amazonmq_broker_instancetype_enum", region) + let amazonmq_brokers = m.resources_of_type("AWS::AmazonMQ::Broker"); + if !amazonmq_brokers.is_empty() + && let Some(allowed) = + region_flat_allowed(&ctx.cached_data.enum_data, "data/aws_amazonmq_broker_instancetype_enum", region) { - for name in m.resources_of_type("AWS::AmazonMQ::Broker") { + for name in amazonmq_brokers { if let Some(val) = resolve_enum_string(m, name, "Properties.HostInstanceType") && !allowed.contains(val.as_str()) { @@ -3203,12 +3206,15 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { } } - if let Some(allowed) = region_flat_allowed( - &ctx.cached_data.enum_data, - "data/aws_emr_cluster_instancetypeconfig_instancetype_enum", - region, - ) { - for name in m.resources_of_type("AWS::EMR::InstanceFleetConfig") { + let emr_instance_fleets = m.resources_of_type("AWS::EMR::InstanceFleetConfig"); + if !emr_instance_fleets.is_empty() + && let Some(allowed) = region_flat_allowed( + &ctx.cached_data.enum_data, + "data/aws_emr_cluster_instancetypeconfig_instancetype_enum", + region, + ) + { + for name in emr_instance_fleets { if let Some(val) = resolve_enum_string(m, name, "Properties.InstanceType") && !allowed.contains(val.as_str()) { @@ -4136,8 +4142,8 @@ fn region_flat_allowed<'a>( enum_data: &'a HashMap, enum_key: &str, region: Option<&str>, -) -> Option> { - region_enums::flat_allowed_values(region_map_for_key(enum_data, enum_key)?, region) +) -> Option> { + region_enums::flat_allowed_value_set(region_map_for_key(enum_data, enum_key)?, region) } /// A scalar string property value, collapsing an `Fn::If`-wrapped value to its diff --git a/src/cfn-validate/tests/security_tests.rs b/src/cfn-validate/tests/security_tests.rs index 93afc694..434aba3a 100644 --- a/src/cfn-validate/tests/security_tests.rs +++ b/src/cfn-validate/tests/security_tests.rs @@ -481,6 +481,37 @@ fn large_resource_count_validates_to_a_bounded_result_on_both_engines() { } } +#[test] +fn dense_cross_resource_fanout_validates_to_a_bounded_result() { + const SCALE_RESOURCES: usize = 500; + const RESOURCE_REFERENCE_EDGES: usize = 11_875; + let bytes = common::load_security("cross_reference_fanout.yaml"); + + let model = SemanticModel::from_bytes(&bytes).expect("the cross-reference fixture must parse to a semantic model"); + assert_eq!(model.resources.len(), SCALE_RESOURCES); + let resource_edges = model + .graph + .edges + .iter() + .filter(|edge| { + model.resources.contains_key(&edge.source_resource) && model.resources.contains_key(&edge.target) + }) + .count(); + assert_eq!(resource_edges, RESOURCE_REFERENCE_EDGES); + + for engine_name in ["rego", "cel"] { + let finished = validate_report_within(COMPLETION_BUDGET, engine_name, bytes.clone()).unwrap_or_else(|| { + panic!( + "{engine_name}: {RESOURCE_REFERENCE_EDGES} resource references must validate within \ + {COMPLETION_BUDGET:?}" + ) + }); + let _ = finished.unwrap_or_else(|error| { + panic!("{engine_name}: dense cross-resource validation must return a structured report: {error}") + }); + } +} + #[test] fn condition_chain_boundary_resolves_within_budget() { // 20 parameters, 40 acyclic chained conditions matching the public CDK repro diff --git a/src/data-source/src/types.rs b/src/data-source/src/types.rs index 631276f8..15235b95 100644 --- a/src/data-source/src/types.rs +++ b/src/data-source/src/types.rs @@ -1,5 +1,5 @@ use serde::{Deserialize, Serialize}; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; #[derive(Debug, Default, Serialize, Deserialize)] pub struct KnownResourceTypes { @@ -61,6 +61,112 @@ pub struct SecretsManagerArnFields { pub secretsmanager_arn_fields: Vec, } +/// The committed `schema_metadata` artifact: a wrapper whose single +/// `schema_metadata` field maps each resource type name to its metadata. +/// +/// This is the authoritative, recursively typed model consumed by the schema +/// validator and both rule engines. It is lossless: every field the artifact +/// carries has a typed home, and any field the current code does not model is +/// preserved verbatim in the per-level `additional` extension maps, so a future +/// artifact deserializes and reserializes without code changes. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +pub struct SchemaMetadataDocument { + pub schema_metadata: SchemaMetadataCatalog, +} + +/// Resource type name to its typed schema metadata. +pub type SchemaMetadataCatalog = HashMap; + +/// Per-resource-type schema metadata, also used for every nested object level. +/// +/// The model is recursive: a nested object reappears as a `SchemaMetadataEntry` +/// under [`SchemaPropertyConstraints::sub_properties`], and an array element +/// object reappears under [`SchemaItemsMetadata::schema`]. +/// +/// `properties`, `required`, `property_types`, and `property_enums` always +/// serialize, even when empty, because the generator emits them at every level; +/// preserving that presence is required for lossless round-tripping. Fields the +/// current code does not model are retained in [`Self::additional`]. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +pub struct SchemaMetadataEntry { + #[serde(default)] + pub properties: Vec, + #[serde(default)] + pub required: Vec, + #[serde(default)] + pub property_types: HashMap, + #[serde(default)] + pub property_enums: HashMap>, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub property_constraints: HashMap, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub dependent_required: HashMap>, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub dependent_excluded: HashMap>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub required_or: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub required_xor: Vec, + #[serde(flatten, default)] + pub additional: BTreeMap, +} + +/// The constraints attached to a single property: scalar bounds, format, nested +/// object sub-properties, array item schema, and inter-property dependencies. +/// +/// `minimum`/`maximum` keep the authored JSON number verbatim as a +/// [`serde_json::Number`], so an integer bound stays an integer and a decimal or +/// exponent bound keeps its precision. Length and item bounds are non-negative +/// integers and use `u64`. Unknown fields are preserved in [`Self::additional`]. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +pub struct SchemaPropertyConstraints { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pattern: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub minimum: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub maximum: Option, + #[serde(rename = "minLength", default, skip_serializing_if = "Option::is_none")] + pub min_length: Option, + #[serde(rename = "maxLength", default, skip_serializing_if = "Option::is_none")] + pub max_length: Option, + #[serde(rename = "minItems", default, skip_serializing_if = "Option::is_none")] + pub min_items: Option, + #[serde(rename = "maxItems", default, skip_serializing_if = "Option::is_none")] + pub max_items: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub format: Option, + #[serde(rename = "uniqueItems", default, skip_serializing_if = "Option::is_none")] + pub unique_items: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sub_properties: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub items: Option>, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub dependent_required: HashMap>, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub dependent_excluded: HashMap>, + #[serde(flatten, default)] + pub additional: BTreeMap, +} + +/// Array element metadata: the element type, a nested object schema when the +/// elements are objects, and any element-level dependencies. Recursive through +/// [`Self::schema`]. Unknown fields are preserved in [`Self::additional`]. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +pub struct SchemaItemsMetadata { + #[serde(rename = "type", default, skip_serializing_if = "Option::is_none")] + pub item_type: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub schema: Option>, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub dependent_required: HashMap>, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub dependent_excluded: HashMap>, + #[serde(flatten, default)] + pub additional: BTreeMap, +} + #[cfg(test)] mod tests { use super::*; @@ -77,6 +183,7 @@ mod tests { assert!(serde_json::from_str::("{}").is_err()); assert!(serde_json::from_str::("{}").is_err()); assert!(serde_json::from_str::("{}").is_err()); + assert!(serde_json::from_str::("{}").is_err()); } #[test] @@ -121,4 +228,182 @@ mod tests { let restored: RetentionPeriodRequirements = postcard::from_bytes(&bytes).unwrap(); assert_eq!(restored.retention_period_requirements["AWS::SQS::Queue"], vec!["MessageRetentionPeriod"]); } + + /// The whole committed `schema_metadata` artifact must survive a typed + /// round-trip with no loss: parse it as an untyped value, parse it through + /// the typed model, serialize the typed model back, and require the two + /// values to be equal. `serde_json::Value` equality is order-insensitive, so + /// only content is compared, not key order or whitespace. + #[test] + fn committed_schema_metadata_typed_round_trip_is_lossless() { + let bytes = &crate::embedded::SCHEMA_METADATA_BYTES; + let original: serde_json::Value = + serde_json::from_slice(bytes).expect("committed schema_metadata must be valid JSON"); + let document: SchemaMetadataDocument = + serde_json::from_slice(bytes).expect("committed schema_metadata must parse through the typed model"); + assert!(!document.schema_metadata.is_empty(), "committed schema_metadata must not be empty"); + let reserialized = serde_json::to_value(&document).expect("typed model must reserialize"); + assert_eq!( + original, reserialized, + "typed round-trip of the committed schema_metadata artifact dropped or changed a field" + ); + } + + /// Every entry in the committed artifact must parse through the typed model + /// without any value landing in an `additional` extension map - if it does, + /// the model is missing a typed field for a value the generator emits today. + #[test] + fn committed_schema_metadata_has_no_unmodeled_fields() { + let document: SchemaMetadataDocument = + serde_json::from_slice(&crate::embedded::SCHEMA_METADATA_BYTES).expect("typed parse"); + for (type_name, entry) in &document.schema_metadata { + assert!( + entry.additional.is_empty(), + "{type_name}: entry carries unmodeled fields {:?}", + entry.additional.keys().collect::>() + ); + for (prop, constraints) in &entry.property_constraints { + assert_constraints_fully_modeled(type_name, prop, constraints); + } + } + } + + fn assert_constraints_fully_modeled(type_name: &str, prop: &str, constraints: &SchemaPropertyConstraints) { + assert!( + constraints.additional.is_empty(), + "{type_name}.{prop}: constraint carries unmodeled fields {:?}", + constraints.additional.keys().collect::>() + ); + if let Some(sub) = &constraints.sub_properties { + assert!( + sub.additional.is_empty(), + "{type_name}.{prop}: sub_properties carries unmodeled fields {:?}", + sub.additional.keys().collect::>() + ); + for (nested_prop, nested) in &sub.property_constraints { + assert_constraints_fully_modeled(type_name, &format!("{prop}.{nested_prop}"), nested); + } + } + if let Some(items) = &constraints.items { + assert!( + items.additional.is_empty(), + "{type_name}.{prop}: items carries unmodeled fields {:?}", + items.additional.keys().collect::>() + ); + if let Some(schema) = &items.schema { + for (nested_prop, nested) in &schema.property_constraints { + assert_constraints_fully_modeled(type_name, &format!("{prop}[].{nested_prop}"), nested); + } + } + } + } + + /// Unknown fields at the entry, constraint, and item levels are preserved + /// verbatim, and numbers keep their integer, decimal, and exponent JSON + /// semantics through the flattened extension maps. + #[test] + fn unknown_fields_and_number_forms_survive_round_trip() { + let synthetic = serde_json::json!({ + "schema_metadata": { + "AWS::Test::Synthetic": { + "properties": ["A", "B"], + "required": ["A"], + "property_types": {"A": "string", "B": "array"}, + "property_enums": {}, + "property_constraints": { + "A": { + "minLength": 1, + "maxLength": 64, + "minimum": 0, + "maximum": 3.5, + "pattern": "^x$", + "future_int": 42, + "future_decimal": 1.25, + "future_exponent": 1e10 + }, + "B": { + "items": { + "type": "object", + "schema": { + "properties": ["K"], + "required": [], + "property_types": {"K": "string"}, + "property_enums": {}, + "future_item_field": 7 + }, + "future_items_int": 99 + } + } + }, + "future_entry_field": {"nested": [1, 2, 3]}, + "future_entry_int": 123 + } + } + }); + + let document: SchemaMetadataDocument = + serde_json::from_value(synthetic.clone()).expect("synthetic document parses"); + let reserialized = serde_json::to_value(&document).expect("synthetic document reserializes"); + assert_eq!(synthetic, reserialized, "an unknown field or number form was not preserved through the model"); + + // The integer forms must remain integers, not be widened to floats. + let entry = &document.schema_metadata["AWS::Test::Synthetic"]; + assert_eq!(entry.additional["future_entry_int"], serde_json::json!(123)); + let a = &entry.property_constraints["A"]; + assert_eq!(a.additional["future_int"], serde_json::json!(42)); + assert_eq!(a.additional["future_exponent"], serde_json::json!(1e10)); + assert_eq!(a.minimum, Some(serde_json::Number::from(0))); + } + + /// The four always-present entry fields serialize even when empty, at the + /// top level and at every nested level (`sub_properties`, `items.schema`). + #[test] + fn present_empty_base_fields_are_retained_at_every_level() { + let source = serde_json::json!({ + "schema_metadata": { + "AWS::Test::Empty": { + "properties": [], + "required": [], + "property_types": {}, + "property_enums": {}, + "property_constraints": { + "Nested": { + "sub_properties": { + "properties": [], + "required": [], + "property_types": {}, + "property_enums": {} + } + }, + "Arr": { + "items": { + "type": "array", + "schema": { + "properties": [], + "required": [], + "property_types": {}, + "property_enums": {} + } + } + } + } + } + } + }); + + let document: SchemaMetadataDocument = serde_json::from_value(source.clone()).expect("parses"); + let reserialized = serde_json::to_value(&document).expect("reserializes"); + assert_eq!(source, reserialized, "a present-empty base field was dropped during round-trip"); + + let entry = &reserialized["schema_metadata"]["AWS::Test::Empty"]; + for level in [ + entry, + &entry["property_constraints"]["Nested"]["sub_properties"], + &entry["property_constraints"]["Arr"]["items"]["schema"], + ] { + for field in ["properties", "required", "property_types", "property_enums"] { + assert!(level.get(field).is_some(), "expected present-empty '{field}' at this level: {level}"); + } + } + } } diff --git a/src/performance-harness/Cargo.toml b/src/performance-harness/Cargo.toml new file mode 100644 index 00000000..f76b98f7 --- /dev/null +++ b/src/performance-harness/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "performance-harness" +publish = false +version.workspace = true +edition.workspace = true +license.workspace = true +description.workspace = true +repository.workspace = true +keywords.workspace = true +categories.workspace = true + +[dependencies] +cel-engine = { workspace = true } +diagnostics = { workspace = true } +rego-engine = { workspace = true } +rules = { workspace = true } +schema-validator = { workspace = true } +validation-engine = { workspace = true } +serde_json = "1" + +[lints] +workspace = true diff --git a/src/performance-harness/src/main.rs b/src/performance-harness/src/main.rs new file mode 100644 index 00000000..2d9b5148 --- /dev/null +++ b/src/performance-harness/src/main.rs @@ -0,0 +1,199 @@ +use cel_engine::CelEngine; +use diagnostics::{DetailLevel, ValidationReport}; +use rego_engine::RegoEngine; +use rules::Severity; +use schema_validator::SchemaValidator; +use std::env; +use std::fs; +use std::hint::black_box; +use std::time::Instant; +use validation_engine::{EngineConfig, ValidateConfig, ValidationEngine, validate_bytes_with_path}; + +fn percentile(samples: &[f64], fraction: f64) -> f64 { + let mut sorted = samples.to_vec(); + sorted.sort_by(f64::total_cmp); + sorted[((sorted.len() - 1) as f64 * fraction).round() as usize] +} + +fn fingerprint_bytes(bytes: &[u8]) -> u64 { + bytes.iter().fold(0xcbf29ce484222325_u64, |hash, byte| (hash ^ u64::from(*byte)).wrapping_mul(0x100000001b3)) +} + +fn fingerprint(report: &ValidationReport) -> u64 { + let value = serde_json::json!({ + "status": &report.status, + "diagnostics": &report.diagnostics, + "counts": &report.metadata.counts, + "budgetExhaustions": &report.metadata.budget_exhaustions, + }); + let bytes = serde_json::to_vec(&value).expect("report fingerprint input must serialize"); + fingerprint_bytes(&bytes) +} + +fn error_fingerprint(error: &impl std::fmt::Display) -> u64 { + fingerprint_bytes(format!("validation-error:{error}").as_bytes()) +} + +fn main() { + let args: Vec = env::args().collect(); + if args.len() < 6 { + eprintln!("usage: performance-harness