Skip to content

Commit 8e3848c

Browse files
authored
fix(elasticloadbalancerV2): logicalId supports switch from addTargetGroups (under feature flag) (#29513)
### Issue ELBv2 logicalId inconsistency of ApplicationListenerRule logicalIds Mitigates #29496 ### Reason for this change People using ALBs who need to migrate from the `addTargetGroups()` convenience method to the lower level `addAction()` method should not be blocked due to inconsistent logicalId's. Further, the logicalIds should be consistent going forward. ### Description of changes There are two feature flags, one which sets a migration compat mode and another which fixed the behaviour to be consistent. ### Description of how you validated changes Unit testing. ### 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 36fd79d commit 8e3848c

File tree

6 files changed

+114
-7
lines changed

6 files changed

+114
-7
lines changed

CONTRIBUTING.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -1196,18 +1196,18 @@ Adding a new flag looks as follows:
11961196
with the name of the context key that enables this new feature (for
11971197
example, `ENABLE_STACK_NAME_DUPLICATES`). The context key should be in the
11981198
form `module.Type:feature` (e.g. `@aws-cdk/core:enableStackNameDuplicates`).
1199+
2. Add your feature flag to the `FLAGS` map in
1200+
[cx-api/lib/features.ts](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/cx-api/lib/features.ts).
11991201
- Set `introducedIn.v2` to the literal string `'V2NEXT'`.
12001202
- Double negatives should be avoided. If you want to add a flag that disables something that was previously
12011203
enabled, set `default.v2` to `true` and the `recommendedValue` to `false`. You will need to update
12021204
a test in `features.test.ts` -- this is okay if you have a good reason.
1203-
2. Use `FeatureFlags.of(construct).isEnabled(cxapi.ENABLE_XXX)` to check if this feature is enabled
1204-
in your code. If it is not defined, revert to the legacy behavior.
1205-
3. Add your feature flag to the `FLAGS` map in
1206-
[cx-api/lib/features.ts](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/cx-api/lib/features.ts). In
1207-
your description, be sure to cover the following:
1205+
In your description, be sure to cover the following:
12081206
- Consciously pick the type of feature flag. Can the flag be removed in a future major version, or not?
12091207
- Motivate why the feature flag exists. What is the change to existing infrastructure and why is it not safe?
12101208
- In case of a "default change flag", describe what the user needs to do to restore the old behavior.
1209+
3. Use `FeatureFlags.of(construct).isEnabled(cxapi.ENABLE_XXX)` to check if this feature is enabled
1210+
in your code. If it is not defined, revert to the legacy behavior.
12111211
4. Add an entry for your feature flag in the [README](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/cx-api/README.md) file.
12121212
5. In your tests, ensure that you test your feature with and without the feature flag enabled. You can do this by passing the feature flag to the `context` property when instantiating an `App`.
12131213
```ts

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

+9-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { ApplicationTargetGroup, IApplicationLoadBalancerTarget, IApplicationTar
77
import { ListenerCondition } from './conditions';
88
import * as ec2 from '../../../aws-ec2';
99
import * as cxschema from '../../../cloud-assembly-schema';
10-
import { Duration, Lazy, Resource, Token } from '../../../core';
10+
import { Duration, FeatureFlags, Lazy, Resource, Token } from '../../../core';
1111
import * as cxapi from '../../../cx-api';
1212
import { BaseListener, BaseListenerLookupOptions, IListener } from '../shared/base-listener';
1313
import { HealthCheck } from '../shared/base-target-group';
@@ -664,15 +664,22 @@ 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+
* Warning, if you are attempting to migrate an existing `ListenerAction`
669+
* which was declared by the {@link addTargetGroups} method, you will
670+
* need to enable the
671+
* `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction`
672+
* feature flag.
667673
*/
668674
public addAction(id: string, props: AddApplicationActionProps): void {
669675
checkAddRuleProps(props);
670676

671677
if (props.priority !== undefined) {
678+
const idSuffix = FeatureFlags.of(this).isEnabled(cxapi.ALBV2_EXTERNALAPPLICATIONLISTENER_SWITCH_FROM_ADDTARGETGROUP_TO_ADDACTION) ? '' : 'Rule';
672679
// New rule
673680
//
674681
// TargetGroup.registerListener is called inside ApplicationListenerRule.
675-
new ApplicationListenerRule(this, id + 'Rule', {
682+
new ApplicationListenerRule(this, id + idSuffix, {
676683
listener: this,
677684
priority: props.priority,
678685
...props,

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

+35
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Metric } from '../../../aws-cloudwatch';
66
import * as ec2 from '../../../aws-ec2';
77
import * as cdk from '../../../core';
88
import { SecretValue } from '../../../core';
9+
import * as cxapi from '../../../cx-api';
910
import * as elbv2 from '../../lib';
1011
import { FakeSelfRegisteringTarget } from '../helpers';
1112

@@ -1681,6 +1682,40 @@ describe('tests', () => {
16811682
}).toThrow(/specify only one/);
16821683
});
16831684

1685+
describe('ExternalApplicationListener logicalId support', () => {
1686+
1687+
test('compatibility mode for addAction', () => {
1688+
// GIVEN
1689+
const context = { [cxapi.ALBV2_EXTERNALAPPLICATIONLISTENER_SWITCH_FROM_ADDTARGETGROUP_TO_ADDACTION]: true };
1690+
const app = new cdk.App({ context });
1691+
const stack = new cdk.Stack(app, 'stack', {
1692+
env: {
1693+
account: '123456789012',
1694+
region: 'us-west-2',
1695+
},
1696+
});
1697+
const vpc = new ec2.Vpc(stack, 'Stack');
1698+
const targetGroup = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', { vpc, port: 80 });
1699+
const listener = elbv2.ApplicationListener.fromLookup(stack, 'a', {
1700+
loadBalancerTags: {
1701+
some: 'tag',
1702+
},
1703+
});
1704+
// WHEN
1705+
const identifierToken = 'SuperMagicToken';
1706+
listener.addAction(identifierToken, {
1707+
action: elbv2.ListenerAction.weightedForward([{ targetGroup, weight: 1 }]),
1708+
conditions: [elbv2.ListenerCondition.pathPatterns(['/fake'])],
1709+
priority: 42,
1710+
});
1711+
1712+
// THEN
1713+
const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
1714+
expect(applicationListenerRule).toBeDefined();
1715+
expect(applicationListenerRule!.node.id).toBe(identifierToken); // Should not have `Rule` suffix
1716+
});
1717+
});
1718+
16841719
test('not allowed to specify defaultTargetGroups and defaultAction together', () => {
16851720
// GIVEN
16861721
const stack = new cdk.Stack();

packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md

+17
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ Flags come in three types:
6767
| [@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse](#aws-cdkaws-codepipelinecrossaccountkeysdefaultvaluetofalse) | Enables Pipeline to set the default value for crossAccountKeys to false. | 2.127.0 | (default) |
6868
| [@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2](#aws-cdkaws-codepipelinedefaultpipelinetypetov2) | Enables Pipeline to set the default pipeline type to V2. | 2.133.0 | (default) |
6969
| [@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope](#aws-cdkaws-kmsreducecrossaccountregionpolicyscope) | When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only. | 2.134.0 | (fix) |
70+
| [@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction](#aws-cdkaws-elasticloadbalancingv2externalapplicationlistener-norulesuffixforaddaction) | When enabled, you can switch from the `addTargetGroups()` method of declaring a `ListenerRule` to the `addAction()` method,
71+
without changing the logicalId and replacing your resource. | V2NEXT | (fix) |
7072

7173
<!-- END table -->
7274

@@ -1265,4 +1267,19 @@ When this feature flag is enabled and calling KMS key grant method, the created
12651267
| 2.134.0 | `false` | `true` |
12661268

12671269

1270+
### @aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction
1271+
1272+
*When enabled, you can switch from the `addTargetGroups()` method of declaring a `ListenerRule` to the `addAction()` method,
1273+
without changing the logicalId and replacing your resource.* (fix)
1274+
1275+
Setting this feature flag will cause the `addAction()` method to not add the `Rule` suffix on the logicalId.
1276+
This allows you to switch from the `addTargetGroups()` method without having CloudFormation deadlock while attempting to replace the resource.
1277+
1278+
1279+
| Since | Default | Recommended |
1280+
| ----- | ----- | ----- |
1281+
| (not in v1) | | |
1282+
| V2NEXT | `false` | `false` |
1283+
1284+
12681285
<!-- END details -->

packages/aws-cdk-lib/cx-api/README.md

+26
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,29 @@ _cdk.json_
309309
}
310310
}
311311
```
312+
313+
* `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction`
314+
315+
Enable this feature flag if you have deployed a `ListenerRule` using the `addTargetGroups()`
316+
convenience method against an `ExternalApplicationListener` and you need to migrate to
317+
using the `addAction()` method for more complex rule configurations.
318+
This will prevent `Rule` from being added as a suffix to the logicalId so that the logicalId will remain the same.
319+
320+
Do not enable this if you have already deployed `ListenerRule` resources using the
321+
`addAction()` method.
322+
Instead consider the [cdk-logical-id-mapper](https://github.com/mbonig/cdk-logical-id-mapper),
323+
possibly in conjunction with `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupsConsistentLogicalId` (see below).
324+
325+
* `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupsConsistentLogicalId`
326+
327+
Enable this feature flag to ensure that the logicalIds of `ListenerRule`s created
328+
on a `ExternalApplicationListener` by the `addTargetGroups()` method are consistent
329+
with logicalIds for `ListenerRules` generated by other methods.
330+
This will allow you to migrate between the different methods
331+
without causing a replacement of the `ListenerRule` resource.
332+
333+
You should enable this on new apps, before creating any resources.
334+
If you have already created resources with the previous behavior,
335+
you may still enable this flag, but will need to use something like the
336+
[cdk-logical-id-mapper](https://github.com/mbonig/cdk-logical-id-mapper).
337+
Alternatively, do not enable this feature flag and instead consider the `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction` as necessary.

packages/aws-cdk-lib/cx-api/lib/features.ts

+22
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export const LAMBDA_PERMISSION_LOGICAL_ID_FOR_LAMBDA_ACTION = '@aws-cdk/aws-clou
101101
export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse';
102102
export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
103103
export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';
104+
export const ALBV2_EXTERNALAPPLICATIONLISTENER_SWITCH_FROM_ADDTARGETGROUP_TO_ADDACTION = '@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction';
104105

105106
export const FLAGS: Record<string, FlagInfo> = {
106107
//////////////////////////////////////////////////////////////////////
@@ -1034,6 +1035,27 @@ export const FLAGS: Record<string, FlagInfo> = {
10341035
introducedIn: { v2: '2.134.0' },
10351036
recommendedValue: true,
10361037
},
1038+
1039+
//////////////////////////////////////////////////////////////////////
1040+
[ALBV2_EXTERNALAPPLICATIONLISTENER_SWITCH_FROM_ADDTARGETGROUP_TO_ADDACTION]: {
1041+
type: FlagType.VisibleContext,
1042+
summary: 'When enabled, you can switch from the \`addTargetGroups()\` method of declaring a \`ListenerRule\` to the \`addAction()\` method, without changing the logicalId and replacing your resource.',
1043+
detailsMd: `
1044+
When switching from a less complex to a more complex use of ALB,
1045+
you will eventually need features not available in the \`addTargetGroups()\` convenience method.
1046+
In this case you will want to use the \`addAction()\` method.
1047+
Before this feature is enabled, switching over to \`addAction()\` from using \`addTargetGroups()\`
1048+
will add a \`Rule\` suffix to the logicalId of your \`ListenerRule\`,
1049+
causing CloudFormation to attempt to replace the resource.
1050+
Since \`ListenerRule\`s have a unique priority,
1051+
CloudFormation will always fail when generating the new \`ListenerRule\`.
1052+
1053+
Setting this feature flag will cause the \`addAction()\` method to not add the \`Rule\` suffix on the logicalId.
1054+
This allows you to switch from the \`addTargetGroups()\` method without having CloudFormation deadlock while attempting to replace the resource.
1055+
`,
1056+
introducedIn: { v2: 'V2NEXT' },
1057+
recommendedValue: false,
1058+
},
10371059
};
10381060

10391061
const CURRENT_MV = 'v2';

0 commit comments

Comments
 (0)