feat(deploy): Dockerfile + nginx for Bunnyshell ephemeral env - #4125
rvignesh89 wants to merge 1 commit into
Conversation
Container build for the staff console so it can run as a component of a Bunnyshell environment (orchestrated by glific/bunnyshell.yaml): - Dockerfile — multi-stage Vite build; VITE_GLIFIC_BACKEND_URL build arg (bare backend hostname; config/index.ts derives /api + /socket URLs) - nginx.conf — single-page-app serving - .dockerignore Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnEEUjf5kpVzsmQbbdkZpd
WalkthroughThe change adds Docker build-context exclusions, a multi-stage Dockerfile, and an Nginx configuration. The build stage installs dependencies with Yarn, generates flow-editor assets, and runs the Vite build with configurable environment variables. The runtime stage serves the generated files on port 80. Nginx provides SPA fallback routing and disables caching for Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
🚀 Deployed on https://deploy-preview-4125--glific-frontend.netlify.app |
![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 `@Dockerfile`:
- Around line 13-22: Update the Docker build-argument flow around
VITE_GLIFIC_BACKEND_URL so the caller passes the required bare backend hostname
instead of leaving it empty; align the relevant buildspec argument names with
this Dockerfile declaration. Before removing or changing VITE_GLIFIC_API,
VITE_WEB_SOCKET, or VITE_FLOW_EDITOR_API, verify that no other build path
consumes them, and preserve any still-required arguments.
- Around line 33-37: Update the runtime stage based on nginx:1.27-alpine to run
as a non-root user, preferably by using nginxinc/nginx-unprivileged. Ensure
nginx.conf listens on port 8080, and keep the EXPOSE declaration and Bunnyshell
service target aligned with that port.
In `@nginx.conf`:
- Around line 12-14: Update the Cache-Control value in the `/index.html`
location block to `no-store` if the documented policy requires preventing caches
from storing the response; otherwise retain `no-cache` and align the
documentation with that revalidation-only behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c202cb9c-ac1e-4ac8-84d8-e3280df0ff49
📒 Files selected for processing (3)
.dockerignoreDockerfilenginx.conf
| ARG VITE_GLIFIC_BACKEND_URL | ||
| # Leave prefix/port empty: when VITE_GLIFIC_BACKEND_URL is set, config/index.ts uses the | ||
| # host verbatim over https:443, so an "api." prefix or :4001 port would break the URL. | ||
| ARG VITE_API_PREFIX="" | ||
| ARG VITE_GLIFIC_API_PORT="" | ||
| ARG VITE_APPLICATION_NAME="Glific: Two way communication platform" | ||
| ENV VITE_GLIFIC_BACKEND_URL=$VITE_GLIFIC_BACKEND_URL \ | ||
| VITE_API_PREFIX=$VITE_API_PREFIX \ | ||
| VITE_GLIFIC_API_PORT=$VITE_GLIFIC_API_PORT \ | ||
| VITE_APPLICATION_NAME=$VITE_APPLICATION_NAME |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the Docker build arguments with the existing build path.
buildspec.yml.sample passes VITE_GLIFIC_API, VITE_WEB_SOCKET, and VITE_FLOW_EDITOR_API on Lines 14-19. This Dockerfile declares none of those arguments. Therefore, VITE_GLIFIC_BACKEND_URL remains empty when that build path is used.
src/config/index.ts then falls back to window.location.hostname. Deployments with a separate backend can send API and socket requests to the staff-console host.
Update the caller and pass VITE_GLIFIC_BACKEND_URL as the required bare backend hostname. Remove or update the legacy arguments only after confirming that no other build path consumes them.
🤖 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 `@Dockerfile` around lines 13 - 22, Update the Docker build-argument flow
around VITE_GLIFIC_BACKEND_URL so the caller passes the required bare backend
hostname instead of leaving it empty; align the relevant buildspec argument
names with this Dockerfile declaration. Before removing or changing
VITE_GLIFIC_API, VITE_WEB_SOCKET, or VITE_FLOW_EDITOR_API, verify that no other
build path consumes them, and preserve any still-required arguments.
| FROM nginx:1.27-alpine AS runtime | ||
| COPY nginx.conf /etc/nginx/conf.d/default.conf | ||
| COPY --from=build /app/build /usr/share/nginx/html | ||
| EXPOSE 80 | ||
| CMD ["nginx", "-g", "daemon off;"] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env sh
set -eu
docker run --rm --entrypoint id nginx:1.27-alpine -uRepository: glific/glific-frontend
Length of output: 204
🌐 Web query:
nginxinc docker-nginx 1.27 alpine Dockerfile USER nginx non-root port 80
💡 Result:
Running the official NGINX Docker image (e.g., nginx:1.27-alpine) as a non-root user and attempting to bind to port 80 will fail because ports numbered 1–1024 are privileged; a non-root user does not have permission to bind to them [1][2]. If your goal is to run NGINX in a container without root privileges, you have two primary options: 1. Use the official unprivileged NGINX image: The NGINX maintainers provide a dedicated image, nginxinc/nginx-unprivileged, which is preconfigured to run as a non-root user [3][4][2]. This image listens on port 8080 by default (avoiding the need for root privileges), has the NGINX user directive removed, and moves the PID file and temporary paths to writable locations like /tmp [3][4][2]. You should update your Dockerfile or compose file to use this image and map your host port to 8080 instead of 80 [4][2]. 2. Configure the official NGINX image manually: If you must use the standard official NGINX image, you cannot simply add a USER instruction in your Dockerfile to switch to the nginx user [2]. To make it work, you must: * Change the listening port in your NGINX configuration (e.g., server { listen 8080; }) to a value above 1024 [1][2]. * Modify your nginx.conf to redirect the PID file and all temporary path directives (such as client_body_temp_path, proxy_temp_path, etc.) to directories that the non-root user has write access to, such as /tmp [5][6][2]. * Ensure the container is started with a user that has the necessary permissions for the configured directories [5][2]. In summary, for production hardening or strict security requirements, using the nginxinc/nginx-unprivileged image is the recommended best practice as it significantly reduces the configuration overhead required to safely run NGINX without root access [3][2].
Citations:
- 1: https://stackoverflow.com/questions/65626346/running-nginx-container-as-non-root-from-nginx-alpine-image
- 2: https://www.dash0.com/faq/run-nginx-in-docker
- 3: https://github.com/nginxinc/docker-nginx-unprivileged
- 4: https://hub.docker.com/r/nginxinc/nginx-unprivileged/
- 5: https://github.com/docker-library/docs/blob/ccc8b3ef/nginx/README.md
- 6: https://hub.docker.com/_/nginx
🌐 Web query:
nginxinc docker-nginx 1.27 alpine Dockerfile USER nginx 1.27-alpine group nginx
💡 Result:
In the official nginx:1.27-alpine Docker image, a system user and group named "nginx" are created during the build process [1][2][3]. The Dockerfile explicitly executes the following command to set this up: addgroup -g 101 -S nginx && adduser -S -D -H -u 101 -h /var/cache/nginx -s /sbin/nologin -G nginx -g nginx nginx [1][2][3] However, the Docker image does not contain a USER instruction (e.g., USER nginx). Consequently, the container starts by default as the root user [4][5]. While the container runs as root, the NGINX worker processes are designed to drop privileges and run as the "nginx" user (UID 101, GID 101) [4][5]. If you wish to run the entire container process as a non-root user, you must add a USER instruction to your own Dockerfile derived from this image or use Docker's built-in options to specify a user at runtime [4][5]. Note that simply changing the user requires you to ensure that the user has the necessary permissions for the directories NGINX uses (such as logs, cache, and PID files), which may require adjusting your NGINX configuration [4][5].
Citations:
- 1: https://github.com/nginxinc/docker-nginx/blob/94a27ac42d45670d941a55334d89e80760f7cc8e/mainline/alpine-slim/Dockerfile
- 2: https://github.com/nginxinc/docker-nginx/blob/1.21.6/Dockerfile-alpine.template
- 3: https://github.com/nginx/docker-nginx/blob/71081b25390771f6b1275ddf7c73c965f304493f/mainline/alpine-slim/Dockerfile
- 4: https://hub.docker.com/_/nginx?tab=description
- 5: https://hub.docker.com/_/nginx
Run the runtime image as a non-root user.
nginx:1.27-alpine creates the nginx user but does not set USER, so this stage starts as root and the master process retains elevated privileges. Use nginxinc/nginx-unprivileged for a preconfigured unprivileged image, or switch to a non-root user with writable PID, cache, and temp directories and move the listen port above 1024. If the non-root setup listens on port 8080, update nginx.conf, EXPOSE, and the Bunnyshell service target together.
🤖 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 `@Dockerfile` around lines 33 - 37, Update the runtime stage based on
nginx:1.27-alpine to run as a non-root user, preferably by using
nginxinc/nginx-unprivileged. Ensure nginx.conf listens on port 8080, and keep
the EXPOSE declaration and Bunnyshell service target aligned with that port.
Source: Linters/SAST tools
| location = /index.html { | ||
| add_header Cache-Control "no-cache"; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== nginx.conf present and relevant lines =="
if git ls-files | grep -qx 'nginx.conf'; then
nl -ba nginx.conf | sed -n '1,80p'
else
fd -a 'nginx.conf$' .
fi
echo "== search for PR/cache statements =="
rg -n -i 'caching|cache|cache control|Cache-Control|no-store|no-cache|index.html|disabled|disable' . -g '!*.lock' -g '!node_modules' | sed -n '1,200p'Repository: glific/glific-frontend
Length of output: 23798
🌐 Web query:
MDN Cache-Control no-cache storage allowed revalidation no-store not stored
💡 Result:
The no-cache and no-store directives serve distinct purposes in HTTP caching, and they are often misunderstood [1][2]. no-cache The no-cache directive indicates that a response can be stored in caches, but it cannot be reused to satisfy subsequent requests without first being validated with the origin server [3][4][5]. It does not mean "do not cache" [3][6]. When a cache has a no-cache response, it must send a conditional request (using headers like If-None-Match or If-Modified-Since) to the origin [1][2][7]. If the origin confirms the content is still valid (e.g., returning a 304 Not Modified status), the cache can then serve the stored response [1][7][8]. no-store The no-store directive is the mechanism to prevent caching entirely [3][6]. It instructs caches (both private and shared) not to store any part of the request or the corresponding response [3][4][9]. It requires that no information be intentionally kept in non-volatile storage, and caches should make a best-effort attempt to remove the information from volatile storage as soon as possible after forwarding it [9][5]. In summary, no-cache permits storage but mandates revalidation before use, while no-store prohibits storage altogether [1][2]. For sensitive data, such as personal or financial information, no-store is the appropriate choice [1][2][7].
Citations:
- 1: https://http.dev/cache-control
- 2: https://httpfixer.dev/blog/cache/no-cache-vs-no-store/
- 3: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cache-Control
- 4: https://github.com/mdn/content/blob/main/files/en-us/web/http/reference/headers/cache-control/index.md
- 5: https://datatracker.ietf.org/doc/html/rfc9111.html
- 6: https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Caching
- 7: https://orankit.com/en/blog/http-cache-control-deep-dive/
- 8: https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Conditional_requests
- 9: https://greenbytes.de/tech/specs/rfc9111.pdf
Use no-store if /index.html must not be cached.
Cache-Control: no-cache permits caches to store the response and only requires revalidation before reuse. If the intended policy is to disable caching storage, use no-store; otherwise keep no-cache but align it with the documented policy.
Proposed cache policy
- add_header Cache-Control "no-cache";
+ add_header Cache-Control "no-store";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| location = /index.html { | |
| add_header Cache-Control "no-cache"; | |
| } | |
| location = /index.html { | |
| add_header Cache-Control "no-store"; | |
| } |
🤖 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 `@nginx.conf` around lines 12 - 14, Update the Cache-Control value in the
`/index.html` location block to `no-store` if the documented policy requires
preventing caches from storing the response; otherwise retain `no-cache` and
align the documentation with that revalidation-only behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4125 +/- ##
=======================================
Coverage 82.19% 82.20%
=======================================
Files 346 346
Lines 15119 15119
Branches 3582 3582
=======================================
+ Hits 12427 12428 +1
+ Misses 1634 1633 -1
Partials 1058 1058 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Glific
 |
||||||||||||||||||||||||||||||
| Project |
Glific
|
| Branch Review |
bunnyshell
|
| Run status |
|
| Run duration | 07m 25s |
| Commit |
|
| Committer | Vignesh Rajasekaran |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
32
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
This branch was previously deployed
1 participant
Container build for the staff console so it can run as a component of a Bunnyshell ephemeral environment. Orchestrated by
bunnyshell.yamlin the backend repo (glific#5524).What's included
VITE_GLIFIC_BACKEND_URLbuild arg (bare backend hostname;config/index.tsderives the/api+/socketURLs)Infra only — no application code changes.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores