Add support for Amazon side private ASN for aws_vpn_gateway

[ewbankkit]

Fixes #1859
Acceptance tests:

Running tool: /usr/local/go/bin/go test -timeout 30s github.com/terraform-providers/terraform-provider-aws/aws -run ^TestValidateAmazonSideAsn$

ok  	github.com/terraform-providers/terraform-provider-aws/aws	0.014s
Success: Tests passed.

make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVpnGateway_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSVpnGateway_ -timeout 120m
=== RUN   TestAccAWSVpnGateway_importBasic
--- PASS: TestAccAWSVpnGateway_importBasic (43.61s)
=== RUN   TestAccAWSVpnGateway_basic
--- PASS: TestAccAWSVpnGateway_basic (77.10s)
=== RUN   TestAccAWSVpnGateway_withAvailabilityZoneSetToState
--- PASS: TestAccAWSVpnGateway_withAvailabilityZoneSetToState (38.76s)
=== RUN   TestAccAWSVpnGateway_withAmazonSideAsnSetToState
--- PASS: TestAccAWSVpnGateway_withAmazonSideAsnSetToState (47.32s)
=== RUN   TestAccAWSVpnGateway_disappears
--- PASS: TestAccAWSVpnGateway_disappears (36.77s)
=== RUN   TestAccAWSVpnGateway_reattach
--- PASS: TestAccAWSVpnGateway_reattach (101.08s)
=== RUN   TestAccAWSVpnGateway_delete
--- PASS: TestAccAWSVpnGateway_delete (52.53s)
=== RUN   TestAccAWSVpnGateway_tags
--- PASS: TestAccAWSVpnGateway_tags (64.17s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	461.351s

make testacc TEST=./aws/ TESTARGS='-run=TestAccDataSourceAwsVpnGateway_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccDataSourceAwsVpnGateway_ -timeout 120m
=== RUN   TestAccDataSourceAwsVpnGateway_unattached
--- PASS: TestAccDataSourceAwsVpnGateway_unattached (18.27s)
=== RUN   TestAccDataSourceAwsVpnGateway_attached
--- PASS: TestAccDataSourceAwsVpnGateway_attached (52.50s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	70.778s

[ewbankkit]

amazon_side_asn is defined as schema.TypeString as schema.TypeInt doesn't handle 64-bit integers.

[radeksimko]

Hi @ewbankkit
thanks for the PR.

amazon_side_asn is defined as schema.TypeString as schema.TypeInt doesn't handle 64-bit integers.

can you be more specific? The schema should support int64 just fine and I don't remember we'd ever need to use TypeString for integers for any such reason.

[ewbankkit]

@radeksimko According to the comment here TypeInt is equivalent to golang int which is a 32-bit integer and code like this looks like it would break if a value > 2147483647 (which these ASNs can be) was stored in the field.
Agreed that my implementation seems hacky.

[ewbankkit]

Cleaner would be to add TypeInt64 to valuetype.go.

[radeksimko]

is equivalent to golang int which is a 32-bit integer

That depends purely on the architecture of the binary you build, I think. It will be int64 on 64bit. That said we do release four 32bit packages of Terraform https://www.terraform.io/downloads.html

This may in fact be causing issues elsewhere... and if it is, we do need to come up with more reasonable solution.

[edmundcraske]

I thought that ASNs are 32 bits at most (with most common ones being 16 bit)? https://en.wikipedia.org/wiki/Autonomous_system_%28Internet%29 seems to agree with me, and links to https://tools.ietf.org/html/rfc6793.

[ewbankkit]

I think the problem is that int is a signed integer and on 32-bit systems can only represent values up to 2147483647. 32-bit ASNs need to be between 4200000000 and 4294967294.

[jonstacks]

Is there any update on this? We would really love this feature to get merged soon so that we can use terraform to manage our transit-vpc setup across regions. If there is anything that can be done to help out, let me know.

[ewbankkit]

@jonstacks The correct implementation of this (typeof(amazon_side_asn) != string) requires changes to upstream Terraform core to support either 64-bit or "big" integers. Not sure when that will be available.

[jonstacks]

@ewbankkit,

Thanks for the update on this. I'd assume there is already a branch of terraform core that supports int64 or big.Int. I'll try building core against that branch and try running the code from this branch against it.

[ewbankkit]

Opened https://github.com/hashicorp/terraform/issues/17077 for the 64-bit/big int support in Terraform.

[ewbankkit]

The guidance from @apparentlymart in https://github.com/hashicorp/terraform/issues/17077 for now is to use TypeString for attributes that require 64-bit (or larger) integers.

[pmoust]

@apparentlymart ASN's are 4-Byte max atm, unless something drastically changes in the RFC/RIPE/ARIN there is no reason to worry on that.
On the topic of signed integers, 32bit etc. Can we just accept the situation for now and go with TypeString as suggested?

We would love to see this in.

[radeksimko]

@ewbankkit Thanks for the wait and opening upstream issue.

[pmoust]

Thanks @ewbankkit for tackling it and thanks @radeksimko for accepting the interim solution.

[bflad]

This has been released in terraform-provider-aws version 1.9.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

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