CustomResources dropping "null" values from arrays

[caseydavenport]

What happened:

While attempting to work around #98079, I encountered an issue where data was not preserved when writing a custom resource instance to the apiserver with kubectl.

Specifically, I am using a CRD that has a properly which is an array of values which can either be an integer, or null:

spec:
  properties:
    allocations:
      items:
        nullable: true
        type: integer
      type: array

When using kubectl to modify the resource, all of the "null" values are lost upon write. So, if I attempt to write a resource with the following:

allocations:
- 1
- 5
- null
- null

It is written successfully. However, when I query the resource from the API afterwards, this is what I get back:

allocations:
- 1
- 5

The null entries have been lost.

What you expected to happen:

The null entries in the array should be maintained.

How to reproduce it (as minimally and precisely as possible):

Create a CustomResourceDefinition with a property like the one specified above, and create an instance of it (you may need to pass --validate=false in order to bypass the bug in #98079).

Notice that null values are removed from the array.

Anything else we need to know?:

Environment:

• Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8", GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.2", GitCommit:"52c56ce7a8272c798dbc29846288d7cd9fbae032", GitTreeState:"clean", BuildDate:"2020-04-30T20:19:45Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}

Also reproduced on master by @liggitt

CC @sttts @apelisse

/sig api-machinery

[apelisse]

That's clearly a bug, thanks for the easy steps to reproduce, I'll look at it.

[sttts]

This is about data loss. Set to urgent to investigate.

[liggitt]

Digging briefly, it looks at first glance like json-patch is dropping all null values in https://github.com/evanphx/json-patch/blob/master/merge.go#L81 (haven't verified with a test yet, just manual inspection)

[apelisse]

Based on Jordan's reading and mine, this is a bug/misreading of the JSON Merge patch. We should probably patch upstream.
Can the spec be updated to improve readability/add a test case?

[fedebongio]

/triage accepted

[fejta-bot]

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.

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

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[fejta-bot]

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-contributor-experience at kubernetes/community.
/lifecycle rotten

[DraggonFantasy]

The patch about preserving null values in the arrays was merged to the json-patch, so the issue should be fixed

[apelisse]

We probably need to update the version of json-patch used in kubernetes now!

[pacoxu]

@apelisse is there a new release version with the fix？

I asked @evanphx to make one.

/remove-lifecycle rotten

[evanphx]

@pacoxu Yup, v5.5.0 has been released with the fixes.

[pacoxu]

Currently, v4.5.0 cannot be removed for kustomize

• sigs.k8s.io/kustomize/api@v0.8.10 is using github.com/evanphx/json-patch@v4.5.0+incompatible.
• staging/src/k8s.io/sample-cli-plugin/go.sum is using v4.5.0+incompatible.

I opened kubernetes-sigs/kustomize#3938 for kustomize.

[pacoxu]

Update to v4.11.0 to fix it.
v5.5.0 has some break changes and the upgrade will be discussed later.