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

Fix MySQL privileges so that it is compatible with RDS #663

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
3 participants
@mnapoli
Copy link
Contributor

commented Nov 8, 2018

Fix #660

@luceos

This comment has been minimized.

Copy link
Member

commented Nov 8, 2018

I'm curious whether this will impact forward and backward compatibility with MySQL and MariaDB.. Tests for the current version seem to be AOK with this 🤔

@mnapoli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 8, 2018

Here are the privileges that are "removed" with this change:

  • SUPER
  • FILE
  • SHUTDOWN

Those don't seem like privileges you would want to give anyway right? (and that makes sense that they are not enabled on RDS)

The only exception might be FILE, this is the one that allows importing from CSV (LOAD DATA INFILE). But if you don't use it then it's fine.

@luceos

This comment has been minimized.

Copy link
Member

commented Nov 9, 2018

One advantage is that you can create a listener to add the necessary permissions after creation of the user anyway.

I'm still not too happy about this change, fearing for unexpected consequences.

I'll merge this as a feature for a new minor release.

@Dries-SF

This comment has been minimized.

Copy link

commented Nov 9, 2018

With regards to the CSV import, you could still technically use the system DB connection to import into a tenant DB. It's a little more hassle though.

@mnapoli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 9, 2018

@luceos understood, I've also opened #665 if you prefer this: it adds a configuration option for the privilege list.

Feel free to choose this one or the other and close the PR you will not use :)

@luceos

This comment has been minimized.

Copy link
Member

commented Nov 12, 2018

Preferring the other one, thanks for the consideration. Great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.