std.parseYaml

[sbarzowski]

Importing YAML files was requested by multiple people, it was proposed in #196 and popped up in various discussions. Also YAML files are very popular in the Kubernetes communities which are heavy users of jsonnet.

Implementing library function (probably builtin) parseYaml would allow processing yaml files with jsonnet. It would require importing it first through importstr then invoking parseYaml on the result. Later we can think about automatically applying it based on extension or something.

Relatedly imporstr and parseJson should be used rather than import when dealing with json (not jsonnet) files, especially if they are not 100% trusted (consider imporstr "/etc/passwd").

Challenges:

• YAML is complicated, making it very difficult to keep byte-for-byte compatibility
• The existing parsers may be slightly non-compliant
• Safe vs unsafe yaml

@benley, @nikolay

[anguslees]

Fwiw, here's an example implementation:
https://github.com/ksonnet/kubecfg/blob/5197d89819dfe3527e29cc4cbb0afc3fec9e72f5/utils/nativefuncs.go#L63

In particular, note that a YAML file is actually an array (aka stream) of YAML-encoded values, which implies a function signature slightly different to an otherwise-similar parseJson

[sbarzowski]

Nice

In particular, note that a YAML file is actually an array (aka stream) of YAML-encoded values, which implies a function signature slightly different to an otherwise-similar parseJson

Perhaps we should have both parseYamlStream and parseYaml then.

[PaulRudin]

Regarding the importing of json. Presumably this is just a recommendation, since valid json is valid jsonnet, there's nothing to stop people using import on json files?

[sparkprime]

Yes it'd be a way to allow people to write scripts that can import "untrusted" JSON, i.e. JSON that worst case has bad values in it, not JSON that reads weird files. Although, to be honest, we should probably just sandbox the filesystem, e.g. not allow imports to escape the CWD.

[teralype]

Any progress on this?

[sbarzowski]

The std.parseJson part is released. There is no concrete progress on parseYaml/parseYamlStream. We currently have quite a few things in the queue, so it's going to be a while until we get to it. Ofc we're happy to accept PRs.

[aglover-zendesk]

Although, to be honest, we should probably just sandbox the filesystem, e.g. not allow imports to escape the CWD.

Feature request - please don't do this. I'd rather do imporstr and parseJson or something else to ensure security, rather than be restricted to CWD and child directories. Directory limitations is one of the reasons Kustomize is so unpleasantly restrictive. Storing shared JSONNET libraries and templates outside of the CWD has been a useful pattern.

[sbarzowski]

@aglover-zendesk

Just to be clear, it would still be possible to explicitly add the directory that you are interested in (by adding them to Jsonnet library path, e.g. jsonnet -J "/home/my_user/my_jsonet_stuff"). The proposal was to disallow things like import "/some/absolute/path" or import "../../../../../../etc/passwd". Do you think that would be annoying?

[jjo]

It would be pretty impactful for us, we use something like <cluster>/<namespace>/component.jsonnet importing e.g. ../../common/component.libsonnet while relying on -J for "base" libraries.
Obviously non-issue if it'd be behind a runtime flag.
Note that projects embedding jsonnet are other stakeholders on this policy/change.

[jjo]

It would be pretty impactful for us, we use something like <cluster>/<namespace>/component.jsonnet importing e.g. ../../common/component.libsonnet while relying on -J for "base" libraries.
Obviously non-issue if it'd be behind a runtime flag.
Note that projects embedding jsonnet are other stakeholders on this policy/change.

[aglover-zendesk]

@sbarzowski using the -J flag for importing content from other dirs seems reasonable. Alternatively having a separate flag to disable the CWD limitation would work, that way users would have to opt in to use the less secure behavior.

[sbarzowski]

Thanks for the feedback. FYI there are no immediate plans to enforce FS sandboxing for importing and if we come back to this in the future, we'll try to do it in the least disruptive way.

[underrun]

is there any sense of a timeline on when this issue might be more deeply considered?

[sbarzowski]

We already have parseJson. I would like to have parseYaml in the next release. We have go-jsonnet implementation ready. We need to add it here as well. There is more discussion about it in in google/go-jsonnet#339.

[pmorch]

std.parseYaml() exists in the cpp version in the master branch since commit da1490f. It was introduced by the merge of #899.

Interestingly, the documentation says it is:

Available since version x.y.z.

No official release contains commit da1490f yet.

[igorwwwwwwwwwwwwwwwwwwww]

That cpp commit landed in https://github.com/google/jsonnet/releases/tag/v0.18.0.

[sbarzowski]

To be closed when we update the docs

[groodt]

Can't this be closed now @sbarzowski ?

The docs website does include parseYaml...

[sbarzowski]

@groodt thanks!