Skip to content

Commit

Permalink
feat(servicediscovery): add support for API only services within a DN…
Browse files Browse the repository at this point in the history
…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*
  • Loading branch information
jmortlock authored Aug 16, 2022
1 parent 58101a6 commit 1920313
Show file tree
Hide file tree
Showing 13 changed files with 297 additions and 18 deletions.
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-servicediscovery/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ registers an instance to it:
The following example creates an AWS Cloud Map namespace that
supports both API calls and DNS queries within a vpc, creates a
service in that namespace, and registers a loadbalancer as an
instance:
instance.

A secondary service is also configured which only supports API based discovery, a
non ip based resource is registered to this service:

[Creating a Cloud Map service within a Private DNS namespace](test/integ.service-with-private-dns-namespace.lit.ts)

Expand Down
7 changes: 4 additions & 3 deletions packages/@aws-cdk/aws-servicediscovery/lib/non-ip-instance.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Construct } from 'constructs';
import { BaseInstanceProps, InstanceBase } from './instance';
import { NamespaceType } from './namespace';
import { IService } from './service';
import { defaultDiscoveryType } from './private/utils';
import { IService, DiscoveryType } from './service';
import { CfnInstance } from './servicediscovery.generated';

export interface NonIpInstanceBaseProps extends BaseInstanceProps {
Expand Down Expand Up @@ -37,7 +37,8 @@ export class NonIpInstance extends InstanceBase {
constructor(scope: Construct, id: string, props: NonIpInstanceProps) {
super(scope, id);

if (props.service.namespace.type !== NamespaceType.HTTP) {
const discoveryType = props.service.discoveryType || defaultDiscoveryType(props.service.namespace);
if (discoveryType !== DiscoveryType.API) {
throw new Error('This type of instance can only be registered for HTTP namespaces.');
}

Expand Down
6 changes: 6 additions & 0 deletions packages/@aws-cdk/aws-servicediscovery/lib/private/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { INamespace, NamespaceType } from '../namespace';
import { DiscoveryType } from '../service';

export function defaultDiscoveryType(namespace : INamespace): DiscoveryType {
return namespace.type == NamespaceType.HTTP ? DiscoveryType.API: DiscoveryType.DNS_AND_API;
}
68 changes: 59 additions & 9 deletions packages/@aws-cdk/aws-servicediscovery/lib/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { IInstance } from './instance';
import { IpInstance, IpInstanceBaseProps } from './ip-instance';
import { INamespace, NamespaceType } from './namespace';
import { NonIpInstance, NonIpInstanceBaseProps } from './non-ip-instance';
import { defaultDiscoveryType } from './private/utils';
import { CfnService } from './servicediscovery.generated';

export interface IService extends IResource {
Expand Down Expand Up @@ -42,6 +43,11 @@ export interface IService extends IResource {
* The Routing Policy used by the service
*/
readonly routingPolicy: RoutingPolicy;

/**
* The discovery type used by the service
*/
readonly discoveryType: DiscoveryType;
}

/**
Expand Down Expand Up @@ -87,6 +93,13 @@ export interface BaseServiceProps {
* PublicDnsNamespace
*/
export interface DnsServiceProps extends BaseServiceProps {
/**
* Controls how instances within this service can be discovered
*
* @default DNS_AND_API
*/
readonly discoveryType?: DiscoveryType;

/**
* The DNS type of the record that you want AWS Cloud Map to create. Supported record types
* include A, AAAA, A and AAAA (A_AAAA), CNAME, and SRV.
Expand Down Expand Up @@ -136,6 +149,7 @@ abstract class ServiceBase extends Resource implements IService {
public abstract dnsRecordType: DnsRecordType;
public abstract routingPolicy: RoutingPolicy;
public abstract readonly serviceName: string;
public abstract discoveryType: DiscoveryType;
}

export interface ServiceAttributes {
Expand All @@ -145,6 +159,7 @@ export interface ServiceAttributes {
readonly serviceArn: string;
readonly dnsRecordType: DnsRecordType;
readonly routingPolicy: RoutingPolicy;
readonly discoveryType?: DiscoveryType;
}

/**
Expand All @@ -160,6 +175,7 @@ export class Service extends ServiceBase {
public dnsRecordType = attrs.dnsRecordType;
public routingPolicy = attrs.routingPolicy;
public serviceName = attrs.serviceName;
public discoveryType = attrs.discoveryType || defaultDiscoveryType(attrs.namespace);
}

return new Import(scope, id);
Expand Down Expand Up @@ -195,14 +211,31 @@ export class Service extends ServiceBase {
*/
public readonly routingPolicy: RoutingPolicy;

/**
* The discovery type used by this service.
*/
public readonly discoveryType: DiscoveryType;

constructor(scope: Construct, id: string, props: ServiceProps) {
super(scope, id);

const namespaceType = props.namespace.type;
const discoveryType = props.discoveryType || defaultDiscoveryType(props.namespace);

if (namespaceType == NamespaceType.HTTP && discoveryType == DiscoveryType.DNS_AND_API) {
throw new Error(
'Cannot specify `discoveryType` of DNS_AND_API when using an HTTP namespace.',
);
}

// Validations
if (namespaceType === NamespaceType.HTTP && (props.routingPolicy || props.dnsRecordType)) {
throw new Error('Cannot specify `routingPolicy` or `dnsRecord` when using an HTTP namespace.');
if (
discoveryType === DiscoveryType.API &&
(props.routingPolicy || props.dnsRecordType)
) {
throw new Error(
'Cannot specify `routingPolicy` or `dnsRecord` when using an HTTP namespace.',
);
}

if (props.healthCheck && props.customHealthCheck) {
Expand Down Expand Up @@ -247,13 +280,14 @@ export class Service extends ServiceBase {
throw new Error('Must support `A` or `AAAA` records to register loadbalancers.');
}

const dnsConfig: CfnService.DnsConfigProperty | undefined = props.namespace.type === NamespaceType.HTTP
? undefined
: {
dnsRecords: renderDnsRecords(dnsRecordType, props.dnsTtl),
namespaceId: props.namespace.namespaceId,
routingPolicy,
};
const dnsConfig: CfnService.DnsConfigProperty | undefined =
discoveryType === DiscoveryType.API
? undefined
: {
dnsRecords: renderDnsRecords(dnsRecordType, props.dnsTtl),
namespaceId: props.namespace.namespaceId,
routingPolicy,
};

const healthCheckConfigDefaults = {
type: HealthCheckType.HTTP,
Expand All @@ -274,6 +308,7 @@ export class Service extends ServiceBase {
healthCheckConfig,
healthCheckCustomConfig,
namespaceId: props.namespace.namespaceId,
type: props.discoveryType == DiscoveryType.API ? 'HTTP' : undefined,
});

this.serviceName = service.attrName;
Expand All @@ -282,6 +317,7 @@ export class Service extends ServiceBase {
this.namespace = props.namespace;
this.dnsRecordType = dnsRecordType;
this.routingPolicy = routingPolicy;
this.discoveryType = discoveryType;
}

/**
Expand Down Expand Up @@ -384,6 +420,20 @@ export interface HealthCheckCustomConfig {
readonly failureThreshold?: number;
}

/**
* Specifies information about the discovery type of a service
*/
export enum DiscoveryType {
/**
* Instances are discoverable via API only
*/
API = 'API',
/**
* Instances are discoverable via DNS or API
*/
DNS_AND_API = 'DNS_AND_API'
}

export enum DnsRecordType {
/**
* An A record
Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-servicediscovery/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
"props-physical-name:@aws-cdk/aws-servicediscovery.IpInstanceProps",
"props-physical-name:@aws-cdk/aws-servicediscovery.NonIpInstanceProps",
"props-physical-name:@aws-cdk/aws-servicediscovery.ServiceProps",
"props-default-doc:@aws-cdk/aws-servicediscovery.ServiceAttributes.discoveryType",
"docs-public-apis:@aws-cdk/aws-servicediscovery.CnameInstanceProps",
"docs-public-apis:@aws-cdk/aws-servicediscovery.RoutingPolicy",
"docs-public-apis:@aws-cdk/aws-servicediscovery.NamespaceType",
Expand All @@ -131,6 +132,7 @@
"docs-public-apis:@aws-cdk/aws-servicediscovery.ServiceAttributes.routingPolicy",
"docs-public-apis:@aws-cdk/aws-servicediscovery.ServiceAttributes.namespace",
"docs-public-apis:@aws-cdk/aws-servicediscovery.ServiceAttributes.dnsRecordType",
"docs-public-apis:@aws-cdk/aws-servicediscovery.ServiceAttributes.discoveryType",
"docs-public-apis:@aws-cdk/aws-servicediscovery.HttpNamespace.httpNamespaceArn",
"docs-public-apis:@aws-cdk/aws-servicediscovery.HttpNamespace.httpNamespaceId",
"docs-public-apis:@aws-cdk/aws-servicediscovery.HttpNamespace.httpNamespaceName",
Expand Down
35 changes: 34 additions & 1 deletion packages/@aws-cdk/aws-servicediscovery/test/instance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,40 @@ describe('instance', () => {

});

test('Throws when registering NonIpInstance for an Public namespace', () => {
test('Register NonIpInstance, DNS Namespace, API Only service', () => {
// GIVEN
const stack = new cdk.Stack();

const namespace = new servicediscovery.PublicDnsNamespace(
stack,
'MyNamespace',
{
name: 'http',
},
);

const service = namespace.createService('MyService', { discoveryType: servicediscovery.DiscoveryType.API } );

service.registerNonIpInstance('NonIpInstance', {
customAttributes: { dogs: 'good' },
});

// THEN
Template.fromStack(stack).hasResourceProperties(
'AWS::ServiceDiscovery::Instance',
{
InstanceAttributes: {
dogs: 'good',
},
ServiceId: {
'Fn::GetAtt': ['MyNamespaceMyService365E2470', 'Id'],
},
InstanceId: 'MyNamespaceMyServiceNonIpInstance7EFD703A',
},
);
});

test('Throws when registering NonIpInstance for an DNS discoverable service', () => {
// GIVEN
const stack = new cdk.Stack();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,12 @@ const loadbalancer = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc, inter

service.registerLoadBalancer('Loadbalancer', loadbalancer);

const arnService = namespace.createService('ArnService', {
discoveryType: servicediscovery.DiscoveryType.API,
});

arnService.registerNonIpInstance('NonIpInstance', {
customAttributes: { arn: 'arn://' },
});

app.synth();
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "20.0.0",
"files": {
"d0b994983cc3cd6c314558cfe207b6dee3d75c2b30ac611f4cf4e2a657af5375": {
"source": {
"path": "aws-servicediscovery-integ.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "d0b994983cc3cd6c314558cfe207b6dee3d75c2b30ac611f4cf4e2a657af5375.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,33 @@
"InstanceId": "awsservicediscoveryintegNamespaceServiceLoadbalancerA3D252B2"
}
},
"NamespaceArnService1F427735": {
"Type": "AWS::ServiceDiscovery::Service",
"Properties": {
"NamespaceId": {
"Fn::GetAtt": [
"Namespace9B63B8C8",
"Id"
]
},
"Type": "HTTP"
}
},
"NamespaceArnServiceNonIpInstance91BB57CE": {
"Type": "AWS::ServiceDiscovery::Instance",
"Properties": {
"InstanceAttributes": {
"arn": "arn://"
},
"ServiceId": {
"Fn::GetAtt": [
"NamespaceArnService1F427735",
"Id"
]
},
"InstanceId": "awsservicediscoveryintegNamespaceArnServiceNonIpInstance80DE1A4E"
}
},
"LB8A12904C": {
"Type": "AWS::ElasticLoadBalancingV2::LoadBalancer",
"Properties": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"17.0.0"}
{"version":"20.0.0"}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "17.0.0",
"version": "20.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
Expand Down Expand Up @@ -171,6 +171,18 @@
"data": "NamespaceServiceLoadbalancerD271391A"
}
],
"/aws-servicediscovery-integ/Namespace/ArnService/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "NamespaceArnService1F427735"
}
],
"/aws-servicediscovery-integ/Namespace/ArnService/NonIpInstance/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "NamespaceArnServiceNonIpInstance91BB57CE"
}
],
"/aws-servicediscovery-integ/LB/Resource": [
{
"type": "aws:cdk:logicalId",
Expand Down
Loading

0 comments on commit 1920313

Please sign in to comment.