MEDIUM: httpclient/logs: rely on per-proxy post-check instead of global one

httpclient used to register a global post-check function to iterate over
all known proxies and post-initialize httpclient related ones (mainly
for logs initialization).

But we currently have an issue: post_sink_resolve() function which is
also registered using REGISTER_POST_CHECK() macro conflicts with
httpclient_postcheck() function.

This is because post_sink_resolve() relies on proxy->logsrvs to be
correctly initialized already, and httpclient_postcheck() may create
and insert new logsrvs entries to existing proxies when executed.

So depending on which function runs first, we could run into trouble.

Hopefully, to this day, everything works "by accident" due to
http_client.c file being loaded before sink.c file when compiling source
code.

But as soon as we would move one of the two functions to other files, or
if we rename files or make changes to the Makefile build recipe, we could
break this at any time.

To prevent post_sink_resolve() from randomly failing in the future, we now
make httpclient postcheck rely on per-proxy post-checks by slightly
modifying httpclient_postcheck() function so that it can be registered
using REGISTER_POST_PROXY_CHECK() macro.

As per-proxy post-check functions are executed right after config parsing
for each known proxy (vs global post-check which are executed a bit later
in the init process), we can be certain that functions registered using
global post-check macro, ie: post_sink_resolve(), will always be executed
after httpclient postcheck, effectively resolving the ordering conflict.

This should normally not cause visible behavior changes, and while it
could be considered as a bug, it's probably not worth backporting it
since the only way to trigger the issue is through code refactors,
unless we want to backport it to ease code maintenance of course,
in which case it should easily apply for >= 2.7.

diff --git a/src/http_client.c b/src/http_client.c
index 21fdb80b2da77e402e502f1bd940d0b0c47a236f..8346f02f6db3c7b5f7ef419e2f493b475d82a1f5 100644 (file)
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -1350,11 +1350,11 @@ static int httpclient_precheck()
        return 0;
 }
 
-static int httpclient_postcheck()
+/* Initialize the logs for every proxy dedicated to the httpclient */
+static int httpclient_postcheck_proxy(struct proxy *curproxy)
 {
        int err_code = ERR_NONE;
        struct logsrv *logsrv;
-       struct proxy *curproxy = NULL;
        char *errmsg = NULL;
 #ifdef USE_OPENSSL
        struct server *srv = NULL;
@@ -1364,61 +1364,57 @@ static int httpclient_postcheck()
        if (global.mode & MODE_MWORKER_WAIT)
                return ERR_NONE;
 
-       /* Initialize the logs for every proxy dedicated to the httpclient */
-       for (curproxy = proxies_list; curproxy; curproxy = curproxy->next) {
+       if (!(curproxy->cap & PR_CAP_HTTPCLIENT))
+               return ERR_NONE; /* nothing to do */
 
-               if (!(curproxy->cap & PR_CAP_HTTPCLIENT))
-                       continue;
-
-               /* copy logs from "global" log list */
-               list_for_each_entry(logsrv, &global.logsrvs, list) {
-                       struct logsrv *node = malloc(sizeof(*node));
+       /* copy logs from "global" log list */
+       list_for_each_entry(logsrv, &global.logsrvs, list) {
+               struct logsrv *node = malloc(sizeof(*node));
 
-                       if (!node) {
-                               memprintf(&errmsg, "out of memory.");
-                               err_code |= ERR_ALERT | ERR_FATAL;
-                               goto err;
-                       }
-
-                       memcpy(node, logsrv, sizeof(*node));
-                       LIST_INIT(&node->list);
-                       LIST_APPEND(&curproxy->logsrvs, &node->list);
-                       node->ring_name = logsrv->ring_name ? strdup(logsrv->ring_name) : NULL;
-                       node->conf.file = logsrv->conf.file ? strdup(logsrv->conf.file) : NULL;
+               if (!node) {
+                       memprintf(&errmsg, "out of memory.");
+                       err_code |= ERR_ALERT | ERR_FATAL;
+                       goto err;
                }
-               if (curproxy->conf.logformat_string) {
-                       curproxy->conf.args.ctx = ARGC_LOG;
-                       if (!parse_logformat_string(curproxy->conf.logformat_string, curproxy, &curproxy->logformat,
-                                                   LOG_OPT_MANDATORY|LOG_OPT_MERGE_SPACES,
-                                                   SMP_VAL_FE_LOG_END, &errmsg)) {
-                               memprintf(&errmsg, "failed to parse log-format : %s.", errmsg);
-                               err_code |= ERR_ALERT | ERR_FATAL;
-                               goto err;
-                       }
-                       curproxy->conf.args.file = NULL;
-                       curproxy->conf.args.line = 0;
+
+               memcpy(node, logsrv, sizeof(*node));
+               LIST_INIT(&node->list);
+               LIST_APPEND(&curproxy->logsrvs, &node->list);
+               node->ring_name = logsrv->ring_name ? strdup(logsrv->ring_name) : NULL;
+               node->conf.file = logsrv->conf.file ? strdup(logsrv->conf.file) : NULL;
+       }
+       if (curproxy->conf.logformat_string) {
+               curproxy->conf.args.ctx = ARGC_LOG;
+               if (!parse_logformat_string(curproxy->conf.logformat_string, curproxy, &curproxy->logformat,
+                                           LOG_OPT_MANDATORY|LOG_OPT_MERGE_SPACES,
+                                           SMP_VAL_FE_LOG_END, &errmsg)) {
+                       memprintf(&errmsg, "failed to parse log-format : %s.", errmsg);
+                       err_code |= ERR_ALERT | ERR_FATAL;
+                       goto err;
                }
+               curproxy->conf.args.file = NULL;
+               curproxy->conf.args.line = 0;
+       }
 
 #ifdef USE_OPENSSL
-               /* initialize the SNI for the SSL servers */
+       /* initialize the SNI for the SSL servers */
 
-               for (srv = curproxy->srv; srv != NULL; srv = srv->next) {
-                       if (srv->xprt == xprt_get(XPRT_SSL)) {
-                               srv_ssl = srv;
-                       }
+       for (srv = curproxy->srv; srv != NULL; srv = srv->next) {
+               if (srv->xprt == xprt_get(XPRT_SSL)) {
+                       srv_ssl = srv;
                }
-               if (srv_ssl && !srv_ssl->sni_expr) {
-                       /* init the SNI expression */
-                       /* always use the host header as SNI, without the port */
-                       srv_ssl->sni_expr = strdup("req.hdr(host),field(1,:)");
-                       err_code |= server_parse_sni_expr(srv_ssl, curproxy, &errmsg);
-                       if (err_code & ERR_CODE) {
-                               memprintf(&errmsg, "failed to configure sni: %s.", errmsg);
-                               goto err;
-                       }
+       }
+       if (srv_ssl && !srv_ssl->sni_expr) {
+               /* init the SNI expression */
+               /* always use the host header as SNI, without the port */
+               srv_ssl->sni_expr = strdup("req.hdr(host),field(1,:)");
+               err_code |= server_parse_sni_expr(srv_ssl, curproxy, &errmsg);
+               if (err_code & ERR_CODE) {
+                       memprintf(&errmsg, "failed to configure sni: %s.", errmsg);
+                       goto err;
                }
-#endif
        }
+#endif
 
 err:
        if (err_code & ERR_CODE) {
@@ -1432,7 +1428,7 @@ err:
 /* initialize the proxy and servers for the HTTP client */
 
 REGISTER_PRE_CHECK(httpclient_precheck);
-REGISTER_POST_CHECK(httpclient_postcheck);
+REGISTER_POST_PROXY_CHECK(httpclient_postcheck_proxy);
 
 static int httpclient_parse_global_resolvers(char **args, int section_type, struct proxy *curpx,
                                         const struct proxy *defpx, const char *file, int line,