-
Notifications
You must be signed in to change notification settings - Fork 692
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
containers definition - allow to define caddy_routes #3192
base: main
Are you sure you want to change the base?
Conversation
082d37c
to
475c28e
Compare
In favour of #3192 Signed-off-by: Simon L <[email protected]>
Signed-off-by: Simon L <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
ddf198f
to
70224e6
Compare
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
Testing script for caddy.sh #!/bin/bash
export CADDY_ROUTES="|/browser,0,nextcloud-aio-collabora,9980;/hosting,0,nextcloud-aio-collabora,9980;/cool,0,nextcloud-aio-collabora,9980;/push,1,nextcloud-aio-notify-push,7867;/standalone-signaling,1,nextcloud-aio-talk,8081@mail.|,0,[email protected].|/a/b,0,server;/a/b/c,0,server;,0,server;/a,0,server"
export APACHE_PORT="443"
export APACHE_HOST="localhost"
export TRUSTED_DOMAINS="{\$ADDITIONAL_TRUSTED_DOMAIN},{\$NC_DOMAIN}"
export CADDY_ROUTES PROTOCOL NC_DOMAIN APACHE_PORT TRUSTED_DOMAINS
bash caddyfile.sh Output : {
auto_https disable_redirects
storage file_system {
root /mnt/data/caddy
}
servers {
trusted_proxies static 127.0.0.0/16
}
log {
level ERROR
}
}
https://verif.sort.{$ADDITIONAL_TRUSTED_DOMAIN}:443,
https://verif.sort.{$NC_DOMAIN}:443 {
header -Server
header -X-Powered-By
route /a/b/c/* {
reverse_proxy server
}
route /a/b/* {
reverse_proxy server
}
route /a/* {
reverse_proxy server
}
route {
reverse_proxy server
}
}
https://mail.{$ADDITIONAL_TRUSTED_DOMAIN}:443,
https://mail.{$NC_DOMAIN}:443 {
header -Server
header -X-Powered-By
route {
reverse_proxy nextcloud-aio-stalwart
}
}
https://{$ADDITIONAL_TRUSTED_DOMAIN}:443,
https://{$NC_DOMAIN}:443 {
header -Server
header -X-Powered-By
route /standalone-signaling/* {
uri strip_prefix /standalone-signaling
reverse_proxy nextcloud-aio-talk:8081
}
route /push/* {
uri strip_prefix /push
reverse_proxy nextcloud-aio-notify-push:7867
}
route /hosting/* {
reverse_proxy nextcloud-aio-collabora:9980
}
route /cool/* {
reverse_proxy nextcloud-aio-collabora:9980
}
route /browser/* {
reverse_proxy nextcloud-aio-collabora:9980
}
route {
header Strict-Transport-Security max-age=31536000;
reverse_proxy localhost:8000
}
redir /.well-known/carddav /remote.php/dav/ 301
redir /.well-known/caldav /remote.php/dav/ 301
tls {
issuer acme {
disable_http_challenge
}
}
} |
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
What do you think @szaimen? I tried to make it as generic as possible. Known issue, subdomains are incompatible with trusted proxy mode. |
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.
Hi @docjyJ , thanks a lot for working on this!
I had a look at your changes and I fear I have some general problems with this which is why I didnt push it further...
- We need to keep the Caddyfile as fallback for cases where the mastercontainer is not there (see the manual-install)
- I'd for now like to remove sub_domain from the implementation as this was actually only intended for routes --> since this then limits the use-case to pretty much to the already bundled services, I didn't really pushed this forward... However we could in theory move the onlyoffice container then to community-containers to make it easier to discover... But I am not certain about this yet.
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
Hi! I attempted to fix the issues you mentioned. Don't hesitate to give feedback! Take care, |
Signed-off-by: Jean-Yves <[email protected]>
@szaimen ready for review |
Hi @docjyJ sorry for my late reply. So as I said, I would like to keep the Caddyfile as fallback if it is used with the manual-install. You could adjust the logic in start.sh to copy the file from there if the variable CADDY_ROUTES was not found. Additionally, we then need to keep the env variables: However I wonder how useful this change then is in the end if we need to maintain two places and keep it in sync without a real benefit at least in the current state? Sorry for bringing this only up so late 🙈 |
This PR will make sense as soon as we have a real use-case for it, e.g. when a community-container needs such a route. As long as that is not the case, I think it does not make much sense to merge this. I am sorry 🙈 |
No worries, I understand. It will come in handy someday. By the way, what do you think of using Caddy without Apache: https://help.nextcloud.com/t/the-optimal-fastest-nextcloud-fpm-docker-setup-with-caddy-as-webserver-and-https-proxy/110515 I don't know how much it can help make AIO faster, but it could be interesting. |
I have seen this and looks interesting but only Apache is officially supported: https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html#nginx-configuration Also only Apache can read the shipped htaccess files, e.g. this one, so we would need to keep the config always manually up-to-date which adds some maintenance burden... However would of course be interesting to see if it potentially speeds up things... |
Address part of #3889
todo