cloudfront_distribution: Always add field_level_encryption_id to cache behaviour (#61271)

* cloudfront_distribution: (integration tests) Migrate to using module_defaults

* cloudfront_distribution: (integration tests) Use the ID rather than the alias

Using aliases requires providing a valid SSL certificate, as such we're not longer able to test using an arbitrary hostname

* cloudfront_distribution: (integration tests) Make sure we delete the test s3 bucket when tests fail

* cloudfront_distribution: field_level_encryption_id is now a mandatory field always add it

Setting the field to an empty string has the same effect as the original behaviour.

* Copy & Paste fixup

Co-Authored-By: Jill R <4121322+jillr@users.noreply.github.com>

Co-authored-by: Jill R <4121322+jillr@users.noreply.github.com>

changelogs/fragments/61271-cloudfront_distribution-encryptionid.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- cloudfront_distribution - Always add field_level_encryption_id to cache behaviour to match AWS requirements

lib/ansible/modules/cloud/amazon/cloudfront_distribution.py

@@ -1859,11 +1859,12 @@ class CloudFrontValidationManager(object):
             self.module.fail_json_aws(e, msg="Error validating lambda function associations")
 
     def validate_field_level_encryption_id(self, config, field_level_encryption_id, cache_behavior):
-        # only set field_level_encryption_id if it's already set or if it was passed
         if field_level_encryption_id is not None:
             cache_behavior['field_level_encryption_id'] = field_level_encryption_id
         elif 'field_level_encryption_id' in config:
             cache_behavior['field_level_encryption_id'] = config.get('field_level_encryption_id')
+        else:
+            cache_behavior['field_level_encryption_id'] = ""
         return cache_behavior
 
     def validate_allowed_methods(self, config, allowed_methods, cache_behavior):

test/integration/targets/cloudfront_distribution/defaults/main.yml

@@ -1,6 +1,14 @@
 cloudfront_hostname: "{{ resource_prefix }}01"
+
 # Use a domain that has a wildcard DNS
-cloudfront_alias: "{{ cloudfront_hostname }}.github.io"
+# Using an alias requires also having an SSL cert...
+#cloudfront_alias: "{{ cloudfront_hostname }}.github.io"
+#cloudfront_viewer_cert:
+#  acm_certificate_arn: ...
+#  certificate: ...
+#  certificate_source: ...
+#  minimum_protocol_version: ...
+#  ssl_support_method: ...
 
 cloudfront_test_cache_behaviors:
   - path_pattern: /test/path

test/integration/targets/cloudfront_distribution/tasks/main.yml

@@ -1,15 +1,15 @@
-- block:
+- module_defaults:
-  - name: set yaml anchor
+    group/aws:
-    set_fact:
+      aws_access_key: "{{ aws_access_key }}"
-      aws_connection_info: &aws_connection_info
+      aws_secret_key: "{{ aws_secret_key }}"
-        aws_access_key: "{{ aws_access_key }}"
+      security_token: "{{ security_token | default(omit) }}"
-        aws_secret_key: "{{ aws_secret_key }}"
+    cloudfront_distribution:
-        security_token: "{{ security_token }}"
+      alias: "{{ cloudfront_alias | default(omit) }}"
-    no_log: yes
+      viewer_certificate: "{{ cloudfront_viewer_cert | default(omit) }}"
+  block:
 
   - name: create cloudfront distribution using defaults
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
       origins:
       - domain_name: "{{ cloudfront_hostname }}-origin.example.com"
         id: "{{ cloudfront_hostname }}-origin.example.com"
@@ -17,15 +17,17 @@
         target_origin_id: "{{ cloudfront_hostname }}-origin.example.com"
       state: present
       purge_origins: yes
-      <<: *aws_connection_info
+    register: cf_distribution
+
+  - set_fact:
+      distribution_id: '{{ cf_distribution.id }}'
 
   - name: re-run cloudfront distribution with same defaults
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ cloudfront_hostname }}-origin.example.com"
       state: present
-      <<: *aws_connection_info
     register: cf_dist_no_update
 
   - name: ensure distribution was not updated
@@ -35,10 +37,9 @@
 
   - name: re-run cloudfront distribution using distribution id
     cloudfront_distribution:
-      distribution_id: "{{ cf_dist_no_update.id }}"
+      distribution_id: "{{ distribution_id }}"
       purge_origins: no
       state: present
-      <<: *aws_connection_info
     register: cf_dist_with_id
 
   - name: ensure distribution was not updated
@@ -48,13 +49,12 @@
 
   - name: update origin http port
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ cloudfront_hostname }}-origin.example.com"
         custom_origin_config:
           http_port: 8080
       state: present
-      <<: *aws_connection_info
     register: update_origin_http_port
 
   - name: ensure http port was updated
@@ -64,14 +64,13 @@
 
   - name: update restrictions
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       restrictions:
         geo_restriction:
           restriction_type: "whitelist"
           items:
             - "US"
       state: present
-      <<: *aws_connection_info
     register: update_restrictions
 
   - name: ensure restrictions was updated
@@ -85,10 +84,9 @@
 
   - name: update comment
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       comment: "{{ comment }}"
       state: present
-      <<: *aws_connection_info
     register: cf_comment
 
   - name: ensure comment was updated
@@ -99,14 +97,13 @@
 
   - name: create second origin
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
         id: "{{ resource_prefix }}2.example.com"
       default_root_object: index.html
       state: present
       wait: yes
-      <<: *aws_connection_info
     register: cf_add_origin
 
   - name: ensure origin was added
@@ -118,7 +115,7 @@
 
   - name: re-run second origin
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ cloudfront_hostname }}-origin.example.com"
         custom_origin_config:
@@ -127,7 +124,6 @@
       default_root_object: index.html
       wait: yes
       state: present
-      <<: *aws_connection_info
     register: cf_rerun_second_origin
 
   - name: ensure nothing changed after re-run
@@ -138,14 +134,13 @@
 
   - name: run with origins in reverse order
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       - domain_name: "{{ cloudfront_hostname }}-origin.example.com"
         custom_origin_config:
           http_port: 8080
       state: present
-      <<: *aws_connection_info
     register: cf_rerun_second_origin_reversed
 
   - name: ensure nothing changed after reversed re-run
@@ -157,14 +152,13 @@
 
   - name: purge first origin
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       default_cache_behavior:
         target_origin_id: "{{ resource_prefix }}2.example.com"
       purge_origins: yes
       state: present
-      <<: *aws_connection_info
     register: cf_purge_origin
 
   - name: ensure origin was removed
@@ -175,12 +169,11 @@
 
   - name: update default_root_object of existing distribution
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       default_root_object: index.php
       state: present
-      <<: *aws_connection_info
     register: cf_update_default_root_object
 
   - name: ensure origin was updated
@@ -191,15 +184,14 @@
 
   - name: add tags to existing distribution
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       tags:
-        Name: "{{ cloudfront_alias }}"
+        ATag: tag1
         Another: tag
       default_root_object: index.php
       state: present
-      <<: *aws_connection_info
     register: cf_add_tags
 
   - name: ensure tags were added
@@ -210,61 +202,61 @@
 
   - name: delete distribution
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       enabled: no
       wait: yes
       state: absent
-      <<: *aws_connection_info
 
   - name: create distribution with tags
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
         id: "{{ resource_prefix }}2.example.com"
       tags:
-        Name: "{{ cloudfront_alias }}"
+        ATag: tag1
         Another: tag
       state: present
-      <<: *aws_connection_info
     register: cf_second_distribution
 
+  - set_fact:
+      distribution_id: '{{ cf_second_distribution.id }}'
+
   - name: ensure tags were set on creation
     assert:
       that:
         - cf_second_distribution.changed
         - cf_second_distribution.tags|length == 2
-        - "'Name' in cf_second_distribution.tags"
+        - "'ATag' in cf_second_distribution.tags"
+        - "'Another' in cf_second_distribution.tags"
 
   - name: re-run create distribution with same tags and purge_tags
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
         id: "{{ resource_prefix }}2.example.com"
       tags:
-        Name: "{{ cloudfront_alias }}"
+        ATag: tag1
         Another: tag
       purge_tags: yes
       state: present
-      <<: *aws_connection_info
     register: rerun_with_purge_tags
 
   - name: ensure that re-running didn't change
     assert:
       that:
         - not rerun_with_purge_tags.changed
+        - rerun_with_purge_tags.tags|length == 2
 
   - name: add new tag to distribution
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       tags:
         Third: thing
       purge_tags: no
       state: present
-      <<: *aws_connection_info
     register: update_with_new_tag
 
   - name: ensure tags are correct
@@ -273,25 +265,25 @@
         - update_with_new_tag.changed
         - "'Third' in update_with_new_tag.tags"
         - "'Another' in update_with_new_tag.tags"
+        - "'Atag' in update_with_new_tag.tags"
+        - update_with_new_tag.tags|length == 3
 
   - name: create some cache behaviors
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       cache_behaviors: "{{ cloudfront_test_cache_behaviors }}"
       state: present
-      <<: *aws_connection_info
     register: add_cache_behaviors
 
   - name: reverse some cache behaviors
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       cache_behaviors: "{{ cloudfront_test_cache_behaviors|reverse|list }}"
       state: present
-      <<: *aws_connection_info
     register: reverse_cache_behaviors
 
   - name: check that reversing cache behaviors changes nothing when purge_cache_behaviors unset
@@ -302,13 +294,12 @@
 
   - name: reverse some cache behaviors properly
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}2.example.com"
       cache_behaviors: "{{ cloudfront_test_cache_behaviors|reverse|list }}"
       purge_cache_behaviors: yes
       state: present
-      <<: *aws_connection_info
     register: reverse_cache_behaviors_with_purge
 
   - name: check that reversing cache behaviors changes nothing when purge_cache_behaviors unset
@@ -319,13 +310,12 @@
 
   - name: update origin that changes target id (failure expected)
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}3.example.com"
         id: "{{ resource_prefix }}3.example.com"
       purge_origins: yes
       state: present
-      <<: *aws_connection_info
     register: remove_origin_in_use
     ignore_errors: yes
 
@@ -338,7 +328,6 @@
   # not clear whether to hope they fix or prevent this issue from happening
   #- name: update origin and update cache behavior to point to new origin
   #  cloudfront_distribution:
-  #    alias: "{{ cloudfront_alias }}"
   #    origins:
   #    - domain_name: "{{ resource_prefix }}3.example.com"
   #      id: "{{ resource_prefix }}3.example.com"
@@ -360,17 +349,15 @@
     aws_s3:
       bucket: "{{ resource_prefix }}-bucket"
       mode: create
-      <<: *aws_connection_info
 
   - name: update origin to point to the s3 bucket
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}-bucket.s3.amazonaws.com"
         id: "{{ resource_prefix }}3.example.com"
         s3_origin_access_identity_enabled: yes
       state: present
-      <<: *aws_connection_info
     register: update_origin_to_s3
 
   - name: check that s3 origin access is in result
@@ -382,13 +369,12 @@
 
   - name: update origin to remove s3 origin access identity
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}-bucket.s3.amazonaws.com"
         id: "{{ resource_prefix }}3.example.com"
         s3_origin_access_identity_enabled: no
       state: present
-      <<: *aws_connection_info
     register: update_origin_to_s3_without_origin_access
 
   - name: check that s3 origin access is not in result
@@ -402,11 +388,10 @@
     aws_s3:
       bucket: "{{ resource_prefix }}-bucket"
       mode: delete
-      <<: *aws_connection_info
 
   - name: check that custom_origin_config can't be used with origin_access_identity enabled
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       origins:
       - domain_name: "{{ resource_prefix }}-bucket.s3.amazonaws.com"
         id: "{{ resource_prefix }}3.example.com"
@@ -414,7 +399,6 @@
         custom_origin_config:
           origin_protocol_policy: 'http-only'
       state: present
-      <<: *aws_connection_info
     register: update_origin_to_s3_with_origin_access_and_with_custom_origin_config
     ignore_errors: True
 
@@ -425,10 +409,14 @@
 
   always:
   # TEARDOWN STARTS HERE
+  - name: delete the s3 bucket
+    aws_s3:
+      bucket: "{{ resource_prefix }}-bucket"
+      mode: delete
+
   - name: clean up cloudfront distribution
     cloudfront_distribution:
-      alias: "{{ cloudfront_alias }}"
+      distribution_id: "{{ distribution_id }}"
       enabled: no
       wait: yes
       state: absent
-      <<: *aws_connection_info