Repository navigation
test(features): run feature tests against environment services, starting with Keycloak - #4891
matthewgrossman wants to merge 9 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
be103d6 to
f244394
Compare
531138c to
df2bcbd
Compare
|
Label |
|
/ok to test df2bcbd |
f244394 to
6106cd0
Compare
df2bcbd to
79cc6ae
Compare
6106cd0 to
d6f93fa
Compare
3e9b04c to
38e728c
Compare
drew
left a comment
There was a problem hiding this comment.
The provider-refresh port preserves the existing test's assertions. Two concerns are noted inline: service credentials can appear in SSH error context, and the runner's two-minute per-test limit is shorter than the fixture's three-minute readiness allowance.
This branch also needs restacking onto #4773: it currently lacks the latest log-retention and Core E2E gating changes, and GitHub reports merge conflicts.
Validation: runner unit tests, eight tmachine tests, two lockfile-check tests, and alignment checks for all three enrolled workspaces passed. VM E2E suites were not rerun.
| format!("OPENSHELL_GATEWAY={}", run.gateway), | ||
| format!("OPENSHELL_BIN={openshell_bin}"), | ||
| ]; | ||
| env.extend( |
There was a problem hiding this comment.
Service credentials can appear in CI errors. Previously the runner passed only gateway and CLI settings over SSH; these additional variables include service passwords. Session::exec_timeout adds format!("run {argv:?} in tmachine VM") as error context, and the runner prints the full error chain. An SSH timeout or launch failure therefore prints the credential values, despite the parser avoiding them in its own errors. Please redact environment values in that context or pass the service variables through a guest file.
There was a problem hiding this comment.
Good catch: the argv context in exec_timeout bypasses the parser's redaction. Fixed in f644909 by keeping the values in the guest. The runner still checks env.d, but it no longer puts any value in a command. The nextest command now runs through a small sh wrapper that exports each env.d/*.env line literally (IFS= read -r + export "$line", not source) before execing nextest, so only OPENSHELL_GATEWAY and OPENSHELL_BIN appear in argv. The parser no longer trims, to match the wrapper, and a unit test runs the wrapper against files with spaces, $, backticks and = in values.
| pub const USERNAME_ENV: &str = "OPENSHELL_TEST_KEYCLOAK_USERNAME"; | ||
| pub const PASSWORD_ENV: &str = "OPENSHELL_TEST_KEYCLOAK_PASSWORD"; | ||
| /// How long Keycloak has to start answering, e.g. after the environment boots. | ||
| pub const READY_TIMEOUT: Duration = Duration::from_mins(3); |
There was a problem hiding this comment.
The runner terminates each test before this readiness allowance expires. Previously Keycloak readiness was checked by Ansible before starting the test process; this fixture now waits inside the test with a three-minute allowance, while tests/runner/src/bundle.rs sets slow-timeout = { period = "30s", terminate-after = 4 }. A slow Keycloak restart can therefore cause the test to be killed after two minutes instead of allowing readiness to finish, and skips provider/profile cleanup. Please move service readiness outside the test budget or increase the applicable nextest timeout.
There was a problem hiding this comment.
Agreed, the budgets conflicted. Fixed in 0f9ad51: READY_TIMEOUT is now 90s, with a comment tying it to the runner's 2-minute per-test budget, so a slow Keycloak fails with a readiness error instead of being killed. The wait runs before the test creates its provider or profile, so a kill there wouldn't leave anything to clean up anyway. Keycloak restarts while the runner waits for the gateway, so in practice it's ready before the first test. If 90s turns out to be flaky, I'll move readiness out of the test.
6cf04de to
0a5abba
Compare
Add Cli::run_with_env and Context::cli_with_env so tests can hand credentials to the CLI by name instead of on its command line. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Environment setup can now take inputs, like installers do, and hashes them only when present so existing cached disks stay valid. ubuntu-docker-rootful-keycloak runs the Docker setup plus a Keycloak playbook that starts the container with a restart policy, since setup is cached as a disk, and writes how to reach it as OPENSHELL_TEST_* lines in /etc/openshell-test/env.d/keycloak.env. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
After boot, read every /etc/openshell-test/env.d/*.env file, accept only OPENSHELL_TEST_* names, and add the variables to the testkit-ready and nextest commands through guest_env. Errors name the key, never the value. The runner doesn't know which services an environment runs; suites read the variables, so they also run against a deployment that sets them. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Add a keycloak features crate with a suite-local KeycloakContext fixture that reads OPENSHELL_TEST_KEYCLOAK_*, waits for the realm to answer, and provides password grants, revocation and the token endpoint. Port the provider refresh test onto it. Align the features lockfile with the root and add it to the alignment check. The old provider-refresh crate stays until the required lanes switch. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Add a matrix row for features/keycloak on ubuntu-docker-rootful-keycloak, include the Keycloak realm in the tmachine disk cache key, and document environments that provide services in TESTING.md. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
38e728c to
4c29f9e
Compare
The runner passed the variables in env.d on the nextest command line, so an SSH timeout or launch failure printed their values, which can be credentials, in the error context. The runner now only checks the files; the guest shell exports them before starting nextest, so only the gateway name and CLI path appear in the command. Lines are read literally on both sides: no trimming, and only empty lines and lines starting with # are skipped. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
The fixture waited up to three minutes for Keycloak inside the test, but the runner's nextest profile kills a test after two. Wait up to 90 seconds so a slow Keycloak fails with a readiness error instead. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Summary
A worked example of a test that needs a specific deployment feature, Keycloak. tmachine declares an environment that runs Keycloak and publishes how to reach it as
OPENSHELL_TEST_*variables. The runner passes those variables to the tests, and a suite-local fixture reads them. The Keycloak provider-refresh test is ported to testkit on top of this.Second in a linear stack: on top of #4773 (the runner), and #4774 (the conformance suite port) goes on top of this. Draft for discussion.
Related Issue
Part of #3954
How it fits together
ubuntu-docker-rootful-keycloakintests/tmachine/config.nixruns the usual Docker setup plusansible/playbooks/keycloak.yaml. That playbook starts Keycloak with--restart unless-stoppedand writes/etc/openshell-test/env.d/keycloak.env. Setup is cached as a disk image, so the Keycloak image is cached too, and the container comes back on every boot.test_env.rsreads every/etc/openshell-test/env.d/*.envfile and rejects names outside theOPENSHELL_TEST_prefix (errors never print values).guest_envadds the variables to thetestkit-readyand guest nextest commands. The runner has no idea what Keycloak is.KeycloakContext::new(cx).awaitreadsOPENSHELL_TEST_KEYCLOAK_{ISSUER,CLIENT_ID,USERNAME,PASSWORD}and waits for the realm to answer. Set the same variables to run the suite against your own deployment.Run it locally:
nix run .#openshell-test-runner -- \ --environment ubuntu-docker-rootful-keycloak --installer deb \ --family features --package keycloakCombinations such as Podman + Keycloak are more tmachine environments. They can be generated as a cross product later without changing the runner or the tests.
Changes
Cli::run_with_envandContext::cli_with_env, so secrets reach the CLI through env vars rather than argv.ubuntu-docker-rootful-keycloakenvironment. Environment setup can now takeinputs, used for the realm file; environments without inputs keep their cache keys. Thekeycloakrole adds a restart policy.test_env.rs, andguest_envtakes the published variables.tests/suites/features/keycloak):KeycloakContextderefs totestkit::Contextand addspassword_grant(),revoke()andtoken_endpoint(). A missing variable fails the test.provider_refresh.rs, with the same CLI calls and checks as the old one and run-unique provider and profile names.tests/suites/features/Cargo.lockis aligned with the root lock and added to the alignment check.features/keycloakon the new environment; the tmachine disk cache key includes the realm file.TESTING.md.The old
tests/suites/features/provider-refresh/keycloakcrate and its tmachine testsuite are unchanged.Open questions
env.ditself is deferred.envarguments over SSH.Testing
cargo nextest list -p keycloaklistskeycloak::provider_refresh(host target)Checklist