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 Gateway Generations #333

Closed
dan-dimitrov opened this issue Apr 11, 2022 · 4 comments · Fixed by #345
Closed

VPN Gateway Generations #333

dan-dimitrov opened this issue Apr 11, 2022 · 4 comments · Fixed by #345
Assignees
Labels
enhancement New feature or request PR-merged

Comments

@dan-dimitrov
Copy link

dan-dimitrov commented Apr 11, 2022

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

An error message appears when attempting to deploy the VPN gateway in the hub with the SKU VpnGw5AZ. This deployment automatically determined the gateway generation as generation 1 and the SKU is not available. The following advanced configuration needs to be used in order to successfully deploy the VPN gateway as this SKU.

  advanced = {
    custom_settings_by_resource_type = {
      azurerm_virtual_network_gateway = {
        connectivity = {
          vpngw = {
            (var.connectivity_resources_location) = {
              generation = "Generation2"
            }
          }
        }
      }
    }
  }

It would be good if this could be rectified automatically, or if we can specify the gateway generation manually without using the advanced configuration.

@ghost ghost added the Needs: Triage 🔍 Needs triaging by the team label Apr 11, 2022
@krowlandson krowlandson self-assigned this Apr 12, 2022
@ghost ghost removed the Needs: Triage 🔍 Needs triaging by the team label Apr 12, 2022
@krowlandson krowlandson added the enhancement New feature or request label Apr 12, 2022
@krowlandson
Copy link
Contributor

Thank you for logging this @dan-dimitrov

Adding this to the input variable schema will be a breaking change so I will try to include this on the v1.2.0 release where we will be introducing other breaking changes if we cannot automatically correlate the generation value based on sku alone.

@krowlandson krowlandson added this to the v1.2.0 release milestone Apr 12, 2022
@krowlandson
Copy link
Contributor

@krowlandson
Copy link
Contributor

Considerations for auto-determination of generation by sku:

  1. The product team has a preference for customers to move to Generation 2 SKUs
  2. Given the overlap between Generation1 and Generation2 SKUs, customers who have already deployed SKUs from VpnGw1 to VpnGw3 will be using Generation1
  3. If we auto-set Generation2 for all supported SKUs, customers within the overlapping SKUs (VpnGw1 and VpnGw2) will have their gateways re-deployed.
  4. Generation setting is not required for ExpressRoute SKUs.

To avoid unexpected redeployments, we are thinking to tackle this the following way:

  1. Auto-set generation for SKU VpnGw4/VpnGw4AZ and above to Generation2
  2. Because of consideration 4 above, we will not add the generation attribute within the gateway config input and this will continue to be handled via the advanced block
  3. We will target a future release for updating the preference to Generation2 for all supported SKUs

krowlandson pushed a commit to krowlandson/terraform-azurerm-caf-enterprise-scale that referenced this issue Apr 19, 2022
@krowlandson
Copy link
Contributor

Following further discussions around this issue, we have decided to set a preference to Generation2 and will include a breaking change warning in the release notes along with instructions on how to use the advanced block if a customer wants to continue using Generation1.

@krowlandson krowlandson mentioned this issue Apr 19, 2022
6 tasks
@ghost ghost added the PR-referenced label Apr 19, 2022
krowlandson pushed a commit that referenced this issue Apr 25, 2022
* Cast `tomap()` to fix #340

* Add advanced input to fix #342

* Add logic to handle VPN GW generation to fix #333

* Add `advanced_vpn_settings`
- Fix #232
- Fix #334

* Custom `ip_configuration` disables PIP creation
- Fix #232

* Update to support provider version `v3.0.2`

* Link `secondary_location` to settings module

* Update to test VPN gateway without AZ

* Update resource type in ID prefix

* Update baseline for `v3.0.2` provider support

* Fix sensitive values error

* Update strategy to test using Terraform `v1.1.x`

* Update OPA guidance
@ghost ghost added PR-merged and removed PR-referenced labels Apr 25, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request PR-merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants