Skip to content

Commit

Permalink
fix(elasticloadbalancingv2): Incorrect validation on `NetworkLoadBala…
Browse files Browse the repository at this point in the history 
…ncer.configureHealthCheck()` (#16445)

## Summary

The [`NetworkLoadBalancer`'s](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-elasticloadbalancingv2.NetworkLoadBalancer.html) [`configureHealthCheck()`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-elasticloadbalancingv2.ApplicationTargetGroup.html#configurewbrhealthwbrcheckhealthcheck) method is incorrectly throwing a validation error when provided a valid `protocol` and the same value for both `interval` and `timeout`. 

```sh
Error: Healthcheck interval 10 seconds must be greater than the timeout 10 seconds
```

This rule only applies to Application Load Balancers and not Network Load Balancers.

This PR:

- Moves the mentioned validation logic from the `BaseTargetGroup` class to the `ApplicationTargetGroup` class.
- Adds tests that check a validation error is thrown when **invalid** `protocol`, `interval`, `timeout`, & `path` combinations are provided for the respected TargetGroup type.
- Adds tests that check a validation error is **not** thrown when **valid** `protocol`, `interval`, `timeout`, & `path` combinations are provided for the respected TargetGroup type.

Provides fix for SIM ticket: V427669955
Fixes issue: #16446

Refs:
- [The mentioned validation logic was implemented by this PR.](#16107)
- [CreateTargetGroup CloudFormation docs](https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_CreateTargetGroup.html)
- [Application Load Balancer's Health check configuration docs](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/target-group-health-checks.html)
- [Network Load Balancer's Health check configuration docs](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/target-group-health-checks.html)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@ryparker
ryparker committed Sep 29, 2021
1 parent f24a1ae commit 140892a
Show file tree
Hide file tree
Showing 4 changed files with 413 additions and 77 deletions.
20 changes: 15 additions & 5 deletions packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts
View file
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
7 changes: 1 addition & 6 deletions packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts
View file
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
193 changes: 168 additions & 25 deletions packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/target-group.test.ts
View file
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');
});
});

0 comments on commit 140892a

Please sign in to comment.