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

fix(aws-s3): adding support for underscore (_) in S3 bucket names as legacy bucket names can have underscores in their names #22992

Closed
wants to merge 1 commit into from

Conversation

virajmavani
Copy link

This PR makes fix for issue: #22640


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Nov 19, 2022

@github-actions github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Nov 19, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team November 19, 2022 08:44
@virajmavani virajmavani marked this pull request as ready for review November 19, 2022 08:51
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: def1ca4
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@@ -1724,9 +1724,9 @@ export class Bucket extends BucketBase {
if (bucketName.length < 3 || bucketName.length > 63) {
errors.push('Bucket name must be at least 3 and no more than 63 characters');
}
const charsetMatch = bucketName.match(/[^a-z0-9.-]/);
const charsetMatch = bucketName.match(/[^a-z0-9._-]/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is used to validate the names of both imported buckets and buckets created in the app. But the underscore character should only be allowed when importing a bucket.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I see that the pre-2018 bucket naming rules are as below:

Before March 1, 2018, buckets created in the US East (N. Virginia) Region could have names that were up to 255 characters long and included uppercase letters and underscores. Beginning March 1, 2018, new buckets in US East (N. Virginia) must conform to the same rules applied in all other Regions.

Seeing that this is a unique case only in us-east-1 and also has a further validation conditions like a longer length and capital letters being allowed, I do not think it makes sense to add a very specific validation check for this scenario. Closing the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants