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(expect-proxy): provide alternative health endpoint for expect_proxy_cidrs #717

Merged
merged 2 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions acceptance-tests/proxy_protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ package acceptance_tests

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

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
proxyproto "github.com/pires/go-proxyproto"
)

var _ = Describe("Proxy Protocol", func() {
Expand Down Expand Up @@ -72,6 +73,12 @@ var _ = Describe("Proxy Protocol", func() {
- type: replace
path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/accept_proxy?
value: false
- 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: true
- type: replace
path: /instance_groups/name=haproxy/jobs/name=haproxy/properties/ha_proxy/expect_proxy_cidrs?
value:
Expand All @@ -83,13 +90,21 @@ var _ = Describe("Proxy Protocol", func() {
haproxyBackendPort: haproxyBackendPort,
haproxyBackendServers: []string{"127.0.0.1"},
deploymentName: deploymentNameForTestNode(),
}, []string{opsfileExpectProxyProtocol}, map[string]interface{}{}, true)
}, []string{opsfileExpectProxyProtocol}, map[string]interface{}{}, false)

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

closeBackendTunnel := setupTunnelFromHaproxyToTestServer(haproxyInfo, haproxyBackendPort, localPort)
defer closeBackendTunnel()
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 5 minutes for the status to stabilise
Eventually(func() string {
return boshInstances(deploymentNameForTestNode())[0].ProcessState
}, 5*time.Minute, time.Second).Should(Equal("running"))

By("Checking that Proxy Protocol is expected for requests from IPs in the list")
// Requests without Proxy Protocol Header should be rejected
Expand All @@ -98,7 +113,16 @@ var _ = Describe("Proxy Protocol", func() {

// Requests with Proxy Protocol Header should succeed
err = performProxyProtocolRequest(haproxyInfo.PublicIP, 80, "/")
Expect(err).NotTo(HaveOccurred())
Expect(err).NotTo(HaveOccurred(), "Requests with Proxy Protocol Header did not succeed, but should have: %v", err)

By("Sending a request without Proxy Protocol Header to HAProxy health check port should succeed")
_, err = http.Get(fmt.Sprintf("http://%s:8080/health", haproxyInfo.PublicIP))
Expect(err).NotTo(HaveOccurred(), "Sending a request without Proxy Protocol Header to HAProxy health check port did not succeed, but should have: %v", err)

By("Sending a request with Proxy Protocol Header to HAProxy health check port, should succeed.")
err = performProxyProtocolRequest(haproxyInfo.PublicIP, 8081, "/health")
Expect(err).NotTo(HaveOccurred(),
"Sending a request with Proxy Protocol Header to HAProxy health check port did not succeed, but should have: %v", err)

By("Checking that Proxy Protocol is NOT expected for requests from IPs NOT in the list")
// Set up a tunnel so that requests to localhost:11000 appear to come from localhost
Expand Down
2 changes: 1 addition & 1 deletion jobs/haproxy/spec
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ properties:
description: "The HAProxy health check endpoint returns a healthy status if at least one backend server is responding. By default when enable_health_check_http: true, Bosh will consider the HAProxy VM unhealthy if the HAProxy health check returns an unhealthy status. In some cases this might not be desired, for example when deploying HAProxy before deploying the backend servers. To prevent Bosh from considering the HAProxy VM unhealthy when all backend servers are unhealthy set disable_monit_health_check_http: true. Note that this flag is ignored unless enable_health_check_http: true."
default: false
ha_proxy.health_check_port:
description: "port for http health-check"
description: "port for http health-check. Please note that `health_check_port` + 1 (e.g. 8081) is used with the `accept-proxy` directive, if `expect_proxy_cidrs` are defined. You will need to direct your health checker to the appropriate port (with or without Proxy Protocol) for correct functionality."
default: 8080
ha_proxy.disable_http:
description: "Disable port 80 traffic"
Expand Down
16 changes: 11 additions & 5 deletions jobs/haproxy/templates/haproxy.config.erb
Original file line number Diff line number Diff line change
Expand Up @@ -365,14 +365,20 @@ listen health_check_http_url
<%- if p("ha_proxy.accept_proxy") && !p("ha_proxy.disable_health_check_proxy") -%>
tcp-request connection expect-proxy layer4 unless LOCALHOST
<%- end -%>
<%- if p("ha_proxy.expect_proxy_cidrs", []).size > 0 -%>
<%- if !p("ha_proxy.disable_health_check_proxy") -%>
tcp-request connection expect-proxy layer4 if { src -f /var/vcap/jobs/haproxy/config/expect_proxy_cidrs.txt }
<%- end -%>
<%- end -%>

acl http-routers_down nbsrv(<%= backends.first[:name] %>) eq 0
monitor fail if http-routers_down

<%- if p("ha_proxy.expect_proxy_cidrs", []).size > 0 -%>
listen health_check_http_url_proxy_protocol
bind :<%= p("ha_proxy.health_check_port") + 1 %> accept-proxy
mode http
option httpclose
monitor-uri /health
acl http-routers_down nbsrv(<%= backends.first[:name] %>) eq 0
monitor fail if http-routers_down
<% end -%>
<%- end -%>

<% if_p("ha_proxy.resolvers") do |resolvers| -%>
resolvers default
Expand Down