feat: add `do` command to update the authentication plugin of MySQL users #1097

Danyal-Faheem · 2024-07-18T13:18:24Z

This PR is a continuation of the issue mentioned in #1089.

Add a do command to update the authentication plugin of MySQL users from mysql_native_password to caching_sha2_password.
Can specify either all to update all users or specific users to update the authentication plugin of.
Command expects entries present in the config in the format USER_MYSQL_USERNAME and USER_MYSQL_PASSWORD to work.

DawoudSheraz · 2024-07-23T13:25:04Z

tutor/commands/upgrade/common.py

@@ -43,6 +43,39 @@ def upgrade_from_nutmeg(context: click.Context, config: Config) -> None:
    )


+def get_mysql_change_authentication_plugin_query(config: Config) -> str:
+    """Helper function to generate queries depending on the loaded plugins"""


the docstring can be upgraded to highlight what is done by default and what is done based on plugin's availability.

tutor/commands/upgrade/compose.py

DawoudSheraz · 2024-07-23T13:31:12Z

tutor/commands/upgrade/k8s.py

+    # We need to first revert MySQL back to v8.1 to build the data dictionary on it
+    # And also to update the authentication plugin as it is disabled on v8.4
+    message = f"""Automatic release upgrade is unsupported in Kubernetes. If you are upgrading from Olive or an earlier release to Redwood, you
+    should upgrade the authentication plugin of your users. To upgrade, run something similar to:


Instead of saying run something similar, we should add the exact commands the user should run.

These are the exact commands that the user should run. I've updated the wording in the message.

changelog.d/20240718_171945_danyal.faheem_mysql_authentication_plugin_change.md

DawoudSheraz · 2024-08-01T13:14:42Z

tutor/commands/upgrade/common.py

@@ -43,6 +43,46 @@ def upgrade_from_nutmeg(context: click.Context, config: Config) -> None:
    )


+def get_mysql_change_authentication_plugin_query(config: Config) -> str:


Wondering if we should have an override/additional param provided by user to only upgrade a particular plugin's database. Lets say they have upgraded the default already and do not have plugins enabled. Once plugins are enabled, they will be re-doing the migration for default.

Yes, I was also thinking that we could add a --users option with comma separated name of users to the do command. This way, users can also choose to upgrade their choice of databases.

I've added a --users option as well.

DawoudSheraz · 2024-08-01T13:15:46Z

tutor/commands/upgrade/common.py

+    ) -> str:
+        return f"ALTER USER '{username}'@'{host}' IDENTIFIED with caching_sha2_password BY '{password}';"
+
+    host = "%"


why is the host set to %?

The host for users created by Tutor is always set to '%'. The users created by MySQL itself uses the 'localhost' host.

tutor/commands/upgrade/compose.py

tutor/templates/local/docker-compose.yml

DawoudSheraz · 2024-09-04T07:19:50Z

tutor/templates/local/docker-compose.yml

@@ -44,7 +44,11 @@ services:
      --character-set-server=utf8mb4
      --collation-server=utf8mb4_unicode_ci
      --binlog-expire-logs-seconds=259200
+      # We only require this option for MySQL 8.4 and above
+      # Breaks MySQL for previous versions as this option does not exist on versions earlier than 8.4
+      {% if DOCKER_IMAGE_MYSQL.split(':')[-1] == "latest" or (DOCKER_IMAGE_MYSQL.split(':')[-1].split('.') | map('int') | list >= '8.4.0'.split('.') | map('int') | list) -%}


nit: we can set DOCKER_IMAGE_MYSQL.split(':')[-1] to a variable and use that in comparison. It will make the code readable.

DawoudSheraz · 2024-09-06T07:27:04Z

docs/local.rst

+
+    tutor local do update_mysql_authentication_plugin --users=discovery,ecommerce
+
+Do note that if you are updating a specific user, there should be corresponding entries in the configuration for the mysql username and password for that user. For example, if you are trying to update the user ``myuser``, the following case sensitive entries need to be present in the configuration::


this paragraph can be reworded to focus that for any particular user, the command expects the configuration settings in a certain format. Then, the myuser examples can be added afterwards/

I've updated this paragraph.

tutor/templates/local/docker-compose.yml

DawoudSheraz

While testing, I came across the following error

Error: Username or Password for User mfe not found in config. Please make sure that the following entries are present in the configuration:
MFE_MYSQL_USERNAME
MFE_MYSQL_PASSWORD

The current code raises the exception if config is not found. It is iterating on all the plugins. Not all plugins would have DB users. Instead of raising, we can log warning and continue.

Faraz32123 · 2024-09-24T09:10:38Z

Hello @regisb, can u have a look at it for it's final iteration?

regisb · 2024-09-27T09:08:23Z

tutor/fmt.py

@@ -46,6 +46,14 @@ def alert(text: str) -> str:
    return click.style("⚠️  " + text, fg="yellow", bold=True)


+def warning(text: str) -> str:


What's the purpose of warning when we already have alert?

Alert prints to standard error and this seemed like a situation where we just wanted to display a warning rather than an error.

Let's keep an alert. We don't want to write to stdout because it might mangle output from other commands, such as tutor config printvalue/printroot. And if we write to stderr than we are better off just using alert.

tutor/templates/local/docker-compose.yml

regisb · 2024-09-27T09:17:23Z

tutor/commands/jobs.py

@@ -315,6 +317,49 @@ def sqlshell(args: list[str]) -> t.Iterable[tuple[str, str]]:
    yield ("lms", command)


+@click.command(context_settings={"ignore_unknown_options": True})
+@click.option(


This should not be an option, but an argument, because it's mandatory. The command signature should not be:

tutor local do update_mysql_authentication_plugin --users=regis,danyal

but:

tutor local do update_mysql_authentication_plugin regis danyal

Moreover, we should be able to update all users with users=all:

tutor local do update_mysql_authentication_plugin all

tutor/commands/jobs.py

regisb · 2024-09-27T09:29:36Z

tutor/commands/jobs_utils.py

+        fmt.echo_info(
+            f"Authentication plugin of user {username} will be updated to caching_sha2_password"
+        )
+        return f"ALTER USER '{username}'@'{host}' IDENTIFIED with caching_sha2_password BY '{password}';"


This function is extremely confusing, and there are many ways in which it can fail. In order to improve it, I'd like to understand if it's possible to automatically detect all mysql users that were created with the legacy authentication plugin?

The only way to do that would be to get the output of a MySQL query inside tutor codebase. I am not aware of any method that we can use to get the output of a command run inside a container inside the tutor codebase.

Assuming we could get the output of a mysql query in tutor, what would be that query?

Something similar to this:

select user from mysql.user where plugin='mysql_native_password' and host='%';

We need to limit the host to '%' as Tutor uses that host value by default.

Hey Regis, any thoughts on how we should proceed with this?

I'm not sure. basically, we need to perform all tasks inside a single templated bash script -- including the mysql --user... part. This bash script can be templated, such that we don't have to pass config values to the script-generating function. I'm sorry I don't have time to think about this more...

So maybe something similar to a stored procedure? Something akin to what we did in #1079?

We could get the names of all the users and update them inside that procedure. Although, I'd have to look into how we would manage passwords then.

…rs to caching_sha2_password

DawoudSheraz

nit: once merged and rebase to sumac, we can add a "warning" in upgrade that users should do the auth plugin updates to ensure future upgrades of MySQL do not break their systems.

regisb · 2024-12-09T14:19:39Z

tutor/commands/jobs_utils.py

+            user_uppercase = user.upper()
+            if not (
+                f"{user_uppercase}_MYSQL_USERNAME" in config
+                and f"{user_uppercase}_MYSQL_PASSWORD" in config


This looks very fragile. We can't rely on the arbitrary naming of some configuration settings to auto-detect user names.

We found that this was a convention used across the plugins, but I can understand your position here. However, we still need the exact username and password to perform this operation. Therefore, I propose that we can provide an option to provide the exact username and password. Something similar to:

tutor local do update-mysql-authentication-plugin discovery ecommerce --usernames discovery-username,ecommerce-username --passwords discovery-password,ecommerce-password

This isn't very intuitive but since this would be a one time operation, maybe it is fine? Another con I see here is that users will have to have passwords out in the open for headless scripts unless they utilize environment variables.

I am open to any suggestions you might have.

Danyal-Faheem self-assigned this Jul 18, 2024

Danyal-Faheem requested a review from DawoudSheraz July 18, 2024 13:18

DawoudSheraz reviewed Jul 23, 2024

View reviewed changes

tutor/commands/upgrade/compose.py Outdated Show resolved Hide resolved

DawoudSheraz reviewed Jul 23, 2024

View reviewed changes

DawoudSheraz requested a review from regisb July 30, 2024 07:27

DawoudSheraz reviewed Aug 1, 2024

View reviewed changes

changelog.d/20240718_171945_danyal.faheem_mysql_authentication_plugin_change.md Outdated Show resolved Hide resolved

DawoudSheraz reviewed Aug 1, 2024

View reviewed changes

tutor/commands/upgrade/compose.py Outdated Show resolved Hide resolved

Danyal-Faheem force-pushed the danyalfaheem/mysql-authentication-plugin-change branch 3 times, most recently from adaa02d to 820cebe Compare August 21, 2024 13:25

hinakhadim reviewed Aug 29, 2024

View reviewed changes

tutor/templates/local/docker-compose.yml Outdated Show resolved Hide resolved

Danyal-Faheem force-pushed the danyalfaheem/mysql-authentication-plugin-change branch from ef6022d to 4d0042c Compare August 30, 2024 14:37

DawoudSheraz reviewed Sep 4, 2024

View reviewed changes

DawoudSheraz reviewed Sep 6, 2024

View reviewed changes

tutor/templates/local/docker-compose.yml Outdated Show resolved Hide resolved

DawoudSheraz reviewed Sep 6, 2024

View reviewed changes

Danyal-Faheem changed the title ~~fix: update authentication plugin of MySQL users in v8.4~~ feat: add do command to update the authentication plugin of MySQL users Sep 9, 2024

DawoudSheraz approved these changes Sep 16, 2024

View reviewed changes

regisb requested changes Sep 27, 2024

View reviewed changes

Danyal-Faheem marked this pull request as draft October 28, 2024 15:03

Danyal-Faheem force-pushed the danyalfaheem/mysql-authentication-plugin-change branch from c872809 to 458c6de Compare November 24, 2024 15:46

feat: add do command to update the authentication plugin of MySQL use…

98d6753

…rs to caching_sha2_password

Danyal-Faheem force-pushed the danyalfaheem/mysql-authentication-plugin-change branch from 458c6de to 98d6753 Compare November 29, 2024 15:09

Danyal-Faheem marked this pull request as ready for review November 29, 2024 15:11

DawoudSheraz approved these changes Dec 6, 2024

View reviewed changes

DawoudSheraz requested a review from regisb December 6, 2024 14:08

regisb reviewed Dec 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `do` command to update the authentication plugin of MySQL users #1097

feat: add `do` command to update the authentication plugin of MySQL users #1097

Danyal-Faheem commented Jul 18, 2024 •

edited

Loading

DawoudSheraz Jul 23, 2024

Danyal-Faheem Jul 24, 2024

DawoudSheraz Jul 23, 2024

Danyal-Faheem Jul 24, 2024

DawoudSheraz Aug 1, 2024

Danyal-Faheem Aug 1, 2024

Danyal-Faheem Aug 21, 2024

DawoudSheraz Aug 1, 2024

Danyal-Faheem Aug 1, 2024

DawoudSheraz Sep 4, 2024 •

edited

Loading

DawoudSheraz Sep 6, 2024

Danyal-Faheem Sep 9, 2024

DawoudSheraz left a comment

Faraz32123 commented Sep 24, 2024

regisb Sep 27, 2024

Danyal-Faheem Sep 27, 2024

regisb Oct 18, 2024

regisb Sep 27, 2024

regisb Sep 27, 2024

Danyal-Faheem Sep 27, 2024

regisb Sep 27, 2024

Danyal-Faheem Sep 27, 2024

Danyal-Faheem Oct 14, 2024

regisb Oct 18, 2024

Danyal-Faheem Oct 18, 2024

DawoudSheraz left a comment

regisb Dec 9, 2024

Danyal-Faheem Dec 13, 2024 •

edited

Loading

		@@ -43,6 +43,46 @@ def upgrade_from_nutmeg(context: click.Context, config: Config) -> None:
		)


		def get_mysql_change_authentication_plugin_query(config: Config) -> str:


		tutor local do update_mysql_authentication_plugin --users=discovery,ecommerce

		Do note that if you are updating a specific user, there should be corresponding entries in the configuration for the mysql username and password for that user. For example, if you are trying to update the user ``myuser``, the following case sensitive entries need to be present in the configuration::

		@@ -46,6 +46,14 @@ def alert(text: str) -> str:
		return click.style("⚠️ " + text, fg="yellow", bold=True)


		def warning(text: str) -> str:

feat: add do command to update the authentication plugin of MySQL users #1097

Are you sure you want to change the base?

feat: add do command to update the authentication plugin of MySQL users #1097

Conversation

Danyal-Faheem commented Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DawoudSheraz Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DawoudSheraz left a comment

Choose a reason for hiding this comment

Faraz32123 commented Sep 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DawoudSheraz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Danyal-Faheem Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

feat: add `do` command to update the authentication plugin of MySQL users #1097

feat: add `do` command to update the authentication plugin of MySQL users #1097

Danyal-Faheem commented Jul 18, 2024 •

edited

Loading

DawoudSheraz Sep 4, 2024 •

edited

Loading

Danyal-Faheem Dec 13, 2024 •

edited

Loading