aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.36k stars 3.77k forks source link

aws_servicediscovery: PublicDnsNamespace NS records #20510

Open bhsp opened 2 years ago

bhsp commented 2 years ago

Describe the bug

When creating a public Service Discovery namespace, the nameserver records are not added to the hosted zone. There doesn't appear to be a way to get the hosted zone ID created from the public Service Discovery namespace. For the example below, NS records are not added to the hosted zone for "sub2.sub1.mydomain.com". A hosted zone is added for "sub3.sub2.sub1.mydomain.com" but the NS records from this HZ are not linked to the HZ for "sub2.sub1.mydomain.com".

Expected Behavior

NS record(s) for sub3.sub2.sub1.mydomain.com is added to HZ for sub2.sub1.mydomain.com

Current Behavior

NS records not added, public Service Discovery namespace fails DNS look-ups

Reproduction Steps


#!/usr/bin/env python3
from constructs import Construct
from aws_cdk import App, Environment, Stack, CfnOutput
from aws_cdk import (aws_servicediscovery as sd)

class SampleStack(Stack):
    def __init__(self, scope: Construct, id: str, **kwargs) -> None:
        super().__init__(scope, id, **kwargs)

        # This FQDN is a hosted zone in the target account
        fqdn = "sub2.sub1.mydomain.com"
        service_discovery_subdomain = "sub3"

        # Does not add NS records to hosted zone $fqdn
        service_discovery = sd.PublicDnsNamespace(
            self,
            "service-discovery",
            name=f"{service_discovery_subdomain}.{fqdn}")

        CfnOutput(
            self,
            "service-discovery-namespace-name",
            value=service_discovery.namespace_name,
            export_name="SDNSName")

env = Environment(account="111111111111", region='us-east-1')
app = App()

lambda_stack = SampleStack(app, "sample-stack", env=env)

app.synth()

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.25.0 (build ae1cb4b)

Framework Version

No response

Node.js Version

v14.19.1

OS

Windows

Language

Python

Language Version

No response

Other information

No response

peterwoodworth commented 2 years ago

I've created a PR which allows you to access the created hosted zone id from the namespace construct.

Otherwise - any functionality about adding record sub3.sub2.sub1.mydomain.com record to HZ for sub2.sub1.mydomain.com I'm unaware of. Our PublicDnsNamespace construct pretty much just creates the same named CloudFormation resource - are you expecting this to happen just when you create a Cloudformation PublicDnsNamespace?

github-actions[bot] commented 2 years ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

bhsp commented 2 years ago

@peterwoodworth This is great! Thank you. Given if/when the PR is merged, can I use the created hosted zone id (namespace_hosted_zone_id) from the namespace construct in the same stack to retrieve the Name Server records and then create the NS record in the parent hosted zone? Something like, POC addition to my Python previously submitted.

dns.NsRecord(
            self,
            "ns",
            zone=dns.PublicHostedZone.from_lookup(
                 self,
                 "parent-hosted-zone",
                 domain_name=fqdn
            ),
            record_name=service_discovery.namespace_name,
            values=dns.PublicHostedZone.from_public_hosted_zone_id(
                self,
                id="sd-hosted-zone",
                public_hosted_zone_id=service_discovery.namespace_hosted_zone_id).hosted_zone_name_servers,
            ttl=Duration.seconds(60)
        )
peterwoodworth commented 2 years ago

Not quite, but close - the hosted_zone_name_servers prop will be undefined if your hosted zone is imported. You can instead directly pass in the new namespace_hosted_zone_id prop to the NsRecord.values prop

bhsp commented 2 years ago

@peterwoodworth I'm not seeing where/how that's an option from the documentation, if I'm understanding your correction. I see that values accepts "(Sequence[str]) – The NS values." to my understanding values=['n1.example.com', 'n2.example.com']

Is my example below correct from your last message?


dns.NsRecord(
            self,
            "ns",
            zone=dns.PublicHostedZone.from_lookup(
                 self,
                 "parent-hosted-zone",
                 domain_name=fqdn
            ),
            record_name=service_discovery.namespace_name,
            values=service_discovery.namespace_hosted_zone_id,
            ttl=Duration.seconds(60)
        )
peterwoodworth commented 2 years ago

namespaceHostedZoneId will return a string! So I would expect you to be able to put it in an array of strings

values=[service_discovery.namespace_hosted_zone_id, ...]

bhsp commented 2 years ago

@peterwoodworth Ah yes, I meant to use as a list above, I didn't realize CFN would see a hosted_zone_id in place of name server records and look-up said records auto-magically given the hosted_zone_id in-place of the name servers. Neat.

peterwoodworth commented 2 years ago

Ack, I spaced pretty bad here, I thought you were describing something slightly different. No, CloudFormation will not magically do that unfortunately. You want to use the the name

I'm not sure what the path forward is for you here is. The way we do this for our regular HostedZone construct is that we leverage CloudFormation's attribute on their HostedZone (which doesn't apply here of course). We currently don't provide any other way to look this up to my knowledge

peterwoodworth commented 2 years ago

You could use a custom resource to get the resource records associated with the hosted zone using a custom resource and the LIstResourceRecordSets API call

bhsp commented 2 years ago

No worries, kind of didn't seem to be the case, but I went with it :-). Truly appreciate the help + insight. I have a side channel Python Boto3 script that looks everything up after the formation event and adds the record to the parent hosted zone. Having it all in the CDK would be brilliant, oh well.

Just noticed another response from you while typing this. I do have a footnote in my script to move it to a custom resource Boto3 Lambda, I simply haven't experimented w/ custom resources yet.

Still - providing the namespace_hosted_zone_id would provide a tighter lookup, rather than matching on FQDN to find the HostedZone and then finding it's name records.

peterwoodworth commented 2 years ago

We should be supporting the attribute by next release 🙂

bhsp commented 2 years ago

Awesome, very excited to incorporate. Thank you