Feat(opa): implement singleton OPA SDK to eliminate duplicate bundle …#491
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe OPA plugin is refactored to use a process-wide shared singleton for the OPA SDK with reference counting, introduces slog-based logging for OPA activity, and changes bundle fetching to use SPIFFE ID query parameters instead of path-based URIs. ChangesOPA Plugin Shared Singleton & SPIFFE Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/authlib/plugins/opa/plugin_test.go`:
- Around line 1277-1311: Update TestStartOPA_SingletonDifferentBundleURL to
assert that differing bundle_url does NOT reuse the existing singleton: call
p.startOPA() and expect either an init error or a new sharedSDK instead of
reusing singleton; specifically replace the current reuse assertions with checks
that p.shared != singleton, singleton.refCount remains 1, and the new shared
instance (p.shared) has bundleURL "http://second-service:9090" (or assert a
non-nil error from startOPA if the intended behavior is to fail). Reference
TestStartOPA_SingletonDifferentBundleURL, startOPA, singleton, sharedSDK,
bundleURL, refCount and p.shared when making the change.
In `@authbridge/authlib/plugins/opa/plugin.go`:
- Around line 265-278: The startOPA singleton reuse currently only checks
singleton != nil and can bind plugins with different p.cfg.BundleURL or agentID
to the same SDK, causing policy mismatches; update startOPA to key the shared
SDK by at least {bundleURL, agentID} (or return an error on mismatch) instead of
unconditionally reusing singleton: change the singleton logic used in startOPA
to lookup or store a map keyed by bundleURL+agentID (or compare both
singleton.bundleURL and singleton.agentID and fail fast), ensure singletonMu
still guards the map, increment refCount only for a matching key, and set
p.shared from the matched entry (or return an error if mismatched) so that
p.shared is never assigned a singleton with a different bundle or agent
identity.
- Around line 176-181: The p.shared field is accessed concurrently and can be
nilled in Shutdown, causing a data race and intermittent nil derefs; change
p.shared from a plain pointer to an atomic.Pointer[*sharedSDK] (or protect it
with a single mutex) and update startOPA to Store the pointer and Shutdown to
Store(nil) atomically, then modify Ready, OnRequest, and OnResponse to
atomically Load p.shared into a local variable once (e.g., local :=
p.shared.Load()) and use that snapshot for all checks and dereferences
(decider/ready) for the lifetime of the handler; apply the same pattern to other
reader sites mentioned (the ranges around lines noted) so all reads/writes use
the atomic load/store (or the same mutex) to eliminate the race and possible nil
deref.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8aed0a82-ddac-4de9-952b-5e7f20978d84
📒 Files selected for processing (2)
authbridge/authlib/plugins/opa/plugin.goauthbridge/authlib/plugins/opa/plugin_test.go
| func TestStartOPA_SingletonDifferentBundleURL(t *testing.T) { | ||
| resetSingleton() | ||
| defer resetSingleton() | ||
|
|
||
| // Seed the singleton with bundle_url "http://first-service:8080". | ||
| var dec decider = &mockDecider{result: &sdk.DecisionResult{Result: true}} | ||
| singletonMu.Lock() | ||
| singleton = &sharedSDK{ | ||
| decider: dec, | ||
| bundleURL: "http://first-service:8080", | ||
| agentID: "spiffe://example.com/ns/team1/sa/agent1", | ||
| refCount: 1, | ||
| } | ||
| singleton.ready.Store(true) | ||
| singletonMu.Unlock() | ||
|
|
||
| // Second instance with a DIFFERENT bundle_url should still reuse the | ||
| // singleton (warning logged, second config skipped). | ||
| p := &OPA{ | ||
| cfg: opaConfig{BundleURL: "http://second-service:9090"}, | ||
| agentID: "spiffe://example.com/ns/team1/sa/agent1", | ||
| } | ||
| if err := p.startOPA(); err != nil { | ||
| t.Fatalf("startOPA failed: %v", err) | ||
| } | ||
| if p.shared != singleton { | ||
| t.Fatal("expected second instance to reuse singleton despite different bundle_url") | ||
| } | ||
| if singleton.refCount != 2 { | ||
| t.Errorf("expected refCount=2, got %d", singleton.refCount) | ||
| } | ||
| // The singleton should retain the FIRST bundle_url. | ||
| if singleton.bundleURL != "http://first-service:8080" { | ||
| t.Errorf("expected singleton to keep first bundle_url, got %q", singleton.bundleURL) | ||
| } |
There was a problem hiding this comment.
This test is asserting the wrong reuse contract.
The PR objective says the SDK is shared when bundle_url and agent_id match, but this test expects reuse when bundle_url differs. Keeping that expectation will hide the policy-isolation bug in startOPA; it should assert a separate instance or an init failure instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@authbridge/authlib/plugins/opa/plugin_test.go` around lines 1277 - 1311,
Update TestStartOPA_SingletonDifferentBundleURL to assert that differing
bundle_url does NOT reuse the existing singleton: call p.startOPA() and expect
either an init error or a new sharedSDK instead of reusing singleton;
specifically replace the current reuse assertions with checks that p.shared !=
singleton, singleton.refCount remains 1, and the new shared instance (p.shared)
has bundleURL "http://second-service:9090" (or assert a non-nil error from
startOPA if the intended behavior is to fail). Reference
TestStartOPA_SingletonDifferentBundleURL, startOPA, singleton, sharedSDK,
bundleURL, refCount and p.shared when making the change.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/auto-sync-and-merge.yml (1)
48-52: 💤 Low valueConsider parameterizing the hardcoded username.
The username
davidhadasis hardcoded, reducing maintainability. Consider using a repository variable or workflow input.♻️ Suggested refactor using repository variable
+env: + FORK_OWNER: ${{ vars.FORK_OWNER || 'davidhadas' }} + jobs: sync-and-merge:Then in the jq filter:
- --jq '.[] | select(.headRepositoryOwner.login=="davidhadas") | .headRefName') + --jq '.[] | select(.headRepositoryOwner.login=="'"$FORK_OWNER"'") | .headRefName')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/auto-sync-and-merge.yml around lines 48 - 52, The jq filter currently hardcodes the username ("davidhadas") inside the gh pr list pipeline (variable prs); change this to use a parameter or environment variable (e.g., a workflow input or repo variable like USERNAME) and reference it in the jq expression instead of the literal string; update the gh pr list invocation that produces prs to interpolate the variable into the jq filter (ensuring proper quoting/escaping for shell and jq) and add the corresponding workflow input or env default so the username can be configured without editing the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/auto-sync-and-merge.yml:
- Around line 58-77: The step currently expands ${{
steps.prlist.outputs.branches }} directly into the shell which can allow shell
metacharacters from branch names to be interpreted; instead assign that output
to a quoted environment variable (e.g., BRANCHES) and consume it safely in the
loop (preserve the existing while IFS= read -r branch; do ... done <<<
"$BRANCHES") so the content is not re-parsed by the shell; update the job to
pass BRANCHES: ${{ steps.prlist.outputs.branches }} in env and replace the
unquoted heredoc expansion with the quoted environment variable in the merge
loop (keep git checkout/merge/push logic intact).
- Around line 15-21: Update the Checkout step to pin actions/checkout to the
immutable SHA by replacing "uses: actions/checkout@v4" with "uses:
actions/checkout@4823ef7dded2f227249189a5d8cc3efa39f98340" and add
"persist-credentials: false" under the step's with: block to disable credential
persistence; also remove the trailing blank-line whitespace after the "Fetch all
branches" step (the git fetch origin '+refs/heads/*:refs/remotes/origin/*' run)
so there is no extra whitespace line.
---
Nitpick comments:
In @.github/workflows/auto-sync-and-merge.yml:
- Around line 48-52: The jq filter currently hardcodes the username
("davidhadas") inside the gh pr list pipeline (variable prs); change this to use
a parameter or environment variable (e.g., a workflow input or repo variable
like USERNAME) and reference it in the jq expression instead of the literal
string; update the gh pr list invocation that produces prs to interpolate the
variable into the jq filter (ensuring proper quoting/escaping for shell and jq)
and add the corresponding workflow input or env default so the username can be
configured without editing the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 70bcb820-d7eb-4a6b-8bec-6ffbb7c98ff3
📒 Files selected for processing (1)
.github/workflows/auto-sync-and-merge.yml
| - name: Checkout fork | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| token: ${{ secrets.AUTO_MERGE_TOKEN }} | ||
|
|
||
| - name: Fetch all branches | ||
| run: git fetch origin '+refs/heads/*:refs/remotes/origin/*' |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest commit SHA for actions/checkout v4?
💡 Result:
The tag v4 for actions/checkout is a mutable reference, meaning it can point to different commit SHAs over time as the repository is updated [1]. As of June 10, 2026, the v4 branch in the actions/checkout repository points to the latest commit, which is 4823ef7dded2f227249189a5d8cc3efa39f98340 [2]. Because tags like v4 are mutable, it is a common best practice in GitHub Actions to pin your workflow references to a specific, immutable full-length commit SHA (e.g., 4823ef7dded2f227249189a5d8cc3efa39f98340) rather than a version tag [1][3]. This ensures that your workflow behavior remains consistent and is not affected by future updates to the tagged reference [1].
Citations:
- 1: chore: pin GitHub Actions to commit SHA docker/compose-for-agents#150
- 2: actions/checkout@4823ef7
- 3: chore: pin GitHub Actions workflows to commit SHAs hivemoot/colony#812
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/auto-sync-and-merge.yml"
echo "== File = $FILE =="
nl -ba "$FILE" | sed -n '1,120p'
echo
echo "== Trailing whitespace check (show lines) =="
# Print only lines with trailing whitespace
awk '{ if (match($0, /[ \t]+$/)) print NR ":" $0 }' "$FILE" | head -n 200Repository: kagenti/kagenti-extensions
Length of output: 186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/auto-sync-and-merge.yml"
echo "== File = $FILE =="
awk '{printf "%4d:%s\n", NR, $0}' "$FILE" | sed -n '1,140p'
echo
echo "== Trailing whitespace check (lines with trailing spaces/tabs) =="
awk '{ if ($0 ~ /[ \t]+$/) printf "%4d:%s\n", NR, $0 }' "$FILE" | head -n 200Repository: kagenti/kagenti-extensions
Length of output: 2947
Pin actions/checkout to an immutable SHA, disable credential persistence, and remove trailing whitespace.
Pin actions/checkout@v4 to actions/checkout@4823ef7dded2f227249189a5d8cc3efa39f98340 (mutable tag). Add persist-credentials: false under the Checkout fork step’s with: block. Remove trailing whitespace on the blank line after the Fetch all branches git fetch origin ... step (line 22).
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 15-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/auto-sync-and-merge.yml around lines 15 - 21, Update the
Checkout step to pin actions/checkout to the immutable SHA by replacing "uses:
actions/checkout@v4" with "uses:
actions/checkout@4823ef7dded2f227249189a5d8cc3efa39f98340" and add
"persist-credentials: false" under the step's with: block to disable credential
persistence; also remove the trailing blank-line whitespace after the "Fetch all
branches" step (the git fetch origin '+refs/heads/*:refs/remotes/origin/*' run)
so there is no extra whitespace line.
Source: Linters/SAST tools
| - name: Merge upstream main into each PR branch | ||
| run: | | ||
| echo "Branches to update:" | ||
| echo "${{ steps.prlist.outputs.branches }}" | ||
|
|
||
| while IFS= read -r branch; do | ||
| if [ -z "$branch" ]; then | ||
| continue | ||
| fi | ||
|
|
||
| echo "Updating branch: $branch" | ||
|
|
||
| git checkout "$branch" | ||
| git merge main --no-edit || exit 1 | ||
| git push origin "$branch" | ||
|
|
||
| echo "Merged and pushed for $branch" | ||
| done <<< "${{ steps.prlist.outputs.branches }}" | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.AUTO_MERGE_TOKEN }} |
There was a problem hiding this comment.
Mitigate template injection by using environment variables.
Direct expansion of ${{ steps.prlist.outputs.branches }} in the shell script is vulnerable to injection if branch names contain shell metacharacters. While the source is filtered to a specific owner, branch names can contain characters like $(), ;, or backticks that would be interpreted by the shell.
Pass the value through an environment variable instead, which prevents shell interpretation of the content.
🔒 Proposed fix
- name: Merge upstream main into each PR branch
+ env:
+ GITHUB_TOKEN: ${{ secrets.AUTO_MERGE_TOKEN }}
+ BRANCHES: ${{ steps.prlist.outputs.branches }}
run: |
echo "Branches to update:"
- echo "${{ steps.prlist.outputs.branches }}"
+ echo "$BRANCHES"
while IFS= read -r branch; do
if [ -z "$branch" ]; then
continue
fi
echo "Updating branch: $branch"
git checkout "$branch"
git merge main --no-edit || exit 1
git push origin "$branch"
echo "Merged and pushed for $branch"
- done <<< "${{ steps.prlist.outputs.branches }}"
- env:
- GITHUB_TOKEN: ${{ secrets.AUTO_MERGE_TOKEN }}
+ done <<< "$BRANCHES"🧰 Tools
🪛 zizmor (1.25.2)
[info] 61-61: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 75-75: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/auto-sync-and-merge.yml around lines 58 - 77, The step
currently expands ${{ steps.prlist.outputs.branches }} directly into the shell
which can allow shell metacharacters from branch names to be interpreted;
instead assign that output to a quoted environment variable (e.g., BRANCHES) and
consume it safely in the loop (preserve the existing while IFS= read -r branch;
do ... done <<< "$BRANCHES") so the content is not re-parsed by the shell;
update the job to pass BRANCHES: ${{ steps.prlist.outputs.branches }} in env and
replace the unquoted heredoc expansion with the quoted environment variable in
the merge loop (keep git checkout/merge/push logic intact).
Source: Linters/SAST tools
3e5249e to
ce9910d
Compare
huang195
left a comment
huang195
left a comment
There was a problem hiding this comment.
Review Summary
Clean refactor with strong test coverage — the singleton + ref-counting logic is correct (lock held across SDK init, ref-count drives shutdown, double-Shutdown is guarded by shared.Swap(nil)), and the new tests cover reuse, ref-counting, shutdown, and SPIFFE encoding.
Two things worth settling before merge:
- The
bundles?spiffe=resource is a contract change with the Kagenti Bundle Server (a separate component) — confirm the server speaks the query form, and update the pluginREADME.md(line 12) which still documentsbundles/<agent-id>.tar.gz. - The singleton's reuse guard keys on
bundle_urlrather thanagent_id, which is the field that actually selects the policy bundle — fine under one-agent-per-process, but worth making the invariant explicit so a future multi-agent process fails loud.
Dependency relaxation (ordering vs jwt-validation/parsers) and the ready-goroutine leak are softer notes inline.
Areas reviewed: Go (plugin + tests), docs · Commits: 1, signed-off ✅ · CI: all green
Conventions: title feat(opa): is lowercase — the org rule (embedded in your PR template) requires exact-case Feat; and the body has no ## Summary heading. Minor, but the title may block at the ruleset.
No code blockers under the current deployment model — approving, contingent on confirming the bundle-server contract.
Assisted-By: Claude Code
| "authz": map[string]any{ | ||
| "service": "kagenti", | ||
| "resource": fmt.Sprintf("bundles/%s.tar.gz", escapedAgentID), | ||
| "resource": fmt.Sprintf("bundles?spiffe=%s", escapedSPIFFEID), |
There was a problem hiding this comment.
suggestion (verify before merge) — The bundle resource changes from bundles/<id>.tar.gz to bundles?spiffe=<url-encoded-id>. This is a contract change with the Kagenti Bundle Server (a separate component). If the server isn't already serving the query form, every bundle download 404s → OPA never becomes Ready → OnRequest/OnResponse deny everything. Please confirm the server supports bundles?spiffe= before merge.
Also: the plugin's own README.md:12 still documents bundles/<agent-id>.tar.gz — update it to match the new path.
(Minor: a ? embedded in OPA's bundle resource field is unusual — worth a sanity check that the OPA SDK forwards it to the service URL un-re-escaped.)
There was a problem hiding this comment.
The contract indeed has intentionally changed.
The Readme need to be updated.
| defer singletonMu.Unlock() | ||
|
|
||
| if singleton != nil { | ||
| if singleton.bundleURL != p.cfg.BundleURL { |
There was a problem hiding this comment.
suggestion — This reuse guard compares bundleURL, but the value that actually selects the policy bundle is the agent identity (bundles?spiffe=<agentID>), and different agents share the same bundle_url (the service base). So for a same-URL/different-agent case this warning never fires and the second instance silently reuses the first agent's SDK — meaning agent B would be evaluated against agent A's policy.
That's safe today (one agent per AuthBridge process; inbound + outbound share one agentID), but the guard is checking the wrong field for the invariant it protects. Suggest keying on agentID (or asserting it matches) so that if a process ever serves more than one agent, it fails loud rather than cross-applying policy.
There was a problem hiding this comment.
Seems that we need to keep it as is - we cant have two different urls on the config. This is what we protect here.
The entire design is for a single identity.
Agent id is not part of the config, it comes from the client id file - there is one file for one agent and therefore one id which is used for both upstream and downstream. This is what we support for now and if this changes, we ned to rethink the OPA singleton use and other aspects of the design.
| @@ -108,8 +189,6 @@ func (p *OPA) Name() string { return "opa" } | |||
|
|
|||
| func (p *OPA) Capabilities() pipeline.PluginCapabilities { | |||
There was a problem hiding this comment.
suggestion — Dropping Requires: [jwt-validation] and RequiresAny: [parsers] removes the pipeline's guarantee that identity claims and parsed request data are populated before OPA evaluates. A pipeline without jwt-validation was previously rejected; now OPA runs with missing claims in buildInput, which can silently weaken policy decisions. Worth confirming the default pipeline still orders jwt-validation/parsers ahead of opa, and that policies tolerate absent claims defensively.
There was a problem hiding this comment.
OPA is used also in outbound where jwt-validationis missing
OPA also rely on a2a parser and mcp parser etc. but non is mandatory.
Use cases where OPA is needed although non of the above are used is not impossible.
Therefore failing the agent due to a pipeline config that has OPA without specific other plugins seems counter productive. OPA should use what it has. Rego inside may fail requests or responses inbound or outbound if info is missing on the context (leading to missing info on the input document. This is left for the admins to decide instead of failing the agent and not allowing it to start.
| s.ready.Store(true) | ||
| slog.Info("opa: bundle loaded and policy activated", "agent_id", agentID) | ||
| return | ||
| case <-time.After(30 * time.Second): |
There was a problem hiding this comment.
nit — This readiness goroutine isn't tied to Shutdown/bgCancel. If the plugin shuts down before the bundle becomes ready, the trailing <-readyCh blocks forever and the goroutine leaks. Consider selecting on a cancel/done channel alongside readyCh.
|
@davidhadas we can merge now or would you like to make some changes based on the reviews? Will wait until next week in case you want to make some changes. |
|
Let me fix the readme and readiness goroutine |
…downloads
Previously, the OPA plugin created separate SDK instances for inbound and
outbound pipelines, resulting in:
- Two bundle downloads from the bundle service
- Duplicate memory usage for policy storage
- Redundant OPA SDK initialization logs
This commit introduces a process-wide singleton pattern that shares a single
OPA SDK instance across both pipelines when they use the same bundle_url and
agent_id.
Key changes:
1. **Singleton OPA SDK Management**
- Added `sharedSDK` struct with reference counting
- Package-level `singleton` variable protected by `singletonMu` mutex
- First plugin instance creates the SDK, subsequent instances reuse it
- SDK is stopped only when the last reference is released
2. **OPA Logging Bridge**
- Added `slogBridge` to integrate OPA SDK logs into authbridge's slog
- OPA bundle downloads, parse errors, and activation now visible in logs
- Tagged with "component: opa-sdk" for easy filtering
3. **SPIFFE ID Encoding**
- Changed bundle resource path from `bundles/{agent-id}.tar.gz` to
`bundles?spiffe={url-encoded-spiffe-id}`
- Properly URL-encodes SPIFFE IDs (slashes become %2F)
- Strips "spiffe://" prefix before encoding
- Supports both SPIFFE ID and legacy agent ID formats
4. **Dependency Relaxation**
- Removed hard `Requires: ["jwt-validation"]` dependency
- Removed `RequiresAny: ["a2a-parser", "mcp-parser", "inference-parser"]`
- OPA can now run independently without requiring other plugins
5. **Test Infrastructure**
- Added `resetSingleton()` for test isolation
- Added `newTestOPA()` helper to simplify test setup
- New tests for singleton reuse, ref counting, and shutdown behavior
- Updated bundle resource path tests for SPIFFE ID encoding
Benefits:
- Single bundle download per agent (50% reduction in network traffic)
- Single OPA SDK instance (50% reduction in memory usage)
- Cleaner logs with OPA SDK messages properly integrated
- More flexible plugin ordering (no hard dependencies)
Signed-off-by: David Hadas <david.hadas@gmail.com>
ce9910d to
3ada324
Compare
davidhadas
changed the title
github-project-automation
Bot
moved this from New /:ToDo
to Done
in Rossoctl Issue Prioritization
3 participants
Previously, the OPA plugin created separate SDK instances for inbound and outbound pipelines, resulting in:
This commit introduces a process-wide singleton pattern that shares a single OPA SDK instance across both pipelines when they use the same bundle_url and agent_id.
Key changes:
Singleton OPA SDK Management
sharedSDKstruct with reference countingsingletonvariable protected bysingletonMumutexOPA Logging Bridge
slogBridgeto integrate OPA SDK logs into authbridge's slogSPIFFE ID Encoding
bundles/{agent-id}.tar.gztobundles?spiffe={url-encoded-spiffe-id}Dependency Relaxation
Requires: ["jwt-validation"]dependencyRequiresAny: ["a2a-parser", "mcp-parser", "inference-parser"]Test Infrastructure
resetSingleton()for test isolationnewTestOPA()helper to simplify test setupBenefits:
Summary by CodeRabbit
New Features
Chores