Custom object json-patch

[HaraldGustafsson]

I noticed that most patch calls support all 3 patch types (json-patch, merge-patch, strategic-merge-patch), but the patch calls for custom objects (including for status and scale sub-resources) only support merge-patch.

For example compare consumes for the custom object from the swagger.json file:

  "patch": {
    "responses": {
      "200": {
        "description": "OK",
        "schema": {
          "type": "object"
        }
      },
      "401": {
        "description": "Unauthorized"
      }
    },
    "schemes": [
      "https"
    ],
    "description": "patch the specified cluster scoped custom object",
    "parameters": [
      {
        "schema": {
          "type": "object"
        },
        "description": "The JSON schema of the Resource to patch.",
        "required": true,
        "name": "body",
        "in": "body"
      }
    ],
    "produces": [
      "application/json"
    ],
    "x-codegen-request-body-name": "body",
    "tags": [
      "custom_objects"
    ],
    "consumes": [
      "application/merge-patch+json"
    ],
    "operationId": "patchClusterCustomObject"
  },

and consumes from Pod:

  "patch": {
    "description": "partially update the specified Pod",
    "consumes": [
      "application/json-patch+json",
      "application/merge-patch+json",
      "application/strategic-merge-patch+json"
    ],
    "produces": [
      "application/json",
      "application/yaml",
      "application/vnd.kubernetes.protobuf"
    ],
   ...

This then prevents using json-patch instead of merge-patch. Kubectl handles it.
The code in rest.py then have some strange things since it actually use the first one only and then if a json-patch contenttype but not list switch to merge-patch. So would be good to have json-patch+json first also for custom objects.

[tomplus]

As you checked, patching custom objects supports application/merge-patch+json only so I guess API Server (or CRD) rejects a request with different type of merge, doesn't it?

[HaraldGustafsson]

When I tried to patch with json patch on a custom object using kubectl it worked, so assuming that the api server handles it. We should leave out the strategic patch since that does not make sense for a custom object, my wrong, at least until strategic patching functionality can be specified in a CRD.

So I think that both json and merge patch should be specified in the swagger file.

[tomplus]

Fixed in the latest release - v10.0.0

[HaraldGustafsson]

Hi, seems to have got some issues now.

Reason: Unsupported Media Type
HTTP response headers: <CIMultiDictProxy('Content-Type': 'application/json', 'Date': 'Thu, 19 Sep 2019 12:24:53 GMT', 'Content-Length': '263')>
HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json","reason":"UnsupportedMediaType","code":415}

Generated custom_objects_api.py have:

def patch_namespaced_custom_object_with_http_info(self, group, version, namespace, plural, name, body, **kwargs):
...
    header_params['Content-Type'] = self.api_client.select_header_content_type(  # noqa: E501
        ['application/json-patch+json', 'application/merge-patch+json'])  # noqa: E501

select_header_content_typereturns first i.e. 'application/json-patch+json'

Then in rest.pywe have:

    if method in ['POST', 'PUT', 'PATCH', 'OPTIONS', 'DELETE']:
        if re.search('json', headers['Content-Type'], re.IGNORECASE):
            if headers['Content-Type'] == 'application/json-patch+json':
                if not isinstance(body, list):
                    headers['Content-Type'] = 'application/strategic-merge-patch+json'

When patching, Content-Type contains 'json' and now since content type is 'application/json-patch+json' is true it checks if body not a list which is also true, leads to that the content type is set to 'application/strategic-merge-patch+json', which is not supported by custom objects APIs. The api-server returns unsupported media type (415).

The last setting of content type would need to be 'application/merge-patch+json'

So best solution would be if the generated code would select the correct content type directly, based on method, body is list and allowed content types. Would that be possible? Alternatively, the select_header_content_type should return a list when it can't select and then allow the request in rest.py to do the proper selection and replace the content type list with a specific string.

What do you think @tomplus ?

[tomplus]

Thanks, definitely it needs to be fixed.

[gcarrarom]

Hi, I'm running into this issue now. Any quick way of working around the "media type not supported" error while your fix is not yet merged?

[tomplus]

Could you tell more about your use case? Do you try to use different merge strategy?

[gcarrarom]

I'm just trying to patch a custom object:

api_response = await api_instance.patch_namespaced_custom_object(group, version, manifest['metadata'].get('namespace'), plural, manifest['metadata']['name'], manifest)

But it's throwing me an error on the Content Type:

~ Failed to deploy the manifest for HelmRelease tmp -> (415)
Reason: Unsupported Media Type
HTTP response headers: <CIMultiDictProxy('Audit-Id': '75d76ce6-7de6-432f-a5d9-302ee6790a16', 'Cache-Control': 'no-cache, private', 'Content-Type': 'application/json', 'Date': 'Tue, 21 Jul 2020 12:43:59 GMT', 'Content-Length': '293')>
HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml","reason":"UnsupportedMediaType","code":415}

All I want to do is to patch the resource whith the changes coming in, that used to work fine when I used the default library.
Is there a different way I should be calling this method?

The manifest I'm sending is rather simple:

        {
          "apiVersion": "helm.fluxcd.io/v1",
          "kind": "HelmRelease",
          "metadata": {
            "generation": 1,
            "name": "tmp",
            "namespace": "tmp"
          },
          "spec": {
            "chart": {
              "name": "tmp",
              "repository": "https://<redacted>/repo",
              "version": "0.1.1"
            },
            "helmVersion": "v3",
            "releaseName": "tmp",
            "values": {}
          }
        }

[tomplus]

Thanks. As a workaround you can try to send list with JSON Patch instructions to modify your CRD. This nice library https://github.com/stefankoegl/python-json-patch creates such patches from differences between dicts.

[gcarrarom]

Thanks @tomplus ! That did work. I had to make sure to remove the patches I didn't want as the new manifest would not have the UID and the other information from the existing resource:

request_body = jsonpatch.make_patch(api_response, new_manifest)
request_patch = [patch for patch in request_body if patch['path'] != "/metadata/resourceVersion" and patch['path'] != "/metadata/uid" and patch['path'] != "/status" and patch['path'] != "/metadata/selfLink" and patch['path'] != "/metadata/creationTimestamp" and patch['path'] != "/metadata/generation"] 

api_response is the response from the K8s api with the current resource.
new_manifest is the new manifest with the changes.

It's now working fine!

[hapatrick]

I have also been frustrated by this recently, specifically because the behavior differs from the official Python kubernetes client. There is a discussion here of why custom object patching in kubernetes-client works the way it does:

kubernetes-client/python#866

So basically, it seems, kubernetes-client forces the caller to use application/merge-patch+json, where kubernetes_asyncio forces the use of application/json-patch+json. Both are probably wrong, or at least sub-optimal.

I don't know if kubernetes_asyncio has a goal of 100% matching the kubernetes-client API, but it is a convenient quality when you are trying to convert from kubernetes-client to kubernetes_asyncio as I am.

[tomplus]

Totally agree, patching in this library should work in the same way as in the kubernetes-client. I wasn't aware of the differences but as you noticed both behavior are problematic. I've started working on fixing both libraries (by extending openapi-generator) and your post gives me additional motivation to end it up. Thanks.

[linkous8]

This issue continues to frustrate me as well. I am able to workaround it for now but just wanted to add my +1

kubernetes-client/python#866

[tomplus]

There is a PR to track kubernetes-client/python#959 - it's already prepared and waits for review.

[linkous8]

First off, a huge thank you to @tomplus for his ongoing work to resolve this issue. In the meantime, I've found a somewhat ugly workaround that can be used in the interim. It turns out these headers can be set using ApiClient.set_default_header which will override the headers that are set by the CustomObjectsApi. eg. api_instance.api_client.set_default_header('content-type', 'application/merge-patch+json')

@HaraldGustafsson @gcarrarom @hapatrick I know its been a while since your comments but heads up WRT to the above workaround

[spolcyn]

@linkous8 Thanks for your workaround here -- when I used this, I needed to use "Content-Type" instead of "content-type" per the naming of the variable here: https://github.com/kubernetes-client/python/blob/master/kubernetes/client/api/custom_objects_api.py#L2427

[tomplus]

Starting from v22.6.0 it's possible to call patch methods with forcing a content type. In a next big release (v30.x) there is a new version of logic to detect a content-type for a patch. There is also an example how to use different type of patch. Changes are already merged to master branch so feel free to test it in the meantime. Thanks.