-
Notifications
You must be signed in to change notification settings - Fork 37
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
Make mail module available dynamically #279
Make mail module available dynamically #279
Conversation
The [NGINX mail module](https://nginx.org/en/docs/mail/ngx_mail_core_module.html) is a core NGINX module, useful for constructing proxies. [There are built-in configure flags for it; see docs here.](https://nginx.org/en/docs/configure.html#:~:text=%2D%2Dwith%2Dmail%3Ddynamic,run%20this%20module.)
Note I'm working from this precedent, noting that The wording there...
...makes me think that the other core modules that can be compiled dynamically...
...are candidates for the same treatment. I'll leave it for someone else to decide if that is a good idea! |
cc @cloudfoundry/wg-app-runtime-interfaces-buildpacks-web-servers-approvers - can't request for approval as that team isn't in CODEOWNERS |
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.
This makes sense to me in principle, though I haven't dug into it particularly deeply. My understanding is that the --with-mail=dynamic
instructions nginx to compile the plugin but not enable it until the user configures nginx to do so. I'm assuming that --with-mail_ssl_module
compiles the module with ssl
functionality.
Thanks so much for the merge! Any idea when the next release of the buildpack might happen? |
We aim to ship monthly, so probably in the order of days or maybe weeks. |
Noting here from Slack:
|
I think that output is coming from running |
I meant making sure that the |
Gotcha. I'm not 100% sure, but I don't see any reference to handling |
Ah, that's frustrating. Glad you were able to find a solution with the buildpack though, even if it was a different module. |
Since I'm the person who asked for this and supplied the PR, I'd understand if you wanted to roll it back out. |
I think this is really a question for @cloudfoundry/wg-app-runtime-interfaces-buildpacks-web-servers-approvers but personally I'd be happy to leave it in. The CF/V2 buildpacks tend to err on the side of compatibility rather than minimal surface area or footprint. |
OK, in that case I'll draw attention to this again!
|
The NGINX mail module is a core NGINX module, useful for constructing proxies (my use-case). There are built-in configure flags for it; see docs here.
This PR builds the module dynamically so it will not change the buildpack's behavior for anyone who doesn't deliberately enable it. Including this module in the available dynamic modules removes the need for people (eg me) to separately maintain builds that are compatible with each new version of nginx supported by the buildpack!