feat(appmesh): allow setting the DnsResponseType in DNS ServiceDiscov…

…ery (#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*

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)],
     });

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,
 });
 ```
-

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

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 {

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<CfnVirtualNode.BackendProperty>();
   private readonly listeners = new Array<VirtualNodeListenerConfig>();
 
@@ -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;
+}

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"
             }
           }
         },

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,

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) {