Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: remove When serialized in JSON, the format MUST adhere to the following pattern #1178

Merged
merged 1 commit into from
Feb 8, 2023
Merged

runtime: remove When serialized in JSON, the format MUST adhere to the following pattern #1178

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions runtime.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ The state of a container includes the following properties:

The state MAY include additional properties.

When serialized in JSON, the format MUST adhere to the following pattern:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

w/o the MUST we lose the normative statement about how the JSON should look.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(👋) You mean the fields included? Aren't those already described in the runtimeState section above? Or do you mean the formatting of the JSON?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe your concern is already covered by https://github.com/opencontainers/runtime-spec/blob/main/schema/state-schema.json

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, there's a reason the word "pattern" was used instead of just saying "it MUST adhere to the following".

Copy link
Member

@thaJeztah thaJeztah Feb 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the PR:

When serialized in JSON, the format MUST adhere to the JSON Schema [`schema/state-schema.json`](schema/state-schema.json).

When serialized in JSON, the format MUST adhere to the JSON Schema [`schema/state-schema.json`](schema/state-schema.json).

See [Query State](#query-state) for information on retrieving the state of a container.

### Example

```json
{
Expand All @@ -47,8 +51,6 @@ When serialized in JSON, the format MUST adhere to the following pattern:
}
```

See [Query State](#query-state) for information on retrieving the state of a container.

## <a name="runtimeLifecycle" />Lifecycle
The lifecycle describes the timeline of events that happen from when a container is created to when it ceases to exist.

Expand Down