Configure jsonpatch ApplyOptions

[St0rmingBr4in]

Problem

JSONpatch does not support adding a value if it does not exist already. In fact the spec says that A member to add to an existing object - whereupon the supplied value is added to that object at the indicated location. If the member already exists, it is replaced by the specified value.

It is expected to first use the test operation to check for the presence of the object before doing the operation.

Example

In the current implementation it is not possible to do the following:

Let's say we have multiple Namespaces and we want to add a label on each of them:

# manifest-original.yaml
---
apiVersion: v1
metadata:
  name: ns-1
---
apiVersion: v1
metadata:
  name: ns-2
  labels:
    do-not: override-me

# manifest-target.yaml
---
apiVersion: v1
metadata:
  name: ns-1
  labels:
    new: label
  
---
apiVersion: v1
metadata:
  name: ns-2
  labels:
    do-not: override-me
    new: label

Attempt 1 using the current implementation

Trying to do it like this fails since metadata/labels is not defined in ns-1

# kustomize.yaml
---
resources:
- manifest.yaml
patches:
- patch: -|
  - op: add
    path: /metadata/labels/new
    value: label

Attempt 2 using the current implementation

# kustomize.yaml
---
resources:
- manifest.yaml
patches:
- patch: -|
  - op: add
    path: /metadata/labels
    value: null
- patch: -|
  - op: add
    path: /metadata/labels/new
    value: label

This works but overrides the predified label in ns-2 There is no way to create an empty /metadata/labels path without overriding the existing labels.

How to fix this

Configure the json-patch module with custom options

According to the json-patch module documentation: Structure jsonpatch.ApplyOptions includes the configuration options above and adds two new options: AllowMissingPathOnRemove and EnsurePathExistsOnAdd.

Being able to configure this in the kustomize file directly means we can solve the problem:

# kustomize.yaml
---
resources:
- manifest.yaml
patches:
- patch: -|
  - op: add
    path: /metadata/labels/new
    value: label
jsonpatchoptions:
  AllowMissingPathOnRemove: false
  EnsurePathExistsOnAdd: true

We just need to replace the Apply method here with the ApplyWithOptions method and pass the jsonpatch.ApplyOptions as parameter.

Other explored options

The JSONPatch spec specifies that Members that are not explicitly defined for the operation in question MUST be ignored (i.e., the operation will complete as if the undefined member did not appear in the object). Thus we could extend the JSONPatch spec by introducing a condition field.

However there is no way to check for existence using the JSONPatch test operator it seems. We could check the error message generated by the json-patch module to distinguish absence of an object versus the test operator failed but it seems fragile. https://github.com/evanphx/json-patch/blob/6a7ad8a3ea2159744c01c0354b7c41646bd4f70b/v5/patch.go#L34

The kustomize file could look like this:

# kustomize.yaml
---
resources:
- manifest.yaml
patches:
- patch: -|
  - op: add
    path: /metadata/labels
    value: null
    condition:
      value: 404
      test:
      - op: test
        path: /metadata/labels
        value: null
- patch: -|
  - op: add
    path: /metadata/labels/new
    value: label 

[k8s-ci-robot]

@St0rmingBr4in: This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

[KnVerey]

Why do you need to use jsonpatch specifically for this? Strategic merge patch supports creation and will merge correctly with existing values. For annotations specifically, you can also use the annotations field.

[St0rmingBr4in]

Why do you need to use jsonpatch specifically for this?

I don't strictly need this since using Strategic Merge Patch works yes. It would be nice if jsonpatch could support doing the same.

[natasha41575]

Since we have other kustomization fields that handle the use case you've described, and we'd like to limit the complexity of the kustomization file, I encourage you to use the other fields instead. If you have a use case that is not currently supported by any current kustomization fields, please feel free to reopen the issue with your example.

[marziply]

Why do you need to use jsonpatch specifically for this? Strategic merge patch supports creation and will merge correctly with existing values. For annotations specifically, you can also use the annotations field.

I don't believe this is the case for arrays. When attempting to use the add operation in conjunction with the - path item, if the value does not exist, an error is thrown.

add operation does not apply: doc is missing path: \"/spec/template/spec/containers/0/volumeMounts/-\": missing value"