-
Notifications
You must be signed in to change notification settings - Fork 86
Feat/support authentication plugins like ldap simple #133
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
base: master
Are you sure you want to change the base?
Feat/support authentication plugins like ldap simple #133
Conversation
…es it Signed-off-by: Alejandro Recalde <[email protected]>
…revious behavior working Signed-off-by: Alejandro Recalde <[email protected]>
Signed-off-by: Alejandro Recalde <[email protected]>
Signed-off-by: Alejandro Recalde <[email protected]>
Signed-off-by: Alejandro Recalde <[email protected]>
…ser spec field diverged Signed-off-by: Alejandro Recalde <[email protected]>
Signed-off-by: Alejandro Recalde <[email protected]>
Signed-off-by: Alejandro Recalde <[email protected]>
31c0af7 to
6529b9e
Compare
|
Would you mind rebasing? |
|
@alereca are you still interested in this feature? I'd like to see this merged, I can rebase and open a new PR crediting your work if you want. |
@mcanevet I would greatly appreciate it. Currently, I'm quite busy working on some quarterly job tasks and finishing my studies. Please let me know if you have any questions about anything here. |
@Duologic Sorry I didn't answer before |
|
@alereca I think I underestimated the complexity of the conflicts. |
|
As MariaDB does not support mysql_native_password, and mysql_native_password has been removed in later MySQL releases, I think we should support not managing the auth. plugin too. So either nil or "" should mean the default plugin.
Additionally, there are syntax differences here:
So https://mariadb.com/kb/en/create-user/ Unless there are license issues I think this means we should run mysql in CI to catch issues like this, or at least enable running that locally. WIP fix of compatibility issues + rebase: https://github.com/crossplane-contrib/provider-sql/compare/master...chlunde:provider-sql:feat/support-authentication-plugins-like-ldap-simple?expand=1 |
|
Is there any chance of this being merged in the near future? It would really help us out, we have a case where we need to setup IAM Auth for RDS and #186 would solve our issues but it seems it was dropped in favour of this solution. |
…g as default plugin > Instead of mysql_native_password that has been deprecated from MySQL 8.0.34 Signed-off-by: Alejandro Recalde <[email protected]>
Signed-off-by: Alejandro Recalde <[email protected]>
199bb8c to
89c48e4
Compare
…instead of mysql_native_password Signed-off-by: Alejandro Recalde <[email protected]>
Signed-off-by: Alejandro Recalde <[email protected]>
e41d1a2 to
7f0f5a2
Compare
Signed-off-by: Alejandro Recalde <[email protected]>
…e logic Signed-off-by: Alejandro Recalde <[email protected]>
…r resource Signed-off-by: Alejandro Recalde <[email protected]>
|
@mcanevet Thank you for trying to help with the rebase. I was swamped at the time, but I've now managed to rebase it onto the current master and address all the issues. Hopefully it's in better shape now! |
|
I've now rebased this PR onto the current master and addressed all the compatibility issues that were raised. The PR now:
@Duologic When you have a moment, would you mind taking a look? I believe this is now ready for review. Thanks! |
Description of your changes
Fixes #125
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Added unit test for user controller create and update methods