Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add JSON Patch support for namespaced custom objects #2040

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions kubernetes/client/api/custom_objects_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2548,7 +2548,7 @@ def patch_cluster_custom_object_with_http_info(self, group, version, plural, nam

# HTTP header `Content-Type`
header_params['Content-Type'] = self.api_client.select_header_content_type( # noqa: E501
['application/merge-patch+json']) # noqa: E501
['application/json-patch+json' if isinstance(body, list) else 'application/merge-patch+json']) # noqa: E501
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the change backwards compatible? This file is generated by openapi-generator, and we already carry a patch to make sure merge patch is used for custom objects:

# Ref: https://github.com/kubernetes-client/python/pull/995/commits/9959273625b999ae9a8f0679c4def2ee7d699ede

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had no idea these files were auto-generated. In that case, If I understand correctly, if this PR gets accepted, a cherry-pick to this commit will by added to the apply-hotfixes.sh script. Is that correct?

To your question - it is backwards compatible. As discussed before, when the body patch is a dict, the application/merge-patch+json content-type will be used, which was used in all previous versions and thus is backwards compatible.

When the body patch is a list, which indicates it contains a JSONPatch (which is a new feature, not supported before on custom objects on this library) - the content-type shall be application/json-patch+json.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a known issue and the solution was discussed here: #959
To solve this we need to upgrade openapi-generator to the latest versions.


# Authentication setting
auth_settings = ['BearerToken'] # noqa: E501
Expand Down Expand Up @@ -2717,7 +2717,7 @@ def patch_cluster_custom_object_scale_with_http_info(self, group, version, plura

# HTTP header `Content-Type`
header_params['Content-Type'] = self.api_client.select_header_content_type( # noqa: E501
['application/merge-patch+json']) # noqa: E501
['application/json-patch+json' if isinstance(body, list) else 'application/merge-patch+json']) # noqa: E501

# Authentication setting
auth_settings = ['BearerToken'] # noqa: E501
Expand Down Expand Up @@ -2886,7 +2886,7 @@ def patch_cluster_custom_object_status_with_http_info(self, group, version, plur

# HTTP header `Content-Type`
header_params['Content-Type'] = self.api_client.select_header_content_type( # noqa: E501
['application/merge-patch+json']) # noqa: E501
['application/json-patch+json' if isinstance(body, list) else 'application/merge-patch+json']) # noqa: E501

# Authentication setting
auth_settings = ['BearerToken'] # noqa: E501
Expand Down Expand Up @@ -3064,7 +3064,7 @@ def patch_namespaced_custom_object_with_http_info(self, group, version, namespac

# HTTP header `Content-Type`
header_params['Content-Type'] = self.api_client.select_header_content_type( # noqa: E501
['application/merge-patch+json']) # noqa: E501
['application/json-patch+json' if isinstance(body, list) else 'application/merge-patch+json']) # noqa: E501

# Authentication setting
auth_settings = ['BearerToken'] # noqa: E501
Expand Down Expand Up @@ -3242,7 +3242,7 @@ def patch_namespaced_custom_object_scale_with_http_info(self, group, version, na

# HTTP header `Content-Type`
header_params['Content-Type'] = self.api_client.select_header_content_type( # noqa: E501
['application/merge-patch+json']) # noqa: E501
['application/json-patch+json' if isinstance(body, list) else 'application/merge-patch+json']) # noqa: E501

# Authentication setting
auth_settings = ['BearerToken'] # noqa: E501
Expand Down Expand Up @@ -3420,7 +3420,7 @@ def patch_namespaced_custom_object_status_with_http_info(self, group, version, n

# HTTP header `Content-Type`
header_params['Content-Type'] = self.api_client.select_header_content_type( # noqa: E501
['application/merge-patch+json']) # noqa: E501
['application/json-patch+json' if isinstance(body, list) else 'application/merge-patch+json']) # noqa: E501

# Authentication setting
auth_settings = ['BearerToken'] # noqa: E501
Expand Down