Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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 @@ -404,6 +404,14 @@ export class ApplicationTargetGroup extends TargetGroupBase implements IApplicat
this.setAttribute('load_balancing.algorithm.anomaly_mitigation', props.enableAnomalyMitigation ? 'on' : 'off');
}
}

// Add a warning if neither protocol nor port was explicitly specified for non-Lambda targets
if (props.protocol === undefined && props.port === undefined && this.targetType !== TargetType.LAMBDA) {
Annotations.of(this).addWarningV2(
'@aws-cdk/aws-elbv2:target-group-protocol-default',
'ApplicationTargetGroup protocol and port not specified. Please specify either protocol (e.g., ApplicationProtocol.HTTP or ApplicationProtocol.HTTPS) or port number. Note: Missing both will cause deployment failures.',
);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation should remain in the validateTargetGroup method to maintain consistency with existing validation patterns. This change weakens validation by converting an error to a warning and moves logic outside the dedicated validation method.

I noticed also that the original validation logic was incorrect (used || instead of &&). We can fix this since, changing it to && logic actually loosens the validation, so it's not a breaking change.

We can do:

  1. Fix the existing error validation in validateTargetGroup (change || to && logic)
  2. Add the warning annotation as supplementary feedback within validateTargetGroup when targetType is undefined


public get metrics(): IApplicationTargetGroupMetrics {
Expand Down Expand Up @@ -614,10 +622,8 @@ export class ApplicationTargetGroup extends TargetGroupBase implements IApplicat
protected validateTargetGroup(): string[] {
const ret = super.validateTargetGroup();

if (this.targetType !== undefined && this.targetType !== TargetType.LAMBDA
&& (this.protocol === undefined || this.port === undefined)) {
ret.push('At least one of \'port\' or \'protocol\' is required for a non-Lambda TargetGroup');
}
// Note: Protocol/port validation is now handled as a warning in the constructor
// instead of a hard validation error to maintain backward compatibility

if (this.healthCheck) {
if (this.healthCheck.interval && this.healthCheck.timeout &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,91 @@ describe('tests', () => {
expect(() => app.synth()).toThrow(/port\/protocol should not be specified for Lambda targets/);
});

test('ApplicationTargetGroup shows warning when neither protocol nor port specified', () => {
// GIVEN
const vpc = new ec2.Vpc(stack, 'Vpc');

// WHEN - Create ApplicationTargetGroup without protocol or port
new elbv2.ApplicationTargetGroup(stack, 'TG', {
vpc,
// Neither protocol nor port specified - this should generate a warning
});

// THEN - Should synthesize successfully but show warning
const assembly = app.synth();
const stackArtifact = assembly.getStackByName(stack.stackName);
const warnings = stackArtifact.messages.filter(m => m.level === 'warning');
expect(warnings.some(w => w.entry.data && typeof w.entry.data === 'string' &&
w.entry.data.includes('ApplicationTargetGroup protocol and port not specified'))).toBe(true);
});

test('ApplicationTargetGroup with port but no protocol should not show warning', () => {
// GIVEN
const vpc = new ec2.Vpc(stack, 'Vpc');

// WHEN - Create ApplicationTargetGroup without protocol but with port
new elbv2.ApplicationTargetGroup(stack, 'TG', {
vpc,
port: 80, // Providing port satisfies the requirement - protocol will be determined automatically
});

// THEN - Should synthesize successfully without warning
const assembly = app.synth();
const stackArtifact = assembly.getStackByName(stack.stackName);
const warnings = stackArtifact.messages.filter(m => m.level === 'warning');
expect(warnings.some(w => w.entry.data && typeof w.entry.data === 'string' &&
w.entry.data.includes('ApplicationTargetGroup protocol and port not specified'))).toBe(false);
});

test('ApplicationTargetGroup with protocol should not throw validation error', () => {
// GIVEN
const vpc = new ec2.Vpc(stack, 'Vpc');

// WHEN - Create ApplicationTargetGroup with protocol
new elbv2.ApplicationTargetGroup(stack, 'TG', {
vpc,
port: 80,
protocol: elbv2.ApplicationProtocol.HTTP,
});

// THEN - Should not throw
expect(() => app.synth()).not.toThrow();
});

test('ApplicationTargetGroup with Lambda target does not require protocol', () => {
// GIVEN

// WHEN - Create ApplicationTargetGroup for Lambda without protocol
new elbv2.ApplicationTargetGroup(stack, 'TG', {
targetType: elbv2.TargetType.LAMBDA,
// protocol is missing but should be allowed for Lambda
});

// THEN - Should not throw
expect(() => app.synth()).not.toThrow();
});

test('ApplicationTargetGroup requires protocol or port even when targets added later', () => {
// GIVEN
const vpc = new ec2.Vpc(stack, 'Vpc');

// WHEN - Create ApplicationTargetGroup without protocol or port, then add non-Lambda target
const tg = new elbv2.ApplicationTargetGroup(stack, 'TG', {
vpc,
// Neither protocol nor port specified - this should generate a warning
});

// Add a non-Lambda target
tg.addTarget(new elbv2.InstanceTarget('i-1234'));

// THEN - Should synthesize successfully but show warning
const assembly = app.synth();
const stackArtifact = assembly.getStackByName(stack.stackName);
const warnings = stackArtifact.messages.filter(m => m.level === 'warning');
expect(warnings.some(w => w.entry.data && typeof w.entry.data === 'string' &&
w.entry.data.includes('ApplicationTargetGroup protocol and port not specified'))).toBe(true);
});

test('Can add self-registering target to imported TargetGroup', () => {
// GIVEN
const vpc = new ec2.Vpc(stack, 'Vpc');
Expand Down
Loading