Re: HAProxy/Lua survey about lua configuration form
On Wed, Sep 23, 2015 at 09:26:33PM +0200, Thierry FOURNIER wrote: > On Wed, 23 Sep 2015 21:15:08 +0200 > Willy Tarreau wrote: > > > On Wed, Sep 23, 2015 at 09:09:09PM +0200, Thierry FOURNIER wrote: > > > Ok thank you for your opinions. The patch set is in attachment. > > > > Thanks. Is it for testing only or for applying ? Just let me know. > > > You can apply these patch. There are tested. Merged, thanks! Willy
Re: HAProxy/Lua survey about lua configuration form
On Wed, 23 Sep 2015 21:15:08 +0200 Willy Tarreau wrote: > On Wed, Sep 23, 2015 at 09:09:09PM +0200, Thierry FOURNIER wrote: > > Ok thank you for your opinions. The patch set is in attachment. > > Thanks. Is it for testing only or for applying ? Just let me know. You can apply these patch. There are tested. Thierry
Re: HAProxy/Lua survey about lua configuration form
On Wed, Sep 23, 2015 at 09:09:09PM +0200, Thierry FOURNIER wrote: > Ok thank you for your opinions. The patch set is in attachment. Thanks. Is it for testing only or for applying ? Just let me know. BTW I've started to sort out the applet issues, I guess I should be able to produce something by tomorrow evening. No promises yet, there's a surprize at the end of each wire I'm pulling, I feel like a fisherman! Willy
Re: HAProxy/Lua survey about lua configuration form
Ok thank you for your opinions. The patch set is in attachment.
Thierry
On Wed, 23 Sep 2015 19:50:24 +0200
Willy Tarreau wrote:
> On Wed, Sep 23, 2015 at 07:37:56PM +0200, Thierry FOURNIER wrote:
> > The pending "use-service" code already embbed some changes in the
> > généric actions, like requirement of opaque data. So the change is easy
> > and the risk of regressions is low. In fact, the main complicated code
> > in the Lua is the runtime code and not the configuration parser nor
> > the registration of functions.
>
> OK, thanks for clarifying. Then indeed we should mostly focus on code
> reliability and correct behaviour and I agree that your proposal goes
> into that direction.
>
> Willy
>
>
>From b8698194be79e095741a28e5a58dfacad80e2636 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER
Date: Tue, 22 Sep 2015 18:26:42 +0200
Subject: [PATCH 1/3] MINOR: action: add private configuration
This private configuration pointer is used for storing some configuration
data associated the keyword, So many keywords can use the same parse
function, and this one can use a discriminator.
---
include/types/action.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/types/action.h b/include/types/action.h
index 2461f79..d74e5ba 100644
--- a/include/types/action.h
+++ b/include/types/action.h
@@ -154,6 +154,7 @@ struct action_kw {
enum act_parse_ret (*parse)(const char **args, int *cur_arg, struct proxy *px,
struct act_rule *rule, char **err);
int match_pfx;
+ void *private;
};
struct action_kw_list {
--
2.1.4
>From fd4321b3824e84b789e2441e73399bf6c540e710 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER
Date: Tue, 22 Sep 2015 19:14:35 +0200
Subject: [PATCH 2/3] MINOR: action: add reference to the original keywork
matched for the called parser.
This is usefull because the keyword can contains some condifiguration
data set while the keyword registration.
---
include/types/action.h | 1 +
src/proto_http.c | 2 ++
src/proto_tcp.c| 3 +++
3 files changed, 6 insertions(+)
diff --git a/include/types/action.h b/include/types/action.h
index d74e5ba..b7e1063 100644
--- a/include/types/action.h
+++ b/include/types/action.h
@@ -93,6 +93,7 @@ struct act_rule {
short deny_status; /* HTTP status to return to user when denying */
enum act_return (*action_ptr)(struct act_rule *rule, struct proxy *px,
struct session *sess, struct stream *s); /* ptr to custom action */
+ struct action_kw *kw;
union {
struct {
char *realm;
diff --git a/src/proto_http.c b/src/proto_http.c
index 57cc354..9acf0a2 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -9442,6 +9442,7 @@ struct act_rule *parse_http_req_cond(const char **args, const char *file, int li
cur_arg = 1;
/* try in the module list */
rule->from = ACT_F_HTTP_REQ;
+ rule->kw = custom;
if (custom->parse(args, &cur_arg, proxy, rule, &errmsg) == ACT_RET_PRS_ERR) {
Alert("parsing [%s:%d] : error detected in %s '%s' while parsing 'http-request %s' rule : %s.\n",
file, linenum, proxy_type_str(proxy), proxy->id, args[0], errmsg);
@@ -9798,6 +9799,7 @@ struct act_rule *parse_http_res_cond(const char **args, const char *file, int li
cur_arg = 1;
/* try in the module list */
rule->from = ACT_F_HTTP_RES;
+ rule->kw = custom;
if (custom->parse(args, &cur_arg, proxy, rule, &errmsg) == ACT_RET_PRS_ERR) {
Alert("parsing [%s:%d] : error detected in %s '%s' while parsing 'http-response %s' rule : %s.\n",
file, linenum, proxy_type_str(proxy), proxy->id, args[0], errmsg);
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index e671e41..2ea7161 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -1435,6 +1435,7 @@ static int tcp_parse_response_rule(char **args, int arg, int section_type,
if (kw) {
arg++;
rule->from = ACT_F_TCP_RES_CNT;
+ rule->kw = kw;
if (kw->parse((const char **)args, &arg, curpx, rule, err) == ACT_RET_PRS_ERR)
return -1;
} else {
@@ -1636,9 +1637,11 @@ static int tcp_parse_request_rule(char **args, int arg, int section_type,
struct action_kw *kw;
if (where & SMP_VAL_FE_CON_ACC) {
kw = tcp_req_conn_action(args[arg]);
+ rule->kw = kw;
rule->from = ACT_F_TCP_REQ_CON;
} else {
kw = tcp_req_cont_action(args[arg]);
+ rule->kw = kw;
rule->from = ACT_F_TCP_REQ_CNT;
}
if (kw) {
--
2.1.4
>From 86dc0a15d0545731c02022c837a9407ec9f5910f Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER
Date: Wed, 23 Sep 2015 21:03:35 +0200
Subject: [PATCH 3/3] MINOR: lua: change actions registration
The current Lua action are not registered. The executed function is
selected according with a function name writed in the HAProxy configuration.
This patch add an action registration function. The configuration mode
described above disappear.
This change make some incompatibilities with existing configuration files for
HAProxy 1.6-dev.
---
doc/lua-api/index.rst |
Re: HAProxy/Lua survey about lua configuration form
On Wed, Sep 23, 2015 at 07:37:56PM +0200, Thierry FOURNIER wrote: > The pending "use-service" code already embbed some changes in the > généric actions, like requirement of opaque data. So the change is easy > and the risk of regressions is low. In fact, the main complicated code > in the Lua is the runtime code and not the configuration parser nor > the registration of functions. OK, thanks for clarifying. Then indeed we should mostly focus on code reliability and correct behaviour and I agree that your proposal goes into that direction. Willy
Re: HAProxy/Lua survey about lua configuration form
On Wed, 23 Sep 2015 09:37:41 +0200 Willy Tarreau wrote: > On Tue, Sep 22, 2015 at 10:18:10PM -0400, Michael Ezzell wrote: > > I don't see a significant problem, if there are benefits to be gained. > > > > I assume part of the motivation is to prevent inadvertently calling an > > inappropriate function as an action, to perhaps to allow earlier validation > > of the configuration of actions referencing Lua functions, and to separate > > the namespaces of Lua functions and HAProxy Lua actions. > > I'm seeing the same benefit. In fact I even *thought* that actions were > registered like fetch/convs. But the current state of affairs now scares > me for the future because indeed we don't want to call random functions > by accident. Also, I don't know if some functions are shipped with Lua, > but it could be painful if a new version of Lua brings new functions > which collide with ones that people use in their configs. With a > registration mechanism this cannot happen at all, so it's much safer. > > It will also allow to use a wider character set in action names (eg: '-' > or '.' that we use in the core config language). So all in all I think it's > a good idea that's worth being done *before* the actual release otherwise > we could regret it later. > > Thierry, what's the impact of such a change (in terms of complexity, amount > of code changed, risks of regressions, etc) ? I've found the issues I've been > debugging since last week-end, they're fixed on paper now, I just need to fix > the code and I'm thinking about issuing dev6 with all the pending fixes and > having an extra dev7 next week before -final. Do you think such a change > could be ready for dev6 so that people don't have to wait an extra week to > test a change ? The pending "use-service" code already embbed some changes in the généric actions, like requirement of opaque data. So the change is easy and the risk of regressions is low. In fact, the main complicated code in the Lua is the runtime code and not the configuration parser nor the registration of functions. Thierry
Re: HAProxy/Lua survey about lua configuration form
On Tue, 22 Sep 2015 22:18:10 -0400
Michael Ezzell wrote:
> I don't see a significant problem, if there are benefits to be gained.
>
> I assume part of the motivation is to prevent inadvertently calling an
> inappropriate function as an action, to perhaps to allow earlier validation
> of the configuration of actions referencing Lua functions, and to separate
> the namespaces of Lua functions and HAProxy Lua actions.
>
> As long as no capabilities are lost or performance penalties are created, I
> don't see the change as being extreme.
>
> One point that is not clear is how this "facilitates the distribution of
> Lua scripts for HAProxy." You would need to look at the script to find the
> registered action name, the same as if you had to look at the script to
> find the function name...
Sure, you're right. When I wrote this, I thought to an Lua file like an
addon with documentation who add some functions, perfectly defined. So,
with the registration function name based, an administrator can call
anything function, even bad functions.
> though perhaps the intention is to express the
> idea that "not every Lua function is appropriate to directly reference in
> the configuration" since there may be "private" functions in a script that
> are only intended to support other (exposed) functions, rather than being
> called directly.
Exactly.
> Perhaps you could provide more insight, particularly if I incorrect on some
> of these assumptions. Otherwise, I don't see a reason to object (but I am
> not yet using Lua in production, so for me the change has minimal impact.)
I thought that I gave all the detail possibles. The change that I
proposes is just two things:
- Add a call to the registration in lu Lua file,
(core.action_register("name", function) ).
- Change a little bit the configuration file:
"http-request lua.name" instead of "http-request lua name"
(only the 'dot' between "lua" and "name" who will become one word in
place od two separate words).
Thierry
> On Sep 22, 2015 9:27 PM, wrote:
>
> > Hi list,
> >
> > Actually we two Lua registration forms:
> >
> > The style of fetches and converters (and coming soon the "services").
> > With this registering style, the Lua developer register the function in
> > the haproxy core like this:
> >
> >core.register_fetch("fetch-name", function(...)
> > ... code ...
> >end)
> >
> > In the HAProxy configuration file, the registered Lua fetch can be used
> > like other fetch. it is automatically prefixed by "lua.". For example:
> >
> >use-backend service if { lua.fetch-name }
> >
> > The second mode is used with actions. Each action doesn't require any
> > registration. The user just give the Lua function name in the ha proxy
> > configuration. For example, in Lua file:
> >
> >function my_action(...)
> > ... code ...
> >end
> >
> > And the associated HAProxy file:
> >
> >http-request lua my_action
> >
> > I want to remove the second declaration mode. After this the actions
> > will be registered like fetches and converters. The previous example
> > should become:
> >
> >core.register_action("action-name", function(...)
> > ... code ...
> >end)
> >
> > And
> >
> >http-request lua.action-name
> >
> > I think that this mode of registration facilitates the distribution of
> > Lua scripts for HAProxy. The administrator doesn't look into the file
> > bout any function name, and the administrator cannot call a function
> > which is not designed for running in an HAProxy action.
> >
> > My problem, is that the second configuration mode exists in HAProxy
> > from a long time. I don't want to remove it without a consultation of
> > the people who already use this feature. My other problem, is that if
> > the version 1.6 is released with the second mode, we will keep (and
> > maintain) for a long time :)
> >
> > the discussion is open, please give me your opinions.
> >
> > Thierry
> >
> >
Re: HAProxy/Lua survey about lua configuration form
On Tue, Sep 22, 2015 at 10:18:10PM -0400, Michael Ezzell wrote: > I don't see a significant problem, if there are benefits to be gained. > > I assume part of the motivation is to prevent inadvertently calling an > inappropriate function as an action, to perhaps to allow earlier validation > of the configuration of actions referencing Lua functions, and to separate > the namespaces of Lua functions and HAProxy Lua actions. I'm seeing the same benefit. In fact I even *thought* that actions were registered like fetch/convs. But the current state of affairs now scares me for the future because indeed we don't want to call random functions by accident. Also, I don't know if some functions are shipped with Lua, but it could be painful if a new version of Lua brings new functions which collide with ones that people use in their configs. With a registration mechanism this cannot happen at all, so it's much safer. It will also allow to use a wider character set in action names (eg: '-' or '.' that we use in the core config language). So all in all I think it's a good idea that's worth being done *before* the actual release otherwise we could regret it later. Thierry, what's the impact of such a change (in terms of complexity, amount of code changed, risks of regressions, etc) ? I've found the issues I've been debugging since last week-end, they're fixed on paper now, I just need to fix the code and I'm thinking about issuing dev6 with all the pending fixes and having an extra dev7 next week before -final. Do you think such a change could be ready for dev6 so that people don't have to wait an extra week to test a change ? Thanks, Willy
Re: HAProxy/Lua survey about lua configuration form
I don't see a significant problem, if there are benefits to be gained.
I assume part of the motivation is to prevent inadvertently calling an
inappropriate function as an action, to perhaps to allow earlier validation
of the configuration of actions referencing Lua functions, and to separate
the namespaces of Lua functions and HAProxy Lua actions.
As long as no capabilities are lost or performance penalties are created, I
don't see the change as being extreme.
One point that is not clear is how this "facilitates the distribution of
Lua scripts for HAProxy." You would need to look at the script to find the
registered action name, the same as if you had to look at the script to
find the function name... though perhaps the intention is to express the
idea that "not every Lua function is appropriate to directly reference in
the configuration" since there may be "private" functions in a script that
are only intended to support other (exposed) functions, rather than being
called directly.
Perhaps you could provide more insight, particularly if I incorrect on some
of these assumptions. Otherwise, I don't see a reason to object (but I am
not yet using Lua in production, so for me the change has minimal impact.)
On Sep 22, 2015 9:27 PM, wrote:
> Hi list,
>
> Actually we two Lua registration forms:
>
> The style of fetches and converters (and coming soon the "services").
> With this registering style, the Lua developer register the function in
> the haproxy core like this:
>
>core.register_fetch("fetch-name", function(...)
> ... code ...
>end)
>
> In the HAProxy configuration file, the registered Lua fetch can be used
> like other fetch. it is automatically prefixed by "lua.". For example:
>
>use-backend service if { lua.fetch-name }
>
> The second mode is used with actions. Each action doesn't require any
> registration. The user just give the Lua function name in the ha proxy
> configuration. For example, in Lua file:
>
>function my_action(...)
> ... code ...
>end
>
> And the associated HAProxy file:
>
>http-request lua my_action
>
> I want to remove the second declaration mode. After this the actions
> will be registered like fetches and converters. The previous example
> should become:
>
>core.register_action("action-name", function(...)
> ... code ...
>end)
>
> And
>
>http-request lua.action-name
>
> I think that this mode of registration facilitates the distribution of
> Lua scripts for HAProxy. The administrator doesn't look into the file
> bout any function name, and the administrator cannot call a function
> which is not designed for running in an HAProxy action.
>
> My problem, is that the second configuration mode exists in HAProxy
> from a long time. I don't want to remove it without a consultation of
> the people who already use this feature. My other problem, is that if
> the version 1.6 is released with the second mode, we will keep (and
> maintain) for a long time :)
>
> the discussion is open, please give me your opinions.
>
> Thierry
>
>
HAProxy/Lua survey about lua configuration form
Hi list,
Actually we two Lua registration forms:
The style of fetches and converters (and coming soon the "services").
With this registering style, the Lua developer register the function in
the haproxy core like this:
core.register_fetch("fetch-name", function(...)
... code ...
end)
In the HAProxy configuration file, the registered Lua fetch can be used
like other fetch. it is automatically prefixed by "lua.". For example:
use-backend service if { lua.fetch-name }
The second mode is used with actions. Each action doesn't require any
registration. The user just give the Lua function name in the ha proxy
configuration. For example, in Lua file:
function my_action(...)
... code ...
end
And the associated HAProxy file:
http-request lua my_action
I want to remove the second declaration mode. After this the actions
will be registered like fetches and converters. The previous example
should become:
core.register_action("action-name", function(...)
... code ...
end)
And
http-request lua.action-name
I think that this mode of registration facilitates the distribution of
Lua scripts for HAProxy. The administrator doesn't look into the file
bout any function name, and the administrator cannot call a function
which is not designed for running in an HAProxy action.
My problem, is that the second configuration mode exists in HAProxy
from a long time. I don't want to remove it without a consultation of
the people who already use this feature. My other problem, is that if
the version 1.6 is released with the second mode, we will keep (and
maintain) for a long time :)
the discussion is open, please give me your opinions.
Thierry
