Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide multiple entries as a variable for lists in 'spin.toml' #2526

Open
mikkelhegn opened this issue May 22, 2024 · 17 comments
Open

Provide multiple entries as a variable for lists in 'spin.toml' #2526

mikkelhegn opened this issue May 22, 2024 · 17 comments
Labels
good first issue Good for newcomers

Comments

@mikkelhegn
Copy link
Member

Considering I have an application, which needs an undefined number of entries in allowed_outbound_hosts passed in as a variable.

What I would like to be able to do:

[variables]
allowed_hosts = { default = "https://fermyon.com" }
...
[component.foo]
allowed_outbound_hosts = "[{{ allowed_hosts }}]"

And provide something like this:

SPIN_VARIABLE_ALLOWED_HOSTS="https://www.google.com","https://www.fermyon.com" spin up

A scenario for doing this, is if you have multiple endpoints you need to connect to, but want to make it a runtiem decision what those endpoints are.

@lann
Copy link
Collaborator

lann commented May 23, 2024

I think the most straightforward way to implement this would be to add new syntax to the allowed_outbound_hosts entries, e.g.:

allowed_outbound_hosts = ["(http://host1.com,http://host2.com)"]
# which would enable:
allowed_outbound_hosts = ["({{service_endpoints}})"]
# SPIN_VARIABLE_SERVICE_ENDPOINTS="https://www.google.com,https://www.fermyon.com"

This could be extended to allow expansion of just parts of the URL which could be useful even outside of this variable interpolation case, e.g.

  • Multiple ports: ["*://host.com:(1000,2000,3000)"]
  • Multiple domains with the same scheme: ["https://(google.com,fermyon.com)"]

Implementation-wise, this seems reasonably straightforward to implement as a sort of "pre-processing" step in AllowedHostsConfig::parse.

@lann
Copy link
Collaborator

lann commented May 23, 2024

One thing to give a bit of thought to is that this could potentially introduce a kind of variable expansion vulnerability, e.g.

allowed_outbound_hosts = ["(https://production.com,http://localhost:{{dev_port}})"]
# Attacker injects SPIN_VARIABLE_DEV_PORT="8000,http://evil.com"

This seems pretty marginal though; any situation where an attacker can inject freeform text at this level is likely to be broken anyway.

@mikkelhegn
Copy link
Member Author

Implementation-wise, this seems reasonably straightforward to implement as a sort of "pre-processing" step in AllowedHostsConfig::parse.

Could this be a good-first-issue?

@lann
Copy link
Collaborator

lann commented May 23, 2024

Could this be a good-first-issue?

Yeah I think so. Not a great "first Rust issue" but probably fine as a "first Spin issue" with just a bit of guidance.

Note for anyone looking: definitely needs a bit of discussion on the syntax before starting.

@mikkelhegn mikkelhegn added the good first issue Good for newcomers label May 23, 2024
@rylev
Copy link
Collaborator

rylev commented May 23, 2024

FWIW we intended to support lists of items such that allowed_outbound_hosts = ["{http,https,redis}://{example.com,foobar.com}:{80, 1024}"] would work. There is some code already for supporting this, but some of it is stubbed out (e.g., here).

As far as I'm aware though, we don't have any support for injecting from variables so that would have to be added.

@lann
Copy link
Collaborator

lann commented May 23, 2024

As far as I'm aware though, we don't have any support for injecting from variables so that would have to be added.

We do now: #2299

@lann
Copy link
Collaborator

lann commented May 23, 2024

re: curly braces: while that was the first thing I thought of too, I quickly recoiled in horror when I wrote out the example: "{{{{ allowed_hosts }}}}"

edit: Actually the current variable template syntax just straight up doesn't allow for that. It would need to be something like "{ {{ allowed_hosts }} }" (#2528)

@itowlson
Copy link
Contributor

My spidey sense is concerned that this one will involve enough bikeshedding, edge cases, and general finickiness that it may not turn out to be a good-first-issue after all. (I will be delighted to be wrong.)

@mikkelhegn
Copy link
Member Author

involve enough bikeshedding, edge cases, and general finickiness

Consider yourself warned 😀

@itowlson
Copy link
Contributor

Another poorly-thought-through idea could be to have an expand operator rather than relying on simple text substitution with comma separation within the resulting string. E.g. allowed_outbound_hosts = ["http://google.com", "{{ my_hosts | expand }}", "postgres://{{ db-host }}"] would take the my_hosts variable, CSV-parse it, and splice that list into the AOH list after parsing. This would (I think) avoid the escaping syntax worry, might help avoid Lann's concatenation vulnerability (or of course might introduce a whole new vulnerability), but could imply other syntax that makes me all jittery e.g. "http://google.com:{{ my_ports | expand }}". Anyway just throwing it into the mix

@lann
Copy link
Collaborator

lann commented May 28, 2024

Backing up: is there a concrete use case behind this issue? Not that the request is unreasonable either way, but concrete holds more water than...ideas? 🤔

@mikkelhegn
Copy link
Member Author

A scenario for doing this, is if you have multiple endpoints you need to connect to, but want to make it a runtiem decision what those endpoints are.

One specific use case is to wanting to use this with .NET aspire or Docker Compose, where serviceA needs access to an endpoint, which is hosted locally as you develop. If serviceA has multiple endpoints to reach (e.g., a Database and an API), then both endpoints needs to be substituted at deployment off of the local machine.

@lann
Copy link
Collaborator

lann commented May 28, 2024

both endpoints needs to be substituted at deployment off of the local machine.

I would expect to be able to do this today as e.g.:

[variables]
database_host = { default = "localhost:5432" }
api_url = { default = "http://localhost:8888" }

[component.foo]
# ...
allowed_outbound_hosts = [
  "*://{{ database_host }}",
  "{{ api_url }}",
]

[component.foo.variables]
database_url = "postgres://{{ database_host }}"
api_url = "{{ api_url }}"

@ThorstenHans
Copy link
Contributor

@lann although the suggested solution would work in some scenarios, here some additional thoughts:

  • We don't want to modify the Spin Manifest from within .NET Aspire (we wanna treat the manifest as immutable)
  • Outbound Hosts could include HTTP(s) endpoints, database endpoints, etc
  • There could be 0-n allowed endpoints of different kinds
  • Endpoints may differ between subsequent runs

These circumstances led to our idea of specifying a variable from .NET Aspire SPIN_VARIABLE_OUTBOUND_HOSTS=... and use templating syntax in spin.toml.

This allows users to create their spin app in a "more generic" way and have actual endpoints of app dependencies injected at startup time.

@lann
Copy link
Collaborator

lann commented May 29, 2024

I'm not sure I completely understand the gap between my suggested solution and these requirements. I promise I'm not trying to be contrarian, just trying to understand the problem to help design a solution. 🙂

  • We don't want to modify the Spin Manifest from within .NET Aspire (we wanna treat the manifest as immutable)
  • Endpoints may differ between subsequent runs

The example manifest above wouldn't need to be modified just to change endpoints; it would use e.g. environment variables just like your idea.

  • Outbound Hosts could include HTTP(s) endpoints, database endpoints, etc

The example shows a mix of HTTP and PostgreSQL (implying sockets) endpoints. Is there another example that expresses the problem better?

  • There could be 0-n allowed endpoints of different kinds

Could you expand on this? Is the requirement here multiple endpoints for one service? Native service discovery is definitely a missing feature in Spin which we can try to tackle if necessary.

This allows users to create their spin app in a "more generic" way and have actual endpoints of app dependencies injected at startup time.

I'm not sure I understand exactly what this means, but it sounds like we may be fighting against the Spin security model (and to some extent, the Component Model itself).

One of the things I try to do with this kind of problem is match it against planned Component Model features so that we can align toward the future. Component Model "value imports" are currently under development and would provide a way for components to express their config dependencies as part of their public interface, which is intended to help with this kind of problem. I'm trying to understand if it would represent a solution to this specific problem, which would help inform whether we're looking at a long-term solution or a short-term interim solution.

@ThorstenHans
Copy link
Contributor

I'm not sure I understand exactly what this means, but it sounds like we may be fighting against the Spin security model (and to some extent, the Component Model itself).

I think the idea is not against the Spin Security model. Let me explain it in a different way.

To me, outbound connectivity is environment specific. Consider having a Spin App that is using a backend service via HTTP. As application developer, I want to use the backend service, no matter which origin it is actually being served from.

Deploying the Spin App to different environments should neither require modifying spin.toml, re-packaging the app as OCI artifact, nor distributing it again through a registry. Ops should be able to set the desired endpoint (url) for the backend service upon deployment for the actual environment.

Maybe, this is an indicator to come up with a different way of defining allowed_outbound_hosts:

It would be great to use aliases to specify allowed outbound hosts in spin.toml as we do with key-value stores. Ending up with something like:

# spin.toml

[component.foo]
allowed_outbound_hosts = [ {name: api } ]

Actual URLs could be moved to the runtime config files:

Development:

# dev.runtime-config.yaml
[allowed_outbound_hosts]
name: api
url: http://some-service:8080

Production

# dev.runtime-config.yaml
[allowed_outbound_hosts]
name: api
url: http://super-service.vendor.svc.cluster.local:3000

@lann
Copy link
Collaborator

lann commented Jun 10, 2024

It would be great to use aliases to specify allowed outbound hosts in spin.toml as we do with key-value stores.

Specifying an API endpoint as a capability would be interesting I think it would be somewhat tricky to get the design right; there are a lot of knobs on an HTTP request and it isn't immediately obvious how you would split that responsibility between the runtime config and the application code.

Looking at this example, it seems like you should be able to get the same outcome today with variables, though annoyingly you would currently need to use an indirection through a "dotenv_path". As part of the "Spin Factors" project I am adding a static variables provider that would let you inline values into the runtime config file: https://github.com/fermyon/spin/pull/2519/files/b194e293d491fed95cff291cdb33f4f350ae1eaa#diff-496fce369401c1d30d796126b6e8dae9999248593e46d701f3fcb3019d16b671R88-R91

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants