-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fix: allow running as non-root #40642
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
Conversation
WalkthroughThe changes update Docker build and entrypoint scripts to improve package installation, image cleanup, and user identity handling. The base Dockerfile now installs additional packages, consolidates cleanup steps, and sets up NSS Wrapper for non-root user support. The entrypoint script configures NSS Wrapper when running as a non-root user and prevents embedded Postgres initialization in non-root mode. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Entrypoint Script
participant NSS Wrapper
User->>Entrypoint Script: Start container
Entrypoint Script->>Entrypoint Script: Check if running as non-root
alt Non-root user
Entrypoint Script->>NSS Wrapper: Create temp passwd/group files with current UID/GID
Entrypoint Script->>Entrypoint Script: Export LD_PRELOAD with NSS Wrapper symlink
end
Entrypoint Script->>Entrypoint Script: Log current UID
Entrypoint Script->>Entrypoint Script: If init_postgres, call fail_if_non_root to prevent embedded DB init
Entrypoint Script->>User: Continue application startup
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14976096450. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14976813608. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14980450347. |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14980505949. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14980819023. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14981979160. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15005175710. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15031794549. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15048572151. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15048924049. |
Deploy-Preview-URL: https://ce-40642.dp.appsmith.com |
|
||
RUN mv /opt/caddy/caddy /opt/caddy/caddy_vanilla | ||
| tar -xz -C /opt/caddy && \ | ||
mv /opt/caddy/caddy /opt/caddy/caddy_vanilla |
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.
This removed an extra layer, removing ~50MB from the image.
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
## Description Allows the Appsmith container to run as a non-root user, specified at runtime through either docker-compose or Kubernetes pod security context. I didn't specify the user in the `Dockerfile` because environments like OpenShift choose a user at runtime, so it can't be known at build time. This needs to be followed by an update to docs and changes in the Helm chart to finish it off, but that has a separate release cycle and this needs to go ahead of that. Ideally we would run as non-root by default, but since there's data persisted on the filesystem automatically transitioning the default is impossible without a lot of pain. This moves us in that direction and enables it in the future if we go down that path. Required to fix #38787 ## Automation /ok-to-test tags="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!WARNING] > Tests have not run on the HEAD 50ba745 yet > <hr>Thu, 15 May 2025 15:56:31 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for user identity emulation when running as a non-root user, improving compatibility in certain deployment environments. - Prevented embedded database initialization when running as a non-root user to ensure proper operation. - **Chores** - Installed additional system packages to the base image for enhanced functionality. - Optimized image size by consolidating and improving cleanup steps during the build process. - Updated base image and refined installation commands for improved build consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Description Allows the Appsmith container to run as a non-root user, specified at runtime through either docker-compose or Kubernetes pod security context. I didn't specify the user in the `Dockerfile` because environments like OpenShift choose a user at runtime, so it can't be known at build time. This needs to be followed by an update to docs and changes in the Helm chart to finish it off, but that has a separate release cycle and this needs to go ahead of that. Ideally we would run as non-root by default, but since there's data persisted on the filesystem automatically transitioning the default is impossible without a lot of pain. This moves us in that direction and enables it in the future if we go down that path. Required to fix #38787 ## Automation /ok-to-test tags="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!WARNING] > Tests have not run on the HEAD 50ba745 yet > <hr>Thu, 15 May 2025 15:56:31 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for user identity emulation when running as a non-root user, improving compatibility in certain deployment environments. - Prevented embedded database initialization when running as a non-root user to ensure proper operation. - **Chores** - Installed additional system packages to the base image for enhanced functionality. - Optimized image size by consolidating and improving cleanup steps during the build process. - Updated base image and refined installation commands for improved build consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="" ### :mag: Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No Co-authored-by: Wyatt Walter <wyattwalter@gmail.com>
## Description Requires: #40642 Adds an environment variable activating nss_wrapper when the `securityPolicy.runAsUser` value is set so the UID can be set dynamically. This avoids the `I have no name!` in the prompt when doing a `kubectl exec` with that value set. I am also introducing [helm-unittest](https://github.com/helm-unittest/helm-unittest) for ensuring that changes to our defaults are made explicit. Fixes #38787 ## Automation /ok-to-test tags="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!WARNING] > Tests have not run on the HEAD d30d87f yet > <hr>Mon, 19 May 2025 18:59:10 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added automated unit testing for Helm charts, including snapshot and security context tests. - Introduced documentation for running and understanding Helm chart unit tests. - Added a GitHub Actions workflow to run Helm chart unit tests on pull requests and manually. - **Bug Fixes** - Ensured the LD_PRELOAD environment variable is set when a specific security context is configured in deployments. - **Documentation** - Updated Helm chart README to reference new testing documentation and improve clarity. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Goutham Pratapa <goutham@appsmith.com>
Description
Allows the Appsmith container to run as a non-root user, specified at runtime through either docker-compose or Kubernetes pod security context. I didn't specify the user in the
Dockerfile
because environments like OpenShift choose a user at runtime, so it can't be known at build time.This needs to be followed by an update to docs and changes in the Helm chart to finish it off, but that has a separate release cycle and this needs to go ahead of that.
Ideally we would run as non-root by default, but since there's data persisted on the filesystem automatically transitioning the default is impossible without a lot of pain. This moves us in that direction and enables it in the future if we go down that path.
Required to fix #38787
Automation
/ok-to-test tags=""
🔍 Cypress test results
Warning
Tests have not run on the HEAD 50ba745 yet
Thu, 15 May 2025 15:56:31 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Chores