Feat/add rust fs and local storage support#1670
Conversation
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR enables local filesystem storage as an alternative to AWS S3 throughout the platform. Environment flags ( ChangesLocal Filesystem Storage Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The PR adds conditional local-filesystem logic across multiple methods in AwsService with varying complexity (simple reads/writes, JSON serialization, URL construction). Changes are consistent in pattern but spread across five files with distinct roles. Requires verification that all file operations (upload, read, delete, persist) correctly switch between local and S3 modes, and that URL construction works for both paths. Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@coderabbitai fullreview |
|
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 @.env.demo:
- Around line 137-139: Replace the concrete credentials in the .env template by
removing the weak defaults and using empty placeholders for RUSTFS_ACCESS_KEY_ID
and RUSTFS_SECRET_ACCESS_KEY (e.g. set them to empty values) and keep
RUSTFS_REGION as a default region if desired; also add a short inline comment in
the file near RUSTFS_ACCESS_KEY_ID / RUSTFS_SECRET_ACCESS_KEY that clearly
states these must be supplied at deploy time and must not be checked in. This
ensures the environment variables RUSTFS_ACCESS_KEY_ID and
RUSTFS_SECRET_ACCESS_KEY are not hard-coded and documents the requirement to
provide real secrets during deployment.
In `@apps/api-gateway/src/main.ts`:
- Line 125: The unconditional mount app.use('/uploadedFiles',
express.static('uploadedFiles')) exposes the whole storage tree; change this to
only mount explicit safe subdirectories (e.g., '/uploadedFiles/public' ->
express.static(path.join('uploadedFiles','public'))) and/or wrap the mount in a
feature-flag check (e.g., isLocalStorageEnabled() or
process.env.LOCAL_STORAGE_ENABLED) so the static middleware is registered only
when local storage serving is allowed; ensure you reference and update the
app.use call and the '/uploadedFiles' mount point accordingly and prefer
path.join for safe paths and a whitelist of allowed subfolders.
In `@libs/aws/src/aws.service.ts`:
- Around line 20-26: The constructor returns early when this.isLocalFs is true
but later code still expects AWS_BUCKET, AWS_ORG_LOGO_BUCKET_NAME, and
AWS_S3_STOREOBJECT_BUCKET to be set, causing path.join with undefined; update
the constructor (around the this.isLocalFs / localStoragePath logic) to validate
those env vars when this.isLocalFs is true and fail fast by throwing a clear
error (or set safe defaults) before returning so subsequent local file path
operations (that use those bucket envs) won’t receive undefined.
- Around line 155-158: The local delete in aws.service.ts currently does await
fs.unlink(filePath) inside the isLocalFs branch which will throw if the file
doesn't exist; make this idempotent like S3 by wrapping the unlink in a
try/catch inside the same method (the isLocalFs branch) and swallow errors whose
code === 'ENOENT' but rethrow any other errors so only missing-file cases are
ignored; ensure the try/catch surrounds the call referencing the same
localStoragePath/process.env.AWS_BUCKET/key resolution used to build filePath.
- Around line 81-87: Validate and normalize any user-supplied path segments
(pathAWS, key, objKey, filename) before joining them into filesystem paths:
strip or reject path traversal like "../" (e.g., replace with "" or use
path.basename for filename segments), use path.normalize on the combined path,
then compute the final filePath by path.join(bucketDir, normalizedSegment) and
verify the resulting absolute path starts with the bucketDir absolute path
(using path.resolve) — if it does not, throw an error. Apply this same
validation/sanitization logic wherever pathAWS/key/objKey are used (references:
bucketDir, fileKey, filePath, ensureDir, localStoragePath) to prevent escaping
uploadedFiles/<bucket> in local mode. Ensure returned URL still uses the
original safe URL-encoded filename but only after the filesystem checks pass.
- Around line 116-120: uploadCsvFile currently only ensures the bucket root dir
(using ensureDir on bucketDir) but then writes filePath directly, which fails
for nested keys like "foo/bar.csv"; update uploadCsvFile to create parent
directories for the final file by calling ensureDir on the file's directory
(e.g. path.dirname(filePath)) before calling fs.writeFile so nested prefixes in
key behave like S3; reference the existing localStoragePath,
process.env.AWS_BUCKET, key, filePath, ensureDir, and fs.writeFile when making
this change.
- Around line 49-64: The S3 clients (this.s3, this.s4, this.s3StoreObject)
always read AWS_*_REGION env vars but when IS_LOCAL_RUSTFS is set you should
prefer RUSTFS_REGION; update the region selection logic in the constructor
(where these S3 instances are created) to use process.env.RUSTFS_REGION when
process.env.IS_LOCAL_RUSTFS is truthy, falling back to the existing AWS_*_REGION
values otherwise so RustFS mode uses the correct region.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0c19a79-f348-4b2d-9ee3-990730efae19
📒 Files selected for processing (5)
.env.demoapps/api-gateway/src/main.tsapps/issuance/src/issuance.service.tsapps/utility/src/utilities.service.tslibs/aws/src/aws.service.ts
💤 Files with no reviewable changes (1)
- apps/issuance/src/issuance.service.ts
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
|
1 participant
What
Summary by CodeRabbit
New Features
Chores