Skip to content

Commit

Permalink
provider/aws: fix db_subnet acc test
Browse files Browse the repository at this point in the history
AWS accepts uppercase DB Subnet Group names - it just automatically
downcases them. We already had logic to handle that - so we
intentionally had an acctest with uppercase characters that was now
failing.

Loosening the regexp to allow uppercase letters for now - we can discuss
if we want to tighten the validation as a separate question.

/cc @radeksimko @catsby
  • Loading branch information
phinze committed Jun 30, 2015
1 parent 5798034 commit 8fa96d2
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions builtin/providers/aws/resource_aws_db_subnet_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func resourceAwsDbSubnetGroup() *schema.Resource {
Required: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)
if !regexp.MustCompile(`^[0-9a-z-]+$`).MatchString(value) {
if !regexp.MustCompile(`^[0-9A-Za-z-]+$`).MatchString(value) {
errors = append(errors, fmt.Errorf(
"only lowercase alphanumeric characters and hyphens allowed in %q", k))
"only alphanumeric characters and hyphens allowed in %q", k))
}
if len(value) > 255 {
errors = append(errors, fmt.Errorf(
Expand Down

3 comments on commit 8fa96d2

@radeksimko
Copy link
Member

Choose a reason for hiding this comment

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

Loosening the regexp to allow uppercase letters for now - we can discuss
if we want to tighten the validation as a separate question.

I'm personally 👍 for tightening the validation, that was basically one of the original reasons. I think that making things lowercase under the hood just hides that problem away. User should be aware of what's happening - i.e. the definition in DSL should be IMO as close to reality as possible with as little magic in the middle as possible.

@phinze
Copy link
Contributor Author

@phinze phinze commented on 8fa96d2 Jun 30, 2015

Choose a reason for hiding this comment

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

@radeksimko yeah I was just in "fix the tests" mode - I'd be okay with that. We'd just need to clean up the extra code for the downcase comparison as well.

@radeksimko
Copy link
Member

Choose a reason for hiding this comment

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

@phinze Understood. If tighten, then properly 😃

Please sign in to comment.