Always print warning on blocked outbound HTTP #1833
Conversation
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
crates/outbound-http/src/lib.rs
Outdated
| @@ -53,6 +53,9 @@ impl outbound_http::Host for OutboundHttp { | |||
| .map_err(|_| HttpError::RuntimeError)? | |||
| { | |||
| tracing::log::info!("Destination not allowed: {}", req.uri); | |||
| terminal::warn!("A component tried to make a HTTP request to '{}'.", req.uri); | |||
| eprintln!("The component was configured not to allow requests to this host."); | |||
| eprintln!("To allow requests, add the host to `allowed_http_posts`."); | |||
There was a problem hiding this comment.
| eprintln!("To allow requests, add the host to `allowed_http_posts`."); | |
| eprintln!("To allow requests, add the host name to the `allowed_http_posts` key/value pair in the component section of spin.toml."); |
There was a problem hiding this comment.
Could we perhaps even include a link to the developer hub page about this? Something like
See https://developer.fermyon.com/spin/manifest-reference#the-component-tables for more information.
There was a problem hiding this comment.
🚲🏚️
To allow requests, add e.g. `allowed_http_hosts = ["<host from actual request>"]`
to the the component's section of spin.toml.
There was a problem hiding this comment.
I did wonder to what extent to make this a "explain what is happening" vs "write exhaustive documentation on how to fix it." E.g. I thought about warning them to restart after making the change, but was wary of turning it into a wall of text that the eyes just slid off. It may be too much in that direction already; I'd have preferred to keep it to 1 or 2 lines.
crates/outbound-http/src/lib.rs
Outdated
| @@ -53,6 +53,9 @@ impl outbound_http::Host for OutboundHttp { | |||
| .map_err(|_| HttpError::RuntimeError)? | |||
| { | |||
| tracing::log::info!("Destination not allowed: {}", req.uri); | |||
| terminal::warn!("A component tried to make a HTTP request to '{}'.", req.uri); | |||
| eprintln!("The component was configured not to allow requests to this host."); | |||
| eprintln!("To allow requests, add the host to `allowed_http_posts`."); | |||
There was a problem hiding this comment.
Could we perhaps even include a link to the developer hub page about this? Something like
See https://developer.fermyon.com/spin/manifest-reference#the-component-tables for more information.
|
Based on feedback and angst I'm trying a new message:
I sent this as a separate commit so it's easy to back out if people hate this more than the previous one. |
I'm not sure that people will know what this means. Reference |
|
I avoided mentioning |
|
Ah yeah. Maybe something like "manifest component section" then? We have been discussing having a separate "component manifest" thing... |
|
SOLD |
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
itowlson
force-pushed
the
report-blocked-http-hosts
branch
from
ca555e1
to
ac4d6c6
Compare
Fixes #1090.
With the proposed text, it looks like this:
Get painting those bikesheds.
(Note that the host component doesn't know which component it's attached to, so we can't provide a component ID without more work (heaven forfend.)