From f3bd15cc9fabe5bc3ef1a2e098b65320f48e5eb8 Mon Sep 17 00:00:00 2001 From: The Magician Date: Fri, 4 Jan 2019 10:17:09 -0800 Subject: [PATCH] add diffsuppress for composer image version (#293) /cc @danawillow --- google-beta/resource_composer_environment.go | 68 +++++++++++++++++-- .../resource_composer_environment_test.go | 24 +++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/google-beta/resource_composer_environment.go b/google-beta/resource_composer_environment.go index d157c9dd4c1..a11d329eeb9 100644 --- a/google-beta/resource_composer_environment.go +++ b/google-beta/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" "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{}{ @@ -177,10 +179,12 @@ func resourceComposerEnvironment() *schema.Resource { ValidateFunc: validateComposerEnvironmentEnvVariables, }, "image_version": { - Type: schema.TypeString, - Computed: true, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Computed: true, + Optional: true, + ForceNew: true, + ValidateFunc: validateRegexp(composerEnvironmentVersionRegexp), + DiffSuppressFunc: composerImageVersionDiffSuppress, }, "python_version": { Type: schema.TypeString, @@ -916,3 +920,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 +} diff --git a/google-beta/resource_composer_environment_test.go b/google-beta/resource_composer_environment_test.go index 009f321a172..48e263e3f14 100644 --- a/google-beta/resource_composer_environment_test.go +++ b/google-beta/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()