filter_kubernetes: fix parser annotation leak

[erain]

Summary

• free previous parser SDS when annotations override defaults
• add runtime test/data for parser-first annotation order\n\n

Testing

valgrind --leak-check=full --show-leak-kinds=definite --errors-for-leak-kinds=definite --error-exitcode=1 build/valgrind/bin/flb-rt-filter_kubernetes --no-exec

Summary by CodeRabbit

• Bug Fixes

  • Improved cleanup when updating parser configuration to prevent stale data and ensure proper state management.

• Tests

  • Added a test and accompanying metadata exercising multiple parser annotations and annotation ordering across containers.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds defensive cleanup in prop_set_parser to destroy existing stdout_parser and stderr_parser SDS strings before reassigning them, and adds a new test plus metadata fixture exercising multiple parser annotations.

Changes

Cohort / File(s)	Summary
Memory safety cleanup plugins/filter_kubernetes/kube_property.c	Destroy existing stdout_parser/stderr_parser (via flb_sds_destroy) when non-NULL before assigning new parser strings in prop_set_parser.
Tests and test data tests/runtime/filter_kubernetes.c, tests/runtime/data/kubernetes/.../annotations-parser-order_multiple-1.meta	Add new test flb_test_annotations_parser_order_multiple_1_container_1_stdout and corresponding metadata fixture containing multiple fluentbit.io/parser* annotations to exercise parser selection order.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

backport to v4.0.x, backport to v4.1.x

Suggested reviewers

• edsiper

Poem

🐰 I tidy strings with nimble paws,

Destroy the old to mend the flaws.
Parsers line up, neat and bright—
No leaks tonight, the code hops light. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'filter_kubernetes: fix parser annotation leak' accurately and concisely describes the main change—fixing a memory leak in the Kubernetes filter's parser annotation handling.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e86c8ac and f9b90cf.

⛔ Files ignored due to path filters (2)

• tests/runtime/data/kubernetes/log/annotations-parser/annotations-parser-order_multiple-1_container-1.log is excluded by !**/*.log
• tests/runtime/data/kubernetes/out/annotations-parser/annotations-parser-order_multiple-1_container-1_stdout.out is excluded by !**/*.out

📒 Files selected for processing (3)

• plugins/filter_kubernetes/kube_property.c
• tests/runtime/data/kubernetes/meta/annotations-parser-order_multiple-1.meta
• tests/runtime/filter_kubernetes.c

✅ Files skipped from review due to trivial changes (1)

• tests/runtime/data/kubernetes/meta/annotations-parser-order_multiple-1.meta

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)

• GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
• GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
• GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
• GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
• GitHub Check: pr-compile-centos-7
• GitHub Check: pr-compile-without-cxx (3.31.6)
• GitHub Check: PR - fuzzing test

🔇 Additional comments (3)

plugins/filter_kubernetes/kube_property.c (1)

87-89: LGTM! Memory leak fix is correct.

The defensive cleanup properly addresses the leak scenario: when container-specific or stream-specific parser annotations override default parser annotations, the existing SDS string is now freed before reassignment. The NULL checks are appropriate defensive coding.

Also applies to: 96-98

tests/runtime/filter_kubernetes.c (2)

634-638: LGTM! Test function is well-structured.

The new test function follows the established pattern and correctly exercises the annotation parser order variant where default parser annotations precede container-specific ones, which is the scenario that triggered the memory leak.

---

1054-1054: LGTM! Test registration is correct.

The new test is properly registered in TEST_LIST with consistent naming and placement.

---

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[erain]

Detailed bug/fix notes:

• The kubernetes filter parses annotations into flb_kube_props via flb_kube_prop_set(). In prop_set_parser(), we assign props->stdout_parser / props->stderr_parser using flb_sds_create().
• When a default parser annotation (fluentbit.io/parser) is processed before a container- or stream-specific parser annotation, the code overwrote the existing SDS pointer without freeing it, leaking the previous allocation.
• This is order-dependent and becomes a steady leak in clusters with many pods/containers and multiple parser annotations.
• Fix: destroy any existing SDS before overwriting props->stdout_parser / props->stderr_parser, then assign the new value.
• Added a runtime test/data variant that orders fluentbit.io/parser before container-specific parsers to exercise the overwrite path without changing existing fixtures.

[erain]

Valgrind report (full filter_kubernetes runtime suite):

Command:
valgrind --leak-check=full --show-leak-kinds=definite --errors-for-leak-kinds=definite --error-exitcode=1 build/valgrind/bin/flb-rt-filter_kubernetes --no-exec

Result:
SUCCESS: All unit tests have passed.
HEAP SUMMARY:
in use at exit: 0 bytes in 0 blocks
total heap usage: 331,572 allocs, 331,572 frees, 115,359,417 bytes allocated
ERROR SUMMARY: 0 errors from 0 contexts

[erain]

Valgrind report without the fix (baseline commit a981f66):

To exercise the order-dependent leak, I used the same runtime test but reordered the annotations in the metadata file so fluentbit.io/parser appears before container-specific parsers, and updated the expected output accordingly (local-only in the baseline worktree).

Command:
valgrind --leak-check=full --show-leak-kinds=definite --errors-for-leak-kinds=definite --error-exitcode=1 build/valgrind/bin/flb-rt-filter_kubernetes --no-exec kube_annotations_parser_multiple_1_container_1_stdout

Result:
Test kube_annotations_parser_multiple_1_container_1_stdout... [ OK ]
HEAP SUMMARY:
in use at exit: 62 bytes in 2 blocks
total heap usage: 3,613 allocs, 3,611 frees, 1,317,649 bytes allocated
LEAK SUMMARY:
definitely lost: 62 bytes in 2 blocks
Stack traces point to prop_set_parser() in kube_property.c (lines 87/93) via flb_kube_prop_set().

[edsiper]

@erain thanks for this contribution!

would you please split the commits in 2 ? , e.g:

• filter_kubernetes: ...
• tests: runtime: kubernetes: ...

[erain]

@erain thanks for this contribution!

would you please split the commits in 2 ? , e.g:

• filter_kubernetes: ...
• tests: runtime: kubernetes: ...

Thanks a lot for the review! I have split the commit into two as mentioned and updated the PR.

Also, since this is for fixing a memory leak problem, is it possible to backport this fix to 4.0.x and 4.1.x releases as well? (GKE is still on 4.0.x at the moment)

[edsiper]

@cosmo0920 would you please help with backport of this one ?

[cosmo0920]

Yes, let's do that.