Skip to content

Conversation

itowlson
Copy link
Collaborator

Fixes #3264.

There are several ways to approach this. I don't much love this but adopted it in order to preserve the error in app splitting scenarios.

An alternative would be to do it as part of TOML manifest normalisation before we even get to the locking stage: this would result in OCI artifacts having the wildcard pre-expanded which I dunno if it's a good thing or a bad thing or just a thing. However it would result in app splitting giving an error like "component alice service chains to component bob which doesn't exist," which seems less clear and helpful than "you can't use a wildcard if you're gonna app split."

(Note that even if we did pre-expand OCI artifacts we'd still need to either do this as well, or deal with the possibility of wildcards at run time trigger, because existing OCI artifacts could contain star chains.)

Anyway open to feedback and discussion as ever

@lann
Copy link
Collaborator

lann commented Sep 11, 2025

Another alternative to consider might be to normalize this in AllowedHostsConfig::parse, which would require passing in an extra component_ids param, but that seems feasible looking at its one ("real") call site.

@itowlson itowlson force-pushed the expand-service-chaining-wildcard branch from c4a16de to 8a2a2d3 Compare September 11, 2025 22:27
@@ -465,12 +465,14 @@ impl AllowedHostsConfig {
pub fn parse<S: AsRef<str>>(
hosts: &[S],
resolver: &spin_expressions::PreparedResolver,
component_ids: &[String],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can the copy be avoided?

Suggested change
component_ids: &[String],
component_ids: &[&str],

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't yet found a way to do so. This was what I originally tried. Unfortunately the compiler doesn't like it because it reckons the PrepareContext that the IDs are borrowed from doesn't live long enough (although this may be a spurious report from its other error which is that it results in the PrepareContext having mutable and immutable borrows at the same time). I tried it again and still no joy.

I don't trust my expertise enough to say the copy can't be avoided. It sure seems like it should be possible. But I haven't managed to find a way. I'll merge as is, but if you do spot a way, I'm ready to learn!

@itowlson itowlson merged commit f5f00ae into spinframework:main Sep 14, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider expanding *.spin.internal in allowed_outbound_hosts
2 participants