validate null values in shimmed configs

A list-like attribute containing null values will present a list to
helper/schema with nils, which can cause panics. Since null values were
not possible in configuration before HCL2 and not supported by the
legacy SDK, return an error to the user.
This commit is contained in:
James Bardin 2019-04-02 16:53:51 -04:00
parent 7ced46425e
commit f5395bd98a
2 changed files with 148 additions and 0 deletions

View File

@ -183,6 +183,12 @@ func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto
return resp, nil
}
// Ensure there are no nulls that will cause helper/schema to panic.
if err := validateConfigNulls(configVal, nil); err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
config := terraform.NewResourceConfigShimmed(configVal, blockForShimming)
schema.FixupAsSingleResourceConfigIn(config, s.provider.Schema)
@ -233,6 +239,12 @@ func (s *GRPCProviderServer) ValidateDataSourceConfig(_ context.Context, req *pr
return resp, nil
}
// Ensure there are no nulls that will cause helper/schema to panic.
if err := validateConfigNulls(configVal, nil); err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
config := terraform.NewResourceConfigShimmed(configVal, blockForShimming)
schema.FixupAsSingleResourceConfigIn(config, s.provider.DataSourcesMap[req.TypeName].Schema)
@ -424,6 +436,12 @@ func (s *GRPCProviderServer) Configure(_ context.Context, req *proto.Configure_R
s.provider.TerraformVersion = req.TerraformVersion
// Ensure there are no nulls that will cause helper/schema to panic.
if err := validateConfigNulls(configVal, nil); err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
config := terraform.NewResourceConfigShimmed(configVal, blockForShimming)
schema.FixupAsSingleResourceConfigIn(config, s.provider.Schema)
err = s.provider.Configure(config)
@ -553,6 +571,12 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
priorState.Meta = priorPrivate
// Ensure there are no nulls that will cause helper/schema to panic.
if err := validateConfigNulls(proposedNewStateVal, nil); err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
// turn the proposed state into a legacy configuration
cfg := terraform.NewResourceConfigShimmed(proposedNewStateVal, blockForShimming)
schema.FixupAsSingleResourceConfigIn(cfg, s.provider.ResourcesMap[req.TypeName].Schema)
@ -978,6 +1002,12 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa
Type: req.TypeName,
}
// Ensure there are no nulls that will cause helper/schema to panic.
if err := validateConfigNulls(configVal, nil); err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
config := terraform.NewResourceConfigShimmed(configVal, blockForShimming)
schema.FixupAsSingleResourceConfigIn(config, s.provider.DataSourcesMap[req.TypeName].Schema)
@ -1291,3 +1321,52 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
return dst
}
// validateConfigNulls checks a config value for unsupported nulls before
// attempting to shim the value. While null values can mostly be ignored in the
// configuration, since they're not supported in HCL1, the case where a null
// appears in a list-like attribute (list, set, tuple) will present a nil value
// to helper/schema which can panic. Return an error to the user in this case,
// indicating the attribute with the null value.
func validateConfigNulls(v cty.Value, path cty.Path) []*proto.Diagnostic {
var diags []*proto.Diagnostic
if v.IsNull() || !v.IsKnown() {
return diags
}
switch {
case v.Type().IsListType() || v.Type().IsSetType() || v.Type().IsTupleType():
it := v.ElementIterator()
for it.Next() {
kv, ev := it.Element()
if ev.IsNull() {
diags = append(diags, &proto.Diagnostic{
Severity: proto.Diagnostic_ERROR,
Summary: "null value found in list",
Attribute: convert.PathToAttributePath(append(path, cty.IndexStep{Key: kv})),
})
continue
}
d := validateConfigNulls(ev, append(path, cty.IndexStep{Key: kv}))
diags = convert.AppendProtoDiag(diags, d)
}
case v.Type().IsMapType() || v.Type().IsObjectType():
it := v.ElementIterator()
for it.Next() {
kv, ev := it.Element()
var step cty.PathStep
switch {
case v.Type().IsMapType():
step = cty.IndexStep{Key: kv}
case v.Type().IsObjectType():
step = cty.GetAttrStep{Name: kv.AsString()}
}
d := validateConfigNulls(ev, append(path, step))
diags = convert.AppendProtoDiag(diags, d)
}
}
return diags
}

View File

@ -3,6 +3,7 @@ package plugin
import (
"context"
"fmt"
"strconv"
"strings"
"testing"
"time"
@ -11,6 +12,7 @@ import (
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/terraform/helper/schema"
proto "github.com/hashicorp/terraform/internal/tfplugin5"
"github.com/hashicorp/terraform/plugin/convert"
"github.com/hashicorp/terraform/terraform"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/msgpack"
@ -1010,3 +1012,70 @@ func TestNormalizeNullValues(t *testing.T) {
})
}
}
func TestValidateNulls(t *testing.T) {
for i, tc := range []struct {
Cfg cty.Value
Err bool
}{
{
Cfg: cty.ObjectVal(map[string]cty.Value{
"list": cty.ListVal([]cty.Value{
cty.StringVal("string"),
cty.NullVal(cty.String),
}),
}),
Err: true,
},
{
Cfg: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapVal(map[string]cty.Value{
"string": cty.StringVal("string"),
"null": cty.NullVal(cty.String),
}),
}),
Err: false,
},
{
Cfg: cty.ObjectVal(map[string]cty.Value{
"object": cty.ObjectVal(map[string]cty.Value{
"list": cty.ListVal([]cty.Value{
cty.StringVal("string"),
cty.NullVal(cty.String),
}),
}),
}),
Err: true,
},
{
Cfg: cty.ObjectVal(map[string]cty.Value{
"object": cty.ObjectVal(map[string]cty.Value{
"list": cty.ListVal([]cty.Value{
cty.StringVal("string"),
cty.NullVal(cty.String),
}),
"list2": cty.ListVal([]cty.Value{
cty.StringVal("string"),
cty.NullVal(cty.String),
}),
}),
}),
Err: true,
},
} {
t.Run(strconv.Itoa(i), func(t *testing.T) {
d := validateConfigNulls(tc.Cfg, nil)
diags := convert.ProtoToDiagnostics(d)
switch {
case tc.Err:
if !diags.HasErrors() {
t.Fatal("expected error")
}
default:
if diags.HasErrors() {
t.Fatalf("unexpected error: %q", diags.Err())
}
}
})
}
}