I've had a quick look over this out of curiosity and I have a few initial thoughts:

1. Since this is such a large scale refactor. I think it's worth standardising how requests are parsed. I have a few ideas for this.
2. I think it's worth formalising what a request context is and what it should contain. Currently we have a lot of request.CtxKey definitions, where the true nature of the request typically needs to be computed from a combination of them (e.g. CtxUsername vs. CtxForwardedUsername have a very important distinction when it comes to auth). This could be something like:

type RequestContext interface {
    context.Context
    ID() string // ID of request
    IsAuthenticated() bool
    AuthenticationMethod() string // Always returning the authentication method of the original requestor, even if forwarded
    Identifier() string // Always returning the identifier of the original requestor, even if forwarded
    IsForwarded() bool // Indicate that the request has been forwarded.
    ForwardedFrom() string
    /* etc. */
}

Then, when a request arrives, an implementation of this interface is instantiated and passed around as a context.Context. Callers can then check if it's an internal call via type assertion (e.g. requestContext, ok := ctx.(RequestContext), if !ok { /* internal */).
4. It would be nice to have an interface for the business logic that looks similar to the client interface. This would separate handling of e.g. GetInstances and GetInstancesFull, meaning that implementations would be lots of smaller functions, rather than one giant handler that catches lots of cases.

Thanks for taking a look at this 🚀

We previously briefly spoken about that and I completely agree.
Although, I would personally avoid passing RequestContext as context.Context when this is not entirely necessary. This way we avoid the need for type assertion, and we can pass nil to emphasize that there is no request context (and make the linter happy at the same time).

Also we avoid confusion where functions accept multiple contexts someFunc(ctx context.Context, reqContext context.Context, ...).

And this still leaves us the room to pass the request context as context.Context if really necessary (e.g. in helper function). In such case, the receiver can still use the type assertion as per your suggestion instead of detecting the request context by checking if a specific context key is set.

4. It would be nice to have an interface for the business logic that looks similar to the client interface. This would separate handling of e.g. GetInstances and GetInstancesFull, meaning that implementations would be lots of smaller functions, rather than one giant handler that catches lots of cases.

Could you elaborate on this one a bit? I'm a bit confused because those smaller functions generally accept different input arguments, and we would introduce an interface that just maps our current implementation?

I'm not entirely convinced on the need for an interface (as opposed to a concrete struct), is there going to be multiple implementations? If not then a concrete struct should be sufficient to hold the functions/variables, rather than needing to mask it behind an interface. See https://medium.com/@rahulgorai/why-interfaces-can-be-expensive-in-go-and-when-you-should-still-use-them-a1510c162851

Could you elaborate on this one a bit? I'm a bit confused because those smaller functions generally accept different input arguments, and we would introduce an interface that just maps our current implementation?

Currently we have handlers that accept various parameters and return different things based on path arguments and query parameters.

My meaning here is the business logic should never return e.g. []any to represent either []api.Instance or []api.InstanceFull based an a recursion parameter. Instead, the business logic should have something like:

GetInstanceNames(ctx context.Context, projectName string) ([]string, error)
GetInstances(ctx context.Context, projectName string) ([]api.Instance, error)
GetInstancesFull(ctx context.Context, projectName string) ([]api.InstanceFull, error)
GetInstanceNamesAllProjects(ctx context.Context) ([]string, error)
GetInstancesAllProjects(ctx context.Context) ([]api.Instance, error)
GetInstanceFullAllProjects(ctx context.Context) ([]api.InstanceFull, error)

Depending on the initial request parsing, the handler will call out to one of these functions. These functions would be almost identical to those defined in client/interfaces.go but with have a context parameter.

I'm not entirely convinced on the need for an interface (as opposed to a concrete struct), is there going to be multiple implementations? If not then a concrete struct should be sufficient to hold the functions/variables, rather than needing to mask it behind an interface. See https://medium.com/@rahulgorai/why-interfaces-can-be-expensive-in-go-and-when-you-should-still-use-them-a1510c162851

I agree, there's no reason for the interface. Instead we could just have a struct for this that is set in the request context. If this struct is not present then it is an internal request. This would just bundle the various existing context keys together to make them easier to use.

My meaning here is the business logic should never return e.g. []any to represent either []api.Instance or []api.InstanceFull based an a recursion parameter. Instead, the business logic should have something like: ...

Yep, that's the goal. Effectively replacing response.Response outputs with concrete types. I started with storage pools/volumes/snapshots, but will apply to all handlers once we are satisfied with the approach.

EDIT: Although the function names can be adjusted according to your suggestion. Currently I went with StorageVolumePostHandler for handlers and StorageVolumePost for BL. IMO, would be nicer to have StorageVolumePostHandler/StorageVolumePost for handlers and GetStorageVolume, GetStorageVolumeNames,... for BL.

@markylaing @tomponline Removed handler refactoring logic and just left the bits that pass around the request context instead of the http.Request. The request info is coming in a followup PR.