fix: route53 CrossAccountZoneDelegationRecord fails at deployment time with imported delegatedZone
#30440
Conversation
github-actions
bot
added
bug
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
samson-keung
changed the title
delegatedZonedelegatedZone
|
Exemption Request Reason: Added build time validation. No changes to deployment behaviour, hence, no integ test update |
aaythapa
added
the
pr-linter/exempt-integ-test
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…e with imported `delegatedZone` (aws#30440) ### Issue # (if applicable) Closes aws#28581. ### Reason for this change An imported `delegatedZone` will not have info about the Name Servers. When it is passed to `CrossAccountZoneDelegationRecord`, the handler will see `undefined` when trying to retrieve the Name Servers info on `delegatedZone`, then throw exception during deployment. This change throws the exception at build time for a faster feedback loop. ### Description of changes `CrossAccountZoneDelegationRecord` throws exception if `delegatedZone.hostedZoneNameServers` is undefined. ### Description of how you validated changes Add unit test to cover the case of passing an imported HostedZone to `CrossAccountZoneDelegationRecord` ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…e with imported `delegatedZone` (aws#30440) ### Issue # (if applicable) Closes aws#28581. ### Reason for this change An imported `delegatedZone` will not have info about the Name Servers. When it is passed to `CrossAccountZoneDelegationRecord`, the handler will see `undefined` when trying to retrieve the Name Servers info on `delegatedZone`, then throw exception during deployment. This change throws the exception at build time for a faster feedback loop. ### Description of changes `CrossAccountZoneDelegationRecord` throws exception if `delegatedZone.hostedZoneNameServers` is undefined. ### Description of how you validated changes Add unit test to cover the case of passing an imported HostedZone to `CrossAccountZoneDelegationRecord` ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
This implementation is problematic for us. We are using the imported delegatedZone method, and since the nameservers are missing we are fetching them separately using a customResource and then by using the Is it possible to add an attribute which would bypass the |
Thanks for reporting the issue. I have created #30600 to track and will be pushing a fix to replace the error with a warning instead. |
…time with imported `delegatedZone` (#30440)" (#30606) This reverts commit a3d9b10. ### Issue # (if applicable) Closes #30600. ### Reason for this change Reverting #30440 so that `cdk synth` succeed when using imported `delegatedZone` ### Description of changes Removed the `throw new Error()` ### Description of how you validated changes ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…time with imported `delegatedZone` (#30440)" (#30606) This reverts commit a3d9b10. ### Issue # (if applicable) Closes #30600. ### Reason for this change Reverting #30440 so that `cdk synth` succeed when using imported `delegatedZone` ### Description of changes Removed the `throw new Error()` ### Description of how you validated changes ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…e with imported `delegatedZone` (aws#30440) ### Issue # (if applicable) Closes aws#28581. ### Reason for this change An imported `delegatedZone` will not have info about the Name Servers. When it is passed to `CrossAccountZoneDelegationRecord`, the handler will see `undefined` when trying to retrieve the Name Servers info on `delegatedZone`, then throw exception during deployment. This change throws the exception at build time for a faster feedback loop. ### Description of changes `CrossAccountZoneDelegationRecord` throws exception if `delegatedZone.hostedZoneNameServers` is undefined. ### Description of how you validated changes Add unit test to cover the case of passing an imported HostedZone to `CrossAccountZoneDelegationRecord` ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one. |
Issue # (if applicable)
Closes #28581.
Reason for this change
An imported
delegatedZonewill not have info about the Name Servers. When it is passed toCrossAccountZoneDelegationRecord, the handler will seeundefinedwhen trying to retrieve the Name Servers info ondelegatedZone, then throw exception during deployment.This change throws the exception at build time for a faster feedback loop.
Description of changes
CrossAccountZoneDelegationRecordthrows exception ifdelegatedZone.hostedZoneNameServersis undefined.Description of how you validated changes
Add unit test to cover the case of passing an imported HostedZone to
CrossAccountZoneDelegationRecordChecklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license