API best practices

juju
,
1

Facades

A facade is a collection of API methods organised for a specific purpose. It’s a pretty fundamental concept: our versioning is at facade granularity, and a facade’s most fundamental responsibility is to validate incoming API calls and ensure that they’re sensible before taking action.

What exactly is a Facade?

Any type can be a facade; but if it doesn’t have exported methods with suitable signatures, it won’t be very interesting. Specifically, when a tyype is treated as a facade, any exported methods which (1) take either 0 or 1 args; and (2) returns either (result) or (result, error) will be exposed over the websocket rpc connection.

How is a Facade used by other code?

When an api request is received, the apiserver chooses a facade constructor, based on the request’s rootName and version; then constructs the facade and calls the method corresponding to the request’s methodName. (Many details elided.) Note that the facade is created for the use of, and persists only for the lifetime of, a single request.

Why would I want to write a Facade?

If any of the following conditions applies:

  • you’re writing a new worker inside an agent
  • you’re changing some existing worker, and it needs new capabilities
  • you’re exposing some new domain of functionality to an external user
  • you’re changing or extending some such exposed functionality

…you need to be writing and registering a new Facade. If it’s a new worker or a new domain, it should definitely be in a new subpackage of apiserver; if it’s a change to an existing one it still needs a new facade – to be registered under a new version – but it should go in the same package as its predecessors.

How do I write a Facade?

  • don’t import state

  • really, please, don’t import state

  • define or locate interfaces that expose the capabilities your facade requires

    • these might well be or want to be defined under core, if they’re semi-mature
    • if it’s a locally defined interface that’s fine too
    • if it’s defined somewhere else in the codebase, be suspicious: probably, either you’re depending on unnecessary concretions, or that interface is defined in the wrong place.

  • write a simple constructor for your facade, most likely using the config-struct pattern, and making sure to include an apiserver/common.Authorizer:

    package whatever

// Backend exposes capabilities required by the whatever facade.
//
// It's an example of a locally defined interface that's fine.
//
// Like all good interfaces, it should be tightly focused on its
// actual purpose and prefer not to include methods that won't be
// used.
type Backend interface {
    DoSomething(to Something) error
    // more..?
}

// Config holds the dependencies and parameters needed for a
// whatever Facade.
//
// Keep in mind that this is *completely specific to a class of
// client*: the internal `whatever` facade is intended for the
// exclusive use of the `whatever` worker; and all external facades
// are for the exclusive use of, uh, external users.
//
// That is to say: there probably won't be much to configure apart
// from the direct dependencies. Anything else is part of the
// fundamental character of the facade in play, and should probably
// not be tweakable at all.
type Config struct {

    // Authorizer represents the authenticated entity that you're 
    // responsible for granting or restricting access to. Possibly
    // "Authorisee" would have been a better name? Wouldn't object
    // if someone fixed it, I think it just came from the time when
    // Fooer was the default name for everything.
    Authorizer common.Authorizer

    // Backend exposes something-doing capabilities blah blah.
    Backend Backend

    // more..? many facades will want to access several distinct
    // backendy capabilities, consider carefully the granularity
    // at which you expose them. It's not a bad thing to pass in
    // the same object in several different fields, for example.
}

// Validate returns an error if any part of the Config is unfit for use.
func (config Config) Validate() error {

    // This bit is significant! This is the moment at which you get to
    // choose who can use the facade at all. So a user-facing facade
    // would want to AuthClient(), and ErrPerm if false; an environment
    // management job would want to AuthEnvironManager(); or perhaps this
    // is just for some worker that runs on all machine agents, in which
    // case we do:
    if !config.Auth.AuthMachineAgent() {
        return common.ErrPerm
    }
    // ...and now, bingo, you know your methods will not be called without
    // the certainty that you're dealing with an authenticated machine agent.
    // Yay!

    // May as well check these too, it's better than panicking if someone
    // has messed up and passed us garbage.
    if config.Backend == nil {
        return errors.NotValidf("nil Backend")
    }
    return nil
}

// NewFacade creates a Facade according to the supplied configuration, or
// returns an error. Note that it returns a concrete exported type, which
// is right and proper for a constructor.
func NewFacade(config Config) (*Facade, error) {
    if err := config.Validate(); err != nil {
        return nil, errors.Trace(err)
    }
    return Facade{config}, nil
}

// Facade exposes methods for the use of the whatever worker. It lives only
// for the duration of a single method call, and should usually be stateless
// in itself.
type Facade struct {
    config Config
}

  • then write some tests for your validation and your constructor, to make sure they error when they should, and in the same way, etc.

  • now you can write a super-clean test fixture that sets up a valid config and gives you easy access to all the bits you need

  • and finally you can get to implementing the actual API methods :). Do realise, though, that that’s really quite a small amount of code, and the responsibilities are hopefully as unambiguous as can be.

What should Facade methods actually look like?

Any exceptions to the below should be discussed with fwereade before landing.

  • They should always take a single argument, of a type defined in apiserver/params, that contains a slice of independent instructions.
  • That type should very frequently be params.Entities, which specifies any number of juju-defined entities using the canonical homogeneous id representation.
  • They may or may not wish to return an error, and frequently shouldn’t; if a facade method does return a top-level error, that error should indicate complete failure of the required machinery – e.g. failure to construct a fine-grained authorization func – rather than mere failure of one of the list of operations requested in the call.
  • With that in mind, many Facade methods will want to return params.ErrorResults alone, which contains the list of errors corresponding to the list of requested operations. It’s fine and expected to, e.g., return a whole list of permissions errors: that wouldn’t mean the request itself was denied – just that the specific operations all happened to be disallowed for some specific reasons.

For example:

// Resnyctopate causes resnyctopation to be applied to the supplied entities.
//
// Let's keep going with the worker-that-runs-on-every-machine-agent approach;
// in this case, we think a little bit about it and decide that the only entity
// a machine is actually allowed to resnyctopate is itself. 
func (facade *Facade) Resnyctopate(arg params.Entities) (result ErrorResults) {

    // Boilerplate, pretty much:
    result.Results = make([]params.ErrorResult, len(arg.Entities))

    // Also boilerplate, because "only allow the caller to know about itself" is
    // such a common pattern. Needing to write a more complex AuthFunc, and being
    // unable to do so, would be a good reason to return an actual error from this
    // method.
    authFunc := facade.config.Auth.AuthOwner()

    // ...
    for i, entity := range arg.Entities {

        // We don't seem to have common code for this stuff. We should either marshal
        // the over-the-wire strings into actual tags in another layer, or redefine:
        //
        //     type AuthFunc(tag string) error
        //
        // ...which would be a lot more flexible anyway.
        //
        // Note that the forces here push us towards an `if success {` model so we can
        // translate errors Once And Only Once, below. (Or we could introspect the
        // results and translate via marshalling in an outer layer..? nobody has yet,
        // though...)
        tag, err := names.ParseTag(entity.Tag)
        if err == nil {  
             switch {
             case authFunc(tag):
                 err = facade.config.Backend.DoSomething(tag)
             default:
                 err = common.ErrPerm
             }
        }

        // This step adds stuff like result codes that the client can derive
        // meaning from. It's kinda ugly once, and it's terribly ugly seeing
        // it repeated; if your loop body is any more complex than the above,
        // *strongly* consider extracting the complications into their own
        // method in service of simplicity here.
        result.Results[i].Error = common.ServerError(err) 
    }
    return result
}

How do I test a Facade?

Assuming you’ve followed the config-struct pattern, it should be easy to create a fixture that lets you write single test cases that are each as simple as “create args; call method; check results; check stub calls”; with whatever obvious commonalities extracted to keep the important bits of each test front and centre.

I think that the general approach should be to make a large number of simple single-entity calls, to test per-instruction responses in detail; not forgetting a single multi-entity call to check that (say) the second entity will still work even if the first caused an error.

Also, because you followed the advice not to import state, you can actually test your code and know that you’re passing exactly the expected bits to the backend interface(s), and you can run a large number of tests in a few milliseconds because you haven’t pulled in a mongodb dependency.

How do I register my Facade?

note this dependency on mutable global state is bad. If someone were to fix it I would be most grateful.

  • in your facade package, write an Init function something like the following:

    func init() {
    common.RegisterStandardFacade(

        //   * "Whatever" is the root name, and 1 is the version of the facade.
        //   * new facades start at 1, to prevent pre-versioning clients from
        //     calling them accidentally with implicit 0 versions
        "Whatever", 1,

        //   * yeah, there's a dependency on *state.State. That's one reason we
        //     should get rid of the global registry; if we constructed and
        //     passed around more tightly-scoped ones, we could keep the state
        //     dependency hidden away inside the registry creation functions.
        //   * you can ignore common.Resources for now, you mainly need it when
        //     exposing watchers, because they live longer than the request (and
        //     hence longer than the facade -- it's a place to put stuff that
        //     should live a long time, but should also definitely be cleaned up
        //     if the connection is dropped).
        //   * You can and should return the concrete facade type -- the anonymous
        //     func is passed as interface{}, and there's no benefit to defining
        //     an interface that your facade implements.
        func(st *state.State, _ *common.Resources, auth common.Authorizer) (*Facade, error) {
            return NewFacade(Config{

                // Authorizer could be checked here, sure, but this bit is hard
                // to test. Best to keep it as simple as possible, and be sure
                // that every client hits the exact same validation path.
                Authorizer: auth,

                // Strongly consider implementing your required backend interface(s)
                // inside the state package: please avoid leaking the existing
                // `state.*Whatever` types into new facades. I'm not opposed to clean,
                // isolated capability implementations inside state: so long as they're
                // properly tested, I'm very happy with them. But if your implementation
                // depends on the database -- which it almost certainly does -- it's
                // *very* hard to cleanly extract to another package, and if you want to
                // try you should be prepared to rework it if it doesn't pass muster.
                Backend: st.WhateverBackend()
            })
        },
    )
}

  • …and then, import your package in apiserver/allfacades.go.

Global mutable state is terribad and evil, and we should never use it

…

Migrated from the GitHub wiki, originally written by William Reade