Conversation
Add comprehensive BeakerLib test for the keylimectl CLI tool covering: - Help flags for all subcommands - Runtime policy generation (IMA log, allowlist, excludelist, rootfs, hash algorithms, ramdisk, local and remote RPM repos, keyrings, verification keys in PEM and DER encoding for keys and certificates) - Policy signing with ECDSA and X509 DSSE backends (including RSA keys) - Measured boot policy generation - Runtime policy CRUD on verifier (push/show/list/update/delete) - Measured boot policy CRUD on verifier - Agent lifecycle (add/status/list/update/reactivate/remove) - Agent failure and recovery scenarios The tests supports push and pull model test variants using the AGENT_SERVICE environment variable pattern, consistent with other tests in the repository. Also update install_upstream_rust_keylime setup task to explicitly enable non-default features (keylimectl/rpm-repo, keylimectl/tpm-local, keylimectl/tpm-quote-validation) instead of --all-features, avoiding deprecated features (with-zmq, legacy-python-actions). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Test that agents enrolled with keylime_tenant (v2 API) can be managed with keylimectl (v3 API) and vice versa. Both tools map to the same verifiermain DB table so no re-enrollment is needed when migrating. The test covers two enrollment scenarios in each attestation mode: - Enroll with keylime_tenant, remove with keylimectl - Enroll with keylimectl, delete with keylime_tenant Run with AGENT_SERVICE=Agent (default) for pull mode or AGENT_SERVICE=PushAgent for push mode. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Test that runtime policies created and signed by keylime-policy are usable with keylimectl and vice versa. Both tools produce DSSE-format policies from the same underlying library, so cross-tool usage should be transparent to the verifier. Covers: - Both tools produce structurally valid runtime policies - keylime-policy policy accepted by keylimectl agent add - keylimectl policy accepted by keylime_tenant add - Signature produced by keylime-policy verified by keylimectl - keylimectl policy push with keylime-policy-created policy Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewer's GuideAdds new beakerlib test suites for keylimectl functionality and interoperability with keylime-policy and keylime_tenant, and updates the Rust keylime build to enable required keylimectl features. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
ansasaki
marked this pull request as draft
August 20, 2026 15:41
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -0,0 +1,122 @@ | |||
| #!/bin/bash | |||
| # vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k | |||
| . /usr/share/beakerlib/beakerlib.sh || exit 1 | |||
| rlRun "jq . policy-from-kp.json > /dev/null" 0 "keylime-policy output is valid JSON" | ||
| rlRun "jq . policy-from-kctl.json > /dev/null" 0 "keylimectl output is valid JSON" | ||
| rlRun -s "jq -r 'keys | sort | .[]' policy-from-kp.json" | ||
| KP_KEYS="$rlRun_LOG" |
| @@ -0,0 +1,103 @@ | |||
| #!/bin/bash | |||
| # vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k | |||
| . /usr/share/beakerlib/beakerlib.sh || exit 1 | |||
| rlRun "limeWaitForAgentStatus ${AGENT_ID} 'Get Quote'" | ||
| fi | ||
| rlRun -s "keylimectl agent list" | ||
| rlAssertGrep "${AGENT_ID}" "$rlRun_LOG" |
| @@ -0,0 +1,615 @@ | |||
| #!/bin/bash | |||
| # vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k | |||
| . /usr/share/beakerlib/beakerlib.sh || exit 1 | |||
|
|
||
| rlPhaseStartTest "measured-boot list" | ||
| rlRun -s "keylimectl measured-boot list" | ||
| rlAssertGrep "testmb1" $rlRun_LOG |
|
|
||
| rlPhaseStartTest "agent list" | ||
| rlRun -s "keylimectl agent list" | ||
| rlAssertGrep "${AGENT_ID}" $rlRun_LOG |
|
|
||
| rlPhaseStartTest "agent list --registrar" | ||
| rlRun -s "keylimectl agent list --registrar" | ||
| rlAssertGrep "${AGENT_ID}" $rlRun_LOG |
| rlRun "limeWaitForAgentStatus $AGENT_ID '(Failed|Invalid Quote)'" | ||
| rlRun "rlWaitForCmd 'tail -n 30 \$(limeVerifierLogfile) | grep -q \"Agent $AGENT_ID failed\"' -m 10 -d 1 -t 10" | ||
| fi | ||
| limeExtendNextExcludelist $TESTDIR |
| limeSubmitCommonLogs | ||
| limeClearData | ||
| limeRestoreConfig | ||
| limeExtendNextExcludelist $TESTDIR |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The repeated TPM/IMA emulator and keylime service setup/cleanup logic across the new test scripts could be consolidated into shared helpers to reduce duplication and make future changes easier.
- The 600+ line functional/keylimectl-commands/test.sh script is quite large; consider splitting it into smaller, focused test scripts (e.g., policy generation, signing, agent lifecycle) to improve readability and maintainability.
- Some temporary files (e.g., /tmp/enroll.expect and background HTTP server processes) are created inline in tests; it may be safer to wrap these in helper functions that ensure cleanup on failure to avoid leaking files or processes between runs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The repeated TPM/IMA emulator and keylime service setup/cleanup logic across the new test scripts could be consolidated into shared helpers to reduce duplication and make future changes easier.
- The 600+ line functional/keylimectl-commands/test.sh script is quite large; consider splitting it into smaller, focused test scripts (e.g., policy generation, signing, agent lifecycle) to improve readability and maintainability.
- Some temporary files (e.g., /tmp/enroll.expect and background HTTP server processes) are created inline in tests; it may be safer to wrap these in helper functions that ensure cleanup on failure to avoid leaking files or processes between runs.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
kkaarreell
reviewed
Aug 21, 2026
| rlRun "limeWaitForRegistrar" | ||
| if [ "${AGENT_SERVICE}" == "PushAgent" ]; then | ||
| rlRun "limeUpdateConf verifier mode 'push'" | ||
| rlRun "limeUpdateConf verifier push_attestation_period 3" |
Collaborator
There was a problem hiding this comment.
I am afraid that such a low value will be causing issues with test stability, better use >=10.
kkaarreell
approved these changes
Aug 21, 2026
kkaarreell
left a comment
kkaarreell
left a comment
Collaborator
There was a problem hiding this comment.
I do not have any particular requirement, tests look good. At some point would be good to add also some checks for keylimectl console output (where there is only exit code checking).
Also, TF infra seems disrupted ATM but some failing tests seems suspicious (e.g. durable attestation test) because the are passing in other PRs and maybe they could be caused by some changes on the keylime side.
Fix policy sign x509 tests to use -C (--cert-file) instead of -c (--cert-outfile) for input certificate when -k is provided, matching the updated keylimectl CLI flags. Add tests for previously untested commands: policy validate, policy verify-signature, policy convert, policy merge, info subcommands (verifier, registrar, agent, tls), configure --non-interactive, and command aliases (mb for measured-boot, diag for info). Add tests for agent lifecycle flags: --wait-for-attestation, --runtime-policy-name, --registrar removal, and --detailed listing. Add missing help flag tests for verify, verify evidence, policy convert, policy merge, policy generate tpm, and info subcommands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
| rlPhaseStartTest "mb alias for measured-boot" | ||
| rlRun "keylimectl mb push testmb-alias --file mb-verifier-policy.json" 0 "Push via mb alias" | ||
| rlRun -s "keylimectl mb show testmb-alias" | ||
| rlAssertGrep "testmb-alias" $rlRun_LOG |
| rlRun -s "keylimectl mb show testmb-alias" | ||
| rlAssertGrep "testmb-alias" $rlRun_LOG | ||
| rlRun -s "keylimectl mb list" | ||
| rlAssertGrep "testmb-alias" $rlRun_LOG |
|
|
||
| rlPhaseStartTest "agent list --detailed" | ||
| rlRun -s "keylimectl agent list --detailed" | ||
| rlAssertGrep "${AGENT_ID}" $rlRun_LOG |
Use extracted public keys instead of private keys for policy validate and policy verify-signature tests — these commands require an X.509 certificate or ECDSA public key, not a private key. Restart the agent service after remove --registrar to trigger immediate re-registration with the registrar before attempting to re-add the agent with --runtime-policy-name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Update policy validate/verify-signature tests to expect exit code 10 (negative result) instead of 1 when using the wrong key. Exit code 1 is reserved for actual errors (e.g. invalid DSSE envelope). Use runtime-policy-updated.json for --wait-for-attestation and --runtime-policy-name tests since the IMA log contains entries from the earlier "Fail keylime agent" phase that are not in the original runtime-policy.json. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
keylimectl now returns exit code 10 (negative result) when: - Combined agent status query finds the agent not fully attested (e.g. in "Get Quote" state) - Agent status returns not_found after removal These are intentional keylimectl semantics: exit 0 = positive/success result, exit 10 = operation succeeded but result is negative. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
In push mode the first attestation cycle takes longer than the default 20s timeout. Increase limeWaitForAgentStatus timeout to 120s for all attestation_status PASS/FAIL checks. Also make the combined 'agent status' exit code mode-aware: - Pull mode: agent is in 'Get Quote' (not fully attested) → exit 10 - Push mode: agent has attested successfully (PASS) → exit 0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
In push mode, the agent accumulates exponential backoff from repeated failed authentication attempts after the previous 'agent remove'. The backoff can reach up to 300s, causing the --wait-for-attestation 120s timeout to expire before the agent retries. Restart the push agent before this test phase to reset backoff state, ensuring the agent will attempt authentication promptly after enrollment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Clean up any leftover measured-boot refstate before pushing in the CRUD test phase, to handle VMs reused across test runs. In push mode, regenerate the runtime policy from the current IMA measurement list before --wait-for-attestation. The IMA log grows throughout the test run, so the earlier runtime-policy-updated.json no longer covers all entries by this phase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
…meout Add pre-test cleanup for testpolicy1 and testmb-alias to handle policies left behind by a previous failed test run. Increase --attestation-timeout to 240s for the push model --wait-for-attestation test. The push model agent's exponential backoff (10s → 20s → 40s → 80s) means the worst case is ~125s before the agent successfully attests after a fresh restart. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Clean up any leftover testpolicy-named from a previous failed run before pushing it in the --runtime-policy-name test phase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
…n phase The IMA emulator adds open-file measurements for the bad script after the regular IMA measurement. The excludelist for TESTDIR is consumed by 'agent update after failure', so it must be re-applied before generating runtime-policy-current.json in the --wait-for-attestation phase. Without this, IMA emulator measurements of the bad script appear after policy generation and cause 'File not found in allowlist' failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
…station policy Replace limeExtendNextExcludelist with limeSyncIMAExcludelist to exclude all /keylime-tests/ directories found in the IMA log, not just the current run's TESTDIR. This covers bad script entries from all previous test runs that accumulate in the IMA log. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
The IMA emulator adds open-file measurements asynchronously. If new measurements appear between policy generation and attestation, the verifier rejects them as 'not in allowlist'. Fix: stop the push agent first, wait 3s for pending emulator events to settle, generate the policy (with limeSyncIMAExcludelist to exclude test directories), then start the agent and enroll. This minimizes the window for new IMA emulator measurements to appear. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
…r-attestation limeSyncIMAExcludelist populates the base excludelist file, but 'keylimectl policy generate runtime --ima-measurement-list' wasn't reading it. Pass the file explicitly with --excludelist so that all /keylime-tests/ directories (including the bad script) are excluded from the generated policy and don't cause IMA verification failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Instead of using --excludelist (which requires the verifier to apply excludes — a feature not yet working in the push model path), generate the policy from the full IMA measurement list so all current entries (kernel IMA and emulator) are included in the digests allowlist. Wait 3s first for the IMA emulator to record pending open-file events before policy generation, minimizing post-policy emulator measurements. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Commands run without rlRun may execute in a different subshell context, causing them to run at an unexpected time relative to the test commands. This caused DELETE requests to arrive at the verifier after GET requests (e.g. deleting testmb1 after show was already in-flight). Wrap all cleanup delete commands with 'rlRun ... 0-255' to ensure they execute synchronously and in the correct order relative to the push. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Contributor
Author
|
/packit test |
When a file named {section}.conf exists in the config directory, modify
only that file instead of scanning all *.conf files. This prevents
unintended side-effects: e.g. 'limeUpdateConf verifier mode push' was
also modifying keylimectl.conf (which shares [verifier] but uses TOML
format), causing keylimectl to fail to parse the file due to unquoted
INI-style values.
With this fix, if {section}.conf exists the search is limited to that
file only. The fallback (scanning all .conf files) is preserved for
sections without a dedicated file.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Contributor
Author
|
/packit test |
2 similar comments
Contributor
Author
|
/packit test |
Contributor
Author
|
/packit test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add the following tests:
functional/keylimectl-commands: Test individual keylimectl commandscompatibility/policy-tool-interop: Test that policies created and signed bykeylime-policyworks withkeylimectland vice-versacompatibility/tenant-keylimectl-interop: Test that agents enrolled with thekeylime_tenantcan be managed bykeylimectland vice-versaSummary by Sourcery
Expand integration coverage for keylimectl commands and interoperability while tightening test configuration handling and upstream feature setup.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: