Skip to content

Commit

Permalink
fix(elbv2): connections not created for chained listener actions (#21939
Browse files Browse the repository at this point in the history
)

When you add an action to a listener the `bind` method is called, and one of the things that is typically done is to configure security group ingress. When you chain actions together, i.e.

```ts
listener.addAction('first-action', {
  action: ListenerAction.authenticateOidc({
    next: ListenerAction.forward([secondAction]),
    ...,
  }),
});
```

Bind is never called for the second action (i.e. `next`) which means the security group ingress rules are not created.

This PR updates the `ListenerAction.bind` method to call `bind` for any `next` action that is configured.

fixes #12994


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
corymhall authored Sep 8, 2022
1 parent ab76681 commit 46cf825
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,7 @@ export class ListenerAction implements IListenerAction {
* Called when the action is being used in a listener
*/
public bind(scope: Construct, listener: IApplicationListener, associatingConstruct?: IConstruct) {
// Empty on purpose
Array.isArray(scope);
Array.isArray(listener);
Array.isArray(associatingConstruct);
this.next?.bind(scope, listener, associatingConstruct);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Metric } from '@aws-cdk/aws-cloudwatch';
import * as ec2 from '@aws-cdk/aws-ec2';
import { describeDeprecated, testDeprecated } from '@aws-cdk/cdk-build-tools';
import * as cdk from '@aws-cdk/core';
import { SecretValue } from '@aws-cdk/core';
import * as constructs from 'constructs';
import * as elbv2 from '../../lib';
import { FakeSelfRegisteringTarget } from '../helpers';
Expand Down Expand Up @@ -261,6 +262,53 @@ describe('tests', () => {
});
});

test('bind is called for all next targets', () => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');
const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc });
const listener = lb.addListener('Listener', { port: 80 });
const fake = new FakeSelfRegisteringTarget(stack, 'FakeTG', vpc);
const group = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
port: 80,
targets: [fake],
});

// WHEN
listener.addAction('first-action', {
action: elbv2.ListenerAction.authenticateOidc({
next: elbv2.ListenerAction.forward([group]),
issuer: 'dummy',
clientId: 'dummy',
clientSecret: SecretValue.unsafePlainText('dummy'),
tokenEndpoint: 'dummy',
userInfoEndpoint: 'dummy',
authorizationEndpoint: 'dummy',
}),
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::EC2::SecurityGroupIngress', {
IpProtocol: 'tcp',
Description: 'Load balancer to target',
FromPort: 80,
ToPort: 80,
GroupId: {
'Fn::GetAtt': [
'FakeTGSG50E257DF',
'GroupId',
],
},
SourceSecurityGroupId: {
'Fn::GetAtt': [
'LBSecurityGroup8A41EA2B',
'GroupId',
],
},
});
});

testDeprecated('Can implicitly create target groups with and without conditions', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down

0 comments on commit 46cf825

Please sign in to comment.