On Thu, Jul 31, 2014 at 5:12 PM, William Reade <william.re...@canonical.com> wrote: > On Thu, Jul 31, 2014 at 7:30 AM, David Cheney <david.che...@canonical.com> > wrote: >> >> Proposal: remove Authoriser.GetAuthEntity() and replace all calls with >> Authoriser.GetAuthTag(). I have a branch for this, it's < 30 lines as >> there are only 3 places in the apiserver where we do this and they >> usually call the .Tag method on the entity they get back from >> GetAuthEntity. > > > SGTM. > >> 2. This extends from point 1, while the svrRoot derives everything >> from svrRoot.entity, the FakeAuthoriser allows the caller to pass a >> unique value for the tag and the entity of the authorisee. This leads >> to impossible situations where the FakeAuthorizer returns nil for >> AuthGetEntity but a non nil value for AuthGetTag. Other permutations >> exist in our tests and are expected by the test logic. >> >> Propsal: Once step 1 is fixed the difference between svrRoot and >> FakeAuthoriser is the former starts from a state.Entity and derives a >> tag from which other values are derived, the latter skips the initial >> step and starts from a tag. This work falls out of the solution to >> steps 1 and 3. >> >> 3. The helper methods, AuthMachineAgent(), AuthUnitAgent() on the >> Authoriser interface are implemented differently in svrRoot and >> FakeAuthoriser. In tests it is quite common to take the FakeAuthoriser >> from the test suite, copy it and change some of these values leading >> to impossible situations, ie, the tag or entity of the FakeAuthoriser >> pointing to a Unit, but AuthMachineAgent() returning true. >> >> Proposal: The simplest solution is to copy the implementation of these >> helper methods from svrRoot to FakeAuthoriser. A more involved >> solution would be to factor these methods out to be functions in the >> apiserver/common package that take an Authorizer. This second step may >> not pay for itself. > > > 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.
You got it boss. > > Cheers > William > >> These steps resolves the majority of the discontinuity between the two >> implementations and will resolve a set of blocking issues I've hit >> converting the apiserver and state packages to speak tags natively. >> >> Thanks for your time >> >> Dave >> >> -- >> Juju-dev mailing list >> Juju-dev@lists.ubuntu.com >> Modify settings or unsubscribe at: >> https://lists.ubuntu.com/mailman/listinfo/juju-dev > > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
