Skip to content
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

Fix proxy protocol for Health Check frontend & Set stick-table size to IPv6 #633

Merged

Conversation

a18e
Copy link
Contributor

@a18e a18e commented Mar 15, 2024

This Pull Request introduces two changes to enhance the IPv6 support of haproxy-boshrelease:

1. Proxy Protocol Adjustment for Health Check Frontend

AWS Network Load Balancers (NLBs) utilize the same Proxy Protocol setting for both forwarded traffic and health checks, as outlined in the AWS documentation.

The existing accept_proxy configuration property only applies the accept-proxy setting (refer to the HAProxy documentation) to the http and https frontends, excluding the health check frontend. This exclusion leads to the HAProxies being perceived as unhealthy by the load balancer once the feature is activated.

It is not feasible to globally enable accept-proxy for the health check frontend because monit also utilizes this frontend for the Remote host Service check, and it does not support the Proxy Protocol. Therefore, we are employing the expect-proxy feature to avoid expecting the proxy protocol for requests originating from localhost (i.e., monit).

2. Stick-Table Size Adjustment for IPv6

Currently, requests from IPv6 source addresses (either via Proxy Protocol or when HAProxy operates in dualstack/v4v6 enabled-mode) cannot be rate-limited. This limitation is due to the stick tables used for rate limiting being of the ip type, which cannot store the longer IPv6 addresses. This PR modifies the type to ipv6, enabling the storage of both IPv4 and IPv6 addresses (see HAProxy documentation). The additional memory overhead is negligible.

@a18e a18e force-pushed the fix-accept-proxy-feature-for-aws branch from 6fb9121 to 3c4cede Compare March 15, 2024 15:07
@a18e a18e marked this pull request as ready for review March 18, 2024 08:52
@a18e a18e requested review from CFN-CI and a team as code owners March 18, 2024 08:52
@b1tamara b1tamara added the run-ci Allow this PR to be tested on Concourse label Mar 18, 2024
jobs/haproxy/spec Outdated Show resolved Hide resolved
@a18e a18e marked this pull request as draft March 18, 2024 10:13
@a18e a18e force-pushed the fix-accept-proxy-feature-for-aws branch 2 times, most recently from 8b953fb to 66ba663 Compare March 18, 2024 11:27
@a18e a18e changed the title Fix proxy protocol for Health Check frontend Fix proxy protocol for Health Check frontend & Set stick-table size to IPv6 Mar 18, 2024
@a18e a18e force-pushed the fix-accept-proxy-feature-for-aws branch from 367aab3 to 34195df Compare March 19, 2024 08:01
peanball
peanball previously approved these changes Mar 20, 2024
@a18e a18e marked this pull request as ready for review March 26, 2024 11:32
@a18e a18e force-pushed the fix-accept-proxy-feature-for-aws branch from 34195df to fc77930 Compare March 26, 2024 13:10
peanball
peanball previously approved these changes Mar 26, 2024
Copy link
Contributor

@peanball peanball left a comment

Choose a reason for hiding this comment

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

approval for CI only

@peanball peanball dismissed their stale review March 26, 2024 13:16

remove ci-only approval (didn't work)

@a18e a18e force-pushed the fix-accept-proxy-feature-for-aws branch from fc77930 to 6acb0ac Compare March 26, 2024 14:19
maxmoehl
maxmoehl previously approved these changes Mar 28, 2024
Copy link
Member

@maxmoehl maxmoehl left a comment

Choose a reason for hiding this comment

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

Approval for CI.

@b1tamara b1tamara added run-ci Allow this PR to be tested on Concourse and removed run-ci Allow this PR to be tested on Concourse labels Apr 9, 2024
maxmoehl
maxmoehl previously approved these changes Apr 9, 2024
Copy link
Member

@maxmoehl maxmoehl left a comment

Choose a reason for hiding this comment

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

Approve for CI.

maxmoehl
maxmoehl previously approved these changes Apr 9, 2024
Copy link
Member

@maxmoehl maxmoehl left a comment

Choose a reason for hiding this comment

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

Approving for CI.

peanball
peanball previously approved these changes Apr 9, 2024
Copy link
Contributor

@peanball peanball left a comment

Choose a reason for hiding this comment

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

approve for CI

@peanball peanball force-pushed the fix-accept-proxy-feature-for-aws branch 2 times, most recently from a6907fe to de6f849 Compare April 23, 2024 08:36
b1tamara
b1tamara previously approved these changes Apr 25, 2024
Copy link
Contributor

@b1tamara b1tamara left a comment

Choose a reason for hiding this comment

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

Approval for CI

@a18e a18e force-pushed the fix-accept-proxy-feature-for-aws branch 2 times, most recently from 31549be to f9e30b4 Compare May 3, 2024 08:48
@a18e a18e force-pushed the fix-accept-proxy-feature-for-aws branch from f9e30b4 to 82bf964 Compare May 3, 2024 08:55
maxmoehl
maxmoehl previously approved these changes May 3, 2024
Copy link
Member

@maxmoehl maxmoehl left a comment

Choose a reason for hiding this comment

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

Approval for CI.

peanball
peanball previously approved these changes May 6, 2024
Copy link
Contributor

@peanball peanball left a comment

Choose a reason for hiding this comment

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

approve for CI

Copy link
Contributor

@plowin plowin left a comment

Choose a reason for hiding this comment

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

lgtm

@plowin plowin merged commit a41840d into cloudfoundry:master May 6, 2024
4 checks passed
Mrizwanshaik pushed a commit to sap-contributions/haproxy-boshrelease that referenced this pull request May 6, 2024
…o IPv6 (cloudfoundry#633)

* fix(proxy-protocol): fix proxy protocol for Health Check frontend

* docs: Adjust spec description for accept_proxy

* test: Add spec test

* feat: Set stick tables to type ipv6

* test: fix linter error in test case

* fix: Check the value of accept_proxy for http health checks

* feat: Add check for required files to run-local.sh
(bpm, bosh-stemcells)

* feat: (WIP) Proxy Protocol Check for Traffic Endpoint

* test: add auto-download of stemcells for local test

* fix(test): fix auto-download method for  when all files are present

* fix: Finish proxy_protocol acceptance test

* fix: Linter fixes, remove unneeded changes

* fix: remove go toolchain

* fix: typo in docstring

---------

Co-authored-by: Tamara Boehm <tamara.boehm@sap.com>
Co-authored-by: Alexander Lais <alexander.lais@sap.com>
Mrizwanshaik pushed a commit to sap-contributions/haproxy-boshrelease that referenced this pull request May 6, 2024
…o IPv6 (cloudfoundry#633)

* fix(proxy-protocol): fix proxy protocol for Health Check frontend

* docs: Adjust spec description for accept_proxy

* test: Add spec test

* feat: Set stick tables to type ipv6

* test: fix linter error in test case

* fix: Check the value of accept_proxy for http health checks

* feat: Add check for required files to run-local.sh
(bpm, bosh-stemcells)

* feat: (WIP) Proxy Protocol Check for Traffic Endpoint

* test: add auto-download of stemcells for local test

* fix(test): fix auto-download method for  when all files are present

* fix: Finish proxy_protocol acceptance test

* fix: Linter fixes, remove unneeded changes

* fix: remove go toolchain

* fix: typo in docstring

---------

Co-authored-by: Tamara Boehm <tamara.boehm@sap.com>
Co-authored-by: Alexander Lais <alexander.lais@sap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-ci Allow this PR to be tested on Concourse
Projects
Development

Successfully merging this pull request may close these issues.

5 participants