KEP-5295: KYAML #5296
KEP-5295: KYAML #5296
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
kind/kep
k8s-ci-robot
added
the
size/XXL
|
@thockin: GitHub didn't allow me to request PR reviews from the following users: sftim. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
typos fixed and small changes to clarify strings |
thockin
force-pushed
the
kyaml
branch
2 times, most recently
from
fc2ba3c to
0b92cc3
Compare
|
For multi-line strings I added a variation of option 2 with slightly different indenting. |
| which they copy and modify. | ||
| By changing the examples we seed the change in the ecosystem. | ||
| One success metric might be | ||
| that KYAML becomes the preferred format in the most common helm charts. |
There was a problem hiding this comment.
I don't see how adopting KYAML for Helm chart templates is a tenable idea.
example https://github.com/bitnami/charts/blob/main/bitnami/appsmith/templates/client/deployment.yaml
Getting the Go templating to work correctly is already challenging.
Adding more curly braces to the curly brace soup is not going to be a nice DX.
Cuddling closing YAML curlies makes it worse.
There was a problem hiding this comment.
I adapted some of that helm chart and it's (IMO) easier to read and gets rid of all the | nindent 8 crap.
There was a problem hiding this comment.
That is about all it can get rid of while adding a lot more curlies.
But I would guess that if kyaml is used with Go templating people will still want the indentation to make helm template ... output readable, right?
There was a problem hiding this comment.
You can run the output of that thru yamlfmt and get either canonical YAML or KYAML.
Yes, the input has strictly more curlies, squares, quotes, and commas, but the result is that you can avoid having to think about the hardest to debug (IMO) problem, the one that has no visible characters involved .
Panacea? No. It's a tradeoff.
| Integers and floats are rendered as their natural numeric representation. | ||
|
|
||
| Simple strings are rendered as double-quoted and escaped. Multi-line strings | ||
| require special consideration, see below. |
There was a problem hiding this comment.
Double-quoting is often problematic in YAML.
Using them for all string values will require backslash escaping of all double-quotes and backslashes, so code snippets will get harder to write, read and understand.
Given the backslash requirements that KYAML adds for multiline scalars, it will get messier.
Imagine a KYAML formatted scalar that itself is a KYAML formatted multiline scalar containing code with double-quotes and backslashes.
The backlashes will become problematic. Especially for newer users for which KYAML was created to protect.
YAML's literal scalar syntax allows you to encode any multiline text string (including other YAML files) by simply indenting.
Allowing single-quoting could help a little here. YAML's single quote only needs escaping for one character; the single quote.
Multilines wouldn't work the same way with single-quotes, as double-quotes.
There was a problem hiding this comment.
YAML's literal scalar syntax allows you to encode any multiline text string (including other YAML files) by simply indenting.
Not once you are inside a flow, though?
go run ./cmd/yamlfmt/
foo: {
bar: "quoted",
qux: |
non-quoted
failed to decode input: yaml: line 3: found character that cannot start any token
exit status 1
There was a problem hiding this comment.
Right, I was just pointing out that enforcing an all flow style, forgoes literal, which is likely going to be an ongoing pain point for this formatting.
It's not a coincidence that this is coming out of YAML's largest user ecosystem. As Tim said, "The things that make YAML great at small scale become a liability at large scale."
If JSON had comments and trailing commas, probably.
Because KYAML only needs to concern itself with the cases that are used for representing k8s objects. For example.:
No k8s API objects contain stringified YAML. |
To be fair, people DO embed YAML and JSON into annotations or configmaps. The uglitude of additional escaping is not lost on me |
|
Embedded YAML (and JSON) show up in some extension APIs too. Such is life. |
|
Is anything pending on this to prevent merge? |
|
Dear reviewers - have I missed anything to get this merged? |
| In addition to `kubectl` output, | ||
| most Kubernetes config files are either YAML or JSON. | ||
| JSON lacks comments, | ||
| which are very valuable for configuration, |
There was a problem hiding this comment.
KYAML style formatting certainly doesn't make YAML nicer to read by people. Just less problematic to read by code.
If that is the goal, isn't JSON by far the most reliable format?
I'm seeing KYAML as a sweet spot between JSON's reliability and YAML's readability. Obviously it has it's pros and cons, as every other format, but for k8s use cases I trust the pros outweigh the cons in this case.
| To make this change, we will: | ||
| 1. Create new code (see details below) which serializes any object into KYAML. | ||
| 2. Introduce a kubectl output format, `-o=kyaml`, which uses this new package. | ||
| 3. Create tooling which reformats YAML or JSON as KYAML (see details below). |
There was a problem hiding this comment.
I think it might be best IF we want to move it to k-sigs to move it to it's own repo as a thin CLI wrapper around sigs.k8s.io/yaml, but then we have to manage releasing etc seperately.
That's a reasonable middle-ground. But before doing so we can investigate which parts in Tim's branch we could modify such that it could live as part of sigs.k8s.io/yaml. I'm pretty sure that the KYAMLEncoder should be part of that yaml library of ours, which only leaves the diff library at question. But resolving this is definitely not something we need to deal with here.
| 4. Work with SIG Docs to update examples to this format, both example command-lines and examples of API objects. | ||
| 5. Look for other examples across kubernetes-affiliated git repositories, and offer PRs to convert to KYAML. | ||
| 6. Make a lot of noise about it, and try to get more people to use it | ||
|
|
There was a problem hiding this comment.
There's an explicit non-goal which states: 3. Require projects to migrate to KYAML by default. and that's a reasonable approach.
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
lgtm
|
/approve As far as I can tell, KYAML don't have any production impacting implications given that it is an opt-in strict subset of YAML. |
|
/hold |
| Perhaps worst of all, | ||
| template-expanding systems, | ||
| such as the widely used `helm`, | ||
| force users to get whitespace "just right" |
There was a problem hiding this comment.
Helm charts done with kyaml would be much less readable/maintainable with all the curlies already required by Go templating.
Also while indentation isn't a parsing issue in kyaml, it certainly is an issue wrt readability.
It's disingenuous to say that the nindents will go away.
They won't since the output of helm template ... needs to be human readable/parsable.
There was a problem hiding this comment.
helm template | yamlfmt (or yq) works
There was a problem hiding this comment.
I would argue the development of Helm templates will be more difficult. Most users of Helm never look at the generated output. They just see what's installed.
My concern would be that YAML structures and practices for k8s (helm included) are already widely set. Making changes to that is going to cause confusion.
The developer experience for Helm templates, and k8s yaml based things in general, will go down by using a style that's not widely used or documented in the world at large.
| many of the most common traps in YAML. | ||
| For example, unlike standard YAML output, | ||
| this dialect is not whitespace-sensitive, | ||
| which makes it vastly easier to patch correctly in things like Helm charts. |
There was a problem hiding this comment.
I brought up this KEP in the Helm Weekly Developer Meeting which I regularly participate.
I'd like to get Helm release manage @mattfarina 's take on all of it noted here.
My take is that while you are saying this is optional, you really are suggesting that people use/recommend/document an entirely new format, that while technically valid YAML, will not be recognized by most users as such.
YAML has been in k8s since the start. That's 10+ years of establishment that is trying to be changed.
And for what I see is: trading a couple common issues for a whole new set of them.
cc: @banjoh @scottrigby @gjenkins8 from https://github.com/helm/helm
There was a problem hiding this comment.
I'm fine to take feedback, but I don't think we should block merge on that. The KEP window closes soon, and I am travelling in a week, so I don't want to miss it on technicality.
I'd like to wait and see what some of the Helm maintainers I referenced in my recent review have to offer here, before it gets merged. They are very well seasoned in k8s and its ecosystem. |
|
I think we should move forward with the KEP by merging this PR. I can't imagine any feedback from other projects, Helm included, that would change my mind.
Behind my thinking, some elements:
• it's alpha; we can get feedback (including before the release) and revise what we do
• we can drop the whole thing if we decide as a project that we don't like the idea
• we have no plan to enforce that people write YAML as KYAML
(we do plan to provide tools so that you can check if you have done)
|
|
+1 @lmktfy |
This KEP proposes to add a new output format for kubectl, tentatively called “KYAML” (as in `kubectl get -o kyaml ...`). This format is a strict subset (aka “dialect”) of standard YAML, and so should be parseable by the existing ecosystem. This dialect seeks to emphasize syntactical choices which avoid many of the most common traps in YAML. For example, unlike standard YAML output, this dialect is not whitespace-sensitive, which makes it vastly easier to patch correctly in things like Helm charts. This KEP further proposes to make KYAML the standard format for all project-owned documentation and examples.
k8s-ci-robot
removed
the
lgtm
|
@kubernetes/sig-docs-leads may want to look, too, since this proposes to edit docs. |
There was a problem hiding this comment.
/lgtm
/hold cancel
I don't see a reason why we should wait further with merging this document. Doing so does not mean we're not open to feedback, which can be addressed at any later point in time, either during implementation or during later stages (beta, ga) as this enhancement progresses.
k8s-ci-robot
added
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpbetz, soltysh, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This KEP proposes to add a new output format for kubectl, tentatively called “KYAML” (as in
kubectl get -o kyaml ...). This format is a strict subset (aka “dialect”) of standard YAML, and so should be parseable by the existing ecosystem. This dialect seeks to emphasize syntactical choices which avoid many of the most common traps in YAML. For example, unlike standard YAML output, this dialect is not whitespace-sensitive, which makes it vastly easier to patch correctly in things like Helm charts.This KEP further proposes to make KYAML the standard format for all project-owned documentation and examples.
One-line PR description: First draft
Issue link: KYAML #5295
Other comments: