From 6fb7a2b7ccdeeea8a474565e1e2012692508ce31 Mon Sep 17 00:00:00 2001 From: The Magician Date: Fri, 12 Jul 2019 12:11:23 -0700 Subject: [PATCH] Bug fixes for GCP modules (#58903) --- .../cloud/google/gcp_iam_service_account.py | 2 +- .../google/gcp_iam_service_account_facts.py | 4 +- .../cloud/google/gcp_pubsub_subscription.py | 41 +++++++++++-------- .../modules/cloud/google/gcp_pubsub_topic.py | 28 +++++++------ .../gcp_compute_url_map/tasks/main.yml | 2 - .../gcp_compute_vpn_tunnel/tasks/main.yml | 2 - .../gcp_dns_managed_zone/tasks/main.yml | 2 - .../tasks/main.yml | 2 - .../gcp_iam_service_account/defaults/main.yml | 1 - .../gcp_iam_service_account/tasks/main.yml | 14 +++---- 10 files changed, 48 insertions(+), 50 deletions(-) diff --git a/lib/ansible/modules/cloud/google/gcp_iam_service_account.py b/lib/ansible/modules/cloud/google/gcp_iam_service_account.py index 47a0587f2f5..46674cb7ba9 100644 --- a/lib/ansible/modules/cloud/google/gcp_iam_service_account.py +++ b/lib/ansible/modules/cloud/google/gcp_iam_service_account.py @@ -61,7 +61,7 @@ extends_documentation_fragment: gcp EXAMPLES = ''' - name: create a service account gcp_iam_service_account: - name: "{{ sa_name }}" + name: sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com display_name: My Ansible test key project: test_project auth_kind: serviceaccount diff --git a/lib/ansible/modules/cloud/google/gcp_iam_service_account_facts.py b/lib/ansible/modules/cloud/google/gcp_iam_service_account_facts.py index 0f4c760bc61..09b417329cc 100644 --- a/lib/ansible/modules/cloud/google/gcp_iam_service_account_facts.py +++ b/lib/ansible/modules/cloud/google/gcp_iam_service_account_facts.py @@ -108,8 +108,8 @@ def main(): module.params['scopes'] = ['https://www.googleapis.com/auth/iam'] items = fetch_list(module, collection(module)) - if items.get('items'): - items = items.get('items') + if items.get('accounts'): + items = items.get('accounts') else: items = [] return_value = {'resources': items} diff --git a/lib/ansible/modules/cloud/google/gcp_pubsub_subscription.py b/lib/ansible/modules/cloud/google/gcp_pubsub_subscription.py index 392ee15a790..7425a4a88d6 100644 --- a/lib/ansible/modules/cloud/google/gcp_pubsub_subscription.py +++ b/lib/ansible/modules/cloud/google/gcp_pubsub_subscription.py @@ -295,6 +295,7 @@ expirationPolicy: from ansible.module_utils.gcp_utils import navigate_hash, GcpSession, GcpModule, GcpRequest, remove_nones_from_dict, replace_resource_dict import json +import re ################################################################################ # Main @@ -385,8 +386,8 @@ def delete(module, link): def resource_to_request(module): request = { - u'name': module.params.get('name'), - u'topic': replace_resource_dict(module.params.get(u'topic', {}), 'name'), + u'name': name_pattern(module.params.get('name'), module), + u'topic': topic_pattern(replace_resource_dict(module.params.get(u'topic', {}), 'name'), module), u'labels': module.params.get('labels'), u'pushConfig': SubscriptionPushconfig(module.params.get('push_config', {}), module).to_request(), u'ackDeadlineSeconds': module.params.get('ack_deadline_seconds'), @@ -394,7 +395,6 @@ def resource_to_request(module): u'retainAckedMessages': module.params.get('retain_acked_messages'), u'expirationPolicy': SubscriptionExpirationpolicy(module.params.get('expiration_policy', {}), module).to_request(), } - request = encode_request(request, module) return_vals = {} for k, v in request.items(): if v or v is False: @@ -431,8 +431,6 @@ def return_if_object(module, response, allow_not_found=False): except getattr(json.decoder, 'JSONDecodeError', ValueError): module.fail_json(msg="Invalid JSON response with error: %s" % response.text) - result = decode_request(result, module) - if navigate_hash(result, ['error', 'errors']): module.fail_json(msg=navigate_hash(result, ['error', 'errors'])) @@ -442,7 +440,6 @@ def return_if_object(module, response, allow_not_found=False): def is_different(module, response): request = resource_to_request(module) response = response_to_hash(module, response) - request = decode_request(request, module) # Remove all output-only from response. response_vals = {} @@ -462,8 +459,8 @@ def is_different(module, response): # This is for doing comparisons with Ansible's current parameters. def response_to_hash(module, response): return { - u'name': module.params.get('name'), - u'topic': replace_resource_dict(module.params.get(u'topic', {}), 'name'), + u'name': name_pattern(module.params.get('name'), module), + u'topic': topic_pattern(replace_resource_dict(module.params.get(u'topic', {}), 'name'), module), u'labels': response.get(u'labels'), u'pushConfig': SubscriptionPushconfig(response.get(u'pushConfig', {}), module).from_response(), u'ackDeadlineSeconds': response.get(u'ackDeadlineSeconds'), @@ -473,21 +470,29 @@ def response_to_hash(module, response): } -def decode_request(response, module): - if 'name' in response: - response['name'] = response['name'].split('/')[-1] +def name_pattern(name, module): + if name is None: + return - if 'topic' in response: - response['topic'] = response['topic'].split('/')[-1] + regex = r"projects/.*/subscriptions/.*" - return response + if not re.match(regex, name): + name = "projects/{project}/subscriptions/{name}".format(**module.params) + + return name -def encode_request(request, module): - request['topic'] = '/'.join(['projects', module.params['project'], 'topics', replace_resource_dict(request['topic'], 'name')]) - request['name'] = '/'.join(['projects', module.params['project'], 'subscriptions', module.params['name']]) +def topic_pattern(name, module): + if name is None: + return - return request + regex = r"projects/.*/topics/.*" + + if not re.match(regex, name): + formatted_params = {'project': module.params['project'], 'topic': replace_resource_dict(module.params['topic'], 'name')} + name = "projects/{project}/topics/{topic}".format(**formatted_params) + + return name class SubscriptionPushconfig(object): diff --git a/lib/ansible/modules/cloud/google/gcp_pubsub_topic.py b/lib/ansible/modules/cloud/google/gcp_pubsub_topic.py index 46038c6a229..017cc508d95 100644 --- a/lib/ansible/modules/cloud/google/gcp_pubsub_topic.py +++ b/lib/ansible/modules/cloud/google/gcp_pubsub_topic.py @@ -107,6 +107,7 @@ labels: from ansible.module_utils.gcp_utils import navigate_hash, GcpSession, GcpModule, GcpRequest, replace_resource_dict import json +import re ################################################################################ # Main @@ -181,8 +182,11 @@ def delete(module, link): def resource_to_request(module): - request = {u'name': module.params.get('name'), u'kmsKeyName': module.params.get('kms_key_name'), u'labels': module.params.get('labels')} - request = encode_request(request, module) + request = { + u'name': name_pattern(module.params.get('name'), module), + u'kmsKeyName': module.params.get('kms_key_name'), + u'labels': module.params.get('labels'), + } return_vals = {} for k, v in request.items(): if v or v is False: @@ -219,8 +223,6 @@ def return_if_object(module, response, allow_not_found=False): except getattr(json.decoder, 'JSONDecodeError', ValueError): module.fail_json(msg="Invalid JSON response with error: %s" % response.text) - result = decode_request(result, module) - if navigate_hash(result, ['error', 'errors']): module.fail_json(msg=navigate_hash(result, ['error', 'errors'])) @@ -230,7 +232,6 @@ def return_if_object(module, response, allow_not_found=False): def is_different(module, response): request = resource_to_request(module) response = response_to_hash(module, response) - request = decode_request(request, module) # Remove all output-only from response. response_vals = {} @@ -249,18 +250,19 @@ def is_different(module, response): # Remove unnecessary properties from the response. # This is for doing comparisons with Ansible's current parameters. def response_to_hash(module, response): - return {u'name': module.params.get('name'), u'kmsKeyName': module.params.get('kms_key_name'), u'labels': response.get(u'labels')} + return {u'name': name_pattern(module.params.get('name'), module), u'kmsKeyName': module.params.get('kms_key_name'), u'labels': response.get(u'labels')} -def decode_request(response, module): - if 'name' in response: - response['name'] = response['name'].split('/')[-1] - return response +def name_pattern(name, module): + if name is None: + return + regex = r"projects/.*/topics/.*" -def encode_request(request, module): - request['name'] = '/'.join(['projects', module.params['project'], 'topics', module.params['name']]) - return request + if not re.match(regex, name): + name = "projects/{project}/topics/{name}".format(**module.params) + + return name if __name__ == '__main__': diff --git a/test/integration/targets/gcp_compute_url_map/tasks/main.yml b/test/integration/targets/gcp_compute_url_map/tasks/main.yml index e359dd70476..7d8d45afb8b 100644 --- a/test/integration/targets/gcp_compute_url_map/tasks/main.yml +++ b/test/integration/targets/gcp_compute_url_map/tasks/main.yml @@ -113,7 +113,6 @@ assert: that: - result.changed == true - - result.has_key('kind') == False - name: verify that url_map was deleted gcp_compute_url_map_facts: filters: @@ -142,7 +141,6 @@ assert: that: - result.changed == false - - result.has_key('kind') == False #--------------------------------------------------------- # Post-test teardown # If errors happen, don't crash the playbook! diff --git a/test/integration/targets/gcp_compute_vpn_tunnel/tasks/main.yml b/test/integration/targets/gcp_compute_vpn_tunnel/tasks/main.yml index 9e9db557791..b13d306bc41 100644 --- a/test/integration/targets/gcp_compute_vpn_tunnel/tasks/main.yml +++ b/test/integration/targets/gcp_compute_vpn_tunnel/tasks/main.yml @@ -128,7 +128,6 @@ assert: that: - result.changed == true - - result.has_key('kind') == False - name: verify that vpn_tunnel was deleted gcp_compute_vpn_tunnel_facts: filters: @@ -161,7 +160,6 @@ assert: that: - result.changed == false - - result.has_key('kind') == False #--------------------------------------------------------- # Post-test teardown # If errors happen, don't crash the playbook! diff --git a/test/integration/targets/gcp_dns_managed_zone/tasks/main.yml b/test/integration/targets/gcp_dns_managed_zone/tasks/main.yml index e274a380633..3b6c1d67974 100644 --- a/test/integration/targets/gcp_dns_managed_zone/tasks/main.yml +++ b/test/integration/targets/gcp_dns_managed_zone/tasks/main.yml @@ -82,7 +82,6 @@ assert: that: - result.changed == true - - result.has_key('kind') == False - name: verify that managed_zone was deleted gcp_dns_managed_zone_facts: dns_name: test.somewild2.example.com. @@ -111,4 +110,3 @@ assert: that: - result.changed == false - - result.has_key('kind') == False diff --git a/test/integration/targets/gcp_dns_resource_record_set/tasks/main.yml b/test/integration/targets/gcp_dns_resource_record_set/tasks/main.yml index 9128d5a7bdb..cccacb6eaf4 100644 --- a/test/integration/targets/gcp_dns_resource_record_set/tasks/main.yml +++ b/test/integration/targets/gcp_dns_resource_record_set/tasks/main.yml @@ -108,7 +108,6 @@ assert: that: - result.changed == true - - result.has_key('kind') == False - name: verify that resource_record_set was deleted gcp_dns_resource_record_set_facts: managed_zone: "{{ managed_zone }}" @@ -141,7 +140,6 @@ assert: that: - result.changed == false - - result.has_key('kind') == False #--------------------------------------------------------- # Post-test teardown # If errors happen, don't crash the playbook! diff --git a/test/integration/targets/gcp_iam_service_account/defaults/main.yml b/test/integration/targets/gcp_iam_service_account/defaults/main.yml index 5595dfb48ec..ba66644fc1c 100644 --- a/test/integration/targets/gcp_iam_service_account/defaults/main.yml +++ b/test/integration/targets/gcp_iam_service_account/defaults/main.yml @@ -1,3 +1,2 @@ --- resource_name: "{{ resource_prefix }}" -sa_name: sa-{{ 100000 | random }}@graphite-playground.google.com.iam.gserviceaccount.com diff --git a/test/integration/targets/gcp_iam_service_account/tasks/main.yml b/test/integration/targets/gcp_iam_service_account/tasks/main.yml index aa6dab354f2..ec1ae99b592 100644 --- a/test/integration/targets/gcp_iam_service_account/tasks/main.yml +++ b/test/integration/targets/gcp_iam_service_account/tasks/main.yml @@ -15,7 +15,7 @@ # Pre-test setup - name: delete a service account gcp_iam_service_account: - name: "{{ sa_name }}" + name: sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com display_name: My Ansible test key project: "{{ gcp_project }}" auth_kind: "{{ gcp_cred_kind }}" @@ -24,7 +24,7 @@ #---------------------------------------------------------- - name: create a service account gcp_iam_service_account: - name: "{{ sa_name }}" + name: sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com display_name: My Ansible test key project: "{{ gcp_project }}" auth_kind: "{{ gcp_cred_kind }}" @@ -46,11 +46,11 @@ - name: verify that command succeeded assert: that: - - results['resources'] | map(attribute='name') | select("match", ".*{{ sa_name }}.*") | list | length == 1 + - results['resources'] | map(attribute='name') | select("match", ".*sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com.*") | list | length == 1 # ---------------------------------------------------------------------------- - name: create a service account that already exists gcp_iam_service_account: - name: "{{ sa_name }}" + name: sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com display_name: My Ansible test key project: "{{ gcp_project }}" auth_kind: "{{ gcp_cred_kind }}" @@ -64,7 +64,7 @@ #---------------------------------------------------------- - name: delete a service account gcp_iam_service_account: - name: "{{ sa_name }}" + name: sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com display_name: My Ansible test key project: "{{ gcp_project }}" auth_kind: "{{ gcp_cred_kind }}" @@ -86,11 +86,11 @@ - name: verify that command succeeded assert: that: - - results['resources'] | map(attribute='name') | select("match", ".*{{ sa_name }}.*") | list | length == 0 + - results['resources'] | map(attribute='name') | select("match", ".*sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com.*") | list | length == 0 # ---------------------------------------------------------------------------- - name: delete a service account that does not exist gcp_iam_service_account: - name: "{{ sa_name }}" + name: sa-{{ resource_name.split("-")[-1] }}@graphite-playground.google.com.iam.gserviceaccount.com display_name: My Ansible test key project: "{{ gcp_project }}" auth_kind: "{{ gcp_cred_kind }}"