Repository navigation
chore: build java release CLIs against Maven Central - #652
Merged
Merged
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 48 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 38.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 4 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: building Java release CLIs against Maven Central when needed.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 38.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checked the tag at dawn
Then fetched the CLI, neat and drawn
When GitHub’s jar was out of sight
Maven built the tool just right
The bunny hopped away with cheer
Comment @coderabbitai help to get the list of available commands.
dmihalcik-virtru
changed the title
java-sdk releases don't attach cmdline.jar to their GitHub Releases, so every java release xtest used fell back to a full source build, whose sdk module runs `buf generate` against buf.build. With a dozen xtest jobs doing that at once, BSR rate limits fail the run. When a release has no cmdline.jar asset, install_java_release now checks Maven Central for io.opentdf.platform:sdk at that version and, if present, builds only the parent pom and the cmdline module of the tag (mvn -pl .,cmdline), resolving sdk* from Central. No codegen, no BSR calls. It uses its own local Maven repo so a same-versioned head installed into ~/.m2 can't shadow the release. Any failure still raises InstallError, so setup-cli-tool falls back to the full source build. java `latest` no longer resolves to head:true when no release has a cmdline.jar asset; it is a normal release row, so it takes the path above and is tested as published rather than rebuilt against the platform under test. setup-cli-tool gains a needs-build output (any head, or any release whose artifact install failed), and the java make steps gate on it instead of on heads. Previously a run whose only java versions were releases checked out the fallback source but never built it. DSPX-5177 Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-5177-locobuf
branch
from
October 15, 2026 17:29
7639a94 to
e7dcb36
Compare
|
X-Test Failure Report |
pflynn-virtru
approved these changes
Oct 15, 2026
dmihalcik-virtru
added a commit
that referenced
this pull request
Oct 21, 2026
…plicitly (#663) ## What and why [DSPX-3499](https://virtru.atlassian.net/browse/DSPX-3499) asks that the PQC and hybrid PQ/T tests run whenever every component could support them. An audit of the scheduled X-Test run (39383110889) found go and java at main and latest already run and pass all six `test_pqc.py` cells. js is the gap: - **js "latest" was `@opentdf/ctl@0.4.0`.** `otdf-sdk-mgr` matched only `sdk/vX.Y.Z` tags. web-sdk moved to `sdk-vX.Y.Z` at 0.4.1, so `latest` has been pinned to 0.4.0 ever since. That build fails the `key_management` gate, so it skipped every PQC cell and most of `test_abac.py`. - **The newest released CLI isn't on npm.** web-sdk's delivery job failed on sdk 0.21.0 and 0.22.0 before it reached `@opentdf/ctl` ([DSPX-5277](https://virtru.atlassian.net/browse/DSPX-5277), filed separately; this PR doesn't depend on it). npm `latest` for the CLI is still 0.20.0, which has no ML-KEM. - **The js shim had no `mechanism-secpmlkem` case.** The probe fell through to `Unknown feature`. That answers "no", which is correct today but only by accident. web-sdk really does lack the hybrids at main and 0.22.0, so js@main skipping X-Wing and secp+ML-KEM is correct. opentdf/web-sdk#1049 (open) adds the secp hybrids. X-Wing isn't planned there. ## Changes - **`otdf-sdk-mgr` resolve:** tag infixes match both `<infix>/` and `<infix>-`, as a prefix rather than a substring. `release` is now the tag as git names it (`sdk-v0.22.0`; `service/…` and `otdfctl/…` are unchanged). `versions resolve js latest` → `sdk-v0.22.0`. - **`install_js_release`:** installs `@opentdf/ctl` from npm when it's there. Otherwise it shallow-clones the release tag and packs just `cli/` against the `@opentdf/sdk` published to npm, the same way java release CLIs build against Maven Central (#652). Failures raise `InstallError`, so `setup-cli-tool` can still fall back to a full source build, and never leave a partial dist dir. `install stable js` now follows the same release tag instead of the newest CLI on npm. - **`xtest.yml`:** the js `make` steps gate on `setup-cli-tool`'s `needs-build`, as java's already do, instead of on "has a head". A js release that falls back to source now gets built even when no js head is in the run. - **`xtest/sdk/js/cli.sh`:** explicit `mechanism-secpmlkem` probe, and both hybrid probes key on the `hpqt:` names in `--encapKeyType`. When #1049 merges, the secp cells turn on with no edit here. - **`xtest/sdk/js/cli.sh`, no more `npx`** (SonarCloud): when a dist dir's install is missing, `npx @opentdf/ctl` fetches and runs npm's latest, so the cell reports on the wrong build. The shim now runs `node_modules/.bin/opentdf` (the bin name in every `@opentdf/ctl` and `@opentdf/cli` release) and fails loudly without it. `npx --no` doesn't work as a substitute: it swallows the package name. - **`spec/DSPX-3499.md`:** findings, plan, acceptance criteria. ## Expected CI differences - The js release lane becomes `js@v0.22.0` instead of `js@v0.4.0`. It should run both ML-KEM cells and every `key_management` cell in `test_abac.py`. Its hybrid cells should skip on `mechanism-xwing` / `mechanism-secpmlkem`. - `bench (js)` compares against 0.22.0 rather than 0.4.0. - New js@v0.22.0 cells may surface real interop failures that ctl 0.4.0 was skipping past. ## How it was tested - `otdf-sdk-mgr`: `ruff check`, `ruff format --check`, `pyright`, `pytest` (175 passed, including new resolver and js installer cases). - Local end-to-end: `otdf-sdk-mgr install artifact --sdk js --version sdk-v0.22.0 --dist-name v0.22.0` built `@opentdf/ctl@0.22.0` against `@opentdf/sdk@0.22.0` in about a minute. Against that build, `cli.sh supports` answers `key_management`, `mechanism-mlkem`, `ecwrap`, `hexless` and `multikao` yes, and `mechanism-xwing` / `mechanism-secpmlkem` no (exit 1, not 2). - `shellcheck xtest/sdk/js/cli.sh`. - Not run locally: the pytest suite against a live platform. This PR's X-Test run is the end-to-end check. ## Out of scope js also skips the extended-mechanism ABAC cells for reasons that aren't PQ: `mechanism-rsa-4096` has no js shim case, and `mechanism-ec-curves-384-521` is hard-coded `False` for js in `tdfs.py`. Both are noted in the spec. [DSPX-3499]: https://virtru.atlassian.net/browse/DSPX-3499?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DSPX-5277]: https://virtru.atlassian.net/browse/DSPX-5277?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit - **New Features** - JavaScript CLI installation now supports published packages and fallback builds from matching SDK releases. - Release detection supports both slash- and hyphen-formatted version tags. - CLI checks now include X-Wing and secp256r1-ML-KEM capabilities. - **Bug Fixes** - Improved cleanup and error handling when JavaScript installation or builds fail. - Test workflows now correctly run required JavaScript builds for applicable configurations. - **Documentation** - Updated installation guidance with fallback behavior, prerequisites, version selection, and direct local CLI execution. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
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
2 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.
Summary
java-sdk releases don't attach
cmdline.jarto their GitHub Releases, andcmdlineisn't published to Maven Central. So every Java release that xtest uses has been built from source with a fullmvn clean install. That build'ssdkmodule runsbuf generateagainst buf.build. With a dozen xtest jobs doing this at the same moment, BSR returnsresource_exhausted: too many requestsand fails the run (e.g. 38632037584 on #571). This is DSPX-5177.Everything
cmdlinedepends on (sdk-pom,sdk,sdk-pqc-bc,sdk-fips-bc) is on Maven Central. This PR builds just the CLI against those published jars.Changes (
otdf-sdk-mgr)install_java_release: when the GitHub Release has nocmdline.jar, the installer checks Maven Central forio.opentdf.platform:sdk:<version>. If it's there, it shallow-clones the tag into a temp dir and runsmvn -pl .,cmdline package, sosdk*resolves from Central and no codegen or BSR calls happen.src/so the javaMakefiledoesn't rebuild it.~/.m2/otdf-sdk-mgr-releases. java-sdkmaincarries the last release's version (currently0.19.1), so ansdkthat a head buildmvn installed into~/.m2/repositorywould otherwise shadow the release. I hit this locally: the v0.19.1cmdlinefailed to compile againstmain's sdk.InstallError, sosetup-cli-toolfalls back to today's full source build.versions resolve java latest: no longer returnshead: truewhen no release has acmdline.jarasset. It's now an ordinary release row and takes the path above.latestis no longer rebuilt against the platform under test viaPLATFORM_BRANCH. It's tested as published.install stable java: drops thehas_clifilter, which had made it skip Java entirely.No workflow changes: JDK and Maven are already set up before
setup-cli-toolin every job that installs Java.Expected effect
In a run like 38632037584, each job's v0.19.1 build no longer calls BSR. Combined with opentdf/java-sdk#419 (which stops
attach-sourcesfrom running codegen a second time), BSR traffic per run drops from about 96 to about 24buf generatepasses. Only real heads still build from source. Building each head once per run is a separate follow-up.Testing
sdkfor every release since 0.7.2 except 0.8.0 and 0.17.0. Those fall back to a source build.mvn -pl .,cmdline packagechecked against an empty local repo:latest): builds, zerobufcalls.bufcalls, with Maven 3.9.11 as on the runners. Maven 3.10 rejects that tag'ssdk/pom.xmlfor a duplicate plugin declaration in thefuzzprofile.otdf-sdk-mgr install artifact --sdk java --version v0.19.1 --dist-name v0.19.1takes the new path, andcmdline.jar --versionreports{"version":"0.19.1","tdfSpecVersion":"4.3.0"}, whichcli.sh supportsgates on.tests/test_installers.pyand updatedtest_resolve.py.ruff check,ruff format,pyrightandpytest(144 passed) are all clean.Summary by CodeRabbit