[locker95]

Fixes #12788.

flb_upstream_ha_from_file() had two problems with YAML upstream files:

• With more than one entry under upstream_servers, each group was created but only the last one was returned, so the earlier groups leaked (and they were picked silently). Now the previous group is destroyed and a warning says which one is used. I kept the "last group wins" behavior so existing configs don't start failing, it's just explicit now.
• ups wasn't initialized, so a YAML file with no upstream_servers hit flb_upstream_ha_destroy() on garbage in the c == 0 path. It's NULL-initialized now and only destroyed when set.

While there I also switched the "cannot register node" errors to ups->name, since tmp had already been freed at that point, and freed the current group when a later entry is missing name.

Tested with out_forward + upstream: pointing at the files from the issue, under valgrind:

• two groups, master: definitely lost: 32 bytes ... flb_upstream_ha_create <- flb_upstream_ha_from_file. patched: warning ignoring 'group-a' and using 'group-b', no leak from upstream_ha.
• file with only service:, master: Use of uninitialised value ... flb_upstream_ha_destroy (flb_upstream_ha.c:70) then SIGSEGV. patched: no nodes defined and a clean exit.
• classic [UPSTREAM]/[NODE] file: unchanged, 0 valgrind errors.

(the failed-init cases still show a 112 byte leak from cb_forward_init, that's out_forward's own error path and not touched here.)

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling when upstream configuration is missing required information or contains no nodes.
  • Configuration loading now handles files with multiple upstream groups consistently and reports clearer details when node registration fails.

[chatgpt-codex-connector]

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-14T07:23:03.045673Z	f37e044	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The upstream HA file loader initializes its context pointer, cleans up contexts on failure and when it encounters another YAML group, and reports the created upstream’s name in node-registration errors.

Changes: Upstream HA file loading

Layer / File(s)	Summary
YAML group handling and cleanup src/flb_upstream_ha.c	The loader initializes ups to NULL, warns when a YAML file contains multiple upstream groups, destroys the previously selected context, and uses the last group. Failure paths destroy the context only when it exists. Node-registration errors now report the created upstream’s name.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Minimal · up to f37e0

This change fixes a memory leak when a YAML upstream file defines multiple groups. It warns about the ignored groups and keeps the existing last-group-wins behavior. It also makes the no-groups error path safe. No merge-blocking risk was found.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: fixing YAML upstream file group handling in the upstream_ha component. The changeset handles multiple YAML upstream groups (warning, destroying the previous context, k…
Linked Issues check		Issue #12788 has two coding requirements. (1) With multiple YAML groups, the file must either fail clearly or log which group is used, without leaking the others. The diff in src/flb_upstream_ha.c add…
Out of Scope Changes check		All hunks in the whole-PR diff (merge base 27d5624 to head f37e044) touch only src/flb_upstream_ha.c and relate to flb_upstream_ha_from_file(). The NULL initialization, the multi-group warning and…

Full details: Title check

Explanation

The title names the main change: fixing YAML upstream file group handling in the upstream_ha component. The changeset handles multiple YAML upstream groups (warning, destroying the previous context, keeping last-group-wins) and initializes and cleans up ups correctly, which matches the title's subject. The title is short, specific, and accurate.

Full details: Linked Issues check

Explanation

Issue #12788 has two coding requirements. (1) With multiple YAML groups, the file must either fail clearly or log which group is used, without leaking the others. The diff in src/flb_upstream_ha.c adds a check inside the YAML loop. When ups is already set, it logs flb_warn naming the ignored group (ups->name) and the used group (tmp), then calls flb_upstream_ha_destroy(ups) before creating the next group. This satisfies the logging and no-leak requirement. The PR keeps last-group-wins, which the issue allows as a documented or logged behavior. (2) ups must be NULL-initialized and destroyed on the c == 0 path only when set. The diff sets struct flb_upstream_ha *ups = NULL; and guards flb_upstream_ha_destroy(ups) with if (ups). Both requirements are met in code. The diff also fixes the tmp use-after-free in the 'cannot register node' errors (now ups->name) and destroys the current group when a later entry lacks name. These support the same error-path cleanup described in the issue. No automated test was added, but the issue does not request one.

Full details: Out of Scope Changes check

Explanation

All hunks in the whole-PR diff (merge base 27d5624 to head f37e044) touch only src/flb_upstream_ha.c and relate to flb_upstream_ha_from_file(). The NULL initialization, the multi-group warning and cleanup, the guarded destroy on the no-nodes path, the ups->name error messages, and the missing-name cleanup all address issue #12788 or the error-path leaks it describes. The classic [UPSTREAM] path is not functionally altered except for the error message argument, which is the same fix. No unrelated changes were found in the diff.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 42cd72e6-997f-44af-9d72-e889c1ca5652

Commits

Reviewing files that changed from the base of the PR and between 27d5624 and f37e044.

Files selected for processing: 1

• src/flb_upstream_ha.c

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.