StackStorm-Exchange / stackstorm-kubernetes

st2 content pack containing Kubernetes sensors
https://exchange.stackstorm.org/
Apache License 2.0
19 stars 17 forks source link

Patch actions fail due to 415 UnsupportedMediaType #30

Open aletundo opened 6 years ago

aletundo commented 6 years ago

I encountered the problem with the patchCoreV1NamespacedService action, but probably the same bug affect all of the patch actions.

"result": {
    "status_code": 415,
    "data": {
      "status": "Failure",
      "kind": "Status",
      "code": 415,
      "apiVersion": "v1",
      "reason": "UnsupportedMediaType",
      "message": "the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/strategic-merge-patch+json",
      "metadata": {}
    }
  }

Accordingly to Kubernetes API Reference Documentation, the supported Content-Type header is application/strategic-merge-patch+json. I linked to version v1.7 since it is the older available version in the documentation, but the same value is specified for all the newer versions.

I made some tries specifying different arrangements of the header values. However, the only one accepted is application/strategic-merge-patch+json. Also, the application/json-patch+json requires an array of patches as the payload, which is impossible to be supplied since the body parameter is an object and not an array.

Furthermore, I would like to highlight another minor issue: at line 43, you format the args['url'] string providing also the body parameter, which is never used. I suppose it is a copy-and-paste refuse.

I did not verify all the patch-like actions, but looking at a couple of them, they are all affected. Let me know if you confirm the bug and I can proceed with a PR.

aletundo commented 6 years ago

@LindsayHill Hello, any news about it?

gamunu commented 6 years ago

@aletundo The library generated from OpenAPI spec that explains why args['url'] generated an additional parameter. I can reproduce this issue. But acording to the spec, body is an object https://github.com/kubernetes-client/python/blob/master/kubernetes/docs/CoreV1Api.md#patch_namespaced_service

My guess, problem is with python requests and how we define headers: https://github.com/StackStorm-Exchange/stackstorm-kubernetes/blob/51d31d31524d004951d238a36d80db3dd617e64c/etc/st2packgen/files/actions/lib/k8s.py#L77 https://github.com/StackStorm-Exchange/stackstorm-kubernetes/blob/4c1e5a8b95f5496d1392e2ea1ce2b8a04fcee9a4/actions/patchCoreV1NamespacedService.py#L41

AndyMoore commented 6 years ago

actions are generated straight from the swagger spec from kubernetes directly. In my experience with investigating this kind of problem it has frequently turned out to be swagger errors, rather than the generated client.

A copy of the 1.9 spec can be seen at https://raw.githubusercontent.com/kubernetes/kubernetes/v1.9.11/api/openapi-spec/swagger.json

In this you'll see that under the "/api/v1/namespaces/{namespace}/services/{name}" key there's a patch sub-key, with the following:

     "consumes": [
      "application/json-patch+json",
      "application/merge-patch+json",
      "application/strategic-merge-patch+json"
     ],
     "produces": [
      "application/json",
      "application/yaml",
      "application/vnd.kubernetes.protobuf"
     ],

These match the headers you see in the action https://github.com/StackStorm-Exchange/stackstorm-kubernetes/blob/73b54106731046f4755e177bf20e00a3157a25c1/actions/patchCoreV1NamespacedService.py#L41

the extra 'body' entry in the formatting won't cause any issues as they're kwargs in this case, but iirc it helps elsewhere (should be confirmed)

Lastly, you'll see that while the api is returning a 415, the protocol in the swagger doc shows it should only return either a 200 or a 401...

In short, if we're generating our own library as we are here, I dont think it'd matter which implementation we'd use. We'll need to look at manually patching the st2packgen rather than changing the individual actions. It'd be interesting to know if the official python library has any manual additions/patching to fix this kind of problem

aletundo commented 5 years ago

Hello @gamunu and @AndyMoore!

First, thank you for your detailed replies. I see what you explained, but the 415 error happens even if it wasn't documented by the Swagger doc and even if all the headers should be valid.

I can investigate more using the official Kubernetes Python client (which is generated by the Swagger doc in turn) and try to reproduce the bug.

What do you think about it?

blag commented 3 years ago

I can confirm this bug - none of the patch actions work.

Hey @aletundo, if this is still relevant to you, using the official Kubernetes Python client would be a great way to rewrite this pack's autogeneration script. Is that something you are still interested in doing?

aletundo commented 3 years ago

I can confirm this bug - none of the patch actions work.

Hey @aletundo, if this is still relevant to you, using the official Kubernetes Python client would be a great way to rewrite this pack's autogeneration script. Is that something you are still interested in doing?

Hi @blag! I'm sorry, but I'm not working on this subject anymore and I have no spare time at the moment.