Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 24, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
28 changes: 13 additions & 15 deletions ...ges/@aws-cdk/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 });
Expand All @@ -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 });
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
12 changes: 5 additions & 7 deletions packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 5 additions & 7 deletions packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s-v2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,16 +235,6 @@ export = {
}
}));

expect(stack).notTo(haveResourceLike('AWS::ServiceDiscovery::Service', {
DnsConfig: {
DnsRecords: [
{
Type: "A"
}
]
}
}));

test.done();
},

Expand Down
73 changes: 0 additions & 73 deletions packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down