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 8 compatibility in user management #1092

Merged
merged 3 commits into from
Oct 11, 2018
Merged

Conversation

zpetr
Copy link
Contributor

@zpetr zpetr commented Jun 22, 2018

MySQL 8 doesn't support "... IDENTIFIED BY PASSWORD ..." syntax and resources options (used after WITH in GRANT command) were moved from GRANT to ALTER USER.
If plugin is ommited, 'mysql_native_password' plugin is used by default (because the default MySQL 8 user plugin is 'caching_sha2_password')

@david22swan
Copy link
Member

@zpetr Your PR has failed the syntax check.

@david22swan david22swan merged commit e289392 into puppetlabs:master Oct 11, 2018
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 17, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 17, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.
kevindew added a commit to alphagov/govuk-puppet that referenced this pull request Dec 17, 2021
The mysql puppet version we use does not have the correct syntax for
creating users for MySQL 8 [1] (well not in a version that is compatible with
Puppet 3.x, which is understandable)

[1]: puppetlabs/puppetlabs-mysql#1092

This runs the commands to do these tasks without utilising the puppet
module and instead running the queries directly.

This worked (after a bit of trial and error) on a Whitehall DB Admin
box:

```
kevindew@ec2-integration-blue-whitehall_db_admin-ip-10-1-5-75:~$ govuk_puppet --test
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Loading facts
Info: Caching catalog for i-0a640e087db1a2f7a
Info: Applying configuration version 'c61812e33b1338ea166bac466374c58daae2a454'
Notice: /Stage[main]/Govuk::Apps::Whitehall::Db/Exec[create_whitehall_user]/returns: executed successfully
Notice: /Stage[main]/Govuk::Apps::Whitehall::Db/Exec[create_whitehall_fe_user]/returns: executed successfully
Notice: /Stage[main]/Govuk::Apps::Whitehall::Db/Exec[create_whitehall_production_db]/returns: executed successfully
Notice: /Stage[main]/Govuk::Apps::Whitehall::Db/Exec[grant_whitehall_fe_user]/returns: executed successfully
Notice: /Stage[main]/Govuk::Apps::Whitehall::Db/Exec[grant_whitehall_user]/returns: executed successfully
Notice: Finished catalog run in 7.01 seconds
```

There's a few things about this though which don't make it suitable as a
merge contender currently:

1) This uses syntax that isn't understood by MySQL 5.6 - so it will
   error when ran on an existing box, it'll need some way to alternate
   between approaches
2) I'm not sure I've got puppet dependencies set-up particularly
   elegantly, I couldn't remember how things worked and guessed a bit
   with require - I think some commands should depend on the MySQL
   package but that didn't work for me. It might be better to just have
   one script of MySQL that's executed rather than a bunch of commands
3) I'm not sure how to get it to not execute every puppet run, like the
   MySQL module currently does - I don't think it'll be

This does rely on the mysql_password function that I believe the puppet
module exposes.

The alternative to do something like this is creating the database and
users on the machines manually - this isn't the most arduous process as
they are created very infrequently
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 20, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 20, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.

NB I was not able to delete each application's `db.pp` file
because it is required by `s_db_admin.pp` and `s_backup.pp`. It
is safer to simply keep the `db.pp` files until we've fully
migrated to the 1-app-per-RDS-instance approach.
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 20, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.

NB I was not able to delete each application's `db.pp` file
because it is required by `s_db_admin.pp` and `s_backup.pp`. It
is safer to simply keep the `db.pp` files until we've fully
migrated to the 1-app-per-RDS-instance approach.
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 20, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.

NB I was not able to delete each application's `db.pp` file
because it is required by `s_db_admin.pp` and `s_backup.pp`. It
is safer to simply keep the `db.pp` files until we've fully
migrated to the 1-app-per-RDS-instance approach.
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 20, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.

NB I was not able to delete each application's `db.pp` file
because it is required by `s_db_admin.pp` and `s_backup.pp`. It
is safer to simply keep the `db.pp` files until we've fully
migrated to the 1-app-per-RDS-instance approach.
ChrisBAshton added a commit to alphagov/govuk-puppet that referenced this pull request Dec 20, 2021
The MySQL db admin machines are throwing a SQL syntax error when
puppet tries to create the database user. The error is below, with
the password hash omitted:

```
Error: /Stage[main]/Govuk::Apps::Collections_publisher::Db/Mysql::Db[collections_publisher_production]/Mysql_user[collections_pub@%]/ensure: change from absent to present failed: Execution of '/usr/bin/mysql --defaults-extra-file=/root/.my.cnf --database=mysql -e CREATE USER 'collections_pub'@'%' IDENTIFIED BY PASSWORD '(omitted)'' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PASSWORD '(omitted)'' at line 1
```

It looks like 5.6 allowed the IDENTIFIED BY PASSWORD syntax:
https://dev.mysql.com/doc/refman/5.6/en/create-user.html

But 8.0 is just IDENTIFIED BY:
https://dev.mysql.com/doc/refman/8.0/en/create-user.html

And that doesn't look overridable in the puppet module:
https://github.com/puppetlabs/puppetlabs-mysql/blob/a48069e89a4c06abccbb3595d2c782c7cd6e3254/lib/puppet/provider/mysql_user/mysql.rb#L66-L78

puppetlabs-mysql dropped support for the Puppet version we use
before they put in [the fix](puppetlabs/puppetlabs-mysql#1092)
for creating users with MySQL 8, so upgrading is not an option.

NB I was not able to delete each application's `db.pp` file
because it is required by `s_db_admin.pp` and `s_backup.pp`. It
is safer to simply keep the `db.pp` files until we've fully
migrated to the 1-app-per-RDS-instance approach.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants