Implement --reverse-proxy <SOURCE_PATH_PATTERN>=<DESTINATION_URL_PREFIX>

[mlaily]

I would like to use dotnet-serve as a convenient and easy way to setup a Fable development environment while staying in the .NET world, and one thing that is missing is a reverse proxy feature so I can quickly iterate on the frontend while keeping a backend api running separately.

I saw that I'm not the first to want a reverse proxy (#11), but it looks like the last person lost interest...

This PR implements a --reverse-proxy <SOURCE_PATH_PATTERN>=<DESTINATION_URL_PREFIX> directive, using https://github.com/microsoft/reverse-proxy (YARP)

It seems to work great, and was easy enough to implement, even though YARP is still in preview.

One drawback is that YARP does not support .NET Core 2.1, so its support in dotnet-serve would have to be dropped. (Or we would need to add lots of #if NETCOREAPP2_1)
I'm not sure it's a big problem: after all dotnet-serve is a tool, so it's likely to be run on up-to-date versions of .NET don't you think?

I added some unit tests for the configuration and for the feature itself.

I cleaned up existing #if NETCOREAPP2_1 in a separate commit since I wasn't sure you were ready to drop its support completely. If you absolutely want to keep this target, I could drop this commit and wrap all the reverse proxy related stuff in new #if NETCOREAPP2_1 instead (please don't make me do that).

Tell me what you think! :)

[codecov]

Codecov Report

Merging #85 (3a321aa) into main (28ad8d4) will increase coverage by 3.16%.
The diff coverage is 54.83%.

@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
+ Coverage   24.02%   27.19%   +3.16%     
==========================================
  Files          17       17              
  Lines         541      603      +62     
==========================================
+ Hits          130      164      +34     
- Misses        411      439      +28     

Impacted Files	Coverage Δ
src/dotnet-serve/Startup.cs	0.00% <0.00%> (ø)
src/dotnet-serve/Program.cs	55.35% <72.72%> (+1.89%)	⬆️
src/dotnet-serve/CommandLineOptions.cs	57.44% <100.00%> (+16.27%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28ad8d4...3a321aa. Read the comment docs.

[mlaily]

Hi, did you get a chance to take a look?

I'm in no rush of course. I just think it would be a nice addition to this project, especially since this feature has been requested by others in the past...

[natemcmaster]

Thanks for proposing this improvement. I would like to wait until Yarp releases a stable version before incorporating it into this project. My experience in the past has been that you can't rely on previews of .NET projects for production-ready code. Once there is a stable release, I would be happy to add the reverse proxy feature.

Side note: as a matter of preference, I would like to keep PRs focused on a single thing. If you think this project needs cleanup or needs to drop the .NET Core 2 support, can you do those as separate pull requests?

Thanks for your effort here. I am glad you decided to contribute!

[natemcmaster]

Also, I see some of the workflow checks on this PR are failing. I won't merge a change unless it passes all tests. Can you check the reason why and make changes if necessary?

[mlaily]

I would like to wait until Yarp releases a stable version before incorporating it into this project. My experience in the past has been that you can't rely on previews of .NET projects for production-ready code. Once there is a stable release, I would be happy to add the reverse proxy feature.

Alright, I can get behind that.

Side note: as a matter of preference, I would like to keep PRs focused on a single thing. If you think this project needs cleanup or needs to drop the .NET Core 2 support, can you do those as separate pull requests?

I agree with you, but since Yarp does not support .NET Core 2.1, removing support for .NET Core 2.1 is part of the changes required to add Yarp... (As I said, it would be possible to add lots of #ifdefs instead, but I don't really see the point if we are going to remove them immediately after)

Also, I see some of the workflow checks on this PR are failing. I won't merge a change unless it passes all tests. Can you check the reason why and make changes if necessary?

I believe the CI is broken for other reasons than my PR:

error NU1101: Unable to find package dotnet-format. No packages exist with this id in source(s): Microsoft Visual Studio Offline Packages

Package "dotnet-format" failed to restore, due to Microsoft.DotNet.ToolPackage.ToolPackageException: The tool package could not be restored.
   at Microsoft.DotNet.Tools.Tool.Install.ProjectRestorer.Restore(FilePath project, PackageLocation packageLocation, String verbosity)
   at Microsoft.DotNet.ToolPackage.ToolPackageInstaller.InstallPackageToExternalManagedLocation(PackageLocation packageLocation, PackageId packageId, VersionRange versionRange, String targetFramework, String verbosity)
   at Microsoft.DotNet.Tools.Tool.Restore.ToolRestoreCommand.InstallPackages(ToolManifestPackage package, Nullable`1 configFile)

Restore failed.
Write-Error: D:\a\dotnet-serve\dotnet-serve\build.ps1:23

It seems this is a known issue, and there is a workaround: actions/setup-dotnet#155 (comment)

I can try to fix it in another PR if you wish...

[natemcmaster]

Removing .NET Core 2 can be done separately. I would be okay taking that change first before adding Yarp later this year when it releases a stable version

[natemcmaster]

.NET Core 2 has been dropped, btw...

[natemcmaster]

#98

[mlaily]

Hello, thanks for the heads-up!
I also see that YARP 1.0 has shipped a few days ago.
I will see about updating my PR...

[mlaily]

The PR should be ok to review now.

[natemcmaster]

Did you consider taking advantage of the APIs that YARP already provides for managing HTTP clients and forwarding? From reading the sample in https://github.com/microsoft/reverse-proxy/tree/main/samples/BasicYarpSample and https://microsoft.github.io/reverse-proxy/articles/config-providers.html, I got the impression that the code would look much more like this:

if (_options.HasReverseProxyMappings)
{
               // Enable endpoint routing, required for the reverse proxy
            app.UseRouting();
            // Register the reverse proxy routes
            app.UseEndpoints(endpoints =>
            {
                endpoints.MapReverseProxy();
            });
}

And then we could implement something like https://github.com/microsoft/reverse-proxy/blob/main/samples/ReverseProxy.Code.Sample/InMemoryConfigProvider.cs (see also https://microsoft.github.io/reverse-proxy/articles/config-providers.html).

Thoughts?

[mlaily]

Did you consider taking advantage of the APIs that YARP already provides for managing HTTP clients and forwarding?

The sample you linked to is short because all the proxy configuration is read from a config file with a structure controlled and known by YARP.
The downside is that we would have to fit into that (relatively complex) structure, even though our use case is much simpler.

The in-memory config example is even more complex since we have to maintain both the in-memory configuration provider implementation and the actual proxy configuration in Startup.cs using this provider. The nice endpoints.MapReverseProxy(); method does not work without first creating the in-memory configuration, which is done here: https://github.com/microsoft/reverse-proxy/blob/main/samples/ReverseProxy.Code.Sample/Startup.cs#L61-L98

I instead followed the Direct Forwarding approach that seems to be more appropriate for our use case (we currently have no use for advanced features like clustering and load balancing).
The corresponding sample is here: https://github.com/microsoft/reverse-proxy/blob/main/samples/ReverseProxy.Direct.Sample/Startup.cs (which has more code because it demonstrates all the available features of this approach, but the gist of it is very similar to what I did)

I think starting with a simple IHttpForwarder is more appropriate for now, and we can always switch to a custom in-memory configuration provider in the future if we want to expose all the advanced features YARP supports.

[natemcmaster]

Ok, thanks for the explanation. Let's give this a try! Thanks for the contribution 😄