fix: Cloudfront distribution now uses provided origin_access_identity

[TheOptimisticFactory]

SUMMARY

• This PR was originally opened at fix: Cloudfront distribution now uses provided origin_access_identity ansible/ansible#68845

• Currently, a new access-identity is created even though the parameters specify a given access-identity ID in origin.s3_origin_config.origin_access_identity.

• It will instead a new access-identity instead of using the provided origin-access-identity/cloudfront/ANYTHING

• This PR adds the retrieval of origin.s3_origin_config.origin_access_identity when applicable

- name: Create associated Cloudfront distribution
  cloudfront_distribution_fixed:
    # <other parameters omitted, see below for full parameters
    origins:
      - id: "{{ targetBucket }}"
        domain_name: "{{ targetBucket }}.s3.amazonaws.com"
        s3_origin_access_identity_enabled: true
        s3_origin_config:
          origin_access_identity: origin-access-identity/cloudfront/ANYTHING

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

• cloudfront_distribution

ADDITIONAL INFORMATION

Call parameters

# https://docs.ansible.com/ansible/latest/modules/cloudfront_distribution_module.html
# NOTE: "region" must be "us-east-1" as per https://aws.amazon.com/premiumsupport/knowledge-center/cloudfront-invalid-viewer-certificate/
- name: Create associated Cloudfront distribution
  cloudfront_distribution_fixed:
    alias: "{{ targetBucket }}"
    aws_access_key: "{{ VAULT_AWS_ACCESS_KEY }}"
    aws_secret_key: "{{ VAULT_AWS_SECRET_KEY }}"
    caller_reference: "{{ targetBucket }}"
    comment: "{{ targetBucket }} distribution created by Ansible"
      allowed_methods:
        items:
          - GET
          - HEAD
          - OPTIONS
        cached_methods:
          - GET
          - HEAD
      compress: true
      forwarded_values:
        cookies:
          forward: none
        headers:
          - Access-Control-Request-Headers
          - Access-Control-Request-Method
          - Origin
        query_string: false
      target_origin_id: "{{ targetBucket }}"
      trusted_signers:
        enabled: true
        items:
          - self
      viewer_protocol_policy: redirect-to-https
    ipv6_enabled: yes
    origins:
      - id: "{{ targetBucket }}"
        domain_name: "{{ targetBucket }}.s3.amazonaws.com"
        s3_origin_access_identity_enabled: true
        s3_origin_config:
          origin_access_identity: origin-access-identity/cloudfront/ANYTHING
    region: us-east-1
    state: present
    viewer_certificate:
      acm_certificate_arn: "{{ aws_certificate_arn }}"
      certificate_source: acm
      minimum_protocol_version: TLSv1.1_2016
      ssl_support_method: sni-only
  register: distribution_data

Output (with fix)

TASK [../roles/deployments/setup_s3 : Create associated Cloudfront distribution] ************************************************************************************************************************************
Friday 10 April 2020  16:50:23 +0200 (0:00:00.034)       0:00:46.841 ********** 
changed: [REDACTED] => {
    "active_trusted_signers": {
        "enabled": true, 
        "items": [
            {
                "aws_account_number": "self", 
                "key_pair_ids": {
                    "items": [
                        "REDACTED", 
                        "REDACTED"
                    ], 
                    "quantity": 2
                }
            }
        ], 
        "quantity": 1
    }, 
    "alias_icp_recordals": [
        {
            "cname": "REDACTED", 
            "icp_recordal_status": "APPROVED"
        }
    ], 
    "aliases": {
        "items": [
            "REDACTED"
        ], 
        "quantity": 1
    }, 
    "arn": "arn:aws:cloudfront::REDACTED:distribution/REDACTED", 
    "cache_behaviors": {
        "quantity": 0
    }, 
    "caller_reference": "REDACTED", 
    "changed": true, 
    "comment": "REDACTED distribution created by Ansible", 
    "custom_error_responses": {
        "quantity": 0
    }, 
    "default_cache_behavior": {
        "allowed_methods": {
            "cached_methods": {
                "items": [
                    "HEAD", 
                    "GET"
                ], 
                "quantity": 2
            }, 
            "items": [
                "HEAD", 
                "GET", 
                "OPTIONS"
            ], 
            "quantity": 3
        }, 
        "compress": true, 
        "default_ttl": 86400, 
        "field_level_encryption_id": "", 
        "forwarded_values": {
            "cookies": {
                "forward": "none"
            }, 
            "headers": {
                "items": [
                    "Origin", 
                    "Access-Control-Request-Method", 
                    "Access-Control-Request-Headers"
                ], 
                "quantity": 3
            }, 
            "query_string": false, 
            "query_string_cache_keys": {
                "quantity": 0
            }
        }, 
        "lambda_function_associations": {
            "quantity": 0
        }, 
        "max_ttl": 31536000, 
        "min_ttl": 0, 
        "smooth_streaming": false, 
        "target_origin_id": "REDACTED", 
        "trusted_signers": {
            "enabled": true, 
            "items": [
                "self"
            ], 
            "quantity": 1
        }, 
        "viewer_protocol_policy": "redirect-to-https"
    }, 
    "default_root_object": "", 
    "domain_name": "REDACTED.cloudfront.net", 
    "enabled": true, 
    "http_version": "http2", 
    "id": "REDACTED", 
    "in_progress_invalidation_batches": 0, 
    "is_ipv6_enabled": true, 
    "last_modified_time": "2020-04-10T14:50:31.744000+00:00", 
    "logging": {
        "bucket": "", 
        "enabled": false, 
        "include_cookies": false, 
        "prefix": ""
    }, 
    "origin_groups": {
        "quantity": 0
    }, 
    "origins": {
        "items": [
            {
                "custom_headers": {
                    "quantity": 0
                }, 
                "domain_name": "REDACTED", 
                "id": "REDACTED", 
                "origin_path": "", 
                "s3_origin_config": {
                    "origin_access_identity": "origin-access-identity/cloudfront/ANYTHING"
                }
            }
        ], 
        "quantity": 1
    }, 
    "price_class": "PriceClass_All", 
    "restrictions": {
        "geo_restriction": {
            "quantity": 0, 
            "restriction_type": "none"
        }
    }, 
    "status": "InProgress", 
    "viewer_certificate": {
        "acm_certificate_arn": "arn:aws:acm:us-east-1:REDACTED:certificate/REDACTED", 
        "certificate": "arn:aws:acm:us-east-1:REDACTED:certificate/REDACTED", 
        "certificate_source": "acm", 
        "minimum_protocol_version": "TLSv1.1_2016", 
        "ssl_support_method": "sni-only"
    }, 
    "web_acl_id": ""
}

[tremble]

Thank you for taking the time to submit this patch.

Please could you also update the integration tests to ensure we don't see a regression:
tests/integration/targets/cloudfront_distribution/tasks/main.yml

[TheOptimisticFactory]

I have added the tests 2 weeks ago, and clicked on the re-request review button.
@tremble, just in case the re-request review button did not work :)

[tremble]

Apologies, I didn't see the re-request.

[tremble]

Generally looks sane, I'll run the test suite later today.

@s-hertel, @jillr can we get some extra eyes on this please.

[TheOptimisticFactory]

What are the next steps for this PR?

[tremble]

@TheOptimisticFactory Sorry about the delays here, I'm trying to follow up with a couple of folks. In the mean time I noticed a copy and paste artefact which would be good to clean up.

[s-hertel]

Thanks for patch (and patience) @TheOptimisticFactory, sorry for not taking a look sooner. The code looks great. The tests are failing for me locally (one of the very first tests is asserting changed should be false but it's true - unrelated to this change), but I can't remember if there's a trick to getting them to run normally - @tremble is the test suite working for you? (wondering specifically since these are marked unsupported and don't run in CI)

[jillr]

targetBucket is not defined anywhere in the test suite, which causes this task to fail. I think you may have meant to use resource_prefix here?

[TheOptimisticFactory]

You are correct 👍

[TheOptimisticFactory]

I have updated my PR to use resource_prefix instead

[jillr]

I had to comment out the failing assertion to get the tests to proceed, but they seem to do ok after that point. I've opened an issue for the bad assertion and added a card to the CI tracking board for it. I'm ok with testing that way for this PR, once the tests to cover this change are passing.

[ansibullbot]

cc @s-hertel @willthames @wilvk @wimnat
click here for bot help

[tremble]

I'm sorry about the delays involved here.

I retried with the failing 'changed' assertions commented out and was unable to successfully complete the tests.

Since additional changes are also required, please also add a changelog entry: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to

[tremble]

AWS appears to validate the OAI it's passed, so this results in a failure:

An error occurred (InvalidOriginAccessIdentity) when calling the UpdateDistribution operation: The specified origin access identity does not exist or is not valid.
`` 

[tremble]

Suggested change

	domain_name: "{{ resource_prefix }}.s3.amazonaws.com"
	domain_name: "{{ resource_prefix }}-bucket.s3.amazonaws.com"

[jillr]

As we discussed on irc during pr_day, the broken test suite exceeds the scope of this change. This change should not affect any existing use cases and we feel confident with the code as it is here.

[jillr]

Thanks very much for your work and patience on this one @TheOptimisticFactory!

[tremble]

We've had a discussion on IRC.

We're reasonably certain that your change itself is going to do the right thing. As it stands the integration tests are broken, but this is not directly a result of your change. We're going to merge this and (at some point) try to follow up and fix the tests.

Thank you for your time on this and sorry it's taken so long.