The patch command should always fail when a given patch is not applied

[rspurgeon]

Examples

Assume in.yaml:

_plugin_configs: {}
data:
  hello: "world"
  stuffs: ["abc", "123"]
  foo: null
  bar:
  obj:
    dog: "barks"

A value that is null, an array, or a scalar cannot have an object patched. This should result in an error and a non-zero exit code. Currently the tool returns a 0 exit code and the unaffected input.

cat in.yaml | deck file patch --selector "$.data.foo" --value 'xyz:"abc"'
_plugin_configs: {}
data:
  bar: null
  foo: null
  hello: world
  obj:
    dog: barks

This ticket should also capture any additional ways that the patch command fails silently that we are aware of.

These are bugs and not breaking changes. Any existing users that "rely" on this broken behavior have the opportunity to resolve the incorrect usage of the tool because of the new raised failure.

[Tieske]

Repeating what I wrote in the linked issue, but more on topic here;

I do not think that is helpful in this case; The patch operations explicitly defined the patch to be on an array or an object. So the user clearly expresses their intent to patch only those types. So it is safe to ignore other values. Consider the following json:

{
  "filter_by": {
    "tags": true,
    "name": false
  },
  "data": {
    "key1": {
      "tags": { "hello": "world" } 
    },
    "key2":{
      "tags": { "hello": "world" } 
    },
    "tags": { "hello": "world" } 
  },
  "tags": { "hello": "world" } 
}

Currently a recursive patch for all "tags" objects, setting field; "hello": "universe", will work as intended. Since it will skip $.filter_by.tags since it is a boolean.

If we throw a hard error on this then for a user to work around this would be quite cumbersome I think. Especially because of the dynamic data we're dealing with. Any custom plugin can introduce a field that has a name collision with an existing Kong field.

How about adding a flag to make it configurable such that cases like this will still work?

eg. --fail-on-non-matching-type (or preferably something shorter), and print a warning otherwise.

[rspurgeon]

will work as intended

This feels like an assumption. It's not obvious why the patch shouldn't work in this case, the user has to know we skip because of a type check that may or may not be useful to them.

The selector provided by the user should scope the targets of the patch such that only the intended values are updated