azurerm_virtual_hub_connection - support the `routing` block #8950

neil-yechenwei · 2020-10-20T07:02:13Z

API Spec: https://docs.microsoft.com/en-us/rest/api/virtualwan/hubvirtualnetworkconnections/createorupdate#routingconfiguration

…_connection

tombuildsstuff

hi @neil-yechenwei

Thanks for this PR.

Taking a look through it appears the happy path is covered, but since we're missing update/removal test cases we're unable to confirm these work. As such can we look to add some more test cases and fix up the relevant bugs so that this can work?

Thanks!

azurerm/internal/services/network/virtual_hub_connection_resource.go

tombuildsstuff · 2020-10-20T10:25:48Z

azurerm/internal/services/network/virtual_hub_connection_resource.go

+									},
+
+									"route_table_ids": {
+										Type:     schema.TypeSet,


since this is a route table, the ordering likely matters - so why is this a set?

azurerm/internal/services/network/virtual_hub_connection_resource.go

tombuildsstuff · 2020-10-20T10:26:05Z

azurerm/internal/services/network/virtual_hub_connection_resource.go

+										Computed: true,
+										Elem: &schema.Schema{
+											Type:         schema.TypeString,
+											ValidateFunc: azure.ValidateResourceID,


we can use more specific validation here

I intend to use the validation of hub route table once PR 8939 is merged.

azurerm/internal/services/network/virtual_hub_connection_resource.go

tombuildsstuff · 2020-10-20T10:36:32Z

website/docs/r/virtual_hub_connection.html.markdown

+
+* `name` - (Optional) The name which should be used for this static_route.
+
+* `address_prefixes` - (Optional) The list of all address prefixes.


Suggested change

* `address_prefixes` - (Optional) The list of all address prefixes.

* `address_prefixes` - (Optional) A list of CIDR Blocks (presumably?) which should be used [..].

tombuildsstuff · 2020-10-20T10:36:55Z

website/docs/r/virtual_hub_connection.html.markdown

+
+* `address_prefixes` - (Optional) The list of all address prefixes.
+
+* `next_hop_ip_address` - (Optional) The ip address of the next hop.


Suggested change

* `next_hop_ip_address` - (Optional) The ip address of the next hop.

* `next_hop_ip_address` - (Optional) The IP Address which should be used for the Next Hop [..].

tombuildsstuff · 2020-10-20T11:04:06Z

azurerm/internal/services/network/virtual_hub_connection_resource.go

+
+func flattenArmVirtualHubConnectionRoutingConfiguration(input *network.RoutingConfiguration) []interface{} {
+	if input == nil {
+		return nil


this needs to be set to an empty list

tombuildsstuff · 2020-10-20T11:04:20Z

azurerm/internal/services/network/virtual_hub_connection_resource.go

+
+func expandArmVirtualHubConnectionVnetStaticRoute(input []interface{}) *network.VnetRoute {
+	if len(input) == 0 {
+		return nil


so how is this cleared?

tombuildsstuff · 2020-10-20T11:05:05Z

azurerm/internal/services/network/tests/virtual_hub_connection_resource_test.go

+					testCheckAzureRMVirtualHubConnectionExists(data.ResourceName),
+				),
+			},
+			data.ImportStep(),


we need more tests here, this only just one create case, can we add multiple routes, updates, removing the route etc?

neil-yechenwei · 2020-10-22T10:58:36Z

@tombuildsstuff , thanks for your comments. I've updated code by your comments. Please have a look. Thanks.

katbyte

Thanks @neil-yechenwei, overall this looks good but i've left some comments inline that should be addressed before merge

azurerm/internal/services/network/virtual_hub_connection_resource.go

katbyte · 2020-10-28T17:50:00Z

azurerm/internal/services/network/virtual_hub_connection_resource.go

@@ -76,6 +77,80 @@ func resourceArmVirtualHubConnection() *schema.Resource {
 				ForceNew: true,
 				Default:  false,
 			},
+
+			"routing_configuration": {


i don't think we need configuration here?

Suggested change

"routing_configuration": {

"routing": {

katbyte · 2020-10-28T17:51:39Z

azurerm/internal/services/network/virtual_hub_connection_resource.go

+							},
+						},
+
+						"vnet_static_route": {


would this make more sense as

Suggested change

"vnet_static_route": {

"static_vnet_route": {

katbyte · 2020-10-28T17:53:05Z

website/docs/r/virtual_hub_connection.html.markdown

+
+* `name` - (Optional) The name which should be used for this Static Route.
+
+* `address_prefixes` - (Optional) A list of CIDR Blocks which should be used for Virtual Hub Connection.


could we elaborate what an address prefix is?

katbyte · 2020-10-28T17:54:02Z

website/docs/r/virtual_hub_connection.html.markdown

+
+* `labels` - (Optional) The list of labels to provide a mechanism to logically group route tables.
+
+* `route_table_ids` - (Optional) A list of Route Table ID's which is used for Virtual Hub Connection.


grammer

Suggested change

* `route_table_ids` - (Optional) A list of Route Table ID's which is used for Virtual Hub Connection.

* `route_table_ids` - (Optional) A list of Route Table ID's to associated with this Virtual Hub Connection.

katbyte · 2020-10-28T18:05:18Z

website/docs/r/virtual_hub_connection.html.markdown

+
+A `routing_configuration` block supports the following:
+
+* `associated_route_table_id` - (Optional) The ID of route table associated with Virtual Hub connection.


grammer

Suggested change

* `associated_route_table_id` - (Optional) The ID of route table associated with Virtual Hub connection.

* `associated_route_table_id` - (Optional) The ID of the route table associated with this Virtual Hub connection.

katbyte · 2020-10-28T18:05:35Z

website/docs/r/virtual_hub_connection.html.markdown

+
+A `propagated_route_table` block supports the following:
+
+* `labels` - (Optional) The list of labels to provide a mechanism to logically group route tables.


Suggested change

* `labels` - (Optional) The list of labels to provide a mechanism to logically group route tables.

* `labels` - (Optional) The list of labels to assign to this route table.

neil-yechenwei · 2020-10-29T06:59:44Z

@katbyte , thanks for your comments. I've updated code. Please have a look. Thanks.

katbyte

We should still validate that the names are not emptyt?

neil-yechenwei · 2020-10-30T02:00:12Z

@katbyte , sure, I added empty string validation.

katbyte

Thanks @neil-yechenwei - this LGTM now 👍

ghost · 2020-12-04T14:50:36Z

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. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

neil-yechenwei added 2 commits October 14, 2020 16:43

Add support for property routing_configuration of azurerm_virtual_hub…

66f824e

…_connection

Add support for routingConfiguration of azurerm_virtual_hub_connection

b622b03

ghost added size/XL documentation labels Oct 20, 2020

Merge remote-tracking branch 'upstream/master' into vhubvnetconn

b580565

tombuildsstuff suggested changes Oct 20, 2020

View reviewed changes

tombuildsstuff added enhancement service/virtual-hub waiting-response labels Oct 20, 2020

ghost removed the waiting-response label Oct 20, 2020

tombuildsstuff added the waiting-response label Oct 21, 2020

tombuildsstuff assigned neil-yechenwei Oct 21, 2020

update code

6307e39

ghost removed the waiting-response label Oct 22, 2020

neil-yechenwei requested a review from tombuildsstuff October 22, 2020 10:58

katbyte requested changes Oct 28, 2020

View reviewed changes

update code

17cf6b6

neil-yechenwei requested a review from katbyte October 29, 2020 06:59

katbyte requested changes Oct 29, 2020

View reviewed changes

update code

8dd69e2

neil-yechenwei requested a review from katbyte October 30, 2020 02:00

katbyte approved these changes Nov 3, 2020

View reviewed changes

katbyte changed the title ~~Add support for routingConfiguration of azurerm_virtual_hub_connection~~ azurerm_virtual_hub_connection - support the routing block Nov 3, 2020

katbyte merged commit 6c0d71b into hashicorp:master Nov 3, 2020

katbyte added a commit that referenced this pull request Nov 3, 2020

update CHANGELOG.md for #8950

ee0d431

ghost locked as resolved and limited conversation to collaborators Dec 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azurerm_virtual_hub_connection - support the `routing` block #8950

azurerm_virtual_hub_connection - support the `routing` block #8950

neil-yechenwei commented Oct 20, 2020

tombuildsstuff left a comment

tombuildsstuff Oct 20, 2020

neil-yechenwei Oct 22, 2020

tombuildsstuff Oct 20, 2020

neil-yechenwei Oct 22, 2020

tombuildsstuff Oct 20, 2020

neil-yechenwei Oct 22, 2020

tombuildsstuff Oct 20, 2020

neil-yechenwei Oct 22, 2020

tombuildsstuff Oct 20, 2020

neil-yechenwei Oct 22, 2020

tombuildsstuff Oct 20, 2020

neil-yechenwei Oct 22, 2020

tombuildsstuff Oct 20, 2020

neil-yechenwei Oct 22, 2020

neil-yechenwei commented Oct 22, 2020

katbyte left a comment

katbyte Oct 28, 2020

neil-yechenwei Oct 29, 2020

katbyte Oct 28, 2020

neil-yechenwei Oct 29, 2020

katbyte Oct 28, 2020

neil-yechenwei Oct 29, 2020

katbyte Oct 28, 2020

neil-yechenwei Oct 29, 2020

katbyte Oct 28, 2020

neil-yechenwei Oct 29, 2020

katbyte Oct 28, 2020

neil-yechenwei Oct 29, 2020

neil-yechenwei commented Oct 29, 2020

katbyte left a comment

neil-yechenwei commented Oct 30, 2020

katbyte left a comment

ghost commented Dec 4, 2020


		* `name` - (Optional) The name which should be used for this static_route.

		* `address_prefixes` - (Optional) The list of all address prefixes.

	* `address_prefixes` - (Optional) The list of all address prefixes.
	* `address_prefixes` - (Optional) A list of CIDR Blocks (presumably?) which should be used [..].


		* `address_prefixes` - (Optional) The list of all address prefixes.

		* `next_hop_ip_address` - (Optional) The ip address of the next hop.


		* `labels` - (Optional) The list of labels to provide a mechanism to logically group route tables.

		* `route_table_ids` - (Optional) A list of Route Table ID's which is used for Virtual Hub Connection.


		A `routing_configuration` block supports the following:

		* `associated_route_table_id` - (Optional) The ID of route table associated with Virtual Hub connection.


		A `propagated_route_table` block supports the following:

		* `labels` - (Optional) The list of labels to provide a mechanism to logically group route tables.

azurerm_virtual_hub_connection - support the routing block #8950

azurerm_virtual_hub_connection - support the routing block #8950

Conversation

neil-yechenwei commented Oct 20, 2020

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei commented Oct 22, 2020

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei commented Oct 29, 2020

katbyte left a comment

Choose a reason for hiding this comment

neil-yechenwei commented Oct 30, 2020

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Dec 4, 2020

azurerm_virtual_hub_connection - support the `routing` block #8950

azurerm_virtual_hub_connection - support the `routing` block #8950