IoT Hub add endpoints and routes #1693

andrey-moor · 2018-07-31T19:51:40Z

Adds endpoint and route blocks to allow configuring routes as part of the IoT Hub resource.

tombuildsstuff

Thanks for this PR :)

I've taken a look through and left some comments in-line but this is otherwise off to a good start - if we can fix up the comments then we should be able to run the tests and get this merged :)

Thanks!

tombuildsstuff · 2018-08-01T14:33:33Z

azurerm/resource_arm_iothub.go

+						"condition": {
+							Type:     schema.TypeString,
+							Optional: true,
+							Default:  "true",


is there a reason why this is a String rather than a Bool?

That is intentional - there a special query syntax to filter events before routing them (
https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-devguide-query-language#device-to-cloud-message-routes-query-expression). But I think it makes sense to add a comment to avoid confusion in the future.

tombuildsstuff · 2018-08-01T14:34:24Z

azurerm/resource_arm_iothub.go

@@ -142,13 +243,26 @@ func resourceArmIotHubCreateAndUpdate(d *schema.ResourceData, meta interface{})
 	skuInfo := expandIoTHubSku(d)
 	tags := d.Get("tags").(map[string]interface{})

+	endpoints, err := expandIoTHubEndpoints(d, subscriptionID)


can we raise this error? e.g.

if err != nil { return fmt.Errorf("Error expanding `endpoints`: %+v", err) }

Yeah, good point - I've added error message.

tombuildsstuff · 2018-08-01T14:35:14Z

azurerm/resource_arm_iothub.go

+		condition := route["condition"].(string)
+
+		endpointNamesRaw := route["endpoint_names"].([]interface{})
+		endpointsNames := make([]string, 0, len(endpointNamesRaw))


can we ditch the last argument here (len(endpointNamesRaw) ) - this'll cause a series of blank items to be appended to the end of the array

tombuildsstuff · 2018-08-01T16:10:51Z

azurerm/resource_arm_iothub.go

+
+	if input != nil && input.Endpoints != nil {
+
+		for _, container := range *input.Endpoints.StorageContainers {


can we nil-check around this to ensure input.Endpoints.StorageContainers isn't nil?

tombuildsstuff · 2018-08-01T16:10:59Z

azurerm/resource_arm_iothub.go

+			results = append(results, output)
+		}
+
+		for _, queue := range *input.Endpoints.ServiceBusQueues {


can we nil-check around this to ensure input.Endpoints.ServiceBusQueues isn't nil?

tombuildsstuff · 2018-08-01T16:11:12Z

azurerm/resource_arm_iothub.go

+			results = append(results, output)
+		}
+
+		for _, topic := range *input.Endpoints.ServiceBusTopics {


can we nil-check around this to ensure input.Endpoints.ServiceBusTopics isn't nil?

tombuildsstuff · 2018-08-01T16:11:26Z

azurerm/resource_arm_iothub.go

+			results = append(results, output)
+		}
+
+		for _, eventHub := range *input.Endpoints.EventHubs {


can we nil-check around this to ensure input.Endpoints.EventHubs isn't nil?

tombuildsstuff · 2018-08-01T16:13:45Z

azurerm/resource_arm_iothub_test.go

@@ -100,9 +115,28 @@ resource "azurerm_iothub" "test" {
    capacity = "1"
  }

+  endpoint {


can we split this out into it's own test?

Agree - that will make it cleaner

andrey-moor · 2018-08-01T18:00:26Z

@tombuildsstuff thanks for the comments. Is there a clean way to execute acceptance tests against specific resource type? (wasn't able to figure it out by a quick look)

tombuildsstuff · 2018-08-02T09:37:44Z

@andrey-moor you should be able to do that via:

TF_ACC=1 go test ./azurerm -v -timeout 120m -run="TestAcc" -count=1

where TestAcc is the prefix of the tests to be run

I'll kick these off in our CI system in the mean time :)

tombuildsstuff

a couple of changes needed but this otherwise LGTM 👍

tombuildsstuff · 2018-08-02T09:37:17Z

azurerm/resource_arm_iothub.go

+
+	if input != nil && input.Endpoints != nil {
+
+		storageContainers := *input.Endpoints.StorageContainers


there's actually a crash here if .StorageContainers is nil (since the * is dereferencing the pointer) - we can instead make this:

if container := input.Endpoints.StorageContainers; container != nil { # .. use *container as needed }

tombuildsstuff · 2018-08-02T09:37:23Z

azurerm/resource_arm_iothub.go

+		}
+
+		sbTopics := *input.Endpoints.ServiceBusTopics
+		if sbTopics != nil {


tombuildsstuff · 2018-08-02T09:37:27Z

azurerm/resource_arm_iothub.go

+		}
+
+		eventHubs := *input.Endpoints.EventHubs
+		if eventHubs != nil {


tombuildsstuff · 2018-08-02T09:37:32Z

azurerm/resource_arm_iothub.go

+		}
+
+		sbQueues := *input.Endpoints.ServiceBusQueues
+		if sbQueues != nil {


tombuildsstuff · 2018-08-02T09:39:46Z

azurerm/resource_arm_iothub.go

+							},
+							Required: true,
+						},
+						"is_enabled": {


I'd suggest this could better match other resources in Terraform by being enabled rather than is_enabled - but this isn't a blocker; what do you think?

Good point 👍

andrey-moor · 2018-08-10T00:58:09Z

@tombuildsstuff is there anything else we can do here?

tombuildsstuff · 2018-08-15T13:06:10Z

@andrey-moor hey sorry for the delayed re-review here - I've pushed one commit to fix a minor documentation issue but this now LGTM - we're prepping for the release of v1.13.0 but we'll kickoff the tests for this once that's out :)

tombuildsstuff · 2018-08-21T12:39:30Z

Tests pass:

tombuildsstuff

hey @andrey-moor

Thanks for pushing those changes - I've taken a look through and this now LGTM - thanks for this / sorry for the delay getting this re-reviewed 👍

Thanks!

* master: (95 commits) Some more AzureStack changes for route table resource and datasource (hashicorp#1807) backport AzureStack route resource PR comments (hashicorp#1810) Updating to include hashicorp#1799 Bug Fix of azurerm_virtual_machine.storage_os_disk.image_uri (hashicorp#1799) Updating to include hashicorp#1693 IoT Hub add endpoints and routes (hashicorp#1693) Updating to include hashicorp#1789 Added support for EventHub compatible EndPoints and Paths (hashicorp#1789) Updating to include hashicorp#1798 Remove client side validation for azure network plugin (hashicorp#1798) backport azure stack route_table PR review comments (hashicorp#1790) Update CHAGELOG.md to include hashicorp#1795 Fix: Corrected regexp for eventhub name reset verb fix some pointer dereferences Fix indentation on application_gateway.html.markdown (hashicorp#1780) folded subscription and [az]schema packages into azure consolidate CaseDifference Supression functions Cleanup after v1.13.0 release v1.13.0 ...

tombuildsstuff · 2018-09-06T17:58:44Z

hey @andrey-moor

Just to let you know that this has been released in v1.14.0 of the AzureRM Provider which is now available: https://github.com/terraform-providers/terraform-provider-azurerm/blob/v1.14.0/CHANGELOG.md

Thanks!

ghost · 2019-03-06T14:41:33Z

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!

andrey-moor added 2 commits July 31, 2018 12:32

Add endpoint and route attributes to an IoTHub resource

4d0e48d

Fix route flattening error description

67bb114

tombuildsstuff suggested changes Aug 1, 2018

View reviewed changes

tombuildsstuff added enhancement service/iot-hub labels Aug 1, 2018

Address PR comments

2bada26

Fix error message description

f8456dd

tombuildsstuff reviewed Aug 2, 2018

View reviewed changes

tombuildsstuff added the waiting-response label Aug 2, 2018

andrey-moor added 2 commits August 2, 2018 10:39

Fix acceptance tests

b417541

Format resource_arm_iothub.go properly

0bd62d0

metacpp requested a review from WodansSon August 3, 2018 00:03

metacpp assigned WodansSon Aug 3, 2018

tombuildsstuff unassigned WodansSon Aug 3, 2018

tombuildsstuff removed the request for review from WodansSon August 3, 2018 07:22

tombuildsstuff added this to the 1.13.0 milestone Aug 3, 2018

andrey-moor and others added 3 commits August 3, 2018 09:35

Merge branch 'master' into master

7450461

Fix formatting

df6427e

Fix formatting

20f435b

tombuildsstuff removed the waiting-response label Aug 15, 2018

is_enabled -> enabled

abe4990

tombuildsstuff modified the milestones: 1.13.0, 1.14.0 Aug 15, 2018

tombuildsstuff approved these changes Aug 21, 2018

View reviewed changes

tombuildsstuff merged commit 2db1532 into hashicorp:master Aug 21, 2018

tombuildsstuff added a commit that referenced this pull request Aug 21, 2018

Updating to include #1693

8b25b8c

ghost locked and limited conversation to collaborators Mar 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IoT Hub add endpoints and routes #1693

IoT Hub add endpoints and routes #1693

andrey-moor commented Jul 31, 2018

tombuildsstuff left a comment

tombuildsstuff Aug 1, 2018

andrey-moor Aug 1, 2018

tombuildsstuff Aug 1, 2018

andrey-moor Aug 1, 2018

tombuildsstuff Aug 1, 2018

tombuildsstuff Aug 1, 2018

tombuildsstuff Aug 1, 2018

tombuildsstuff Aug 1, 2018

tombuildsstuff Aug 1, 2018

tombuildsstuff Aug 1, 2018

andrey-moor Aug 1, 2018

andrey-moor commented Aug 1, 2018 •

edited

Loading

tombuildsstuff commented Aug 2, 2018

tombuildsstuff left a comment

tombuildsstuff Aug 2, 2018

tombuildsstuff Aug 2, 2018

tombuildsstuff Aug 2, 2018

tombuildsstuff Aug 2, 2018

tombuildsstuff Aug 2, 2018

andrey-moor Aug 2, 2018

andrey-moor commented Aug 10, 2018

tombuildsstuff commented Aug 15, 2018 •

edited

Loading

tombuildsstuff commented Aug 21, 2018

tombuildsstuff left a comment

tombuildsstuff commented Sep 6, 2018

ghost commented Mar 6, 2019


		if input != nil && input.Endpoints != nil {

		for _, container := range *input.Endpoints.StorageContainers {


		if input != nil && input.Endpoints != nil {

		storageContainers := *input.Endpoints.StorageContainers

IoT Hub add endpoints and routes #1693

IoT Hub add endpoints and routes #1693

Conversation

andrey-moor commented Jul 31, 2018

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

andrey-moor commented Aug 1, 2018 • edited Loading

tombuildsstuff commented Aug 2, 2018

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

andrey-moor commented Aug 10, 2018

tombuildsstuff commented Aug 15, 2018 • edited Loading

tombuildsstuff commented Aug 21, 2018

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff commented Sep 6, 2018

ghost commented Mar 6, 2019

andrey-moor commented Aug 1, 2018 •

edited

Loading

tombuildsstuff commented Aug 15, 2018 •

edited

Loading