Add diff suppress to dataproc image version (#2091)

Merged PR #2091.
GoogleCloudPlatform · Jul 24, 2019 · 556890a · 556890a
1 parent e157ca5
commit 556890a
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 9 deletions.
diff --git a/build/terraform b/build/terraform
diff --git a/build/terraform-beta b/build/terraform-beta
diff --git a/third_party/terraform/resources/resource_dataproc_cluster.go.erb b/third_party/terraform/resources/resource_dataproc_cluster.go.erb
@@ -16,6 +16,8 @@ import (
 	"google.golang.org/api/dataproc/v1beta2"
 )
 
+var resolveDataprocImageVersion = regexp.MustCompile(`(?P<Major>[^\s.-]+)\.(?P<Minor>[^\s.-]+)(?:\.(?P<Subminor>[^\s.-]+))?(?:\-(?P<Distr>[^\s.-]+))?`)
+
 func resourceDataprocCluster() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceDataprocClusterCreate,
@@ -262,10 +264,11 @@ func resourceDataprocCluster() *schema.Resource {
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									"image_version": {
-										Type:     schema.TypeString,
-										Optional: true,
-										Computed: true,
-										ForceNew: true,
+										Type:             schema.TypeString,
+										Optional:         true,
+										Computed:         true,
+										ForceNew:         true,
+										DiffSuppressFunc: dataprocImageVersionDiffSuppress,
 									},
 
 									"override_properties": {
@@ -1025,3 +1028,51 @@ func configOptions(d *schema.ResourceData, option string) (map[string]interface{
 	}
 	return nil, false
 }
+
+func dataprocImageVersionDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
+	oldV, err := parseDataprocImageVersion(old)
+	if err != nil {
+		return false
+	}
+	newV, err := parseDataprocImageVersion(new)
+	if err != nil {
+		return false
+	}
+
+	if newV.major != oldV.major {
+		return false
+	}
+	if newV.minor != oldV.minor {
+		return false
+	}
+	// Only compare subminor version if set in config version.
+	if newV.subminor != "" && newV.subminor != oldV.subminor {
+		return false
+	}
+	// Only compare os if it is set in config version.
+	if newV.osName != "" && newV.osName != oldV.osName {
+		return false
+	}
+	return true
+}
+
+type dataprocImageVersion struct {
+	major    string
+	minor    string
+	subminor string
+	osName   string
+}
+
+func parseDataprocImageVersion(version string) (*dataprocImageVersion, error) {
+	matches := resolveDataprocImageVersion.FindStringSubmatch(version)
+	if len(matches) != 5 {
+		return nil, fmt.Errorf("invalid image version %q", version)
+	}
+
+	return &dataprocImageVersion{
+		major:    matches[1],
+		minor:    matches[2],
+		subminor: matches[3],
+		osName:   matches[4],
+	}, nil
+}
diff --git a/third_party/terraform/tests/resource_dataproc_cluster_test.go.erb b/third_party/terraform/tests/resource_dataproc_cluster_test.go.erb
@@ -17,7 +17,7 @@ import (
 	"google.golang.org/api/googleapi"
 )
 
-func TestExtractInitTimeout(t *testing.T) {
+func TestDataprocExtractInitTimeout(t *testing.T) {
 	t.Parallel()
 
 	actual, err := extractInitTimeout("500s")
@@ -30,7 +30,7 @@ func TestExtractInitTimeout(t *testing.T) {
 	}
 }
 
-func TestExtractInitTimeout_nonSeconds(t *testing.T) {
+func TestDataprocExtractInitTimeout_nonSeconds(t *testing.T) {
 	t.Parallel()
 
 	actual, err := extractInitTimeout("5m")
@@ -43,7 +43,7 @@ func TestExtractInitTimeout_nonSeconds(t *testing.T) {
 	}
 }
 
-func TestExtractInitTimeout_empty(t *testing.T) {
+func TestDataprocExtractInitTimeout_empty(t *testing.T) {
 	t.Parallel()
 
 	_, err := extractInitTimeout("")
@@ -54,6 +54,85 @@ func TestExtractInitTimeout_empty(t *testing.T) {
 	t.Fatalf("Expected an error with message '%s', but got %v", expected, err.Error())
 }
 
+func TestDataprocParseImageVersion(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]dataprocImageVersion{
+		"1.2":             {"1", "2", "", ""},
+		"1.2.3":           {"1", "2", "3", ""},
+		"1.2.3rc":         {"1", "2", "3rc", ""},
+		"1.2-debian9":     {"1", "2", "", "debian9"},
+		"1.2.3-debian9":   {"1", "2", "3", "debian9"},
+		"1.2.3rc-debian9": {"1", "2", "3rc", "debian9"},
+	}
+
+	for v, expected := range testCases {
+		actual, err := parseDataprocImageVersion(v)
+		if actual.major != expected.major {
+			t.Errorf("parsing version %q returned error: %v", v, err)
+		}
+		if err != nil {
+			t.Errorf("parsing version %q returned error: %v", v, err)
+		}
+		if actual.minor != expected.minor {
+			t.Errorf("parsing version %q returned error: %v", v, err)
+		}
+		if actual.subminor != expected.subminor {
+			t.Errorf("parsing version %q returned error: %v", v, err)
+		}
+		if actual.osName != expected.osName {
+			t.Errorf("parsing version %q returned error: %v", v, err)
+		}
+	}
+
+	errorTestCases := []string{
+		"",
+		"1",
+		"notaversion",
+		"1-debian",
+	}
+	for _, v := range errorTestCases {
+		if _, err := parseDataprocImageVersion(v); err == nil {
+			t.Errorf("expected parsing invalid version %q to return error", v)
+		}
+	}
+}
+
+func TestDataprocDiffSuppress(t *testing.T) {
+	t.Parallel()
+
+	doSuppress := [][]string{
+		{"1.3.10-debian9", "1.3"},
+		{"1.3.10-debian9", "1.3-debian9"},
+		{"1.3.10", "1.3"},
+		{"1.3-debian9", "1.3"},
+	}
+
+	noSuppress := [][]string{
+		{"1.3.10-debian9", "1.3.10-ubuntu"},
+		{"1.3.10-debian9", "1.3.9-debian9"},
+		{"1.3.10-debian9", "1.3-ubuntu"},
+		{"1.3.10-debian9", "1.3.9"},
+		{"1.3.10-debian9", "1.4"},
+		{"1.3.10-debian9", "2.3"},
+		{"1.3.10", "1.3.10-debian9"},
+		{"1.3", "1.3.10"},
+		{"1.3", "1.3.10-debian9"},
+		{"1.3", "1.3-debian9"},
+	}
+
+	for _, tup := range doSuppress {
+		if !dataprocImageVersionDiffSuppress("", tup[0], tup[1], nil) {
+			t.Errorf("expected (old: %q, new: %q) to be suppressed", tup[0], tup[1])
+		}
+	}
+	for _, tup := range noSuppress {
+		if dataprocImageVersionDiffSuppress("", tup[0], tup[1], nil) {
+			t.Errorf("expected (old: %q, new: %q) to not be suppressed", tup[0], tup[1])
+		}
+	}
+}
+
 func TestAccDataprocCluster_missingZoneGlobalRegion1(t *testing.T) {
 	t.Parallel()