From 647acfa3fdca6013614dfb9ebf0a2d55ea74e828 Mon Sep 17 00:00:00 2001 From: Seiya6329 Date: Tue, 6 Jul 2021 11:33:28 -0700 Subject: [PATCH] feat(appmesh): allow setting the DnsResponseType in DNS ServiceDiscovery (#15388) #### REV: - Adding new property `responseType` to DNS `ServiceDiscovery` - Changing `.cloudMap()` factory method to accept positional argument - Adding a runtime-error to check if the service discovery is defined when listener is specified BREAKING CHANGE: `ServiceDiscovery.cloudMap()` method has been changed to accept positional arguments ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/extensions/appmesh.ts | 4 +- packages/@aws-cdk/aws-appmesh/README.md | 23 +++--- .../@aws-cdk/aws-appmesh/lib/private/utils.ts | 2 +- .../aws-appmesh/lib/service-discovery.ts | 67 ++++++++++------- .../@aws-cdk/aws-appmesh/lib/virtual-node.ts | 23 ++++-- .../aws-appmesh/test/integ.mesh.expected.json | 3 +- .../@aws-cdk/aws-appmesh/test/integ.mesh.ts | 2 +- .../@aws-cdk/aws-appmesh/test/test.mesh.ts | 47 +++++++++++- .../aws-appmesh/test/test.virtual-node.ts | 75 +++++++++++++++++++ 9 files changed, 188 insertions(+), 58 deletions(-) diff --git a/packages/@aws-cdk-containers/ecs-service-extensions/lib/extensions/appmesh.ts b/packages/@aws-cdk-containers/ecs-service-extensions/lib/extensions/appmesh.ts index 5442cb4fd6d15..ccd9a2ece2bc2 100644 --- a/packages/@aws-cdk-containers/ecs-service-extensions/lib/extensions/appmesh.ts +++ b/packages/@aws-cdk-containers/ecs-service-extensions/lib/extensions/appmesh.ts @@ -285,9 +285,7 @@ export class AppMeshExtension extends ServiceExtension { mesh: this.mesh, virtualNodeName: this.parentService.id, serviceDiscovery: service.cloudMapService - ? appmesh.ServiceDiscovery.cloudMap({ - service: service.cloudMapService, - }) + ? appmesh.ServiceDiscovery.cloudMap(service.cloudMapService) : undefined, listeners: [addListener(this.protocol, containerextension.trafficPort)], }); diff --git a/packages/@aws-cdk/aws-appmesh/README.md b/packages/@aws-cdk/aws-appmesh/README.md index f4f9930f4676c..e2436bed89295 100644 --- a/packages/@aws-cdk/aws-appmesh/README.md +++ b/packages/@aws-cdk/aws-appmesh/README.md @@ -149,9 +149,7 @@ const namespace = new servicediscovery.PrivateDnsNamespace(this, 'test-namespace const service = namespace.createService('Svc'); const node = mesh.addVirtualNode('virtual-node', { - serviceDiscovery: appmesh.ServiceDiscovery.cloudMap({ - service: service, - }), + serviceDiscovery: appmesh.ServiceDiscovery.cloudMap(service), listeners: [appmesh.VirtualNodeListener.httpNodeListener({ port: 8081, healthCheck: appmesh.HealthCheck.http({ @@ -171,9 +169,7 @@ Create a `VirtualNode` with the constructor and add tags. ```ts const node = new VirtualNode(this, 'node', { mesh, - serviceDiscovery: appmesh.ServiceDiscovery.cloudMap({ - service: service, - }), + serviceDiscovery: appmesh.ServiceDiscovery.cloudMap(service), listeners: [appmesh.VirtualNodeListener.http({ port: 8080, healthCheck: appmesh.HealthCheck.http({ @@ -205,9 +201,7 @@ Create a `VirtualNode` with the constructor and add backend virtual service. ```ts const node = new VirtualNode(this, 'node', { mesh, - serviceDiscovery: appmesh.ServiceDiscovery.cloudMap({ - service: service, - }), + serviceDiscovery: appmesh.ServiceDiscovery.cloudMap(service), listeners: [appmesh.VirtualNodeListener.httpNodeListener({ port: 8080, healthCheck: appmesh.HealthCheck.http({ @@ -360,9 +354,7 @@ const namespace = new servicediscovery.PrivateDnsNamespace(this, 'test-namespace const service = namespace.createService('Svc'); const node = mesh.addVirtualNode('virtual-node', { - serviceDiscovery: appmesh.ServiceDiscovery.cloudMap({ - service: service, - }), + serviceDiscovery: appmesh.ServiceDiscovery.cloudMap(service), outlierDetection: { baseEjectionDuration: cdk.Duration.seconds(10), interval: cdk.Duration.seconds(30), @@ -381,7 +373,11 @@ connection pool properties per listener protocol types. // A Virtual Node with a gRPC listener with a connection pool set const node = new appmesh.VirtualNode(stack, 'node', { mesh, - serviceDiscovery: appmesh.ServiceDiscovery.dns('node'), + // DNS service discovery can optionally specify the DNS response type as either LOAD_BALANCER or ENDPOINTS. + // LOAD_BALANCER means that the DNS resolver returns a loadbalanced set of endpoints, + // whereas ENDPOINTS means that the DNS resolver is returning all the endpoints. + // By default, the response type is assumed to be LOAD_BALANCER + serviceDiscovery: appmesh.ServiceDiscovery.dns('node', appmesh.ResponseType.ENDPOINTS), listeners: [appmesh.VirtualNodeListener.http({ port: 80, connectionPool: { @@ -690,4 +686,3 @@ new appmesh.VirtualNode(stack, 'test-node', { mesh: sharedMesh, }); ``` - diff --git a/packages/@aws-cdk/aws-appmesh/lib/private/utils.ts b/packages/@aws-cdk/aws-appmesh/lib/private/utils.ts index ad277985250cb..daa95f18bc410 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/private/utils.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/private/utils.ts @@ -1,7 +1,7 @@ +import { Token, TokenComparison } from '@aws-cdk/core'; import { CfnVirtualNode } from '../appmesh.generated'; import { ListenerTlsOptions } from '../listener-tls-options'; import { TlsClientPolicy } from '../tls-client-policy'; -import { Token, TokenComparison } from '@aws-cdk/core'; // keep this import separate from other imports to reduce chance for merge conflicts with v2-main // eslint-disable-next-line no-duplicate-imports, import/order diff --git a/packages/@aws-cdk/aws-appmesh/lib/service-discovery.ts b/packages/@aws-cdk/aws-appmesh/lib/service-discovery.ts index 961357945a16b..013eefec5b8af 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/service-discovery.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/service-discovery.ts @@ -5,27 +5,6 @@ import { CfnVirtualNode } from './appmesh.generated'; // eslint-disable-next-line no-duplicate-imports, import/order import { Construct } from '@aws-cdk/core'; - -/** - * Represents the properties needed to define CloudMap Service Discovery - */ -export interface CloudMapServiceDiscoveryOptions { - /** - * The AWS Cloud Map Service to use for service discovery - */ - readonly service: cloudmap.IService; - - /** - * A string map that contains attributes with values that you can use to - * filter instances by any custom attribute that you specified when you - * registered the instance. Only instances that match all of the specified - * key/value pairs will be returned. - * - * @default - no instance attributes - */ - readonly instanceAttributes?: {[key: string]: string}; -} - /** * Properties for VirtualNode Service Discovery */ @@ -45,22 +24,49 @@ export interface ServiceDiscoveryConfig { readonly cloudmap?: CfnVirtualNode.AwsCloudMapServiceDiscoveryProperty; } +/** + * Enum of DNS service discovery response type + */ +export enum DnsResponseType { + /** + * DNS resolver returns a loadbalanced set of endpoints and the traffic would be sent to the given endpoints. + * It would not drain existing connections to other endpoints that are not part of this list. + */ + LOAD_BALANCER = 'LOADBALANCER', + + /** + * DNS resolver is returning all the endpoints. + * This also means that if an endpoint is missing, it would drain the current connections to the missing endpoint. + */ + ENDPOINTS = 'ENDPOINTS', +} + /** * Provides the Service Discovery method a VirtualNode uses */ export abstract class ServiceDiscovery { /** * Returns DNS based service discovery + * + * @param hostname + * @param responseType Specifies the DNS response type for the virtual node. + * The default is `DnsResponseType.LOAD_BALANCER`. */ - public static dns(hostname: string): ServiceDiscovery { - return new DnsServiceDiscovery(hostname); + public static dns(hostname: string, responseType?: DnsResponseType): ServiceDiscovery { + return new DnsServiceDiscovery(hostname, responseType); } /** * Returns Cloud Map based service discovery + * + * @param service The AWS Cloud Map Service to use for service discovery + * @param instanceAttributes A string map that contains attributes with values that you can use to + * filter instances by any custom attribute that you specified when you + * registered the instance. Only instances that match all of the specified + * key/value pairs will be returned. */ - public static cloudMap(options: CloudMapServiceDiscoveryOptions): ServiceDiscovery { - return new CloudMapServiceDiscovery(options); + public static cloudMap(service: cloudmap.IService, instanceAttributes?: {[key: string]: string}): ServiceDiscovery { + return new CloudMapServiceDiscovery(service, instanceAttributes); } /** @@ -71,16 +77,19 @@ export abstract class ServiceDiscovery { class DnsServiceDiscovery extends ServiceDiscovery { private readonly hostname: string; + private readonly responseType?: DnsResponseType; - constructor(hostname: string) { + constructor(hostname: string, responseType?: DnsResponseType) { super(); this.hostname = hostname; + this.responseType = responseType; } public bind(_scope: Construct): ServiceDiscoveryConfig { return { dns: { hostname: this.hostname, + responseType: this.responseType, }, }; } @@ -90,10 +99,10 @@ class CloudMapServiceDiscovery extends ServiceDiscovery { private readonly service: cloudmap.IService; private readonly instanceAttributes?: {[key: string]: string}; - constructor(options: CloudMapServiceDiscoveryOptions) { + constructor(service: cloudmap.IService, instanceAttributes?: {[key: string]: string}) { super(); - this.service = options.service; - this.instanceAttributes = options.instanceAttributes; + this.service = service; + this.instanceAttributes = instanceAttributes; } public bind(_scope: Construct): ServiceDiscoveryConfig { diff --git a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts index ffc56239c47f0..bba83feb0b3d1 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts @@ -4,7 +4,7 @@ import { Construct } from 'constructs'; import { CfnVirtualNode } from './appmesh.generated'; import { IMesh, Mesh } from './mesh'; import { renderMeshOwner, renderTlsClientPolicy } from './private/utils'; -import { ServiceDiscovery } from './service-discovery'; +import { ServiceDiscovery, ServiceDiscoveryConfig } from './service-discovery'; import { AccessLog, BackendDefaults, Backend } from './shared-interfaces'; import { VirtualNodeListener, VirtualNodeListenerConfig } from './virtual-node-listener'; @@ -176,6 +176,8 @@ export class VirtualNode extends VirtualNodeBase { */ public readonly mesh: IMesh; + private readonly serviceDiscoveryConfig?: ServiceDiscoveryConfig; + private readonly backends = new Array(); private readonly listeners = new Array(); @@ -185,11 +187,11 @@ export class VirtualNode extends VirtualNodeBase { }); this.mesh = props.mesh; + this.serviceDiscoveryConfig = props.serviceDiscovery?.bind(this); props.backends?.forEach(backend => this.addBackend(backend)); props.listeners?.forEach(listener => this.addListener(listener)); const accessLogging = props.accessLog?.bind(this); - const serviceDiscovery = props.serviceDiscovery?.bind(this); const node = new CfnVirtualNode(this, 'Resource', { virtualNodeName: this.physicalName, @@ -205,10 +207,7 @@ export class VirtualNode extends VirtualNodeBase { }, } : undefined, - serviceDiscovery: { - dns: serviceDiscovery?.dns, - awsCloudMap: serviceDiscovery?.cloudmap, - }, + serviceDiscovery: renderServiceDiscovery(this.serviceDiscoveryConfig), logging: accessLogging !== undefined ? { accessLog: accessLogging.virtualNodeAccessLog, } : undefined, @@ -234,6 +233,9 @@ export class VirtualNode extends VirtualNodeBase { * @see https://github.com/aws/aws-app-mesh-roadmap/issues/120 */ public addListener(listener: VirtualNodeListener) { + if (!this.serviceDiscoveryConfig) { + throw new Error('Service discovery information is required for a VirtualNode with a listener.'); + } this.listeners.push(listener.bind(this)); } @@ -259,3 +261,12 @@ export interface VirtualNodeAttributes { */ readonly mesh: IMesh; } + +function renderServiceDiscovery(config?: ServiceDiscoveryConfig): CfnVirtualNode.ServiceDiscoveryProperty | undefined { + return config + ? { + dns: config?.dns, + awsCloudMap: config?.cloudmap, + } + : undefined; +} diff --git a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json index 34333711b8488..3744fe7583cf1 100644 --- a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json +++ b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json @@ -1202,7 +1202,8 @@ }, "ServiceDiscovery": { "DNS": { - "Hostname": "node4.domain.local" + "Hostname": "node4.domain.local", + "ResponseType": "ENDPOINTS" } } }, diff --git a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts index 3520b49331f05..53a539204ef36 100644 --- a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts +++ b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts @@ -112,7 +112,7 @@ const node3 = mesh.addVirtualNode('node3', { }); const node4 = mesh.addVirtualNode('node4', { - serviceDiscovery: appmesh.ServiceDiscovery.dns(`node4.${namespace.namespaceName}`), + serviceDiscovery: appmesh.ServiceDiscovery.dns(`node4.${namespace.namespaceName}`, appmesh.DnsResponseType.ENDPOINTS), listeners: [appmesh.VirtualNodeListener.http({ tls: { mode: appmesh.TlsMode.STRICT, diff --git a/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts b/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts index dfecbc4292c88..61a51b9578431 100644 --- a/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts +++ b/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts @@ -105,9 +105,7 @@ export = { // WHEN new appmesh.VirtualNode(stack, 'test-node', { mesh, - serviceDiscovery: appmesh.ServiceDiscovery.cloudMap({ - service: service, - }), + serviceDiscovery: appmesh.ServiceDiscovery.cloudMap(service), }); // THEN @@ -125,6 +123,49 @@ export = { test.done(); }, + 'VirtualService can use CloudMap service with instanceAttributes'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const mesh = new appmesh.Mesh(stack, 'mesh', { + meshName: 'test-mesh', + }); + const vpc = new ec2.Vpc(stack, 'vpc'); + const namespace = new cloudmap.PrivateDnsNamespace(stack, 'test-namespace', { + vpc, + name: 'domain.local', + }); + const service = namespace.createService('Svc'); + + const instanceAttribute : { [key: string]: string} = {}; + instanceAttribute.testKey = 'testValue'; + + // WHEN + new appmesh.VirtualNode(stack, 'test-node', { + mesh, + serviceDiscovery: appmesh.ServiceDiscovery.cloudMap(service, instanceAttribute), + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::AppMesh::VirtualNode', { + Spec: { + ServiceDiscovery: { + AWSCloudMap: { + NamespaceName: 'domain.local', + ServiceName: { 'Fn::GetAtt': ['testnamespaceSvcB55702EC', 'Name'] }, + Attributes: [ + { + Key: 'testKey', + Value: 'testValue', + }, + ], + }, + }, + }, + })); + + test.done(); + }, + 'When adding a VirtualNode to a mesh': { 'with empty default listeners and backends': { 'should create default resource'(test: Test) { diff --git a/packages/@aws-cdk/aws-appmesh/test/test.virtual-node.ts b/packages/@aws-cdk/aws-appmesh/test/test.virtual-node.ts index 171c754f09954..19d36126380fb 100644 --- a/packages/@aws-cdk/aws-appmesh/test/test.virtual-node.ts +++ b/packages/@aws-cdk/aws-appmesh/test/test.virtual-node.ts @@ -507,6 +507,7 @@ export = { }, }, )], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -552,6 +553,7 @@ export = { certificate: appmesh.TlsCertificate.file('path/to/certChain', 'path/to/privateKey'), }, })], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -595,6 +597,7 @@ export = { certificate: appmesh.TlsCertificate.sds('secret_certificate'), }, })], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -638,6 +641,7 @@ export = { certificate: appmesh.TlsCertificate.file('path/to/certChain', 'path/to/privateKey'), }, })], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -683,6 +687,7 @@ export = { }, }), ], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -723,6 +728,7 @@ export = { }, }), ], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -762,6 +768,7 @@ export = { }, }), ], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -801,6 +808,7 @@ export = { }, }), ], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // THEN @@ -872,6 +880,7 @@ export = { certificate: appmesh.TlsCertificate.file('path/to/certChain', 'path/to/privateKey'), }, })], + serviceDiscovery: appmesh.ServiceDiscovery.dns('test'), }); // WHEN @@ -923,6 +932,72 @@ export = { test.done(); }, }, + + 'with DNS service discovery': { + 'should allow set response type'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + const mesh = new appmesh.Mesh(stack, 'mesh', { + meshName: 'test-mesh', + }); + + // WHEN + new appmesh.VirtualNode(stack, 'test-node', { + mesh, + serviceDiscovery: appmesh.ServiceDiscovery.dns('test', appmesh.DnsResponseType.LOAD_BALANCER), + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::AppMesh::VirtualNode', { + Spec: { + ServiceDiscovery: { + DNS: { + Hostname: 'test', + ResponseType: 'LOADBALANCER', + }, + }, + }, + })); + + test.done(); + }, + }, + + 'with listener and without service discovery': { + 'should throw an error'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + const mesh = new appmesh.Mesh(stack, 'mesh', { + meshName: 'test-mesh', + }); + + const node = new appmesh.VirtualNode(stack, 'test-node', { + mesh, + }); + + // WHEN + THEN + test.throws(() => { + new appmesh.VirtualNode(stack, 'test-node-2', { + mesh, + listeners: [appmesh.VirtualNodeListener.http()], + }); + }, /Service discovery information is required for a VirtualNode with a listener/); + + test.throws(() => { + mesh.addVirtualNode('test-node-3', { + listeners: [appmesh.VirtualNodeListener.http()], + }); + }, /Service discovery information is required for a VirtualNode with a listener/); + + test.throws(() => { + node.addListener(appmesh.VirtualNodeListener.http()); + }, /Service discovery information is required for a VirtualNode with a listener/); + + test.done(); + }, + }, }, };