Hi William,

On Mon, Jan 22, 2018 at 08:03:55PM +0100, William Dauchy wrote:
> 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,
> 

Thanks for testing !

Willy, can you please push it ?

Regards,

Olivier
>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.
---
 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

Reply via email to