Skip to content
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

[Route53]: CrossAccountZoneDelegationRecord should support RemovalPolicy #15211

Closed
2 tasks
epheat opened this issue Jun 19, 2021 · 3 comments · Fixed by #15782
Closed
2 tasks

[Route53]: CrossAccountZoneDelegationRecord should support RemovalPolicy #15211

epheat opened this issue Jun 19, 2021 · 3 comments · Fixed by #15782
Labels
@aws-cdk/aws-route53 Related to Amazon Route 53 effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1

Comments

@epheat
Copy link

epheat commented Jun 19, 2021

CrossAccountZoneDelegationRecord currently does not support the application of a RemovalPolicy. When the resource is deleted, it will assume the DelegationRole and issue a ChangeResourceRecordSets call with action DELETE. In our case, we would rather leave the delegation intact (at least in production-like environments) so we don't risk tearing down our endpoints accidentally.

Use Case

We would like to apply a removal policy of RETAIN on the CrossAccountZoneDelegationRecord resource, in order to protect against accidental deletion of the resource.

Proposed Solution

let hostedZone = new HostedZone(this, 'HostedZone', {
  zoneName: 'my.cool.zone'
});

let delegation = new CrossAccountZoneDelegationRecord(this, 'CrossAccountDelegation', {
  delegatedZone: hostedZone,
  parentHostedZoneName: 'cool.zone',
  delegationRole: Role.fromRoleArn(this, 'DelegationRole', 'arn:aws:iam::000000000000:role/delegator')
});

// vvv NEW vvv
delegation.applyRemovalPolicy(RemovalPolicy.RETAIN);

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@epheat epheat added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 19, 2021
@github-actions github-actions bot added the @aws-cdk/aws-route53 Related to Amazon Route 53 label Jun 19, 2021
@epheat
Copy link
Author

epheat commented Jun 19, 2021

Alternatively, the CrossAccountZoneDelegationRecord could somehow inherit the RemovalPolicy from the HostedZone that it delegates to. It would be an antipattern to have a HostedZone with RemovalPolicy.DESTROY, meanwhile a Delegation with RemovalPolicy.RETAIN, since that would leave you with a dangling delegation.

@njlynch njlynch added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jun 23, 2021
@njlynch
Copy link
Contributor

njlynch commented Jun 23, 2021

Agreed that this construct should expose a removalPolicy, and potentially default to the removal policy of the delegatedZone.

I've tagged it as P1, which means it should be on our near-term roadmap.

We welcome community contributions! If you are able, we encourage you to contribute a bug fix or new feature to the CDK. If you decide to contribute, please start an engineering discussion in this issue to ensure there is a commonly understood design before submitting code. This will minimize the number of review cycles and get your code merged faster.

@njlynch njlynch removed their assignment Jun 23, 2021
@mergify mergify bot closed this as completed in #15782 Jul 30, 2021
mergify bot pushed a commit that referenced this issue Jul 30, 2021
…ationRecord (#15782)

add support for RemovalPolicy in CrossAccountZoneDelegationRecord

closes #15211

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Aug 3, 2021
…ationRecord (aws#15782)

add support for RemovalPolicy in CrossAccountZoneDelegationRecord

closes aws#15211

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…ationRecord (aws#15782)

add support for RemovalPolicy in CrossAccountZoneDelegationRecord

closes aws#15211

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
david-doyle-as24 pushed a commit to david-doyle-as24/aws-cdk that referenced this issue Sep 7, 2021
…ationRecord (aws#15782)

add support for RemovalPolicy in CrossAccountZoneDelegationRecord

closes aws#15211

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-route53 Related to Amazon Route 53 effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants