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

(aws-cloudfront): Unexpected diffs caused by cloudfront.Function #15523

Closed
iliapolo opened this issue Jul 13, 2021 · 20 comments
Closed

(aws-cloudfront): Unexpected diffs caused by cloudfront.Function #15523

iliapolo opened this issue Jul 13, 2021 · 20 comments
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@iliapolo
Copy link
Contributor

iliapolo commented Jul 13, 2021

Non deterministic auto-generated function name causing unexpected diffs.

Reproduction Steps

Using this code:

new cloudfront.Function(stack, 'Function', {
  code: cloudfront.FunctionCode.fromInline('function handler(event) { return event.request }'),
});
  1. synth and extract the function name
  2. synth again and extract the function name

The function name is different for every synth.

What did you expect to happen?

The function name should be the same.

What actually happened?

Function name changes per synth.

Environment

  • CDK CLI Version : ALL
  • Framework Version: 1.109.0
  • Node.js Version: ALL
  • OS : ALL
  • Language (Version): ALL

Other

This happens because of:

private generateName(): string {
const name = Stack.of(this).region + Names.uniqueId(this);
if (name.length > 64) {
return name.substring(0, 32) + name.substring(name.length - 32);
}
return name;
}
}

Where Stack.of(this).region is an unresolved token that can have a different sequence number every time the program is executed.

A workaround right now would be to provide a name explicitly and not rely on this logic.

Workaround

Provide an explicit name for cloudfront.Function. Using this.node.addr or Node.of(this).addr will return a stable fixed length unique id. Example:

const functionId = `MyFunction${this.node.addr}`;

new cloudfront.Function(stack, functionId, {
  code: cloudfront.FunctionCode.fromInline('function handler(event) { return event.request }'),
  functionName: functionId
});

This is 🐛 Bug Report

@iliapolo iliapolo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 13, 2021
@github-actions github-actions bot added the @aws-cdk/aws-cloudfront Related to Amazon CloudFront label Jul 13, 2021
@hoegertn
Copy link
Contributor

Shouldn't the region be !Ref AWS::Region in the template and be stable?

@eladb
Copy link
Contributor

eladb commented Jul 13, 2021

I would recommend using Node.of(this).addr which returns a stable fixed length unique id

@hoegertn
Copy link
Contributor

@njlynch 's wish was to add the region information as CF is a global service. But if this is no longer needed happy to remove it. This would be a breaking change for the implementation afaik.

@eladb
Copy link
Contributor

eladb commented Jul 13, 2021

@njlynch 's wish was to add the region information as CF is a global service. But if this is no longer needed happy to remove it. This would be a breaking change for the implementation afaik.

What kind of breaking change?

@hoegertn
Copy link
Contributor

If we change the generation of the name it will recreate the CF functions on the next deployment. The name is the physicalName of the function.

@eladb
Copy link
Contributor

eladb commented Jul 13, 2021

Will it cause outage or availability risk?

@hoegertn
Copy link
Contributor

The ARN will change and this will break CF Distributions using it if not within the same stack. A short outage might as well be a thing as it removes one CFF and then registers the new CFF.

@eladb
Copy link
Contributor

eladb commented Jul 13, 2021

I think CFN will first create the new CFF, update the distribution and only then remove the old one (that's how resource replacement works)

@hoegertn
Copy link
Contributor

I meant on the distribution. So there might be a time where no CFF is registered.

@jogold
Copy link
Contributor

jogold commented Jul 14, 2021

Where Stack.of(this).region is an unresolved token that can have a different sequence number every time the program is executed.

I don't understand how it can be different... at synth time the token is resolved no? so you get a Fn::Join:

Name: {
'Fn::Join': [
'',
[
{
Ref: 'AWS::Region',
},
'StackCF2CE3F783F',
],
],
},

If you snapshot it, it should always return the same Name?

@eladb
Copy link
Contributor

eladb commented Jul 15, 2021

I think the problem is not the sequence number but actually the length of the unresolved token which is truncated (but I am not 100% sure). This requires taking a closer look.

@eladb
Copy link
Contributor

eladb commented Jul 15, 2021

@njlynch as you are fixing this, please notice that there seems to be an issue with the AWS::CloudFront::Function resource which causes an update failure if the function name is changed:

Resource handler returned message: "Resource of type 'AWS::CloudFront::Function' with identifier 'us-west-2-c84c29a89f49a9c52cb7670f010af5cc67269f8077' was not found." (RequestToken: dc479a93-2786-8270-92a1-81e3e808eb91, HandlerErrorCode: NotFound)

This is not very surprising since the physical name is explicitly specified. CFN will send an UPDATE operation with the new physical name and the resource provider won't be able to find it (because it should actually delete the old resource and create a new one instead of attempting to update).

A reasonable work around is to find the logical ID of the function to the function name itself so that when the function name changes, a new logical ID will be allocated and the resource will effectively be replaced.

See cdklabs/construct-hub#171

eladb pushed a commit to cdklabs/construct-hub that referenced this issue Jul 15, 2021
Explicitly set the CloudFront function name so snapshot is stable (see aws/aws-cdk#15523).

This also requires updating the logical ID of the AWS::CloudFront::Function resource because it seems like there's a bug in the resource provider that won't allow changing the function name. So changing the logical ID would effectively replace the resource.

Re-enable the snapshot test.
@madeline-k
Copy link
Contributor

@njlynch Is on vacation until July 22 and I am triaging his issues in the meantime. It looks like this should be a p2 since customers have a workaround to provide an explicit name. Please let me know if anyone disagrees! I will make sure to raise this to Nick as soon as he is back.

@madeline-k madeline-k added effort/medium Medium work item – several days of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 16, 2021
@eladb
Copy link
Contributor

eladb commented Jul 18, 2021

@madeline-k I don't believe I would tag it as p2 as basically the entire cloudfront.Function feature is broken in this situation. I would tag it as a p1.

Also, please add the workaround to the issue description so people don't have to search for it.

Here is what we did in construct-hub (source).

@madeline-k madeline-k added p1 and removed p2 labels Jul 19, 2021
@njlynch njlynch removed their assignment Jul 23, 2021
@KTamas
Copy link

KTamas commented Aug 16, 2021

Tracking this, we got bitten by this as well. Using the workaround for now.

stekern added a commit to capralifecycle/liflig-cdk that referenced this issue Nov 8, 2021
There's an upstream bug in CDK that results in unstable generation of
CloudFront function names: aws/aws-cdk#15523
@kiernan
Copy link

kiernan commented May 16, 2022

I think the problem is not the sequence number but actually the length of the unresolved token which is truncated (but I am not 100% sure).

This sounds like the behaviour I've just seen.

I had deployed a stack containing a cloudfront function, then went to deploy it again and got an error that it couldn't find a function with the expected ID:

3:58:23 pm | UPDATE_FAILED        | AWS::CloudFront::Function                       | CfnFunctionD7182995
Resource handler returned message: "Resource of type 'AWS::CloudFront::Function' with identifier 'ap-southeast-2kiernanSuSheetWebStackCfnFunction6F1B71FB' was not fo
und."

The ID it's trying to find is almost correct, but has one character missing in the middle compared with the actual ID of the created function.

I hadn't actually made any changes to the anything in the stack containing the cloudfront function, diff shows just the ID changing:

cdk diff kiernan-...omitted/WebStack
Resources
[~] AWS::CloudFront::Function WebStack/CfnFunction CfnFunctionD7182995
 ├─ [~] FunctionConfig
 │   └─ [~] .Comment:
 │       └─ [~] .Fn::Join:
 │           └─ @@ -4,6 +4,6 @@
 │              [ ]     {
 │              [ ]       "Ref": "AWS::Region"
 │              [ ]     },
 │              [-]     "kiernanSupSheetWebStackCfnFunction6F1B71FB"
 │              [+]     "kiernanSuSheetWebStackCfnFunction6F1B71FB"
 │              [ ]   ]
 │              [ ] ]
 └─ [~] Name
     └─ [~] .Fn::Join:
         └─ @@ -4,6 +4,6 @@
            [ ]     {
            [ ]       "Ref": "AWS::Region"
            [ ]     },
            [-]     "kiernanSupSheetWebStackCfnFunction6F1B71FB"
            [+]     "kiernanSuSheetWebStackCfnFunction6F1B71FB"
            [ ]   ]
            [ ] ]

kikuomax added a commit to kikuomax/codemonger that referenced this issue Jun 13, 2022
- The bug that a CloudFront function name fluctuated over deployment to
  deployment is worked around. There is a bug in CDK that it may
  generate different function names at different deployments. Please see
  the following issue for more details,
    - aws/aws-cdk#15523

  As suggested in the above issue, I decided to give a fixed name to my
  CloudFront function. Function names never conflict between development
  and production stacks because `Node.addr` that is different between
  them is appended.

issue codemonger-io#2
@SachinShekhar
Copy link
Contributor

Any update on this? Sometimes my deployment succeeds; sometimes it doesn’t.

@joyfulelement
Copy link

I think the problem is not the sequence number but actually the length of the unresolved token which is truncated (but I am not 100% sure).

This sounds like the behaviour I've just seen.

I had deployed a stack containing a cloudfront function, then went to deploy it again and got an error that it couldn't find a function with the expected ID:

3:58:23 pm | UPDATE_FAILED        | AWS::CloudFront::Function                       | CfnFunctionD7182995
Resource handler returned message: "Resource of type 'AWS::CloudFront::Function' with identifier 'ap-southeast-2kiernanSuSheetWebStackCfnFunction6F1B71FB' was not fo
und."

The ID it's trying to find is almost correct, but has one character missing in the middle compared with the actual ID of the created function.

I hadn't actually made any changes to the anything in the stack containing the cloudfront function, diff shows just the ID changing:

Encountering the exact same issue which causes the same stack to be re-deployed unexpectedly.

Here is one example with no change of code, but still causing the diff when running cdk diff.

Was using "aws-cdk": "2.99.1"

Resources
[~] AWS::CloudFront::Function ViewerRequestFunction ViewerRequestFunction48E73F66 
 ├─ [~] FunctionConfig
 │   └─ [~] .Comment:
 │       └─ [~] .Fn::Join:
 │           └─ @@ -4,6 +4,6 @@
 │              [ ]     {
 │              [ ]       "Ref": "AWS::Region"
 │              [ ]     },
 │              [-]     "webappriskntxViewerRequestFunction6E8AC013"
 │              [+]     "webapprisntxViewerRequestFunction6E8AC013"
 │              [ ]   ]
 │              [ ] ]
 └─ [~] Name
     └─ [~] .Fn::Join:
         └─ @@ -4,6 +4,6 @@
            [ ]     {
            [ ]       "Ref": "AWS::Region"
            [ ]     },
            [-]     "webappriskntxViewerRequestFunction6E8AC013"
            [+]     "webapprisntxViewerRequestFunction6E8AC013"
            [ ]   ]
            [ ] ]
[~] AWS::CloudFront::Function Function Function76856677 
 ├─ [~] FunctionConfig
 │   └─ [~] .Comment:
 │       └─ [~] .Fn::Join:
 │           └─ @@ -4,6 +4,6 @@
 │              [ ]     {
 │              [ ]       "Ref": "AWS::Region"
 │              [ ]     },
 │              [-]     "webapprisk4proddeploymentxFunction7C5F687C"
 │              [+]     "webappris4proddeploymentxFunction7C5F687C"
 │              [ ]   ]
 │              [ ] ]
 └─ [~] Name
     └─ [~] .Fn::Join:
         └─ @@ -4,6 +4,6 @@
            [ ]     {
            [ ]       "Ref": "AWS::Region"
            [ ]     },
            [-]     "webapprisk4proddeploymentxFunction7C5F687C"
            [+]     "webappris4proddeploymentxFunction7C5F687C"
            [ ]   ]
            [ ] ]
[~] Custom::CDKBucketDeployment DeployS3BucketContentWithCloudFrontInvalidation/CustomResource DeployS3BucketContentWithCloudFrontInvalidationCustomResource101552DD 
 └─ [~] SourceObjectKeys
     └─ @@ -1,3 +1,3 @@
        [ ] [
        [-]   "da1[22](https://adc.github.trendmicro.com/Cloud-ASRM/web-app-risk-explorer/actions/runs/2137227/job/6035773#step:21:23)bd1f16[28](https://adc.github.trendmicro.com/Cloud-ASRM/web-app-risk-explorer/actions/runs/2137227/job/6035773#step:21:29)bbd4164cb84bdcc0457d066c1c83bb28feef9034f9114a5d995.zip"
        [+]   "5aec016f2bc03e9f75f88e434ef2e747e5d47aff17a9d0465eee0[30](https://adc.github.trendmicro.com/Cloud-ASRM/web-app-risk-explorer/actions/runs/2137227/job/6035773#step:21:31)5bb[36](https://adc.github.trendmicro.com/Cloud-ASRM/web-app-risk-explorer/actions/runs/2137227/job/6035773#step:21:37)9eb6.zip"
        [ ] ]


✨  Number of stacks with differences: 1

@kiernan
Copy link

kiernan commented Oct 30, 2023

Here's a CDK Aspect we've used to apply this work-around to functions created within 3rd party constructs, where we can't control the functionName property directly.

import { IAspect } from "aws-cdk-lib";
import { CfnFunction } from "aws-cdk-lib/aws-cloudfront";
import { IConstruct } from "constructs";

/**
 * Work-around a CDK bug which is causing CloudFront Functions to
 * have their Logical ID changed unnecessarily each deployment:
 * https://github.com/aws/aws-cdk/issues/15523
 *
 * Usage example:
 * cdk.Aspects.of(app).add(new RenameCloudFrontFunctionAspect());
 */
export class RenameCloudFrontFunctionAspect implements IAspect {
  visit(node: IConstruct): void {
    if (node instanceof CfnFunction) {
      const name = `CloudFrontFunction${node.node.addr}`;
      node.name = name;
      node.overrideLogicalId(node.name);
    }
  }
}

If you had a lot of functions, you could also look at getting a better prefix to use from the ID of the construct (or its parent, if any were intentionally given the special name, 'Default').
That is just so they are more recognisable when viewing in the console, or in the generated CloudFormation output.

mergify bot pushed a commit that referenced this issue Jun 11, 2024
### Issue # (if applicable)

Closes #20017 as well as #15523 and #28629 

### Reason for this change

Due to the way function names are generated using token strings with either single- or double-digit numbers, longer function names can be truncated differently, leading to inconsistency in generated CloudFormation templates.

### Description of changes

To ensure backwards compatibility, if names are longer than 64 characters and use region tokens, if the token uses a single-digit region number, it takes the first **31** characters + the last 32 characters; if the token uses a double-digit region number or otherwise, it takes the first **32** characters + the last 32 characters. This ensures it will always take the same first chunk of the actual function's name.

### Description of how you validated changes

A new unit test was added to verify the consistency of function names in the template.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@pahud pahud added p2 and removed p1 labels Jun 11, 2024
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

mazyu36 pushed a commit to mazyu36/aws-cdk that referenced this issue Jun 22, 2024
### Issue # (if applicable)

Closes aws#20017 as well as aws#15523 and aws#28629 

### Reason for this change

Due to the way function names are generated using token strings with either single- or double-digit numbers, longer function names can be truncated differently, leading to inconsistency in generated CloudFormation templates.

### Description of changes

To ensure backwards compatibility, if names are longer than 64 characters and use region tokens, if the token uses a single-digit region number, it takes the first **31** characters + the last 32 characters; if the token uses a double-digit region number or otherwise, it takes the first **32** characters + the last 32 characters. This ensures it will always take the same first chunk of the actual function's name.

### Description of how you validated changes

A new unit test was added to verify the consistency of function names in the template.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests