-
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
[feature] bump nginx to 1.25.5 and add http3 module #11470
[feature] bump nginx to 1.25.5 and add http3 module #11470
Conversation
…am, lua_ngx, mimaloc, opentelemetry_cpp, opentelemetry_proto and opentelemtry_contrib Signed-off-by: Stepan Paksashvili <[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 @ipaqsa! |
Hi @ipaqsa. 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 canceled.
|
Thank you |
Signed-off-by: Stepan Paksashvili <[email protected]>
@tao12345666333 I have passed e2e and signed CLA, could please review ? And I didn't change the nginx version in the NGINX_BASE file and in the opentelemetry image |
@ipaqsa I am not a developer so @tao12345666333 and others will review and advise. But out of curiosity, I looked. Even though you have not changed the base-image, you are making a lot of changes besides the flag for HTTP3. My opinion is that you should NOT make any changes besides your HTTP3 module build flag. If that is redundant, then you should let @tao12345666333 or @rikatz add HTTP3 related changes. My concern is that you are unaware of the impact project-wide and you are good-intentionally making changes like the TAG. These are extremely sensitive changes and there is unconventional awareness of the project needed like @tao12345666333 & @rikatz have. Apologies but I had to say something after seeing the changes. I guess eventually it will all work out. |
@longwuyuan Thanks! The main point is that we want to add HTTP/3 support (#11172) and @tao12345666333 suggested that we should wait for an update to 1.25.5 (#11429), however there was a problem with building 1.25.5, but I found a fix for it in the latest commit of the stream-lua module(openresty/stream-lua-nginx-module@bea8a0c), and i thought it would be nice to try to contribute. I bumped up the versions of nginx and lua-stream module and added the HTTP/3 module as well. I passed e2e tests and hope that if I did something wrong, the community will help me. And other my changes are related to fixing wrong references in comments and I might be wrong about the TAG, so I want to get a review to correct. |
@ipaqsa than you for the update. I see one example is the TAG that should not change by a user. That is always bumped only during release process. Else it creates problems. I did not dig into the impact of changing the other SHA & stuff. I could try to run But since Tao already commented, I think this will get to its appropriate destination, eventually. Thanks again |
@tao12345666333 is there anything we can help with to move this forward? |
/lgtm |
@strongjz: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you. 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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ipaqsa, strongjz 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 |
@strongjz: cannot checkout 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.10 |
@Gacko: new pull request created: #11541 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. |
Bumping nginx to 1.25.5 and add http3 module.
Nginx 1.25.3 is end of life since 29.05.2024, and 1.25.5 has important bug fixes, and fixes for http3, so to improve security and prepare ingress-nginx for adding http3 support, we should bump current version.
It doesn't add HTTP/3 support, it just adds the HTTP/3 module during build, full HTTP/3 support requires additional steps.
What this PR does / why we need it:
It bumps nginx to 1.25.5, updates some others components and adds http3 module(to further add http 3 support) .
Types of changes
Which issue/s this PR fixes
Fixes #11401
How Has This Been Tested?
It has been tested by e2e tests.
Checklist: