New resource: azurerm_iothub_shared_access_policy

[maxbog]

also, add test and docs

(resolves #2201)

[tombuildsstuff]

this is going to need to become:

Suggested change

	Optional: true,
	Type: schema.TypeSet,
	Optional: true,
	Computed: true,

This needs to remain computed since it's possible that users may only want to retrieve these and not set the values; otherwise this'll be a breaking change.

In addition - since these can now be defined in-line this'll need to be a TypeSet, which is a breaking change - but is required since these could be returned in any order at this point (and may differ from the ordering defined in a users config). Unfortunately this would also be a breaking change; as such I believe defining these in-line may not be the best way to expose these fields.

Instead I believe it would make sense to expose a Virtual Resource (ala the azurerm_virtual_Machine_data_disk_attachment` resource which looks up the VM and adds the Disk, then returns) as an independent resource - rather than defining these in-line. This would also allow these to be retrieved as a Data Source.

Unfortunately this would require the use of a Lock to ensure concurrent changes aren't made; however it should be possible to do so - as it appears there's no independent API available to manage Shared Access Policies within an IoTHub.

What do you think?

[tombuildsstuff]

hey @maxbog

Thanks for this PR :)

Taking a quick look through this I believe the Shared Access Policies may be better managed as a separate resource, rather than defined in-line (I've left a comment inline with more information) - WDYT?

Thanks!

[maxbog]

hey @tombuildsstuff,
This was actually my original idea, but I didn't feel comfortable enough with golang and terraform internals to implement this from scratch. However, now that I have something similar I can look at, I think I'll manage to create a separate resource.

However, there is a matter of the default access policies that are created together with the IoTHub. The new resource could "take over" the default policies if a policy with the same name is defined in the .tf file.

For example, when we would have a .tf file as below, I would expect the azurerm_iothub_shared_access_policy.iothubowner to redefine the default iothubowner policy.

resource "azurerm_iothub" "test" {
  resource_group_name = "${azurerm_resource_group.iothub_sap.name}"
  location = "${azurerm_resource_group.iothub_sap.location}"

  name = "test_iothub"

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

resource "azurerm_iothub_shared_access_policy" "iothubowner" {
  name = "iothubowner"
  registry_read = true
  registry_write = true
}

Also, I think that the user should have control over whether the default policies should be created, so I was thinking about a new provision_default_shared_access_policies property, with a default value of true, to not break backward compatibility.

Taking the above example further, if the azurerm_iothub.test.provision_default_shared_access_policies property was set to true, I would expect to provision an IoTHub with a single Access Policy.

However, if it were to false, I would like to have all the default policies provisioned and the iothubowner taken over by the azurerm_iothub_shared_access_policy.iothubowner resource.

Another thing is the existing shared_access_policy attribute. I would expect that after updating any of the azurerm_iothub_shared_access_policy resources, the azurerm_iothub resource should be "refreshed" with the latest changes, but I have no idea whether this is actually possible. Or, we could write a note in the documentation that the two cannot be reliably used together.

Could you point me in the right direction here?

All in all, I think that the existing attribute should be deprecated in favour of a new data source and actually removed in v2.0. What do you think?

[maxbog]

@tombuildsstuff: I created a new azurerm_iothub_shared_access_policy resource, reverted the changes in this PR and renamed it. Could you please review the code?

[maxbog]

@tombuildsstuff I'm stumped why the build is failing :(
In the terraform repo, tagged 0.11.9, there clearly are .go files, but Travis complains that the directory does not contain any.

Here is the file that I am using:
https://github.com/hashicorp/terraform/blob/v0.11.9/helper/customdiff/compose.go

Do you have any pointers to why the build is failing?

[tombuildsstuff]

@maxbog sorry missed these pings, will take a look now

[maxbog]

@tombuildsstuff Sorry for pestering, but did you have any time to review the PR? Is there anything I could do to make the review easier for you?

[katbyte]

Hi @maxbog, sorry for the delay. I've left some comments inline with my main comment being that i'm surprised by the number of dependencies affects by simply including the customDiff.all function? feels like some of those changes are spurious.

[maxbog]

@katbyte Thanks for the review! I addressed all the issues

[katbyte]

Thanks for the revisions @maxbog,

Aside from some minor comments my one question is the suppress.CaseDifference on the iothub_name, we don't suppress it anywhere else so i question if it is required here?

Overall this is looking great!

[maxbog]

BTW this solves #2201

[katbyte]

Thanks for the updates @maxbog, LGTM 👍

[ghost]

This has been released in version 1.25.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.25.0"
}
# ... other configuration ...

[ghost]

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!