Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

Stop auto-creating MySQL db/users on the new DB admin machines #11372

Merged
merged 3 commits into from
Dec 20, 2021

Conversation

ChrisBAshton
Copy link
Contributor

@ChrisBAshton ChrisBAshton commented Dec 17, 2021

In #11353 and #11359, new DB admin machines were created for apps that use MySQL. This was done as part of RFC-143, which agrees that every app should have its own RDS instance.

The new RDS instances run MySQL version 8, where the previous shared RDS instance (blue-mysql-primary) is on version 5.6.

Traditionally, we've used Puppet to create the MySQL databases and users via the DB admin machine. However, this approach throws a SQL syntax error when applied to MySQL 8 environments:

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

This is because version 5.6 allowed the IDENTIFIED BY PASSWORD syntax, whereas 8.0 is just IDENTIFIED BY.

This isn't overridable in the puppetlabs-mysql module, and whilst the issue has been fixed in later versions of the module, we use an old version of Puppet which does not allow us to upgrade the module.

A spike investigated how we could use Puppet to create the database and user(s) in a MySQL 8 environment, but it had a number of unresolved complexities, so it was decided we would manually create the databases instead, given how infrequently we spin up new RDS instances.

Trello: https://trello.com/c/HjK4AbUS/49-configure-puppet-for-new-db-admin-mysql-instances

@ChrisBAshton ChrisBAshton changed the title Don't use 'puppetlabs-mysql' plugin to create MySQL db/users WIP Don't use 'puppetlabs-mysql' plugin to create MySQL db/users Dec 17, 2021
@ChrisBAshton ChrisBAshton force-pushed the fix-mysql-puppet branch 3 times, most recently from c27cf54 to 1718421 Compare December 20, 2021 08:03
This database user was for the contacts-frontend application,
which is now retired:
https://github.com/alphagov/contacts-frontend
CKAN uses Postgres, not MySQL, so this was in an extremely
confusing place!

It didn't break anything because the important thing is that the
db.pp file was imported at all - not what line it was imported on.
@ChrisBAshton ChrisBAshton force-pushed the fix-mysql-puppet branch 4 times, most recently from 2dcbbdd to d044198 Compare December 20, 2021 10:30
See the new 'create-mysql-db-and-users.md' file for an explanation
as to why we've stopped attempting to create the databases and
users automatically.

This will now be a manual process. A link to where the document
will live in the GOV.UK Developer Docs has been added to each
relevant db_admin definition, for ease of finding.

Whilst this renders each app's `db.pp` file essentially useless
at this point, I was not able to delete the files because they are
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 RFC-143 approach.
@ChrisBAshton ChrisBAshton changed the title WIP Don't use 'puppetlabs-mysql' plugin to create MySQL db/users Stop auto-creating MySQL db/users on the new DB admin machines Dec 20, 2021

```
sudo mysql --defaults-extra-file=/root/.my.cnf -e \
"CREATE USER IF NOT EXISTS '$TMP_MYSQL_DB_USER'@'%' IDENTIFIED WITH 'mysql_native_password' BY '${TMP_MYSQL_DB_PASSWORD}';"
Copy link
Contributor Author

@ChrisBAshton ChrisBAshton Dec 20, 2021

Choose a reason for hiding this comment

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

Note that I originally tried to use the same command we usually use, i.e. AS instead of BY:

- IDENTIFIED WITH 'mysql_native_password' AS '${TMP_MYSQL_DB_PASSWORD}';"
+ IDENTIFIED WITH 'mysql_native_password' BY '${TMP_MYSQL_DB_PASSWORD}';"

But this led to an error: The password hash doesn't have the expected format..
I attempted to fix this by changing TMP_MYSQL_DB_PASSWORD for mysql_password(TMP_MYSQL_DB_PASSWORD) (as per how we've done things previously) but found that I'd get mysql_password: command not found (even if running the process as puppet).

I was concerned that, without the mysql_password call, perhaps the password is stored unhashed in the database, but running sudo mysql --defaults-extra-file=/root/.my.cnf -e "SELECT * FROM mysql.user WHERE user='collections_pub';", I can't see the original password in the output, so I don't think we're losing any security in taking this approach.

Copy link
Contributor

@richardTowers richardTowers left a comment

Choose a reason for hiding this comment

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

Looks good to me. Nice job documenting this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants