s3_bucket_name: add length validation

[davimmt]

From this discussion.

[bendrucker]

Can you avoid hardcoding this and find somewhere to reference it? AWS has a lot of schemas for their API that should specify this somewhere...

[bendrucker]

Hmm, seems not:

https://github.com/aws/aws-sdk-go/blob/2ac2bb7f4efd6406c91b30e668024928ba8a63bc/models/apis/s3/2006-03-01/api-2.json#L1350

[davimmt]

Yeah, it seems rather bad to hardcode, but I also haven't found somewhere to reference it from.

[bendrucker]

Seems like this could be added to the existing aws_s3_bucket_name rule. Ideally that would have been aws_s3_bucket_name_prefix and aws_s3_bucket_name would just enforce the official rules. In addition to the length limit, the rule should enforce all the other requirements.

[bendrucker]

Suggested change

	fmt.Sprintf(`Bucket name "%s" length must be within %d - %d character range`, name, bucketNameMinLength, bucketNameMaxLength),
	fmt.Sprintf("Bucket name %q must be between %d and %d characters", name, bucketNameMinLength, bucketNameMaxLength),

[davimmt]

Nice. I've just commited it through rebasing.

[davimmt]

Seems like this could be added to the existing aws_s3_bucket_name rule. Ideally that would have been aws_s3_bucket_name_prefix and aws_s3_bucket_name would just enforce the official rules. In addition to the length limit, the rule should enforce all the other requirements.

I thought about it and I think it's probably the right thing to do; however, for what I've seen, the aws_s3_bucket_name rule requires an explicit regex configuration and it's not enabled by default (as the bucket name length enforcement should always be), so I decided to make it separate.

[bendrucker]

Right, so:

requires an explicit regex configuration

That would need to become optional

it's not enabled by default

Then this can happen

[davimmt]

Yeah, that's right. I was hoping your feedback on this matter, if you agreed on using the same rule. I will work on it, then.

[davimmt]

Seems all right.

Have you found anything about the min and max numbers from the API?

Also, your opinion on this. The bucket name length should throw an Error severity, but that was not the case for the regex and prefix checks.

[bendrucker]

Thanks, will review and respond within the next 1-2 weeks (traveling)

[davimmt]

We could work with this, maybe:

https://github.com/aws/aws-sdk-go/blob/0f30069a590dd0a55ef0375fd9cb0d3b40284cd9/models/apis/cloudformation/2010-05-15/api-2.json#L3338-L3343

Or:

https://github.com/hashicorp/terraform-provider-aws/blob/c97d68f8f03a752ecf179f6ca0ef82fb50e4fedb/internal/service/s3/bucket.go#L94

If you search it like this, a lot of them reference the min max bucket name size.

Companies such as MinIO have this hardcoded:

https://github.com/minio/minio-go/blob/v7.0.63/pkg/s3utils/utils.go#L356-L361

[bendrucker]

Thanks for your patience!