Add Reverse Proxy (YARP) docker image to the nightly branch #6155
Conversation
benjaminpetit
force-pushed
the
add-yarp
branch
from
708e9c9 to
4d9151b
Compare
|
|
||
| # Installer image | ||
| FROM mcr.microsoft.com/azurelinux/base/core:3.0 AS installer | ||
| RUN tdnf install -y \ |
There was a problem hiding this comment.
Nit: There should be a blank line between the FROM and RUN statements. This improves the readability/scanability. It also makes the Dockerfile consistent with the others e.g. https://github.com/dotnet/dotnet-docker/blob/main/src/monitor/9.0/azurelinux-distroless/amd64/Dockerfile
|
|
||
| Tags | Dockerfile | OS Version | ||
| -----------| -------------| ------------- | ||
| 2.3.0-preview.1, 2.3-preview, 2-preview, latest | [Dockerfile](src/reverse-proxy/2.3/azurelinux-distroless/amd64/Dockerfile) | Azure Linux 3.0 |
There was a problem hiding this comment.
Do you want "Preview" images published to main or will you wait until you have an official release? If you want to merge to main, we always place the preview tags under a preview heading. It is not so critical now with only one supported version. It is more important when there is a release version(s) + preview versions. Example
Releasing a preview to main would also impact the featured tags section. Currently the featured tags gives the impression the image is a non-preview release which I don't think we would want to do if merged to main.
|
|
||
| See [documentation](https://microsoft.github.io/reverse-proxy/articles/index.html) for how to configure the image and documentation for the reverse proxy configuration. | ||
|
|
||
| The support policy can be found [here](https://github.com/microsoft/reverse-proxy/blob/main/docs/roadmap.md). |
There was a problem hiding this comment.
It feels to me that this would be better placed in the Support section.
There was a problem hiding this comment.
That'll require some conditional content in https://github.com/dotnet/dotnet-docker/blob/nightly/eng/readme-templates/Support.md?plain=1. There are variables defined at the top of that file that you can follow for a pattern to match with reverse-proxy and then condition on that.
| && rm reverse-proxy.zip | ||
|
|
||
|
|
||
| # YARP Base image |
There was a problem hiding this comment.
I was under the impression that this wasn't considered a Base image and that it was intended to be used directly. Advanced configuration user's would not be using this as a base image. If this is the case, I think 'Base' should be removed here.
|
|
||
| See [Basic YARP Sample](https://github.com/microsoft/reverse-proxy/tree/main/samples/BasicYarpSample) for a sample config to use with this container. | ||
|
|
||
| YARP expect the config file to be in `/etc/reverse-proxy.config`, and listen by default on port 5000. |
There was a problem hiding this comment.
| YARP expect the config file to be in `/etc/reverse-proxy.config`, and listen by default on port 5000. | |
| YARP expects the config file to be in `/etc/reverse-proxy.config` and listens by default on port 5000. |
|
|
||
| See [documentation](https://microsoft.github.io/reverse-proxy/articles/index.html) for how to configure the image and documentation for the reverse proxy configuration. | ||
|
|
||
| The support policy can be found [here](https://github.com/microsoft/reverse-proxy/blob/main/docs/roadmap.md). |
There was a problem hiding this comment.
That'll require some conditional content in https://github.com/dotnet/dotnet-docker/blob/nightly/eng/readme-templates/Support.md?plain=1. There are variables defined at the top of that file that you can follow for a pattern to match with reverse-proxy and then condition on that.
|
|
||
| namespace Microsoft.DotNet.Docker.Tests; | ||
|
|
||
| [Trait("Category", "reverse-proxy")] |
There was a problem hiding this comment.
You'll want to add reverse-proxy to the list of categories in the build/test script:
dotnet-docker/build-and-test.ps1
Lines 23 to 24 in ad8bbc9
Related issue: #6154