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

VPN Connection Tunnels incorrectly ordered #396

Closed
hashibot opened this issue Jun 13, 2017 · 9 comments · Fixed by #19077
Closed

VPN Connection Tunnels incorrectly ordered #396

hashibot opened this issue Jun 13, 2017 · 9 comments · Fixed by #19077
Assignees
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service.
Milestone

Comments

@hashibot
Copy link

This issue was originally opened by @kitforbes as hashicorp/terraform#10411. It was migrated here as part of the provider split. The original body of the issue is below.


Terraform Version

Terraform v0.7.13

Affected Resource(s)

  • aws_vpn_connection

Terraform Configuration Files

module "vpc" {
  source = "../../../modules/aws/network/vpc"

  name        = "${var.name}-vpc"
  environment = "${var.environment}"
  cidr        = "${var.vpc_cidr}"
}

resource "aws_customer_gateway" "edinburgh" {
  type       = "${var.vpn_connection_type}"
  bgp_asn    = "${var.vpn_bgs_asn}"
  ip_address = "${var.vpn_local_ip}"

  tags {
    Name        = "${var.name}-customer-gateway"
    Environment = "${var.environment}"
    Creator     = "Terraform"
  }
}

resource "aws_vpn_gateway" "edinburgh" {
  vpc_id = "${module.vpc.vpc_id}"

  tags {
    Name        = "${var.name}-edinburgh"
    Environment = "${var.environment}"
    Creator     = "Terraform"
  }
}

resource "aws_vpn_connection" "edinburgh" {
  vpn_gateway_id      = "${aws_vpn_gateway.edinburgh.id}"
  customer_gateway_id = "${aws_customer_gateway.edinburgh.id}"
  type                = "${var.vpn_connection_type}"
  static_routes_only  = true

  tags {
    Name        = "${var.name}-edinburgh"
    Environment = "${var.environment}"
    Creator     = "Terraform"
  }
}

output "vpn_config" {
  value = <<CONFIGURATION

  Tunnel 1
    Address:     ${aws_vpn_connection.edinburgh.tunnel1_address}
  Tunnel 2
    Address:     ${aws_vpn_connection.edinburgh.tunnel2_address}
CONFIGURATION
}

Expected Behavior

${aws_vpn_connection.edinburgh.tunnel1_address} should output the value of Tunnel 1 in the AWS Console.

Actual Behavior

It actually outputs the value of tunnel 2, and tunnel 2 outputs the value of tunnel 1. Not a critical issue by any means.

Steps to Reproduce

Create an AWS VPN connection and compare the Terraform output to actual AWS Console information in the VPC section.

@hashibot hashibot added the bug Addresses a defect in current functionality. label Jun 13, 2017
@radeksimko radeksimko added the service/ec2 Issues and PRs that pertain to the ec2 service. label Jan 25, 2018
@tyrken
Copy link

tyrken commented Apr 6, 2018

Not sure, but suspect the AWS Console GUI is sorting by Outside IP address. Would recommend TF does NOT do so.

@jochen42
Copy link

jochen42 commented Oct 21, 2019

perhaps the reason is this sort-statement:

https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_vpn_connection.go#L595-L596

this sort is inside the code since the first version:

https://github.com/terraform-providers/terraform-provider-aws/blob/757055f2a5dfb4ea62207de6c4376088c33e3948/aws/resource_aws_vpn_connection.go#L468-L469

but why the xml from the api should be in the wrong order?

@jochen42
Copy link

for users who running in this issue before merging/releasing the pull-request:

use the xml-output "aws_vpn_connection.<resource_name>.customer_gateway_configuration". the tunnels inside this xml are always sorted exactly like the aws console does. aditionaly it contains some more informations about the connection-parameters.

https://www.terraform.io/docs/providers/aws/r/vpn_connection.html#virtual-private-gateway

@MRinalducci
Copy link

This triage bug becomes very annoying since last version of the provider v3.22.0 with PR #14740 which adds read/refresh of tunnel options.
Any ideas how to solve that?

@dthvt
Copy link
Contributor

dthvt commented Apr 23, 2021

Configuring tunnel1/tunnel2 with differing options (DH group, etc.) will trigger a dual tunnel outage when applying due to the reordering of the tunnels in terraform. I would support ANY solution that results in consistent ordering of the tunnels, even if it doesn't match the Console. As it is now, having different tunnel configs will trigger a dual tunnel outage and full loss of connectivity during applies, which is just not acceptable in a production environment.

@francescoprovino
Copy link

I have found a workaround: because tunnel1 is always the lowest IP of the two, I just wrote a few lines to convert it to a number, and tied the assignment to this convention. You can do it in two-three lines with a simple condition. I think this could be fixed this way in the backend, also.

@bflad
Copy link
Contributor

bflad commented Apr 23, 2021

Hi folks 👋 Sorry this is still causing issues. We are working on a fix now.

@bflad bflad self-assigned this Apr 23, 2021
bflad added a commit that referenced this issue Apr 23, 2021
…_*` ordering when `tunnel1_inside_cidr`, `tunnel1_inside_ipv6_cidr`, or `tunnel1_preshared_key` is configured

Reference: #396
Reference: #3359
Reference: #4728
Reference: #5809
Reference: #11293

Previously (race condition of automatically assigned outside IP addresses):

```
=== CONT  TestAccAWSVpnConnection_tunnelOptions
resource_aws_vpn_connection_test.go:210: Step 15/15 error: Check failed: Check 4/6 error: aws_vpn_connection.test: Attribute 'tunnel1_preshared_key' expected "12345678", got "abcdefgh"
--- FAIL: TestAccAWSVpnConnection_tunnelOptions (738.28s)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSVpnConnection_Tunnel1PresharedKey (251.02s)
--- PASS: TestAccAWSVpnConnection_withoutStaticRoutes (263.77s)
--- PASS: TestAccAWSVpnConnection_Tunnel1InsideCidr (335.14s)
--- PASS: TestAccAWSVpnConnection_tunnelOptions (342.30s)
--- PASS: TestAccAWSVpnConnection_disappears (388.07s)
--- PASS: TestAccAWSVpnConnection_tags (445.29s)
--- PASS: TestAccAWSVpnConnection_basic (797.33s)
--- PASS: TestAccAWSVpnConnection_withIpv6 (1235.35s)
--- PASS: TestAccAWSVpnConnection_TransitGatewayID (1235.72s)
--- PASS: TestAccAWSVpnConnection_withEnableAcceleration (1352.28s)
--- PASS: TestAccAWSVpnConnection_Tunnel1InsideIpv6Cidr (1595.79s)
```
bflad added a commit that referenced this issue Apr 23, 2021
…_*` ordering when `tunnel1_inside_cidr`, `tunnel1_inside_ipv6_cidr`, or `tunnel1_preshared_key` is configured (#19077)

* resource/aws_vpn_connection: Prevent flipped `tunnel1_*` and `tunnel2_*` ordering when `tunnel1_inside_cidr`, `tunnel1_inside_ipv6_cidr`, or `tunnel1_preshared_key` is configured

Reference: #396
Reference: #3359
Reference: #4728
Reference: #5809
Reference: #11293

Previously (race condition of automatically assigned outside IP addresses):

```
=== CONT  TestAccAWSVpnConnection_tunnelOptions
resource_aws_vpn_connection_test.go:210: Step 15/15 error: Check failed: Check 4/6 error: aws_vpn_connection.test: Attribute 'tunnel1_preshared_key' expected "12345678", got "abcdefgh"
--- FAIL: TestAccAWSVpnConnection_tunnelOptions (738.28s)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSVpnConnection_Tunnel1PresharedKey (251.02s)
--- PASS: TestAccAWSVpnConnection_withoutStaticRoutes (263.77s)
--- PASS: TestAccAWSVpnConnection_Tunnel1InsideCidr (335.14s)
--- PASS: TestAccAWSVpnConnection_tunnelOptions (342.30s)
--- PASS: TestAccAWSVpnConnection_disappears (388.07s)
--- PASS: TestAccAWSVpnConnection_tags (445.29s)
--- PASS: TestAccAWSVpnConnection_basic (797.33s)
--- PASS: TestAccAWSVpnConnection_withIpv6 (1235.35s)
--- PASS: TestAccAWSVpnConnection_TransitGatewayID (1235.72s)
--- PASS: TestAccAWSVpnConnection_withEnableAcceleration (1352.28s)
--- PASS: TestAccAWSVpnConnection_Tunnel1InsideIpv6Cidr (1595.79s)
```

* tests/resource/aws_vpn_connection: Add nosemgrep comment for errant situation

* resource/aws_vpn_connection: Fix comment typo
@github-actions github-actions bot added this to the v3.38.0 milestone Apr 23, 2021
bflad added a commit that referenced this issue Apr 23, 2021
…_*` ordering when `tunnel1_inside_cidr`, `tunnel1_inside_ipv6_cidr`, or `tunnel1_preshared_key` is configured (#19077)

* resource/aws_vpn_connection: Prevent flipped `tunnel1_*` and `tunnel2_*` ordering when `tunnel1_inside_cidr`, `tunnel1_inside_ipv6_cidr`, or `tunnel1_preshared_key` is configured

Reference: #396
Reference: #3359
Reference: #4728
Reference: #5809
Reference: #11293

Previously (race condition of automatically assigned outside IP addresses):

```
=== CONT  TestAccAWSVpnConnection_tunnelOptions
resource_aws_vpn_connection_test.go:210: Step 15/15 error: Check failed: Check 4/6 error: aws_vpn_connection.test: Attribute 'tunnel1_preshared_key' expected "12345678", got "abcdefgh"
--- FAIL: TestAccAWSVpnConnection_tunnelOptions (738.28s)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSVpnConnection_Tunnel1PresharedKey (251.02s)
--- PASS: TestAccAWSVpnConnection_withoutStaticRoutes (263.77s)
--- PASS: TestAccAWSVpnConnection_Tunnel1InsideCidr (335.14s)
--- PASS: TestAccAWSVpnConnection_tunnelOptions (342.30s)
--- PASS: TestAccAWSVpnConnection_disappears (388.07s)
--- PASS: TestAccAWSVpnConnection_tags (445.29s)
--- PASS: TestAccAWSVpnConnection_basic (797.33s)
--- PASS: TestAccAWSVpnConnection_withIpv6 (1235.35s)
--- PASS: TestAccAWSVpnConnection_TransitGatewayID (1235.72s)
--- PASS: TestAccAWSVpnConnection_withEnableAcceleration (1352.28s)
--- PASS: TestAccAWSVpnConnection_Tunnel1InsideIpv6Cidr (1595.79s)
```

* tests/resource/aws_vpn_connection: Add nosemgrep comment for errant situation

* resource/aws_vpn_connection: Fix comment typo
@ghost
Copy link

ghost commented Apr 30, 2021

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.