-
Notifications
You must be signed in to change notification settings - Fork 1.9k
azure_kusto: Added support for region-based(Global and China cloud) authentication for Azure Kusto #11395
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?
azure_kusto: Added support for region-based(Global and China cloud) authentication for Azure Kusto #11395
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds cloud-environment awareness to the Azure Kusto plugin: new cloud enum and per-cloud macros/helpers, detection of Global vs China from endpoints, and runtime selection of cloud-specific MSAL auth URL templates, OAuth scopes, and IMDS resources across auth and config flows. Changes
Sequence DiagramsequenceDiagram
participant Config as Configuration Handler
participant Detector as Cloud Detector
participant Helpers as Cloud Helpers
participant Auth as OAuth/MSI Client
Config->>Detector: parse ingestion_endpoint
Detector-->>Config: return cloud_environment (GLOBAL / CHINA)
Config->>Helpers: get MSAL template / IMDS resource / scope (cloud_environment)
Helpers-->>Config: return template / resource / scope
Config->>Auth: build OAuth/MSI request with returned values
Auth->>Auth: append cloud-specific scope/resource and request token
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
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.
💡 Codex Review
fluent-bit/plugins/out_azure_kusto/azure_kusto_conf.c
Lines 836 to 837 in 10ec400
| flb_sds_snprintf(&ctx->oauth_url, flb_sds_alloc(ctx->oauth_url), | |
| FLB_AZURE_MSIAUTH_URL_TEMPLATE, "", ""); |
The MSI URL template now includes a third %s for the IMDS resource (FLB_AZURE_MSIAUTH_URL_TEMPLATE in azure_msiauth.h), but this call (and the user-assigned branch below) still passes only two arguments. With the extra %s, vsnprintf reads an invalid pointer, producing a corrupted URL or crashing, so managed-identity auth will fail (both global and China). Please pass the resource string (e.g., get_imds_resource(ctx->cloud_environment)) when formatting.
ℹ️ 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".
10ec400 to
ddde2b3
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: 2
🤖 Fix all issues with AI agents
In `@plugins/out_azure_kusto/azure_msiauth.c`:
- Around line 180-188: The two flb_sds_cat calls in azure_msiauth.c are passing
-1 which causes memcpy overflow; update the first call to pass the literal byte
length of the constant (use (int)(sizeof("&scope=") - 1)) instead of -1, and
ensure the second flb_sds_cat uses (int)strlen(scope) (or equivalent) rather
than relying on -1/implicit conversion; modify the calls that construct body
(referencing flb_sds_cat and flb_azure_kusto_get_scope) to use explicit positive
int lengths for both the "&scope=" string and the scope variable.
In `@plugins/out_azure_kusto/azure_msiauth.h`:
- Around line 23-24: The FLB_AZURE_MSIAUTH_URL_TEMPLATE has three %s
placeholders but the two call sites in azure_kusto_conf.c use only two
arguments; update both sprintf/http URL constructions that reference
FLB_AZURE_MSIAUTH_URL_TEMPLATE to pass the resource parameter as the third
argument (e.g., use the existing resource variable in the context such as
ctx->resource or the constant used for Kusto/AAD resource) so the format
receives three strings: the optional prefix ("", "&client_id="), the client_id
when applicable, and the resource string.
7644c54 to
8ef09ed
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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)
plugins/out_azure_kusto/azure_kusto_conf.c (1)
833-868: Fix oauth_url sizing: pointer sizeof and missing imds_resource lengthLine 860 uses
sizeof(tmpl)wheretmplis aconst char*pointer (returns ~8 bytes instead of actual string length). Additionally, lines 834, 845, and 860 don't account forimds_resourcelength in size calculations. Thoughflb_sds_snprintfhas retry logic that auto-grows, undersized allocations trigger unnecessary reallocations.🛠️ Proposed fix
- ctx->oauth_url = flb_sds_create_size(sizeof(FLB_AZURE_MSIAUTH_URL_TEMPLATE) - 1); + ctx->oauth_url = flb_sds_create_size(sizeof(FLB_AZURE_MSIAUTH_URL_TEMPLATE) - 1 + + strlen(imds_resource)); ... - ctx->oauth_url = flb_sds_create_size(sizeof(FLB_AZURE_MSIAUTH_URL_TEMPLATE) - 1 + - sizeof("&client_id=") - 1 + - flb_sds_len(ctx->client_id)); + ctx->oauth_url = flb_sds_create_size(sizeof(FLB_AZURE_MSIAUTH_URL_TEMPLATE) - 1 + + sizeof("&client_id=") - 1 + + flb_sds_len(ctx->client_id) + + strlen(imds_resource)); ... - ctx->oauth_url = flb_sds_create_size(sizeof(tmpl) - 1 + flb_sds_len(ctx->tenant_id)); + ctx->oauth_url = flb_sds_create_size(strlen(tmpl) + flb_sds_len(ctx->tenant_id));
a84efca to
31f4547
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)
plugins/out_azure_kusto/azure_kusto_conf.c (1)
833-856: MSI OAuth URL buffer size ignoresimds_resourcelength.Line 834 and Line 845 allocate based on the template and client_id only. The template now includes a
%sforimds_resource, so the buffer can be too small andflb_sds_snprintfmay truncate the URL, breaking MSI auth (especially for China endpoints). Includestrlen(imds_resource)in the size.🐛 Proposed fix
- ctx->oauth_url = flb_sds_create_size(sizeof(FLB_AZURE_MSIAUTH_URL_TEMPLATE) - 1); + ctx->oauth_url = flb_sds_create_size(strlen(FLB_AZURE_MSIAUTH_URL_TEMPLATE) + + strlen(imds_resource) + 1); ... - ctx->oauth_url = flb_sds_create_size(sizeof(FLB_AZURE_MSIAUTH_URL_TEMPLATE) - 1 + - sizeof("&client_id=") - 1 + - flb_sds_len(ctx->client_id)); + ctx->oauth_url = flb_sds_create_size(strlen(FLB_AZURE_MSIAUTH_URL_TEMPLATE) + + strlen("&client_id=") + + flb_sds_len(ctx->client_id) + + strlen(imds_resource) + 1);
🧹 Nitpick comments (1)
plugins/out_azure_kusto/azure_kusto_conf.c (1)
35-52: Remove or use the unusedget_kusto_scopehelper.
get_kusto_scopeisn’t referenced in this file, so a-Wunused-functionbuild can warn/fail. Either wire it into call sites or drop it.♻️ Proposed cleanup
-static const char *get_kusto_scope(int cloud_env) -{ - return flb_azure_kusto_get_scope(cloud_env); -} -
…uthentication for Azure Kusto Signed-off-by: thimmegowni.venkatesu <[email protected]>
…ce support Signed-off-by: thimmegowni.venkatesu <[email protected]>
Signed-off-by: thimmegowni.venkatesu <[email protected]>
Signed-off-by: thimmegowni.venkatesu <[email protected]>
31f4547 to
cbfaf63
Compare
Signed-off-by: thimmegowni.venkatesu <[email protected]>
Added Authentication based on two different clouds, Azure global and Azure china
Direct log ingestion from Fluent Bit to Azure Data Explorer via the azure_kusto plugin previously supported only Azure Global, as token generation relied on the Global Azure authentication authority.
Azure China uses a different authentication host, which was not supported by the existing Fluent Bit image, preventing direct ingestion into China cloud clusters.
This change adds support for Azure China cloud by updating the authentication configuration, enabling direct ingestion to Azure Data Explorer in China cloud.
Testing
Before we can approve your change; please submit the following in a comment:
Use case1: Using Global ingestion endpoint
Config file used
Output :

Use case2: Using China ingestion endpoint

Config file used
Output:

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
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
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.