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

acceptance-tests/acceptance_tests_suite_test.go

@@ -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)

acceptance-tests/go.mod

@@ -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

acceptance-tests/go.sum

@@ -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=
@@ -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=

acceptance-tests/proxy_protocol_test.go

@@ -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
+}

acceptance-tests/run-local.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-set -e
+set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")/" && pwd)"
 REPO_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
@@ -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

docs/rate_limiting.md

@@ -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
@@ -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
@@ -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
     # [...]

jobs/haproxy/spec

@@ -509,11 +509,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: ""

jobs/haproxy/templates/haproxy.config.erb

@@ -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 -%>
@@ -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") -%>

spec/haproxy/templates/haproxy_config/healthcheck_listener_spec.rb

@@ -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

spec/haproxy/templates/haproxy_config/rate_limit_spec.rb

@@ -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
@@ -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