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

Create ROLES, assign them and SET default role #50

Closed
Markus- opened this issue Oct 29, 2020 · 20 comments · Fixed by #189
Closed

Create ROLES, assign them and SET default role #50

Markus- opened this issue Oct 29, 2020 · 20 comments · Fixed by #189

Comments

@Markus-
Copy link

Markus- commented Oct 29, 2020

SUMMARY

Implement ROLE feature which is part of MySQL 8 / MariaDB 10
https://mariadb.com/kb/en/roles_overview/
https://dev.mysql.com/doc/refman/8.0/en/create-role.html

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

mysql_user

ADDITIONAL INFORMATION

I didn't find any way to create a ROLE (collection of GRANTS) which can be assigned to a USER.
Those ROLES doesn't have a host and can make permission handling easier when you have many users.
You also can set a default role of a user so it is transparent for the application.

Did I miss anything?
Regards
Markus

@Andersson007
Copy link
Collaborator

@Markus- hi, thanks for the feature idea!
I'm not a mysql user, so i've no idea if it is useful or not but sounds like a missed thing.
While it's not implemented, you can use mysql_query module (that's more convenient than using shell).
If anybody decides to create this, please put a comment here first.

@Jorge-Rodriguez
Copy link
Contributor

This looks like it would require both a new module and an overhaul of the user module.
I could give this a go if nobody takes it earlier, but I have another two feature requests on my backlog that are lower hanging fruit so I'm unlikely to take on this very soon

@Andersson007
Copy link
Collaborator

Andersson007 commented Nov 7, 2020

@Jorge-Rodriguez yes, looks like that.
To implement such a module promises to be a great challenge.
We could use the basic approaches from mysql_user module, i.e. to use a similar interface relatively to privs, etc.
Once you're ready, feel free to put your thoughts here / in a WIP PR to discuss. In case you're feel you're not going to write the module, also, please, put a comment here.

@RealKelsar
Copy link

RealKelsar commented Mar 1, 2021

Today I stumbled uppon this, since mysql_user stopped working for me, since the parse function for the GRANT throws an exception, IF a 'SET DEFAULT ROLE' Statement is found. So atleast this module should ignore those lines for the time beeing. IMHO

@Jorge-Rodriguez
Copy link
Contributor

The comment from @RealKelsar points to this being related to #77

@camillehuot
Copy link

Hi @Jorge-Rodriguez , any update regarding this mariadb_role module?

@camillehuot
Copy link

Hi @Jorge-Rodriguez , any update regarding this mariadb_role module?

I meant mysql_role of course.

@Jorge-Rodriguez
Copy link
Contributor

No updates so far, sorry. I'm in the middle of a major move so my backlog is not getting any love :(

@Andersson007
Copy link
Collaborator

Hi everyone, I created the PR #189.
Please help define the module's interface (i.e. options), see the EXAMPLES block.

@camillehuot
Copy link

I'm busy on something else but here is my module to manage ROLES with MariaDB https://gist.github.com/camillehuot/e53e0e5ecbff3527386f1bac5dc6bdcd, in case it helps whoever wants to work on an official one (I really can't).

I've taken the mysql_user.py file and stripped/adapted it to manage roles.

About the interface, here's mine:

        role=dict(type="str", required=True, aliases=["name"]),
        with_admin=dict(type="str", default="CURRENT_USER"),
        state=dict(type="str", default="present", choices=["absent", "present"]),
        priv=dict(type="raw"),
        append_privs=dict(type="bool", default=False),
        check_implicit_admin=dict(type="bool", default=False),
        sql_log_bin=dict(type="bool", default=True),
        resource_limits=dict(type="dict"),

The only new attribute is with_admin to tell who is the admin of the role.
Hope it helps

Best regards
Camille

@Jorge-Rodriguez
Copy link
Contributor

Thanks @camillehuot that's very helpful!

@Andersson007
Copy link
Collaborator

@camillehuot thanks for sharing your work! We'll take a look

@camillehuot
Copy link

camillehuot commented Jun 30, 2021

FYI I've update the file with a modified API

  1. Now there are:
    admin_user = module.params["admin_user"]
    admin_host = module.params["admin_host"]

instead of 'with_admin'. I think this change is a good idea.

  1. I've added a 'all' alias for grants that allow to grant privileges on all schemas but system ones. Not sure this one would fit for a general purpose module..

I don't expect more changes from me.

@Andersson007
Copy link
Collaborator

@camillehuot thanks for the info!

I have a question to everyone who knows: does the mysql_user module always return changed=True when append_privs=yes?

@Andersson007
Copy link
Collaborator

Andersson007 commented Jun 30, 2021

It looks like a bug of user_mod function. I added

 99     - name: Append privileges that were appended by the previous task
100       mysql_user:
101         <<: *mysql_params
102         name: '{{ user_name_4 }}'
103         password: '{{ user_password_4 }}'
104         priv: 'data1.*:DELETE/data2.*:SELECT'
105         append_privs: yes
106         state: present
107       check_mode: '{{ enable_check_mode }}'
108       register: result
109 
110     - name: Assert that there was no change because permissions were added to data1.* by the prev task
111       assert:
112         that:
113           - "result.changed == false"

to tests/integration/targets/test_mysql_user/tasks/test_priv_append.yml after the similar task which added the same privileges and it also failed. I'll create an issue after this module is merged to avoid conflicts.

@Andersson007
Copy link
Collaborator

@camillehuot do i understand correctly that:

  1. WITH ADMIN is supported only by MariaDB ?
  2. ALTER ROLE statement is currently not supported (for example, to change role's admin)?

If yes, I'd suggest skipping admin_user until ALTER ROLE with changing admin is added. My main concerns are:

  1. If we recreate a role from scratch to change its admin, it would be dangerous for productions because users loose their permissions between dropping and adding the role again.
  2. If we recreate a role from scratch and new_priv is passed in a task invoked with append_privs=yes, the old grants will be lost.

So I personally see the only option to change the admin via the future ALTER ROLE command.

Alternatively, I'd put that the admin_user option is relevant only when the role is newly created and the option is ignored otherwise (we could also add a warning if a current one and a desired one don't match).

What do you guys think?

@Andersson007
Copy link
Collaborator

i added the option + warning. Needs help with manual testing of #189 with MariaDB and admin option (as we don't have MariaDB in CI). We have a guide how to test a PR locally https://github.com/ansible/community-docs/blob/main/test_pr_locally_guide.rst, it's pretty simple.

@Andersson007
Copy link
Collaborator

Finished #189. Ready for review

@Andersson007
Copy link
Collaborator

PR #189 is ready for review

@Andersson007
Copy link
Collaborator

community.mysql 2.2.0-a1 has been released and available to install via galaxy or directly from https://galaxy.ansible.com/community/mysql.
It contains the mysql_role new module.
Pay attention that this is a pre-release and is not considered ready for production.
Any feedback would be much appreciated.
After confirming that everything is find with the module (and with mysql_user module as well because both the modules share some code), we can release 2.2.0.

Thanks everyone! I'll be waiting for your feedback

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 a pull request may close this issue.

5 participants