Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Rez0k opened this issue Feb 7, 2024 · 4 comments
Open

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

Rez0k opened this issue Feb 7, 2024 · 4 comments
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec

Comments

@Rez0k
Copy link

Rez0k commented Feb 7, 2024

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).

@Rez0k Rez0k added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Feb 7, 2024
@t0yv0
Copy link
Member

t0yv0 commented Feb 7, 2024

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!

@t0yv0 t0yv0 added awaiting-feedback Blocked on input from the author kind/bug Some behavior is incorrect or out of spec and removed needs-triage Needs attention from the triage team kind/enhancement Improvements or new features labels Feb 7, 2024
@Rez0k
Copy link
Author

Rez0k commented Feb 8, 2024

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.

@mjeffryes mjeffryes added needs-triage Needs attention from the triage team and removed awaiting-feedback Blocked on input from the author labels Apr 19, 2024
@corymhall corymhall added impact/usability Something that impacts users' ability to use the product easily and intuitively area/refresh and removed needs-triage Needs attention from the triage team labels Apr 19, 2024
@t0yv0 t0yv0 added bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. and removed area/refresh labels Apr 24, 2024
@t0yv0
Copy link
Member

t0yv0 commented Apr 24, 2024

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:
pulumi/pulumi-terraform-bridge#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:

pulumi/pulumi-terraform-bridge#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
Copy link
Member

t0yv0 commented Apr 24, 2024

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

4 participants