Serialization sets invalid default values about dotnet-kube-client HOT 14 CLOSED

Yeah I'm thinking these should be nullable :)

Let me see what I can do.

from dotnet-kube-client.

What version of the K8s are you using, by the way? The field readinessGates does appear in the YAML for PodSpecV1:

io.k8s.api.core.v1.PodSpec:
  description: PodSpec is a description of a pod.
  properties:
    # ...<snip>...
    containers:
      description: List of containers belonging to the pod. Containers cannot currently
        be added or removed. There must be at least one container in a Pod. Cannot
        be updated.
      items:
        $ref: '#/definitions/io.k8s.api.core.v1.Container'
      type: array
      x-kubernetes-patch-merge-key: name
      x-kubernetes-patch-strategy: merge
    readinessGates:
      description: 'If specified, all readiness gates will be evaluated for pod
        readiness. A pod is ready when all its containers are ready AND all conditions
        specified in the readiness gates have status equal to "True" More info:
        https://github.com/kubernetes/community/blob/master/keps/sig-network/0007-pod-ready%2B%2B.md'
      items:
        $ref: '#/definitions/io.k8s.api.core.v1.PodReadinessGate'
      type: array
    # ...<snip>...
    terminationGracePeriodSeconds:
      description: Optional duration in seconds the pod needs to terminate gracefully.
        May be decreased in delete request. Value must be non-negative integer.
        The value zero indicates delete immediately. If this value is nil, the default
        grace period will be used instead. The grace period is the duration in seconds
        after the processes running in the pod are sent a termination signal and
        the time when the processes are forcibly halted with a kill signal. Set
        this value longer than the expected cleanup time for your process. Defaults
        to 30 seconds.
      format: int64
      type: integer
    tolerations:
      description: If specified, the pod's tolerations.
      items:
        $ref: '#/definitions/io.k8s.api.core.v1.Toleration'
      type: array
    volumes:
      description: 'List of volumes that can be mounted by containers belonging
        to the pod. More info: https://kubernetes.io/docs/concepts/storage/volumes'
      items:
        $ref: '#/definitions/io.k8s.api.core.v1.Volume'
      type: array
      x-kubernetes-patch-merge-key: name
      x-kubernetes-patch-strategy: merge,retainKeys
  required:
  - containers

from dotnet-kube-client.

I think the broader problem is that we don't currently respect the required property in the OpenAPI YAML which would tell us which properties can be null (i.e. any not specified in required). But that's fixable, I think.

from dotnet-kube-client.

One way to fix this is detailed in 0e9021d and 2eb8756, but this would be a breaking change for existing consumers. Will have a think about this overnight to see if I can come up with a less disruptive option :)

from dotnet-kube-client.

Still trying out ideas (sorry for the delay).

from dotnet-kube-client.

No worries, better to get it right than a quick-fix that breaks stuff. 👍

from dotnet-kube-client.

I am thinking that, for list (array) properties, they'll be read-only if they're mandatory, but read/write (and null by default) if they're optional. The ergonomics are a bit...meh... but it's a compromise. What do you think?

CC: @felixfbecker - is this likely to mess you up at all? I imagine it might affect resource diffs, but at least you'd know if something is null then they probably didn't specify it...

from dotnet-kube-client.

For list / dict properties, an alternative approach would be to use a custom JsonConverter that treats an empty collection as null (and, consequently, omits it from the resulting JSON).

from dotnet-kube-client.

(that way the collection properties themselves are never null, preserving the current client-side behaviour)

from dotnet-kube-client.

Ok, we now generate a ShouldSerializeXXX method for all optional collection properties which ensures that they are not serialised if the collection is empty (see 99af4b7 for details).

This does make it hard to empty out a collection property while updating a resource but, given that the K8s API is kinda braindead anyway when it comes to updates (you more or less have to use JSON-patch rather than a PUT to accomplish most useful modifications) I think we could live with this behaviour.

What do you think?

from dotnet-kube-client.

Sounds good to me. I also much prefer not having to initialize collection properties.

Would it maybe make sense to pre-initialize required, non-collection properties (such as Spec) as well? This could enable nice and concise code like this:

var deployment = new DeploymentV1
{
    Metadata = {Name = "test"},
    Spec =
    {
        Template =
        {
            Metadata =
            {
                Labels = {["app"] = "test"}
            },
            Spec =
            {
                Containers =
                {
                    new ContainerV1
                    {
                        Name = "test",
                        Image = "nginx"
                    }
                }
            }
        }
    }
}));

from dotnet-kube-client.

For sure - all collection properties are read-only now. What I also wound up doing is also generating a ShouldSerializeXXX() method for optional collection properties - if collection is empty, it's omitted :)

from dotnet-kube-client.

Ok, this should be resolved in the latest release @bastianeicher :)

Would you mind giving it a try to see if it produces the results you'd expect?

from dotnet-kube-client.

Jep, works perfectly now. Thanks. :)

from dotnet-kube-client.