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

provider/aws: StateFunc on db_subnet_group name to be lowercase #4340

Merged
merged 1 commit into from
Dec 16, 2015

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Dec 16, 2015

Fixes: #4255

We actually excluded uppercase chars on the DBSubnetName. This was causing a plan loop and would have been hidden by using a StateFunc to LowerCase

We actually throw a validate error now on any uppercase letters

TF_LOG=1 make testacc TEST=./builtin/providers/aws TESTARGS='-run=AWSDBSubnetGroup' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=AWSDBSubnetGroup -timeout 90m
=== RUN   TestAccAWSDBSubnetGroup_basic
--- PASS: TestAccAWSDBSubnetGroup_basic (27.01s)
=== RUN   TestAccAWSDBSubnetGroup_withUndocumentedCharacters
--- PASS: TestAccAWSDBSubnetGroup_withUndocumentedCharacters (28.28s)
=== RUN   TestResourceAWSDBSubnetGroupNameValidation
--- PASS: TestResourceAWSDBSubnetGroupNameValidation (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    55.307s

@@ -149,7 +149,7 @@ resource "aws_subnet" "bar" {
}

resource "aws_db_subnet_group" "foo" {
name = "FOO"
name = "foo"
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick... shouldn't this be OK to leave as FOO? The StateFunc should downcast it, so really leaving it as caps or mixed case would test that we're actually doing this.

@stack72
Copy link
Contributor Author

stack72 commented Dec 16, 2015

@catsby I was just talking to @jen20 about this. When I did that, I got this:

=== RUN   TestAccAWSDBSubnetGroup_basic
--- FAIL: TestAccAWSDBSubnetGroup_basic (20.85s)
    testing.go:144: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        DESTROY/CREATE: aws_db_subnet_group.foo
          arn:                   "arn:aws:rds:us-west-2:946579370547:subgrp:FOO" => "<computed>"
          description:           "foo description" => "foo description"
          name:                  "FOO" => "foo" (forces new resource)
          subnet_ids.#:          "2" => "2"
          subnet_ids.2708023650: "subnet-6f92b718" => "subnet-6f92b718"
          subnet_ids.3026020190: "subnet-9b6c6afe" => "subnet-9b6c6afe"
          tags.#:                "1" => "1"
          tags.Name:             "tf-dbsubnet-group-test" => "tf-dbsubnet-group-test"

        STATE:

        aws_db_subnet_group.foo:
          ID = FOO
          arn = arn:aws:rds:us-west-2:946579370547:subgrp:FOO
          description = foo description
          name = FOO
          subnet_ids.# = 2
          subnet_ids.2708023650 = subnet-6f92b718
          subnet_ids.3026020190 = subnet-9b6c6afe
          tags.# = 1
          tags.Name = tf-dbsubnet-group-test

          Dependencies:
            aws_subnet.foo
            aws_subnet.bar
        aws_subnet.bar:
          ID = subnet-6f92b718
          availability_zone = us-west-2b
          cidr_block = 10.1.2.0/24
          map_public_ip_on_launch = false
          tags.# = 1
          tags.Name = tf-dbsubnet-test-2
          vpc_id = vpc-245e6341

          Dependencies:
            aws_vpc.foo
        aws_subnet.foo:
          ID = subnet-9b6c6afe
          availability_zone = us-west-2a
          cidr_block = 10.1.1.0/24
          map_public_ip_on_launch = false
          tags.# = 1
          tags.Name = tf-dbsubnet-test-1
          vpc_id = vpc-245e6341

          Dependencies:
            aws_vpc.foo
        aws_vpc.foo:
          ID = vpc-245e6341
          cidr_block = 10.1.0.0/16
          default_network_acl_id = acl-96f5cbf3
          default_security_group_id = sg-34497050
          dhcp_options_id = dopt-889435e3
          main_route_table_id = rtb-f87e739d
          tags.# = 0

@catsby
Copy link
Contributor

catsby commented Dec 16, 2015

So... that's indication that the code isn't doing it's job, correct? The plan output you received is the same as the OP's, a perpetual diff. I'll pull the code down and take a peek 😄

@stack72
Copy link
Contributor Author

stack72 commented Dec 16, 2015

@catsby the actual code seems to work. Currently, when I tail the logs, I see this:

CREATE: aws_db_subnet_group.foo
  arn:          "" => "<computed>"
  description:  "" => "foo description" (forces new resource)
  name:         "" => "foo" (forces new resource)
  subnet_ids.#: "" => "<computed>"
  tags.#:       "" => "1"
  tags.Name:    "" => "tf-dbsubnet-group-test"

So I see it working there. I am wondering if this is to do with the testing framework

@catsby
Copy link
Contributor

catsby commented Dec 16, 2015

My state file after running a config file shows FOO for the name, so something is missing... digging 😄

@stack72 stack72 force-pushed the f-aws-db_subnet_group_name-downcase branch from 47c4e1f to 2406c4b Compare December 16, 2015 15:58
@jen20
Copy link
Contributor

jen20 commented Dec 16, 2015

Actually after discussing with @catsby, it might be better to just adjust the validation here to ensure the name is lower case as prescribed by the AWS documentation. Anyone who has already applied a plan with the upper case name will see a plan loop until they change their configuration, but future users will be prevented from falling into this.

@catsby
Copy link
Contributor

catsby commented Dec 16, 2015

My state file after running a config file shows FOO for the name, so something is missing... digging 😄

We found this to be an issue where we were later re-setting the name in the Read method:

d.Set("name", d.Id())
d.Set("description", *subnetGroup.DBSubnetGroupDescription)

@stack72 is fixing things up 😄

@stack72 stack72 force-pushed the f-aws-db_subnet_group_name-downcase branch from 2406c4b to 8c47b3c Compare December 16, 2015 16:13
this happens, throw a validation error

Add some ValidationTests for the DBSubnetGroupName ValidateFunc
@stack72 stack72 force-pushed the f-aws-db_subnet_group_name-downcase branch from 8c47b3c to 57bcb49 Compare December 16, 2015 16:33
@jen20
Copy link
Contributor

jen20 commented Dec 16, 2015

LGTM. Thanks @stack72!

jen20 added a commit that referenced this pull request Dec 16, 2015
…case

provider/aws: StateFunc on db_subnet_group name to be lowercase
@jen20 jen20 merged commit 19e68da into hashicorp:master Dec 16, 2015
@stack72 stack72 deleted the f-aws-db_subnet_group_name-downcase branch December 19, 2015 16:48
@ghost
Copy link

ghost commented Apr 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants