-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
out_es: add cloud_apikey configuration #7935
base: master
Are you sure you want to change the base?
Conversation
Example configuration
Debug output and Valgrind
|
@@ -75,6 +75,27 @@ static flb_sds_t add_aws_auth(struct flb_http_client *c, | |||
} | |||
#endif /* FLB_HAVE_AWS */ | |||
|
|||
static int es_http_add_cloud_apikey(struct flb_http_client *c, |
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.
There seems to be some opportunities for refactoring http auths in the flb_http_client
library, but should probably be tackled in another PR.
44e3dba
to
38b7710
Compare
90d77d6
to
cd0fbdf
Compare
@patrick-stephens could you assist to re-run the integration tests? not quite sure why the integration runs have been failed the first time round. I've rebased master |
Do you mean unit tests? Integration tests are not run unless this is labelled. |
Ah, that's what I meant. I noticed the failing macOS test and wasn't sure if that was the blocker for the PR. What would be the next steps to move this PR forward? |
It's on the codeowners to review so will be in the queue. |
Can we extend this feature?
|
Could you elaborate a use case where the Bearer header is used in the context of elasticsearch? This change in particular is to support integration with Elastic Cloud via API Keys (see https://www.elastic.co/guide/en/cloud/current/ec-api-authentication.html) Regardless, I'm not quite sure that allowing users to specify arbitrary authorization headers is ideal, especially if the set of the allowable authorization type for the plugin could be well defined.
Looking at other fluentbit output plugins, this does not appear to be a common pattern. (The only exception seems to be Google Cloud Credential json, which seem to contain quite a bit of auth information, which would probably not be the norm). I would be hesistant to make this change in this PR without maintainers' inputs, since this feels like a config design change that would also be applicable to other plugins. |
I think personally I would be of the opinion to keep things simple in a PR, land one feature before adding more. |
Hi We do a similar thing with Prometheus sending metrics to a remote store. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#remote_write |
Hi any updates on merging this? This feature will be of great help ❤️ |
that's a shame, i won't be able to use fluent bit because it does not support sending logs using elastic api keys |
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
cd0fbdf
to
b2f5248
Compare
rebased @patrick-stephens @edsiper @PettitWesley could you remove the stale label? |
b2f5248
to
99659bf
Compare
This patch adds support for the Elastic Cloud API Keys. It updates the elastic search plugin to add a cloud_apikey configuration option. Usage: [OUTPUT] name es Cloud_Id elastic_cloud_id Cloud_Apikey elastic_apikey Signed-off-by: Soedarsono <[email protected]>
599f48d
to
1c6df25
Compare
adding the comment to move it from the stale state. What ar the current blockers? as this is really long avaited feature. Thank you @soedar for making this. |
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Still waiting on this PR, is there anything to be done to move it along? |
It looks like it needs a rebase to build for CentOS 7, it cannot be merged if it breaks builds. |
Adds Elastic Cloud API Key support to the
out_es
plugin. This patch adds a new config option,cloud_apikey
, which would be added to the HTTP request through theAuthorization: Apikey <cloud_apikey>
header.Addresses #6727. While we can re-use the
cloud_auth
config option, we would have to make additional assumptions on the API Key to identify it properly (i.e. does does not contain:
, is base64 encoded, etc).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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-test
label to test for all targets (requires maintainer to do).Documentation
fluent/fluent-bit-docs#1213
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.