From 8e77840931e407e0f329da03190f9129142dc890 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 25 Jul 2024 13:16:03 +0100 Subject: [PATCH 1/6] refactor: Safely handle nil values in assignment filter resource construction --- go.mod | 2 +- .../beta/assignmentFilter/construct.go | 10 +++++----- .../beta/assignmentFilter/state.go | 18 +++++++----------- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index 396dbc7c..479d2cd5 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/hashicorp/terraform-plugin-docs v0.19.4 github.com/hashicorp/terraform-plugin-framework v1.10.0 github.com/hashicorp/terraform-plugin-framework-timeouts v0.4.1 + github.com/hashicorp/terraform-plugin-framework-validators v0.13.0 github.com/hashicorp/terraform-plugin-log v0.9.0 github.com/microsoft/kiota-http-go v1.4.2 github.com/microsoftgraph/msgraph-beta-sdk-go v0.106.0 @@ -50,7 +51,6 @@ require ( github.com/hashicorp/hc-install v0.7.0 // indirect github.com/hashicorp/terraform-exec v0.21.0 // indirect github.com/hashicorp/terraform-json v0.22.1 // indirect - github.com/hashicorp/terraform-plugin-framework-validators v0.13.0 // indirect github.com/hashicorp/terraform-plugin-go v0.23.0 // indirect github.com/hashicorp/terraform-registry-address v0.2.3 // indirect github.com/hashicorp/terraform-svchost v0.1.1 // indirect diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go index 0c6e5309..28a00943 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go @@ -49,13 +49,13 @@ func constructResource(ctx context.Context, data *AssignmentFilterResourceModel) } } - if !data.RoleScopeTags.IsNull() && len(data.RoleScopeTags.Elements()) > 0 { - roleScopeTags := make([]string, len(data.RoleScopeTags.Elements())) - for i, tag := range data.RoleScopeTags.Elements() { - roleScopeTags[i] = tag.(types.String).ValueString() + roleScopeTags := make([]string, 0) + if !data.RoleScopeTags.IsNull() { + for _, tag := range data.RoleScopeTags.Elements() { + roleScopeTags = append(roleScopeTags, tag.(types.String).ValueString()) } - requestBody.SetRoleScopeTags(roleScopeTags) } + requestBody.SetRoleScopeTags(roleScopeTags) payloads, err := convertPayloads(data.Payloads) if err != nil { diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go index f22ff012..42c9d38c 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go @@ -2,6 +2,7 @@ package graphBetaAssignmentFilter import ( "context" + "fmt" "time" "github.com/deploymenttheory/terraform-provider-microsoft365/internal/helpers" @@ -19,40 +20,32 @@ func mapRemoteStateToTerraform(ctx context.Context, data *AssignmentFilterResour return } - tflog.Debug(ctx, "Mapping ID") data.ID = types.StringValue(helpers.StringPtrToString(remoteResource.GetId())) - tflog.Debug(ctx, "Mapping DisplayName") data.DisplayName = types.StringValue(helpers.StringPtrToString(remoteResource.GetDisplayName())) - tflog.Debug(ctx, "Mapping Description") data.Description = types.StringValue(helpers.StringPtrToString(remoteResource.GetDescription())) - tflog.Debug(ctx, "Mapping Platform") if platform := remoteResource.GetPlatform(); platform != nil { data.Platform = types.StringValue(platform.String()) } else { data.Platform = types.StringNull() } - tflog.Debug(ctx, "Mapping Rule") data.Rule = types.StringValue(helpers.StringPtrToString(remoteResource.GetRule())) - tflog.Debug(ctx, "Mapping AssignmentFilterManagementType") if managementType := remoteResource.GetAssignmentFilterManagementType(); managementType != nil { data.AssignmentFilterManagementType = types.StringValue(managementType.String()) } else { data.AssignmentFilterManagementType = types.StringNull() } - tflog.Debug(ctx, "Mapping CreatedDateTime") if createdDateTime := remoteResource.GetCreatedDateTime(); createdDateTime != nil { data.CreatedDateTime = types.StringValue(createdDateTime.Format(time.RFC3339)) } else { data.CreatedDateTime = types.StringNull() } - tflog.Debug(ctx, "Mapping LastModifiedDateTime") if lastModifiedDateTime := remoteResource.GetLastModifiedDateTime(); lastModifiedDateTime != nil { data.LastModifiedDateTime = types.StringValue(lastModifiedDateTime.Format(time.RFC3339)) } else { @@ -61,12 +54,15 @@ func mapRemoteStateToTerraform(ctx context.Context, data *AssignmentFilterResour tflog.Debug(ctx, "Mapping RoleScopeTags") roleScopeTags := remoteResource.GetRoleScopeTags() - if roleScopeTags != nil && len(roleScopeTags) > 0 { - data.RoleScopeTags = types.ListValueMust(types.StringType, roleScopeTagsToValueSlice(roleScopeTags)) - } else { + tflog.Debug(ctx, fmt.Sprintf("Received RoleScopeTags from API: %v", roleScopeTags)) + + if len(roleScopeTags) == 0 { data.RoleScopeTags = types.ListValueMust(types.StringType, []attr.Value{}) + } else { + data.RoleScopeTags = types.ListValueMust(types.StringType, roleScopeTagsToValueSlice(roleScopeTags)) } + tflog.Debug(ctx, fmt.Sprintf("Mapped RoleScopeTags: %v", data.RoleScopeTags)) tflog.Debug(ctx, "Mapping Payloads") if payloads := remoteResource.GetPayloads(); len(payloads) > 0 { data.Payloads = types.ListValueMust(types.ObjectType{AttrTypes: payloadAttributeTypes()}, payloadsToValueSlice(payloads)) From fed5a1efe7a333c3ccbfd577209f3e20c07256c0 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 25 Jul 2024 13:22:51 +0100 Subject: [PATCH 2/6] refactor: Safely handle nil values in assignment filter resource construction --- .../beta/assignmentFilter/construct.go | 5 ++++- .../beta/assignmentFilter/state.go | 12 ++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go index 28a00943..3cf37c93 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go @@ -52,7 +52,10 @@ func constructResource(ctx context.Context, data *AssignmentFilterResourceModel) roleScopeTags := make([]string, 0) if !data.RoleScopeTags.IsNull() { for _, tag := range data.RoleScopeTags.Elements() { - roleScopeTags = append(roleScopeTags, tag.(types.String).ValueString()) + tagValue := tag.(types.String).ValueString() + if tagValue != "0" { + roleScopeTags = append(roleScopeTags, tagValue) + } } } requestBody.SetRoleScopeTags(roleScopeTags) diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go index 42c9d38c..bc3949f3 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go @@ -56,13 +56,21 @@ func mapRemoteStateToTerraform(ctx context.Context, data *AssignmentFilterResour roleScopeTags := remoteResource.GetRoleScopeTags() tflog.Debug(ctx, fmt.Sprintf("Received RoleScopeTags from API: %v", roleScopeTags)) - if len(roleScopeTags) == 0 { + filteredRoleScopeTags := make([]string, 0) + for _, tag := range roleScopeTags { + if tag != "0" { // Ignore the "0" value + filteredRoleScopeTags = append(filteredRoleScopeTags, tag) + } + } + + if len(filteredRoleScopeTags) == 0 { data.RoleScopeTags = types.ListValueMust(types.StringType, []attr.Value{}) } else { - data.RoleScopeTags = types.ListValueMust(types.StringType, roleScopeTagsToValueSlice(roleScopeTags)) + data.RoleScopeTags = types.ListValueMust(types.StringType, roleScopeTagsToValueSlice(filteredRoleScopeTags)) } tflog.Debug(ctx, fmt.Sprintf("Mapped RoleScopeTags: %v", data.RoleScopeTags)) + tflog.Debug(ctx, "Mapping Payloads") if payloads := remoteResource.GetPayloads(); len(payloads) > 0 { data.Payloads = types.ListValueMust(types.ObjectType{AttrTypes: payloadAttributeTypes()}, payloadsToValueSlice(payloads)) From 6d0134e3d8b9bf0173fa62001501f2939cfaf59c Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 25 Jul 2024 13:42:00 +0100 Subject: [PATCH 3/6] refactor: Safely handle nil values in assignment filter resource construction --- internal/resources/common/error.go | 12 +++-- .../beta/assignmentFilter/crud.go | 47 +++++++------------ 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/internal/resources/common/error.go b/internal/resources/common/error.go index 966ee41b..84f2f242 100644 --- a/internal/resources/common/error.go +++ b/internal/resources/common/error.go @@ -41,22 +41,24 @@ func IsNotFoundError(err error) bool { odataErr, ok := err.(*odataerrors.ODataError) if !ok { - return false + // If it's not an ODataError, check the error string + return strings.Contains(strings.ToLower(err.Error()), "not found") } mainError := odataErr.GetErrorEscaped() if mainError != nil { if code := mainError.GetCode(); code != nil { switch strings.ToLower(*code) { - case "request_resourcenotfound", "resourcenotfound": + case "request_resourcenotfound", "resourcenotfound", "notfound": return true } } if message := mainError.GetMessage(); message != nil { - if strings.Contains(strings.ToLower(*message), "not found") { - return true - } + lowerMessage := strings.ToLower(*message) + return strings.Contains(lowerMessage, "not found") || + strings.Contains(lowerMessage, "could not be found") || + strings.Contains(lowerMessage, "does not exist") } } diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go index ca20757e..58f33ac5 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go @@ -3,6 +3,7 @@ package graphBetaAssignmentFilter import ( "context" "fmt" + "strings" "time" "github.com/deploymenttheory/terraform-provider-microsoft365/internal/resources/common" @@ -54,19 +55,17 @@ func (r *AssignmentFilterResource) Create(ctx context.Context, req resource.Crea resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) - tflog.Debug(ctx, fmt.Sprintf("Finished creation of resource: %s_%s", r.ProviderTypeName, r.TypeName)) + tflog.Debug(ctx, fmt.Sprintf("Finished Create Method: %s_%s", r.ProviderTypeName, r.TypeName)) } +// Read handles the Read operation. func (r *AssignmentFilterResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { var state AssignmentFilterResourceModel - tflog.Debug(ctx, "Starting Read method for assignment filter") - resp.Diagnostics.Append(req.State.Get(ctx, &state)...) if resp.Diagnostics.HasError() { return } - if state.ID.IsNull() || state.ID.ValueString() == "" { resp.Diagnostics.AddWarning( "Unable to read assignment filter", @@ -74,9 +73,7 @@ func (r *AssignmentFilterResource) Read(ctx context.Context, req resource.ReadRe ) return } - tflog.Debug(ctx, fmt.Sprintf("Reading assignment filter with ID: %s", state.ID.ValueString())) - readTimeout, diags := state.Timeouts.Read(ctx, 30*time.Second) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { @@ -87,40 +84,35 @@ func (r *AssignmentFilterResource) Read(ctx context.Context, req resource.ReadRe assignmentFilter, err := r.client.DeviceManagement().AssignmentFilters().ByDeviceAndAppManagementAssignmentFilterId(state.ID.ValueString()).Get(ctx, nil) if err != nil { - if common.IsNotFoundError(err) { - resp.Diagnostics.AddWarning( - "Assignment filter not found", - fmt.Sprintf("Assignment filter with ID %s was not found. Removing from state.", state.ID.ValueString()), - ) + if common.IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { + tflog.Warn(ctx, fmt.Sprintf("%s_%s with ID %s not found, removing from state", r.ProviderTypeName, r.TypeName, state.ID.ValueString())) resp.State.RemoveResource(ctx) return } resp.Diagnostics.AddError( "Error reading assignment filter", - fmt.Sprintf("Could not read assignment filter with ID %s: %s", state.ID.ValueString(), err.Error()), + fmt.Sprintf("Could not read %s_%s with ID %s: %s", r.ProviderTypeName, r.TypeName, state.ID.ValueString(), err.Error()), ) return } mapRemoteStateToTerraform(ctx, &state, assignmentFilter) - resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) - - tflog.Debug(ctx, "Finished Read method for assignment filter") + tflog.Debug(ctx, fmt.Sprintf("Finished Read Method: %s_%s", r.ProviderTypeName, r.TypeName)) } // Update handles the Update operation. func (r *AssignmentFilterResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - var data AssignmentFilterResourceModel + var plan AssignmentFilterResourceModel tflog.Debug(ctx, fmt.Sprintf("Starting Update of resource: %s_%s", r.ProviderTypeName, r.TypeName)) - resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) if resp.Diagnostics.HasError() { return } - updateTimeout, diags := data.Timeouts.Update(ctx, 30*time.Second) + updateTimeout, diags := plan.Timeouts.Update(ctx, 30*time.Second) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return @@ -128,7 +120,7 @@ func (r *AssignmentFilterResource) Update(ctx context.Context, req resource.Upda ctx, cancel := context.WithTimeout(ctx, updateTimeout) defer cancel() - requestBody, err := constructResource(ctx, &data) + requestBody, err := constructResource(ctx, &plan) if err != nil { resp.Diagnostics.AddError( @@ -138,26 +130,23 @@ func (r *AssignmentFilterResource) Update(ctx context.Context, req resource.Upda return } - _, err = r.client.DeviceManagement().AssignmentFilters().ByDeviceAndAppManagementAssignmentFilterId(data.ID.ValueString()).Patch(ctx, requestBody, nil) + _, err = r.client.DeviceManagement().AssignmentFilters().ByDeviceAndAppManagementAssignmentFilterId(plan.ID.ValueString()).Patch(ctx, requestBody, nil) if err != nil { - if common.IsNotFoundError(err) && !r.isCreate { - resp.Diagnostics.AddWarning( - "Resource Not Found", - fmt.Sprintf("The resource: %s_%s with ID %s was not found and will be removed from the state.", r.ProviderTypeName, r.TypeName, data.ID.ValueString()), - ) + if common.IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { + tflog.Warn(ctx, fmt.Sprintf("%s_%s with ID %s not found, removing from state", r.ProviderTypeName, r.TypeName, plan.ID.ValueString())) resp.State.RemoveResource(ctx) return } resp.Diagnostics.AddError( "Error reading assignment filter", - fmt.Sprintf("Could not update resource: %s_%s: %s", r.ProviderTypeName, r.TypeName, err.Error()), + fmt.Sprintf("Could not read %s_%s with ID %s: %s", r.ProviderTypeName, r.TypeName, plan.ID.ValueString(), err.Error()), ) return } - resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) - tflog.Debug(ctx, fmt.Sprintf("Finished Update of resource: %s_%s", r.ProviderTypeName, r.TypeName)) + tflog.Debug(ctx, fmt.Sprintf("Finished Update Method: %s_%s", r.ProviderTypeName, r.TypeName)) } // Delete handles the Delete operation. @@ -185,7 +174,7 @@ func (r *AssignmentFilterResource) Delete(ctx context.Context, req resource.Dele return } - tflog.Debug(ctx, fmt.Sprintf("Completed deletion of resource: %s_%s", r.ProviderTypeName, r.TypeName)) + tflog.Debug(ctx, fmt.Sprintf("Finished Delete Method: %s_%s", r.ProviderTypeName, r.TypeName)) resp.State.RemoveResource(ctx) } From 8b650afe16e2b12681bf1d8222d828d7e04788e8 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 25 Jul 2024 13:54:11 +0100 Subject: [PATCH 4/6] refactor:handling of resources removed from the server --- .../deviceandappmanagement/beta/assignmentFilter/crud.go | 8 ++++---- .../beta/assignmentFilter/resource.go | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go index 58f33ac5..94a43c03 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go @@ -85,13 +85,13 @@ func (r *AssignmentFilterResource) Read(ctx context.Context, req resource.ReadRe assignmentFilter, err := r.client.DeviceManagement().AssignmentFilters().ByDeviceAndAppManagementAssignmentFilterId(state.ID.ValueString()).Get(ctx, nil) if err != nil { if common.IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { - tflog.Warn(ctx, fmt.Sprintf("%s_%s with ID %s not found, removing from state", r.ProviderTypeName, r.TypeName, state.ID.ValueString())) + tflog.Warn(ctx, fmt.Sprintf("%s with ID %s not found, removing from state", r.TypeName, state.ID.ValueString())) resp.State.RemoveResource(ctx) return } resp.Diagnostics.AddError( "Error reading assignment filter", - fmt.Sprintf("Could not read %s_%s with ID %s: %s", r.ProviderTypeName, r.TypeName, state.ID.ValueString(), err.Error()), + fmt.Sprintf("Could not update %s with ID %s: %s", r.TypeName, state.ID.ValueString(), err.Error()), ) return } @@ -133,13 +133,13 @@ func (r *AssignmentFilterResource) Update(ctx context.Context, req resource.Upda _, err = r.client.DeviceManagement().AssignmentFilters().ByDeviceAndAppManagementAssignmentFilterId(plan.ID.ValueString()).Patch(ctx, requestBody, nil) if err != nil { if common.IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { - tflog.Warn(ctx, fmt.Sprintf("%s_%s with ID %s not found, removing from state", r.ProviderTypeName, r.TypeName, plan.ID.ValueString())) + tflog.Warn(ctx, fmt.Sprintf("%s with ID %s not found, removing from state", r.TypeName, plan.ID.ValueString())) resp.State.RemoveResource(ctx) return } resp.Diagnostics.AddError( "Error reading assignment filter", - fmt.Sprintf("Could not read %s_%s with ID %s: %s", r.ProviderTypeName, r.TypeName, plan.ID.ValueString(), err.Error()), + fmt.Sprintf("Could not update %s with ID %s: %s", r.TypeName, plan.ID.ValueString(), err.Error()), ) return } diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go index ce4bf0c9..5fe081d9 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go @@ -31,7 +31,6 @@ type AssignmentFilterResource struct { client *msgraphbetasdk.GraphServiceClient ProviderTypeName string TypeName string - isCreate bool } type AssignmentFilterResourceModel struct { From b390995dba866d7b579c638aeca39881b90341c8 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 25 Jul 2024 14:15:16 +0100 Subject: [PATCH 5/6] refactor: Improve assignment filter resource construction Refactor the construction of the assignment filter resource in `state.go` and `object.go` for improved readability and visibility. The changes include: - Safely dereferencing string pointers to avoid nil pointer errors - Safely converting enums to their string representation - Handling nil values for various fields in the assignment filter resource These refactorings enhance the code readability and provide better visibility into the constructed assignment filter resource. --- .../beta/assignmentFilter/construct.go | 39 ------------- .../beta/assignmentFilter/resource.go | 28 ---------- .../beta/assignmentFilter/state.go | 55 ------------------- .../beta/assignmentFilter/validators.go | 18 ------ 4 files changed, 140 deletions(-) diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go index 3cf37c93..b6845955 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/construct.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-log/tflog" - "github.com/deploymenttheory/terraform-provider-microsoft365/internal/resources/common" "github.com/microsoftgraph/msgraph-beta-sdk-go/models" ) @@ -60,14 +59,6 @@ func constructResource(ctx context.Context, data *AssignmentFilterResourceModel) } requestBody.SetRoleScopeTags(roleScopeTags) - payloads, err := convertPayloads(data.Payloads) - if err != nil { - return nil, err - } - if payloads != nil { - requestBody.SetPayloads(payloads) - } - requestBodyJSON, err := json.MarshalIndent(map[string]interface{}{ "displayName": requestBody.GetDisplayName(), "description": requestBody.GetDescription(), @@ -75,7 +66,6 @@ func constructResource(ctx context.Context, data *AssignmentFilterResourceModel) "rule": requestBody.GetRule(), "managementType": requestBody.GetAssignmentFilterManagementType(), "roleScopeTags": requestBody.GetRoleScopeTags(), - "payloads": payloads, }, "", " ") if err != nil { return nil, fmt.Errorf("error marshalling request body to JSON: %s", err) @@ -85,32 +75,3 @@ func constructResource(ctx context.Context, data *AssignmentFilterResourceModel) return requestBody, nil } - -// convertPayloads -func convertPayloads(payloads types.List) ([]models.PayloadByFilterable, error) { - if payloads.IsNull() || len(payloads.Elements()) == 0 { - return nil, nil - } - - result := make([]models.PayloadByFilterable, len(payloads.Elements())) - for i, elem := range payloads.Elements() { - payloadElem := elem.(types.Object) - payload := models.NewPayloadByFilter() - - common.SetStringValueFromAttributes(payloadElem.Attributes(), "payload_id", payload.SetPayloadId) - if err := common.SetParsedValueFromAttributes(payloadElem.Attributes(), "payload_type", func(val *models.AssociatedAssignmentPayloadType) { - payload.SetPayloadType(val) - }, models.ParseAssociatedAssignmentPayloadType); err != nil { - return nil, fmt.Errorf("invalid payload type: %s", err) - } - common.SetStringValueFromAttributes(payloadElem.Attributes(), "group_id", payload.SetGroupId) - if err := common.SetParsedValueFromAttributes(payloadElem.Attributes(), "assignment_filter_type", func(val *models.DeviceAndAppManagementAssignmentFilterType) { - payload.SetAssignmentFilterType(val) - }, models.ParseAssociatedAssignmentPayloadType); err != nil { - return nil, fmt.Errorf("invalid assignment filter type: %s", err) - } - - result[i] = payload - } - return result, nil -} diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go index 5fe081d9..af31759f 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go @@ -139,34 +139,6 @@ func (r *AssignmentFilterResource) Schema(ctx context.Context, req resource.Sche Description: "Indicates role scope tags assigned for the assignment filter.", ElementType: types.StringType, }, - "payloads": schema.ListNestedAttribute{ - Optional: true, - Description: "Indicates associated assignments for a specific filter.", - NestedObject: schema.NestedAttributeObject{ - Attributes: map[string]schema.Attribute{ - "payload_id": schema.StringAttribute{ - Required: true, - Description: "The ID of the payload.", - }, - "payload_type": schema.StringAttribute{ - Required: true, - Description: "The type of the payload.", - }, - "group_id": schema.StringAttribute{ - Required: true, - Description: "The group ID associated with the payload.", - }, - "assignment_filter_type": schema.StringAttribute{ - Required: true, - Description: fmt.Sprintf("The assignment filter type. Supported types: %s", - strings.Join(getValidAssignmentFilterTypes(), ", ")), - Validators: []validator.String{ - stringvalidator.OneOf(getValidAssignmentFilterTypes()...), - }, - }, - }, - }, - }, "timeouts": timeouts.Attributes(ctx, timeouts.Opts{ Create: true, Read: true, diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go index bc3949f3..18e72ff5 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/state.go @@ -2,7 +2,6 @@ package graphBetaAssignmentFilter import ( "context" - "fmt" "time" "github.com/deploymenttheory/terraform-provider-microsoft365/internal/helpers" @@ -52,9 +51,7 @@ func mapRemoteStateToTerraform(ctx context.Context, data *AssignmentFilterResour data.LastModifiedDateTime = types.StringNull() } - tflog.Debug(ctx, "Mapping RoleScopeTags") roleScopeTags := remoteResource.GetRoleScopeTags() - tflog.Debug(ctx, fmt.Sprintf("Received RoleScopeTags from API: %v", roleScopeTags)) filteredRoleScopeTags := make([]string, 0) for _, tag := range roleScopeTags { @@ -69,15 +66,6 @@ func mapRemoteStateToTerraform(ctx context.Context, data *AssignmentFilterResour data.RoleScopeTags = types.ListValueMust(types.StringType, roleScopeTagsToValueSlice(filteredRoleScopeTags)) } - tflog.Debug(ctx, fmt.Sprintf("Mapped RoleScopeTags: %v", data.RoleScopeTags)) - - tflog.Debug(ctx, "Mapping Payloads") - if payloads := remoteResource.GetPayloads(); len(payloads) > 0 { - data.Payloads = types.ListValueMust(types.ObjectType{AttrTypes: payloadAttributeTypes()}, payloadsToValueSlice(payloads)) - } else { - data.Payloads = types.ListNull(types.ObjectType{AttrTypes: payloadAttributeTypes()}) - } - tflog.Debug(ctx, "Finished mapping remote state to Terraform") } @@ -90,46 +78,3 @@ func roleScopeTagsToValueSlice(roleScopeTags []string) []attr.Value { } return values } - -// payloadAttributeTypes returns a map of attribute names to their Terraform types for the Payload object. -// This defines the structure of the Payload object in the Terraform resource model. -func payloadAttributeTypes() map[string]attr.Type { - return map[string]attr.Type{ - "payload_id": types.StringType, - "payload_type": types.StringType, - "group_id": types.StringType, - "assignment_filter_type": types.StringType, - } -} - -// payloadsToValueSlice converts a slice of PayloadByFilterable to a slice of Terraform attr.Value. -// This is used to populate the Payloads field in the Terraform resource model. -func payloadsToValueSlice(payloads []models.PayloadByFilterable) []attr.Value { - values := make([]attr.Value, len(payloads)) - for i, payload := range payloads { - payloadMap := map[string]attr.Value{ - "payload_id": types.StringValue(helpers.StringPtrToString(payload.GetPayloadId())), - "payload_type": types.StringValue(payloadTypeToString(payload.GetPayloadType())), - "group_id": types.StringValue(helpers.StringPtrToString(payload.GetGroupId())), - "assignment_filter_type": types.StringValue(assignmentFilterTypeToString(payload.GetAssignmentFilterType())), - } - values[i] = types.ObjectValueMust(payloadAttributeTypes(), payloadMap) - } - return values -} - -// payloadTypeToString converts AssociatedAssignmentPayloadType to its string representation. -func payloadTypeToString(payloadType *models.AssociatedAssignmentPayloadType) string { - if payloadType == nil { - return "" - } - return (*payloadType).String() -} - -// assignmentFilterTypeToString converts DeviceAndAppManagementAssignmentFilterType to its string representation. -func assignmentFilterTypeToString(filterType *models.DeviceAndAppManagementAssignmentFilterType) string { - if filterType == nil { - return "" - } - return (*filterType).String() -} diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/validators.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/validators.go index 021eb2a5..b76b8e48 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/validators.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/validators.go @@ -23,21 +23,3 @@ var validAssignmentFilterManagementTypes = []string{ "apps", "unknownFutureValue", } - -func getValidAssignmentFilterTypes() []string { - // This reflects the order in the SDK's String() method - return []string{ - "unknown", - "deviceConfigurationAndCompliance", - "application", - "androidEnterpriseApp", - "enrollmentConfiguration", - "groupPolicyConfiguration", - "zeroTouchDeploymentDeviceConfigProfile", - "androidEnterpriseConfiguration", - "deviceFirmwareConfigurationInterfacePolicy", - "resourceAccessPolicy", - "win32app", - "deviceManagmentConfigurationAndCompliancePolicy", - } -} From d62d95744a36437de917fa0967c3978cb1636d29 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 25 Jul 2024 14:52:40 +0100 Subject: [PATCH 6/6] refactor: Improve assignment filter resource construction Refactor the construction of the assignment filter resource in `state.go` and `object.go` for improved readability and visibility. Safely dereference string pointers, convert enums to string representation, and handle nil values. Enhance code readability and provide better visibility into the constructed assignment filter resource. --- .../resource.tf | 36 +++++---------- internal/resources/common/state.go | 45 +++++++++++++++++++ .../beta/assignmentFilter/crud.go | 32 +++---------- .../beta/assignmentFilter/resource.go | 11 ++++- 4 files changed, 71 insertions(+), 53 deletions(-) diff --git a/examples/microsoft365_graph_beta_device_and_app_management_assignment_filter/resource.tf b/examples/microsoft365_graph_beta_device_and_app_management_assignment_filter/resource.tf index 704679aa..f320b41f 100644 --- a/examples/microsoft365_graph_beta_device_and_app_management_assignment_filter/resource.tf +++ b/examples/microsoft365_graph_beta_device_and_app_management_assignment_filter/resource.tf @@ -1,30 +1,16 @@ resource "microsoft365_graph_beta_device_and_app_management_assignment_filter" "example" { - display_name = "Example Assignment Filter" - description = "This is an example assignment filter" - platform = "android" - rule = "device.os == 'Android'" + display_name = "new filter" + description = "This is an example assignment filter" + platform = "iOS" + rule = "(device.manufacturer -eq \"thing\")" assignment_filter_management_type = "devices" - role_scope_tags = ["tag1", "tag2"] + role_scope_tags = [8,9] - payloads { - payload_id = "payload1" - payload_type = "type1" - group_id = "group1" - assignment_filter_type = "include" + timeouts = { + create = "10s" + read = "10s" + update = "10s" + delete = "10s" } - - payloads { - payload_id = "payload2" - payload_type = "type2" - group_id = "group2" - assignment_filter_type = "exclude" - } - - timeouts { - create = "30m" - read = "30m" - update = "30m" - delete = "30m" - } -} +} \ No newline at end of file diff --git a/internal/resources/common/state.go b/internal/resources/common/state.go index 805d0c79..d7ce7ca8 100644 --- a/internal/resources/common/state.go +++ b/internal/resources/common/state.go @@ -1 +1,46 @@ package common + +import ( + "context" + "fmt" + "strings" + + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-log/tflog" +) + +// ResourceWithID is an interface that represents a resource with an ID. +type ResourceWithID interface { + GetTypeName() string +} + +// StateWithID is an interface that represents a state model with an ID. +type StateWithID interface { + GetID() string +} + +// HandleReadStateError handles errors during the read operation. +func HandleReadStateError(ctx context.Context, resp *resource.ReadResponse, resource ResourceWithID, state StateWithID, err error) { + if IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { + tflog.Warn(ctx, fmt.Sprintf("%s with ID %s not found, removing from state", resource.GetTypeName(), state.GetID())) + resp.State.RemoveResource(ctx) + } else { + resp.Diagnostics.AddError( + "Error reading resource", + fmt.Sprintf("Could not update %s with ID %s: %s", resource.GetTypeName(), state.GetID(), err.Error()), + ) + } +} + +// HandleUpdateStateError handles errors during the update operation. +func HandleUpdateStateError(ctx context.Context, resp *resource.UpdateResponse, resource ResourceWithID, state StateWithID, err error) { + if IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { + tflog.Warn(ctx, fmt.Sprintf("%s with ID %s not found, removing from state", resource.GetTypeName(), state.GetID())) + resp.State.RemoveResource(ctx) + } else { + resp.Diagnostics.AddError( + "Error updating resource", + fmt.Sprintf("Could not update %s with ID %s: %s", resource.GetTypeName(), state.GetID(), err.Error()), + ) + } +} diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go index 94a43c03..4acca1a2 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/crud.go @@ -3,7 +3,6 @@ package graphBetaAssignmentFilter import ( "context" "fmt" - "strings" "time" "github.com/deploymenttheory/terraform-provider-microsoft365/internal/resources/common" @@ -62,17 +61,13 @@ func (r *AssignmentFilterResource) Create(ctx context.Context, req resource.Crea func (r *AssignmentFilterResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { var state AssignmentFilterResourceModel tflog.Debug(ctx, "Starting Read method for assignment filter") + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { return } - if state.ID.IsNull() || state.ID.ValueString() == "" { - resp.Diagnostics.AddWarning( - "Unable to read assignment filter", - "Assignment filter ID is empty or null. Unable to read assignment filter.", - ) - return - } + tflog.Debug(ctx, fmt.Sprintf("Reading assignment filter with ID: %s", state.ID.ValueString())) readTimeout, diags := state.Timeouts.Read(ctx, 30*time.Second) resp.Diagnostics.Append(diags...) @@ -84,15 +79,7 @@ func (r *AssignmentFilterResource) Read(ctx context.Context, req resource.ReadRe assignmentFilter, err := r.client.DeviceManagement().AssignmentFilters().ByDeviceAndAppManagementAssignmentFilterId(state.ID.ValueString()).Get(ctx, nil) if err != nil { - if common.IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { - tflog.Warn(ctx, fmt.Sprintf("%s with ID %s not found, removing from state", r.TypeName, state.ID.ValueString())) - resp.State.RemoveResource(ctx) - return - } - resp.Diagnostics.AddError( - "Error reading assignment filter", - fmt.Sprintf("Could not update %s with ID %s: %s", r.TypeName, state.ID.ValueString(), err.Error()), - ) + common.HandleReadStateError(ctx, resp, r, &state, err) return } @@ -121,7 +108,6 @@ func (r *AssignmentFilterResource) Update(ctx context.Context, req resource.Upda defer cancel() requestBody, err := constructResource(ctx, &plan) - if err != nil { resp.Diagnostics.AddError( "Error constructing assignment filter", @@ -132,15 +118,7 @@ func (r *AssignmentFilterResource) Update(ctx context.Context, req resource.Upda _, err = r.client.DeviceManagement().AssignmentFilters().ByDeviceAndAppManagementAssignmentFilterId(plan.ID.ValueString()).Patch(ctx, requestBody, nil) if err != nil { - if common.IsNotFoundError(err) || strings.Contains(err.Error(), "An error has occurred") { - tflog.Warn(ctx, fmt.Sprintf("%s with ID %s not found, removing from state", r.TypeName, plan.ID.ValueString())) - resp.State.RemoveResource(ctx) - return - } - resp.Diagnostics.AddError( - "Error reading assignment filter", - fmt.Sprintf("Could not update %s with ID %s: %s", r.TypeName, plan.ID.ValueString(), err.Error()), - ) + common.HandleUpdateStateError(ctx, resp, r, &plan, err) return } diff --git a/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go b/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go index af31759f..6d5738f6 100644 --- a/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go +++ b/internal/resources/deviceandappmanagement/beta/assignmentFilter/resource.go @@ -43,10 +43,19 @@ type AssignmentFilterResourceModel struct { CreatedDateTime types.String `tfsdk:"created_date_time"` LastModifiedDateTime types.String `tfsdk:"last_modified_date_time"` RoleScopeTags types.List `tfsdk:"role_scope_tags"` - Payloads types.List `tfsdk:"payloads"` Timeouts timeouts.Value `tfsdk:"timeouts"` } +// GetID returns the ID of a resource from the state model. +func (s *AssignmentFilterResourceModel) GetID() string { + return s.ID.ValueString() +} + +// GetTypeName returns the type name of the resource from the state model. +func (r *AssignmentFilterResource) GetTypeName() string { + return r.TypeName +} + // Metadata returns the resource type name. func (r *AssignmentFilterResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = req.ProviderTypeName + "_graph_beta_device_and_app_management_assignment_filter"