Static HTTP responses

[itowlson]

Fixes #3242. Example spin.toml fragment:

[[trigger.http]]
route = "/test"
static_response = { body = "hello!\n\nand more hello!\n" }

[[trigger.http]]
route = "/fermyon-me-up"
static_response = { status_code = 302, headers = { location = "https://www.fermyon.com" } }

This is, in some ways, a bit unwieldy, because the assumption that a trigger maps to a component is fairly deeply embedded. We seem to need a new handler type that can never be executed, and there is some faff around assigning a fake component ID, because everything currently depends on component IDs for lookup. If folks feel we should attempt a larger refactor then I can give it a go before fleeing into the hills.

An implication of this is that applications with static responses can't reliably interact with selective deployment (spin up -c). I believe that spin up -c retains triggers associated with the selected components, and static response triggers are associated with no component. An alternative design that would avoid this would be to have static responses as a pseudo-component type rather than a trigger behaviour (that is, the trigger still points to a named component, but the component is... not a component! ha ha!) - however, this feels extremely can-of-worms-tastic, as the component schema is embedded fairly deeply through Spin. Again, I can investigate it if that's something we want to explore.

I did consider synthesising an in-memory (or temporary filesystem) component that would respond with the static response, but that seemed like it just created large amounts of hairiness elsewhere. However, it would allow applications that used static responses to be deployed to existing hosts.

As it is, I think we need a host_requirements entry in the lockfile to ensure that hosts understand the new field. (And this field is app level because we don't have trigger-level must_understand, so it won't be bypassed by selective deployment, even though selective deployment exterminates static response triggers.) Although maybe I am wrong because this is a schema change so hosts will automatically fail by syntax rather than needing to be warned about semantics? I've put it in a separate commit in case we can back it out.

Draft because I need to write tests because I always forget to do those first, but ready for initial feedback on the approach and on the host-requirements stuff.

[lann]

As it is, I think we need a host_requirements entry in the lockfile to ensure that hosts understand the new field.

I think we're OK here; I'm reasonably confident that HttpTriggerConfig has had deny_unknown_fields at least since manifest v2, and even if it didn't it would be invalid without a component.

[lann]

Based on gazing at the code for a bit my hunch is that having no component associated with the trigger will ultimately be best.

• The way we map components to triggers is already ambiguous today; e.g. you should be able to have two wagi triggers with different argvs pointing at the same component but currently that wouldn't work. The Router probably ought to map routes to the trigger config rather than the component directly; this refactor doesn't look too bad but I have only gazed...

• Given the intended uses of selective deployments I think it would be fine to retain static response triggers unconditionally. If something really needs selective static responses I don't think it would be too hard to add support for selective deployment of triggers, i.e. spin up --trigger-id.

• I think I may have previously suggested adding a new HttpExecutorType for static responses but I would now like to recant. Instead I would suggest handling these somewhere in the vicinity of handle_trigger_route.

[itowlson]

Instead I would suggest handling these somewhere in the vicinity of handle_trigger_route.

I think that's what I do but I think I needed a placeholder executor type because that stuff gets set up before I can arrive there... maybe I'm wrong, I did go through some iterations and might have ended up with dead code

[lann]

I think that's what I do but I think I needed a placeholder executor type because that stuff gets set up before I can arrive there

If you have a dummy component ID then yeah you would need a matching executor; that whole comment apples only to an alternate-universe no-component-id PR.

[itowlson]

I have backed out the host requirement, reverted to using Options for the trigger config, and used a type for the trigger lookup key rather than synthesising a fake component ID. (The RouteMatch can't return the TriggerConfig directly because circularity. We could have it return the trigger ID rather than the component ID, but at the moment we use the route match key for printing component names against routes, so some refactoring would be needed there.)

Kept it as two commits for now, will squash if this looks less unacceptable.

[lann]

SERIALISATION TYPES SHOULD BE FOR 1. SERIALISING 2. DESERIALISING 3. NOTHING ELSE AND THE MOST IMPORTANT OF THESE IS NOTHING ELSE

– Abraham Lincoln (I think)

[lann]

Approach looks reasonable to me!

[lann]

I wonder if it would be easier to partition into component_trigger_configs and static_trigger_configs here?

[itowlson]

@lann I tried this in the "Plan C" commit. I'm not sure it's easier. It does front load some of the fiddliness, but I can't imagine it's a big perf benefit, and in terms of code clarity I think it just moves complexity around. But I've put it up so you can take a look and let me know if this is what you were envisaging and if so how you feel about it.

(I realise "Plan C" still needs a bit of polishing if we decide to go this route.)

[lann]

This suggestion was just a hunch. If it isn't clearly better then pick your favorite.

[lann]

A couple of minor suggestions for your consideration.

[lann]

Should we require at least one of these to be set? It's certainly logically valid to use all defaults but seems unlikely to be intentional.

[itowlson]

My inclination is to punt this: there are all sorts of things that are unlikely to be intentional, and this seems like a low-likelihood omission which will be spotted as soon as you use the route.

(I admit bias because I started looking at it and it turned into more of a faff than I expected, although some of that may have been overthinking.)

[lann]

This is pretty surprising when reading the tests. Maybe you should be even lazier?

Suggested change

	// This is a bit lazy but it saves a lot of faff in the conventional case
	impl From<&str> for TriggerLookupKey {
	fn from(value: &str) -> Self {
	component_key(value)
	}
	}
	fn test_router(component_routes: impl IntoIterator<Item = (&str, &str)>) -> Router {
	...
	}