fix(docs): add PQC key generation to quickstart docker-compose#338
fix(docs): add PQC key generation to quickstart docker-compose#338marythought wants to merge 1 commit into
Conversation
|
Complex PR? Review this PR in Change Stack to move by importance, not file order. Warning Review limit reached
More reviews will be available in 55 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a one-off generate-pqc-keys service to docker-compose that runs the platform Go keygen to produce hybrid PQC keys in a shared /keys volume, and makes Keycloak provisioning, fixtures provisioning, the platform service, and a permission-fix job wait for it to complete; also updates several image tags and an opentdf.yaml download URL. ChangesPQC Key Generation Service and Orchestration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
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 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 introduces a new generate-pqc-keys service in docker-compose.yaml to generate hybrid post-quantum KAS keys and configures other services to depend on its successful completion. The review feedback points out a critical issue where the shell script in the new service does not use set -e. Consequently, if any command fails, the script will still exit with a success status, potentially causing dependent services to fail with confusing errors. Adding set -e is recommended to ensure the script exits immediately upon failure.
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.
|
📄 Preview deployed to https://opentdf-docs-pr-338.surge.sh |
marythought
force-pushed
the
fix/quickstart-pqc-keygen
branch
2 times, most recently
from
5d4b7c3 to
2591fa6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/getting-started/docker-compose.yaml (2)
379-412:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd retries to the new network bootstrap path.
Unlike the other remote-fetch jobs in this file, this service fails on the first transient GitHub or Go module proxy error. Since Lines 203-204, 244-245, 292-293, and 329-330 now gate multiple services on
generate-pqc-keys, one flaky fetch can stop the whole quickstart from starting.Suggested fix
- | set -e apk add --no-cache git + MAX_ATTEMPTS=3 WORKDIR=$(mktemp -d) cd "$$WORKDIR" git init git remote add origin https://github.com/opentdf/platform.git git config core.sparseCheckout true echo "go.work" >> .git/info/sparse-checkout echo "lib/" >> .git/info/sparse-checkout echo "service/cmd/keygen/" >> .git/info/sparse-checkout echo "service/go.mod" >> .git/info/sparse-checkout echo "service/go.sum" >> .git/info/sparse-checkout echo "protocol/" >> .git/info/sparse-checkout echo "sdk/" >> .git/info/sparse-checkout - git pull --depth 1 origin main - cd service - go run ./cmd/keygen -output /keys + for i in $$(seq 1 $$MAX_ATTEMPTS); do + if git pull --depth 1 origin main && cd service && go run ./cmd/keygen -output /keys; then + break + fi + [ $$i -lt $$MAX_ATTEMPTS ] || exit 1 + sleep 2 + done echo "PQC keys generated successfully" rm -rf "$$WORKDIR"Also applies to: 438-472, 544-560
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/getting-started/docker-compose.yaml` around lines 379 - 412, The download/validation block for keycloak_data.yaml currently errors out on the first transient failure; wrap the wget+validation into a retry loop using the existing MAX_ATTEMPTS, URL and OUTPUT variables (as shown) so transient network errors are retried: attempt wget to "$OUTPUT" up to MAX_ATTEMPTS, validate the file exists, is non-empty and looks like YAML (e.g., check head -1 for '---' or key:), remove a failed/invalid file and retry after a short sleep when attempts remain, and only exit 0 on successful validation or exit 1 after exhausting MAX_ATTEMPTS; apply the same retry pattern to the other similar fetch blocks referenced by generate-pqc-keys (the blocks around lines noted) to prevent a single flaky fetch from failing the whole bootstrap.
190-190:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin the keygen checkout to the same platform revision.
Lines 190, 232, 281, and 347 are already pinned to
nightly-a29f108/a29f1087, but Lines 549-560 compile and execute whatever is currently onorigin/main. That makes quickstart non-reproducible and adds a supply-chain execution path ifmainchanges unexpectedly. Use one shared platform ref for the image tag, config download, and keygen checkout instead of hardcodingmain.Suggested fix
- git remote add origin https://github.com/opentdf/platform.git + PLATFORM_REF="${PLATFORM_REF:-a29f1087}" + git remote add origin https://github.com/opentdf/platform.git git config core.sparseCheckout true echo "go.work" >> .git/info/sparse-checkout echo "lib/" >> .git/info/sparse-checkout echo "service/cmd/keygen/" >> .git/info/sparse-checkout echo "service/go.mod" >> .git/info/sparse-checkout echo "service/go.sum" >> .git/info/sparse-checkout echo "protocol/" >> .git/info/sparse-checkout echo "sdk/" >> .git/info/sparse-checkout - git pull --depth 1 origin main + git fetch --depth 1 origin "$$PLATFORM_REF" + git checkout FETCH_HEAD cd service go run ./cmd/keygen -output /keysAlso applies to: 232-232, 281-281, 347-347, 549-560
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/getting-started/docker-compose.yaml` at line 190, The docker-compose file pins the platform image to nightly-a29f108 but the keygen service still checks out origin/main, making the quickstart non-reproducible and unsafe; introduce a single shared variable (e.g., PLATFORM_REF) and use it for the image tag (registry.opentdf.io/platform:${PLATFORM_REF}), the config download, and the keygen git checkout instead of hardcoding main so all references (the image line with registry.opentdf.io/platform:nightly-a29f108, the config download step, and the keygen checkout/clone commands) use the same value (set default to nightly-a29f108 or a29f1087) to ensure consistent, pinned revision across lines shown and the keygen checkout block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/getting-started/docker-compose.yaml`:
- Around line 379-412: The download/validation block for keycloak_data.yaml
currently errors out on the first transient failure; wrap the wget+validation
into a retry loop using the existing MAX_ATTEMPTS, URL and OUTPUT variables (as
shown) so transient network errors are retried: attempt wget to "$OUTPUT" up to
MAX_ATTEMPTS, validate the file exists, is non-empty and looks like YAML (e.g.,
check head -1 for '---' or key:), remove a failed/invalid file and retry after a
short sleep when attempts remain, and only exit 0 on successful validation or
exit 1 after exhausting MAX_ATTEMPTS; apply the same retry pattern to the other
similar fetch blocks referenced by generate-pqc-keys (the blocks around lines
noted) to prevent a single flaky fetch from failing the whole bootstrap.
- Line 190: The docker-compose file pins the platform image to nightly-a29f108
but the keygen service still checks out origin/main, making the quickstart
non-reproducible and unsafe; introduce a single shared variable (e.g.,
PLATFORM_REF) and use it for the image tag
(registry.opentdf.io/platform:${PLATFORM_REF}), the config download, and the
keygen git checkout instead of hardcoding main so all references (the image line
with registry.opentdf.io/platform:nightly-a29f108, the config download step, and
the keygen checkout/clone commands) use the same value (set default to
nightly-a29f108 or a29f1087) to ensure consistent, pinned revision across lines
shown and the keygen checkout block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4461090-725d-4e88-9f3d-fc400db76e01
📒 Files selected for processing (1)
docs/getting-started/docker-compose.yaml
marythought
force-pushed
the
fix/quickstart-pqc-keygen
branch
from
2591fa6 to
b326770
Compare
Add a generate-pqc-keys service that sparse-checkouts the platform's keygen utility and generates hybrid post-quantum key files (X-Wing, P256+ML-KEM-768, P384+ML-KEM-1024) that opentdf-example.yaml requires since opentdf/platform#3276. Supersedes the stopgap pin in #337 — quickstart tracks nightly/main again. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marythought
force-pushed
the
fix/quickstart-pqc-keygen
branch
from
b326770 to
74ea138
Compare
|
Addressing review feedback:
Fair point — pinned the keygen
Valid suggestion but out of scope for this PR. The existing download services in this file have retry logic; adding it to the keygen would be a follow-up improvement.
Already addressed in a prior push. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/getting-started/docker-compose.yaml`:
- Line 347: The quickstart mixes a pinned config download (wget fetching
service/v0.16.0 opentdf-example.yaml) with an unpinned keygen step (the
generate-pqc-keys flow that does a git checkout origin main before running
cmd/keygen); update the key-generation steps to check out the same
ref/tag/commit as the wget (e.g., service/v0.16.0) instead of origin main so
cmd/keygen and generate-pqc-keys run against the identical platform version used
for opentdf.yaml (locate the git checkout and keygen invocation in the
generate-pqc-keys / cmd/keygen script and replace origin main with the matching
service/v0.16.0 tag or the specific commit hash).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 780a2ebc-0dd4-4ec8-9156-32ce1eefda30
📒 Files selected for processing (1)
docs/getting-started/docker-compose.yaml
1 participant
Summary
generate-pqc-keysDocker Compose service that sparse-checkouts the platform'sservice/cmd/keygenand generates the hybrid post-quantum key files (X-Wing, P256+ML-KEM-768, P384+ML-KEM-1024) thatopentdf-example.yamlnow requiresservice/v0.16.0tag instead ofmain— prevents future config changes from breaking the quickstart (retro item from Quickstart broken: missing PQC key generation after platform added hybrid key support #336)nightlyWhat changed
generate-pqc-keysservice usinggolang:1.24-alpine(sparse checkout →go run ./cmd/keygen→ PQC key PEMs to/keys)platform-provision-keycloak,platform-provision-fixtures,platform, andfix-keys-permissionsdepend ongenerate-pqc-keysservice/v0.16.0tag (notmain)nightlyContext
opentdf/platform#3276 added hybrid PQC keyring entries to
opentdf-example.yamlbut the quickstart's key generation only creates RSA/EC keys. See #336 for root-cause analysis. DSPX-3502 tracks cross-repo CI to prevent this class of break.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit