Add aws_vpn_gateway_attachment resource.

[kwilczynski]

This commit adds VPN Gateway attachment resource, and also an initial tests and documentation stubs.

Signed-off-by: Krzysztof Wilczynski krzysztof.wilczynski@linux.com

[kwilczynski]

Resolves #7810.

Things to do:

• Add aws_vpn_gateway_attachment resource.
• Update aws_vpn_gateway resource so that vpc_id attribute is a computed value.
• Correct state detection in aws_vpn_gateway.
• Add acceptance tests.
• Add documentation.
• Fix acceptance tests for the aws_vpn_gateway resource.
• Filter VpcAttachments response which are in the "attached" state only.

[kwilczynski]

I had a chat with @stack72 and we need to change the way how state of VPN Gateway is detected in the #7861, it should be corrected e.g.,

    if len(vpnGateway.VpcAttachments) == 0 || *vpnGateway.VpcAttachments[0].State == "detached" || *vpnGateway.State == "deleted" {
        d.Set("vpc_id", "")
    } else {
        d.Set("vpc_id", vpnGateway.VpcAttachments[0].VpcId)
    }

or

    vpnGateway := resp.VpnGateways[0]
    if vpnGateway == nil || *vpnGateway.State == "deleted" {
        d.SetId("")
        return nil
    }

[stack72]

@kwilczynski we still need the part that does this || *vpnGateway.VpcAttachments[0].State == "detached" that is valid :)

[kwilczynski]

@stack72 what I am suggesting is either to treat deleted gateway as "gone", and so to adda check here; resource_aws_vpn_gateway.go#L82-L87; or checking for "deleted" state along the VPN attachments (as per resource_aws_vpn_gateway.go#L89-L94, which might not be a good idea as "detached" will also satisfy the condition when the gateway is deleted (as VPN Gateway would return for a brief moment as being both deleted and detached) - this results only in clearing out the VPC ID and might not trigger new resource to be added.

What do you think?

I will test later to make sure that the logic holds water.

[kwilczynski]

@stack72 over to you! 🚀

[kwilczynski]

Tests are passing:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVpnGatewayAttachment_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/01 11:26:06 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVpnGatewayAttachment_ -timeout 120m
=== RUN   TestAccAWSVpnGatewayAttachment_basic
--- PASS: TestAccAWSVpnGatewayAttachment_basic (71.04s)
=== RUN   TestAccAWSVpnGatewayAttachment_deleted
--- PASS: TestAccAWSVpnGatewayAttachment_deleted (91.58s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    162.648s

[stack72]

if we are adding state == "deleted" here, then we should remove it from line 89 :)

[kwilczynski]

@stack72 corrected!

[stack72]

few small nit picks and then we are good to merge! :)

[stack72]

Hey @kwilczynski

Thanks for making the changes here - they look good. On a final run of the acceptance tests, I get an error:

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVpnGateway'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVpnGateway -timeout 120m
=== RUN   TestAccAWSVpnGateway_importBasic
--- PASS: TestAccAWSVpnGateway_importBasic (56.88s)
=== RUN   TestAccAWSVpnGatewayAttachment_basic
--- PASS: TestAccAWSVpnGatewayAttachment_basic (90.84s)
=== RUN   TestAccAWSVpnGatewayAttachment_deleted
--- PASS: TestAccAWSVpnGatewayAttachment_deleted (98.34s)
=== RUN   TestAccAWSVpnGateway_basic
--- PASS: TestAccAWSVpnGateway_basic (101.33s)
=== RUN   TestAccAWSVpnGateway_reattach
--- FAIL: TestAccAWSVpnGateway_reattach (69.09s)
    testing.go:264: Step 1 error: Check failed: Check 2/2 error: Expected VPC gateway VPC attachment to be in 'detached' state, but was not: {
          State: "available",
          Type: "ipsec.1",
          VpcAttachments: [{
              State: "attached",
              VpcId: "vpc-6685d602"
            }],
          VpnGatewayId: "vgw-5b06df45"
        }
=== RUN   TestAccAWSVpnGateway_delete
--- PASS: TestAccAWSVpnGateway_delete (78.11s)
=== RUN   TestAccAWSVpnGateway_tags
--- PASS: TestAccAWSVpnGateway_tags (86.17s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    580.793s
make: *** [testacc] Error 1

[kwilczynski]

@stack72 this is a side-effect of setting the vpc_id to be a computed value. It causes the variable when set to empty string (or its zero-value) to be ignored for update, thus the resource is unable to act upon the change.

My recommendation would be to change the re-attachment test, so that it would utilise two VPCs, first attach to the first one, then to the second one, and then back to the first one. Confirmation would: attachment still present with the first VPC and the second VPC attachment state would be detached.

I will get to it shortly.

[kwilczynski]

Updated the test, which is now passing:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVpnGateway_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/04 18:17:20 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVpnGateway_ -timeout 120m
=== RUN   TestAccAWSVpnGateway_importBasic
--- PASS: TestAccAWSVpnGateway_importBasic (54.63s)
=== RUN   TestAccAWSVpnGateway_basic
--- PASS: TestAccAWSVpnGateway_basic (91.75s)
=== RUN   TestAccAWSVpnGateway_reattach
--- PASS: TestAccAWSVpnGateway_reattach (123.66s)
=== RUN   TestAccAWSVpnGateway_delete
--- PASS: TestAccAWSVpnGateway_delete (71.24s)
=== RUN   TestAccAWSVpnGateway_tags
--- PASS: TestAccAWSVpnGateway_tags (77.91s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    419.218s

[kwilczynski]

@stack72 over to you 🚀

[kwilczynski]

Added new item to do. I want to filter unattached and old VPN attachments, as there is no really a guarantee of any sort that the "attached" state is always under the 0 index in the list in the response.

[stack72]

ok this now LGTM!

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVpnGateway_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVpnGateway_ -timeout 120m
=== RUN   TestAccAWSVpnGateway_importBasic
--- PASS: TestAccAWSVpnGateway_importBasic (102.25s)
=== RUN   TestAccAWSVpnGateway_basic
--- PASS: TestAccAWSVpnGateway_basic (141.25s)
=== RUN   TestAccAWSVpnGateway_reattach
--- PASS: TestAccAWSVpnGateway_reattach (213.06s)
=== RUN   TestAccAWSVpnGateway_delete
--- PASS: TestAccAWSVpnGateway_delete (109.62s)
=== RUN   TestAccAWSVpnGateway_tags
--- PASS: TestAccAWSVpnGateway_tags (132.90s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    699.098s

[stacko@Pauls-MBP:~/Code/go/src/github.com/hashicorp/terraform on kwilczynski-feature/resource_aws_vpn_gateway_attachment]
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVpnGatewayAttachment_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVpnGatewayAttachment_ -timeout 120m
=== RUN   TestAccAWSVpnGatewayAttachment_basic
--- PASS: TestAccAWSVpnGatewayAttachment_basic (102.34s)
=== RUN   TestAccAWSVpnGatewayAttachment_deleted
--- PASS: TestAccAWSVpnGatewayAttachment_deleted (126.40s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    228.766s

[ghost]

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.