fix(cloudmap): fix CloudMap Service import, expose ECS CloudMap Service (#4313)

- Fix the missing initialization of `namespace` on CloudMap's
  `Service.fromServiceAttributes()`.
- Expose the created CloudMap Service on ECS services so that
  `fromServiceAttributes()` doesn't need to be called at all.
- Fix a number of property initialization errors across the library.

Fixes #4286.

BREAKING CHANGE: `cloudmap.Service.fromServiceAttributes` takes a newly
required argument `namespace`.

Author: rix0rrr

allowed-breaking-changes.txt

@@ -23,7 +23,7 @@ incompatible-argument:@aws-cdk/aws-apigateway.ProxyResource.addProxy
 incompatible-argument:@aws-cdk/aws-apigateway.Resource.addProxy
 incompatible-argument:@aws-cdk/aws-apigateway.ResourceBase.addProxy
 incompatible-argument:@aws-cdk/aws-apigateway.IResource.addProxy
+incompatible-argument:@aws-cdk/aws-servicediscovery.Service.fromServiceAttributes
 removed:@aws-cdk/core.ConstructNode.addReference
 removed:@aws-cdk/core.ConstructNode.references
 removed:@aws-cdk/core.OutgoingReference
-

packages/@aws-cdk/app-delivery/lib/pipeline-deploy-stack-action.ts

@@ -93,7 +93,7 @@ export class PipelineDeployStackAction implements codepipeline.IAction {
   /**
    * The role used by CloudFormation for the deploy action
    */
-  private _deploymentRole: iam.IRole;
+  private _deploymentRole?: iam.IRole;
 
   private readonly stack: cdk.Stack;
   private readonly prepareChangeSetAction: cpactions.CloudFormationCreateReplaceChangeSetAction;
@@ -147,6 +147,10 @@ export class PipelineDeployStackAction implements codepipeline.IAction {
   }
 
   public get deploymentRole(): iam.IRole {
+    if (!this._deploymentRole) {
+      throw new Error(`Use this action in a pipeline first before accessing 'deploymentRole'`);
+    }
+
     return this._deploymentRole;
   }
 

packages/@aws-cdk/aws-apigateway/lib/restapi.ts

@@ -202,7 +202,7 @@ export class RestApi extends Resource implements IRestApi {
    * If `deploy` is disabled, you will need to explicitly assign this value in order to
    * set up integrations.
    */
-  public deploymentStage: Stage;
+  public deploymentStage!: Stage;
 
   /**
    * The domain name mapped to this API, if defined through the `domainName`
@@ -242,6 +242,7 @@ export class RestApi extends Resource implements IRestApi {
     }
 
     this.root = new RootResource(this, props, resource.attrRootResourceId);
+    this.restApiRootResourceId = resource.attrRootResourceId;
 
     if (props.domainName) {
       this.domainName = this.addDomainName('CustomDomain', props.domainName);

packages/@aws-cdk/aws-appmesh/lib/virtual-router.ts

@@ -1,7 +1,7 @@
 import cdk = require('@aws-cdk/core');
 
 import { CfnVirtualRouter } from './appmesh.generated';
-import { IMesh } from './mesh';
+import { IMesh, Mesh } from './mesh';
 import { Route, RouteBaseProps } from './route';
 import { PortMapping, Protocol } from './shared-interfaces';
 
@@ -119,6 +119,13 @@ export class VirtualRouter extends VirtualRouterBase {
     return new ImportedVirtualRouter(scope, id, { meshName, virtualRouterName });
   }
 
+  /**
+   * Import an existing virtual router given attributes
+   */
+  public static fromVirtualRouterAttributes(scope: cdk.Construct, id: string, attrs: VirtualRouterAttributes): IVirtualRouter {
+    return new ImportedVirtualRouter(scope, id, attrs);
+  }
+
   /**
    * The name of the VirtualRouter
    */
@@ -174,7 +181,7 @@ export class VirtualRouter extends VirtualRouterBase {
 /**
  * Interface with properties ncecessary to import a reusable VirtualRouter
  */
-interface VirtualRouterAttributes {
+export interface VirtualRouterAttributes {
   /**
    * The name of the VirtualRouter
    */
@@ -188,6 +195,11 @@ interface VirtualRouterAttributes {
   /**
    * The AppMesh mesh the VirtualRouter belongs to
    */
+  readonly mesh?: IMesh;
+
+  /**
+   * The name of the AppMesh mesh the VirtualRouter belongs to
+   */
   readonly meshName?: string;
 }
 
@@ -205,14 +217,21 @@ class ImportedVirtualRouter extends VirtualRouterBase {
    */
   public readonly virtualRouterArn: string;
 
-  /**
-   * The AppMesh mesh the VirtualRouter belongs to
-   */
-  public readonly mesh: IMesh;
+  private _mesh?: IMesh;
 
   constructor(scope: cdk.Construct, id: string, props: VirtualRouterAttributes) {
     super(scope, id);
 
+    if (props.mesh) {
+      this._mesh = props.mesh;
+    }
+    if (props.meshName) {
+      if (props.mesh) {
+        throw new Error(`Supply either 'mesh' or 'meshName', but not both`);
+      }
+      this._mesh = Mesh.fromMeshName(this, 'Mesh', props.meshName);
+    }
+
     if (props.virtualRouterArn) {
       this.virtualRouterArn = props.virtualRouterArn;
       this.virtualRouterName = cdk.Fn.select(2, cdk.Fn.split('/', cdk.Stack.of(scope).parseArn(props.virtualRouterArn).resourceName!));
@@ -227,4 +246,14 @@ class ImportedVirtualRouter extends VirtualRouterBase {
       throw new Error('Need either arn or both names');
     }
   }
+
+  /**
+   * The AppMesh mesh the VirtualRouter belongs to
+   */
+  public get mesh(): IMesh {
+    if (!this._mesh) {
+      throw new Error(`Please supply either 'mesh' or 'meshName' when calling 'fromVirtualRouterAttributes'`);
+    }
+    return this._mesh;
+  }
 }

packages/@aws-cdk/aws-appmesh/test/test.virtual-router.ts

@@ -303,4 +303,17 @@ export = {
       test.done();
     },
   },
+
+  'can import a virtual router'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    // WHEN
+    const vr = appmesh.VirtualRouter.fromVirtualRouterName(stack, 'Router', 'MyMesh', 'MyRouter');
+
+    // THEN
+    test.ok(vr.mesh !== undefined);
+
+    test.done();
+  },
 };

packages/@aws-cdk/aws-codebuild/lib/project.ts

@@ -175,7 +175,7 @@ abstract class ProjectBase extends Resource implements IProject {
    * May be unset, in which case this Project is not configured to use a VPC.
    * @internal
    */
-  protected _connections: ec2.Connections;
+  protected _connections: ec2.Connections | undefined;
 
   /**
    * Access the Connections object.

packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts

@@ -233,7 +233,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends cdk.Construct {
   /**
    * Certificate Manager certificate to associate with the load balancer.
    */
-  public readonly certificate: ICertificate;
+  public readonly certificate?: ICertificate;
 
   /**
    * The cluster that hosts the service.

packages/@aws-cdk/aws-ecs/lib/base/base-service.ts

@@ -221,6 +221,13 @@ export abstract class BaseService extends Resource
     }
   }
 
+  /**
+   * The CloudMap service created for this service, if any.
+   */
+  public get cloudMapService(): cloudmap.IService | undefined {
+    return this.cloudmapService;
+  }
+
   /**
    * This method is called to attach this service to an Application Load Balancer.
    *
@@ -303,6 +310,68 @@ export abstract class BaseService extends Resource
     });
   }
 
+  /**
+   * Enable CloudMap service discovery for the service
+   *
+   * @returns The created CloudMap service
+   */
+  public enableCloudMap(options: CloudMapOptions): cloudmap.Service {
+    const sdNamespace = this.cluster.defaultCloudMapNamespace;
+    if (sdNamespace === undefined) {
+      throw new Error("Cannot enable service discovery if a Cloudmap Namespace has not been created in the cluster.");
+    }
+
+    // Determine DNS type based on network mode
+    const networkMode = this.taskDefinition.networkMode;
+    if (networkMode === NetworkMode.NONE) {
+      throw new Error("Cannot use a service discovery if NetworkMode is None. Use Bridge, Host or AwsVpc instead.");
+    }
+
+    // Bridge or host network mode requires SRV records
+    let dnsRecordType = options.dnsRecordType;
+
+    if (networkMode === NetworkMode.BRIDGE || networkMode === NetworkMode.HOST) {
+      if (dnsRecordType ===  undefined) {
+        dnsRecordType = cloudmap.DnsRecordType.SRV;
+      }
+      if (dnsRecordType !== cloudmap.DnsRecordType.SRV) {
+        throw new Error("SRV records must be used when network mode is Bridge or Host.");
+      }
+    }
+
+    // Default DNS record type for AwsVpc network mode is A Records
+    if (networkMode === NetworkMode.AWS_VPC) {
+      if (dnsRecordType ===  undefined) {
+        dnsRecordType = cloudmap.DnsRecordType.A;
+      }
+    }
+
+    // If the task definition that your service task specifies uses the AWSVPC network mode and a type SRV DNS record is
+    // used, you must specify a containerName and containerPort combination
+    const containerName = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.containerName : undefined;
+    const containerPort = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.containerPort : undefined;
+
+    const cloudmapService = new cloudmap.Service(this, 'CloudmapService', {
+      namespace: sdNamespace,
+      name: options.name,
+      dnsRecordType: dnsRecordType!,
+      customHealthCheck: { failureThreshold: options.failureThreshold || 1 }
+    });
+
+    const serviceArn = cloudmapService.serviceArn;
+
+    // add Cloudmap service to the ECS Service's serviceRegistry
+    this.addServiceRegistry({
+      arn: serviceArn,
+      containerName,
+      containerPort
+    });
+
+    this.cloudmapService = cloudmapService;
+
+    return cloudmapService;
+  }
+
   /**
    * This method returns the specified CloudWatch metric name for this service.
    */
@@ -430,66 +499,6 @@ export abstract class BaseService extends Resource
     this.serviceRegistries.push(sr);
   }
 
-  /**
-   * Enable CloudMap service discovery for the service
-   */
-  private enableCloudMap(options: CloudMapOptions): cloudmap.Service {
-    const sdNamespace = this.cluster.defaultCloudMapNamespace;
-    if (sdNamespace === undefined) {
-      throw new Error("Cannot enable service discovery if a Cloudmap Namespace has not been created in the cluster.");
-    }
-
-    // Determine DNS type based on network mode
-    const networkMode = this.taskDefinition.networkMode;
-    if (networkMode === NetworkMode.NONE) {
-      throw new Error("Cannot use a service discovery if NetworkMode is None. Use Bridge, Host or AwsVpc instead.");
-    }
-
-    // Bridge or host network mode requires SRV records
-    let dnsRecordType = options.dnsRecordType;
-
-    if (networkMode === NetworkMode.BRIDGE || networkMode === NetworkMode.HOST) {
-      if (dnsRecordType ===  undefined) {
-        dnsRecordType = cloudmap.DnsRecordType.SRV;
-      }
-      if (dnsRecordType !== cloudmap.DnsRecordType.SRV) {
-        throw new Error("SRV records must be used when network mode is Bridge or Host.");
-      }
-    }
-
-    // Default DNS record type for AwsVpc network mode is A Records
-    if (networkMode === NetworkMode.AWS_VPC) {
-      if (dnsRecordType ===  undefined) {
-        dnsRecordType = cloudmap.DnsRecordType.A;
-      }
-    }
-
-    // If the task definition that your service task specifies uses the AWSVPC network mode and a type SRV DNS record is
-    // used, you must specify a containerName and containerPort combination
-    const containerName = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.containerName : undefined;
-    const containerPort = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.containerPort : undefined;
-
-    const cloudmapService = new cloudmap.Service(this, 'CloudmapService', {
-      namespace: sdNamespace,
-      name: options.name,
-      dnsRecordType: dnsRecordType!,
-      customHealthCheck: { failureThreshold: options.failureThreshold || 1 }
-    });
-
-    const serviceArn = cloudmapService.serviceArn;
-
-    // add Cloudmap service to the ECS Service's serviceRegistry
-    this.addServiceRegistry({
-      arn: serviceArn,
-      containerName,
-      containerPort
-    });
-
-    this.cloudmapService = cloudmapService;
-
-    return cloudmapService;
-  }
-
   /**
    *  Return the default grace period when load balancers are configured and
    *  healthCheckGracePeriod is not already set

packages/@aws-cdk/aws-ecs/lib/images/ecr.ts

@@ -21,13 +21,15 @@ export class EcrImage extends ContainerImage {
    */
   constructor(private readonly repository: ecr.IRepository, private readonly tag: string) {
     super();
+
+    this.imageName = this.repository.repositoryUriForTag(this.tag);
   }
 
   public bind(_scope: Construct, containerDefinition: ContainerDefinition): ContainerImageConfig {
     this.repository.grantPull(containerDefinition.taskDefinition.obtainExecutionRole());
 
     return {
-      imageName: this.repository.repositoryUriForTag(this.tag)
+      imageName: this.imageName
     };
   }
 }

packages/@aws-cdk/aws-ecs/lib/log-drivers/generic-log-driver.ts

@@ -51,17 +51,17 @@ export class GenericLogDriver extends LogDriver {
    *
    * @param props the generic log driver configuration options.
    */
-  constructor(private readonly props: GenericLogDriverProps) {
+  constructor(props: GenericLogDriverProps) {
     super();
+
+    this.logDriver = props.logDriver;
+    this.options = props.options || {};
   }
 
   /**
    * Called when the log driver is configured on a container.
    */
   public bind(_scope: Construct, _containerDefinition: ContainerDefinition): LogDriverConfig {
-    this.logDriver = this.props.logDriver;
-    this.options = this.props.options || {};
-
     return {
       logDriver: this.logDriver,
       options: removeEmpty(this.options),