Skip to content

Commit b82320b

Browse files
authored
revert: "fix(elasticloadbalancerV2): logicalId supports switch from addTargetGroups (under feature flag)" (#29716)
Reverts #29513 We will want to use a property to achieve the desired behavior instead of a feature flag since we are not changing the default behavior.
1 parent 8e3848c commit b82320b

File tree

6 files changed

+7
-114
lines changed

6 files changed

+7
-114
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).
12011199
- Set `introducedIn.v2` to the literal string `'V2NEXT'`.
12021200
- Double negatives should be avoided. If you want to add a flag that disables something that was previously
12031201
enabled, set `default.v2` to `true` and the `recommendedValue` to `false`. You will need to update
12041202
a test in `features.test.ts` -- this is okay if you have a good reason.
1205-
In your description, be sure to cover the following:
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:
12061208
- Consciously pick the type of feature flag. Can the flag be removed in a future major version, or not?
12071209
- Motivate why the feature flag exists. What is the change to existing infrastructure and why is it not safe?
12081210
- 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

+2-9
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, FeatureFlags, Lazy, Resource, Token } from '../../../core';
10+
import { Duration, 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,22 +664,15 @@ 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.
673667
*/
674668
public addAction(id: string, props: AddApplicationActionProps): void {
675669
checkAddRuleProps(props);
676670

677671
if (props.priority !== undefined) {
678-
const idSuffix = FeatureFlags.of(this).isEnabled(cxapi.ALBV2_EXTERNALAPPLICATIONLISTENER_SWITCH_FROM_ADDTARGETGROUP_TO_ADDACTION) ? '' : 'Rule';
679672
// New rule
680673
//
681674
// TargetGroup.registerListener is called inside ApplicationListenerRule.
682-
new ApplicationListenerRule(this, id + idSuffix, {
675+
new ApplicationListenerRule(this, id + 'Rule', {
683676
listener: this,
684677
priority: props.priority,
685678
...props,

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

-35
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ 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';
109
import * as elbv2 from '../../lib';
1110
import { FakeSelfRegisteringTarget } from '../helpers';
1211

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

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-
17191684
test('not allowed to specify defaultTargetGroups and defaultAction together', () => {
17201685
// GIVEN
17211686
const stack = new cdk.Stack();

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

-17
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ 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) |
7270

7371
<!-- END table -->
7472

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

12691267

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-
12851268
<!-- END details -->

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

-26
Original file line numberDiff line numberDiff line change
@@ -309,29 +309,3 @@ _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,7 +101,6 @@ 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';
105104

106105
export const FLAGS: Record<string, FlagInfo> = {
107106
//////////////////////////////////////////////////////////////////////
@@ -1035,27 +1034,6 @@ export const FLAGS: Record<string, FlagInfo> = {
10351034
introducedIn: { v2: '2.134.0' },
10361035
recommendedValue: true,
10371036
},
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-
},
10591037
};
10601038

10611039
const CURRENT_MV = 'v2';

0 commit comments

Comments
 (0)