aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

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

Open iliapolo opened this issue 3 years ago • 17 comments

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:

https://github.com/aws/aws-cdk/blob/94f81c441f9e2cb8dc70eb2e772d2cd75e468b67/packages/%40aws-cdk/aws-cloudfront/lib/function.ts#L178-L185

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: Bug Report

iliapolo avatar Jul 13 '21 11:07 iliapolo

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

hoegertn avatar Jul 13 '21 11:07 hoegertn

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

eladb avatar Jul 13 '21 12:07 eladb

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

hoegertn avatar Jul 13 '21 12:07 hoegertn

@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?

eladb avatar Jul 13 '21 14:07 eladb

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.

hoegertn avatar Jul 13 '21 14:07 hoegertn

Will it cause outage or availability risk?

eladb avatar Jul 13 '21 14:07 eladb

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.

hoegertn avatar Jul 13 '21 14:07 hoegertn

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)

eladb avatar Jul 13 '21 14:07 eladb

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

hoegertn avatar Jul 13 '21 14:07 hoegertn

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:

https://github.com/aws/aws-cdk/blob/94f81c441f9e2cb8dc70eb2e772d2cd75e468b67/packages/%40aws-cdk/aws-cloudfront/test/function.test.ts#L48-L58

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

jogold avatar Jul 14 '21 09:07 jogold

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 avatar Jul 15 '21 07:07 eladb

@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 https://github.com/cdklabs/construct-hub/pull/171

eladb avatar Jul 15 '21 08:07 eladb

@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 avatar Jul 16 '21 22:07 madeline-k

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

eladb avatar Jul 18 '21 08:07 eladb

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

ktamas avatar Aug 16 '21 15:08 ktamas

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"
            [ ]   ]
            [ ] ]

kiernan avatar May 16 '22 08:05 kiernan

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

SachinShekhar avatar Aug 10 '22 15:08 SachinShekhar

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

joyfulelement avatar Oct 06 '23 14:10 joyfulelement

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.

kiernan avatar Oct 30 '23 09:10 kiernan