From 17044152c02e6caf61cf7b01fc84d01b1b4ae59a Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 10 Nov 2023 12:29:41 +0100 Subject: [PATCH 1/2] Minor PEP8 sanity fixes (Ansible 2.16) --- .github/workflows/sanity.yml | 64 ------------------------ changelogs/fragments/20231110-sanity.yml | 3 ++ plugins/modules/autoscaling_group.py | 18 +++---- plugins/modules/rds_instance_snapshot.py | 2 +- 4 files changed, 13 insertions(+), 74 deletions(-) create mode 100644 changelogs/fragments/20231110-sanity.yml diff --git a/.github/workflows/sanity.yml b/.github/workflows/sanity.yml index f9e4b0b78ca..eb568c42fde 100644 --- a/.github/workflows/sanity.yml +++ b/.github/workflows/sanity.yml @@ -6,67 +6,3 @@ on: [workflow_call] # allow this workflow to be called from other workflows jobs: sanity: uses: ansible-network/github_actions/.github/workflows/sanity.yml@main - with: - matrix_include: "[]" - matrix_exclude: >- - [ - { - "ansible-version": "stable-2.9" - }, - { - "ansible-version": "stable-2.12", - "python-version": "3.7" - }, - { - "ansible-version": "stable-2.12", - "python-version": "3.11" - }, - { - "ansible-version": "stable-2.13", - "python-version": "3.7" - }, - { - "ansible-version": "stable-2.13", - "python-version": "3.11" - }, - { - "ansible-version": "stable-2.14", - "python-version": "3.7" - }, - { - "ansible-version": "stable-2.14", - "python-version": "3.8" - }, - { - "ansible-version": "stable-2.15", - "python-version": "3.7" - }, - { - "ansible-version": "stable-2.15", - "python-version": "3.8" - }, - { - "ansible-version": "milestone", - "python-version": "3.7" - }, - { - "ansible-version": "milestone", - "python-version": "3.8" - }, - { - "ansible-version": "milestone", - "python-version": "3.9" - }, - { - "ansible-version": "devel", - "python-version": "3.7" - }, - { - "ansible-version": "devel", - "python-version": "3.8" - }, - { - "ansible-version": "devel", - "python-version": "3.9" - } - ] diff --git a/changelogs/fragments/20231110-sanity.yml b/changelogs/fragments/20231110-sanity.yml new file mode 100644 index 00000000000..5a7faa03cbd --- /dev/null +++ b/changelogs/fragments/20231110-sanity.yml @@ -0,0 +1,3 @@ +minor_changes: +- autoscaling_group - minor PEP8 whitespace sanity fixes (https://github.com/ansible-collections/amazon.aws/pull/1846). +- rds_instance_snapshot - minor PEP8 whitespace sanity fixes (https://github.com/ansible-collections/amazon.aws/pull/1846). diff --git a/plugins/modules/autoscaling_group.py b/plugins/modules/autoscaling_group.py index 830470f253f..de45192adae 100644 --- a/plugins/modules/autoscaling_group.py +++ b/plugins/modules/autoscaling_group.py @@ -1675,15 +1675,15 @@ def get_instances_by_launch_config(props, lc_check, initial_instances): old_instances.append(i) else: - module.debug(f"Comparing initial instances with current: {*initial_instances,}") + module.debug(f"Comparing initial instances with current: {*initial_instances, }") for i in props["instances"]: if i not in initial_instances: new_instances.append(i) else: old_instances.append(i) - module.debug(f"New instances: {len(new_instances)}, {*new_instances,}") - module.debug(f"Old instances: {len(old_instances)}, {*old_instances,}") + module.debug(f"New instances: {len(new_instances)}, {*new_instances, }") + module.debug(f"Old instances: {len(old_instances)}, {*old_instances, }") return new_instances, old_instances @@ -1702,15 +1702,15 @@ def get_instances_by_launch_template(props, lt_check, initial_instances): else: old_instances.append(i) else: - module.debug(f"Comparing initial instances with current: {*initial_instances,}") + module.debug(f"Comparing initial instances with current: {*initial_instances, }") for i in props["instances"]: if i not in initial_instances: new_instances.append(i) else: old_instances.append(i) - module.debug(f"New instances: {len(new_instances)}, {*new_instances,}") - module.debug(f"Old instances: {len(old_instances)}, {*old_instances,}") + module.debug(f"New instances: {len(new_instances)}, {*new_instances, }") + module.debug(f"Old instances: {len(old_instances)}, {*old_instances, }") return new_instances, old_instances @@ -1775,9 +1775,9 @@ def terminate_batch(connection, replace_instances, initial_instances, leftovers= instances_to_terminate = list_purgeable_instances(props, lc_check, lt_check, replace_instances, initial_instances) module.debug(f"new instances needed: {num_new_inst_needed}") - module.debug(f"new instances: {*new_instances,}") - module.debug(f"old instances: {*old_instances,}") - module.debug(f"batch instances: {*instances_to_terminate,}") + module.debug(f"new instances: {*new_instances, }") + module.debug(f"old instances: {*old_instances, }") + module.debug(f"batch instances: {*instances_to_terminate, }") if num_new_inst_needed == 0: decrement_capacity = True diff --git a/plugins/modules/rds_instance_snapshot.py b/plugins/modules/rds_instance_snapshot.py index 0c67d158366..ae1d5d7b190 100644 --- a/plugins/modules/rds_instance_snapshot.py +++ b/plugins/modules/rds_instance_snapshot.py @@ -270,7 +270,7 @@ def get_parameters(parameters, method_name): required_options = get_boto3_client_method_parameters(client, method_name, required=True) if any(parameters.get(k) is None for k in required_options): method_description = get_rds_method_attribute(method_name, module).operation_description - module.fail_json(msg=f"To {method_description} requires the parameters: {*required_options,}") + module.fail_json(msg=f"To {method_description} requires the parameters: {*required_options, }") options = get_boto3_client_method_parameters(client, method_name) parameters = dict((k, v) for k, v in parameters.items() if k in options and v is not None) From 9c1709c7458687f5263f1224c9d88c0ffd6e7174 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 10 Nov 2023 13:56:49 +0100 Subject: [PATCH 2/2] Unit test fixes --- changelogs/fragments/20231110-sanity.yml | 1 + plugins/modules/ec2_ami_info.py | 7 ++-- tests/unit/module_utils/test_acm.py | 1 + .../unit/plugins/modules/test_ec2_ami_info.py | 35 +++++-------------- 4 files changed, 15 insertions(+), 29 deletions(-) diff --git a/changelogs/fragments/20231110-sanity.yml b/changelogs/fragments/20231110-sanity.yml index 5a7faa03cbd..44c4a3379fe 100644 --- a/changelogs/fragments/20231110-sanity.yml +++ b/changelogs/fragments/20231110-sanity.yml @@ -1,3 +1,4 @@ minor_changes: - autoscaling_group - minor PEP8 whitespace sanity fixes (https://github.com/ansible-collections/amazon.aws/pull/1846). - rds_instance_snapshot - minor PEP8 whitespace sanity fixes (https://github.com/ansible-collections/amazon.aws/pull/1846). +- ec2_ami_info - simplify parameters to ``get_image_attribute`` to only pass ID of image (https://github.com/ansible-collections/amazon.aws/pull/1846). diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index 2c60f590996..2929a0292df 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -260,10 +260,10 @@ def get_images(ec2_client, request_args): return images -def get_image_attribute(ec2_client, image): +def get_image_attribute(ec2_client, image_id): try: launch_permissions = ec2_client.describe_image_attribute( - aws_retry=True, Attribute="launchPermission", ImageId=image["image_id"] + aws_retry=True, Attribute="launchPermission", ImageId=image_id ) except (ClientError, BotoCoreError) as err: raise AmiInfoFailure(err, "error describing image attribute") @@ -276,9 +276,10 @@ def list_ec2_images(ec2_client, module, request_args): for image in images: try: + image_id = image["image_id"] image["tags"] = boto3_tag_list_to_ansible_dict(image.get("tags", [])) if module.params.get("describe_image_attributes"): - launch_permissions = get_image_attribute(ec2_client, image).get("LaunchPermissions", []) + launch_permissions = get_image_attribute(ec2_client, image_id).get("LaunchPermissions", []) image["launch_permissions"] = [camel_dict_to_snake_dict(perm) for perm in launch_permissions] except is_boto3_error_code("AuthFailure"): # describing launch permissions of images owned by others is not permitted, but shouldn't cause failures diff --git a/tests/unit/module_utils/test_acm.py b/tests/unit/module_utils/test_acm.py index c7971831f1c..e3b49d146f7 100644 --- a/tests/unit/module_utils/test_acm.py +++ b/tests/unit/module_utils/test_acm.py @@ -17,6 +17,7 @@ # Handled by HAS_BOTO3 pass + from ansible_collections.amazon.aws.plugins.module_utils.acm import ACMServiceManager from ansible_collections.amazon.aws.plugins.module_utils.acm import acm_catch_boto_exception diff --git a/tests/unit/plugins/modules/test_ec2_ami_info.py b/tests/unit/plugins/modules/test_ec2_ami_info.py index c33553ee55a..a5abc77af60 100644 --- a/tests/unit/plugins/modules/test_ec2_ami_info.py +++ b/tests/unit/plugins/modules/test_ec2_ami_info.py @@ -94,33 +94,13 @@ def test_get_image_attribute(): "LaunchPermissions": [{"UserId": "1234567890"}, {"UserId": "0987654321"}], } - image = { - "architecture": "x86_64", - "blockDeviceMappings": [ - { - "device_name": "/dev/sda1", - "ebs": { - "delete_on_termination": "True", - "encrypted": "False", - "snapshot_id": "snap-0f00cba784af62428", - "volume_size": 10, - "volume_Type": "gp2", - }, - } - ], - "image_id": "ami-1234567890", - "image_location": "1234567890/test-ami-uefi-boot", - "image_type": "machine", - "name": "test-ami-uefi-boot", - "owner_id": "1234567890", - "platform_details": "Linux/UNIX", - } + image_id = "ami-1234567890" - get_image_attribute_result = ec2_ami_info.get_image_attribute(ec2_client, image) + get_image_attribute_result = ec2_ami_info.get_image_attribute(ec2_client, image_id) ec2_client.describe_image_attribute.call_count == 1 ec2_client.describe_image_attribute.assert_called_with( - aws_retry=True, Attribute="launchPermission", ImageId=image["image_id"] + aws_retry=True, Attribute="launchPermission", ImageId=image_id ) assert len(get_image_attribute_result["LaunchPermissions"]) == 2 @@ -203,7 +183,10 @@ def test_list_ec2_images(m_get_images, m_get_image_attribute): m_get_images.assert_called_with(ec2_client, request_args) assert m_get_image_attribute.call_count == 2 - assert m_get_image_attribute.has_calls([call(ec2_client, images[0])], [call(ec2_client, images[1])]) + m_get_image_attribute.assert_has_calls( + [call(ec2_client, images[0]["image_id"])], + [call(ec2_client, images[1]["image_id"])], + ) assert len(list_ec2_images_result) == 2 assert list_ec2_images_result[0]["image_id"] == "ami-1234567890" @@ -234,8 +217,8 @@ def test_api_failure_get_images(ec2_client): def test_api_failure_get_image_attribute(ec2_client): - image = {"image_id": "ami-1234567890"} + image_id = "ami-1234567890" ec2_client.describe_image_attribute.side_effect = a_boto_exception() with pytest.raises(ec2_ami_info.AmiInfoFailure): - ec2_ami_info.get_image_attribute(ec2_client, image) + ec2_ami_info.get_image_attribute(ec2_client, image_id)