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

azurerm_virtual_hub - support for virtual_router_asn and virtual_router_ips properties #15741

Merged
merged 8 commits into from
Mar 22, 2022

Conversation

jakubpech
Copy link
Contributor

@jakubpech jakubpech commented Mar 8, 2022

This PR enables deployments of managed NVA appliances (like VMware SD-WAN, Meraki VMX) to Azure Virtual WAN hub.

PR adds two new computed parameters to virtual_hub resource/datasource, which are required for the network integration:
"virtual_router_asn"
"virtual_router_ips"

PR addresses GH issue #13427, where the managed NVA application's read function returns an empty parameter 'tags' of the type map[string]interface{} which is not supported in the current code. To make this clear, these are not resource-level tags, but tags inside of the "parameters" parameter.
I do not see an easy way how to add support of nested tags to the "parameters" schema without making breaking changes so I've at least hot-fixed the problem when the 'tags' are returned but are empty. If the tags will not be empty, the error will be returned as before.

Tested NVA appliances do not use the tags.

@jakubpech
Copy link
Contributor Author

successful deployment and output of newly added computed params

λ terraform apply -auto-approve

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create
 <= read (data resources)

Terraform will perform the following actions:


  # azurerm_managed_application.virtual_edges_eu_west[0] will be created
  + resource "azurerm_managed_application" "virtual_edges_eu_west" {
      + id                          = (known after apply)
      + kind                        = "MarketPlace"
      + location                    = "westeurope"
      + managed_resource_group_name = "sdwan_nva_managed_grp_eu_west_testa"
      + name                        = "sdwanClusterEuWestTesta"
      + outputs                     = (known after apply)
      + parameter_values            = (known after apply)
      + parameters                  = {
          + "activationKey1"   = "XXXX-XXXX-XXXX-XXXX"
          + "activationKey2"   = "YYYY-YYYY-YYYY-YYYY"
          + "asn"              = "64600"
          + "hubId"            = "/subscriptions/xxxxxxxx-75c5-4e54-8d9b-9af81e2fd11b/resourceGroups/vwan_westeurope/providers/Microsoft.Network/virtualHubs/hub_westeurope_1"
          + "ignoreCertErrors" = "false"
          + "location"         = "westeurope"
          + "nvaName"          = "sdwanClusterEuWestTesta"
          + "scaleUnit"        = "2"
          + "vco"              = "vco23-fra1.velocloud.net"
          + "version"          = "latest"
        }
      + resource_group_name         = "velocloud_sdwan_nva_grp_eu_west"

      + plan {
          + name      = "vmwaresdwaninvwan"
          + product   = "vmware_sdwan_in_vwan"
          + publisher = "velocloud"
          + version   = "4.3.0"
        }
    }

  # azurerm_marketplace_agreement.vmware_sdwan_eu_west will be created
  + resource "azurerm_marketplace_agreement" "vmware_sdwan_eu_west" {
      + id                  = (known after apply)
      + license_text_link   = (known after apply)
      + offer               = "vmware_sdwan_in_vwan"
      + plan                = "vmwaresdwaninvwan"
      + privacy_policy_link = (known after apply)
      + publisher           = "velocloud"
    }

Plan: 2 to add, 0 to change, 0 to destroy.

Changes to Outputs:
  + azure_vwan_hub_eu_west_asn             = 65515
  + azure_vwan_hub_eu_west_ips             = [
      + "10.222.0.68",
      + "10.222.0.69",
    ]
azurerm_marketplace_agreement.vmware_sdwan_eu_west: Creating...
azurerm_marketplace_agreement.vmware_sdwan_eu_west: Creation complete after 8s [id=/subscriptions/xxxxxxxx-75c5-4e54-8d9b-9af81e2fd11b/providers/Microsoft.MarketplaceOrdering/agreements/velocloud/offers/vmware_sdwan_in_vwan/plans/vmwaresdwaninvwan]
azurerm_managed_application.virtual_edges_eu_west[0]: Creating...
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [40s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [50s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [1m0s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [1m10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [1m20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [1m30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [1m40s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [1m50s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [2m0s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [2m10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [2m20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [2m30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [2m40s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [2m50s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [3m0s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [3m10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [3m20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [3m30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [3m40s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [3m50s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [4m0s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [4m10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [4m20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [4m30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [4m40s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [4m50s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [5m0s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [5m10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [5m20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [5m30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [5m40s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [5m50s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [6m0s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [6m10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [6m20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [6m30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [6m40s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [6m50s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [7m0s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [7m10s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [7m20s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Still creating... [7m30s elapsed]
azurerm_managed_application.virtual_edges_eu_west[0]: Creation complete after 7m35s [id=/subscriptions/xxxxxxxx-75c5-4e54-8d9b-9af81e2fd11b/resourceGroups/velocloud_sdwan_nva_grp_eu_west/providers/Microsoft.Solutions/applications/sdwanClusterEuWestTesta]

Apply complete! Resources: 2 added, 0 changed, 0 destroyed.

Outputs:

azure_vwan_hub_eu_west_asn = 65515
azure_vwan_hub_eu_west_ips = toset([
  "10.222.0.68",
  "10.222.0.69",
])

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @jakubpech - overall looks good, but could we add these properties to a test ?

@jakubpech
Copy link
Contributor Author

@katbyte new properties added to tests

@jakubpech jakubpech requested a review from katbyte March 14, 2022 07:01
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @jakubpech. I left some suggestions and we also have some test failures.

------- Stdout: -------
=== RUN   TestAccVirtualHub_basic
=== PAUSE TestAccVirtualHub_basic
=== CONT  TestAccVirtualHub_basic
testcase.go:110: Step 1/2 error: Check failed: Check 3/3 error: azurerm_virtual_hub.test: Attribute 'virtual_router_ips' expected to be set
--- FAIL: TestAccVirtualHub_basic (1682.11s)
FAIL

------- Stdout: -------
=== RUN   TestAccDataSourceAzureRMVirtualHub_basic
=== PAUSE TestAccDataSourceAzureRMVirtualHub_basic
=== CONT  TestAccDataSourceAzureRMVirtualHub_basic
testcase.go:110: Step 1/1 error: Check failed: Check 4/4 error: data.azurerm_virtual_hub.test: Attribute 'virtual_router_ips' expected to be set
--- FAIL: TestAccDataSourceAzureRMVirtualHub_basic (1849.56s)
FAIL

@jakubpech
Copy link
Contributor Author

All requested changes are processed and fmt is fixed.

I'm still a bit struggling with running the azurerm acceptance tests from my work windows laptop. I guess some permission issues :(
testcase.go:110: failed to create new working directory: symlink ...

But it builds ok, runs as expected and all checks have passed on GitHub.

@katbyte katbyte changed the title add missing params and hotfix bug required to deploy managed vWAN Network Virtual Appliances azurerm_virtual_hub - support for virtual_router_asn and virtual_router_ips properties Mar 22, 2022
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @jakubpech - LGTM 🦀

@katbyte katbyte merged commit 3865165 into hashicorp:main Mar 22, 2022
@github-actions github-actions bot added this to the v3.0.0 milestone Mar 22, 2022
katbyte added a commit that referenced this pull request Mar 22, 2022
@github-actions
Copy link

This functionality has been released in v3.0.0 of the Terraform 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. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, 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 Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants