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

Support HAProxy Protocol (equivalent to curl --haproxy-protocol option) #2510

Closed
ramukima opened this issue May 2, 2022 · 7 comments
Closed
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature new-http issues that would require (or benefit from) a new HTTP API

Comments

@ramukima
Copy link

ramukima commented May 2, 2022

Feature Description

Need the ability to load test an application that speaks proxy protocol at its core (e.g. envoy proxy application). In order to test such endpoints, the k6 http module must write PROXY headers immediately after dialing the connection. Here is an example - https://blog.rajatjindal.com/post/golang-proxy-protocol-http-client/ that demonstrate how to write PROXY headers on a connection.

k6 recommends building custom extensions (modules) for such things. However, this requirement applies to the core of HTTP module shipped by k6, and building an extension will require a lot of work.

The feature request here is to support proxy protocol in the core of k6 HTTP module.

Suggested Solution (optional)

It may not be ideal to use a custom header in order to support proxy-protocol. However, thats the easiest I could think of (an alternative could be a custom tags). Also, note that context object is added with additional KV for the dialer to know when to write the PROXY headers on connection.

Example Patch to k6 http module

diff --git a/Makefile b/Makefile
index be11de32..3e47380d 100644
--- a/Makefile
+++ b/Makefile
@@ -1,10 +1,12 @@
 GOLANGCI_LINT_VERSION = $(shell head -n 1 .golangci.yml | tr -d '\# ')
 TMPDIR ?= /tmp
+OS   ?= $(shell uname | tr '[:upper:]' '[:lower:]')
+ARCH ?= amd64

 all: build

 build :
-	go build
+	env GOOS=$(OS) GOARCH=$(ARCH) go build

 format :
 	find . -name '*.go' -exec gofmt -s -w {} +
diff --git a/go.mod b/go.mod
index 5bd9cf9b..4400d4d6 100644
--- a/go.mod
+++ b/go.mod
@@ -21,6 +21,7 @@ require (
 	github.com/mccutchen/go-httpbin v1.1.2-0.20190116014521-c5cb2f4802fa
 	github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d
 	github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c
+	github.com/pires/go-proxyproto v0.6.2
 	github.com/pmezard/go-difflib v1.0.0
 	github.com/serenize/snaker v0.0.0-20201027110005-a7ad2135616e
 	github.com/sirupsen/logrus v1.8.1
diff --git a/go.sum b/go.sum
index f62c719b..bdb8d421 100644
--- a/go.sum
+++ b/go.sum
@@ -123,6 +123,8 @@ github.com/onsi/gomega v1.10.1 h1:o0+MgICZLuZ7xjH7Vx6zS/zcu93/BEp1VwkIW1mEXCE=
 github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
 github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c h1:rp5dCmg/yLR3mgFuSOe4oEnDDmGLROTvMragMUXpTQw=
 github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c/go.mod h1:X07ZCGwUbLaax7L0S3Tw4hpejzu63ZrrQiUe6W0hcy0=
+github.com/pires/go-proxyproto v0.6.2 h1:KAZ7UteSOt6urjme6ZldyFm4wDe/z0ZUP0Yv0Dos0d8=
+github.com/pires/go-proxyproto v0.6.2/go.mod h1:Odh9VFOZJCf9G8cLW5o435Xf1J95Jw9Gw5rnCjcwzAY=
 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/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
diff --git a/lib/netext/dialer.go b/lib/netext/dialer.go
index 578d0e88..e482af6b 100644
--- a/lib/netext/dialer.go
+++ b/lib/netext/dialer.go
@@ -28,6 +28,7 @@ import (
 	"sync/atomic"
 	"time"

+	proxyproto "github.com/pires/go-proxyproto"
 	"go.k6.io/k6/lib"
 	"go.k6.io/k6/lib/types"
 	"go.k6.io/k6/metrics"
@@ -75,6 +76,19 @@ func (b BlockedHostError) Error() string {
 	return fmt.Sprintf("hostname (%s) is in a blocked pattern (%s)", b.hostname, b.match)
 }

+// isIPv4 reports whether addr contains an IPv4 address.
+func isIPv4(addr net.Addr) bool {
+	switch addr := addr.(type) {
+	case *net.TCPAddr:
+		return addr.IP.To4() != nil
+	case *net.UDPAddr:
+		return addr.IP.To4() != nil
+	case *net.IPAddr:
+		return addr.IP.To4() != nil
+	}
+	return false
+}
+
 // DialContext wraps the net.Dialer.DialContext and handles the k6 specifics
 func (d *Dialer) DialContext(ctx context.Context, proto, addr string) (net.Conn, error) {
 	dialAddr, err := d.getDialAddr(addr)
@@ -85,6 +99,29 @@ func (d *Dialer) DialContext(ctx context.Context, proto, addr string) (net.Conn,
 	if err != nil {
 		return nil, err
 	}
+
+	// Need to use appropriate transport protocol based on remote address
+	tp := proxyproto.TCPv4
+	if !isIPv4(conn.RemoteAddr()) {
+		tp = proxyproto.TCPv6
+	}
+
+	// Support HAProxy Protocol
+	if ctx.Value("ENABLE_PROXY_PROTOCOL") != nil && ctx.Value("ENABLE_PROXY_PROTOCOL").(bool) {
+		header := &proxyproto.Header{
+			Version:           1,
+			Command:           proxyproto.PROXY,
+			TransportProtocol: tp,
+			SourceAddr:        conn.LocalAddr(),
+			DestinationAddr:   conn.RemoteAddr(),
+		}
+
+		_, err = header.WriteTo(conn)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	conn = &Conn{conn, &d.BytesRead, &d.BytesWritten}
 	return conn, err
 }
diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go
index 47ce4c03..1838cbd8 100644
--- a/lib/netext/httpext/request.go
+++ b/lib/netext/httpext/request.go
@@ -187,6 +187,12 @@ func MakeRequest(ctx context.Context, state *lib.State, preq *ParsedHTTPRequest)
 		}
 	}

+	// Special request header to enable HAProxy protocol
+	if haProxyProto := preq.Req.Header.Get("X-HAProxy-Protocol"); haProxyProto != "" {
+		preq.Req.Header.Del("X-HAProxy-Protocol")
+		ctx = context.WithValue(ctx, "ENABLE_PROXY_PROTOCOL", true)
+	}
+
 	tags := state.CloneTags()
 	// Override any global tags with request-specific ones.
 	for k, v := range preq.Tags {
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 7a154fdb..3bb52aef 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -118,6 +118,9 @@ github.com/nu7hatch/gouuid
 # github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c
 ## explicit; go 1.12
 github.com/oxtoacart/bpool
+# github.com/pires/go-proxyproto v0.6.2
+## explicit; go 1.13
+github.com/pires/go-proxyproto
 # github.com/pmezard/go-difflib v1.0.0
 ## explicit
 github.com/pmezard/go-difflib/difflib
bash-3.2$

An example test may look like (note the special header 'X-HAProxy-Protocol') -

import http from 'k6/http';

export const options = {
  hosts: {
    'proxy-proto-app.example.com': 'A.B.C.D',
  },
};

export default function () {

  let res = http.get('https://proxy-proto-app.example.com', {
    headers: {
        "X-HAProxy-Protocol": "true",
    }
  });
}

Already existing or connected issues / PRs (optional)

No response

@na-- na-- added evaluation needed proposal needs to be validated or tested before fully implementing it in k6 new-http issues that would require (or benefit from) a new HTTP API labels May 3, 2022
@na--
Copy link
Member

na-- commented May 3, 2022

Hmm, yeah, this seems difficult to fit in an extensions, it might be something more suitable for #2461

Though I am confused. Why should k6 send this data, in a client ---> proxy server ---> backend server situation? Isn't this something the proxy server sends to the backend server so that it knows the real IP address of the client? 😕 Why should k6, in the role of the client, do anything? Real web browsers and API clients don't, right?

@ramukima
Copy link
Author

ramukima commented May 3, 2022

Hmm, yeah, this seems difficult to fit in an extensions, it might be something more suitable for #2461

Though I am confused. Why should k6 send this data, in a client ---> proxy server ---> backend server situation? Isn't this something the proxy server sends to the backend server so that it knows the real IP address of the client? 😕 Why should k6, in the role of the client, do anything? Real web browsers and API clients don't, right?

Given that the proxy server ONLY understands the proxy-protocol, the client has to write the PROXY protocol headers on the connection first. Similar option '--haproxy-protocol' was implemented in the 'curl' utility (which is a client). See curl/curl@6baeb6d. In order to test such a proxy server using k6, the k6 http client needs to write those proxy headers on the connection to the proxy server.

@na--
Copy link
Member

na-- commented May 3, 2022

Ah, I think I see, this comment from the curl commit you linked cleared up my confusion:

This option is primarily useful when sending test requests to a service that expects this header.

Please tell me if I've understood things correctly. If your normal production setup is client ---> proxy server ---> backend server, you don't want k6 to load test the proxy server, which can be some ngingx/haproxy/etc. reverse proxy and potentially a load-balancer between multiple backend servers. Instead, you actually want to load test the backend server directly, right? But the backend server expects these PROXY protocol additions, since it normally gets them from the proxied connections by the proxy server. So, you need k6 to send them instead?

Is that a fair explanation?

@ramukima
Copy link
Author

ramukima commented May 3, 2022

Ah, I think I see, this comment from the curl commit you linked cleared up my confusion:

This option is primarily useful when sending test requests to a service that expects this header.

Please tell me if I've understood things correctly. If your normal production setup is client ---> proxy server ---> backend server, you don't want k6 to load test the proxy server, which can be some ngingx/haproxy/etc. reverse proxy and potentially a load-balancer between multiple backend servers. Instead, you actually want to load test the backend server directly, right? But the backend server expects these PROXY protocol additions, since it normally gets them from the proxied connections by the proxy server. So, you need k6 to send them instead?

Is that a fair explanation?

Yes, almost. Except that in my use case, the 'proxy server' application is a multi-tenant proxy that is responsible for authN, then redirects the original request to tenant owned applications. In this case, the tenant applications DO NOT speak proxy protocol. However, my multi-tenant proxy mandates proxy protocol as the only option from north clients. Ofcourse my proxy application is also to be front-ended by another 'thing' that knows how to send proxy protocol headers to my proxy application. However, I intend to test my application with k6 and not the one that is in front of my proxy application. As a matter of fact, I would not care about any service chains created above my application. Hope this makes sense.

@na--
Copy link
Member

na-- commented May 3, 2022

I see, thanks for explaining!

When it comes to how this can be implemented in k6, I don't have time to look into all of the details, but from what I can see in your diff above, it might be possible to implement this with a simple xk6 JS extension as the first step 🤔

You have basically slightly modified the Dialer. And extensions have access to a pointer to the VU State, which contains the Dialer for that VU:

k6/js/modules/modules.go

Lines 84 to 93 in ff4c8fb

// VU gives access to the currently executing VU to a module Instance
type VU interface {
// Context return the context.Context about the current VU
Context() context.Context
// InitEnv returns common.InitEnvironment instance if present
InitEnv() *common.InitEnvironment
// State returns lib.State if any is present
State() *lib.State

k6/js/runner.go

Lines 258 to 262 in ff4c8fb

vu.state = &lib.State{
Logger: vu.Runner.Logger,
Options: vu.Runner.Bundle.Options,
Transport: vu.Transport,
Dialer: vu.Dialer,

Dialer DialContexter

So, basically, a JS extension should be able to wrap and replace the VU State.Dialer with one that understands this PROXY protocol 🤔

In the long run, this may possibly be a built-in part of the k6 core, but we probably won't include it as is in the current k6/http module and it will take a while until we arrive at some suitable design for its replacement (#2461).

For example, I can imagine a valid use case where you'd want only specific HTTP requests (or requests with other protocols) to use this PROXY protocol, while others (e.g. to a separate endpoint of your app) need to be without it. Since the Dialer is currently global for the VU, that would be impossible. This is one class of issues we'd like to solve with #2461.

@ramukima
Copy link
Author

ramukima commented May 4, 2022

I see, thanks for explaining!

When it comes to how this can be implemented in k6, I don't have time to look into all of the details, but from what I can see in your diff above, it might be possible to implement this with a simple xk6 JS extension as the first step 🤔

You have basically slightly modified the Dialer. And extensions have access to a pointer to the VU State, which contains the Dialer for that VU:

k6/js/modules/modules.go

Lines 84 to 93 in ff4c8fb

// VU gives access to the currently executing VU to a module Instance
type VU interface {
// Context return the context.Context about the current VU
Context() context.Context
// InitEnv returns common.InitEnvironment instance if present
InitEnv() *common.InitEnvironment
// State returns lib.State if any is present
State() *lib.State

k6/js/runner.go

Lines 258 to 262 in ff4c8fb

vu.state = &lib.State{
Logger: vu.Runner.Logger,
Options: vu.Runner.Bundle.Options,
Transport: vu.Transport,
Dialer: vu.Dialer,

Dialer DialContexter

So, basically, a JS extension should be able to wrap and replace the VU State.Dialer with one that understands this PROXY protocol 🤔

In the long run, this may possibly be a built-in part of the k6 core, but we probably won't include it as is in the current k6/http module and it will take a while until we arrive at some suitable design for its replacement (#2461).

For example, I can imagine a valid use case where you'd want only specific HTTP requests (or requests with other protocols) to use this PROXY protocol, while others (e.g. to a separate endpoint of your app) need to be without it. Since the Dialer is currently global for the VU, that would be impossible. This is one class of issues we'd like to solve with #2461.

Ok. The reason I tried to introduced a defined header (per request), was to allow for different request types (one using PROXY protocol, others may not include the header). Anyways, will see how it can be built as an extension. Thank you for your response.

@imiric
Copy link
Contributor

imiric commented Mar 28, 2023

As @na-- mentioned, we're unlikely to support this in the current HTTP API, but will consider it as part of the new API (initial design document), which we're starting to work on now. We're still ironing out the design and syntax, so feel free to follow the issue and document for details.

I'll close this in the meantime, as we'd like to work with a clear roadmap.

@imiric imiric closed this as completed Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature new-http issues that would require (or benefit from) a new HTTP API
Projects
None yet
Development

No branches or pull requests

3 participants