provider/google: upgrade our image resolution logic

builtin/providers/google/image.go

@@ -2,96 +2,193 @@ package google
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
+
+	"google.golang.org/api/googleapi"
 )
 
-// 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) {
+const (
+	resolveImageProjectRegex = "[-_a-zA-Z0-9]*"
+	resolveImageFamilyRegex  = "[-_a-zA-Z0-9]*"
+	resolveImageImageRegex   = "[-_a-zA-Z0-9]*"
+)
 
-	if strings.HasPrefix(name, "https://www.googleapis.com/compute/v1/") {
-		return name, nil
+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 {
-		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
-			}
+		return false, fmt.Errorf("Error checking if image %s exists: %s", name, err)
+	}
+}
 
-			// 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
-			}
+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 we match a lookup for an alternate project, then try that next.
-			// If not, we return the original 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", expected, regexType, len(got)-1, name)
+	}
+	return nil
+}
 
-			// 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 == "" {
+// 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
+// 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
+	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
+	case resolveImageProjectImage.MatchString(name): // projects/xyz/global/images/xyz
+		res := resolveImageProjectImage.FindStringSubmatch(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 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 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 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 err := sanityTestRegexMatches(1, res, "family family", name); err != nil {
+			return "", err
+		}
+		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 err := sanityTestRegexMatches(2, res, "project image shorthand", name); 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
+		}
+		if ok, err := resolveImageImageExists(c, res[1], res[2]); err != nil {
+			return "", err
+		} 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 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
+		} 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 err := sanityTestRegexMatches(1, res, "image", name); err != nil {
+			return "", err
+		}
+		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 err := sanityTestRegexMatches(1, res, "family", name); err != nil {
 			return "", err
-
-		} else {
-			return "", fmt.Errorf("Invalid image name, require URL, project/name, or just name: %s", 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 != "" {
+			// 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)
 }

builtin/providers/google/image_test.go

@@ -0,0 +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_resolving(name, fam),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeImageExists(
+						"google_compute_image.foobar", &image),
+					testAccCheckComputeImageResolution("google_compute_image.foobar"),
+				),
+			},
+		},
+	})
+}
+
+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 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)
+}

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 {