feat(aws-appmesh): adds access logging configuration to Virtual Nodes (…

…#10490) Addresses the first point on #9490 by allow access logging to be configured through props 1. Introduces a new `AccessLog` shared-interface as it can be reused in Virtual Gateways and Virtual Nodes 1. Removes the default access logging to stdout in Virtual Nodes and allows it to be configured via props BREAKING CHANGE: VirtualNode no longer has accessLog set to "/dev/stdout" by default ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Sep 30, 2020 · e96b5aa · e96b5aa
1 parent 0d7d07e
commit e96b5aa
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 69 deletions.
diff --git a/...es/@aws-cdk-containers/ecs-service-extensions/test/integ.all-service-addons.expected.json b/...es/@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/...@aws-cdk-containers/ecs-service-extensions/test/integ.multiple-environments.expected.json b/...@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",

diff --git 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,12 +187,13 @@ const node = new VirtualNode(this, 'node', {
       unhealthyThreshold: 2,
     },
   },
+  accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'),
 });
 
 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
@@ -1,4 +1,5 @@
-import { Duration } from '@aws-cdk/core';
+import * as cdk from '@aws-cdk/core';
+import { CfnVirtualNode } from './appmesh.generated';
 
 /**
  * Enum of supported AppMesh protocols
@@ -27,7 +28,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 +53,7 @@ export interface HealthCheck {
    *
    * @default 2 seconds
    */
-  readonly timeout?: Duration;
+  readonly timeout?: cdk.Duration;
   /**
    * Number of failed attempts before considering the node DOWN.
    *
@@ -92,9 +93,69 @@ export interface VirtualNodeListener {
   readonly portMapping?: PortMapping;
 
   /**
-   * Array fo 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 {
+
+  /**
+   * VirtualNode CFN configuration for Access Logging
+   *
+   * @default - no access logging
+   */
+  readonly virtualNodeAccessLog?: CfnVirtualNode.AccessLogProperty;
+}
+
+/**
+ * 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
+ */
+class FileAccessLog extends AccessLog {
+  /**
+   * Path to a file to write access logs to
+   *
+   * @default - no file based access logging
+   */
+  public readonly filePath: string;
+
+  constructor(filePath: string) {
+    super();
+    this.filePath = filePath;
+  }
+
+  public bind(_scope: cdk.Construct): AccessLogConfig {
+    return {
+      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
@@ -3,7 +3,7 @@ import * as cdk from '@aws-cdk/core';
 import { Construct } from 'constructs';
 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;
 }
 
 /**
@@ -252,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,
@@ -267,13 +275,9 @@ export class VirtualNode extends VirtualNodeBase {
             attributes: renderAttributes(props.cloudMapServiceInstanceAttributes),
           } : undefined,
         },
-        logging: {
-          accessLog: {
-            file: {
-              path: '/dev/stdout',
-            },
-          },
-        },
+        logging: accessLogging !== undefined ? {
+          accessLog: accessLogging.virtualNodeAccessLog,
+        } : 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
@@ -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
@@ -95,6 +95,7 @@ const node3 = mesh.addVirtualNode('node3', {
       unhealthyThreshold: 2,
     },
   },
+  accessLog: appmesh.AccessLog.fromFilePath('/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
@@ -276,13 +276,6 @@ export = {
             },
             Spec: {
               // Specifically: no Listeners and Backends
-              Logging: {
-                AccessLog: {
-                  File: {
-                    Path: '/dev/stdout',
-                  },
-                },
-              },
               ServiceDiscovery: {
                 DNS: {
                   Hostname: 'test.domain.local',