Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make IsZeroer behavior consistent between JSON and YAML encoders #2

Open
luxas opened this issue Aug 30, 2021 · 0 comments
Open

Make IsZeroer behavior consistent between JSON and YAML encoders #2

luxas opened this issue Aug 30, 2021 · 0 comments

Comments

@luxas
Copy link
Owner

luxas commented Aug 30, 2021

Any "normal" k8s object represented in YAML is encoded today, looks something like this:

apiVersion: foo.io/v1
kind: Bar
metadata:
  name: baz
  creationTimestamp: null # Unnecessary!
spec:
  ... # whatever the user desires
status: {} # Unnecessary!

This due to that encoding/json being used for marshalling doesn't respect the following IsZeroer interface that'd be pretty handy:

// IsZeroer is used to check whether an object is zero to
// determine whether it should be omitted when marshaling
// with the omitempty flag. One notable implementation
// is time.Time.
type IsZeroer interface {
	IsZero() bool
}

(as defined in yaml.v3: https://pkg.go.dev/gopkg.in/yaml.v3#IsZeroer)

In particular, CreationTimestamp is registered on metav1.ObjectMeta as CreationTimestamp (metav1.)Time; and metav1.Time defines the IsZero method on the pointer receiver, like func (t *Time) IsZero() bool {}; hence it needs some extra care to work for both cases. The code doing this in yaml.v3 is here; it'd be great to get both JSON and YAML consistent with regards to this, and standardized as per a "k8s JSON/YAML spec".

You might ask, that why isn't this working already if yaml.v3 supports it? Well, because sigs.k8s.io/yaml

  1. uses yaml.v2
  2. first does a json.Marshal of the struct, then a json.Unmarshal of the resulting bytes into a map[string]interface{} (this also implicitly "sorts" the keys, as map order otherwise would be random at encode-time), and then finally does a yaml.Marshal of the map[string]interface{}. This means that in order for us to not output "zero" fields in YAML; we must fix the JSON marshaller.

Furthermore, it's worth adding that yaml.v3 also checks recursively if all of the fields are zero, which means that you do not have to define an IsZero method for all of your e.g. FooStatus structs; it can be deducted automatically if all fields are empty.

From what I see in the codebase, it should be possible to implement an extension for jsoniter that applies this logic for JSON encoding.

Finally, it's worth mentioning that for yaml.v3 to even try checking if the field is zero, the omitempty tag must be set. Thanks to the Kubernetes API conventions, that specify that optional (i.e. non-required fields) should have the omitempty tag, this is most often the case (e.g. on ServiceStatus)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant