Repository navigation
Conversation
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Caution Review failedThe pull request is closed. Finishing Touches
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 |
|
Ooops, wrong target repository. |
![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: 018bbec334
ℹ️ 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".
| memcpy(logstash_index, logstash_prefix, len); | ||
| logstash_index[len] = '\0'; | ||
| if (ec->logstash_format == FLB_TRUE) { | ||
| strncpy(logstash_index, ec->logstash_prefix.value, sizeof(logstash_index)); |
There was a problem hiding this comment.
With Logstash_Format On and an explicitly empty output-level Logstash_Prefix (for example, together with an empty separator for a date-only index), config-map assignment suppresses the default and leaves logstash_prefix.value NULL. This strncpy dereferences that NULL pointer on the first record; the previous formatter normalized a missing prefix to "", so retain that normalization or reject the configuration during initialization.
Useful? React with 👍 / 👎.
| static int config_validate(struct flb_elasticsearch_config* ec, | ||
| struct flb_elasticsearch* ctx) | ||
| { | ||
| if (ec->index.value == NULL && ec->logstash_format == FLB_FALSE && ec->generate_id == FLB_FALSE) { |
There was a problem hiding this comment.
When Index is explicitly empty, config-map assignment leaves ec->index.value NULL, but Generate_ID On makes this validation accept the configuration even though that option only creates a document _id. The formatter later uses the NULL value as the bulk action's _index, which can produce invalid metadata or invoke undefined %s formatting behavior; only Logstash_Format can supply an index without this field.
Useful? React with 👍 / 👎.
| if (ec->http_user && ec->http_passwd) { | ||
| flb_http_basic_auth(c, ec->http_user, ec->http_passwd); |
There was a problem hiding this comment.
When an upstream node sets http_user to an empty string to disable inherited basic authentication, the upstream property table returns a non-NULL empty value and http_passwd is still non-NULL (it has an empty default or is inherited). This condition therefore sends a Basic header with an empty username instead of falling through to the node's API key, cloud credentials, or no authentication, whereas the previous check required http_user[0] != '\0'.
Useful? React with 👍 / 👎.
| else if (ec->cloud_user.value && ec->cloud_passwd.value) { | ||
| flb_http_basic_auth(c, ec->cloud_user.value, ec->cloud_passwd.value); | ||
| } | ||
| else if (http_api_key != NULL && http_api_key[0] != '\0') { | ||
| else if (ec->http_api_key) { |
There was a problem hiding this comment.
When output-level Cloud_Auth supplies default credentials and an upstream node overrides authentication with HTTP_API_Key, the node configuration still contains the inherited cloud user/password, so this branch fires before the API-key branch and the explicit node key is never sent. Preserve the prior API-key-before-cloud precedence or clear conflicting inherited credentials when a node selects another authentication scheme.
Useful? React with 👍 / 👎.
| goto retry; | ||
| } | ||
| header_line = tmp_sds; | ||
| header_line = flb_sds_printf(&header_line, "ApiKey %s", ec->http_api_key); |
There was a problem hiding this comment.
Under memory pressure, flb_sds_printf can return NULL while formatting an API-key header, but assigning its result directly discards the surviving SDS pointer and the following flb_sds_len(header_line) dereferences NULL. The removed temporary-result check previously turned this allocation failure into the retry path, so validate the return before adding the header.
Useful? React with 👍 / 👎.
Copy of closed #7608.
Implementation of Upstream feature for the Elasticsearch output plugin.
This pull request is based on pull request #1560 and Forward output plugin.
It was tested in a local setup with:
Fluent Bit without Upstream feature connected to a single node of Elasticsearch cluster consisting of 3 master-eligible/data and 1 coordinating nodes.
Refer to elastic-cluster directory of mabrarov/elastic-stack repository for Docker Compose project used to create target Elasticsearch cluster and Kibana.
fluent-bit.conf Fluent Bit configuration file used for the test - refer to fluent-bit-es/fluent-bit.conf and (same in YAML format) fluent-bit-es/fluent-bit.yaml in mabrarov/elastic-stack repository.
Debug log is available at flb_es.log.
Fluent Bit with Upstream feature connected to all Elasticsearch data nodes of Elasticsearch cluster consisting of 3 master-eligible/data and 1 coordinating nodes.
Refer to elastic-cluster directory of mabrarov/elastic-stack repository for Docker Compose project used to create target Elasticsearch cluster and Kibana.
fluent-bit.conf Fluent Bit configuration file used for the test - refer to fluent-bit-es-cluster/fluent-bit.conf and (same in YAML format) fluent-bit-es-cluster/fluent-bit.yaml in mabrarov/elastic-stack repository.
Debug log is available at flb_es_upstream.log.
Testing
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.