Skip to content

Commit

Permalink
MEDIUM: log: relax some checks and emit diag warnings instead in lf_e…
Browse files Browse the repository at this point in the history
…xpr_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 haproxy#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.
  • Loading branch information
Darlelet committed Aug 16, 2024
1 parent 9788ae1 commit 41ca89b
Showing 1 changed file with 30 additions and 25 deletions.
55 changes: 30 additions & 25 deletions src/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 */
Expand All @@ -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;
}
Expand All @@ -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:
Expand Down

0 comments on commit 41ca89b

Please sign in to comment.