image_version and python_version are ga in composer (#4465)

Signed-off-by: Modular Magician <[email protected]>

google/resource_composer_environment.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/helper/validation"
 	composer "google.golang.org/api/composer/v1beta1"
@@ -15,6 +16,7 @@ import (
 const (
 	composerEnvironmentEnvVariablesRegexp          = "[a-zA-Z_][a-zA-Z0-9_]*."
 	composerEnvironmentReservedAirflowEnvVarRegexp = "AIRFLOW__[A-Z0-9_]+__[A-Z0-9_]+"
+	composerEnvironmentVersionRegexp               = `composer-([0-9]+\.[0-9]+\.[0-9]+|latest)-airflow-([0-9]+\.[0-9]+(\.[0-9]+.*)?)`
 )
 
 var composerEnvironmentReservedEnvVar = map[string]struct{}{
@@ -220,8 +222,17 @@ func resourceComposerEnvironment() *schema.Resource {
 										ValidateFunc: validateComposerEnvironmentEnvVariables,
 									},
 									"image_version": {
+										Type:             schema.TypeString,
+										Computed:         true,
+										Optional:         true,
+										ValidateFunc:     validateRegexp(composerEnvironmentVersionRegexp),
+										DiffSuppressFunc: composerImageVersionDiffSuppress,
+									},
+									"python_version": {
 										Type:     schema.TypeString,
+										Optional: true,
 										Computed: true,
+										ForceNew: true,
 									},
 								},
 							},
@@ -383,6 +394,22 @@ func resourceComposerEnvironmentUpdate(d *schema.ResourceData, meta interface{})
 			return err
 		}
 
+		if d.HasChange("config.0.software_config.0.image_version") {
+			patchObj := &composer.Environment{
+				Config: &composer.EnvironmentConfig{
+					SoftwareConfig: &composer.SoftwareConfig{},
+				},
+			}
+			if config != nil && config.SoftwareConfig != nil {
+				patchObj.Config.SoftwareConfig.ImageVersion = config.SoftwareConfig.ImageVersion
+			}
+			err = resourceComposerEnvironmentPatchField("config.softwareConfig.imageVersion", patchObj, d, tfConfig)
+			if err != nil {
+				return err
+			}
+			d.SetPartial("config")
+		}
+
 		if d.HasChange("config.0.software_config.0.airflow_config_overrides") {
 			patchObj := &composer.Environment{
 				Config: &composer.EnvironmentConfig{
@@ -635,6 +662,7 @@ func flattenComposerEnvironmentConfigSoftwareConfig(softwareCfg *composer.Softwa
 	}
 	transformed := make(map[string]interface{})
 	transformed["image_version"] = softwareCfg.ImageVersion
+	transformed["python_version"] = softwareCfg.PythonVersion
 	transformed["airflow_config_overrides"] = softwareCfg.AirflowConfigOverrides
 	transformed["pypi_packages"] = softwareCfg.PypiPackages
 	transformed["env_variables"] = softwareCfg.EnvVariables
@@ -908,6 +936,7 @@ func expandComposerEnvironmentConfigSoftwareConfig(v interface{}, d *schema.Reso
 	transformed := &composer.SoftwareConfig{}
 
 	transformed.ImageVersion = original["image_version"].(string)
+	transformed.PythonVersion = original["python_version"].(string)
 	transformed.AirflowConfigOverrides = expandComposerEnvironmentConfigSoftwareConfigStringMap(original, "airflow_config_overrides")
 	transformed.PypiPackages = expandComposerEnvironmentConfigSoftwareConfigStringMap(original, "pypi_packages")
 	transformed.EnvVariables = expandComposerEnvironmentConfigSoftwareConfigStringMap(original, "env_variables")
@@ -1072,3 +1101,59 @@ func validateServiceAccountRelativeNameOrEmail(v interface{}, k string) (ws []st
 
 	return
 }
+
+func composerImageVersionDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
+	versionRe := regexp.MustCompile(composerEnvironmentVersionRegexp)
+	oldVersions := versionRe.FindStringSubmatch(old)
+	newVersions := versionRe.FindStringSubmatch(new)
+	if oldVersions == nil || len(oldVersions) < 3 {
+		// Somehow one of the versions didn't match the regexp or didn't
+		// have values in the capturing groups. In that case, fall back to
+		// an equality check.
+		log.Printf("[WARN] Composer version didn't match regexp: %s", old)
+		return old == new
+	}
+	if newVersions == nil || len(newVersions) < 3 {
+		// Somehow one of the versions didn't match the regexp or didn't
+		// have values in the capturing groups. In that case, fall back to
+		// an equality check.
+		log.Printf("[WARN] Composer version didn't match regexp: %s", new)
+		return old == new
+	}
+
+	// Check airflow version using the version package to account for
+	// diffs like 1.10 and 1.10.0
+	eq, err := versionsEqual(oldVersions[2], newVersions[2])
+	if err != nil {
+		log.Printf("[WARN] Could not parse airflow version, %s", err)
+	}
+	if !eq {
+		return false
+	}
+
+	// Check composer version. Assume that "latest" means we should
+	// suppress the diff, because we don't have any other way of
+	// knowing what the latest version actually is.
+	if oldVersions[1] == "latest" || newVersions[1] == "latest" {
+		return true
+	}
+	// If neither version is "latest", check them using the version
+	// package like we did for airflow.
+	eq, err = versionsEqual(oldVersions[1], newVersions[1])
+	if err != nil {
+		log.Printf("[WARN] Could not parse composer version, %s", err)
+	}
+	return eq
+}
+
+func versionsEqual(old, new string) (bool, error) {
+	o, err := version.NewVersion(old)
+	if err != nil {
+		return false, err
+	}
+	n, err := version.NewVersion(new)
+	if err != nil {
+		return false, err
+	}
+	return o.Equal(n), nil
+}

google/resource_composer_environment_test.go

@@ -26,6 +26,30 @@ func init() {
 	})
 }
 
+func TestComposerImageVersionDiffSuppress(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name     string
+		old      string
+		new      string
+		expected bool
+	}{
+		{"matches", "composer-1.4.0-airflow-1.10.0", "composer-1.4.0-airflow-1.10.0", true},
+		{"old latest", "composer-latest-airflow-1.10.0", "composer-1.4.1-airflow-1.10.0", true},
+		{"new latest", "composer-1.4.1-airflow-1.10.0", "composer-latest-airflow-1.10.0", true},
+		{"airflow equivalent", "composer-1.4.0-airflow-1.10.0", "composer-1.4.0-airflow-1.10", true},
+		{"airflow different", "composer-1.4.0-airflow-1.10.0", "composer-1.4-airflow-1.9.0", false},
+		{"airflow different composer latest", "composer-1.4.0-airflow-1.10.0", "composer-latest-airflow-1.9.0", false},
+	}
+
+	for _, tc := range cases {
+		if actual := composerImageVersionDiffSuppress("", tc.old, tc.new, nil); actual != tc.expected {
+			t.Fatalf("'%s' failed, expected %v but got %v", tc.name, tc.expected, actual)
+		}
+	}
+}
+
 // Checks environment creation with minimum required information.
 func TestAccComposerEnvironment_basic(t *testing.T) {
 	t.Parallel()
@@ -164,6 +188,28 @@ func TestAccComposerEnvironment_withNodeConfig(t *testing.T) {
 	})
 }
 
+func TestAccComposerEnvironment_withSoftwareConfig(t *testing.T) {
+	t.Parallel()
+	envName := acctest.RandomWithPrefix(testComposerEnvironmentPrefix)
+	var env composer.Environment
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccComposerEnvironmentDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComposerEnvironment_softwareCfg(envName),
+				Check:  testAccCheckComposerEnvironmentExists("google_composer_environment.test", &env),
+			},
+			{
+				ResourceName:      "google_composer_environment.test",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 // Checks behavior of config for creation for attributes that must
 // be updated during create.
 func TestAccComposerEnvironment_withUpdateOnCreate(t *testing.T) {
@@ -301,6 +347,7 @@ resource "google_composer_environment" "test" {
 		node_count = 4
 
 		software_config {
+			image_version = "${data.google_composer_image_versions.all.image_versions.0.image_version_id}"
 
 			airflow_config_overrides = {
 			  core-load_example = "True"
@@ -366,6 +413,24 @@ resource "google_project_iam_member" "composer-worker" {
 `, environment, network, subnetwork, serviceAccount)
 }
 
+func testAccComposerEnvironment_softwareCfg(name string) string {
+	return fmt.Sprintf(`
+data "google_composer_image_versions" "all" {
+}
+
+resource "google_composer_environment" "test" {
+	name           = "%s"
+	region         = "us-central1"
+	config {
+		software_config {
+			image_version = "${data.google_composer_image_versions.all.image_versions.0.image_version_id}"
+			python_version = "3"
+		}
+	}
+}
+`, name)
+}
+
 func testAccComposerEnvironment_updateOnlyFields(name string) string {
 	return fmt.Sprintf(`
 resource "google_composer_environment" "test" {

website/docs/r/composer_environment.html.markdown

@@ -275,17 +275,16 @@ The `software_config` block supports:
   SQL_USER
   ```
 
-* `image_version` (Optional, [Beta](https://terraform.io/docs/providers/google/provider_versions.html)) -
+* `image_version` (Optional) -
   The version of the software running in the environment. This encapsulates both the version of Cloud Composer
   functionality and the version of Apache Airflow. It must match the regular expression 
   `composer-[0-9]+\.[0-9]+(\.[0-9]+)?-airflow-[0-9]+\.[0-9]+(\.[0-9]+.*)?`.
   The Cloud Composer portion of the version is a semantic version. 
   The portion of the image version following 'airflow-' is an official Apache Airflow repository release name.
   See [documentation](https://cloud.google.com/composer/docs/reference/rest/v1beta1/projects.locations.environments#softwareconfig)
-  for allowed release names. This field can only be set in the [Beta](https://terraform.io/docs/providers/google/provider_versions.html))
-  provider, but is an output-only attribute in the GA provider.
+  for allowed release names.
 
-* `python_version` (Optional, [Beta](https://terraform.io/docs/providers/google/provider_versions.html)) -
+* `python_version` (Optional) -
   The major version of Python used to run the Apache Airflow scheduler, worker, and webserver processes.
   Can be set to '2' or '3'. If not specified, the default is '2'. Cannot be updated.