Feat: AIAC OPA plugin integration + live enforcement (phase 2, PR 1) - #752
Feat: AIAC OPA plugin integration + live enforcement (phase 2, PR 1)#752oblinder wants to merge 41 commits into
Conversation
Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
Define a new ninth component in the Policy/Domain Knowledge RAG Pod: a pre-flight, fail-closed verification gate between the RAG Ingest Service and ChromaDB. Its concrete check set is left TBD; this change fixes the component's architectural placement and its interoperability contract with the RAG Ingest Service and ChromaDB (pod-local only, one call per document, all-or-nothing rejection, no Event Broker interaction). Signed-off-by: Oleg Blinder <oblinder@gmail.com>
…config - CLAUDE.md: issue-tracking section describes the GitHub issues/AIAC Project layout (no migration history); adds an '## Agent skills' block wiring the Matt Pocock engineering skills. - .gitignore: drop obsolete docs/issues/ and docs/gh-issues/ entries. - docs/agents/: issue-tracker, triage-labels, and domain config the skills read from. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
…config - CLAUDE.md: issue-tracking section describes the GitHub issues/AIAC Project layout (no migration history); adds an '## Agent skills' block wiring the Matt Pocock engineering skills. - .gitignore: drop obsolete docs/issues/ and docs/gh-issues/ entries. - docs/agents/: issue-tracker, triage-labels, and domain config the skills read from. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
…c-phase2 Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
Phase 03a of the Policy Store rename: retitle the SQLite-backed structured
policy service to Policy Model Store across docs/specs/** only, freeing the
aiac-policy-store name for reassignment to ChromaDB (Handoff 04).
- Display name Policy Store -> Policy Model Store
- aiac-policy-store{,-service,-config} -> aiac-policy-model-store*
- AIAC_POLICY_STORE_URL -> AIAC_POLICY_MODEL_STORE_URL
- aiac.policy.store[.library] -> aiac.policy.model_store[.library]
- Dockerfile path policy/store/service -> policy/model_store/service
- k8s manifest policy-store-statefulset.yaml -> policy-model-store-statefulset.yaml
- Rename component spec files policy-store.md, library-policy-store.md and
repoint inbound links
- Drift fixes: AGENTPOLICY_DB_PATH -> SERVICEPOLICY_DB_PATH,
/data/state.db -> /data/policy_model.db
Code, manifests, tests, and both CLAUDE.md files are intentionally left on
the old names until phase 03c.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Replace the enumerated source-tree, Docker-image table, and volume-service list with ls/find/grep discovery guidance, keeping only conceptual prose, patterns, and commands. This also removes the last references to the old policy-store name from aiac/CLAUDE.md. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Renames the SQLite-backed policy-model service from aiac-policy-store to aiac-policy-model-store across code, tests, manifests, Docker image, and demo/integration targets, freeing the old name/key/filename for ChromaDB in Handoff 04. - Python package aiac.policy.store -> aiac.policy.model_store (+ tests) - Env key AIAC_POLICY_STORE_URL -> AIAC_POLICY_MODEL_STORE_URL - k8s manifest policy-store-statefulset.yaml -> policy-model-store-statefulset.yaml (all identifiers; SERVICEPOLICY_DB_PATH and securityContext preserved) - Image aiac-policy-store -> aiac-policy-model-store; Dockerfile moves with package - Demo/integration svc target aiac-policy-model-store-service - PRB import-isolation FORBIDDEN guard repointed to aiac.policy.model_store.library Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
… diagram Signed-off-by: Oleg Blinder <oblinder@gmail.com>
…tract, brand, repo paths) Apply the kagenti->rossoctl rebrand driven by the real infra/branding change: - Keycloak realm default kagenti -> rossoctl - Operator contract strings: agent.kagenti.dev -> agent.rossoctl.dev; labels kagenti.io/* -> rossoctl.io/* and protocol.kagenti.io/* -> protocol.rossoctl.io/*; credentials secret prefix, operator name, kind cluster name - Platform-brand prose "Kagenti ..." -> "Rossoctl ..." - Monorepo rename: kagenti-extensions/ paths -> cortex/; MCP link URL -> rossoctl/cortex Preserves genuine upstream references: the Kagenti Developer Guide, github-org sample data in demo prompts, Keycloak test fixtures, and historical PR markers. Unit tests green (466 passed, 155 deselected). Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Complete the kagenti→rossoctl rebrand by normalizing the arbitrary Keycloak-payload fixture data in TestKeycloakRealWorldPayloads that the mechanical rename pass deliberately skipped (alice@kagenti.org, lastName "Kagenti", role kagenti-admin, and a stale docstring). These are round-trip parsing fixtures, so the literal value is arbitrary and the change is behaviour-preserving; the same test class already used containerId "rossoctl". grep -rni kagenti src/ test/ is now clean; genuine upstream carve-outs (github owner=kagenti in test_prereq.py, the Kagenti Developer Guide link) are untouched. Unit suite: 466 passed, 155 deselected. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com> # Conflicts: # aiac/docs/specs/PRD.md # aiac/k8s/aiac-deployment-guide.md
The deployment guide had five stray "Policy Store" references (build/deploy/ verify comments and the env-var table) that predated the Policy Store -> Policy Model Store rename. Align them with the rest of the docs, which already use "Policy Model Store" and the aiac-policy-model-store image/service names. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Move useradd ahead of the COPY steps and add --chown=10001:10001 to each COPY so application files are owned by the non-root aiac user instead of root. pip install still runs as root to write into system site-packages. Applies to the controller, idp/keycloak, pdp/keycloak, pdp/opa, model_store, and demo github_tool images. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Add a stateless GET /health liveness/readiness endpoint to the
Controller API, returning 200 {"status": "ok"}. The Controller holds
no local state and opens no connection at rest, so /health is a bare
process-liveness signal; upstream reachability stays validated
per-request by the handlers.
- routes.py: new GET /health handler.
- test_routes.py: unit test asserting 200/body and that no handler or
PCE is dispatched.
- agent-deployment.yaml: switch readiness+liveness probes from tcpSocket
to httpGet /health.
- integration (uc1_onboard.py): poll /health as the Controller
port-forward ready_url; fix stale 'no /health' comment in launcher.py.
- demo (03-onboard-agent.py, 04-onboard-tool.py): pass ready_url=/health
to the Controller port-forward.
- aiac-agent.md: document GET /health in the Endpoints table.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
install.sh's load_image_to_kind() set a `trap ... RETURN` to clean up a temp tar file, but RETURN traps in bash aren't scoped to the function that set them -- they fire on every subsequent function return until cleared. After the tool image loaded, the stale trap fired again on build_and_load's return and referenced tar_file outside its scope, failing with "unbound variable" under set -u. Replaced the trap with a direct rm -f after the kind load call. Also updated INSTALL.md's verification snippet to port-forward on 18080 instead of 8080, since a rossoctl-installed Kind cluster already binds host port 8080 to the Gateway. Signed-off-by: Oleg Blinder <oblinder@gmail.com>
The Policy Rules Builder built ChatOpenAI with no request timeout, so a stalled LLM socket never raised and POST /apply/service wedged forever. Even with a timeout, openai raises APITimeoutError/APIConnectionError, whose class names were not in is_transient()'s recognized set, so a timed-out call would surface as a hard error instead of being retried. - graph.py: _build_llm() now passes timeout (from LLM_REQUEST_TIMEOUT, default 120s, tolerant of unset/bad values) and max_retries=0 so the tenacity Retrying in _structured_call is the sole retry owner. - shared/upstream.py: is_transient() recognizes APITimeoutError and APIConnectionError by name (no openai import; stays transport-agnostic). - k8s/agent-deployment.yaml: expose LLM_REQUEST_TIMEOUT and UPSTREAM_MAX_RETRIES in the aiac-agent-config ConfigMap. - tests: timeout/connection errors classified transient, retried then reraised, and _build_llm sources timeout from env. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Add init/00-discover-keycloak.sh: a sourceable script that port-forwards the in-cluster Keycloak and exports KEYCLOAK_URL + admin creds from the keycloak-admin-secret, so the demo targets no longer require the caller to export those by hand. Renumber the init/onboard scripts into one 00-05 sequence and rework the Makefile: SHELL=bash, a KC_ENV self-source prefix on every Keycloak-touching recipe (make can't propagate env across recipes), a new 'keycloak' target, renamed onboard-agent/onboard-tool to agent/tool, and grouped phase targets init (00-03) / onboard (04-05) / run, with demo now chaining init -> onboard -> run. Update demo.md to match. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Wave-1 PDP Policy Writer changes (handoffs 01 and 04):
- rego.py: replace slugify with identity_ref; emit fixed
authbridge.client.{inbound,outbound}.request packages matching the
live AuthBridge OPA plugin input shape (input.identity.*,
input.mcp.params.name); de-prefix outbound scope values while
keeping full SPIFFE target keys.
- Remove the superseded Keycloak composite-role writer
(src/aiac/pdp/service/policy/keycloak/ + its tests + component
spec) and fix every dangling reference across the PRD, specs, and
the k8s deployment guide.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Rewrite the OPA policy writer from a filesystem stub into an always-on writer of per-agent AuthorizationPolicy Custom Resources (agent.rossoctl.dev/v1alpha1) on the live Kubernetes API via server-side apply. Metadata name/namespace derive from identity_ref; bundle-service composes these CRs into per-pod OPA bundles. The rego dump to REGO_OUTPUT_DIR is now purely additive local-debug output, gated by POLICY_WRITER_DUMP_REGO (default off); it never disables, replaces, or gates the CR write. Error mapping: malformed agent_id -> 400, Kubernetes API failure -> 502, delete of an absent CR -> 204, health -> 200/503. Add the kubernetes client dependency to requirements.txt. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Finish the wave-3 handoffs of the OPA policy-writer rework:
- 03 (k8s): add aiac-pdp-policy-writer ServiceAccount + cluster-scoped
RBAC (get,list,create,update,patch,delete on authorizationpolicies,
no watch); wire the SA into the aiac-interface pod; turn the prod
rego dump off (drop REGO_OUTPUT_DIR + /rego mount + rego-output
volume, keep read-only rootfs + /tmp); add PLATFORM_SOURCE_CLIENTS.
- 07 (tests): rewrite the OPA writer unit tests for identity_ref, the
fixed authbridge.client.{inbound,outbound}.request packages, nested
input.identity/input.mcp shape, rossoctl platform bypass, de-prefixed
outbound scopes, and the always-on CR writer (SSA args, delete-by-
label, delete-404 idempotency, batch-400, /health, dump-toggle).
- 09 (demo): update uc1-onboarding to the new packages/input shape and
de-prefixed outbound scopes; source rego from the AuthorizationPolicy
CR (spec.policies[].content) via the nested ns/name layout.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Match the spec to the current CR-writer implementation: fixed package
names (authbridge.client.{inbound,outbound}.request) + import rego.v1,
input.identity.* + input.mcp.params.name input shape, per-agent
AuthorizationPolicy CR (server-side apply, delete-by-label), RBAC/auth
model, PLATFORM_SOURCE_CLIENTS / POLICY_WRITER_DUMP_REGO / REGO_OUTPUT_DIR
config, always-on CR write + additive dump, and the Keycloak-writer
removal. Both embedded Rego blocks now match docs/examples/opa-team1-policy.yaml.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
The UC1 suite resolves pods by app.kubernetes.io/name=<workload>, but the checked-in github-tool manifests carried only the legacy app: github-tool label. From a clean deploy the tool-pod selector found nothing, so the suite reported the pipeline unwired and skipped the outbound leg (it only passed on the live cluster because the label was added out-of-band). Add app.kubernetes.io/name: github-tool to the pod template labels in both manifests, mirroring github-agent. The label is intentionally not added to spec.selector.matchLabels (immutable after creation) so in-place kubectl apply keeps working; the existing app: github-tool selector is untouched. Refs: #138 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
The onboarded_stack timeout branch raised only the classified
outbound_outcome ('error'), which collapses two distinct failures: a
token-exchange 503 (OPA never consulted) vs a 200 + OPA error frame (a
real policy stall). Re-probe on timeout and include the raw HTTP status
+ body so a stalled run self-identifies without a rerun (issue #139).
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
test_uc1_onboard_agent_then_tool reproducibly timed out at BUNDLE_TIMEOUT
(300s): the outbound source-read decision stayed "error" for the whole poll
window while both inbound signals converged.
Root cause is a test-harness pod-resolution race, not a product bug. Part B's
restart_agent does a RollingUpdate (replicas=1, terminationGracePeriodSeconds=30),
so the new pod is Ready before the old pod finishes terminating. resolve_pod
returned jsonpath={.items[0].metadata.name} with no phase/ready filter and no
ordering, so it could pin the Terminating pod. Once that pod was gone, every
outbound kubectl exec failed NotFound -> HTTP None -> classified "error" ->
never converged. Inbound was unaffected because inbound_probe reaches the agent
through its Service, which is pod-agnostic.
- resolve_pod now returns the newest live pod (Running+Ready, non-terminating),
with the selection factored into a pure, unit-testable select_live_pod().
- outbound_decision and the timeout re-probe re-resolve the agent pod per probe
(resolve_agent_pod) instead of trusting the once-pinned ctx["agent_pod"], so
later pod churn self-heals the way inbound already does.
- Diagnostic sharpened to separate the three "error" causes: code=None +
"exec failed" = stale/gone pod (harness); 503 = token-exchange down (OPA not
reached); a real policy stall reads "deny", never "error".
- New test_launcher_select_live_pod.py pins the race cluster-free (terminating
pod never chosen, order-independent).
Verified: test_uc1_onboard_agent_then_tool.py -m integration now 33 passed /
0 errors (was 16 passed / 17 errors); non-integration suite 472 passed; new
unit test 5 passed.
Fixes #139.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
|
Important Review skippedToo many files! This PR contains 119 files, which is 19 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (119)
You can disable this status message by setting the 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 |
abigailgold
added
the
ready-for-ai-review
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Adds OPA outbound delegation signals (Phase 2): input.delegation + synthesized input.identity on the outbound leg via token-exchange hop recording, with a live K8s CR writer for the PDP policy service replacing the filesystem stub. CI passes, all DCO signed. One bug found in the dev script overlay.
Findings:
-
[BUG]
scripts/opa-kind-enable.sh:bundle_urlhardcoded torossoctl-system— the previous overlay used${RELEASE_NAMESPACE}(shell-expanded in the here-doc), which allowedRELEASE_NAMESPACEoverrides to work. The new YAML block hardcodesrossoctl-system, silently breaking the overlay wheneverRELEASE_NAMESPACEdiffers from the default. Restore${RELEASE_NAMESPACE}in bothbundle_urllines. -
[NOTE]
subjectFromTokenusesjwt.ParseInsecure— intentional by design (outbound leg has no JWKS trust anchor; the field is observability-only, not an auth decision). Well-documented in the function godoc and tests. Worth a brief inline acknowledgement that this is NOT used for authorization.
Reviewed by clawgenti using the github-pr-review skill
Address PR rossoctl#752 review feedback: - opa-kind-enable.sh hardcoded the bundle_url namespace to rossoctl-system on both OPA legs, silently breaking the RELEASE_NAMESPACE override. Restore ${RELEASE_NAMESPACE} (the heredoc is unquoted, so it expands). - Add an inline comment at the jwt.ParseInsecure call site noting the decoded subject is used for delegation provenance only, never for an auth decision. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
This PR wires AIAC into the live OPA enforcement loop: OPA policy-writer→CR, outbound delegation identity surface in input, and the Kind integration test suite. All CI checks pass (CodeQL, Bandit, Hadolint, Go/Python/Shell lint, DCO — 38/38 commits signed). No .claude/.vscode changes.
- Suggestion:
mint_token()inscripts/opa-kind-driver.shusespassword=${user}(password == username) — appropriate for the local Kind dev cluster and clearly documented, but a comment guarding it from production use would add clarity. - Nit: In the generated outbound Rego,
agent_role_scopesis emitted but explicitly not referenced byallow; a brief in-Rego comment (# informational / debugging only — not part of allow) would prevent future policy authors from wondering why it's there.
Reviewed by clawgenti using the github-pr-review skill
| mint_token() { | ||
| local user="$1" resp tok err | ||
| resp=$(curl -s -X POST "${KC}/realms/${REALM}/protocol/openid-connect/token" \ | ||
| -d client_id=rossoctl -d "username=${user}" -d "password=${user}" \ |
There was a problem hiding this comment.
suggestion: password=${user} is the right pattern for this Kind dev cluster (credentials == username per the runbook), but it's easy to cargo-copy this helper into a staging or production script. A one-liner guard — e.g. # DEV ONLY: password == username; see opa-kind-runbook.md prerequisites — right before the curl call would make the intent explicit and reduce copy-paste risk.
There was a problem hiding this comment.
Done in 2233b70 — added a DEV ONLY guard comment right before the curl grant in mint_token, spelling out that password == username is valid only for this Kind dev cluster's seeded users (per the runbook prerequisites) and must not be cargo-copied into a staging/production script. Thanks.
| _group_rules_deprefixed(model.outbound_subject_rules), | ||
| ), | ||
| _render_map( | ||
| "agent_role_scopes", _group_rules_deprefixed(model.outbound_rules) |
There was a problem hiding this comment.
nit: agent_role_scopes is rendered here but explicitly not used in allow (the docstring notes "informational / debugging"). A short inline comment in the generated Rego template — e.g. # informational only; not referenced by allow right after the variable is rendered — would prevent future policy authors from hunting for where agent_role_scopes feeds into the decision and assuming it is accidentally omitted.
There was a problem hiding this comment.
Done in 2233b70 — the generated outbound Rego now emits # informational/debugging only — not referenced by allow immediately before agent_role_scopes, so a future policy author reading the bundle sees plainly that it feeds logging/debugging only and that target_scopes[input.identity.service_id] is the capability gate.
|
Few more small comments: Must-fix:
Nits:
|
Address PR rossoctl#752 review feedback: - opa-kind-enable.sh hardcoded the bundle_url namespace to rossoctl-system on both OPA legs, silently breaking the RELEASE_NAMESPACE override. Restore ${RELEASE_NAMESPACE} (the heredoc is unquoted, so it expands). - Add an inline comment at the jwt.ParseInsecure call site noting the decoded subject is used for delegation provenance only, never for an auth decision. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
…client) - opa-kind-driver.sh: guard mint_token's password==username grant with a DEV ONLY comment so it is not cargo-copied into staging/production. - rego.py: emit a Rego comment before agent_role_scopes noting it is informational/debugging only and not referenced by allow. - opa/requirements.txt: pin the kubernetes client to the tested major (>=36.0.3,<37) now that the writer mutates cluster state. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
|
@clawgenti thanks for the review — both points are now addressed in 2233b70:
Non-integration suite stays green (472 passed). |
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Large, well-structured PR delivering OPA plugin integration and live enforcement across the AIAC stack. All CI checks pass, all 39 commits carry DCO sign-off, and no agent/IDE config files are touched.
One minor documentation nit on the new driver script (see inline); everything else looks solid.
Reviewed by clawgenti using the github-pr-review skill
| } | ||
|
|
||
| # admin_token — realm master admin token for Keycloak admin API (B.2) | ||
| admin_token() { |
There was a problem hiding this comment.
nit: admin_token() hardcodes admin/admin for the Keycloak master realm, but unlike mint_token() (which has a # DEV ONLY: password == username … guard comment at line 122–124) this function has no equivalent caveat. Recommend adding a short # DEV ONLY: admin/admin is the seeded Kind cluster default — never copy into staging/prod note here for consistency and to protect against cargo-copying.
There was a problem hiding this comment.
Done in ba30155 — added a DEV ONLY guard comment right before the curl grant in admin_token(), mirroring mint_token()'s existing caveat: warns that admin/admin is the seeded Kind cluster default and should never be cargo-copied into a staging/production script.
|
@abigailgold thanks — responding to all three. Must-fix: off by default / least-intrusiveWe were asked to be as least-intrusive as possible — i.e. any new feature should be turned off by default. We're contributing three things in this PR:
Nit:
|
Address PR rossoctl#752 review feedback: - opa-kind-enable.sh hardcoded the bundle_url namespace to rossoctl-system on both OPA legs, silently breaking the RELEASE_NAMESPACE override. Restore ${RELEASE_NAMESPACE} (the heredoc is unquoted, so it expands). - Add an inline comment at the jwt.ParseInsecure call site noting the decoded subject is used for delegation provenance only, never for an auth decision. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Oleg Blinder <oblinder@gmail.com>
Move aiac/docs/opa-kind-runbook.md and scripts/opa-kind-{enable,restore,driver}.sh
into aiac/k8s/, and update every reference to their old locations across
CLAUDE.md, the runbook itself, and the integration test suite.
Signed-off-by: Oleg Blinder <oblinder@gmail.com>
admin_token() hardcodes the Keycloak master realm admin/admin credentials with no caveat, unlike mint_token()'s existing DEV ONLY guard. Add the same warning so the seeded Kind-cluster default is never cargo-copied into a staging/production script. Signed-off-by: Oleg Blinder <oblinder@gmail.com>
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Substantial, well-structured PR: the OPA outbound identity synthesis, delegation chain exposure, CR-writer lift, and Rego generator rework all hang together coherently and the test coverage is solid. A couple of small gaps worth noting.
Findings:
plugin_test.goseveralbuildInputcall sites (e.g. lines 344, 382, 405, 426, 453) pass""asagentID— whenpctx.Extensions.Delegationis non-nil those tests will get a synthesizedinput.identity["client_id"] = "", but none of them assert on that field. Add at least a nil-check or document the empty-string contract.rego.pysource_ok if { not input.identity.client_id }— on the outbound legbuildOutboundIdentityalways emitsclient_id(even whenagentIDis""), so this bypass rule will never fire outbound. On inbound it fires only when the OPA plugin has no validated JWT. Worth a short comment clarifying the intended trigger surface so future authors don't assume it covers the outbound pass-through case.
Reviewed by clawgenti using the github-pr-review skill
| Host: "github-tool-mcp", | ||
| Headers: http.Header{}, | ||
| } | ||
| input := buildInput(pctx, inc, "") |
There was a problem hiding this comment.
This call passes "" as agentID. If pctx.Extensions.Delegation were non-nil here, buildOutboundIdentity would emit "client_id": "" — an observable but untested behavior. The delegation tests below (e.g. TestBuildInput_WithDelegation) also pass "" and don't assert on client_id. Consider adding an assertion on client_id in the delegation-present tests, or explicitly documenting that "" is the expected value when no agentID is configured.
| subject holds a role granting >=1 of ``agent_scopes``) AND ``source_ok``. | ||
|
|
||
| ``source_ok`` passes when there is no calling ``client_id`` (end-user | ||
| traffic), when the ``client_id`` is one of ``platform_clients`` (the |
There was a problem hiding this comment.
source_ok if { not input.identity.client_id } — on the outbound leg buildOutboundIdentity always populates client_id (even as "" when agentID is unset), so not input.identity.client_id will never be true outbound. On inbound it fires only when no JWT was validated. A short comment here explaining the intended trigger (inbound-only, unauthenticated callers) would prevent future readers from assuming this covers outbound pass-through traffic.
5 participants
Summary
First PR in a series toward #646 (AIAC phase 2 — platform integration: live onboarding trigger + live enforcement). This PR delivers OPA plugin integration within the AIAC system plus live enforcement: AIAC now publishes the rules it generates to the enforcement layer's policy resource, and real requests are allowed/denied by the deployed AuthBridge OPA plugin.
Later PRs in the series will cover the remaining #646 scope (e.g. the live IdP-event onboarding trigger path).
What's in this PR
PDP OPA policy writer → live enforcement
AuthorizationPolicyCustom Resources (agent.rossoctl.dev/v1alpha1) on the live Kubernetes API via server-side apply; the bundle-service composes these CRs into per-pod OPA bundles the plugin loads.authbridge.client.{inbound,outbound}.requestpackages matching the live AuthBridge OPA plugin input shape (input.identity.*,input.mcp.params.name), with de-prefixed outbound scope values.REGO_OUTPUT_DIRis now purely additive local-debug output, gated byPOLICY_WRITER_DUMP_REGO(default off); it never gates the CR write. Error mapping: malformedagent_id→ 400, k8s API failure → 502, delete of an absent CR → 204.AuthBridge OPA plugin + token exchange
Kubernetes wiring
aiac-pdp-policy-writerServiceAccount + cluster-scoped RBAC (get,list,create,update,patch,deleteonauthorizationpolicies, no watch) and wired the SA into the aiac-interface pod.REGO_OUTPUT_DIR+/regomount), kept read-only rootfs +/tmp; addedPLATFORM_SOURCE_CLIENTS.Integration tests — real OPA loop
opa eval/ probe model (opano longer needed on PATH). Suite skips cleanly when the cluster is not wired.scripts/opa-kind-{enable,disable,driver,restore}.shto stand up / tear down the OPA pipeline on Kind.Docs
pdp-policy-writer-opaspec to match the CR-writer implementation.Supporting refactors carried in this branch
Testing
.venv/bin/pytest test/ -m "not integration"— green.-m integration) close the real OPA loop against a live rossoctl/Kind cluster with the AuthBridge OPA pipeline wired in (seedocs/opa-kind-runbook.md); they skip cleanly when the cluster/env is absent.Related
Part of #646. Sub-issue of the AIAC epic #643; depends on phase 1 (#645).
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com