Skip to content
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_iot_time_series_insights_access_policy #7202

Merged
merged 7 commits into from
Jun 9, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@ import (
)

type Client struct {
EnvironmentsClient *timeseriesinsights.EnvironmentsClient
AccessPoliciesClient *timeseriesinsights.AccessPoliciesClient
EnvironmentsClient *timeseriesinsights.EnvironmentsClient
}

func NewClient(o *common.ClientOptions) *Client {
EnvironmentsClient := timeseriesinsights.NewEnvironmentsClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId)
o.ConfigureClient(&EnvironmentsClient.Client, o.ResourceManagerAuthorizer)

AccessPoliciesClient := timeseriesinsights.NewAccessPoliciesClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId)
o.ConfigureClient(&AccessPoliciesClient.Client, o.ResourceManagerAuthorizer)

return &Client{
EnvironmentsClient: &EnvironmentsClient,
AccessPoliciesClient: &AccessPoliciesClient,
EnvironmentsClient: &EnvironmentsClient,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package parse

import (
"fmt"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
)

type TimeSeriesInsightsAccessPolicyId struct {
ResourceGroup string
Name string
EnvironmentName string
}

func TimeSeriesInsightsAccessPolicyID(input string) (*TimeSeriesInsightsAccessPolicyId, error) {
id, err := azure.ParseAzureResourceID(input)
if err != nil {
return nil, fmt.Errorf("parsing Time Series Insights Access Policy ID %q: %+v", input, err)
}

service := TimeSeriesInsightsAccessPolicyId{
ResourceGroup: id.ResourceGroup,
}

if service.EnvironmentName, err = id.PopSegment("environments"); err != nil {
return nil, err
}

if service.Name, err = id.PopSegment("accesspolicies"); err != nil {
return nil, err
}

if err := id.ValidateNoEmptySegments(input); err != nil {
return nil, err
}

return &service, nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package parse

import (
"testing"
)

func TestTimeSeriesInsightsAccessPolicyId(t *testing.T) {
testData := []struct {
Name string
Input string
Expected *TimeSeriesInsightsAccessPolicyId
}{
{
Name: "Empty",
Input: "",
Expected: nil,
},
{
Name: "No Resource Groups Segment",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000",
Expected: nil,
},
{
Name: "No Resource Groups Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/",
Expected: nil,
},
{
Name: "Resource Group ID",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/",
Expected: nil,
},
{
Name: "Time Series Insight AccessPolicy Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.TimeSeriesInsights/environments/Environment1/accesspolicies/",
Expected: nil,
},
{
Name: "Time Series Insight Access Policy ID",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.TimeSeriesInsights/environments/Environment1/accesspolicies/Policy1",
Expected: &TimeSeriesInsightsAccessPolicyId{
EnvironmentName: "Environment1",
ResourceGroup: "resGroup1",
Name: "Policy1",
},
},
{
Name: "Wrong Casing",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.TimeSeriesInsights/Environments/Environment1/AccessPolicies/Policy1",
Expected: nil,
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Name)

actual, err := TimeSeriesInsightsAccessPolicyID(v.Input)
if err != nil {
if v.Expected == nil {
continue
}

t.Fatalf("Expected a value but got an error: %s", err)
}

if actual.Name != v.Expected.Name {
t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name)
}

if actual.ResourceGroup != v.Expected.ResourceGroup {
t.Fatalf("Expected %q but got %q for Resource Group", v.Expected.ResourceGroup, actual.ResourceGroup)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func (r Registration) SupportedDataSources() map[string]*schema.Resource {
// SupportedResources returns the supported Resources supported by this Service
func (r Registration) SupportedResources() map[string]*schema.Resource {
return map[string]*schema.Resource{
"azurerm_iot_time_series_insights_access_policy": resourceArmIoTTimeSeriesInsightsAccessPolicy(),
"azurerm_iot_time_series_insights_standard_environment": resourceArmIoTTimeSeriesInsightsStandardEnvironment(),
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
package iottimeseriesinsights

import (
"fmt"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
"regexp"
"time"

"github.com/Azure/azure-sdk-for-go/services/preview/timeseriesinsights/mgmt/2018-08-15-preview/timeseriesinsights"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/iottimeseriesinsights/parse"
azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

func resourceArmIoTTimeSeriesInsightsAccessPolicy() *schema.Resource {
return &schema.Resource{
Create: resourceArmIoTTimeSeriesInsightsAccessPolicyCreateUpdate,
Read: resourceArmIoTTimeSeriesInsightsAccessPolicyRead,
Update: resourceArmIoTTimeSeriesInsightsAccessPolicyCreateUpdate,
Delete: resourceArmIoTTimeSeriesInsightsAccessPolicyDelete,
Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error {
_, err := parse.TimeSeriesInsightsAccessPolicyID(id)
return err
}),

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(30 * time.Minute),
Read: schema.DefaultTimeout(5 * time.Minute),
Update: schema.DefaultTimeout(30 * time.Minute),
Delete: schema.DefaultTimeout(30 * time.Minute),
},

Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringMatch(
regexp.MustCompile(`^[-\w\._\(\)]+$`),
"IoT Time Series Insights Access Policy name must contain only word characters, periods, underscores, and parentheses.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex also says hyphens are allowed, could we please also update the message accordingly? Or please confirm whether the hyphens are allowed?

),
},

"resource_group_name": azure.SchemaResourceGroupName(),

"environment_name": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this might be more clear is time_series_environment_name?

Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringMatch(
regexp.MustCompile(`^[-\w\._\(\)]+$`),
"IoT Time Series Insights Environment name must contain only word characters, periods, underscores, and parentheses.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

),
},

"principal_object_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringIsNotEmpty,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is an object id, could we add a validation of IsUUID?
And I suppose maybe principal_id is sufficient, since there are a lot of principal_ids in the identity blocks of other resources.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going based off the docs in Azure and the API accepts any string. I'd like to keep it matching with what the API accepts over forcing our own rules.

},

"description": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringIsNotEmpty,
},

"roles": {
Type: schema.TypeSet,
Required: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
string(timeseriesinsights.Contributor),
string(timeseriesinsights.Reader),
}, false),
},
},
},
}
}

func resourceArmIoTTimeSeriesInsightsAccessPolicyCreateUpdate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).IoTTimeSeriesInsights.AccessPoliciesClient
ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()

name := d.Get("name").(string)
environmentName := d.Get("environment_name").(string)
resourceGroup := d.Get("resource_group_name").(string)

if d.IsNewResource() {
existing, err := client.Get(ctx, resourceGroup, environmentName, name)
if err != nil {
if !utils.ResponseWasNotFound(existing.Response) {
return fmt.Errorf("checking for presence of existing IoT Time Series Insights Access Policy %q (Resource Group %q): %s", name, resourceGroup, err)
}
}

if existing.ID != nil && *existing.ID != "" {
return tf.ImportAsExistsError("azurerm_iot_time_series_insights_access_policy", *existing.ID)
}
}

policy := timeseriesinsights.AccessPolicyCreateOrUpdateParameters{
&timeseriesinsights.AccessPolicyResourceProperties{
Description: utils.String(d.Get("description").(string)),
PrincipalObjectID: utils.String(d.Get("principal_object_id").(string)),
Roles: expandIoTTimeSeriesInsightsAccessPolicyRoles(d.Get("roles").(*schema.Set).List()),
},
}

_, err := client.CreateOrUpdate(ctx, resourceGroup, environmentName, name, policy)
if err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these lines cna be combines

Suggested change
_, err := client.CreateOrUpdate(ctx, resourceGroup, environmentName, name, policy)
if err != nil {
if _, err := client.CreateOrUpdate(ctx, resourceGroup, environmentName, name, policy); err != nil {

return fmt.Errorf("creating/updating IoT Time Series Insights Access Policy %q (Resource Group %q): %+v", name, resourceGroup, err)
}

resp, err := client.Get(ctx, resourceGroup, environmentName, name)
if err != nil {
return fmt.Errorf("retrieving IoT Time Series Insights Access Policy %q (Resource Group %q): %+v", name, resourceGroup, err)
}

if resp.ID == nil || *resp.ID == "" {
return fmt.Errorf("cannot read IoT Time Series Insights Access Policy %q (Resource Group %q) ID", name, resourceGroup)
}

d.SetId(*resp.ID)

return resourceArmIoTTimeSeriesInsightsAccessPolicyRead(d, meta)
}

func resourceArmIoTTimeSeriesInsightsAccessPolicyRead(d *schema.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).IoTTimeSeriesInsights.AccessPoliciesClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := parse.TimeSeriesInsightsAccessPolicyID(d.Id())
if err != nil {
return err
}

resp, err := client.Get(ctx, id.ResourceGroup, id.EnvironmentName, id.Name)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
d.SetId("")
return nil
}

return fmt.Errorf("retrieving IoT Time Series Insights Access Policy %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err)
}

d.Set("name", resp.Name)
d.Set("resource_group_name", id.ResourceGroup)
d.Set("environment_name", id.EnvironmentName)

if props := resp.AccessPolicyResourceProperties; props != nil {
d.Set("description", props.Description)
d.Set("principal_object_id", props.PrincipalObjectID)
d.Set("roles", flattenIoTTimeSeriesInsightsAccessPolicyRoles(resp.Roles))
}

return nil
}

func resourceArmIoTTimeSeriesInsightsAccessPolicyDelete(d *schema.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).IoTTimeSeriesInsights.AccessPoliciesClient
ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := parse.TimeSeriesInsightsAccessPolicyID(d.Id())
if err != nil {
return err
}

response, err := client.Delete(ctx, id.ResourceGroup, id.EnvironmentName, id.Name)
if err != nil {
if !utils.ResponseWasNotFound(response) {
return fmt.Errorf("deleting IoT Time Series Insights Access Policy %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err)
}
Comment on lines +180 to +182
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not escape 404 errors here. Whether the SDK gives non-nil errors is defined here in the response section of the swagger. For the listed status codes, SDK will not return non-nil errors for them (in this case, they are 200 and 204). And all other status codes, the SDK will return non-nil errors using the error schema defined here.

This means in this case, 404 will always be an error and indicates something is wrong. If this resource does not exist, and we still want to delete this, the service should return 204 for this.

One of the scenario 404 was returned in deletion should be we are deleting this resource, but the resource group does not exist. I believe in this case the service would return 404. But in this case I believe we should call out and throw errors.
What do you think?

}

return nil
}

func expandIoTTimeSeriesInsightsAccessPolicyRoles(input []interface{}) *[]timeseriesinsights.AccessPolicyRole {
roles := make([]timeseriesinsights.AccessPolicyRole, 0)

for _, v := range input {
if v == nil {
continue
}
roles = append(roles, timeseriesinsights.AccessPolicyRole(v.(string)))
}

return &roles
}

func flattenIoTTimeSeriesInsightsAccessPolicyRoles(input *[]timeseriesinsights.AccessPolicyRole) []interface{} {
result := make([]interface{}, 0)
if input != nil {
for _, item := range *input {
result = append(result, string(item))
}
}
return result
}
Loading