-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
update AWS SDK for ECS Task IAM support in discovery-ec2 #26479
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
There is more to updating dependencies than bumping the version. You need to also update SHAs and maybe grant permissions to the JAR over the old version. It could be the new SDK needs more permissions than the current, and those might be permissions we would not even grant. So I would ask you: please at least run |
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.
I think we should update both discovery-ec2 and repository-s3 projects at the same time.
Note that last time I tried to upgrade AWS SDK, I got a SM issue (See #19594). May be it's not the case anymore but @mohit could you try to run your own version of the plugin and see if it does not break anything?
Then, does this upgrade automagically add support for ECS Task? Or is there any other code change need?
Thanks!
Thanks for the instructions! I found various Mocks for AwsEc2Service that broke and am in the process of adding the fixes. As for the runtime, I replaced the jars in my Docker container for No code changes (apart from tests) are required for I'll work on pushing the changes for My current experiments with This comment suggests not using the In either case, thanks for the help and I'll add the changes in the next 24 hours or so (working with Java after a long time and it's taking some time to bootstrap the JDK/Eclipse environment). |
@dadoonet Apologies but I've updated the
|
@mohit Those are classes referenced by dependencies that are not in any of the dependencies on the classpath. You have to figure out if those classes are needed for the functionality we support, or not. If they are needed, you need to add the required dependency. If they are not, they need to be excluded for the audit (in the build.gradle) explaining why. We do not like adding new dependencies so the case must be strong that a new one is needed here. |
Also, I think it will be too much to add upgrading repository-s3 to this PR, please leave it out. |
I don't think the classes are required on account of me already having used the |
- jmespath seems to be used for `waiters` - amazon ion is a protocol not used by EC2 or IAM
With the above commits |
I agree that it's not mandatory. We need to open an issue (or better another PR) to track it IMO. |
@mohit Just checking: did you try to run your version of the plugin on EC2? |
@dadoonet I have tested by adding the updated AWS dependencies to the Unfortunately I've not been able to get a working Any advice on how to proceed will be welcome. Thanks again on all your help! |
@mohit I would recommend cherry-picking your changes to the plugin to the v6.0.0-beta2 tag, building that from source, and using the 6.0.0-beta2 version of our Docker container for testing. |
Okie. So I built a container using I'm not a 100% sure why the |
okay found https://www.elastic.co/guide/en/elasticsearch/plugins/6.0/_settings.html, will test by setting |
Okay was able to build a docker container using "userIdentity": {
"type": "AssumedRole",
"principalId": "AAAAAAAA-5d1ff6c2692d",
"arn": "arn:aws:sts::000000000:assumed-role/elasticsearch-dev/-0000-5d1ff6c2692d",
"accountId": "000000000",
"accessKeyId": "000000000AAA",
"sessionContext": {
"attributes": {
"mfaAuthenticated": "false",
"creationDate": "2017-09-07T00:12:50Z"
},
"sessionIssuer": {
"type": "Role",
"principalId": "000000000AAAA",
"arn": "arn:aws:iam::000000000:role/elasticsearch-dev",
"accountId": "000000000",
"userName": "elasticsearch-dev"
}
}
},
"eventTime": "2017-09-07T00:15:14Z",
"eventSource": "ec2.amazonaws.com",
"eventName": "DescribeInstances",
"awsRegion": "us-west-1",
"sourceIPAddress": "00.000.000.000",
"userAgent": "aws-sdk-java/1.11.187 Linux/4.4.41-35.53.amzn1.x86_64 OpenJDK_64-Bit_Server_VM/25.141-b16/1.8.0_141",
"requestParameters": {
"instancesSet": {},
"filterSet": {
"items": [
{
"name": "instance-state-name",
"valueSet": {
"items": [
{
"value": "running"
},
{
"value": "pending"
}
]
}
}
]
}
} |
I don't think there is a need to add anything to the documentation but like I mentioned above setting |
@dadoonet any other requirements here? |
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.
LGTM
I can hopefully merge it next week unless @jasontedor wants to do it?
This commit contains: * update AWS SDK for ECS Task IAM support * ignore dependencies not essential to `discovery-ec2`: * jmespath seems to be used for `waiters` * amazon ion is a protocol not used by EC2 or IAM Backport of #26479 in 6.x branch
Thanks @mohit. I pushed it in master (7.0.0) and 6.x (6.1.0). |
…rflow * origin/master: (59 commits) Fix Lucene version of 5.6.1. Remove azure deprecated settings (elastic#26099) Handle the 5.6.0 release Allow plugins to validate cluster-state on join (elastic#26595) Remove index mapper dynamic settings (elastic#25734) update AWS SDK for ECS Task IAM support in discovery-ec2 (elastic#26479) Azure repository: Accelerate the listing of files (used in delete snapshot) (elastic#25710) Build: Remove norelease from forbidden patterns (elastic#26592) Fix reference to painless inside expression engine (elastic#26528) Build: Move javadoc linking to root build.gradle (elastic#26529) Test: Remove leftover static bwc test case (elastic#26584) Docs: Remove remaining references to file and native scripts (elastic#26580) Snapshot fallback should consider build.snapshot elastic#26496: Set the correct bwc version after backport to 6.x Fix the MapperFieldType.rangeQuery API. (elastic#26552) Deduplicate `_field_names`. (elastic#26550) [Docs] Update method setSource(byte[] source) (elastic#26561) [Docs] Fix typo in javadocs (elastic#26556) Allow multiple digits in Vagrant 2.x minor versions Support Vagrant 2.x ...
* master: (21 commits) Ensure module is bundled before installing in tests Add boolean similarity to built in similarity types (elastic#26613) [Tests] Remove skip tests in search/30_limits.yml Let search phases override max concurrent requests Add a soft limit for the number of requested doc-value fields (elastic#26574) Support for accessing Azure repositories through a proxy (elastic#23518) Add beta tag to MSI Windows Installer (elastic#26616) Fix Lucene version of 5.6.1. Remove azure deprecated settings (elastic#26099) Handle the 5.6.0 release Allow plugins to validate cluster-state on join (elastic#26595) Remove index mapper dynamic settings (elastic#25734) update AWS SDK for ECS Task IAM support in discovery-ec2 (elastic#26479) Azure repository: Accelerate the listing of files (used in delete snapshot) (elastic#25710) Build: Remove norelease from forbidden patterns (elastic#26592) Fix reference to painless inside expression engine (elastic#26528) Build: Move javadoc linking to root build.gradle (elastic#26529) Test: Remove leftover static bwc test case (elastic#26584) Docs: Remove remaining references to file and native scripts (elastic#26580) Snapshot fallback should consider build.snapshot ...
Updating the AWS SDK to the latest released version. In particular this support ECS IAM Task Roles which are very useful when running Elasticsearch in a docker container on ECS.
Fixes #23039 for
discovery-ec2
. Using this on ECS and tested it locally using docker.