Bug fixes for GCP modules

lib/ansible/module_utils/gcp_utils.py

@@ -165,9 +165,14 @@ def _credentials(self):
             self.module.fail_json(msg="Credential type '%s' not implemented" % cred_type)
 
     def _headers(self):
-        return {
-            'User-Agent': "Google-Ansible-MM-{0}".format(self.product)
-        }
+        if self.module.params.get('env_type'):
+            return {
+                'User-Agent': "Google-Ansible-MM-{0}-{1}".format(self.product, self.module.params.get('env_type'))
+            }
+        else:
+            return {
+                'User-Agent': "Google-Ansible-MM-{0}".format(self.product)
+            }
 
     def _merge_dictionaries(self, a, b):
         new = a.copy()
@@ -189,7 +194,7 @@ def __init__(self, *args, **kwargs):
                     type='str',
                     fallback=(env_fallback, ['GCP_PROJECT'])),
                 auth_kind=dict(
-                    required=False,
+                    required=True,
                     fallback=(env_fallback, ['GCP_AUTH_KIND']),
                     choices=['machineaccount', 'serviceaccount', 'application'],
                     type='str'),
@@ -208,7 +213,11 @@ def __init__(self, *args, **kwargs):
                 scopes=dict(
                     required=False,
                     fallback=(env_fallback, ['GCP_SCOPES']),
-                    type='list')
+                    type='list'),
+                env_type=dict(
+                    required=False,
+                    fallback=(env_fallback, ['GCP_ENV_TYPE']),
+                    type='str')
             )
         )
 

lib/ansible/modules/cloud/google/gcp_iam_service_account.py

@@ -61,7 +61,7 @@
 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

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}

lib/ansible/modules/cloud/google/gcp_pubsub_subscription.py

@@ -295,6 +295,7 @@
 
 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,16 +386,15 @@ 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'),
         u'messageRetentionDuration': module.params.get('message_retention_duration'),
         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
+
+    regex = r"projects/.*/subscriptions/.*"
+
+    if not re.match(regex, name):
+        name = "projects/{project}/subscriptions/{name}".format(**module.params)
+
+    return name
 
-    if 'topic' in response:
-        response['topic'] = response['topic'].split('/')[-1]
 
-    return response
+def topic_pattern(name, module):
+    if name is None:
+        return
 
+    regex = r"projects/.*/topics/.*"
 
-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']])
+    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 request
+    return name
 
 
 class SubscriptionPushconfig(object):

lib/ansible/modules/cloud/google/gcp_pubsub_topic.py

@@ -107,6 +107,7 @@
 
 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 name_pattern(name, module):
+    if name is None:
+        return
 
-def decode_request(response, module):
-    if 'name' in response:
-        response['name'] = response['name'].split('/')[-1]
-    return response
+    regex = r"projects/.*/topics/.*"
 
+    if not re.match(regex, name):
+        name = "projects/{project}/topics/{name}".format(**module.params)
 
-def encode_request(request, module):
-    request['name'] = '/'.join(['projects', module.params['project'], 'topics', module.params['name']])
-    return request
+    return name
 
 
 if __name__ == '__main__':

lib/ansible/plugins/doc_fragments/gcp.py

@@ -37,13 +37,19 @@ class ModuleDocFragment(object):
         description:
             - Array of scopes to be used.
         type: list
+    env_type:
+        description:
+            - Specifies which Ansible environment you're running this module within.
+            - This should not be set unless you know what you're doing.
+            - This only alters the User Agent string for any API requests.
+        type: str
 notes:
-  - For authentication, you can set service_account_file using the
-    C(GCP_SERVICE_ACCOUNT_FILE) env variable.
+  - for authentication, you can set service_account_file using the
+    c(gcp_service_account_file) env variable.
+  - for authentication, you can set service_account_contents using the
+    c(GCP_SERVICE_ACCOUNT_CONTENTS) env variable.
   - For authentication, you can set service_account_email using the
     C(GCP_SERVICE_ACCOUNT_EMAIL) env variable.
-  - For authentication, you can set service_account_contents using the
-    C(GCP_SERVICE_ACCOUNT_CONTENTS) env variable.
   - For authentication, you can set auth_kind using the C(GCP_AUTH_KIND) env
     variable.
   - For authentication, you can set scopes using the C(GCP_SCOPES) env variable.

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!

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!

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

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!

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

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 }}"