Skip to content

Add ability to overwrite default Teleport MySQL Engine Version#24464

Merged
smallinsky merged 2 commits intomasterfrom
smallinsky/proxy_mysql_engine_version
Apr 13, 2023
Merged

Add ability to overwrite default Teleport MySQL Engine Version#24464
smallinsky merged 2 commits intomasterfrom
smallinsky/proxy_mysql_engine_version

Conversation

@smallinsky
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky commented Apr 12, 2023

What

Add mysql_engine_version proxy settings that allows to overwrite the Teleport MySQL Engine Version:

proxy_service:
  mysql_engine_version: 8.0.4

The mysql_engine_version apply only for no TLS Routing connections and in case of TLS Routing connections the dynamic MySQL Engine version propagation mechanism by ALPN extension takes precedens over the mysql_engine_version settings.

Why

There is no way to propagate proper MySQL Version when a client connects to the Teleport Proxy without TLS Routing.
Some GUI Client uses different capability to connect to the MySQL Server instance and without propagation correct Engine the GUI connection can fail.

Changing current DefaultServerVersion version value will break the old behaviour so the mysql_engine_version will add ability for manual overriding default Teleport Proxy MySQL Version.

TODO:

  • Docs update.

@smallinsky smallinsky force-pushed the smallinsky/proxy_mysql_engine_version branch from 20ffa45 to 6a24fef Compare April 12, 2023 18:17
@smallinsky smallinsky marked this pull request as ready for review April 12, 2023 18:17
@github-actions github-actions Bot added database-access Database access related issues and PRs size/sm labels Apr 12, 2023
@github-actions github-actions Bot requested review from AntonAM and r0mant April 12, 2023 18:18
@smallinsky smallinsky removed the request for review from AntonAM April 12, 2023 18:19
greedy52

This comment was marked as resolved.

Comment thread lib/srv/db/mysql/proxy.go
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

With a nit.

Comment thread lib/config/fileconf.go Outdated
MySQLPublicAddr apiutils.Strings `yaml:"mysql_public_addr,omitempty"`

// MySQLEngineVersion allow to overwrite proxy default mysql engine version reported by Teleport proxy.
MySQLEngineVersion string `yaml:"mysql_engine_version,omitempty"`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very much a nitpick, but IMO "engine" spills out implementation details. How about we call it mysql_server_version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My main motivation to use MySQL Engine Version was AWS docs where Engine keyword is often used in terms of MySQL Server Version: https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/MySQL.Concepts.VersionMgmt.html

But I don't have strong preference so I will update this.

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from GavinFrazar April 12, 2023 23:33
@smallinsky smallinsky force-pushed the smallinsky/proxy_mysql_engine_version branch from a43b7b7 to 9af6531 Compare April 13, 2023 07:55
@smallinsky smallinsky force-pushed the smallinsky/proxy_mysql_engine_version branch from 9af6531 to ad3cf9f Compare April 13, 2023 09:35
@smallinsky smallinsky added this pull request to the merge queue Apr 13, 2023
Merged via the queue into master with commit b0e887a Apr 13, 2023
@smallinsky smallinsky deleted the smallinsky/proxy_mysql_engine_version branch April 13, 2023 10:23
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 size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants