have metadata item fail on create if the key is already present (#3063)…

… (#1714) * have metadata item fail on create if the key is already present * name booleans * compile Signed-off-by: Modular Magician <[email protected]>
hashicorp · Feb 3, 2020 · 91f3dac · 91f3dac
1 parent 7e3495b
commit 91f3dac
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 60 deletions.
diff --git a/.changelog/3063.txt b/.changelog/3063.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+compute: updated `google_compute_project_metadata_item` to fail on create if its key is already present in the project metadata. 
+```
diff --git a/google-beta/resource_compute_project_metadata_item.go b/google-beta/resource_compute_project_metadata_item.go
@@ -9,6 +9,13 @@ import (
 	"google.golang.org/api/compute/v1"
 )
 
+type metadataPresentBehavior bool
+
+const (
+	failIfPresent    metadataPresentBehavior = true
+	overwritePresent metadataPresentBehavior = false
+)
+
 func resourceComputeProjectMetadataItem() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceComputeProjectMetadataItemCreate,
@@ -56,7 +63,7 @@ func resourceComputeProjectMetadataItemCreate(d *schema.ResourceData, meta inter
 	key := d.Get("key").(string)
 	val := d.Get("value").(string)
 
-	err = updateComputeCommonInstanceMetadata(config, projectID, key, &val, int(d.Timeout(schema.TimeoutCreate).Minutes()))
+	err = updateComputeCommonInstanceMetadata(config, projectID, key, &val, int(d.Timeout(schema.TimeoutCreate).Minutes()), failIfPresent)
 	if err != nil {
 		return err
 	}
@@ -108,7 +115,7 @@ func resourceComputeProjectMetadataItemUpdate(d *schema.ResourceData, meta inter
 		_, n := d.GetChange("value")
 		new := n.(string)
 
-		err = updateComputeCommonInstanceMetadata(config, projectID, key, &new, int(d.Timeout(schema.TimeoutUpdate).Minutes()))
+		err = updateComputeCommonInstanceMetadata(config, projectID, key, &new, int(d.Timeout(schema.TimeoutUpdate).Minutes()), overwritePresent)
 		if err != nil {
 			return err
 		}
@@ -126,7 +133,7 @@ func resourceComputeProjectMetadataItemDelete(d *schema.ResourceData, meta inter
 
 	key := d.Get("key").(string)
 
-	err = updateComputeCommonInstanceMetadata(config, projectID, key, nil, int(d.Timeout(schema.TimeoutDelete).Minutes()))
+	err = updateComputeCommonInstanceMetadata(config, projectID, key, nil, int(d.Timeout(schema.TimeoutDelete).Minutes()), overwritePresent)
 	if err != nil {
 		return err
 	}
@@ -135,7 +142,7 @@ func resourceComputeProjectMetadataItemDelete(d *schema.ResourceData, meta inter
 	return nil
 }
 
-func updateComputeCommonInstanceMetadata(config *Config, projectID string, key string, afterVal *string, timeout int) error {
+func updateComputeCommonInstanceMetadata(config *Config, projectID string, key string, afterVal *string, timeout int, failIfPresent metadataPresentBehavior) error {
 	updateMD := func() error {
 		log.Printf("[DEBUG] Loading project metadata: %s", projectID)
 		project, err := config.clientCompute.Projects.Get(projectID).Do()
@@ -153,6 +160,9 @@ func updateComputeCommonInstanceMetadata(config *Config, projectID string, key s
 				return nil
 			}
 		} else {
+			if failIfPresent {
+				return fmt.Errorf("key %q already present in metadata for project %q. Use `terraform import` to manage it with Terraform", key, projectID)
+			}
 			if afterVal != nil && *afterVal == val {
 				// Asked to set a value and it's already set - we're done.
 				return nil

diff --git a/google-beta/resource_compute_project_metadata_item_test.go b/google-beta/resource_compute_project_metadata_item_test.go
@@ -2,6 +2,7 @@ package google
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
@@ -10,10 +11,9 @@ import (
 )
 
 func TestAccComputeProjectMetadataItem_basic(t *testing.T) {
-	t.Parallel(
-	// Key must be unique to avoid concurrent tests interfering with each other
-	)
+	t.Parallel()
 
+	// Key must be unique to avoid concurrent tests interfering with each other
 	key := "myKey" + acctest.RandString(10)
 
 	resource.Test(t, resource.TestCase{
@@ -22,10 +22,7 @@ func TestAccComputeProjectMetadataItem_basic(t *testing.T) {
 		CheckDestroy: testAccCheckProjectMetadataItemDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccProjectMetadataItem_basicWithResourceName("foobar", key, "myValue"),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckProjectMetadataItem_hasMetadata(key, "myValue"),
-				),
+				Config: testAccProjectMetadataItem_basic("foobar", key, "myValue"),
 			},
 			{
 				ResourceName:      "google_compute_project_metadata_item.foobar",
@@ -37,33 +34,40 @@ func TestAccComputeProjectMetadataItem_basic(t *testing.T) {
 }
 
 func TestAccComputeProjectMetadataItem_basicMultiple(t *testing.T) {
-	t.Parallel(
+	t.Parallel()
+
 	// Generate a config of two config keys
-	)
+	key1 := "myKey" + acctest.RandString(10)
+	key2 := "myKey" + acctest.RandString(10)
+	config := testAccProjectMetadataItem_basic("foobar", key1, "myValue") +
+		testAccProjectMetadataItem_basic("foobar2", key2, "myOtherValue")
 
-	config := testAccProjectMetadataItem_basic("myKey", "myValue") +
-		testAccProjectMetadataItem_basic("myOtherKey", "myOtherValue")
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckProjectMetadataItemDestroy,
 		Steps: []resource.TestStep{
 			{
 				Config: config,
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckProjectMetadataItem_hasMetadata("myKey", "myValue"),
-					testAccCheckProjectMetadataItem_hasMetadata("myOtherKey", "myOtherValue"),
-				),
+			},
+			{
+				ResourceName:      "google_compute_project_metadata_item.foobar",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				ResourceName:      "google_compute_project_metadata_item.foobar2",
+				ImportState:       true,
+				ImportStateVerify: true,
 			},
 		},
 	})
 }
 
 func TestAccComputeProjectMetadataItem_basicWithEmptyVal(t *testing.T) {
-	t.Parallel(
-	// Key must be unique to avoid concurrent tests interfering with each other
-	)
+	t.Parallel()
 
+	// Key must be unique to avoid concurrent tests interfering with each other
 	key := "myKey" + acctest.RandString(10)
 
 	resource.Test(t, resource.TestCase{
@@ -72,10 +76,7 @@ func TestAccComputeProjectMetadataItem_basicWithEmptyVal(t *testing.T) {
 		CheckDestroy: testAccCheckProjectMetadataItemDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccProjectMetadataItem_basicWithResourceName("foobar", key, ""),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckProjectMetadataItem_hasMetadata(key, ""),
-				),
+				Config: testAccProjectMetadataItem_basic("foobar", key, ""),
 			},
 			{
 				ResourceName:      "google_compute_project_metadata_item.foobar",
@@ -87,10 +88,9 @@ func TestAccComputeProjectMetadataItem_basicWithEmptyVal(t *testing.T) {
 }
 
 func TestAccComputeProjectMetadataItem_basicUpdate(t *testing.T) {
-	t.Parallel(
-	// Key must be unique to avoid concurrent tests interfering with each other
-	)
+	t.Parallel()
 
+	// Key must be unique to avoid concurrent tests interfering with each other
 	key := "myKey" + acctest.RandString(10)
 
 	resource.Test(t, resource.TestCase{
@@ -99,41 +99,52 @@ func TestAccComputeProjectMetadataItem_basicUpdate(t *testing.T) {
 		CheckDestroy: testAccCheckProjectMetadataItemDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccProjectMetadataItem_basicWithResourceName("foobar", key, "myValue"),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckProjectMetadataItem_hasMetadata(key, "myValue"),
-				),
+				Config: testAccProjectMetadataItem_basic("foobar", key, "myValue"),
 			},
 			{
-				Config: testAccProjectMetadataItem_basicWithResourceName("foobar", key, "myUpdatedValue"),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckProjectMetadataItem_hasMetadata(key, "myUpdatedValue"),
-				),
+				ResourceName:      "google_compute_project_metadata_item.foobar",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccProjectMetadataItem_basic("foobar", key, "myUpdatedValue"),
+			},
+			{
+				ResourceName:      "google_compute_project_metadata_item.foobar",
+				ImportState:       true,
+				ImportStateVerify: true,
 			},
 		},
 	})
 }
 
-func testAccCheckProjectMetadataItem_hasMetadata(key, value string) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		config := testAccProvider.Meta().(*Config)
+func TestAccComputeProjectMetadataItem_exists(t *testing.T) {
+	t.Parallel()
 
-		project, err := config.clientCompute.Projects.Get(config.Project).Do()
-		if err != nil {
-			return err
-		}
-
-		metadata := flattenMetadata(project.CommonInstanceMetadata)
+	// Key must be unique to avoid concurrent tests interfering with each other
+	key := "myKey" + acctest.RandString(10)
+	originalConfig := testAccProjectMetadataItem_basic("foobar", key, "myValue")
 
-		val, ok := metadata[key]
-		if !ok {
-			return fmt.Errorf("Unable to find a value for key '%s'", key)
-		}
-		if val != value {
-			return fmt.Errorf("Value for key '%s' does not match. Expected '%s' but found '%s'", key, value, val)
-		}
-		return nil
-	}
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckProjectMetadataItemDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: originalConfig,
+			},
+			{
+				ResourceName:      "google_compute_project_metadata_item.foobar",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			// Add a second resource with the same key
+			{
+				Config:      originalConfig + testAccProjectMetadataItem_basic("foobar2", key, "myValue"),
+				ExpectError: regexp.MustCompile("already present in metadata for project"),
+			},
+		},
+	})
 }
 
 func testAccCheckProjectMetadataItemDestroy(s *terraform.State) error {
@@ -160,11 +171,7 @@ func testAccCheckProjectMetadataItemDestroy(s *terraform.State) error {
 	return nil
 }
 
-func testAccProjectMetadataItem_basic(key, val string) string {
-	return testAccProjectMetadataItem_basicWithResourceName(fmt.Sprintf("test_%s", acctest.RandString(10)), key, val)
-}
-
-func testAccProjectMetadataItem_basicWithResourceName(resourceName, key, val string) string {
+func testAccProjectMetadataItem_basic(resourceName, key, val string) string {
 	return fmt.Sprintf(`
 resource "google_compute_project_metadata_item" "%s" {
   key   = "%s"

diff --git a/google-beta/resource_sql_database_instance_test.go b/google-beta/resource_sql_database_instance_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/terraform"
+
 	sqladmin "google.golang.org/api/sqladmin/v1beta4"
 )