Added support for setting Proxy protocol per target group based on ServicePort #4079

pthak94 · 2025-03-06T14:29:43Z

Issue

Description

Introduced a new annotation "service.beta.kubernetes.io/aws-load-balancer-proxy-protocol-per-target-group" with which, proxy protocol setting can be configured for each target group individually. Example usage is "80, 443", which will enable proxy protocols for the target groups associated to port 80 and 443 listeners. And disable for the rest, for example port 22.

To achieve this, when target group is created, it's service port is compared to the value of this annotation, if it's found, it's value is set to true, otherwise set to false.

"service.beta.kubernetes.io/aws-load-balancer-proxy-protocol" annotation still overrides this new annotation.

Added test cases to check if the new annotation works, and that it is still overriden by "service.beta.kubernetes.io/aws-load-balancer-proxy-protocol" annotation.

Finally, added documentation regarding this new annotation.

This version of controller is tested in my EKS testing environment.

-->

Checklist

Added tests that cover your change (if possible)
Added/modified documentation as required (such as the README.md, or the docs directory)
Manually tested
Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Backfilled missing tests for code in same general area 🎉
Refactored something and made the world a better place 🌟

linux-foundation-easycla · 2025-03-06T14:29:48Z

The committers listed above are authorized under a signed CLA.

✅ login: pthak94 (800bb02, c80b61c, 5fdbfc8, 497a366)

k8s-ci-robot · 2025-03-06T14:29:54Z

Hi @pthak94. 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 /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.

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.

zac-nixon

Thank you for the contribution. Please consider my comment.

zac-nixon · 2025-03-07T17:52:00Z

docs/guide/service/annotations.md

 | [service.beta.kubernetes.io/aws-load-balancer-internal](#lb-internal)                            | boolean                 | false                     | deprecated, in favor of [aws-load-balancer-scheme](#lb-scheme)                                                                                                                                                                      |
 | [service.beta.kubernetes.io/aws-load-balancer-scheme](#lb-scheme)                                | string                  | internal                  |                                                                                                                                                                                                                                     |
 | [service.beta.kubernetes.io/aws-load-balancer-proxy-protocol](#proxy-protocol-v2)                | string                  |                           | Set to `"*"` to enable                                                                                                                                                                                                              |
+| [service.beta.kubernetes.io/aws-load-balancer-proxy-protocol-per-target-group](#proxy-protocol-v2)                | string                  |                           | If specified,configures proxy protocol per targrt group based on ServicePort. For example `"80, true, 443, true, 22, false"` will disable proxy protocol for port 22 only. This annotation is overriden by `"service.beta.kubernetes.io/aws-load-balancer-proxy-protocol"`                                                                                                                                                                                                            |


I don't think this is the correct way to model this information. How about just using a single list 80,443 to denote that those ports should have proxy protocol enabled.

hmm, that indeed makes it simpler. I'll work on your suggestions shortly. Thanks!

@zac-nixon added a new commit with the updated logic. Tweaked the tests and doc as well. Pls take a look.

zac-nixon · 2025-03-11T19:37:33Z

pkg/service/model_build_target_group.go

+        if _, enabled := enabledPorts[currentPortStr]; enabled {
+            rawAttributes[tgAttrsProxyProtocolV2Enabled] = "true"
+        } else {
+            rawAttributes[tgAttrsProxyProtocolV2Enabled] = "false"


to keep existing behavior, can you remove this line? the default for this attribute is false anyways.

@zac-nixon I'm assuming you meant remove only the else block and not the whole line. I can't think of any scenario where this can break the existing behavior. Can you please help me understand?

The intention of having the else block is for a situation where these attributes are set:

"service.beta.kubernetes.io/aws-load-balancer-target-group-attributes": tgAttrsProxyProtocolV2Enabled + "=true"
"service.beta.kubernetes.io/aws-load-balancer-proxy-protocol-per-target-group": "80, 443"

in this case, our new annotation will not do what we say it does, that is disable proxy_protocol for unspecified port 22. It will just enable it for all.

"service.beta.kubernetes.io/aws-load-balancer-proxy-protocol": "*" still overrides everything as the documentation says.

Do let me know what you think?

Thanks for the explanation. I was just thinking that it would be nice to not have the false value set when we didn't need it. But I'm happy with the implementation as-is and I appreciate the work you did here :)

zac-nixon · 2025-03-11T19:38:54Z

overall looks pretty good. please consider my comment.

i appreciate the contribution!

zac-nixon · 2025-03-11T19:39:01Z

/ok-to-test

zac-nixon · 2025-03-15T00:32:35Z

/lgtm

k8s-ci-robot · 2025-03-17T00:49:33Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: pthak94

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:

OWNERS

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

pthak94 · 2025-03-17T14:51:05Z

Thanks @zac-nixon :) Do you know when this will be released? we'd like to deploy it on our production environments.

k8s-ci-robot requested review from oliviassss and wweiwei-li March 6, 2025 14:29

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 6, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 6, 2025

pthak94 changed the title ~~Proxy protocol per target group~~ Added support for setting Proxy protocol per target group based on ServicePort Mar 6, 2025

pthak94 added 3 commits March 6, 2025 15:57

added support for per tg annotation

c80b61c

added test cases for new annotation

5fdbfc8

added documentation about new annotation

800bb02

pthak94 force-pushed the proxy-protocol-per-target-group branch from 862e07e to 800bb02 Compare March 6, 2025 14:59

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 6, 2025

pthak94 mentioned this pull request Mar 7, 2025

Disable proxy protocol for TCP/UDP ports in NLB #3877

Closed

zac-nixon reviewed Mar 7, 2025

View reviewed changes

update logic, tests and doc

497a366

pthak94 requested a review from zac-nixon March 10, 2025 09:57

zac-nixon reviewed Mar 11, 2025

View reviewed changes

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 Mar 11, 2025

k8s-ci-robot assigned zac-nixon Mar 15, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 15, 2025

zac-nixon added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2025

k8s-ci-robot merged commit bee5f8c into kubernetes-sigs:main Mar 17, 2025
8 of 9 checks passed

zac-nixon mentioned this pull request May 6, 2025

🥳 aws-load-balancer-controller v2.13.0 Automated Release! 🥑 aws/eks-charts#1214

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added support for setting Proxy protocol per target group based on ServicePort #4079

Added support for setting Proxy protocol per target group based on ServicePort #4079

Uh oh!

pthak94 commented Mar 6, 2025 •

edited

Loading

Uh oh!

linux-foundation-easycla bot commented Mar 6, 2025 •

edited

Loading

Uh oh!

k8s-ci-robot commented Mar 6, 2025

Uh oh!

zac-nixon left a comment

Uh oh!

zac-nixon Mar 7, 2025

Uh oh!

pthak94 Mar 8, 2025

Uh oh!

pthak94 Mar 8, 2025

Uh oh!

zac-nixon Mar 11, 2025

Uh oh!

pthak94 Mar 12, 2025 •

edited

Loading

Uh oh!

zac-nixon Mar 15, 2025 •

edited

Loading

Uh oh!

zac-nixon commented Mar 11, 2025

Uh oh!

zac-nixon commented Mar 11, 2025

Uh oh!

zac-nixon commented Mar 15, 2025

Uh oh!

k8s-ci-robot commented Mar 17, 2025

Uh oh!

Uh oh!

pthak94 commented Mar 17, 2025

Uh oh!

Uh oh!

Added support for setting Proxy protocol per target group based on ServicePort #4079

Added support for setting Proxy protocol per target group based on ServicePort #4079

Uh oh!

Conversation

pthak94 commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Checklist

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Uh oh!

linux-foundation-easycla bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Mar 6, 2025

Uh oh!

zac-nixon left a comment

Choose a reason for hiding this comment

Uh oh!

zac-nixon Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

pthak94 Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

pthak94 Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

zac-nixon Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

pthak94 Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zac-nixon Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zac-nixon commented Mar 11, 2025

Uh oh!

zac-nixon commented Mar 11, 2025

Uh oh!

zac-nixon commented Mar 15, 2025

Uh oh!

k8s-ci-robot commented Mar 17, 2025

Uh oh!

Uh oh!

pthak94 commented Mar 17, 2025

Uh oh!

Uh oh!

pthak94 commented Mar 6, 2025 •

edited

Loading

linux-foundation-easycla bot commented Mar 6, 2025 •

edited

Loading

pthak94 Mar 12, 2025 •

edited

Loading

zac-nixon Mar 15, 2025 •

edited

Loading