Hello Olivier,
On Wed, Jan 17, 2018 at 05:43:02PM +0100, Olivier Houchard wrote:
> Ok you got me convinced, the attached patch don't check for duplicate
> cookies for disabled server, until we enable them.
I took the time to test it on top of 1.8.x and it works as expected,
removing the warnings.
Thanks,
> From cfc333d2b04686a3c488fdcb495cba64dbfec14b Mon Sep 17 00:00:00 2001
> From: Olivier Houchard <ohouch...@haproxy.com>
> Date: Wed, 17 Jan 2018 17:39:34 +0100
> Subject: [PATCH] MINOR: servers: Don't report duplicate dyncookies for
> disabled servers.
>
> Especially with server-templates, it can happen servers starts with a
> placeholder IP, in the disabled state. In this case, we don't want to report
> that the same cookie was generated for multiple servers. So defer the test
> until the server is enabled.
>
> This should be backported to 1.8.
Reported-by: Pierre Cheynier <p.cheyn...@criteo.com>
Tested-by: William Dauchy <w.dau...@criteo.com>
> ---
> src/server.c | 50 +++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/src/server.c b/src/server.c
> index a37e91968..3901e7d8b 100644
> --- a/src/server.c
> +++ b/src/server.c
> @@ -86,10 +86,34 @@ int srv_getinter(const struct check *check)
> return (check->fastinter)?(check->fastinter):(check->inter);
> }
>
> -void srv_set_dyncookie(struct server *s)
> +/*
> + * Check that we did not get a hash collision.
> + * Unlikely, but it can happen.
> + */
> +static inline void srv_check_for_dup_dyncookie(struct server *s)
> {
> struct proxy *p = s->proxy;
> struct server *tmpserv;
> +
> + for (tmpserv = p->srv; tmpserv != NULL;
> + tmpserv = tmpserv->next) {
> + if (tmpserv == s)
> + continue;
> + if (tmpserv->next_admin & SRV_ADMF_FMAINT)
> + continue;
> + if (tmpserv->cookie &&
> + strcmp(tmpserv->cookie, s->cookie) == 0) {
> + ha_warning("We generated two equal cookies for two different servers.\n"
> + "Please change the secret key for '%s'.\n",
> + s->proxy->id);
> + }
> + }
> +
> +}
> +
> +void srv_set_dyncookie(struct server *s)
> +{
> + struct proxy *p = s->proxy;
> char *tmpbuf;
> unsigned long long hash_value;
> size_t key_len;
> @@ -136,21 +160,13 @@ void srv_set_dyncookie(struct server *s)
> if (!s->cookie)
> return;
> s->cklen = 16;
> - /*
> - * Check that we did not get a hash collision.
> - * Unlikely, but it can happen.
> +
> + /* Don't bother checking if the dyncookie is duplicated if
> + * the server is marked as "disabled", maybe it doesn't have
> + * its real IP yet, but just a place holder.
> */
> - for (tmpserv = p->srv; tmpserv != NULL;
> - tmpserv = tmpserv->next) {
> - if (tmpserv == s)
> - continue;
> - if (tmpserv->cookie &&
> - strcmp(tmpserv->cookie, s->cookie) == 0) {
> - ha_warning("We generated two equal cookies for two different servers.\n"
> - "Please change the secret key for '%s'.\n",
> - s->proxy->id);
> - }
> - }
> + if (!(s->next_admin & SRV_ADMF_FMAINT))
> + srv_check_for_dup_dyncookie(s);
> }
>
> /*
> @@ -4398,6 +4414,10 @@ static int cli_parse_enable_server(char **args, struct
> appctx *appctx, void *pri
> return 1;
>
> srv_adm_set_ready(sv);
> + if (!(sv->flags & SRV_F_COOKIESET)
> + && (sv->proxy->ck_opts & PR_CK_DYNAMIC) &&
> + sv->cookie)
> + srv_check_for_dup_dyncookie(sv);
> return 1;
> }
>
> --
> 2.14.3
