-
Notifications
You must be signed in to change notification settings - Fork 8
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
CSS-6764 update cloud creds #1134
CSS-6764 update cloud creds #1134
Conversation
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.
LGTM modulo a few comments
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.
lgtm bar ales' comments
ddf4f66
to
0b696b8
Compare
internal/jujuapi/controllerroot.go
Outdated
@@ -129,6 +130,14 @@ func newControllerRoot(j JIMM, p Params) *controllerRoot { | |||
return r | |||
} | |||
|
|||
// SetUser sets the logged in user. | |||
// Exported for test purposes. | |||
func (r *controllerRoot) SetUser(u *openfga.User) { |
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.
nitpick: doesn't need to be exported.. you can create a method in export_test.go
var SetUser = func(r *controllerRoot, u *openfga.User) {
r.mu.Lock()
r.user = u
r.mu.Unlock()
}
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.
Done.
0b696b8
to
3910752
Compare
3910752
to
ac99084
Compare
Description
This PR builds on #1127, it adds a facade method for
UpdateServiceAccountCredentials
. The facade method calls the same an underlying JIMM method as the Juju facade handlers for updating user cloud credentials.Note that the rpc handler checks that the user has permission to manage the service account, we do this check in the
jujuapi
package whereas we normally do this check in thejimm
package. I've had to write the tests slightly differently from the norm to account for this - specifically I've add an exportedSetUser
function in JIMM's controllerRoot.Fixes CSS-6764
Engineering checklist
Check only items that apply