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

Proxy supports static config files, with hot reload for handling changes #9

Closed
analogrelay opened this issue Mar 12, 2020 · 12 comments
Closed
Assignees
Labels
Priority:0 Used for divisional .NET planning Type: Enhancement New feature or request User Story Used for divisional .NET planning
Projects

Comments

@analogrelay
Copy link
Contributor

Config based proxies are common and we'll need to support at least basic proxy scenarios from config. Here are some initial considerations:

  • Define routes based on host and/or path
  • A restart should not be needed to pick up config changes
  • You should be able to augment a route with code/middleware. Kestrel does something similar using named endpoints.
  • List multiple back-ends per route for load balancing
  • Configure a load balancing strategy
@analogrelay analogrelay added the Type: Enhancement New feature or request label Mar 12, 2020
@analogrelay analogrelay added this to To Do in Active Work via automation Mar 19, 2020
@analogrelay
Copy link
Contributor Author

  • You should be able to augment a route with code/middleware. Kestrel does something similar using named endpoints.

We also need to think about interleaving here. In theory you want to be able to have a pipeline that looks like this (for example):

[Built-in: Rewrite header] -> Custom Code Middleware -> [Built-in: Rewrite path] -> ...

That makes me wonder if we should be defining these middleware (both our own, and custom ones) as "filters" in the same kind of way Envoy does. Basically, named middleware registered in DI that you can reference in the config file. Then the pipeline is built up by pulling the named middleware out of DI and building up the structure.

@analogrelay
Copy link
Contributor Author

  • List multiple back-ends per route for load balancing
  • Configure a load balancing strategy

I like the way Envoy (and the IslandGateway prototype) does this by defining named "clusters" and just having routes select the appropriate one. I also like this for load balancing. Even if you want the same physical backends with a different load balancing strategy for certain routes you can achieve that by creating two separate "backends" in the config, with the same IPs and different load balancing config.


I wonder if the way we handle the middleware pipeline config complexity is by allowing both Backends and Routes to have separate middleware pipelines that we stitch together based on config. We talked about load balancing as a middleware process, so if Backends and Routes are defined separately, that gets messy unless we give each a separate middleware pipeline.

@Tratcher Tratcher self-assigned this Mar 19, 2020
@samsp-msft
Copy link
Contributor

samsp-msft commented Mar 27, 2020

  • You should be able to augment a route with code/middleware. Kestrel does something similar using named endpoints.

We also need to think about interleaving here. In theory you want to be able to have a pipeline that looks like this (for example):

[Built-in: Rewrite header] -> Custom Code Middleware -> [Built-in: Rewrite path] -> ...

That makes me wonder if we should be defining these middleware (both our own, and custom ones) as "filters" in the same kind of way Envoy does. Basically, named middleware registered in DI that you can reference in the config file. Then the pipeline is built up by pulling the named middleware out of DI and building up the structure.

My gut feel is that the should be able to define pipelines in code kind of like you do in the Configure method:

proxy.CreatePipeline("default", (app, env) =>
{
    app.UseResponseCompression(...);
});

and then when you create a route, you can specify which pipeline is used by name. The route should then be able provide configuration properties for the middleware referenced in the pipeline. This would enable different routes to have different behaviors, but those behaviors are still put together in code without the need for reflection or DI (which cause problems for tree-shaken single file deployments).

@analogrelay
Copy link
Contributor Author

That might be a reasonable way to do things when you need to interleave like that. Another thing I thought of was defining named middleware in code:

services.AddFilterMiddleware("myfunkything", (context, next) => { ... });

And then in the config we could support defining filter chains like Envoy and you can specify your custom middleware there:

{
	"Route": "/foo/bar",
	"Filters": [
		{ "Name": "SomeBuiltInFilter" },
		{ "Name": "myfunkything" },
		{ "Nmae": "SomeOtherBuiltInFilter" }
	]	
}

@Tratcher
Copy link
Member

Activating middleware from config is not a hole we want to dig ourselves into. Keep in mind that the middleware will also need to have its options configured.

@samsp-msft's example matches my expectations, that level of customization would only be available via code.

@analogrelay
Copy link
Contributor Author

The biggest concern I have is the deployment burden. Obviously, a redeployment is needed to introduce new code, but I don't think it should be necessary to redeploy in order to change the "chain" of actions being performed. Envoy doesn't require that, for example.

Named pipelines gets you some of the way there, but if you ever want to change the body of a named pipeline you're going to have to redeploy. That concerns me.

@Tratcher
Copy link
Member

Isn't one of our selling points that code is more powerful and there if you need it? That comes with the re-depoyment trade off baked in.

Yes, the basic scenarios should work directly from config, but customized pipelines per route is not a basic scenario.

@Tratcher
Copy link
Member

Note to self: The config reload code is currently implemented in the sample project. That needs to move to a product assembly.

@samsp-msft
Copy link
Contributor

samsp-msft commented Mar 31, 2020

Changing the sequence of actions is not a "configuration change" in the way I think of configuration. To me config changes are about adding new routes, new end points, changing clustering behavior. Changing which middleware components are in a pipeline, or their order is something that should go through a testing cycle, and so can be part of re-deployment rather than config. Config I am expecting to change in the order of seconds, this is a once a week or so kind of thing.

@analogrelay
Copy link
Contributor Author

Another angle of looking at this: Are there scenarios where the proxy deployment is managed by one team (a core infrastructure team) but the active "filters" (and thus the proxy configuration) for each route need to be maintained by other teams (the individual service teams). That sounds like the scenario @davidni is in.

In that scenario, having to drop to code just to enable custom components is going to be problematic. It would seem to be better if the "infrastructure" team in this scenario can add new components to the proxy and roll out a new version, then have the service teams enable them by deploying configuration changes as they choose.

@Tratcher
Copy link
Member

Tratcher commented May 1, 2020

Feedback from #47 (comment)

Without looking through the implementation, it's not clear why the Action takes a string. It looks like it's for the Backend id. Would it be better to put an id property on Backend similar to what we do for ProxyRoute?

This is a reflection of the asymmetry we have in the config structures. Backends is a set of named objects (with the names on the outside), but routes are an array of objects where the name is a field. There was some concern that routes needed to be specifically ordered because of how route matching works, but backends weren't considered ordered and named objects made the config cleaner. We haven't yet decided if consistency is needed here.

"Backends": {
      "backend1": {
        "LoadBalancing": {
          "Mode": "Random"
        },
        "Endpoints": {
          "backend1/endpoint1": {
            "Address": "https://localhost:10000/"
          },
          "backend1/endpoint2": {
            "Address": "http://localhost:10010/"
          }
        }
      },
      "backend2": {
        "Endpoints": {
          "backend2/endpoint1": {
            "Address": "https://localhost:10001/"
          }
        }
      }
    },
    "Routes": [
      {
        "RouteId": "backend1/route1",
        "BackendId": "backend1",
        "Match": {
          "Methods": [ "GET", "POST" ],
          "Host": "localhost",
          "Path": "/{**catchall}"
        }
      }
    ]

Tratcher added a commit that referenced this issue May 5, 2020
Tratcher added a commit that referenced this issue May 5, 2020
@Tratcher
Copy link
Member

Tratcher commented May 5, 2020

Discussion notes from the above feedback: We can keep the config structure as is and update the Backend and Endpoint classes to have a field for their IDs so it's more consistent with Routes when you're working with them in code. We just have to do a little fixup after the default config binding to set the id fields.

That should be the last item to address in this config epic.

@karelz karelz moved this from 1.0.0-preview1 to 1.0.0-preview2 in YARP Planning May 7, 2020
@karelz karelz modified the milestones: 1.0.0-preview1, 1.0.0 May 7, 2020
Tratcher added a commit that referenced this issue May 12, 2020
Tratcher added a commit that referenced this issue May 12, 2020
@Tratcher Tratcher moved this from 1.0.0-preview2 to 1.0.0-preview1 in YARP Planning May 12, 2020
@Tratcher Tratcher modified the milestones: 1.0.0, 1.0.0-preview1 May 12, 2020
Active Work automation moved this from Working to Done May 12, 2020
@samsp-msft samsp-msft added the User Story Used for divisional .NET planning label Oct 21, 2020
@samsp-msft samsp-msft changed the title Design proxy route config format Proxy supports static config files, with hot reload for handling changes Oct 21, 2020
@samsp-msft samsp-msft added the Priority:0 Used for divisional .NET planning label Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:0 Used for divisional .NET planning Type: Enhancement New feature or request User Story Used for divisional .NET planning
Projects
No open projects
Active Work
  
Done
Development

No branches or pull requests

4 participants