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

Jump to bottom

Add enable_acceleration for aws_vpn_connection #12218

Closed
wants to merge 1 commit into from
Closed

Add enable_acceleration for aws_vpn_connection #12218

wants to merge 1 commit into from

Conversation

stujonesuk
Copy link

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #11135

Release note for CHANGELOG:

* resource/aws_vpn_connection: Accelerated Site-to-Site VPN support

Output from acceptance testing:

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

...

@stujonesuk stujonesuk requested a review from a team March 1, 2020 14:19
@ghost ghost added needs-triage Waiting for first response or review from a maintainer. size/M Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. labels Mar 1, 2020
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Mar 3, 2020
@sutharshan-siva
Copy link

When we could expect this request to merge ? As of now , still we couldn't use enable_acceleration in terraform for vpn connection creation.

@sethbacon
Copy link

sethbacon commented Mar 5, 2020 •
edited
Loading

This is blocker for us ATM. I need to recreate most of our vpn connections and enable_acceleration. I would really like to do this ASAP. Any chance this can make it into 2.56.0? @bflad ?

@sethbacon
Copy link

@sjones-and , perhaps adding another reviewer request for one of the principals? I really need this pr to merge. Because VPNs can't be modified for acceleration after the fact, it means creating them outside of terraform which is a serious blocker for us.

@stujonesuk
Copy link
Author

@bflad sorry to ask, but any chance of getting this reviewed/merged? Thanks in advance.

@sutharshan-siva
Copy link

Adding to this. We were waiting for this one to merge almost more than 2 months which is a blocker as @sethbacon mentioned.

@sutharshan-siva
Copy link

@bflad any chance of getting this reviewed/merged soon in coming release ? This a blocker to enable vpn acceleration and require to create vpn from scratch outside of terraform. Stucked on this almost three months.

@wgr2017
Copy link

wgr2017 commented May 12, 2020

@bflad sorry to ask, but is there any ETA on getting this reviewed/merged? This is causing a blocker for us. Thanks in advance.

@rryke
Copy link

rryke commented May 16, 2020

also waiting

@sethbacon
Copy link

@ewbankkit ?

DrFaust92
DrFaust92 suggested changes Jun 3, 2020
View reviewed changes
aws/resource_aws_vpn_connection.go
"enable_acceleration": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is computed needed here?

aws/resource_aws_vpn_connection.go
@@ -291,8 +298,9 @@ func resourceAwsVpnConnectionCreate(d *schema.ResourceData, meta interface{}) er
}

connectOpts := &ec2.VpnConnectionOptionsSpecification{
StaticRoutesOnly: aws.Bool(d.Get("static_routes_only").(bool)),
TunnelOptions: options,
EnableAcceleration: aws.Bool(d.Get("enable_acceleration").(bool)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

was this tested with test cases where enable_acceleration is not specified?
probably better to add a false default value. it will fail otherwise.

website/docs/r/vpn_connection.html.markdown
@@ -75,6 +75,7 @@ One of the following arguments is required:

Other arguments:

* `enable_acceleration` - (Optional, Default `false`) Whether the VPN connection uses acceleration. Acceleration can only be enabled on VPNs terminated on a Transit Gateway.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Default false is not set in the resource schema

@mikereinhold
Copy link

Closes #12237

@MRinalducci
Copy link

@sjones-and can you have a look at the suggested changes? Not a big deal I think.

@sethbacon
Copy link

Any update on this?

@MRinalducci
Copy link

This PR #14740 covers all options.

@bill-rich bill-rich mentioned this pull request Dec 2, 2020
Merged
@bill-rich
Copy link
Contributor

Thank you for the work on this @sjones-and! It looks like #14740 covers the same options as you've got here, plus some more, so I'd like to manage it there.

@bill-rich bill-rich closed this Dec 2, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 2, 2021
@ghost
Copy link

ghost commented Jan 2, 2021

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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DrFaust92 DrFaust92 DrFaust92 requested changes

Assignees
No one assigned
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. size/M 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Accelerated Site-to-Site VPN support
10 participants
@stujonesuk @sutharshan-siva @sethbacon @wgr2017 @rryke @mikereinhold @MRinalducci @bill-rich @DrFaust92 @bflad