From 00a7f033f6ad19160a7350784243ecf9c71c388b Mon Sep 17 00:00:00 2001 From: Tatsuya Mori Date: Sat, 19 Aug 2023 03:32:10 +0900 Subject: [PATCH] fix(backup): validation for vault name fails when parameters are referred in the name (#25943) Validation for Backup vault name fails when parameters are referred in the name now. Current implementation simply validates vault name with regular expression described in [CFn reference](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-backup-backupvault.html#cfn-backup-backupvault-backupvaultname), so it does not consider the special characters for parameters. This PR solves the issue by checking props.backupVaultName is resolved. Closes #21735 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../cdk-backup.assets.json | 4 +- .../cdk-backup.template.json | 34 ++++- .../test/integ.backup.js.snapshot/integ.json | 2 +- .../integ.backup.js.snapshot/manifest.json | 16 ++- .../test/integ.backup.js.snapshot/tree.json | 131 ++++++++++++------ .../test/aws-backup/test/integ.backup.ts | 13 +- packages/aws-cdk-lib/aws-backup/lib/vault.ts | 4 +- .../aws-cdk-lib/aws-backup/test/vault.test.ts | 17 ++- 8 files changed, 165 insertions(+), 56 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.assets.json index ebc526f444743..19ddf51ac30c3 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.assets.json @@ -1,7 +1,7 @@ { "version": "32.0.0", "files": { - "cf2d89a4eac2c90005b9a1e142f1cac80cf9cd29d1b8f98f002b9f3a1849454c": { + "0c52c355c71ac95690274d7987110017ff9cd1a1bc79fa4206fda2f55d6b62d5": { "source": { "path": "cdk-backup.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "cf2d89a4eac2c90005b9a1e142f1cac80cf9cd29d1b8f98f002b9f3a1849454c.json", + "objectKey": "0c52c355c71ac95690274d7987110017ff9cd1a1bc79fa4206fda2f55d6b62d5.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.template.json index 577d8efd46f37..2722b98da789d 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/cdk-backup.template.json @@ -3,16 +3,16 @@ "TableCD117FA1": { "Type": "AWS::DynamoDB::Table", "Properties": { - "KeySchema": [ + "AttributeDefinitions": [ { "AttributeName": "id", - "KeyType": "HASH" + "AttributeType": "S" } ], - "AttributeDefinitions": [ + "KeySchema": [ { "AttributeName": "id", - "AttributeType": "S" + "KeyType": "HASH" } ], "ProvisionedThroughput": { @@ -50,6 +50,27 @@ "UpdateReplacePolicy": "Delete", "DeletionPolicy": "Delete" }, + "ThirdVault3441C01E": { + "Type": "AWS::Backup::BackupVault", + "Properties": { + "BackupVaultName": { + "Fn::Join": [ + "", + [ + "backupVault-", + { + "Ref": "Env" + } + ] + ] + }, + "LockConfiguration": { + "MinRetentionDays": 5 + } + }, + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" + }, "PlanDAF4E53A": { "Type": "AWS::Backup::BackupPlan", "Properties": { @@ -234,6 +255,11 @@ } }, "Parameters": { + "Env": { + "Type": "String", + "Default": "test", + "Description": "Env" + }, "BootstrapVersion": { "Type": "AWS::SSM::Parameter::Value", "Default": "/cdk-bootstrap/hnb659fds/version", diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/integ.json b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/integ.json index 0a26289ba381b..266124ac58c12 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/integ.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/integ.json @@ -1,5 +1,5 @@ { - "version": "32.0.0", + "version": "33.0.0", "testCases": { "integ.backup": { "stacks": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/manifest.json index 210b015870fa2..aa4d73d1faac8 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "32.0.0", + "version": "33.0.0", "artifacts": { "cdk-backup.assets": { "type": "cdk:asset-manifest", @@ -17,7 +17,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/cf2d89a4eac2c90005b9a1e142f1cac80cf9cd29d1b8f98f002b9f3a1849454c.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/0c52c355c71ac95690274d7987110017ff9cd1a1bc79fa4206fda2f55d6b62d5.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ @@ -57,6 +57,18 @@ "data": "SecondaryVault67665B5E" } ], + "/cdk-backup/Env": [ + { + "type": "aws:cdk:logicalId", + "data": "Env" + } + ], + "/cdk-backup/ThirdVault/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "ThirdVault3441C01E" + } + ], "/cdk-backup/Plan/Resource": [ { "type": "aws:cdk:logicalId", diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/tree.json index e367a29532898..3728b496ca8cf 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.js.snapshot/tree.json @@ -18,16 +18,16 @@ "attributes": { "aws:cdk:cloudformation:type": "AWS::DynamoDB::Table", "aws:cdk:cloudformation:props": { - "keySchema": [ + "attributeDefinitions": [ { "attributeName": "id", - "keyType": "HASH" + "attributeType": "S" } ], - "attributeDefinitions": [ + "keySchema": [ { "attributeName": "id", - "attributeType": "S" + "keyType": "HASH" } ], "provisionedThroughput": { @@ -37,22 +37,22 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_dynamodb.CfnTable", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "ScalingRole": { "id": "ScalingRole", "path": "cdk-backup/Table/ScalingRole", "constructInfo": { - "fqn": "aws-cdk-lib.Resource", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_dynamodb.Table", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "FileSystem": { @@ -63,8 +63,8 @@ "aws:cdk:cloudformation:props": {} }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_efs.CfnFileSystem", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Vault": { @@ -84,14 +84,14 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.CfnBackupVault", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.BackupVault", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "SecondaryVault": { @@ -111,14 +111,59 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.CfnBackupVault", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.BackupVault", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Env": { + "id": "Env", + "path": "cdk-backup/Env", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "ThirdVault": { + "id": "ThirdVault", + "path": "cdk-backup/ThirdVault", + "children": { + "Resource": { + "id": "Resource", + "path": "cdk-backup/ThirdVault/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::Backup::BackupVault", + "aws:cdk:cloudformation:props": { + "backupVaultName": { + "Fn::Join": [ + "", + [ + "backupVault-", + { + "Ref": "Env" + } + ] + ] + }, + "lockConfiguration": { + "minRetentionDays": 5 + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Plan": { @@ -205,8 +250,8 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.CfnBackupPlan", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Selection": { @@ -221,8 +266,8 @@ "id": "ImportRole", "path": "cdk-backup/Plan/Selection/Role/ImportRole", "constructInfo": { - "fqn": "aws-cdk-lib.Resource", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Resource": { @@ -260,14 +305,14 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_iam.CfnRole", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_iam.Role", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Resource": { @@ -349,42 +394,42 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.CfnBackupSelection", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.BackupSelection", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_backup.BackupPlan", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "BootstrapVersion": { "id": "BootstrapVersion", "path": "cdk-backup/BootstrapVersion", "constructInfo": { - "fqn": "aws-cdk-lib.CfnParameter", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "CheckBootstrapVersion": { "id": "CheckBootstrapVersion", "path": "cdk-backup/CheckBootstrapVersion", "constructInfo": { - "fqn": "aws-cdk-lib.CfnRule", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.Stack", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Tree": { @@ -392,13 +437,13 @@ "path": "Tree", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.2.26" + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.App", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } } \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.ts index 32ff7b52f454b..d5138ced8bc41 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-backup/test/integ.backup.ts @@ -1,6 +1,6 @@ import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; import * as efs from 'aws-cdk-lib/aws-efs'; -import { App, Duration, RemovalPolicy, Stack, StackProps } from 'aws-cdk-lib'; +import { App, Duration, RemovalPolicy, Stack, StackProps, CfnParameter } from 'aws-cdk-lib'; import { Construct } from 'constructs'; import * as backup from 'aws-cdk-lib/aws-backup'; @@ -31,6 +31,17 @@ class TestStack extends Stack { minRetention: Duration.days(5), }, }); + + const env = new CfnParameter(this, 'Env', { type: 'String', description: 'Env', default: 'test' }); + + new backup.BackupVault(this, 'ThirdVault', { + removalPolicy: RemovalPolicy.DESTROY, + backupVaultName: `backupVault-${env.valueAsString}`, + lockConfiguration: { + minRetention: Duration.days(5), + }, + }); + const plan = backup.BackupPlan.dailyWeeklyMonthly5YearRetention(this, 'Plan', vault); plan.addSelection('Selection', { diff --git a/packages/aws-cdk-lib/aws-backup/lib/vault.ts b/packages/aws-cdk-lib/aws-backup/lib/vault.ts index ac530bafab2f1..8208cf6c37aba 100644 --- a/packages/aws-cdk-lib/aws-backup/lib/vault.ts +++ b/packages/aws-cdk-lib/aws-backup/lib/vault.ts @@ -3,7 +3,7 @@ import { CfnBackupVault } from './backup.generated'; import * as iam from '../../aws-iam'; import * as kms from '../../aws-kms'; import * as sns from '../../aws-sns'; -import { ArnFormat, Duration, IResource, Lazy, Names, RemovalPolicy, Resource, Stack } from '../../core'; +import { ArnFormat, Duration, IResource, Lazy, Names, RemovalPolicy, Resource, Stack, Token } from '../../core'; /** * A backup vault @@ -270,7 +270,7 @@ export class BackupVault extends BackupVaultBase { constructor(scope: Construct, id: string, props: BackupVaultProps = {}) { super(scope, id); - if (props.backupVaultName && !/^[a-zA-Z0-9\-_]{2,50}$/.test(props.backupVaultName)) { + if (props.backupVaultName && !Token.isUnresolved(props.backupVaultName) && !/^[a-zA-Z0-9\-_]{2,50}$/.test(props.backupVaultName)) { throw new Error('Expected vault name to match pattern `^[a-zA-Z0-9\-_]{2,50}$`'); } diff --git a/packages/aws-cdk-lib/aws-backup/test/vault.test.ts b/packages/aws-cdk-lib/aws-backup/test/vault.test.ts index dc2f5c50bd2c4..46cb2b4a18be2 100644 --- a/packages/aws-cdk-lib/aws-backup/test/vault.test.ts +++ b/packages/aws-cdk-lib/aws-backup/test/vault.test.ts @@ -2,7 +2,7 @@ import { Template } from '../../assertions'; import * as iam from '../../aws-iam'; import * as kms from '../../aws-kms'; import * as sns from '../../aws-sns'; -import { ArnFormat, Duration, Stack } from '../../core'; +import { ArnFormat, Duration, Stack, Fn } from '../../core'; import { BackupVault, BackupVaultEvents } from '../lib'; let stack: Stack; @@ -303,6 +303,21 @@ test('import from name', () => { })); }); +test('specify imported value as vault name', () => { + // WHEN + const vaultName = Fn.importValue('VaultName'); + new BackupVault(stack, 'Vault', { + backupVaultName: vaultName, + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::Backup::BackupVault', { + BackupVaultName: { + 'Fn::ImportValue': 'VaultName', + }, + }); +}); + test('grant action', () => { // GIVEN const vaultName = 'myVaultName';