Skip to content

Add flags to allow PITR binlog client TLS support#6775

Merged
deepthi merged 6 commits intovitessio:masterfrom
planetscale:jg_binlog_tls
Sep 28, 2020
Merged

Add flags to allow PITR binlog client TLS support#6775
deepthi merged 6 commits intovitessio:masterfrom
planetscale:jg_binlog_tls

Conversation

@aquarapid
Copy link
Copy Markdown
Contributor

Fix bug where password was never passed.

Signed-off-by: Jacques Grove aquarapid@gmail.com

Fix bug where password was never passed.

Signed-off-by: Jacques Grove <aquarapid@gmail.com>
Signed-off-by: Jacques Grove <aquarapid@gmail.com>
@aquarapid aquarapid marked this pull request as ready for review September 24, 2020 07:49
@aquarapid
Copy link
Copy Markdown
Contributor Author

aquarapid commented Sep 24, 2020

I cannot easily modify the endtoend tests to test the TLS support; they need to be changed to (also) pull binlogs from an upstream mysql server (since ripple does not support serving via TLS). This will take some time; we can review the rest in the meanwhile.

Signed-off-by: Jacques Grove <aquarapid@gmail.com>
Signed-off-by: Jacques Grove <aquarapid@gmail.com>
Signed-off-by: Jacques Grove <aquarapid@gmail.com>
@aquarapid
Copy link
Copy Markdown
Contributor Author

I have added an e2e test for this (TestTLSPITRRecovery). It has a few issues:

  • It does not test mTLS (client certs)
  • It is distinct from the current binlog tests, so it adds significant testing time to the e2e tests (4-5 mins). May be possible to integrate them to cut this down a bit

I am not planning to fix these issues in this PR.

Run TLS PiTR test in its own workflow "shard"

Signed-off-by: Jacques Grove <aquarapid@gmail.com>
Copy link
Copy Markdown
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

LGTM

@deepthi deepthi merged commit ac3a3d6 into vitessio:master Sep 28, 2020
@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented Sep 28, 2020

@arindamnayak this looked fine to me, but can you please take a look also?
If there's any feedback it can be addressed as a separate PR.

@askdba askdba added this to the v8.0 milestone Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants