aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.65k stars 3.91k forks source link

(cloudfront): Name length of cache policy #23272

Open jnawk opened 1 year ago

jnawk commented 1 year ago

Describe the bug

When introducing an unnamed CachePolicy to a CloudFront distribution which is deeply nested in constructs the automatic generated name could grow beyond 128 characters, in spite of #18918 / #18953. This will cause the CDK to raise an exception

Expected Behavior

CDK is able to figure out how to make a properly unique policy name without raising exceptions

Current Behavior

CDK is unable to figure out how to make a properly unique policy name without raising exceptions.

In the reproduction case, the offending policy's id is a single character, which is still too long.

Reproduction Steps

Attempt to synth the project that can be cloned from here: https://github.com/jnawk/cdk-cfront-policy-name-too-long

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.54.0

Framework Version

2.54.0

Node.js Version

16.18.1

OS

Ubuntu 22.04.1 LTS

Language

Typescript

Language Version

TS4.9.4

Other information

No response

peterwoodworth commented 1 year ago

Thanks for reporting this, the PR looks like it should have fixed this issue. A little bit of investigating reveals that the generated name should be 128 or fewer characters:

const cachePolicyName = props.cachePolicyName ?? `${Names.uniqueId(this).slice(0, 110)}-${Stack.of(this).region}`;

The name should be 110 + - + the region name, which would become a max of length 125 given the regions with the longest names are 14 characters. However, the synth time check is failing because Stack.of(this).region doesn't return a the actual region value, but rather a token like ${Token[AWS.Region.4]} which can be 22-23 characters long.

We will have to further modify this check to account for the fact that the region will be a token value, and thus longer.

In the meantime you can supply your own cachePolicyName when creating this construct to work around the autogenerating bug

grrapport commented 1 year ago

I ran into the same issue with a Lambda Layer- the name is too long in the regions with longer names. Specifically, the same stack worked in us-east-1, but failed in ap-southeast-1.

Like the above example, it's nested a couple constructs deep