-
Notifications
You must be signed in to change notification settings - Fork 1.8k
filter_kubernetes: use service account issuer to detect EKS env #11002
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,11 +74,10 @@ | |
#define SERVICE_NAME_SOURCE_MAX_LEN 64 | ||
|
||
/* | ||
* Configmap used for verifying whether if FluentBit is | ||
* on EKS or native Kubernetes | ||
* 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" | ||
#define AWS_AUTH_CONFIG_MAP "aws-auth" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we keep this as a fallback for old clusters? Or does the issuer exist on both old and new cluster so it should just be the definitive mechanism for determining platforms? |
||
|
||
/* | ||
* Possible platform values for Kubernetes plugin | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
#include <fluent-bit/flb_jsmn.h> | ||
#include <fluent-bit/flb_record_accessor.h> | ||
#include <fluent-bit/flb_ra_key.h> | ||
#include <fluent-bit/flb_utils.h> | ||
|
||
#include "kube_conf.h" | ||
#include "kube_meta.h" | ||
|
@@ -282,19 +283,106 @@ int fetch_pod_service_map(struct flb_kube *ctx, char *api_server_url, | |
return 0; | ||
} | ||
|
||
/* Determine platform by checking aws-auth configmap */ | ||
/* Determine platform by checking serviceaccount token issuer */ | ||
int determine_platform(struct flb_kube *ctx) | ||
{ | ||
int ret; | ||
char *config_buf; | ||
size_t config_size; | ||
|
||
ret = get_api_server_configmap(ctx, KUBE_SYSTEM_NAMESPACE, AWS_AUTH_CONFIG_MAP, &config_buf, &config_size); | ||
if (ret != -1) { | ||
flb_free(config_buf); | ||
return 1; | ||
char *token_buf = NULL; | ||
size_t token_size; | ||
char *payload = NULL; | ||
size_t payload_len; | ||
char *issuer_start, *issuer_end; | ||
|
||
/* Read serviceaccount token */ | ||
ret = flb_utils_read_file(FLB_KUBE_TOKEN, &token_buf, &token_size); | ||
if (ret != 0 || !token_buf) { | ||
return -1; | ||
} | ||
|
||
/* JWT tokens have 3 parts separated by dots: header.payload.signature */ | ||
char *first_dot = strchr(token_buf, '.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure variable declarations are on the top of function: https://github.com/fluent/fluent-bit/blob/master/CONTRIBUTING.md#variable-definitions |
||
if (!first_dot) { | ||
flb_free(token_buf); | ||
return -1; | ||
} | ||
|
||
char *second_dot = strchr(first_dot + 1, '.'); | ||
if (!second_dot) { | ||
flb_free(token_buf); | ||
return -1; | ||
} | ||
|
||
/* Extract and decode the payload (middle part) */ | ||
size_t payload_b64_len = second_dot - (first_dot + 1); | ||
char *payload_b64 = flb_malloc(payload_b64_len + 1); | ||
if (!payload_b64) { | ||
flb_free(token_buf); | ||
return -1; | ||
} | ||
|
||
memcpy(payload_b64, first_dot + 1, payload_b64_len); | ||
payload_b64[payload_b64_len] = '\0'; | ||
|
||
/* Base64 decode the payload */ | ||
payload = flb_malloc(payload_b64_len * 3 / 4 + 4); /* Conservative size estimate */ | ||
if (!payload) { | ||
flb_free(token_buf); | ||
flb_free(payload_b64); | ||
return -1; | ||
} | ||
|
||
ret = flb_base64_decode((unsigned char *)payload, payload_b64_len * 3 / 4 + 4, | ||
&payload_len, (unsigned char *)payload_b64, payload_b64_len); | ||
Comment on lines
+334
to
+335
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
|
||
flb_free(token_buf); | ||
flb_free(payload_b64); | ||
|
||
if (ret != 0) { | ||
flb_free(payload); | ||
return -1; | ||
} | ||
|
||
payload[payload_len] = '\0'; | ||
|
||
/* 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; | ||
} | ||
Comment on lines
+347
to
+375
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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:
Since the file already includes 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
|
||
|
||
/* 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 */ | ||
Comment on lines
+377
to
+385
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix pattern matching to search within issuer value only. The current implementation uses 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
|
||
} | ||
|
||
/* Gather pods list information from Kubelet */ | ||
|
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.
🛠️ 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:
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