Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Procfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
vite: vite dev
build: vite build --mode development --watch
nginx: nginx -c "$PWD/main.nginx.conf" -p "$PWD"
nginx: nginx -c "$PWD/nginx-conf/main.conf" -p "$PWD"
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,6 @@ If you run `npm run dev`, open ODK Central Frontend in a browser, then update th

To build ODK Central Frontend files for production with minification, run `npm run build`. The files will be outputted to `dist/`.

Note that this repository's `main.nginx.conf` is for development only.
Note that the nginx configuration files in `./nginx-conf` are for development only.

For more information on deploying to production, see the [`central`](https://github.com/getodk/central) repository.
2 changes: 1 addition & 1 deletion docs/enketo.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ ODK Central Backend is already configured to connect with Enketo. The following
},
```

ODK Central Frontend is also already configured for Enketo as well. The following lines should already be in [`main.nginx.conf`](../main.nginx.conf) to create a reverse proxy to Enketo.
ODK Central Frontend is also already configured for Enketo as well. The following lines should already be in [`./nginx-conf/main.conf`](../nginx-conf/main.conf) to create a reverse proxy to Enketo.

```
location /- {
Expand Down
File renamed without changes.
8 changes: 4 additions & 4 deletions main.nginx.conf → nginx-conf/main.conf
Copy link
Member

@matthew-white matthew-white Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the command in the Procfile is still -p "$PWD", do any file paths need to change in main.nginx.conf? For example, for something like:

include ./common-headers.nginx.conf;

Isn't ./ still the top-level directory of the repository? I'm wondering whether that line needs to change to:

include ./nginx-conf/common-headers.nginx.conf;

Or are include paths relative to the config file rather than the -p path?

Would it help anything to change the -p path to "$PWD/nginx-conf"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance you could give this branch a quick try? Given the motivation for this PR was #1376, which was motivated by not being able to run nginx -p in the first place...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthew-white did you get a chance to test this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried it, sorry for the delay! It's working. 🥳 I guess that means that some paths in the config file are relative to the -c path, not the -p path. That's good to know.

When I run npm run dev and load Frontend locally, I see:

  • A new nginx.pid file is created. That indicates that the pid directive is working.
  • New entries are added to nginx-access.log. That indicates that the access_log directive is working.
  • The response headers in common-headers.nginx.conf are returned. That indicates that the include directive is working.

What I didn't check:

  • alias ./dist/blank.html; for /-/single/check-submitted. Conceptually, it makes sense to me that alias would be relative to the -p path. We can always change it if that turns out to be wrong.
  • client_body_temp_path and proxy_temp_path. Again, it makes sense to me that those would be relative to the -p path. pid and access_log are both still working and similarly write to files in the .nginx directory.
  • npm run dev:build didn't work, so I wasn't able to check whether root ./dist; works as intended. However, it looks like npm run dev:build is broken on the master branch as well. I can file an issue about that. I almost always use npm run dev, which is probably why I hadn't noticed this already. I don't think this issue needs to block this PR from being merged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like npm run dev:build is broken on the master branch as well. I can file an issue about that.

Filed at getodk/central#1606.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and now I've closed the issue after realizing that npm run dev:build is working after all. I've confirmed that it works on the current master branch, which means that it plays well with the changes from this PR. It also indicates that root ./dist; is working correctly. That implies that root operates relative to the -p path, which makes sense.

Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ http {

server_tokens off;

include ./common-headers.nginx.conf;
include ./common-headers.conf;

client_max_body_size 100m;

Expand Down Expand Up @@ -137,15 +137,15 @@ http {
# first cookie ending in "session" to be the session cookie
add_header Set-Cookie $session_cookie;
# re-add common headers after add_header call
include ./common-headers.nginx.conf;
include ./common-headers.conf;

# Trick central-backend from thinking connections are coming
# over HTTPS so that ExpressJS will set "secure" cookies.
proxy_set_header X-Forwarded-Proto https;
}

location = /client-config.json {
include ./common-headers.nginx.conf;
include ./common-headers.conf;
return 200 "{}";
}
location /version.txt {
Expand All @@ -157,7 +157,7 @@ http {
root ./dist;
try_files $uri $uri/ /index.html;

include ./common-headers.nginx.conf;
include ./common-headers.conf;
# We return this header for more files in development than we do in
# production. That's needed because in development, unlike production,
# many file names don't contain hashes.
Expand Down