Skip to content

Conversation

harmony7
Copy link
Member

@harmony7 harmony7 commented Jul 19, 2025

This PR adds Viceroy code and hostcall implementations to support the hostcalls fastly_http_req#redirect_to_grip_proxy and fastly_http_req#redirect_to_grip_proxy_v2.

It adds a new runtime command-line flag, --local-pushpin-port=<port> that enables this feature.

What does this PR do?

This pull request introduces the foundational support within Viceroy for proxying requests to a local Pushpin instance. This enables developers to locally test Fastly Compute services that use real-time features like HTTP streaming or WebSockets-over-HTTP, which are powered by Fanout/Pushpin.

Specifically, this change implements the host-side logic for the following ABI functions: fastly_http_req#redirect_to_grip_proxy and fastly_http_req#redirect_to_grip_proxy_v2

When a guest module calls one of these functions, Viceroy will now intercept the request and proxy it to a configured Pushpin server instead of handling it directly.

The configuration of Pushpin is performed by a corresponding update to the Fastly CLI, which will set up the appropriate routes and start Pushpin during the fastly compute serve command.

Why is this change important?

Currently, there is no way to locally test Compute features that rely on Fanout. This is a significant gap in the local development workflow, forcing developers to deploy their services to test real-time functionality. By adding this proxying capability to Viceroy, we bridge that gap and create a much more complete and efficient local development experience.

How is this implemented?

A new pushpin.rs Module: A new module is introduced to encapsulate all the proxying logic.

If the guest code calls redirect_to_grip_proxy (or _v2), the request is replayed to the Pushpin instance, using the name of the backend as the route ID (sets the Pushpin-route header value to the backend name). For example, a request to /api/user to a backend named origin will be sent to Pushpin as /api/user with the header Pushpin-route: origin. Fastly CLI configures Pushpin with the accept_pushpin_route=true setting.

During this forwarding, the original body of the request must be replayed from the beginning. To enable this, I've added a new body_tee.rs module, designed to allow forking a stream.

For WebSocket upgrades, the underlying connection is then handed off to a copy_bidirectional task to stream data between the client and Pushpin. In order to allow upgrading the original request, code has been added at the beginning of handle_request to save an on_upgrade future.

Details

  • A new "error" enumeration, NonHttpResponse, has been added, which enables the receiver to signal to handle_request that the guest wants to return a non-http response, allowing the handler to perform handling specific to the non-http response type.
  • A new enumeration DownstreamResponse has been added that represents the response itself, which may be an Http(Response<Body>) or a RedirectToPushpin(PushpinRedirectInfo). The hostcall for redirect_to_grip_proxy (and _v2) sends this RedirectToPushpin response type, allowing the receiver to signal NonHttpResponse to the handler.
    • The existing DownstreamResponse enumeration has been renamed to DownstreamResponseState to better reflect its name and to mirror other uses within Fastly.
  • handle_request adds code to handle NonHttpResponse response types. If the wrapped response is RedirectToPushpin, then if this feature is enabled, proxy_through_pushpin is invoked, proxying the request to Pushpin and streaming the result, and upgrading to WebSocket if the response is 101 Switching Protocols.
  • If a backend definition has a path segment, then it is respected by the Pushpin replace_beg configuration, but that setup is performed in the CLI, not in Viceroy.
  • This has a companion PR in the Fastly CLI which will invoke Pushpin with the appropriate routes before starting Viceroy.

@harmony7 harmony7 requested a review from ulyssa July 19, 2025 13:50
@harmony7 harmony7 marked this pull request as draft July 19, 2025 19:19
@harmony7 harmony7 marked this pull request as ready for review July 22, 2025 10:17
@harmony7 harmony7 force-pushed the kats/proxy-pushpin branch 3 times, most recently from 36b7715 to 1663446 Compare July 23, 2025 05:50
Copy link
Contributor

@ulyssa ulyssa left a comment

Choose a reason for hiding this comment

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

This overall looks good to me, but I'm not sure whether the body teeing behaviour actually matches what h2o is doing in practice, although teeing might be close enough.

) -> Result<(), types::Error> {
Err(Error::NotAvailable("Redirect to Fanout/GRIP proxy").into())
let req = self.session.request_parts(req_handle.into())?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Something I learned last week is that technically this hostcall will accept invalid handles in Compute (ostenstibly because of old SDKs?). I'm not sure if it's really worth duplicating that behaviour here, though, but it's worth noting in case it comes up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I think that's not hard to implement.

Comment on lines 232 to 241
/// This method must only be called once, *after* a channel has been opened with
/// [`Session::set_downstream_response_sender`][set], and *before* the associated
/// [oneshot::Receiver][receiver] has been dropped.
///
/// This method will panic if:
/// * the downstream response channel was never opened
/// * the associated receiver was dropped prematurely
///
/// [set]: struct.Session.html#method.set_downstream_response_sender
/// [receiver]: https://docs.rs/tokio/latest/tokio/sync/oneshot/struct.Receiver.html
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if the set_downstream_response_sender ever actually existed, since I can't find it in the Git history. This (and the other references to it) should probably be updated.

Suggested change
/// This method must only be called once, *after* a channel has been opened with
/// [`Session::set_downstream_response_sender`][set], and *before* the associated
/// [oneshot::Receiver][receiver] has been dropped.
///
/// This method will panic if:
/// * the downstream response channel was never opened
/// * the associated receiver was dropped prematurely
///
/// [set]: struct.Session.html#method.set_downstream_response_sender
/// [receiver]: https://docs.rs/tokio/latest/tokio/sync/oneshot/struct.Receiver.html
/// This method must only be called once per downstream request, after which attempting
/// to send another response will trigger a panic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'm going to do this in both places.

@harmony7
Copy link
Member Author

Thanks @ulyssa, I've applied your suggestions. Please review again =)

@harmony7 harmony7 requested a review from ulyssa July 30, 2025 10:28
@harmony7
Copy link
Member Author

I'm not sure whether the body teeing behaviour actually matches what h2o is doing in practice, although teeing might be close enough.

For this I went with what would have to happen because the original request's body needs to be replayed in full to the proxied request.

@harmony7 harmony7 force-pushed the kats/proxy-pushpin branch from 85f02d1 to c967b69 Compare July 31, 2025 06:25
@harmony7
Copy link
Member Author

Because of recent changes in the main branch, I did a rebase and merged the conflicts.

@harmony7 harmony7 force-pushed the kats/proxy-pushpin branch from 2aed19c to 42a1eaf Compare August 1, 2025 18:07
@harmony7
Copy link
Member Author

harmony7 commented Aug 1, 2025

I made an additional push that makes the following changes:

  • Instead of using a path prefix for routing, I'm now using a Pushpin-Route header that Pushpin understands.

@ulyssa
Copy link
Contributor

ulyssa commented Aug 4, 2025

I'm not sure whether the body teeing behaviour actually matches what h2o is doing in practice, although teeing might be close enough.

For this I went with what would have to happen because the original request's body needs to be replayed in full to the proxied request.

I talked with Fred about this some last week, and it seems like there were some plans for sending bodies to pushpin in Compute at one point that never fully materialized. From what I can tell, it's not doable today, but I'm going to keep looking into the current status. Doing the tee'ing in Viceroy for now seems fine, and we can adjust it to be closer to what the originally planned behaviour for Compute and Fanout was in the future once that's actually implemented.

@harmony7 harmony7 merged commit 47327d9 into main Aug 5, 2025
15 checks passed
@harmony7 harmony7 deleted the kats/proxy-pushpin branch August 5, 2025 04:35
@harmony7
Copy link
Member Author

harmony7 commented Aug 5, 2025

I talked with Fred about this some last week, and it seems like there were some plans for sending bodies to pushpin in Compute at one point that never fully materialized. From what I can tell, it's not doable today, but I'm going to keep looking into the current status. Doing the tee'ing in Viceroy for now seems fine, and we can adjust it to be closer to what the originally planned behaviour for Compute and Fanout was in the future once that's actually implemented.

Thanks! that sounds great =) I appreciate your thoroughness

harmony7 added a commit to fastly/cli that referenced this pull request Aug 6, 2025
This PR adds code to the CLI to enable `fastly compute serve` with
"enable Pushpin", a new mode that locates and runs Pushpin before
starting Viceroy to run guest code.

This is a companion PR to fastly/Viceroy#497,
and will only function after that feature has landed.

## How to enable it:

It can be invoked in one of two ways:

1. set a new flag `--experimental-enable-pushpin`
2. or, set a new flag in `fastly.toml`:

```toml
[local_server.pushpin]
enable = true
```

## What it does:

* `fastly compute serve` starts Pushpin before starting Viceroy, and
detects its startup. It also sets up to shut it down when the CLI
process ends.
* `pushpin` must be started at this time, because routes are set based
on the backends defined in the `fastly.toml` manifest file.
* "kill process" is done differently between Windows and Unix, so build
flags are used to include the right implementation of `killProcess`.

* Pushpin routes are set up based on the `fastly.toml` manifest file.
    * A route is set up per backend.
* The name of each backend is set as an `id=` condition. For example, a
backend named `origin` will set `id=origin`. Viceroy handles Fanout
handoff by forwarding a request for this backend to Pushpin and set the
header `Pushpin-Route: origin`, allowing Pushpin to match the route.
* If the backend definition's URL has a path segment, then it is set as
a `replace_beg` configuration on the route. For example, if a backend is
configured for `http://localhost:3000/realtime`, then this is set as
`replace_beg=realtime`. Pushpin will forward a request for `/api/user`
to this backend as `http://localhost:3000/realtime/api/user`.
* `over_http` is set for every route target, as to always enable
WebSocket-over-HTTP.
    * If the backend URL is HTTPS, then `ssl` is added.
* If the backend definition sets `override_host`, then it will be set as
the `host` value of the route target.

* When starting Viceroy, `--local-pushpin-proxy-port=<port>` is appended
as a command-line argument, which starts Viceroy in "enable Pushpin"
mode.

* Pushpin proxy runs by default on port `7677`, but this can be
overridden:
* using `proxy_port=<port>` under `local_server.pushpin` section of the
`fastly.toml` file, or
   * using `--pushpin-proxy-port=<port>`

* Pushpin's publishing endpoint runs by default on port `5561`, but this
can be overridden:
* using `publish_port=<port>` under `local_server.pushpin` section of
the `fastly.toml` file, or
   * using `--pushpin-publish-port=<port>`

* The `pushpin` binary is searched on the system path, but can be
specified:
* using `pushpin_path=/path/to/pushpin` under the `local_server.pushpin`
section of the `fastly.toml` file, or
   * using `--pushpin-path=/path/to/pushpin`.

* The CLI writes the config and routes to temporary files in the OS
temporary directory. This is cleaned up as part of shutdown.

* Pushpin runner places some runtime files into a temporary directory
specified in pushpin.conf. We set this to an OS temporary directory, so
that multiple instances can run if necessary. This is cleaned up as part
of shutdown.

* Pushpin runner places logs into a directory specified in pushpin.conf.
This is placed in the ./pushpin-logs directory under the current project

All Submissions:

* [x] Have you followed the guidelines in our Contributing document?
* [x] Have you checked to ensure there aren't other open [Pull
Requests](https://github.com/fastly/cli/pulls) for the same
update/change?

### New Feature Submissions:

* [x] Does your submission pass tests?

### Changes to Core Features:

* [x] Have you added an explanation of what your changes do and why
you'd like us to include them?
* [ ] Have you written new tests for your core changes, as applicable?
* [x] Have you successfully run tests with your changes locally?

### User Impact

* [x] What is the user impact of this change?

### Are there any considerations that need to be addressed for release?

The new feature is opt-in using the `--experimental-enable-pushpin`
flag, and does nothing if the new flag is not set.
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.

2 participants