feat: add split horizon dns construct #37
Conversation
IamFlowZ
changed the title
IamFlowZ
changed the title
src/aws-route53/split-horizon-dns.ts
Outdated
| } else if (existingPrivateZone) { | ||
| this.privateZone = existingPrivateZone; | ||
| } else { | ||
| this.privateZone = new route53.HostedZone(this, 'PrivateZone', { |
There was a problem hiding this comment.
Please extract the creation of sub-constructs into a factory method that can be overridden in a subclass
src/aws-route53/split-horizon-dns.ts
Outdated
| } | ||
|
|
||
| if (includeCertificate) { | ||
| this.certificate = new acm.Certificate(this, 'Certificate', { |
There was a problem hiding this comment.
Please extract the creation of sub-constructs into a factory method that can be overridden in a subclass
src/aws-route53/split-horizon-dns.ts
Outdated
| if (existingPublicZone) { | ||
| this.publicZone = existingPublicZone; | ||
| } else { | ||
| this.publicZone = new route53.HostedZone(this, 'PublicZone', { |
There was a problem hiding this comment.
Please extract the creation of sub-constructs into a factory method that can be overridden in a subclass
|
@IamFlowZ are you still working on this PR? |
|
Apologies for the delay. I believe I've got those methods added, I followed the |
src/aws-route53/split-horizon-dns.ts
Outdated
|
|
||
| protected createHostedZone(zoneId: string, props: createHostedZoneProps): route53.IHostedZone { | ||
| return new route53.HostedZone(this, zoneId, { | ||
| zoneName: props.zoneName, |
There was a problem hiding this comment.
Could this just be (this, zoneId, props)?
|
|
||
| public certificate?: acm.ICertificate; | ||
|
|
||
| constructor(scope: Construct, id: string, private props: ISplitHorizonDnsProps) { |
There was a problem hiding this comment.
please make this protected so sub-classes can access this info
There was a problem hiding this comment.
I'm not sure I follow why a protected constructor is needed here?
Fixes #
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license