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

mysql: Pass mysql.Conn through {Hash,PlainText,Caching}Storage interfaces #9264

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

mattrobenolt
Copy link
Contributor

@mattrobenolt mattrobenolt commented Nov 18, 2021

The TLS client certificates can simply be derived based on the
mysql.Conn passed through, but passing through the actual Conn, gives us
access to Conn.ClientData or anything else an Auth{Method,Server} might need to
implement.

It's arguable that if we need access to the lower level Conn, we could
simply implement the actual HandleAuthPluginData instead of these
*Storage interfaces, and that's true. But not all AuthMethods can get
away with this because they need to write to the Conn's connection using
private functions.

For example, the mysqlCachingSha2AuthMethod needs to write more data
on the socket after calling UserEntryWithCacheHash, so to write a new
UserEntryWithCacheHash that needs data from mysql.Conn, I'm left
wtih no other options since I can't vendor or reimplement the rest of
the logic without having access to the private functions/methods within
the mysql package.

Checklist

  • Should this PR be backported? (no)
  • Tests were added or are not required (no, existing tests were modified)
  • Documentation was added or is not required (In changelog probably?)

@mattrobenolt mattrobenolt changed the title RFC: mysql: Pass mysql.Conn through {Hash,PlainText,Caching}Storage interf… RFC: mysql: Pass mysql.Conn through {Hash,PlainText,Caching}Storage interfaces Nov 18, 2021
…aces

The TLS client certificates can simply be derived based on the
mysql.Conn passed through, but passing through the actual Conn, gives us
access to Conn.ClientData or anything else an Auth{Method,Server} might need to
implement.

It's arguable that if we need access to the lower level Conn, we could
simply implement the actual HandleAuthPluginData instead of these
*Storage interfaces, and that's true. But not all AuthMethods can get
away with this because they need to write to the Conn's connection using
private functions.

For example, the `mysqlCachingSha2AuthMethod` needs to write more data
on the socket after calling `UserEntryWithCacheHash`, so to write a new
`UserEntryWithCacheHash` that needs data from `mysql.Conn`, I'm left
wtih no other options since I can't vendor or reimplement the rest of
the logic without having access to the private functions/methods within
the `mysql` package.

Signed-off-by: Matt Robenolt <matt@ydekproductions.com>
@mattrobenolt mattrobenolt changed the title RFC: mysql: Pass mysql.Conn through {Hash,PlainText,Caching}Storage interfaces mysql: Pass mysql.Conn through {Hash,PlainText,Caching}Storage interfaces Nov 30, 2021
@deepthi deepthi added Component: Query Serving release notes Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Nov 30, 2021
@deepthi deepthi merged commit 6d8de8e into vitessio:main Nov 30, 2021
@deepthi
Copy link
Member

deepthi commented Nov 30, 2021

@mattrobenolt welcome to vitess and thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants