Skip to content
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

Implements AWS SigV4 for the HTTP output plugin. #4459

Merged
merged 4 commits into from
Apr 5, 2024

Conversation

dlvenable
Copy link
Contributor

Which issue(s) this PR fixes:
Fixes #4444

What this PR does / why we need it:

This PR adds a new authentication method to the HTTP output plugin for AWS sigV4. We need it so that customers can use Fluentd to send data to Amazon OpenSearch Ingestion which supports the Fluentd output plugin, but requires SigV4 authentication.

Docs Changes:

  • aws_sigv4 - When this option is specified, Fluentd will sign requests using AWS Signature Version 4.

  • aws_service - The AWS service to authenticate against

  • aws_region - The AWS region to use when authenticating

  • aws_role_arn - The AWS role ARN to assume when authenticating

You can optionally specify an aws_role_arn. If you provide it, Fluentd will assume that AWS role and send requests signing from that role. Otherwise, Fluentd will use the credentials found by the credential provider chain as defined in the AWS documentation.

Release Note:

The http output plugin supports AWS Signature Version 4 authentication.

@ashie ashie requested review from ashie and daipom April 3, 2024 00:01
@daipom daipom added the enhancement Feature request or improve operations label Apr 3, 2024
@daipom
Copy link
Contributor

daipom commented Apr 3, 2024

@dlvenable Thanks so much for this enhancement!
I am very happy that adding AWS SigV4 authentication will allow Fluentd to work with AWS services such as Prepper!

fluentd.gemspec Outdated Show resolved Hide resolved
@daipom daipom added this to the v1.17.0 milestone Apr 3, 2024
@dlvenable
Copy link
Contributor Author

Thank you all for the useful feedback. I've pushed changes to address these comments.

Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented on my concerns, but it looks basically good to me.
Thanks so much for this enhancement!

Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, it looks good to me. I added a comment about XML parsers.
aws-sdk-ruby needs to work with XML parsers to handle XML responses from AWS services.

lib/fluent/plugin/out_http.rb Outdated Show resolved Hide resolved
@dlvenable
Copy link
Contributor Author

@daipom , @cosmo0920 , I see that some tests are failing, but they don't appear to be related to http. Are these possibly flaky tests?

I'm very interested in getting this PR in to allow us to move forward. Thank you.

@dlvenable
Copy link
Contributor Author

@ashie , It appears that the comment about gems still has this in changes requested.

@daipom
Copy link
Contributor

daipom commented Apr 4, 2024

I see that some tests are failing, but they don't appear to be related to http. Are these possibly flaky tests?

Yes. These tests are unstable. We can ignore it.

lib/fluent/plugin/out_http.rb Outdated Show resolved Hide resolved
@cosmo0920
Copy link
Contributor

cosmo0920 commented Apr 5, 2024

@daipom , @cosmo0920 , I see that some tests are failing, but they don't appear to be related to http. Are these possibly flaky tests?

I'm very interested in getting this PR in to allow us to move forward. Thank you.

These failed tests are flaky tests on macOS and Windows. They should be able to be ignored for now.

@daipom @ashie Any ETA for getting merged this PR? In Fluent Bit side, SigV4 signature is set up in the almost HTTP related plugins. So, I'm also interested in to equip the similar functionality as a parity for Fluent Bit.
And this PR should be prioritized to process.

@ashie
Copy link
Member

ashie commented Apr 5, 2024

@daipom @ashie Any ETA for getting merged this PR?

It's almost ready for merge.
We'll merge soon when we get @dlvenable's response about #4459 (comment)

@daipom
Copy link
Contributor

daipom commented Apr 5, 2024

Yes. I think we can merge this soon, at least once the concerns pointed out by @ashie are resolved.

… flexible versions and use a return to invert a conditional.

Signed-off-by: David Venable <[email protected]>
@dlvenable
Copy link
Contributor Author

@daipom , @ashie , Thank you for the feedback. I have addressed the latest comments int the most recent commit. Please take another look.

@ashie ashie merged commit 284bf40 into fluent:master Apr 5, 2024
14 of 16 checks passed
@ashie
Copy link
Member

ashie commented Apr 5, 2024

Thanks for your contribution!

@daipom
Copy link
Contributor

daipom commented Apr 8, 2024

@dlvenable Thanks for your contribution!

daipom added a commit to daipom/fluentd-docs-gitbook that referenced this pull request May 1, 2024
New feature of Fluentd v1.17.0.
Related: fluent/fluentd#4459

Signed-off-by: Daijiro Fukuda <[email protected]>
daipom added a commit to daipom/fluentd-docs-gitbook that referenced this pull request May 1, 2024
New feature of Fluentd v1.17.0.
Related: fluent/fluentd#4459

Signed-off-by: Daijiro Fukuda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support AWS SigV4 in the http output plugin
4 participants