-
Notifications
You must be signed in to change notification settings - Fork 835
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
Config-based request and response header transformation #21
Comments
I'd like us to start expanding on this feature a bit (ping @Tratcher @halter73). I'm wary of setting up a "language", but we could have a list of transformations: {
"ReverseProxy": {
"Routes": [
{
"RouteId": "blah",
"Match": {
"Path": "/api/v1/myservice/{**catchAll}"
},
"Transform": [
{ "Transformation": "StripPrefix", "Prefix": "/api/v1/myservice" },
{ "Transformation": "AddPrefix", "Prefix": "/apis" }
{ "Transformation": "AddHeader", "Headers": { "X-MyHeader": "myVal" } }
{ "Transformation": "StripHeader", "Headers": [ "X-SomeUnhelpfulIncomingHeader" ] }
]
}
]
}
} That would transform an incoming request like this:
Into an outgoing request:
(with other headers elided) Basically the idea is that the "Proxy" component copies everything from the incoming request to the outgoing response (all the headers, path, etc.) and then run the transformations over the outgoing response. There are some problems here:
|
The verbosity could be reduced:
|
This is a space with a lot of prior art, we should do some comparisons. |
@davidni I suspect I know the reason, but why is this driven by config rather than code? How often are routes created in your world that need this kind of capability? |
Altering the path is very common, adding or stripping a prefix, and this would be specific per route. Adding and removing headers is also common, usually headers directly related to the proxy activity like x-forwarded-. I'd expect us to provide built in support for things like x-forwarded- so manually adding them per route is less of a concern. Again, this is a case where simple common operations should be possible via config, and advanced ones via code. |
I will work on this issue if it's ok. |
@Kahbazi it needs some design first. You're welcome to make some proposals on this issue. |
I have implemented most of these. I could create a draft PR to discuss there if needed. These classes are for configs public abstract class Transform {}
public class AddHeaderTransform : Transform {string Key; string Value;}
public class AddPrefixTransform : Transform {string Prefix;}
public class StripPrefixTransform : Transform {string Prefix;}
public class StripHeaderTransform : Transform {string Key;} Add a list of public sealed class ProxyRoute
{
public IReadOnlyList<Transform> Transform { get; set; }
} This could be in config and and since it can't be directly bind to
Introduce a new public class UpStreamRequest
{
public string Method { get; set; }
public Stream Body { get; set; }
public Uri Uri { get; set; }
public string Protocol { get; set; }
public IHeaderDictionary Headers { get; set; }
} These classes are for runtime models public abstract class Transformer
{
public abstract ValueTask Transform(UpStreamRequest upStreamRequest);
}
public class StripPrefixTransformer : Transformer {}
public class StripHeaderTransformer : Transformer {}
public class AddPrefixTransformer : Transformer {}
public class AddHeaderTransformer : Transformer {} Add a list of internal sealed class RouteConfig
{
public IReadOnlyList<Transformer> Transformers { get; }
} Use public class TransformerFactory : ITransformerFactory
{
public virtual Transformer Create(Transform transform)
{
return transform switch
{
StripHeaderTransform t => new StripHeaderTransformer(t.Key),
AddHeaderTransform t => new AddHeaderTransformer(t.Key, t.Value),
StripPrefixTransform t => new StripPrefixTransformer(t.Prefix),
AddPrefixTransform t => new AddPrefixTransformer(t.Prefix),
_ => throw new NotImplementedException()
};
}
} Create a |
This I expect is the hardest part of the design: When to do the transformations and on what object model.
This results in copying the information multiple times. How about this? Do the transformations in HttpProxy.ProxyAsync in the process of building the HttpRequestMessage.
|
Do you mean that you prefer we use configuration binding directly to get transform instances?
So do we need a class which holds theses parts and pass it to
Does it mean we need to have multiple abstraction for transformers like |
I wasn't trying to imply a specific design with this point, just outlining principals.
Probably? If the transformation were a single step we could avoid the intermediate object and just pass in the HttpContext and proxy data, but having multiple granular transformations means we need intermediate state of some kind.
It seems like there's a hierarchy of transformation types (transform, uri transform, path transform, header transform, header add transform, version, method, etc.). And yes, every field of the HttpRequestMessage/HttpContent should be customizable in some way. That doesn't mean all of those need to be exposed as config based transformations, only the common ones (headers, url). Really advanced customizations could require plugging in a DelegatingHandler. Intermediate ones could be defined in code. |
Clarification: When adding a header it's important to specify if this is an append or replace operation. You end up with two transformations: |
I can think of another header transformation that gets the value from an incoming header and send it with via another header. We could use this in out current system. |
When would you want to append? |
#133 "x-forwarded-*" headers append. |
Those aren't headers you'd add statically in config though. You might switch it on and off. |
That's an area of high customization, so even if we had x-forwarded-proto built in people would still probably add ones we didn't support natively. |
Would be nice to have transformations for: full path rewrite ignoring the structure of original path, given we might have the tokens already parsed(no need to parse it again). Drop all headers: Move parameters from path to headers: Alternatively we should be able to add middleware executing before a request is submitted to backend server, so we could write even more complex transformations. One final question I have is how the ordering of transformations will affect the output? Is the output of one transformation passed to next one or they will just apply transformations to a request object. |
Nginx comparisons
|
I've done some sketching. Note this is focused on in-code transformations, but I'm putting it in this thread as this is where all the interesting discussion is happening and the config will need to build on top of this. RequestI see two distinct types of transformations on the request: headers and everything else. I'm excluding Body transformations, those require a whole different level of infrastructure (e.g. custom middleware). Request Parameters Transform - Applied before building the HttpRequestMessage
Headers
ResponseResponses consist of status, headers, and body, and only headers are in scope for transformation. HeadersTransform - Same as request, applied during the copy loop
Trailers - Same as headers, different collection
App structure
|
You could draw some inspiration from Azure Function proxy and Azure Api Management on how they handle transformations. |
Maybe this is already the intended design, but I'd like to request that any SetHeader transformations skip the filters defined by _headersToSkipGoingUpstream (applied here). At the moment, it's defined so that the Some applications, such as grocy, simply reflect the I see that the proxy passes an X-Forwarded-Host header by default, but unfortunately some applications don't seem to support it. |
@nemec absolutely. We've had similar feedback elsewhere that people need to flow or be able to set the Host header. |
The design behind ProxyKit was really nice for extensibility and building upstream HttpRequestMessage - just copy most of that and performance optimize it. No reason to invent the wheel again? I wrote a reverse proxy configuration format ontop of proxykit that was able to read : https://gist.github.com/pksorensen/5a0be2b57839ad8a89fe9509a9317aa9 Very similar to your cluster/routes concept. I was very inspired by Nginx and implemented there routing format. So since this project kinda killed proxy kit, i hope that i will be able to swap proxykit out with yarp. |
Comments on closed issues are not tracked, please open a new issue with the details for your scenario. |
In Island Gateway we implemented the notion of
Transformations
, which takes inspiration from Traefik's middlewares (though we're not calling them middlewares for obvious reasons). See e.g. Traefik's StripPrefix.A user can specify e.g.
Transformations=StripPrefix('/api'), AddPrefix('/v2')
, which results in the listed processing steps being applied sequentially at runtime.The ability to specify simple transformations like these through config is a requirement for us.
Example of a route using transformations:
(this is a feature we already implemented in Island Gateway, filing here as requested to track)
The text was updated successfully, but these errors were encountered: