Add "kyaml" support and yamlfmt #132

thockin · 2025-06-10T04:12:48Z

KYAML is a strict subset of YAML, which is sort of halfway between YAML and JSON. It has the following properties:

Does not depend on whitespace (easier to text-patch and template).
Always quotes value strings (no ambiguity aroud things like "no").
Allows quoted keys, but does not require them, and only quotes them if they are not obviously safe (e.g. "no" would always be quoted).
Always uses {} for structs and maps (no more obscure errors about mapping values).
Always uses [] for lists (no more trying to figure out if a dash changes the meaning).
When printing, it includes a header which makes it clear this is YAML and not ill-formed JSON.
Allows trailing commas
Allows comments,
Tries to economize on vertical space by "cuddling" some kinds of brackets together.
Retains comments.

Output from kubectl get -o kyaml can be fed back into kubectl.

Examples:

A struct:

metadata: {
  creationTimestamp: "2024-12-11T00:10:11Z",
  labels: {
    app: "hostnames",
  },
  name: "hostnames",
  namespace: "default",
  resourceVersion: "15231643",
  uid: "f64dbcba-9c58-40b0-bbe7-70495efb5202",
}

A list of primitves:

ipFamilies: [
  "IPv4",
  "IPv6",
]

A list of structs:

ports: [{
  port: 80,
  protocol: "TCP",
  targetPort: 80,
}, {
  port: 443,
  protocol: "TCP",
  targetPort: 443,
}]

A multi-document stream:

---
{
  foo: "bar",
}
---
{
  qux: "zrb",
}

thockin · 2025-06-10T04:24:03Z

@BenTheElder @lmktfy @liggitt @soltysh

BenTheElder

quick pass just on go.mod and yamlfmt/

yamlfmt/yamlfmt.go

BenTheElder · 2025-06-10T16:59:06Z

yamlfmt/yamlfmt.go

+	}
+}
+
+func renderYAML(in io.Reader, format string, printDiff bool, out io.Writer) error {


this looks ripe for a unit test?

Yeah, fair. I didn't write tests for the CLI tool yet.

yamlfmt/yamlfmt.go

thockin · 2025-06-10T17:58:36Z

Pushed with everything but a unit test.

liggitt

super quick sweep with a couple initial comments, didn't dig very deep into the impl

yamlfmt/yamlfmt.go

kyaml/kyaml_test.go

liggitt · 2025-06-10T18:05:26Z

yamlfmt/yamlfmt.go

+	help := flag.Bool("?", false, "print usage and exit")
+	diff := flag.Bool("d", false, "diff input files with their formatted versions")
+	write := flag.Bool("w", false, "write result to input files instead of stdout")
+	format := flag.String("o", "yaml", "output format: may be 'yaml' or 'kyaml'")


I'm surprised to even offer a non-kyaml option here, let alone defaulting to it... don't we want to be opinionated towards kyaml?

Mayber eventually but I don't KNOW that yet? There's enough pushback that I think we should be cautious and open to being wrong.

kyaml/kyaml.go

liggitt · 2025-06-10T18:17:56Z

kyaml/kyaml.go

+		if lazyQuote && !needsQuotes(val) {
+			fmt.Fprint(out, val)
+		} else {
+			fmt.Fprint(out, strconv.Quote(val))


is standard go quoting 100% compatible with yaml string escaping?

fair question. I will have to evaluate that and get back

https://yaml.org/spec/1.2.2/#escaped-characters

All non-printable characters must be escaped. YAML escape sequences use the “\” notation common to most modern computer languages [...]

[...]

YAML escape sequences are a superset of C’s escape sequences:

https://pkg.go.dev/strconv#Quote

Quote returns a double-quoted Go string literal representing s. The returned string uses Go escape sequences (\t, \n, \xFF, \u0100) for control characters and non-printable characters as defined by IsPrint.

https://pkg.go.dev/strconv#IsPrint

IsPrint reports whether the rune is defined as printable by Go, with the same definition as unicode.IsPrint: letters, numbers, punctuation, symbols and ASCII space.

Should be compatible.

\t \n, \xFF \u0100 escape style are all in "Example 5.13 Escaped Characters" in YAML 1.2.2 spec.

I also see these in 1.1 "Example 5.14. Escaped Characters" and even the 1.0 draft "4.6.9. Escaping".

If I read the YAML spec correctly, '\x00' should escape in YAML as "\0" but Go converts it to "\x00" - so I probably have to decode manually.

Actually, I have no idea how to read that table - it includes space and slash, which clearly are not actually escaped

If I read the YAML spec correctly, '\x00' should escape in YAML as "\0" but Go converts it to "\x00" - so I probably have to decode manually.

I don't think \x00 is invalid in yaml, but \0 would probably be canonical. https://go.dev/play/p/a6_gobBEp0k

Added a commit, but not sure we need it.

It's unclear if we actually need to change this logic. The round-trip test works without it, but we could very likely just be making the same bad assumptions as go-yaml.

The spec describes escapes for things like space and forward-slash, which seem wrong to escape, so I may be misreading it.

kyaml/kyaml.go

thockin · 2025-06-10T19:38:34Z

Easy things fixed

kyaml/kyaml.go

kyaml/kyaml_test.go

thockin · 2025-06-25T06:51:52Z

TTBOMK this is ready for review.

thockin · 2025-06-30T18:51:38Z

I have a commit cooking to simplify tests, but the rest of this is "done".

thockin · 2025-07-01T05:39:08Z

Tests are a bit simpler.

KYAML is a strict subset of YAML, which is sort of halfway between YAML and JSON. It has the following properties: * Does not depend on whitespace (easier to text-patch and template). * Always quotes value strings (no ambiguity aroud things like "no"). * Allows quoted keys, but does not require them, and only quotes them if they are not obviously safe (e.g. "no" would always be quoted). * Always uses {} for structs and maps (no more obscure errors about mapping values). * Always uses [] for lists (no more trying to figure out if a dash changes the meaning). * When printing, it includes a header which makes it clear this is YAML and not ill-formed JSON. * Allows trailing commas * Allows comments, * Tries to economize on vertical space by "cuddling" some kinds of brackets together. * Retains comments. Examples: A struct: ```yaml metadata: { creationTimestamp: "2024-12-11T00:10:11Z", labels: { app: "hostnames", }, name: "hostnames", namespace: "default", resourceVersion: "15231643", uid: "f64dbcba-9c58-40b0-bbe7-70495efb5202", } ``` A list of primitves: ```yaml ipFamilies: [ "IPv4", "IPv6", ] ``` A list of structs: ```yaml ports: [{ port: 80, protocol: "TCP", targetPort: 80, }, { port: 443, protocol: "TCP", targetPort: 443, }] ``` A multi-document stream: ```yaml --- { foo: "bar", } --- { qux: "zrb", } ```

* Can read 1 file (cmdline) * Can read multiple files (cmdline) * Can read stdin * Can write traditional YAML or KYAML * Can diff input vs output (-d) * Can write results to the input files (-w)

It's unclear if we actually need this. The round-trip test works without it. The spec describes escapes for things like space and forward-slash, which seem wrong to escape, so I may be misreading it.

Merge the "regular" and "compact" tests together.

thockin · 2025-07-14T18:47:43Z

a k/k PR preview: kubernetes/kubernetes#132942

thockin · 2025-07-18T19:46:40Z

FTR: I will be away for the last 3 days of the code-freeze, so if we want this in, I need it soon. Otherwise we can push to next release.

BenTheElder

/lgtm

soltysh

Sorry was out, but this lgtm

soltysh · 2025-07-22T09:52:30Z

kyaml/kyaml.go

+	}
+	if indent != 0 {
+		return fmt.Errorf("kyaml internal error: line %d: document non-zero indent (%d)", doc.Line, indent)
+	}


Nit: some of the error cases are not covered by tests, would be nice to get that added in followup PRs.

Some of the error paths are more like "assert" - they are no expected to ever trigger, but when they do it will at least fail in a semi-obvious way. Will look at adding test cases. :)

Added some tests around error handling. Coverage is > 89%. What's left is mostly error-returning from YAML and JSON parsing, and some impossible-to-reach (for now?) comment handling (yaml.v3 comments are...inconsistently implemented)

thockin · 2025-07-24T18:03:37Z

Pushed with new test cases for corner cases mostly. Needs LGTM and approve.

jpbetz · 2025-07-24T18:11:19Z

/lgtm
/approve

Expanded test coverage looks excellent

k8s-ci-robot · 2025-07-24T18:11:27Z

[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:

~~OWNERS~~ [jpbetz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

thockin · 2025-07-24T18:37:53Z

This repo seems to use tags - do we need a v1.5.1 or v1.6.0 tag so I can re-vendor to k/k ?

liggitt · 2025-07-24T18:40:02Z

yes, let's add v1.6.0 since this added features

liggitt · 2025-07-24T18:41:04Z

https://github.com/kubernetes-sigs/yaml/releases/tag/v1.6.0

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 10, 2025

k8s-ci-robot requested a review from deads2k June 10, 2025 04:12

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 10, 2025

k8s-ci-robot requested a review from jpbetz June 10, 2025 04:12

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 10, 2025

thockin force-pushed the master branch from 3a847a8 to 59042da Compare June 10, 2025 04:19

thockin changed the title ~~Add "kyaml" support~~ Add "kyaml" support and yamlfmt Jun 10, 2025

thockin force-pushed the master branch 2 times, most recently from c72348b to 8fb29f1 Compare June 10, 2025 04:50

BenTheElder reviewed Jun 10, 2025

View reviewed changes

liggitt reviewed Jun 10, 2025

View reviewed changes

thockin force-pushed the master branch 3 times, most recently from c603bd5 to 17c7943 Compare June 10, 2025 22:07

BenTheElder reviewed Jun 11, 2025

View reviewed changes

kyaml/kyaml.go Outdated Show resolved Hide resolved

BenTheElder reviewed Jun 11, 2025

View reviewed changes

kyaml/kyaml.go Show resolved Hide resolved

BenTheElder reviewed Jun 11, 2025

View reviewed changes

kyaml/kyaml_test.go Outdated Show resolved Hide resolved

kyaml/kyaml_test.go Show resolved Hide resolved

thockin force-pushed the master branch 2 times, most recently from d46fbd4 to 0a0c08a Compare June 12, 2025 23:22

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 25, 2025

thockin force-pushed the master branch from 0a0c08a to 5dd93e3 Compare June 25, 2025 06:49

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 25, 2025

thockin force-pushed the master branch from 5dd93e3 to 4194697 Compare June 26, 2025 08:43

thockin force-pushed the master branch from a4bff92 to 9adc5d0 Compare July 14, 2025 17:11

thockin force-pushed the master branch from 9adc5d0 to 43a7a4f Compare July 14, 2025 18:13

thockin added 6 commits July 14, 2025 11:40

Add a yamlfmt cmd

7749171

* Can read 1 file (cmdline) * Can read multiple files (cmdline) * Can read stdin * Can write traditional YAML or KYAML * Can diff input vs output (-d) * Can write results to the input files (-w)

kyaml: Implement escaping closer to YAML spec

abc1add

It's unclear if we actually need this. The round-trip test works without it. The spec describes escapes for things like space and forward-slash, which seem wrong to escape, so I may be misreading it.

Add compact mode so KYAML can be used in more places

59c2c43

Change which methods get a newline and fix tests

13509ad

Merge the "regular" and "compact" tests together.

Add test case for tabs in multi-line strings

4a4f539

Add compact output tests

2e3340b

thockin force-pushed the master branch from 43a7a4f to 2e3340b Compare July 14, 2025 18:40

thockin mentioned this pull request Jul 14, 2025

Add KYAML support to kubectl kubernetes/kubernetes#132942

Merged

jenshu mentioned this pull request Jul 19, 2025

KYAML kubernetes/enhancements#5295

Open

15 tasks

BenTheElder reviewed Jul 21, 2025

View reviewed changes

k8s-ci-robot assigned BenTheElder Jul 21, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2025

soltysh approved these changes Jul 22, 2025

View reviewed changes

Bolster tests, mostly in error-handling

23c836c

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 24, 2025

k8s-ci-robot assigned jpbetz Jul 24, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 24, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2025

k8s-ci-robot merged commit 048d724 into kubernetes-sigs:master Jul 24, 2025
6 checks passed

Uh oh!

Add "kyaml" support and yamlfmt #132

Add "kyaml" support and yamlfmt #132

Conversation

thockin commented Jun 10, 2025

Uh oh!

thockin commented Jun 10, 2025

Uh oh!

BenTheElder left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thockin commented Jun 10, 2025

Uh oh!

liggitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenTheElder Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thockin commented Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thockin commented Jun 25, 2025

Uh oh!

thockin commented Jun 30, 2025

Uh oh!

thockin commented Jul 1, 2025

Uh oh!

thockin commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thockin commented Jul 18, 2025

Uh oh!

BenTheElder left a comment

Choose a reason for hiding this comment

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

BenTheElder Jun 11, 2025 •

edited

Loading

thockin commented Jul 14, 2025 •

edited

Loading