Skip to content

Commit

Permalink
fix(ecs): validate ecs healthcheck (#24197)
Browse files Browse the repository at this point in the history
----
#22200 

I add a feature to validate some contents for healthcheck.

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
riita10069 authored Feb 18, 2023
1 parent f61d950 commit 89802a9
Show file tree
Hide file tree
Showing 11 changed files with 438 additions and 41 deletions.
17 changes: 17 additions & 0 deletions packages/@aws-cdk/aws-ecs/lib/container-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,23 @@ function renderEnvironmentFiles(partition: string, environmentFiles: Environment
}

function renderHealthCheck(hc: HealthCheck): CfnTaskDefinition.HealthCheckProperty {
if (hc.interval?.toSeconds() !== undefined) {
if (5 > hc.interval?.toSeconds() || hc.interval?.toSeconds() > 300) {
throw new Error('Interval must be between 5 seconds and 300 seconds.');
}
}

if (hc.timeout?.toSeconds() !== undefined) {
if (2 > hc.timeout?.toSeconds() || hc.timeout?.toSeconds() > 120) {
throw new Error('Timeout must be between 2 seconds and 120 seconds.');
}
}
if (hc.interval?.toSeconds() !== undefined && hc.timeout?.toSeconds() !== undefined) {
if (hc.interval?.toSeconds() < hc.timeout?.toSeconds()) {
throw new Error('Health check interval should be longer than timeout.');
}
}

return {
command: getHealthCheckCommand(hc),
interval: hc.interval?.toSeconds() ?? 30,
Expand Down
156 changes: 156 additions & 0 deletions packages/@aws-cdk/aws-ecs/test/container-definition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import * as ecs from '../lib';
import { AppProtocol } from '../lib';
import { Duration } from '@aws-cdk/core';

describe('container definition', () => {
describe('When creating a Task Definition', () => {
Expand Down Expand Up @@ -1691,6 +1692,161 @@ describe('container definition', () => {
}).toThrow(/At least one argument must be supplied for health check command./);
});

test('throws when setting Health Check with invalid interval because of too short', () => {
// GIVEN
const stack = new cdk.Stack();
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

// WHEN
taskDefinition.addContainer('cont', {
image: ecs.ContainerImage.fromRegistry('test'),
memoryLimitMiB: 1024,
healthCheck: {
command: ['CMD-SHELL', 'curl localhost:8000'],
interval: Duration.seconds(4),
timeout: Duration.seconds(30),
},
});

// THEN
expect(() => {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
HealthCheck: {
Command: ['CMD-SHELL', 'curl localhost:8000'],
Interval: 4,
},
},
],
});
}).toThrow(/Interval must be between 5 seconds and 300 seconds./);
});

test('throws when setting Health Check with invalid interval because of too long', () => {
// GIVEN
const stack = new cdk.Stack();
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

// WHEN
taskDefinition.addContainer('cont', {
image: ecs.ContainerImage.fromRegistry('test'),
memoryLimitMiB: 1024,
healthCheck: {
command: ['CMD-SHELL', 'curl localhost:8000'],
interval: Duration.seconds(301),
timeout: Duration.seconds(30),
},
});

// THEN
expect(() => {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
HealthCheck: {
Command: ['CMD-SHELL', 'curl localhost:8000'],
Interval: 4,
},
},
],
});
}).toThrow(/Interval must be between 5 seconds and 300 seconds./);
});

test('throws when setting Health Check with invalid timeout because of too short', () => {
// GIVEN
const stack = new cdk.Stack();
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

// WHEN
taskDefinition.addContainer('cont', {
image: ecs.ContainerImage.fromRegistry('test'),
memoryLimitMiB: 1024,
healthCheck: {
command: ['CMD-SHELL', 'curl localhost:8000'],
interval: Duration.seconds(40),
timeout: Duration.seconds(1),
},
});

// THEN
expect(() => {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
HealthCheck: {
Command: ['CMD-SHELL', 'curl localhost:8000'],
Interval: 4,
},
},
],
});
}).toThrow(/Timeout must be between 2 seconds and 120 seconds./);
});

test('throws when setting Health Check with invalid timeout because of too long', () => {
// GIVEN
const stack = new cdk.Stack();
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

// WHEN
taskDefinition.addContainer('cont', {
image: ecs.ContainerImage.fromRegistry('test'),
memoryLimitMiB: 1024,
healthCheck: {
command: ['CMD-SHELL', 'curl localhost:8000'],
interval: Duration.seconds(150),
timeout: Duration.seconds(130),
},
});

// THEN
expect(() => {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
HealthCheck: {
Command: ['CMD-SHELL', 'curl localhost:8000'],
Interval: 4,
},
},
],
});
}).toThrow(/Timeout must be between 2 seconds and 120 seconds./);
});

test('throws when setting Health Check with invalid interval and timeout because timeout is longer than interval', () => {
// GIVEN
const stack = new cdk.Stack();
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

// WHEN
taskDefinition.addContainer('cont', {
image: ecs.ContainerImage.fromRegistry('test'),
memoryLimitMiB: 1024,
healthCheck: {
command: ['CMD-SHELL', 'curl localhost:8000'],
interval: Duration.seconds(10),
timeout: Duration.seconds(30),
},
});

// THEN
expect(() => {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
HealthCheck: {
Command: ['CMD-SHELL', 'curl localhost:8000'],
Interval: 4,
},
},
],
});
}).toThrow(/Health check interval should be longer than timeout./);
});

test('can specify Health Check values in shell form', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "20.0.0",
"version": "30.0.0",
"files": {
"4dba456b46dc53b954d12cf55bad7b455371f307d7b5df57b5fb2e6cafe4e9ba": {
"1a5bcacf8adc1fb93503daec527cf36ecf57f189012726acf8ad69d9f993d3cb": {
"source": {
"path": "aws-ecs-integ-exec-command.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "4dba456b46dc53b954d12cf55bad7b455371f307d7b5df57b5fb2e6cafe4e9ba.json",
"objectKey": "1a5bcacf8adc1fb93503daec527cf36ecf57f189012726acf8ad69d9f993d3cb.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,15 @@
"ContainerDefinitions": [
{
"Essential": true,
"HealthCheck": {
"Command": [
"CMD-SHELL",
"curl localhost:8000"
],
"Interval": 60,
"Retries": 3,
"Timeout": 40
},
"Image": "amazon/amazon-ecs-sample",
"Name": "web"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"20.0.0"}
{"version":"30.0.0"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "30.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
"path": "execcommandintegtestDefaultTestDeployAssert4F7706FE.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
{
"version": "20.0.0",
"version": "30.0.0",
"testCases": {
"integ.exec-command": {
"exec-command-integ-test/DefaultTest": {
"stacks": [
"aws-ecs-integ-exec-command"
],
"diffAssets": false,
"stackUpdateWorkflow": true
"diffAssets": true,
"cdkCommandOptions": {
"deploy": {
"args": {
"rollback": true
}
}
},
"assertionStack": "exec-command-integ-test/DefaultTest/DeployAssert",
"assertionStackName": "execcommandintegtestDefaultTestDeployAssert4F7706FE"
}
},
"synthContext": {},
"enableLookups": false
}
}
Loading

0 comments on commit 89802a9

Please sign in to comment.