aws_acm Add additional AWSRetry error codes (#67671)

* Cleanup tests

* Auto-Retry on ResourceNotFound and RequestInProgress exceptions

* Use AnsibleModule options for required_if logic

* changelog

* Remove (now) duplicate RequestInProgressException catching

* Allow a single retry when attempting to fetch the information about a cert directly after deleting it.

There is a small chance that it goes away while we pull the details.

2 files changed, 87 insertions, 109 deletions

diff --git a/test/integration/targets/aws_acm/tasks/full_acm_test.yml b/test/integration/targets/aws_acm/tasks/full_acm_test.yml
index 605f7a5d44..3647531958 100644
--- a/test/integration/targets/aws_acm/tasks/full_acm_test.yml
+++ b/test/integration/targets/aws_acm/tasks/full_acm_test.yml
@@ -1,54 +1,47 @@
 - name: AWS ACM integration test
+  module_defaults:
+    group/aws:
+      aws_region: "{{ aws_region }}"
+      aws_access_key: "{{ aws_access_key }}"
+      aws_secret_key: "{{ aws_secret_key }}"
+      security_token: "{{ security_token | default(omit) }}"
   block:
-
-  - set_fact:
-      aws_connection_info: &aws_connection_info
-        aws_region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
-      no_log: True
-
   # just check this task doesn't fail
   # I'm not sure if I can assume there aren't already other certs in this account
   - name: list certs
     aws_acm_info:
-      <<: *aws_connection_info
     register: list_all
     failed_when: list_all.certificates is not defined
-      
+
   - name: ensure absent cert which doesn't exist - first time
     aws_acm:
-      <<: *aws_connection_info
       name_tag: "{{ item.name }}"
       state: absent
     with_items: "{{ local_certs }}"
-      
+
   # just in case it actually existed and was deleted last task
   # check we don't fail when deleting nothing
   - name: ensure absent cert which doesn't exist - second time
     aws_acm:
-      <<: *aws_connection_info
       name_tag: "{{ item.name }}"
       state: absent
     with_items: "{{ local_certs }}"
     register: absent_start_two
     failed_when: absent_start_two.changed
-      
+
   - name: list cert which shouldn't exist
     aws_acm_info:
-      <<: *aws_connection_info
       tags:
         Name: "{{ item.name }}"
     register: list_tag
     with_items: "{{ local_certs }}"
     failed_when: list_tag.certificates | length > 0
-      
+
   - name: check directory was made
     assert:
-      that: 
+      that:
        - remote_tmp_dir is defined
-      
+
   # https://github.com/vbotka/ansible-certificate/blob/master/tasks/cert-self-signed.yml
   - name: Generate private key for local certs
     openssl_privatekey:
@@ -79,16 +72,15 @@
   - name: upload certificates first time
     aws_acm:
       name_tag: "{{ item.name }}"
-      <<: *aws_connection_info
       certificate: "{{ lookup('file', item.cert ) }}"
       private_key: "{{ lookup('file', item.priv_key ) }}"
-      state: present 
+      state: present
     register: upload
     with_items: "{{ local_certs }}"
     until: upload is succeeded
     retries: 5
     delay: 10
-    
+
   - assert:
       that:
         - prev_task.certificate.arn is defined
@@ -100,20 +92,19 @@
     vars:
       original_cert: "{{ item.item }}"
       prev_task: "{{ item }}"
-  
+
   - name: fetch data about cert just uploaded, by ARN
     aws_acm_info:
       certificate_arn: "{{ item.certificate.arn }}"
-      <<: *aws_connection_info
     register: fetch_after_up
     with_items: "{{ upload.results }}"
-    
+
   - name: check output of prior task (fetch data about cert just uploaded, by ARN)
     assert:
       that:
       - fetch_after_up_result.certificates | length == 1
       - fetch_after_up_result.certificates[0].certificate_arn == upload_result.certificate.arn
-      - fetch_after_up_result.certificates[0].domain_name == original_cert.domain 
+      - fetch_after_up_result.certificates[0].domain_name == original_cert.domain
       - (fetch_after_up_result.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', ''))
         ==
         (lookup( 'file', original_cert.cert ) | replace( ' ', '' ) | replace( '\n', '' ))
@@ -129,22 +120,21 @@
     aws_acm_info:
       tags:
         Name: "{{ original_cert.name }}"
-      <<: *aws_connection_info
     register: fetch_after_up_name
     with_items: "{{ upload.results }}"
     vars:
       upload_result: "{{ item }}"
       original_cert: "{{ item.item }}"
-    
+
   - name: check fetched data of cert we just uploaded
     assert:
       that:
       - fetch_after_up_name_result.certificates | length == 1
       - fetch_after_up_name_result.certificates[0].certificate_arn == upload_result.certificate.arn
-      - fetch_after_up_name_result.certificates[0].domain_name == original_cert.domain 
-      - (fetch_after_up_name_result.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', '')) 
+      - fetch_after_up_name_result.certificates[0].domain_name == original_cert.domain
+      - (fetch_after_up_name_result.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', ''))
         ==
-        (lookup('file', original_cert.cert ) | replace( ' ', '' ) | replace( '\n', '')) 
+        (lookup('file', original_cert.cert ) | replace( ' ', '' ) | replace( '\n', ''))
       - "'Name' in fetch_after_up_name_result.certificates[0].tags"
       - fetch_after_up_name_result.certificates[0].tags['Name'] == original_cert.name
     with_items: "{{ fetch_after_up_name.results }}"
@@ -157,7 +147,6 @@
   - name: fetch data about cert just uploaded, by domain name
     aws_acm_info:
       domain_name: "{{ original_cert.domain }}"
-      <<: *aws_connection_info
     register: fetch_after_up_domain
     with_items: "{{ upload.results }}"
     vars:
@@ -169,7 +158,7 @@
       - fetch_after_up_domain_result.certificates | length == 1
       - fetch_after_up_domain_result.certificates[0].certificate_arn == upload_result.certificate.arn
       - fetch_after_up_domain_result.certificates[0].domain_name == original_cert.domain
-      - (fetch_after_up_domain_result.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', '')) 
+      - (fetch_after_up_domain_result.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', ''))
         ==
         (lookup('file', original_cert.cert ) | replace( ' ', '' ) | replace( '\n', ''))
       - "'Name' in fetch_after_up_domain_result.certificates[0].tags"
@@ -179,30 +168,28 @@
       fetch_after_up_domain_result: "{{ item }}"
       upload_result: "{{ item.item }}"
       original_cert: "{{ item.item.item }}"
-    
-    
+
+
   # now upload that certificate
   - name: upload certificates again, check not changed
     aws_acm:
       name_tag: "{{ item.name }}"
-      <<: *aws_connection_info
       certificate: "{{ lookup('file', item.cert ) }}"
       private_key: "{{ lookup('file', item.priv_key ) }}"
-      state: present 
+      state: present
     register: upload2
     with_items: "{{ local_certs }}"
     failed_when: upload2.changed
-    
+
 
   - name: update first cert with body of the second, first time
     aws_acm:
       state: present
-      <<: *aws_connection_info
       name_tag: "{{ local_certs[0].name }}"
       certificate: "{{ lookup('file', local_certs[1].cert ) }}"
       private_key: "{{ lookup('file', local_certs[1].priv_key ) }}"
     register: overwrite
-  
+
   - name: check output of previous task (update first cert with body of the second, first time)
     assert:
       that:
@@ -211,15 +198,14 @@
       - overwrite.certificate.arn == upload.results[0].certificate.arn
       - overwrite.certificate.domain_name == local_certs[1].domain
       - overwrite.changed
-      
+
   - name: check update was sucessfull
     aws_acm_info:
       tags:
         Name: "{{ local_certs[0].name }}"
-      <<: *aws_connection_info
     register: fetch_after_overwrite
-    
-  - name: check output of update fetch 
+
+  - name: check output of update fetch
     assert:
       that:
         - fetch_after_overwrite.certificates | length == 1
@@ -233,15 +219,14 @@
     aws_acm_info:
       tags:
         Name: "{{ local_certs[1].name }}"
-      <<: *aws_connection_info
     register: check_after_overwrite
-    
+
   - name: check other cert unaffected
     assert:
       that:
-      - check_after_overwrite.certificates | length == 1 
+      - check_after_overwrite.certificates | length == 1
       - check_after_overwrite.certificates[0].certificate_arn == fetch_after_up.results[1].certificates[0].certificate_arn
-      - check_after_overwrite.certificates[0].domain_name == local_certs[1].domain 
+      - check_after_overwrite.certificates[0].domain_name == local_certs[1].domain
       - (check_after_overwrite.certificates[0].certificate  | replace( ' ', '' ) | replace( '\n', '')) == (lookup('file', local_certs[1].cert ) | replace( ' ', '' ) | replace( '\n', ''))
       - "'Name' in check_after_overwrite.certificates[0].tags"
       - check_after_overwrite.certificates[0].tags['Name'] == local_certs[1].name
@@ -249,12 +234,11 @@
   - name: update first cert with body of the second again
     aws_acm:
       state: present
-      <<: *aws_connection_info
       name_tag: "{{ local_certs[0].name }}"
       certificate: "{{ lookup('file', local_certs[1].cert ) }}"
       private_key: "{{ lookup('file', local_certs[1].priv_key ) }}"
     register: overwrite2
-    
+
   - name: check output of previous task (update first cert with body of the second again)
     assert:
       that:
@@ -263,14 +247,13 @@
       - overwrite2.certificate.arn == upload.results[0].certificate.arn
       - overwrite2.certificate.domain_name == local_certs[1].domain
       - not overwrite2.changed
-      
+
   - name: delete certs 1 and 2
     aws_acm:
-      <<: *aws_connection_info
       state: absent
       domain_name: "{{ local_certs[1].domain }}"
     register: delete_both
-    
+
   - name: test prev task
     assert:
       that:
@@ -278,25 +261,31 @@
         - check_after_overwrite.certificates[0].certificate_arn in delete_both.arns
         - upload.results[0].certificate.arn in delete_both.arns
         - delete_both.changed
-      
+
   - name: fetch info for certs 1 and 2
     aws_acm_info:
-      <<: *aws_connection_info
       tags:
         Name: "{{ local_certs[item].name }}"
     register: check_del_one
     with_items:
       - 0
       - 1
-      
+    # There is the chance that we're running as the deletion is in progress,
+    # this could trigger ResourceNotFoundException allow a single retry to cope
+    # with this.
+    retries: 2
+    until:
+    - check_del_one is not failed
+    - check_del_one.certificates | length == 0
+    delay: 10
+
   - name: check certs 1 and 2 were already deleted
     with_items: "{{ check_del_one.results }}"
     assert:
       that: item.certificates | length == 0
-    
+
   - name: check cert 3 not deleted
     aws_acm_info:
-      <<: *aws_connection_info
       tags:
         Name: "{{ local_certs[2].name }}"
     register: check_del_one_remain
@@ -304,11 +293,10 @@
 
   - name: delete cert 3
     aws_acm:
-      <<: *aws_connection_info
       state: absent
       domain_name: "{{ local_certs[2].domain }}"
     register: delete_third
-    
+
   - name: check cert 3 deletion went as expected
     assert:
       that:
@@ -316,22 +304,20 @@
       - delete_third.arns | length == 1
       - delete_third.arns[0] == upload.results[2].certificate.arn
       - delete_third.changed
-      
+
   - name: check cert 3 was deleted
     aws_acm_info:
-      <<: *aws_connection_info
       tags:
         Name: "{{ local_certs[2].name }}"
     register: check_del_three
     failed_when: check_del_three.certificates | length != 0
-    
+
   - name: delete cert 3 again
     aws_acm:
-      <<: *aws_connection_info
       state: absent
       domain_name: "{{ local_certs[2].domain }}"
     register: delete_third
-    
+
   - name: check deletion of cert 3 not changed, because already deleted
     assert:
       that:
@@ -341,7 +327,7 @@
 
   - name: check directory was made
     assert:
-      that: 
+      that:
        - remote_tmp_dir is defined
 
   - name: Generate private key for cert to be chained
@@ -356,7 +342,7 @@
       privatekey_path: "{{ chained_cert.priv_key }}"
       common_name: "{{ chained_cert.domain }}"
     with_items: "{{ chained_cert.chains }}"
-    
+
 
   - name: Sign new certs with cert 0 and 1
     openssl_certificate:
@@ -369,7 +355,7 @@
         - 'sha256WithRSAEncryption'
         # - 'sha512WithRSAEncryption'
     with_items: "{{ chained_cert.chains }}"
-  
+
   - name: check files exist (for next task)
     file:
       path: "{{ item }}"
@@ -379,7 +365,7 @@
       - "{{ local_certs[chained_cert.chains[1].ca].cert }}"
       - "{{ chained_cert.chains[0].cert }}"
       - "{{ chained_cert.chains[1].cert }}"
-    
+
   - name: Find chains
     certificate_complete_chain:
       input_chain: "{{ lookup('file', item.cert ) }}"
@@ -391,57 +377,53 @@
   - name: upload chained cert, first chain, first time
     aws_acm:
       name_tag: "{{ chained_cert.name }}"
-      <<: *aws_connection_info
       certificate: "{{ lookup('file', chained_cert.chains[0].cert ) }}"
       certificate_chain: "{{ chains.results[0].complete_chain | join('\n') }}"
       private_key: "{{ lookup('file', chained_cert.priv_key ) }}"
-      state: present 
+      state: present
     register: upload_chain
     failed_when: not upload_chain.changed
-    
+
   - name: fetch chain of cert we just uploaded
     aws_acm_info:
-      <<: *aws_connection_info
       tags:
         Name: "{{ chained_cert.name }}"
     register: check_chain
-    
+
   - name: check chain of cert we just uploaded
     assert:
-      that: 
-        - (check_chain.certificates[0].certificate_chain | replace( ' ', '' ) | replace( '\n', '')) 
-          == 
+      that:
+        - (check_chain.certificates[0].certificate_chain | replace( ' ', '' ) | replace( '\n', ''))
+          ==
           ( chains.results[0].complete_chain | join( '\n' ) | replace( ' ', '' ) | replace( '\n', '') )
-        - (check_chain.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', '')) 
-          == 
+        - (check_chain.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', ''))
+          ==
           ( lookup('file', chained_cert.chains[0].cert ) | replace( ' ', '' ) | replace( '\n', '') )
-        
+
   - name: upload chained cert again, check not changed
     aws_acm:
       name_tag: "{{ chained_cert.name }}"
-      <<: *aws_connection_info
       certificate: "{{ lookup('file', chained_cert.chains[0].cert ) }}"
       certificate_chain: "{{ chains.results[0].complete_chain | join('\n') }}"
       private_key: "{{ lookup('file', chained_cert.priv_key ) }}"
-      state: present 
+      state: present
     register: upload_chain_2
-    
+
   - name: check previous task not changed
     assert:
       that:
         - upload_chain_2.certificate.arn == upload_chain.certificate.arn
-        - not upload_chain_2.changed 
-    
+        - not upload_chain_2.changed
+
   - name: upload chained cert, different chain
     aws_acm:
       name_tag: "{{ chained_cert.name }}"
-      <<: *aws_connection_info
       certificate: "{{ lookup('file', chained_cert.chains[1].cert ) }}"
       certificate_chain: "{{ chains.results[1].complete_chain | join('\n') }}"
       private_key: "{{ lookup('file', chained_cert.priv_key ) }}"
-      state: present 
+      state: present
     register: upload_chain_3
-    
+
   - name: check uploading with different chain is changed
     assert:
       that:
@@ -450,41 +432,38 @@
 
   - name: fetch info about chain of cert we just updated
     aws_acm_info:
-      <<: *aws_connection_info
       tags:
         Name: "{{ chained_cert.name }}"
     register: check_chain_2
-    
+
   - name: check chain of cert we just uploaded
     assert:
-      that: 
-        - (check_chain_2.certificates[0].certificate_chain | replace( ' ', '' ) | replace( '\n', '')) 
-          == 
+      that:
+        - (check_chain_2.certificates[0].certificate_chain | replace( ' ', '' ) | replace( '\n', ''))
+          ==
           ( chains.results[1].complete_chain | join( '\n' ) | replace( ' ', '' ) | replace( '\n', '') )
-        - (check_chain_2.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', '')) 
-          == 
+        - (check_chain_2.certificates[0].certificate | replace( ' ', '' ) | replace( '\n', ''))
+          ==
           ( lookup('file', chained_cert.chains[1].cert ) | replace( ' ', '' ) | replace( '\n', '') )
-        
+
   - name: delete chained cert
     aws_acm:
       name_tag: "{{ chained_cert.name }}"
-      <<: *aws_connection_info
       state: absent
     register: delete_chain_3
-  
+
   - name: check deletion of chained cert 3 is changed
     assert:
       that:
       - delete_chain_3.changed
       - upload_chain.certificate.arn in delete_chain_3.arns
-        
+
 
   always:
 
   - name: delete first bunch of certificates
     aws_acm:
       name_tag: "{{ item.name }}"
-      <<: *aws_connection_info
       state: absent
     with_items: "{{ local_certs }}"
     ignore_errors: yes
@@ -493,7 +472,6 @@
     aws_acm:
       state: absent
       name_tag: "{{ chained_cert.name }}"
-      <<: *aws_connection_info
     ignore_errors: yes
 
 
@@ -501,4 +479,4 @@
     file:
       path: "{{ remote_tmp_dir }}"
       state: directory
-    ignore_errors: yes
\ No newline at end of file
+    ignore_errors: yes
diff --git a/test/integration/targets/aws_acm/tasks/main.yml b/test/integration/targets/aws_acm/tasks/main.yml
index 55e8309c5e..7b85a29b6b 100644
--- a/test/integration/targets/aws_acm/tasks/main.yml
+++ b/test/integration/targets/aws_acm/tasks/main.yml
@@ -3,19 +3,19 @@
   - set_fact:
       virtualenv: "{{ remote_tmp_dir }}/virtualenv"
       virtualenv_command: "{{ ansible_python_interpreter }} -m virtualenv"
-  
+
   - set_fact:
       virtualenv_interpreter: "{{ virtualenv }}/bin/python"
-  
+
   # The CI runs many of these tests in parallel
   # Use this random ID to differentiate which resources
   # are from which test
   - set_fact:
       aws_acm_test_uuid: "{{ (10**9) | random }}"
-  
+
   - pip:
       name: virtualenv
-  
+
   - pip:
       name:
         - 'botocore<1.13.0,>=1.12.211'
@@ -28,12 +28,12 @@
       virtualenv: "{{ virtualenv }}"
       virtualenv_command: "{{ virtualenv_command }}"
       virtualenv_site_packages: no
-  
+
   - include_tasks: full_acm_test.yml
     vars:
       ansible_python_interpreter: "{{ virtualenv_interpreter }}"
-  
+
   always:
   - file:
       path: "{{ virtualenv }}"
-      state: absent
\ No newline at end of file
+      state: absent