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

Cluster config validation does not error on a null destination address #2175

Closed
hahn-kev opened this issue Jul 4, 2023 · 2 comments · Fixed by #2184
Closed

Cluster config validation does not error on a null destination address #2175

hahn-kev opened this issue Jul 4, 2023 · 2 comments · Fixed by #2184
Labels
help wanted We will welcome a contribution Type: Bug Something isn't working
Milestone

Comments

@hahn-kev
Copy link
Contributor

hahn-kev commented Jul 4, 2023

Describe the bug

I'm trying to make sure my config is valid on startup instead of later when we try to use it. To that end I setup a simple hosted service that will get the config on start and run the endpoints and clusters through the validator and throw if there's an error. However even with a null destination address I don't get any warnings.

To Reproduce

setup a yarp with a basic route config and this as your cluster config:

"Clusters": {
  "hgWeb": {
    "Destinations": {
      "hgWebServer": {
            "Address": null
      }
    }
  },
}

validation doesn't throw an error, however when you try to use the route you obviously get an exception:

      System.ArgumentNullException: Value cannot be null. (Parameter 'destinationPrefix')
         at Yarp.ReverseProxy.Forwarder.HttpForwarder.SendAsync(HttpContext context, String destinationPrefix, HttpMessageInvoker httpClient, ForwarderReq
uestConfig requestConfig, HttpTransformer transformer, CancellationToken cancellationToken)
         at Yarp.ReverseProxy.Forwarder.ForwarderMiddleware.Invoke(HttpContext context)
         at LexSyncReverseProxy.ProxyKernel.<>c.<<MapSyncProxy>b__1_1>d.MoveNext() in C:\dev\LexBox\backend\SyncReverseProxy\ProxyKernel.cs:line 74
      --- End of stack trace from previous location ---
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
         at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)

Further technical details

  • version: 2.0.1
  • windows 11
@hahn-kev hahn-kev added the Type: Bug Something isn't working label Jul 4, 2023
@Tratcher
Copy link
Member

Tratcher commented Jul 5, 2023

We try not to be too strict about destination address validation to avoid blocking customization like UDS. That said, I don't know if there's a valid scenario for 'null' or empty ''.

@adityamandaleeka adityamandaleeka added this to the Backlog milestone Jul 6, 2023
@adityamandaleeka adityamandaleeka added the help wanted We will welcome a contribution label Jul 6, 2023
@adityamandaleeka
Copy link
Member

We would welcome a PR to add this check to the validation logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We will welcome a contribution Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants