kubernetes-client icon indicating copy to clipboard operation
kubernetes-client copied to clipboard

Setting existing value to `null` generates wrong JSON Patch (replace, not remove)

Open csviri opened this issue 3 years ago • 2 comments

Is your task related to a problem? Please describe

When calling edit on a status of a custom resource:

 public R patchStatus(R resource, R originalResource) {
      log.trace("Updating status for resource: {}", resource);
      String resourceVersion = resource.getMetadata().getResourceVersion();
      // don't do optimistic locking on patch
      originalResource.getMetadata().setResourceVersion(null);
      resource.getMetadata().setResourceVersion(null);
      try (var bis = new ByteArrayInputStream(
          Serialization.asJson(originalResource).getBytes(StandardCharsets.UTF_8))) {
        return resourceOperation
            .inNamespace(resource.getMetadata().getNamespace())
            .load(bis)
            .editStatus(r -> resource);
      } catch (IOException e) {
        throw new IllegalStateException(e);
      } finally {
        // restore initial resource version
        originalResource.getMetadata().setResourceVersion(resourceVersion);
        resource.getMetadata().setResourceVersion(resourceVersion);
      }
    }
  }

generates the following patch: [{"op":"replace","path":"/status/message","value":null}] if the value is set to null in the resource.

This makes Kubernetes reject the request on older versions (1.19.x and below).

Describe the solution you'd like

The proper patch should be: [{"op":"remove","path":"/status/message"]

Describe alternatives you've considered

No response

Additional context

No response

csviri avatar May 18 '22 08:05 csviri

Ok, not sure if this is actual issue with the patch or the kubernetes API not supporting this. Given that this is an issue only on not supported K8S versions we might want just ignore this problem. ( https://kubernetes.io/releases/ )

csviri avatar May 18 '22 10:05 csviri

The nuance here is that the your status must be defaulting to preserving, rather than omitting, nulls. So the diff utility considers:

status: message: null

replacing the old value with null, but if it were to render as just

status:

the diff utility would consider that a remove.

I am not sure if in every circumstance to the api server if absence and literal null are treated the same - if they are then yes the diff utility could produce a patch with remove in this case.

shawkins avatar May 18 '22 11:05 shawkins

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

stale[bot] avatar Aug 16 '22 23:08 stale[bot]