Skip to content

Commit

Permalink
feat(aws-ecs-patterns): Add a feature flag to override default behavi…
Browse files Browse the repository at this point in the history 
…or of creating public load balancers

If this flag is not set, the default behavior for `ApplicationLoadBalancedFargateService` and `NetworkLoadBalancedFargateService` is to create a public load balancer (not changed).

If this flag is set to false, the behavior is that the load balancer will be private by default.

This is a feature flag as to keep compatibility with the old behavior.

Relevant issue: aws#32274
  • Loading branch information
@axwilliamson-godaddy
axwilliamson-godaddy committed Nov 25, 2024
1 parent 6877c6a commit f789efe
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 6 deletions.
8 changes: 5 additions & 3 deletions packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import { IRole } from '../../../aws-iam';
import { ARecord, IHostedZone, RecordTarget, CnameRecord } from '../../../aws-route53';
import { LoadBalancerTarget } from '../../../aws-route53-targets';
import * as cdk from '../../../core';
import { Duration } from '../../../core';
import { Duration, FeatureFlags } from '../../../core';
import * as cxapi from '../../../cx-api';

/**
* Describes the type of DNS record the service should create
Expand Down Expand Up @@ -474,7 +475,8 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
this.desiredCount = props.desiredCount || 1;
this.internalDesiredCount = props.desiredCount;

const internetFacing = props.publicLoadBalancer ?? true;
const internetFacing = props.publicLoadBalancer ??
FeatureFlags.of(this).isEnabled(cxapi.ECS_PATTERNS_FARGATE_SERVICE_BASE_HAS_PUBLIC_LB_BY_DEFAULT);

if (props.idleTimeout) {
const idleTimeout = props.idleTimeout.toSeconds();
Expand All @@ -486,7 +488,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
const lbProps: ApplicationLoadBalancerProps = {
vpc: this.cluster.vpc,
loadBalancerName: props.loadBalancerName,
internetFacing,
internetFacing: internetFacing,
idleTimeout: props.idleTimeout,
ipAddressType: props.ipAddressType,
};
Expand Down
7 changes: 5 additions & 2 deletions packages/aws-cdk-lib/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { IRole } from '../../../aws-iam';
import { ARecord, CnameRecord, IHostedZone, RecordTarget } from '../../../aws-route53';
import { LoadBalancerTarget } from '../../../aws-route53-targets';
import * as cdk from '../../../core';
import { FeatureFlags } from '../../../core';
import * as cxapi from '../../../cx-api/index';

/**
* Describes the type of DNS record the service should create
Expand Down Expand Up @@ -368,11 +370,12 @@ export abstract class NetworkLoadBalancedServiceBase extends Construct {
this.desiredCount = props.desiredCount || 1;
this.internalDesiredCount = props.desiredCount;

const internetFacing = props.publicLoadBalancer ?? true;
const internetFacing = props.publicLoadBalancer ??
FeatureFlags.of(this).isEnabled(cxapi.ECS_PATTERNS_FARGATE_SERVICE_BASE_HAS_PUBLIC_LB_BY_DEFAULT);

const lbProps: NetworkLoadBalancerProps = {
vpc: this.cluster.vpc,
internetFacing,
internetFacing: internetFacing,
ipAddressType: props.ipAddressType,
};

Expand Down
71 changes: 71 additions & 0 deletions packages/aws-cdk-lib/aws-ecs-patterns/test/ec2/l3s.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ApplicationLoadBalancer, ApplicationProtocol, ApplicationProtocolVersio
import { PublicHostedZone } from '../../../aws-route53';
import * as cloudmap from '../../../aws-servicediscovery';
import * as cdk from '../../../core';
import * as cxapi from '../../../cx-api';
import * as ecsPatterns from '../../lib';

describe('ApplicationLoadBalancedEc2Service', () => {
Expand Down Expand Up @@ -81,6 +82,76 @@ describe('ApplicationLoadBalancedEc2Service', () => {
});
});

test('ECS loadbalanced construct with feature flag private lb override', () => {
// GIVEN
const stack = new cdk.Stack();
stack.node.setContext(cxapi.ECS_PATTERNS_FARGATE_SERVICE_BASE_HAS_PUBLIC_LB_BY_DEFAULT, false);
const vpc = new ec2.Vpc(stack, 'VPC');
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
cluster.addAsgCapacityProvider(new AsgCapacityProvider(stack, 'DefaultAutoScalingGroupProvider', {
autoScalingGroup: new AutoScalingGroup(stack, 'DefaultAutoScalingGroup', {
vpc,
instanceType: new ec2.InstanceType('t2.micro'),
machineImage: MachineImage.latestAmazonLinux(),
}),
}));

// 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',
},
dockerLabels: { label1: 'labelValue1', label2: 'labelValue2' },
entryPoint: ['echo', 'ecs-is-awesome'],
command: ['/bin/bash'],
},
desiredCount: 2,
ipAddressType: IpAddressType.DUAL_STACK,
});

// THEN - stack contains a load balancer and a service
Template.fromStack(stack).resourceCountIs('AWS::ElasticLoadBalancingV2::LoadBalancer', 1);
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internal',
Type: 'application',
IpAddressType: 'dualstack',
});

Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
DesiredCount: 2,
LaunchType: 'EC2',
});

Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
Match.objectLike({
Environment: [
{
Name: 'TEST_ENVIRONMENT_VARIABLE1',
Value: 'test environment variable 1 value',
},
{
Name: 'TEST_ENVIRONMENT_VARIABLE2',
Value: 'test environment variable 2 value',
},
],
Memory: 1024,
DockerLabels: {
label1: 'labelValue1',
label2: 'labelValue2',
},
EntryPoint: ['echo', 'ecs-is-awesome'],
Command: ['/bin/bash'],
}),
],
});
});

test('multiple capacity provider strategies are set', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down
28 changes: 28 additions & 0 deletions packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service-v2.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { ApplicationProtocol, IpAddressType, SslPolicy } from '../../../aws-elas
import { CompositePrincipal, Role, ServicePrincipal } from '../../../aws-iam';
import { PublicHostedZone } from '../../../aws-route53';
import { Duration, Stack } from '../../../core';
import * as cxapi from '../../../cx-api';
import { ApplicationLoadBalancedFargateService, ApplicationMultipleTargetGroupsFargateService, NetworkLoadBalancedFargateService, NetworkMultipleTargetGroupsFargateService } from '../../lib';

const enableExecuteCommandPermissions = {
Expand Down Expand Up @@ -139,6 +140,33 @@ describe('Application Load Balancer', () => {
IpAddressType: 'dualstack',
});
});

test('dualstack application load balancer with feature flag override for private lb', () => {
// GIVEN
const stack = new Stack();
stack.node.setContext(cxapi.ECS_PATTERNS_FARGATE_SERVICE_BASE_HAS_PUBLIC_LB_BY_DEFAULT, false);

const vpc = new Vpc(stack, 'VPC', {
ipProtocol: IpProtocol.DUAL_STACK,
});
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });

// WHEN
new ApplicationLoadBalancedFargateService(stack, 'Service', {
cluster,
taskImageOptions: {
image: ecs.ContainerImage.fromRegistry('test'),
},
ipAddressType: IpAddressType.DUAL_STACK,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internal',
Type: 'application',
IpAddressType: 'dualstack',
});
});
});

describe('ApplicationMultipleTargetGroupsFargateService', () => {
Expand Down
23 changes: 23 additions & 0 deletions packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as iam from '../../../aws-iam';
import * as route53 from '../../../aws-route53';
import * as cloudmap from '../../../aws-servicediscovery';
import * as cdk from '../../../core';
import * as cxapi from '../../../cx-api';
import * as ecsPatterns from '../../lib';

describe('ApplicationLoadBalancedFargateService', () => {
Expand Down Expand Up @@ -1457,6 +1458,28 @@ describe('NetworkLoadBalancedFargateService', () => {
});
});

test('setting loadBalancerType to Network with feature flag override creates an NLB private', () => {
// GIVEN
const stack = new cdk.Stack();
stack.node.setContext(cxapi.ECS_PATTERNS_FARGATE_SERVICE_BASE_HAS_PUBLIC_LB_BY_DEFAULT, false);
const vpc = new ec2.Vpc(stack, 'VPC');
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });

// WHEN
new ecsPatterns.NetworkLoadBalancedFargateService(stack, 'Service', {
cluster,
taskImageOptions: {
image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'),
},
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Type: 'network',
Scheme: 'internal',
});
});

test('setting loadBalancerType to Network and publicLoadBalancer to false creates an NLB Private', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down
19 changes: 18 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ Flags come in three types:
| [@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics](#aws-cdkcorecfnincluderejectcomplexresourceupdatecreatepolicyintrinsics) | When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values. | 2.161.0 | (fix) |
| [@aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy](#aws-cdkaws-stepfunctions-tasksfixrunecstaskpolicy) | When enabled, the resource of IAM Run Ecs policy generated by SFN EcsRunTask will reference the definition, instead of constructing ARN. | 2.163.0 | (fix) |
| [@aws-cdk/aws-dynamodb:resourcePolicyPerReplica](#aws-cdkaws-dynamodbresourcepolicyperreplica) | When enabled will allow you to specify a resource policy per replica, and not copy the source table policy to all replicas | 2.164.0 | (fix) |
| [@aws-cdk/aws-ecs-patterns:fargateServiceBaseHasPublicLBDefault](#aws-ecs-patterns-fargateServiceBaseHasPublicLBDefault) | When enabled LBs created for Fargate Service will be public by default | 2.172.0 | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -150,7 +151,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId": true,
"@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics": true,
"@aws-cdk/aws-lambda-nodejs:sdkV3ExcludeSmithyPackages": true,
"@aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy": true
"@aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy": true,
"@aws-cdk/aws-ecs-patterns:fargateServiceBaseHasPublicLBDefault": false
}
}
```
Expand Down Expand Up @@ -1528,5 +1530,20 @@ This is a feature flag as the old behavior was technically incorrect but users m
| (not in v1) | | |
| 2.164.0 | `false` | `true` |

### @aws-cdk/aws-ecs-patterns:fargateServiceBaseHasPublicLBDefault

*When enabled LBs created for Fargate Service will be public by default* (fix)

If this flag is not set, the default behavior for `ApplicationLoadBalancedFargateService` and `NetworkLoadBalancedFargateService` is to create a public load balancer.

If this flag is set to false, the behavior is that the load balancer will be private by default.

This is a feature flag as to keep compatibility with the old behavior.


| Since | Default | Recommended |
|-------------| ----- | ----- |
| (not in v1) | | |
| 2.172.0 | `true` | `false` |

<!-- END details -->
17 changes: 17 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export const USE_CORRECT_VALUE_FOR_INSTANCE_RESOURCE_ID_PROPERTY = '@aws-cdk/aws
export const CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS = '@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics';
export const LAMBDA_NODEJS_SDK_V3_EXCLUDE_SMITHY_PACKAGES = '@aws-cdk/aws-lambda-nodejs:sdkV3ExcludeSmithyPackages';
export const STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY = '@aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy';
export const ECS_PATTERNS_FARGATE_SERVICE_BASE_HAS_PUBLIC_LB_BY_DEFAULT = '@aws-cdk/ecs-patterns:fargateServiceBaseHasPublicLBDefault';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1250,6 +1251,22 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: '2.163.0' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[ECS_PATTERNS_FARGATE_SERVICE_BASE_HAS_PUBLIC_LB_BY_DEFAULT]: {
type: FlagType.BugFix,
summary: 'When enabled, the load balancers created will be public by default',
detailsMd: `
By default, the load balancers created by ECS Patterns Fargate Service Base construct are public.
This is not ideal for cases where you need everything to be private.
When this feature flag is enabled (default True for compatability), the load balancers will be public by default.
However, if you want to make them private by default, you can set this property to false.
`,
introducedIn: { v2: '2.172.0' },
defaults: { v2: true },
recommendedValue: false,
},
};

const CURRENT_MV = 'v2';
Expand Down

0 comments on commit f789efe

Please sign in to comment.