Add config validation and environment tests #109

mrunalp · 2015-08-12T23:43:27Z

The spec should have testing tools for validating the config and testing that an implementation provides the expected environment for the container process. This will involve launching a container process from a bundle that will introspect to make sure that the runtime set it up correctly.

Config validation (mrunalp)
Environment testing

philips · 2015-08-13T01:22:54Z

@mrunalp by environment testing you mean generating an actual bundle and testing, from inside of the container, that the runtime ran correctly, right?

mrunalp · 2015-08-13T01:26:46Z

@philips Yes, I have updated the description to clarify that.

wking · 2015-08-13T18:02:07Z

On Wed, Aug 12, 2015 at 04:43:28PM -0700, Mrunal Patel wrote:

Config validation (mrunalp)

Environment testing

I'm pretty sure we all agree that these are important, but I don't
think they're part of the spec. Can we get new repositories
implementing each of these? It's possible that OCT is already
the environment-testing portion of this issue.

For config validation, I'm open to including more machine-parsable
annotations for things like whether an attribute is required or
optional, but I'm also fine just leaving that in the human-readable
spec and leaving it to the config-testing implementation to write the
code enforcing those requirements by hand.

It would be nice if we could offer that config validation as a web
service on opencontainers.org.

Should this discussion be happening on the mailing list (see #104)?

vbatts · 2015-08-14T23:09:37Z

@wking having a validator in specs is most logical
On Aug 13, 2015 2:02 PM, "W. Trevor King" [email protected] wrote:

On Wed, Aug 12, 2015 at 04:43:28PM -0700, Mrunal Patel wrote:

Config validation (mrunalp)

Environment testing

I'm pretty sure we all agree that these are important, but I don't
think they're part of the spec. Can we get new repositories
implementing each of these? It's possible that OCT is already
the environment-testing portion of this issue.

For config validation, I'm open to including more machine-parsable
annotations for things like whether an attribute is required or
optional, but I'm also fine just leaving that in the human-readable
spec and leaving it to the config-testing implementation to write the
code enforcing those requirements by hand.

It would be nice if we could offer that config validation as a web
service on opencontainers.org.

Should this discussion be happening on the mailing list (see #104)?

—
Reply to this email directly or view it on GitHub
#109 (comment)
.

wking · 2015-08-15T18:30:05Z

On Fri, Aug 14, 2015 at 04:09:38PM -0700, Vincent Batts wrote:

@wking having a validator in specs is most logical

I imagine the relationship between config-validators and specs is
going to be many-to-many. For example, opencontainers.org will
probably have a web-service validator written in Go that validates all
released versions of the spec. But other folks may want validators
written in Python or C. Since validating a config (like writting a
runtime implementation) seems easy to separate from writing the spec,
it makes more sense to me to use a separate repository, which makes
the many-to-many relationship easy to deal with.

Can you give more background on why it seems more logical to you to
build the validator into the spec repository? How do you intend to
validate past versions of the spec? Do you intend to version-bump the
spec repository when you find and fix bugs in the validator, or change
the validator's user-facing API, but don't touch the spec itself?

vbatts · 2015-08-16T17:51:36Z

On Sat, Aug 15, 2015 at 2:30 PM, W. Trevor King [email protected]
wrote:

On Fri, Aug 14, 2015 at 04:09:38PM -0700, Vincent Batts wrote:

@wking having a validator in specs is most logical

I imagine the relationship between config-validators and specs is
going to be many-to-many. For example, opencontainers.org will
probably have a web-service validator written in Go that validates all
released versions of the spec. But other folks may want validators
written in Python or C. Since validating a config (like writting a
runtime implementation) seems easy to separate from writing the spec,
it makes more sense to me to use a separate repository, which makes
the many-to-many relationship easy to deal with.

By virtue of it being a spec, there is nothing stopping implementations and
validators.

Can you give more background on why it seems more logical to you to

build the validator into the spec repository? How do you intend to
validate past versions of the spec? Do you intend to version-bump the
spec repository when you find and fix bugs in the validator, or change
the validator's user-facing API, but don't touch the spec itself?

bugs in the validator would be addressed as they're encountered. There are
a number of projects that do this. Some came later due to keeping multiple
implementations bug-for-bug compatible (ruby), and others to ensure that
API is met and not regressed, (golang and the goapi cmd). This is not
ruling out any outside validators and web services. This is a sanity check,
which could allow policy that of breakage in a point release vs a major
version bump.

vb

wking · 2015-08-16T18:12:43Z

On Sun, Aug 16, 2015 at 10:51:38AM -0700, Vincent Batts wrote:

Sat, Aug 15, 2015 at 2:30 PM, W. Trevor King:

Can you give more background on why it seems more logical to you
to build the validator into the spec repository? How do you intend
to validate past versions of the spec? Do you intend to
version-bump the spec repository when you find and fix bugs in the
validator, or change the validator's user-facing API, but don't
touch the spec itself?

bugs in the validator would be addressed as they're encountered.

Is that “we don't intend to version the in-repo validator
independently of the spec”?

There are a number of projects that do this. Some came later due to
keeping multiple implementations bug-for-bug compatible (ruby), and
others to ensure that API is met and not regressed, (golang and the
goapi cmd).

I'm not familiar enough with Ruby or Go to know how they handle a
validator in the same repository as their language specification. Can
you point me at those repositories so I can get a better feel for
their workflow?

This is not ruling out any outside validators and web services.

The OCI already has separate repositories for the spec
(opencontainers/specs) and reference runtime (opencontainers/runc).
If the separate-repository approach was good enough for the runtime,
why would it not be good enough for the config validator?

Of course, many projects version their specification in the same
repository as their runtime and a validator, but I think that makes
it harder for alternative runtime/validator implementations to
collaborate on the common spec. For one thing, the issue and PR
queues will be cluttered up with non-spec content (e.g. validator
PRs), that folks working on external validators don't care about. Can
you point me to a project that does this and has alternative
validator implementations?

This is a sanity check, which could allow policy that of breakage in
a point release vs a major version bump.

I'm having trouble parsing this sentence. Are you worried that the
spec will land a backwards-incompatible change but only point-bump the
version when releasing that change? I don't see how an in-repository
validator helps protect from that case. Of course, running validator
tests in a CI suite would be nice if we were bundling a bunch of
example configs (and we do have lots of example fragments). But with
a web-API for validation, I don't see the need to have a local copy of
the validator source just to run that CI validation.

liangchenye · 2015-08-20T10:12:20Z

The 'validating the config' and 'comparing the real runtime environment with what defined in the config file' are both covered in OCT.

I just wrote a simple tool to do the configuration validating work.
It could be used without the OCT framework.
https://github.com/huawei-openlab/oct/tree/master/engine/tools/configvalidator

liangchenye · 2015-08-20T10:20:50Z

By making the validating more automatically, I added extra 'Tags' to the spec struct.
https://github.com/huawei-openlab/oct/blob/master/engine/tools/configvalidator/specs/spec.go

For example:
Version string json:"version" mandatory:"required" check:"SemVer v2.0.0"

It might be better to put such Tag to the spec.go and keep spec.go same with config.md.
It is easier for developers.

wking · 2015-08-20T17:12:24Z

On Thu, Aug 20, 2015 at 03:12:23AM -0700, 梁辰晔 (Liang Chenye) wrote:

The 'validating the config' and 'comparing the real runtime
environment with what defined in the config file' are both covered
in OCT.

Did you mean OCI? I think we all agree on that point. The
disagreement just seems to be whether we should use a separate
repository for the validator code 1 or put it in this repository
2.

I just wrote a simple tool to do the configuration validating work.
It could used without the OCT framework.
https://github.com/huawei-openlab/oct/tree/master/engine/tools/configvalidator

I like this separate-repository approach, but you'll probably want to
vendor the spec with Godeps (like they do in opencontainers/runc 3).
You tweaked the spec a bit to add your additional tags though, so it's
probably not worth switching to Godeps until we figure out whether
those tags will land in this repository.

wking · 2015-08-20T17:22:05Z

Ah, OCT meant 1.

On Thu, Aug 20, 2015 at 03:20:51AM -0700, 梁辰晔 (Liang Chenye) wrote:

Version string json:"version" mandatory:"required" check:"SemVer v2.0.0"

Using the ‘check’ to slot in more complicated logic 2 is a
reasonable safety valve for things that you can't deconstuct into
fields like a ‘mandatory’ enum. But then we'd have to define all
those checks in the spec, or at least list the valid check strings and
explain their semantics. I'm not sure it's that much of a gain
vs. the current human-readable SemVer reference 3. Couldn't you
switch on the configuration key path (here just ‘version’) to turn on
this sort of validation? Another tricky area for validation will be
the cgroups / mknod compound device handling (see #101), and you could
detect that with a key path of ‘linux.devices.*’.

I do think ‘mandatory’ is a clear win, since we use that sort of logic
for many fields.

liangchenye · 2015-08-21T02:05:18Z

You means adding another key-function map to do all the semantics check, for example:
[{ "path": "version", "func": "semvercheck"}, {"path": "linux.device", "func": "device_check"} ] ?

wking · 2015-08-21T04:47:12Z

On Thu, Aug 20, 2015 at 07:05:20PM -0700, 梁辰晔 (Liang Chenye) wrote:

You means adding another key-function map to do all the semantics check, for example:
[{ "path": "version", "func": "semvercheck"}, {"path": "linux.device", "func": "device_check"} ] ?

I mean something like:

func checkExtra(value reflect.Value, path []string) err error {
pathString := strings.join(path, ".");
if (pathString == "version") {
return checkSemVer(value.String());
} else if (pathString == "linux.devices") {
return checkLinuxDevices(value);
}
…
return nil;
}

But if you'd rather store that in a map of path-keys and associated
callbacks, that's fine too. I don't see a reason to have the extra
layer of indirection in path-keys → check-label → callback.

liangchenye · 2015-09-08T11:57:13Z

Misc tiny bugs found during validation tool developing.

runtime-config.md,
destination (string, required) where the source filesystem is mounted relative to the container rootfs. should be removed.
in runtime_config_linux.go, the type of rlimit is int
but in runtime-config-linux.md, it seems a string: "type": "RLIMIT_NPROC"
Suggest to change that to string just like the Capabilities (config-linux.md)
there will be another confusion after 2), the string in rlimit has the full name (RLIMIT_NPPOC)
.which is different with Capabilities.
Valid values are the string after CAP_ for capabilities
Suggest to change value in Capabilities to a full name too.

mrunalp · 2015-09-08T16:52:30Z

@liangchenye Thanks for reporting these issues. I have created #159 to handle 2. Other PRs on the way.

mrunalp · 2015-09-08T16:55:42Z

@liangchenye

destination (string, required) where the source filesystem is mounted relative to the container rootfs. should be removed.

This is valid. What do you find incorrect/unclear about it?

mrunalp · 2015-09-08T16:59:02Z

@liangchenye #160 to address 3.

wking · 2015-09-08T20:16:27Z

On Tue, Sep 08, 2015 at 04:57:15AM -0700, 梁辰晔 (Liang Chenye) wrote:

runtime-config.md,
destination (string, required) where the source filesystem is mounted relative to the container rootfs. should be removed.

Duplicate of #136?

there will be another confusion after 2), the string in rlimit
has the full name (RLIMIT_NPPOC) .which is different with
Capabilities. Valid values are the string after CAP_ for capabilities Suggest to change value in Capabilities to a full
name too.

I'd rather drop the leading RLIMIT_ from linux.rlimits.type.

liangchenye · 2015-09-09T00:43:13Z

Because 'destination' is no longer exist in the 'Mount' struct. @wking
type Mount struct {
// Type specifies the mount kind.
Type string json:"type"
// Source specifies the source path of the mount. In the case of bind mounts on
// linux based systems this would be the file on the host.
Source string json:"source"
// Options are fstab style mount options.
Options []string json:"options"
}

c18c283 (Change layout of mountpoints and mounts, 2015-09-02, opencontainers#136) removed the destination field from the Go type and examples, but forgot to remove it from the documentation [1]. Fix that with this commit. [1]: opencontainers#109 (comment) Reported-by: 梁辰晔 (Liang Chenye) <[email protected]> Signed-off-by: W. Trevor King <[email protected]>

wking · 2015-09-09T16:51:07Z

On Tue, Sep 08, 2015 at 05:43:15PM -0700, 梁辰晔 (Liang Chenye) wrote:

Because 'destination' is no longer exist in the 'Mount' struct.

Oh, I see. #136 missed removing this from the docs (and just removed
it from the Go types and examples). I've filed #165 to fix the
oversight.

vbatts · 2016-03-16T17:45:25Z

closing in favor of the #219 tracker

philips added the component/octool label Aug 13, 2015

wking mentioned this issue Sep 6, 2015

tooling: basic validation tool for config.json #43

Closed

wking mentioned this issue Sep 8, 2015

Change the rlimit type to string instead of int #159

Merged

wking mentioned this issue Sep 9, 2015

runtime-config: Remove 'destination' docs from mounts #165

Merged

vbatts closed this as completed Mar 16, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add config validation and environment tests #109

Add config validation and environment tests #109

mrunalp commented Aug 12, 2015

philips commented Aug 13, 2015

mrunalp commented Aug 13, 2015

wking commented Aug 13, 2015

vbatts commented Aug 14, 2015

wking commented Aug 15, 2015

vbatts commented Aug 16, 2015

wking commented Aug 16, 2015

liangchenye commented Aug 20, 2015

liangchenye commented Aug 20, 2015

wking commented Aug 20, 2015

wking commented Aug 20, 2015

liangchenye commented Aug 21, 2015

wking commented Aug 21, 2015

liangchenye commented Sep 8, 2015

mrunalp commented Sep 8, 2015

mrunalp commented Sep 8, 2015

mrunalp commented Sep 8, 2015

wking commented Sep 8, 2015

liangchenye commented Sep 9, 2015

wking commented Sep 9, 2015

vbatts commented Mar 16, 2016

Add config validation and environment tests #109

Add config validation and environment tests #109

Comments

mrunalp commented Aug 12, 2015

philips commented Aug 13, 2015

mrunalp commented Aug 13, 2015

wking commented Aug 13, 2015

vbatts commented Aug 14, 2015

wking commented Aug 15, 2015

vbatts commented Aug 16, 2015

wking commented Aug 16, 2015

liangchenye commented Aug 20, 2015

liangchenye commented Aug 20, 2015

wking commented Aug 20, 2015

wking commented Aug 20, 2015

liangchenye commented Aug 21, 2015

wking commented Aug 21, 2015

liangchenye commented Sep 8, 2015

mrunalp commented Sep 8, 2015

mrunalp commented Sep 8, 2015

mrunalp commented Sep 8, 2015

wking commented Sep 8, 2015

liangchenye commented Sep 9, 2015

wking commented Sep 9, 2015

vbatts commented Mar 16, 2016