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

[cloud] Add support for updating IAM role with ec2_instance module #38812

Merged
merged 2 commits into from
Apr 17, 2018
Merged

[cloud] Add support for updating IAM role with ec2_instance module #38812

merged 2 commits into from
Apr 17, 2018

Conversation

ryansb
Copy link
Contributor

@ryansb ryansb commented Apr 16, 2018

SUMMARY

IAM instance profiles can't be switched via the modify_instance_attribute API call. This pull request adds checks on already-existing instances for the IAM instance profile ARN and name, then updates the profile association.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_instance

ANSIBLE VERSION
ADDITIONAL INFORMATION

Before:

        "instances": [                                                                                                                                                                                              
            {                                                                                                                                                                                                       
                "ami_launch_index": 0,                                                                                                                                                                              
                "architecture": "x86_64",                                                                                                                                                                           
                "block_device_mappings": [                                                                                                                                                                          
                    {                                                                                                                                                                                               
                        "device_name": "/dev/sda1",                                                                                                                                                                 
                        "ebs": {                                                                                                                                                                                    
                            "attach_time": "2018-04-13T19:52:22+00:00",                                                                                                                                             
                            "delete_on_termination": false,                                                                                                                                                         
                            "status": "attached",                                                                                                                                                                   
                            "volume_id": "vol-0f3d246ba2ba774c9"                                                                                                                                                    
                        }                                                                                                                                                                                           
                    }                                                                                                                                                                                               
                ],                                                                                                                                                                                                  
                "client_token": "a0ef26cc419a4eda822c7ac8dd0d4ec9",                                                                                                                                                 
                "ebs_optimized": false,                                                                                                                                                                             
                "ena_support": true,                                                                                                                                                                                
                "hypervisor": "xen",                                                                                                                                                                                
                "image_id": "ami-0ebdd976",                                                                                                                                                                         
                "instance_id": "i-088a1fbf6d42ad068",                                                                                                                                                               
                "instance_type": "t2.micro",                                                                                                                                                                        
                "key_name": "slscode_hornet_2018",                                                                                                                                                                  
                "launch_time": "2018-04-13T19:52:21+00:00",

After:

    "instances": [                                                                                                                                                                                                  
        {                                                                                                                                                                                                           
            "ami_launch_index": 0,                                                                                                                                                                                  
            "architecture": "x86_64",                                                                                                                                                                               
            "block_device_mappings": [                                                                                                                                                                              
                {                                                                                                                                                                                                   
                    "device_name": "/dev/sda1",                                                                                                                                                                     
                    "ebs": {                                                                                                                                                                                        
                        "attach_time": "2018-04-13T19:52:22+00:00",                                                                                                                                                 
                        "delete_on_termination": false,                                                                                                                                                             
                        "status": "attached",                                                                                                                                                                       
                        "volume_id": "vol-0f3d246ba2ba774c9"                                                                                                                                                        
                    }                                                                                                                                                                                               
                }                                                                                                                                                                                                   
            ],                                                                                                                                                                                                      
            "client_token": "a0ef26cc419a4eda822c7ac8dd0d4ec9",                                                                                                                                                     
            "ebs_optimized": false,                                                                                                                                                                                 
            "ena_support": true,                                                                                                                                                                                    
            "hypervisor": "xen",                                                                                                                                                                                    
            "iam_instance_profile": {                                                                                                                                                                               
                "arn": "arn:aws:iam::509803855674:instance-profile/otherTestRole",                                                                                                                                  
                "id": "AIPAJBBIY7IVIPOJ2SSDW"                                                                                                                                                                       
            },                                                                                                                                                                                                      
            "image_id": "ami-0ebdd976",                                                                                                                                                                             
            "instance_id": "i-088a1fbf6d42ad068",

@ryansb ryansb requested a review from s-hertel April 16, 2018 12:40
@ansibot
Copy link
Contributor

ansibot commented Apr 16, 2018

cc @willthames
click here for bot help

@ansibot ansibot added aws cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community. labels Apr 16, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 16, 2018
s-hertel
s-hertel reviewed Apr 17, 2018
View reviewed changes
lib/ansible/modules/cloud/amazon/ec2_instance.py
if instance_profile_setting.get('Arn') == desired_arn:
return False
# update association
ec2 = module.client('ec2')
Copy link
Contributor

Choose a reason for hiding this comment

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

I keep forgetting module is global :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it? That seems wrong to me. Is it global in every module?

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's not global in every module, but it is global in this module https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/ec2_instance.py#L1409

@ansibot ansibot added test This PR relates to tests. and removed owner_pr This PR is made by the module's maintainer. labels Apr 17, 2018
@ryansb ryansb merged commit 44d06f8 into ansible:devel Apr 17, 2018
@ryansb ryansb deleted the ec2_instance_role_switching branch April 17, 2018 19:02
willthames
willthames reviewed Apr 17, 2018
View reviewed changes
lib/ansible/modules/cloud/amazon/ec2_instance.py
try:
role = iam.get_instance_profile(InstanceProfileName=name_or_arn)
role = iam.get_instance_profile(InstanceProfileName=name_or_arn, aws_retry=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update the GUIDELINES if we're happy enough to use it now (it's so much nicer than creating _backoff methods with @AWSRetry decorators)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm quite happy with it (so nice to use!) but s-hertel brought up an interesting point about it for library/common functions. In module_utils files, we probably can't expect the caller to always have a retry decorator turned on for whatever client they pass in, so I think for utils we still have to use the decorators directly for now. What do you think?

@ryansb ryansb mentioned this pull request Apr 18, 2018
Merged
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
@ryansb @ilicmilan
[cloud] Add support for updating IAM role with ec2_instance module (a…
fa798da 
…nsible#38812)

* [cloud] Add support for updating IAM role with ec2_instance module

* Add test for updating IAM role
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@willthames willthames willthames left review comments

@s-hertel s-hertel s-hertel approved these changes

Assignees
No one assigned
Labels
aws cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ryansb @ansibot @willthames @s-hertel @bcoca