Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(elasticloadbalancerV2): consistent logicalId for addTargetGroups (under feature flag) #29712

Closed
wants to merge 1 commit into from
Closed
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 @@ -625,8 +625,9 @@ abstract class ExternalApplicationListener extends Resource implements IApplicat
checkAddRuleProps(props);

if (props.priority !== undefined) {
const idSuffix = FeatureFlags.of(this).isEnabled(cxapi.ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID) ? '' : 'Rule';
// New rule
new ApplicationListenerRule(this, id, {
new ApplicationListenerRule(this, id + idSuffix, {
listener: this,
priority: props.priority,
...props,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1714,6 +1714,38 @@ describe('tests', () => {
expect(applicationListenerRule).toBeDefined();
expect(applicationListenerRule!.node.id).toBe(identifierToken); // Should not have `Rule` suffix
});

test('backwards compatibility', () => {
// GIVEN
const context = { [cxapi.ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID]: true };
const app = new cdk.App({ context });
const stack = new cdk.Stack(app, 'stack', {
env: {
account: '123456789012',
region: 'us-west-2',
},
});
const vpc = new ec2.Vpc(stack, 'Stack');
const targetGroup = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', { vpc, port: 80 });
const listener = elbv2.ApplicationListener.fromLookup(stack, 'a', {
loadBalancerTags: {
some: 'tag',
},
});

// WHEN
const identifierToken = 'SuperMagicToken';
listener.addTargetGroups(identifierToken, {
conditions: [elbv2.ListenerCondition.pathPatterns(['/fake'])],
priority: 42,
targetGroups: [targetGroup],
});

// THEN
const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
expect(applicationListenerRule).toBeDefined();
expect(applicationListenerRule!.node.id).toBe(identifierToken); // Should NOT have `Rule` suffix
});
});

test('not allowed to specify defaultTargetGroups and defaultAction together', () => {
Expand Down
15 changes: 15 additions & 0 deletions packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Flags come in three types:
| [@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) |
| [@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,
without changing the logicalId and replacing your resource. | V2NEXT | (fix) |
| [@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId](#aws-cdkaws-elasticloadbalancingv2externalapplicationlistener-addtargetgrouplegacylogicalid) | When enabled, the `addTargetGroups()` method will use the legacy naming conventions (no `Rule` suffix) for `ListenerRule`s. | V2NEXT | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -1282,4 +1283,18 @@ This allows you to switch from the `addTargetGroups()` method without having Clo
| V2NEXT | `false` | `false` |


### aws-cdkaws-elasticloadbalancingv2externalapplicationlistener-addtargetgrouplegacylogicalid

*When enabled, the `addTargetGroups()` method will use the legacy naming conventions (no `Rule` suffix) for `ListenerRule`s.* (fix)

Setting this feature flag will case the `addTargetGroups()` method to revert to the legacy behavior and not add the `Rule` suffix on the logicalId for the `ListenerRule` it creates.
This allows you to upgrade to the latest version of the CDK without having CloudFormation deadlock while attempting to replace the resource.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `false` |


<!-- END details -->
19 changes: 7 additions & 12 deletions packages/aws-cdk-lib/cx-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -320,18 +320,13 @@ This will prevent `Rule` from being added as a suffix to the logicalId so that t
Do not enable this if you have already deployed `ListenerRule` resources using the
`addAction()` method.
Instead consider the [cdk-logical-id-mapper](https://github.com/mbonig/cdk-logical-id-mapper),
possibly in conjunction with `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupsConsistentLogicalId` (see below).
possibly in conjunction with `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId` (see below).

* `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupsConsistentLogicalId`
* `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId`

Enable this feature flag to ensure that the logicalIds of `ListenerRule`s created
on a `ExternalApplicationListener` by the `addTargetGroups()` method are consistent
with logicalIds for `ListenerRules` generated by other methods.
This will allow you to migrate between the different methods
without causing a replacement of the `ListenerRule` resource.
Enable this feature flag if you have deployed a `ListenerRule` using the `addTargetGroups()`
method against an `ExternalApplicationListener` and you want to upgrade to using the
latest version of aws-cdk. This will prevent cdk from adding the `Rule` suffix to the logicalId
of the `ListenerRule` involved so that CloudFormation will not deadlock while attempting to replace the resource.

You should enable this on new apps, before creating any resources.
If you have already created resources with the previous behavior,
you may still enable this flag, but will need to use something like the
[cdk-logical-id-mapper](https://github.com/mbonig/cdk-logical-id-mapper).
Alternatively, do not enable this feature flag and instead consider the `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction` as necessary.
Do not enable this feature flag for new deployments. May be used in conjunction with `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction`.
20 changes: 20 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/
export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';
export const ALBV2_EXTERNALAPPLICATIONLISTENER_SWITCH_FROM_ADDTARGETGROUP_TO_ADDACTION = '@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction';
export const ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID = '@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1056,6 +1057,25 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: 'V2NEXT' },
recommendedValue: false,
},

//////////////////////////////////////////////////////////////////////
[ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID]: {
type: FlagType.ApiDefault,
summary: 'When enabled, reverts to the legacy logicalId naming for \`ListenerRule\`s created by the \`addTargetGroups()\` method.',
compatibilityWithOldBehaviorMd: `Enable this feature flag to revert to the old behavior.`,
detailsMd: `
The legacy behavior for the \`addTargetGroups()\` method did not add the \`Rule\` suffix to the logicalId of the \`ListenerRule\`
which it generated. This was inconsistent with all the other methods which create \`ListenerRule\`s.

Replacing \`ListenerRule\`s is disruptive since they have a unique \`priority\`.
This means that when Cfn tries to create the new replacement, it will fail, blocking the upgrade path.

Setting this feature flag will cause the \`addTargetGroups()\` method to not add the \`Rule\` suffix on the logicalId.
This should only be set to support deployed infrastructure.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: false,
}
};

const CURRENT_MV = 'v2';
Expand Down
Loading