Skip to content

Feature - Add support for importing mysql roles#268

Open
jutaz wants to merge 10 commits intopetoju:masterfrom
jutaz:feature/mysql-role-import-support
Open

Feature - Add support for importing mysql roles#268
jutaz wants to merge 10 commits intopetoju:masterfrom
jutaz:feature/mysql-role-import-support

Conversation

@jutaz
Copy link

@jutaz jutaz commented Jan 23, 2026

Closes #267

Copy link
Owner

@petoju petoju left a comment

Choose a reason for hiding this comment

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

Thanks - please fix (don't use) quoteIdentifiers as it's used for different kind of identifiers.

Also please make sure your PR passes all the tests. You can test it easily using make test in your shell or look at failed tests in this PR.

roleName := d.Get("name").(string)

sql := fmt.Sprintf("CREATE ROLE '%s'", roleName)
sql := fmt.Sprintf("CREATE ROLE %s", quoteIdentifier(roleName))
Copy link
Owner

Choose a reason for hiding this comment

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

Please leave the old solution here. It's maybe counter-intuitive, but quote identifier uses backticks - and this needs apostrophes.
Backticks work only on MariaDB as far as I know. That applies to all calls with roles.

d.SetId("")
return nil
errorNumber := mysqlErrorNumber(err)
if errorNumber == 1133 || errorNumber == 1396 {
Copy link
Owner

Choose a reason for hiding this comment

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

When would error 1396 happen? Should we handle that?

Maybe I'd add 1141 here (ER_NONEXISTING_GRANT).

Copy link
Author

Choose a reason for hiding this comment

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

https://www.bytebase.com/reference/mysql/error/1396-operation-failed-for-user/

This error occurs when trying to perform a user management operation (such as CREATE USER, DROP USER, or GRANT) on a MySQL user that either already exists (for CREATE) or doesn't exist (for DROP or GRANT). It indicates a mismatch between the expected and actual state of the user account.

Copy link
Owner

Choose a reason for hiding this comment

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

But you're not running any of these. And based on the link, it could be an "internal error" of non-flushed user. In that case, we probably don't want to arbitrarily drop the resource from TF that exists in reality.

@jutaz
Copy link
Author

jutaz commented Jan 23, 2026

Also please make sure your PR passes all the tests. You can test it easily using make test in your shell or look at failed tests in this PR.

Yeah, doing that as part of the PR. make test doesn't appear to run well on arm64 due to old mysql images.

@jutaz jutaz force-pushed the feature/mysql-role-import-support branch from 8b8353e to c52aabd Compare January 28, 2026 12:09
jutaz added 5 commits January 28, 2026 14:13
- Change quoteRoleName to use single quotes (e.g., 'name') instead of backticks
- Change formatUserIdentifier to use single quotes (e.g., 'user'@'host')
- Both functions now escape single quotes by doubling them ('')
- This provides better compatibility across MySQL/MariaDB versions
- Supports @ in usernames for GCP IAM compatibility
- unescapeRoleName already handles doubled single quotes
- quoteRoleName now uses backticks with doubled-backtick escaping
- UserOrRole.SQLString() uses backticks for roles (Host == ''), single quotes for users
- unescapeRoleName already handles both doubled backticks and doubled single quotes
- formatUserIdentifier continues using single quotes for user@host format
@jutaz jutaz force-pushed the feature/mysql-role-import-support branch from a33f573 to 1ac6e33 Compare February 24, 2026 12:03
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.

Add import support for mysql_role

2 participants