Do not expose unknowns to resource providers.

Before these changes, we were inconsistent in our treatment of unknown
property values across the resource provider RPC interface. `Check` and
`Diff` were retaining unknown properties in inputs and outputs;
`Create`, `Update`, and `Delete` were not. This interacted badly with
recent changes to `Check` to return all provider inputs--i.e. not just
defaults--from that method: if an unknown input was provided, it would
be present in the returned inputs, which would eventually confuse the
differ by giving the appearance of changes where none were present.

These changes remove unknowns from the provider interface entirely:
unknown property values are never passed to a provider, and a provider
must never return an unknown property value.

This is the primary piece of the fix for pulumi/pulumi-terraform#93.
This commit is contained in:
pat@pulumi.com 2018-01-09 12:21:47 -08:00
parent bb533c6547
commit f3cb37ef95
3 changed files with 38 additions and 18 deletions

View file

@ -76,13 +76,11 @@ func (p *provider) Check(urn resource.URN,
label := fmt.Sprintf("%s.Check(%s)", p.label(), urn)
glog.V(7).Infof("%s executing (#olds=%d,#news=%d", label, len(olds), len(news))
molds, err := MarshalProperties(olds, MarshalOptions{
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true})
molds, err := MarshalProperties(olds, MarshalOptions{Label: fmt.Sprintf("%s.olds", label)})
if err != nil {
return nil, nil, err
}
mnews, err := MarshalProperties(news, MarshalOptions{
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true})
mnews, err := MarshalProperties(news, MarshalOptions{Label: fmt.Sprintf("%s.news", label)})
if err != nil {
return nil, nil, err
}
@ -101,7 +99,7 @@ func (p *provider) Check(urn resource.URN,
var inputs resource.PropertyMap
if ins := resp.GetInputs(); ins != nil {
inputs, err = UnmarshalProperties(ins, MarshalOptions{
Label: fmt.Sprintf("%s.inputs", label), KeepUnknowns: true})
Label: fmt.Sprintf("%s.inputs", label), RejectUnknowns: true})
if err != nil {
return nil, nil, err
}
@ -133,8 +131,7 @@ func (p *provider) Diff(urn resource.URN, id resource.ID,
if err != nil {
return DiffResult{}, err
}
mnews, err := MarshalProperties(news, MarshalOptions{
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true})
mnews, err := MarshalProperties(news, MarshalOptions{Label: fmt.Sprintf("%s.news", label)})
if err != nil {
return DiffResult{}, err
}
@ -177,8 +174,7 @@ func (p *provider) Create(urn resource.URN, props resource.PropertyMap) (resourc
label := fmt.Sprintf("%s.Create(%s)", p.label(), urn)
glog.V(7).Infof("%s executing (#props=%v)", label, len(props))
mprops, err := MarshalProperties(props, MarshalOptions{
Label: fmt.Sprintf("%s.inputs", label)})
mprops, err := MarshalProperties(props, MarshalOptions{Label: fmt.Sprintf("%s.inputs", label)})
if err != nil {
return "", nil, resource.StatusOK, err
}
@ -199,7 +195,7 @@ func (p *provider) Create(urn resource.URN, props resource.PropertyMap) (resourc
}
outs, err := UnmarshalProperties(resp.GetProperties(), MarshalOptions{
Label: fmt.Sprintf("%s.outputs", label)})
Label: fmt.Sprintf("%s.outputs", label), RejectUnknowns: true})
if err != nil {
return "", nil, resource.StatusUnknown, err
}
@ -224,8 +220,7 @@ func (p *provider) Update(urn resource.URN, id resource.ID,
if err != nil {
return nil, resource.StatusOK, err
}
mnews, err := MarshalProperties(news, MarshalOptions{
Label: fmt.Sprintf("%s.news", label)})
mnews, err := MarshalProperties(news, MarshalOptions{Label: fmt.Sprintf("%s.news", label)})
if err != nil {
return nil, resource.StatusOK, err
}
@ -244,7 +239,7 @@ func (p *provider) Update(urn resource.URN, id resource.ID,
}
outs, err := UnmarshalProperties(resp.GetProperties(), MarshalOptions{
Label: fmt.Sprintf("%s.outputs", label)})
Label: fmt.Sprintf("%s.outputs", label), RejectUnknowns: true})
if err != nil {
return nil, resource.StatusUnknown, err
}
@ -289,8 +284,7 @@ func (p *provider) Invoke(tok tokens.ModuleMember, args resource.PropertyMap) (r
label := fmt.Sprintf("%s.Invoke(%s)", p.label(), tok)
glog.V(7).Infof("%s executing (#args=%d)", label, len(args))
margs, err := MarshalProperties(args, MarshalOptions{
Label: fmt.Sprintf("%s.args", label), KeepUnknowns: true})
margs, err := MarshalProperties(args, MarshalOptions{Label: fmt.Sprintf("%s.args", label)})
if err != nil {
return nil, nil, err
}
@ -303,7 +297,7 @@ func (p *provider) Invoke(tok tokens.ModuleMember, args resource.PropertyMap) (r
// Unmarshal any return values.
ret, err := UnmarshalProperties(resp.GetReturn(), MarshalOptions{
Label: fmt.Sprintf("%s.returns", label), KeepUnknowns: true})
Label: fmt.Sprintf("%s.returns", label), RejectUnknowns: true})
if err != nil {
return nil, nil, err
}

View file

@ -19,6 +19,7 @@ type MarshalOptions struct {
Label string // an optional label for debugging.
SkipNulls bool // true to skip nulls altogether in the resulting map.
KeepUnknowns bool // true if we are keeping unknown values (otherwise we skip them).
RejectUnknowns bool // true if we should return errors on unknown values. Takes precedence over KeepUnknowns.
ElideAssetContents bool // true if we are eliding the contents of assets.
ComputeAssetHashes bool // true if we are computing missing asset hashes on the fly.
}
@ -114,7 +115,9 @@ func MarshalPropertyValue(v resource.PropertyValue, opts MarshalOptions) (*struc
}
return MarshalStruct(obj, opts), nil
} else if v.IsComputed() {
if opts.KeepUnknowns {
if opts.RejectUnknowns {
return nil, errors.New("unexpected unknown property value")
} else if opts.KeepUnknowns {
return marshalUnknownProperty(v.ComputedValue().Element, opts), nil
}
return nil, nil // return nil and the caller will ignore it.
@ -216,7 +219,9 @@ func UnmarshalPropertyValue(v *structpb.Value, opts MarshalOptions) (*resource.P
// If it's a string, it could be an unknown property, or just a regular string.
s := v.GetStringValue()
if unk, isunk := unmarshalUnknownPropertyValue(s, opts); isunk {
if opts.KeepUnknowns {
if opts.RejectUnknowns {
return nil, errors.New("unexpected unknown property value")
} else if opts.KeepUnknowns {
return &unk, nil
}
return nil, nil

View file

@ -88,3 +88,24 @@ func TestComputedSkip(t *testing.T) {
assert.Nil(t, cprop)
}
}
func TestComputedReject(t *testing.T) {
// Ensure that computed properties produce errors when RejectUnknowns == true.
opts := MarshalOptions{RejectUnknowns: true}
{
cprop, err := MarshalPropertyValue(
resource.NewComputedProperty(
resource.Computed{Element: resource.NewStringProperty("")}), opts)
assert.NotNil(t, err)
assert.Nil(t, cprop)
}
{
cprop, err := MarshalPropertyValue(
resource.NewComputedProperty(
resource.Computed{Element: resource.NewStringProperty("")}), MarshalOptions{KeepUnknowns: true})
assert.Nil(t, err)
cpropU, err := UnmarshalPropertyValue(cprop, opts)
assert.NotNil(t, err)
assert.Nil(t, cpropU)
}
}