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

Fastrobot/fix ctrl hash #469

Closed

Conversation

jcookfastrobot
Copy link
Contributor

Description

Shortened 'username' to 'user' in the ctrl_hash for run_query method in the mysql_user custom resource
Added inspec test to verify non-root users passed in the ctrl_hash can successfully run percona_mysql_user

Issues Resolved

List any existing issues this PR resolves

Check List

  • [ x ] A summary of changes made is included in the CHANGELOG under ## Unreleased
  • New functionality includes testing.
  • New functionality has been documented in the README if applicable.

@jcookfastrobot jcookfastrobot requested a review from a team as a code owner March 17, 2023 23:26
@jcookfastrobot
Copy link
Contributor Author

This is an Obvious fix.

@jcookfastrobot
Copy link
Contributor Author

I have updated the PR after finding a couple of other deployment issues.

One involved managing two users 'user'@'localhost' and 'user'@'127.0.0.1' with the ctrl_host set to ‘localhost’. The code would only update the password for the 'user'@'localhost' but not 'user'@'127.0.0.1' We tracked this issue down to the ctrl_hash being passed to run the test_sql "SELECT 'user can login'" I renamed that hash to user_hash and have updated the host parameter to use the scope (aka host) instead of the ctrl_host to force the mysql command to try and validate 'user'@'host' instead of 'user'@'ctrl_host' (which in our case was always localhost).

The other issue was that the cookbook wasn’t escaping special characters in the password substring set in the sql_command_string method. I wrapped the #{ctrl[:password]} in '' We ran into an issue where one of the passwords had a > and the shell_out created a file in /.

@jcookfastrobot
Copy link
Contributor Author

I have updated this PR to fix the idempotent error on the second converge with mysql8 installs. In the percona_mysql_database custom resource, there was a method (percona_default_encoding) to determine the value for the encoding property if none was passed. For versions => 8, it was using 'utf8mb3'...otherwise it used 'utf8'. The default collate property was just 'utf8_general_ci'. It seems that when the percona_mysql_database custom resource was run to create a mysql8 instance in the first pass, it would create the DB with a default collate value that matched its encoding...aka (utf8mb3_general_ci) and on the second pass it would alter the DB by updating the collate to match the default set in the property (utf8_general_ci)...hence not idempotent between runs.

After talking to some percona versed folks, they pointed out that the default charset and collate for mysql8 should be 'utf8mb4' and 'utf8mb4_0900_ai_ci'. So I have updated the percona_mysql_encoding to be utf8mb4 for versions => 8 and I have added a new method called percona_default_collate method that returns 'utf8mb4_0900_ai_ci' for versions => 8 otherwise it returns 'utf8_general_ci'. Finally, I have updated the collate property to use the new method (default: lazy { percona_default_collate })

@jcookfastrobot
Copy link
Contributor Author

One last update to get the pecona 8.0 client to install on Centos 7. There is a dependency conflict when the recipe is tries to install the percona-server-client-8.0.32-24.1.el7.x86_64.rpm. The existing mariadb-libs rpm must be removed before the recipe will converge successfully.

Unfortunately, yum will also remove postfix when it removes mariadb-libs, so I added a package 'postfix' after the install of the percona-server-client rpm in the client.rb recipe to make sure it is still around.

I also added a boolean helper method called percona_8_on_centos_7 for the only_if guard.

recipes/client.rb Outdated Show resolved Hide resolved
recipes/client.rb Outdated Show resolved Hide resolved
@ramereth
Copy link
Contributor

@jcookfastrobot can you please look into the latest CI failures?

@ramereth ramereth added the Release: Minor Release to Chef Supermarket as a minor release when merged label Apr 18, 2023
@damacus damacus mentioned this pull request Apr 25, 2023
3 tasks
@damacus
Copy link
Member

damacus commented Apr 25, 2023

Thanks @jcookfastrobot. I'm going to roll these changes into #476 ❤️

@damacus damacus closed this Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release: Minor Release to Chef Supermarket as a minor release when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants