Re: Some filter discussion for the future

2023-10-17 Thread Tristan

Hi Aleksandar,

That is a welcome follow-up to the tangent we went on in the announce 
thread.


As there was the discussion about the future of the SPOE filter, let me 
start a discussion about some possible filter options.


[...]

The question which I have is how difficult is it to add a http filter 
based on httpclient similar to SPOE or FCGI filter.


Another option is to add some language specific filter like 
haproxy-rs-api shown in this comment 
https://github.com/khvzak/mlua/issues/320#issuecomment-1762027351 .


I personally find the latter much more appealing. If only because the 
http client is "just" a much more restricted version of it.


And since I was the first (in that thread, certainly not everywhere) to 
complain about the current language of choice for extending HAProxy 
(LUA), I have to say again that a target "language" like WASM sounds 
like an ideal selection:

- no need to pick/enforce/encourage a specific input language
- plenty of languages already compile to it, and likely to continue 
trending up since browsers support it


The Idea to add the http filter is that there are so many http based 
tools out there and with that could HAProxy use such tools based on http.


That is true, but needing an HTTP API + the loss in efficiency sounds a 
bit painful.

And very painful if the response isn't so easy to parse.
Thinking of cases where XML decoding becomes relevant, for example 
SAML-related ones which are common for auth-related matters still.



Any opinion on that?


Well on my end I certainly want to see this too. That said Willy had a 
few counterpoints of relevance in that other thread that are worth 
addressing here:


> WASM on the other hand would provide more performance and compile-time
> checks but I fear that it could also bring new classes of issues such
> as higher memory usage, higher latencies, and would make it less
> convenient to deploy updates since these would require to be rebuilt.

I'd say first that there are interpreters (and JITs) so the rebuild is 
not necessary.
However, even if it was, I'm not sure that the buildless use-case has 
that much traction as long as the build doesn't have to happen on the 
LBs directly.
For example I don't remember seeing complaints that SPOEs essentially 
require a build step.


> Also we don't want to put too much of the application into the load 
balancer.


That's a much more fundamental question however. This is your project, 
not mine, so your call.


But I have to emphasize that one reason I use HAProxy is specifically 
because it's extremely configurable and allows me to offload a lot of 
application-related logic directly at the edge.


In a more impersonal way, that is also a direction many are interested 
in in general. See things like 
https://blog.cloudflare.com/cloudflare-snippets-alpha/ which are 
essentially ACL-triggered filters in HAProxy terms.


One example case I see up and again is tee-ing a request, for various 
reasons:

- for silent A/B testing between 2 backends (ie tee to 1 control and 1 test)
- for routing the request that triggers a cached response both to the 
cache and to something interested in it for statistics; so users gets 
fast response and you still ALSO get to count those requests


And of course that has concerns related to memory used for buffering the 
content if there are 2 targets and thus you can't purely stream through. 
But in some places it has applicability I think.


> But as I said I haven't had a look at the details so I don't know
> if we can yield like in Lua, implement our own bindings for internal
> functions, or limit the memory usage per request being processed.

That is much more difficult for me to answer, so to save you some time 
these seem to be the 3 main C-embeddable runtimes at the time of writing:

- https://github.com/bytecodealliance/wasm-micro-runtime
- https://github.com/wasm3/wasm3
- https://github.com/wasmerio/wasmer

I had a look and however didn't see a way to control memory or force 
yielding... so it's not encouraging. But maybe I missed it.


> During the Lua integration we used to say that it would teach us
> new use cases that we're not aware of and that could ultimately
> end up as native actions/sample fetches/converters for some of them
> if they were popular.

I fully get that, and I think it wouldn't really change either way. For 
example if it didn't exist yet, a cache like the native one would be 
something to show up for sure.


But there are still quite a few more site-specific things that have no 
chance of ever making it in mainline (and that's a good thing) but also 
become more complex than a "just a few lines".
As in you COULD write it in 20 instructions yes, but the source being 
150 would make it more readable even if in the end the actual amount of 
executed instructions remains 20.
And having a decent developer experience while doing that is quite 
helpful rather than randomly tweaking things around until it doesn't 503 
anymo

Re: [ANNOUNCE] haproxy-2.9-dev7

2023-10-17 Thread Tim Düsterhus

Hi

On 10/11/23 16:05, Willy Tarreau wrote:

No, I remember Tim raised this point a while ago basically saying "hey
don't break the DNS I use it for my servers". For me simple server


For reference, you're probably thinking of this email:

https://www.mail-archive.com/haproxy@formilux.org/msg42026.html

Best regards
Tim Düsterhus



How to limit client body/upload size?

2023-10-17 Thread Gilles Van Vlasselaer
Hi, we are currently migrating servers and decided to drop NGINX in 
favour of HAProxy, however we had issues in the past where people would 
bomb us with massive file uploads on some services. Is there an 
equivalent like nginx's 'client_max_body_size' directive?


Thanks in advance,

Gilles Van Vlasselaer


RE: [PATCH 0/4] Support server-side sending and forwarding of arbitrary PPv2 TLVs

2023-10-17 Thread Stephan, Alexander
Hi Willy,

Do you know whether this can/will make it to the next release? It would be 
crucial for us to know.

Best,
Alexander

-Original Message-
From: Willy Tarreau  
Sent: Thursday, October 5, 2023 2:42 PM
To: Stephan, Alexander 
Cc: haproxy@formilux.org
Subject: Re: [PATCH 0/4] Support server-side sending and forwarding of 
arbitrary PPv2 TLVs

Hi Alexander,

On Thu, Oct 05, 2023 at 11:13:16AM +, Stephan, Alexander wrote:
> Hi Willy,
> 
> Ah, what a pity. Anyway, I sent them again with you in CC. Does it look 
> alright now?

Yep, received both ways this time, thank you!
Willy


Re: [PATCH 0/4] Support server-side sending and forwarding of arbitrary PPv2 TLVs

2023-10-17 Thread Willy Tarreau
Hi Alexander,

On Tue, Oct 17, 2023 at 05:38:45PM +, Stephan, Alexander wrote:
> Hi Willy,
> 
> Do you know whether this can/will make it to the next release? It would be 
> crucial for us to know.

I sincerely want it to, but the last annoyance around H2 etc derailed
our activities a bit and I'm still trying to catch up on plenty of
things that others depend on :-/

I'm still having your series in my todo-list and do intend to review it.
I also know that if tiny adaptations were needed you don't mind so we'd
save a round trip anyway.

I'll keep you updated, just trying to do my best :-(

Willy



[PATCH] MINOR: lua: Add a flag to disable logging to stderr

2023-10-17 Thread Tristan

By default, messages printed from LUA log functions are sent both to
the configured log target and additionally to stderr (in most cases).
This introduces tune.lua.also-log-to-stderr for disabling that
second copy of the message being sent to stderr.

Addresses https://github.com/haproxy/haproxy/issues/2316

This could be backported if wanted, since it preserves the behaviour
that existed prior to it.
---
 doc/configuration.txt |  6 ++
 doc/lua.txt   |  4 
 src/hlua.c| 50 +--
 3 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 88a576795..771a569c0 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -1195,6 +1195,7 @@ The following keywords are supported in the 
"global" section :

- tune.lua.service-timeout
- tune.lua.session-timeout
- tune.lua.task-timeout
+   - tune.lua.also-log-to-stderr
- tune.max-checks-per-thread
- tune.maxaccept
- tune.maxpollevents
@@ -3180,6 +3181,11 @@ tune.lua.task-timeout 
   remain alive during of the lifetime of HAProxy. For example, a task 
used to

   check servers.

+tune.lua.also-log-to-stderr { on | off }
+  Enables ('on') or disables ('off') logging the output of lua log 
functions
+  to stderr in addition to the configured log target. To preserve 
historical

+  behaviour, this defaults to 'on'.
+
 tune.max-checks-per-thread 
   Sets the number of active checks per thread above which a thread will
   actively try to search a less loaded thread to run the health check, or
diff --git a/doc/lua.txt b/doc/lua.txt
index 8d5561668..5e5712938 100644
--- a/doc/lua.txt
+++ b/doc/lua.txt
@@ -630,6 +630,10 @@ It displays a log during the HAProxy startup:

[alert] 285/083533 (14465) : Hello World !

+Note: By default, logs created from a LUA script are printed to the log 
target

+in your configuration and additionally to stderr, unless the flag
+tune.lua.also-log-to-stderr is set to 'off'.
+
 Default path and libraries
 --

diff --git a/src/hlua.c b/src/hlua.c
index c686f222a..261aee763 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -69,6 +69,12 @@
 #include 
 #include 

+/* Global LUA on/off flags */
+/* if on, LUA-originating logs are duplicated to stderr */
+#define HLUA_TUNE_ALSO_LOG_TO_STDERR (1<<0)
+
+static int hlua_tune_flags = HLUA_TUNE_ALSO_LOG_TO_STDERR;
+
 /* Lua uses longjmp to perform yield or throwing errors. This
  * macro is used only for identifying the function that can
  * not return because a longjmp is executed.
@@ -1366,8 +1372,9 @@ const char *hlua_show_current_location(const char 
*pfx)

return NULL;
 }

-/* This function is used to send logs. It try to send on screen (stderr)
- * and on the default syslog server.
+/* This function is used to send logs. It tries to send them to:
+ * - the log target applicable in the current context, AND
+ * - stderr if not in quiet mode or explicitly disabled
  */
 static inline void hlua_sendlog(struct proxy *px, int level, const 
char *msg)

 {
@@ -1394,6 +1401,9 @@ static inline void hlua_sendlog(struct proxy *px, 
int level, const char *msg)


send_log(px, level, "%s\n", trash.area);
 	if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE | 
MODE_STARTING))) {

+ if (!(hlua_tune_flags & (HLUA_TUNE_ALSO_LOG_TO_STDERR)))
+   return;
+
if (level == LOG_DEBUG && !(global.mode & MODE_DEBUG))
return;

@@ -12433,6 +12443,23 @@ static int hlua_parse_maxmem(char **args, int 
section_type, struct proxy *curpx,

return 0;
 }

+static int hlua_also_log_to_stderr(char **args, int section_type, 
struct proxy *curpx,
+   const struct proxy *defpx, const 
char *file, int line,

+   char **err)
+{
+   if (too_many_args(1, args, err, NULL))
+   return -1;
+
+   if (strcmp(args[1], "on") == 0)
+   hlua_tune_flags |= HLUA_TUNE_ALSO_LOG_TO_STDERR;
+   else if (strcmp(args[1], "off") == 0)
+   hlua_tune_flags &= ~HLUA_TUNE_ALSO_LOG_TO_STDERR;
+   else {
+		memprintf(err, "'%s' expects either 'on' or 'off' but got '%s'.", 
args[0], args[1]);

+   return -1;
+   }
+   return 0;
+}

 /* This function is called by the main configuration key "lua-load". 
It loads and
  * execute an lua file during the parsing of the HAProxy configuration 
file. It is
@@ -12673,15 +12700,16 @@ static int hlua_config_prepend_path(char 
**args, int section_type, struct proxy


 /* configuration keywords declaration */
 static struct cfg_kw_list cfg_kws = {{ },{
-   { CFG_GLOBAL, "lua-prepend-path", hlua_config_prepend_path },
-   { CFG_GLOBAL, "lua-load", hlua_load },
-   { CFG_GLOBAL, "lua-load-per-thread",  hlua_load_per_thread },
-   { CFG_GLOBAL, "tune.lua.session-timeout", hlua_session_timeout },
- 

subscribe

2023-10-17 Thread Jerry Scharf (he/him/his)
subscribe

-- 
Jerry Scharf
Pure Storage


Re: [PATCH] MINOR: lua: Add a flag to disable logging to stderr

2023-10-17 Thread Willy Tarreau
Hi Tristan,

On Tue, Oct 17, 2023 at 06:19:57PM +, Tristan wrote:
> By default, messages printed from LUA log functions are sent both to
> the configured log target and additionally to stderr (in most cases).
> This introduces tune.lua.also-log-to-stderr for disabling that
> second copy of the message being sent to stderr.
> 
> Addresses https://github.com/haproxy/haproxy/issues/2316
> 
> This could be backported if wanted, since it preserves the behaviour
> that existed prior to it.

I agree with this, it has already annoyed me a few times in the past
when trying to debug something else. I have vague memories of the Lua
integration having been done with lots of fprintf() for debugging and
that finally it was cleaned up and considered good enough for a first
version. Since it was quite new by then, feedback and debugging were
expected so it was not a problem. But it has ossified into a default
behavior which is not longer desirable in my opinion.

I'm fine with the general approach, but I'm having two comments:

  - using the word "also" in the option name is really not welcome
("tune.lua.also-log-to-stderr"), and actually more confusing than
without because one might wonder "why also, where is it also sent".
Most of our options are compatible and cumulable, and not exclusive,
so it should be seen as a boolean like any other one. As such I
would just call it "tune.lua.log-stderr" and be done with it. That
may even allow you not to resize the keywords array, which could
help with backports ;-)

  - should we really stick to "on" as the default behavior in 2.9 ? I
sense that basically nobody wants that by default anymore, and if
we want to change the default, it will only be in an odd version,
hence 3.1 for the next one. Maybe now's the right moment ? Or if
the concern is to lose the messages when no logs are configured,
maybe we can have a 3rd value "auto" which would be the default
and which would only log to stderr if there's no other logger ? I
don't know if we have this info where it's used, though. Hmmm at
first glance we seem to have it by testing if either px->loggers
is non-empty or global.loggers is non-empty. Thus it could be the
nicest approach, having "on" by default in 2.8 and older and "auto"
on 2.9 maybe ?

A few comments on the patch:

> diff --git a/src/hlua.c b/src/hlua.c
> index c686f222a..261aee763 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -69,6 +69,12 @@
>  #include 
>  #include 
> 
> +/* Global LUA on/off flags */
> +/* if on, LUA-originating logs are duplicated to stderr */
> +#define HLUA_TUNE_ALSO_LOG_TO_STDERR (1<<0)


Please leave several spaces between the name and the value so that other
names do not require to realign everything. For settings, it tends to be
more convenient to use hex values (padded left, e.g. 0x0001) because
it allows masks and combined values to be represented as well in a visual
way, something that's basically unreadable when dealing with shifts. If
we implement "off", "auto", "on" here it will already be the case.

> +
> +static int hlua_tune_flags = HLUA_TUNE_ALSO_LOG_TO_STDERR;

When you're using such arrays of flags, please precede them with a short
comment saying "flags made of HLUA_TUNE_*" or something like this so that
if it ever grows and more stuff gets inserted in between, it remains easy
to figure (that's one issue that has affected all other ones over time).

Also for bit fields, I'd prefer to use an unsigned int (we have "uint"
as a short form) so that when you see them in gdb you don't get negative
values that are even more annoying to copy-paste and decode.

> +static int hlua_also_log_to_stderr(char **args, int section_type, struct 
> proxy *curpx,
> +   const struct proxy *defpx, const char 
> *file, int line,
> +   char **err)

It's not obvious at all that this function is there to parse a keyword,
I'm seeing it as the one which will actually log. Almost all of our
keyword parsing functions (others historically have "check"). I'm
seeing that it's not the case for the majority of the historic Lua
functions, but this should not be a reason for not fixing it (and
maxmem was fixed already). Better call it "hlua_parse_log_stderr" or
something like this that reminds the keyword.

Please have a look at these points (or comment if you disagree), and
I'll happily merge it!

Thanks,
Willy