feat: Add support for NLB target group attributes (ProxyProtocolV2, PreserveClientIP)#783
feat: Add support for NLB target group attributes (ProxyProtocolV2, PreserveClientIP)#783
Conversation
3e28007 to
854087b
Compare
controller.go
Outdated
| kingpin.Flag("nlb-proxy-protocol-v2", "Enable Proxy Protocol v2 for Network Load Balancers. This setting only applies to 'network' Load Balancers."). | ||
| Default("false").BoolVar(&nlbProxyProtocolV2Enabled) | ||
| kingpin.Flag("nlb-preserve-client-ip", "Enable preserve client IP address for Network Load Balancers. This setting only applies to 'network' Load Balancers."). | ||
| Default("true").BoolVar(&nlbPreserveClientIPEnabled) |
There was a problem hiding this comment.
I wonder if this is a breaking change to be mentioned in release notes? 🤔
basically if people run this on EKS IPv4 cluster with target type IP (CNI) my understanding is that right now it would implicitly be false in that setup.
Updating to this version would then make it true which could break assumptions.
If we set it to false it would be wrong in other setups, so it has to be mentioned either way. I agree with true as the default.
There was a problem hiding this comment.
Yes agreed. Either way false or true there is some chance that a setup may break. I'll add it to the release notes.
There was a problem hiding this comment.
One question would be if this makes sense to have as ingress annotation, because likely not everyone needs the proxy protocol v2 enabled.
There was a problem hiding this comment.
I think this is a config you don't want to expose to users as it depends on the whole ingress stack e.g. NLB + skipper (or other backend than skipper if anyone has that).
It's a bit similar to how the NLB targets the skipper pods/nodes this is also a config that should be controlled at the controller level and not exposed to users as they shouldn't care about it or it shouldn't influence the behavior for the application.
There was a problem hiding this comment.
Yes I agree. Whether it be the cluster's main nlb or even if a non-shared / exclusive nlb the underlying infrastructure remains the same.
ac5e964 to
763b0d1
Compare
763b0d1 to
0a3a877
Compare
|
👍 |
8c4972f to
61a71a0
Compare
|
👍 |
…eClientIP) This change adds support for configuring two NLB-specific target group attributes: 1. Proxy Protocol v2: Enable with `--nlb-proxy-protocol-v2` flag (default: false) - Enables Proxy Protocol v2 for NLB target groups - Opt-in feature, disabled by default 2. Preserve Client IP: Configure with `--nlb-preserve-client-ip` flag (default: true) - Preserves client IP address in NLB target group connections - Enabled by default These attributes are only applied to Network Load Balancer target groups and do not affect Application Load Balancers. BREAKING CHANGE: The `preserve_client_ip` attribute is now explicitly set to true by the controller on all NLB target groups. Previously, this attribute was not explicitly set, which relied on AWS NLB's default behavior (which is also true). If your NLB target groups currently have `preserve_client_ip=false` set outside of the controller, updating to this version will override it to true. To maintain the previous behavior of `preserve_client_ip=false`, pass the `--nlb-preserve-client-ip=false` flag when running the controller. Changes: - Add NLB attribute flags to controller CLI - Extend AWS adapter with WithNLBProxyProtocolV2() and WithNLBPreserveClientIP() methods - Update CloudFormation template generation to conditionally add NLB attributes - Add comprehensive test coverage (5 test cases) - Update README with v0.20 upgrade notes and breaking change documentation Signed-off-by: speruri <surya.srikar.peruri@zalando.de>
Changed flag names to better reflect their default behavior and fix parsing issues: 1. --nlb-proxy-protocol-v2 → --nlb-proxy-protocol-v2-enabled - Clarifies that setting this flag enables the feature - Default remains false (disabled) 2. --nlb-preserve-client-ip → --nlb-preserve-client-ip-disabled - Renamed to avoid parsing errors with boolean false values - Default remains true (enabled) - Users can now easily disable by setting the flag without parsing issues Updated README with new flag names and descriptions. These flag names are clearer about default behavior and align with standard flag naming conventions. Signed-off-by: speruri <surya.srikar.peruri@zalando.de>
Changed the CloudFormation template generation to always set both NLB target group attributes (proxy_protocol_v2.enabled and preserve_client_ip.enabled) with their explicit boolean values instead of only setting them when true. This ensures we override AWS defaults explicitly rather than relying on them: - proxy_protocol_v2.enabled: set to true/false based on flag - preserve_client_ip.enabled: set to true/false based on flag Updated tests to reflect that NLB target groups now always have 3 attributes (deregistration delay + both NLB-specific attributes). Benefits: - Explicit control over defaults - No reliance on AWS implicit behavior - Clear audit trail of what values are set Signed-off-by: speruri <surya.srikar.peruri@zalando.de>
- Add DefaultNLBProxyProtocolV2Enabled constant (false) - Add DefaultNLBPreserveClientIPEnabled constant (true, since preserve_client_ip is enabled by default) - Initialize test adapter with these default values - Update test data files to include proxy_protocol_v2 and preserve_client_ip attributes This ensures NLB target groups always have these attributes explicitly set in CloudFormation with their default values, matching the actual behavior. Signed-off-by: speruri <surya.srikar.peruri@zalando.de>
f6020f7 to
5e300ec
Compare
Summary
Adds support for two NLB target group attributes that can be controlled via CLI flags:
--nlb-proxy-protocol-v2-enabled): Opt-in feature to enable Proxy Protocol v2 for NLB targets--nlb-preserve-client-ip-disabled): Enabled by default; use flag to disableThese attributes are only applied to NLB target groups and don't affect ALBs.
Key Details
--nlb-preserve-client-ip-disabledif your setup requires it disabledFlag Usage