Diff suppress unconventional public image family naming pattern (#1024)

* DiffSuppress for all public images family names

* Update documentation

* TF_ACC guard

* diskImageEquals should use ==

google/image.go

@@ -24,19 +24,23 @@ var (
 	resolveImageFamily                 = regexp.MustCompile(fmt.Sprintf("^(%s)$", resolveImageFamilyRegex))
 	resolveImageImage                  = regexp.MustCompile(fmt.Sprintf("^(%s)$", resolveImageImageRegex))
 	resolveImageLink                   = regexp.MustCompile(fmt.Sprintf("^https://www.googleapis.com/compute/[a-z0-9]+/projects/(%s)/global/images/(%s)", ProjectRegex, resolveImageImageRegex))
+
+	windowsSqlImage         = regexp.MustCompile("^sql-([0-9]{4})-([a-z]+)-windows-([0-9]{4})(?:-r([0-9]+))?-dc-v[0-9]+$")
+	canonicalUbuntuLtsImage = regexp.MustCompile("^ubuntu-([0-9]+)-")
 )
 
 // built-in projects to look for images/families containing the string
 // on the left in
 var 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",
+	"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",
+	"windows-sql": "windows-sql-cloud",
 }
 
 func resolveImageImageExists(c *Config, project, name string) (bool, error) {

google/resource_compute_disk.go

@@ -426,7 +426,7 @@ func diskImageDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
 		newProject := matches[1]
 		newFamilyName := matches[2]
 
-		return diskImageProjectNameEquals(oldProject, newProject) && strings.Contains(oldName, newFamilyName)
+		return diskImageProjectNameEquals(oldProject, newProject) && diskImageFamilyEquals(oldName, newFamilyName)
 	}
 
 	// Partial or full self link image
@@ -436,7 +436,7 @@ func diskImageDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
 		newProject := matches[1]
 		newImageName := matches[2]
 
-		return diskImageProjectNameEquals(oldProject, newProject) && strings.Contains(oldName, newImageName)
+		return diskImageProjectNameEquals(oldProject, newProject) && diskImageEquals(oldName, newImageName)
 	}
 
 	// Partial link without project family
@@ -445,7 +445,7 @@ func diskImageDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
 		matches := resolveImageGlobalFamily.FindStringSubmatch(new)
 		familyName := matches[1]
 
-		return strings.Contains(oldName, familyName)
+		return diskImageFamilyEquals(oldName, familyName)
 	}
 
 	// Partial link without project image
@@ -454,7 +454,7 @@ func diskImageDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
 		matches := resolveImageGlobalImage.FindStringSubmatch(new)
 		imageName := matches[1]
 
-		return strings.Contains(oldName, imageName)
+		return diskImageEquals(oldName, imageName)
 	}
 
 	// Family shorthand
@@ -463,7 +463,7 @@ func diskImageDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
 		matches := resolveImageFamilyFamily.FindStringSubmatch(new)
 		familyName := matches[1]
 
-		return strings.Contains(oldName, familyName)
+		return diskImageFamilyEquals(oldName, familyName)
 	}
 
 	// Shorthand for image or family
@@ -473,11 +473,12 @@ func diskImageDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
 		newProject := matches[1]
 		newName := matches[2]
 
-		return diskImageProjectNameEquals(oldProject, newProject) && strings.Contains(oldName, newName)
+		return diskImageProjectNameEquals(oldProject, newProject) &&
+			(diskImageEquals(oldName, newName) || diskImageFamilyEquals(oldName, newName))
 	}
 
 	// Image or family only
-	if strings.Contains(oldName, new) {
+	if diskImageEquals(oldName, new) || diskImageFamilyEquals(oldName, new) {
 		// Value is "{image-name}" or "{family-name}"
 		return true
 	}
@@ -495,3 +496,91 @@ func diskImageProjectNameEquals(project1, project2 string) bool {
 
 	return project1 == project2
 }
+
+func diskImageEquals(oldImageName, newImageName string) bool {
+	return oldImageName == newImageName
+}
+
+func diskImageFamilyEquals(imageName, familyName string) bool {
+	// Handles the case when the image name includes the family name
+	// e.g. image name: debian-9-drawfork-v20180109, family name: debian-9
+	if strings.Contains(imageName, familyName) {
+		return true
+	}
+
+	if suppressCanonicalFamilyDiff(imageName, familyName) {
+		return true
+	}
+
+	if suppressWindowsSqlFamilyDiff(imageName, familyName) {
+		return true
+	}
+
+	if suppressWindowsFamilyDiff(imageName, familyName) {
+		return true
+	}
+
+	return false
+}
+
+// e.g. image: ubuntu-1404-trusty-v20180122, family: ubuntu-1404-lts
+func suppressCanonicalFamilyDiff(imageName, familyName string) bool {
+	parts := canonicalUbuntuLtsImage.FindStringSubmatch(imageName)
+	if len(parts) == 2 {
+		f := fmt.Sprintf("ubuntu-%s-lts", parts[1])
+		if f == familyName {
+			return true
+		}
+	}
+
+	return false
+}
+
+// e.g. image: sql-2017-standard-windows-2016-dc-v20180109, family: sql-std-2017-win-2016
+// e.g. image: sql-2017-express-windows-2012-r2-dc-v20180109, family: sql-exp-2017-win-2012-r2
+func suppressWindowsSqlFamilyDiff(imageName, familyName string) bool {
+	parts := windowsSqlImage.FindStringSubmatch(imageName)
+	if len(parts) == 5 {
+		edition := parts[2] // enterprise, standard or web.
+		sqlVersion := parts[1]
+		windowsVersion := parts[3]
+
+		// Translate edition
+		switch edition {
+		case "enterprise":
+			edition = "ent"
+		case "standard":
+			edition = "std"
+		case "express":
+			edition = "exp"
+		}
+
+		var f string
+		if revision := parts[4]; revision != "" {
+			// With revision
+			f = fmt.Sprintf("sql-%s-%s-win-%s-r%s", edition, sqlVersion, windowsVersion, revision)
+		} else {
+			// No revision
+			f = fmt.Sprintf("sql-%s-%s-win-%s", edition, sqlVersion, windowsVersion)
+		}
+
+		if f == familyName {
+			return true
+		}
+	}
+
+	return false
+}
+
+// e.g. image: windows-server-1709-dc-core-v20180109, family: windows-1709-core
+// e.g. image: windows-server-1709-dc-core-for-containers-v20180109, family: "windows-1709-core-for-containers
+func suppressWindowsFamilyDiff(imageName, familyName string) bool {
+	updatedFamilyString := strings.Replace(familyName, "windows-", "windows-server-", 1)
+	updatedFamilyString = strings.Replace(updatedFamilyString, "-core", "-dc-core", 1)
+
+	if strings.Contains(imageName, updatedFamilyString) {
+		return true
+	}
+
+	return false
+}

google/resource_compute_disk_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
 	"google.golang.org/api/compute/v1"
+	"os"
 )
 
 func TestDiskImageDiffSuppress(t *testing.T) {
@@ -86,11 +87,21 @@ func TestDiskImageDiffSuppress(t *testing.T) {
 			New:                "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/family/debian-8",
 			ExpectDiffSuppress: true,
 		},
+		"matching unconventional image family self link": {
+			Old:                "https://www.googleapis.com/compute/v1/projects/ubuntu-os-cloud/global/images/ubuntu-1404-trusty-v20180122",
+			New:                "https://www.googleapis.com/compute/v1/projects/projects/ubuntu-os-cloud/global/images/family/ubuntu-1404-lts",
+			ExpectDiffSuppress: true,
+		},
 		"matching image family partial self link": {
 			Old:                "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20171213",
 			New:                "projects/debian-cloud/global/images/family/debian-8",
 			ExpectDiffSuppress: true,
 		},
+		"matching unconventional image family partial self link": {
+			Old:                "https://www.googleapis.com/compute/v1/projects/ubuntu-os-cloud/global/images/ubuntu-1404-trusty-v20180122",
+			New:                "projects/ubuntu-os-cloud/global/images/family/ubuntu-1404-lts",
+			ExpectDiffSuppress: true,
+		},
 		"matching image family partial no project self link": {
 			Old:                "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20171213",
 			New:                "global/images/family/debian-8",
@@ -106,6 +117,11 @@ func TestDiskImageDiffSuppress(t *testing.T) {
 			New:                "debian/debian-8",
 			ExpectDiffSuppress: true,
 		},
+		"matching unconventional image family short hand": {
+			Old:                "https://www.googleapis.com/compute/v1/projects/ubuntu-os-cloud/global/images/ubuntu-1404-trusty-v20180122",
+			New:                "ubuntu-os-cloud/ubuntu-1404-lts",
+			ExpectDiffSuppress: true,
+		},
 		"different image family": {
 			Old:                "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-8-jessie-v20171213",
 			New:                "family/debian-7",
@@ -155,6 +171,35 @@ func TestDiskImageDiffSuppress(t *testing.T) {
 	}
 }
 
+// Test that all the naming pattern for public images are supported.
+func TestAccComputeDisk_imageDiffSuppressPublicVendorsFamilyNames(t *testing.T) {
+	t.Parallel()
+
+	if os.Getenv(resource.TestEnvVar) == "" {
+		t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar))
+	}
+
+	config := getInitializedConfig(t)
+
+	for _, publicImageProject := range imageMap {
+		token := ""
+		for paginate := true; paginate; {
+			resp, err := config.clientCompute.Images.List(publicImageProject).Filter("deprecated.replacement ne .*images.*").PageToken(token).Do()
+			if err != nil {
+				t.Fatalf("Can't list public images for project %q", publicImageProject)
+			}
+
+			for _, image := range resp.Items {
+				if !diskImageDiffSuppress("image", image.SelfLink, "family/"+image.Family, nil) {
+					t.Errorf("should suppress diff for image %q and family %q", image.SelfLink, image.Family)
+				}
+			}
+			token := resp.NextPageToken
+			paginate = token != ""
+		}
+	}
+}
+
 func TestAccComputeDisk_basic(t *testing.T) {
 	t.Parallel()
 

website/docs/r/compute_disk.html.markdown

@@ -47,8 +47,14 @@ 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's `self_link`, of a full name and version, e.g.
-    `debian-8-jessie-v20170523`
+    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}`. If referred by family, the
+    images names must include the family name. If they don't, use the
+    [google_compute_image data source](/docs/providers/google/d/datasource_compute_image.html).
+    For instance, the image `centos-6-v20180104` includes its family name `centos-6`.
+    These images can be referred by family name here.
 
 * `project` - (Optional) The project in which the resource belongs. If it
     is not provided, the provider project is used.

website/docs/r/compute_instance.html.markdown

@@ -156,9 +156,10 @@ The `initialize_params` block supports:
     `projects/{project}/global/images/family/{family}`, `global/images/{image}`,
     `global/images/family/{family}`, `family/{family}`, `{project}/{family}`,
     `{project}/{image}`, `{family}`, or `{image}`. If referred by family, the
-    images names must include the family name. For instance, the image
-    `centos-6-v20180104` includes its family name `centos-6`. These images can
-    be referred by family name here.
+    images names must include the family name. If they don't, use the
+    [google_compute_image data source](/docs/providers/google/d/datasource_compute_image.html).
+    For instance, the image `centos-6-v20180104` includes its family name `centos-6`.
+    These images can be referred by family name here.
 
 The `scratch_disk` block supports: