filter_kubernetes: use service account issuer to detect EKS env #11002
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaced ConfigMap-based EKS detection with serviceaccount JWT issuer inspection and removed the exported Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FLB as Fluent Bit
participant K8S as kubernetes_aws.c
participant FS as Filesystem
Note right of K8S #F0F4FF: determine_platform(ctx)
K8S->>FS: read serviceaccount token file
alt read fails
K8S-->>FLB: return -1
else token read
K8S->>K8S: split JWT (header.payload.signature)
alt malformed JWT
K8S-->>FLB: return -1
else JWT ok
K8S->>K8S: base64-decode payload
alt decode fails
K8S-->>FLB: return -1
else decoded payload
K8S->>K8S: extract "iss" value
alt issuer matches oidc.eks.*.amazonaws.com/id/...
Note right of K8S #DFF2E1: EKS detected
K8S-->>FLB: return 1
else no match
K8S-->>FLB: return -1
end
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)plugins/filter_kubernetes/kubernetes_aws.c (2)
🔇 Additional comments (4)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
| ret = flb_base64_decode((unsigned char *)payload, payload_b64_len * 3 / 4 + 4, | ||
| &payload_len, (unsigned char *)payload_b64, payload_b64_len); |
There was a problem hiding this comment.
determine_platform decodes the service account token payload via flb_base64_decode, which only accepts standard Base64 characters (+ and /) and padding. Kubernetes service-account tokens are JWTs encoded with Base64URL (- and _, typically unpadded), so this call fails with FLB_BASE64_ERR_INVALID_CHARACTER and the function always returns -1. As a result EKS environments are never detected and the platform stays k8s, breaking Explore Related tagging on EKS clusters. Use a Base64URL decoder or normalize the payload before decoding.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/filter_kubernetes/kubernetes_aws.c (1)
297-300: Validate token_size after reading the Kubernetes token
Token constant is defined (kube_meta.h:55). Add a check thattoken_size > 0andtoken_size <= FLB_KUBE_TOKEN_BUF_SIZEbefore using the buffer to guard against empty or overly large token reads.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/filter_kubernetes/kube_conf.h(1 hunks)plugins/filter_kubernetes/kubernetes_aws.c(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/filter_kubernetes/kubernetes_aws.c (2)
src/flb_utils.c (1)
flb_utils_read_file(1937-1940)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
🔇 Additional comments (5)
plugins/filter_kubernetes/kube_conf.h (1)
77-80: LGTM!The updated comment accurately reflects the new platform detection approach using serviceaccount token issuer inspection. The removal of the AWS_AUTH_CONFIG_MAP macro (mentioned in the summary) aligns with the shift away from ConfigMap-based detection.
plugins/filter_kubernetes/kubernetes_aws.c (4)
26-26: LGTM!The inclusion of
flb_utils.his necessary for theflb_utils_read_filefunction used in the new platform detection logic.
302-313: LGTM!The JWT structure validation correctly checks for the expected three-part format (header.payload.signature) and properly handles cleanup on parse failures.
315-346: Locate flb_base64_decode implementation.Run:
#!/bin/bash rg -n "flb_base64_decode" -g '*.c' -g '*.h' -C3
286-386: No changes needed for determine_platform return handling.The only caller in kube_meta.c correctly treats
ret == -1as non-EKS and all other values (i.e.1) as EKS.
| /* Look for "iss" field in the JSON payload */ | ||
| issuer_start = strstr(payload, "\"iss\":"); | ||
| if (!issuer_start) { | ||
| flb_free(payload); | ||
| return -1; | ||
| } | ||
|
|
||
| /* Skip to the value part */ | ||
| issuer_start = strchr(issuer_start, ':'); | ||
| if (!issuer_start) { | ||
| flb_free(payload); | ||
| return -1; | ||
| } | ||
| return -1; | ||
| issuer_start++; | ||
|
|
||
| /* Skip whitespace and opening quote */ | ||
| while (*issuer_start == ' ' || *issuer_start == '\t') issuer_start++; | ||
| if (*issuer_start != '"') { | ||
| flb_free(payload); | ||
| return -1; | ||
| } | ||
| issuer_start++; | ||
|
|
||
| /* Find closing quote */ | ||
| issuer_end = strchr(issuer_start, '"'); | ||
| if (!issuer_end) { | ||
| flb_free(payload); | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace manual JSON parsing with a proper parser.
The current string-based parsing for the "iss" field is fragile and may fail or produce false positives in the following scenarios:
- The "iss" field appears in a nested JSON object
- The issuer value contains escaped quotes
- Another field name contains "iss" as a substring (e.g., "missing")
Since the file already includes flb_jsmn.h (line 23), use the jsmn JSON parser for robust and reliable extraction of the issuer field.
Consider applying this approach:
/* Parse JSON payload using jsmn */
jsmn_parser parser;
jsmntok_t tokens[64]; /* Adjust size as needed */
int token_count;
jsmn_init(&parser);
token_count = jsmn_parse(&parser, payload, payload_len, tokens, sizeof(tokens)/sizeof(tokens[0]));
if (token_count < 0) {
flb_free(payload);
return -1;
}
/* Find and extract "iss" field value */
char *issuer = NULL;
size_t issuer_len = 0;
for (int i = 1; i < token_count; i++) {
if (tokens[i].type == JSMN_STRING &&
strncmp(payload + tokens[i].start, "iss", tokens[i].end - tokens[i].start) == 0) {
/* Next token is the value */
if (i + 1 < token_count && tokens[i + 1].type == JSMN_STRING) {
issuer = payload + tokens[i + 1].start;
issuer_len = tokens[i + 1].end - tokens[i + 1].start;
break;
}
}
}
if (!issuer) {
flb_free(payload);
return -1;
}🤖 Prompt for AI Agents
plugins/filter_kubernetes/kubernetes_aws.c lines 347-375: Replace the brittle
string-based "iss" extraction with jsmn parsing: initialize a jsmn_parser, parse
payload using payload_len into a tokens array (resize tokens if needed), check
token_count for errors and free payload on failure, then iterate tokens to find
a JSMN_STRING token whose content exactly equals "iss" (use
tokens[i].start/end), verify the next token exists and is a string, and set
issuer_start = payload + tokens[i+1].start and issuer_end = payload +
tokens[i+1].end; on any failure free payload and return -1; this handles nested
objects and escaped characters correctly.
| /* Check if issuer contains EKS OIDC URL pattern */ | ||
| /* EKS OIDC URLs follow pattern: https://oidc.eks.{region}.amazonaws.com/id/{cluster-id} */ | ||
| if (strstr(issuer_start, "oidc.eks.") && strstr(issuer_start, ".amazonaws.com/id/")) { | ||
| flb_free(payload); | ||
| return 1; /* EKS detected */ | ||
| } | ||
|
|
||
| flb_free(payload); | ||
| return -1; /* Not EKS */ |
There was a problem hiding this comment.
Fix pattern matching to search within issuer value only.
The current implementation uses strstr(issuer_start, ...) which searches the entire remaining payload string, not just the issuer value between issuer_start and issuer_end. This could produce false positives if the EKS patterns appear elsewhere in the JSON payload after the issuer field.
Apply this diff to limit the pattern search to the issuer value:
- /* Check if issuer contains EKS OIDC URL pattern */
- /* EKS OIDC URLs follow pattern: https://oidc.eks.{region}.amazonaws.com/id/{cluster-id} */
- if (strstr(issuer_start, "oidc.eks.") && strstr(issuer_start, ".amazonaws.com/id/")) {
- flb_free(payload);
- return 1; /* EKS detected */
- }
+ /* Check if issuer contains EKS OIDC URL pattern */
+ /* EKS OIDC URLs follow pattern: https://oidc.eks.{region}.amazonaws.com/id/{cluster-id} */
+ size_t issuer_len = issuer_end - issuer_start;
+ char *issuer_value = flb_strndup(issuer_start, issuer_len);
+ if (!issuer_value) {
+ flb_free(payload);
+ return -1;
+ }
+
+ int is_eks = (strstr(issuer_value, "oidc.eks.") && strstr(issuer_value, ".amazonaws.com/id/"));
+ flb_free(issuer_value);
+
+ if (is_eks) {
+ flb_free(payload);
+ return 1; /* EKS detected */
+ }🤖 Prompt for AI Agents
In plugins/filter_kubernetes/kubernetes_aws.c around lines 377 to 385, the code
currently calls strstr on issuer_start which searches the rest of the payload
and can hit patterns outside the issuer value; instead limit the search to only
the issuer substring between issuer_start and issuer_end. Extract or reference
the issuer value length (issuer_end - issuer_start) and either copy that range
into a NUL-terminated buffer or use a bounded search (e.g., memmem with the
length) to look for "oidc.eks." and ".amazonaws.com/id/" only inside that issuer
value, then free any temporary buffer and return 1 for EKS or -1 otherwise.
movence
force-pushed
the
aws-eks-detection
branch
from
721ae39 to
1a27fbf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/filter_kubernetes/kube_conf.h(1 hunks)plugins/filter_kubernetes/kubernetes_aws.c(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/filter_kubernetes/kubernetes_aws.c
🔇 Additional comments (1)
plugins/filter_kubernetes/kube_conf.h (1)
77-80: All references to AWS_AUTH_CONFIG_MAP have been removed
Search forAWS_AUTH_CONFIG_MAPacross.cand.hfiles returned no results, confirming safe removal.
| * Namespace and token path used for verifying whether FluentBit is | ||
| * on EKS or native Kubernetes by inspecting serviceaccount token issuer | ||
| */ | ||
| #define KUBE_SYSTEM_NAMESPACE "kube-system" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Clarify comment accuracy regarding namespace usage.
The updated comment states "Namespace and token path used for verifying..." but based on the new JWT issuer inspection approach described in the PR and AI summary, the detection logic only reads and parses the serviceaccount token to check the issuer field—it doesn't appear to query or use the KUBE_SYSTEM_NAMESPACE for platform verification itself.
Please verify whether KUBE_SYSTEM_NAMESPACE is actually utilized in the new detection flow, or if it's a legacy constant that remains for other purposes. If the namespace is no longer part of platform detection, consider updating the comment to reflect this more accurately.
Run the following script to verify how KUBE_SYSTEM_NAMESPACE is used in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check where KUBE_SYSTEM_NAMESPACE is referenced and how it's used
# Find all references to KUBE_SYSTEM_NAMESPACE
echo "=== References to KUBE_SYSTEM_NAMESPACE ==="
rg -n --type=c --type=h 'KUBE_SYSTEM_NAMESPACE' -A 3 -B 3
echo ""
echo "=== Check if it's used in determine_platform ==="
ast-grep --pattern $'determine_platform($$$) {
$$$
}'Length of output: 623
Update header comment to reflect JWT issuer–only detection
The KUBE_SYSTEM_NAMESPACE constant isn’t referenced by the new platform-detection logic; revise the comment to mention only the serviceaccount token path and issuer inspection.
🤖 Prompt for AI Agents
In plugins/filter_kubernetes/kube_conf.h around lines 77 to 80, the header
comment incorrectly mentions namespace-based detection though the new
platform-detection only checks the serviceaccount token path and inspects the
token issuer; update the comment to remove reference to
namespace/namespace-based detection and state that detection is performed solely
by reading the serviceaccount token and checking its issuer field (JWT
issuer-only detection).
Signed-off-by: Hyunsoo Kim <[email protected]>
movence
force-pushed
the
aws-eks-detection
branch
from
1a27fbf to
1c0104a
Compare
Background
Explored Related introduced
filter_kubernetesto add Kubernetes metadata to logs and metrics. The filter historically relied on the presence of theaws-authConfigMap to determine if the cluster is running on Amazon EKS. However, with the launch of EKS access entries, theaws-authConfigMap is no longer guaranteed to be present in EKS clusters.Problem
With the launch of EKS access entries,
aws-authConfigMap is no longer guaranteed to be present in EKS clusters. Missing ConfigMap causes two issues:- Noisy audit logs: Generates 404 not found errors in k8s audit logs when the
- Incorrect platform tagging: EKS clusters using access entries (
NAaws-authConfigMap doesn't existAPI) are falsely tagged asGenericorK8splatform instead ofAWS::EKSfor "Explore Related" functionalityEnter
[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
New Features
Bug Fixes
Documentation