kubernetes: kubectl apply does not properly minimize patch when using JSON merge patch

Is this a BUG REPORT or FEATURE REQUEST?: Bug Report /kind bug

What happened: Assume a CustomResource original:

{
    "apiVersion": "company/v1",
    "kind": "CustomResource",
    "metadata": {
        "annotations": {},
        "name": "secrets",
        "namespace": "test"
    }
}

current:

{
    "apiVersion": "company/v1",
    "kind": "CustomResource",
    "metadata": {
        "annotations": {
            "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"company/v1\",\"kind\":\"VaultSecret\",\"metadata\":{\"annotations\":{},\"name\":\"secrets\",\"namespace\":\"test\"}}\n"
        },
        "clusterName": "",
        "creationTimestamp": "2018-01-24T22:49:30Z",
        "deletionGracePeriodSeconds": null,
        "deletionTimestamp": null,
        "generation": 0,
        "name": "secrets",
        "namespace": "test",
        "resourceVersion": "4800",
        "uid": "d6e8e5d8-0158-11e8-be12-080027cce81e"
    },
    "time-processed": "2018-01-24T22:49:30"
}

modified:

{
    "apiVersion": "company/v1",
    "kind": "CustomResource",
    "metadata": {
        "annotations": {
            "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"company/v1\",\"kind\":\"VaultSecret\",\"metadata\":{\"annotations\":{},\"name\":\"secrets\",\"namespace\":\"test\"}}\n"
        },
        "name": "secrets",
        "namespace": "test"
    }
}

original add-and-change patch current -> modified

{
    "metadata": {
        "clusterName": null,
        "creationTimestamp": null,
        "deletionGracePeriodSeconds": null,
        "deletionTimestamp": null,
        "generation": null,
        "resourceVersion": null,
        "selfLink": null,
        "uid": null
    }
}

filtered add-and-change patch (non-nulls only) Note that at this point, the patch could have been optimized to {}

{"metadata":{}}

original deleted patch between original -> modified

{
    "metadata": {
        "annotations": {
            "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"company/v1\",\"kind\":\"VaultSecret\",\"metadata\":{\"annotations\":{},\"name\":\"secrets\",\"namespace\":\"test\"}}\n"
        }
    }
}

filtered deleted patch (nulls only) Note that at this point, the patch could have been optimized to {}

{"metadata":{"annotations":{}}}

This results in a non-empty patch, which causes issues since we forbid patches to most users. We’d like for unchanged files to not result in unnecessary PATCH requests, as they result in forbidden requests.

What you expected to happen: The modified object above should resolve to an empty patch, and kubectl should not send a PATCH request to the apiserver.

How to reproduce it (as minimally and precisely as possible): create a CustomResourceDefinition, create an instance of the resource using a file, forbid patching to the custom resource, re-apply the same file - you should receive a forbidden error.

Anything else we need to know?: This is totally kubectl-side issue - kubectl is calculating a patch, and then sends an unnecessary PATCH request to a non-optimal patch resolution. I have a fix ready that fixes the issue, but need to wait for my company to read over the CLA.

The issue lies with the code block here: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch.go#L116 The block fails to clean up nested maps that no longer contain any patches, eg {a: { b: nil } } filters nils to {a: {} } when really it can go further and resolve to {}

There’s 2 solutions that I can see, I chose the former:

1. Modify the linked block above to remove empty filtered maps if they weren’t already explicitly set in the original map
2. Preserve the existing code, and then in kubectl’s apply.go go over the filtered patch and remove empty patches there

Environment:

• Kubernetes version (use kubectl version): 1.8.6 client/server, but verified kubectl built off current master has the same issue.
• Cloud provider or hardware configuration:
• OS (e.g. from /etc/os-release): N/A (kubectl running on Mac)
• Kernel (e.g. uname -a): N/A
• Install tools:
• Others: