fix(cloudfront): OriginShield not easily disabled once enabled on an … #22334
Conversation
|
|
github-actions
bot
added
beginning-contributor
aws-cdk-automation
dismissed
their stale review
Pull Request updated. Dissmissing previous PRLinter Review.
aws-cdk-automation
dismissed
their stale review
Pull Request updated. Dissmissing previous PRLinter Review.
aws-cdk-automation
dismissed
their stale review
Pull Request updated. Dissmissing previous PRLinter Review.
…tible with a disabled originShield
mergify
bot
dismissed
aws-cdk-automation’s stale review
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Changes have been made to snapshots because the change means a value is returned where it was previously undefined. Existing integ tests should cover this scenario, please review and let me know your thoughts. Thanks! |
| @@ -194,7 +194,7 @@ export abstract class OriginBase implements IOrigin { | |||
| private renderOriginShield(originShieldRegion?: string): CfnDistribution.OriginShieldProperty | undefined { | |||
| return originShieldRegion | |||
| ? { enabled: true, originShieldRegion } | |||
| : undefined; | |||
| : { enabled: false }; | |||
There was a problem hiding this comment.
I think I would rather add a new property originShieldEnabled which the user can explicitly set to false if they want disable it.
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. |
…origin (#22791) Fixes #22233. Previous PR now closed #22334. Added new prop originShieldEnabled as suggested by @corymhall which can be set to false if the user needs to explicitly disable origin shield. Updated unit test origin.test.ts Added new integ test. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…origin
Fixes #22233. Solution provided by @s-j-white. Fix covered by existing unit test. Updated integ-distribution-snapshots so they expect originShield returns enabled: false where previously tests would ignore this field.
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license