From e183e7994b0ffc643c98b0589a40a4bcfd7e82e5 Mon Sep 17 00:00:00 2001 From: Dominic Fezzie Date: Tue, 22 Sep 2020 21:14:08 -0700 Subject: [PATCH 1/5] feat(aws-appmesh): adds access logging configuration to virtual nodes --- .../@aws-cdk/aws-appmesh/lib/shared-interfaces.ts | 12 +++++++++++- packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts | 15 +++++++++++---- .../aws-appmesh/test/integ.mesh.expected.json | 14 -------------- packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts | 3 +++ packages/@aws-cdk/aws-appmesh/test/test.mesh.ts | 7 ------- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts b/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts index bed18a8616c03..14e03c6690521 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts @@ -92,9 +92,19 @@ export interface VirtualNodeListener { readonly portMapping?: PortMapping; /** - * Array fo HealthCheckProps for the node(s) + * Array of HealthCheckProps for the node(s) * * @default - no healthcheck */ readonly healthCheck?: HealthCheck; } + +/** + * Configuration for Envoy Access logs for mesh endpoints + */ +export interface AccessLog { + /** + * Path to a file to write access logs to + */ + readonly filePath: string; +} diff --git a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts index f7a2548a48e7b..b5be39c4c79c5 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts @@ -3,7 +3,7 @@ import * as cdk from '@aws-cdk/core'; import { CfnVirtualNode } from './appmesh.generated'; import { IMesh } from './mesh'; -import { HealthCheck, PortMapping, Protocol, VirtualNodeListener } from './shared-interfaces'; +import { AccessLog, HealthCheck, PortMapping, Protocol, VirtualNodeListener } from './shared-interfaces'; import { IVirtualService } from './virtual-service'; /** @@ -90,6 +90,13 @@ export interface VirtualNodeBaseProps { * @default - No listeners */ readonly listener?: VirtualNodeListener; + + /** + * Access Logging Configuration for the virtual node + * + * @default - No access logging + */ + readonly accessLog?: AccessLog; } /** @@ -267,13 +274,13 @@ export class VirtualNode extends VirtualNodeBase { attributes: renderAttributes(props.cloudMapServiceInstanceAttributes), } : undefined, }, - logging: { + logging: props.accessLog !== undefined ? { accessLog: { file: { - path: '/dev/stdout', + path: props.accessLog.filePath, }, }, - }, + } : 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 8b85dbbd4c515..5b1b1657f0e20 100644 --- a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json +++ b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json @@ -672,13 +672,6 @@ } } ], - "Logging": { - "AccessLog": { - "File": { - "Path": "/dev/stdout" - } - } - }, "ServiceDiscovery": { "DNS": { "Hostname": "node1.domain.local" @@ -727,13 +720,6 @@ } } ], - "Logging": { - "AccessLog": { - "File": { - "Path": "/dev/stdout" - } - } - }, "ServiceDiscovery": { "DNS": { "Hostname": "node2.domain.local" diff --git a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts index a5b4b1c30032e..c87615c441e80 100644 --- a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts +++ b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts @@ -95,6 +95,9 @@ const node3 = mesh.addVirtualNode('node3', { unhealthyThreshold: 2, }, }, + accessLog: { + filePath: '/dev/stdout', + }, }); router.addRoute('route-2', { diff --git a/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts b/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts index f3d9d1ea9e8e4..f8e6ebcbb716d 100644 --- a/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts +++ b/packages/@aws-cdk/aws-appmesh/test/test.mesh.ts @@ -276,13 +276,6 @@ export = { }, Spec: { // Specifically: no Listeners and Backends - Logging: { - AccessLog: { - File: { - Path: '/dev/stdout', - }, - }, - }, ServiceDiscovery: { DNS: { Hostname: 'test.domain.local', From 6ef47f2d1cf264666280506ca39172751e774544 Mon Sep 17 00:00:00 2001 From: Dominic Fezzie Date: Fri, 25 Sep 2020 15:23:05 -0700 Subject: [PATCH 2/5] Reworks the AcessLog class to use bind methods to future proof for mutual exclusive properties --- packages/@aws-cdk/aws-appmesh/README.md | 2 +- .../aws-appmesh/lib/shared-interfaces.ts | 58 +++++++++++++++++-- .../@aws-cdk/aws-appmesh/lib/virtual-node.ts | 9 +-- .../@aws-cdk/aws-appmesh/test/integ.mesh.ts | 4 +- 4 files changed, 59 insertions(+), 14 deletions(-) diff --git a/packages/@aws-cdk/aws-appmesh/README.md b/packages/@aws-cdk/aws-appmesh/README.md index e7478ed75438f..5df099b575d06 100644 --- a/packages/@aws-cdk/aws-appmesh/README.md +++ b/packages/@aws-cdk/aws-appmesh/README.md @@ -191,7 +191,7 @@ const node = new VirtualNode(this, 'node', { cdk.Tag.add(node, 'Environment', 'Dev'); ``` -The listeners property can be left blank dded later with the `mesh.addListeners()` method. The `healthcheck` property is optional but if specifying a listener, the `portMappings` must contain at least one property. +The listeners property can be left blank and added later with the `mesh.addListeners()` method. The `healthcheck` property is optional but if specifying a listener, the `portMappings` must contain at least one property. ## Adding a Route diff --git a/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts b/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts index 14e03c6690521..f4799cfca0533 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts @@ -1,4 +1,4 @@ -import { Duration } from '@aws-cdk/core'; +import * as cdk from '@aws-cdk/core'; /** * Enum of supported AppMesh protocols @@ -27,7 +27,7 @@ export interface HealthCheck { * * @default 5 seconds */ - readonly interval?: Duration; + readonly interval?: cdk.Duration; /** * The path where the application expects any health-checks, this can also be the application path. * @@ -52,7 +52,7 @@ export interface HealthCheck { * * @default 2 seconds */ - readonly timeout?: Duration; + readonly timeout?: cdk.Duration; /** * Number of failed attempts before considering the node DOWN. * @@ -92,19 +92,65 @@ export interface VirtualNodeListener { readonly portMapping?: PortMapping; /** - * Array of HealthCheckProps for the node(s) + * Health checking strategy upstream nodes should use when communicating with the listener * * @default - no healthcheck */ readonly healthCheck?: HealthCheck; } +/** + * All Properties for Envoy Access logs for mesh endpoints + */ +export interface AccessLogConfig { + /** + * Path to a file to write access logs to + * + * @default - no file based access logging + */ + readonly filePath?: string; +} + +/** + * Configuration for Envoy Access logs for mesh endpoints + */ +export abstract class AccessLog { + /** + * Path to a file to write access logs to + * + * @default - no file based access logging + */ + public static fromFilePath(filePath: string): AccessLog { + return new FileAccessLog(filePath); + } + + /** + * Called when the AccessLog type is initialized. Can be used to enforce + * mutual exclusivity with future properties + * + */ + public abstract bind(scope: cdk.Construct): AccessLogConfig; +} + /** * Configuration for Envoy Access logs for mesh endpoints */ -export interface AccessLog { +export class FileAccessLog extends AccessLog { /** * Path to a file to write access logs to + * + * @default - no file based access logging */ - readonly filePath: string; + public readonly filePath: string; + + constructor(filePath: string) { + super(); + this.filePath = filePath; + } + + public bind(_scope: cdk.Construct): AccessLogConfig { + return { + filePath: this.filePath, + }; + } } diff --git a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts index b5be39c4c79c5..9bd901541aab8 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts @@ -259,6 +259,7 @@ export class VirtualNode extends VirtualNodeBase { this.addBackends(...props.backends || []); this.addListeners(...props.listener ? [props.listener] : []); + const accessLogging = props.accessLog?.bind(this); const node = new CfnVirtualNode(this, 'Resource', { virtualNodeName: this.physicalName, @@ -274,11 +275,11 @@ export class VirtualNode extends VirtualNodeBase { attributes: renderAttributes(props.cloudMapServiceInstanceAttributes), } : undefined, }, - logging: props.accessLog !== undefined ? { + logging: accessLogging !== undefined ? { accessLog: { - file: { - path: props.accessLog.filePath, - }, + file: accessLogging.filePath != undefined ? { + path: accessLogging.filePath, + } : undefined, }, } : undefined, }, diff --git a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts index c87615c441e80..df55025f1365c 100644 --- a/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts +++ b/packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts @@ -95,9 +95,7 @@ const node3 = mesh.addVirtualNode('node3', { unhealthyThreshold: 2, }, }, - accessLog: { - filePath: '/dev/stdout', - }, + accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'), }); router.addRoute('route-2', { From 76f97c22424d1bc8d0b2668c6cf545600f80a757 Mon Sep 17 00:00:00 2001 From: Dominic Fezzie Date: Tue, 29 Sep 2020 14:28:25 -0700 Subject: [PATCH 3/5] Changes bind method to use the CFN property for access logging --- .../aws-appmesh/lib/shared-interfaces.ts | 17 +++++++++++------ .../@aws-cdk/aws-appmesh/lib/virtual-node.ts | 6 +----- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts b/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts index f4799cfca0533..7cd55df23565d 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts @@ -1,4 +1,5 @@ import * as cdk from '@aws-cdk/core'; +import { CfnVirtualNode } from './appmesh.generated'; /** * Enum of supported AppMesh protocols @@ -103,12 +104,13 @@ export interface VirtualNodeListener { * All Properties for Envoy Access logs for mesh endpoints */ export interface AccessLogConfig { + /** - * Path to a file to write access logs to + * VirtualNode CFN configuration for Access Logging * - * @default - no file based access logging + * @default - no access logging */ - readonly filePath?: string; + readonly virtualNodeAccessLog?: CfnVirtualNode.AccessLogProperty; } /** @@ -127,7 +129,6 @@ export abstract class AccessLog { /** * Called when the AccessLog type is initialized. Can be used to enforce * mutual exclusivity with future properties - * */ public abstract bind(scope: cdk.Construct): AccessLogConfig; } @@ -135,7 +136,7 @@ export abstract class AccessLog { /** * Configuration for Envoy Access logs for mesh endpoints */ -export class FileAccessLog extends AccessLog { +class FileAccessLog extends AccessLog { /** * Path to a file to write access logs to * @@ -150,7 +151,11 @@ export class FileAccessLog extends AccessLog { public bind(_scope: cdk.Construct): AccessLogConfig { return { - filePath: this.filePath, + virtualNodeAccessLog: { + file: { + path: this.filePath, + }, + }, }; } } diff --git a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts index 9bd901541aab8..9cfaa9d26132d 100644 --- a/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts +++ b/packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts @@ -276,11 +276,7 @@ export class VirtualNode extends VirtualNodeBase { } : undefined, }, logging: accessLogging !== undefined ? { - accessLog: { - file: accessLogging.filePath != undefined ? { - path: accessLogging.filePath, - } : undefined, - }, + accessLog: accessLogging.virtualNodeAccessLog, } : undefined, }, }); From f8782424219b414a52bffb99a1fe40a4df80dd82 Mon Sep 17 00:00:00 2001 From: Dominic Fezzie Date: Tue, 29 Sep 2020 20:54:44 -0700 Subject: [PATCH 4/5] Add use of the access log property to the README --- packages/@aws-cdk/aws-appmesh/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@aws-cdk/aws-appmesh/README.md b/packages/@aws-cdk/aws-appmesh/README.md index 5df099b575d06..1becd4cf81772 100644 --- a/packages/@aws-cdk/aws-appmesh/README.md +++ b/packages/@aws-cdk/aws-appmesh/README.md @@ -161,6 +161,7 @@ const node = mesh.addVirtualNode('virtual-node', { unhealthyThreshold: 2, }, }, + accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'), }) ``` @@ -186,6 +187,7 @@ const node = new VirtualNode(this, 'node', { unhealthyThreshold: 2, }, }, + accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'), }); cdk.Tag.add(node, 'Environment', 'Dev'); From 7788b20d68d7d0bc68b63a70dbb78b4fc9422545 Mon Sep 17 00:00:00 2001 From: Dominic Fezzie Date: Wed, 30 Sep 2020 13:49:18 -0700 Subject: [PATCH 5/5] Remove the default access logging config from the ECS patterns integ --- .../integ.all-service-addons.expected.json | 21 ------------------- .../integ.multiple-environments.expected.json | 14 ------------- 2 files changed, 35 deletions(-) diff --git a/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.all-service-addons.expected.json b/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.all-service-addons.expected.json index 6e6d796875817..6ec717eee8cbe 100644 --- a/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.all-service-addons.expected.json +++ b/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.all-service-addons.expected.json @@ -1303,13 +1303,6 @@ } } ], - "Logging": { - "AccessLog": { - "File": { - "Path": "/dev/stdout" - } - } - }, "ServiceDiscovery": { "AWSCloudMap": { "NamespaceName": "production", @@ -2179,13 +2172,6 @@ } } ], - "Logging": { - "AccessLog": { - "File": { - "Path": "/dev/stdout" - } - } - }, "ServiceDiscovery": { "AWSCloudMap": { "NamespaceName": "production", @@ -3210,13 +3196,6 @@ } } ], - "Logging": { - "AccessLog": { - "File": { - "Path": "/dev/stdout" - } - } - }, "ServiceDiscovery": { "AWSCloudMap": { "NamespaceName": "production", diff --git a/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.multiple-environments.expected.json b/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.multiple-environments.expected.json index bd31a465f6862..baffae9541166 100644 --- a/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.multiple-environments.expected.json +++ b/packages/@aws-cdk-containers/ecs-service-extensions/test/integ.multiple-environments.expected.json @@ -1491,13 +1491,6 @@ } } ], - "Logging": { - "AccessLog": { - "File": { - "Path": "/dev/stdout" - } - } - }, "ServiceDiscovery": { "AWSCloudMap": { "NamespaceName": "production", @@ -2022,13 +2015,6 @@ } } ], - "Logging": { - "AccessLog": { - "File": { - "Path": "/dev/stdout" - } - } - }, "ServiceDiscovery": { "AWSCloudMap": { "NamespaceName": "development",