Skip to content

Commit f4af330

Browse files
authored
feat(elasticloadbalancingv2): add removeSuffix param for ExternalApplicationListener.addAction() (#29746)
### Issue # (if applicable) Closes #29496 ### Reason for this change See #29513 (props based solution instead of feature-flag) ### Description of changes Adds a `removeSuffix` property to addAction method to address problems due to logicalId inconsistency. ### Description of how you validated changes UTs. Per IT document, integration tests are not necessary. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 0da25e5 commit f4af330

File tree

3 files changed

+74
-1
lines changed

3 files changed

+74
-1
lines changed

packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md

+13
Original file line numberDiff line numberDiff line change
@@ -765,3 +765,16 @@ const targetGroup = elbv2.ApplicationTargetGroup.fromTargetGroupAttributes(this,
765765

766766
const targetGroupMetrics: elbv2.IApplicationTargetGroupMetrics = targetGroup.metrics; // throws an Error()
767767
```
768+
769+
## logicalIds on ExternalApplicationListener.addTargetGroups() and .addAction()
770+
771+
By default, the `addTargetGroups()` method does not follow the standard behavior
772+
of adding a `Rule` suffix to the logicalId of the `ListenerRule` it creates.
773+
If you are deploying new `ListenerRule`s using `addTargetGroups()` the recommendation
774+
is to set the `removeRuleSuffixFromLogicalId: false` property.
775+
If you have `ListenerRule`s deployed using the legacy behavior of `addTargetGroups()`,
776+
which you need to switch over to being managed by the `addAction()` method,
777+
then you will need to enable the `removeRuleSuffixFromLogicalId: true` property in the `addAction()` method.
778+
779+
`ListenerRule`s have a unique `priority` for a given `Listener`.
780+
Because the `priority` must be unique, CloudFormation will always fail when creating a new `ListenerRule` to replace the existing one, unless you change the `priority` as well as the logicalId.

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

+20-1
Original file line numberDiff line numberDiff line change
@@ -664,15 +664,21 @@ abstract class ExternalApplicationListener extends Resource implements IApplicat
664664
* It is not possible to add a default action to an imported IApplicationListener.
665665
* In order to add actions to an imported IApplicationListener a `priority`
666666
* must be provided.
667+
*
668+
* If you previously deployed a `ListenerRule` using the `addTargetGroups()` method
669+
* and need to migrate to using the more feature rich `addAction()`
670+
* method, then you will need to set the `removeRuleSuffixFromLogicalId: true`
671+
* property here to avoid having CloudFormation attempt to replace your resource.
667672
*/
668673
public addAction(id: string, props: AddApplicationActionProps): void {
669674
checkAddRuleProps(props);
670675

671676
if (props.priority !== undefined) {
677+
const ruleId = props.removeSuffix ? id : id + 'Rule';
672678
// New rule
673679
//
674680
// TargetGroup.registerListener is called inside ApplicationListenerRule.
675-
new ApplicationListenerRule(this, id + 'Rule', {
681+
new ApplicationListenerRule(this, ruleId, {
676682
listener: this,
677683
priority: props.priority,
678684
...props,
@@ -807,6 +813,19 @@ export interface AddApplicationActionProps extends AddRuleProps {
807813
* Action to perform
808814
*/
809815
readonly action: ListenerAction;
816+
/**
817+
* `ListenerRule`s have a `Rule` suffix on their logicalId by default. This allows you to remove that suffix.
818+
*
819+
* Legacy behavior of the `addTargetGroups()` convenience method did not include the `Rule` suffix on the logicalId of the generated `ListenerRule`.
820+
* At some point, increasing complexity of requirements can require users to switch from the `addTargetGroups()` method
821+
* to the `addAction()` method.
822+
* When migrating `ListenerRule`s deployed by a legacy version of `addTargetGroups()`,
823+
* you will need to enable this flag to avoid changing the logicalId of your resource.
824+
* Otherwise Cfn will attempt to replace the `ListenerRule` and fail.
825+
*
826+
* @default - use standard logicalId with the `Rule` suffix
827+
*/
828+
readonly removeSuffix?: boolean;
810829
}
811830

812831
/**

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/listener.test.ts

+41
Original file line numberDiff line numberDiff line change
@@ -1698,6 +1698,47 @@ describe('tests', () => {
16981698
}).toThrow(/Specify at most one/);
16991699
});
17001700

1701+
describe('Rule suffix for logicalId', () => {
1702+
const identifierToken = 'SuperMagicToken';
1703+
interface TestCase {
1704+
readonly removeSuffix?: boolean;
1705+
readonly expectedLogicalId: string;
1706+
};
1707+
const nonDefaultTestCases: TestCase[] = [
1708+
{ removeSuffix: true, expectedLogicalId: identifierToken },
1709+
{ removeSuffix: false, expectedLogicalId: identifierToken + 'Rule' },
1710+
];
1711+
test.each<TestCase>([
1712+
// Default is consistent, which means it has the `Rule` suffix. This means no change from legacy behavior
1713+
{ removeSuffix: undefined, expectedLogicalId: identifierToken + 'Rule' },
1714+
...nonDefaultTestCases,
1715+
])('addAction %s', ({ removeSuffix, expectedLogicalId }) => {
1716+
// GIVEN
1717+
const app = new cdk.App();
1718+
const stack = new cdk.Stack(app, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
1719+
const vpc = new ec2.Vpc(stack, 'Stack');
1720+
const targetGroup = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', { vpc, port: 80 });
1721+
const listener = elbv2.ApplicationListener.fromLookup(stack, 'a', {
1722+
loadBalancerTags: {
1723+
some: 'tag',
1724+
},
1725+
});
1726+
1727+
// WHEN
1728+
listener.addAction(identifierToken, {
1729+
action: elbv2.ListenerAction.weightedForward([{ targetGroup, weight: 1 }]),
1730+
conditions: [elbv2.ListenerCondition.pathPatterns(['/fake'])],
1731+
priority: 42,
1732+
removeSuffix,
1733+
});
1734+
1735+
// THEN
1736+
const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
1737+
expect(applicationListenerRule).toBeDefined();
1738+
expect(applicationListenerRule!.node.id).toBe(expectedLogicalId);
1739+
});
1740+
});
1741+
17011742
describe('lookup', () => {
17021743
test('Can look up an ApplicationListener', () => {
17031744
// GIVEN

0 commit comments

Comments
 (0)