-
Notifications
You must be signed in to change notification settings - Fork 191
fix: add support for anyuid to Dockerfile #658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add support for anyuid to Dockerfile #658
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughModified Dockerfile permission setup for /app and /data directories. Replaced direct chown of /data with a multi-step approach: /app now receives recursive chown to sourcebot, chgrp to root group (0), and chmod for group write permissions; /data receives chown to sourcebot only. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
fc09e55 to
a812b23
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
Dockerfile (1)
252-253: Inconsistency: /data setup doesn't match stated PR intent or /app setup.The review identifies a potential discrepancy: if the PR description states "both /app and /data to use the root group by default," but only
/appreceiveschgrp -R 0andchmod -R g=upermissions, then/datalacks group-write access needed in OpenShift anyuid mode.Line 253 only runs
chown -R sourcebot /datawithout group or permission changes. In anyuid execution contexts, this prevents non-sourcebot users (running with arbitrary UID) from writing to/data.If
/datarequires write access: apply the same permission setup as/app.
If/datais intentionally read-only: clarify in PR description.
This change updates the Containerfile to support arbitrary userids to enable running in Openshift.
It does this by adding group write permissions to
/appand setting both/appand/datato use the root group by default.This should not have any impact on existing environments running as
sourcebotuser since all of the directories changed are owned by thesourcebotuser.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.