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

feat(elasticloadbalancingv2): add removeSuffix param for ExternalApplicationListener.addAction() #29746

Merged
merged 7 commits into from
Apr 17, 2024
Merged
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
13 changes: 13 additions & 0 deletions packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -765,3 +765,16 @@ const targetGroup = elbv2.ApplicationTargetGroup.fromTargetGroupAttributes(this,

const targetGroupMetrics: elbv2.IApplicationTargetGroupMetrics = targetGroup.metrics; // throws an Error()
```

## logicalIds on ExternalApplicationListener.addTargetGroups() and .addAction()

By default, the `addTargetGroups()` method does not follow the standard behavior
of adding a `Rule` suffix to the logicalId of the `ListenerRule` it creates.
If you are deploying new `ListenerRule`s using `addTargetGroups()` the recommendation
is to set the `removeRuleSuffixFromLogicalId: false` property.
If you have `ListenerRule`s deployed using the legacy behavior of `addTargetGroups()`,
which you need to switch over to being managed by the `addAction()` method,
then you will need to enable the `removeRuleSuffixFromLogicalId: true` property in the `addAction()` method.

`ListenerRule`s have a unique `priority` for a given `Listener`.
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.
Original file line number Diff line number Diff line change
Expand Up @@ -664,15 +664,21 @@ abstract class ExternalApplicationListener extends Resource implements IApplicat
* It is not possible to add a default action to an imported IApplicationListener.
* In order to add actions to an imported IApplicationListener a `priority`
* must be provided.
*
* If you previously deployed a `ListenerRule` using the `addTargetGroups()` method
* and need to migrate to using the more feature rich `addAction()`
* method, then you will need to set the `removeRuleSuffixFromLogicalId: true`
* property here to avoid having CloudFormation attempt to replace your resource.
*/
public addAction(id: string, props: AddApplicationActionProps): void {
checkAddRuleProps(props);

if (props.priority !== undefined) {
const ruleId = props.removeSuffix ? id : id + 'Rule';
// New rule
//
// TargetGroup.registerListener is called inside ApplicationListenerRule.
new ApplicationListenerRule(this, id + 'Rule', {
new ApplicationListenerRule(this, ruleId, {
listener: this,
priority: props.priority,
...props,
Expand Down Expand Up @@ -807,6 +813,19 @@ export interface AddApplicationActionProps extends AddRuleProps {
* Action to perform
*/
readonly action: ListenerAction;
/**
* `ListenerRule`s have a `Rule` suffix on their logicalId by default. This allows you to remove that suffix.
*
* Legacy behavior of the `addTargetGroups()` convenience method did not include the `Rule` suffix on the logicalId of the generated `ListenerRule`.
* At some point, increasing complexity of requirements can require users to switch from the `addTargetGroups()` method
* to the `addAction()` method.
* When migrating `ListenerRule`s deployed by a legacy version of `addTargetGroups()`,
* you will need to enable this flag to avoid changing the logicalId of your resource.
* Otherwise Cfn will attempt to replace the `ListenerRule` and fail.
*
* @default - use standard logicalId with the `Rule` suffix
*/
readonly removeSuffix?: boolean;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1698,6 +1698,47 @@ describe('tests', () => {
}).toThrow(/Specify at most one/);
});

describe('Rule suffix for logicalId', () => {
const identifierToken = 'SuperMagicToken';
interface TestCase {
readonly removeSuffix?: boolean;
readonly expectedLogicalId: string;
};
const nonDefaultTestCases: TestCase[] = [
{ removeSuffix: true, expectedLogicalId: identifierToken },
{ removeSuffix: false, expectedLogicalId: identifierToken + 'Rule' },
];
test.each<TestCase>([
// Default is consistent, which means it has the `Rule` suffix. This means no change from legacy behavior
{ removeSuffix: undefined, expectedLogicalId: identifierToken + 'Rule' },
...nonDefaultTestCases,
])('addAction %s', ({ removeSuffix, expectedLogicalId }) => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
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
listener.addAction(identifierToken, {
action: elbv2.ListenerAction.weightedForward([{ targetGroup, weight: 1 }]),
conditions: [elbv2.ListenerCondition.pathPatterns(['/fake'])],
priority: 42,
removeSuffix,
});

// THEN
const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
expect(applicationListenerRule).toBeDefined();
expect(applicationListenerRule!.node.id).toBe(expectedLogicalId);
});
});

describe('lookup', () => {
test('Can look up an ApplicationListener', () => {
// GIVEN
Expand Down