[DPE-7322] Support predefined roles #652
Conversation
sinclert-canonical
added
the
enhancement
sinclert-canonical
force-pushed
the
sinclert/7322/predefined-roles
branch
17 times, most recently
from
350dc94 to
90bb3c8
Compare
sinclert-canonical
force-pushed
the
sinclert/7322/predefined-roles
branch
2 times, most recently
from
2bb4cdc to
ef8e039
Compare
sinclert-canonical
force-pushed
the
sinclert/7322/predefined-roles
branch
3 times, most recently
from
d6ef3e2 to
0d020f4
Compare
sinclert-canonical
force-pushed
the
sinclert/7322/predefined-roles
branch
6 times, most recently
from
e9ba0fb to
8000b34
Compare
sinclert-canonical
force-pushed
the
sinclert/7322/predefined-roles
branch
from
8000b34 to
2c8e14b
Compare
sinclert-canonical
force-pushed
the
sinclert/7322/predefined-roles
branch
from
2c8e14b to
bd05e71
Compare
| if len(role_name) >= ROLE_MAX_LENGTH: | ||
| logger.warning(f"Pruning application database role name {role_name}") | ||
| role_name = role_name[:ROLE_MAX_LENGTH] |
There was a problem hiding this comment.
This snippet is here to decouple this PR from changes that will be proposed on the MySQL Test app.
In short: there is a length limit on how roles can be named in MySQL (32 characters). MySQL Test app default database name is continuous_writes_database, which in addition to the charmed_dba prefix surpasses the limit. I would suggest shorten the default database name to just continuous_writes.
| # The granting of all privileges to the MySQL Router role | ||
| # can only be restricted when the privileges to the users | ||
| # created by such role are restricted as well | ||
| # https://github.com/canonical/mysql-router-operator/blob/main/src/mysql_shell/__init__.py#L134-L136 | ||
| f"GRANT ALL ON *.* TO {role} WITH GRANT OPTION", |
There was a problem hiding this comment.
This set of privileges is the only difference between what got approved in the spec, and the reality. We need to grant MySQL Router users ALL privileges as they are also used to setup databases / relation users.
My long-term proposal would be to phase out this behavior, so the creation of databases / relation users are done in a centralized place (i.e. MySQL operators), via some custom event or other Juju compatible signaling. However, I recognize the design challenges of that approach, so they are worth discussing + planning whenever we have bandwidth.
There was a problem hiding this comment.
what's the difference between LEGACY_ROLE_ROUTER and MODERN_ROLE_ROUTER then?
done in a centralized place (i.e. MySQL operators)
would also prefer this, but this would require the mysql <-> mysql router relation to use a different interface (i.e. not mysql_client)
There was a problem hiding this comment.
what's the difference between LEGACY_ROLE_ROUTER and MODERN_ROLE_ROUTER then?
Just the name. Starting after the merging of this PR, the name charmed_router would be the preferable choice (given the standardized prefix). However, we recognize there may be folks already relying on the mysqlrouter "role" (in truth it is just a magic word) to define their setups.
Paulo and I chatted about dropping the legacy mysqlrouter "role" when we work on supporting MySQL 8.4.
There was a problem hiding this comment.
To check my understanding: so we would switch mysql router operator from mysqlrouter to charmed_router, but the permissions will be the same?
would it be easier to just stay with mysqlrouter so that newer versions of mysql-router-operator are compatible with older versions of mysql server and then make the switchover in both charms on 8.4?
There was a problem hiding this comment.
To check my understanding: so we would switch mysql router operator from mysqlrouter to charmed_router, but the permissions will be the same?
Yes, unless the centralized creation of databases / relation users work is scoped + agreed + allocated.
would it be easier to just stay with mysqlrouter so that newer versions of mysql-router-operator are compatible with older versions of mysql server and then make the switchover in both charms on 8.4?
Yes, I am not changing the requested role within MySQL Router operators (see VM and K8s). Backwards compatibility is preserved.
There was a problem hiding this comment.
not changing the requested role within MySQL Router operators
what's the purpose in adding charmed_router then? seems like it might make more sense to add that when we add the new behavior for that role, or on 8.4 if we choose to keep the current behavior
There was a problem hiding this comment.
what's the purpose in adding charmed_router then?
Standardizing naming for all the predefined roles.
There was a problem hiding this comment.
but nobody will be using that role? and if we want to change the behavior in the future, it will be a breaking change—or we'll have to use a different role name for the new behavior
not sure if I'm missing something
sinclert-canonical
requested review from
paulomach,
taurus-forever and
carlcsaposs-canonical
This PR implements the set of predefined roles proposed in this specification.
In addition to the predefined roles proposed to be on sync with PostgreSQL 16 operators, there is an additional one in order to solve a long time standing time debt:
mysqlrouter. This predefined role is proposed in order to stop treating mysqlrouter as a magic word used to assignALL PRIVILEGESto the created users whoseextra-user-rolesconfig argument contained such word. That exact name has been chosen to preserve backwards compatibility.Additional changes:
extra-user-rolesconfig argument._) instead of mixing dashes and underscores.