Skip to content

Commit

Permalink
Fix proxy protocol for Health Check frontend & Set stick-table size t…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
3 people authored and Mrizwanshaik committed May 6, 2024
1 parent 8307f8f commit 9ff5f5f
Show file tree
Hide file tree
Showing 10 changed files with 216 additions and 11 deletions.
4 changes: 4 additions & 0 deletions acceptance-tests/acceptance_tests_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ func expectConnectionRefusedErr(err error) {
checkNetOpErr(err, "connect: connection refused")
}

func expectConnectionResetErr(err error) {
checkNetOpErr(err, "read: connection reset by peer")
}

func checkNetOpErr(err error, expectString string) {
Expect(err).To(HaveOccurred())
urlErr, ok := err.(*url.Error)
Expand Down
3 changes: 2 additions & 1 deletion acceptance-tests/go.mod
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
module github.com/cloudfoundry/haproxy-boshrelease/acceptance-tests

go 1.18
go 1.21

require (
github.com/bramvdbogaerde/go-scp v1.4.0
github.com/gorilla/websocket v1.5.1
github.com/onsi/ginkgo/v2 v2.17.2
github.com/onsi/gomega v1.33.1
github.com/pires/go-proxyproto v0.7.0
golang.org/x/crypto v0.22.0
golang.org/x/net v0.24.0
gopkg.in/yaml.v2 v2.4.0
Expand Down
7 changes: 7 additions & 0 deletions acceptance-tests/go.sum
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
github.com/bramvdbogaerde/go-scp v1.4.0 h1:jKMwpwCbcX1KyvDbm/PDJuXcMuNVlLGi0Q0reuzjyKY=
github.com/bramvdbogaerde/go-scp v1.4.0/go.mod h1:on2aH5AxaFb2G0N5Vsdy6B0Ml7k9HuHSwfo1y0QzAbQ=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ=
github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI=
Expand All @@ -15,20 +16,26 @@ github.com/onsi/ginkgo/v2 v2.17.2 h1:7eMhcy3GimbsA3hEnVKdw/PQM9XN9krpKVXsZdph0/g
github.com/onsi/ginkgo/v2 v2.17.2/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/PRJ1eCc=
github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk=
github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0=
github.com/pires/go-proxyproto v0.7.0 h1:IukmRewDQFWC7kfnb66CSomk2q/seBuilHBYFwyq0Hs=
github.com/pires/go-proxyproto v0.7.0/go.mod h1:Vz/1JPY/OACxWGQNIRY2BeyDmpoaWmEP40O9LbuiFR4=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w=
golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8=
golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q=
golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk=
golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
golang.org/x/tools v0.20.0 h1:hz/CVckiOxybQvFw6h7b/q80NTr9IUQb4s1IIzW7KNY=
golang.org/x/tools v0.20.0/go.mod h1:WvitBU7JJf6A4jOdg4S1tviW9bhUxkgeCui/0JHctQg=
google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI=
google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
Expand Down
120 changes: 120 additions & 0 deletions acceptance-tests/proxy_protocol_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package acceptance_tests

import (
"fmt"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
proxyproto "github.com/pires/go-proxyproto"
"net"
"net/http"
"time"
)

var _ = Describe("Proxy Protocol", func() {
opsfileProxyProtocol := `---
# Enable Proxy Protocol
- type: replace
path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/accept_proxy?
value: true
- type: replace
path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/enable_health_check_http?
value: true
- type: replace
path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/disable_health_check_proxy?
value: false
`

It("Correctly proxies Proxy Protocol requests", func() {
haproxyBackendPort := 12000
haproxyInfo, _ := deployHAProxy(baseManifestVars{
haproxyBackendPort: haproxyBackendPort,
haproxyBackendServers: []string{"127.0.0.1"},
deploymentName: deploymentNameForTestNode(),
}, []string{opsfileProxyProtocol}, map[string]interface{}{}, false)

closeLocalServer, localPort := startDefaultTestServer()
defer closeLocalServer()

closeTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort)
defer closeTunnel()

By("Waiting for monit to report that HAProxy is healthy")
// Since the backend is now listening, HAProxy healthcheck should start returning healthy
// and monit should in turn start reporting a healthy process
// We will wait up to one minute for the status to stabilise
Eventually(func() string {
return boshInstances(deploymentNameForTestNode())[0].ProcessState
}, time.Minute, time.Second).Should(Equal("running"))

By("Sending a request with Proxy Protocol Header to HAProxy traffic port")
err := performProxyProtocolRequest(haproxyInfo.PublicIP, 80, "/")
Expect(err).NotTo(HaveOccurred())

By("Sending a request without Proxy Protocol Header to HAProxy")
_, err = http.Get(fmt.Sprintf("http://%s", haproxyInfo.PublicIP))
expectConnectionResetErr(err)

By("Sending a request with Proxy Protocol Header to HAProxy health check port")
err = performProxyProtocolRequest(haproxyInfo.PublicIP, 8080, "/health")
Expect(err).NotTo(HaveOccurred())

By("Sending a request without Proxy Protocol Header to HAProxy health check port")
_, err = http.Get(fmt.Sprintf("http://%s:8080/health", haproxyInfo.PublicIP))
expectConnectionResetErr(err)
})
})

func performProxyProtocolRequest(ip string, port int, endpoint string) error {
// Create a connection to the HAProxy instance
conn, err := net.Dial("tcp", fmt.Sprintf("%s:%d", ip, port))
if err != nil {
return err
}

defer conn.Close()

// Create proxy protocol header
header := &proxyproto.Header{
Version: 1,
Command: proxyproto.PROXY,
TransportProtocol: proxyproto.TCPv4,
SourceAddr: &net.TCPAddr{
IP: net.ParseIP("10.1.1.1"),
Port: 1000,
},
DestinationAddr: &net.TCPAddr{
IP: net.ParseIP(ip),
Port: port,
},
}

// Write header to the connection
_, err = header.WriteTo(conn)
if err != nil {
return err
}

// Send HTTP Request
request := fmt.Sprintf("GET %s HTTP/1.1\r\n"+
"Host: %s\r\n"+
"Content-Length: 0\r\n"+
"Content-Type: text/plain\r\n"+
"\r\n", endpoint, ip)
_, err = conn.Write([]byte(request))
if err != nil {
return err
}

// Read the response
buf := make([]byte, 1024)
_, err = conn.Read(buf)
if err != nil {
return err
}

// Get HTTP status code
if string(buf[9:12]) != "200" {
return fmt.Errorf("expected HTTP status code 200, got %s", string(buf))
}
return nil
}
42 changes: 41 additions & 1 deletion acceptance-tests/run-local.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

set -e
set -eu

SCRIPT_DIR="$(cd "$(dirname "$0")/" && pwd)"
REPO_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
Expand Down Expand Up @@ -29,6 +29,46 @@ docker_mac_check_cgroupsv1() {
fi
}

check_required_files() {
PIDS=""
REQUIRED_FILE_PATTERNS=(
../ci/scripts/bpm/bpm-release-*.tgz!https://bosh.io/d/github.com/cloudfoundry/bpm-release
../ci/scripts/stemcell/bosh-stemcell-*-ubuntu-jammy-*.tgz!https://bosh.io/d/stemcells/bosh-warden-boshlite-ubuntu-jammy-go_agent
../ci/scripts/stemcell-bionic/bosh-stemcell-*-ubuntu-bionic-*.tgz!https://bosh.io/d/stemcells/bosh-warden-boshlite-ubuntu-bionic-go_agent
)

for entry in "${REQUIRED_FILE_PATTERNS[@]}"; do
pattern=$(cut -f1 -d! <<<"$entry")
url=$(cut -f2 -d! <<<"$entry")
folder=$(realpath "$(dirname "$SCRIPT_DIR/$pattern")")
filepattern=$(basename "$pattern")
pattern=$folder/$filepattern

# shellcheck disable=SC2086
# glob resolution is desired here.
if [ -f $pattern ]; then
continue
fi

(
echo "$filepattern not found, downloading latest."
cd "$folder" && \
resolved=$(curl -s --write-out '\n%{redirect_url}' "$url" | tail -n1) && \
curl -s --remote-name --remote-header-name --location "$resolved" && \
echo "Downloaded '$url' successfully." && \
ls -1lh "$folder/"$filepattern
)&

PIDS="$PIDS $!"

done
# shellcheck disable=SC2086
# expansion is desired, as $PIDS is a list of PIDs. Wait on all of those PIDs.
wait $PIDS
}

check_required_files

if [ "$(uname)" == "Darwin" ]; then
docker_mac_check_cgroupsv1
fi
Expand Down
8 changes: 4 additions & 4 deletions docs/rate_limiting.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ config:
#### Resulting `haproxy.config`
```ini
backend st_http_req_rate
stick-table type ip size 1m expire 10s store http_req_rate(10s)
stick-table type ipv6 size 1m expire 10s store http_req_rate(10s)
# [...]
frontend http-in
http-request track-sc1 src table st_http_req_rate
Expand All @@ -68,7 +68,7 @@ config:
#### Resulting `haproxy.config`
```ini
backend st_http_req_rate
stick-table type ip size 1m expire 10s store http_req_rate(10s)
stick-table type ipv6 size 1m expire 10s store http_req_rate(10s)
# [...]
frontend http-in
http-request track-sc1 src table st_http_req_rate
Expand All @@ -95,10 +95,10 @@ config:
#### Resulting `haproxy.config`
```ini
backend st_http_req_rate
stick-table type ip size 1m expire 10s store http_req_rate(10s)
stick-table type ipv6 size 1m expire 10s store http_req_rate(10s)
backend st_tcp_conn_rate
stick-table type ip size 1m expire 10s store conn_rate(10s)
stick-table type ipv6 size 1m expire 10s store conn_rate(10s)
# [...]
frontend http-in
# [...]
Expand Down
5 changes: 4 additions & 1 deletion jobs/haproxy/spec
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,14 @@ properties:
description: "Number of dns queries to send to resolve a server name before giving up"
default: 3
ha_proxy.accept_proxy:
description: "Turned off by default. Enforces the use of the PROXY protocol for all incoming connections to all frontends. When enabled standard tcp connections to these port no longer work."
description: "Turned off by default. Enforces the use of the PROXY protocol for all incoming connections to all frontends, with the exception of local requests to the health check endpoint, since Monit does not support PROXY protocol. When enabled, standard TCP connections to these ports no longer work."
default: false
ha_proxy.disable_tcp_accept_proxy:
description: "Disables the PROXY protocol on tcp backends. Only applies if `ha_proxy.accept_proxy` is enabled."
default: false
ha_proxy.disable_health_check_proxy:
description: "Disables the use of the PROXY protocol for health checks. Only applies if `ha_proxy.accept_proxy` is enabled."
default: false
ha_proxy.binding_ip:
description: "If there are multiple ethernet interfaces, specify which one to bind. Set to `::` to bind to all IPv6 interfaces (no IPv4). IPv6 must be enabled on the HAProxy VM in the deployment manifest."
default: ""
Expand Down
7 changes: 5 additions & 2 deletions jobs/haproxy/templates/haproxy.config.erb
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,9 @@ listen health_check_http_url
mode http
option httpclose
monitor-uri /health
<% if p("ha_proxy.accept_proxy") && !p("ha_proxy.disable_health_check_proxy") -%>
tcp-request connection expect-proxy layer4 unless LOCALHOST
<%- end -%>
acl http-routers_down nbsrv(<%= backends.first[:name] %>) eq 0
monitor fail if http-routers_down
<% end -%>
Expand All @@ -359,12 +362,12 @@ resolvers default
<% if_p("ha_proxy.requests_rate_limit.table_size", "ha_proxy.requests_rate_limit.window_size") do |table_size, window_size| %>
backend st_http_req_rate
stick-table type ip size <%= table_size %> expire <%= window_size %> store http_req_rate(<%= window_size %>)
stick-table type ipv6 size <%= table_size %> expire <%= window_size %> store http_req_rate(<%= window_size %>)
<% end %>
<% if_p("ha_proxy.connections_rate_limit.table_size", "ha_proxy.connections_rate_limit.window_size") do |table_size, window_size| %>
backend st_tcp_conn_rate
stick-table type ip size <%= table_size %> expire <%= window_size %> store conn_rate(<%= window_size %>)
stick-table type ipv6 size <%= table_size %> expire <%= window_size %> store conn_rate(<%= window_size %>)
<% end %>
<% unless p("ha_proxy.disable_http") -%>
Expand Down
27 changes: 27 additions & 0 deletions spec/haproxy/templates/haproxy_config/healthcheck_listener_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,32 @@
expect(healthcheck_listener).to include('bind :1234')
end
end

context 'when ha_proxy.accept_proxy is true' do
let(:properties) do
{
'enable_health_check_http' => true,
'accept_proxy' => true
}
end

it 'sets expect-proxy with exception for LOCALHOST' do
expect(healthcheck_listener).to include('tcp-request connection expect-proxy layer4 unless LOCALHOST')
end

context 'when ha_proxy.disable_health_check_proxy is also true' do
let(:properties) do
{
'enable_health_check_http' => true,
'accept_proxy' => true,
'disable_health_check_proxy' => true
}
end

it 'does not set expect-proxy for the healthcheck' do
expect(healthcheck_listener).not_to include('tcp-request connection expect-proxy layer4 unless LOCALHOST')
end
end
end
end
end
4 changes: 2 additions & 2 deletions spec/haproxy/templates/haproxy_config/rate_limit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
let(:properties) { temp_properties }

it 'sets up stick-tables' do
expect(backend_req_rate).to include('stick-table type ip size 10m expire 10s store http_req_rate(10s)')
expect(backend_req_rate).to include('stick-table type ipv6 size 10m expire 10s store http_req_rate(10s)')
end

it 'tracks requests in stick tables' do
Expand Down Expand Up @@ -78,7 +78,7 @@
let(:properties) { temp_properties }

it 'sets up stick-tables' do
expect(backend_conn_rate).to include('stick-table type ip size 10m expire 10s store conn_rate(10s)')
expect(backend_conn_rate).to include('stick-table type ipv6 size 10m expire 10s store conn_rate(10s)')
end

it 'tracks connections in stick tables' do
Expand Down

0 comments on commit 9ff5f5f

Please sign in to comment.