runtime: remove When serialized in JSON, the format MUST adhere to the following pattern#1178
Conversation
|
|
||
| The state MAY include additional properties. | ||
|
|
||
| When serialized in JSON, the format MUST adhere to the following pattern: |
There was a problem hiding this comment.
w/o the MUST we lose the normative statement about how the JSON should look.
There was a problem hiding this comment.
(👋) You mean the fields included? Aren't those already described in the runtimeState section above? Or do you mean the formatting of the JSON?
There was a problem hiding this comment.
HOWDY! :-)
I mean the format of the JSON. For example, while it seems obvious what it should look like, technically w/o the MUST this JSON could be generated and claim to be compliant:
{
"metadata" {
"ociVersion": "0.2.0",
"id": "oci-container1",
"annotations": {
"myKey": "myValue"
}
},
"bundle": "/containers/redis",
"state": {
"status": "running",
"pid": 4422
}
}
since it still has all of the specified properties. The problem doesn't really show itself in JSON as well as other formats like XML where values could appear as new XML elements or as attributes on XML elements, but I believe the problem still does exist.
Basically, w/o the MUST we're doing "spec by example".
If there is an issue here (which I don't really think there is), we should resolve it by just adding a statement that whitespace isn't significant (per the json spec: https://www.rfc-editor.org/rfc/rfc7159 and https://www.json.org/json-en.html )
There was a problem hiding this comment.
I believe your concern is already covered by https://github.com/opencontainers/runtime-spec/blob/main/schema/state-schema.json
There was a problem hiding this comment.
If the spec said that people MUST adhere to the json schema then probably yes, but it doesn't say that. W/o that normative link there's no requirement to even look at it.
There was a problem hiding this comment.
btw, there's a reason the word "pattern" was used instead of just saying "it MUST adhere to the following".
There was a problem hiding this comment.
technically w/o the MUST this JSON could be generated and claim to be compliant:
(...) since it still has all of the specified properties.
Hm.. I didn't immediately interpret that definition as "fields may appear anywhere (nested)", and assumed defining them ("The state of a container includes the following properties") there (as "required" for some), should be interpreted as "top level field". Do you think that needs further detailing?
believe your concern is already covered by https://github.com/opencontainers/runtime-spec/blob/main/schema/state-schema.json
We could mention that for a JSON representation, there's a JSON-schema available to verify if the format is correct.
There was a problem hiding this comment.
Updated the PR:
When serialized in JSON, the format MUST adhere to the JSON Schema [`schema/state-schema.json`](schema/state-schema.json).…he following pattern` The sentence looked like as if it required a specific indentation pattern. Fix issue 1177 Signed-off-by: Akihiro Suda <[email protected]>
d679079 to
72efacb
Compare
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM if this helps removing the ambiguity 👍
|
LGTM |
The sentence looked like as if it required a specific indentation pattern.
Fix #1177