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

[ Config | Root Config ] Clarify readonly #476

Closed
wants to merge 1 commit into from
Closed

[ Config | Root Config ] Clarify readonly #476

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
1 change: 1 addition & 0 deletions config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Each container has exactly one *root filesystem*, specified in the *root* object

* **`path`** (string, required) Specifies the path to the root filesystem for the container. A directory MUST exist at the path declared by the field.
* **`readonly`** (bool, optional) If true then the root filesystem MUST be read-only inside the container. Defaults to false.
If readonly is present and set to true, but not supported by the platform, the platform MAY ignore.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that failing may be a better option.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree failing seems better.

Copy link
Member

@cyphar cyphar Jun 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

In general with configs, failing is much better than silently ignoring errors. A readonly path means that the user has made certain assumptions about the setup and might be quite confused if suddenly things break underneath then.

For the Microsoft case, just make it so that your runtime errors out if a user specifies readonly (though we should make it clear somewhere in the spec whether or not all runtimes need to implement all optional options).

Copy link
Member

Choose a reason for hiding this comment

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

+1; @RobDolinMS can you either update, close, or make a good argument for why you think ignoring is preferable? 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

On Thu, Jun 09, 2016 at 10:25:55PM -0700, Aleksa Sarai wrote:

For the Microsoft case, just make it so that your runtime errors out
if a user specifies readonly (though we should make it clear
somewhere in the spec whether or not all runtimes need to implement
all optional options).

I've taken a stab at part of the general behavior in #559. I'm not
quite sure how to flag options which a runtime can not support while
still being certified. Some thoughts on that in #559 too.


### Example

Expand Down