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

upgrade aws-sdk to v3 to support webidentitytoken via service account IAM #195

Merged

Conversation

hardproblems
Copy link
Contributor

Issue #, if available:
#169

Description of changes:
Support IMDS by upgrading to the latest version documented in https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts-minimum-sdk.html

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dlvenable

@hardproblems hardproblems changed the title upgrade aws-sdk to v3 to support ec2 instance profiles (IMDS) upgrade aws-sdk to v3 to support webidentitytoken via service account IAM Aug 8, 2022
Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

Thanks @hardproblems for providing this change!

What versions of Amazon OpenSearch Service did you test against for this? We'll need to be sure that we retain compatibility for 6.x, 7.x, and OpenSearch 1.x.

Also, this plugin is deprecated in favor of logstash-output-opensearch. We can still take this change and should be able to release it. It might be good to additionally make this change in the logstash-output-opensearch plugin which will receive more updates.

logstash-output-amazon_es.gemspec Outdated Show resolved Hide resolved
@hardproblems
Copy link
Contributor Author

hardproblems commented Aug 12, 2022

@dlvenable we built this plugin with 6.8 and tested that it works against ES 6.8 and 7.1. Yea the reason we had to upgrade this one is that we haven't yet upgraded all our ES clusters to 7.x and OpenSearch 1.x and based on opensearch plugin's docs it doesn't officially support ES 6.x

@hardproblems hardproblems force-pushed the rong--aws-sdk-version-upgrade branch from 2129798 to d3a6672 Compare August 12, 2022 17:56

Starting in 7.2.0, the aws sdk version is bumped to v3. In order for all other AWS plugins to work together, please remove pre-installed plugins and install logstash-integration-aws plugin as follows. See also https://github.com/logstash-plugins/logstash-mixin-aws/issues/38
```
# Remove existing logstash aws plugins and install logstash-integration-aws to keep sdk dependency the same
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for adding this documentation. This is a significant step, so I think it will make sense to have this be a major version bump to 8.0.0. Thoughts? @hardproblems @deepdatta

Copy link
Contributor Author

Choose a reason for hiding this comment

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

major version bump sgtm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dlvenable updated. Will port this change to opensearch plugin as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opensearch-project/logstash-output-opensearch#171 here's the logic ported over to opensearch

Copy link
Member

Choose a reason for hiding this comment

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

@hardproblems , Do you still want this one merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes please 🙇

@dlvenable dlvenable merged commit d8a693d into awslabs:master Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants