From 8f7ff29415c8411d69243d80dc28dd712a09fc1f Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Fri, 5 Aug 2022 10:28:21 +0200 Subject: [PATCH] Fix #247, #242 Trim CloudFront function names to stay under the limit --- src/constructs/aws/ServerSideWebsite.ts | 7 ++++ src/constructs/aws/SinglePageApp.ts | 8 ++++- src/constructs/aws/StaticWebsite.ts | 8 ++++- .../aws/abstracts/StaticWebsiteAbstract.ts | 8 ++++- src/utils/naming.ts | 11 ++++++ test/unit/serverSideWebsite.test.ts | 33 ++++++++++++++++++ test/unit/singlePageApp.test.ts | 31 +++++++++++++++++ test/unit/staticWebsite.test.ts | 34 +++++++++++++++++++ test/unit/utils/naming.test.ts | 14 ++++++++ 9 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 src/utils/naming.ts create mode 100644 test/unit/utils/naming.test.ts diff --git a/src/constructs/aws/ServerSideWebsite.ts b/src/constructs/aws/ServerSideWebsite.ts index 1ab40e51..77133a23 100644 --- a/src/constructs/aws/ServerSideWebsite.ts +++ b/src/constructs/aws/ServerSideWebsite.ts @@ -31,6 +31,7 @@ import * as cloudfront from "aws-cdk-lib/aws-cloudfront"; import { AwsConstruct } from "@lift/constructs/abstracts"; import type { ConstructCommands } from "@lift/constructs"; import type { AwsProvider } from "@lift/providers"; +import { ensureNameMaxLength } from "../../utils/naming"; import { s3Put, s3Sync } from "../../utils/s3-sync"; import { emptyBucket, invalidateCloudFrontCache } from "../../classes/aws"; import ServerlessError from "../../utils/error"; @@ -456,7 +457,13 @@ export class ServerSideWebsite extends AwsConstruct { return request; }`; + const functionName = ensureNameMaxLength( + `${this.provider.stackName}-${this.provider.region}-${this.id}-request`, + 64 + ); + return new cloudfront.Function(this, "RequestFunction", { + functionName, code: cloudfront.FunctionCode.fromInline(code), }); } diff --git a/src/constructs/aws/SinglePageApp.ts b/src/constructs/aws/SinglePageApp.ts index 5f11dbf2..77a82b74 100644 --- a/src/constructs/aws/SinglePageApp.ts +++ b/src/constructs/aws/SinglePageApp.ts @@ -4,6 +4,7 @@ import type { Construct as CdkConstruct } from "constructs"; import type { AwsProvider } from "@lift/providers"; import { redirectToMainDomain } from "../../classes/cloudfrontFunctions"; import { getCfnFunctionAssociations } from "../../utils/getDefaultCfnFunctionAssociations"; +import { ensureNameMaxLength } from "../../utils/naming"; import type { CommonStaticWebsiteConfiguration } from "./abstracts/StaticWebsiteAbstract"; import { COMMON_STATIC_WEBSITE_DEFINITION, StaticWebsiteAbstract } from "./abstracts/StaticWebsiteAbstract"; @@ -58,8 +59,13 @@ function handler(event) { return event.request; }`; + const functionName = ensureNameMaxLength( + `${this.provider.stackName}-${this.provider.region}-${this.id}-request`, + 64 + ); + return new cloudfront.Function(this, "RequestFunction", { - functionName: `${this.provider.stackName}-${this.provider.region}-${this.id}-request`, + functionName, code: cloudfront.FunctionCode.fromInline(code), }); } diff --git a/src/constructs/aws/StaticWebsite.ts b/src/constructs/aws/StaticWebsite.ts index 336135c9..18cc6d71 100644 --- a/src/constructs/aws/StaticWebsite.ts +++ b/src/constructs/aws/StaticWebsite.ts @@ -6,6 +6,7 @@ import type { BucketProps } from "aws-cdk-lib/aws-s3"; import { RemovalPolicy } from "aws-cdk-lib"; import { redirectToMainDomain } from "../../classes/cloudfrontFunctions"; import { getCfnFunctionAssociations } from "../../utils/getDefaultCfnFunctionAssociations"; +import { ensureNameMaxLength } from "../../utils/naming"; import type { CommonStaticWebsiteConfiguration } from "./abstracts/StaticWebsiteAbstract"; import { COMMON_STATIC_WEBSITE_DEFINITION, StaticWebsiteAbstract } from "./abstracts/StaticWebsiteAbstract"; @@ -52,8 +53,13 @@ export class StaticWebsite extends StaticWebsiteAbstract { return request; }`; + const functionName = ensureNameMaxLength( + `${this.provider.stackName}-${this.provider.region}-${this.id}-request`, + 64 + ); + return new cloudfront.Function(this, "RequestFunction", { - functionName: `${this.provider.stackName}-${this.provider.region}-${this.id}-request`, + functionName, code: cloudfront.FunctionCode.fromInline(code), }); } diff --git a/src/constructs/aws/abstracts/StaticWebsiteAbstract.ts b/src/constructs/aws/abstracts/StaticWebsiteAbstract.ts index 94734e30..bef11f4b 100644 --- a/src/constructs/aws/abstracts/StaticWebsiteAbstract.ts +++ b/src/constructs/aws/abstracts/StaticWebsiteAbstract.ts @@ -26,6 +26,7 @@ import { emptyBucket, invalidateCloudFrontCache } from "../../../classes/aws"; import ServerlessError from "../../../utils/error"; import type { Progress } from "../../../utils/logger"; import { getUtils } from "../../../utils/logger"; +import { ensureNameMaxLength } from "../../../utils/naming"; import { s3Sync } from "../../../utils/s3-sync"; export const COMMON_STATIC_WEBSITE_DEFINITION = { @@ -343,8 +344,13 @@ export abstract class StaticWebsiteAbstract extends AwsConstruct { return response; }`; + const functionName = ensureNameMaxLength( + `${this.provider.stackName}-${this.provider.region}-${this.id}-response`, + 64 + ); + return new cloudfront.Function(this, "ResponseFunction", { - functionName: `${this.provider.stackName}-${this.provider.region}-${this.id}-response`, + functionName, code: cloudfront.FunctionCode.fromInline(code), }); } diff --git a/src/utils/naming.ts b/src/utils/naming.ts new file mode 100644 index 00000000..b98cb180 --- /dev/null +++ b/src/utils/naming.ts @@ -0,0 +1,11 @@ +import crypto from "crypto"; + +export function ensureNameMaxLength(name: string, maxLength: number): string { + if (name.length <= maxLength) { + return name; + } + + const uniqueSuffix = crypto.createHash("md5").update(name).digest("hex").slice(0, 6); + + return name.slice(0, maxLength - uniqueSuffix.length - 1) + "-" + uniqueSuffix; +} diff --git a/test/unit/serverSideWebsite.test.ts b/test/unit/serverSideWebsite.test.ts index 6a9902c5..9ab0adde 100644 --- a/test/unit/serverSideWebsite.test.ts +++ b/test/unit/serverSideWebsite.test.ts @@ -187,6 +187,17 @@ describe("server-side website", () => { }, }, }); + expect(cfTemplate.Resources[requestFunction]).toMatchObject({ + Type: "AWS::CloudFront::Function", + Properties: { + Name: "app-dev-us-east-1-backend-request", + FunctionConfig: { + Comment: "app-dev-us-east-1-backend-request", + Runtime: "cloudfront-js-1.0", + }, + AutoPublish: true, + }, + }); expect(cfTemplate.Outputs).toMatchObject({ [computeLogicalId("backend", "AssetsBucketName")]: { Description: "Name of the bucket that stores the website assets.", @@ -704,4 +715,26 @@ describe("server-side website", () => { ObjectLockEnabled: true, }); }); + + it("trims CloudFront function names to stay under the limit", async () => { + const { cfTemplate, computeLogicalId } = await runServerless({ + command: "package", + options: { + stage: "super-long-stage-name", + }, + config: Object.assign(baseConfig, { + constructs: { + "suuuper-long-construct-name": { + type: "server-side-website", + }, + }, + }), + }); + expect(cfTemplate.Resources[computeLogicalId("suuuper-long-construct-name", "RequestFunction")]).toMatchObject({ + Type: "AWS::CloudFront::Function", + Properties: { + Name: "app-super-long-stage-name-us-east-1-suuuper-long-construc-f3b7e1", + }, + }); + }); }); diff --git a/test/unit/singlePageApp.test.ts b/test/unit/singlePageApp.test.ts index 6b1a1d19..01706d6b 100644 --- a/test/unit/singlePageApp.test.ts +++ b/test/unit/singlePageApp.test.ts @@ -161,4 +161,35 @@ describe("single page app", () => { ObjectLockEnabled: true, }); }); + + it("trims CloudFront function names to stay under the limit", async () => { + const { cfTemplate, computeLogicalId } = await runServerless({ + command: "package", + options: { + stage: "super-long-stage-name", + }, + config: Object.assign(baseConfig, { + constructs: { + "suuuper-long-construct-name": { + type: "single-page-app", + path: ".", + }, + }, + }), + }); + expect(cfTemplate.Resources[computeLogicalId("suuuper-long-construct-name", "RequestFunction")]).toMatchObject({ + Type: "AWS::CloudFront::Function", + Properties: { + Name: "app-super-long-stage-name-us-east-1-suuuper-long-construc-f3b7e1", + }, + }); + expect(cfTemplate.Resources[computeLogicalId("suuuper-long-construct-name", "ResponseFunction")]).toMatchObject( + { + Type: "AWS::CloudFront::Function", + Properties: { + Name: "app-super-long-stage-name-us-east-1-suuuper-long-construc-8c1f76", + }, + } + ); + }); }); diff --git a/test/unit/staticWebsite.test.ts b/test/unit/staticWebsite.test.ts index 8b18b810..3763d9e2 100644 --- a/test/unit/staticWebsite.test.ts +++ b/test/unit/staticWebsite.test.ts @@ -610,4 +610,38 @@ describe("static websites", () => { ObjectLockEnabled: true, }); }); + + it("trims CloudFront function names to stay under the limit", async () => { + const { cfTemplate, computeLogicalId } = await runServerless({ + command: "package", + options: { + stage: "super-long-stage-name", + }, + config: Object.assign(baseConfig, { + constructs: { + "suuuper-long-construct-name": { + type: "static-website", + path: ".", + domain: ["foo.com", "bar.com"], + certificate: "arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234", + redirectToMainDomain: true, + }, + }, + }), + }); + expect(cfTemplate.Resources[computeLogicalId("suuuper-long-construct-name", "RequestFunction")]).toMatchObject({ + Type: "AWS::CloudFront::Function", + Properties: { + Name: "app-super-long-stage-name-us-east-1-suuuper-long-construc-f3b7e1", + }, + }); + expect(cfTemplate.Resources[computeLogicalId("suuuper-long-construct-name", "ResponseFunction")]).toMatchObject( + { + Type: "AWS::CloudFront::Function", + Properties: { + Name: "app-super-long-stage-name-us-east-1-suuuper-long-construc-8c1f76", + }, + } + ); + }); }); diff --git a/test/unit/utils/naming.test.ts b/test/unit/utils/naming.test.ts new file mode 100644 index 00000000..7dff37de --- /dev/null +++ b/test/unit/utils/naming.test.ts @@ -0,0 +1,14 @@ +import { ensureNameMaxLength } from "../../../src/utils/naming"; + +describe("naming", () => { + it("should not change names shorter than the limit", () => { + expect(ensureNameMaxLength("foo", 3)).toEqual("foo"); + }); + + it("should trim names with a unique suffix to stay under the limit", () => { + expect(ensureNameMaxLength("foobarfoobarfoobarfoobar", 15)).toEqual("foobarfo-7ca709"); + expect(ensureNameMaxLength("foobarfoobarfoobarfoobar", 15)).toHaveLength(15); + // The suffix changes based on teh full string to avoid duplicates + expect(ensureNameMaxLength("foobarfoofoofoofoofoofoo", 15)).not.toEqual("foobarfo-7ca709"); + }); +});