Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-19 Thread William Lallemand
On Wed, Jan 19, 2022 at 03:32:35PM +0100, Willy Tarreau wrote: > Subject: Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with > set server ssl > > On Wed, Jan 19, 2022 at 03:24:44PM +0100, William Lallemand wrote: > > On Tue, Jan 18, 2022 at 12:07:21PM +0100, W

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-19 Thread Willy Tarreau
On Wed, Jan 19, 2022 at 03:24:44PM +0100, William Lallemand wrote: > On Tue, Jan 18, 2022 at 12:07:21PM +0100, Willy Tarreau wrote: > > > > On Mon, Jan 17, 2022 at 07:46:24PM +0100, William Dauchy wrote: > > > Hello Christopher, > > > > > > On Wed, Jan 12, 2022 at 12:45 PM William Dauchy wrote: >

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-19 Thread William Lallemand
On Tue, Jan 18, 2022 at 12:07:21PM +0100, Willy Tarreau wrote: > > On Mon, Jan 17, 2022 at 07:46:24PM +0100, William Dauchy wrote: > > Hello Christopher, > > > > On Wed, Jan 12, 2022 at 12:45 PM William Dauchy wrote: > > > my approach was to say: > > > - remove the implicit behavior > > > - then

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-18 Thread Willy Tarreau
On Mon, Jan 17, 2022 at 07:46:24PM +0100, William Dauchy wrote: > Hello Christopher, > > On Wed, Jan 12, 2022 at 12:45 PM William Dauchy wrote: > > my approach was to say: > > - remove the implicit behavior > > - then work on the missing commands for the health checks > > Do you think we can con

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-17 Thread William Dauchy
Hello Christopher, On Wed, Jan 12, 2022 at 12:45 PM William Dauchy wrote: > my approach was to say: > - remove the implicit behavior > - then work on the missing commands for the health checks Do you think we can conclude on it? -- William

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-12 Thread William Dauchy
On Wed, Jan 12, 2022 at 10:02 AM Christopher Faulet wrote: > I don't know what is the expected behavior on the stable releases for users. > Honestly, I've misread you patch and kept in mind you alternative solution... > But as said, if there is no implicit change (and I'm fine with this > solutio

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-12 Thread Willy Tarreau
On Wed, Jan 12, 2022 at 10:02:30AM +0100, Christopher Faulet wrote: > I don't know what is the expected behavior on the stable releases for users. > The actual state is buggy because health-check are only updated when ssl is > disabled. When SSL is enabled on a server, there is no implicit change o

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-12 Thread Amaury Denoyelle
On Tue, Jan 11, 2022 at 06:55:10PM +0100, Christopher Faulet wrote: > Le 1/10/22 à 23:19, Willy Tarreau a écrit : > > At this point I'm starting to think that we should probably avoid as > > much as possible to use implicit settings for whatever is dynamic. > > Originally a lot of settings were imp

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-12 Thread Christopher Faulet
Le 1/11/22 à 22:47, William Dauchy a écrit : Agree. But, if possible, a warning may be added in the documentation to warn about implicit changes. From the discussion, I would be tempted to say the opposite, as I feel like keeping implicit things for this command is worse. I don't know what i

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-11 Thread William Dauchy
Hello Christopher, Thanks for your research, On Tue, Jan 11, 2022 at 6:55 PM Christopher Faulet wrote: > Le 1/10/22 à 23:19, Willy Tarreau a écrit : > > At this point I'm starting to think that we should probably avoid as > > much as possible to use implicit settings for whatever is dynamic. > >

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-11 Thread Christopher Faulet
Le 1/10/22 à 23:19, Willy Tarreau a écrit : At this point I'm starting to think that we should probably avoid as much as possible to use implicit settings for whatever is dynamic. Originally a lot of settings were implicit because we don't want to have huge config lines to enforce lots of obvious

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-11 Thread Christopher Faulet
Le 1/10/22 à 23:19, Willy Tarreau a écrit : w options were still configurable on the CLI by then. "check-ssl" has been available for a long time, so that's not the reason behind it, but I guess you were referring to something else. I suspect I did a dumb copy/paste from the new_server function

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-10 Thread Willy Tarreau
Hi William! On Mon, Jan 10, 2022 at 10:49:38PM +0100, William Dauchy wrote: > On Mon, Jan 10, 2022 at 7:51 AM Willy Tarreau wrote: > > It's important to always keep in mind that checks are not necessarily > > related to the production traffic, and that configuring one part should > > not have any

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-10 Thread William Dauchy
On Sat, Jan 8, 2022 at 3:03 PM Tim Düsterhus wrote: > Causes issues when applying the patch, because git gets confused and > believes this to be the patch. > I tend to indent this type of "literal code block" within my commit > message with 4 spaces for clarity. indeed, good point, will fix if I

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-09 Thread Willy Tarreau
On Thu, Jan 06, 2022 at 04:57:15PM +0100, William Dauchy wrote: > While giving a fresh try to `set server ssl` (which I wrote), I realised > the behavior is a bit inconsistent. Indeed when using this command over > a server with ssl enabled for the data path but also for the health > check path we

Re: [PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-08 Thread Tim Düsterhus
William, as a heads up, this part of the commit message: On 1/6/22 4:57 PM, William Dauchy wrote: The alternative solution was to restore the previous state, but I believe this will create even more confusion in the future: --- a/src/server.c +++ b/src/server.c @@ -2113,8 +2113,11 @@ void srv_

[PATCH] BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

2022-01-06 Thread William Dauchy
While giving a fresh try to `set server ssl` (which I wrote), I realised the behavior is a bit inconsistent. Indeed when using this command over a server with ssl enabled for the data path but also for the health check path we have: - data and health check done using tls - emit `set server be_foo/