From 7b341d6a91988efbe648c59efe1b52c936a1db2b Mon Sep 17 00:00:00 2001 From: Sumu Date: Mon, 24 Apr 2023 10:51:32 -0400 Subject: [PATCH 1/6] add validation to LambdaRestApi class Signed-off-by: Sumu --- .../aws-apigateway/lib/lambda-api.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts index 179edc274ee21..8b64986a3fc91 100644 --- a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts +++ b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts @@ -1,4 +1,5 @@ import * as lambda from '../../aws-lambda'; +import * as cdk from '../../core'; import { Construct } from 'constructs'; import { LambdaIntegration, LambdaIntegrationOptions } from './integrations'; import { Method } from './method'; @@ -68,6 +69,23 @@ export class LambdaRestApi extends RestApi { this.root.addMethod = addMethodThrows; this.root.addProxy = addProxyThrows; } + + this.node.addValidation({ + validate() { + if (props.deployOptions?.variables && cdk.Resource.isOwnedResource(scope)) { + for (let key in props.deployOptions.variables) { + // Checks that variable Stage values match regex + const regexp = /[A-Za-z0-9-._~:/?#&=,]+/; + const value = props.deployOptions.variables[key]; + const matchesRegex = value.match(regexp); + if (!matchesRegex) { + return ['Stage variables value must match regex: ' + value + ' does not match ' + regexp + '.']; + } + } + } + return []; + }, + }); } } From fec283d8a87589e5e37bb17818a4a02a314faa56 Mon Sep 17 00:00:00 2001 From: Sumu Date: Mon, 24 Apr 2023 12:18:48 -0400 Subject: [PATCH 2/6] create unit test for changes Signed-off-by: Sumu --- .../aws-apigateway/test/lambda-api.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts b/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts index 334c601e40bbe..b3fdbe864491a 100644 --- a/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts +++ b/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts @@ -2,6 +2,7 @@ import { Match, Template } from '../../assertions'; import * as lambda from '../../aws-lambda'; import * as cdk from '../../core'; import * as apigw from '../lib'; +import { LambdaRestApi } from '../lib'; describe('lambda api', () => { test('LambdaRestApi defines a REST API with Lambda proxy integration', () => { @@ -405,4 +406,35 @@ describe('lambda api', () => { }, }); }); + + test('setting deployOptions variable with invalid value throws validation error', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app); + + const handler = new lambda.Function(stack, 'handler', { + handler: 'index.handler', + code: lambda.Code.fromInline('boom'), + runtime: lambda.Runtime.NODEJS_10_X, + }); + + const versionAlias = lambda.Version.fromVersionAttributes(stack, 'VersionInfo', { + lambda: handler, + version: '${stageVariables.lambdaAlias}', + }); + + // const api: RestApi = + new LambdaRestApi(stack, 'RestApi', { + restApiName: 'my-test-api', + handler: versionAlias, + deployOptions: { + variables: { + functionName: handler.functionName, + // lambdaAlias: handler.latestVersion, // == '$LATEST' + }, + }, + }); + + expect(() => app.synth()).toThrow(/Stage variables value must match regex: handler does not match [A-Za-z0-9-._~:/?#&=,]+./); + }); }); From fb3e958ae9662702c26c0f807292b35e6d23bcaf Mon Sep 17 00:00:00 2001 From: Sumu Date: Thu, 27 Apr 2023 14:14:02 -0400 Subject: [PATCH 3/6] fix validation and update test error message Signed-off-by: Sumu --- packages/@aws-cdk/cx-api/FEATURE_FLAGS.md | 21 ++++++++++++++++++- .../aws-apigateway/lib/lambda-api.ts | 9 ++++---- .../aws-apigateway/test/lambda-api.test.ts | 6 ++---- packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md | 21 ++++++++++++++++++- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md b/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md index 8162a400fa326..e96f3879769b0 100644 --- a/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md +++ b/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md @@ -17,6 +17,7 @@ Flags come in three types: | Flag | Summary | Since | Type | | ----- | ----- | ----- | ----- | +| [@aws-cdk/aws-apigateway:requestValidatorUniqueId](#aws-cdkaws-apigatewayrequestvalidatoruniqueid) | Generate a unique id for each RequestValidator added to a method | V2·NEXT | (fix) | | [@aws-cdk/aws-route53-patters:useCertificate](#aws-cdkaws-route53-pattersusecertificate) | Use the official `Certificate` resource instead of `DnsValidatedCertificate` | V2·NEXT | (default) | | [@aws-cdk/core:newStyleStackSynthesis](#aws-cdkcorenewstylestacksynthesis) | Switch to new stack synthesis method which enables CI/CD | 2.0.0 | (fix) | | [@aws-cdk/core:stackRelativeExports](#aws-cdkcorestackrelativeexports) | Name exports based on the construct paths relative to the stack, rather than the global construct path | 2.0.0 | (fix) | @@ -90,7 +91,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou "@aws-cdk/aws-ec2:launchTemplateDefaultUserData": true, "@aws-cdk/aws-secretsmanager:useAttachedSecretResourcePolicyForSecretTargetAttachments": true, "@aws-cdk/aws-redshift:columnId": true, - "@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2": true + "@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2": true, + "@aws-cdk/aws-apigateway:requestValidatorUniqueId": true } } ``` @@ -320,6 +322,23 @@ Encryption can also be configured explicitly using the `encrypted` property. **Compatibility with old behavior:** Pass the `encrypted: false` property to the `FileSystem` construct to disable encryption. +### @aws-cdk/aws-apigateway:requestValidatorUniqueId + +*Generate a unique id for each RequestValidator added to a method* (fix) + +This flag allows multiple RequestValidators to be added to a RestApi when +providing the `RequestValidatorOptions` in the `addMethod()` method. + +If the flag is not set then only a single RequestValidator can be added in this way. +Any additional RequestValidators have to be created directly with `new RequestValidator`. + + +| Since | Default | Recommended | +| ----- | ----- | ----- | +| (not in v1) | | | +| V2·NEXT | `false` | `true` | + + ### @aws-cdk/aws-route53-patters:useCertificate *Use the official `Certificate` resource instead of `DnsValidatedCertificate`* (default) diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts index 8b64986a3fc91..845f9caaf4bb6 100644 --- a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts +++ b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts @@ -1,10 +1,10 @@ -import * as lambda from '../../aws-lambda'; -import * as cdk from '../../core'; +// import * as cdk from '../../core'; import { Construct } from 'constructs'; import { LambdaIntegration, LambdaIntegrationOptions } from './integrations'; import { Method } from './method'; import { ProxyResource, Resource } from './resource'; import { RestApi, RestApiProps } from './restapi'; +import * as lambda from '../../aws-lambda'; export interface LambdaRestApiProps extends RestApiProps { /** @@ -72,13 +72,12 @@ export class LambdaRestApi extends RestApi { this.node.addValidation({ validate() { - if (props.deployOptions?.variables && cdk.Resource.isOwnedResource(scope)) { + if (props.deployOptions?.variables) { for (let key in props.deployOptions.variables) { // Checks that variable Stage values match regex const regexp = /[A-Za-z0-9-._~:/?#&=,]+/; const value = props.deployOptions.variables[key]; - const matchesRegex = value.match(regexp); - if (!matchesRegex) { + if (value.match(regexp) === null) { return ['Stage variables value must match regex: ' + value + ' does not match ' + regexp + '.']; } } diff --git a/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts b/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts index b3fdbe864491a..7b6d9beecfaee 100644 --- a/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts +++ b/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts @@ -423,18 +423,16 @@ describe('lambda api', () => { version: '${stageVariables.lambdaAlias}', }); - // const api: RestApi = new LambdaRestApi(stack, 'RestApi', { restApiName: 'my-test-api', handler: versionAlias, deployOptions: { variables: { - functionName: handler.functionName, - // lambdaAlias: handler.latestVersion, // == '$LATEST' + functionName: '$$$', }, }, }); - expect(() => app.synth()).toThrow(/Stage variables value must match regex: handler does not match [A-Za-z0-9-._~:/?#&=,]+./); + expect(() => app.synth()).toThrow('Validation failed with the following errors: \n[Default/RestApi] Stage variables value must match regex: $$$ does not match /[A-Za-z0-9-._~:/?#&=,]+/.'); }); }); diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index 8162a400fa326..e96f3879769b0 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md @@ -17,6 +17,7 @@ Flags come in three types: | Flag | Summary | Since | Type | | ----- | ----- | ----- | ----- | +| [@aws-cdk/aws-apigateway:requestValidatorUniqueId](#aws-cdkaws-apigatewayrequestvalidatoruniqueid) | Generate a unique id for each RequestValidator added to a method | V2·NEXT | (fix) | | [@aws-cdk/aws-route53-patters:useCertificate](#aws-cdkaws-route53-pattersusecertificate) | Use the official `Certificate` resource instead of `DnsValidatedCertificate` | V2·NEXT | (default) | | [@aws-cdk/core:newStyleStackSynthesis](#aws-cdkcorenewstylestacksynthesis) | Switch to new stack synthesis method which enables CI/CD | 2.0.0 | (fix) | | [@aws-cdk/core:stackRelativeExports](#aws-cdkcorestackrelativeexports) | Name exports based on the construct paths relative to the stack, rather than the global construct path | 2.0.0 | (fix) | @@ -90,7 +91,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou "@aws-cdk/aws-ec2:launchTemplateDefaultUserData": true, "@aws-cdk/aws-secretsmanager:useAttachedSecretResourcePolicyForSecretTargetAttachments": true, "@aws-cdk/aws-redshift:columnId": true, - "@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2": true + "@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2": true, + "@aws-cdk/aws-apigateway:requestValidatorUniqueId": true } } ``` @@ -320,6 +322,23 @@ Encryption can also be configured explicitly using the `encrypted` property. **Compatibility with old behavior:** Pass the `encrypted: false` property to the `FileSystem` construct to disable encryption. +### @aws-cdk/aws-apigateway:requestValidatorUniqueId + +*Generate a unique id for each RequestValidator added to a method* (fix) + +This flag allows multiple RequestValidators to be added to a RestApi when +providing the `RequestValidatorOptions` in the `addMethod()` method. + +If the flag is not set then only a single RequestValidator can be added in this way. +Any additional RequestValidators have to be created directly with `new RequestValidator`. + + +| Since | Default | Recommended | +| ----- | ----- | ----- | +| (not in v1) | | | +| V2·NEXT | `false` | `true` | + + ### @aws-cdk/aws-route53-patters:useCertificate *Use the official `Certificate` resource instead of `DnsValidatedCertificate`* (default) From 91e3be431e8d3e87137058d6abd10e255a5aa58a Mon Sep 17 00:00:00 2001 From: Sumu Date: Thu, 27 Apr 2023 14:31:38 -0400 Subject: [PATCH 4/6] fix typo in error message Signed-off-by: Sumu --- packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts | 2 +- packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts index 845f9caaf4bb6..5532b96ac2502 100644 --- a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts +++ b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts @@ -78,7 +78,7 @@ export class LambdaRestApi extends RestApi { const regexp = /[A-Za-z0-9-._~:/?#&=,]+/; const value = props.deployOptions.variables[key]; if (value.match(regexp) === null) { - return ['Stage variables value must match regex: ' + value + ' does not match ' + regexp + '.']; + return ['Stage variable value ' + value + ' does not match the regex.']; } } } diff --git a/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts b/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts index 7b6d9beecfaee..ea9c27b5a6139 100644 --- a/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts +++ b/packages/aws-cdk-lib/aws-apigateway/test/lambda-api.test.ts @@ -433,6 +433,6 @@ describe('lambda api', () => { }, }); - expect(() => app.synth()).toThrow('Validation failed with the following errors: \n[Default/RestApi] Stage variables value must match regex: $$$ does not match /[A-Za-z0-9-._~:/?#&=,]+/.'); + expect(() => app.synth()).toThrow('Validation failed with the following errors:\n [Default/RestApi] Stage variable value $$$ does not match the regex.'); }); }); From 74094e02d517081940bc6f570a0307f44d9eb06d Mon Sep 17 00:00:00 2001 From: Sumu Date: Mon, 1 May 2023 11:29:43 -0400 Subject: [PATCH 5/6] Cory suggestion + fix regex typo Signed-off-by: Sumu --- .../aws-cdk-lib/aws-apigateway/lib/lambda-api.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts index 5532b96ac2502..6dbea4c17a12f 100644 --- a/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts +++ b/packages/aws-cdk-lib/aws-apigateway/lib/lambda-api.ts @@ -72,14 +72,11 @@ export class LambdaRestApi extends RestApi { this.node.addValidation({ validate() { - if (props.deployOptions?.variables) { - for (let key in props.deployOptions.variables) { - // Checks that variable Stage values match regex - const regexp = /[A-Za-z0-9-._~:/?#&=,]+/; - const value = props.deployOptions.variables[key]; - if (value.match(regexp) === null) { - return ['Stage variable value ' + value + ' does not match the regex.']; - } + for (const value of Object.values(props.deployOptions?.variables ?? {})) { + // Checks that variable Stage values match regex + const regexp = /[A-Za-z0-9-._~:/?#&=,]+/; + if (value.match(regexp) === null) { + return ['Stage variable value ' + value + ' does not match the regex.']; } } return []; From 5492180050bc293456b3ac945191863a7f6ba3f7 Mon Sep 17 00:00:00 2001 From: Sumu Date: Mon, 1 May 2023 14:20:47 -0400 Subject: [PATCH 6/6] typo in stage docs - wrong regex Signed-off-by: Sumu --- packages/aws-cdk-lib/aws-apigateway/lib/stage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/stage.ts b/packages/aws-cdk-lib/aws-apigateway/lib/stage.ts index ea052cd6d765d..0a8561c8ab951 100644 --- a/packages/aws-cdk-lib/aws-apigateway/lib/stage.ts +++ b/packages/aws-cdk-lib/aws-apigateway/lib/stage.ts @@ -102,7 +102,7 @@ export interface StageOptions extends MethodDeploymentOptions { /** * A map that defines the stage variables. Variable names must consist of * alphanumeric characters, and the values must match the following regular - * expression: [A-Za-z0-9-._~:/?#&=,]+. + * expression: [A-Za-z0-9-._~:/?#&=,]+. * * @default - No stage variables. */