Skip to content

Commit

Permalink
fix(servicediscovery): allow to register multiple instances on a serv…
Browse files Browse the repository at this point in the history
…ice (#2207)
  • Loading branch information
jogold authored and rix0rrr committed Apr 10, 2019
1 parent d46b814 commit 9f88696
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 35 deletions.
25 changes: 9 additions & 16 deletions packages/@aws-cdk/aws-servicediscovery/lib/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,37 +262,30 @@ export class Service extends cdk.Construct implements IService {
/**
* Registers a resource that is accessible using values other than an IP address or a domain name (CNAME).
*/
public registerNonIpInstance(props: NonIpInstanceBaseProps): IInstance {
return new NonIpInstance(this, "NonIpInstance", {
public registerNonIpInstance(id: string, props: NonIpInstanceBaseProps): IInstance {
return new NonIpInstance(this, id, {
service: this,
instanceId: props.instanceId,
customAttributes: props.customAttributes
...props
});
}

/**
* Registers a resource that is accessible using an IP address.
*/
public registerIpInstance(props: IpInstanceBaseProps): IInstance {
return new IpInstance(this, "IpInstance", {
public registerIpInstance(id: string, props: IpInstanceBaseProps): IInstance {
return new IpInstance(this, id, {
service: this,
instanceId: props.instanceId,
ipv4: props.ipv4,
ipv6: props.ipv6,
port: props.port,
customAttributes: props.customAttributes
...props
});
}

/**
* Registers a resource that is accessible using a CNAME.
*/
public registerCnameInstance(props: CnameInstanceBaseProps): IInstance {
return new CnameInstance(this, "CnameInstance", {
public registerCnameInstance(id: string, props: CnameInstanceBaseProps): IInstance {
return new CnameInstance(this, id, {
service: this,
instanceId: props.instanceId,
instanceCname: props.instanceCname,
customAttributes: props.customAttributes
...props
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const service = namespace.createService('Service', {
dnsTtlSec: 30
});

service.registerCnameInstance({
service.registerCnameInstance('CnameInstance', {
instanceCname: 'service.pizza',
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const service1 = namespace.createService('NonIpService', {
description: 'service registering non-ip instances',
});

service1.registerNonIpInstance({
service1.registerNonIpInstance('NonIpInstance', {
customAttributes: { arn: 'arn:aws:s3:::mybucket' }
});

Expand All @@ -24,7 +24,7 @@ const service2 = namespace.createService('IpService', {
}
});

service2.registerIpInstance({
service2.registerIpInstance('IpInstance', {
ipv4: '54.239.25.192',
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const service = namespace.createService('Service', {
}
});

service.registerIpInstance({
service.registerIpInstance('IpInstance', {
ipv4: '54.239.25.192',
port: 443
});
Expand Down
55 changes: 40 additions & 15 deletions packages/@aws-cdk/aws-servicediscovery/test/test.instance.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource } from '@aws-cdk/assert';
import { countResources, expect, haveResource } from '@aws-cdk/assert';
import ec2 = require('@aws-cdk/aws-ec2');
import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2');
import cdk = require('@aws-cdk/cdk');
Expand All @@ -18,7 +18,7 @@ export = {
name: 'service',
});

service.registerIpInstance({
service.registerIpInstance('IpInstance', {
ipv4: '10.0.0.0',
ipv6: '0:0:0:0:0:ffff:a00:0',
port: 443
Expand Down Expand Up @@ -56,7 +56,7 @@ export = {
dnsRecordType: servicediscovery.DnsRecordType.A_AAAA
});

service.registerIpInstance({
service.registerIpInstance('IpInstance', {
ipv4: '54.239.25.192',
ipv6: '0:0:0:0:0:ffff:a00:0',
port: 443
Expand Down Expand Up @@ -96,7 +96,7 @@ export = {
dnsRecordType: servicediscovery.DnsRecordType.A_AAAA
});

service.registerIpInstance({
service.registerIpInstance('IpInstance', {
ipv4: '10.0.0.0',
ipv6: '0:0:0:0:0:ffff:a00:0',
port: 443
Expand Down Expand Up @@ -136,7 +136,7 @@ export = {

// THEN
test.throws(() => {
service.registerIpInstance({
service.registerIpInstance('IpInstance', {
instanceId: 'id',
});
}, /A `port` must be specified for a service using a `SRV` record./);
Expand All @@ -159,7 +159,7 @@ export = {

// THEN
test.throws(() => {
service.registerIpInstance({
service.registerIpInstance('IpInstance', {
port: 3306
});
}, /At least `ipv4` or `ipv6` must be specified for a service using a `SRV` record./);
Expand All @@ -182,7 +182,7 @@ export = {

// THEN
test.throws(() => {
service.registerIpInstance({
service.registerIpInstance('IpInstance', {
port: 3306
});
}, /An `ipv4` must be specified for a service using a `A` record./);
Expand All @@ -205,7 +205,7 @@ export = {

// THEN
test.throws(() => {
service.registerIpInstance({
service.registerIpInstance('IpInstance', {
port: 3306
});
}, /An `ipv6` must be specified for a service using a `AAAA` record./);
Expand All @@ -228,7 +228,7 @@ export = {

// THEN
test.throws(() => {
service.registerIpInstance({
service.registerIpInstance('IpInstance', {
port: 3306
});
}, /Service must support `A`, `AAAA` or `SRV` records to register this instance type./);
Expand Down Expand Up @@ -338,7 +338,7 @@ export = {
dnsRecordType: servicediscovery.DnsRecordType.CNAME
});

service.registerCnameInstance({
service.registerCnameInstance('CnameInstance', {
instanceCname: 'foo.com',
customAttributes: { dogs: 'good' }
});
Expand Down Expand Up @@ -375,7 +375,7 @@ export = {

// THEN
test.throws(() => {
service.registerCnameInstance({
service.registerCnameInstance('CnameInstance', {
instanceCname: 'foo.com',
});
}, /Namespace associated with Service must be a DNS Namespace/);
Expand All @@ -393,7 +393,7 @@ export = {

const service = namespace.createService('MyService');

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

Expand Down Expand Up @@ -426,7 +426,7 @@ export = {

// THEN
test.throws(() => {
service.registerNonIpInstance({
service.registerNonIpInstance('NonIpInstance', {
instanceId: 'nonIp',
});
}, /This type of instance can only be registered for HTTP namespaces./);
Expand All @@ -446,7 +446,7 @@ export = {

// THEN
test.throws(() => {
service.registerNonIpInstance({
service.registerNonIpInstance('NonIpInstance', {
instanceId: 'nonIp',
});
}, /You must specify at least one custom attribute for this instance type./);
Expand All @@ -466,12 +466,37 @@ export = {

// THEN
test.throws(() => {
service.registerNonIpInstance({
service.registerNonIpInstance('NonIpInstance', {
instanceId: 'nonIp',
customAttributes: {}
});
}, /You must specify at least one custom attribute for this instance type./);

test.done();
},

'Register multiple instances on the same service'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

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

const service = namespace.createService('MyService');

// WHEN
service.registerIpInstance('First', {
ipv4: '10.0.0.0'
});

service.registerIpInstance('Second', {
ipv4: '10.0.0.1'
});

// THEN
expect(stack).to(countResources('AWS::ServiceDiscovery::Instance', 2));

test.done();
}
};

0 comments on commit 9f88696

Please sign in to comment.