Skip to content

implementing backend-protocol#286

Merged
k8s-ci-robot merged 23 commits intokubernetes-sigs:mainfrom
eladmotola:feature/nginxingress-backend-protocol
Feb 27, 2026
Merged

implementing backend-protocol#286
k8s-ci-robot merged 23 commits intokubernetes-sigs:mainfrom
eladmotola:feature/nginxingress-backend-protocol

Conversation

@eladmotola
Copy link
Copy Markdown
Contributor

@eladmotola eladmotola commented Jan 2, 2026 •
edited by Stevenjin8
Loading

What type of PR is this?
/kind feature

What this PR does / why we need it:
Implementing backend-protocol annotation for nginx ingress controller
I also had to implement grpc route in this pr

Which issue(s) this PR fixes:

Fixes #266

Does this PR introduce a user-facing change?:

Add initial support for GRPC Backend Protocol

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 2, 2026
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 2, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jan 2, 2026

Hi @eladmotola. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 2, 2026
Stevenjin8
Stevenjin8 reviewed Jan 2, 2026
View reviewed changes
Comment thread pkg/i2gw/provider_intermediate/intermediate_representation.go Outdated
Stevenjin8
Stevenjin8 reviewed Jan 2, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/ingressnginx/backend_protocol.go Outdated
Stevenjin8
Stevenjin8 reviewed Jan 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Stevenjin8 Stevenjin8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. took a quick look at seems broadly what I was thinking. Will take a closer look next week.

@Stevenjin8
Copy link
Copy Markdown
Contributor

Stevenjin8 commented Jan 2, 2026

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 2, 2026
Stevenjin8
Stevenjin8 reviewed Jan 5, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/common/converter.go Outdated
Stevenjin8
Stevenjin8 reviewed Jan 5, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/common/converter.go
Stevenjin8
Stevenjin8 reviewed Jan 5, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/common/converter.go
Stevenjin8
Stevenjin8 reviewed Jan 5, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/common/converter.go Outdated
@eladmotola
Copy link
Copy Markdown
Contributor Author

eladmotola commented Jan 12, 2026

@Stevenjin8 Hey
i made some changes
waiting for your review ^^

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 13, 2026
@Stevenjin8
Copy link
Copy Markdown
Contributor

Stevenjin8 commented Jan 13, 2026

@eladmotola hey this is on my list

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2026
Stevenjin8
Stevenjin8 reviewed Jan 15, 2026
View reviewed changes
Comment thread pkg/i2gw/emitter_intermediate/gce/gce.go Outdated
@Stevenjin8
Copy link
Copy Markdown
Contributor

Stevenjin8 commented Jan 15, 2026

trying this out rn

Stevenjin8
Stevenjin8 reviewed Jan 15, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/common/converter.go Outdated
@eladmotola
Copy link
Copy Markdown
Contributor Author

eladmotola commented Feb 5, 2026

Hey @Stevenjin8
how can i fix the ci? how can we push this pr?

@Stevenjin8
Copy link
Copy Markdown
Contributor

Stevenjin8 commented Feb 6, 2026

@eladmotola you can run 'make verify' locally to see what's going on. We're pretty strained on reviewers and we need two people to merge this PR. I'm pushing for it though!

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 6, 2026
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 6, 2026
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2026
@Stevenjin8 Stevenjin8 mentioned this pull request Feb 11, 2026
Closed
@Stevenjin8
Copy link
Copy Markdown
Contributor

Stevenjin8 commented Feb 11, 2026

@eladmotola I'm pushing to get this merged soon. Thanks for being patient

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 18, 2026
bexxmodd
bexxmodd reviewed Feb 19, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/ingressnginx/backend_protocol.go Outdated
Comment thread pkg/i2gw/providers/ingressnginx/converter.go Outdated
Comment thread pkg/i2gw/providers/common/converter.go

func toGRPCRouteMatch(routePath networkingv1.HTTPIngressPath, _ *field.Path) *gatewayv1.GRPCRouteMatch {
// Parse the path to extract service and method
// Example: /hello.HelloService/SayHello -> service="hello.HelloService", method="SayHello"
Copy link
Copy Markdown

@bexxmodd bexxmodd Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will be the behavior if the path is / ?

Copy link
Copy Markdown
Contributor Author

@eladmotola eladmotola Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so no method will be added - you will go to the svc

@Stevenjin8 Stevenjin8 mentioned this pull request Feb 19, 2026
Closed
kkk777-7
kkk777-7 reviewed Feb 22, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/ingressnginx/annotations.go Outdated
@kkk777-7
Copy link
Copy Markdown
Member

kkk777-7 commented Feb 22, 2026

@eladmotola thanks for working this! Overall, LGTM. left nits one comment.

@eladmotola
Copy link
Copy Markdown
Contributor Author

eladmotola commented Feb 22, 2026

Hey @bexxmodd and @kkk777-7
I addressed the comments

kkk777-7
kkk777-7 reviewed Feb 22, 2026
View reviewed changes
Comment thread pkg/i2gw/providers/ingressnginx/canary.go
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Feb 23, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eladmotola, kkk777-7, Stevenjin8

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Stevenjin8
Copy link
Copy Markdown
Contributor

Stevenjin8 commented Feb 25, 2026

@kkk777-7 can you /lgtm?

@kkk777-7
Copy link
Copy Markdown
Member

kkk777-7 commented Feb 27, 2026

/lgtm
@eladmotola thanks for working this!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 27, 2026
@k8s-ci-robot k8s-ci-robot merged commit e52b9f9 into kubernetes-sigs:main Feb 27, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Stevenjin8 Stevenjin8 Stevenjin8 approved these changes

@rikatz rikatz Awaiting requested review from rikatz

@LiorLieberman LiorLieberman Awaiting requested review from LiorLieberman LiorLieberman is a code owner

@robscott robscott Awaiting requested review from robscott robscott is a code owner

@youngnick youngnick Awaiting requested review from youngnick youngnick is a code owner

+2 more reviewers

@bexxmodd bexxmodd bexxmodd left review comments

@kkk777-7 kkk777-7 kkk777-7 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kkk777-7 kkk777-7

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Ingress NGINX] Support backend protocol annotation

5 participants

@eladmotola @k8s-ci-robot @Stevenjin8 @kkk777-7 @bexxmodd