From d73db7f060369a200730be03b9a2868b57fa13bc Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Fri, 22 Mar 2019 14:27:12 +0800 Subject: [PATCH] azure subnet's properties should not be changed if parameter set to None (route table, nsg...) (#54019) --- .../modules/cloud/azure/azure_rm_subnet.py | 45 +++++++++---------- .../targets/azure_rm_subnet/tasks/main.yml | 12 ++++- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/lib/ansible/modules/cloud/azure/azure_rm_subnet.py b/lib/ansible/modules/cloud/azure/azure_rm_subnet.py index 1d84f23b20..f90e9af7fd 100644 --- a/lib/ansible/modules/cloud/azure/azure_rm_subnet.py +++ b/lib/ansible/modules/cloud/azure/azure_rm_subnet.py @@ -36,7 +36,6 @@ options: description: - CIDR defining the IPv4 address space of the subnet. Must be valid within the context of the virtual network. - required: true aliases: - address_prefix security_group: @@ -205,10 +204,6 @@ class AzureRMSubnet(AzureRMModuleBase): ) ) - required_if = [ - ('state', 'present', ['address_prefix_cidr']) - ] - self.results = dict( changed=False, state=dict() @@ -224,8 +219,7 @@ class AzureRMSubnet(AzureRMModuleBase): self.service_endpoints = None super(AzureRMSubnet, self).__init__(self.module_arg_spec, - supports_check_mode=True, - required_if=required_if) + supports_check_mode=True) def exec_module(self, **kwargs): @@ -235,9 +229,10 @@ class AzureRMSubnet(AzureRMModuleBase): for key in self.module_arg_spec: setattr(self, key, kwargs[key]) - if self.state == 'present' and not CIDR_PATTERN.match(self.address_prefix_cidr): + if self.address_prefix_cidr and not CIDR_PATTERN.match(self.address_prefix_cidr): self.fail("Invalid address_prefix_cidr value {0}".format(self.address_prefix_cidr)) + nsg = dict() if self.security_group: nsg = self.parse_nsg() @@ -262,19 +257,17 @@ class AzureRMSubnet(AzureRMModuleBase): results = subnet_to_dict(subnet) if self.state == 'present': - if self.address_prefix_cidr: - if results['address_prefix'] != self.address_prefix_cidr: - self.log("CHANGED: subnet {0} address_prefix_cidr".format(self.name)) - changed = True - results['address_prefix'] = self.address_prefix_cidr + if self.address_prefix_cidr and results['address_prefix'] != self.address_prefix_cidr: + self.log("CHANGED: subnet {0} address_prefix_cidr".format(self.name)) + changed = True + results['address_prefix'] = self.address_prefix_cidr - if nsg: - if results['network_security_group'].get('id') != nsg.get('id'): - self.log("CHANGED: subnet {0} network security group".format(self.name)) - changed = True - results['network_security_group']['id'] = nsg.get('id') - results['network_security_group']['name'] = nsg.get('name') - if self.route_table != results['route_table'].get('id'): + if self.security_group is not None and results['network_security_group'].get('id') != nsg.get('id'): + self.log("CHANGED: subnet {0} network security group".format(self.name)) + changed = True + results['network_security_group']['id'] = nsg.get('id') + results['network_security_group']['name'] = nsg.get('name') + if self.route_table is not None and self.route_table != results['route_table'].get('id'): changed = True results['route_table']['id'] = self.route_table self.log("CHANGED: subnet {0} route_table to {1}".format(self.name, route_table.get('name'))) @@ -310,6 +303,8 @@ class AzureRMSubnet(AzureRMModuleBase): if self.state == 'present' and changed: if not subnet: # create new subnet + if not self.address_prefix_cidr: + self.fail('address_prefix_cidr is not set') self.log('Creating subnet {0}'.format(self.name)) subnet = self.network_models.Subnet( address_prefix=self.address_prefix_cidr @@ -324,13 +319,13 @@ class AzureRMSubnet(AzureRMModuleBase): subnet = self.network_models.Subnet( address_prefix=results['address_prefix'] ) - if results['network_security_group'].get('id'): + if results['network_security_group'].get('id') is not None: subnet.network_security_group = self.network_models.NetworkSecurityGroup(id=results['network_security_group'].get('id')) - if self.route_table: - subnet.route_table = self.network_models.RouteTable(id=self.route_table) + if results['route_table'].get('id') is not None: + subnet.route_table = self.network_models.RouteTable(id=results['route_table'].get('id')) - if self.service_endpoints: - subnet.service_endpoints = self.service_endpoints + if results.get('service_endpoints') is not None: + subnet.service_endpoints = results['service_endpoints'] self.results['state'] = self.create_or_update_subnet(subnet) elif self.state == 'absent' and changed: diff --git a/test/integration/targets/azure_rm_subnet/tasks/main.yml b/test/integration/targets/azure_rm_subnet/tasks/main.yml index 2d9bd4ce81..1d8d71d567 100644 --- a/test/integration/targets/azure_rm_subnet/tasks/main.yml +++ b/test/integration/targets/azure_rm_subnet/tasks/main.yml @@ -48,6 +48,16 @@ - assert: that: output.changed +- name: Add the subnet back (idempontent) + azure_rm_subnet: + name: foobar + virtual_network_name: My_Virtual_Network + resource_group: "{{ resource_group }}" + register: output + +- assert: + that: not output.changed + - name: Create network security group azure_rm_securitygroup: name: secgroupfoo @@ -77,7 +87,6 @@ virtual_network_name: My_Virtual_Network resource_group: "{{ resource_group }}" address_prefix_cidr: "10.1.0.0/16" - security_group: secgroupfoo service_endpoints: - service: Microsoft.Sql locations: @@ -123,7 +132,6 @@ virtual_network_name: My_Virtual_Network resource_group: "{{ resource_group }}" address_prefix_cidr: "10.1.0.0/16" - route_table: "{{ route_table.id }}" security_group: "{{ nsg.state.id }}" tags: testing: testing