From bf3764fe6046799a2a6e074b39b9a5f4474c603c Mon Sep 17 00:00:00 2001 From: Kazuho Cryer-Shinozuka Date: Fri, 13 Sep 2024 21:13:41 +0900 Subject: [PATCH] chore(elasticloadbalancingv2): add validation for `RedirectOptions.path` (#31202) ### Issue # (if applicable) Closes #31201 . ### Reason for this change If we set a string without a leading `/` for the path property, deployment will fail as shown below. Given: ```ts httpListener.addAction('RedirectAction', { conditions: [elbv2.ListenerCondition.pathPatterns(['/*'])], action: elbv2.ListenerAction.redirect({ protocol: elbv2.ApplicationProtocol.HTTPS, path: 'example/path? ', permanent: true }), }) ``` Result: ```sh Failed resources: MainStack-develop | 1:34:02 AM | CREATE_FAILED | AWS::ElasticLoadBalancingV2::ListenerRule | Alb/HttpListener/RedirectActionRule (AlbHttpListenerRedirectActionRule1D930694) Internal error reported from downstream service during operation 'The Path parameter must be a valid path, should start with a '/', and may contain up to one of each of these placeholders: '#{path}', '#{host}', '#{port}'. (Service: ElasticLoadBalancingV2, Status Code: 400, Request ID: fd4c7f01-9c97-44c1-a177-30ac04b2db26)'. ``` Related docs: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticloadbalancingv2-listenerrule-redirectconfig.html#cfn-elasticloadbalancingv2-listenerrule-redirectconfig-path ### Description of changes Add validation for `path` prop like below. ```ts if (options.path && !options.path.startsWith('/')) { throw new Error('Redirect path must start with a \'/\''); } ``` ### Description of how you validated changes Add unit test. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-elasticloadbalancingv2/README.md | 2 ++ .../lib/alb/application-listener-action.ts | 5 ++++- .../test/alb/actions.test.ts | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md index c410c1e45f9d3..18d7d24e3f5db 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md @@ -193,6 +193,8 @@ If you do not provide any options for this method, it redirects HTTP port 80 to By default all ingress traffic will be allowed on the source port. If you want to be more selective with your ingress rules then set `open: false` and use the listener's `connections` object to selectively grant access to the listener. +**Note**: The `path` parameter must start with a `/`. + ### Application Load Balancer attributes You can modify attributes of Application Load Balancers: diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts index 589c3f1f24cb7..9cc89cd125b1c 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener-action.ts @@ -2,7 +2,7 @@ import { Construct, IConstruct } from 'constructs'; import { IApplicationListener } from './application-listener'; import { IApplicationTargetGroup } from './application-target-group'; import { Port } from '../../../aws-ec2'; -import { Duration, SecretValue, Tokenization } from '../../../core'; +import { Duration, SecretValue, Token, Tokenization } from '../../../core'; import { CfnListener, CfnListenerRule } from '../elasticloadbalancingv2.generated'; import { IListenerAction } from '../shared/listener-action'; @@ -115,6 +115,9 @@ export class ListenerAction implements IListenerAction { if ([options.host, options.path, options.port, options.protocol, options.query].findIndex(x => x !== undefined) === -1) { throw new Error('To prevent redirect loops, set at least one of \'protocol\', \'host\', \'port\', \'path\', or \'query\'.'); } + if (options.path && !Token.isUnresolved(options.path) && !options.path.startsWith('/')) { + throw new Error(`Redirect path must start with a \'/\', got: ${options.path}`); + } return new ListenerAction({ type: 'redirect', diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/actions.test.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/actions.test.ts index c3b1d090ceb98..11c804a052032 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/actions.test.ts +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/actions.test.ts @@ -338,4 +338,19 @@ describe('tests', () => { ], }); }); + + test('throw error for invalid path pattern for redirect action', () => { + // GIVEN + const listener = lb.addListener('Listener', { port: 80 }); + + // THEN + expect(() => { + listener.addAction('RedirectAction', { + action: elbv2.ListenerAction.redirect({ + protocol: elbv2.ApplicationProtocol.HTTPS, + path: 'example', + }), + }); + }).toThrow('Redirect path must start with a \'/\', got: example'); + }); });