Add default description #6104

joshuaspence · 2016-04-09T12:35:30Z

Ref #6100. Set the default value for the description field to be "Managed by Terraform".

This is my first time contributing so let me know if anything else is required. I know that I need to update the documentation, and I plan to do that next. I wasn't sure about testing this change because I couldn't see any existing tests that verify the default values.

stack72 · 2016-04-10T15:07:44Z

Hi @joshuaspence

thanks for the PR - currently it fails to build because of go fmt errors

Please can you run make fmt and fix the 2 test files (https://travis-ci.org/hashicorp/terraform/builds/121998198#L166)

Thanks

Paul

catsby · 2016-04-11T13:11:51Z

Hey @joshuaspence – I'm curious to know why all the changing of .bar to .test in so many of these test files? A quick glance also indicated there were tests added for this change to some resources, but not all. I imagine entirely new tests aren't needed, we could just remove the description from some current tests and verify that attribute in any of the other checks.

Let us know if you have any questions about this

joshuaspence · 2016-04-18T21:07:17Z

Sorry, I got a bit carried away. Let me tidy this up and finish it off.

joshuaspence · 2016-04-18T21:31:00Z

One thing that I wondered whilst working on this PR... should this change be a part of the Terraform core rather than being specific to the AWS plugin? As in, should the Terraform core provides some sort of default schema which says that "if there is a description field, it shall be optional and have a default value of 'Managed by Terraform'"? This also seems like it would be useful for #6099 as well.

stack72 · 2016-04-20T11:28:01Z

Hi @joshuaspence

Why are there changes to the vendor folder contents in this PR? What IDE are you using to work with go?

P.

joshuaspence · 2016-04-20T11:32:44Z

I mentioned this in #6227 as well... I just ran make fmt and it formatted the vendor/ directory. I'm assuming that this is not the intended behavior but I'm not sure what went wrong.

stack72 · 2016-04-20T11:35:18Z

yeah this isn't the intended behaviour - I am not sure what quite went wrong there

P.

joshuaspence · 2016-04-20T11:37:23Z

I'll just manually undo the vendor changes.

stack72 · 2016-04-20T11:42:31Z

<3 I'll get this PR tested today :)

stack72 · 2016-04-20T11:51:51Z

Test runs threw the following:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSDBParameterGroup' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSDBParameterGroup -timeout 120m
=== RUN   TestAccAWSDBParameterGroup_basic
--- FAIL: TestAccAWSDBParameterGroup_basic (16.94s)
    testing.go:154: Step 0 error: Check failed: Check 2/12 error: bad description: (*string)(0xc820481388)
=== RUN   TestAccAWSDBParameterGroup_Only
--- PASS: TestAccAWSDBParameterGroup_Only (19.16s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    36.125s

stack72 · 2016-04-20T11:52:31Z

This is because of this func

func testAccCheckAWSDBParameterGroupAttributes(v *rds.DBParameterGroup, name string) resource.TestCheckFunc {
    return func(s *terraform.State) error {

        if *v.DBParameterGroupName != name {
            return fmt.Errorf("Bad Parameter Group name, expected (%s), got (%s)", name, *v.DBParameterGroupName)
        }

        if *v.DBParameterGroupFamily != "mysql5.6" {
            return fmt.Errorf("bad family: %#v", v.DBParameterGroupFamily)
        }

        if *v.Description != "Test parameter group for terraform" {
            return fmt.Errorf("bad description: %#v", v.Description)
        }

        return nil
    }
}

stack72 · 2016-04-20T11:54:27Z

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSDBSubnetGroup' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSDBSubnetGroup -timeout 120m
=== RUN   TestAccAWSDBSubnetGroup_basic
--- PASS: TestAccAWSDBSubnetGroup_basic (45.76s)
=== RUN   TestAccAWSDBSubnetGroup_withUndocumentedCharacters
--- PASS: TestAccAWSDBSubnetGroup_withUndocumentedCharacters (49.55s)
=== RUN   TestAccAWSDBSubnetGroup_updateDescription
--- FAIL: TestAccAWSDBSubnetGroup_updateDescription (35.23s)
    testing.go:154: Step 0 error: Check failed: Check 2/2 error: aws_db_subnet_group.foo: Attribute 'description' expected "foo description", got "Managed by Terraform"
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    130.569s

stack72 · 2016-04-20T11:56:02Z

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSElasticacheParameterGroup' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSElasticacheParameterGroup -timeout 120m
=== RUN   TestAccAWSElasticacheParameterGroup_basic
--- FAIL: TestAccAWSElasticacheParameterGroup_basic (13.15s)
    testing.go:154: Step 0 error: Check failed: Check 2/7 error: bad description: (*string)(0xc82047c2f8)
=== RUN   TestAccAWSElasticacheParameterGroupOnly
--- PASS: TestAccAWSElasticacheParameterGroupOnly (13.94s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    27.111s

Due to this func

func testAccCheckAWSElasticacheParameterGroupAttributes(v *elasticache.CacheParameterGroup) resource.TestCheckFunc {
    return func(s *terraform.State) error {

        if *v.CacheParameterGroupName != "parameter-group-test-terraform" {
            return fmt.Errorf("bad name: %#v", v.CacheParameterGroupName)
        }

        if *v.CacheParameterGroupFamily != "redis2.8" {
            return fmt.Errorf("bad family: %#v", v.CacheParameterGroupFamily)
        }

        if *v.Description != "Test parameter group for terraform" {
            return fmt.Errorf("bad description: %#v", v.Description)
        }

        return nil
    }
}

stack72 · 2016-04-20T12:05:22Z

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSRedshiftSubnetGroup' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRedshiftSubnetGroup -timeout 120m
=== RUN   TestAccAWSRedshiftSubnetGroup_basic
--- PASS: TestAccAWSRedshiftSubnetGroup_basic (41.07s)
=== RUN   TestAccAWSRedshiftSubnetGroup_updateSubnetIds
--- FAIL: TestAccAWSRedshiftSubnetGroup_updateSubnetIds (66.61s)
    testing.go:154: Step 1 error: After applying this step and refreshing, the plan was not empty:

        DIFF:

        UPDATE: aws_redshift_subnet_group.foo
          description: "Managed by Terraform" => "foo description"

        STATE:

        aws_redshift_subnet_group.foo:
          ID = foo
          description = Managed by Terraform
          name = foo
          subnet_ids.# = 3
          subnet_ids.1651240058 = subnet-5a706403
          subnet_ids.3366685492 = subnet-80f859e4
          subnet_ids.361564745 = subnet-c5a258b3

          Dependencies:
            aws_subnet.foo
            aws_subnet.bar
            aws_subnet.foobar
        aws_subnet.bar:
          ID = subnet-c5a258b3
          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-c4d70ea0

          Dependencies:
            aws_vpc.foo
        aws_subnet.foo:
          ID = subnet-80f859e4
          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-c4d70ea0

          Dependencies:
            aws_vpc.foo
        aws_subnet.foobar:
          ID = subnet-5a706403
          availability_zone = us-west-2c
          cidr_block = 10.1.3.0/24
          map_public_ip_on_launch = false
          tags.# = 1
          tags.Name = tf-dbsubnet-test-3
          vpc_id = vpc-c4d70ea0

          Dependencies:
            aws_vpc.foo
        aws_vpc.foo:
          ID = vpc-c4d70ea0
          cidr_block = 10.1.0.0/16
          default_network_acl_id = acl-68d67e0c
          default_security_group_id = sg-71072116
          dhcp_options_id = dopt-60f40f05
          enable_classiclink = false
          main_route_table_id = rtb-ddd057b9
          tags.# = 0
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    107.705s

stack72 · 2016-04-20T12:05:46Z

@joshuaspence When these small issues with the tests are fixed - we can merge :)

This is good. Thanks for all the work here

Paul

stack72 · 2016-04-25T12:51:06Z

@joshuaspence how is this progressing? Need anything from me here?

P.

stack72 · 2016-05-13T16:47:32Z

pinging @joshuaspence on this - if you have not got any spare cycles on this atm I can help :)

Paul

joshuaspence · 2016-05-22T12:37:24Z

Sorry about the delay, let me take another look at this now.

Closes #6100. Set the default value for the `description` field to be "Managed by Terraform".

stack72 · 2016-05-23T11:09:58Z

Awesome! Thanks @joshuaspence. Look forward to it :)

joshuaspence · 2016-05-23T12:16:17Z

It should be ready now.

stack72 · 2016-05-23T12:37:37Z

Hi @joshuaspence

There are a few small test errors which i will fix up now and then merge :)

Thanks for this

Paul

catsby · 2016-06-02T19:37:33Z

This caused TestAccAWSElasticacheSecurityGroup_basic to fail because we didn't update the description in the test config

I patched it in e427880

ghost · 2020-04-25T01:59:51Z

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.

jen20 added enhancement provider/aws labels Apr 9, 2016

apparentlymart added the waiting-response An issue/pull request is waiting for a response from the community label Apr 17, 2016

stack72 self-assigned this Apr 20, 2016

Add default description

c193cbd

Closes #6100. Set the default value for the `description` field to be "Managed by Terraform".

stack72 removed the waiting-response An issue/pull request is waiting for a response from the community label May 23, 2016

stack72 merged commit c193cbd into hashicorp:master May 23, 2016

joshuaspence deleted the description branch June 16, 2016 12:15

ghost locked and limited conversation to collaborators Apr 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add default description #6104

Add default description #6104

joshuaspence commented Apr 9, 2016 •

edited

Loading

stack72 commented Apr 10, 2016

catsby commented Apr 11, 2016

joshuaspence commented Apr 18, 2016

joshuaspence commented Apr 18, 2016

stack72 commented Apr 20, 2016

joshuaspence commented Apr 20, 2016

stack72 commented Apr 20, 2016

joshuaspence commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 25, 2016

stack72 commented May 13, 2016

joshuaspence commented May 22, 2016

stack72 commented May 23, 2016

joshuaspence commented May 23, 2016

stack72 commented May 23, 2016

catsby commented Jun 2, 2016

ghost commented Apr 25, 2020

Add default description #6104

Add default description #6104

Conversation

joshuaspence commented Apr 9, 2016 • edited Loading

stack72 commented Apr 10, 2016

catsby commented Apr 11, 2016

joshuaspence commented Apr 18, 2016

joshuaspence commented Apr 18, 2016

stack72 commented Apr 20, 2016

joshuaspence commented Apr 20, 2016

stack72 commented Apr 20, 2016

joshuaspence commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 20, 2016

stack72 commented Apr 25, 2016

stack72 commented May 13, 2016

joshuaspence commented May 22, 2016

stack72 commented May 23, 2016

joshuaspence commented May 23, 2016

stack72 commented May 23, 2016

catsby commented Jun 2, 2016

ghost commented Apr 25, 2020

joshuaspence commented Apr 9, 2016 •

edited

Loading