Drupal 10, the latest version of the open-source digital experience platform with even more features, is here.I'd like to implement support in Provision's nginx/nginx_ssl servers for the X-Forwarded-For and PROXY protocol headers.
Would a patch be accepted that does not provide this support to the apache servers?
I think this needs to be in provision itself because there are no hooks for altering nginx config lines which has already been defined (such as listen). Correct me if I'm wrong and I'll make my own module instead.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | interdiff.txt | 599 bytes | bdragon |
| #19 | 2829411_plus_hosting_https_helper.patch | 16.97 KB | bdragon |
| #18 | 2829411-18.patch | 16.8 KB | JamesK |
Comments
Comment #2
colanYes, an Nginx-only patch would be fine. Apache work on this can be done later. Thanks for doing this!
Please be aware that the SSL/HTTPS functionality in core (Provision) is essentially deprecated. We're about to roll out Aegir HTTPS which is the replacement. If you could add a merge request over there for the HTTPS stuff as well, it would be fantastic.
Comment #3
JamesK CreditAttribution: JamesK at Advisor Websites commentedThanks for the quick reply. How stable is Aegir HTTPS?
Comment #4
colanIt will be ready for testing as soon as I get Replace all references to upstream project letsencrypt.sh with 'dehydrated' completed, which I'm working on right now. Take a look at the issue queue, but the one thing that'll be missing is support for certificates other than Let's Encrypt. The issue for that is Add a 'manual' Certificate implementation. Patches welcome! The original goal of the project was to add Let's Encrypt support to Aegir as per #2629560: [meta] Let's encrypt support, so nobody's looking at other certificates yet.
So it's not ready for Production today, but I'm planning to have it in that state for one of my projects (php7+nginx) by the end of the month, having LE certs for all hosted sites.
Other testers would really help. :)
Comment #5
ergonlogicNote that reverse proxy support has been discussed for a long time: #1047174: Reverse proxy support
Ideally, this'd be implemented as a separate service type (proxy), for which we could have a default implementation (nginx). This'd make Apache support moot, as an Nginx proxy can sit in front of an Apache web server. This would also allow Varnish (or other) support later on.
Comment #6
JamesK CreditAttribution: JamesK at Advisor Websites commented@ergonlogic Thanks for linking that issue. The discussion is related, but this issue is about having the nginx config support putting a reverse proxy in front of it, not for configuring nginx as a reverse proxy. PROXY protocol support can't be provided at the PHP level and requires changing the listen directive in the nginx config, so this has to be patched into the nginx feature itself.
Comment #7
ergonlogicAh, my mistake. Would context appended to the server or platform config, or in the pre.d/ dir not work?
Comment #8
JamesK CreditAttribution: JamesK at Advisor Websites commentedOnly with an API change to allow hooks into setting `listen` in the vhost config.
Comment #9
JamesK CreditAttribution: JamesK at Advisor Websites commentedHere's the provision patch.
Comment #10
JamesK CreditAttribution: JamesK at Advisor Websites commentedHosting patch is in #2830365: Support load balancers (UI)
Comment #11
colanComment #12
JamesK CreditAttribution: JamesK at Advisor Websites commentedIs any further work needed?
Comment #13
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedThe patch looks good to me, but you can't assume that the server was built with
--with-http_realip_module, so perhaps the extra check would be a good idea, like we do this for some other modules?Comment #14
JamesK CreditAttribution: JamesK at Advisor Websites commentededit: never mind
Comment #15
JamesK CreditAttribution: JamesK at Advisor Websites commentedI've addressed #13 and also made the default server block respond to requests for health checking.
Comment #16
JamesK CreditAttribution: JamesK at Advisor Websites commentedComment #17
JamesK CreditAttribution: JamesK at Advisor Websites commentedNeed to fix the server ssl location block
Comment #18
JamesK CreditAttribution: JamesK at Advisor Websites commentedComment #19
bdragon CreditAttribution: bdragon at Tag1 Consulting for Advisor Websites commentedI need a little helper on top of these changes to support using this patch for its Provision_Service_http_public public changes to fix #2952716: Support load balancers (hosting_https+ui) not getting https_proxy_type passed through to provisioning properly.
Comment #20
dpovshed CreditAttribution: dpovshed commentedUnfortunately even with the latest patch the settings from UI are not reflected either in the backend nor in the drush aliases file.
When debugging, I noticed that $this->server->https_proxy_type is always 0 disregarding of the selected value.