Refactor Keycloak direct-access-grants into shared helper; harden eval port-forwards; add cluster/dry flags to ibac comparison

[yoavkatz]

Summary

• Extract shared Keycloak helper. The "enable Direct Access Grants for the rossoctl client" logic was duplicated across deploy-agent.sh and deploy-benchmark.sh. It now lives in a new libsh/keycloak-direct-access.sh as enable_direct_access_grants, which resolves master-realm admin creds from env vars → the keycloak-initial-admin secret → admin/admin defaults, and exits with a clear diagnostic on any failure.
• Fix delete-all-deployments.sh. Its token requests use grant_type=password against the rossoctl client, which requires Direct Access Grants to be enabled first. It now sources the helper and calls it before requesting a token.
• Harden evaluate-benchmark.sh port-forwards. Before binding a local port, detect a stale (or suspended) leftover kubectl port-forward holding it and clean it up — refusing to touch any non-kubectl process. Port-forward output is captured to a temp file so the real error is surfaced when startup fails; temp files are removed on cleanup.
• Add cluster/dry flags to run-ibac-comparison.sh. New --kind / --openshift DOMAIN / --in-cluster cluster flags and a --dry run mode, all forwarded verbatim to deploy-and-evaluate.sh (which already validates and supports them).

Testing

• bash -n syntax-checks pass on all six changed scripts.
• Verified downstream deploy-and-evaluate.sh accepts the forwarded --dry, --kind, --openshift, and --in-cluster flags.

[kellyaa]

Tested this on a local Kind cluster (kind-rossoctl) against a live Keycloak. All four changes work as described, and the core fix is verifiably real. One bug worth fixing before merge, plus two minor notes.

The core fix is confirmed

I proved the delete-all-deployments.sh fix with a before/after against the same cluster state — set directAccessGrantsEnabled=false on the rossoctl client, then ran both versions:

• main's version → ⚠ Warning: Fetched password from cluster but authentication failed, exit 1
• this branch → enables the grant, obtains a token, exit 0

So the password-grant prerequisite really was broken on main, and this fixes it.

What else I verified

Keycloak helper — works against live Keycloak, and the flag genuinely persists (checked via the admin API, not just an HTTP 200 on the PUT). All three failure paths exit 1 with a useful diagnostic: wrong admin password, nonexistent client, unreachable Keycloak.

Port-forward hardening — both branches behave correctly:

• a real stale kubectl port-forward holding the port → detected, killed, port freed
• a non-kubectl process holding the port (a Python listener) → refused to kill it, exited 1 and printed the offending process. The safety guard does what it claims.

--dry / cluster flags — dry run completes and flags forward correctly. Confirmed delete-all-deployments.sh does accept --kind/--openshift, so that forwarding is valid.

Refactor fidelity — bash -n clean on all 7 shell scripts, py_compile clean on the Python. The extracted helper is a faithful superset of the two copies it replaces: it adopts deploy-benchmark.sh's stronger version, so deploy-agent.sh picks up credential resolution and real error handling it previously lacked (it used to silently swallow every failure). The single shellcheck warning in evaluate-benchmark.sh (SC2155) is pre-existing and unrelated.

Bug: analyze-run.sh doesn't receive the cluster flag

run-ibac-comparison.sh forwards CLUSTER_FLAG to delete-all-deployments.sh and deploy-and-evaluate.sh, but not to the final analyze-run.sh call. analyze-run.sh does accept cluster flags and defaults to kind when none is given (analyze-run.sh:146-148), so with --openshift DOMAIN the comparison step will target a Kind-style MLflow URL and fail — after both benchmark runs have already completed, which is the expensive way to find out.

Dry run with --openshift apps.mycluster.example.com shows the flag missing:

[DRY RUN] Would execute:
.../analyze-run.sh -c gsm8k-10-parallel-1-cejqm-ibac\,gsm8k-10-parallel-1-cejqm

One-line fix:

run_step "$SCRIPT_DIR/analyze-run.sh" -c "${EXPERIMENT_PLUGIN},${EXPERIMENT_BASE}" ${CLUSTER_FLAG[@]+"${CLUSTER_FLAG[@]}"}

This is invisible on kind, which is probably why it slipped through — the testing notes are syntax-checks only, no cluster run.

Minor notes

1. Misleading diagnostic when Keycloak is unreachable. A connection failure prints Set KEYCLOAK_ADMIN_PASSWORD in your .env..., blaming credentials for what is actually a network problem:

   Error: Could not obtain master-realm admin token from Keycloak
     Response:
     Set KEYCLOAK_ADMIN_PASSWORD in your .env if the master realm admin password is not 'admin'.
   

   Worth distinguishing an empty response (unreachable) from a rejected one (bad creds).

2. analyze-run.sh (+123/-26) and download_mlflow_traces.py aren't mentioned in the PR description — together that's the largest diff here. The change itself looks good: a distinct exit code 75 for "MLflow rejected the token", raised via MLflowAuthError and handled at analyze-run.sh:514, with urllib.error correctly imported. Worth adding to the summary so reviewers know to look at it.

One caveat on my testing

My keycloak-initial-admin secret happens to contain admin/admin*, so the secret-fallback path and the hardcoded admin/admin default are indistinguishable on this cluster. I confirmed the env-var override path works, but the secret-resolution path is not independently proven — that needs a cluster with a non-default admin password (e.g. a real RHBK deployment).

[clawgenti]

This PR introduces significant improvements to MLflow authentication, error handling, and script robustness within the A2A runner. It also adds better cluster deployment options. All checks pass. Ready for human review.

Reviewed by clawgenti using the github-pr-review skill