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

Patch fails with error decoding patch: json: cannot unmarshal object into Go value of type []handlers.jsonPatchOp #2160

Open
yubinnng opened this issue Jan 14, 2025 · 10 comments

Comments

@yubinnng
Copy link

yubinnng commented Jan 14, 2025

Describe the bug

I would like to patch a resource with a request body in JSON Merge Patch, RFC 7386 format. However, even if I specify the Content-Type application/merge-patch+json in a PromiseMiddlewareWrapper, add it into a ServerConfiguration and then use the configuration in the client method k8sApi.patchNamespacedPod({xxx}, configuration), the error still occurs:

ApiException [Error]: HTTP-Code: 400
Message: Unknown API Status Code!
Body: "{\"kind\":\"Status\",\"apiVersion\":\"v1\",\"metadata\":{},\"status\":\"Failure\",\"message\":\"error decoding patch: json: cannot unmarshal object into Go value of type []handlers.jsonPatchOp\",\"reason\":\"BadRequest\",\"code\":400}\n"
Headers: {"audit-id":"f2f1b40e-9d4a-49cf-af2b-989bed7de17e","cache-control":"no-cache, private","connection":"close","content-length":"211","content-type":"application/json","date":"Tue, 14 Jan 2025 02:17:01 GMT","x-kubernetes-pf-flowschema-uid":"93068417-329d-4e0f-9768-8a2b6794f954","x-kubernetes-pf-prioritylevel-uid":"f7dc9ee7-016c-48fa-afc3-77112540d77f"}
    at CoreV1ApiResponseProcessor.patchNamespacedPodWithHttpInfo (/Users/I584817/Downloads/js-examples/node_modules/@kubernetes/client-node/dist/gen/apis/CoreV1Api.js:15722:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async file:///Users/XXX/Downloads/js-examples/merge-patch-example.js:46:5 {
  code: 400,
  body: '{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"error decoding patch: json: cannot unmarshal object into Go value of type []handlers.jsonPatchOp","reason":"BadRequest","code":400}\n',
  headers: {
    'audit-id': 'f2f1b40e-9d4a-49cf-af2b-989bed7de17e',
    'cache-control': 'no-cache, private',
    connection: 'close',
    'content-length': '211',
    'content-type': 'application/json',
    date: 'Tue, 14 Jan 2025 02:17:01 GMT',
    'x-kubernetes-pf-flowschema-uid': '93068417-329d-4e0f-9768-8a2b6794f954',
    'x-kubernetes-pf-prioritylevel-uid': 'f7dc9ee7-016c-48fa-afc3-77112540d77f'
  }
}

Please find my code in the example code below.

I tried to debug, and it seems that the pre function in the PromiseMiddlewareWrapper does not be executed.

I saw a Patch related issue [release-1.x] Patch namespace secret trigger an error due to unsupported media type, but it does not solve this issue.

Client Version
1.0.0

Server Version
1.31.0

To Reproduce

Save the below example code into file merge-patch-example.js, then execute the following commands.

npm i @kubernetes/client-node
node merge-patch-example.js

Expected behavior
The server should not raise any error.

Example Code

import * as k8s from '@kubernetes/client-node';
import { PromiseMiddlewareWrapper } from '@kubernetes/client-node/dist/gen/middleware.js';

const kc = new k8s.KubeConfig();
kc.loadFromDefault();

const k8sApi = kc.makeApiClient(k8s.CoreV1Api);

const namespace = 'default';

try {
    const res = await k8sApi.listNamespacedPod({ namespace });
    const body = {
        metadata: {
            labels: {
                foo: 'bar2'
            }
        }
    };
    const headerPatchMiddleware = new PromiseMiddlewareWrapper({
        pre: async (requestContext) => {
            requestContext.setHeaderParam('Content-type', 'application/merge-patch+json');
            return requestContext;
        },
        post: async (responseContext) => responseContext,
    });
    let currentContext = kc.getCurrentContext();
    let currentCluster = kc.getCluster(currentContext);
    if (currentCluster === undefined || currentCluster === null) {
        throw new Error('Cluster is undefined');
    }
    let server = currentCluster.server;
    if (server === undefined) {
        throw new Error('Server is undefined');
    }

    const baseServerConfig = new k8s.ServerConfiguration(server, {});
    const configuration = k8s.createConfiguration({
        middleware: [headerPatchMiddleware],
        baseServer: baseServerConfig,
        authMethods: {
            default: kc,
        },
    });

    await k8sApi
        .patchNamespacedPod(
            { name: res?.items?.[0].metadata?.name ?? '', namespace, body: body },
            configuration,
        )
    console.log('Merged.');
} catch (err) {
    console.error('Error: ');
    console.error(err);
}

Environment (please complete the following information):

  • OS: MacOS
  • Node.js version 22.13.0
  • Cloud runtime: Local Minikube
@yubinnng
Copy link
Author

image

@koooge
Copy link

koooge commented Jan 20, 2025

I confirmed this with patchNamespacedStatefulSet. pre() and post() are not called at all.

@kopernic-pl
Copy link

kopernic-pl commented Jan 27, 2025

I found this as well today, with the code snippet very similar to the one of original author.

The API response is 400 Bad Request,
"error decoding patch: json: cannot unmarshal object into Go value of type []handlers.jsonPatchOp

Which hints that the servrer is expecting the array and not getting one.

I am using the middleware copypasted from the example

  const headerPatchMiddleware = new PromiseMiddlewareWrapper({
    pre: async (requestContext) => {
      // requestContext.setHeaderParam("content-type", PatchStrategy.MergePatch);
      requestContext.setHeaderParam("content-type", "application/merge-patch+json");
      return requestContext;
    },
    post: async (responseContext) => responseContext,
  });

Edit: it seems like middleware is not called at all when running the example code, neither pre nor post.

@cjihrig
Copy link
Contributor

cjihrig commented Jan 28, 2025

It looks like patchNamespacedPodWithHttpInfo() in dist/gen/types/ObservableAPI.js is not using the provided configuration object at all. It receives the custom configuration as its _options argument, but doesn't use it when setting up the middleware. Instead, it uses its this.configuration object.

Updating that function to do _config = _options || this.configuration; and then use _config, similar to what is done in other places in the codebase seems to make the middleware run.

I'm not sure if that is the ideal fix or not, because I'm not super familiar with the code generator codebase.

A second issue I noticed is that requestContext.setHeaderParam('Content-type', 'application/merge-patch+json'); needs to be Content-Type (note the capital T) in order to properly override the previous Content-Type header. I'm not sure if this is intentional or not. It kinda seems like a bug to me though.

After I made those two fixes locally, the code in the original post worked for me.

@mer135-csiro
Copy link

I can confirm the middleware just is not executing. Followed example from https://github.com/kubernetes-client/javascript/blob/main/examples/patch-example.js

It seems the primary use case for middleware is manually setting headers is to define the patch strategy. Could this just be added as an option on the patch methods like it is for KubernetesObjectApi.patch?

javascript/src/object.ts

Lines 230 to 239 in 247e4ed

public async patch<T extends KubernetesObject | KubernetesObject>(
spec: T,
pretty?: string,
dryRun?: string,
fieldManager?: string,
force?: boolean,
patchStrategy: PatchStrategy = PatchStrategy.StrategicMergePatch,
options?: Configuration,
): Promise<T> {
const _config = options || this.configuration;

@cjihrig
Copy link
Contributor

cjihrig commented Jan 29, 2025

@brendandburns @mstruebing @davidgamero by any chance, do you know if my approach in #2160 (comment) sounds correct? If so, I can send a patch to the code generator repo.

@davidgamero
Copy link
Contributor

Your breakdown looks correct.

PR on the generator to attempt to fix this currently:

OpenAPITools/openapi-generator#20430

The current discussion direction is going towards merging the middleware arrays from the base configuration and the _options.middleware

Let me know if any cases are missing from that approach

@cjihrig
Copy link
Contributor

cjihrig commented Jan 29, 2025

Thanks for the quick reply. I guess we will wait for that to land.

@waterscar
Copy link

Hi guys, is there any workaround for this issue?

@davidgamero
Copy link
Contributor

davidgamero commented Feb 8, 2025

@waterscar
To fix this we'll need to regen once OpenAPITools/openapi-generator#20430 merges

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants