From 270bbdb19bb77f2a74ba268167be855ce2df2b2c Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Wed, 2 Aug 2017 11:18:59 -0400 Subject: [PATCH] core: Add `GetOkRaw` schema function Adds `GetOkRaw` as a schema function. This should only be used to verify boolean attributes are either set or not set, regardless of their zero value for their type. There are a few small use cases outside of the boolean type where this will be helpful as well. Overall, this shouldn't detract from the zero-value checks that `GetOK()` currently has, and should only be used when absolutely needed. However, there are enough use-cases for this addition without checking for the zero-value of the type, that this is needed. Primary use case is for a boolean attribute that is `Optional` and `Computed`, without a default value. There's currently no way to verify that the boolean attribute was explicitly set to the zero-value literal with the current `GetOk()` function. This new function allows for that check, keeping the `Computed` check for the returned `exists` boolean. ``` $ make test TEST=./helper/schema TESTARGS="-run=TestResourceDataGetOkRaw" ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/08/02 11:17:32 Generated command/internal_plugin_list.go go test -i ./helper/schema || exit 1 echo ./helper/schema | \ xargs -t -n4 go test -run=TestResourceDataGetOkRaw -timeout=60s -parallel=4 go test -run=TestResourceDataGetOkRaw -timeout=60s -parallel=4 ./helper/schema ok github.com/hashicorp/terraform/helper/schema 0.005s ``` --- helper/schema/resource_data.go | 16 +++ helper/schema/resource_data_test.go | 198 ++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index b2bc8f6c7c..fcb44a2f81 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -104,6 +104,22 @@ func (d *ResourceData) GetOk(key string) (interface{}, bool) { return r.Value, exists } +// GetOkRaw returns the data for a given key and whether or not the key +// has been set to a non-zero value. This is only useful for determining +// if boolean attributes have been set, if they are Optional but do not +// have a Default value. +// +// This is nearly the same function as GetOk, yet it does not check +// for the zero value of the attribute's type. This allows for attributes +// without a default, to fully check for a literal assignment, regardless +// of the zero-value for that type. +// This should only be used if absolutely required/needed. +func (d *ResourceData) GetOkRaw(key string) (interface{}, bool) { + r := d.getRaw(key, getSourceSet) + exists := r.Exists && !r.Computed + return r.Value, exists +} + func (d *ResourceData) getRaw(key string, level getSource) getResult { var parts []string if key != "" { diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index 615a0f7f7a..bb79f2bca9 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -1082,6 +1082,204 @@ func TestResourceDataGetOk(t *testing.T) { } } +func TestResourceDataGetOkRaw(t *testing.T) { + cases := []struct { + Name string + Schema map[string]*Schema + State *terraform.InstanceState + Diff *terraform.InstanceDiff + Key string + Value interface{} + Ok bool + }{ + /* + * Primitives + */ + { + Name: "string-literal-empty", + Schema: map[string]*Schema{ + "availability_zone": { + Type: TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + }, + + State: nil, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": { + Old: "", + New: "", + }, + }, + }, + + Key: "availability_zone", + Value: "", + Ok: true, + }, + + { + Name: "string-computed-empty", + Schema: map[string]*Schema{ + "availability_zone": { + Type: TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + }, + + State: nil, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": { + Old: "", + New: "", + NewComputed: true, + }, + }, + }, + + Key: "availability_zone", + Value: "", + Ok: false, + }, + + { + Name: "string-optional-computed-nil-diff", + Schema: map[string]*Schema{ + "availability_zone": { + Type: TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + }, + + State: nil, + + Diff: nil, + + Key: "availability_zone", + Value: "", + Ok: false, + }, + + /* + * Lists + */ + + { + Name: "list-optional", + Schema: map[string]*Schema{ + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeInt}, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports", + Value: []interface{}{}, + Ok: false, + }, + + /* + * Map + */ + + { + Name: "map-optional", + Schema: map[string]*Schema{ + "ports": { + Type: TypeMap, + Optional: true, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports", + Value: map[string]interface{}{}, + Ok: false, + }, + + /* + * Set + */ + + { + Name: "set-optional", + Schema: map[string]*Schema{ + "ports": { + Type: TypeSet, + Optional: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { return a.(int) }, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports", + Value: []interface{}{}, + Ok: false, + }, + + { + Name: "set-optional-key", + Schema: map[string]*Schema{ + "ports": { + Type: TypeSet, + Optional: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { return a.(int) }, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports.0", + Value: 0, + Ok: false, + }, + } + + for _, tc := range cases { + d, err := schemaMap(tc.Schema).Data(tc.State, tc.Diff) + if err != nil { + t.Fatalf("%s err: %s", tc.Name, err) + } + + v, ok := d.GetOkRaw(tc.Key) + if s, ok := v.(*Set); ok { + v = s.List() + } + + if !reflect.DeepEqual(v, tc.Value) { + t.Fatalf("Bad %s: \n%#v", tc.Name, v) + } + if ok != tc.Ok { + t.Fatalf("%s: expected ok: %t, got: %t", tc.Name, tc.Ok, ok) + } + } +} + func TestResourceDataTimeout(t *testing.T) { cases := []struct { Name string