Skip to content

Commit

Permalink
fix(elbv2): healthcheck interval is overly restrictive (#24157)
Browse files Browse the repository at this point in the history
The health check interval of network target groups can only be set to 10 or 30 seconds. [The allowed range is 5-300
](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/target-group-health-checks.html)
Closes #24156

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
peterwoodworth committed Feb 14, 2023
1 parent 6c935c7 commit 4f83e02
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,12 @@ export class NetworkTargetGroup extends TargetGroupBase implements INetworkTarge

const healthCheck: HealthCheck = this.healthCheck || {};

const allowedIntervals = [10, 30];
const lowHealthCheckInterval = 5;
const highHealthCheckInterval = 300;
if (healthCheck.interval) {
const seconds = healthCheck.interval.toSeconds();
if (!cdk.Token.isUnresolved(seconds) && !allowedIntervals.includes(seconds)) {
ret.push(`Health check interval '${seconds}' not supported. Must be one of the following values '${allowedIntervals.join(',')}'.`);
if (!cdk.Token.isUnresolved(seconds) && (seconds < lowHealthCheckInterval || seconds > highHealthCheckInterval)) {
ret.push(`Health check interval '${seconds}' not supported. Must be between ${lowHealthCheckInterval} and ${highHealthCheckInterval}.`);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ export interface HealthCheck {

/**
* The approximate number of seconds between health checks for an individual target.
* Must be 5 to 300 seconds
*
* @default Duration.seconds(30)
* @default 10 seconds if protocol is `GENEVE`, 35 seconds if target type is `lambda`, else 30 seconds
*/
readonly interval?: cdk.Duration;

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": {
"4ac709cf496678c5d2ee9fb122c995688f00a12297a81f2ecf125722d79e9bf4": {
"c7b59451188880618122593d9b5f98c0e30ff60bb10bb205c4c1a053fcdc4e79": {
"source": {
"path": "aws-cdk-elbv2-integ.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "4ac709cf496678c5d2ee9fb122c995688f00a12297a81f2ecf125722d79e9bf4.json",
"objectKey": "c7b59451188880618122593d9b5f98c0e30ff60bb10bb205c4c1a053fcdc4e79.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 @@ -439,6 +439,8 @@
"LBListenerTargetGroupF04FCF6D": {
"Type": "AWS::ElasticLoadBalancingV2::TargetGroup",
"Properties": {
"HealthCheckIntervalSeconds": 250,
"HealthCheckProtocol": "TCP",
"Port": 443,
"Protocol": "TCP",
"Targets": [
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
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "30.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{
"version": "20.0.0",
"version": "30.0.0",
"testCases": {
"elbv2-integ/DefaultTest": {
"stacks": [
"aws-cdk-elbv2-integ"
],
"assertionStack": "elbv2-integ/DefaultTest/DeployAssert"
"assertionStack": "elbv2-integ/DefaultTest/DeployAssert",
"assertionStackName": "elbv2integDefaultTestDeployAssert6120E394"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
{
"version": "20.0.0",
"version": "30.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
"properties": {
"file": "tree.json"
}
},
"aws-cdk-elbv2-integ.assets": {
"type": "cdk:asset-manifest",
"properties": {
Expand All @@ -23,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/4ac709cf496678c5d2ee9fb122c995688f00a12297a81f2ecf125722d79e9bf4.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/c7b59451188880618122593d9b5f98c0e30ff60bb10bb205c4c1a053fcdc4e79.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -256,6 +250,12 @@
]
},
"displayName": "elbv2-integ/DefaultTest/DeployAssert"
},
"Tree": {
"type": "cdk:tree",
"properties": {
"file": "tree.json"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@
"id": "App",
"path": "",
"children": {
"Tree": {
"id": "Tree",
"path": "Tree",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
}
},
"aws-cdk-elbv2-integ": {
"id": "aws-cdk-elbv2-integ",
"path": "aws-cdk-elbv2-integ",
Expand Down Expand Up @@ -91,8 +83,8 @@
"id": "Acl",
"path": "aws-cdk-elbv2-integ/VPC/PublicSubnet1/Acl",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Resource",
"version": "0.0.0"
}
},
"RouteTable": {
Expand Down Expand Up @@ -258,8 +250,8 @@
"id": "Acl",
"path": "aws-cdk-elbv2-integ/VPC/PublicSubnet2/Acl",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Resource",
"version": "0.0.0"
}
},
"RouteTable": {
Expand Down Expand Up @@ -425,8 +417,8 @@
"id": "Acl",
"path": "aws-cdk-elbv2-integ/VPC/PrivateSubnet1/Acl",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Resource",
"version": "0.0.0"
}
},
"RouteTable": {
Expand Down Expand Up @@ -544,8 +536,8 @@
"id": "Acl",
"path": "aws-cdk-elbv2-integ/VPC/PrivateSubnet2/Acl",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Resource",
"version": "0.0.0"
}
},
"RouteTable": {
Expand Down Expand Up @@ -732,6 +724,8 @@
"attributes": {
"aws:cdk:cloudformation:type": "AWS::ElasticLoadBalancingV2::TargetGroup",
"aws:cdk:cloudformation:props": {
"healthCheckIntervalSeconds": 250,
"healthCheckProtocol": "TCP",
"port": 443,
"protocol": "TCP",
"targets": [
Expand Down Expand Up @@ -767,11 +761,27 @@
"fqn": "@aws-cdk/aws-elasticloadbalancingv2.NetworkLoadBalancer",
"version": "0.0.0"
}
},
"BootstrapVersion": {
"id": "BootstrapVersion",
"path": "aws-cdk-elbv2-integ/BootstrapVersion",
"constructInfo": {
"fqn": "@aws-cdk/core.CfnParameter",
"version": "0.0.0"
}
},
"CheckBootstrapVersion": {
"id": "CheckBootstrapVersion",
"path": "aws-cdk-elbv2-integ/CheckBootstrapVersion",
"constructInfo": {
"fqn": "@aws-cdk/core.CfnRule",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Stack",
"version": "0.0.0"
}
},
"elbv2-integ": {
Expand All @@ -787,15 +797,33 @@
"path": "elbv2-integ/DefaultTest/Default",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"version": "10.1.249"
}
},
"DeployAssert": {
"id": "DeployAssert",
"path": "elbv2-integ/DefaultTest/DeployAssert",
"children": {
"BootstrapVersion": {
"id": "BootstrapVersion",
"path": "elbv2-integ/DefaultTest/DeployAssert/BootstrapVersion",
"constructInfo": {
"fqn": "@aws-cdk/core.CfnParameter",
"version": "0.0.0"
}
},
"CheckBootstrapVersion": {
"id": "CheckBootstrapVersion",
"path": "elbv2-integ/DefaultTest/DeployAssert/CheckBootstrapVersion",
"constructInfo": {
"fqn": "@aws-cdk/core.CfnRule",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Stack",
"version": "0.0.0"
}
}
},
Expand All @@ -809,11 +837,19 @@
"fqn": "@aws-cdk/integ-tests.IntegTest",
"version": "0.0.0"
}
},
"Tree": {
"id": "Tree",
"path": "Tree",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.249"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.App",
"version": "0.0.0"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ const group = listener.addTargets('Target', {
targets: [new elbv2.IpTarget('10.0.1.1')],
});

group.configureHealthCheck({
interval: cdk.Duration.seconds(250),
protocol: elbv2.Protocol.TCP,
});

vpc.publicSubnets.forEach(subnet => group.node.addDependency(subnet));
group.node.addDependency(vpc.internetConnectivityEstablished);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,12 @@ describe('tests', () => {
const targetGroup = listener.addTargets('ECS', {
port: 80,
healthCheck: {
interval: cdk.Duration.seconds(60),
interval: cdk.Duration.seconds(350),
},
});

const validationErrors: string[] = targetGroup.node.validate();
const intervalError = validationErrors.find((err) => /Health check interval '60' not supported. Must be one of the following values/.test(err));
const intervalError = validationErrors.find((err) => /Health check interval '350' not supported. Must be between/.test(err));
expect(intervalError).toBeDefined();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ describe('tests', () => {
vpc,
port: 80,
healthCheck: {
interval: cdk.Duration.seconds(5),
interval: cdk.Duration.seconds(3),
},
});

expect(() => {
app.synth();
}).toThrow(/Health check interval '5' not supported. Must be one of the following values '10,30'./);
}).toThrow(/Health check interval '3' not supported. Must be between 5 and 300./);
});

test('targetGroupName unallowed: more than 32 characters', () => {
Expand Down

0 comments on commit 4f83e02

Please sign in to comment.