From 6076ed99ae1bf11e5e1df7c7a8d483fcb178bda7 Mon Sep 17 00:00:00 2001 From: Gehan Gonsalkorale Date: Thu, 30 Apr 2020 15:45:36 +0100 Subject: [PATCH 1/4] Improve handling of s3 origins * Handle s3 website urls properly * Allow multiple origins from same s3 bucket * Allow settings of original protocol polic --- .../cache-behavior-options.test.js.snap | 10 +- .../__snapshots__/s3-origin.test.js.snap | 178 ++++++++++++++++++ __tests__/cache-behavior-options.test.js | 3 +- __tests__/s3-origin.test.js | 54 ++++++ lib/getOriginConfig.js | 9 +- lib/index.js | 10 +- 6 files changed, 251 insertions(+), 13 deletions(-) diff --git a/__tests__/__snapshots__/cache-behavior-options.test.js.snap b/__tests__/__snapshots__/cache-behavior-options.test.js.snap index bc1d314..ac89fd0 100644 --- a/__tests__/__snapshots__/cache-behavior-options.test.js.snap +++ b/__tests__/__snapshots__/cache-behavior-options.test.js.snap @@ -57,7 +57,7 @@ Object { "MinTTL": 0, "PathPattern": "/sample/path", "SmoothStreaming": false, - "TargetOriginId": "mycustomorigin.com", + "TargetOriginId": "mycustomorigin.com/path", "TrustedSigners": Object { "Enabled": false, "Quantity": 0, @@ -108,7 +108,7 @@ Object { "MaxTTL": 31536000, "MinTTL": 0, "SmoothStreaming": false, - "TargetOriginId": "mycustomorigin.com", + "TargetOriginId": "mycustomorigin.com/path", "TrustedSigners": Object { "Enabled": false, "Items": Array [], @@ -129,7 +129,7 @@ Object { "HTTPPort": 80, "HTTPSPort": 443, "OriginKeepaliveTimeout": 5, - "OriginProtocolPolicy": "https-only", + "OriginProtocolPolicy": "http-only", "OriginReadTimeout": 30, "OriginSslProtocols": Object { "Items": Array [ @@ -139,8 +139,8 @@ Object { }, }, "DomainName": "mycustomorigin.com", - "Id": "mycustomorigin.com", - "OriginPath": "", + "Id": "mycustomorigin.com/path", + "OriginPath": "/path", }, ], "Quantity": 1, diff --git a/__tests__/__snapshots__/s3-origin.test.js.snap b/__tests__/__snapshots__/s3-origin.test.js.snap index 37ef4ef..2003cbd 100644 --- a/__tests__/__snapshots__/s3-origin.test.js.snap +++ b/__tests__/__snapshots__/s3-origin.test.js.snap @@ -330,3 +330,181 @@ Object { "IfMatch": "etag", } `; + +exports[`S3 origins When origin is an S3 bucket URL with path creates distribution 1`] = ` +Object { + "DistributionConfig": Object { + "Aliases": Object { + "Items": Array [], + "Quantity": 0, + }, + "CacheBehaviors": Object { + "Items": Array [], + "Quantity": 0, + }, + "CallerReference": "1566599541192", + "Comment": "", + "DefaultCacheBehavior": Object { + "AllowedMethods": Object { + "CachedMethods": Object { + "Items": Array [ + "HEAD", + "GET", + ], + "Quantity": 2, + }, + "Items": Array [ + "HEAD", + "GET", + ], + "Quantity": 2, + }, + "Compress": false, + "DefaultTTL": 86400, + "FieldLevelEncryptionId": "", + "ForwardedValues": Object { + "Cookies": Object { + "Forward": "none", + }, + "Headers": Object { + "Items": Array [], + "Quantity": 0, + }, + "QueryString": false, + "QueryStringCacheKeys": Object { + "Items": Array [], + "Quantity": 0, + }, + }, + "LambdaFunctionAssociations": Object { + "Items": Array [], + "Quantity": 0, + }, + "MaxTTL": 31536000, + "MinTTL": 0, + "SmoothStreaming": false, + "TargetOriginId": "mybucket/static", + "TrustedSigners": Object { + "Enabled": false, + "Items": Array [], + "Quantity": 0, + }, + "ViewerProtocolPolicy": "redirect-to-https", + }, + "Enabled": true, + "HttpVersion": "http2", + "Origins": Object { + "Items": Array [ + Object { + "CustomHeaders": Object { + "Items": Array [], + "Quantity": 0, + }, + "DomainName": "mybucket.s3.amazonaws.com", + "Id": "mybucket/static", + "OriginPath": "/static", + "S3OriginConfig": Object { + "OriginAccessIdentity": "", + }, + }, + ], + "Quantity": 1, + }, + "PriceClass": "PriceClass_All", + }, +} +`; + +exports[`S3 origins When origin is an S3 website URL creates custom origin not s3 origin distribution 1`] = ` +Object { + "DistributionConfig": Object { + "Aliases": Object { + "Items": Array [], + "Quantity": 0, + }, + "CacheBehaviors": Object { + "Items": Array [], + "Quantity": 0, + }, + "CallerReference": "1566599541192", + "Comment": "", + "DefaultCacheBehavior": Object { + "AllowedMethods": Object { + "CachedMethods": Object { + "Items": Array [ + "HEAD", + "GET", + ], + "Quantity": 2, + }, + "Items": Array [ + "HEAD", + "GET", + ], + "Quantity": 2, + }, + "Compress": false, + "DefaultTTL": 86400, + "FieldLevelEncryptionId": "", + "ForwardedValues": Object { + "Cookies": Object { + "Forward": "none", + }, + "Headers": Object { + "Items": Array [], + "Quantity": 0, + }, + "QueryString": false, + "QueryStringCacheKeys": Object { + "Items": Array [], + "Quantity": 0, + }, + }, + "LambdaFunctionAssociations": Object { + "Items": Array [], + "Quantity": 0, + }, + "MaxTTL": 31536000, + "MinTTL": 0, + "SmoothStreaming": false, + "TargetOriginId": "mybucket.s3-website.amazonaws.com", + "TrustedSigners": Object { + "Enabled": false, + "Items": Array [], + "Quantity": 0, + }, + "ViewerProtocolPolicy": "redirect-to-https", + }, + "Enabled": true, + "HttpVersion": "http2", + "Origins": Object { + "Items": Array [ + Object { + "CustomHeaders": Object { + "Items": Array [], + "Quantity": 0, + }, + "CustomOriginConfig": Object { + "HTTPPort": 80, + "HTTPSPort": 443, + "OriginKeepaliveTimeout": 5, + "OriginProtocolPolicy": "https-only", + "OriginReadTimeout": 30, + "OriginSslProtocols": Object { + "Items": Array [ + "TLSv1.2", + ], + "Quantity": 1, + }, + }, + "DomainName": "mybucket.s3-website.amazonaws.com", + "Id": "mybucket.s3-website.amazonaws.com", + "OriginPath": "", + }, + ], + "Quantity": 1, + }, + "PriceClass": "PriceClass_All", + }, +} +`; diff --git a/__tests__/cache-behavior-options.test.js b/__tests__/cache-behavior-options.test.js index 4edcf52..6f48dff 100644 --- a/__tests__/cache-behavior-options.test.js +++ b/__tests__/cache-behavior-options.test.js @@ -44,7 +44,8 @@ describe('Input origin as a custom url', () => { }, origins: [ { - url: 'https://mycustomorigin.com', + url: 'https://mycustomorigin.com/path', + protocolPolicy: 'http-only', pathPatterns: { '/sample/path': { ttl: 0, diff --git a/__tests__/s3-origin.test.js b/__tests__/s3-origin.test.js index a5ef639..86aff3e 100644 --- a/__tests__/s3-origin.test.js +++ b/__tests__/s3-origin.test.js @@ -77,6 +77,60 @@ describe('S3 origins', () => { }) }) + describe('When origin is an S3 bucket URL with path', () => { + it('creates distribution', async () => { + await component.default({ + origins: ['https://mybucket.s3.amazonaws.com/static'] + }) + + assertHasOrigin(mockCreateDistribution, { + Id: 'mybucket/static', + DomainName: 'mybucket.s3.amazonaws.com', + S3OriginConfig: { + OriginAccessIdentity: '' + }, + CustomHeaders: { + Quantity: 0, + Items: [] + }, + OriginPath: '/static' + }) + + expect(mockCreateDistribution.mock.calls[0][0]).toMatchSnapshot() + }) + }) + + describe('When origin is an S3 website URL', () => { + it('creates custom origin not s3 origin distribution', async () => { + await component.default({ + origins: ['https://mybucket.s3-website.amazonaws.com'] + }) + + assertHasOrigin(mockCreateDistribution, { + Id: 'mybucket.s3-website.amazonaws.com', + DomainName: 'mybucket.s3-website.amazonaws.com', + CustomHeaders: { + Quantity: 0, + Items: [] + }, + CustomOriginConfig: { + HTTPPort: 80, + HTTPSPort: 443, + OriginProtocolPolicy: 'https-only', + OriginSslProtocols: { + Quantity: 1, + Items: ['TLSv1.2'] + }, + OriginReadTimeout: 30, + OriginKeepaliveTimeout: 5 + }, + OriginPath: '' + }) + + expect(mockCreateDistribution.mock.calls[0][0]).toMatchSnapshot() + }) + }) + describe('When origin is an S3 URL only accessible via CloudFront', () => { it('creates distribution', async () => { mockCreateCloudFrontOriginAccessIdentityPromise.mockResolvedValueOnce({ diff --git a/lib/getOriginConfig.js b/lib/getOriginConfig.js index fdecd6b..56ce5ca 100644 --- a/lib/getOriginConfig.js +++ b/lib/getOriginConfig.js @@ -6,7 +6,7 @@ module.exports = (origin, { originAccessIdentityId = '' }) => { const { hostname, pathname } = url.parse(originUrl) const originConfig = { - Id: hostname, + Id: `${hostname}${pathname}`.replace(/\/$/, ''), DomainName: hostname, CustomHeaders: { Quantity: 0, @@ -15,9 +15,10 @@ module.exports = (origin, { originAccessIdentityId = '' }) => { OriginPath: pathname === '/' ? '' : pathname } - if (originUrl.includes('s3')) { + if (originUrl.includes('s3') && !originUrl.includes('s3-website')) { + // attach s3 origin for buckets, but don't do this for buckets configured as website const bucketName = hostname.split('.')[0] - originConfig.Id = bucketName + originConfig.Id = pathname === '/' ? bucketName : `${bucketName}${pathname}` originConfig.DomainName = `${bucketName}.s3.amazonaws.com` originConfig.S3OriginConfig = { OriginAccessIdentity: originAccessIdentityId @@ -28,7 +29,7 @@ module.exports = (origin, { originAccessIdentityId = '' }) => { originConfig.CustomOriginConfig = { HTTPPort: 80, HTTPSPort: 443, - OriginProtocolPolicy: 'https-only', + OriginProtocolPolicy: origin.protocolPolicy || 'https-only', OriginSslProtocols: { Quantity: 1, Items: ['TLSv1.2'] diff --git a/lib/index.js b/lib/index.js index 0bc74e5..796172f 100644 --- a/lib/index.js +++ b/lib/index.js @@ -8,11 +8,15 @@ const servePrivateContentEnabled = (inputs) => return origin && origin.private === true }) +const unique = (value, index, self) => self.indexOf(value) === index const updateBucketsPolicies = async (s3, origins, s3CanonicalUserId) => { // update bucket policies with cloudfront access - const bucketNames = origins.Items.filter((origin) => origin.S3OriginConfig).map( - (origin) => origin.Id - ) + const bucketNames = origins.Items.filter((origin) => origin.S3OriginConfig) + .map((origin) => { + // remove path from bucketname if origin had pathname + return origin.Id.split('/')[0] + }) + .filter(unique) return Promise.all( bucketNames.map((bucketName) => grantCloudFrontBucketAccess(s3, bucketName, s3CanonicalUserId)) From 1e0086c6431c7a309eda1cb04083b5f754b140b3 Mon Sep 17 00:00:00 2001 From: Gehan Gonsalkorale Date: Fri, 1 May 2020 19:57:22 +0100 Subject: [PATCH 2/4] Handle IncludeBody for lambda triggers correctly --- README.md | 3 ++ .../__snapshots__/lambda-at-edge.test.js.snap | 6 +-- __tests__/lambda-at-edge.test.js | 43 ++++++++++++++++--- lib/addLambdaAtEdgeToCacheBehavior.js | 29 ++++++++++--- 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index a00bf23..3dc4e02 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,9 @@ distribution: ttl: 10 lambda@edge: viewer-request: arn:aws:lambda:us-east-1:123:function:myFunc:version # lambda ARN including version + response-request: # can also send object to not include body + arn: lambda-arn + includeBody: false ``` #### Private S3 Content diff --git a/__tests__/__snapshots__/lambda-at-edge.test.js.snap b/__tests__/__snapshots__/lambda-at-edge.test.js.snap index f3c0233..021fac1 100644 --- a/__tests__/__snapshots__/lambda-at-edge.test.js.snap +++ b/__tests__/__snapshots__/lambda-at-edge.test.js.snap @@ -50,17 +50,17 @@ Object { }, Object { "EventType": "origin-request", - "IncludeBody": true, + "IncludeBody": false, "LambdaFunctionARN": "arn:aws:lambda:us-east-1:123:function:originRequestFunction", }, Object { "EventType": "origin-response", - "IncludeBody": true, + "IncludeBody": false, "LambdaFunctionARN": "arn:aws:lambda:us-east-1:123:function:originResponseFunction", }, Object { "EventType": "viewer-response", - "IncludeBody": true, + "IncludeBody": false, "LambdaFunctionARN": "arn:aws:lambda:us-east-1:123:function:viewerResponseFunction", }, ], diff --git a/__tests__/lambda-at-edge.test.js b/__tests__/lambda-at-edge.test.js index 3ae0693..183918d 100644 --- a/__tests__/lambda-at-edge.test.js +++ b/__tests__/lambda-at-edge.test.js @@ -24,8 +24,14 @@ describe('Input origin as a custom url', () => { '/some/path': { ttl: 10, 'lambda@edge': { - 'viewer-request': 'arn:aws:lambda:us-east-1:123:function:viewerRequestFunction', - 'origin-request': 'arn:aws:lambda:us-east-1:123:function:originRequestFunction', + 'viewer-request': { + arn: 'arn:aws:lambda:us-east-1:123:function:viewerRequestFunction', + includeBody: true + }, + 'origin-request': { + arn: 'arn:aws:lambda:us-east-1:123:function:originRequestFunction', + includeBody: false + }, 'origin-response': 'arn:aws:lambda:us-east-1:123:function:originResponseFunction', 'viewer-response': 'arn:aws:lambda:us-east-1:123:function:viewerResponseFunction' } @@ -48,17 +54,17 @@ describe('Input origin as a custom url', () => { { EventType: 'origin-request', LambdaFunctionARN: 'arn:aws:lambda:us-east-1:123:function:originRequestFunction', - IncludeBody: true + IncludeBody: false }, { EventType: 'origin-response', LambdaFunctionARN: 'arn:aws:lambda:us-east-1:123:function:originResponseFunction', - IncludeBody: true + IncludeBody: false }, { EventType: 'viewer-response', LambdaFunctionARN: 'arn:aws:lambda:us-east-1:123:function:viewerResponseFunction', - IncludeBody: true + IncludeBody: false } ] } @@ -92,4 +98,31 @@ describe('Input origin as a custom url', () => { ) } }) + + it('throws error when includeBody given for event types other than request', async () => { + expect.assertions(1) + + try { + await component.default({ + origins: [ + { + url: 'https://exampleorigin.com', + pathPatterns: { + '/some/path': { + ttl: 10, + 'lambda@edge': { + 'viewer-response': { + arn: 'arn:aws:lambda:us-east-1:123:function:viewerRequestFunction', + includeBody: true + } + } + } + } + } + ] + }) + } catch (err) { + expect(err.message).toEqual('"includeBody" not allowed for viewer-response lambda triggers.') + } + }) }) diff --git a/lib/addLambdaAtEdgeToCacheBehavior.js b/lib/addLambdaAtEdgeToCacheBehavior.js index 6728691..c101e9f 100644 --- a/lib/addLambdaAtEdgeToCacheBehavior.js +++ b/lib/addLambdaAtEdgeToCacheBehavior.js @@ -4,6 +4,27 @@ const validLambdaTriggers = [ 'origin-response', 'viewer-response' ] +const triggersAllowedBody = ['viewer-request', 'origin-request'] + +const makeCacheItem = (eventType, lambdaConfig) => { + let arn, includeBody + if (typeof lambdaConfig === 'string') { + arn = lambdaConfig + includeBody = triggersAllowedBody.includes(eventType) + } else { + ;({ arn, includeBody } = lambdaConfig) + if (includeBody && !triggersAllowedBody.includes(eventType)) { + throw new Error( + `"includeBody" not allowed for ${eventType} lambda triggers.` + ) + } + } + return { + EventType: eventType, + LambdaFunctionARN: arn, + IncludeBody: includeBody + } +} // adds lambda@edge to cache behavior passed module.exports = (cacheBehavior, lambdaAtEdgeConfig = {}) => { @@ -14,12 +35,10 @@ module.exports = (cacheBehavior, lambdaAtEdgeConfig = {}) => { ) } + cacheBehavior.LambdaFunctionAssociations.Items.push( + makeCacheItem(eventType, lambdaAtEdgeConfig[eventType]) + ) cacheBehavior.LambdaFunctionAssociations.Quantity = cacheBehavior.LambdaFunctionAssociations.Quantity + 1 - cacheBehavior.LambdaFunctionAssociations.Items.push({ - EventType: eventType, - LambdaFunctionARN: lambdaAtEdgeConfig[eventType], - IncludeBody: true - }) }) } From 2cedbe3f5bb803eda97ba83439d84f4d331989bd Mon Sep 17 00:00:00 2001 From: Gehan Gonsalkorale Date: Fri, 1 May 2020 20:07:10 +0100 Subject: [PATCH 3/4] Fix prettier lint error --- lib/addLambdaAtEdgeToCacheBehavior.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/addLambdaAtEdgeToCacheBehavior.js b/lib/addLambdaAtEdgeToCacheBehavior.js index c101e9f..9e0c6f5 100644 --- a/lib/addLambdaAtEdgeToCacheBehavior.js +++ b/lib/addLambdaAtEdgeToCacheBehavior.js @@ -14,9 +14,7 @@ const makeCacheItem = (eventType, lambdaConfig) => { } else { ;({ arn, includeBody } = lambdaConfig) if (includeBody && !triggersAllowedBody.includes(eventType)) { - throw new Error( - `"includeBody" not allowed for ${eventType} lambda triggers.` - ) + throw new Error(`"includeBody" not allowed for ${eventType} lambda triggers.`) } } return { From 4cc1b70b253045e14a8c28308583d69f2b2fb389 Mon Sep 17 00:00:00 2001 From: Gehan Gonsalkorale Date: Wed, 19 Aug 2020 17:37:11 +0100 Subject: [PATCH 4/4] Use set instead of unique, name funcion better --- lib/addLambdaAtEdgeToCacheBehavior.js | 4 ++-- lib/index.js | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/addLambdaAtEdgeToCacheBehavior.js b/lib/addLambdaAtEdgeToCacheBehavior.js index 9e0c6f5..fa499dc 100644 --- a/lib/addLambdaAtEdgeToCacheBehavior.js +++ b/lib/addLambdaAtEdgeToCacheBehavior.js @@ -6,7 +6,7 @@ const validLambdaTriggers = [ ] const triggersAllowedBody = ['viewer-request', 'origin-request'] -const makeCacheItem = (eventType, lambdaConfig) => { +const createLambdaFnAssociation = (eventType, lambdaConfig) => { let arn, includeBody if (typeof lambdaConfig === 'string') { arn = lambdaConfig @@ -34,7 +34,7 @@ module.exports = (cacheBehavior, lambdaAtEdgeConfig = {}) => { } cacheBehavior.LambdaFunctionAssociations.Items.push( - makeCacheItem(eventType, lambdaAtEdgeConfig[eventType]) + createLambdaFnAssociation(eventType, lambdaAtEdgeConfig[eventType]) ) cacheBehavior.LambdaFunctionAssociations.Quantity = cacheBehavior.LambdaFunctionAssociations.Quantity + 1 diff --git a/lib/index.js b/lib/index.js index 796172f..906c8d2 100644 --- a/lib/index.js +++ b/lib/index.js @@ -8,18 +8,19 @@ const servePrivateContentEnabled = (inputs) => return origin && origin.private === true }) -const unique = (value, index, self) => self.indexOf(value) === index const updateBucketsPolicies = async (s3, origins, s3CanonicalUserId) => { // update bucket policies with cloudfront access - const bucketNames = origins.Items.filter((origin) => origin.S3OriginConfig) - .map((origin) => { + const bucketNames = new Set( + origins.Items.filter((origin) => origin.S3OriginConfig).map((origin) => { // remove path from bucketname if origin had pathname return origin.Id.split('/')[0] }) - .filter(unique) + ) return Promise.all( - bucketNames.map((bucketName) => grantCloudFrontBucketAccess(s3, bucketName, s3CanonicalUserId)) + [...bucketNames].map((bucketName) => + grantCloudFrontBucketAccess(s3, bucketName, s3CanonicalUserId) + ) ) }