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(elasticloadbalancingv2): Incorrect validation on NetworkLoadBalancer.configureHealthCheck() #16445

Merged
merged 7 commits into from
Sep 29, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,21 @@ export class ApplicationTargetGroup extends TargetGroupBase implements IApplicat
ret.push('At least one of \'port\' or \'protocol\' is required for a non-Lambda TargetGroup');
}

if (this.healthCheck && this.healthCheck.protocol && !ALB_HEALTH_CHECK_PROTOCOLS.includes(this.healthCheck.protocol)) {
ret.push([
`Health check protocol '${this.healthCheck.protocol}' is not supported. `,
`Must be one of [${ALB_HEALTH_CHECK_PROTOCOLS.join(', ')}]`,
].join(''));
if (this.healthCheck && this.healthCheck.protocol) {

if (ALB_HEALTH_CHECK_PROTOCOLS.includes(this.healthCheck.protocol)) {
if (this.healthCheck.interval && this.healthCheck.timeout &&
this.healthCheck.interval.toMilliseconds() <= this.healthCheck.timeout.toMilliseconds()) {
ret.push(`Healthcheck interval ${this.healthCheck.interval.toHumanString()} must be greater than the timeout ${this.healthCheck.timeout.toHumanString()}`);
}
}

if (!ALB_HEALTH_CHECK_PROTOCOLS.includes(this.healthCheck.protocol)) {
ret.push([
`Health check protocol '${this.healthCheck.protocol}' is not supported. `,
`Must be one of [${ALB_HEALTH_CHECK_PROTOCOLS.join(', ')}]`,
].join(''));
}
}

return ret;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ export abstract class TargetGroupBase extends CoreConstruct implements ITargetGr
healthyThresholdCount: cdk.Lazy.number({ produce: () => this.healthCheck?.healthyThresholdCount }),
unhealthyThresholdCount: cdk.Lazy.number({ produce: () => this.healthCheck?.unhealthyThresholdCount }),
matcher: cdk.Lazy.any({
produce: () => this.healthCheck?.healthyHttpCodes !== undefined || this.healthCheck?.healthyGrpcCodes !== undefined ? {
produce: () => this.healthCheck?.healthyHttpCodes !== undefined || this.healthCheck?.healthyGrpcCodes !== undefined ? {
grpcCode: this.healthCheck.healthyGrpcCodes,
httpCode: this.healthCheck.healthyHttpCodes,
} : undefined,
Expand Down Expand Up @@ -297,11 +297,6 @@ export abstract class TargetGroupBase extends CoreConstruct implements ITargetGr
* Set/replace the target group's health check
*/
public configureHealthCheck(healthCheck: HealthCheck) {
if (healthCheck.interval && healthCheck.timeout) {
if (healthCheck.interval.toMilliseconds() <= healthCheck.timeout.toMilliseconds()) {
throw new Error(`Healthcheck interval ${healthCheck.interval.toHumanString()} must be greater than the timeout ${healthCheck.timeout.toHumanString()}`);
}
}
this.healthCheck = healthCheck;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,44 +282,187 @@ describe('tests', () => {
});
});

test('Interval equal to timeout', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});
test.each([elbv2.Protocol.UDP, elbv2.Protocol.TCP_UDP, elbv2.Protocol.TLS])(
'Throws validation error, when `healthCheck` has `protocol` set to %s',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});

// WHEN
new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
healthCheck: {
protocol: protocol,
},
});

// WHEN
const tg = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
// THEN
expect(() => {
app.synth();
}).toThrow(`Health check protocol '${protocol}' is not supported. Must be one of [HTTP, HTTPS]`);
});

// THEN
expect(() => {
test.each([elbv2.Protocol.UDP, elbv2.Protocol.TCP_UDP, elbv2.Protocol.TLS])(
'Throws validation error, when `configureHealthCheck()` has `protocol` set to %s',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});
const tg = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
});

// WHEN
tg.configureHealthCheck({
protocol: protocol,
});

// THEN
expect(() => {
app.synth();
}).toThrow(`Health check protocol '${protocol}' is not supported. Must be one of [HTTP, HTTPS]`);
});

test.each([elbv2.Protocol.HTTP, elbv2.Protocol.HTTPS])(
'Does not throw validation error, when `healthCheck` has `protocol` set to %s',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});

// WHEN
new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
healthCheck: {
protocol: protocol,
},
});

// THEN
expect(() => {
app.synth();
}).not.toThrowError();
});

test.each([elbv2.Protocol.HTTP, elbv2.Protocol.HTTPS])(
'Does not throw validation error, when `configureHealthCheck()` has `protocol` set to %s',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});
const tg = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
});

// WHEN
tg.configureHealthCheck({
protocol: protocol,
});

// THEN
expect(() => {
app.synth();
}).not.toThrowError();
});

test.each([elbv2.Protocol.HTTP, elbv2.Protocol.HTTPS])(
'Throws validation error, when `healthCheck` has `protocol` set to %s and `interval` is equal to `timeout`',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});

// WHEN
new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
healthCheck: {
interval: cdk.Duration.seconds(60),
timeout: cdk.Duration.seconds(60),
protocol: protocol,
},
});

// THEN
expect(() => {
app.synth();
}).toThrow('Healthcheck interval 1 minute must be greater than the timeout 1 minute');
});

test.each([elbv2.Protocol.HTTP, elbv2.Protocol.HTTPS])(
'Throws validation error, when `healthCheck` has `protocol` set to %s and `interval` is smaller than `timeout`',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});

// WHEN
new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
healthCheck: {
interval: cdk.Duration.seconds(60),
timeout: cdk.Duration.seconds(120),
protocol: protocol,
},
});

// THEN
expect(() => {
app.synth();
}).toThrow('Healthcheck interval 1 minute must be greater than the timeout 2 minutes');
});

test.each([elbv2.Protocol.HTTP, elbv2.Protocol.HTTPS])(
'Throws validation error, when `configureHealthCheck()` has `protocol` set to %s and `interval` is equal to `timeout`',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});
const tg = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
});

// WHEN
tg.configureHealthCheck({
interval: cdk.Duration.seconds(60),
timeout: cdk.Duration.seconds(60),
protocol: protocol,
});
}).toThrow(/Healthcheck interval 1 minute must be greater than the timeout 1 minute/);
});

test('Interval smaller than timeout', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});

// WHEN
const tg = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
// THEN
expect(() => {
app.synth();
}).toThrow('Healthcheck interval 1 minute must be greater than the timeout 1 minute');
});

// THEN
expect(() => {
test.each([elbv2.Protocol.HTTP, elbv2.Protocol.HTTPS])(
'Throws validation error, when `configureHealthCheck()` has `protocol` set to %s and `interval` is smaller than `timeout`',
(protocol) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'VPC', {});
const tg = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
});

// WHEN
tg.configureHealthCheck({
interval: cdk.Duration.seconds(60),
timeout: cdk.Duration.seconds(120),
protocol: protocol,
});
}).toThrow(/Healthcheck interval 1 minute must be greater than the timeout 2 minutes/);
});

// THEN
expect(() => {
app.synth();
}).toThrow('Healthcheck interval 1 minute must be greater than the timeout 2 minutes');
});
});
Loading