Kubectl -o jsonpath throws an error if field is not present in json

[pwittrock]

Moving the discussion to an issue

cc @smarterclayton @bgrant0607

In 1.5, the kubectl -o jsonpath command was changed to throw an error if the field referred to by the path is not found. Pre 1.4 the field might have been defaulted to a value and returned this value instead.

I am not sure if we consider this a api compatibility breaking change. The json path doc does NOT explicitly state what the behavior is when the field is not present in the JSON.

1.4 behavior:
If the field is present (non-pointer) in the struct, but missing from the JSON, return empty string
If the field is missing from the struct, exit 1 with an error

1.5 behavior:
If the field is missing from the JSON, exit 1 with an error

I have not been able to find a record of the original intention when the field is missing from the JSON. It would probably be best to support either behavior by exposing a flag to control the knobs introduced in #31714. I am not sure it is worth the risk introduced to get that into 1.5 though.

Causing skew test failures: #35741

[pwittrock]

Release 1.5 known issues link: #37134

kubectl get -o jsonpath=... will now throw an error if the path is to a field not present in the json, even if it has a default value in the underlying type.  This is a change from the old behavior, which would return the default value for some fields even if not present in the json.

[pwittrock]

I think I figured out the issue.

Correction: In 1.4 the default field value was returned, not just an empty string.

My current best theory is that in 1.4 the responses were round-tripped through the unversioned golang structs objects and back to JSON. At least some unversioned objects do not contain the omitempty tag that is present on the versioned objects (e.g. NodePort). This would result in the default fields being set on the struct and then written in the output json. This would explain why jsonpath was able to find these fields.

RE: The best 1.5 behavior
AFAIK we don't have a good way to emulate the 1.4 behavior of providing the defaults in jsonpath when they are missing from the json response. It is not clear to me that this is even the correct thing to do. It is relatively simple to change kubectl to return an empty string instead, but this doesn't seem to be a clear win either.

[smarterclayton]

According to JSONPath the spec an empty field is supposed to return empty value. It may be that we just implemented the spec in a way that resulted in this hidden error and did not notice. I think we should be more spec compliant rather than less, where possible. On Dec 2, 2016, at 6:59 PM, Phillip Wittrock <notifications@github.com> wrote: I think I figured out the issue. *Correction: In 1.4 the default field value was returned, not just an empty string.* My current best theory is that in 1.4 the responses were round-tripped through the unversioned golang structs objects and back to JSON. At least some unversioned objects do not contain the omitempty tag that is present on the versioned objects (e.g. NodePort). This would result in the default fields being set on the struct and then written in the output json. This would explain why jsonpath was able to find these fields. RE: The best 1.5 behavior AFAIK we don't have a good way to emulate the 1.4 behavior of providing the defaults in jsonpath when they are missing from the json response. It is not clear to me that this is even the correct thing to do. It is relatively simple to change kubectl to return an empty string instead, but this doesn't seem to be a clear win either. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#37991 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p1gS22ZQjH4PSRtJmWzG_cdL4Qkgks5rELDQgaJpZM4LDEs-> .

[smarterclayton]

- An alternative for jsonPath to return false in case of *no match* may be to return an empty array in future.

[smarterclayton]

Returning the default value seems wrong. Trying to find a clear answer among the maze of implementations. It does not seem to be correct to error in any of these implementations. Client code may differ from server side schema, and clients may not even know the schema (tpr, extension apis).

[dims]

@pwittrock Is it appropriate to move this to the next milestone or clear the 1.5 milestone? (and remove the non-release-blocker tag as well)

[pwittrock]

@smarterclayton

Trying to be more Spec compliant and returning an empty value when the field is empty makes sense to me. I'll clarify the new behavior in the release notes for 1.5 and we can figure out if we want to change this in 1.6

[liggitt]

I just ran into this... it depends if we're working off a go struct or the unstructured json

when we're dealing with unstructured json, missing fields return an error

when we're dealing with a go struct, missing fields return the default value

[pwittrock]

kubectl jsonpath was switch from gostruct to unstructured in 1.5 right?

[liggitt]

It's inconsistent… it depends what is passed in, and whether you are working with a list or not :(

https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/resource_printer.go#L2680

[smarterclayton]

I think this was a breaking change. It bit openshift very badly, and we depend on the behavior of JSONPath for a lot of scripts.

[smarterclayton]

@ncdc can you describe how prevalent this was?

[ncdc]

1. We have a test that evaluates a field in our DeploymentConfig.Status to ensure that its value is 0. Because the field is set to omitempty, and go's json encoder doesn't emit a field if its value is the default value (i.e. 0) or nil, we had to remove the omitempty tags so we didn't have to change the test
2. We have a test that is supposed to check for a pod's {.status.podIP} frequently, waiting up to 1 minute for it to be set. But because podIP is omitempty, you get this error instead: error executing jsonpath "{.status.podIP}": podIP is not found.
3. We have an ansible playbook that was using the jsonpath expression '{.spec.template.spec.volumes[*].secret.secretName}' and then grepping for 'secretName is not found', but because of this change, the output became secret is not found so our grep failed. Admittedly, however, the code in the playbook needed fixing anyways, and this change just highlighted that fact.

I'll add more as I find/remember them

[smarterclayton]

So basically anyone using jsonpath in scripting this way is potentially broken? All things considered, we have stopped releases for much less than this.

[smarterclayton]

Bumping priority so we can assess whether this constitutes an API regression

Copying in @kubernetes/api-reviewers @kubernetes/sig-cli-misc

[ncdc]

It looks like the implementation from http://goessner.net/articles/JsonPath/ returns "false" if the input doesn't match against the jsonpath expression. How about if we just always allow missing keys, instead of making it optional and off by default?

[ncdc]

I'm going to do a PR that:

1. Makes jsonpath relaxed by default; i.e., allow missing keys
2. Adds a --strict-templates flag that, if set, would disallow missing keys

[nikhiljindal]

Another example where this change broke a script: #36287 (we have since fixed the script so its not a problem any more).