fix(entity_tags): fixed improper error handling in the `newrelic_enti…

…ty_tags` resource (#2710)

Co-authored-by: pranav-new-relic <[email protected]>

newrelic/resource_newrelic_entity_tags.go

@@ -77,11 +77,13 @@ func resourceNewRelicEntityTagsCreate(ctx context.Context, d *schema.ResourceDat
 	guid := common.EntityGUID(d.Get("guid").(string))
 	tags := expandEntityTags(d.Get("tag").(*schema.Set).List())
 
-	_, err := client.Entities.TaggingAddTagsToEntityWithContext(ctx, guid, tags)
+	res, err := client.Entities.TaggingAddTagsToEntityWithContext(ctx, guid, tags)
 	if err != nil {
 		return diag.FromErr(err)
 	}
-
+	if res != nil && len(res.Errors) > 0 {
+		return handleEntityTagsMutationEmbeddedErrors(res)
+	}
 	d.SetId(string(guid))
 
 	retryErr := resource.RetryContext(ctx, d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {
@@ -147,11 +149,13 @@ func resourceNewRelicEntityTagsUpdate(ctx context.Context, d *schema.ResourceDat
 	log.Printf("[INFO] Updating New Relic entity tags for entity guid %s", d.Id())
 
 	tags := expandEntityTags(d.Get("tag").(*schema.Set).List())
-
-	_, err := client.Entities.TaggingReplaceTagsOnEntityWithContext(ctx, common.EntityGUID(d.Id()), tags)
+	res, err := client.Entities.TaggingReplaceTagsOnEntityWithContext(ctx, common.EntityGUID(d.Id()), tags)
 	if err != nil {
 		return diag.FromErr(err)
 	}
+	if res != nil && len(res.Errors) > 0 {
+		return handleEntityTagsMutationEmbeddedErrors(res)
+	}
 
 	retryErr := resource.RetryContext(ctx, d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {
 		t, err := client.Entities.GetTagsForEntityMutable(common.EntityGUID(d.Id()))
@@ -161,10 +165,6 @@ func resourceNewRelicEntityTagsUpdate(ctx context.Context, d *schema.ResourceDat
 
 		currentTags := convertTagTypes(t)
 
-		if err != nil {
-			return resource.NonRetryableError(fmt.Errorf("error retrieving entity tags for guid %s: %s", d.Id(), err))
-		}
-
 		for _, t := range tags {
 			var tag *entities.TaggingTagInput
 			if tag = getTag(currentTags, t.Key); tag == nil {
@@ -214,11 +214,13 @@ func resourceNewRelicEntityTagsDelete(ctx context.Context, d *schema.ResourceDat
 	tags := expandEntityTags(d.Get("tag").(*schema.Set).List())
 	tagKeys := getTagKeys(tags)
 
-	_, err := client.Entities.TaggingDeleteTagFromEntityWithContext(ctx, common.EntityGUID(d.Id()), tagKeys)
+	res, err := client.Entities.TaggingDeleteTagFromEntityWithContext(ctx, common.EntityGUID(d.Id()), tagKeys)
 	if err != nil {
 		return diag.FromErr(err)
 	}
-
+	if res != nil && len(res.Errors) > 0 {
+		return handleEntityTagsMutationEmbeddedErrors(res)
+	}
 	return nil
 }
 
@@ -303,3 +305,14 @@ func getTag(tags []*entities.TaggingTagInput, key string) *entities.TaggingTagIn
 
 	return nil
 }
+
+func handleEntityTagsMutationEmbeddedErrors(res *entities.TaggingMutationResult) diag.Diagnostics {
+	var diags diag.Diagnostics
+	for _, Error := range res.Errors {
+		diags = append(diags, diag.Diagnostic{
+			Severity: diag.Error,
+			Summary:  Error.Message + ": " + string(Error.Type),
+		})
+	}
+	return diags
+}

newrelic/resource_newrelic_entity_tags_test.go

@@ -6,12 +6,12 @@ package newrelic
 import (
 	"context"
 	"fmt"
-	"testing"
-	"time"
-
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 	"github.com/newrelic/newrelic-client-go/v2/pkg/common"
+	"regexp"
+	"testing"
+	"time"
 )
 
 func TestAccNewRelicEntityTags_Basic(t *testing.T) {
@@ -24,19 +24,36 @@ func TestAccNewRelicEntityTags_Basic(t *testing.T) {
 		Steps: []resource.TestStep{
 			// Test: Create
 			{
-				Config: testAccNewRelicEntityTagsConfig(testAccExpectedApplicationName),
+				Config:      testAccNewRelicEntityTagsConfig(testAccExpectedApplicationName, "account", "test-account"),
+				ExpectError: regexp.MustCompile("reserved"), // Error: Tag Key 'account' is a reserved key
+				PreConfig: func() {
+					time.Sleep(10 * time.Second)
+				},
+			},
+			{
+				Config: testAccNewRelicEntityTagsConfig(testAccExpectedApplicationName, "test_key", "test_value"),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckNewRelicEntityTagsExist(resourceName, []string{"test_key"}),
 					testAccCheckNewRelicEntityUnmutableExists(resourceName, []string{"account", "guid", "language"}),
 				),
+				PreConfig: func() {
+					time.Sleep(10 * time.Second)
+				},
 			},
 			// Test: Update
 			{
-				Config: testAccNewRelicEntityTagsConfigUpdated(testAccExpectedApplicationName),
+				Config: testAccNewRelicEntityTagsConfig(testAccExpectedApplicationName, "test_key_2", "test_value_2"),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckNewRelicEntityTagsExist(resourceName, []string{"test_key_2"}),
 					testAccCheckNewRelicEntityUnmutableExists(resourceName, []string{"account", "guid", "language"}),
 				),
+				PreConfig: func() {
+					time.Sleep(10 * time.Second)
+				},
+			},
+			{
+				Config:      testAccNewRelicEntityTagsConfig(testAccExpectedApplicationName, "account", "test-account-2"),
+				ExpectError: regexp.MustCompile("reserved"), // Error: Tag Key 'account' is a reserved key
 			},
 			// Test: Import
 			//{
@@ -140,26 +157,7 @@ func testAccCheckNewRelicEntityUnmutableExists(n string, keysToCheck []string) r
 	}
 }
 
-func testAccNewRelicEntityTagsConfig(appName string) string {
-	return fmt.Sprintf(`
-data "newrelic_entity" "foo" {
-  name = "%s"
-  type = "APPLICATION"
-  domain = "APM"
-}
-
-resource "newrelic_entity_tags" "foo" {
-  guid = data.newrelic_entity.foo.guid
-
-  tag {
-	key = "test_key"
-	values = ["test_value"]
-  }
-}
-`, appName)
-}
-
-func testAccNewRelicEntityTagsConfigUpdated(appName string) string {
+func testAccNewRelicEntityTagsConfig(appName string, tagKey string, tagValue string) string {
 	return fmt.Sprintf(`
 data "newrelic_entity" "foo" {
   name = "%s"
@@ -171,9 +169,9 @@ resource "newrelic_entity_tags" "foo" {
   guid = data.newrelic_entity.foo.guid
 
   tag {
-	key = "test_key_2"
-	values = ["test_value_2"]
+	key = "%s"
+	values = ["%s"]
   }
 }
-`, appName)
+`, appName, tagKey, tagValue)
 }

website/docs/r/entity_tags.html.markdown

@@ -85,7 +85,9 @@ The following arguments are supported:
 
 All nested `tag` blocks support the following common arguments:
 
-  * `key` - (Required) The tag key.
+  * `key` - (Required) The key of the tag.
+
+-> **NOTE:** One should not use reserved (immutable) keys with this resource. It is recommended to choose unique and descriptive keys which do not conflict with existing reserved keys.
   * `values` - (Required) The tag values.
 
 ## Import