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 proxy protocol v2 in MySQL #12424

Merged
merged 18 commits into from
May 18, 2022
Merged

Conversation

probakowski
Copy link
Contributor

@probakowski probakowski commented May 4, 2022

#11684 added support for proxy protocol v2 for SSH and Postgres but MySQL uses different code path and it was missing. This change fixes that.
It also adds tests for v2 protocol support for MySQL, Postgres, Mongo and Redis

@probakowski probakowski added enhancement db/mysql MySQL related database access issues labels May 4, 2022
@github-actions github-actions bot added the database-access Database access related issues and PRs label May 4, 2022
require.NoError(t, err)
require.NoError(t, psql.Close(ctx))
for _, v2 := range []bool{false, true} {
v2 := v2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is now (we pass it down to closure to NewTestProxy, I had hardcoded false before)

Copy link
Collaborator

Choose a reason for hiding this comment

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

But there's no parallel code here, so I think v2 will be correct on each iteration of the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added it as a safeguard in case we add something like t.Parallel somewhere down the road. Currently it's true it's not needed. I removed it for now.

lib/srv/db/proxy_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Apologies for the delay.

lib/multiplexer/proxyline.go Outdated Show resolved Hide resolved
lib/multiplexer/proxyline.go Outdated Show resolved Hide resolved
@@ -60,6 +60,38 @@ func (p *ProxyLine) String() string {
return fmt.Sprintf("PROXY %s %s %s %d %d\r\n", p.Protocol, p.Source.IP.String(), p.Destination.IP.String(), p.Source.Port, p.Destination.Port)
}

// Bytes returns on-the wire bytes representation of proxy line conforming to the proxy v2 protocol
func (p *ProxyLine) Bytes() []byte {
var b []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: use a bytes.Buffer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I changed it

lib/multiplexer/proxyline.go Outdated Show resolved Hide resolved
@probakowski probakowski enabled auto-merge (squash) May 10, 2022 18:33
@probakowski probakowski merged commit 17fc073 into master May 18, 2022
@probakowski probakowski deleted the probakowski/proxyv2-mysql branch May 18, 2022 16:08
probakowski added a commit that referenced this pull request May 27, 2022
#11684 added support for proxy protocol v2 for SSH and Postgres but MySQL uses different code path and it was missing. This change fixes that.
It also adds tests for v2 protocol support for MySQL, Postgres, Mongo and Redis

(cherry picked from commit 17fc073)
@probakowski
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
branch/v9

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

probakowski added a commit that referenced this pull request Jun 7, 2022
#11684 added support for proxy protocol v2 for SSH and Postgres but MySQL uses different code path and it was missing. This change fixes that.
It also adds tests for v2 protocol support for MySQL, Postgres, Mongo and Redis

(cherry picked from commit 17fc073)
@webvictim webvictim mentioned this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database-access Database access related issues and PRs db/mysql MySQL related database access issues enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants