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

Pt 2247 #686

Merged
merged 13 commits into from
May 21, 2024
Merged

Pt 2247 #686

merged 13 commits into from
May 21, 2024

Conversation

Tusamarco
Copy link
Contributor

  • [ x] The contributed code is licensed under GPL v2.0
  • [ x] Contributor Licence Agreement (CLA) is signed
  • [ NA] util/update-modules has been ran
  • [ x] Documentation updated
  • [NA ] Test suite update

@svetasmirnova please check and review also https://jira.percona.com/browse/PT-2247

Tusamarco and others added 7 commits September 18, 2023 15:16
1) OR instead and in the if line 2042
2) evaluate array create_user if it has some value
3) filter out the user creation in Mariadb inside the Grants
- Use of VIA
- Use of USING
- Incompatible syntax with MySQL
- ADD IF NOT EXISTS
- convert the script to us AS to maintain the hash as they are

Add a parameter --convert-MariaDB as parameter (default false)
correct an IF condition
removed a delete action on mysql.user. Always use DROP not delete
…2_password plugin.

The problem is that when caching_sha2_password is used the character utilised in the string may be invalid for MySQL itself to process during creation time.
Given that the password must be converted to HEX and then pushed as binary using the AS <bin password> format
Tusamarco and others added 4 commits September 25, 2023 15:13
…n alternative to the conversion

Param --print_identified_with_as_hex
Adding the option to use in session print_identified_with_as_hex as a…
Adding a small check if we have MariaDB in order to prevent usage of …
Copy link
Collaborator

@svetasmirnova svetasmirnova left a comment

Choose a reason for hiding this comment

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

PLease add test case. You can find how in the contribution guide and examples in t/pt-show-grants/

@@ -2039,25 +2039,52 @@ sub main {

# If MySQL 5.7.6+ then we need to use SHOW CREATE USER
my @create_user;
if (( VersionCompare::cmp($version, '5.7.6') >= 0 ) &&
if (( VersionCompare::cmp($version, '5.7.6') >= 0 ) ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

With "||" this expression is always true.

bin/pt-show-grants Outdated Show resolved Hide resolved

When set it convert some of the proprietary MariaDB syntax into valid MySQL form

=item --print_identified_with_as_hex
Copy link
Collaborator

Choose a reason for hiding this comment

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

According to the user manual at https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_print_identified_with_as_hex:

Hash values that do not contain unprintable characters still display as regular string literals, even with this variable enabled.

Do we really need this option or checking for correct MySQL version is enough?

- Removed option print_identified_with_as_hex, because it was already
  implemented in the PT-2190 fix
- Simplified patch
- Kept CREATE USER/ALTER USER sequence and extra DELETE from mysql.user
  table
- Added test case
@svetasmirnova svetasmirnova self-requested a review May 16, 2024 23:35
@svetasmirnova
Copy link
Collaborator

@Tusamarco I changed your PR: simplified a few things, removed the option --print_identified_with_hex (made it default and only behavior), and added a test case. Please review and see if you agree with the changes.

Copy link
Contributor Author

@Tusamarco Tusamarco left a comment

Choose a reason for hiding this comment

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

LGTM

@svetasmirnova svetasmirnova merged commit a74cf58 into 3.x May 21, 2024
3 of 4 checks passed
@svetasmirnova svetasmirnova deleted the pt-2247 branch May 21, 2024 14:30
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