From 1cbfb8f28813c49417c4c74c687e9c5d0fd525c0 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Wed, 29 Jul 2020 02:06:17 -0700 Subject: [PATCH] chore(cloudfront): small refactoring of the Origin API (#9281) Change the Origin to an interface, from an abstract class, and change its `bind` protocol to return an `OriginBindConfig` interface. This is in preparation for handling Origin Groups in #9109 - when time comes to handle Origin Groups, we will add a new (optional) property to `OriginBindConfig`, of type `CfnDistribution.OriginGroupProperty`, and handle it in `Distribution`. BREAKING CHANGE: the property Origin.domainName has been removed ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-cloudfront-origins/lib/s3-origin.ts | 21 ++----- .../test/http-origin.test.ts | 8 +-- .../test/integ.s3-origin.expected.json | 8 +-- .../test/load-balancer-origin.test.ts | 8 +-- .../test/s3-origin.test.ts | 14 ++--- .../aws-cloudfront/lib/distribution.ts | 43 +++++++++----- .../@aws-cdk/aws-cloudfront/lib/origin.ts | 58 +++++++++++-------- .../lib/private/cache-behavior.ts | 17 ++---- .../aws-cloudfront/test/distribution.test.ts | 19 +++--- .../aws-cloudfront/test/origin.test.ts | 24 ++++---- .../test/private/cache-behavior.test.ts | 21 +++---- 11 files changed, 121 insertions(+), 120 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts index b976e482b2276..2896ba2a5df79 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts @@ -26,9 +26,8 @@ export interface S3OriginProps { * * @experimental */ -export class S3Origin extends cloudfront.Origin { - - private readonly origin: cloudfront.Origin; +export class S3Origin implements cloudfront.IOrigin { + private readonly origin: cloudfront.IOrigin; constructor(bucket: s3.IBucket, props: S3OriginProps = {}) { let proxyOrigin; @@ -43,22 +42,10 @@ export class S3Origin extends cloudfront.Origin { ...props, }); } - - super(proxyOrigin.domainName); - this.origin = proxyOrigin; } - public get id() { - return this.origin.id; - } - - public bind(scope: cdk.Construct, options: cloudfront.OriginBindOptions) { - this.origin.bind(scope, options); - } - - public renderOrigin() { - return this.origin.renderOrigin(); + public bind(scope: cdk.Construct, options: cloudfront.OriginBindOptions): cloudfront.OriginBindConfig { + return this.origin.bind(scope, options); } - } diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts index a5475dc4c760b..246392355586e 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts @@ -15,9 +15,9 @@ beforeEach(() => { test('Renders minimal example with just a domain name', () => { const origin = new HttpOrigin('www.example.com'); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', customOriginConfig: { @@ -32,9 +32,9 @@ test('Can customize properties of the origin', () => { readTimeout: Duration.seconds(10), protocolPolicy: cloudfront.OriginProtocolPolicy.MATCH_VIEWER, }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', originCustomHeaders: [{ diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json index 64455613ffcbc..173044c7a0dab 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json @@ -23,7 +23,7 @@ "Principal": { "CanonicalUser": { "Fn::GetAtt": [ - "DistributionS3Origin115FD918D", + "DistributionOrigin1S3Origin5F5C0696", "S3CanonicalUserId" ] } @@ -56,7 +56,7 @@ } } }, - "DistributionS3Origin115FD918D": { + "DistributionOrigin1S3Origin5F5C0696": { "Type": "AWS::CloudFront::CloudFrontOriginAccessIdentity", "Properties": { "CloudFrontOriginAccessIdentityConfig": { @@ -92,7 +92,7 @@ [ "origin-access-identity/cloudfront/", { - "Ref": "DistributionS3Origin115FD918D" + "Ref": "DistributionOrigin1S3Origin5F5C0696" } ] ] @@ -104,4 +104,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts index c06c9cc7a7b84..e4ad46372fbef 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts @@ -21,9 +21,9 @@ test('Renders minimal example with just a load balancer', () => { }); const origin = new LoadBalancerV2Origin(loadBalancer); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: loadBalancer.loadBalancerDnsName, customOriginConfig: { @@ -43,9 +43,9 @@ test('Can customize properties of the origin', () => { connectionTimeout: Duration.seconds(5), protocolPolicy: cloudfront.OriginProtocolPolicy.MATCH_VIEWER, }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: loadBalancer.loadBalancerDnsName, connectionAttempts: 3, diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts index c851f53b122a0..9c0380411e494 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts @@ -17,9 +17,9 @@ test('With non-website bucket, renders all required properties, including S3Orig const bucket = new s3.Bucket(stack, 'Bucket'); const origin = new S3Origin(bucket); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketRegionalDomainName, s3OriginConfig: { @@ -34,9 +34,9 @@ test('With website bucket, renders all required properties, including custom ori }); const origin = new S3Origin(bucket); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketWebsiteDomainName, customOriginConfig: { @@ -51,9 +51,9 @@ test('Respects props passed down to underlying origin', () => { }); const origin = new S3Origin(bucket, { originPath: '/website' }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketWebsiteDomainName, originPath: '/website', @@ -61,4 +61,4 @@ test('Respects props passed down to underlying origin', () => { originProtocolPolicy: 'http-only', }, }); -}); \ No newline at end of file +}); diff --git a/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts index e32693aa79888..19516b8a59ae8 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts @@ -2,7 +2,7 @@ import * as acm from '@aws-cdk/aws-certificatemanager'; import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, IResource, Lazy, Resource, Stack, Token, Duration } from '@aws-cdk/core'; import { CfnDistribution } from './cloudfront.generated'; -import { Origin } from './origin'; +import { IOrigin, OriginBindConfig, OriginBindOptions } from './origin'; import { CacheBehavior } from './private/cache-behavior'; /** @@ -53,6 +53,10 @@ export interface DistributionAttributes { readonly distributionId: string; } +interface BoundOrigin extends OriginBindOptions, OriginBindConfig { + readonly origin: IOrigin; +} + /** * Properties for a Distribution * @@ -127,7 +131,7 @@ export class Distribution extends Resource implements IDistribution { private readonly defaultBehavior: CacheBehavior; private readonly additionalBehaviors: CacheBehavior[] = []; - private readonly origins: Set = new Set(); + private readonly boundOrigins: BoundOrigin[] = []; private readonly errorResponses: ErrorResponse[]; private readonly certificate?: acm.ICertificate; @@ -142,8 +146,8 @@ export class Distribution extends Resource implements IDistribution { } } - this.defaultBehavior = new CacheBehavior({ pathPattern: '*', ...props.defaultBehavior }); - this.addOrigin(this.defaultBehavior.origin); + const originId = this.addOrigin(props.defaultBehavior.origin); + this.defaultBehavior = new CacheBehavior(originId, { pathPattern: '*', ...props.defaultBehavior }); if (props.additionalBehaviors) { Object.entries(props.additionalBehaviors).forEach(([pathPattern, behaviorOptions]) => { this.addBehavior(pathPattern, behaviorOptions.origin, behaviorOptions); @@ -172,26 +176,38 @@ export class Distribution extends Resource implements IDistribution { * Adds a new behavior to this distribution for the given pathPattern. * * @param pathPattern the path pattern (e.g., 'images/*') that specifies which requests to apply the behavior to. + * @param origin the origin to use for this behavior * @param behaviorOptions the options for the behavior at this path. */ - public addBehavior(pathPattern: string, origin: Origin, behaviorOptions: AddBehaviorOptions = {}) { + public addBehavior(pathPattern: string, origin: IOrigin, behaviorOptions: AddBehaviorOptions = {}) { if (pathPattern === '*') { throw new Error('Only the default behavior can have a path pattern of \'*\''); } - this.additionalBehaviors.push(new CacheBehavior({ pathPattern, origin, ...behaviorOptions })); - this.addOrigin(origin); + const originId = this.addOrigin(origin); + this.additionalBehaviors.push(new CacheBehavior(originId, { pathPattern, ...behaviorOptions })); } - private addOrigin(origin: Origin) { - if (!this.origins.has(origin)) { - this.origins.add(origin); - origin.bind(this, { originIndex: this.origins.size }); + private addOrigin(origin: IOrigin): string { + const existingOrigin = this.boundOrigins.find(boundOrigin => boundOrigin.origin === origin); + if (existingOrigin) { + return existingOrigin.originId; + } else { + const originIndex = this.boundOrigins.length + 1; + const scope = new Construct(this, `Origin${originIndex}`); + const originId = scope.node.uniqueId; + const originBindConfig = origin.bind(scope, { originId }); + this.boundOrigins.push({ origin, originId, ...originBindConfig }); + return originId; } } private renderOrigins(): CfnDistribution.OriginProperty[] { const renderedOrigins: CfnDistribution.OriginProperty[] = []; - this.origins.forEach(origin => renderedOrigins.push(origin.renderOrigin())); + this.boundOrigins.forEach(boundOrigin => { + if (boundOrigin.originProperty) { + renderedOrigins.push(boundOrigin.originProperty); + } + }); return renderedOrigins; } @@ -229,7 +245,6 @@ export class Distribution extends Resource implements IDistribution { minimumProtocolVersion: SecurityPolicyProtocol.TLS_V1_2_2018, }; } - } /** @@ -443,5 +458,5 @@ export interface BehaviorOptions extends AddBehaviorOptions { /** * The origin that you want CloudFront to route requests to when they match this behavior. */ - readonly origin: Origin; + readonly origin: IOrigin; } diff --git a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts index ee15b4106f0e1..5e680f5e29f48 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts @@ -4,6 +4,28 @@ import { CfnDistribution } from './cloudfront.generated'; import { OriginProtocolPolicy } from './distribution'; import { OriginAccessIdentity } from './origin_access_identity'; +/** The struct returned from {@link IOrigin.bind}. */ +export interface OriginBindConfig { + /** + * The CloudFormation OriginProperty configuration for this Origin. + * + * @default - nothing is returned + */ + readonly originProperty?: CfnDistribution.OriginProperty; +} + +/** + * Represents the concept of a CloudFront Origin. + * You provide one or more origins when creating a Distribution. + */ +export interface IOrigin { + /** + * The method called when a given Origin is added + * (for the first time) to a Distribution. + */ + bind(scope: Construct, options: OriginBindOptions): OriginBindConfig; +} + /** * Properties to define an Origin. * @@ -48,9 +70,10 @@ export interface OriginProps { */ export interface OriginBindOptions { /** - * The positional index of this origin within the distribution. Used for ensuring unique IDs. + * The identifier of this Origin, + * as assigned by the Distribution this Origin has been used added to. */ - readonly originIndex: number; + readonly originId: string; } /** @@ -59,13 +82,8 @@ export interface OriginBindOptions { * * @experimental */ -export abstract class Origin { - - /** - * The domain name of the origin. - */ - public readonly domainName: string; - +export abstract class Origin implements IOrigin { + private readonly domainName: string; private readonly originPath?: string; private readonly connectionTimeout?: Duration; private readonly connectionAttempts?: number; @@ -97,14 +115,9 @@ export abstract class Origin { /** * Binds the origin to the associated Distribution. Can be used to grant permissions, create dependent resources, etc. */ - public bind(scope: Construct, options: OriginBindOptions): void { - this.originId = new Construct(scope, `Origin${options.originIndex}`).node.uniqueId; - } + public bind(_scope: Construct, options: OriginBindOptions): OriginBindConfig { + this.originId = options.originId; - /** - * Creates and returns the CloudFormation representation of this origin. - */ - public renderOrigin(): CfnDistribution.OriginProperty { const s3OriginConfig = this.renderS3OriginConfig(); const customOriginConfig = this.renderCustomOriginConfig(); @@ -112,7 +125,7 @@ export abstract class Origin { throw new Error('Subclass must override and provide either s3OriginConfig or customOriginConfig'); } - return { + return { originProperty: { domainName: this.domainName, id: this.id, originPath: this.originPath, @@ -121,7 +134,7 @@ export abstract class Origin { originCustomHeaders: this.renderCustomHeaders(), s3OriginConfig, customOriginConfig, - }; + }}; } // Overridden by sub-classes to provide S3 origin config. @@ -153,7 +166,6 @@ export abstract class Origin { if (path.endsWith('/')) { path = path.substr(0, path.length - 1); } return path; } - } /** @@ -184,12 +196,12 @@ export class S3Origin extends Origin { this.bucket = props.bucket; } - public bind(scope: Construct, options: OriginBindOptions) { - super.bind(scope, options); + public bind(scope: Construct, options: OriginBindOptions): OriginBindConfig { if (!this.originAccessIdentity) { - this.originAccessIdentity = new OriginAccessIdentity(scope, `S3Origin${options.originIndex}`); + this.originAccessIdentity = new OriginAccessIdentity(scope, 'S3Origin'); this.bucket.grantRead(this.originAccessIdentity); } + return super.bind(scope, options); } protected renderS3OriginConfig(): CfnDistribution.S3OriginConfigProperty | undefined { @@ -275,4 +287,4 @@ function validateIntInRangeOrUndefined(name: string, min: number, max: number, v const seconds = isDuration ? ' seconds' : ''; throw new Error(`${name}: Must be an int between ${min} and ${max}${seconds} (inclusive); received ${value}.`); } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts b/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts index 363590333e777..2e44abcfc05c0 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts @@ -1,11 +1,10 @@ import { CfnDistribution } from '../cloudfront.generated'; -import { BehaviorOptions, ViewerProtocolPolicy } from '../distribution'; -import { Origin } from '../origin'; +import { AddBehaviorOptions, ViewerProtocolPolicy } from '../distribution'; /** * Properties for specifying custom behaviors for origins. */ -export interface CacheBehaviorProps extends BehaviorOptions { +export interface CacheBehaviorProps extends AddBehaviorOptions { /** * The pattern (e.g., `images/*.jpg`) that specifies which requests to apply the behavior to. * There must be exactly one behavior associated with each `Distribution` that has a path pattern @@ -21,14 +20,10 @@ export interface CacheBehaviorProps extends BehaviorOptions { * CloudFrontWebDistribution implementation. */ export class CacheBehavior { + private readonly originId: string; - /** - * Origin that this behavior will route traffic to. - */ - public readonly origin: Origin; - - constructor(private readonly props: CacheBehaviorProps) { - this.origin = props.origin; + constructor(originId: string, private readonly props: CacheBehaviorProps) { + this.originId = originId; } /** @@ -42,7 +37,7 @@ export class CacheBehavior { public _renderBehavior(): CfnDistribution.CacheBehaviorProperty { return { pathPattern: this.props.pathPattern, - targetOriginId: this.origin.id, + targetOriginId: this.originId, allowedMethods: this.props.allowedMethods?.methods ?? undefined, forwardedValues: { queryString: this.props.forwardQueryString ?? false, diff --git a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts index 3db81d0b7ae1c..aa29d8da5e8ce 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts @@ -3,7 +3,7 @@ import * as acm from '@aws-cdk/aws-certificatemanager'; import * as lambda from '@aws-cdk/aws-lambda'; import * as s3 from '@aws-cdk/aws-s3'; import { App, Duration, Stack } from '@aws-cdk/core'; -import { Distribution, LambdaEdgeEventType, Origin, PriceClass, S3Origin } from '../lib'; +import { Distribution, IOrigin, LambdaEdgeEventType, S3Origin, PriceClass } from '../lib'; let app: App; let stack: Stack; @@ -32,7 +32,7 @@ test('minimal example renders correctly', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }], @@ -83,7 +83,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }], @@ -121,7 +121,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }, @@ -130,7 +130,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin20B96F3AD', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin2E88F08BB' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin2S3Origin26A1EB27' } ], ]}, }, }], @@ -175,7 +175,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }, @@ -184,14 +184,13 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin20B96F3AD', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin2E88F08BB' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin2S3Origin26A1EB27' } ], ]}, }, }], }, }); }); - }); describe('certificates', () => { @@ -291,7 +290,7 @@ describe('custom error responses', () => { describe('with Lambda@Edge functions', () => { let lambdaFunction: lambda.Function; - let origin: Origin; + let origin: IOrigin; beforeEach(() => { lambdaFunction = new lambda.Function(stack, 'Function', { @@ -398,6 +397,6 @@ test('price class is included if provided', () => { }); }); -function defaultS3Origin(): Origin { +function defaultS3Origin(): IOrigin { return new S3Origin({ bucket: new s3.Bucket(stack, 'Bucket') }); } diff --git a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts index 716af7e3225f5..fb260dc221c9b 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts @@ -1,7 +1,7 @@ import '@aws-cdk/assert/jest'; import * as s3 from '@aws-cdk/aws-s3'; import { App, Stack, Duration } from '@aws-cdk/core'; -import { CfnDistribution, Distribution, Origin, OriginProps, HttpOrigin, OriginProtocolPolicy, S3Origin } from '../lib'; +import { CfnDistribution, Distribution, HttpOrigin, OriginProtocolPolicy, S3Origin, Origin, OriginProps } from '../lib'; let app: App; let stack: Stack; @@ -18,9 +18,9 @@ describe('S3Origin', () => { const bucket = new s3.Bucket(stack, 'Bucket'); const origin = new S3Origin({ bucket }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketRegionalDomainName, s3OriginConfig: { @@ -44,7 +44,7 @@ describe('S3Origin', () => { PolicyDocument: { Statement: [{ Principal: { - CanonicalUser: { 'Fn::GetAtt': [ 'DistS3Origin1C4519663', 'S3CanonicalUserId' ] }, + CanonicalUser: { 'Fn::GetAtt': [ 'DistOrigin1S3Origin87D64058', 'S3CanonicalUserId' ] }, }, }], }, @@ -55,9 +55,9 @@ describe('S3Origin', () => { describe('HttpOrigin', () => { test('renders a minimal example with required props', () => { const origin = new HttpOrigin('www.example.com'); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', customOriginConfig: { @@ -78,9 +78,9 @@ describe('HttpOrigin', () => { readTimeout: Duration.seconds(45), keepaliveTimeout: Duration.seconds(3), }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', originPath: '/app', @@ -127,7 +127,7 @@ describe('HttpOrigin', () => { }); }).toThrow(`keepaliveTimeout: Must be an int between 1 and 60 seconds (inclusive); received ${keepaliveTimeout.toSeconds()}.`); }); -});; +}); describe('Origin', () => { test.each([ @@ -158,9 +158,9 @@ describe('Origin', () => { const origin = new TestOrigin('www.example.com', { originPath, }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: '0' }); - expect(origin.renderOrigin().originPath).toEqual('/api'); + expect(originBindConfig.originProperty?.originPath).toEqual('/api'); }); }); @@ -170,4 +170,4 @@ class TestOrigin extends Origin { protected renderS3OriginConfig(): CfnDistribution.S3OriginConfigProperty | undefined { return { originAccessIdentity: 'origin-access-identity/cloudfront/MyOAIName' }; } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts b/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts index 81dd84d716dd9..4477a411cf790 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts @@ -1,28 +1,24 @@ import '@aws-cdk/assert/jest'; import { App, Stack } from '@aws-cdk/core'; -import { AllowedMethods, HttpOrigin } from '../../lib'; +import { AllowedMethods } from '../../lib'; import { CacheBehavior } from '../../lib/private/cache-behavior'; let app: App; -let stack: Stack; beforeEach(() => { app = new App(); - stack = new Stack(app, 'Stack', { + new Stack(app, 'Stack', { env: { account: '1234', region: 'testregion' }, }); }); test('renders the minimum template with an origin and path specified', () => { - const origin = new HttpOrigin('www.example.com'); - const behavior = new CacheBehavior({ - origin, + const behavior = new CacheBehavior('origin_id', { pathPattern: '*', }); - origin.bind(stack, { originIndex: 0 }); expect(behavior._renderBehavior()).toEqual({ - targetOriginId: behavior.origin.id, + targetOriginId: 'origin_id', pathPattern: '*', forwardedValues: { queryString: false }, viewerProtocolPolicy: 'allow-all', @@ -30,18 +26,15 @@ test('renders the minimum template with an origin and path specified', () => { }); test('renders with all properties specified', () => { - const origin = new HttpOrigin('www.example.com'); - const behavior = new CacheBehavior({ - origin, + const behavior = new CacheBehavior('origin_id', { pathPattern: '*', allowedMethods: AllowedMethods.ALLOW_ALL, forwardQueryString: true, forwardQueryStringCacheKeys: ['user_id', 'auth'], }); - origin.bind(stack, { originIndex: 0 }); expect(behavior._renderBehavior()).toEqual({ - targetOriginId: behavior.origin.id, + targetOriginId: 'origin_id', pathPattern: '*', allowedMethods: ['GET', 'HEAD', 'OPTIONS', 'PUT', 'PATCH', 'POST', 'DELETE'], forwardedValues: { @@ -50,4 +43,4 @@ test('renders with all properties specified', () => { }, viewerProtocolPolicy: 'allow-all', }); -}); \ No newline at end of file +});