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.

Sign up for GitHub

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

service/directconnect: vpn_gateway_id Argument Removals and Increase aws_dx_gateway_association Default Timeouts #14144

Merged
merged 3 commits into from
Jul 24, 2020
Merged

service/directconnect: vpn_gateway_id Argument Removals and Increase aws_dx_gateway_association Default Timeouts #14144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44aa348
resource/aws_dx_gateway_association: Increase default create/update/d…
bflad Jul 10, 2020
444df24
service/directconnect: Remove vpn_gateway_id arguments
bflad Jul 10, 2020
5de3318
tests/resource/aws_dx_gateway_association: Ensure v0 state upgrade is…
bflad Jul 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 12 additions & 37 deletions aws/resource_aws_dx_gateway_association.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package aws

import (
"errors"
"fmt"
"log"
"strings"
Expand Down Expand Up @@ -49,7 +48,7 @@ func resourceAwsDxGatewayAssociation() *schema.Resource {
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"associated_gateway_owner_account_id", "proposal_id", "vpn_gateway_id"},
ConflictsWith: []string{"associated_gateway_owner_account_id", "proposal_id"},
},

"associated_gateway_owner_account_id": {
Expand All @@ -58,7 +57,7 @@ func resourceAwsDxGatewayAssociation() *schema.Resource {
Computed: true,
ForceNew: true,
ValidateFunc: validateAwsAccountId,
ConflictsWith: []string{"associated_gateway_id", "vpn_gateway_id"},
ConflictsWith: []string{"associated_gateway_id"},
},

"associated_gateway_type": {
Expand Down Expand Up @@ -86,22 +85,14 @@ func resourceAwsDxGatewayAssociation() *schema.Resource {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"associated_gateway_id", "vpn_gateway_id"},
},

"vpn_gateway_id": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"associated_gateway_id", "associated_gateway_owner_account_id", "proposal_id"},
Deprecated: "use 'associated_gateway_id' argument instead",
ConflictsWith: []string{"associated_gateway_id"},
},
},

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(15 * time.Minute),
Update: schema.DefaultTimeout(10 * time.Minute),
Delete: schema.DefaultTimeout(15 * time.Minute),
Create: schema.DefaultTimeout(30 * time.Minute),
Update: schema.DefaultTimeout(30 * time.Minute),
Delete: schema.DefaultTimeout(30 * time.Minute),
},
}
}
Expand All @@ -111,7 +102,6 @@ func resourceAwsDxGatewayAssociationCreate(d *schema.ResourceData, meta interfac

dxgwId := d.Get("dx_gateway_id").(string)
gwIdRaw, gwIdOk := d.GetOk("associated_gateway_id")
vgwIdRaw, vgwIdOk := d.GetOk("vpn_gateway_id")
gwAcctIdRaw, gwAcctIdOk := d.GetOk("associated_gateway_owner_account_id")
proposalIdRaw, proposalIdOk := d.GetOk("proposal_id")

Expand All @@ -120,8 +110,8 @@ func resourceAwsDxGatewayAssociationCreate(d *schema.ResourceData, meta interfac
if !(gwAcctIdOk && proposalIdOk) {
return fmt.Errorf("associated_gateway_owner_account_id and proposal_id must be configured")
}
} else if !(gwIdOk || vgwIdOk) {
return fmt.Errorf("either associated_gateway_owner_account_id and proposal_id or one of associated_gateway_id or vpn_gateway_id must be configured")
} else if !gwIdOk {
return fmt.Errorf("either associated_gateway_owner_account_id and proposal_id, or associated_gateway_id, must be configured")
}

associationId := ""
Expand All @@ -143,17 +133,12 @@ func resourceAwsDxGatewayAssociationCreate(d *schema.ResourceData, meta interfac
associationId = aws.StringValue(resp.DirectConnectGatewayAssociation.AssociationId)
d.SetId(dxGatewayAssociationId(dxgwId, aws.StringValue(resp.DirectConnectGatewayAssociation.AssociatedGateway.Id)))
} else {
gwId := gwIdRaw.(string)

req := &directconnect.CreateDirectConnectGatewayAssociationInput{
AddAllowedPrefixesToDirectConnectGateway: expandDxRouteFilterPrefixes(d.Get("allowed_prefixes").(*schema.Set)),
DirectConnectGatewayId: aws.String(dxgwId),
}
gwId := ""
if gwIdOk {
gwId = gwIdRaw.(string)
req.GatewayId = aws.String(gwId)
} else {
gwId = vgwIdRaw.(string)
req.VirtualGatewayId = aws.String(gwId)
GatewayId: aws.String(gwId),
}

log.Printf("[DEBUG] Creating Direct Connect gateway association: %#v", req)
Expand Down Expand Up @@ -196,11 +181,7 @@ func resourceAwsDxGatewayAssociationRead(d *schema.ResourceData, meta interface{
return fmt.Errorf("error setting allowed_prefixes: %s", err)
}

if _, ok := d.GetOk("vpn_gateway_id"); ok {
d.Set("vpn_gateway_id", assoc.VirtualGatewayId)
} else {
d.Set("associated_gateway_id", assoc.AssociatedGateway.Id)
}
d.Set("associated_gateway_id", assoc.AssociatedGateway.Id)
d.Set("associated_gateway_owner_account_id", assoc.AssociatedGateway.OwnerAccount)
d.Set("associated_gateway_type", assoc.AssociatedGateway.Type)
d.Set("dx_gateway_association_id", assoc.AssociationId)
Expand All @@ -213,12 +194,6 @@ func resourceAwsDxGatewayAssociationRead(d *schema.ResourceData, meta interface{
func resourceAwsDxGatewayAssociationUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).dxconn

_, gwIdOk := d.GetOk("associated_gateway_id")
_, vgwIdOk := d.GetOk("vpn_gateway_id")
if !gwIdOk && !vgwIdOk {
return errors.New("one of associated_gateway_id or vpn_gateway_id must be configured")
}

if d.HasChange("allowed_prefixes") {
associationId := d.Get("dx_gateway_association_id").(string)

Expand Down
1 change: 0 additions & 1 deletion aws/resource_aws_dx_gateway_association_migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ func resourceAwsDxGatewayAssociationResourceV0() *schema.Resource {
Optional: true,
ForceNew: true,
ConflictsWith: []string{"associated_gateway_id", "associated_gateway_owner_account_id", "proposal_id"},
Deprecated: "use 'associated_gateway_id' argument instead",
},
},
}
Expand Down
33 changes: 5 additions & 28 deletions aws/resource_aws_dx_gateway_association_proposal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ func resourceAwsDxGatewayAssociationProposal() *schema.Resource {
Elem: &schema.Schema{Type: schema.TypeString},
},
"associated_gateway_id": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"vpn_gateway_id"},
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"associated_gateway_owner_account_id": {
Type: schema.TypeString,
Expand All @@ -67,13 +66,6 @@ func resourceAwsDxGatewayAssociationProposal() *schema.Resource {
ForceNew: true,
ValidateFunc: validateAwsAccountId,
},
"vpn_gateway_id": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"associated_gateway_id"},
Deprecated: "use 'associated_gateway_id' argument instead",
},
},
}
}
Expand All @@ -86,20 +78,9 @@ func resourceAwsDxGatewayAssociationProposalCreate(d *schema.ResourceData, meta
AddAllowedPrefixesToDirectConnectGateway: allowedPrefixes,
DirectConnectGatewayId: aws.String(d.Get("dx_gateway_id").(string)),
DirectConnectGatewayOwnerAccount: aws.String(d.Get("dx_gateway_owner_account_id").(string)),
}
var gwID string
if v, ok := d.GetOk("vpn_gateway_id"); ok {
gwID = v.(string)
} else if v, ok := d.GetOk("associated_gateway_id"); ok {
gwID = v.(string)
GatewayId: aws.String(d.Get("associated_gateway_id").(string)),
}

if gwID == "" {
return fmt.Errorf("gateway id not provided, one of associated_gateway_id or vpn_gateway_id must be configured")
}

input.GatewayId = aws.String(gwID)

log.Printf("[DEBUG] Creating Direct Connect Gateway Association Proposal: %s", input)
output, err := conn.CreateDirectConnectGatewayAssociationProposal(input)

Expand Down Expand Up @@ -141,11 +122,7 @@ func resourceAwsDxGatewayAssociationProposalRead(d *schema.ResourceData, meta in
return fmt.Errorf("error setting allowed_prefixes: %s", err)
}

if _, ok := d.GetOk("vpn_gateway_id"); ok {
d.Set("vpn_gateway_id", aws.StringValue(proposal.AssociatedGateway.Id))
} else {
d.Set("associated_gateway_id", aws.StringValue(proposal.AssociatedGateway.Id))
}
d.Set("associated_gateway_id", aws.StringValue(proposal.AssociatedGateway.Id))
d.Set("associated_gateway_owner_account_id", proposal.AssociatedGateway.OwnerAccount)
d.Set("associated_gateway_type", proposal.AssociatedGateway.Type)
d.Set("dx_gateway_id", aws.StringValue(proposal.DirectConnectGatewayId))
Expand Down
45 changes: 0 additions & 45 deletions aws/resource_aws_dx_gateway_association_proposal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,39 +78,6 @@ func testSweepDirectConnectGatewayAssociationProposals(region string) error {
return nil
}

func TestAccAwsDxGatewayAssociationProposal_VpnGatewayId(t *testing.T) {
var proposal1 directconnect.GatewayAssociationProposal
var providers []*schema.Provider
rBgpAsn := randIntRange(64512, 65534)
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_dx_gateway_association_proposal.test"
resourceNameDxGw := "aws_dx_gateway.test"
resourceNameVgw := "aws_vpn_gateway.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
testAccPreCheck(t)
testAccAlternateAccountPreCheck(t)
},
ProviderFactories: testAccProviderFactories(&providers),
CheckDestroy: testAccCheckAwsDxGatewayAssociationProposalDestroy,
Steps: []resource.TestStep{
{
Config: testAccDxGatewayAssociationProposalConfig_vpnGatewayId(rName, rBgpAsn),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsDxGatewayAssociationProposalExists(resourceName, &proposal1),
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "vpn_gateway_id", resourceNameVgw, "id"),
resource.TestCheckNoResourceAttr(resourceName, "associated_gateway_id"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "virtualPrivateGateway"),
resource.TestCheckResourceAttr(resourceName, "allowed_prefixes.#", "1"),
),
},
},
})
}

func TestAccAwsDxGatewayAssociationProposal_basicVpnGateway(t *testing.T) {
var proposal1 directconnect.GatewayAssociationProposal
var providers []*schema.Provider
Expand All @@ -134,7 +101,6 @@ func TestAccAwsDxGatewayAssociationProposal_basicVpnGateway(t *testing.T) {
testAccCheckAwsDxGatewayAssociationProposalExists(resourceName, &proposal1),
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "associated_gateway_id", resourceNameVgw, "id"),
resource.TestCheckNoResourceAttr(resourceName, "vpn_gateway_id"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "virtualPrivateGateway"),
resource.TestCheckResourceAttr(resourceName, "allowed_prefixes.#", "1"),
Expand Down Expand Up @@ -173,7 +139,6 @@ func TestAccAwsDxGatewayAssociationProposal_basicTransitGateway(t *testing.T) {
testAccCheckAwsDxGatewayAssociationProposalExists(resourceName, &proposal1),
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "associated_gateway_id", resourceNameTgw, "id"),
resource.TestCheckNoResourceAttr(resourceName, "vpn_gateway_id"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "transitGateway"),
resource.TestCheckResourceAttr(resourceName, "allowed_prefixes.#", "2"),
Expand Down Expand Up @@ -358,16 +323,6 @@ resource "aws_vpn_gateway" "test" {
`, rName, rBgpAsn)
}

func testAccDxGatewayAssociationProposalConfig_vpnGatewayId(rName string, rBgpAsn int) string {
return testAccDxGatewayAssociationProposalConfigBase_vpnGateway(rName, rBgpAsn) + `
resource "aws_dx_gateway_association_proposal" "test" {
dx_gateway_id = "${aws_dx_gateway.test.id}"
dx_gateway_owner_account_id = "${aws_dx_gateway.test.owner_account_id}"
vpn_gateway_id = "${aws_vpn_gateway.test.id}"
}
`
}

func testAccDxGatewayAssociationProposalConfig_basicVpnGateway(rName string, rBgpAsn int) string {
return testAccDxGatewayAssociationProposalConfigBase_vpnGateway(rName, rBgpAsn) + `
resource "aws_dx_gateway_association_proposal" "test" {
Expand Down
31 changes: 4 additions & 27 deletions aws/resource_aws_dx_gateway_association_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,9 @@ func testSweepDirectConnectGatewayAssociations(region string) error {
return nil
}

func TestAccAwsDxGatewayAssociation_deprecatedSingleAccount(t *testing.T) {
// V0 state upgrade testing must be done via acceptance testing due to API call
func TestAccAwsDxGatewayAssociation_V0StateUpgrade(t *testing.T) {
resourceName := "aws_dx_gateway_association.test"
resourceNameDxGw := "aws_dx_gateway.test"
resourceNameVgw := "aws_vpn_gateway.test"
rName := fmt.Sprintf("terraform-testacc-dxgwassoc-%d", acctest.RandInt())
rBgpAsn := randIntRange(64512, 65534)

Expand All @@ -195,18 +194,9 @@ func TestAccAwsDxGatewayAssociation_deprecatedSingleAccount(t *testing.T) {
CheckDestroy: testAccCheckAwsDxGatewayAssociationDestroy,
Steps: []resource.TestStep{
{
Config: testAccDxGatewayAssociationConfig_deprecatedSingleAccount(rName, rBgpAsn),
Config: testAccDxGatewayAssociationConfig_basicVpnGatewaySingleAccount(rName, rBgpAsn),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsDxGatewayAssociationExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "vpn_gateway_id", resourceNameVgw, "id"),
resource.TestCheckResourceAttrSet(resourceName, "dx_gateway_association_id"),
resource.TestCheckNoResourceAttr(resourceName, "associated_gateway_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "virtualPrivateGateway"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
testAccCheckResourceAttrAccountID(resourceName, "dx_gateway_owner_account_id"),
resource.TestCheckResourceAttr(resourceName, "allowed_prefixes.#", "1"),
tfawsresource.TestCheckTypeSetElemAttr(resourceName, "allowed_prefixes.*", "10.255.255.0/28"),
testAccCheckAwsDxGatewayAssociationStateUpgradeV0(resourceName),
Copy link
Member

Choose a reason for hiding this comment

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

This is the only call to testAccCheckAwsDxGatewayAssociationStateUpgradeV0 so it needs to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was actually a good catch by the linter. We still want to acceptance test the state upgrade function since it requires an API call. Reinstated this test using a 3.0 valid configuration using associated_gateway_id and updated testAccCheckAwsDxGatewayAssociationStateUpgradeV0() to appropriately setup the state upgrade using the new attribute.

Output from acceptance testing:

--- PASS: TestAccAwsDxGatewayAssociation_V0StateUpgrade (2605.48s)

),
},
Expand All @@ -233,7 +223,6 @@ func TestAccAwsDxGatewayAssociation_basicVpnGatewaySingleAccount(t *testing.T) {
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "associated_gateway_id", resourceNameVgw, "id"),
resource.TestCheckResourceAttrSet(resourceName, "dx_gateway_association_id"),
resource.TestCheckNoResourceAttr(resourceName, "vpn_gateway_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "virtualPrivateGateway"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
testAccCheckResourceAttrAccountID(resourceName, "dx_gateway_owner_account_id"),
Expand Down Expand Up @@ -281,7 +270,6 @@ func TestAccAwsDxGatewayAssociation_basicVpnGatewayCrossAccount(t *testing.T) {
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "associated_gateway_id", resourceNameVgw, "id"),
resource.TestCheckResourceAttrSet(resourceName, "dx_gateway_association_id"),
resource.TestCheckNoResourceAttr(resourceName, "vpn_gateway_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "virtualPrivateGateway"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
// dx_gateway_owner_account_id is the "aws.alternate" provider's account ID.
Expand Down Expand Up @@ -313,7 +301,6 @@ func TestAccAwsDxGatewayAssociation_basicTransitGatewaySingleAccount(t *testing.
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "associated_gateway_id", resourceNameTgw, "id"),
resource.TestCheckResourceAttrSet(resourceName, "dx_gateway_association_id"),
resource.TestCheckNoResourceAttr(resourceName, "vpn_gateway_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "transitGateway"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
testAccCheckResourceAttrAccountID(resourceName, "dx_gateway_owner_account_id"),
Expand Down Expand Up @@ -362,7 +349,6 @@ func TestAccAwsDxGatewayAssociation_basicTransitGatewayCrossAccount(t *testing.T
resource.TestCheckResourceAttrPair(resourceName, "dx_gateway_id", resourceNameDxGw, "id"),
resource.TestCheckResourceAttrPair(resourceName, "associated_gateway_id", resourceNameTgw, "id"),
resource.TestCheckResourceAttrSet(resourceName, "dx_gateway_association_id"),
resource.TestCheckNoResourceAttr(resourceName, "vpn_gateway_id"),
resource.TestCheckResourceAttr(resourceName, "associated_gateway_type", "transitGateway"),
testAccCheckResourceAttrAccountID(resourceName, "associated_gateway_owner_account_id"),
// dx_gateway_owner_account_id is the "aws.alternate" provider's account ID.
Expand Down Expand Up @@ -537,7 +523,7 @@ func testAccCheckAwsDxGatewayAssociationStateUpgradeV0(name string) resource.Tes

rawState := map[string]interface{}{
"dx_gateway_id": rs.Primary.Attributes["dx_gateway_id"],
"vpn_gateway_id": rs.Primary.Attributes["vpn_gateway_id"],
"vpn_gateway_id": rs.Primary.Attributes["associated_gateway_id"], // vpn_gateway_id was removed in 3.0, but older state still has it
}

updatedRawState, err := resourceAwsDxGatewayAssociationStateUpgradeV0(rawState, testAccProvider.Meta())
Expand Down Expand Up @@ -615,15 +601,6 @@ resource "aws_dx_gateway" "test" {
`, rName, rBgpAsn)
}

func testAccDxGatewayAssociationConfig_deprecatedSingleAccount(rName string, rBgpAsn int) string {
return testAccDxGatewayAssociationConfigBase_vpnGatewaySingleAccount(rName, rBgpAsn) + `
resource "aws_dx_gateway_association" "test" {
dx_gateway_id = "${aws_dx_gateway.test.id}"
vpn_gateway_id = "${aws_vpn_gateway_attachment.test.vpn_gateway_id}"
}
`
}

func testAccDxGatewayAssociationConfig_basicVpnGatewaySingleAccount(rName string, rBgpAsn int) string {
return testAccDxGatewayAssociationConfigBase_vpnGatewaySingleAccount(rName, rBgpAsn) + `
resource "aws_dx_gateway_association" "test" {
Expand Down
Loading