state/apiserver/keymanager: canRead/canWrite is not a common.AuthFunc #571
Conversation
This PR is a prereq of #556. On investigation the `canRead`, `canWrite` authz methods in the keymanager are not implementations of `common.AuthFunc` as they do not use tags. Although with the current defintion of `common.AuthFunc` the are interchangable, when #556 lands they not be as the `user` field transmitted over the api is not a tag. To make this clear, stop defining `canRead` and `canWrite` in terms of `common.AuthFunc` to make this difference crystal clear.
| @@ -40,8 +40,8 @@ type KeyManagerAPI struct { | |||
| state *state.State | |||
| resources *common.Resources | |||
| authorizer common.Authorizer | |||
| getCanRead common.GetAuthFunc | |||
| getCanWrite common.GetAuthFunc | |||
| getCanRead func() (func(string) bool, error) | |||
There was a problem hiding this comment.
I think we can just store func(string) bool. No errors are ever returned from getCanRead or getCanWrite, and there's nothing dynamic about them.
There was a problem hiding this comment.
SGTM. I'll clean this up then repropose
On Thu, Aug 21, 2014 at 3:15 PM, Andrew Wilkins notifications@github.com
wrote:
In state/apiserver/keymanager/keymanager.go:
@@ -40,8 +40,8 @@ type KeyManagerAPI struct {
state *state.State
resources *common.Resources
authorizer common.Authorizer
- getCanRead common.GetAuthFunc
- getCanWrite common.GetAuthFunc
- getCanRead func() (func(string) bool, error)
I think we can just store func(string) bool. No errors are ever returned
from getCanRead or getCanWrite, and there's nothing dynamic about them.—
Reply to this email directly or view it on GitHub
https://github.com/juju/juju/pull/571/files#r16520862.
|
PTAL |
|
LGTM |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
…r-canread-is-not-an-authfunc state/apiserver/keymanager: canRead/canWrite is not a common.AuthFunc This PR is a prereq of #556. On investigation the `canRead`, `canWrite` authz methods in the keymanager are not implementations of `common.AuthFunc` as they do not use tags. Although with the current defintion of `common.AuthFunc` the are interchangable, when #556 lands they not be as the `user` field transmitted over the api is not a tag. To make this clear, stop defining `canRead` and `canWrite` in terms of `common.AuthFunc` to make this difference crystal clear.
This PR is a prereq of #556.
On investigation the
canRead,canWriteauthz methods in the keymanager are not implementations ofcommon.AuthFuncas they do not use tags.Although with the current defintion of
common.AuthFuncthe are interchangable, when #556 lands they not be as theuserfield transmitted over the api is not a tag.To make this clear, stop defining
canReadandcanWritein terms ofcommon.AuthFuncto make this difference crystal clear.