MEDIUM: log: relax some checks and emit diag warnings instead in lf_expr_postcheck()

With 7a21c3a ("MAJOR: log: implement proper postparsing for logformat
expressions") which finally made postparsing checks reliable, we started
to get report from users that couldn't start haproxy 3.0 with configs that
used to work in the past. The current situation is described in GH #2642.

While the checks are mostly relevant, it turns out there are not strictly
needed anymore from a technical point of view. Most of them were useful in
early logformat implementation to prevent runtime bugs due to the use of
an alias or fetch at runtime from an incompatible proxy. It's been a few
versions already that the code handling fetches and log aliases is robust
enough to support fetches/aliases used from the wrong context: all it
does is that the fetch/alias will silently fail if it's not available.

This can be proved by the fact that even if the postparsing checks were
partially broken in the past, it didn't cause runtime issues (at least
on recent haproxy versions).

Most of these checks can now be seen as configuration hints: when a check
triggers, it will indicate a configuration inconsistency in most cases,
but they are some corner cases where it is not possible to know at config
time if the conditions will be met for the alias/fetch to work properly..
so instead of failing with a hard error like we did so far, let's just be
more permissive and report our findings using "diag_warning": such
warnings are only emitted when haproxy is started with '-dD' cli option.

We also took this opportunity to improve messages clarity and make them
more precise (report the offending item instead of complaining about the
whole expression because of a single element).

With this patch, configs that used to start before 7a21c3a shouldn't
trigger hard errors anymore.

This may be backported in 3.0.

diff --git a/src/log.c b/src/log.c
index a58c6fc3cf7cc9a0791c2a58cfe89697f6b7e986..fd8be525fd21e0bf659f40ae15ec3967a83baa2b 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -989,20 +989,23 @@ static int lf_expr_postcheck_node_opt(struct lf_expr *lf_expr, struct logformat_
 /* Performs a postparsing check on logformat expression <expr> for a given <px>
  * proxy. The function will behave differently depending on the proxy state
  * (during parsing we will try to adapt proxy configuration to make it
- * compatible with logformat expression, but once the proxy is checked, we fail
- * as soon as we face incompatibilities)
+ * compatible with logformat expression, but once the proxy is checked, we
+ * cannot help anymore so all we can do is raise a diag warning and hope that
+ * the conditions will be met when executing the logformat expression)
  *
- * If the proxy is a default section, then allow the postcheck to succeed:
- * the logformat expression may or may not work properly depending on the
- * actual proxy that effectively runs it during runtime, but we have to stay
- * permissive since we cannot assume it won't work.
+ * If the proxy is a default section, then allow the postcheck to succeed
+ * without errors nor warnings: the logformat expression may or may not work
+ * properly depending on the actual proxy that effectively runs it during
+ * runtime, but we have to stay permissive since we cannot assume it won't work.
  *
- * It returns 1 on success and 0 on error, <err> will be set in case of error.
+ * It returns 1 on success (although diag_warnings may have been emitted) and 0
+ * on error (cannot be recovered from), <err> will be set in case of error.
  */
 int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err)
 {
        struct logformat_node *lf;
        int default_px = (px->cap & PR_CAP_DEF);
+       uint8_t http_mode = (px->mode == PR_MODE_HTTP || (px->options & PR_O_HTTP_UPG));
 
        if (!(px->flags & PR_FL_CHECKED))
                px->to_log |= LW_INIT;
@@ -1015,13 +1018,18 @@ int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err)
                        struct sample_expr *expr = lf->expr;
                        uint8_t http_needed = !!(expr->fetch->use & SMP_USE_HTTP_ANY);
 
+                       if (!default_px && !http_mode && http_needed)
+                               ha_diag_warning("parsing [%s:%d]: sample fetch '%s' used from %s '%s' may not work as expected (item depends on HTTP proxy mode).\n",
+                                               lf_expr->conf.file, lf_expr->conf.line,
+                                               expr->fetch->kw,
+                                               proxy_type_str(px), px->id);
+
                        if ((px->flags & PR_FL_CHECKED)) {
-                               /* fail as soon as proxy properties are not compatible */
-                               if (http_needed && !px->http_needed) {
-                                       memprintf(err, "sample fetch '%s' requires HTTP enabled proxy which is not available here",
-                                                 expr->fetch->kw);
-                                       goto fail;
-                               }
+                               if (http_needed && !px->http_needed)
+                                       ha_diag_warning("parsing [%s:%d]: sample fetch '%s' used from %s '%s' requires HTTP-specific proxy attributes, but the current proxy lacks them.\n",
+                                                       lf_expr->conf.file, lf_expr->conf.line,
+                                                       expr->fetch->kw,
+                                                       proxy_type_str(px), px->id);
                                goto next_node;
                        }
                        /* check if we need to allocate an http_txn struct for HTTP parsing */
@@ -1037,15 +1045,17 @@ int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err)
                                px->to_log |= LW_REQ;
                }
                else if (lf->type == LOG_FMT_ALIAS) {
-                       if (lf->alias->mode == PR_MODE_HTTP &&
-                           !default_px && px->mode != PR_MODE_HTTP) {
-                               memprintf(err, "format alias '%s' is reserved for HTTP mode",
-                                         lf->alias->name);
-                               goto fail;
-                       }
+                       if (!default_px && !http_mode &&
+                           (lf->alias->mode == PR_MODE_HTTP ||
+                           (lf->alias->lw & ((LW_REQ | LW_RESP)))))
+                               ha_diag_warning("parsing [%s:%d]: format alias '%s' used from %s '%s' may not work as expected (item depends on HTTP proxy mode).\n",
+                                               lf_expr->conf.file, lf_expr->conf.line,
+                                               lf->alias->name,
+                                               proxy_type_str(px), px->id);
+
                        if (lf->alias->config_callback &&
                            !lf->alias->config_callback(lf, px)) {
-                               memprintf(err, "cannot configure format alias '%s' in this context",
+                               memprintf(err, "error while configuring format alias '%s'",
                                          lf->alias->name);
                                goto fail;
                        }
@@ -1057,11 +1067,6 @@ int lf_expr_postcheck(struct lf_expr *lf_expr, struct proxy *px, char **err)
                if (!lf_expr_postcheck_node_opt(lf_expr, lf, err))
                        goto fail;
        }
-       if (!default_px && (px->to_log & (LW_REQ | LW_RESP)) &&
-           (px->mode != PR_MODE_HTTP && !(px->options & PR_O_HTTP_UPG))) {
-               memprintf(err, "logformat expression not usable here (at least one item depends on HTTP mode)");
-               goto fail;
-       }
 
        return 1;
  fail: