Firewall uses v1 API if the priority is unset or has the default value. (#500)

* api_versions supports default value
* Firewall use v1 API if the priority is set to default value (1000)

google/api_versions.go

@@ -45,7 +45,7 @@ type TerraformResourceData interface {
 func getComputeApiVersion(d TerraformResourceData, resourceVersion ComputeApiVersion, features []Feature) ComputeApiVersion {
 	versions := map[ComputeApiVersion]struct{}{resourceVersion: struct{}{}}
 	for _, feature := range features {
-		if feature.InUseBy(d) {
+		if feature.InUseByDefault(d) {
 			versions[feature.Version] = struct{}{}
 		}
 	}
@@ -58,8 +58,12 @@ func getComputeApiVersion(d TerraformResourceData, resourceVersion ComputeApiVer
 // version, to determine what version of the API is required in order to update the resource.
 func getComputeApiVersionUpdate(d TerraformResourceData, resourceVersion ComputeApiVersion, features, updateOnlyFields []Feature) ComputeApiVersion {
 	versions := map[ComputeApiVersion]struct{}{resourceVersion: struct{}{}}
-	schemaVersion := getComputeApiVersion(d, resourceVersion, features)
+
-	versions[schemaVersion] = struct{}{}
+	for _, feature := range features {
+		if feature.InUseByUpdate(d) {
+			versions[feature.Version] = struct{}{}
+		}
+	}
 
 	for _, feature := range updateOnlyFields {
 		if feature.HasChangeBy(d) {
@@ -83,6 +87,10 @@ type Feature struct {
 	//
 	// Note: beta field nested inside a SET are NOT supported at the moment.
 	Item string
+
+	// Optional, only set if your field has a default value.
+	// If the value for the field is equal to the DefaultValue, we assume the beta feature is not activated.
+	DefaultValue interface{}
 }
 
 // Returns true when a feature has been modified.
@@ -93,16 +101,37 @@ func (s Feature) HasChangeBy(d TerraformResourceData) bool {
 	return d.HasChange(s.Item)
 }
 
-// Return true when a feature appears in schema or has been modified.
+type InUseFunc func(d TerraformResourceData, path string, defaultValue interface{}) bool
-func (s Feature) InUseBy(d TerraformResourceData) bool {
+
-	return inUseBy(d, s.Item)
+func defaultInUseFunc(d TerraformResourceData, path string, defaultValue interface{}) bool {
+	// At read and delete time, there is no change.
+	// At create time, all fields are marked has changed. We should only consider the feature active if the field has
+	// a value set and that this value is not the default value.
+	value, ok := d.GetOk(path)
+	return ok && value != defaultValue
 }
 
-func inUseBy(d TerraformResourceData, path string) bool {
+func updateInUseFunc(d TerraformResourceData, path string, defaultValue interface{}) bool {
+	// During a resource update, if the beta field has changes, the feature is considered active even if the new value
+	// is the default value. This is because the beta API must be called to change the value of the field back to the
+	// default value.
+	value, ok := d.GetOk(path)
+	return (ok && value != defaultValue) || d.HasChange(path)
+}
+
+// Return true when a feature appears in schema and doesn't hold the default value.
+func (s Feature) InUseByDefault(d TerraformResourceData) bool {
+	return inUseBy(d, s.Item, s.DefaultValue, defaultInUseFunc)
+}
+
+func (s Feature) InUseByUpdate(d TerraformResourceData) bool {
+	return inUseBy(d, s.Item, s.DefaultValue, updateInUseFunc)
+}
+
+func inUseBy(d TerraformResourceData, path string, defaultValue interface{}, inUseFunc InUseFunc) bool {
 	pos := strings.Index(path, "*")
 	if pos == -1 {
-		_, ok := d.GetOk(path)
+		return inUseFunc(d, path, defaultValue)
-		return ok || d.HasChange(path)
 	}
 
 	prefix := path[0:pos]
@@ -117,7 +146,7 @@ func inUseBy(d TerraformResourceData, path string) bool {
 	count := v.(int)
 	for i := 0; i < count; i++ {
 		nestedPath := fmt.Sprintf("%s%d%s", prefix, i, suffix)
-		if inUseBy(d, nestedPath) {
+		if inUseBy(d, nestedPath, defaultValue, inUseFunc) {
 			return true
 		}
 	}

google/api_versions_test.go

@@ -19,7 +19,7 @@ func TestComputeApiVersion(t *testing.T) {
 		UpdateOnlyFields []Feature
 		ExpectedApiVersions
 	}{
-		"no beta fields": {
+		"no beta field": {
 			FieldsInSchema: map[string]interface{}{
 				"normal_field": "foo",
 			},
@@ -29,7 +29,7 @@ func TestComputeApiVersion(t *testing.T) {
 				Update:     baseVersion,
 			},
 		},
-		"beta fields no set": {
+		"beta field not set": {
 			Features: []Feature{{Version: betaVersion, Item: "beta_field"}},
 			FieldsInSchema: map[string]interface{}{
 				"normal_field": "foo",
@@ -40,7 +40,7 @@ func TestComputeApiVersion(t *testing.T) {
 				Update:     baseVersion,
 			},
 		},
-		"beta fields set": {
+		"beta field set": {
 			Features: []Feature{{Version: betaVersion, Item: "beta_field"}},
 			FieldsInSchema: map[string]interface{}{
 				"normal_field": "foo",
@@ -52,7 +52,7 @@ func TestComputeApiVersion(t *testing.T) {
 				Update:     betaVersion,
 			},
 		},
-		"update only beta fields": {
+		"update only beta field": {
 			FieldsInSchema: map[string]interface{}{
 				"normal_field": "foo",
 			},
@@ -64,7 +64,7 @@ func TestComputeApiVersion(t *testing.T) {
 				Update:     betaVersion,
 			},
 		},
-		"nested beta fields not set": {
+		"nested beta field not set": {
 			Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field"}},
 			FieldsInSchema: map[string]interface{}{
 				"list_field.#":              2,
@@ -77,7 +77,7 @@ func TestComputeApiVersion(t *testing.T) {
 				Update:     baseVersion,
 			},
 		},
-		"nested beta fields set": {
+		"nested beta field set": {
 			Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field"}},
 			FieldsInSchema: map[string]interface{}{
 				"list_field.#":                   2,
@@ -104,6 +104,60 @@ func TestComputeApiVersion(t *testing.T) {
 				Update:     betaVersion,
 			},
 		},
+		"beta field has default value": {
+			Features: []Feature{{Version: betaVersion, Item: "beta_field", DefaultValue: "bar"}},
+			FieldsInSchema: map[string]interface{}{
+				"normal_field": "foo",
+				"beta_field":   "bar",
+			},
+			ExpectedApiVersions: ExpectedApiVersions{
+				Create:     baseVersion,
+				ReadDelete: baseVersion,
+				Update:     baseVersion,
+			},
+		},
+		"beta field is updated to default value": {
+			Features: []Feature{{Version: betaVersion, Item: "beta_field", DefaultValue: "bar"}},
+			FieldsInSchema: map[string]interface{}{
+				"normal_field": "foo",
+				"beta_field":   "bar",
+			},
+			UpdatedFields: []string{"beta_field"},
+			ExpectedApiVersions: ExpectedApiVersions{
+				Create:     baseVersion,
+				ReadDelete: baseVersion,
+				Update:     betaVersion,
+			},
+		},
+		"nested beta field has default value": {
+			Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field", DefaultValue: "baz"}},
+			FieldsInSchema: map[string]interface{}{
+				"list_field.#":                   2,
+				"list_field.0.normal_field":      "foo",
+				"list_field.1.normal_field":      "bar",
+				"list_field.1.beta_nested_field": "baz",
+			},
+			ExpectedApiVersions: ExpectedApiVersions{
+				Create:     baseVersion,
+				ReadDelete: baseVersion,
+				Update:     baseVersion,
+			},
+		},
+		"nested beta field is updated default value": {
+			Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field", DefaultValue: "baz"}},
+			FieldsInSchema: map[string]interface{}{
+				"list_field.#":                   2,
+				"list_field.0.normal_field":      "foo",
+				"list_field.1.normal_field":      "bar",
+				"list_field.1.beta_nested_field": "baz",
+			},
+			UpdatedFields: []string{"list_field.1.beta_nested_field"},
+			ExpectedApiVersions: ExpectedApiVersions{
+				Create:     baseVersion,
+				ReadDelete: baseVersion,
+				Update:     betaVersion,
+			},
+		},
 	}
 
 	for tn, tc := range cases {

google/resource_compute_firewall.go

@@ -20,7 +20,7 @@ var FirewallVersionedFeatures = []Feature{
 	Feature{Version: v0beta, Item: "deny"},
 	Feature{Version: v0beta, Item: "direction"},
 	Feature{Version: v0beta, Item: "destination_ranges"},
-	Feature{Version: v0beta, Item: "priority"},
+	Feature{Version: v0beta, Item: "priority", DefaultValue: COMPUTE_FIREWALL_PRIORITY_DEFAULT},
 }
 
 func resourceComputeFirewall() *schema.Resource {