fix(xtest): use head-built otdfctl for admin key ops so PQ/T tests run on branch refs#534
Conversation
Instrument PlatformFeatureSet PQ/T mechanism detection (mechanism-xwing, mechanism-secpmlkem, mechanism-mlkem) to diagnose why these features go undetected even when km1's 'kas trust mechanisms initialized' log line lists hpqt:xwing and hpqt:secp256r1-mlkem768. Logs: resolved KAS_KM1_LOG_FILE path + existence, whether the mechanisms line matched (and what it contained), HTTP-probe fallback results, the semver gate decision, and which features were ultimately added.
PQ/T mechanism tests skip (not fail) when a mechanism is undetected, and pytest hides captured output for skipped tests, so the pqc-detect DEBUG lines would not appear in CI. A pytest_report_header hook forces detection and echoes the captured xtest logs into the always-visible session header.
When kas-registry key create rejects an hpqt algorithm, include the
underlying otdfctl/platform error in the skip reason. This distinguishes a
client-side otdfctl mapping rejection ('invalid algorithm' from
sdkHelpers) from a server-side protovalidate rejection
('key_algorithm_defined' CEL), which the generic message hid.
The admin otdfctl fixture (used to register kas-registry keys for ABAC/PQC
tests) is loaded by conftest.load_otdfctl(), which picks the head build via
the OTDFCTL_HEADS env var, else dist/main/otdfctl.sh, else falls back to
'go run github.com/opentdf/otdfctl@latest'.
OTDFCTL_HEADS was never set by the workflow, so for any platform/go ref that
is a branch (not 'main') the admin CLI silently fell back to the released
otdfctl@latest. That released CLI predates the hpqt:* algorithm mapping, so
'kas-registry key create --algorithm=hpqt:xwing' was rejected client-side and
every PQ/T test skipped as 'Algorithm ... not supported by platform' -- even
though the platform (KAS keyring + policy CEL) fully supported it.
Set OTDFCTL_HEADS from configure-go.outputs.heads on each pytest step so the
admin otdfctl uses the head build (sdk/go/dist/{tag}/otdfctl) that matches
what's under test.
Replace the verbose per-call pqc-detect DEBUG logging with a concise always-visible session-header summary of the detected platform version and feature set (feature detection gates skips, and pytest hides captured output for skipped tests). Keep lightweight logger.debug detail on the failure paths in tdfs.py (km1 log missing/unreadable, KAS algorithm probe failure) so there's still a breadcrumb when PQ/T detection comes up empty.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughImproves xtest observability by adding debug logging to platform feature detection helpers in ChangesXtest diagnostics and CI wiring
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances test reporting and diagnostic logging in the test suite. Specifically, it adds a 'pytest_report_header' to display detected platform features and versions in the pytest session header, improves the skip message in '_get_or_create_key' to include the underlying exception and required features when an algorithm is unsupported, and introduces debug logging in 'xtest/tdfs.py' for missing or unreadable logs and failed KAS algorithm probes. There are no review comments, and we have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
2 participants
Problem
PQ/T tests (
test_pqc.py: X-Wing, secp+ML-KEM hybrids) silently skipped on every branch/PR run, only ever exercised onmain/nightly. They skipped with:even when the platform fully supported it (km1's keyring loads
hpqt:*, and the policy-servicekey_algorithm_definedCEL allows the enum values).Root cause
The PQ tests register their keys via the admin
otdfctlfixture (conftest.load_otdfctl()), which picks the CLI in this order:OTDFCTL_HEADS[0]→sdk/go/dist/{tag}/otdfctl.sh(the head build)sdk/go/dist/main/otdfctl.shgo run github.com/opentdf/otdfctl@latestOTDFCTL_HEADSwas never set by the workflow. For any platform/go ref that is a branch (notmain),dist/main/otdfctl.shdoesn't exist either, so the admin CLI fell through to the releasedotdfctl@latest— which predates thehpqt:*algorithm mapping. It rejected--algorithm=hpqt:xwingclient-side, before the request reached the (fully capable) platform. (Older algorithms like rsa-4096 / ec-384-521 still passed because@latestknows them — which is why only hpqt skipped.)Fix
Set
OTDFCTL_HEADS: ${{ steps.configure-go.outputs.heads }}on each pytest step so the admin otdfctl uses the head build (sdk/go/dist/{tag}/otdfctl) that matches what's under test.headsis[.tag], which matches thedist/{tag}build-dir naming.Supporting changes
fixtures/keys.py— include the underlying otdfctl/platform error in the PQ key-create skip reason. The previous opaque "not supported" message is what hid this for so long; the enriched message is what surfaced thego run …@latestsmoking gun. Worth keeping permanently.conftest.py—pytest_report_headerechoes the detected platform version + feature set into the always-visible session header (feature detection gates skips, and pytest hides captured output for skipped tests).tdfs.py— lightweightlogger.debugbreadcrumbs on the PQ-detection failure paths (km1 log missing/unreadable, KAS algorithm probe failure).Validation
Dispatched
xtest.ymlagainst platform branchrevert-3625-DSPX-3396-disable-hybrid(platform + go both): themechanism-xwing/mechanism-secpmlkemtests went from SKIPPED → PASSED.Summary by CodeRabbit
Release Notes