-
Notifications
You must be signed in to change notification settings - Fork 619
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
support for adding headers #168
Comments
Do these headers have static or dynamic values, i.e. does the header value depend on the request? |
I would say this should be static only |
👍 on this, for my case static headers would also be sufficient :) |
Well, static was a lie... My usecase is |
I am also wondering if this will eventually be supported. Static would be fine on my end. Cheers |
If the headers need to be dynamic I might be able to re-use the formatting language I'm using for the access logger from #80. In this case I would need to wrap the values in |
I guess they should be configurable per route as well so this needs to become part of the |
where is this ticket in the OSS pipeline @magiconair ? :) |
@jippi there is no pipeline. Generally, I pick up things that people are asking for and for which I have an idea on how to implement it. Adding a static option to add a set of headers to all requests is a no-brainer. But is that really enough? I think I have an idea on how to approach this but for this I need to understand what you need and I'd like to ask you to think a bit outside the box of your specific use-case, if possible. I think it is reasonable to expect that people would want to modify headers per route. The main challenge with this approach is that if different services announce different options for the same route then the behavior is undefined. As long as that is documented I think I can live with that. This problem will remain for all other features that modify a route. fabio could detect these things and log them to make detection easier. Also, I'm thinking that there are three operations:
Does that look like a reasonable approach? This requires to first update the k/v parser to accept quoted values. Then I'd have to figure out which values would be needed and whether the access logging code could be re-used for that. Then I need to actually modify the headers. Question: Should the route header modification happen before or after the normal header mangling (e.g. X-Forward-Proto, ...) |
the use-cases i've had multiple times is only allowing pure forwarding of headers - by whitelisting - |
@jippi like what? fabio doesn't remove headers AFAIK, or does it? |
@magiconair: We also need this facility for modifying response headers. |
@bkmit Can you provide a concrete example of what it is you're trying to do? |
@magiconair We use nginx (and we want to get rid) for adding Access-Control-*, Public-Key-Pins headers with different values depending on route. Also I want to add X-Request-Id generated by fabio and maybe others to response for debug/investigation purpose. |
I'm looking at fabio to replace a consul aware openresty setup. |
I need to think a bit on how to express that since I think I've moved myself somewhat into a corner with the config language. For example, should the header definition be part of the
How should I translate this from the
|
What about a second tag?
|
I like the idea of add and delete (set could just be delete and add, right?), but currently my main issue is adding a static header to the request that is passed on |
Adding my use-case, as suggested in #528 I would like to remove some of the custom headers added by my load balancer to not expose their values to the registered services. In my configuration it would be enough to set value of this headers globally, but probably per-route configuration could be even better |
I'd like to be able to remove response headers as well like "Server" or "X-Powered-By" added by some servers. |
I'd like to add custom headers such as CSP headers. Those can be static headers or maybe even pulled from Consul. |
We're running into an issue where we have basic auth enabled on a fabio route (fabio doing the auth) ... but then fabio is passing that So we'd like to strip that before it hits the backend. |
@tecnobrat That sounds like a separate issue with the implementation of the basic authorization handler on routes. Fabio doesn't add that header but maybe it should strip it from being passed to the backend if it's handling the authentication. Could you open a separate issue for that? |
@leprechau The route in question doesn't have basic auth on it, but other routes for the same domain do. Which means that your browser sends it regardless if fabio requires it for that route path. For example domain.com requires basic auth, but domain.com/thing does not. However your browser sends the header to domain.com/thing, which passes it on to the backend. Happy to open a new issue, but I just want to make sure its really a separate thing. |
@tecnobrat if the route on which fabio is handling the authentication also sends the header I think that's a separate issue related to leaking Authentication headers past the point where they are authenticated. Routes where fabio does not handle authentication but you still desire header manipulation/stripping might fall here. I could also see that being something that should be handled by fabio though since it's the one sending the |
To summarize: Users want to set and remove both request and response headers. Some use-cases require the values to be dynamic (e.g. X-Request-Start). The route options would be the obvious place to configure this, since that's where other request modifications happen as well (such as stripping a path prefix), and it translates easily to Consul tags. So we would like to add four options
Removing headers can be covered with the set option and an empty value. That's what nginx does, for instance. Reusing the syntax for access logs would be nice, but it somewhat collides with the options syntax and makes quoting difficult. That being said, I don't think I have ever seen a literal single quote appear in an HTTP header. Perhaps it is acceptable to start with the limitation that single quotes are not supported.
This would
parseOpts would become significantly more complex because it will have to deal with single-quoted values. Empty values are not supported and, as mentioned before, neither are literal single quotes in header values. Is this agreeable? |
It'd be amazing 😅 |
What's the status of this please? I'm just setting up a fabio instance that's backed by Nomad + Consul, and I need to be able to set a custom HTTP header for a given route. Reading this through, it sounds like there are multiple sets of http header-related features here. Perhaps this could be made more manageable by breaking it down into multiple stages? |
What's the status of this please? I cant use fabio for web gRPC requests as they get blocked by cors allow origin, so being able to add the headers would great! |
Any progress? Support CORS Header? |
Yes! My use case: We deploy services via nomad, so it would be great to be able to add custom headers so that when we inspect the the headers of a request (say one of the instances of a server is not behaving as expected) we can see via the custom headers which instance of that service it's coming from, rather than have to go in and tail all the logs of all the services to see which one is having issues with certain requests as a way to simply identify which node is having issues. |
Kind of like #110 but for arbitrary headers like HSTS or HPKP, etc.
The text was updated successfully, but these errors were encountered: