<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 31, 2014 at 7:30 AM, David Cheney <span dir="ltr"><<a href="mailto:david.cheney@canonical.com" target="_blank">david.cheney@canonical.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

Proposal: remove Authoriser.GetAuthEntity() and replace all calls with<br>
Authoriser.GetAuthTag(). I have a branch for this, it's < 30 lines as<br>
there are only 3 places in the apiserver where we do this and they<br>
usually call the .Tag method on the entity they get back from<br>
GetAuthEntity.<br></blockquote><div><br></div><div>SGTM.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2. This extends from point 1, while the svrRoot derives everything<br>
from svrRoot.entity, the FakeAuthoriser allows the caller to pass a<br>
unique value for the tag and the entity of the authorisee. This leads<br>
to impossible situations where the FakeAuthorizer returns nil for<br>
AuthGetEntity but a non nil value for AuthGetTag. Other permutations<br>
exist in our tests and are expected by the test logic.<br>
<br>
Propsal: Once step 1 is fixed the difference between svrRoot and<br>
FakeAuthoriser is the former starts from a state.Entity and derives a<br>
tag from which other values are derived, the latter skips the initial<br>
step and starts from a tag. This work falls out of the solution to<br>
steps 1 and 3.<br>
<br>
3. The helper methods, AuthMachineAgent(), AuthUnitAgent() on the<br>
Authoriser interface are implemented differently in svrRoot and<br>
FakeAuthoriser. In tests it is quite common to take the FakeAuthoriser<br>
from the test suite, copy it and change some of these values leading<br>
to impossible situations, ie, the tag or entity of the FakeAuthoriser<br>
pointing to a Unit, but AuthMachineAgent() returning true.<br>
<br>
Proposal: The simplest solution is to copy the implementation of these<br>
helper methods from svrRoot to FakeAuthoriser. A more involved<br>
solution would be to factor these methods out to be functions in the<br>
apiserver/common package that take an Authorizer. This second step may<br>
not pay for itself.<br></blockquote><div><br></div><div>I would strongly favour the latter solution. The benefit of having a fake authorizer whose behaviour diverges is that (at least in theory) it allows for comprehensive testing against arbitrary authorizer behaviour; constraining that behaviour so we're only testing realistic situations will be great, but I fear that doing so by copy-pasting code will only lead to more divergences in future. Let's make sure we're using the same logic across the board.</div>
<div><br></div><div>Cheers</div><div>William</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
These steps resolves the majority of the discontinuity between the two<br>
implementations and will resolve a set of blocking issues I've hit<br>
converting the apiserver and state packages to speak tags natively.<br>
<br>
Thanks for your time<br>
<br>
Dave<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Juju-dev mailing list<br>
<a href="mailto:Juju-dev@lists.ubuntu.com">Juju-dev@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
</font></span></blockquote></div><br></div></div>