Azure / terraform

Source code for the Azure Marketplace Terraform development VM package.
MIT License
693 stars 784 forks source link

New Sample for iothub with private link #314

Closed abeebu closed 3 months ago

abeebu commented 7 months ago

This PR adds a new quickstart sample for deploying iothub with device provisioning service behind a virtual network

abeebu commented 6 months ago

@lonegunmanb Would you be able to review this PR please?

lonegunmanb commented 6 months ago

@lonegunmanb Would you be able to review this PR please?

Hi @abeebu thanks for opening this pr! Overall it LGTM but I found that you've created a private dns zone for event hub, but no resource for event hub, no azurerm_eventhub, no private endpoint or private link for eventhub. Is that correct?

abeebu commented 6 months ago

@lonegunmanb Would you be able to review this PR please?

Hi @abeebu thanks for opening this pr! Overall it LGTM but I found that you've created a private dns zone for event hub, but no resource for event hub, no azurerm_eventhub, no private endpoint or private link for eventhub. Is that correct?

@lonegunmanb yes, this configuration utilizes the built-in eventhub endpoint for iothub. So we(the customer) does not need to deploy a separate eventhub resource. However to ensure private connectivity between the iothub and its builtin endpoint, we need to add the private dns zone and create a dns record for its IP.

See here for more details on the builtin endpoint.

In a follow up PR later this month, I will also show the configuration for deploying the iothub together with a separate storage resource and eventhub resource for message routing all behind a VNET.

abeebu commented 6 months ago

Just a nudge that this PR is awaiting your action @lonegunmanb 😁

abeebu commented 3 months ago

Tagging you @stemaMSFT and @TomArcherMsft 🙂

TomArcherMsft commented 3 months ago

@abeebu Thanks much for the contribution.

If you haven't done so already, please read the Contributor Guide article, "Publish a Terraform Quickstart".

  1. [main.tf] Please utilize the pattern required by the PG for the rg name. You can look at the following code for an example.
  2. [providers.tf] You currently have a provider version constraint of: >= 3.35.0, < 4.0.0. I'm assuming you're specifying 3.35 and above but less than 4.0. Is that correct? If so, the canonical way to write that is ~> 3.351. The pessimistic constraint operator (the tilde) basically tells Terraform that you want the latest version but only incrementing the right-most version component.
  3. [variables.tf] Use the pattern for declaring the rg vars as used in the following example: variables.tf
  4. [readme.md] Use the table format as in https://github.com/Azure/terraform/blob/master/quickstart/101-resource-group/variables.tf
  5. [readme.md] The first two rows of the table should be the rg prefix and rg location as in https://github.com/Azure/terraform/blob/master/quickstart/101-resource-group/variables.tf
  6. @grayzu This PR has both a main.tf and a network.tf. Are you ok with that, or should everything go into main.tf?
  7. @abeebu What is the Azure CLI command to display the main resource created with this sample? Are all the params required by the Azure CLI command in the outputs.tf file?
  8. @lonegunmanb Could you please review when time permits? I've put it on the kanban board. Thanks!
lonegunmanb commented 3 months ago

Apology for the late reply @abeebu & @TomArcherMsft , LGTM!