Repository navigation
fix #10: adapt to d-engine main data_dir API, fix docker-release CI + local CVE gate - #11
Merged
Merged
Conversation
… local CVE gate
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe pull request updates Docker release execution and vulnerability checks. It adds exclusive LMDB data-directory locking, updates three-node storage paths, removes legacy directory settings, and broadens ignored directory patterns. ChangesDocker validation and release workflow
LMDB data-directory locking
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/docker-release.yml:
- Line 66: Update the Docker release step that assigns TARGET to avoid
interpolating `${{ env.IMAGE }}` inside the Bash script; pass the value through
the step environment and reference `$IMAGE` instead. Validate IMAGE before
constructing the target if it is mutable, while preserving the existing Docker
image tag format and workflow behavior.
In `@d-lmdb/src/db.rs`:
- Line 62: Acquire the d-lmdb node/data-directory lock in LmdbStorageEngine::new
and LmdbStateMachine::new’s setup path before either custom LMDB environment is
opened or initialized, and hold it across both constructors until startup
proceeds to start_custom or start_node. Reuse the existing lock mechanism or
d-engine entry point that supports a pre-acquired lock.
In `@Makefile`:
- Around line 323-336: Update the retry loop around docker scout cves to capture
each command’s exit status separately from OUT, and after retries fail when the
final status is nonzero by printing OUT and exiting before parsing LINE or
vulnerability counts. Preserve the existing FAILSIG retry behavior and only
continue to the vulnerability-count parsing when the command completed
successfully.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b7368a1-2244-4b5d-9c4d-fe2af0e90127
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/docker-release.yml.gitignoreMakefiled-lmdb/src/db.rsexamples/single-node/config.tomlexamples/three-node/config/n1.tomlexamples/three-node/config/n2.tomlexamples/three-node/config/n3.toml
💤 Files with no reviewable changes (1)
- examples/single-node/config.toml
… CVE gate and workflow injection
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
256-276: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftGate each platform image before publishing it.
The release workflow bypasses
docker-preflightanddocker-cve-check. Add an equivalent check after the smoke test and beforePush tested image by digest. Scand-lmdb-server:composewith${{ matrix.platform }}; the existingdocker-cve-checktarget rebuilds and scansd-lmdb-server:localinstead.🤖 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 `@Makefile` around lines 256 - 276, Add a release-workflow check after the smoke test and before “Push tested image by digest” that scans d-lmdb-server:compose for the current matrix platform using docker scout, rather than invoking docker-cve-check, which rebuilds and scans d-lmdb-server:local. Ensure each platform image is gated before its digest is published.
🤖 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 `@d-lmdb/src/lock.rs`:
- Around line 25-30: Update the lock acquisition error mapping around
file.try_lock so only TryLockError::WouldBlock produces the existing Error::Path
contention message; map TryLockError::Error(err) to Error::Io while preserving
the original I/O error.
In `@Makefile`:
- Around line 318-322: Replace the human-readable output parsing after the
docker scout cves invocation with a status-based gate using --only-fixed
--only-severity critical,high,medium --exit-code. Treat exit status 2 as a
vulnerability finding, while treating every other non-zero status as a scanner
failure; remove the LINE summary extraction and related validation.
---
Outside diff comments:
In `@Makefile`:
- Around line 256-276: Add a release-workflow check after the smoke test and
before “Push tested image by digest” that scans d-lmdb-server:compose for the
current matrix platform using docker scout, rather than invoking
docker-cve-check, which rebuilds and scans d-lmdb-server:local. Ensure each
platform image is gated before its digest is published.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 83c857d3-ac69-4b02-8ba1-66fbbf0fa8a1
📒 Files selected for processing (7)
.github/workflows/docker-release.yml.gitignoreMakefiled-lmdb/src/db.rsd-lmdb/src/lib.rsd-lmdb/src/lock.rsd-lmdb/src/lock_test.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
… scout --exit-code, document lock test intent
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
1 participant
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.
What Does This PR Do?
Adapts to d-engine main's breaking data_dir API change; fixes 2 broken CI/CVE gates.
Type:
Why Is This Needed?
data_dirarg on start_custom/start_node — d-lmdb didn't compile against it.Checklist
make checkpassesmake testpassesTesting
cargo nextest run --all-features: 91/91 passedmake test(build + smoke test + CVE gate): clean, multiple runsDoes This Fit d-lmdb's Scope?
Reviewer Notes
Also added
docker-preflight: checks buildx/scout are reachable before build — this repo's.envrcsets a project-scopedDOCKER_CONFIGmissingcli-plugins/, which otherwise fails several steps later with a confusing error.Estimated review complexity:
Summary by CodeRabbit
New Features
Bug Fixes
Chores