-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
New Resource: aws_dx_private_virtual_interface #3253
Conversation
Acceptance tests require an active Direct Connect connection which should be specified via the
|
Relates also to #2861. |
does this PR require any testing, I'm currently working on some AWS direct connect deployments where I could test this if it helps to validate the PR before merging it? |
Rebased to remove conflicts. |
Acceptance test is running successfully in $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPrivateVirtualInterface_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPrivateVirtualInterface_ -timeout 120m
=== RUN TestAccAwsDxPrivateVirtualInterface_basic
--- PASS: TestAccAwsDxPrivateVirtualInterface_basic (494.87s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 494.877s |
Additional acceptance test with Direct Connect gateway after merge of #4896: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPrivateVirtualInterface_dxGateway'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPrivateVirtualInterface_dxGateway -timeout 120m
=== RUN TestAccAwsDxPrivateVirtualInterface_dxGateway
--- PASS: TestAccAwsDxPrivateVirtualInterface_dxGateway (643.22s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 643.768s |
Confirming acceptance tests in $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPrivateVirtualInterface_dxGateway'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPrivateVirtualInterface_dxGateway -timeout 120m
=== RUN TestAccAwsDxPrivateVirtualInterface_dxGateway
--- PASS: TestAccAwsDxPrivateVirtualInterface_dxGateway (522.64s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 522.650s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good @ewbankkit! Can you please take a look at the below feedback and let us know if you have any questions?
aws/dx_vif.go
Outdated
func dxVirtualInterfaceUpdate(d *schema.ResourceData, meta interface{}) error { | ||
conn := meta.(*AWSClient).dxconn | ||
|
||
arn := arn.ARN{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should prefer to create the arn
manually once during create or read, set it in the Terraform state via d.Set("arn", ...)
, then use d.Get("arn").(string)
or pass by value as necessary.
Once that's done, this function basically becomes nothing and the setTagsDX()
bits can be duplicated where necessary to remove the dxVirtualInterfaceUpdate
function. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'll Set
the ARN after creation, once we have the Id
and Get
it elsewhere.
aws/dx_vif.go
Outdated
VirtualInterfaceId: aws.String(d.Id()), | ||
}) | ||
if err != nil { | ||
if isAWSErr(err, "DirectConnectClientException", "does not exist") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nitpick: looks like the SDK has an available constant for this: directconnect.ErrCodeClientException
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
aws/dx_vif.go
Outdated
} | ||
|
||
// Attributes common to public VIFs and creator side of hosted public VIFs. | ||
func dxPublicVirtualInterfaceAttributes(d *schema.ResourceData, meta interface{}, vif *directconnect.VirtualInterface) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally prefer this logic to be duplicated across resources as necessary for long-term maintainability. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was thinking of that after removing mergeSchemas
...
aws/dx_vif.go
Outdated
return nil | ||
} | ||
|
||
func dxVirtualInterfaceArnAttribute(d *schema.ResourceData, meta interface{}) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential nitpick: this indirection to save two lines of code seems extraneous when it can just be duplicated to the relevant resources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll consolidate and inline all the attribute setting code directly into the resource's Read
method.
AddressFamily: aws.String(d.Get("address_family").(string)), | ||
}, | ||
} | ||
if vgwOk { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For easier module support should this, and the other d.GetOk()
conditionals below, also check for != ""
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
key := "DX_CONNECTION_ID" | ||
connectionId := os.Getenv(key) | ||
if connectionId == "" { | ||
t.Skipf("Environment variable %s is not set", key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
* `vlan` - (Required) The VLAN ID. | ||
* `bgp_asn` - (Required) The autonomous system (AS) number for Border Gateway Protocol (BGP) configuration. | ||
* `bgp_auth_key` - (Optional) The authentication key for BGP configuration. | ||
* `address_family` - (Required) The address family for the BGP peer. `ipv4 ` or `ipv6`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nitpick: Can you organize these with Required
at the top, alphabetical, then Optional
, alphabetical. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Made review changes. $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPrivateVirtualInterface_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPrivateVirtualInterface_ -timeout 120m
=== RUN TestAccAwsDxPrivateVirtualInterface_basic
--- PASS: TestAccAwsDxPrivateVirtualInterface_basic (598.84s)
=== RUN TestAccAwsDxPrivateVirtualInterface_dxGateway
--- PASS: TestAccAwsDxPrivateVirtualInterface_dxGateway (676.64s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 1289.955s |
@n3ph could you please also run the two acceptance tests on your connection? 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM pending someone else running the acceptance tests on a separate account and providing the output 👍
Acceptance tests in $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPrivateVirtualInterface_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPrivateVirtualInterface_ -timeout 120m
=== RUN TestAccAwsDxPrivateVirtualInterface_basic
--- PASS: TestAccAwsDxPrivateVirtualInterface_basic (666.82s)
=== RUN TestAccAwsDxPrivateVirtualInterface_dxGateway
--- PASS: TestAccAwsDxPrivateVirtualInterface_dxGateway (661.49s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 1328.321s |
Here is a small test case - So far everything is working as expected.. provider "aws" {
max_retries = 3
region = "eu-central-1"
profile = "connect"
}
variable "connection_id" {
default = "dxcon-xxxxxxxxx"
}
resource "aws_vpn_gateway" "test" {
amazon_side_asn = 65100
}
resource "aws_dx_gateway" "test" {
name = "test"
amazon_side_asn = 65200
}
resource "aws_dx_private_virtual_interface" "test_dx" {
connection_id = "${var.connection_id}"
name = "vif-test"
vlan = 1
address_family = "ipv4"
bgp_asn = 65001
dx_gateway_id = "${aws_dx_gateway.test.id}"
}
resource "aws_dx_private_virtual_interface" "test_vpn" {
connection_id = "${var.connection_id}"
name = "vif-test"
vlan = 2
address_family = "ipv4"
bgp_asn = 65002
vpn_gateway_id = "${aws_vpn_gateway.test.id}"
} $ terraform apply
data.aws_vpc.default: Refreshing state...
An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
+ create
Terraform will perform the following actions:
+ aws_dx_gateway.test
id: <computed>
amazon_side_asn: "65200"
name: "test"
+ aws_dx_private_virtual_interface.test_dx
id: <computed>
address_family: "ipv4"
amazon_address: <computed>
arn: <computed>
bgp_asn: "65001"
bgp_auth_key: <computed>
connection_id: "dxcon-xxxxxxxx"
customer_address: <computed>
dx_gateway_id: "${aws_dx_gateway.test.id}"
name: "vif-test_dx"
vlan: "1"
+ aws_dx_private_virtual_interface.test_vpn
id: <computed>
address_family: "ipv4"
amazon_address: <computed>
arn: <computed>
bgp_asn: "65002"
bgp_auth_key: <computed>
connection_id: "dxcon-xxxxxxxx"
customer_address: <computed>
name: "vif-test_vpn"
vlan: "2"
vpn_gateway_id: "${aws_vpn_gateway.test.id}"
+ aws_vpn_gateway.test
id: <computed>
amazon_side_asn: "65100"
vpc_id: <computed>
Plan: 4 to add, 0 to change, 0 to destroy.
Do you want to perform these actions?
Terraform will perform the actions described above.
Only 'yes' will be accepted to approve.
Enter a value: yes
aws_dx_gateway.test: Creating...
amazon_side_asn: "" => "65200"
name: "" => "test"
aws_vpn_gateway.test: Creating...
amazon_side_asn: "" => "65100"
vpc_id: "" => "<computed>"
aws_vpn_gateway.test: Creation complete after 0s (ID: vgw-abf96c9b)
aws_dx_private_virtual_interface.test_vpn: Creating...
address_family: "" => "ipv4"
amazon_address: "" => "<computed>"
arn: "" => "<computed>"
bgp_asn: "" => "65002"
bgp_auth_key: "" => "<computed>"
connection_id: "" => "dxcon-xxxxxxxx"
customer_address: "" => "<computed>"
name: "" => "vif-test_vpn"
vlan: "" => "2"
vpn_gateway_id: "" => "vgw-abf96c9b"
aws_dx_gateway.test: Still creating... (10s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (10s elapsed)
aws_dx_gateway.test: Creation complete after 10s (ID: c84df25b-e13f-4c07-9e67-e00b3b6e4e71)
aws_dx_private_virtual_interface.test_dx: Creating...
address_family: "" => "ipv4"
amazon_address: "" => "<computed>"
arn: "" => "<computed>"
bgp_asn: "" => "65001"
bgp_auth_key: "" => "<computed>"
connection_id: "" => "dxcon-xxxxxxxx"
customer_address: "" => "<computed>"
dx_gateway_id: "" => "c84df25b-e13f-4c07-9e67-e00b3b6e4e71"
name: "" => "vif-test_dx"
vlan: "" => "1"
aws_dx_private_virtual_interface.test_vpn: Still creating... (20s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (10s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (30s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (20s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (40s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (30s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (50s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (40s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (1m0s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (50s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (1m10s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (1m0s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (1m20s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (1m10s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (1m30s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (1m20s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still creating... (1m40s elapsed)
aws_dx_private_virtual_interface.test_dx: Still creating... (1m30s elapsed)
aws_dx_private_virtual_interface.test_dx: Creation complete after 1m39s (ID: dxvif-ffnc6ih3)
aws_dx_private_virtual_interface.test_vpn: Creation complete after 1m50s (ID: dxvif-fg15d1du)
Apply complete! Resources: 4 added, 0 changed, 0 destroyed. $ terraform apply
aws_vpn_gateway.test: Refreshing state... (ID: vgw-abf96c9b)
aws_dx_gateway.test: Refreshing state... (ID: c84df25b-e13f-4c07-9e67-e00b3b6e4e71)
data.aws_vpc.default: Refreshing state...
aws_dx_private_virtual_interface.test_dx: Refreshing state... (ID: dxvif-ffnc6ih3)
aws_dx_private_virtual_interface.test_vpn: Refreshing state... (ID: dxvif-fg15d1du)
Apply complete! Resources: 0 added, 0 changed, 0 destroyed. $ terraform destroy
aws_vpn_gateway.test: Refreshing state... (ID: vgw-abf96c9b)
aws_dx_gateway.test: Refreshing state... (ID: c84df25b-e13f-4c07-9e67-e00b3b6e4e71)
data.aws_vpc.default: Refreshing state...
aws_dx_private_virtual_interface.test_dx: Refreshing state... (ID: dxvif-ffnc6ih3)
aws_dx_private_virtual_interface.test_vpn: Refreshing state... (ID: dxvif-fg15d1du)
An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
- destroy
Terraform will perform the following actions:
- aws_dx_gateway.test
- aws_dx_private_virtual_interface.test_dx
- aws_dx_private_virtual_interface.test_vpn
- aws_vpn_gateway.test
Plan: 0 to add, 0 to change, 4 to destroy.
Do you really want to destroy?
Terraform will destroy all your managed infrastructure, as shown above.
There is no undo. Only 'yes' will be accepted to confirm.
Enter a value: yes
aws_dx_private_virtual_interface.test_dx: Destroying... (ID: dxvif-ffnc6ih3)
aws_dx_private_virtual_interface.test_vpn: Destroying... (ID: dxvif-fg15d1du)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 10s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 10s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 20s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 20s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 30s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 30s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 40s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 40s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 50s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 50s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 1m0s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 1m0s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 1m10s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 1m10s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 1m20s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 1m20s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 1m30s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 1m30s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 1m40s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 1m40s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 1m50s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 1m50s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 2m0s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 2m0s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 2m10s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 2m10s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 2m20s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 2m20s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 2m30s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 2m30s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 2m40s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 2m40s elapsed)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 2m50s elapsed)
aws_dx_private_virtual_interface.test_dx: Still destroying... (ID: dxvif-ffnc6ih3, 2m50s elapsed)
aws_dx_private_virtual_interface.test_dx: Destruction complete after 3m0s
aws_dx_gateway.test: Destroying... (ID: c84df25b-e13f-4c07-9e67-e00b3b6e4e71)
aws_dx_private_virtual_interface.test_vpn: Still destroying... (ID: dxvif-fg15d1du, 3m0s elapsed)
aws_dx_private_virtual_interface.test_vpn: Destruction complete after 3m10s
aws_vpn_gateway.test: Destroying... (ID: vgw-abf96c9b)
aws_dx_gateway.test: Still destroying... (ID: c84df25b-e13f-4c07-9e67-e00b3b6e4e71, 10s elapsed)
aws_vpn_gateway.test: Destruction complete after 0s
aws_dx_gateway.test: Destruction complete after 10s
Destroy complete! Resources: 4 destroyed. |
This has been released in version 1.25.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
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! |
Fixes #3249.
aws/dx_vif.go
and changes toaws/utils*.go
shared with::aws/tagsDX*.go
copied from: