-
Notifications
You must be signed in to change notification settings - Fork 1.9k
filter_kuberntes: fix accidentally removed u in prefix #11365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughA regex pattern fix is applied to the Kubernetes filter's pod name extraction logic, changing a capturing group to a non-capturing group. This addresses a bug where pod names starting with "u" lose their first character. A test case with a pod named "uat-myapp-12345" is added to verify the fix. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-14T18:14:41.453ZApplied to files:
🪛 Checkov (3.2.334)tests/runtime/data/kubernetes/meta/short-prefix-uat-podname.json[medium] 2-17: Containers should not run with allowPrivilegeEscalation (CKV_K8S_20) [medium] 2-17: Minimize the admission of root containers (CKV_K8S_23) 🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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. Comment |
67aa0a2 to
ce21b18
Compare
ce21b18 to
9cf4c9f
Compare
Signed-off-by: Hiroshi Hatake <[email protected]>
…n regex Signed-off-by: Hiroshi Hatake <[email protected]>
50e62c9 to
6343780
Compare
Signed-off-by: Hiroshi Hatake <[email protected]>
6343780 to
5fbd345
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/runtime/filter_kubernetes.c (1)
71-110: Nice fix adding NUL-termination—consider hardening empty-file handling to avoid underflow in the trim logic.If an
.outfile is empty (out_size == 0),p = out_buf + (out_size - 1)underflows; alsofread(..., st.st_size, 1, ...)fails forst.st_size == 0.Suggested hardening
static int file_to_buf(const char *path, char **out_buf, size_t *out_size) { int ret; - long bytes; + size_t bytes; char *buf; FILE *fp; struct stat st; + size_t sz; ret = stat(path, &st); if (ret == -1) { return -1; } + sz = (size_t) st.st_size; fp = fopen(path, "r"); if (!fp) { return -1; } - buf = flb_malloc(st.st_size + 1); + buf = flb_malloc(sz + 1); if (!buf) { flb_errno(); fclose(fp); return -1; } - bytes = fread(buf, st.st_size, 1, fp); - if (bytes != 1) { + bytes = fread(buf, 1, sz, fp); + if (bytes != sz) { flb_errno(); flb_free(buf); fclose(fp); return -1; } - buf[st.st_size] = '\0'; + buf[sz] = '\0'; fclose(fp); *out_buf = buf; - *out_size = st.st_size; + *out_size = sz; return 0; } @@ /* Sanitize content, get rid of ending \n */ - p = out_buf + (out_size - 1); - while (*p == '\n' || *p == '\r') p--; - *++p = '\0'; + p = out_buf + out_size; + while (p > out_buf && (p[-1] == '\n' || p[-1] == '\r')) p--; + *p = '\0';Also applies to: 134-139
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/runtime/data/kubernetes/log/core/core_uat-myapp-12345_fluent-bit.logis excluded by!**/*.logtests/runtime/data/kubernetes/out/core/core_uat-myapp-12345_fluent-bit.outis excluded by!**/*.out
📒 Files selected for processing (3)
plugins/filter_kubernetes/kube_regex.htests/runtime/data/kubernetes/meta/short-prefix-uat-podname.jsontests/runtime/filter_kubernetes.c
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: zhihonl
Repo: fluent/fluent-bit PR: 10586
File: plugins/filter_kubernetes/kube_regex.h:29-29
Timestamp: 2025-08-14T18:14:41.453Z
Learning: Kubernetes ReplicaSet suffixes generated from pod-template-hash can vary in length and are not always 10 characters. Examples include "9dbdfc456" (9 chars) from "amazon-cloudwatch-observability-controller-manager-9dbdfc456" and "914055854" (9 chars) from "kairosdb-914055854". The DEPLOYMENT_REGEX pattern using {6,10} range for the ID group correctly accommodates this variable length behavior.
Learnt from: zhihonl
Repo: fluent/fluent-bit PR: 10586
File: plugins/filter_kubernetes/kube_regex.h:29-29
Timestamp: 2025-08-14T18:14:41.453Z
Learning: Kubernetes ReplicaSet suffixes are not always 10 characters. The suffix length can vary, as demonstrated by the example "amazon-cloudwatch-observability-controller-manager-9dbdfc456" which has a 9-character suffix. The original DEPLOYMENT_REGEX pattern using {6,10} range for the ID group is more accurate than assuming a fixed 10-character length.
Learnt from: hlein
Repo: fluent/fluent-bit PR: 11168
File: conf/parsers_mult.yaml:8-14
Timestamp: 2025-11-16T22:16:26.032Z
Learning: In Fluent Bit parser configurations (both .conf and .yaml formats), the regex engine automatically strips leading and trailing `/` characters from regex patterns, so patterns like `/Processing by .../` and `Processing by ...` are functionally equivalent and both work correctly.
📚 Learning: 2025-08-14T18:14:41.453Z
Learnt from: zhihonl
Repo: fluent/fluent-bit PR: 10586
File: plugins/filter_kubernetes/kube_regex.h:29-29
Timestamp: 2025-08-14T18:14:41.453Z
Learning: Kubernetes ReplicaSet suffixes generated from pod-template-hash can vary in length and are not always 10 characters. Examples include "9dbdfc456" (9 chars) from "amazon-cloudwatch-observability-controller-manager-9dbdfc456" and "914055854" (9 chars) from "kairosdb-914055854". The DEPLOYMENT_REGEX pattern using {6,10} range for the ID group correctly accommodates this variable length behavior.
Applied to files:
plugins/filter_kubernetes/kube_regex.h
🪛 Checkov (3.2.334)
tests/runtime/data/kubernetes/meta/short-prefix-uat-podname.json
[medium] 2-17: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 2-17: Minimize the admission of root containers
(CKV_K8S_23)
🔇 Additional comments (3)
tests/runtime/filter_kubernetes.c (1)
415-423: No action required—all required fixture files already exist.The fixture files for
kube_short_prefix_uat_podnameare present:
tests/runtime/data/kubernetes/log/core/core_uat-myapp-12345_fluent-bit.log✓tests/runtime/data/kubernetes/out/core/core_uat-myapp-12345_fluent-bit.out✓tests/runtime/data/kubernetes/meta/short-prefix-uat-podname.json✓The test is properly registered at line 1128 in the test array.
Likely an incorrect or invalid review comment.
tests/runtime/data/kubernetes/meta/short-prefix-uat-podname.json (1)
1-16: No action needed—test fixtures are not scanned by any security tool in CI.This repository uses Trivy (only for container image scanning), not Checkov. The
tests/runtime/data/directory is not gated by any security scanner, and test fixtures do not require production-level security configurations. The fixture is valid as-is.Likely an incorrect or invalid review comment.
plugins/filter_kubernetes/kube_regex.h (1)
25-25: No action needed – regex extraction uses only named capture groups, not numeric positions.The code uses
flb_regex_parse()with a callback that extracts match values by group name (viastrcmp()on "pod_name", "namespace_name", "container_name", "docker_id"), not by numeric capture indices. Non-capturing groups(?:...)do not affect named group extraction, so the change is safe.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Closes #11322
In the current filter_kubernetes, the pod_name placeholder on regex sometimes causes backtracking and overlooked the beginning letter of u.
With this patch, preventing to execute backtracking and plug the occurrences of missing u prefix in pod_name.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.