Repository navigation
fix(test): system tests set up Direct Access Grants themselves, no demo/ dependency #238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
0ed16f6
c15c693
a589c4f
3c34d8f
4f8043b
b2af90d
519ca7f
942730b
47c184e
6a82248
d6ca1cc
97dbbf8
8a0071a
5f4ce92
0a9e881
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -221,6 +221,41 @@ such pods, it skips this check. | |
|
|
||
| --- | ||
|
|
||
| ## Direct Access Grants on the login client | ||
|
|
||
| A manual probe that mints a user token with the password grant through the | ||
| login client `rossoctl` needs Direct Access Grants on that client. In a fresh | ||
| realm it is off, and the mint fails with `unauthorized_client`. The system | ||
| tests do not need this step: their fixture enables the grant itself | ||
| (`ensure_direct_access_grants` in `test/system/uc1_onboard.py`) and turns it off | ||
| again at teardown if it was off. To enable it by hand (a partial update: only | ||
| this field changes), source the repo-root `.env` first. The snippet uses the | ||
| same variables as the harness: | ||
|
|
||
| ```bash | ||
| ( set -e # a subshell: a failed check stops the snippet, not your shell | ||
| set -a; . .env; set +a | ||
| KC=${KEYCLOAK_URL:?}; REALM=${AIAC_TEST_REALM:-rossoctl} | ||
| ADMIN=$(curl -s -X POST "$KC/realms/master/protocol/openid-connect/token" \ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 10. The snippet hard-codes the admin realm. The manual DAG snippet says it "uses the same variables as the harness", but it hard-codes the admin realm Failure scenario: A
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 4f8043b. The runbook snippet now sets |
||
| -d client_id=admin-cli -d grant_type=password \ | ||
| --data-urlencode "username=${KEYCLOAK_ADMIN_USERNAME:?}" \ | ||
| --data-urlencode "password=${KEYCLOAK_ADMIN_PASSWORD:?}" \ | ||
| | python3 -c 'import sys,json;print(json.load(sys.stdin).get("access_token",""))') | ||
| [ -n "$ADMIN" ] || { echo "admin login failed: check KEYCLOAK_ADMIN_*"; exit 1; } | ||
| CID=$(curl -s -H "Authorization: Bearer $ADMIN" "$KC/admin/realms/$REALM/clients?clientId=rossoctl" \ | ||
| | python3 -c 'import sys,json;c=json.load(sys.stdin);print(c[0]["id"] if c else "")') | ||
| [ -n "$CID" ] || { echo "login client rossoctl not found in realm $REALM"; exit 1; } | ||
| curl -s -o /dev/null -w "enable DAG HTTP %{http_code}\n" -X PUT \ | ||
| -H "Authorization: Bearer $ADMIN" -H "Content-Type: application/json" \ | ||
| "$KC/admin/realms/$REALM/clients/$CID" -d '{"directAccessGrantsEnabled": true}' | ||
| ) | ||
| ``` | ||
|
|
||
| Expect `enable DAG HTTP 204`. The grant stays on; set the field | ||
| to `false` the same way to turn it off again. | ||
|
|
||
| --- | ||
|
|
||
| ## Restore | ||
|
|
||
| ```bash | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -167,11 +167,12 @@ def test_cross_service_conflict_is_surfaced_as_422_conflict_report() -> None: | |
| # Open the cleanup guard BEFORE the first shared-state mutation: the clean-slate steps below | ||
| # (Agent CR delete, Keycloak cleanup, Policy Store clear, provisioning) already mutate shared | ||
| # cluster state, so a failure mid-setup must still hit the ``finally`` teardown. | ||
| grant_enabled = False | ||
| try: | ||
| uc1.delete_workload_crs() # clean policy slate (drop any prior run's CRs) | ||
| uc1.cleanup_provisioned(admin, TEST_REALM) # clean slate (Keycloak) | ||
| uc1.clear_policy_store() # clean slate ONCE — NOT between phases (the agent must see the tool's rule) | ||
| uc1.provision_realm_and_users(admin, TEST_REALM) # PRB reads the role universe | ||
| grant_enabled = uc1.provision_realm_and_users(admin, TEST_REALM) # PRB reads the role universe | ||
| uc1.verify_login_subject( | ||
| keycloak_url=keycloak_url, realm=TEST_REALM, user="test-user", password=scn.USER_PASSWORD | ||
| ) | ||
|
|
@@ -187,6 +188,8 @@ def test_cross_service_conflict_is_surfaced_as_422_conflict_report() -> None: | |
| finally: | ||
| uc1.delete_workload_crs() # after — drop any CR (there should be none on the raising path) | ||
| uc1.cleanup_provisioned(admin, TEST_REALM) # restore the pre-run Keycloak state | ||
| if grant_enabled: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Grant restore can be skipped. In the Failure scenario:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 4f8043b. The disabled cross-service test now wraps its |
||
| uc1.restore_direct_access_grants(admin, TEST_REALM) | ||
| # Phase 1 persisted a tool-side deny; clear it so later integration tests don't read stale | ||
| # inbound rules and become order-dependent. | ||
| uc1.clear_policy_store() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9. The ConfigMap reload quietly resets tuned keys. The new "always reload the ConfigMap" flow runs
kubectl apply -f k8s/agent-deployment.yamlon every agent rebuild. It keeps onlyAIAC_ENFORCEMENT_SIDEandLOG_LEVEL, so the apply quietly sets all other live keys back to the manifest values.Failure scenario: An operator has tuned
LLM_REQUEST_TIMEOUT,LLM_MAX_RETRIESorLLM_RETRY_BACKOFF_*on the cluster. The guide says not to skip the reload even after a code-only rebuild, and the reload puts those keys back to the defaults (120/3/1/30) with no warning. This change is also outside the scope of this PR (no demo dependency in the tests).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4f8043b. The reload block in
k8s/aiac-deployment-guide.mdnow reads all live keys before the apply. The patch then writes them back, with the.envLLM values on top ($live + {LLM_BASE_URL, LLM_MODEL}). Tuned keys such asLLM_REQUEST_TIMEOUT,LLM_MAX_RETRIESandLLM_RETRY_BACKOFF_*, the side and the log level keep their live values. Only a key that is new in the manifest gets the manifest value. I also addedpipefail, so a failed read stops the block before the apply. The scope question is still open: this section came in with commit a589c4f, and that commit can move to its own PR.