-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
aws-cdk-lib: CrossAccountZoneDelegationRecord does not validate hostedZoneNameServers #28581
Closed
Labels
aws-cdk-lib
Related to the aws-cdk-lib package
bug
This issue is a bug.
effort/medium
Medium work item – several days of effort
p1
Comments
u873838
added
bug
This issue is a bug.
needs-triage
This issue or PR still needs to be triaged.
labels
Jan 4, 2024
Thank you for the report. Yes it should throw out an error and the assertion might need more consideration. |
pahud
added
p1
effort/medium
Medium work item – several days of effort
and removed
needs-triage
This issue or PR still needs to be triaged.
labels
Jan 5, 2024
1 task
mergify bot
pushed a commit
that referenced
this issue
Jun 4, 2024
…e with imported `delegatedZone` (#30440) ### Issue # (if applicable) Closes #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*
|
vdahlberg
pushed a commit
to vdahlberg/aws-cdk
that referenced
this issue
Jun 10, 2024
…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*
Leo10Gama
pushed a commit
to Leo10Gama/aws-cdk
that referenced
this issue
Jun 11, 2024
…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*
mazyu36
pushed a commit
to mazyu36/aws-cdk
that referenced
this issue
Jun 22, 2024
…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 was referenced Jun 29, 2024
This was referenced Jul 12, 2024
This was referenced Jul 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
aws-cdk-lib
Related to the aws-cdk-lib package
bug
This issue is a bug.
effort/medium
Medium work item – several days of effort
p1
Describe the bug
Our deployment failed when trying to use
CrossAccountZoneDelegationRecord
with anIHostedZone
that did not havehostedZoneNameServers
set. In our particular case,IHostedZone
came fromPublicHostedZone.fromHostedZoneAttributes
.Expected Behavior
Either an error should be thrown during the CDK build process, or the construct should support an
IHostedZone
without unset.Current Behavior
We received the error during our CF deployment:
Reproduction Steps
Our internal CDK code looked something like:
Possible Solution
This line forcefully asserts that
hostedZoneNameServers
is defined:aws-cdk/packages/aws-cdk-lib/aws-route53/lib/record-set.ts
Line 793 in a40f2ec
Then the handler reads from that possibly undefined array:
aws-cdk/packages/aws-cdk-lib/aws-route53/lib/cross-account-zone-delegation-handler/index.ts
Line 58 in d69c51a
Additional Information/Context
No response
CDK CLI Version
2.77.0
Framework Version
No response
Node.js Version
v18.12.1
OS
Ubuntu 22.04.3
Language
TypeScript
Language Version
No response
Other information
We are an internal AWS team using SuperNova.
The text was updated successfully, but these errors were encountered: