helper/schema: Better ResourceDiff schema key validation

This fixes nil pointer issues that could come up if an invalid key was
referenced (ie: not one in the schema). Also ships a helper validation
function to streamline things.
This commit is contained in:
Chris Marchesi 2017-06-01 09:19:03 -07:00 committed by Martin Atkins
parent 5d5a670d69
commit 3ac0cdf0c0
2 changed files with 49 additions and 7 deletions

View File

@ -215,8 +215,8 @@ func (d *ResourceDiff) UpdatedKeys() []string {
// Note that this does not wipe an override. This function is only allowed on // Note that this does not wipe an override. This function is only allowed on
// computed keys. // computed keys.
func (d *ResourceDiff) Clear(key string) error { func (d *ResourceDiff) Clear(key string) error {
if !d.schema[key].Computed { if err := d.checkKey(key, "Clear"); err != nil {
return fmt.Errorf("Clear is allowed on computed attributes only - %s is not one", key) return err
} }
return d.clear(key) return d.clear(key)
@ -256,9 +256,10 @@ func (d *ResourceDiff) diffChange(key string) (interface{}, interface{}, bool, b
// //
// This function is only allowed on computed attributes. // This function is only allowed on computed attributes.
func (d *ResourceDiff) SetNew(key string, value interface{}) error { func (d *ResourceDiff) SetNew(key string, value interface{}) error {
if !d.schema[key].Computed { if err := d.checkKey(key, "SetNew"); err != nil {
return fmt.Errorf("SetNew only operates on computed keys - %s is not one", key) return err
} }
return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false) return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false)
} }
@ -267,9 +268,10 @@ func (d *ResourceDiff) SetNew(key string, value interface{}) error {
// //
// This function is only allowed on computed attributes. // This function is only allowed on computed attributes.
func (d *ResourceDiff) SetNewComputed(key string) error { func (d *ResourceDiff) SetNewComputed(key string) error {
if !d.schema[key].Computed { if err := d.checkKey(key, "SetNewComputed"); err != nil {
return fmt.Errorf("SetNewComputed only operates on computed keys - %s is not one", key) return err
} }
return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, nil, true) return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, nil, true)
} }
@ -295,7 +297,7 @@ func (d *ResourceDiff) setDiff(key string, old, new interface{}, computed bool)
// specific ResourceDiff instance. // specific ResourceDiff instance.
func (d *ResourceDiff) ForceNew(key string) error { func (d *ResourceDiff) ForceNew(key string) error {
if !d.HasChange(key) { if !d.HasChange(key) {
return fmt.Errorf("ResourceDiff.ForceNew: No changes for %s", key) return fmt.Errorf("ForceNew: No changes for %s", key)
} }
old, new := d.GetChange(key) old, new := d.GetChange(key)
@ -444,3 +446,15 @@ func childAddrOf(child, parent string) bool {
} }
return reflect.DeepEqual(ps, cs[:len(ps)]) return reflect.DeepEqual(ps, cs[:len(ps)])
} }
// checkKey checks the key to make sure it exists and is computed.
func (d *ResourceDiff) checkKey(key, caller string) error {
s, ok := d.schema[key]
if !ok {
return fmt.Errorf("%s: invalid key: %s", caller, key)
}
if !s.Computed {
return fmt.Errorf("%s only operates on computed keys - %s is not one", caller, key)
}
return nil
}

View File

@ -520,6 +520,34 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
NewValue: "qux", NewValue: "qux",
ExpectedError: true, ExpectedError: true,
}, },
resourceDiffTestCase{
Name: "bad key, should error",
Schema: map[string]*Schema{
"foo": &Schema{
Type: TypeString,
Required: true,
},
},
State: &terraform.InstanceState{
Attributes: map[string]string{
"foo": "bar",
},
},
Config: testConfig(t, map[string]interface{}{
"foo": "baz",
}),
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"foo": &terraform.ResourceAttrDiff{
Old: "bar",
New: "baz",
},
},
},
Key: "bad",
NewValue: "qux",
ExpectedError: true,
},
} }
} }