Validate variable keys as part of schema

[itowlson]

This works towards #2520 by enforcing variable key constraints in the manifest schema instead of at trigger time: this allows them to get picked up by spin build.

There's a risk this earlier validation could cause something to break, or to break in different ways. (Indeed I see some unit tests that I forgot to run already going a cheery red.) In particular, the error message at spin up changes from:

# Before
Error: invalid variable name: "DB_URL": invalid character 'D'. Variable names may contain only lower-case letters, numbers, and underscores.

to:

# After
Error: TOML parse error at line 10, column 1
   |
10 | DB_URL = { default = "..." }
   | ^^^^^^
Lower-case identifiers must be all lowercase; got Id("DB_URL")

which is slightly less specific in terms of explaining the error but slightly more specific in terms of locating it in the TOML!

There is also, I guess, a risk that this could block something that was legal before, but I'm pretty sure it doesn't. Not sure how to validate this other than by the power of reviewers' brains!

I completely understand if folks feel this is not worth the extra code or the extra risk, just putting it out there if we want it.

Quick note on the implementation: I layered this onto the Id type rather than the raw String type. This arguably creates unnecessary indirection, but means it is guaranteed to inherit tweaks or bug fixes to the Id reader if there ever are any. I'm happy to back this out and put it directly over String with a shared helper for the separator stuff if people prefer!

[itowlson]

The unit tests that fail are angry about the capitalised TWO in this line (and its v1 and spin-doctor equivalents):

spin/crates/manifest/tests/ui/maximal.toml

Line 17 in 418129a

var_TWO = { required = true, secret = true }

My understanding is that this is an illegal name and would be rejected when the application ran. Are we happy to modify these UI tests, or is the mixed-case variable name intentional? I'm happy to close this PR if we want to retain this existing behaviour.

[lann]

A couple of non-load-bearing comments

[lann]

I think I would either make this even more generic:

Suggested change

	pub struct Id<const DELIM: char>(String);
	pub struct Id<const DELIM: char, const LOWER: bool>(String);

or flatten these all out into separate impls.

🤷

[itowlson]

The even more generic approach works out really nicely. Thanks!

[lann]

Git tells me I wrote id_from_string; why didn't I make a TryFrom<String> I wonder? 🤔

[lann]

The unit tests that fail are angry about the capitalised TWO in this line

I think I was vaguely thinking about forward-compatibility with value imports (which will allow mixed case like this), but given that the env provider can't currently distinguish casing like this I think its fine to enforce lowercase for now.