Skip to content

Commit

Permalink
Fix set marshaling.
Browse files Browse the repository at this point in the history
With the recent change to use provider diffs in `Update`, we ceased to
produce usable inputs for sets. The break involves the specific
representation of set members in a `terraform.InstanceState` value: each
set member is flattened into a set of key-value pairs under the key
`prefix.<hash-of-member>.*`. When we were constructing the diffs
ourselves, we were able to ensure that each `<hash-of-member>` component
lined up betweemn the old state and the new config (note that this was
itself perhaps subtly wrong, as TF would likely have considered a change
to a set member to have changed its hash and thus its identity; most
changes of this sort in TF appear as a set member removal and a set member
addition). We lost this capability when we moved to TF diffs: when using
TF diffs, the TF state presented to diff must use TF's typical encoding
for set elements.

These changes reimplement old state -> TF state marshaling s.t. set
elements are flattened with the correct prefix. This turned out to be
much more challenging than it ought to be, as TF does not directly
expose the APIs used to compute set member hashes. At first, I attempted
to use a `MapFieldWriter` to construct the flat map, but this particular
type appears to have some problems handling `TypeMap` values (namely, it
expects each field of a `TypeMap` value to be represented as a
`map[string]interface{}`, which is clearly incorrect). Instead, I chose
to use the TF set member hashing functions directly, which required two
irritating adjustments:
- These hash functions expect any set elements to be represented using
  whatever types would have been produced by a `FieldReader`. As a
  result, the state goes through the following transforms:
    grpc.Struct -> pulumi.PropertyMap -> TF inputs -> TF config
  The resulting TF config is then fed into a `ConfigFieldReader`. The
  top-level values are read out and flattened in order to produce the TF
  attributes.
- These has functions do not produce the exact hash as used by TF: if
  the value they produce is negative, TF will invert it s.t. it is
  always positive. We do the same.

With these changes, I am able to update a CloudFront distribution's
caching rules (which involves set marshaling) without issue.
  • Loading branch information
pgavlin committed Jan 24, 2018
1 parent 6bf1e00 commit 76123f1
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 18 deletions.
2 changes: 1 addition & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

105 changes: 88 additions & 17 deletions pkg/tfbridge/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package tfbridge

import (
"fmt"
"strconv"

"github.com/golang/glog"

Expand Down Expand Up @@ -145,22 +146,23 @@ func MakeTerraformInput(res *PulumiResource, name string,
oldArr = old.ArrayValue()
}

var etfs *schema.Schema
if tfs != nil {
if sch, issch := tfs.Elem.(*schema.Schema); issch {
etfs = sch
} else if _, isres := tfs.Elem.(*schema.Resource); isres {
// The IsObject case below expects a schema whose `Elem` is
// a Resource, so just pass the full List schema
etfs = tfs
}
}
var eps *SchemaInfo
if ps != nil {
eps = ps.Elem
}

var arr []interface{}
for i, elem := range v.ArrayValue() {
var etfs *schema.Schema
if tfs != nil {
if sch, issch := tfs.Elem.(*schema.Schema); issch {
etfs = sch
} else if _, isres := tfs.Elem.(*schema.Resource); isres {
// The IsObject case below expects a schema whose `Elem` is
// a Resource, so just pass the full List schema
etfs = tfs
}
}
var eps *SchemaInfo
if ps != nil {
eps = ps.Elem
}
var oldElem resource.PropertyValue
if i < len(oldArr) {
oldElem = oldArr[i]
Expand Down Expand Up @@ -376,7 +378,7 @@ func MakeTerraformAttributes(res *PulumiResource, m resource.PropertyMap,
if err != nil {
return nil, err
}
return MakeTerraformAttributesFromInputs(inputs), nil
return MakeTerraformAttributesFromInputs(inputs, tfs)
}

// MakeTerraformAttributesFromRPC unmarshals an RPC property map and calls through to MakeTerraformAttributes.
Expand All @@ -391,9 +393,78 @@ func MakeTerraformAttributesFromRPC(res *PulumiResource, m *pbstruct.Struct,
return MakeTerraformAttributes(res, props, tfs, ps, defaults)
}

func flattenValue(result map[string]string, prefix string, value interface{}) {
if value == nil {
return
}

switch t := value.(type) {
case bool:
if t {
result[prefix] = "true"
} else {
result[prefix] = "false"
}
case int:
result[prefix] = strconv.FormatInt(int64(t), 10)
case float64:
result[prefix] = strconv.FormatFloat(t, 'G', -1, 64)
case string:
result[prefix] = t
case []interface{}:
// Flatten each element.
for i, elem := range t {
flattenValue(result, prefix+"."+strconv.FormatInt(int64(i), 10), elem)
}

// Set the count.
result[prefix+".#"] = strconv.FormatInt(int64(len(t)), 10)
case *schema.Set:
// Flatten each element.
setList := t.List()
for _, elem := range setList {
code := t.F(elem)
if code < 0 {
code = -code
}

flattenValue(result, prefix+"."+strconv.Itoa(code), elem)
}

// Set the count.
result[prefix+".#"] = strconv.FormatInt(int64(len(setList)), 10)
case map[string]interface{}:
for k, v := range t {
flattenValue(result, prefix+"."+k, v)
}

// Set the count.
result[prefix+".%"] = strconv.Itoa(len(t))
default:
contract.Failf("Unexpected TF input value: %v", t)
}
}

// MakeTerraformAttributesFromInputs creates a flat Terraform map from a structured set of Terraform inputs.
func MakeTerraformAttributesFromInputs(inputs map[string]interface{}) map[string]string {
return flatmap.Flatten(inputs)
func MakeTerraformAttributesFromInputs(inputs map[string]interface{},
tfs map[string]*schema.Schema) (map[string]string, error) {

cfg, err := MakeTerraformConfigFromInputs(inputs)
if err != nil {
return nil, err
}

result := make(map[string]string)
reader := &schema.ConfigFieldReader{Config: cfg, Schema: tfs}
for k := range inputs {
f, err := reader.ReadField([]string{k})
if err != nil {
return nil, errors.Wrapf(err, "could not read field %v", k)
}

flattenValue(result, k, f.Value)
}
return result, nil
}

// MakeTerraformDiff takes a bag of old and new properties, and returns two things: the existing resource's state as
Expand Down

0 comments on commit 76123f1

Please sign in to comment.