Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws-backup: Attribute BackupVault.props.backupVaultName fails a validation when used with parameters #21735

Closed
igormukhin opened this issue Aug 24, 2022 · 5 comments · Fixed by #25943
Labels
@aws-cdk/aws-backup Related AWS Backup bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@igormukhin
Copy link

Describe the bug

Trying to put a CF-parameter into the name of backup vault fails.

Expected Behavior

should run (allow the usage of tokens/parameters)

Current Behavior

fails with:

Error: Expected vault name to match pattern `^[a-zA-Z0-9-_]{2,50}$`
    at new BackupVault (D:\Sources\HRAR\iac\node_modules\aws-cdk-lib\aws-backup\lib\vault.js:1:2076)
    ...

Reproduction Steps

Run:

const stage = new CfnParameter(this, 'Stage', { type: 'String', description: 'Stage' });

new backup.BackupVault(this, 'BackupVault', {
    backupVaultName: `backupVault-${stage.valueAsString}`
});

Possible Solution

Remove the regEx validation from https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-backup/lib/vault.ts:264

if (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}$`');
}

I think the validation is misplaced here, because it should support expressions like bla-${Token[TOKEN.285]}-bla.

Additional Information/Context

No response

CDK CLI Version

2.38.1 (build a5ced21)

Framework Version

2.38.1

Node.js Version

v16.15.1

OS

Windows 10

Language

Typescript

Language Version

4.7.4

Other information

No response

@igormukhin igormukhin added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 24, 2022
@github-actions github-actions bot added the @aws-cdk/aws-backup Related AWS Backup label Aug 24, 2022
@igormukhin
Copy link
Author

igormukhin commented Aug 24, 2022

Workaround:

const stage = new CfnParameter(this, 'Stage', { type: 'String', description: 'Stage' });

new backup.BackupVault(this, 'BackupVault', {
    backupVaultName: `WILL-REPLACE-BELOW`
});

const cfnBackupVault = backupVault.node.findChild('Resource') as backup.CfnBackupVault;
cfnBackupVault.backupVaultName = `backupVault-${stage.valueAsString}`;

@kaizencc
Copy link
Contributor

Whoops the validation needs to check if there is a token inside before running the regex. Something like if:

if (props.backupVaultName && !Token.isUnresolved(props.backupVaultName) && ...) {
  validate
}

@kaizencc kaizencc added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 24, 2022
@kaizencc kaizencc removed their assignment Aug 24, 2022
@heyosi
Copy link

heyosi commented Aug 30, 2022

Hi @kaizencc , I want to fix this issue, could you assign to me?

@tam0ri
Copy link
Contributor

tam0ri commented Jun 12, 2023

Previous PR was automatically closed, so I newly submitted PR.

@mergify mergify bot closed this as completed in #25943 Aug 18, 2023
mergify bot pushed a commit that referenced this issue Aug 18, 2023
…rred 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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-backup Related AWS Backup bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants