feat(EC-1836): add ec.oci.parsed_blob builtin for cached JSON parsing#3341
feat(EC-1836): add ec.oci.parsed_blob builtin for cached JSON parsing#3341robnester-rh wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughA new OPA rego builtin ChangesociParsedBlob OPA Builtin
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
Code Review by Qodo
Context used✅ Compliance rules (platform):
27 rules 1. No singleflight for parsing
|
PR Summary by QodoAdd ec.oci.parsed_blob OPA builtin with per-component JSON parse caching WalkthroughsDescription• Register new OPA builtin "ec.oci.parsed_blob(ref)" returning parsed JSON from an OCI blob. • Cache parsed AST terms per component to avoid repeated JSON unmarshalling across namespaces. • Add unit tests covering success, invalid JSON, caching behavior, and error cases. Diagramgraph TD
P["OPA policy eval"] --> B["Builtin: ec.oci.parsed_blob"] --> C["ComponentCache"]
C -- "miss" --> O["ociBlobInternal"] --> R{{"OCI registry"}}
O --> J["json.Unmarshal"] --> T["ast.Term"] --> C
C -- "hit" --> B
High-Level AssessmentThe following are alternative approaches to this PR: 1. Rely on OPA builtin memoization only (no explicit parsed cache)
2. Extend ec.oci.blob to optionally return parsed JSON (mode flag)
3. Generic cached JSON parse builtin keyed by string (e.g., ec.json.parse_cached)
Recommendation: The current approach (a dedicated ec.oci.parsed_blob builtin with a per-component parsed-term cache) is a good fit: it keeps the existing ec.oci.blob contract intact, leverages existing blob fetch/digest verification, and scopes memory growth to component evaluation. The dedicated builtin also makes policy intent explicit ("this blob is JSON") and enables strong caching semantics (including pointer-identical reuse) validated by tests. File ChangesEnhancement (1)
Tests (1)
|
|
🤖 Finished Review · ✅ Success · Started 1:39 PM UTC · Completed 1:51 PM UTC |
ReviewFindingsLow
Info
Previous runReviewFindingsLow
Info
Previous run (2)ReviewFindingsLow
Info
Previous run (3)ReviewFindingsHigh
Medium
Low
Previous run (4)ReviewFindingsLow
Info
|
|
🤖 Review · Started 1:58 PM UTC |
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
🤖 Finished Review · ✅ Success · Started 1:58 PM UTC · Completed 2:09 PM UTC |
|
Fixed in 20d275f — added |
|
🤖 Finished Review · ✅ Success · Started 2:38 PM UTC · Completed 2:47 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 3:07 PM UTC · Completed 3:15 PM UTC |
Register a new OPA builtin ec.oci.parsed_blob that fetches an OCI blob and returns the parsed JSON value, caching the result in a per-component sync.Map. This eliminates redundant json.unmarshal calls when the same blob is evaluated across multiple policy namespaces. The builtin reuses ociBlobInternal for fetching (which already caches the raw blob), then parses once and caches the ast.Term. Subsequent calls return the cached parsed result instantly. resolves: EC-1836 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 9:35 PM UTC · Completed 9:49 PM UTC |
Summary
ec.oci.parsed_blob(ref)that fetches an OCI blob, parses it as JSON, and caches the parsed AST in a per-componentsync.Mapjson.unmarshalcalls when the same blob is evaluated across multiple policy namespaces (currently 9x for SBOMs)ociBlobInternalfor fetching (raw blob caching already handled), then parses once and caches theast.TermTest plan
TestFunctionsRegisteredincludesec.oci.parsed_blobresolves: EC-1836
🤖 Generated with Claude Code