fix(aws-ecs-patterns): revert commit f31f4e1 (#6987)

aws · Mar 24, 2020 · 0af2d2e · 0af2d2e
1 parent 3254c5d
commit 0af2d2e
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 120 deletions.
diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts
@@ -351,15 +351,13 @@ export abstract class ApplicationLoadBalancedServiceBase extends cdk.Construct {
         throw new Error('A Route53 hosted domain zone name is required to configure the specified domain name');
       }
 
-      if (internetFacing) {
-        const record = new ARecord(this, "DNS", {
-          zone: props.domainZone,
-          recordName: props.domainName,
-          target: RecordTarget.fromAlias(new LoadBalancerTarget(loadBalancer)),
-        });
+      const record = new ARecord(this, "DNS", {
+        zone: props.domainZone,
+        recordName: props.domainName,
+        target: RecordTarget.fromAlias(new LoadBalancerTarget(loadBalancer)),
+      });
 
-        domainName = record.domainName;
-      }
+      domainName = record.domainName;
     }
 
     if (loadBalancer instanceof ApplicationLoadBalancer) {

diff --git a/...ges/@aws-cdk/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts b/...ges/@aws-cdk/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts
@@ -365,8 +365,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
 
     if (props.loadBalancers) {
       for (const lbProps of props.loadBalancers) {
-        const internetFacing = lbProps.publicLoadBalancer !== undefined ? lbProps.publicLoadBalancer : true;
-        const lb = this.createLoadBalancer(lbProps.name, internetFacing);
+        const lb = this.createLoadBalancer(lbProps.name, lbProps.publicLoadBalancer);
         this.loadBalancers.push(lb);
         const protocolType = new Set<ApplicationProtocol>();
         for (const listenerProps of lbProps.listeners) {
@@ -385,7 +384,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
           });
           this.listeners.push(listener);
         }
-        const domainName = this.createDomainName(lb, internetFacing, lbProps.domainName, lbProps.domainZone);
+        const domainName = this.createDomainName(lb, lbProps.domainName, lbProps.domainZone);
         new CfnOutput(this, `LoadBalancerDNS${lb.node.id}`, { value: lb.loadBalancerDnsName });
         for (const protocol of protocolType) {
           new CfnOutput(this, `ServiceURL${lb.node.id}${protocol.toLowerCase()}`, { value: protocol.toLowerCase() + '://' + domainName });
@@ -395,13 +394,13 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
       this.loadBalancer = this.loadBalancers[0];
       this.listener = this.listeners[0];
     } else {
-      this.loadBalancer = this.createLoadBalancer('LB', true);
+      this.loadBalancer = this.createLoadBalancer('LB');
       const protocol = this.createListenerProtocol();
       this.listener = this.configListener(protocol, {
         listenerName: "PublicListener",
         loadBalancer: this.loadBalancer,
       });
-      const domainName = this.createDomainName(this.loadBalancer, true);
+      const domainName = this.createDomainName(this.loadBalancer);
 
       new CfnOutput(this, 'LoadBalancerDNS', { value: this.loadBalancer.loadBalancerDnsName });
       new CfnOutput(this, 'ServiceURL', { value: protocol.toLowerCase() + '://' + domainName });
@@ -515,7 +514,8 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
     }
   }
 
-  private createLoadBalancer(name: string, internetFacing: boolean): ApplicationLoadBalancer {
+  private createLoadBalancer(name: string, publicLoadBalancer?: boolean): ApplicationLoadBalancer {
+    const internetFacing = publicLoadBalancer !== undefined ? publicLoadBalancer : true;
     const lbProps = {
       vpc: this.cluster.vpc,
       internetFacing
@@ -551,22 +551,20 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
     });
   }
 
-  private createDomainName(loadBalancer: ApplicationLoadBalancer, internetFacing: boolean, name?: string, zone?: IHostedZone): string {
+  private createDomainName(loadBalancer: ApplicationLoadBalancer, name?: string, zone?: IHostedZone): string {
     let domainName = loadBalancer.loadBalancerDnsName;
     if (typeof name !== 'undefined') {
       if (typeof zone === 'undefined') {
         throw new Error('A Route53 hosted domain zone name is required to configure the specified domain name');
       }
 
-      if (internetFacing) {
-        const record = new ARecord(this, `DNS${loadBalancer.node.id}`, {
-          zone,
-          recordName: name,
-          target: RecordTarget.fromAlias(new LoadBalancerTarget(loadBalancer)),
-        });
+      const record = new ARecord(this, `DNS${loadBalancer.node.id}`, {
+        zone,
+        recordName: name,
+        target: RecordTarget.fromAlias(new LoadBalancerTarget(loadBalancer)),
+      });
 
-        domainName = record.domainName;
-      }
+      domainName = record.domainName;
     }
     return domainName;
   }

diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts
@@ -293,13 +293,11 @@ export abstract class NetworkLoadBalancedServiceBase extends cdk.Construct {
         throw new Error('A Route53 hosted domain zone name is required to configure the specified domain name');
       }
 
-      if (internetFacing) {
-        new ARecord(this, "DNS", {
-          zone: props.domainZone,
-          recordName: props.domainName,
-          target: RecordTarget.fromAlias(new LoadBalancerTarget(loadBalancer)),
-        });
-      }
+      new ARecord(this, "DNS", {
+        zone: props.domainZone,
+        recordName: props.domainName,
+        target: RecordTarget.fromAlias(new LoadBalancerTarget(loadBalancer)),
+      });
     }
 
     if (loadBalancer instanceof NetworkLoadBalancer) {

diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts
@@ -297,23 +297,20 @@ export abstract class NetworkMultipleTargetGroupsServiceBase extends Construct {
 
     if (props.loadBalancers) {
       for (const lbProps of props.loadBalancers) {
-        const internetFacing = lbProps.publicLoadBalancer !== undefined ? lbProps.publicLoadBalancer : true;
-        const lb = this.createLoadBalancer(lbProps.name, internetFacing);
+        const lb = this.createLoadBalancer(lbProps.name, lbProps.publicLoadBalancer);
         this.loadBalancers.push(lb);
         for (const listenerProps of lbProps.listeners) {
           const listener = this.createListener(listenerProps.name, lb, listenerProps.port || 80);
           this.listeners.push(listener);
         }
-        if (internetFacing) {
-          this.createDomainName(lb, lbProps.domainName, lbProps.domainZone);
-        }
+        this.createDomainName(lb, lbProps.domainName, lbProps.domainZone);
         new CfnOutput(this, `LoadBalancerDNS${lb.node.id}`, { value: lb.loadBalancerDnsName });
       }
       // set up default load balancer and listener.
       this.loadBalancer = this.loadBalancers[0];
       this.listener = this.listeners[0];
     } else {
-      this.loadBalancer = this.createLoadBalancer('LB', true);
+      this.loadBalancer = this.createLoadBalancer('LB');
       this.listener = this.createListener('PublicListener', this.loadBalancer, 80);
       this.createDomainName(this.loadBalancer);
 
@@ -408,7 +405,8 @@ export abstract class NetworkMultipleTargetGroupsServiceBase extends Construct {
     }
   }
 
-  private createLoadBalancer(name: string, internetFacing: boolean): NetworkLoadBalancer {
+  private createLoadBalancer(name: string, publicLoadBalancer?: boolean): NetworkLoadBalancer {
+    const internetFacing = publicLoadBalancer !== undefined ? publicLoadBalancer : true;
     const lbProps = {
       vpc: this.cluster.vpc,
       internetFacing

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s-v2.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s-v2.ts
@@ -235,16 +235,6 @@ export = {
         }
       }));
 
-      expect(stack).notTo(haveResourceLike('AWS::ServiceDiscovery::Service', {
-        DnsConfig: {
-          DnsRecords: [
-            {
-              Type: "A"
-            }
-          ]
-        }
-      }));
-
       test.done();
     },
 

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s.ts
@@ -60,44 +60,6 @@ export = {
     test.done();
   },
 
-  'test ECS loadbalanced construct with private load balancer'(test: Test) {
-    // GIVEN
-    const stack = new cdk.Stack();
-    const vpc = new ec2.Vpc(stack, 'VPC');
-    const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
-    cluster.addCapacity('DefaultAutoScalingGroup', { instanceType: new ec2.InstanceType('t2.micro') });
-
-    // WHEN
-    new ecsPatterns.ApplicationLoadBalancedEc2Service(stack, 'Service', {
-      cluster,
-      memoryLimitMiB: 1024,
-      taskImageOptions: {
-        image: ecs.ContainerImage.fromRegistry('test'),
-        environment: {
-          TEST_ENVIRONMENT_VARIABLE1: "test environment variable 1 value",
-          TEST_ENVIRONMENT_VARIABLE2: "test environment variable 2 value"
-        }
-      },
-      desiredCount: 2,
-      publicLoadBalancer: false,
-    });
-
-    // THEN - stack contains a load balancer and a service
-    expect(stack).to(haveResource('AWS::ElasticLoadBalancingV2::LoadBalancer'));
-
-    expect(stack).notTo(haveResourceLike('AWS::ServiceDiscovery::Service', {
-      DnsConfig: {
-        DnsRecords: [
-          {
-            Type: "A"
-          }
-        ]
-      }
-    }));
-
-    test.done();
-  },
-
   'set vpc instead of cluster'(test: Test) {
     // GIVEN
     const stack = new cdk.Stack();
@@ -374,41 +336,6 @@ export = {
     test.done();
   },
 
-  'test Fargate loadbalanced construct with private load balancer'(test: Test) {
-    // GIVEN
-    const stack = new cdk.Stack();
-    const vpc = new ec2.Vpc(stack, 'VPC');
-    const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
-
-    // WHEN
-    new ecsPatterns.ApplicationLoadBalancedFargateService(stack, 'Service', {
-      cluster,
-      taskImageOptions: {
-        image: ecs.ContainerImage.fromRegistry('test'),
-        environment: {
-          TEST_ENVIRONMENT_VARIABLE1: "test environment variable 1 value",
-          TEST_ENVIRONMENT_VARIABLE2: "test environment variable 2 value"
-        }
-      },
-      desiredCount: 2,
-      publicLoadBalancer: false,
-    });
-
-    // THEN - stack contains a load balancer and a service
-    expect(stack).to(haveResource('AWS::ElasticLoadBalancingV2::LoadBalancer'));
-    expect(stack).notTo(haveResourceLike('AWS::ServiceDiscovery::Service', {
-      DnsConfig: {
-        DnsRecords: [
-          {
-            Type: "A"
-          }
-        ]
-      }
-    }));
-
-    test.done();
-  },
-
   'test Fargate loadbalanced construct opting out of log driver creation'(test: Test) {
     // GIVEN
     const stack = new cdk.Stack();