OpenShift incorrectly applies RFC 6902 JSON PATCH

[k-wall]

The OpenShift server side appears to be incorrectly applying valid RFC 6902 JSON PATCH documents when patching existing statefulset resources. This leads to resources in an unexpected state and in some cases, this is leading the server rejecting the request with errors such as:

io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: PATCH at: https://172.30.0.1/apis/apps/v1/namespaces/xxxxxx-infra/statefulsets/broker-mdh1f76xc7-mqv7. Message: Unable to access invalid index: 20.

We have investigated the issue, and believe the issue is the implementation of the RFC 6902 move operation within OpenShift. Instead of inserting the moved value back into the document, OpenShift's implementation is overwriting the value at the target. This leads to an incorrect result document. Subsequent processing of following operations may also end in error, such as the one above.

I believe the OpenShift implementation is not compliant with the RFC.

RFC 6092, 4.4. Move, says:

The "move" operation removes the value at a specified location and adds it to the target location.

it then goes on to say:

This operation is functionally identical to a "remove" operation on
the "from" location, followed immediately by an "add" operation at
the target location with the value that was just removed.

section 4.1 defines add like this:

If the target location specifies an array index, a new value is
inserted into the array at the specified index.

Version

oc v3.11.0+0cbc58b
kubernetes v1.11.0+d4cacc0
features: Basic-Auth GSSAPI Kerberos SPNEGO

Server https://127.0.0.1:8443
kubernetes v1.11.0+d4cacc0

Steps To Reproduce

See: https://github.com/k-wall/rfc6902-json-patch/blob/master/README.md

I've provided a simple example. This example provides a statefulstate and a patch to mutate the environment variables of a statefulset using the following PATCH:

[
{
  "op" : "move",
  "from" : "/spec/template/spec/containers/0/env/1",
  "path" : "/spec/template/spec/containers/0/env/2"
}
 ]

Current Result

B has replaced D. This is incorrect.

                        "env": [
                            {
                                "name": "A",
                                "value": "a"
                            },
                            {
                                "name": "C",
                                "value": "c"
                            },
                            {
                                "name": "B",
                                "value": "b"
                            }
                        ],

Expected Result

B is moved to after C.

          "env" : [ {
            "name" : "A",
            "value" : "a"
          }, {
            "name" : "C",
            "value" : "c"
          }, {
            "name" : "B",
            "value" : "b"
          }, {
            "name" : "D",
            "value" : "d"
          } ],

[lucab]

Ack, I think you are hitting this logic bug in the underlying library evanphx/json-patch#69.

[k-wall]

@lucab thanks, indeed, that looks very likely to be our issue. I think 6eeaa8d means that this change is already in OpenShift 4.1, is that right? (Unfortunately, I can't easily test on on 4.1 at the moment but should be able to soon).

Do you know if there is a plan to backport to 3.11?

[lucab]

I don't know, but someone across @deads2k @sttts @soltysh may.

[sttts]

@lucab yes, looks like it is fixed in 4.1 with #22267.

[soltysh]

Based on previous comment I'm going to close this issue since it's solved with the current release.
/close

[openshift-ci-robot]

@soltysh: Closing this issue.

Details

In response to this:

Based on previous comment I'm going to close this issue since it's solved with the current release.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.