Implement caching_sha2_password for mysql 8+#165
Merged
eileencodes merged 4 commits intomainfrom Mar 21, 2024
Merged
Conversation
Reproduction for #26
5417986 to
04d407b
Compare
96b52a4 to
adf593a
Compare
bb69dff to
db62763
Compare
fb277a9 to
febaaa9
Compare
jhawthorn
reviewed
Mar 13, 2024
caching_sha2 username should only run against that 8.0, otherwise it fails in 5.7. Co-authored-by: John Hawthorn <john@hawthorn.email>
This PR implements caching_sha2_password for mysql 8. Note that we have chosen on purpose to only implement the path where TSL is used. We will not be implementing the non-TSL path. Co-authored-by: John Hawthorn <john@hawthorn.email> Co-authored-by: Aaron Patterson (tenderlove) <tenderlove@ruby-lang.org>
d866285 to
e5af495
Compare
e5af495 to
8731936
Compare
dbussink
reviewed
Mar 18, 2024
| brew services start mysql@${{ matrix.mysql }} | ||
| sleep 5 | ||
| $(brew --prefix mysql@${{ matrix.mysql }})/bin/mysql -uroot -e 'CREATE DATABASE test' | ||
| [[ "$MYSQL_VERSION" == "8.0" ]] && $(brew --prefix mysql@${{ matrix.mysql }})/bin/mysql -uroot < docker-entrypoint-initdb.d/caching_sha2_password_user.sql |
Contributor
There was a problem hiding this comment.
Should this be any 8.x version and later? Since we also have versions up to 8.3.0 today for MySQL and each quarter there's a new one.
Member
Author
There was a problem hiding this comment.
This is the image name and we don't have a matrix for 5.7, 8.0, and 8.3, just 5.7 and 8.0. If we added 8.3 and then this wasn't updated, the build would fail. I'm not against doing a >= but it's also not broken currently. 🤷🏼♀️
Contributor
There was a problem hiding this comment.
Oh yeah, this was more future proofing.
composerinteralia
approved these changes
Mar 19, 2024
Collaborator
composerinteralia
left a comment
There was a problem hiding this comment.
One thought about the "unsupported" error class, but otherwise makes sense to me.
We aren't supporting `caching_sha2_password` in Trilogy unless mysql is running with TLS or a unix socket, so raise an error if using `caching_sha2_password` in that case.
8731936 to
f95e12d
Compare
jhawthorn
approved these changes
Mar 21, 2024
eileencodes
added a commit
that referenced
this pull request
Mar 21, 2024
composerinteralia
added a commit
that referenced
this pull request
Mar 21, 2024
Add changelog entry for #165
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements caching_sha2_password for mysql 8. Note that we have
chosen on purpose to only implement the path where TLS or a unix socket is used. We will
not be implementing the non-TLS/non-unix socket path.
Co-authored-by: John Hawthorn john@hawthorn.email
Co-authored-by: Aaron Patterson (tenderlove) tenderlove@ruby-lang.org
Fixes: #26
For testing:
Point trilogy at this branch in your Gemfile:
Update your mysql auth plugin / policy to use
caching_sha2_passwordovermysql_native_password