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

Consider splitting azurerm_iothub resource into several smaller ones #3303

Closed
maxbog opened this issue Apr 24, 2019 · 8 comments
Closed

Consider splitting azurerm_iothub resource into several smaller ones #3303

maxbog opened this issue Apr 24, 2019 · 8 comments
Labels
new-resource new-virtual-resource Resources which are split out to enhance the user experience service/iot-hub
Milestone

Comments

@maxbog
Copy link
Contributor

maxbog commented Apr 24, 2019

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

Right now, all endpoints and routes for IoTHub must be known at the moment of IoTHub creation. In my case, this causes unwanted coupling, which I'd like to eliminate. It's best to show an example.

Let's say we have two terraform modules:

  • iothub - this one contains (among other things) the iothub configuration
  • reporting - this one has some resources which will be used to process the messages from IoTHub.

I want IoTHub to have several routes which forward messages to different Event Hubs. Right now, I would have to define all of those event hubs in the iothub module, so that I can pass the connection string to the azurerm_iothub resource. However, what I'd like to be able to do is to define IoTHub only in the iothub module and configure endpoints and routing in the reporting module.

New or Affected Resource(s)

  • azurerm_iothub
  • azurerm_iothub_route
  • azurerm_iothub_fallback_route
  • azurerm_iothub_endpoint_eventhub
  • azurerm_iothub_endpoint_storage_container
  • azurerm_iothub_endpoint_servicebus_queue
  • azurerm_iothub_endpoint_servicebus_topic

Potential Terraform Configuration

resource "azurerm_iothub" "example" {
  resource_group_name = "example"
  location = "East US"

  name = "example"

  sku {
    capacity = 1
    name = "S1"
    tier = "Standard"
  }
}

resource "azurerm_iothub_endpoint_eventhub" "example_endpoint1" {
  resource_group_name = "${azurerm_iothub.example.resource_group_name}"
  iothub_name = "${azurerm_iothub.example.name}"

  connection_string = "..."
  name = "example-endpoint1"
}

resource "azurerm_iothub_endpoint_storage_container" "example_endpoint2" {
  resource_group_name = "${azurerm_iothub.example.resource_group_name}"
  iothub_name = "${azurerm_iothub.example.name}"

  connection_string = "..."
  batch_frequency_in_seconds = 60
  container_name = "example"
  file_name_format = "{iothub}/{partition}/{YYYY}/{MM}/{DD}/{HH}/{mm}"
  encoding = "avro"
  max_chunk_size_in_bytes = 10485760
  name = "example-endpoint2"
}

resource "azurerm_iothub_route" "example_route1" {
  resource_group_name = "${azurerm_iothub.example.resource_group_name}"
  iothub_name = "${azurerm_iothub.example.name}"
  enabled = true
  endpoint_names = [ "${azurerm_iothub_endpoint.example_endpoint1.name}"]
  name = "example-route1"
  source = "DeviceMessages"
  condition = "message-type = 'type1'"
}

resource "azurerm_iothub_route" "example_route2" {
  resource_group_name = "${azurerm_iothub.example.resource_group_name}"
  iothub_name = "${azurerm_iothub.example.name}"
  enabled = true
  endpoint_names = [ "${azurerm_iothub_endpoint.example_endpoint2.name}"]
  name = "example-route2"
  source = "DeviceMessages"
  condition = "message-type = 'type2'"
}

I am willing to work on this if you think it's in line with how the azurerm provider is developing.

References

@maxbog
Copy link
Contributor Author

maxbog commented May 8, 2019

Have you had any time to think about this issue? Like I said, I am willing to implement the change, so the only thing needed right now is an OK/nope from the core team with some possible comments :)

@maxbog
Copy link
Contributor Author

maxbog commented May 17, 2019

ping

@tombuildsstuff
Copy link
Contributor

hey @maxbog

Thanks for opening this issue / apologies for the delayed response here.

Taking a look into this, I think splitting this out would make sense - from what I can see I think this'd be simplest to start with the endpoint field/resource. Since the only API endpoint available for IoTHubs appears to be the one we're currently using (and I can't find a Data Plane SDK/API we could use to make a more granular API call) - we'll have to fake this in several ways:

  1. We'll need to use Locks inside the IoTHub resources, to ensure that they're not being modified at the same time (example)
  2. We'll also need to build/fake a Resource ID for the IoTHub child resources (example) - since all of the information we require to look it up must be available in the ID (at Import time)
  3. Finally we'll need to deprecate the built-in endpoints and route blocks (and make them Computed) so that we can replace them with the new resources, and then remove them in 2.0.

WDYT?

Thanks!

@tombuildsstuff tombuildsstuff added waiting-response new-virtual-resource Resources which are split out to enhance the user experience and removed question thinking labels May 21, 2019
@maxbog
Copy link
Contributor Author

maxbog commented May 21, 2019

Hi, thanks for the response!

I agree with you on all the points and this is actually how I planned to proceed.

However, I'd like to make one clarification before I proceed. Currently, there is a single field named endpoint in the azurerm_iothub resource, which is actually used to define 4 different types of endpoints (Storage Container, Service Bus Queue, Service Bus Topic and Event Hub). I was thinking about splitting it into 4 resources, one for each of the types, as each of them has different mandatory fields. They would of course share the majority of the code underneath, but I think having separate resources with their own, simpler documentation would be beneficial for the user.

What do you think?

Either way, please expect some PRs in the coming weeks :)

@ghost ghost removed the waiting-response label May 21, 2019
@tombuildsstuff
Copy link
Contributor

tombuildsstuff commented May 22, 2019

@maxbog yep, totally agree on splitting those out into the 4 sub-resources 👍 (sorry I should have clarified that)

Thanks!

@tombuildsstuff
Copy link
Contributor

Closing since #3679 has been merged via #4965

@tombuildsstuff tombuildsstuff added this to the v1.38.0 milestone Nov 28, 2019
@ghost
Copy link

ghost commented Dec 7, 2019

This has been released in version 1.38.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.38.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Dec 28, 2019

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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Dec 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-resource new-virtual-resource Resources which are split out to enhance the user experience service/iot-hub
Projects
None yet
Development

No branches or pull requests

3 participants