-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
New resources: azurerm_iothub_device_update_account
azurerm_iothub_device_update_instance
#18789
Conversation
03e8048
to
2f3f508
Compare
…_device_update_instance`
2f3f508
to
1b75770
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR @myc2h6o, there are still some test failures relating to the region though
------- Stdout: -------
=== RUN TestAccIotHubDeviceUpdateAccount_sku
=== PAUSE TestAccIotHubDeviceUpdateAccount_sku
=== CONT TestAccIotHubDeviceUpdateAccount_sku
testcase.go:110: Step 1/4 error: Error running apply: exit status 1
Error: creating Account (Subscription: "*******"
Resource Group Name: "acctest-rg-221018105259715989"
Account Name: "acc-dua-eu3sy"): performing AccountsCreate: deviceupdates.DeviceupdatesClient#AccountsCreate: Failure sending request: StatusCode=0 -- Original Error: Code="LocationNotAvailableForResourceType" Message="The provided location 'westeurope' is not available for resource type 'Microsoft.DeviceUpdate/accounts'. List of available regions for the resource type is 'westus2,northeurope,southeastasia,eastus,eastus2,southcentralus,swedencentral,uksouth,westus3'."
with azurerm_iothub_device_update_account.test,
on terraform_plugin_test.tf line 13, in resource "azurerm_iothub_device_update_account" "test":
13: resource "azurerm_iothub_device_update_account" "test" {
creating Account (Subscription: "*******"
Resource Group Name: "acctest-rg-221018105259715989"
Account Name: "acc-dua-eu3sy"): performing AccountsCreate:
deviceupdates.DeviceupdatesClient#AccountsCreate: Failure sending request:
StatusCode=0 -- Original Error: Code="LocationNotAvailableForResourceType"
Message="The provided location 'westeurope' is not available for resource
type 'Microsoft.DeviceUpdate/accounts'. List of available regions for the
resource type is
'westus2,northeurope,southeastasia,eastus,eastus2,southcentralus,swedencentral,uksouth,westus3'."
--- FAIL: TestAccIotHubDeviceUpdateAccount_sku (120.85s)
FAIL
Hi @stephybun thanks for reviewing the pr! I've already updated the region here: https://github.com/hashicorp/terraform-provider-azurerm/pull/18789/files#diff-c9a4f5ece9fe921f274cae0220ba39a13027c503c6bd80e3cc9ee926565a5c94, to replace the |
Hi @stephybun I've double checked, looks like the .teamcity location settings only take effect after merged. I've split out the test location change to #18884 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @myc2h6o
The update methods I think need revisiting. The removal of the data from the payloads there is an anti-pattern that shouldn't be necessary and we should avoid within the provider. Can you speak with the service team on your side to resolve?
Thanks!
existing.Properties.HostName = nil | ||
existing.Properties.Locations = nil | ||
existing.SystemData = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't need to be zeroed, ARM spec says the service is supposed to ignore R/O fields on update payloads? Needing this would indicated a bug?
existing.Tags = &model.Tags | ||
} | ||
|
||
existing.SystemData = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, this shouldn't be necessary.
} | ||
|
||
if matched := regexp.MustCompile(`^[A-Za-z0-9]+(-[A-Za-z0-9]+)*$`).Match([]byte(value)); !matched { | ||
errors = append(errors, fmt.Errorf("%q may only contain alphanumeric characters and dashes, and consecutive dashes (-) are not allowed", k)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errors = append(errors, fmt.Errorf("%q may only contain alphanumeric characters and dashes, and consecutive dashes (-) are not allowed", k)) | |
errors = append(errors, fmt.Errorf("%q must start with an alphanumeric, may only contain alphanumeric characters and dashes, and consecutive dashes (-) are not allowed", k)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message updated.
Hi @jackofallops thanks for reviewing the pr! I've updated the error message, and removed the unexpected assignment of |
|
||
* `identity` - (Optional) An `identity` block as defined below. | ||
|
||
* `public_network_access` - (Optional) The Public Network Access setting of the IoT Hub Device Update Account. Possible values are `Enabled` and `Disabled`. Defaults to `Enabled`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a bool?
* `public_network_access` - (Optional) The Public Network Access setting of the IoT Hub Device Update Account. Possible values are `Enabled` and `Disabled`. Defaults to `Enabled`. | |
* `public_network_access_enabled` - (Optional) The Public Network Access setting of the IoT Hub Device Update Account. Possible values are `Enabled` and `Disabled`. Defaults to `Enabled`. |
|
||
* `name` - (Required) Specifies the name which should be used for this IoT Hub Device Update Instance. Changing this forces a new resource to be created. | ||
|
||
* `iothub_device_update_account_id` - (Required) Specifies the ID of the IoT Hub Device Update Account where the IoT Hub Device Update Instance exists. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iothub is in the rsource name so
* `iothub_device_update_account_id` - (Required) Specifies the ID of the IoT Hub Device Update Account where the IoT Hub Device Update Instance exists. Changing this forces a new resource to be created. | |
* `device_update_account_id` - (Required) Specifies the ID of the IoT Hub Device Update Account where the IoT Hub Device Update Instance exists. Changing this forces a new resource to be created. |
Hi @katbyte thanks for reviewing the pr! I've renamed |
2060108
to
cde3fd0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @myc2h6o - LGTM 🗺️
This functionality has been released in v3.29.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
iothub
directorywesteurope
, so updating the teamcity locations withnortheurope
instead