Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
❦ 9 janvier 2020 20:07 +01, Tim Düsterhus : > If you would package the haproxy-lua-http library for Debian > (https://github.com/haproxytech/haproxy-lua-http) [1], what would you > believe would be most "useful" / "in spirit of Debian packaging" / "your > choice"? > > a) Install the library into the regular Lua library path, even if it > would not be useful / broken outside of HAProxy. Using require within > HAProxy simply works without doing anything. > b) Install the library into a HAProxy specific Lua library path (e.g. > /usr/share/haproxy/lua-path/) and add proper lua-prepend-path to > HAProxy's default configuration. The user would need to copy that into > their own configuration, otherwise things break. > c) Install the library into a HAProxy specific Lua library path and add > that path to HAProxy at compile time via a dedicated compile option. For > the user it would work out of the box, similar to option (a), but it > would not clutter the global Lua library path. > d) Something entirely different. > > Best regards > Tim Düsterhus > > [1] I'd like to see it packaged if something useful comes out of this > discussion :-) Sorry for the late answer! b) doesn't seem a good solution as it prevents sharing configuration files between distributions. I think the default prepend path should be a compile-time option (eg `/usr/share/haproxy/lua`) but can be overriden with a specific directive. Therefore, this is transparent for most users, but if a user prefer to put additional stuff in another directory, it's still easily possible. -- Don't stop with your first draft. - The Elements of Programming Style (Kernighan & Plauger)
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
> On 9 Jan 2020, at 22:02, Willy Tarreau wrote: > > On Thu, Jan 09, 2020 at 08:22:29PM +0100, Tim D�sterhus wrote: >> Bob, >> >> Am 09.01.20 um 19:44 schrieb Zakharychev, Bob: >>> I'd say that since the issue can be solved naturally and completely within >>> the confines of Lua itself, new parameters should not be introduced in >>> HAProxy to give one ability to solve it differently while obscuring the >>> solution in configuration of the host program, however tempting this might >>> look. >> >> To put my POV into a different perspective: HAProxy is the host program >> of the Lua scripts and thus it is responsible for providing a working >> environment to the script. It should not need to know how the file >> system layout works. >> Configuring this environment within HAProxy just feels natural even if >> it might be possible to configure this setting from within a dedicated >> "configuration script" as well. > > The useful point I'm seeing here is that it allows users to use some > external Lua programs with no modification and only specify the paths > in the global section which centralizes everything specific to the > deployment. Also it even allows to use environment variables there, > which can be quite useful. > > However, Bob, your concern confirms mine about the risks involved by > adding the ability to execute some Lua code inlined directly from the > haproxy config. So I think that if we want to stay reasonable, at least > for a while, we should take Tim's patch as it is now to help configure > the path, but not put Lua code in the config file. Hi guys, I don’t have any opinion about this. But I extend the problem: There are not only package.path, but also package.cpath which provides default location for .so libraries. https://www.lua.org/manual/5.3/manual.html#pdf-package.cpath Thierry
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
On Thu, Jan 09, 2020 at 08:22:29PM +0100, Tim Düsterhus wrote: > Bob, > > Am 09.01.20 um 19:44 schrieb Zakharychev, Bob: > > I'd say that since the issue can be solved naturally and completely within > > the confines of Lua itself, new parameters should not be introduced in > > HAProxy to give one ability to solve it differently while obscuring the > > solution in configuration of the host program, however tempting this might > > look. > > To put my POV into a different perspective: HAProxy is the host program > of the Lua scripts and thus it is responsible for providing a working > environment to the script. It should not need to know how the file > system layout works. > Configuring this environment within HAProxy just feels natural even if > it might be possible to configure this setting from within a dedicated > "configuration script" as well. The useful point I'm seeing here is that it allows users to use some external Lua programs with no modification and only specify the paths in the global section which centralizes everything specific to the deployment. Also it even allows to use environment variables there, which can be quite useful. However, Bob, your concern confirms mine about the risks involved by adding the ability to execute some Lua code inlined directly from the haproxy config. So I think that if we want to stay reasonable, at least for a while, we should take Tim's patch as it is now to help configure the path, but not put Lua code in the config file. Willy
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
Willy, Am 09.01.20 um 21:57 schrieb Willy Tarreau: > OK so the second path will be looked up before the first one. This is > something I'll probably add to the doc as well then. Yes. Look up works left to right as within the PATH you know and by prepending we are adding to the left. Best regards Tim Düsterhus
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
On Thu, Jan 09, 2020 at 07:35:45PM +0100, Tim Düsterhus wrote: > > > Prepends the given string followed by a semicolon to Lua's package.path > variable. > > Lua's package.path is a semicolon delimited list of patterns that > specify how the `require` function attempts to find the source file of a > library. Question marks (?) within a pattern will be replaced by module > name. > > Example: > > By specifying the following path: > > lua-prepend-path /usr/share/haproxy-lua/?.lua > > Lua will attempt to load the /usr/share/haproxy-lua/example.lua script > when `require "example"` is being called. > > See https://www.lua.org/pil/8.1.html for the details within the Lua > documentation. > > OK, it looks pretty good like this, I was not asking for more. I'll take your patch and paste this inside. > > doing. The link is then provided to have the details. And by the way it > > should also be clear what happens if multiple such directives are specified > > (I have no idea). > > It prepends multiple times (instead of overriding the to-be-prepended > value). So I suppose it works like you expect it to work. In fact that > was part of the example given in my initial mail. I prepended two > different "paths" and both variants appear in the error message in > addition to the default ones. OK so the second path will be looked up before the first one. This is something I'll probably add to the doc as well then. Thanks! Willy
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
Bob, Am 09.01.20 um 19:44 schrieb Zakharychev, Bob: > I'd say that since the issue can be solved naturally and completely within > the confines of Lua itself, new parameters should not be introduced in > HAProxy to give one ability to solve it differently while obscuring the > solution in configuration of the host program, however tempting this might > look. To put my POV into a different perspective: HAProxy is the host program of the Lua scripts and thus it is responsible for providing a working environment to the script. It should not need to know how the file system layout works. Configuring this environment within HAProxy just feels natural even if it might be possible to configure this setting from within a dedicated "configuration script" as well. Best regards Tim Düsterhus
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
Willy, Vincent, Am 09.01.20 um 17:41 schrieb Willy Tarreau: > I'm seeing some merit in this for quite some stuff which annoys us by > requiring a dedicated file. I'm just a bit anxious about how this can > steer from the original intent, with users seeing it as a way to inline > code anywhere, then complaining that we don't have multi-line strings > nor 64kB long lines :-/ > > I'm interested in other opinions on this as well. > In fact I thought about this topic of "HAProxy specific Lua libraries" a bit more and also about making this useful for Linux distributions when we opt to change something about this within HAProxy. Consider that a Linux distribution wants to distribute the haproxy-lua-http library. That library is not useful outside of HAProxy, because it relies on the internal API. To be a useful package it should be "easy" for the administrator to `require` that library within custom Lua scripts (for a certain definition of easy). Vincent, a few questions for you as a maintainer of the distro of my choice (full thread is here if you need context: https://www.mail-archive.com/haproxy@formilux.org/msg35839.html). If you would package the haproxy-lua-http library for Debian (https://github.com/haproxytech/haproxy-lua-http) [1], what would you believe would be most "useful" / "in spirit of Debian packaging" / "your choice"? a) Install the library into the regular Lua library path, even if it would not be useful / broken outside of HAProxy. Using require within HAProxy simply works without doing anything. b) Install the library into a HAProxy specific Lua library path (e.g. /usr/share/haproxy/lua-path/) and add proper lua-prepend-path to HAProxy's default configuration. The user would need to copy that into their own configuration, otherwise things break. c) Install the library into a HAProxy specific Lua library path and add that path to HAProxy at compile time via a dedicated compile option. For the user it would work out of the box, similar to option (a), but it would not clutter the global Lua library path. d) Something entirely different. Best regards Tim Düsterhus [1] I'd like to see it packaged if something useful comes out of this discussion :-)
RE: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
I'd say that since the issue can be solved naturally and completely within the confines of Lua itself, new parameters should not be introduced in HAProxy to give one ability to solve it differently while obscuring the solution in configuration of the host program, however tempting this might look. Just my two cents. Bob -Original Message- From: Willy Tarreau Sent: Thursday, January 9, 2020 11:42 AM To: Tim Düsterhus Cc: Adis Nezirovic ; haproxy@formilux.org; tfourn...@arpalert.org Subject: Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option On Thu, Jan 09, 2020 at 03:00:34PM +0100, Tim Düsterhus wrote: > However this morning I started thinking whether it might make sense to > generalize `lua-prepend-path` to `lua-run-string` that would execute > the Lua script given as the parameter. It might provide better > flexibility and would be as readable as lua-prepend-path: > > lua-run-string "package.path = package.path .. ';/some/path/?.lua'" > > Opinions? I'm seeing some merit in this for quite some stuff which annoys us by requiring a dedicated file. I'm just a bit anxious about how this can steer from the original intent, with users seeing it as a way to inline code anywhere, then complaining that we don't have multi-line strings nor 64kB long lines :-/ I'm interested in other opinions on this as well. Willy
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
Willy, Am 09.01.20 um 17:38 schrieb Willy Tarreau: >> I concede that the HAProxy documentation should mention `package.path` >> to be clear what kind of path it refers to. But I disagree that we >> should explain how this works in detail, because this is not about any >> HAProxy specific stuff, but about the "inner workings" of Lua and thus >> it's best explained by the Lua documentation. We would only be able to >> "re-explain" the existing documentation, but probably worse, because we >> miss out on important details. > > Except that the terminology here is totally misleading. I didn't understand > your proposed example even when reading the commit message and the proposed > doc. This solely because Lua uses the term "path" to designate a pattern, > while in traditional environments a path is more a list of directories. > *This* must be mentioned. It's not acceptable to have a config documentation > saying "hehe I caught you, you thought you would find the info you were > looking for here, but no, please visit this link instead, and too bad for > you if the net access is down due to haproxy not starting because of the > wrong config you're trying to fix, have a good night sir". I'm not suggesting > to paraphrase the Lua's doc, just to mention the few elements that are > sufficient for someone having to edit an entry to understand what he/she's Fair enough. Without creating a completely new patch, what about something like this: Prepends the given string followed by a semicolon to Lua's package.path variable. Lua's package.path is a semicolon delimited list of patterns that specify how the `require` function attempts to find the source file of a library. Question marks (?) within a pattern will be replaced by module name. Example: By specifying the following path: lua-prepend-path /usr/share/haproxy-lua/?.lua Lua will attempt to load the /usr/share/haproxy-lua/example.lua script when `require "example"` is being called. See https://www.lua.org/pil/8.1.html for the details within the Lua documentation. > doing. The link is then provided to have the details. And by the way it > should also be clear what happens if multiple such directives are specified > (I have no idea). It prepends multiple times (instead of overriding the to-be-prepended value). So I suppose it works like you expect it to work. In fact that was part of the example given in my initial mail. I prepended two different "paths" and both variants appear in the error message in addition to the default ones. Best regards Tim Düsterhus
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
On Thu, Jan 09, 2020 at 03:00:34PM +0100, Tim Düsterhus wrote: > However this morning I started thinking whether it might make sense to > generalize `lua-prepend-path` to `lua-run-string` that would execute the > Lua script given as the parameter. It might provide better flexibility > and would be as readable as lua-prepend-path: > > lua-run-string "package.path = package.path .. ';/some/path/?.lua'" > > Opinions? I'm seeing some merit in this for quite some stuff which annoys us by requiring a dedicated file. I'm just a bit anxious about how this can steer from the original intent, with users seeing it as a way to inline code anywhere, then complaining that we don't have multi-line strings nor 64kB long lines :-/ I'm interested in other opinions on this as well. Willy
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
On Thu, Jan 09, 2020 at 03:27:17PM +0100, Tim Düsterhus wrote: > Willy, > > Am 09.01.20 um 04:07 schrieb Willy Tarreau: > > I'm personally not opposed to this at all, however after reading the link > > above and given that Lua's concept of "path" is all but obvious as described > > in the link above, we definitely need a bit more text in the doc to quickly > > explain that the path is rather a pattern and that the question mark is > > replaced by the file in "require", and probably put a short example. Having > > the link for more details is fine but not as a redirect to figure how to > > use the config keyword. > > I concede that the HAProxy documentation should mention `package.path` > to be clear what kind of path it refers to. But I disagree that we > should explain how this works in detail, because this is not about any > HAProxy specific stuff, but about the "inner workings" of Lua and thus > it's best explained by the Lua documentation. We would only be able to > "re-explain" the existing documentation, but probably worse, because we > miss out on important details. Except that the terminology here is totally misleading. I didn't understand your proposed example even when reading the commit message and the proposed doc. This solely because Lua uses the term "path" to designate a pattern, while in traditional environments a path is more a list of directories. *This* must be mentioned. It's not acceptable to have a config documentation saying "hehe I caught you, you thought you would find the info you were looking for here, but no, please visit this link instead, and too bad for you if the net access is down due to haproxy not starting because of the wrong config you're trying to fix, have a good night sir". I'm not suggesting to paraphrase the Lua's doc, just to mention the few elements that are sufficient for someone having to edit an entry to understand what he/she's doing. The link is then provided to have the details. And by the way it should also be clear what happens if multiple such directives are specified (I have no idea). Regards, Willy
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
Willy, Am 09.01.20 um 04:07 schrieb Willy Tarreau: > I'm personally not opposed to this at all, however after reading the link > above and given that Lua's concept of "path" is all but obvious as described > in the link above, we definitely need a bit more text in the doc to quickly > explain that the path is rather a pattern and that the question mark is > replaced by the file in "require", and probably put a short example. Having > the link for more details is fine but not as a redirect to figure how to > use the config keyword. I concede that the HAProxy documentation should mention `package.path` to be clear what kind of path it refers to. But I disagree that we should explain how this works in detail, because this is not about any HAProxy specific stuff, but about the "inner workings" of Lua and thus it's best explained by the Lua documentation. We would only be able to "re-explain" the existing documentation, but probably worse, because we miss out on important details. Best regards Tim Düsterhus
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
Adis, Am 09.01.20 um 10:27 schrieb Adis Nezirovic: > global > lua-load /path/to/config.lua > lua-load /path/to/app.lua > […] > And now for the main topic, if you need to set any paths for Lua > scripts/libraries in non standard locations, you can directly use > package.path in config.lua script > > package.path = package.path .. ';/some/path/?.lua' > > > That way, there is no need for users to edit app script, and they can > override configuration or Lua path settings. IMO, we don't need another > directive in HAProxy just to set Lua paths. Yes, I am aware that this would work. I dislike needing an additional configuration file that would contain a single line more often than not. Because then I need to context switch to understand the configuration. It would also make it easier to explain for administrators to set-up. I can just tell them: "put haproxy-lua-http into /etc/haproxy/lua-modules and add that folder to `lua-prepend-path` and lua-load haproxy-auth-request" vs "put haproxy-lua-http somewhere, then create another Lua script containing package.path = package.path .. ';/that/folder/?.lua' and then lua-load that one in addition to haproxy-auth-request". However this morning I started thinking whether it might make sense to generalize `lua-prepend-path` to `lua-run-string` that would execute the Lua script given as the parameter. It might provide better flexibility and would be as readable as lua-prepend-path: lua-run-string "package.path = package.path .. ';/some/path/?.lua'" Opinions? Best regards Tim Düsterhus
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
On 1/8/20 10:54 PM, Tim Duesterhus wrote:
List,
while working on updating my haproxy-auth-request [1] to make use of
haproxy-lua-http [2] instead of the crappy lua-socket [3] I realized
that it would be difficult for the administrator to use the script
afterwards.
The reason is that `require` in Lua attempts to search the modules
either in the current working directory (which is different from
the filename of the script executing the `require` and which most
likely is not anywhere near HAProxy's configuration folder) or within
Lua's library path which is compiled into Lua (in my case this is
something like `/usr/share/lua/5.3/`).
Add a `lua-prepend-path` configuration option that prepend the given
string to Lua's path, allowing scripts loaded with `lua-load` to find
libraries within that path.
Example configuration:
global
lua-prepend-path /etc/haproxy/lua-modules/?/init.lua
lua-prepend-path /etc/haproxy/lua-modules/?.lua
lua-load /etc/haproxy/lua-modules/auth-request.lua
Hey Tim,
For what is worth, we solve situations like this with regular script,
loaded before main script:
global
lua-load /path/to/config.lua
lua-load /path/to/app.lua
Inside config.lua you can override your app configuration options like this
conf = { opt1 = "val" }
Then, app.lua contains the following pattern
if not conf then
conf = { ... } -- defaults
else
if not conf.opt1 then conf.opt1 = "val" end
...
end
And now for the main topic, if you need to set any paths for Lua
scripts/libraries in non standard locations, you can directly use
package.path in config.lua script
package.path = package.path .. ';/some/path/?.lua'
That way, there is no need for users to edit app script, and they can
override configuration or Lua path settings. IMO, we don't need another
directive in HAProxy just to set Lua paths.
Best regards
--
Adis Nezirovic
Software Engineer
HAProxy Technologies - Powering your uptime!
375 Totten Pond Road, Suite 302 | Waltham, MA 02451, US
+1 (844) 222-4340 | https://www.haproxy.com
Re: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
Hi Tim, On Wed, Jan 08, 2020 at 10:54:47PM +0100, Tim Duesterhus wrote: > global > lua-prepend-path /etc/haproxy/lua-modules/?/init.lua > lua-prepend-path /etc/haproxy/lua-modules/?.lua > lua-load /etc/haproxy/lua-modules/auth-request.lua (...) > diff --git a/doc/configuration.txt b/doc/configuration.txt > index d0bb97415..47a4db344 100644 > --- a/doc/configuration.txt > +++ b/doc/configuration.txt > @@ -1080,6 +1081,11 @@ lua-load >This global directive loads and executes a Lua file. This directive can be >used multiple times. > > +lua-prepend-path > + Prepends the given string followed by a semicolon to Lua's search path. > + > + see: https://www.lua.org/pil/8.1.html > + I'm personally not opposed to this at all, however after reading the link above and given that Lua's concept of "path" is all but obvious as described in the link above, we definitely need a bit more text in the doc to quickly explain that the path is rather a pattern and that the question mark is replaced by the file in "require", and probably put a short example. Having the link for more details is fine but not as a redirect to figure how to use the config keyword. Thanks! Willy
[PATCH] MINOR: lua: Add lua-prepend-path configuration option
List,
while working on updating my haproxy-auth-request [1] to make use of
haproxy-lua-http [2] instead of the crappy lua-socket [3] I realized
that it would be difficult for the administrator to use the script
afterwards.
The reason is that `require` in Lua attempts to search the modules
either in the current working directory (which is different from
the filename of the script executing the `require` and which most
likely is not anywhere near HAProxy's configuration folder) or within
Lua's library path which is compiled into Lua (in my case this is
something like `/usr/share/lua/5.3/`).
Add a `lua-prepend-path` configuration option that prepend the given
string to Lua's path, allowing scripts loaded with `lua-load` to find
libraries within that path.
Example configuration:
global
lua-prepend-path /etc/haproxy/lua-modules/?/init.lua
lua-prepend-path /etc/haproxy/lua-modules/?.lua
lua-load /etc/haproxy/lua-modules/auth-request.lua
would result in something like this:
[ALERT] 007/225248 (6638) : parsing [./haproxy.cfg:5] : Lua runtime error:
auth-request.lua:23: module 'haproxy-lua-http' not found:
no field package.preload['haproxy-lua-http']
no file '/etc/haproxy/lua-modules/haproxy-lua-http.lua'
no file '/etc/haproxy/lua-modules/haproxy-lua-http/init.lua'
no file '/usr/local/share/lua/5.3/haproxy-lua-http.lua'
no file '/usr/local/share/lua/5.3/haproxy-lua-http/init.lua'
no file '/usr/local/lib/lua/5.3/haproxy-lua-http.lua'
no file '/usr/local/lib/lua/5.3/haproxy-lua-http/init.lua'
no file '/usr/share/lua/5.3/haproxy-lua-http.lua'
no file '/usr/share/lua/5.3/haproxy-lua-http/init.lua'
no file './haproxy-lua-http.lua'
no file './haproxy-lua-http/init.lua'
no file '/usr/local/lib/lua/5.3/haproxy-lua-http.so'
no file '/usr/lib/x86_64-linux-gnu/lua/5.3/haproxy-lua-http.so'
no file '/usr/lib/lua/5.3/haproxy-lua-http.so'
no file '/usr/local/lib/lua/5.3/loadall.so'
no file './haproxy-lua-http.so'
I'd appreciate if we could backport this to 2.1. It should be fairly
self-contained.
Best regards
Tim Düsterhus
[1] https://github.com/TimWolla/haproxy-auth-request
[2] https://github.com/haproxytech/haproxy-lua-http
[3] https://github.com/TimWolla/haproxy-auth-request/issues/4
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
Subject: [PATCH] MINOR: lua: Add lua-prepend-path configuration option
lua-prepend-path allows the administrator to specify a custom Lua library
path to load custom Lua modules that are useful within the context of HAProxy
without polluting the global Lua library folder.
---
doc/configuration.txt | 6 ++
src/hlua.c| 16
2 files changed, 22 insertions(+)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index d0bb97415..47a4db344 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -603,6 +603,7 @@ The following keywords are supported in the "global"
section :
- log-tag
- log-send-hostname
- lua-load
+ - lua-prepend-path
- mworker-max-reloads
- nbproc
- nbthread
@@ -1080,6 +1081,11 @@ lua-load
This global directive loads and executes a Lua file. This directive can be
used multiple times.
+lua-prepend-path
+ Prepends the given string followed by a semicolon to Lua's search path.
+
+ see: https://www.lua.org/pil/8.1.html
+
master-worker [no-exit-on-failure]
Master-worker mode. It is equivalent to the command line "-W" argument.
This mode will launch a "master" which will monitor the "workers". Using
diff --git a/src/hlua.c b/src/hlua.c
index 37f786687..4f39755b3 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -7458,8 +7458,24 @@ static int hlua_load(char **args, int section_type,
struct proxy *curpx,
return 0;
}
+static int hlua_prepend_path(char **args, int section_type, struct proxy
*curpx,
+ struct proxy *defpx, const char *file, int line,
+ char **err)
+{
+ lua_getglobal(gL.T, "package"); /* push package*/
+ lua_pushstring(gL.T, args[1]); /* push given path */
+ lua_pushstring(gL.T, ";"); /* push semicolon */
+ lua_getfield(gL.T, -3, "path"); /* push old path */
+ lua_concat(gL.T, 3);/* concatenate to new path */
+ lua_setfield(gL.T, -2, "path"); /* store new path */
+ lua_pop(gL.T, 1); /* pop package */
+
+ return 0;
+}
+
/* configuration keywords declaration */
static struct cfg_kw_list cfg_kws = {{ },{
+ { CFG_GLOBAL, "lua-prepend-path", hlua_prepend_path },
{ CFG_GLOBAL, "lua-load", hlua_load },
{ CFG_GLOBAL, "tune.lua.session-timeout", hlua_session_timeout },
{ CFG_GLOBAL, "tune.lua.task-timeout",hlua_task_timeout },
--
2.24.1
