From 59e632435ebf48487eb3e96430e1406d031bbb91 Mon Sep 17 00:00:00 2001
From: Paddy <paddy@carvers.co>
Date: Mon, 23 Jan 2017 16:45:06 -0800
Subject: [PATCH 1/7] Start adding tests for image resolution.

Add tests that show what we want image input strings to resolve to, so
we can test that behaviour.
---
 builtin/providers/google/image_test.go        | 61 +++++++++++++++++++
 .../providers/google/resource_compute_disk.go |  1 +
 2 files changed, 62 insertions(+)
 create mode 100644 builtin/providers/google/image_test.go

diff --git a/builtin/providers/google/image_test.go b/builtin/providers/google/image_test.go
new file mode 100644
index 0000000000..f500c9a4ca
--- /dev/null
+++ b/builtin/providers/google/image_test.go
@@ -0,0 +1,61 @@
+package google
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/resource"
+)
+
+func TestAccComputeImage_resolveImage(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeImageDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeImage_basedondisk,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeImageExists(
+						"google_compute_image.foobar", &image),
+				),
+			},
+		},
+	})
+	images := map[string]string{
+		"family/debian-8":                                                                                     "projects/debian-cloud/global/images/family/debian-8-jessie",
+		"projects/debian-cloud/global/images/debian-8-jessie-v20170110":                                       "projects/debian-cloud/global/images/debian-8-jessie-v20170110",
+		"debian-8-jessie":                                                                                     "projects/debian-cloud/global/images/family/debian-8-jessie",
+		"debian-8-jessie-v20170110":                                                                           "projects/debian-cloud/global/images/debian-8-jessie-v20170110",
+		"https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20170110": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20170110",
+
+		// TODO(paddy): we need private images/families here to actually test this
+		"global/images/my-private-image":         "global/images/my-private-image",
+		"global/images/family/my-private-family": "global/images/family/my-private-family",
+		"my-private-image":                       "global/images/my-private-image",
+		"my-private-family":                      "global/images/family/my-private-family",
+		"my-project/my-private-image":            "projects/my-project/global/images/my-private-image",
+		"my-project/my-private-family":           "projects/my-project/global/images/family/my-private-family",
+		"insert-URL-here":                        "insert-URL-here",
+	}
+	config := &Config{
+		Credentials: credentials,
+		Project:     project,
+		Region:      region,
+	}
+
+	err := config.loadAndValidate()
+	if err != nil {
+		t.Fatalf("Error loading config: %s\n", err)
+	}
+	for input, expectation := range images {
+		result, err := resolveImage(config, input)
+		if err != nil {
+			t.Errorf("Error resolving input %s to image: %+v\n", input, err)
+			continue
+		}
+		if result != expectation {
+			t.Errorf("Expected input '%s' to resolve to '%s', it resolved to '%s' instead.\n", input, expectation, result)
+			continue
+		}
+	}
+}
diff --git a/builtin/providers/google/resource_compute_disk.go b/builtin/providers/google/resource_compute_disk.go
index c8ef8007aa..94d23d3402 100644
--- a/builtin/providers/google/resource_compute_disk.go
+++ b/builtin/providers/google/resource_compute_disk.go
@@ -112,6 +112,7 @@ func resourceComputeDiskCreate(d *schema.ResourceData, meta interface{}) error {
 		}
 
 		disk.SourceImage = imageUrl
+		log.Printf("[DEBUG] Image name resolved to: %s", imageUrl)
 	}
 
 	if v, ok := d.GetOk("type"); ok {

From 90254b945178a8bf6d048f317c250e3cffd3b64e Mon Sep 17 00:00:00 2001
From: Paddy <paddy@hashicorp.com>
Date: Thu, 23 Feb 2017 21:55:30 -0800
Subject: [PATCH 2/7] provider/google: update image resolution code.

Add tests that ensure that image syntax resolves to API input the way we
want it to.

Add a lot of different input forms for images, to more closely map to
what the API accepts, so anything that's valid input to the API should
also be valid input in a config.

Stop resolving image families to specific image URLs, allowing things
like instance templates to evolve over time as new images are pushed.
---
 builtin/providers/google/image.go      | 232 +++++++++++++++++--------
 builtin/providers/google/image_test.go | 114 ++++++++----
 2 files changed, 237 insertions(+), 109 deletions(-)

diff --git a/builtin/providers/google/image.go b/builtin/providers/google/image.go
index e4a50905ef..912821b59c 100644
--- a/builtin/providers/google/image.go
+++ b/builtin/providers/google/image.go
@@ -2,96 +2,178 @@ package google
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
+
+	"google.golang.org/api/googleapi"
+)
+
+const (
+	resolveImageProjectRegex = "[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?" // TODO(paddy): this isn't based on any documentation; we're just copying the image name restrictions. Need to follow up with @danawillow and/or @evandbrown and see if there's an actual limit to this
+	resolveImageFamilyRegex  = "[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?" // TODO(paddy): this isn't based on any documentation; we're just copying the image name restrictions. Need to follow up with @danawillow and/or @evandbrown and see if there's an actual limit to this
+	resolveImageImageRegex   = "[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?" // 1-63 characters, lowercase letters, numbers, and hyphens only, beginning and ending in a lowercase letter or number
+)
+
+var (
+	resolveImageProjectImage           = regexp.MustCompile(fmt.Sprintf("^projects/(%s)/global/images/(%s)$", resolveImageProjectRegex, resolveImageImageRegex))
+	resolveImageProjectFamily          = regexp.MustCompile(fmt.Sprintf("^projects/(%s)/global/images/family/(%s)$", resolveImageProjectRegex, resolveImageFamilyRegex))
+	resolveImageGlobalImage            = regexp.MustCompile(fmt.Sprintf("^global/images/(%s)$", resolveImageImageRegex))
+	resolveImageGlobalFamily           = regexp.MustCompile(fmt.Sprintf("^global/images/family/(%s)$", resolveImageFamilyRegex))
+	resolveImageFamilyFamily           = regexp.MustCompile(fmt.Sprintf("^family/(%s)$", resolveImageFamilyRegex))
+	resolveImageProjectImageShorthand  = regexp.MustCompile(fmt.Sprintf("^(%s)/(%s)$", resolveImageProjectRegex, resolveImageImageRegex))
+	resolveImageProjectFamilyShorthand = regexp.MustCompile(fmt.Sprintf("^(%s)/(%s)$", resolveImageProjectRegex, resolveImageFamilyRegex))
+	resolveImageFamily                 = regexp.MustCompile(fmt.Sprintf("^(%s)$", resolveImageFamilyRegex))
+	resolveImageImage                  = regexp.MustCompile(fmt.Sprintf("^(%s)$", resolveImageImageRegex))
+	resolveImageLink                   = regexp.MustCompile(fmt.Sprintf("^https://www.googleapis.com/compute/v1/projects/(%s)/global/images/(%s)", resolveImageProjectRegex, resolveImageImageRegex))
 )
 
+func resolveImageImageExists(c *Config, project, name string) (bool, error) {
+	if _, err := c.clientCompute.Images.Get(project, name).Do(); err == nil {
+		return true, nil
+	} else if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
+		return false, nil
+	} else {
+		return false, fmt.Errorf("Error checking if image %s exists: %s", name, err)
+	}
+}
+
+func resolveImageFamilyExists(c *Config, project, name string) (bool, error) {
+	if _, err := c.clientCompute.Images.GetFromFamily(project, name).Do(); err == nil {
+		return true, nil
+	} else if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
+		return false, nil
+	} else {
+		return false, fmt.Errorf("Error checking if family %s exists: %s", name, err)
+	}
+}
+
 // If the given name is a URL, return it.
 // If it is of the form project/name, search the specified project first, then
 // search image families in the specified project.
 // If it is of the form name then look in the configured project, then hosted
 // image projects, and lastly at image families in hosted image projects.
 func resolveImage(c *Config, name string) (string, error) {
-
-	if strings.HasPrefix(name, "https://www.googleapis.com/compute/v1/") {
+	// built-in projects to look for images/families containing the string
+	// on the left in
+	imageMap := map[string]string{
+		"centos":   "centos-cloud",
+		"coreos":   "coreos-cloud",
+		"debian":   "debian-cloud",
+		"opensuse": "opensuse-cloud",
+		"rhel":     "rhel-cloud",
+		"sles":     "suse-cloud",
+		"ubuntu":   "ubuntu-os-cloud",
+		"windows":  "windows-cloud",
+	}
+	var builtInProject string
+	for k, v := range imageMap {
+		if strings.Contains(name, k) {
+			builtInProject = v
+			break
+		}
+	}
+	switch {
+	case resolveImageLink.MatchString(name): // https://www.googleapis.com/compute/v1/projects/xyz/global/images/xyz
 		return name, nil
-
-	} else {
-		splitName := strings.Split(name, "/")
-		if len(splitName) == 1 {
-
-			// Must infer the project name:
-
-			// First, try the configured project for a specific image:
-			image, err := c.clientCompute.Images.Get(c.Project, name).Do()
-			if err == nil {
-				return image.SelfLink, nil
-			}
-
-			// If it doesn't exist, try to see if it works as an image family:
-			image, err = c.clientCompute.Images.GetFromFamily(c.Project, name).Do()
-			if err == nil {
-				return image.SelfLink, nil
-			}
-
-			// If we match a lookup for an alternate project, then try that next.
-			// If not, we return the original error.
-
-			// If the image name contains the left hand side, we use the project from
-			// the right hand side.
-			imageMap := map[string]string{
-				"centos":   "centos-cloud",
-				"coreos":   "coreos-cloud",
-				"debian":   "debian-cloud",
-				"opensuse": "opensuse-cloud",
-				"rhel":     "rhel-cloud",
-				"sles":     "suse-cloud",
-				"ubuntu":   "ubuntu-os-cloud",
-				"windows":  "windows-cloud",
-			}
-			var project string
-			for k, v := range imageMap {
-				if strings.Contains(name, k) {
-					project = v
-					break
-				}
-			}
-			if project == "" {
+	case resolveImageProjectImage.MatchString(name): // projects/xyz/global/images/xyz
+		res := resolveImageProjectImage.FindStringSubmatch(name)
+		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d project image regex matches, got %d for %s", 2, len(res)-1, name)
+		}
+		return fmt.Sprintf("projects/%s/global/images/%s", res[1], res[2]), nil
+	case resolveImageProjectFamily.MatchString(name): // projects/xyz/global/images/family/xyz
+		res := resolveImageProjectFamily.FindStringSubmatch(name)
+		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d project family regex matches, got %d for %s", 2, len(res)-1, name)
+		}
+		return fmt.Sprintf("projects/%s/global/images/family/%s", res[1], res[2]), nil
+	case resolveImageGlobalImage.MatchString(name): // global/images/xyz
+		res := resolveImageGlobalImage.FindStringSubmatch(name)
+		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d global image regex matches, got %d for %s", 1, len(res)-1, name)
+		}
+		return fmt.Sprintf("global/images/%s", res[1]), nil
+	case resolveImageGlobalFamily.MatchString(name): // global/images/family/xyz
+		res := resolveImageGlobalFamily.FindStringSubmatch(name)
+		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d global family regex matches, got %d for %s", 1, len(res)-1, name)
+		}
+		return fmt.Sprintf("global/images/family/%s", res[1]), nil
+	case resolveImageFamilyFamily.MatchString(name): // family/xyz
+		res := resolveImageFamilyFamily.FindStringSubmatch(name)
+		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d family family regex matches, got %d for %s", 1, len(res)-1, name)
+		}
+		if ok, err := resolveImageFamilyExists(c, c.Project, res[1]); err != nil {
+			return "", err
+		} else if ok {
+			return fmt.Sprintf("global/images/family/%s", res[1]), nil
+		}
+		if builtInProject != "" {
+			if ok, err := resolveImageFamilyExists(c, builtInProject, res[1]); err != nil {
 				return "", err
+			} else if ok {
+				return fmt.Sprintf("projects/%s/global/images/family/%s", builtInProject, res[1]), nil
 			}
-
-			// There was a match, but the image still may not exist, so check it:
-			image, err = c.clientCompute.Images.Get(project, name).Do()
-			if err == nil {
-				return image.SelfLink, nil
-			}
-
-			// If it doesn't exist, try to see if it works as an image family:
-			image, err = c.clientCompute.Images.GetFromFamily(project, name).Do()
-			if err == nil {
-				return image.SelfLink, nil
-			}
-
+		}
+	case resolveImageProjectImageShorthand.MatchString(name): // xyz/xyz
+		res := resolveImageProjectImageShorthand.FindStringSubmatch(name)
+		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d project image shorthand regex matches, got %d for %s", 2, len(res)-1, name)
+		}
+		if ok, err := resolveImageImageExists(c, res[1], res[2]); err != nil {
 			return "", err
-
-		} else if len(splitName) == 2 {
-
-			// Check if image exists in the specified project:
-			image, err := c.clientCompute.Images.Get(splitName[0], splitName[1]).Do()
-			if err == nil {
-				return image.SelfLink, nil
-			}
-
-			// If it doesn't, check if it exists as an image family:
-			image, err = c.clientCompute.Images.GetFromFamily(splitName[0], splitName[1]).Do()
-			if err == nil {
-				return image.SelfLink, nil
+		} else if ok {
+			return fmt.Sprintf("projects/%s/global/images/%s", res[1], res[2]), nil
+		}
+		fallthrough // check if it's a family
+	case resolveImageProjectFamilyShorthand.MatchString(name): // xyz/xyz
+		res := resolveImageProjectFamilyShorthand.FindStringSubmatch(name)
+		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d project family shorthand regex matches, got %d for %s", 2, len(res)-1, name)
+		}
+		if ok, err := resolveImageFamilyExists(c, res[1], res[2]); err != nil {
+			return "", err
+		} else if ok {
+			return fmt.Sprintf("projects/%s/global/images/family/%s", res[1], res[2]), nil
+		}
+	case resolveImageImage.MatchString(name): // xyz
+		res := resolveImageImage.FindStringSubmatch(name)
+		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d image regex matches, got %d for %s", 1, len(res)-1, name)
+		}
+		if ok, err := resolveImageImageExists(c, c.Project, res[1]); err != nil {
+			return "", err
+		} else if ok {
+			return fmt.Sprintf("global/images/%s", res[1]), nil
+		}
+		if builtInProject != "" {
+			// check the images GCP provides
+			if ok, err := resolveImageImageExists(c, builtInProject, res[1]); err != nil {
+				return "", err
+			} else if ok {
+				return fmt.Sprintf("projects/%s/global/images/%s", builtInProject, res[1]), nil
 			}
-
+		}
+		fallthrough // check if the name is a family, instead of an image
+	case resolveImageFamily.MatchString(name): // xyz
+		res := resolveImageFamily.FindStringSubmatch(name)
+		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
+			return "", fmt.Errorf("Expected %d family regex matches, got %d for %s", 1, len(res)-1, name)
+		}
+		if ok, err := resolveImageFamilyExists(c, c.Project, res[1]); err != nil {
 			return "", err
-
-		} else {
-			return "", fmt.Errorf("Invalid image name, require URL, project/name, or just name: %s", name)
+		} else if ok {
+			return fmt.Sprintf("global/images/family/%s", res[1]), nil
+		}
+		if builtInProject != "" {
+			// check the families GCP provides
+			if ok, err := resolveImageFamilyExists(c, builtInProject, res[1]); err != nil {
+				return "", err
+			} else if ok {
+				return fmt.Sprintf("projects/%s/global/images/family/%s", builtInProject, res[1]), nil
+			}
 		}
 	}
-
+	return "", fmt.Errorf("Could not find image or family %s", name)
 }
diff --git a/builtin/providers/google/image_test.go b/builtin/providers/google/image_test.go
index f500c9a4ca..e0f56518af 100644
--- a/builtin/providers/google/image_test.go
+++ b/builtin/providers/google/image_test.go
@@ -1,61 +1,107 @@
 package google
 
 import (
+	"fmt"
 	"testing"
 
+	compute "google.golang.org/api/compute/v1"
+
+	"github.com/hashicorp/terraform/helper/acctest"
 	"github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/terraform"
 )
 
 func TestAccComputeImage_resolveImage(t *testing.T) {
+	var image compute.Image
+	rand := acctest.RandString(10)
+	name := fmt.Sprintf("test-image-%s", rand)
+	fam := fmt.Sprintf("test-image-family-%s", rand)
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckComputeImageDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccComputeImage_basedondisk,
+				Config: testAccComputeImage_resolving(name, fam),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeImageExists(
 						"google_compute_image.foobar", &image),
+					testAccCheckComputeImageResolution("google_compute_image.foobar"),
 				),
 			},
 		},
 	})
-	images := map[string]string{
-		"family/debian-8":                                                                                     "projects/debian-cloud/global/images/family/debian-8-jessie",
-		"projects/debian-cloud/global/images/debian-8-jessie-v20170110":                                       "projects/debian-cloud/global/images/debian-8-jessie-v20170110",
-		"debian-8-jessie":                                                                                     "projects/debian-cloud/global/images/family/debian-8-jessie",
-		"debian-8-jessie-v20170110":                                                                           "projects/debian-cloud/global/images/debian-8-jessie-v20170110",
-		"https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20170110": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20170110",
-
-		// TODO(paddy): we need private images/families here to actually test this
-		"global/images/my-private-image":         "global/images/my-private-image",
-		"global/images/family/my-private-family": "global/images/family/my-private-family",
-		"my-private-image":                       "global/images/my-private-image",
-		"my-private-family":                      "global/images/family/my-private-family",
-		"my-project/my-private-image":            "projects/my-project/global/images/my-private-image",
-		"my-project/my-private-family":           "projects/my-project/global/images/family/my-private-family",
-		"insert-URL-here":                        "insert-URL-here",
-	}
-	config := &Config{
-		Credentials: credentials,
-		Project:     project,
-		Region:      region,
-	}
+}
 
-	err := config.loadAndValidate()
-	if err != nil {
-		t.Fatalf("Error loading config: %s\n", err)
-	}
-	for input, expectation := range images {
-		result, err := resolveImage(config, input)
-		if err != nil {
-			t.Errorf("Error resolving input %s to image: %+v\n", input, err)
-			continue
+func testAccCheckComputeImageResolution(n string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		config := testAccProvider.Meta().(*Config)
+		project := config.Project
+
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("Resource not found: %s", n)
+		}
+
+		if rs.Primary.ID == "" {
+			return fmt.Errorf("No ID is set")
+		}
+		if rs.Primary.Attributes["name"] == "" {
+			return fmt.Errorf("No image name is set")
+		}
+		if rs.Primary.Attributes["family"] == "" {
+			return fmt.Errorf("No image family is set")
 		}
-		if result != expectation {
-			t.Errorf("Expected input '%s' to resolve to '%s', it resolved to '%s' instead.\n", input, expectation, result)
-			continue
+		if rs.Primary.Attributes["self_link"] == "" {
+			return fmt.Errorf("No self_link is set")
 		}
+
+		name := rs.Primary.Attributes["name"]
+		family := rs.Primary.Attributes["family"]
+		link := rs.Primary.Attributes["self_link"]
+
+		images := map[string]string{
+			"family/debian-8":                                               "projects/debian-cloud/global/images/family/debian-8",
+			"projects/debian-cloud/global/images/debian-8-jessie-v20170110": "projects/debian-cloud/global/images/debian-8-jessie-v20170110",
+			"debian-8":                                                                                            "projects/debian-cloud/global/images/family/debian-8",
+			"debian-8-jessie-v20170110":                                                                           "projects/debian-cloud/global/images/debian-8-jessie-v20170110",
+			"https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20170110": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20170110",
+
+			"global/images/" + name:          "global/images/" + name,
+			"global/images/family/" + family: "global/images/family/" + family,
+			name:                   "global/images/" + name,
+			family:                 "global/images/family/" + family,
+			"family/" + family:     "global/images/family/" + family,
+			project + "/" + name:   "projects/" + project + "/global/images/" + name,
+			project + "/" + family: "projects/" + project + "/global/images/family/" + family,
+			link: link,
+		}
+
+		for input, expectation := range images {
+			result, err := resolveImage(config, input)
+			if err != nil {
+				return fmt.Errorf("Error resolving input %s to image: %+v\n", input, err)
+			}
+			if result != expectation {
+				return fmt.Errorf("Expected input '%s' to resolve to '%s', it resolved to '%s' instead.\n", input, expectation, result)
+			}
+		}
+		return nil
 	}
 }
+
+func testAccComputeImage_resolving(name, family string) string {
+	return fmt.Sprintf(`
+resource "google_compute_disk" "foobar" {
+	name = "%s"
+	zone = "us-central1-a"
+	image = "debian-8-jessie-v20160803"
+}
+resource "google_compute_image" "foobar" {
+	name = "%s"
+	family = "%s"
+	source_disk = "${google_compute_disk.foobar.self_link}"
+}
+`, name, name, family)
+}

From 6868ba72c7107651aa21c30db865e798a1babcb2 Mon Sep 17 00:00:00 2001
From: Paddy <paddy@hashicorp.com>
Date: Thu, 23 Feb 2017 22:09:07 -0800
Subject: [PATCH 3/7] Update the docs for resolveImage.

Update the explanation of the logic being followed in resolveImage.
---
 builtin/providers/google/image.go | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/builtin/providers/google/image.go b/builtin/providers/google/image.go
index 912821b59c..e772d95e43 100644
--- a/builtin/providers/google/image.go
+++ b/builtin/providers/google/image.go
@@ -48,10 +48,18 @@ func resolveImageFamilyExists(c *Config, project, name string) (bool, error) {
 }
 
 // If the given name is a URL, return it.
-// If it is of the form project/name, search the specified project first, then
-// search image families in the specified project.
-// If it is of the form name then look in the configured project, then hosted
-// image projects, and lastly at image families in hosted image projects.
+// If it's in the form projects/{project}/global/images/{image}, return it
+// If it's in the form projects/{project}/global/images/family/{family}, return it
+// If it's in the form global/images/{image}, return it
+// If it's in the form global/images/family/{family}, return it
+// If it's in the form family/{family}, check if it's a family in the current project. If it is, return it as global/images/family/{family}.
+//    If not, check if it could be a GCP-provided family, and if it exists. If it does, return it as projects/{project}/global/images/family/{family}.
+// If it's in the form {project}/{family-or-image}, check if it's an image in the named project. If it is, return it as projects/{project}/global/images/{image}.
+//    If not, check if it's a family in the named project. If it is, return it as projects/{project}/global/images/family/{family}.
+// If it's in the form {family-or-image}, check if it's an image in the current project. If it is, return it as global/images/{image}.
+//    If not, check if it could be a GCP-provided image, and if it exists. If it does, return it as projects/{project}/global/images/{image}.
+//    If not, check if it's a family in the current project. If it is, return it as global/images/family/{family}.
+//    If not, check if it could be a GCP-provided family, and if it exists. If it does, return it as projects/{project}/global/images/family/{family}
 func resolveImage(c *Config, name string) (string, error) {
 	// built-in projects to look for images/families containing the string
 	// on the left in

From 4c41729f98d840e497b33c9f3728c7458ebca57b Mon Sep 17 00:00:00 2001
From: Paddy <paddy@hashicorp.com>
Date: Mon, 13 Mar 2017 16:39:42 -0700
Subject: [PATCH 4/7] Update with @danawillow's feedback.

* Make our regexes more permissive (though still separated out for
  readability, despite being identical)
* Add a helper that will improve readability while sanity testing our
  regex results.
---
 builtin/providers/google/image.go | 49 ++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/builtin/providers/google/image.go b/builtin/providers/google/image.go
index e772d95e43..500b601fe8 100644
--- a/builtin/providers/google/image.go
+++ b/builtin/providers/google/image.go
@@ -9,9 +9,9 @@ import (
 )
 
 const (
-	resolveImageProjectRegex = "[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?" // TODO(paddy): this isn't based on any documentation; we're just copying the image name restrictions. Need to follow up with @danawillow and/or @evandbrown and see if there's an actual limit to this
-	resolveImageFamilyRegex  = "[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?" // TODO(paddy): this isn't based on any documentation; we're just copying the image name restrictions. Need to follow up with @danawillow and/or @evandbrown and see if there's an actual limit to this
-	resolveImageImageRegex   = "[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?" // 1-63 characters, lowercase letters, numbers, and hyphens only, beginning and ending in a lowercase letter or number
+	resolveImageProjectRegex = "[-_a-zA-Z0-9]*"
+	resolveImageFamilyRegex  = "[-_a-zA-Z0-9]*"
+	resolveImageImageRegex   = "[-_a-zA-Z0-9]*"
 )
 
 var (
@@ -47,6 +47,13 @@ func resolveImageFamilyExists(c *Config, project, name string) (bool, error) {
 	}
 }
 
+func sanityTestRegexMatches(expected int, got []string, regexType, name string) error {
+	if len(got)-1 != expected { // subtract one, index zero is the entire matched expression
+		return fmt.Errorf("Expected %d %s regex matches, got %d for %s", 2, regexType, len(got)-1, name)
+	}
+	return nil
+}
+
 // If the given name is a URL, return it.
 // If it's in the form projects/{project}/global/images/{image}, return it
 // If it's in the form projects/{project}/global/images/family/{family}, return it
@@ -85,32 +92,32 @@ func resolveImage(c *Config, name string) (string, error) {
 		return name, nil
 	case resolveImageProjectImage.MatchString(name): // projects/xyz/global/images/xyz
 		res := resolveImageProjectImage.FindStringSubmatch(name)
-		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d project image regex matches, got %d for %s", 2, len(res)-1, name)
+		if err := sanityTestRegexMatches(2, res, "project image", name); err != nil {
+			return "", err
 		}
 		return fmt.Sprintf("projects/%s/global/images/%s", res[1], res[2]), nil
 	case resolveImageProjectFamily.MatchString(name): // projects/xyz/global/images/family/xyz
 		res := resolveImageProjectFamily.FindStringSubmatch(name)
-		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d project family regex matches, got %d for %s", 2, len(res)-1, name)
+		if err := sanityTestRegexMatches(2, res, "project family", name); err != nil {
+			return "", err
 		}
 		return fmt.Sprintf("projects/%s/global/images/family/%s", res[1], res[2]), nil
 	case resolveImageGlobalImage.MatchString(name): // global/images/xyz
 		res := resolveImageGlobalImage.FindStringSubmatch(name)
-		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d global image regex matches, got %d for %s", 1, len(res)-1, name)
+		if err := sanityTestRegexMatches(1, res, "global image", name); err != nil {
+			return "", err
 		}
 		return fmt.Sprintf("global/images/%s", res[1]), nil
 	case resolveImageGlobalFamily.MatchString(name): // global/images/family/xyz
 		res := resolveImageGlobalFamily.FindStringSubmatch(name)
-		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d global family regex matches, got %d for %s", 1, len(res)-1, name)
+		if err := sanityTestRegexMatches(1, res, "global family", name); err != nil {
+			return "", err
 		}
 		return fmt.Sprintf("global/images/family/%s", res[1]), nil
 	case resolveImageFamilyFamily.MatchString(name): // family/xyz
 		res := resolveImageFamilyFamily.FindStringSubmatch(name)
-		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d family family regex matches, got %d for %s", 1, len(res)-1, name)
+		if err := sanityTestRegexMatches(1, res, "family family", name); err != nil {
+			return "", err
 		}
 		if ok, err := resolveImageFamilyExists(c, c.Project, res[1]); err != nil {
 			return "", err
@@ -126,8 +133,8 @@ func resolveImage(c *Config, name string) (string, error) {
 		}
 	case resolveImageProjectImageShorthand.MatchString(name): // xyz/xyz
 		res := resolveImageProjectImageShorthand.FindStringSubmatch(name)
-		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d project image shorthand regex matches, got %d for %s", 2, len(res)-1, name)
+		if err := sanityTestRegexMatches(2, res, "project image shorthand", name); err != nil {
+			return "", err
 		}
 		if ok, err := resolveImageImageExists(c, res[1], res[2]); err != nil {
 			return "", err
@@ -137,8 +144,8 @@ func resolveImage(c *Config, name string) (string, error) {
 		fallthrough // check if it's a family
 	case resolveImageProjectFamilyShorthand.MatchString(name): // xyz/xyz
 		res := resolveImageProjectFamilyShorthand.FindStringSubmatch(name)
-		if len(res)-1 != 2 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d project family shorthand regex matches, got %d for %s", 2, len(res)-1, name)
+		if err := sanityTestRegexMatches(2, res, "project family shorthand", name); err != nil {
+			return "", err
 		}
 		if ok, err := resolveImageFamilyExists(c, res[1], res[2]); err != nil {
 			return "", err
@@ -147,8 +154,8 @@ func resolveImage(c *Config, name string) (string, error) {
 		}
 	case resolveImageImage.MatchString(name): // xyz
 		res := resolveImageImage.FindStringSubmatch(name)
-		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d image regex matches, got %d for %s", 1, len(res)-1, name)
+		if err := sanityTestRegexMatches(1, res, "image", name); err != nil {
+			return "", err
 		}
 		if ok, err := resolveImageImageExists(c, c.Project, res[1]); err != nil {
 			return "", err
@@ -166,8 +173,8 @@ func resolveImage(c *Config, name string) (string, error) {
 		fallthrough // check if the name is a family, instead of an image
 	case resolveImageFamily.MatchString(name): // xyz
 		res := resolveImageFamily.FindStringSubmatch(name)
-		if len(res)-1 != 1 { // subtract one, index zero is the entire matched expression
-			return "", fmt.Errorf("Expected %d family regex matches, got %d for %s", 1, len(res)-1, name)
+		if err := sanityTestRegexMatches(1, res, "family", name); err != nil {
+			return "", err
 		}
 		if ok, err := resolveImageFamilyExists(c, c.Project, res[1]); err != nil {
 			return "", err

From 72bfc435adaf2902fbd44d6cb421ab51717bf5f3 Mon Sep 17 00:00:00 2001
From: Paddy <paddy@hashicorp.com>
Date: Mon, 13 Mar 2017 22:04:08 -0700
Subject: [PATCH 5/7] Update typo.

We never updated the error to use the expectation, not hardcode it to 2.
---
 builtin/providers/google/image.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/providers/google/image.go b/builtin/providers/google/image.go
index 500b601fe8..d21210d991 100644
--- a/builtin/providers/google/image.go
+++ b/builtin/providers/google/image.go
@@ -49,7 +49,7 @@ func resolveImageFamilyExists(c *Config, project, name string) (bool, error) {
 
 func sanityTestRegexMatches(expected int, got []string, regexType, name string) error {
 	if len(got)-1 != expected { // subtract one, index zero is the entire matched expression
-		return fmt.Errorf("Expected %d %s regex matches, got %d for %s", 2, regexType, len(got)-1, name)
+		return fmt.Errorf("Expected %d %s regex matches, got %d for %s", expected, regexType, len(got)-1, name)
 	}
 	return nil
 }

From ec93fb5ebb661ab05bff11749da7c576a29226f6 Mon Sep 17 00:00:00 2001
From: Paddy <paddy@hashicorp.com>
Date: Mon, 13 Mar 2017 22:15:37 -0700
Subject: [PATCH 6/7] Update the documentation.

Document the acceptable inputs everywhere that uses the function.
---
 .../providers/google/r/compute_disk.html.markdown     |  8 +++++---
 .../providers/google/r/compute_instance.html.markdown | 11 +++++------
 .../google/r/compute_instance_template.html.markdown  |  8 ++++++--
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/website/source/docs/providers/google/r/compute_disk.html.markdown b/website/source/docs/providers/google/r/compute_disk.html.markdown
index faeb20cc7a..8f2558657c 100644
--- a/website/source/docs/providers/google/r/compute_disk.html.markdown
+++ b/website/source/docs/providers/google/r/compute_disk.html.markdown
@@ -37,9 +37,11 @@ The following arguments are supported:
     encoded in [RFC 4648 base64](https://tools.ietf.org/html/rfc4648#section-4)
     to encrypt this disk.
 
-* `image` - (Optional) The image from which to initialize this disk. Either the
-    full URL, a contraction of the form "project/name", or just a name (in which
-    case the current project is used).
+* `image` - (Optional) The image from which to initialize this disk. This can be
+    one of: the image self_link, projects/{project}/global/images/{image},
+    projects/{project}/global/images/family/{family}, global/images/{image},
+    global/images/family/{family}, family/{family}, {project}/{family},
+    {project}/{image}, {family}, or {image}.
 
 * `project` - (Optional) The project in which the resource belongs. If it
     is not provided, the provider project is used.
diff --git a/website/source/docs/providers/google/r/compute_instance.html.markdown b/website/source/docs/providers/google/r/compute_instance.html.markdown
index 174feee9d9..c2bb65f416 100644
--- a/website/source/docs/providers/google/r/compute_instance.html.markdown
+++ b/website/source/docs/providers/google/r/compute_instance.html.markdown
@@ -117,12 +117,11 @@ the type is "local-ssd", in which case scratch must be true).
 * `disk` - The name of the existing disk (such as those managed by
     `google_compute_disk`) to attach.
 
-* `image` - The image from which to initialize this
-    disk. Either the full URL, a contraction of the form "project/name", the
-    name of a Google-supported
-    [image family](https://cloud.google.com/compute/docs/images#image_families),
-    or simple the name of an image or image family (in which case the current
-    project is used).
+* `image` - The image from which to initialize this disk. This can be
+    one of: the image self_link, projects/{project}/global/images/{image},
+    projects/{project}/global/images/family/{family}, global/images/{image},
+    global/images/family/{family}, family/{family}, {project}/{family},
+    {project}/{image}, {family}, or {image}.
 
 * `auto_delete` - (Optional) Whether or not the disk should be auto-deleted.
     This defaults to true. Leave true for local SSDs.
diff --git a/website/source/docs/providers/google/r/compute_instance_template.html.markdown b/website/source/docs/providers/google/r/compute_instance_template.html.markdown
index 0774f0e25c..86290107c6 100644
--- a/website/source/docs/providers/google/r/compute_instance_template.html.markdown
+++ b/website/source/docs/providers/google/r/compute_instance_template.html.markdown
@@ -176,8 +176,12 @@ The `disk` block supports:
 * `disk_name` - (Optional) Name of the disk. When not provided, this defaults
     to the name of the instance.
 
-* `source_image` - (Required if source not set) The name of the image to base
-    this disk off of. Accepts same arguments as a [google_compute_instance image](https://www.terraform.io/docs/providers/google/r/compute_instance.html#image).
+* `source_image` - (Required if source not set) The image from which to
+    initialize this disk. This can be one of: the image self_link,
+    projects/{project}/global/images/{image},
+    projects/{project}/global/images/family/{family}, global/images/{image},
+    global/images/family/{family}, family/{family}, {project}/{family},
+    {project}/{image}, {family}, or {image}.
 
 * `interface` - (Optional) Specifies the disk interface to use for attaching
     this disk.

From 32c1bb4067f1470b7aba4d5dc2eff98b6b1b23b8 Mon Sep 17 00:00:00 2001
From: Paddy <paddy@hashicorp.com>
Date: Mon, 13 Mar 2017 23:39:07 -0700
Subject: [PATCH 7/7] Make all image string templates code formatted.

Update our docs to code format all the image template strings that a
disk can use.
---
 .../docs/providers/google/r/compute_disk.html.markdown |  8 ++++----
 .../providers/google/r/compute_instance.html.markdown  |  8 ++++----
 .../google/r/compute_instance_template.html.markdown   | 10 +++++-----
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/website/source/docs/providers/google/r/compute_disk.html.markdown b/website/source/docs/providers/google/r/compute_disk.html.markdown
index 8f2558657c..cd7d1a4a93 100644
--- a/website/source/docs/providers/google/r/compute_disk.html.markdown
+++ b/website/source/docs/providers/google/r/compute_disk.html.markdown
@@ -38,10 +38,10 @@ The following arguments are supported:
     to encrypt this disk.
 
 * `image` - (Optional) The image from which to initialize this disk. This can be
-    one of: the image self_link, projects/{project}/global/images/{image},
-    projects/{project}/global/images/family/{family}, global/images/{image},
-    global/images/family/{family}, family/{family}, {project}/{family},
-    {project}/{image}, {family}, or {image}.
+    one of: the image's `self_link`, `projects/{project}/global/images/{image}`,
+    `projects/{project}/global/images/family/{family}`, `global/images/{image}`,
+    `global/images/family/{family}`, `family/{family}`, `{project}/{family}`,
+    `{project}/{image}`, `{family}`, or `{image}`.
 
 * `project` - (Optional) The project in which the resource belongs. If it
     is not provided, the provider project is used.
diff --git a/website/source/docs/providers/google/r/compute_instance.html.markdown b/website/source/docs/providers/google/r/compute_instance.html.markdown
index c2bb65f416..3481d4009b 100644
--- a/website/source/docs/providers/google/r/compute_instance.html.markdown
+++ b/website/source/docs/providers/google/r/compute_instance.html.markdown
@@ -118,10 +118,10 @@ the type is "local-ssd", in which case scratch must be true).
     `google_compute_disk`) to attach.
 
 * `image` - The image from which to initialize this disk. This can be
-    one of: the image self_link, projects/{project}/global/images/{image},
-    projects/{project}/global/images/family/{family}, global/images/{image},
-    global/images/family/{family}, family/{family}, {project}/{family},
-    {project}/{image}, {family}, or {image}.
+    one of: the image's `self_link`, `projects/{project}/global/images/{image}`,
+    `projects/{project}/global/images/family/{family}`, `global/images/{image}`,
+    `global/images/family/{family}`, `family/{family}`, `{project}/{family}`,
+    `{project}/{image}`, `{family}`, or `{image}`.
 
 * `auto_delete` - (Optional) Whether or not the disk should be auto-deleted.
     This defaults to true. Leave true for local SSDs.
diff --git a/website/source/docs/providers/google/r/compute_instance_template.html.markdown b/website/source/docs/providers/google/r/compute_instance_template.html.markdown
index 86290107c6..5670409b7e 100644
--- a/website/source/docs/providers/google/r/compute_instance_template.html.markdown
+++ b/website/source/docs/providers/google/r/compute_instance_template.html.markdown
@@ -177,11 +177,11 @@ The `disk` block supports:
     to the name of the instance.
 
 * `source_image` - (Required if source not set) The image from which to
-    initialize this disk. This can be one of: the image self_link,
-    projects/{project}/global/images/{image},
-    projects/{project}/global/images/family/{family}, global/images/{image},
-    global/images/family/{family}, family/{family}, {project}/{family},
-    {project}/{image}, {family}, or {image}.
+    initialize this disk. This can be one of: the image's `self_link`,
+    `projects/{project}/global/images/{image}`,
+    `projects/{project}/global/images/family/{family}`, `global/images/{image}`,
+    `global/images/family/{family}`, `family/{family}`, `{project}/{family}`,
+    `{project}/{image}`, `{family}`, or `{image}`.
 
 * `interface` - (Optional) Specifies the disk interface to use for attaching
     this disk.