feat(s3-deployment): add CloudFront invalidation to deployments #3213
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
requested a review
from 
eladb
suggested changes
Jul 23, 2019
packages/@aws-cdk/aws-s3-deployment/test/integ.bucket-deployment.ts
Outdated
Show resolved
Hide resolved
| source: s3deploy.Source.asset(path.join(__dirname, 'my-website.zip')), | ||
| destinationBucket: bucket, | ||
| distribution, | ||
| distributionPaths: ['/images/*'] |
There was a problem hiding this comment.
test what happens if "distribution" is provided but "distributionPaths" is omitted
| handler.addToRolePolicy(new PolicyStatement({ | ||
| effect: Effect.ALLOW, | ||
| actions: ['cloudfront:GetInvalidation', 'cloudfront:CreateInvalidation'], | ||
| resources: ['*'], |
There was a problem hiding this comment.
Why not restrict to this distribution ID?
There was a problem hiding this comment.
I will re-read the documentation but as I understand it there are no resource-level constraints for invalidations.
| if (props.distributionPaths && !props.distribution) { | ||
| throw new Error("Distribution must be specified if distribution paths are specified"); | ||
| } | ||
|
|
||
| const handler = new lambda.SingletonFunction(this, 'CustomResourceHandler', { |
There was a problem hiding this comment.
Would be nice to be able to validate that the cloudfront distribution has the bucket as a source origin.
Can you please raise an issue?
There was a problem hiding this comment.
It might be tricky to find all possible and valid ways. You can add S3 in many different ways. What issue should be raised and where?
There was a problem hiding this comment.
Just raise an issue that describes the desire to be able to check that a web distribution has a specific bucket origin.
eladb
approved these changes
Jul 23, 2019
|
@eladb Can we get this merged? |
eladb
pushed a commit
that referenced
this pull request
Aug 6, 2019
mergify bot
pushed a commit
that referenced
this pull request
Aug 7, 2019
* chore: update package-lock.json * feat(eks): define kubernetes resources This change allows defining arbitrary Kubernetes resources within an EKS cluster. * nice! * update readme * Update README.md * feat(events): ability to add cross-account targets (#3323) This adds the capability of adding a target to an event rule that belongs to a different account than the rule itself. Required for things like cross-account CodePipelines with source actions triggered by events. * chore(ci): add mergify config file (#3502) * chore: update jsii to 0.14.3 (#3513) * fix(iam): correctly limit the default PolicyName to 128 characters (#3487) Our logic for trimming the length of the default IAM policy name was not working, as it wasn't updated when logicalId became a Token rather than a literate string, and so it was never actually triggered (it just checked that the display name of the Token was less than 128 characters, which it always is). The fix is to resolve the logical ID Token before applying the trimming logic. Fixes #3402 * v1.3.0 (#3516) See CHANGELOG * fix: typo in restapi.ts (#3530) * feat(ecs): container dependencies (#3032) Add new addContainerDependencies method to allow for container dependencies Fixes #2490 * feat(s3-deployment): CloudFront invalidation (#3213) see #3106 * docs(core): findChild gets direct child only (#3512) * doc(iam): update references to addManagedPolicy (#3511) * fix(sqs): do not emit grants to the AWS-managed encryption key (#3169) Grants on the `alias/aws/sqs` KMS key alias are not necessary since the key will implicitly allow for it's intended usage to be fulfilled (as opposed to how you have to manage grants yourself when using a user-managed key instead). This removes the statement that was generated using an invalid resource entry. Fixes #2794 * fix(lambda): allow ArnPrincipal in grantInvoke (#3501) Fixes #3264 I'm trying to allow a lambda function in another account to be able to invoke my CDK generated lambda function. This works through the CLI like so: aws lambda add-permission --function-name=myFunction --statement-id=ABoldStatement --action=lambda:InvokeFunction --principal=arn:aws:iam::{account_id}:role/a_lambda_execution_role But CDK doesn't seem to allow me to add an ArnPrincipal doing something like this: myFunction.grantInvoke(new iam.ArnPrincipal(props.myARN)) With the error: Invalid principal type for Lambda permission statement: ArnPrincipal. Supported: AccountPrincipal, ServicePrincipal This PR allows ArnPrincipal to be passed to lambda.grantInvoke. There might be some additional validation required on the exact ARN as I believe only some ARNs are supported by lambda add-permission * chore(contrib): remove API stabilization disclaimer * fix(ssm): add GetParameters action to grantRead() (#3546) * misc * rename `KubernetesManifest` to `KubernetesResource` and `addResource` * move AWS Auth APIs to `cluster.awsAuth` and expose `AwsAuth` * remove the yaml library (we can just use a JSON stream) * add support for adding accounts to aws-auth * fix cluster deletion bug * move kubctl app info to constants * addManifest => addResource * update test expectations * add unit test for customresrouce.ref * fix sample link
5 tasks
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
see #3106
Please read the contribution guidelines and follow the pull-request checklist.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license