vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: nginx improvements #1545

Merged
konrad merged 1 commits from dpschen/frontend:feature/nginx-improvements into main 2022-04-02 16:12:47 +00:00
Conversation 4 Commits 1 Files Changed 1 +88 -57
1 changed files with 88 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

145
nginx.conf
View File

@ -6,79 +6,110 @@ pid /var/run/nginx.pid;
events {
worker_connections 1024;
worker_connections 1024;
}
http {
include /etc/nginx/mime.types;
default_type application/octet-stream;
include /etc/nginx/mime.types;
default_type application/octet-stream;
log_format main '$remote_addr - $remote_user [$time_local] "$request" '
'$status $body_bytes_sent "$http_referer" '
'"$http_user_agent" "$http_x_forwarded_for"';
types {
dpschen marked this conversation as resolved Outdated
dpschen commented 2022-02-15 13:15:39 +00:00
Outdated
Review
Copy Link

See: https://vite-plugin-pwa.netlify.app/deployment/nginx.html#configure-manifest-webmanifest-mime-type
This seems to be necessary, since it returns:

→ curl -s -I -X GET https://try.vikunja.io/manifest.webmanifest | grep content-type -i
content-type: application/octet-stream
x-content-type-options: nosniff
See: https://vite-plugin-pwa.netlify.app/deployment/nginx.html#configure-manifest-webmanifest-mime-type This seems to be necessary, since it returns: ```sh → curl -s -I -X GET https://try.vikunja.io/manifest.webmanifest | grep content-type -i content-type: application/octet-stream x-content-type-options: nosniff ```
✅ 1
application/manifest+json webmanifest;
}
access_log /var/log/nginx/access.log main;
log_format main '$remote_addr - $remote_user [$time_local] "$request" '
'$status $body_bytes_sent "$http_referer" '
'"$http_user_agent" "$http_x_forwarded_for"';
sendfile on;
#tcp_nopush on;
access_log /var/log/nginx/access.log main;
keepalive_timeout 65;
sendfile on;
#tcp_nopush on;
gzip on;
gzip_disable "msie6";
keepalive_timeout 65;
gzip_vary on;
gzip_proxied any;
gzip_comp_level 6;
gzip_buffers 16 8k;
gzip_http_version 1.1;
gzip_min_length 256;
gzip_types text/plain text/css application/json application/x-javascript application/javascript text/xml application/xml application/xml+rss text/javascript application/vnd.ms-fontobject application/x-font-ttf font/opentype image/svg+xml image/x-icon audio/wav;
gzip on;
map_hash_max_size 128;
map_hash_bucket_size 128;
gzip_vary on;
gzip_proxied any;
gzip_comp_level 6;
gzip_buffers 16 8k;
gzip_http_version 1.1;
gzip_min_length 256;
gzip_types
dpschen marked this conversation as resolved Outdated
dpschen commented 2022-02-20 09:48:21 +00:00
Outdated
Review
Copy Link

This stayed the same.
It's just indented

This stayed the same. It's just indented
✅ 1
text/plain
text/css
application/json
application/x-javascript
application/javascript
text/xml
application/xml
application/xml+rss
text/javascript
application/vnd.ms-fontobject
application/x-font-ttf
font/opentype
image/svg+xml
image/x-icon
audio/wav;
# Expires map
map $sent_http_content_type $expires {
default off;
text/html max;
dpschen marked this conversation as resolved Outdated
dpschen commented 2022-02-15 13:10:06 +00:00
Outdated
Review
Copy Link

If we cache html, how should the browser know of new changes? Or am I missing something?

If we cache html, how should the browser know of new changes? Or am I missing something?
konrad commented 2022-02-19 21:10:03 +00:00
Outdated
Review
Copy Link

That's correct. We should not cache the html.

That's correct. We should not cache the html.
text/css max;
application/javascript max;
text/javascript max;
application/vnd.ms-fontobject max;
application/x-font-ttf max;
font/opentype max;
font/woff2 max;
image/svg+xml max;
image/x-icon max;
audio/wav max;
~image/ max;
~font/ max;
}
map_hash_max_size 128;
map_hash_bucket_size 128;
dpschen commented 2022-02-15 13:10:20 +00:00
Outdated
Review
Copy Link

This path was wrong

This path was wrong
konrad commented 2022-02-19 21:11:12 +00:00
Outdated
Review
Copy Link

What about mime types like image/png? Those would not get cached with the new implementation you proposed here, but would get cached with the old one iirc.

What about mime types like `image/png`? Those would not get cached with the new implementation you proposed here, but would get cached with the old one iirc.
dpschen commented 2022-02-20 09:33:26 +00:00
Outdated
Review
Copy Link

Where did it get cached in the old implementation? Not sure where I would have removed that?

Where did it get cached in the old implementation? Not sure where I would have removed that?
konrad commented 2022-02-20 16:29:39 +00:00
Outdated
Review
Copy Link

IIRC using ~ at the beginning tells nginx to handle this like a regular expression. That would still not cache images I think hmmmm. I'm not even sure what this does. Did you test it?

These may even duplicated since we're already adding a caching header to everything in /assets?

IIRC using `~` at the beginning tells nginx to handle this like a regular expression. That would still not cache images I think hmmmm. I'm not even sure what this does. Did you test it? These may even duplicated since we're already adding a caching header to everything in `/assets`?
dpschen commented 2022-02-20 21:17:14 +00:00
Outdated
Review
Copy Link

I didn't test.
To be honest I'm only guessing here 🤷‍♂️

I didn't test. To be honest I'm only guessing here 🤷‍♂️
konrad commented 2022-02-20 21:34:49 +00:00
Outdated
Review
Copy Link

Can you test it?

Can you test it?
dpschen commented 2022-02-21 21:21:49 +00:00
Outdated
Review
Copy Link

Yes, but probably my time is better spent reviewing other branches :D
I've almost no experience setting nginx up (mostly I was just messing around with an existing config).

Yes, but probably my time is better spent reviewing other branches :D I've almost no experience setting nginx up (mostly I was just messing around with an existing config).
😆 1
konrad commented 2022-02-21 21:38:49 +00:00
Outdated
Review
Copy Link

huh yeah I get that :D

The nginx config in the repo is the one used by the docker file. That means running

docker build . -t vikunja/frontend:dev
docker run -p 10002:80 vikunja/frontend:dev

should build everything and make the frontend accessible on port 10002 on your machine with the new config (at least that's what I used to test it).

Will check tomorrow (or after that) if everything works as expected.

huh yeah I get that :D The nginx config in the repo is the one used by the docker file. That means running ``` docker build . -t vikunja/frontend:dev docker run -p 10002:80 vikunja/frontend:dev ``` should build everything and make the frontend accessible on port 10002 on your machine with the new config (at least that's what I used to test it). Will check tomorrow (or after that) if everything works as expected.
👍 1
dpschen commented 2022-02-21 21:42:40 +00:00
Outdated
Review
Copy Link

That helps, will try.

That helps, will try.
server {
listen 80;
listen 81 default_server http2 proxy_protocol; ## Needed when behind HAProxy with SSL termination + HTTP/2 support
# Expires map
map $sent_http_content_type $expires {
default off;
text/css max;
application/javascript max;
text/javascript max;
application/vnd.ms-fontobject max;
application/x-font-ttf max;
font/opentype max;
font/woff2 max;
image/svg+xml max;
image/x-icon max;
audio/wav max;
dpschen marked this conversation as resolved Outdated
dpschen commented 2022-02-15 13:12:17 +00:00
Outdated
Review
Copy Link

Not sure if this is a duplication.
I remember also that we had some problems with try_files.

Not sure if this is a duplication. I remember also that we had some problems with `try_files`.
konrad commented 2022-02-19 21:13:06 +00:00
Outdated
Review
Copy Link

I think this is fine. Back when I initially created the nginx.conf file, we didn't have vite so all assets would go in top-level css/, js/, etc folders, not all bundled up in assets/.

I think this is fine. Back when I initially created the nginx.conf file, we didn't have vite so all assets would go in top-level `css/`, `js/`, etc folders, not all bundled up in `assets/`.
👍 1
~images/ max;
~font/ max;
}
server_name _;
server {
listen 80;
listen 81 default_server http2 proxy_protocol; ## Needed when behind HAProxy with SSL termination + HTTP/2 support
expires $expires;
server_name _;
location ~* .(txt|webmanifest|css|js|mjs|map|svg|jpg|jpeg|png|ico|ttf|woff|woff2|wav)$ {
root /usr/share/nginx/html;
dpschen marked this conversation as resolved Outdated
dpschen commented 2022-02-15 13:09:06 +00:00
Outdated
Review
Copy Link

See https://www.nginx.com/resources/wiki/start/topics/tutorials/config_pitfalls/#root-inside-location-block

See https://www.nginx.com/resources/wiki/start/topics/tutorials/config_pitfalls/#root-inside-location-block
✅ 1
try_files $uri $uri/ =404;
}
expires $expires;
location / {
root /usr/share/nginx/html;
try_files $uri $uri/ /index.html;
index index.html;
}
root /usr/share/nginx/html;
error_page 500 502 503 504 /50x.html;
location = /50x.html {
root /usr/share/nginx/html;
}
}
# all assets contain hash in filename, cache forever
location ^~ /assets/ {
add_header Cache-Control "public, max-age=31536000, s-maxage=31536000, immutable";
try_files $uri =404;
}
# all workbox scripts are compiled with hash in filename, cache forever3
location ^~ /workbox- {
add_header Cache-Control "public, max-age=31536000, s-maxage=31536000, immutable";
try_files $uri =404;
dpschen marked this conversation as resolved Outdated
konrad commented 2022-02-19 21:09:23 +00:00
Outdated
Review
Copy Link

Now there's two location / blocks. This breaks the config:

2022/02/19 21:06:32 [emerg] 30#30: duplicate location "/" in /etc/nginx/nginx.conf:96
nginx: [emerg] duplicate location "/" in /etc/nginx/nginx.conf:96
Now there's two `location /` blocks. This breaks the config: ``` 2022/02/19 21:06:32 [emerg] 30#30: duplicate location "/" in /etc/nginx/nginx.conf:96 nginx: [emerg] duplicate location "/" in /etc/nginx/nginx.conf:96 ```
✅ 1
dpschen commented 2022-02-20 09:51:41 +00:00
Outdated
Review
Copy Link

I removed the old block

I removed the old block
}
# assume that everything else is handled by the application router, by injecting the index.html.
location / {
autoindex off;
expires off;
add_header Cache-Control "public, max-age=0, s-maxage=0, must-revalidate" always;
try_files $uri /index.html =404;
dpschen marked this conversation as resolved Outdated
dpschen commented 2022-02-20 09:53:40 +00:00
Outdated
Review
Copy Link

I hope this works =)

I hope this works =)
konrad commented 2022-02-20 16:36:30 +00:00
Outdated
Review
Copy Link

Looks like it :)

Looks like it :)
👍 1
}
location ~* .(txt|webmanifest|css|js|mjs|map|svg|jpg|jpeg|png|ico|ttf|woff|woff2|wav)$ {
dpschen commented 2022-02-20 09:51:06 +00:00
Outdated
Review
Copy Link

Not sure if this still makes sense

Not sure if this still makes sense
konrad commented 2022-02-20 16:30:41 +00:00
Outdated
Review
Copy Link

Why wouldn't it?

Why wouldn't it?
dpschen commented 2022-02-20 21:15:11 +00:00
Outdated
Review
Copy Link

Because all those files should be in the assets folder or have their own rule.

Because all those files should be in the assets folder or have their own rule.
konrad commented 2022-02-20 21:34:00 +00:00
Outdated
Review
Copy Link

I guess it won't hurt to keep them as a backup.

I guess it won't hurt to keep them as a backup.
✅ 1
try_files $uri $uri/ =404;
}
error_page 500 502 503 504 /50x.html;
location = /50x.html {
}
}
}