-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
Remove EWC support & move LDAP/RBAC configuration to Stackstorm.st2 #290
Conversation
…ck as notify is not a blog parameter
…and st2_(ldap|auth)_enable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+112 −740
- nice diff, love it! Makes things so much easier now 👍
Some early pointers for this PR, - looks also that the RBAC logic is missing notifying the st2-apply-rbac-definitions --config-file /etc/st2/st2.conf
to apply the new definitions & assignments.
Related to this PR, st2docs may need an update: https://docs.stackstorm.com/install/ansible.html to reflect this new change. It's possible to include the instructions from the history to provide the RBAC & LDAP configuration example: |
I'll look into that over the weekend. |
I provided a PR for the st2docs update: StackStorm/st2docs#1061 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for migrating the RBAC smoketests.
All looks good, however based on TravisCI jobs RBAC smoke tests are skipped. Could we enable them to make sure that the added functionality is tested in CI?
Enabling the RBAC tests take a bit more time than expected but I'm on it and provide them in the next days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the tests are failing on idempotence checks.
I've provided more pointers around the RBAC some tests, hope that helps.
@armab this PR passes the tests now and I ran them multiple times in my own dev environment. So I hope I got everything right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 👍
Thanks a lot for the diligence refactoring this.
This PR:
close #260