feat(aws-route53): cross account DNS delegations

[ayush987goyal]

feat(aws-route53): cross account DNS delegations

closes #8776

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[ayush987goyal]

Added some tests for the handler but seems like this modules is not migrated to jest yet. Need to see how to make it compatible to jest.

EDIT: Adde usage to nodeunit-shim to migrate tests to jest. Had to skip one test because it was failing from before but was never got caught since it was written in jest.

[njlynch]

Thanks for this PR! Great stuff. I've left a first wave of comments.

Pull request has been modified.

[ayush987goyal]

Hi @njlynch , I have implemented all the suggestions and this PR is ready for the second round.

[njlynch]

Looks great, thanks for the updates!

One minor typo, and one question about one of the tests you fixed (thanks again for the Jest update, btw!)

[ayush987goyal]

@njlynch Addressed the comments.

[njlynch]

Awesome, thanks!

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 4b7fd59
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[OperationalFallacy]

@ayush987goyal you rock!
I wanted to try it, but it doesn't look like it is in the releases yet? Does cdk have nightly builds or something like that?

[ayush987goyal]

@OperationalFallacy Yeah I think this will be release as part of next cut. You might have to wait for it unfortunately :(

[OperationalFallacy]

Hey @ayush987goyal, can this implementation work with cdk-pipeline?
The resources in readme example are created in different accounts and then passed around, but that's not what cdk pipeline can do - it goes and deploys a synth stack in whatever account you tell it to.

https://github.com/aws/aws-cdk/pull/12680/files#diff-8cab009a3f46f19bf8281bf63ebeeeda8a204a2c7a2980ec672f5bb7819795cbR117

What I see the problem of passing NS servers from "sub-zone" account to "parent zone" account now became a problem of passing tld zone id and crossAccountZoneDelegationPrincipal between accounts. Which conceptually is the same problem.

Also, for the delegation role, the trust policy allows "sub-zone" root principal (anybody?) make changes in tld zone? That should be locked to a specific role, but again that goes back to original problem: how to pass something between accounts created in independent stacks.

[ayush987goyal]

Hi @OperationalFallacy ,

The usage of this construct should not actually involve passing the zoneId and role around but to import them using IDs. Dealing with cross account resources has some challenges around with it and for this one you will have to hard code the zoneId and roleArn from the parent account so that they can be imported like so:

const delegationRole = iam.Role.fromRoleArn(this, 'Role', roleArn);

new route53.CrossAccountZoneDelegationRecord(this, 'delegate', {
  delegatedZone: subZone,
  parentHostedZoneId: hostedZoneId,
  delegationRole: delegationRole
});

@njlynch Please add anything else if I missed out or do let me know if we should include such examples in the readme.

[OperationalFallacy]

Ok, I managed to run it in the pipeline
https://github.com/OperationalFallacy/CdkRouter53/pull/1/files#diff-28e171489876432b7aa1c09d7d1a446d777028d46e763081600b19ead74c9111R12

Imho it is not production ready. The role assume permissions can't be locked to specific entity, only to account root principals.

Another problem is lookups in the pipeline, you're right the zone ID has to be hardcoded since PublicHostedZone.fromLookup would return DUMMY (see #9922)

Putting aside all these technical details for this implementation, I wonder why Cloudformation team can't implement cross-account, cross-region exports so CDK can do what it knows the best?

[McDoit]

Nice work @ayush987goyal !
Didn't see this PR when I started on mine with #13916
But for sure great synergy there