-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
add ssl patches to nginx-1.25 image for coroutines to work in lua client hello and cert ssl blocks #11485
Conversation
Signed-off-by: Jon Carl <[email protected]>
This issue is currently awaiting triage. If Ingress contributors determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Welcome @grounded042! |
Hi @grounded042. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for kubernetes-ingress-nginx ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
/assign |
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.
Thank you! You are right. When we upgraded the NGINX, we did not synchronize all patches.
Signed-off-by: Jon Carl <[email protected]>
@tao12345666333 as we talked about in Slack I've updated this PR to include 1.25.3 patches from openresty. I've updated the description with these details as well, but some notes: I renamed the patches to order them numerically as the don't apply correctly if they are not applied in order. I did not include the following patches:
|
Thank you! Let me take a look |
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.
Thank you!
/ok-to-test
I'll merge this first, then initiate a new NGINX build. After that, we can verify if everything is working properly. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: grounded042, tao12345666333 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cherry-pick release-1.10 |
@Gacko: new pull request created: #11534 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/cherry-pick release-1.11 |
@Gacko: new pull request created: #11535 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What this PR does / why we need it:
In #11037 the base image was changed from
registry.k8s.io/ingress-nginx/nginx
toregistry.k8s.io/ingress-nginx/nginx-1.25
. This removed many patches to nginx which were previously included from the https://github.com/kubernetes/ingress-nginx/tree/main/images/nginx/rootfs/patches directory.This PR adds openresty patches to the nginx-1.25 base image.
I originally opened this PR because of SSL patches, but in a Slack discussion
it was determined that including almost all patches openresty had for 1.25.3
was the best patch forward. Note that I did not include header and footer
patches which change headers and footers from
nginx
toopenresty
.I renamed the patches to order them numerically as the don't apply correctly
if they are not applied in order.
I did not include the following patches:
Types of changes
Which issue/s this PR fixes
How Has This Been Tested?
I tested this locally via a simple nginx configuration. Before adding in the needed patches the
ssl_client_hello_by_lua_block
andssl_certificate_by_lua_block
would cause errors duringSSL_connect
(eg.OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to localhost:443
). After the patches were added no errors would occur.Checklist: