feat(aws-cloudfront-mediastore): new aws-cloudfront-mediastore pattern implementation #107
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| import * as s3 from '@aws-cdk/aws-s3'; | ||
| import * as defaults from '@aws-solutions-constructs/core'; | ||
| import { Construct, Aws} from '@aws-cdk/core'; | ||
| import { MediaStoreContainer } from '../../core/lib/mediastore-helper'; |
There was a problem hiding this comment.
Please do not reference core by relative path, instead reference it from defaults.MediaStoreContainer which will require it to be exported to core/index.ts
There was a problem hiding this comment.
Oops, it was my fault. I forgot to change that one after testing. Fixed this one.
| } else { | ||
| this.cloudFrontOriginAccessIdentity = defaults.CloudFrontOriginAccessIdentity(scope); | ||
|
|
||
| mediaStoreProps = { |
There was a problem hiding this comment.
Are theses the default props for mediastore.CfnContainerProps, how is it different from what's in core/lib/mediastore-defaults.ts ?
There was a problem hiding this comment.
The default props in core/lib/mediastore-defaults.ts doesn't have aws:UserAgent in the policy. aws:UserAgent is going to be used to only allow if the User-Agent header matches. To make the core/lib/mediastore-defaults.ts flexible, I didn't put the aws:UserAgent policy by default.
| * @default - Default props are used | ||
| */ | ||
| readonly cloudFrontDistributionProps?: cloudfront.DistributionProps | any; | ||
| } |
There was a problem hiding this comment.
Please add a flag to enable/disable the automatic injection of best practice HTTP security headers in all responses from cloudfront
/**
* Optional user provided props to turn on/off the automatic injection of best practice HTTP
* security headers in all responses from cloudfront
*
* @default - true
*/
readonly insertHttpSecurityHeaders?: boolean;
There was a problem hiding this comment.
Added insertHttpSecurityHeader.
| exposeHeaders: [ '*' ], | ||
| maxAgeSeconds: 3000 | ||
| }], | ||
| policy: JSON.stringify({ |
There was a problem hiding this comment.
Does this imply that it's accessible publicly over HTTPS -or- just the Cloudfront Origin?
There was a problem hiding this comment.
In case there would be more pattern with MediaStore, by default, it's available accessible publicly over HTTPS. Link. For the new pattern, it will only allow the specific CloudFront with the User-Agent header by default.
| }, | ||
| action: 'EXPIRE', | ||
| }] | ||
| }) |
There was a problem hiding this comment.
Should there a default Metrics policy for MediaStore to send the CW metrics as described here ?
There was a problem hiding this comment.
That's a good point. Let me see what would be the recommended default policy for the Metrics policy.
There was a problem hiding this comment.
Added the default MetricPolicy.
| { numeric: ['>', 30] } | ||
| ] | ||
| }, | ||
| action: 'EXPIRE', |
There was a problem hiding this comment.
Suggest 'ARCHIVE' instead of 'EXPIRE' in line with the S3 constructs lifecycle policy that transitions the non-current versions to Glacier, for reference see here
| new origins.HttpOrigin(mediaStoreContainerDomainName); | ||
|
|
||
| return { | ||
| defaultBehavior: { |
There was a problem hiding this comment.
Missing edgeLambdas for reference see here
There was a problem hiding this comment.
Added edgeLambdas.
| && cloudFrontDistributionProps.defaultBehavior.originRequestPolicy) { | ||
| originRequestPolicy = cloudFrontDistributionProps.defaultBehavior.originRequestPolicy; | ||
| } else { | ||
| const originRequestPolicyProps: cloudfront.OriginRequestPolicyProps = { |
There was a problem hiding this comment.
Is there any reference on how did you come up with this origin request policy? Wonder if it makes sense to apply for the other aws-cloudfront-* constructs as well ?
There was a problem hiding this comment.
Oh, for the existing solution we have is using the legacy setting for the default cache behavior, but CDK doesn't support the legacy way. To mitigate it, I've put the origin request policy instead.
2) Added HTTP security header prop for CloudFront 3) Changed scope to this to create resources in the pattern 4) Updated the unit tests
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
… the linting rule
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thanks for the contribution, the updates will be pushed out in v1.77.0 release |
Issue #, if available: #104
Description of changes:
This PR introduces a new pattern which can be used by live streaming solutions. The pattern creates an Amazon CloudFront distribution whose origin is an AWS Elemental MediaStore container.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.