From 76123f1575b0026de148d065bc2164c0c2828e28 Mon Sep 17 00:00:00 2001 From: Pat Gavlin Date: Tue, 23 Jan 2018 13:37:44 -0800 Subject: [PATCH] Fix set marshaling. 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..*`. When we were constructing the diffs ourselves, we were able to ensure that each `` 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. --- Gopkg.lock | 2 +- pkg/tfbridge/schema.go | 105 ++++++++++++++++++++++++++++++++++------- 2 files changed, 89 insertions(+), 18 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 5a6b3f154..b20db28f9 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -535,6 +535,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "c34abb95649cd47795739efe9ee9ff7ce991ffde3223fcc85cf79dd41ebf0c97" + inputs-digest = "0ac6a477b8d23b640e631ca905d3dd0365f3be07b664826ede7860baaeb4dff9" solver-name = "gps-cdcl" solver-version = 1 diff --git a/pkg/tfbridge/schema.go b/pkg/tfbridge/schema.go index fab3aebe0..5c234fdef 100644 --- a/pkg/tfbridge/schema.go +++ b/pkg/tfbridge/schema.go @@ -4,6 +4,7 @@ package tfbridge import ( "fmt" + "strconv" "github.com/golang/glog" @@ -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] @@ -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. @@ -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