From ed8a5be882fecc14d30e11a887cb716724fda888 Mon Sep 17 00:00:00 2001 From: corymhall <43035978+corymhall@users.noreply.github.com> Date: Wed, 7 Sep 2022 12:21:14 +0000 Subject: [PATCH 1/2] fix(elbv2): connections not created for chained listener actions 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 --- .../lib/alb/application-listener-action.ts | 5 +- .../test/alb/listener.test.ts | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts index 6f7e72d0218ab..ed25ff04055d9 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts @@ -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); } /** diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts index d4095c93c5525..768ef98ab1fc5 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts @@ -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'; @@ -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.plainText('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(); From 112a5f9b7ecfad4cf2439c89b565bb47106b84d7 Mon Sep 17 00:00:00 2001 From: corymhall <43035978+corymhall@users.noreply.github.com> Date: Wed, 7 Sep 2022 14:23:34 +0000 Subject: [PATCH 2/2] fixing test --- .../aws-elasticloadbalancingv2/test/alb/listener.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts index 768ef98ab1fc5..e1e100db6e9ff 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts @@ -281,7 +281,7 @@ describe('tests', () => { next: elbv2.ListenerAction.forward([group]), issuer: 'dummy', clientId: 'dummy', - clientSecret: SecretValue.plainText('dummy'), + clientSecret: SecretValue.unsafePlainText('dummy'), tokenEndpoint: 'dummy', userInfoEndpoint: 'dummy', authorizationEndpoint: 'dummy',