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).