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

r/aws_dx_gateway_association: Allowed prefix support #8222

Merged
merged 2 commits into from
Apr 24, 2019

Conversation

ewbankkit
Copy link
Contributor

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #8199.

@ghost ghost added size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/directconnect Issues and PRs that pertain to the directconnect service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Apr 5, 2019
@ewbankkit
Copy link
Contributor Author

Current acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxGatewayAssociation_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsDxGatewayAssociation_basic -timeout 120m
=== RUN   TestAccAwsDxGatewayAssociation_basic
=== PAUSE TestAccAwsDxGatewayAssociation_basic
=== CONT  TestAccAwsDxGatewayAssociation_basic
--- FAIL: TestAccAwsDxGatewayAssociation_basic (1239.62s)
    testing.go:538: Step 0 error: After applying this step and refreshing, the plan was not empty:
        
        DIFF:
        
        UPDATE: aws_dx_gateway_association.test
          allowed_prefixes.#:          "1" => "0"
          allowed_prefixes.1216997074: "10.255.255.0/28" => ""
        
        STATE:
        
        aws_dx_gateway.test:
          ID = 1a8ca6fb-bbfc-4e3d-ac7f-da8bec989173
          provider = provider.aws
          amazon_side_asn = 65362
          name = terraform-testacc-dxgwassoc-1463777356999477301
        aws_dx_gateway_association.test:
          ID = ga-1a8ca6fb-bbfc-4e3d-ac7f-da8bec989173vgw-09d41462383e45e80
          provider = provider.aws
          allowed_prefixes.# = 1
          allowed_prefixes.1216997074 = 10.255.255.0/28
          dx_gateway_association_id = 5297e438-a084-435a-afd7-b6b4c4dae015
          dx_gateway_id = 1a8ca6fb-bbfc-4e3d-ac7f-da8bec989173
          vpn_gateway_id = vgw-09d41462383e45e80
        
          Dependencies:
            aws_dx_gateway.test
            aws_vpn_gateway_attachment.test
        aws_vpc.test:
          ID = vpc-0c35d0d9035c6ffe2
          provider = provider.aws
          arn = arn:aws:ec2:us-west-2:346386234494:vpc/vpc-0c35d0d9035c6ffe2
          assign_generated_ipv6_cidr_block = false
          cidr_block = 10.255.255.0/28
          default_network_acl_id = acl-0d932fbe72553c2d5
          default_route_table_id = rtb-0f5e45ab5fa9bae1e
          default_security_group_id = sg-05e844d9132964d03
          dhcp_options_id = dopt-249eba40
          enable_classiclink = false
          enable_classiclink_dns_support = false
          enable_dns_hostnames = false
          enable_dns_support = true
          instance_tenancy = default
          ipv6_association_id = 
          ipv6_cidr_block = 
          main_route_table_id = rtb-0f5e45ab5fa9bae1e
          owner_id = 346386234494
          tags.% = 1
          tags.Name = terraform-testacc-dxgwassoc-1463777356999477301
        aws_vpn_gateway.test:
          ID = vgw-09d41462383e45e80
          provider = provider.aws
          amazon_side_asn = 64512
          tags.% = 1
          tags.Name = terraform-testacc-dxgwassoc-1463777356999477301
          vpc_id = vpc-0c35d0d9035c6ffe2
        aws_vpn_gateway_attachment.test:
          ID = vpn-attachment-8937c7e5
          provider = provider.aws
          vpc_id = vpc-0c35d0d9035c6ffe2
          vpn_gateway_id = vgw-09d41462383e45e80
        
          Dependencies:
            aws_vpc.test
            aws_vpn_gateway.test
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	1239.658s
GNUmakefile:20: recipe for target 'testacc' failed
make: *** [testacc] Error 1


Schema: map[string]*schema.Schema{
"allowed_prefixes": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is okay to mark this as Computed: true for now, given that it automatically includes the VPC CIDR if no prefixes are provided. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I think a missing call to resourceAwsDxGatewayAssociationRead() at the end of resourceAwsDxGatewayAssociationCreate() doesn't help either 😄.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxGatewayAssociation_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsDxGatewayAssociation_basic -timeout 120m
=== RUN   TestAccAwsDxGatewayAssociation_basic
=== PAUSE TestAccAwsDxGatewayAssociation_basic
=== CONT  TestAccAwsDxGatewayAssociation_basic
--- PASS: TestAccAwsDxGatewayAssociation_basic (943.05s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	943.077s

@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. waiting-response Maintainers are waiting on response from community or contributor. labels Apr 16, 2019
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Apr 19, 2019
@ewbankkit
Copy link
Contributor Author

Removed WIP.
Ready for review.
Acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxGatewayAssociation_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsDxGatewayAssociation_basic -timeout 120m
=== RUN   TestAccAwsDxGatewayAssociation_basic
=== PAUSE TestAccAwsDxGatewayAssociation_basic
=== CONT  TestAccAwsDxGatewayAssociation_basic
--- PASS: TestAccAwsDxGatewayAssociation_basic (943.05s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	943.077s
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxGatewayAssociation_allowedPrefixes'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsDxGatewayAssociation_allowedPrefixes -timeout 120m
=== RUN   TestAccAwsDxGatewayAssociation_allowedPrefixes
=== PAUSE TestAccAwsDxGatewayAssociation_allowedPrefixes
=== CONT  TestAccAwsDxGatewayAssociation_allowedPrefixes
--- PASS: TestAccAwsDxGatewayAssociation_allowedPrefixes (1379.20s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1379.232s

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Apr 22, 2019
@ewbankkit ewbankkit changed the title [WIP] r/aws_dx_gateway_association: Allowed prefix support r/aws_dx_gateway_association: Allowed prefix support Apr 22, 2019
@bflad bflad added this to the v2.8.0 milestone Apr 24, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

This looks great, thanks @ewbankkit! 🚀 Two very minor documentation changes below will be handled on merge.

Output from acceptance testing (aside: the multiVgws test is very flakey, likely due to some missing retry logic the aws_vpn_gateway resource, but unrelated to this PR):

--- PASS: TestAccAwsDxGatewayAssociation_basic (1197.11s)
--- PASS: TestAccAwsDxGatewayAssociation_allowedPrefixes (1719.97s)

}

resource "aws_vpn_gateway" "example" {
vpc_id = "${aws_vpc.test.id}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vpc_id = "${aws_vpc.test.id}"
vpc_id = "${aws_vpc.example.id}"

* `vpn_gateway_id` - (Required) The ID of the VGW with which to associate the gateway.
* `allowed_prefixes` - (Optional) The Amazon VPC prefixes to advertise to the Direct Connect gateway.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor improvements here for clarification:

Suggested change
* `allowed_prefixes` - (Optional) The Amazon VPC prefixes to advertise to the Direct Connect gateway.
* `allowed_prefixes` - (Optional) VPC prefixes (CIDRs) to advertise to the Direct Connect gateway. Defaults to the CIDR block of the VPC associated with the Virtual Gateway. To enable drift detection, must be configured.

@bflad bflad merged commit c1021a3 into hashicorp:master Apr 24, 2019
bflad added a commit that referenced this pull request Apr 24, 2019
bflad added a commit that referenced this pull request Apr 24, 2019
@ewbankkit
Copy link
Contributor Author

@bflad Yes, when I ran the tests in parallel in my account I regularly got timeouts on the gateway disassociation but no problems when running serially.
Thanks for cleaning up the doc. Copy pasta.

@ewbankkit ewbankkit deleted the issue-8199 branch April 24, 2019 16:59
@nywilken
Copy link
Contributor

This has been released in version 2.8.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@ghost
Copy link

ghost commented Mar 30, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/directconnect Issues and PRs that pertain to the directconnect service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

r/aws_dx_gateway_association: Allowed prefix support
3 participants