Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: Make json structs exported #332

Closed
wata727 opened this issue Jan 3, 2020 · 3 comments
Closed

Request: Make json structs exported #332

wata727 opened this issue Jan 3, 2020 · 3 comments
Labels
enhancement syntax/json v2 Relates to the v2 line of releases

Comments

@wata727
Copy link
Contributor

wata727 commented Jan 3, 2020

Unlike hclsyntax, the json struct is unexported:

type body struct {

type expression struct {

type node interface {

Can we make these exported? If possible, I'm going to make a PR.

Background

I'm planning to build a plugin to send and receive structs that satisfy hcl.Expression with go-plugin. However, to handle an interface, we need to gob.Register structs that satisfies it.

hclsyntax is exported, so we can gob.Regitser from outside of the package, but json is not exported, so we cannot avoid the following errors:

16:16:12 server.go:418: rpc: gob error encoding body: gob: type not registered for interface: json.expression
@apparentlymart apparentlymart added enhancement syntax/json v2 Relates to the v2 line of releases labels Jan 3, 2020
@apparentlymart
Copy link
Contributor

Hi @wata727! Thanks for sharing this use-case.

I'm a little concerned with the idea of sending expression nodes via gob because that would make their current shape a compatibility constraint for future versions. The hclsyntax nodes are exported because they directly represent concepts from the HCL specification that applications might want to interact with, but the intermediate JSON representations we use in the json package are more of an implementation detail and not something I'd want callers to depend on in case we want to change that implementation in the future.

The problem of sending not-yet-evaluated expressions over the wire to be evaluated later was something I started exploring in hclpack prior to the stable release of HCL 2, but that wasn't really a successful experiment because it added considerable bloat to the input in its wire encoding and I couldn't find a reasonable way to implement packing of JSON configuration with it. Therefore we dropped that package as part of preparing the stable 2.0 release. 😖

With that said, I continue to believe that the best way to send HCL expressions over a wire protocol for evaluation elsewhere is in their source form rather than in their decoded form, because the source syntax is already a compatibility constraint and is already pretty compact. The challenge is what makes a good API for dealing with that on both the producer and consumer sides, which each have some challenges:

  • It's not possible in general to go from an hcl.Expression to the source code that produced it, though in practice if you are using hclsyntax and json directly (not via any higher-level abstractions like the dynamic blocks extension) you can in principle use the hcl.Range associated with an expression to slice out the bytes that created it from the source file, in the same way that the diagnostic message printer does.

  • When dealing with both native and JSON syntax, the result of slicing out the expression bytes will be different for each of them, because in JSON the expression will always be part of a JSON string:

    "${foo}"
    

    That means the code that is producing these strings needs to know which syntax is being used by each file and needs to include that as an annotation for the byte slice so that the consumer knows how to decode it.

  • On the consumer side, the consumer code needs to check whether the given bytes are a native syntax expression or a JSON expression and call into the appropriate package to produce the corresponding hcl.Expression.

    A missing piece of this currently is that the json package only includes a function for parsing a "file", analogous to hclsyntax.ParseConfig. To support passing JSON-based expressions over the network would require a new json.ParseExpression function that can take an expression as would be expected by the JSON parser as part of a bigger configuration file and return an hcl.Expression for it directly. It would need to take an hcl.Pos as an argument, just like hclsyntax.ParseExpression does, so that the consumer can preserve the original source location information while it parses and thus produce accurate diagnostic messages if there are errors.

I'd rather continue down this investigation path than create new compatibility constraints on the implementation details of the json package, but unfortunately I don't have more time to spend researching this right now. However, if you're interested to look into it I'd be happy to help as best I can.

@wata727
Copy link
Contributor Author

wata727 commented Jan 4, 2020

@apparentlymart Thank you for your quick reply!

I totally agree with your opinion that the source code is the preferred format for transmission.
I hope to work on implementing the json.ParseExpression function in the near future.

By the way, I think a similar problem can be considered in hcl.Block etc. What do you think about it? Should we implement the ParseBlock function?

@apparentlymart
Copy link
Contributor

Hi @wata727,

The HCL decoding API was designed to ensure that only attribute values can require dynamic data from an hcl.EvalContext, in order to allow a top-level decoder to handle the structural analysis to see what blocks and attributes are present but to delay evaluation of the expressions. Based on implementation experience so far (with Terraform, etc) I'd suggest handling body content in a different way, by having a defined schema that the main part of your application can use to call body.Content and get access to the expressions, and then use the techniques we've been discussing here to send those individual expressions over the wire for final evaluation.

While sending the source code for a specific block over a wire protocol could work in principle, it seems like quite a complicated way to handle it and I expect it would have a lot more edge-cases to deal with.

Terraform takes an approach like this with its provider plugins: the plugins are required to report to Terraform ahead of time what schema they are expecting, and then Terraform handles the decoding step using that schema. In Terraform's case the expression evaluation also happens in prior to calling the provider plugins, but expression evaluation is a separate step so in principle it could be done on the provider plugin side, using a mechanism like we're describing, without affecting how the overall structure is decoded.

With that said, I think our existing hclsyntax.ParseConfig and json.Parse functions could already decode such a thing, because a block (or the contents thereof) should be parseable in the main parsing mode. The separate expression parsing function is needed just because expression parsing is a separate mode in HCL, and so the separate function allows us to activate that mode immediately when parsing begins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement syntax/json v2 Relates to the v2 line of releases
Projects
None yet
Development

No branches or pull requests

2 participants