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

(aws-servicediscovery): DNSNamespace to allow non ip instances to be registered #21490

Closed
1 of 2 tasks
jmortlock opened this issue Aug 7, 2022 · 1 comment · Fixed by #21494
Closed
1 of 2 tasks

(aws-servicediscovery): DNSNamespace to allow non ip instances to be registered #21490

jmortlock opened this issue Aug 7, 2022 · 1 comment · Fixed by #21494
Assignees
Labels
@aws-cdk/aws-servicediscovery Related to Amazon ECS Service Discovery feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@jmortlock
Copy link
Contributor

jmortlock commented Aug 7, 2022

Describe the feature

Currently when using either public or private DNS namespaces CDK restricts the ability to register a nonIPInstance and fails with the following message

Error: This type of instance can only be registered for HTTP namespaces.

The reason for this is because CDK creates all services within this namespace as discoverable via DNS; however it is actually possible to create services within a DNS namespace which are API only discoverable, this is done by setting the CFN attribute Type to the value of HTTP.
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-servicediscovery-service.html

Escape hatches do not work in this instance as the check happens within a L2 construct; so to workaround this both service and instances need to be created using L1 constructs.

Use Case

Cloudmap with a combination of services which are discoverable via API Calls and DNS Queries or API Calls Only

Proposed Solution

Add a new attribute to DnsServiceProps to allow controlling discoverability mode.

i.e.

  namespace.createService("service", {
      discoverableVia: Discovery.HTTP_ONLY
    });
  namespace.createService("service", {
      discoverableVia: Discovery.HTTP_AND_DNS
    });

Alternative could just expose the "type" attribute directly

Other Information

No response

Acknowledgements

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

CDK version used

2.35.0

Environment details (OS name and version, etc.)

Ubuntu 20.0.4

@jmortlock jmortlock added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 7, 2022
@github-actions github-actions bot added the @aws-cdk/aws-servicediscovery Related to Amazon ECS Service Discovery label Aug 7, 2022
@mergify mergify bot closed this as completed in #21494 Aug 16, 2022
mergify bot pushed a commit that referenced this issue Aug 16, 2022
…S namespace (#21494)

----
Closes: #21490

DNS namespaces are currently setup in a way which only allow for discoverability via DNS or API, this means it is not currently possible to register non-ip instances within a DNS based namespace.

After this change you can create a DNS based service with an optional ```discoveryType``` If this ```discoveryType``` is set to ```DiscoveryType.API``` then you can register non IP based instances to this service using the ```registerNonIpInstance``` function

If no DiscoveryType is passed than the default from the namespace is used, so for an HTTP namespace this is ```API``` 
and for DNS derived namespaces this is ```DNS_AND_API```

This means DNS type namespaces can have services registered with a combination of discovery types.

----

### All Submissions:

* [✓] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [✗] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [✓] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [✓] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*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.

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
…S namespace (aws#21494)

----
Closes: aws#21490

DNS namespaces are currently setup in a way which only allow for discoverability via DNS or API, this means it is not currently possible to register non-ip instances within a DNS based namespace.

After this change you can create a DNS based service with an optional ```discoveryType``` If this ```discoveryType``` is set to ```DiscoveryType.API``` then you can register non IP based instances to this service using the ```registerNonIpInstance``` function

If no DiscoveryType is passed than the default from the namespace is used, so for an HTTP namespace this is ```API``` 
and for DNS derived namespaces this is ```DNS_AND_API```

This means DNS type namespaces can have services registered with a combination of discovery types.

----

### All Submissions:

* [✓] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [✗] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [✓] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [✓] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*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-servicediscovery Related to Amazon ECS Service Discovery feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants