jsonnet icon indicating copy to clipboard operation
jsonnet copied to clipboard

std.parseYaml

Open sbarzowski opened this issue 7 years ago • 17 comments

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

sbarzowski avatar Feb 22 '18 23:02 sbarzowski

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

anguslees avatar Mar 05 '18 06:03 anguslees

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.

sbarzowski avatar Mar 05 '18 09:03 sbarzowski

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?

PaulRudin avatar Oct 10 '19 08:10 PaulRudin

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.

sparkprime avatar Oct 10 '19 20:10 sparkprime

Any progress on this?

teralype avatar Nov 05 '19 12:11 teralype

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.

sbarzowski avatar Nov 05 '19 19:11 sbarzowski

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.

aglover-zendesk avatar Dec 06 '19 18:12 aglover-zendesk

@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?

sbarzowski avatar Dec 07 '19 07:12 sbarzowski

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 avatar Dec 07 '19 12:12 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 avatar Dec 07 '19 13:12 jjo

@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.

aglover-zendesk avatar Dec 08 '19 02:12 aglover-zendesk

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.

sbarzowski avatar Dec 08 '19 09:12 sbarzowski

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

underrun avatar Jan 07 '21 21:01 underrun

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 https://github.com/google/go-jsonnet/pull/339.

sbarzowski avatar Jan 08 '21 11:01 sbarzowski

std.parseYaml() exists in the cpp version in the master branch since commit da1490f6. 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 da1490f6 yet.

pmorch avatar Oct 18 '21 07:10 pmorch

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

To be closed when we update the docs

sbarzowski avatar Mar 03 '22 22:03 sbarzowski

Can't this be closed now @sbarzowski ?

The docs website does include parseYaml...

groodt avatar Sep 07 '22 01:09 groodt

@groodt thanks!

sbarzowski avatar Sep 11 '22 15:09 sbarzowski