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.

Sign up for GitHub

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

Jump to bottom

IAM role support for EC2 dynamic inventory #15196

Merged
merged 4 commits into from
Mar 20, 2017
Merged

IAM role support for EC2 dynamic inventory #15196

merged 4 commits into from
Mar 20, 2017

Conversation

areian
Copy link
Contributor

@areian areian commented Mar 29, 2016

ISSUE TYPE
  • Feature Pull Request
ANSIBLE VERSION
ansible 2.1.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = Default w/o overrides

Tested on both 2.1.0 and 1.9.4

SUMMARY

This change allows the EC2 dynamic inventory to assume an IAM role prior to connecting to the AWS modules.
We needed this functionality at my company due to how we have structured our AWS setup, and I thought others might find it useful.

@jimi-c jimi-c added this to the 2.1.0 milestone Mar 30, 2016
@MichaelBaydoun
Copy link
Contributor

It would be great if we could do this for the entire playbook, and not just for ec2 dynamic inventory.

@ryansb
Copy link
Contributor

ryansb commented May 20, 2016

Looks good to me.

@MichaelBaydoun do you mean for all the other AWS modules? IMO ec2.py is at least a little separate so I wouldn't block this pending IAM support for all other modules.

@MichaelBaydoun
Copy link
Contributor

@ryansb Agreed, I'm not suggesting holding this merge up, it's a good improvement. Just floating the idea that down the road, it would be great if we had iam role assumption capability in the core, for all the modules.

@jimi-c jimi-c modified the milestones: stable-2.1, 2.2.0 Jun 22, 2016
@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 aws cloud needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 13, 2016
ryansb
ryansb suggested changes Dec 16, 2016
View reviewed changes
Copy link
Contributor

@ryansb ryansb left a comment

Choose a reason for hiding this comment

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

Tweak on the if check, other than that looks good.

contrib/inventory/ec2.py
@@ -475,6 +479,13 @@ def connect_to_aws(self, module, region):
connect_args['profile_name'] = self.boto_profile
self.boto_fix_security_token_in_profile(connect_args)

if self.iam_role:
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a guard, since self.iam_role will be unset if the option isn't specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed more in line with the rest of the code to define the variable with a default value in read_settings.

@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 16, 2016
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 3, 2017
@ansibot
Copy link
Contributor

ansibot commented Feb 14, 2017

cc @willthames
click here for bot help

@ryansb
Copy link
Contributor

ryansb commented Feb 16, 2017

shipit

1 similar comment
@willthames
Copy link
Contributor

shipit

@willthames willthames mentioned this pull request Feb 16, 2017
Closed
@nethershaw
Copy link

-1

This merge would cause regression of #15890.

@willthames
Copy link
Contributor

@nethershaw it doesn't seem to relate to the change in #15890, so might need some more detail as to how you identified that it would cause such a regression.

@HontoNoRoger
Copy link

I tried your changes with current Ansible 2.2.0 with the dynamic inventory from the docs. Works as intended!

The only thing missing was

from boto import sts

at the top of ec2.py script. Did I overlook that or is it still missing in the PR?

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Feb 24, 2017
@areian
Copy link
Contributor Author

areian commented Mar 10, 2017 •
edited

@HontoNoRoger You are absolutely correct. An oversight on my part when creating the PR. It has been fixed.

Will fix merge conflict later today

@ansibot
Copy link
Contributor

ansibot commented Mar 10, 2017

@areian this PR contains the following merge comits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot ansibot added the merge_commit This PR contains at least one merge commit. Please resolve! label Mar 10, 2017
@ansibot ansibot removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 10, 2017
@ryansb ryansb merged commit 6804d69 into ansible:devel Mar 20, 2017
@afeld afeld mentioned this pull request Mar 23, 2017
Closed
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ryansb ryansb ryansb approved these changes

Assignees
No one assigned
Labels
affects_2.3 This issue/PR affects Ansible v2.3 aws cloud feature This issue/PR relates to a feature request.
Projects
None yet
Milestone
2.2.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@areian @MichaelBaydoun @ryansb @ansibot @willthames @nethershaw @HontoNoRoger @jimi-c