API Login validation #84
Conversation
|
LGTM, though you'd want thumper's opinion. I like that you've split the PR into separate commits - makes each change clear. Do you amend as you go or rebase? |
|
Just repeating here for posterity: in this case there was no rebasing, just: (make some changes, commit) x 3 |
| c.Assert(err, gc.ErrorMatches, `unknown object type "Machiner"`) | ||
|
|
||
| // Since these are user login tests, the nonce is empty. | ||
| return st.Login("user-admin", "dummy-secret", "") |
There was a problem hiding this comment.
A part of me squirms here, but it isn't your fault. It is the magic of the JujuConnSuite. The hard coded "user-admin" and "dummy-secret". I'd much prefer constants, but they are used all over. So don't bother here and now.
There was a problem hiding this comment.
I'll leave this for now but am making a note to revisit this later on.
|
LGTM with minor tweaks if you feel the desire. |
There were already too many arguments being passed to this function and another one is about to be added.
This allows an option function to be supplied when the API server is constructed which will decide whether a login request can proceed. This will be used to prevent agent logins during upgrades (while still allowing client logins).
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
API Login validation These change the API server so that it can be configured with an optional validator function for API logins. This will be used by machine agents to block agent logins during software upgrades while still allowing client logins (coming in a later PR). There are 3 commits. The first is just a comment improvement, the second modifies apiserver.NewServer to take a struct instead of a long list of args and the third adds the validation support. I'm not especially wedded to the "validator" name. Other suggestions welcome.
Rename deploy to konf
These change the API server so that it can be configured with an optional validator function for API logins. This will be used by machine agents to block agent logins during software upgrades while still allowing client logins (coming in a later PR).
There are 3 commits. The first is just a comment improvement, the second modifies apiserver.NewServer to take a struct instead of a long list of args and the third adds the validation support.
I'm not especially wedded to the "validator" name. Other suggestions welcome.