From cc9d8dc2e813c319fd6ecb095a9633baff2735ff Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 6 Jul 2022 17:30:29 +0000 Subject: [PATCH] Fix NoneType errors with elb_classic_lb (#915) (#919) [PR #915/8ce9c198 backport][stable-3] Fix NoneType errors with elb_classic_lb This is a backport of PR #915 as merged into main (8ce9c19). SUMMARY fixes: #589 fixes: #914 Fixes two NoneType related bugs when creating new ELBs. (includes extra tests this time to trigger the bugs) ISSUE TYPE Bugfix Pull Request COMPONENT NAME plugins/modules/elb_classic_lb.py ADDITIONAL INFORMATION Reviewed-by: Mark Chappell --- ...14-elb_classic_lb-security_group_names.yml | 3 + plugins/modules/elb_classic_lb.py | 4 +- .../elb_classic_lb/tasks/complex_changes.yml | 330 ++++++++++++++++++ .../targets/elb_classic_lb/tasks/main.yml | 1 + 4 files changed, 336 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/914-elb_classic_lb-security_group_names.yml create mode 100644 tests/integration/targets/elb_classic_lb/tasks/complex_changes.yml diff --git a/changelogs/fragments/914-elb_classic_lb-security_group_names.yml b/changelogs/fragments/914-elb_classic_lb-security_group_names.yml new file mode 100644 index 00000000000..cba89ddadb6 --- /dev/null +++ b/changelogs/fragments/914-elb_classic_lb-security_group_names.yml @@ -0,0 +1,3 @@ +bugfixes: +- elb_classic_lb - fix ``'NoneType' object has no attribute`` bug when creating a new ELB using security group names (https://github.com/ansible-collections/amazon.aws/issues/914). +- elb_classic_lb - fix ``'NoneType' object has no attribute`` bug when creating a new ELB in check mode with a health check (https://github.com/ansible-collections/amazon.aws/pull/915). diff --git a/plugins/modules/elb_classic_lb.py b/plugins/modules/elb_classic_lb.py index 2d7913ac044..61c77580583 100644 --- a/plugins/modules/elb_classic_lb.py +++ b/plugins/modules/elb_classic_lb.py @@ -744,7 +744,7 @@ def __init__(self, module): if security_group_names: # Use the subnets attached to the VPC to find which VPC we're in and # limit the search - if self.elb.get('Subnets', None): + if self.elb and self.elb.get('Subnets', None): subnets = set(self.elb.get('Subnets') + list(self.subnets or [])) else: subnets = set(self.subnets) @@ -1440,7 +1440,7 @@ def _set_health_check(self): """Set health check values on ELB as needed""" health_check_config = self._format_healthcheck() - if health_check_config == self.elb['HealthCheck']: + if self.elb and health_check_config == self.elb['HealthCheck']: return False self.changed = True diff --git a/tests/integration/targets/elb_classic_lb/tasks/complex_changes.yml b/tests/integration/targets/elb_classic_lb/tasks/complex_changes.yml new file mode 100644 index 00000000000..5f75f84d3c7 --- /dev/null +++ b/tests/integration/targets/elb_classic_lb/tasks/complex_changes.yml @@ -0,0 +1,330 @@ +--- +- block: + - name: Create ELB for testing complex updates (CHECK) + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ default_listeners }}' + health_check: '{{ default_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-a', '{{ resource_prefix }}-b'] + tags: '{{ default_tags }}' + cross_az_load_balancing: True + idle_timeout: '{{ default_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ default_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + check_mode: True + + - name: Verify that we expect to change + assert: + that: + - result is changed + + - name: Create ELB for testing complex updates + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ default_listeners }}' + health_check: '{{ default_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-a', '{{ resource_prefix }}-b'] + tags: '{{ default_tags }}' + cross_az_load_balancing: True + idle_timeout: '{{ default_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ default_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + + - name: Verify that simple parameters were set + assert: + that: + - result is changed + - result.elb.status == "created" + - availability_zone_a in result.elb.zones + - availability_zone_b in result.elb.zones + - subnet_a in result.elb.subnets + - subnet_b in result.elb.subnets + - default_listener_tuples[0] in result.elb.listeners + - default_listener_tuples[1] in result.elb.listeners + - sg_a in result.elb.security_group_ids + - sg_b in result.elb.security_group_ids + - sg_c not in result.elb.security_group_ids + - result.elb.health_check.healthy_threshold == default_health_check['healthy_threshold'] + - result.elb.health_check.interval == default_health_check['interval'] + - result.elb.health_check.target == default_health_check_target + - result.elb.health_check.timeout == default_health_check['response_timeout'] + - result.elb.health_check.unhealthy_threshold == default_health_check['unhealthy_threshold'] + - result.elb.tags == default_tags + - result.elb.cross_az_load_balancing == 'yes' + - result.elb.idle_timeout == default_idle_timeout + - result.elb.connection_draining_timeout == default_drain_timeout + - result.elb.proxy_policy == None + - result.load_balancer.load_balancer_attributes.access_log.emit_interval == default_logging_interval + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_name == s3_logging_bucket_a + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_prefix == default_logging_prefix + - result.load_balancer.load_balancer_attributes.access_log.enabled == True + + - name: Create ELB for testing complex updates - idempotency (CHECK) + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ default_listeners }}' + health_check: '{{ default_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-a', '{{ resource_prefix }}-b'] + tags: '{{ default_tags }}' + cross_az_load_balancing: True + idle_timeout: '{{ default_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ default_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + check_mode: True + + - name: Verify that we expect to not change + assert: + that: + - result is not changed + + - name: Create ELB for testing complex updates - idempotency + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ default_listeners }}' + health_check: '{{ default_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-a', '{{ resource_prefix }}-b'] + tags: '{{ default_tags }}' + cross_az_load_balancing: True + idle_timeout: '{{ default_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ default_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + + - name: Verify that simple parameters were set + assert: + that: + - result is not changed + - result.elb.status == "exists" + - availability_zone_a in result.elb.zones + - availability_zone_b in result.elb.zones + - subnet_a in result.elb.subnets + - subnet_b in result.elb.subnets + - default_listener_tuples[0] in result.elb.listeners + - default_listener_tuples[1] in result.elb.listeners + - sg_a in result.elb.security_group_ids + - sg_b in result.elb.security_group_ids + - sg_c not in result.elb.security_group_ids + - result.elb.health_check.healthy_threshold == default_health_check['healthy_threshold'] + - result.elb.health_check.interval == default_health_check['interval'] + - result.elb.health_check.target == default_health_check_target + - result.elb.health_check.timeout == default_health_check['response_timeout'] + - result.elb.health_check.unhealthy_threshold == default_health_check['unhealthy_threshold'] + - result.elb.tags == default_tags + - result.elb.cross_az_load_balancing == 'yes' + - result.elb.idle_timeout == default_idle_timeout + - result.elb.connection_draining_timeout == default_drain_timeout + - result.elb.proxy_policy == None + - result.load_balancer.load_balancer_attributes.access_log.emit_interval == default_logging_interval + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_name == s3_logging_bucket_a + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_prefix == default_logging_prefix + - result.load_balancer.load_balancer_attributes.access_log.enabled == True + + ### + + - name: Perform complex update (CHECK) + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ updated_listeners }}' + health_check: '{{ updated_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-c', '{{ resource_prefix }}-b'] + tags: '{{ updated_tags }}' + cross_az_load_balancing: False + idle_timeout: '{{ updated_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ updated_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + check_mode: True + + - name: Verify that we expect to change + assert: + that: + - result is changed + + - name: Perform complex update + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ updated_listeners }}' + health_check: '{{ updated_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-c', '{{ resource_prefix }}-b'] + tags: '{{ updated_tags }}' + cross_az_load_balancing: False + idle_timeout: '{{ updated_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ updated_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + + - name: Verify that simple parameters were set + assert: + that: + - result is changed + - result.elb.status == "exists" + - availability_zone_a in result.elb.zones + - availability_zone_b in result.elb.zones + - subnet_a in result.elb.subnets + - subnet_b in result.elb.subnets + - updated_listener_tuples[0] in result.elb.listeners + - updated_listener_tuples[1] in result.elb.listeners + - sg_a not in result.elb.security_group_ids + - sg_b in result.elb.security_group_ids + - sg_c in result.elb.security_group_ids + - result.elb.health_check.healthy_threshold == updated_health_check['healthy_threshold'] + - result.elb.health_check.interval == updated_health_check['interval'] + - result.elb.health_check.target == updated_health_check_target + - result.elb.health_check.timeout == updated_health_check['response_timeout'] + - result.elb.health_check.unhealthy_threshold == updated_health_check['unhealthy_threshold'] + - result.elb.tags == updated_tags + - result.elb.cross_az_load_balancing == 'no' + - result.elb.idle_timeout == updated_idle_timeout + - result.elb.connection_draining_timeout == default_drain_timeout + - result.elb.proxy_policy == None + - result.load_balancer.load_balancer_attributes.access_log.emit_interval == updated_logging_interval + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_name == s3_logging_bucket_a + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_prefix == default_logging_prefix + - result.load_balancer.load_balancer_attributes.access_log.enabled == True + + - name: Perform complex update idempotency (CHECK) + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ updated_listeners }}' + health_check: '{{ updated_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-c', '{{ resource_prefix }}-b'] + tags: '{{ updated_tags }}' + cross_az_load_balancing: False + idle_timeout: '{{ updated_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ updated_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + check_mode: True + + - name: Verify we expect to not change + assert: + that: + - result is not changed + + - name: Perform complex update - idempotency + elb_classic_lb: + name: "{{ elb_name }}" + state: present + # zones: ['{{ availability_zone_a }}', '{{ availability_zone_b }}'] + listeners: '{{ updated_listeners }}' + health_check: '{{ updated_health_check }}' + wait: true + scheme: 'internal' + subnets: ['{{ subnet_a }}', '{{ subnet_b }}'] + security_group_names: ['{{ resource_prefix }}-c', '{{ resource_prefix }}-b'] + tags: '{{ updated_tags }}' + cross_az_load_balancing: False + idle_timeout: '{{ updated_idle_timeout }}' + connection_draining_timeout: '{{ default_drain_timeout }}' + access_logs: + interval: '{{ updated_logging_interval }}' + s3_location: '{{ s3_logging_bucket_a }}' + s3_prefix: '{{ default_logging_prefix }}' + enabled: true + register: result + + - name: Verify that simple parameters were set + assert: + that: + - result is not changed + - result.elb.status == "exists" + - availability_zone_a in result.elb.zones + - availability_zone_b in result.elb.zones + - subnet_a in result.elb.subnets + - subnet_b in result.elb.subnets + - updated_listener_tuples[0] in result.elb.listeners + - updated_listener_tuples[1] in result.elb.listeners + - sg_a not in result.elb.security_group_ids + - sg_b in result.elb.security_group_ids + - sg_c in result.elb.security_group_ids + - result.elb.health_check.healthy_threshold == updated_health_check['healthy_threshold'] + - result.elb.health_check.interval == updated_health_check['interval'] + - result.elb.health_check.target == updated_health_check_target + - result.elb.health_check.timeout == updated_health_check['response_timeout'] + - result.elb.health_check.unhealthy_threshold == updated_health_check['unhealthy_threshold'] + - result.elb.tags == updated_tags + - result.elb.cross_az_load_balancing == 'no' + - result.elb.idle_timeout == updated_idle_timeout + - result.elb.connection_draining_timeout == default_drain_timeout + - result.elb.proxy_policy == None + - result.load_balancer.load_balancer_attributes.access_log.emit_interval == updated_logging_interval + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_name == s3_logging_bucket_a + - result.load_balancer.load_balancer_attributes.access_log.s3_bucket_prefix == default_logging_prefix + - result.load_balancer.load_balancer_attributes.access_log.enabled == True + + always: + + # ============================================================ + - name: remove the test load balancer + elb_classic_lb: + name: "{{ elb_name }}" + state: absent + wait: true + register: result + ignore_errors: true diff --git a/tests/integration/targets/elb_classic_lb/tasks/main.yml b/tests/integration/targets/elb_classic_lb/tasks/main.yml index d964c47d15c..40ffd112728 100644 --- a/tests/integration/targets/elb_classic_lb/tasks/main.yml +++ b/tests/integration/targets/elb_classic_lb/tasks/main.yml @@ -36,6 +36,7 @@ - include_tasks: schema_change.yml - include_tasks: simple_changes.yml + - include_tasks: complex_changes.yml always: