Fix: Make upload size configurable via env and propagate to Nginx (Issue #30)#31
Fix: Make upload size configurable via env and propagate to Nginx (Issue #30)#31Musicminion wants to merge 1 commit intoserver-profrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a1342898f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export NGINX_KEEPALIVE_TIMEOUT="${NGINX_KEEPALIVE_TIMEOUT:-65}" | ||
| export NGINX_WORKER_CONNECTIONS="${NGINX_WORKER_CONNECTIONS:-768}" | ||
| export NGINX_WORKER_PROCESSES="${NGINX_WORKER_PROCESSES:-4}" | ||
| export MAX_UPLOAD_SIZE_NGINX="${MAX_UPLOAD_SIZE:-50}m" |
There was a problem hiding this comment.
Normalize MAX_UPLOAD_SIZE before building Nginx size
MAX_UPLOAD_SIZE_NGINX is built by blindly appending m to MAX_UPLOAD_SIZE, so values that the web config currently accepts via parseInt (for example MAX_UPLOAD_SIZE=100m or 100MB) become invalid Nginx sizes like 100mm/100MBm. In that case nginx -t in this script fails and the container cannot finish startup, even though the JS side still interprets the same env var as 100 MB. This introduces a parser mismatch between services/web/config/settings.defaults.js and the Nginx templating path.
Useful? React with 👍 / 👎.
Motivation
See #30
Description
MAX_UPLOAD_SIZE(in MB) inservices/web/config/settings.defaults.jssomaxUploadSizeis nowparseInt(process.env.MAX_UPLOAD_SIZE || '50', 10) * 1024 * 1024instead of a hardcoded 50 MB.MAX_UPLOAD_SIZE_NGINXfromserver-ce/init_scripts/200_nginx_config_template.shdefaulting to50mand include it in theenvsubstpass-list.client_max_body_size 50m;inserver-ce/nginx/nginx.conf.templatewithclient_max_body_size ${MAX_UPLOAD_SIZE_NGINX};so the generatednginx.confrespects the env var.Testing
npm testand all tests passed.npm run lintandshellcheckon the shell script and no issues were reported.nginx -texecuted by the init script (invoked in CI) returns successful configuration validation.Codex Task