From 123a4f46e96767bfaa5b4d380b2f68220b6e3e7b Mon Sep 17 00:00:00 2001 From: Luca Ioffredo Date: Wed, 11 Sep 2024 13:26:56 +0200 Subject: [PATCH 1/2] Added new context functions. Now the code handles the error in the response on resource tags calls. --- .../resource_ibm_resource_tag.go | 114 +++++++++++++----- .../resource_ibm_resource_tag_test.go | 23 ++++ 2 files changed, 110 insertions(+), 27 deletions(-) diff --git a/ibm/service/globaltagging/resource_ibm_resource_tag.go b/ibm/service/globaltagging/resource_ibm_resource_tag.go index 34df86fc29e..709910ad34b 100644 --- a/ibm/service/globaltagging/resource_ibm_resource_tag.go +++ b/ibm/service/globaltagging/resource_ibm_resource_tag.go @@ -5,6 +5,7 @@ package globaltagging import ( "context" + "encoding/json" "fmt" "log" "os" @@ -18,6 +19,7 @@ import ( "github.com/IBM-Cloud/terraform-provider-ibm/ibm/conns" "github.com/IBM-Cloud/terraform-provider-ibm/ibm/flex" "github.com/IBM-Cloud/terraform-provider-ibm/ibm/validate" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" ) const ( @@ -32,11 +34,11 @@ const ( func ResourceIBMResourceTag() *schema.Resource { return &schema.Resource{ - Create: resourceIBMResourceTagCreate, - Read: resourceIBMResourceTagRead, - Update: resourceIBMResourceTagUpdate, - Delete: resourceIBMResourceTagDelete, - Importer: &schema.ResourceImporter{}, + CreateContext: resourceIBMResourceTagCreate, + ReadContext: resourceIBMResourceTagRead, + UpdateContext: resourceIBMResourceTagUpdate, + DeleteContext: resourceIBMResourceTagDelete, + Importer: &schema.ResourceImporter{}, CustomizeDiff: customdiff.Sequence( func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { @@ -125,19 +127,23 @@ func ResourceIBMResourceTagValidator() *validate.ResourceValidator { return &ibmResourceTagValidator } -func resourceIBMResourceTagCreate(d *schema.ResourceData, meta interface{}) error { +func resourceIBMResourceTagCreate(context context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var rType, tType string resources := []globaltaggingv1.Resource{} userDetails, err := meta.(conns.ClientSession).BluemixUserDetails() if err != nil { - return err + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "create") + log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) + return tfErr.GetDiag() } accountID := userDetails.UserAccount gtClient, err := meta.(conns.ClientSession).GlobalTaggingAPIv1() if err != nil { - return fmt.Errorf("[ERROR] Error getting global tagging client settings: %s", err) + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "create") + log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) + return tfErr.GetDiag() } resourceID := d.Get(resourceID).(string) @@ -187,9 +193,24 @@ func resourceIBMResourceTagCreate(d *schema.ResourceData, meta interface{}) erro } if len(add) > 0 { - _, resp, err := gtClient.AttachTag(AttachTagOptions) + results, fullResponse, err := gtClient.AttachTagWithContext(context, AttachTagOptions) if err != nil { - return fmt.Errorf("[ERROR] Error attaching resource tags : %v\n%s", resp, err) + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "create") + return tfErr.GetDiag() + } + + // Check if there are errors on the attach internal response + if results != nil { + errMap := make([]globaltaggingv1.TagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return diag.FromErr(fmt.Errorf("Error while creating tag: %s - Full response: %s", string(output), fullResponse)) + } } response, errored := flex.WaitForTagsAvailable(meta, resourceID, resourceType, tagType, news, d.Timeout(schema.TimeoutCreate)) if errored != nil { @@ -204,15 +225,17 @@ func resourceIBMResourceTagCreate(d *schema.ResourceData, meta interface{}) erro d.SetId(fmt.Sprintf("%s/%s", resourceID, rType)) } - return resourceIBMResourceTagRead(d, meta) + return resourceIBMResourceTagRead(context, d, meta) } -func resourceIBMResourceTagRead(d *schema.ResourceData, meta interface{}) error { +func resourceIBMResourceTagRead(context context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var rID, rType, tType string userDetails, err := meta.(conns.ClientSession).BluemixUserDetails() if err != nil { - return err + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "read") + log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) + return tfErr.GetDiag() } acctID := userDetails.UserAccount @@ -221,10 +244,12 @@ func resourceIBMResourceTagRead(d *schema.ResourceData, meta interface{}) error } else { parts, err := flex.VmIdParts(d.Id()) if err != nil { - return err + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "read") + log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) + return tfErr.GetDiag() } if len(parts) < 2 { - return fmt.Errorf("[ERROR] Incorrect ID %s: Id should be a combination of resourceID/resourceType", d.Id()) + return diag.FromErr(fmt.Errorf("Incorrect ID %s: Id should be a combination of resourceID/resourceType", d.Id())) } rID = parts[0] rType = parts[1] @@ -240,7 +265,7 @@ func resourceIBMResourceTagRead(d *schema.ResourceData, meta interface{}) error tagList, err := flex.GetGlobalTagsUsingSearchAPI(meta, rID, rType, tType) if err != nil { - return fmt.Errorf("[ERROR] Error getting resource tags for: %s with error : %s", rID, err) + return diag.FromErr(fmt.Errorf("Error getting resource tags for: %s with error : %s", rID, err)) } d.Set(resourceID, rID) @@ -250,7 +275,7 @@ func resourceIBMResourceTagRead(d *schema.ResourceData, meta interface{}) error return nil } -func resourceIBMResourceTagUpdate(d *schema.ResourceData, meta interface{}) error { +func resourceIBMResourceTagUpdate(context context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var rID, rType, tType string if strings.HasPrefix(d.Id(), "crn:") { @@ -258,7 +283,9 @@ func resourceIBMResourceTagUpdate(d *schema.ResourceData, meta interface{}) erro } else { parts, err := flex.VmIdParts(d.Id()) if err != nil { - return err + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "update") + log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) + return tfErr.GetDiag() } rID = parts[0] rType = parts[1] @@ -272,14 +299,14 @@ func resourceIBMResourceTagUpdate(d *schema.ResourceData, meta interface{}) erro oldList, newList := d.GetChange(tags) err := flex.UpdateGlobalTagsUsingCRN(oldList, newList, meta, rID, rType, tType) if err != nil { - return fmt.Errorf("[ERROR] Error on create of resource tags: %s", err) + return diag.FromErr(fmt.Errorf("Error on create of resource tags: %s", err)) } } - return resourceIBMResourceTagRead(d, meta) + return resourceIBMResourceTagRead(context, d, meta) } -func resourceIBMResourceTagDelete(d *schema.ResourceData, meta interface{}) error { +func resourceIBMResourceTagDelete(context context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var rID, rType string if strings.HasPrefix(d.Id(), "crn:") { @@ -287,7 +314,9 @@ func resourceIBMResourceTagDelete(d *schema.ResourceData, meta interface{}) erro } else { parts, err := flex.VmIdParts(d.Id()) if err != nil { - return err + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "delete") + log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) + return tfErr.GetDiag() } rID = parts[0] rType = parts[1] @@ -295,7 +324,9 @@ func resourceIBMResourceTagDelete(d *schema.ResourceData, meta interface{}) erro gtClient, err := meta.(conns.ClientSession).GlobalTaggingAPIv1() if err != nil { - return fmt.Errorf("[ERROR] Error getting global tagging client settings: %s", err) + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "delete") + log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) + return tfErr.GetDiag() } var remove []string @@ -322,17 +353,46 @@ func resourceIBMResourceTagDelete(d *schema.ResourceData, meta interface{}) erro TagType: &tType, } - _, resp, err := gtClient.DetachTag(detachTagOptions) + results, fullResponse, err := gtClient.DetachTagWithContext(context, detachTagOptions) if err != nil { - return fmt.Errorf("[ERROR] Error detaching resource tags %v: %s\n%s", remove, err, resp) + tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "delete") + return tfErr.GetDiag() } + + // Check if there are errors on the detach internal response + if results != nil { + errMap := make([]globaltaggingv1.TagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return diag.FromErr(fmt.Errorf("Error while detaching tag: %s - Full response: %s", string(output), fullResponse)) + } + } + for _, v := range remove { delTagOptions := &globaltaggingv1.DeleteTagOptions{ TagName: flex.PtrToString(v), } - _, resp, err := gtClient.DeleteTag(delTagOptions) + results, fullResponse, err := gtClient.DeleteTagWithContext(context, delTagOptions) if err != nil { - return fmt.Errorf("[ERROR] Error deleting resource tag %v: %s\n%s", v, err, resp) + return diag.FromErr(fmt.Errorf("Error deleting resource tag %v: %s\n%s", v, err, fullResponse)) + } + + if results != nil { + errMap := make([]globaltaggingv1.DeleteTagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return diag.FromErr(fmt.Errorf("Error while deleting tag: %s - Full response: %s", string(output), fullResponse)) + } } } } diff --git a/ibm/service/globaltagging/resource_ibm_resource_tag_test.go b/ibm/service/globaltagging/resource_ibm_resource_tag_test.go index b7dca8d395c..3867dc422e9 100644 --- a/ibm/service/globaltagging/resource_ibm_resource_tag_test.go +++ b/ibm/service/globaltagging/resource_ibm_resource_tag_test.go @@ -41,6 +41,20 @@ func TestAccResourceTag_Basic(t *testing.T) { }, }) } + +func TestAccResourceTag_AccountOutOfScopeError(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + Providers: acc.TestAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCheckResourceTagCreateAccountOutOfScope(), + ExpectError: regexp.MustCompile("\"is_error\": true"), + }, + }, + }) +} + func TestAccResourceTag_Wait(t *testing.T) { name := fmt.Sprintf("tf-cos-%d", acctest.RandIntRange(10, 100)) @@ -122,6 +136,15 @@ func testAccCheckResourceTagCreate(name string) string { `, name) } +func testAccCheckResourceTagCreateAccountOutOfScope() string { + return fmt.Sprintf(` + resource "ibm_resource_tag" "tag" { + resource_id = "crn:v1:staging:public:cloud-object-storage:global:a/d99e99999dfe99ee999999f99bddd099:ab25d9be-5e0c-44dd-ad89-0bced3992758::" + tags = ["env:dev", "cpu:4"] + } +`) +} + func TestAccResourceTag_replace_Basic(t *testing.T) { name := fmt.Sprintf("tf-cos-%d", acctest.RandIntRange(10, 100)) From 44c4f4baa99a448d8acca1d561039d97ccad7816 Mon Sep 17 00:00:00 2001 From: Luca Ioffredo Date: Thu, 12 Sep 2024 06:47:16 +0200 Subject: [PATCH 2/2] Fixed 5635 --- ibm/flex/structures.go | 94 +++++++++++++++++-- .../resource_ibm_resource_tag.go | 63 +------------ .../resource_ibm_resource_tag_test.go | 6 +- 3 files changed, 90 insertions(+), 73 deletions(-) diff --git a/ibm/flex/structures.go b/ibm/flex/structures.go index da354497e63..79909fc004e 100644 --- a/ibm/flex/structures.go +++ b/ibm/flex/structures.go @@ -2532,17 +2532,42 @@ func UpdateGlobalTagsUsingCRN(oldList, newList interface{}, meta interface{}, re } } - _, resp, err := gtClient.DetachTag(detachTagOptions) + results, fullResponse, err := gtClient.DetachTag(detachTagOptions) if err != nil { - return fmt.Errorf("[ERROR] Error detaching database tags %v: %s\n%s", remove, err, resp) + return fmt.Errorf("[ERROR] Error detaching database tags %v: %s\n%s", remove, err, fullResponse) + } + if results != nil { + errMap := make([]globaltaggingv1.TagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return fmt.Errorf("[ERROR] Error detaching tag %v: %s\n%s", remove, string(output), fullResponse) + } } for _, v := range remove { delTagOptions := &globaltaggingv1.DeleteTagOptions{ TagName: PtrToString(v), } - _, resp, err := gtClient.DeleteTag(delTagOptions) + results, fullResponse, err := gtClient.DeleteTag(delTagOptions) if err != nil { - return fmt.Errorf("[ERROR] Error deleting database tag %v: %s\n%s", v, err, resp) + return fmt.Errorf("[ERROR] Error deleting database tag %v: %s\n%s", v, err, fullResponse) + } + + if results != nil { + errMap := make([]globaltaggingv1.DeleteTagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return fmt.Errorf("[ERROR] Error deleting tag %s: %s\n%s", PtrToString(v), string(output), fullResponse) + } } } } @@ -2624,7 +2649,7 @@ func GetTagsUsingCRN(meta interface{}, resourceCRN string) (*schema.Set, error) } func UpdateTagsUsingCRN(oldList, newList interface{}, meta interface{}, resourceCRN string) error { - gtClient, err := meta.(conns.ClientSession).GlobalTaggingAPI() + gtClient, err := meta.(conns.ClientSession).GlobalTaggingAPIv1() if err != nil { return fmt.Errorf("[ERROR] Error getting global tagging client settings: %s", err) } @@ -2654,24 +2679,75 @@ func UpdateTagsUsingCRN(oldList, newList interface{}, meta interface{}, resource add = append(add, envTags...) } + resources := []globaltaggingv1.Resource{} + r := globaltaggingv1.Resource{ResourceID: &resourceCRN} + resources = append(resources, r) + if len(remove) > 0 { - _, err := gtClient.Tags().DetachTags(resourceCRN, remove) + detachTagOptions := &globaltaggingv1.DetachTagOptions{} + detachTagOptions.Resources = resources + detachTagOptions.TagNames = remove + + results, fullResponse, err := gtClient.DetachTag(detachTagOptions) if err != nil { return fmt.Errorf("[ERROR] Error detaching database tags %v: %s", remove, err) } + if results != nil { + errMap := make([]globaltaggingv1.TagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return fmt.Errorf("[ERROR] Error detaching tag %v: %s\n%s", remove, string(output), fullResponse) + } + } for _, v := range remove { - _, err := gtClient.Tags().DeleteTag(v) + delTagOptions := &globaltaggingv1.DeleteTagOptions{ + TagName: PtrToString(v), + } + results, fullResponse, err := gtClient.DeleteTag(delTagOptions) if err != nil { - return fmt.Errorf("[ERROR] Error deleting database tag %v: %s", v, err) + return fmt.Errorf("[ERROR] Error deleting database tag %v: %s\n%s", v, err, fullResponse) + } + + if results != nil { + errMap := make([]globaltaggingv1.DeleteTagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return fmt.Errorf("[ERROR] Error deleting tag %s: %s\n%s", PtrToString(v), string(output), fullResponse) + } } } } if len(add) > 0 { - _, err := gtClient.Tags().AttachTags(resourceCRN, add) + AttachTagOptions := &globaltaggingv1.AttachTagOptions{} + AttachTagOptions.Resources = resources + AttachTagOptions.TagNames = add + results, fullResponse, err := gtClient.AttachTag(AttachTagOptions) if err != nil { return fmt.Errorf("[ERROR] Error updating database tags %v : %s", add, err) } + if results != nil { + errMap := make([]globaltaggingv1.TagResultsItem, 0) + for _, res := range results.Results { + if res.IsError != nil && *res.IsError { + errMap = append(errMap, res) + } + } + if len(errMap) > 0 { + output, _ := json.MarshalIndent(errMap, "", " ") + return fmt.Errorf("Error while updating tag: %s - Full response: %s", string(output), fullResponse) + } + } } return nil diff --git a/ibm/service/globaltagging/resource_ibm_resource_tag.go b/ibm/service/globaltagging/resource_ibm_resource_tag.go index 709910ad34b..58b89f1534b 100644 --- a/ibm/service/globaltagging/resource_ibm_resource_tag.go +++ b/ibm/service/globaltagging/resource_ibm_resource_tag.go @@ -322,19 +322,8 @@ func resourceIBMResourceTagDelete(context context.Context, d *schema.ResourceDat rType = parts[1] } - gtClient, err := meta.(conns.ClientSession).GlobalTaggingAPIv1() - if err != nil { - tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "delete") - log.Printf("[DEBUG]\n%s", tfErr.GetDebugMessage()) - return tfErr.GetDiag() - } - var remove []string removeTags := d.Get(tags).(*schema.Set) - remove = make([]string, len(removeTags.List())) - for i, v := range removeTags.List() { - remove[i] = fmt.Sprint(v) - } var tType string if v, ok := d.GetOk(tagType); ok && v != nil { tType = v.(string) @@ -343,57 +332,9 @@ func resourceIBMResourceTagDelete(context context.Context, d *schema.ResourceDat } if len(remove) > 0 { - resources := []globaltaggingv1.Resource{} - r := globaltaggingv1.Resource{ResourceID: flex.PtrToString(rID), ResourceType: flex.PtrToString(rType)} - resources = append(resources, r) - - detachTagOptions := &globaltaggingv1.DetachTagOptions{ - Resources: resources, - TagNames: remove, - TagType: &tType, - } - - results, fullResponse, err := gtClient.DetachTagWithContext(context, detachTagOptions) + err := flex.UpdateGlobalTagsUsingCRN(removeTags, nil, meta, rID, rType, tType) if err != nil { - tfErr := flex.TerraformErrorf(err, err.Error(), "ibm_resource_tag", "delete") - return tfErr.GetDiag() - } - - // Check if there are errors on the detach internal response - if results != nil { - errMap := make([]globaltaggingv1.TagResultsItem, 0) - for _, res := range results.Results { - if res.IsError != nil && *res.IsError { - errMap = append(errMap, res) - } - } - if len(errMap) > 0 { - output, _ := json.MarshalIndent(errMap, "", " ") - return diag.FromErr(fmt.Errorf("Error while detaching tag: %s - Full response: %s", string(output), fullResponse)) - } - } - - for _, v := range remove { - delTagOptions := &globaltaggingv1.DeleteTagOptions{ - TagName: flex.PtrToString(v), - } - results, fullResponse, err := gtClient.DeleteTagWithContext(context, delTagOptions) - if err != nil { - return diag.FromErr(fmt.Errorf("Error deleting resource tag %v: %s\n%s", v, err, fullResponse)) - } - - if results != nil { - errMap := make([]globaltaggingv1.DeleteTagResultsItem, 0) - for _, res := range results.Results { - if res.IsError != nil && *res.IsError { - errMap = append(errMap, res) - } - } - if len(errMap) > 0 { - output, _ := json.MarshalIndent(errMap, "", " ") - return diag.FromErr(fmt.Errorf("Error while deleting tag: %s - Full response: %s", string(output), fullResponse)) - } - } + return diag.FromErr(fmt.Errorf("Error on deleting tags: %s", err)) } } return nil diff --git a/ibm/service/globaltagging/resource_ibm_resource_tag_test.go b/ibm/service/globaltagging/resource_ibm_resource_tag_test.go index 3867dc422e9..3451162b478 100644 --- a/ibm/service/globaltagging/resource_ibm_resource_tag_test.go +++ b/ibm/service/globaltagging/resource_ibm_resource_tag_test.go @@ -42,13 +42,13 @@ func TestAccResourceTag_Basic(t *testing.T) { }) } -func TestAccResourceTag_AccountOutOfScopeError(t *testing.T) { +func TestAccResourceTag_FakeCrnExpectingError(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { acc.TestAccPreCheck(t) }, Providers: acc.TestAccProviders, Steps: []resource.TestStep{ { - Config: testAccCheckResourceTagCreateAccountOutOfScope(), + Config: testAccCheckResourceTagCreateFakeCrnExpectingError(), ExpectError: regexp.MustCompile("\"is_error\": true"), }, }, @@ -136,7 +136,7 @@ func testAccCheckResourceTagCreate(name string) string { `, name) } -func testAccCheckResourceTagCreateAccountOutOfScope() string { +func testAccCheckResourceTagCreateFakeCrnExpectingError() string { return fmt.Sprintf(` resource "ibm_resource_tag" "tag" { resource_id = "crn:v1:staging:public:cloud-object-storage:global:a/d99e99999dfe99ee999999f99bddd099:ab25d9be-5e0c-44dd-ad89-0bced3992758::"