Add NGINX provider support#224
Conversation
|
|
|
Welcome @sarthyparty! |
|
Hi @sarthyparty. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. DetailsInstructions 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. |
|
Hey, sorry for the extremely large PR, I did not realize it was this large. I'm going to remove virtual server conversion code and some annotations that are not fully done to make it smaller. Feel free to toss any suggestions to break it into smaller chunks |
|
If this PR is still too large, I'd be happy to break it into smaller chunks |
sjberman
left a comment
There was a problem hiding this comment.
Haven't gone through unit tests yet, but did a first pass of the implementation.
|
This is great to see, thanks @sarthyparty! Ping me whenever this is ready for review. /ok-to-test |
sjberman
left a comment
There was a problem hiding this comment.
Approving from my point of view. I'll let others finish their reviews too in case there's anything else :) Nice work.
|
Let me have a look today
…On Wed, 27 Aug 2025 at 7:34 Saylor Berman ***@***.***> wrote:
*sjberman* left a comment (kubernetes-sigs/ingress2gateway#224)
<#224 (comment)>
@levikobi <https://github.com/levikobi> @Xunzhuo
<https://github.com/Xunzhuo> You two are listed as reviewers on this PR.
Would you be able to take a look or recommend someone else who has write
access that can? We're hoping to get this in ASAP due to time constraints
on our own team.
—
Reply to this email directly, view it on GitHub
<#224 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASLANQVQFZMXKONGOLARNJ33PW6YJAVCNFSM6AAAAACA64XPXWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMRYGQ3DAOBVG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
/assign |
LiorLieberman
left a comment
There was a problem hiding this comment.
This is very well written, thanks for putting the time.
Left a few comments after my initial review (and yes, it is hard to review 58 files lol - but I dont think you need to split after all the time you've waited :) )
|
/lgtm 🎉 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: LiorLieberman, sarthyparty The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* Add NGINX provider with annotation support * add more test * fix tls for custom ports * update readme * go mod tidy * align table * remove kubernetes from name * link issue in annotations readme * move issue link * add notifications for grpcfilter conversion * update grpc services test * update error message to be clearer * fix comments on header and rewrite * rename test functions * switch t error instead of t fatal * clean tests * rework tests to use error instead of fatal * resolve comments * Update banner to 2025 * Update readme, remove managed by labels * set target ref for btp to be core * update toir and merge irs * move grpc and btp logic to common * fix verify * remove core group var * Update comment and separate btp name gen function * remove unecessary labels * move btp name gen function to nginx provider * fix gofumpt
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds support for NGINX provider.
Which issue(s) this PR fixes:
Fixes #223
Does this PR introduce a user-facing change?:
If this PR is too large, I'd be happy to break it into smaller chunks