Proposal: JSON serialization of `cty.Path`

[wata727]

Background

I would like to transfer the marked cty.Value over a wire protocol so that the plugin can handle marked as sensitive values in Terraform. Currently, marked values cannot be serialized by either JSON/msgpack, so it just returns an error. https://github.com/terraform-linters/tflint/blob/v0.45.0/plugin/server.go#L128-L137

cty.Value is marshaled/unmarshaled here: https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.15.0/plugin/plugin2host/server.go#L159 https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.15.0/plugin/plugin2host/client.go#L329

As per Marks Under Serialization, I'm thinking of the following code that serializes marks in a different way:

unmarked, pvm := value.UnmarkDeepWithPaths()

serializedVal, _ := msgpack.Marshal(unmarked, ty)
serializedMarks := make([]SerializedMark, len(pvm))
for idx, v := range pvm {
  serializedMarks[idx].Marks = marshalMarks(v.Marks)
  serializedMarks[idx].Path = marshalPath(v.Path)
}

Response{Value: serializedValue, Marks: serializedMarks}

// plugin side

val, _ := msgpack.Unmarshal(resp.Value, ty)
pvm := make([]cty.PathValueMarks, len(resp.Marks))
for idx, v := range resp.Marks {
  pvm[idx].Marks = unmarshalMarks(v.Marks)
  pvm[idx].Path = unmarshalPath(v.Path)
}

val.MarkWithPaths(pvm)

Looking at the above, marshalMarks and unmarshalMarks is not trivial, but marshalPath and unmarshalPath seem like functions provided by cty package.

Proposal

Add json.MarshalPath just like json.MarshalType. Imagine something like this:

json.MarshalPath(cty.IndexStringPath("boop"))
// => [{"type": "IndexStep", "key": "boop"}]

json.MarshalPAth(cty.GetAttrPath("foo"))
// => [{"type": "GetAttrStep", "name": "foo"}]

What do you think? If this proposal is accepted, I can open a pull request. Thanks!

[wata727]

I found that Terraform declares a proto message to serialize cty.Path. https://github.com/hashicorp/terraform/blob/v1.3.9/docs/plugin-protocol/tfplugin6.3.proto#L43-L56

Is this approach preferable?

[apparentlymart]

Hi @wata727,

Terraform also has some functions that produce JSON representation of paths, it seems: https://github.com/hashicorp/terraform/blob/729408333e3c0de2ae42207bb9147bf240cfad7b/internal/command/jsonplan/plan.go#L824

It feels a little weird to me to implement a "standard" JSON serialisation of a concept that doesn't really exist in JSON. Of course the same argument could be made for types, but those ended up having a JSON serialisation because it was necessary for it to be possible to encode to DynamicPseudoType; it doesn't seem so important for this package to define a serialisation for paths because each application can decide its own serialisation depending on what it needs to capture. For example, it seems like what Terraform is doing in the code I linked above is "lossy", I think because this Terraform format is designed with the assumption that the consumer knows the resource type schema and so can infer automatically what each element of the path represents.

As you saw, Terraform also uses a protocol buffers version of this information in the plugin wire protocol, which I think was desirable because it avoided an extra level of encoding and decoding since this protocol is already using protocol buffers for other reasons.

Therefore my instinct is to not define some fixed serialisation of these paths and instead leave each application to choose a suitable serialisation based on its own constraints and requirements.

Would that work for you, or does your goal rely on this being built in to this library? Thanks!

[wata727]

Thank you for your reply. At first, I thought this proposal was reasonable because I didn't understand why only cty.Type and cty.Value implement a JSON serialization, but your explanation made me understand.

As with Terraform, I'm going to implement our own serialization in our application. Thanks again!