out_stackdriver: support external_account (Workload Identity Federation)#11758
Open
cagataygurturk wants to merge 2 commits intofluent:masterfrom
Open
out_stackdriver: support external_account (Workload Identity Federation)#11758cagataygurturk wants to merge 2 commits intofluent:masterfrom
cagataygurturk wants to merge 2 commits intofluent:masterfrom
Conversation
Add Workload Identity Federation (external_account) credential support to the Stackdriver output plugin so workloads running outside GCP can write to Cloud Logging using short-lived federated tokens instead of long-lived service account keys. The new auth flow reads a subject token from credential_source (file or URL, text or JSON format), exchanges it at sts.googleapis.com using the RFC 8693 token-exchange grant, and optionally trades the federated token for a service account token via IAM Credentials. Both direct WIF principal bindings (principal://) and service-account impersonation (with delegation chains) are supported. Behavior is aligned with the Google reference SDK: - quota_project_id forwarded as x-goog-user-project on outbound calls - universe_domain substituted into default STS and Logging endpoints - audience and subject_token_type validated as required - workforce_pool_user_project rejected unless the audience matches the workforce-pool pattern - STS responses with expires_in == 0 rejected - x-goog-api-client telemetry header set on STS calls - service_account_impersonation.delegates parsed and forwarded - form bodies use a strict RFC 3986 encoder; impersonation and STS options bodies are built via yyjson_mut so values with quotes, backslashes or form-separator characters cannot corrupt requests File and URL credential_source variants are supported, with both text and JSON format types. The executable, AWS (environment_id) and X.509 certificate variants are not yet supported and are rejected at parse time with a clear error message; they can be added in follow-up changes. Migrating the credentials parser to yyjson ========================================== external_account credential files have three levels of nested objects plus arrays (credential_source.format, credential_source.headers, service_account_impersonation.delegates). The previous JSMN-based parser handled this with parent-index walks across multiple passes, which is brittle and verbose for nested data. The existing service_account parser was written in 2018 when JSMN was the only option available in the tree. yyjson was vendored into fluent-bit in fluent#10766 (Sep 2025) and made the default JSON backend in fluent#11562 (Mar 2026), with explicit guidance to migrate components incrementally. Adding a deeply nested credential type is a natural moment to do that here. Both the service_account and external_account paths now share one yyjson-based parser. Verified end-to-end against a Google Cloud project: subject token fetched from a Kubernetes service account token, exchanged at sts.googleapis.com, used to write log entries to Cloud Logging via a principal:// binding. The URL credential_source path was verified against a local HTTP token server in both text and JSON formats. Signed-off-by: Cagatay Gurturk <963018+cagataygurturk@users.noreply.github.com>
Add runtime tests for the new external_account (Workload Identity Federation) credential handling in out_stackdriver: - external_account_with_impersonation - external_account_no_impersonation - external_account_url_source - external_account_rejects_executable_source - external_account_rejects_aws_source - external_account_rejects_certificate_source - external_account_rejects_missing_audience - external_account_rejects_missing_subject_token_type - external_account_rejects_workforce_mismatch Each test boots a fluent-bit instance with a fixture credentials JSON under tests/runtime/data/stackdriver/, attaches a formatter test-mode callback that flips a flag on first format, ingests one record, and asserts whether the formatter ran. Accepted shapes must reach the formatter (init succeeded, output enabled). Rejected shapes must not: flb_start must fail and the formatter callback must never fire. URL-sourced credential_source is verified at init time only — token acquisition is lazy (first flush) and the runtime test does not stand up a local HTTP server. End-to-end coverage of the URL path (real HTTP GET, STS exchange, log delivery) is exercised by an out-of-tree smoke harness and is not part of ctest. Signed-off-by: Cagatay Gurturk <963018+cagataygurturk@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request adds Workload Identity Federation support to the Stackdriver output plugin, enabling authentication via external account credentials (subject tokens from files or URLs, STS token exchange, optional service account impersonation) without requiring GCP metadata server access. It also introduces x-goog-user-project header support for quota project attribution. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as Stackdriver Plugin
participant CredSource as Credential Source<br/>(File or HTTP)
participant STS as Google STS
participant IAM as Google IAM<br/>(optional)
Plugin->>CredSource: Read subject token
CredSource-->>Plugin: Raw/JSON token payload
Plugin->>Plugin: Parse credential_source.format<br/>(text or json)
Plugin->>STS: Exchange subject token<br/>(RFC 8693 token exchange)
STS-->>Plugin: Federated access token +<br/>expires_in
alt service_account_impersonation_url set
Plugin->>IAM: generateAccessToken<br/>(Bearer federated token +<br/>optional delegation chain)
IAM-->>Plugin: Service account access token +<br/>expireTime (RFC3339)
Plugin->>Plugin: Parse expireTime,<br/>compute expires_in
end
Plugin->>Plugin: Store final token<br/>in context (o)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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🧪 Generate unit tests (beta)
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.
Reviewed commit: ee6e7167a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+573
to
+587
| /* Required-field validation for external_account credentials. */ | ||
| if (!ctx->creds->audience || | ||
| flb_sds_len(ctx->creds->audience) == 0) { | ||
| flb_plg_error(ctx->ins, "external_account: 'audience' is " | ||
| "required"); | ||
| flb_stackdriver_conf_destroy(ctx); | ||
| return NULL; | ||
| } | ||
| if (!ctx->creds->subject_token_type || | ||
| flb_sds_len(ctx->creds->subject_token_type) == 0) { | ||
| flb_plg_error(ctx->ins, "external_account: " | ||
| "'subject_token_type' is required"); | ||
| flb_stackdriver_conf_destroy(ctx); | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
The new init-time validation for external_account checks audience and subject_token_type, but it never verifies that credential_source.file or credential_source.url is present. In this misconfiguration, plugin startup succeeds here, then every flush fails later in stackdriver_external_account_read_token() with FLB_RETRY, causing continuous retry churn and no successful log delivery instead of a clear startup failure.
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: 4
🧹 Nitpick comments (2)
plugins/out_stackdriver/stackdriver_conf.c (1)
695-698: Avoid fixed-size URL buffer foruniverse_domainendpoint construction.Line 695 through Line 698 use a hardcoded capacity (
96). Please size this dynamically from actual domain length to avoid truncation risk on non-default domains.Proposed refactor
- ctx->cloud_logging_write_url = flb_sds_create_size(96); - flb_sds_snprintf(&ctx->cloud_logging_write_url, 96, - "https://logging.%s%s", - ctx->creds->universe_domain, FLB_STD_WRITE_URI); + cloud_logging_write_url_size = (sizeof("https://logging.") - 1) + + flb_sds_len(ctx->creds->universe_domain) + + FLB_STD_WRITE_URI_SIZE; + ctx->cloud_logging_write_url = + flb_sds_create_size(cloud_logging_write_url_size); + if (!ctx->cloud_logging_write_url) { + flb_errno(); + flb_stackdriver_conf_destroy(ctx); + return NULL; + } + flb_sds_snprintf(&ctx->cloud_logging_write_url, + cloud_logging_write_url_size + 1, + "https://logging.%s%s", + ctx->creds->universe_domain, FLB_STD_WRITE_URI);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/out_stackdriver/stackdriver_conf.c` around lines 695 - 698, The code currently allocates a fixed 96-byte buffer for ctx->cloud_logging_write_url then writes "https://logging.%s%s" with ctx->creds->universe_domain and FLB_STD_WRITE_URI which can overflow or truncate for long domains; change to compute the required size dynamically (strlen("https://logging.") + strlen(ctx->creds->universe_domain) + strlen(FLB_STD_WRITE_URI) + 1) and call flb_sds_create_size with that length, then use flb_sds_snprintf (or flb_sds_printf) to populate ctx->cloud_logging_write_url so the buffer always fits the composed URL.plugins/out_stackdriver/stackdriver_external_account.c (1)
668-687: Checkflb_sds_printfresult before addingx-goog-api-clientheader.This keeps the OOM/error path deterministic and avoids propagating an invalid buffer into header emission.
♻️ Suggested robustness tweak
- flb_sds_printf(&api_client_header, - "fluent-bit/%s google-byoid-sdk source/%s " - "sa-impersonation/%s config-lifetime/%s", - FLB_VERSION_STR, provider, - (ctx->creds->service_account_impersonation_url && - flb_sds_len(ctx->creds->service_account_impersonation_url) > 0) - ? "true" : "false", - (ctx->creds->sa_impersonation_lifetime_seconds > 0) - ? "true" : "false"); - flb_http_add_header(c, "x-goog-api-client", 17, - api_client_header, - flb_sds_len(api_client_header)); + if (!flb_sds_printf(&api_client_header, + "fluent-bit/%s google-byoid-sdk source/%s " + "sa-impersonation/%s config-lifetime/%s", + FLB_VERSION_STR, provider, + (ctx->creds->service_account_impersonation_url && + flb_sds_len(ctx->creds->service_account_impersonation_url) > 0) + ? "true" : "false", + (ctx->creds->sa_impersonation_lifetime_seconds > 0) + ? "true" : "false")) { + flb_sds_destroy(api_client_header); + api_client_header = NULL; + } + else { + flb_http_add_header(c, "x-goog-api-client", 17, + api_client_header, + flb_sds_len(api_client_header)); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/out_stackdriver/stackdriver_external_account.c` around lines 668 - 687, The code constructs api_client_header with flb_sds_create_size and then calls flb_sds_printf but does not check flb_sds_printf's return for failure before passing api_client_header into flb_http_add_header; update the block around api_client_header to verify flb_sds_printf succeeded (and that api_client_header is a valid non-NULL, non-error SDS) before calling flb_http_add_header, and skip or free api_client_header on failure; reference the variables and functions api_client_header, flb_sds_printf, flb_sds_len, flb_http_add_header and the ctx->creds fields (cred_source_url, service_account_impersonation_url, sa_impersonation_lifetime_seconds) when making the check and cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/out_stackdriver/stackdriver_external_account.c`:
- Around line 154-160: After trimming the extracted subject token with
wif_sds_trim_ws(raw), add a fast-failure check that if flb_sds_len(raw) == 0
(i.e. the subject token is empty) you immediately return an error/NULL instead
of continuing to build the STS body; update the same check in the other block
mentioned (around 197-209) so both code paths validate the trimmed raw token and
fail fast. Ensure you reference the same variables (raw, format_type,
ctx->creds->cred_source_format_type) and return the same error value used by the
surrounding function for consistency.
- Around line 320-325: The code currently accepts "http" by checking prot and
setting port_n = 80 (see the prot variable and port_n assignment), which can
expose subject tokens; change the URL protocol validation to reject plaintext
schemes: require prot to be non-NULL and equal to "https" (strcasecmp(prot,
"https") == 0) and return/propagate an error if it is "http" or any other
non-https value instead of defaulting to 443 or 80; apply this same validation
to the other URL-parsing block that uses prot and port_n (the similar logic
around the second occurrence) so only HTTPS is allowed and no plaintext
credential_source.url is accepted.
In `@plugins/out_stackdriver/stackdriver_external_account.h`:
- Around line 34-35: Update the header comment in stackdriver_external_account.h
to reflect that subject tokens can be sourced from either a local file or a URL
by mentioning both credential_source.file and credential_source.url; change the
two-step docstring to say: 1) obtain the subject token from
credential_source.file or credential_source.url (performing an HTTP GET when
using url), and 2) POST the token to the STS token URL to obtain a federated
access token, so the comment matches the runtime behavior.
In `@tests/runtime/out_stackdriver.c`:
- Around line 6608-6611: The post-push wait of sleep(1) is too short and causes
flaky assertions for ext_acct_format_called; after calling flb_lib_push(ctx,
in_ffd, (char *) dummy_record, sizeof(dummy_record) - 1) replace the
single-second sleep with a more robust wait (e.g., increase to sleep(2) or
implement a short polling loop) that waits until ext_acct_format_called is set
or a reasonable timeout elapses before calling flb_stop(ctx); ensure the change
references flb_lib_push, dummy_record, ext_acct_format_called, and flb_stop so
tests reliably observe the callback.
---
Nitpick comments:
In `@plugins/out_stackdriver/stackdriver_conf.c`:
- Around line 695-698: The code currently allocates a fixed 96-byte buffer for
ctx->cloud_logging_write_url then writes "https://logging.%s%s" with
ctx->creds->universe_domain and FLB_STD_WRITE_URI which can overflow or truncate
for long domains; change to compute the required size dynamically
(strlen("https://logging.") + strlen(ctx->creds->universe_domain) +
strlen(FLB_STD_WRITE_URI) + 1) and call flb_sds_create_size with that length,
then use flb_sds_snprintf (or flb_sds_printf) to populate
ctx->cloud_logging_write_url so the buffer always fits the composed URL.
In `@plugins/out_stackdriver/stackdriver_external_account.c`:
- Around line 668-687: The code constructs api_client_header with
flb_sds_create_size and then calls flb_sds_printf but does not check
flb_sds_printf's return for failure before passing api_client_header into
flb_http_add_header; update the block around api_client_header to verify
flb_sds_printf succeeded (and that api_client_header is a valid non-NULL,
non-error SDS) before calling flb_http_add_header, and skip or free
api_client_header on failure; reference the variables and functions
api_client_header, flb_sds_printf, flb_sds_len, flb_http_add_header and the
ctx->creds fields (cred_source_url, service_account_impersonation_url,
sa_impersonation_lifetime_seconds) when making the check and cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06356f37-d2bd-48ab-920b-67bdbfae3da1
📒 Files selected for processing (16)
plugins/out_stackdriver/CMakeLists.txtplugins/out_stackdriver/stackdriver.cplugins/out_stackdriver/stackdriver.hplugins/out_stackdriver/stackdriver_conf.cplugins/out_stackdriver/stackdriver_external_account.cplugins/out_stackdriver/stackdriver_external_account.htests/runtime/data/stackdriver/stackdriver-credentials-external-account-aws.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account-cert.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account-exec.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account-no-audience.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account-no-impersonation.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account-no-subject-token-type.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account-url.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account-workforce-mismatch.jsontests/runtime/data/stackdriver/stackdriver-credentials-external-account.jsontests/runtime/out_stackdriver.c
Comment on lines
+154
to
+160
| wif_sds_trim_ws(raw); | ||
|
|
||
| format_type = ctx->creds->cred_source_format_type; | ||
| if (!format_type || | ||
| flb_sds_len(ctx->creds->cred_source_format_type) == 0 || | ||
| strcasecmp(format_type, "text") == 0) { | ||
| return raw; |
There was a problem hiding this comment.
Fail fast when extracted subject token is empty.
An empty token currently proceeds into STS body construction, producing a less clear downstream failure.
🛡️ Suggested validation
wif_sds_trim_ws(raw);
+ if (flb_sds_len(raw) == 0) {
+ flb_plg_error(ctx->ins, "external_account: subject token from %s is empty",
+ source_label);
+ flb_sds_destroy(raw);
+ return NULL;
+ }
@@
else {
token = flb_sds_create(yyjson_get_str(v));
+ if (token) {
+ wif_sds_trim_ws(token);
+ if (flb_sds_len(token) == 0) {
+ flb_plg_error(ctx->ins, "external_account: '%s' in %s is empty",
+ field_name, source_label);
+ flb_sds_destroy(token);
+ token = NULL;
+ }
+ }
}Also applies to: 197-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/out_stackdriver/stackdriver_external_account.c` around lines 154 -
160, After trimming the extracted subject token with wif_sds_trim_ws(raw), add a
fast-failure check that if flb_sds_len(raw) == 0 (i.e. the subject token is
empty) you immediately return an error/NULL instead of continuing to build the
STS body; update the same check in the other block mentioned (around 197-209) so
both code paths validate the trimmed raw token and fail fast. Ensure you
reference the same variables (raw, format_type,
ctx->creds->cred_source_format_type) and return the same error value used by the
surrounding function for consistency.
Comment on lines
+320
to
+325
| else if (prot && strcasecmp(prot, "http") == 0) { | ||
| port_n = 80; | ||
| } | ||
| else { | ||
| port_n = 443; | ||
| } |
There was a problem hiding this comment.
Reject plaintext credential_source.url to avoid subject-token exposure.
Current logic allows http:// transport, which can leak subject tokens over the network and weaken the credential trust boundary.
🔒 Suggested hardening
static flb_sds_t read_subject_token_from_url(struct flb_stackdriver *ctx)
{
@@
url = ctx->creds->cred_source_url;
@@
+ if (strncasecmp(url, "https://", 8) != 0) {
+ flb_plg_error(ctx->ins, "external_account: credential_source.url "
+ "must use https://");
+ return NULL;
+ }
+
u = wif_get_upstream(ctx, &ctx->wif_subject_url_u, url);Also applies to: 471-483
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/out_stackdriver/stackdriver_external_account.c` around lines 320 -
325, The code currently accepts "http" by checking prot and setting port_n = 80
(see the prot variable and port_n assignment), which can expose subject tokens;
change the URL protocol validation to reject plaintext schemes: require prot to
be non-NULL and equal to "https" (strcasecmp(prot, "https") == 0) and
return/propagate an error if it is "http" or any other non-https value instead
of defaulting to 443 or 80; apply this same validation to the other URL-parsing
block that uses prot and port_n (the similar logic around the second occurrence)
so only HTTPS is allowed and no plaintext credential_source.url is accepted.
Author
There was a problem hiding this comment.
This is intentional. Sometimes tokens are sourced from sidecars or similar local URLs that do not necessarily support TLS -- the user is responsible for this.
There was a problem hiding this comment.
@cagataygurturk, understood — that's a well-known pattern (K8s sidecar token providers, IMDS-style local metadata endpoints, etc.) and HTTP is perfectly reasonable there. Acknowledged and noted.
One lightweight option, if you'd like belt-and-suspenders observability without blocking anything, would be a flb_plg_warn when the scheme is non-HTTPS, so operators can see in the logs that a plaintext endpoint is in use — but entirely up to you.
(ᴖ‿‿ᴖ) ∫
✏️ Learnings added
Learnt from: cagataygurturk
Repo: fluent/fluent-bit PR: 11758
File: plugins/out_stackdriver/stackdriver_external_account.c:320-325
Timestamp: 2026-04-29T02:20:27.326Z
Learning: In the Fluent Bit Stackdriver output plugin (`plugins/out_stackdriver/stackdriver_external_account.c`), allowing `http://` (plaintext) for `credential_source.url` is intentional. Subject tokens are sometimes sourced from local sidecars or metadata-style endpoints (e.g., Kubernetes sidecar token providers) that do not support TLS. Do not flag HTTP `credential_source.url` as a security issue — the operator is responsible for the transport security of local endpoints.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: edsiper
Repo: fluent/fluent-bit PR: 11286
File: src/flb_http_client.c:1657-1715
Timestamp: 2025-12-17T18:45:02.283Z
Learning: In the Fluent Bit OAuth2 implementation (src/flb_oauth2.c), the function flb_oauth2_get_access_token returns a pointer to the cached token stored in the OAuth2 context (ctx->access_token), not a new allocation. The OAuth2 context manages the token lifecycle, so callers must not free the returned token pointer.
Learnt from: edsiper
Repo: fluent/fluent-bit PR: 11686
File: tests/integration/src/server/http_server.py:329-331
Timestamp: 2026-04-09T17:53:21.430Z
Learning: In the fluent/fluent-bit repository, the integration test HTTP server (`tests/integration/src/server/http_server.py`) intentionally binds to `0.0.0.0` to support cross-container/cross-host access in CI environments. Do not flag this as a security issue (e.g., Ruff S104) — it is expected and acceptable for integration test infrastructure.
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11711
File: tests/runtime/http_client_chunked.c:172-175
Timestamp: 2026-04-14T20:30:06.356Z
Learning: In this project (fluent/fluent-bit), use K&R brace style for C/C++: for control statements (`if`, `else`, `while`, `do`, `for`), place the opening brace on the same line as the control statement (e.g., `if (cond) {`). Do not use Allman style (opening brace on the next line).
Comment on lines
+34
to
+35
| * 1. Read the subject token from credential_source.file | ||
| * 2. POST it to the STS token URL to obtain a federated access token |
There was a problem hiding this comment.
Docstring should mention URL-based subject-token sources too.
Line 34 through Line 35 currently state file-only retrieval, but this feature also supports credential_source.url. Please update the header comment so it matches runtime behavior.
Proposed doc fix
- * 1. Read the subject token from credential_source.file
+ * 1. Read the subject token from credential_source.file or
+ * credential_source.url📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * 1. Read the subject token from credential_source.file | |
| * 2. POST it to the STS token URL to obtain a federated access token | |
| * 1. Read the subject token from credential_source.file or | |
| * credential_source.url | |
| * 2. POST it to the STS token URL to obtain a federated access token |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/out_stackdriver/stackdriver_external_account.h` around lines 34 - 35,
Update the header comment in stackdriver_external_account.h to reflect that
subject tokens can be sourced from either a local file or a URL by mentioning
both credential_source.file and credential_source.url; change the two-step
docstring to say: 1) obtain the subject token from credential_source.file or
credential_source.url (performing an HTTP GET when using url), and 2) POST the
token to the STS token URL to obtain a federated access token, so the comment
matches the runtime behavior.
Comment on lines
+6608
to
+6611
| flb_lib_push(ctx, in_ffd, (char *) dummy_record, | ||
| sizeof(dummy_record) - 1); | ||
| sleep(1); | ||
| flb_stop(ctx); |
There was a problem hiding this comment.
Increase the post-push wait to reduce flaky callback assertions.
Using sleep(1) with flush=1 is timing-sensitive; intermittent scheduler delay can leave ext_acct_format_called unset even when behavior is correct.
🔧 Suggested stabilization
- sleep(1);
+ sleep(2);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| flb_lib_push(ctx, in_ffd, (char *) dummy_record, | |
| sizeof(dummy_record) - 1); | |
| sleep(1); | |
| flb_stop(ctx); | |
| flb_lib_push(ctx, in_ffd, (char *) dummy_record, | |
| sizeof(dummy_record) - 1); | |
| sleep(2); | |
| flb_stop(ctx); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/runtime/out_stackdriver.c` around lines 6608 - 6611, The post-push wait
of sleep(1) is too short and causes flaky assertions for ext_acct_format_called;
after calling flb_lib_push(ctx, in_ffd, (char *) dummy_record,
sizeof(dummy_record) - 1) replace the single-second sleep with a more robust
wait (e.g., increase to sleep(2) or implement a short polling loop) that waits
until ext_acct_format_called is set or a reasonable timeout elapses before
calling flb_stop(ctx); ensure the change references flb_lib_push, dummy_record,
ext_acct_format_called, and flb_stop so tests reliably observe the callback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds support for the
external_accountcredential type to the Stackdriver output plugin so workloads outside GCP can authenticate via Workload Identity Federation, alternative to mounting long-lived service-account JSON keys.Fixes #8804.
When the credentials file's
typeisexternal_account, the plugin reads a subject token fromcredential_source(a local file or an HTTPS GET to a configured URL, intextorjsonformat), exchanges it atsts.googleapis.com(RFC 8693 token-exchange), and either uses the federated token directly or trades it for a service-account access token via IAM Credentials'generateAccessToken(with optionaldelegateschain). Directprincipal://bindings and service-account impersonation both work.A few related fields from the credentials file are honored:
quota_project_idis forwarded asx-goog-user-projecton outbound Cloud Logging calls.universe_domainis substituted into default STS and Logging endpoints.workforce_pool_user_projectis sent as the STSoptions.userProject, but only if the audience matches the workforce-pool pattern (otherwise rejected at init).audienceandsubject_token_typeare required. STS responses withexpires_in == 0are rejected so a zero-lifetime token doesn't get cached and trigger a refresh storm.x-goog-api-clientis set on STS calls.Scope
credential_sourcevariants supported:fileandurl, intextorjsonformat.executable,environment_id(AWS) andcertificate(X.509 mTLS) are explicitly rejected at parse time. They can be added later without affecting the API here.JSMN → yyjson
The credentials parser used JSMN. That worked for the flat
service_accountJSON (added in 2018, when JSMN was the only option), butexternal_accounthas three levels of nesting plus arrays —credential_source.format,credential_source.headers,service_account_impersonation.delegates. JSMN's parent-index walks turn into multi-pass bookkeeping at that point.yyjsonwas vendored in #10766 and made the default JSON backend in #11562, and apparently the existing components should migrate incrementally. This PR migrates the credentials parser as part of the work, the original SA-key path now goes through the same parser.Compatibility
If a user already had
quota_project_idin their credentials JSON, fluent-bit now sendsx-goog-user-projectfor it, which changes which project is billed/quota-counted for Logging calls. Matches Cloud SDK behavior; users not setting it are unaffected.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:
With a credentials file like:
{ "type": "external_account", "audience": "//iam.googleapis.com/projects/123456789/locations/global/workloadIdentityPools/pool/providers/provider", "subject_token_type": "urn:ietf:params:oauth:token-type:jwt", "token_url": "https://sts.googleapis.com/v1/token", "credential_source": { "file": "/var/run/secrets/kubernetes.io/serviceaccount/token", "format": { "type": "text" } } }End-to-end verified against a real GCP project: subject token from a Kubernetes service-account token, exchanged at
sts.googleapis.com, log entries written to Cloud Logging via aprincipal://IAM binding. URLcredential_sourceverified against a local HTTP token server in both text and JSON formats. Runtime tests intests/runtime/out_stackdriver.ccover the accepted shapes plus the rejected variants and missing-field cases.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
Release Notes
New Features
Tests