From 53617419e7e47e2a497852aef073aa13c507e632 Mon Sep 17 00:00:00 2001 From: Michal Dobaczewski Date: Thu, 25 Jul 2024 15:56:51 +0300 Subject: [PATCH] Standardise UUID parsing and error reporting in already tested code --- .../aclmapping/acl_mapping_resource.go | 20 +++++------ internal/provider/project/project_resource.go | 35 ++++++++++++++----- internal/provider/team/team_data_source.go | 16 +++------ internal/provider/team/team_resource.go | 4 +-- .../teamapikey/team_api_key_resource.go | 2 +- .../team_permission_resource.go | 10 +++--- internal/utils/terraform.go | 14 ++++++++ internal/utils/utils_test.go | 27 ++++++++++++++ 8 files changed, 90 insertions(+), 38 deletions(-) diff --git a/internal/provider/aclmapping/acl_mapping_resource.go b/internal/provider/aclmapping/acl_mapping_resource.go index fae3f25..b08fdf3 100644 --- a/internal/provider/aclmapping/acl_mapping_resource.go +++ b/internal/provider/aclmapping/acl_mapping_resource.go @@ -89,10 +89,10 @@ func (r *ACLMappingResource) Create(ctx context.Context, req resource.CreateRequ return } - teamID, teamIDDiags := utils.ParseUUID(plan.TeamID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(teamIDDiags...) - projectID, projectIDDiags := utils.ParseUUID(plan.ProjectID.ValueString()) + projectID, projectIDDiags := utils.ParseAttributeUUID(plan.ProjectID.ValueString(), "project_id") resp.Diagnostics.Append(projectIDDiags...) if resp.Diagnostics.HasError() { @@ -125,10 +125,10 @@ func (r *ACLMappingResource) Read(ctx context.Context, req resource.ReadRequest, return } - teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(teamIDDiags...) - projectID, projectIDDiags := utils.ParseUUID(state.ProjectID.ValueString()) + projectID, projectIDDiags := utils.ParseAttributeUUID(state.ProjectID.ValueString(), "project_id") resp.Diagnostics.Append(projectIDDiags...) if resp.Diagnostics.HasError() { @@ -171,16 +171,16 @@ func (r *ACLMappingResource) Update(ctx context.Context, req resource.UpdateRequ return } - oldTeamID, oldTeamIDDiags := utils.ParseUUID(state.TeamID.ValueString()) + oldTeamID, oldTeamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(oldTeamIDDiags...) - oldProjectID, oldProjectIDDiags := utils.ParseUUID(state.ProjectID.ValueString()) + oldProjectID, oldProjectIDDiags := utils.ParseAttributeUUID(state.ProjectID.ValueString(), "project_id") resp.Diagnostics.Append(oldProjectIDDiags...) - newTeamID, newTeamIDDiags := utils.ParseUUID(plan.TeamID.ValueString()) + newTeamID, newTeamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(newTeamIDDiags...) - newProjectID, newProjectIDDiags := utils.ParseUUID(plan.ProjectID.ValueString()) + newProjectID, newProjectIDDiags := utils.ParseAttributeUUID(plan.ProjectID.ValueString(), "project_id") resp.Diagnostics.Append(newProjectIDDiags...) if resp.Diagnostics.HasError() { @@ -224,10 +224,10 @@ func (r *ACLMappingResource) Delete(ctx context.Context, req resource.DeleteRequ return } - teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(teamIDDiags...) - projectID, projectIDDiags := utils.ParseUUID(state.ProjectID.ValueString()) + projectID, projectIDDiags := utils.ParseAttributeUUID(state.ProjectID.ValueString(), "project_id") resp.Diagnostics.Append(projectIDDiags...) if resp.Diagnostics.HasError() { diff --git a/internal/provider/project/project_resource.go b/internal/provider/project/project_resource.go index ec96e4b..b032a95 100644 --- a/internal/provider/project/project_resource.go +++ b/internal/provider/project/project_resource.go @@ -7,9 +7,9 @@ import ( "context" "fmt" - dtrack "github.com/futurice/dependency-track-client-go" - "github.com/google/uuid" + "github.com/futurice/terraform-provider-dependencytrack/internal/utils" + dtrack "github.com/futurice/dependency-track-client-go" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -120,13 +120,15 @@ func (r *ProjectResource) Create(ctx context.Context, req resource.CreateRequest var plan, state ProjectResourceModel resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) - if resp.Diagnostics.HasError() { return } dtProject, diags := TFProjectToDTProject(ctx, plan) resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } respProject, err := r.client.Project.Create(ctx, dtProject) if err != nil { @@ -157,12 +159,17 @@ func (r *ProjectResource) Read(ctx context.Context, req resource.ReadRequest, re var diags diag.Diagnostics resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + projectID, projectIDDiags := utils.ParseAttributeUUID(state.ID.ValueString(), "id") + resp.Diagnostics.Append(projectIDDiags...) if resp.Diagnostics.HasError() { return } - respProject, err := r.client.Project.Get(ctx, uuid.MustParse(state.ID.ValueString())) + respProject, err := r.client.Project.Get(ctx, projectID) if err != nil { if apiErr, ok := err.(*dtrack.APIError); ok && apiErr.StatusCode == 404 { resp.State.RemoveResource(ctx) @@ -184,13 +191,15 @@ func (r *ProjectResource) Update(ctx context.Context, req resource.UpdateRequest resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) resp.Diagnostics.Append(req.State.Get(ctx, &state)...) - if resp.Diagnostics.HasError() { return } dtProject, diags := TFProjectToDTProject(ctx, plan) resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } respProject, err := r.client.Project.Update(ctx, dtProject) if err != nil { @@ -211,12 +220,17 @@ func (r *ProjectResource) Delete(ctx context.Context, req resource.DeleteRequest var state ProjectResourceModel resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + projectID, projectIDDiags := utils.ParseAttributeUUID(state.ID.ValueString(), "id") + resp.Diagnostics.Append(projectIDDiags...) if resp.Diagnostics.HasError() { return } - err := r.client.Project.Delete(ctx, uuid.MustParse(state.ID.ValueString())) + err := r.client.Project.Delete(ctx, projectID) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete project, got error: %s", err)) return @@ -255,6 +269,7 @@ func DTProjectToTFProject(ctx context.Context, dtProject dtrack.Project) (Projec func TFProjectToDTProject(ctx context.Context, tfProject ProjectResourceModel) (dtrack.Project, diag.Diagnostics) { var diags diag.Diagnostics + project := dtrack.Project{ Name: tfProject.Name.ValueString(), Classifier: tfProject.Classifier.ValueString(), @@ -263,11 +278,15 @@ func TFProjectToDTProject(ctx context.Context, tfProject ProjectResourceModel) ( } if tfProject.ID.ValueString() != "" { - project.UUID = uuid.MustParse(tfProject.ID.ValueString()) + projectID, projectIDDiags := utils.ParseAttributeUUID(tfProject.ID.ValueString(), "id") + project.UUID = projectID + diags.Append(projectIDDiags...) } if !tfProject.ParentID.IsNull() { - project.ParentRef = &dtrack.ParentRef{UUID: uuid.MustParse(tfProject.ParentID.ValueString())} + parentProjectID, parentProjectIDDiags := utils.ParseAttributeUUID(tfProject.ParentID.ValueString(), "parent_id") + project.ParentRef = &dtrack.ParentRef{UUID: parentProjectID} + diags.Append(parentProjectIDDiags...) } return project, diags diff --git a/internal/provider/team/team_data_source.go b/internal/provider/team/team_data_source.go index a63475a..74a6705 100644 --- a/internal/provider/team/team_data_source.go +++ b/internal/provider/team/team_data_source.go @@ -7,12 +7,11 @@ import ( "context" "fmt" - dtrack "github.com/futurice/dependency-track-client-go" - "github.com/google/uuid" + "github.com/futurice/terraform-provider-dependencytrack/internal/utils" + dtrack "github.com/futurice/dependency-track-client-go" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/datasource/schema" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" ) @@ -90,19 +89,12 @@ func (d *TeamDataSource) Read(ctx context.Context, req datasource.ReadRequest, r var model TeamDataSourceModel resp.Diagnostics.Append(req.Config.Get(ctx, &model)...) - if resp.Diagnostics.HasError() { return } - teamID, err := uuid.Parse(model.ID.ValueString()) - if err != nil { - resp.Diagnostics.AddAttributeError(path.Root("id"), - "Invalid team ID", - "Team ID has to be a valid UUID.", - ) - } - + teamID, teamIDDiags := utils.ParseAttributeUUID(model.ID.ValueString(), "id") + resp.Diagnostics.Append(teamIDDiags...) if resp.Diagnostics.HasError() { return } diff --git a/internal/provider/team/team_resource.go b/internal/provider/team/team_resource.go index f6c8e02..47b473b 100644 --- a/internal/provider/team/team_resource.go +++ b/internal/provider/team/team_resource.go @@ -112,7 +112,7 @@ func (r *TeamResource) Read(ctx context.Context, req resource.ReadRequest, resp return } - teamID, teamIDDiags := utils.ParseUUID(state.ID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(state.ID.ValueString(), "id") resp.Diagnostics.Append(teamIDDiags...) if resp.Diagnostics.HasError() { return @@ -206,7 +206,7 @@ func TFTeamToDTTeam(ctx context.Context, tfTeam TeamResourceModel) (dtrack.Team, } if tfTeam.ID.ValueString() != "" { - teamID, teamIDDiags := utils.ParseUUID(tfTeam.ID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(tfTeam.ID.ValueString(), "id") team.UUID = teamID diags.Append(teamIDDiags...) } diff --git a/internal/provider/teamapikey/team_api_key_resource.go b/internal/provider/teamapikey/team_api_key_resource.go index 7400c2a..bc5484f 100644 --- a/internal/provider/teamapikey/team_api_key_resource.go +++ b/internal/provider/teamapikey/team_api_key_resource.go @@ -89,7 +89,7 @@ func (r *TeamAPIKeyResource) Create(ctx context.Context, req resource.CreateRequ return } - teamID, teamIDDiags := utils.ParseUUID(plan.TeamID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(teamIDDiags...) if resp.Diagnostics.HasError() { return diff --git a/internal/provider/teampermission/team_permission_resource.go b/internal/provider/teampermission/team_permission_resource.go index 62a066c..1fc5818 100644 --- a/internal/provider/teampermission/team_permission_resource.go +++ b/internal/provider/teampermission/team_permission_resource.go @@ -89,7 +89,7 @@ func (r *TeamPermissionResource) Create(ctx context.Context, req resource.Create return } - teamID, teamIDDiags := utils.ParseUUID(plan.TeamID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(teamIDDiags...) if resp.Diagnostics.HasError() { return @@ -129,7 +129,7 @@ func (r *TeamPermissionResource) Read(ctx context.Context, req resource.ReadRequ return } - teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(teamIDDiags...) if resp.Diagnostics.HasError() { return @@ -171,10 +171,10 @@ func (r *TeamPermissionResource) Update(ctx context.Context, req resource.Update return } - newTeamID, newTeamIDDiags := utils.ParseUUID(plan.TeamID.ValueString()) + newTeamID, newTeamIDDiags := utils.ParseAttributeUUID(plan.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(newTeamIDDiags...) - oldTeamID, oldTeamIDDiags := utils.ParseUUID(state.TeamID.ValueString()) + oldTeamID, oldTeamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(oldTeamIDDiags...) if resp.Diagnostics.HasError() { @@ -216,7 +216,7 @@ func (r *TeamPermissionResource) Delete(ctx context.Context, req resource.Delete return } - teamID, teamIDDiags := utils.ParseUUID(state.TeamID.ValueString()) + teamID, teamIDDiags := utils.ParseAttributeUUID(state.TeamID.ValueString(), "team_id") resp.Diagnostics.Append(teamIDDiags...) if resp.Diagnostics.HasError() { return diff --git a/internal/utils/terraform.go b/internal/utils/terraform.go index 08b2b4e..6ab3a9e 100644 --- a/internal/utils/terraform.go +++ b/internal/utils/terraform.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" ) // ParseUUID parses the UUID value returning a possible error as Diagnostics instead of error. @@ -18,3 +19,16 @@ func ParseUUID(uuidString string) (uuid.UUID, diag.Diagnostics) { return id, diags } + +// ParseAttributeUUID works just as ParseUUID but returns any errors as attribute errors on the specified attribute path. +func ParseAttributeUUID(uuidString string, attributePath string) (uuid.UUID, diag.Diagnostics) { + var diags diag.Diagnostics + + id, err := uuid.Parse(uuidString) + if err != nil { + diags.AddAttributeError(path.Root(attributePath), "Invalid UUID", fmt.Sprintf("Failed to parse string [%s] as UUID: %v", uuidString, err)) + return uuid.UUID{}, diags + } + + return id, diags +} diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index eedb497..17bef46 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -3,6 +3,8 @@ package utils_test import ( "github.com/futurice/terraform-provider-dependencytrack/internal/utils" "github.com/google/uuid" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" "testing" ) @@ -26,3 +28,28 @@ func TestParseUUID_invalid(t *testing.T) { t.Errorf("Error expected, but received none") } } + +func TestParseAttributeUUID_basic(t *testing.T) { + testUUID := uuid.MustParse("8ffb30fb-77e6-4886-9f32-ff142f9bf90b") + result, diags := utils.ParseAttributeUUID(testUUID.String(), "id") + + if result != testUUID { + t.Errorf("Parsed UUID [%s] is different than expected [%s]", result, testUUID) + } + + if diags.HasError() { + t.Errorf("Unexpected error: %v", diags) + } +} + +func TestParseAttributeUUID_invalid(t *testing.T) { + _, diags := utils.ParseAttributeUUID("not-an-UUID", "id") + + if !diags.HasError() { + t.Errorf("Error expected, but received none") + } + + if !diags.Contains(diag.NewAttributeErrorDiagnostic(path.Root("id"), "Invalid UUID", "Failed to parse string [not-an-UUID] as UUID: invalid UUID length: 11")) { + t.Errorf("Diags do not contain the expected attribute error: %v", diags) + } +}