Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(elbv2): correct wrong timeout validation #26031

Merged
merged 6 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,17 @@ export class NetworkTargetGroup extends TargetGroupBase implements INetworkTarge
`Must be one of [${NLB_PATH_HEALTH_CHECK_PROTOCOLS.join(', ')}]`,
].join(' '));
}
if (healthCheck.timeout && healthCheck.timeout.toSeconds() !== NLB_HEALTH_CHECK_TIMEOUTS[healthCheck.protocol]) {
ret.push([
'Custom health check timeouts are not supported for Network Load Balancer health checks.',
`Expected ${NLB_HEALTH_CHECK_TIMEOUTS[healthCheck.protocol]} seconds for ${healthCheck.protocol}, got ${healthCheck.timeout.toSeconds()}`,
].join(' '));

const lowHealthCheckTimeout = 2;
const highHealthCheckTimeout = 120;
if (healthCheck.timeout) {
tmyoda marked this conversation as resolved.
Show resolved Hide resolved
const timeoutSeconds = healthCheck.timeout.toSeconds();
if (timeoutSeconds < lowHealthCheckTimeout || timeoutSeconds > highHealthCheckTimeout) {
ret.push(`Health check timeout '${timeoutSeconds}' not supported. Must be a number between ${lowHealthCheckTimeout} and ${highHealthCheckTimeout}.`);
}
if (healthCheck.interval && healthCheck.interval.toSeconds() < timeoutSeconds) {
tmyoda marked this conversation as resolved.
Show resolved Hide resolved
ret.push(`Health check timeout '${timeoutSeconds}' must not be greater than the interval '${healthCheck.interval.toSeconds()}'`);
tmyoda marked this conversation as resolved.
Show resolved Hide resolved
}
}

return ret;
Expand Down Expand Up @@ -365,9 +371,4 @@ export interface INetworkLoadBalancerTarget {
}

const NLB_HEALTH_CHECK_PROTOCOLS = [Protocol.HTTP, Protocol.HTTPS, Protocol.TCP];
const NLB_PATH_HEALTH_CHECK_PROTOCOLS = [Protocol.HTTP, Protocol.HTTPS];
const NLB_HEALTH_CHECK_TIMEOUTS: { [protocol in Protocol]?: number } = {
[Protocol.HTTP]: 6,
[Protocol.HTTPS]: 10,
[Protocol.TCP]: 10,
};
const NLB_PATH_HEALTH_CHECK_PROTOCOLS = [Protocol.HTTP, Protocol.HTTPS];
Original file line number Diff line number Diff line change
Expand Up @@ -375,16 +375,40 @@ describe('tests', () => {
},
});

targetGroup.configureHealthCheck({
interval: cdk.Duration.seconds(150),
protocol: elbv2.Protocol.HTTP,
timeout: cdk.Duration.seconds(130),
});

// THEN
const validationErrors: string[] = targetGroup.node.validate();
const timeoutError = validationErrors.find((err) => /Health check timeout '130' not supported. Must be a number between/.test(err));
expect(timeoutError).toBeDefined();
});

test('validation error if Health check timeout is greater than the interval', () => {
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');
const lb = new elbv2.NetworkLoadBalancer(stack, 'LB', { vpc });
const listener = lb.addListener('PublicListener', { port: 80 });
const targetGroup = listener.addTargets('ECS', {
port: 80,
healthCheck: {
interval: cdk.Duration.seconds(60),
},
});

targetGroup.configureHealthCheck({
interval: cdk.Duration.seconds(30),
protocol: elbv2.Protocol.HTTP,
timeout: cdk.Duration.seconds(10),
timeout: cdk.Duration.seconds(40),
});

// THEN
const validationErrors: string[] = targetGroup.node.validate();
expect(validationErrors).toEqual([
'Custom health check timeouts are not supported for Network Load Balancer health checks. Expected 6 seconds for HTTP, got 10',
"Health check timeout '40' must not be greater than the interval '30'",
]);
});

Expand Down