-
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 Resource: azurerm_iotcentral_application #5446
Conversation
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 the pr @xuzhang3! I've left some comments inline that need to be addressed before merging
azurerm/internal/services/iotcentral/resource_arm_iotcentral_application.go
Outdated
Show resolved
Hide resolved
return fmt.Errorf("Error setting `sku`: %+v", err) | ||
} | ||
|
||
d.Set("sub_domain", resp.AppProperties.Subdomain) |
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.
We should nil check here:
if props := resp.AppProperties; props != nil {
d.Set("sub_domain", props.Subdomain)
...
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.
fixed
azurerm/internal/services/iotcentral/resource_arm_iotcentral_application.go
Outdated
Show resolved
Hide resolved
name := d.Get("name").(string) | ||
resourceGroup := d.Get("resource_group_name").(string) |
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.
We'll need to parse the ID here. An example of this can be found in this PR: #5409
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.
fixed. add parseId function
azurerm/internal/services/iotcentral/resource_arm_iotcentral_application.go
Outdated
Show resolved
Hide resolved
func testAccAzureRMIotCentralApplication_basic(data acceptance.TestData) string { | ||
return fmt.Sprintf(` | ||
resource "azurerm_resource_group" "test" { | ||
name = "acctestRG-%[2]d" |
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.
Could we use
name = "acctestRG-%[2]d" | |
name = "acctestRG-iotc-%[1]d" |
the first indexed argument should be 1
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.
fixed
sku = "S1" | ||
template = "[email protected]" | ||
tags = { | ||
|
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.
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.
fixed
"a1", "11", "1a", "aa", | ||
"1-1", "aaa-aa", "a--a-aa", | ||
"a1-1", "a1-a", "1a-1", "1a-a-1-2", | ||
"abcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcde", | ||
"abcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcde123", |
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.
Could we make this consistent with other validation tests in the provideR?
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.
fixed
|
||
The following attributes are exported: | ||
|
||
* `id` - The ID of the IoTCentralApplication. |
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.
* `id` - The ID of the IoTCentralApplication. | |
* `id` - The ID of the IoT Central Application. |
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.
fixed
IoTCentralApplication can be imported using the `resource id`, e.g. | ||
|
||
```shell | ||
terraform import azurerm_iotcentral_application.app1 /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/providers/Microsoft.IoTCentral/IoTApps/app1 |
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.
terraform import azurerm_iotcentral_application.app1 /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/providers/Microsoft.IoTCentral/IoTApps/app1 | |
terraform import azurerm_iotcentral_application.example /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/providers/Microsoft.IoTCentral/IoTApps/app1 |
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.
fixed
fixed review code problems |
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.
Hi @xuzhang3,
I tried to run the tests for this today and got the following error:
[12:28:41] kt@katbook:~/hashi/tf/azure/azurerm▸f/resource_iotcentral$ testazure TestAccAzureRMIoTCentralApplication
==> Fixing source code with gofmt...
# This logic should match the search logic in scripts/gofmtcheck.sh
find . -name '*.go' | grep -v vendor | xargs gofmt -s -w
# github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/provider
azurerm/internal/provider/services.go:96:26: cannot use "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/iotcentral".Registration literal (type "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/iotcentral".Registration) as type "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/common".ServiceRegistration in array or slice literal:
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/iotcentral".Registration does not implement "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/common".ServiceRegistration (missing WebsiteCategories method)
FAIL github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/iotcentral/tests [build failed]
I don't have permission to your branch so would you mind merging master and fixing the test? thanks!
…tral # Conflicts: # azurerm/internal/clients/client.go
fix gofmtcheck.sh test issue |
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 the revisions @xuzhang3! LGTM now
This has been released in version 2.1.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 = "~> 2.1.0"
}
# ... other configuration ... |
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! |
No description provided.