From ac74520b6f2e51b9722826c5867d2455b023309d Mon Sep 17 00:00:00 2001 From: Alex Lo Date: Sun, 26 Mar 2017 09:33:29 -0400 Subject: [PATCH] ec2_group: description property is immutable (#19790) * update integration tests for updated boto exception message * integration tests fail on both "test credential" test cases exception bubbles out of module. instead catch and wrap * ec2_group does not support updating a security group's description AWS security group descriptions are immutable. if ec2_group finds a group that matches by name, but the descriptions do not match, the module does not support this case previously it would check if the group was used, but would not do anything if it was old behavior was erroneous because it could make a user expect that the description change of a group was fine when in fact it did not occur also, it made an expensive check against all ec2 instances for no good reason * comments not doc strings * else must have pass w/o doc-string statement * Catch specific BotoServerException, give context around error when fetching SGs * python3 compatible exception blocks * add traceback to fail_json * two blank lines before first function --- lib/ansible/modules/cloud/amazon/ec2_group.py | 34 ++++++++++--------- .../roles/test_ec2_group/tasks/main.yml | 24 +++++++++++-- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index ad220ddb18..0b5dbcb109 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -140,10 +140,13 @@ EXAMPLES = ''' try: import boto.ec2 from boto.ec2.securitygroup import SecurityGroup + from boto.exception import BotoServerError HAS_BOTO = True except ImportError: HAS_BOTO = False +import traceback + def make_rule_key(prefix, rule, group_id, cidr_ip): """Creates a unique key for an individual group rule""" @@ -283,7 +286,13 @@ def main(): # find the group if present group = None groups = {} - for curGroup in ec2.get_all_security_groups(): + + try: + security_groups = ec2.get_all_security_groups() + except BotoServerError as e: + module.fail_json(msg="Error in get_all_security_groups: %s" % e.message, exception=traceback.format_exc()) + + for curGroup in security_groups: groups[curGroup.id] = curGroup if curGroup.name in groups: # Prioritise groups from the current VPC @@ -298,36 +307,29 @@ def main(): # Ensure requested group is absent if state == 'absent': if group: - '''found a match, delete it''' + # found a match, delete it try: if not module.check_mode: group.delete() - except Exception as e: - module.fail_json(msg="Unable to delete security group '%s' - %s" % (group, e)) + except BotoServerError as e: + module.fail_json(msg="Unable to delete security group '%s' - %s" % (group, e.message), exception=traceback.format_exc()) else: group = None changed = True else: - '''no match found, no changes required''' + # no match found, no changes required + pass # Ensure requested group is present elif state == 'present': if group: - '''existing group found''' - # check the group parameters are correct - group_in_use = False - rs = ec2.get_all_instances() - for r in rs: - for i in r.instances: - group_in_use |= reduce(lambda x, y: x | (y.name == 'public-ssh'), i.groups, False) - + # existing group if group.description != description: - if group_in_use: - module.fail_json(msg="Group description does not match, but it is in use so cannot be changed.") + module.fail_json(msg="Group description does not match existing group. ec2_group does not support this case.") # if the group doesn't exist, create it now else: - '''no match found, create it''' + # no match found, create it if not module.check_mode: group = ec2.create_security_group(name, description, vpc_id=vpc_id) diff --git a/test/integration/roles/test_ec2_group/tasks/main.yml b/test/integration/roles/test_ec2_group/tasks/main.yml index 8435794bce..cc27967cb8 100644 --- a/test/integration/roles/test_ec2_group/tasks/main.yml +++ b/test/integration/roles/test_ec2_group/tasks/main.yml @@ -74,7 +74,7 @@ assert: that: - 'result.failed' - - 'result.msg.startswith("value of region must be one of:")' + - 'result.msg.startswith("Region asdf querty 1234 does not seem to be available for aws module boto.ec2. If the region definitely exists, you may need to upgrade boto or extend with endpoints_path")' # ============================================================ - name: test valid region parameter @@ -155,7 +155,7 @@ assert: that: - 'result.failed' - - '"EC2ResponseError: 401 Unauthorized" in result.msg' + - '"Error in get_all_security_groups: AWS was not able to validate the provided access credentials" in result.msg' # ============================================================ - name: test credential parameters @@ -172,7 +172,7 @@ assert: that: - 'result.failed' - - '"EC2ResponseError: 401 Unauthorized" in result.msg' + - '"Error in get_all_security_groups: AWS was not able to validate the provided access credentials" in result.msg' # ============================================================ - name: test state=absent @@ -207,6 +207,24 @@ - 'result.changed' - 'result.group_id.startswith("sg-")' +# ============================================================ +- name: test state=present different description raises error + ec2_group: + name='{{ec2_group_name}}' + description='{{ec2_group_description}}CHANGED' + ec2_region='{{ec2_region}}' + ec2_access_key='{{ec2_access_key}}' + ec2_secret_key='{{ec2_secret_key}}' + state=present + ignore_errors: true + register: result + +- name: assert matching group with non-matching description raises error + assert: + that: + - 'result.failed' + - '"Group description does not match existing group. ec2_group does not support this case." in result.msg' + # ============================================================ - name: test state=present (expected changed=false) ec2_group: