From 3f4ef206985c8934ffef4371ba5ecafa9b7ddf54 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Tue, 13 Aug 2024 17:49:46 +0200 Subject: [PATCH] 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. (cherry picked from commit 41ca89bc6fe96a660fea992643dbc2c0844a609e) [ada: ctx adjt] Signed-off-by: Aurelien DARRAGON --- src/log.c | 55 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/src/log.c b/src/log.c index ef144db2c2487..a663d7acdca92 100644 --- a/src/log.c +++ b/src/log.c @@ -950,20 +950,23 @@ static int lf_expr_postcheck_node_opt(struct lf_expr *lf_expr, struct logformat_ /* Performs a postparsing check on logformat expression for a given * 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, 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), 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; @@ -976,13 +979,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 */ @@ -998,15 +1006,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; } @@ -1018,11 +1028,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 node depends on HTTP mode)"); - goto fail; - } return 1; fail: