-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat: add support for Kong Enterprise RBAC resources #276
Conversation
- diff/diff.go: fix races by adding barriers - diff/rbac_role.go: clean up error and use Identifier() instead of ID - file/builder.go: minor refactor for organizing code for another upcoming feature - file/types_test.go: remove leftover debug statements - state/rbac_role*.go: do not allow empty names in RBAC role kong's schema has the same validation; also fixes test cases accordingly - file/writer.go: zeroOutID when possible - solver/rbac_endpoint_permissions.go: use PATCH instead of POST for update operations because a POST violates DB schema and errors out
Exporting RBAC resources by default is a breaking change because not all flavors of Kong have RBAC resources. Furthermore, not all users would like to manage RBAC resources using decK.
--rbac-resources-only flag has been implemented for diff, sync, validate and dump command. When this flag is specified, only RBAC resources are expected and synced. Specifying any other resources will result in an error. Also, if RBAC resources are provided without providing this flag, decK will error out.
cc @tjrivera |
Issues and points of interest found in user/black box testing:
|
Some of the above are easy to fix: #278
Edit: okay, the PATCH behavior appears to be because PATCH cares about the endpoint in the body only. PATCHing with |
Thanks for the great feedback @rainest!
As you said, we can't do anything about this.
We can take care of this using #278 (comment)
This is solvable. We need to change the endpoint when it is passed on to go-kong and not in the FromStruct method. That will delete the permission in Kong and not cause the postProcess error. |
Trim leading slashes from RBAC endpoint permissions' endpoint before passing them to go-kong when deleting. Including the leading slash makes go-kong build a URL with a double slash, which will not actually delete the permission.
Add the --rbac-resources-only flag to the reset command, and delete all RBAC roles in the state passed to reset. Deleting roles also deletes their associated permissions.
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.
Nah, other than the now-fixed issues I mentioned previously, everything worked as I'd expect. Good to go from my perspective.
Amazing, thanks for all your hard work and making this possible, @tjrivera! |
@tjrivera FYI, we will have a release of decK with a few other features in a couple of weeks and this will be included then. |
feat: add support for Kong Enterprise RBAC resources
This takes the patch from #271 and adds in some machinery to avoid any potential breakage for existing users.
Please see #271 (comment) for reasoning behind additional commits on top of the initial patch.
Please review commit-by-commit.
Fix #270