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.

Sign up for GitHub

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

Jump to bottom

Delete user rbac attachments on deletion of users #271

Merged
merged 5 commits into from
Jul 15, 2020
Merged

Delete user rbac attachments on deletion of users #271

merged 5 commits into from
Jul 15, 2020

Conversation

speza
Copy link
Contributor

@speza speza commented Jul 7, 2020

  • Add a DeleteUser method so we can delete rbac attachments for a user.
  • Make noop.go fully no-op (makes sure that a call to DeleteUser) even with RBAC off is OK. All other endpoints now fully no-op for consistency.
  • Introduce a UserHandler to contain dependencies for the user handlers

speza added 3 commits July 7, 2020 21:26
@speza
Delete user rbac attachments on deletion of users
5f5b1bd 
* Add a DeleteUser method so we can delete rbac attachments for a user.
* Make noop.go fully no-op (makes sure that a call to DeleteUser) even with RBAC off is OK. All other endpoints now fully no-op for consistency.
* Introduce a UserHandler to contain dependencies for the user handlers
@speza
Merge remote-tracking branch 'upstream/master' into delete-user-rbac
52db251 
# Conflicts:
#	handlers/user_test.go
@speza
Fix tests post merge
3e91851
@Skarlso Skarlso self-requested a review July 11, 2020 12:16
Skarlso
Skarlso requested changes Jul 12, 2020
View reviewed changes
Copy link
Member

@Skarlso Skarlso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine but please extract the user handler creation into server.go so init handler remains testable. InitHandler just used the user handler and shouldn't create it. It should be injected like the rest of the handlers.

@speza
Copy link
Contributor Author

speza commented Jul 12, 2020 •
edited
Loading

This looks fine but please extract the user handler creation into server.go so init handler remains testable. InitHandler just used the user handler and shouldn't create it. It should be injected like the rest of the handlers.

@Skarlso do you want me to make a UserProivder? and I did the same with RBACHandler, so could make an RBACProvider?

speza added 2 commits July 12, 2020 08:36
@speza
Move handler deps to providers
01fb892 
* Created a RBAC Provider
* Created a User Provider
@speza
Fix linting issues
e32ca80
@Skarlso
Copy link
Member

Skarlso commented Jul 12, 2020

LGTM will merge after testing on the test instance.

@Skarlso
Copy link
Member

Skarlso commented Jul 13, 2020

Will merge soon

@Skarlso Skarlso merged commit f588ae4 into gaia-pipeline:master Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Skarlso Skarlso Skarlso approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@speza @Skarlso