search

pulumi / pulumi-aws

An Amazon Web Services (AWS) Pulumi resource package, providing multi-language access to AWS
Apache License 2.0
466 stars 157 forks source link

[Bug]: CLoudfront ignore_changes=['origins[*].originPath'] #3396

Open Rez0k opened 10 months ago

Rez0k commented 10 months ago

What happened?

Expected Behavior

When adding the ignore_changes=['origins[*].originPath'] to cloudfront.Distribution(opts=...) I expect the origin path to be ignored (we change the path very often)

If I try to ignore_changes=['origins'] I will get the error:

aws:cloudfront/distribution:Distribution resource 'XXX' has a problem: origin.0.origin_access_control_id must not be empty, got . Examine values at 'XXX.origins'.

I have 2 origins, s3 bucket which has an origin_access_control_id and a lb origin which does not has origin_access_control_id why am I getting this error? If I remove the ignore_changes everything is working just fine.

note: I am using refresh: always

Actual Behavior

The origin path does get ignored but pulumi wants to change the origin(!) to a different one, I have 2 origins, one that point to a load balancer and one that point to a s3 bucket. I have origin_path set only on the s3 bucket origin, but when I use the ignore originPath, pulumi set the originPath to the lb instead of the s3!

~ [2]: {
                      ~ connectionAttempts   : 3 => 3
                      ~ connectionTimeout    : 10 => 10
                      + customOriginConfig   : {
                          + httpPort              : 80
                          + httpsPort             : 443
                          + originKeepaliveTimeout: 5
                          + originProtocolPolicy  : "https-only"
                          + originReadTimeout     : 30
                          + originSslProtocols    : [
                          +     [0]: "TLSv1.2"
                            ]
                        }
                      ~ domainName           : "<original origin (s3 bucket)>" => "<second origin (should not be here -> alb)>"
                      - originAccessControlId: "XXXXXX"
                      ~ originId             : "<original origin (s3 bucket)>" => "<second origin (should not be here -> alb)>"
                      ~ originPath           : "<stays the same>" => "<stays the same>"
                    }

Output of pulumi about

CLI
Version 3.97.0 Go Version go1.21.5 Go Compiler gc

Host
OS ubuntu Version 22.04 Arch x86_64

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

t0yv0 commented 10 months ago

Thanks for reporting this @Rez0k and sorry that Pulumi is not working as expected for you. Could you please include a more complete program as well as output of pulumi about from your program directory that would include pulumi-aws version in the output? That would be very helpful for my team as we pick this up to debug what is going on. Appreciated!

Rez0k commented 9 months ago

Thanks for reporting this @Rez0k and sorry that Pulumi is not working as expected for you. Could you please include a more complete program as well as output of pulumi about from your program directory that would include pulumi-aws version in the output? That would be very helpful for my team as we pick this up to debug what is going on. Appreciated!

pulumi about output:

CLI          
Version      3.97.0
Go Version   go1.21.5
Go Compiler  gc

Plugins
NAME    VERSION
aws     6.17.0
python  unknown
tls     5.0.0

Host     
OS       ubuntu
Version  22.04
Arch     x86_64

This project is written in python: executable='/usr/bin/python3' version='3.10.12'

Backend        
Name           XXX
URL            s3://XXX
User           XXX
Organizations  
Token type     personal

Dependencies:
NAME                   VERSION
Jinja2                 3.1.2
jsonschema             4.20.0
pip                    23.3.2
pulumi-aws             6.17.0
pulumi-tls             5.0.0
setuptools             69.0.3
wheel                  0.42.0

complete program:

def create(self, args: dict, opts = None):
    cf_origin_access_control = None
    if len(args['s3_origins']) > 0:
        cf_origin_access_control = cloudfront.OriginAccessControl(
            f'{args["env"]}-{args["s3_origins"][0]["origin_id"]}'[:64],
            name = f'{args["env"]}-{args["s3_origins"][0]["origin_id"]}'[:64],
            description = args['s3_origins'][0]['origin_id'],
            origin_access_control_origin_type = "s3",
            signing_behavior = "always",
            signing_protocol = "sigv4",
            opts=pulumi.ResourceOptions(parent=self)
        )

    cf_origins = []
    for s3_origin in args["s3_origins"]:
        cf_origin = cloudfront.DistributionOriginArgs(
            origin_id=s3_origin['origin_id'],
            domain_name=s3_origin['domain_name'],
            origin_path=s3_origin['origin_path'],
            origin_access_control_id=cf_origin_access_control.id
        )
        cf_origins.append(cf_origin)

    cf_custom_origins = []
    for custom_origin in args['custom_origins']:
        cf_custom_origin = cloudfront.DistributionOriginArgs(
            origin_id=custom_origin['origin_id'],
            domain_name=custom_origin['domain_name'],
            origin_path=custom_origin['origin_path'],
            custom_origin_config=cloudfront.DistributionOriginCustomOriginConfigArgs(
                http_port="80",
                https_port="443",
                origin_protocol_policy="https-only",
                origin_ssl_protocols=["TLSv1.2"],
                origin_read_timeout=custom_origin["origin_read_timeout"]
            )
        )
        cf_custom_origins.append(cf_custom_origin)

    # Create CloudFront default cache behavior
    default_cache_behavior = cloudfront.DistributionDefaultCacheBehaviorArgs(
        allowed_methods=args['default_cache_methods']['allowed_methods'],
        cached_methods=args['default_cache_methods']['cached_methods'],
        target_origin_id=args['default_cache_methods']['target_origin_id'],
        cache_policy_id=args['default_cache_methods']['cache_policy_id'],
        origin_request_policy_id=args['default_cache_methods']['origin_request_policy_id'],
        response_headers_policy_id=args['default_cache_methods']['response_headers_policy_id'],

        min_ttl=args['default_cache_methods'].get('min_ttl', 30),
        default_ttl=args['default_cache_methods'].get('default_ttl', 30),
        max_ttl=args['default_cache_methods'].get('max_ttl', 30),
        compress=args['default_cache_methods']['compress'],
        viewer_protocol_policy = args['default_cache_methods']['viewer_protocol_policy'],

        function_associations=[cloudfront.DistributionDefaultCacheBehaviorFunctionAssociationArgs(
            event_type='viewer-response',
            function_arn=args['default_cache_methods']['function_association_arn']
        )]
    )

    cf_ordered_cache_behaviors = []
    for behavior in args['ordered_cache_behaviors']:
        cf_ordered_behavior = cloudfront.DistributionOrderedCacheBehaviorArgs(
            path_pattern=behavior['path_pattern'],
            allowed_methods=behavior['allowed_methods'],
            cached_methods=behavior['cached_methods'],
            target_origin_id=behavior['target_origin_id'],
            cache_policy_id=behavior['cache_policy_id'],
            origin_request_policy_id=behavior['origin_request_policy_id'],
            response_headers_policy_id=behavior['response_headers_policy_id'],
            min_ttl=behavior.get('min_ttl', 30),
            default_ttl=behavior.get('default_ttl', 30),
            max_ttl=behavior.get('max_ttl', 30),
            compress=behavior['compress'],
            viewer_protocol_policy=behavior['viewer_protocol_policy']
        )
        if behavior.get('restrict_viewer_access'):
            cf_ordered_behavior.trusted_key_groups=[trusted_key_group.id]
        cf_ordered_cache_behaviors.append(cf_ordered_behavior)

    # Create CloudFront custom error responses
    cf_custom_error_responses = []
    for error_response in args['custom_error_responses']:
        cf_error_response = cloudfront.DistributionCustomErrorResponseArgs(
            error_code=error_response['error_code'],
            response_code=error_response['response_code'],
            error_caching_min_ttl=error_response.get('error_caching_min_ttl', 30),
            response_page_path=error_response['response_page_path'],
        )
        cf_custom_error_responses.append(cf_error_response)

    # Create CloudFront distribution
    distribution = cloudfront.Distribution(
        args['name'],
        enabled=True,
        comment=args['name'],
        default_root_object=args['default_root_object'],
        price_class=args['price_class'],
        aliases=args['alternate_domain_names'],
        origins=cf_origins + cf_custom_origins,
        default_cache_behavior=default_cache_behavior,
        ordered_cache_behaviors=cf_ordered_cache_behaviors,
        custom_error_responses=cf_custom_error_responses,
        restrictions=cloudfront.DistributionRestrictionsArgs(
            geo_restriction=cloudfront.DistributionRestrictionsGeoRestrictionArgs(restriction_type="none")
        ),
        viewer_certificate=cloudfront.DistributionViewerCertificateArgs(
            cloudfront_default_certificate=False,
            acm_certificate_arn=args['acm_certificate_arn'],
            minimum_protocol_version=args['minimum_protocol_version'],
            ssl_support_method="sni-only"
        ),
        opts=pulumi.ResourceOptions(parent=self, ignore_changes=["origins[*].originPath"]),
    )

    self.distribution_id = distribution.id

This is the program I have, I have a yaml config file that helps me to create the cloudfront distribution. The problem is that when I try to ignore the originPath of each origin (I have 3) with ignore_changes=["origins[*].originPath"], one origin is deleted for no reason. I can't understand why. note: I use refresh always at my pulumi config.

t0yv0 commented 7 months ago

Hi @Rez0k I'm having another look here, it would be really great to have a minimal fully working repro to make sure I narrow in on your exact issue. But it looks like I can make an educated guess.

Consider this program:

import pulumi
import pulumi_aws as aws

s3_distribution = aws.cloudfront.Distribution("s3_distribution",
    origins=[aws.cloudfront.DistributionOriginArgs(
        domain_name="mybucket2.s3.amazonaws.com", # change this
        origin_id="myS3Origin",
        connection_attempts=2,
        connection_timeout=10,
    )],
    enabled=True,
    default_cache_behavior=aws.cloudfront.DistributionDefaultCacheBehaviorArgs(
        allowed_methods=[
            "DELETE",
            "GET",
            "HEAD",
            "OPTIONS",
            "PATCH",
            "POST",
            "PUT",
        ],
        cached_methods=[
            "GET",
            "HEAD",
        ],
        target_origin_id="myS3Origin",
        forwarded_values=aws.cloudfront.DistributionDefaultCacheBehaviorForwardedValuesArgs(
            query_string=False,
            cookies=aws.cloudfront.DistributionDefaultCacheBehaviorForwardedValuesCookiesArgs(
                forward="none",
            ),
        ),
        viewer_protocol_policy="allow-all",
        min_ttl=0,
        default_ttl=3600,
        max_ttl=86400,
    ),
    restrictions=aws.cloudfront.DistributionRestrictionsArgs(
        geo_restriction=aws.cloudfront.DistributionRestrictionsGeoRestrictionArgs(
            restriction_type="whitelist",
            locations=[
                "US",
                "CA",
                "GB",
                "DE",
            ],
        ),
    ),
    viewer_certificate=aws.cloudfront.DistributionViewerCertificateArgs(
        cloudfront_default_certificate=True,
    ),
    # opts=pulumi.ResourceOptions(ignore_changes=["origins[*].originPath"]))

If I provision this in Pulumi, and then change the origin, I get a confusing diff.

Due to the issue with how Pulumi handles set types: https://github.com/pulumi/pulumi-terraform-bridge/issues/186

TF thinks that this change actually REMOVES one origin, and ADDS another origin instead of editing in-place, because it computes a new identity key for the origin after the change. When translating this to Pulumi diffs it is very confusing. We need to do better here as we are tracking in the above issue.

If at this point I add this:

opts=pulumi.ResourceOptions(ignore_changes=["origins"]))

Then it indeed suppresses the diff and pulumi up shows no changes as expected.

However if I try to do this:

opts=pulumi.ResourceOptions(ignore_changes=["origins[*].originPath"]))

It fails to suppress the diff and actually seems to be a no-op, this is a known issue tracked below but essentially it also is an artifact of the problem that TF thinks this is a REMOVE, ADD diff:

https://github.com/pulumi/pulumi-terraform-bridge/issues/1756

If I start from an empty state and do this:

opts=pulumi.ResourceOptions(ignore_changes=["origins"]))

Then Pulumi might be copying prior state (no origin) which is indeed an error. So this bit may be working as expected.

t0yv0 commented 7 months ago

https://github.com/pulumi/pulumi-aws/issues/2095 is another issue that runs into very similar problems with this resource.

I believe this is unrelated to using --refresh. We'll circle back here when we have a good solution to the referenced issues.