Skip to content

Commit

Permalink
revert: Revert #1891
Browse files Browse the repository at this point in the history 
* revert: revert #1891

Passing subnets to the construct causes CDK to generate custom resources
Reverting this while the issue is being investigated by AWS.
  • Loading branch information
@marsavar
marsavar authored Jun 20, 2023
1 parent effb29e commit c8207ed
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 261 deletions.
222 changes: 1 addition & 221 deletions src/constructs/ecs/__snapshots__/ecs-task.test.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,107 +3,6 @@
exports[`The GuEcsTask pattern should create the correct resources with lots of config 1`] = `
{
"Mappings": {
"DefaultCrNodeVersionMap": {
"af-south-1": {
"value": "nodejs16.x",
},
"ap-east-1": {
"value": "nodejs16.x",
},
"ap-northeast-1": {
"value": "nodejs16.x",
},
"ap-northeast-2": {
"value": "nodejs16.x",
},
"ap-northeast-3": {
"value": "nodejs16.x",
},
"ap-south-1": {
"value": "nodejs16.x",
},
"ap-south-2": {
"value": "nodejs16.x",
},
"ap-southeast-1": {
"value": "nodejs16.x",
},
"ap-southeast-2": {
"value": "nodejs16.x",
},
"ap-southeast-3": {
"value": "nodejs16.x",
},
"ca-central-1": {
"value": "nodejs16.x",
},
"cn-north-1": {
"value": "nodejs16.x",
},
"cn-northwest-1": {
"value": "nodejs16.x",
},
"eu-central-1": {
"value": "nodejs16.x",
},
"eu-central-2": {
"value": "nodejs16.x",
},
"eu-north-1": {
"value": "nodejs16.x",
},
"eu-south-1": {
"value": "nodejs16.x",
},
"eu-south-2": {
"value": "nodejs16.x",
},
"eu-west-1": {
"value": "nodejs16.x",
},
"eu-west-2": {
"value": "nodejs16.x",
},
"eu-west-3": {
"value": "nodejs16.x",
},
"me-central-1": {
"value": "nodejs16.x",
},
"me-south-1": {
"value": "nodejs16.x",
},
"sa-east-1": {
"value": "nodejs16.x",
},
"us-east-1": {
"value": "nodejs16.x",
},
"us-east-2": {
"value": "nodejs16.x",
},
"us-gov-east-1": {
"value": "nodejs16.x",
},
"us-gov-west-1": {
"value": "nodejs16.x",
},
"us-iso-east-1": {
"value": "nodejs14.x",
},
"us-iso-west-1": {
"value": "nodejs14.x",
},
"us-isob-east-1": {
"value": "nodejs14.x",
},
"us-west-1": {
"value": "nodejs16.x",
},
"us-west-2": {
"value": "nodejs16.x",
},
},
"ServiceprincipalMap": {
"af-south-1": {
"states": "states.af-south-1.amazonaws.com",
Expand Down Expand Up @@ -210,7 +109,6 @@ exports[`The GuEcsTask pattern should create the correct resources with lots of
"gu:cdk:constructs": [
"GuStack",
"GuEcsTask",
"GuSubnetListParameter",
"GuDistributionBucketParameter",
],
"gu:cdk:version": "TEST",
Expand All @@ -223,124 +121,13 @@ exports[`The GuEcsTask pattern should create the correct resources with lots of
},
},
"Parameters": {
"AssetParameters28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0ArtifactHashAF1370F8": {
"Description": "Artifact hash for asset "28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0"",
"Type": "String",
},
"AssetParameters28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0S3BucketCD1790E7": {
"Description": "S3 bucket for asset "28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0"",
"Type": "String",
},
"AssetParameters28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0S3VersionKeyCE63AE8F": {
"Description": "S3 key for asset version "28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0"",
"Type": "String",
},
"DistributionBucketName": {
"Default": "/account/services/artifact.bucket",
"Description": "SSM parameter containing the S3 bucket name holding distribution artifacts",
"Type": "AWS::SSM::Parameter::Value<String>",
},
"ecstestPrivateSubnets": {
"Default": "/account/vpc/primary/subnets/private",
"Description": "A list of private subnets",
"Type": "AWS::SSM::Parameter::Value<List<AWS::EC2::Subnet::Id>>",
},
},
"Resources": {
"AWSCDKCfnUtilsProviderCustomResourceProviderHandlerCF82AA57": {
"DependsOn": [
"AWSCDKCfnUtilsProviderCustomResourceProviderRoleFE0EE867",
],
"Properties": {
"Code": {
"S3Bucket": {
"Ref": "AssetParameters28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0S3BucketCD1790E7",
},
"S3Key": {
"Fn::Join": [
"",
[
{
"Fn::Select": [
0,
{
"Fn::Split": [
"||",
{
"Ref": "AssetParameters28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0S3VersionKeyCE63AE8F",
},
],
},
],
},
{
"Fn::Select": [
1,
{
"Fn::Split": [
"||",
{
"Ref": "AssetParameters28739348edff6f1084f6a50d8d934e2d3fc2a3bb77442d8a9a1361d51ccd03c0S3VersionKeyCE63AE8F",
},
],
},
],
},
],
],
},
},
"Handler": "__entrypoint__.handler",
"MemorySize": 128,
"Role": {
"Fn::GetAtt": [
"AWSCDKCfnUtilsProviderCustomResourceProviderRoleFE0EE867",
"Arn",
],
},
"Runtime": "nodejs16.x",
"Timeout": 900,
},
"Type": "AWS::Lambda::Function",
},
"AWSCDKCfnUtilsProviderCustomResourceProviderRoleFE0EE867": {
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "lambda.amazonaws.com",
},
},
],
"Version": "2012-10-17",
},
"ManagedPolicyArns": [
{
"Fn::Sub": "arn:\${AWS::Partition}:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole",
},
],
},
"Type": "AWS::IAM::Role",
},
"CdkJsonStringify2": {
"DeletionPolicy": "Delete",
"Properties": {
"ServiceToken": {
"Fn::GetAtt": [
"AWSCDKCfnUtilsProviderCustomResourceProviderHandlerCF82AA57",
"Arn",
],
},
"Value": {
"Ref": "ecstestPrivateSubnets",
},
},
"Type": "Custom::AWSCDKCfnJsonStringify",
"UpdateReplacePolicy": "Delete",
},
"ecstestexecutionfailedC93F511B": {
"Properties": {
"ActionsEnabled": true,
Expand Down Expand Up @@ -456,14 +243,7 @@ exports[`The GuEcsTask pattern should create the correct resources with lots of
"Arn",
],
},
"","TaskDefinition":"test-stack-TEST-ecs-test","NetworkConfiguration":{"AwsvpcConfiguration":{"Subnets":",
{
"Fn::GetAtt": [
"CdkJsonStringify2",
"Value",
],
},
","SecurityGroups":["id-123"]}},"Overrides":{"ContainerOverrides":[{"Name":"test-ecs-task-ecs-test-TaskContainer"}]},"LaunchType":"FARGATE","PlatformVersion":"LATEST"}}}}",
"","TaskDefinition":"test-stack-TEST-ecs-test","NetworkConfiguration":{"AwsvpcConfiguration":{"Subnets":["abc-123"],"SecurityGroups":["id-123"]}},"Overrides":{"ContainerOverrides":[{"Name":"test-ecs-task-ecs-test-TaskContainer"}]},"LaunchType":"FARGATE","PlatformVersion":"LATEST"}}}}",
],
],
},
Expand Down
35 changes: 2 additions & 33 deletions src/constructs/ecs/ecs-task.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Template } from "aws-cdk-lib/assertions";
import type { ISubnet, IVpc } from "aws-cdk-lib/aws-ec2";
import type { IVpc } from "aws-cdk-lib/aws-ec2";
import { SecurityGroup, Vpc } from "aws-cdk-lib/aws-ec2";
import { Effect, PolicyStatement } from "aws-cdk-lib/aws-iam";
import { GuTemplate, simpleGuStackForTesting } from "../../utils/test";
import type { GuStack } from "../core";
import { GuVpc, SubnetType } from "../ec2/vpc";
import { GuEcsTask } from "./ecs-task";

const makeVpc = (stack: GuStack) =>
Expand Down Expand Up @@ -39,11 +38,10 @@ describe("The GuEcsTask pattern", () => {
});
});

const generateComplexStack = (stack: GuStack, app: string, vpc: IVpc, subnets?: ISubnet[]) => {
const generateComplexStack = (stack: GuStack, app: string, vpc: IVpc) => {
new GuEcsTask(stack, `test-ecs-task-${app}`, {
containerConfiguration: { id: "node:10", type: "registry" },
vpc,
subnets,
app: app,
taskTimeoutInMinutes: 60,
cpu: 1024,
Expand Down Expand Up @@ -77,33 +75,4 @@ describe("The GuEcsTask pattern", () => {
template.hasGuTaggedResource("AWS::ECS::TaskDefinition", { appIdentity: { app: "ecs-test" } });
template.hasGuTaggedResource("AWS::ECS::TaskDefinition", { appIdentity: { app: "ecs-test2" } });
});

it("should default to private subnets when no subnet prop is specified", () => {
const stack = simpleGuStackForTesting();

generateComplexStack(stack, "ecs-private-subnet-test", makeVpc(stack));

const template = Template.fromStack(stack);

template.hasParameter("ecsprivatesubnettestPrivateSubnets", {
Default: "/account/vpc/primary/subnets/private",
});
});

it("should override private subnets when the `subnets` prop is specified", () => {
const stack = simpleGuStackForTesting();
const app = "ecs-public-subnet-test";

generateComplexStack(
stack,
app,
makeVpc(stack),
GuVpc.subnetsFromParameter(stack, { type: SubnetType.PUBLIC, app })
);

const template = Template.fromStack(stack);
template.hasParameter("ecspublicsubnettestPublicSubnets", {
Default: "/account/vpc/primary/subnets/public",
});
});
});
7 changes: 1 addition & 6 deletions src/constructs/ecs/ecs-task.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { CfnOutput, Duration } from "aws-cdk-lib";
import { Alarm, TreatMissingData } from "aws-cdk-lib/aws-cloudwatch";
import { SnsAction } from "aws-cdk-lib/aws-cloudwatch-actions";
import type { ISecurityGroup, ISubnet, IVpc } from "aws-cdk-lib/aws-ec2";
import type { ISecurityGroup, IVpc } from "aws-cdk-lib/aws-ec2";
import type { IRepository } from "aws-cdk-lib/aws-ecr";
import {
Cluster,
Expand All @@ -20,7 +20,6 @@ import { Construct } from "constructs";
import type { NoMonitoring } from "../cloudwatch";
import type { GuStack } from "../core";
import { AppIdentity } from "../core";
import { GuVpc, SubnetType } from "../ec2";
import { GuGetDistributablePolicyStatement } from "../iam";

/**
Expand Down Expand Up @@ -113,7 +112,6 @@ export interface GuEcsTaskProps extends AppIdentity {
customTaskPolicies?: PolicyStatement[];
environmentOverrides?: TaskEnvironmentVariable[];
storage?: number;
subnets?: ISubnet[];
}

/**
Expand All @@ -138,7 +136,6 @@ const getContainer = (config: ContainerConfiguration) => {
* Note that if your task reliably completes in less than 15 minutes then you should probably use a [[`GuLambda`]] instead. This
* pattern was mainly created to work around the 15 minute lambda timeout.
*
* If the `subnets` prop is not defined, the task will run in a private subnet by default.
*/
export class GuEcsTask extends Construct {
stateMachine: StateMachine;
Expand All @@ -158,7 +155,6 @@ export class GuEcsTask extends Construct {
taskTimeoutInMinutes = 15,
customTaskPolicies,
vpc,
subnets = GuVpc.subnetsFromParameter(scope, { type: SubnetType.PRIVATE, app }),
monitoringConfiguration,
securityGroups = [],
environmentOverrides,
Expand Down Expand Up @@ -205,7 +201,6 @@ export class GuEcsTask extends Construct {

const task = new EcsRunTask(scope, `${id}-task`, {
cluster,
subnets: { subnets },
launchTarget: new EcsFargateLaunchTarget({
platformVersion: FargatePlatformVersion.LATEST,
}),
Expand Down
1 change: 0 additions & 1 deletion src/patterns/scheduled-ecs-task.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export interface GuScheduledEcsTaskProps extends GuEcsTaskProps {
* Note that if your task reliably completes in less than 15 minutes then you should probably use a [[`GuScheduledLambda`]] instead. This
* pattern was mainly created to work around the 15 minute lambda timeout.
*
* If the `subnets` prop is not defined, the task will run in a private subnet by default.
*/
export class GuScheduledEcsTask extends GuAppAwareConstruct(GuEcsTask) {
constructor(scope: GuStack, id: string, props: GuScheduledEcsTaskProps) {
Expand Down

0 comments on commit c8207ed

Please sign in to comment.