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 committed Nov 25, 2024
1 parent 6b85a18 commit 0a59d99
Show file tree
Hide file tree
Showing 8 changed files with 171 additions and 6 deletions.
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/aws-ecs-patterns/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ Fargate services will use the `LATEST` platform version by default, but you can

Fargate services use the default VPC Security Group unless one or more are provided using the `securityGroups` property in the constructor.

Fargate services will be created with a public load balancer by default. If you wish to override this behavior,
you can set the `publicLoadBalancer` property to `false`
or change the `@aws-cdk/aws-ecs-patterns:fargateServiceBaseHasPublicLBDefault` feature flag to false.

By setting `redirectHTTP` to true, CDK will automatically create a listener on port 80 that redirects HTTP traffic to the HTTPS port.

If you specify the option `recordType` you can decide if you want the construct to use CNAME or Route53-Aliases as record sets.
Expand Down
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
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
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
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
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
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
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 0a59d99

Please sign in to comment.