Skip to content

fix(ci): always generate opentdf.yaml even when PQC keys are absent - #3750

Merged
dmihalcik-virtru merged 1 commit into
mainfrom
fix/ci-startup-yaml-pqc-guard
Jul 15, 2026
Merged

fix(ci): always generate opentdf.yaml even when PQC keys are absent#3750
dmihalcik-virtru merged 1 commit into
mainfrom
fix/ci-startup-yaml-pqc-guard

Conversation

@dmihalcik-virtru

@dmihalcik-virtru dmihalcik-virtru commented Jul 14, 2026

Copy link
Copy Markdown
Member

Problem

The reusable platform-xtest job (e.g. run 29361028930, job xct (v0.9.0, go@main)) fails in Check out and start up platform with:

Error: stat opentdf.yaml: no such file or directory

thrown by the Enable ECC wrapping for TDFs step (yq ... -i opentdf.yaml).

Root cause

opentdf.yaml is never created. In test/start-up-with-containers/action.yaml, the only command that generates it — <opentdf-dev.yaml >opentdf.yaml yq e "${yq_command}" — is the last line of the Map the config to the keys step.

#3594 inserted an early exit 0 near the top of that same step:

# Only allow PQC algorithms if the platform generated PQC key files
if [[ ! -f kas-xwing-private.pem ]]; then
  printf "PQC key files not found; skipping PQC configuration ...\n" 1>&2
  exit 0
fi

When the platform under test lacks PQC key generation (e.g. released tags like v0.9.0, which predate service/cmd/keygen), kas-xwing-private.pem is never produced, the step bails before writing opentdf.yaml, and every later step that edits it fails.

Fix

Remove the misplaced guard from the base-config step. This is safe:

  • PQC extra keys are already filtered by the allowed_algorithms allowlist, which no longer lists hpqt:* (also from fix(ci): DSPX-3499 skip PQC key config when platform lacks PQC support #3594) — so a PQC extra key is skipped by the loop's continue, not fatal.
  • The dedicated Enable PQ (mlkem, xwing, and hybrid) wrapping step keeps its own if [ ! -f kas-xwing-private.pem ] guard, so PQC config is still correctly skipped when unsupported.

Net effect: opentdf.yaml is always generated with the base + allowed keys; PQC is still skipped when the platform can't produce PQC keys.

Verification

Simulated the patched step in a scratch dir with no kas-xwing-private.pem and PLATFORM_VERSION=0.9.0: opentdf.yaml is now produced and yq e '.services.kas.preview.ec_tdf_enabled = true' -i opentdf.yaml succeeds.

A follow-up will replace the version/file heuristics with a more principled capability probe.

Regression from #3594.

Summary by CodeRabbit

  • Bug Fixes
    • Improved startup configuration handling across supported platform versions.
    • Ensured compatible RSA and elliptic-curve algorithms are recognized consistently, even when optional post-quantum key files are unavailable.
    • Updated validation coverage for containerized startup scenarios.

PR #3594 added an early `exit 0` to the "Map the config to the keys"
step to skip PQC config when the platform under test lacks PQC key
generation. But that step is also where the base opentdf.yaml is created
(`<opentdf-dev.yaml >opentdf.yaml yq e ...` at the end of the step), so
the guard skipped opentdf.yaml creation entirely whenever
kas-xwing-private.pem was missing (e.g. testing released tags like
v0.9.0 that predate service/cmd/keygen). Every later step then failed
with "stat opentdf.yaml: no such file or directory".

Remove the misplaced guard. PQC extra keys are already filtered by the
allowed_algorithms allowlist (which no longer lists hpqt:*), and the
dedicated "Enable PQ ... wrapping" step keeps its own file-existence
guard, so PQC is still correctly skipped when unsupported.
@dmihalcik-virtru
dmihalcik-virtru requested a review from a team as a code owner July 14, 2026 21:13
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a regression in the CI pipeline where the opentdf.yaml configuration file failed to generate for older platform versions. By removing a premature exit guard, the process now correctly proceeds to generate the base configuration while still safely skipping PQC-specific steps where appropriate.

Highlights

  • CI Configuration Fix: Removed an early exit condition in the CI test startup script that was incorrectly preventing the generation of opentdf.yaml when PQC keys were missing.
  • Improved Reliability: Ensures that the platform configuration file is always created, preventing downstream CI failures in older versions that do not support PQC.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.


The config file was nowhere to be found, / Because the script had exited on the ground. / We removed the guard that blocked the way, / Now tests will run throughout the day.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the check for the presence of the 'kas-xwing-private.pem' file and the corresponding early exit logic in the 'test/start-up-with-containers/action.yaml' script. There are no review comments, and I 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.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f30dc855-e2f9-411a-ad72-c1e599046d7b

📥 Commits

Reviewing files that changed from the base of the PR and between baba709 and c4038bd.

📒 Files selected for processing (1)
  • test/start-up-with-containers/action.yaml
💤 Files with no reviewable changes (1)
  • test/start-up-with-containers/action.yaml

📝 Walkthrough

Walkthrough

The container startup key-mapping script now conditionally adds RSA and ECC algorithms for newer platform versions and no longer exits when the PQC private key file is absent.

Changes

Algorithm allowlist update

Layer / File(s) Summary
Version-based key mapping
test/start-up-with-containers/action.yaml
The script adds rsa:4096, ec:secp384r1, and ec:secp521r1 for qualifying platform versions and removes the kas-xwing-private.pem early-exit guard.

Estimated code review effort: 2 (Simple) | ~5 minutes

Possibly related PRs

Suggested reviewers: jakedoublev

Poem

I’m a rabbit with keys in a row,
New algorithms now know where to go.
No missing key makes me flee,
RSA and ECC hop happily.
The startup map is ready to grow!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main CI fix: generating opentdf.yaml even when PQC keys are absent.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ci-startup-yaml-pqc-guard

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 207.409126ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 121.46511ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 411.180692ms
Throughput 243.20 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 45.383675548s
Average Latency 452.226644ms
Throughput 110.17 requests/second

jakedoublev pushed a commit to opentdf/tests that referenced this pull request Jul 14, 2026
Ppins the `start-up-with-containers` action to the fix branch for
testing.

## Why

opentdf/platform#3594 introduced a regression: a misplaced `exit 0` in
the `Map the config to the keys` step of
`test/start-up-with-containers/action.yaml` skips creation of
`opentdf.yaml` whenever the platform under test lacks PQC keys (e.g.
released tags like v0.9.0 that predate `service/cmd/keygen`). Every
later step then fails with `stat opentdf.yaml: no such file or
directory`.

Fix: opentdf/platform#3750 (branch `fix/ci-startup-yaml-pqc-guard`,
`c4038bdd72b8777654aa36266ad721710d220f4d`).

## Change

- `.github/workflows/xtest.yml`: pin `start-up-with-containers` from
`0612ea89 # main` → `c4038bd` (the fix branch) so CI exercises the fix.
- `.github/workflows/vulnerability.yml` is intentionally **left** at
`11af44a5 # pqc-enabled` — it's on a different lineage and isn't
affected by this bug.

## Follow-up

- Merge opentdf/platform#3750, then re-pin this back to the resulting
`main` SHA.
- A more principled replacement for the version/file heuristics
(capability probe via keygen) is planned separately.
@github-actions

Copy link
Copy Markdown
Contributor

X-Test Failure Report

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 139.530138ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 72.800367ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 312.896753ms
Throughput 319.59 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 32.560585886s
Average Latency 324.589126ms
Throughput 153.56 requests/second

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 162.745909ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 81.204889ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 355.846753ms
Throughput 281.02 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 35.184285394s
Average Latency 350.090412ms
Throughput 142.11 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • examples
  • otdfctl
  • sdk
  • service
  • lib/fixtures
  • tests-bdd

See the workflow run for details.

@dmihalcik-virtru
dmihalcik-virtru added this pull request to the merge queue Jul 15, 2026
Merged via the queue into main with commit 6dd5f64 Jul 15, 2026
109 of 118 checks passed
@dmihalcik-virtru
dmihalcik-virtru deleted the fix/ci-startup-yaml-pqc-guard branch July 15, 2026 14:18
jakedoublev pushed a commit to opentdf/tests that referenced this pull request Jul 15, 2026
Follow-up to #559. Now that
[opentdf/platform#3750](opentdf/platform#3750)
has merged, re-pin the `opentdf/platform` test actions from the
temporary fix branch / stale main SHA to a **stable, named snapshot** of
current platform main.

To avoid the pins reading as "floating `main`", the target SHA is tagged
in opentdf/platform as **`ci-startup-yaml-fix`** (annotated tag →
`6dd5f649347fb2314c6090ea6d090a5c673d58a6`). Pins stay SHA-locked
(supply-chain best practice); the tag is the human-readable comment.

## Changes (`.github/workflows/xtest.yml`)

| Action | Before | After (SHA `6dd5f649`, tag `ci-startup-yaml-fix`) |
|---|---|---|
| `start-up-with-containers` (L309) | `c4038bd` (fix branch) | ✅ |
| `start-additional-kas` ×6 (L604–670) | `0612ea89` (stale main) | ✅ |

## Notes

- `6dd5f649` is the squash-merge of #3750 ("always generate
opentdf.yaml") and was current platform `main` HEAD; the
`ci-startup-yaml-fix` tag makes it a durable pointer.
- `start-additional-kas/action.yaml` is **unchanged** between `0612ea89`
and `6dd5f649` (verified via `compare`) — a SHA-only sync.
- `vulnerability.yml` intentionally left on its `# pqc-enabled` pin
(separate lineage).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants