-
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_iothub
#887
Changes from 67 commits
e5c5c62
ee1bf60
bc22b50
f9bf64d
531319c
12bd9e0
293ab2c
bbd1481
7fda639
e58ffde
b569ba9
95b677a
6d8a41d
5dd8cef
677794f
f2acf20
0a1b3fc
ffac304
3d76c7a
1ebedec
838271c
f378215
77c92ff
14aaec9
ff6e44d
c7d4aab
9b6a7dc
93c67b3
d6f58e7
96bf5f6
67469d3
1baeb52
a22eeb2
304b9d2
eeb4804
7be2c54
6b69422
faf8e4b
93c02b5
8e38252
60cd688
0c2e6aa
3468db1
92e0ede
79cfb28
421eda6
67c093b
5a82725
4324420
bcf55f5
b425073
abbedb0
53a039a
44b85f2
57a22e3
4258525
e96d5ca
daa1ba8
a907636
ffd3c56
106c0de
fabaef4
16cac29
3e927a6
2ea2cfc
b68afc6
4d9bfc8
4114e23
ae9b604
48cfcd7
fe759b8
422862d
d55a1b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package azurerm | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform/helper/acctest" | ||
"github.com/hashicorp/terraform/helper/resource" | ||
) | ||
|
||
func TestAccAzureRMIotHub_importBasic(t *testing.T) { | ||
resourceName := "azurerm_iothub.test" | ||
|
||
ri := acctest.RandInt() | ||
config := testAccAzureRMIotHub_basicStandard(ri, testLocation()) | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testCheckAzureRMIotHubDestroy, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: config, | ||
}, | ||
{ | ||
ResourceName: resourceName, | ||
ImportState: true, | ||
ImportStateVerify: true, | ||
}, | ||
}, | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,271 @@ | ||
package azurerm | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
|
||
"github.com/Azure/azure-sdk-for-go/services/iothub/mgmt/2017-07-01/devices" | ||
"github.com/hashicorp/terraform/helper/schema" | ||
"github.com/hashicorp/terraform/helper/validation" | ||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/response" | ||
) | ||
|
||
func resourceArmIotHub() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceArmIotHubCreateAndUpdate, | ||
Read: resourceArmIotHubRead, | ||
Update: resourceArmIotHubCreateAndUpdate, | ||
Delete: resourceArmIotHubDelete, | ||
Importer: &schema.ResourceImporter{ | ||
State: schema.ImportStatePassthrough, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given we're supporting Import here - could we add a test for it? Here's an example of how we're doing this for Subnets There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is added :) I'll add more import and resource tests given the time. |
||
|
||
Schema: map[string]*schema.Schema{ | ||
"name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
"tags": tagsSchema(), | ||
"location": locationSchema(), | ||
|
||
"resource_group_name": resourceGroupNameSchema(), | ||
"sku": { | ||
Type: schema.TypeList, | ||
MaxItems: 1, | ||
Required: true, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
DiffSuppressFunc: ignoreCaseDiffSuppressFunc, | ||
ValidateFunc: validation.StringInSlice([]string{ | ||
string(devices.F1), | ||
string(devices.S1), | ||
string(devices.S2), | ||
string(devices.S3), | ||
}, true), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since we're allowing this to be case insensitive, can we add:
|
||
}, | ||
|
||
"tier": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
DiffSuppressFunc: ignoreCaseDiffSuppressFunc, | ||
ValidateFunc: validation.StringInSlice([]string{ | ||
string(devices.Free), | ||
string(devices.Standard), | ||
}, true), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since we're allowing this to be case insensitive, can we add:
|
||
}, | ||
|
||
"capacity": { | ||
Type: schema.TypeInt, | ||
Required: true, | ||
ValidateFunc: validation.IntAtLeast(1), | ||
}, | ||
}, | ||
}, | ||
}, | ||
|
||
"type": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
|
||
"hostname": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
|
||
"shared_access_policy": { | ||
Type: schema.TypeList, | ||
Computed: true, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"key_name": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"primary_key": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"secondary_key": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"permissions": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
|
||
} | ||
|
||
func resourceArmIotHubCreateAndUpdate(d *schema.ResourceData, meta interface{}) error { | ||
|
||
armClient := meta.(*ArmClient) | ||
iothubClient := armClient.iothubResourceClient | ||
ctx := armClient.StopContext | ||
|
||
rg := d.Get("resource_group_name").(string) | ||
name := d.Get("name").(string) | ||
|
||
res, err := iothubClient.CheckNameAvailability(ctx, devices.OperationInputs{ | ||
Name: &name, | ||
}) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
if !*res.NameAvailable { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this fail when updating a resource? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It updates the resource correctly. I want to add an accTest for this as well. |
||
_, err := iothubClient.Get(ctx, rg, name) | ||
if err != nil { | ||
return errors.New(string(res.Reason)) | ||
} | ||
} | ||
|
||
location := d.Get("location").(string) | ||
|
||
subscriptionID := armClient.subscriptionId | ||
skuInfo := expandAzureRmIotHubSku(d) | ||
|
||
desc := devices.IotHubDescription{ | ||
Resourcegroup: &rg, | ||
Name: &name, | ||
Location: &location, | ||
Subscriptionid: &subscriptionID, | ||
Sku: &skuInfo, | ||
} | ||
|
||
if tagsI, ok := d.GetOk("tags"); ok { | ||
tags := tagsI.(map[string]interface{}) | ||
desc.Tags = *expandTags(tags) | ||
} | ||
|
||
future, err := iothubClient.CreateOrUpdate(ctx, rg, name, desc, "") | ||
if err != nil { | ||
return err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we wrap this error to be more descriptive with what went wrong:
|
||
} | ||
|
||
err = future.WaitForCompletion(ctx, iothubClient.Client) | ||
if err != nil { | ||
return fmt.Errorf("Error creating or updating IotHub %q (Resource Group %q): %+v", name, rg, err) | ||
} | ||
|
||
desc, err = iothubClient.Get(ctx, rg, name) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
d.SetId(*desc.ID) | ||
return resourceArmIotHubRead(d, meta) | ||
|
||
} | ||
|
||
func expandAzureRmIotHubSku(d *schema.ResourceData) devices.IotHubSkuInfo { | ||
skuList := d.Get("sku").([]interface{}) | ||
skuMap := skuList[0].(map[string]interface{}) | ||
cap := int64(skuMap["capacity"].(int)) | ||
|
||
name := skuMap["name"].(string) | ||
tier := skuMap["tier"].(string) | ||
|
||
return devices.IotHubSkuInfo{ | ||
Name: devices.IotHubSku(name), | ||
Tier: devices.IotHubSkuTier(tier), | ||
Capacity: &cap, | ||
} | ||
|
||
} | ||
|
||
func resourceArmIotHubRead(d *schema.ResourceData, meta interface{}) error { | ||
armClient := meta.(*ArmClient) | ||
iothubClient := armClient.iothubResourceClient | ||
ctx := armClient.StopContext | ||
|
||
id, err := parseAzureResourceID(d.Id()) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
iothubName := id.Path["IotHubs"] | ||
desc, err := iothubClient.Get(ctx, id.ResourceGroup, iothubName) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
keysResp, err := iothubClient.ListKeys(ctx, id.ResourceGroup, iothubName) | ||
keyList := keysResp.Response() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we check the error returned here? |
||
|
||
var keys []map[string]interface{} | ||
for _, key := range *keyList.Value { | ||
keyMap := make(map[string]interface{}) | ||
|
||
if keyName := key.KeyName; keyName != nil { | ||
keyMap["key_name"] = *keyName | ||
} | ||
|
||
if primaryKey := key.PrimaryKey; primaryKey != nil { | ||
keyMap["primary_key"] = *primaryKey | ||
} | ||
|
||
if secondaryKey := key.SecondaryKey; secondaryKey != nil { | ||
keyMap["secondary_key"] = *secondaryKey | ||
} | ||
|
||
keyMap["permissions"] = string(key.Rights) | ||
keys = append(keys, keyMap) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we pull this out into a separate function |
||
|
||
if err := d.Set("shared_access_policy", keys); err != nil { | ||
return fmt.Errorf("Error flattening `shared_access_policy` in IoTHub %q: %+v", iothubName, err) | ||
} | ||
|
||
if properties := desc.Properties; properties != nil { | ||
d.Set("hostname", properties.HostName) | ||
} | ||
|
||
d.Set("type", desc.Type) | ||
flattenAndSetTags(d, &desc.Tags) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we also set the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so the Import test (
on that note - could we pull this out into a new
|
||
|
||
return nil | ||
} | ||
|
||
func resourceArmIotHubDelete(d *schema.ResourceData, meta interface{}) error { | ||
|
||
id, err := parseAzureResourceID(d.Id()) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
iothubClient := meta.(*ArmClient).iothubResourceClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
iotHubName := id.Path["IotHubs"] | ||
|
||
future, err := iothubClient.Delete(ctx, id.ResourceGroup, iotHubName) | ||
if err != nil { | ||
if response.WasNotFound(future.Response()) { | ||
return nil | ||
} | ||
return err | ||
} | ||
|
||
err = future.WaitForCompletion(ctx, iothubClient.Client) | ||
if err != nil { | ||
if response.WasNotFound(future.Response()) { | ||
return nil | ||
} | ||
return fmt.Errorf("Error waiting for the deletion of IoTHub %q", iotHubName) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a timeout issue here. IoTHub is deleted in Azure, but terraform times out with an error destroying resource. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so this timeout is configured in the client initialisation above: https://github.com/terraform-providers/terraform-provider-azurerm/pull/887/files/3e927a6d96d0c91b3dc132d9b9e83f4b1fc96650#diff-848235249455ee48aaf3ea5d431960aeR597 we can actually swap that out for this function: https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/config.go#L435 Hope that helps :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very helpful, thank you! |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should be able to make this:
|
||
|
||
return 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.
rather than these 4 lines:
can we swap this over to using the newer style registration function? e.g.