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

cloudfront: Maximum number of allowlisted headers in Cache Policy is incorrectly set to 10 #13903

Closed
encron opened this issue Mar 31, 2021 · 6 comments · Fixed by #13904
Closed
Assignees
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p1

Comments

@encron
Copy link
Contributor

encron commented Mar 31, 2021

After upgrading our CDK CLI version and library for cloudfront we ran into the following error:

Error: Maximum allowed headers in Cache Policy is 10; got 13.

This seems to be enforced via https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-cloudfront/lib/cache-policy.ts#L234.

The maximum amount of 10 headers is in fact a soft limit that can be increased through AWS support (which we did) and should therefore not be treated as a hard limit here. See https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/cloudfront-limits.html#limits-policies.

It seems this if check is not in place for cookies or querystrings though, so there are no issues there.

Reproduction Steps

Create a cache policy with more than 10 whitelisted headers in the headerBehavior.

const exampleCachePolicy = new cloudfront.CachePolicy(this, 'ExampleCachePolicy', {
            cachePolicyName: 'example-policy',
            headerBehavior: cloudfront.CacheHeaderBehavior.allowList(
                'Header-1',
                'Header-2',
                'Header-3',
                'Header-4',
                'Header-5',
                'Header-6',
                'Header-7',
                'Header-8',
                'Header-9',
                'Header-10',
                'Header-11'
            ),
        });

What did you expect to happen?

The cache policy's creation/update should be based on the account's proper quotas instead of failing on the hardcoded check within the cdk lib. F.e. if the account had its quota increased to 20, it should only fail if there are >20 headers in the allowList.

What actually happened?

The cdk diff fails with Error: Maximum allowed headers in Cache Policy is 10; got 11.

Environment

  • CDK CLI Version : 1.95.1 (build ed2bbe6)
  • Framework Version:
  • Node.js Version: v15.12.0
  • OS : Debian
  • Language (Version): TypeScript

This is 🐛 Bug Report

@encron encron added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 31, 2021
@github-actions github-actions bot added the @aws-cdk/aws-cloudfront Related to Amazon CloudFront label Mar 31, 2021
@njlynch njlynch added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Mar 31, 2021
njlynch added a commit that referenced this issue Mar 31, 2021
Validation was added in #13425 to enforce a limit of the number of headers
allowed in the allow list for a Cache Policy; that limit is a soft limit and
should not be hard-enforced in code.

fixes #13903

This reverts commit e08213f.
@njlynch
Copy link
Contributor

njlynch commented Mar 31, 2021

Thanks for the report; when the validation was created, we missed the fact it was a soft (increasable) limit. We unfortunately can't dynamically validate the limit client-side based on each account's limits, but we can remove the validation entirely and leave it to CloudFormation to enforce.

@njlynch njlynch added the in-progress This issue is being actively worked on. label Mar 31, 2021
@njlynch
Copy link
Contributor

njlynch commented Mar 31, 2021

Tagging @robertd as an FYI.

@encron
Copy link
Contributor Author

encron commented Mar 31, 2021

Hey @njlynch, thanks for the fast follow-up! I figured verifying the account's limits itself might not be possible but just removing the validation in cdk itself is totally fine for me 👍

If anyone else is running into this, I downgraded our library to v1.92.0 to circumvent the validation.

@robertd
Copy link
Contributor

robertd commented Mar 31, 2021

@njlynch @encron Sorry about that. I've somehow missed that this was a soft limit. Also, PR #13907 has been created to revert similar checks in Origin Request Policy.

mergify bot pushed a commit that referenced this issue Mar 31, 2021
…10 (#13907)

Validation was added in #13410 to enforce a limit of the number of headers
allowed in the allow list for a Origin Request Policy; that limit is a soft limit and
should not be hard-enforced in code.

Relates to #13903

This commit partially reverts changes introduced in 42f3740.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Mar 31, 2021
…10 (aws#13907)

Validation was added in aws#13410 to enforce a limit of the number of headers
allowed in the allow list for a Origin Request Policy; that limit is a soft limit and
should not be hard-enforced in code.

Relates to aws#13903

This commit partially reverts changes introduced in 42f3740.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@robertd
Copy link
Contributor

robertd commented Apr 2, 2021

@njlynch how come this one didn’t get merged in but #13907 did? Is mergify acting up again? :)

I guess @encron will have to wait until 1.97.0 then. :)

@mergify mergify bot closed this as completed in #13904 Apr 2, 2021
mergify bot pushed a commit that referenced this issue Apr 2, 2021
Validation was added in #13425 to enforce a limit of the number of headers
allowed in the allow list for a Cache Policy; that limit is a soft limit and
should not be hard-enforced in code.

fixes #13903

This reverts commit e08213f.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Apr 2, 2021

⚠️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.

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…10 (aws#13907)

Validation was added in aws#13410 to enforce a limit of the number of headers
allowed in the allow list for a Origin Request Policy; that limit is a soft limit and
should not be hard-enforced in code.

Relates to aws#13903

This commit partially reverts changes introduced in 42f3740.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…3904)

Validation was added in aws#13425 to enforce a limit of the number of headers
allowed in the allow list for a Cache Policy; that limit is a soft limit and
should not be hard-enforced in code.

fixes aws#13903

This reverts commit e08213f.

----

*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/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants