Hi Bertrand,
On Sun, Mar 23, 2014 at 04:18:44PM +0100, Bertrand Jacquin wrote:
> Hi,
>
> I did this patch for dev19 some time ago but I am still not sure whether
> it is the best way to do it or not, and did not have the time to discuss
> it since. As the latest changes broke it and forced me to rebase it, and
> it's very useful for us, I'd like to propose it for inclusion before the
> final release if you think it's OK, or to discuss how it should be done.
Great!
> Main purpose wanted to achieve is it be able to use many backends
> without the need to declare each routing process from frontend to
> backend and instead use generic and dynamic switching when a sane
> parameter can be used from user request using the logformat logic. For
> example when we have a backend farm dedicated to each 'Host: ' http-header,
> it's pain in the ass to have to declare the backend and the relevant
> use_backend.
Yes I know there's this request coming from time to time. In fact it
was even planned to work like this before we finally went with ACLs
and use_backend, but we felt it would be a too limited design (eg: no
choice of other routing key).
> With the proposed solution, you first need to declare a dynamic
> use_backend as the following :
>
> use_backend bk_cust_%[hdr(Host)] if { hdr(Host) -m found }
>
> And then to declare the needed backend. For every new vhost hosted will only
> need to add the backend section to the configuration.
I'm not opposed to the feature at all, in fact I've even been involved
in a discussion about something more or less in this vein recently. But
I'm having some fears about the use of the %[] form in a use_backend
directive. Indeed, this string format was initially done only for
logformat. Then it was adopted for unique-id. Then for all http-request
directives. And we start to see from time to time people trying to use
it in places which have no relation with it (eg: in ACL declaration).
I'm seeing several solutions in fact :
- yours above
- append some argument to use_backend to indicate that it's a logformat
string or a dynamic backend (eg: use_backend -d foo%[bar]), but "-d"
might be a valid backend name, so ...
- have a different directive name (eg: use_backend_dyn or use_backend_lf),
but that might increase the confusion for some users who will not
necessarily know that they're part of the same ruleset.
- put use_backend in http-request rules and clearly state that only
http-request can use logformat, but then it means that we can't do
it on TCP, and it can further confuse users who will try to chain
multiple backends using http-request rules in backends.
So in the end, I tend to think that your solution might still be the best
one, or the least confusing one. But I'd like to read other people's comments
about this, maybe someone has a better idea.
> More detailed usage and implementation in patch itself.
>
> It was rebased on commit 0e9b1b4d1f0efc5e46a10371d9be21e97581faab.
OK thanks!
> A current limitation of that patch can be that if a dynamic use_backend
> is evaluated and the named backend is not found, the default_backend is
> then used. This is not a need we have, but some may complain about it.
Well, *all* use_backend rules are final today. So if the condition after
use_backend is true, then the rule is executed and the evaluation is
stopped. So I think this is the proper behaviour. Doing it differently
could instead increase confusion, because if we changed the way it works,
some people might then complain for example that when use_backend directs
to a backend whose all servers are dead, we ought to go back to evaluate
all the rules instead.
Also I could easily see some security issues by not having this behaviour,
because is multiple dynamic rules are chained and it is not at all expected
that someone can reach someone else's backend in a multi-hosted environment,
we could end up still mixing the sets.
It could be argued that defaulting to default_backend is not logical however
and that instead we should return a 503. But the current design allows both
behaviours if I understand it right, if you don't want to have a
default_backend, by not setting one you'll get the 503.
One step further would be to make the condition optional on use_backend,
as it is on redirect for example.
> Future enhancement could be to use the same logic for use-server and
> many other part such as reqadd..
OK for use-server, but NAK for req*. These ones have been used for many
years in legacy configs and we would definitely break them by doing so.
The problem is the % character which is used a lot in HTTP and has a lot
of chances of being already present in some configs. That's exactly the
same reason why we support logformat in "http-request redirect" and not
in "redirect". One is compatible with deployed configs and the other one
is new and supports extensions.
The patch looks fine. I'm tempted to merge it as-is, but I'd like to gather
some opinions first in case we'd have to change some behaviour or syntax.
In the absence of comments, I'll merge it soon.
Thanks!
Willy
