@saad-ali, @caesarxuchao is this related to something we discovered on kubectl version issue

@kubernetes/kubectl

As you've pointed out, Kubernetes uses ghodss/yaml which is just a wrapper for candiedyaml. candiedyaml's README states that it's a "YAML 1.1 parser with support for YAML 1.2 features" ... I would recommend filing an issue with candiedyaml if you think that the behavior should be different.

Alternatively, ghodss/yaml used to be a wrapper around go-yaml, but we switched it to candiedyaml due to licensing concerns. It seems, however, that those concerns were recently addressed and go-yaml changed its license, so if you find that go-yaml has better behavior we could consider switching back.

go-yaml has the same issues, but I would recommend you still switch back due to compatibility issues. Kube is using an older version of your code which relies on go-yaml.

@fabianofranz Would you be able to own this or find it an owner?

@pwittrock on it. ;)

Before anything else I'll submit a PR to ghodss/yaml to switch back to go-yaml since it seems more reliable and it's now licensed as Apache 2.0 (go-yaml/yaml#160 (comment) and go-yaml/yaml@e4d366f).

SGTM.

The YAML 1.2 spec became, as one of the biggest changes, more restrictive about what's supported as booleans, mostly now only recommending true and false (no longer using on/off and yes/no as booleans, but just regular strings).

I opened a go-yaml issue about this, but I'm not sure a fix is going to happen since go-yaml wants to claim to "support most of YAML 1.1 and 1.2", so they will most likely want to keep supporting the specs disjunction instead of becoming more restrictive and leaving 1.1 out.

If we really want to fix this, I mostly see two options: we either a) fully move to 1.2, everywhere (and find a library that does it according to the spec); or b) try to extend go-yaml by proposing more flexibility to choose how to handle the differences between 1.1 and 1.2. In case of b), for example, have a UnmarshallingStrategy structure that would be passed to Unmarshal which carry flags that would affect the unmarshalling behavior, case by case. For example, StrictYAML12Booleans would specifically make booleans strict to the 1.2 spec. In kube that would be global, meaning booleans would always be strict to 1.2, but trying to keep 1.1 and 1.2 disjunction support for everything else as of now.

Thoughts?

In my limited experience it's usually better to fully support standard(s) for maximum interoperability and less user surprises. From a distance, I'd guess that supporting a disjointed set of standards adds confusion and technical debt to a community. And piling on to standards confusion just because other projects do it doesn't seem right. But this may not reflect the reasons for the current disjointed support.

There might be another option because YAML 1.2 and 1.1 allow version directives like so:

%YAML 1.2
---
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  name: redis
spec:
  replicas: 1
  selector:
    matchLabels:
      name: redis
  template:
    metadata:
      labels:
        name: redis
    spec:
      containers:
        -
          name: redis
          image: 'redis:3.2'
          command:
            - redis-server
            - '--appendonly'
            - no

currently kubectl apply -f redis-above.yaml returns: yaml: found incompatible YAML document

The downside of requiring directives for boolean handling are that we'd still need to find a compatible library and I generally don't see yaml libraries including them in output, so we'd probably need to add version directive printing support to libraries/tools that are missing the option. The upside is that it is 100% standards compliant.

If go-yaml exposes an Encoder/Decoder, one could set an option to enable "strict" YAML 1.2 parsing. Booleans are just one change in YAML 1.2, there are others.

There is also a separate issue opened on allowing longer lines to be emitted which could be made possible as well.

/sig cli
/sig api-machinery

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

/triage accepted

This issue is labeled with priority/important-soon but has not been updated in over 90 days, and should be re-triaged.
Important-soon issues must be staffed and worked on either currently, or very soon, ideally in time for the next release.

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Deprioritize it with /priority important-longterm or /priority backlog
• Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

I wonder if this still merits the important-soon priority. I'm going to take initiative here:
/remove-priority important-soon
/priority important-longterm
/triage accepted