microsoft / reverse-proxy

A toolkit for developing high-performance HTTP reverse proxy applications.
https://microsoft.github.io/reverse-proxy
MIT License
8.46k stars 832 forks source link

Require that routes have matching clusters #797

Open Tratcher opened 3 years ago

Tratcher commented 3 years ago

Today routes and clusters are validated individually before officially loading the model. However, this step does not validate if every route references a real cluster. When the model is rendered later it's possible that a route will be created that has no matching cluster.

Today this is allowed and then checks are done in middleware to short circuit routes without a cluster. https://github.com/microsoft/reverse-proxy/blob/d9eeb548817364c7199acadd177f699781091a48/src/ReverseProxy/Middleware/DestinationInitializerMiddleware.cs#L32-L37

I reviewed this with @davidni and he said the expected scenarios for this (a more dynamic pipeline) never materialized.

Proposal:

Kahbazi commented 3 years ago

While we are enforcing the validation, how about to also throw an error if a Cluster has no Destination?

Tratcher commented 3 years ago

While we are enforcing the validation, how about to also throw an error if a Cluster has no Destination?

I'm not sure, destinations can be much more ephemeral. E.g. You may create the route and cluster first, and then incrementally add destinations as they come online. The system already has to handle the case where there are no healthy destinations, so having no destinations isn't much worse.

samsp-msft commented 3 years ago

From an extensibility perspective, I would prefer the validation to be done as late as possible, preferably at pipeline stages that need data. That way people can add their own stages to the pipeline that may add/remove or otherwise change the data attached to an individual request.

karelz commented 3 years ago

Triage: We should validate earlier -- we can loosen it later, when extensibility features require that.

samsp-msft commented 2 years ago

We should discuss this in the context of extensibility around cluster selection.