Drop image tools cmd/ and image/ #337
Conversation
|
@wking travis build needs to be updated as well. |
|
I thought the plan was to maintain the validation package here. How will we continue to validate the specification if we accept this PR? |
|
On Wed, Sep 21, 2016 at 03:30:30PM -0700, Stephen Day wrote:
I don't really have a preference for where validation lives. The While runtime-spec occasionally has to fix buggy JSON Schema or Go However, I agree that having local validation to support ‘make
is sufficient to catch most example errors. Weighing the benefit of So having outlined the runtime approach and given some background on |
|
@wking Could you summarize what you are trying to say? |
|
On Thu, Sep 22, 2016 at 06:49:58PM -0700, Stephen Day wrote:
I like the PR as it stands, for the reasons I've listed 1. It |
|
|
On Fri, Sep 23, 2016 at 03:49:52PM -0700, Stephen Day wrote:
This PR keeps all of the JSON Schema stuff locally, and I explain in |
|
@wking Explain it in a single paragraph so I don't have to read a book. Again, please answer the question:
Rules for your response:
To be clear, the agreement we had when creating the tools repo was to leave the validation code in the spec repo. This includes multiple versions of validation, since the specification has to consider multiple versions, as well. The tools repo was just supposed to be a cmd directory. |
|
On Mon, Sep 26, 2016 at 11:59:06AM -0700, Stephen Day wrote:
I can give one paragraph responses, but they cut corners. Linking to
I think it is better to have validation beyond comparison with the
I'm not sure there is a consensus around that among image-spec |
We had this consensus in the face to face meeting. I consider the fact that this in the process changed an oversight at minimum.
Specifications don't exist in a vacuum. v1.4 needs to be backwards compatible with v1.0 to meet semantic versioning requirements. This isn't that hard and having the specification and validator in sync is more important than these other concerns. |
|
On Mon, Sep 26, 2016 at 12:33:33PM -0700, Stephen Day wrote:
Agreed. But the backward-compatibility test is “Does v1.4 tooling
Not having typos in the spec is important. But the v1.4 spec isn't |
|
On Mon, Sep 26, 2016 at 11:59:06AM -0700, Stephen Day wrote:
I took a local stab at rerolling this branch to only drop cmd/ (since |
|
@stevvooe is this PR in line with what you described last week on the call? |
These have been spun off into [1]. # glide.yaml * Drop runtime-spec (the last reference was removed with image/config.go). * Drop cobra (the last references were removed with cmd/). # HACKING The http/FileSystem stuff still applies, since the JSON Schema validation is staying in this repo. I've made it's earlier oci-image-tool references more generic, and adopted the README's recommended one sentence per line where I touched a line. [1]: https://github.com/opencontainers/image-tools Signed-off-by: W. Trevor King <[email protected]>
With: $ rm -rf vendor $ make update-deps Signed-off-by: W. Trevor King <[email protected]>
|
On Wed, Oct 19, 2016 at 04:49:43PM -0700, Stephen Day wrote:
I think it's easier to review and understand when the more detailed |
With image-tools split off into its own repository, the plan seems to
be to keep all intra-blob JSON validation in this repository and to
move all other validation (e.g. for layers or for walking Merkle
trees) in image-tools [1]. All the non-validation logic currently in
image/ is moving into image-tools as well [2].
Some requirements (e.g. multi-parameter checks like allowed OS/arch
pairs [3]) are difficult to handle in JSON Schema but easy to handle
in Go. And callers won't care if we're using JSON Schema or not; they
just want to know if their blob is valid.
This commit restructures intra-blob validation to ease the path going
forward (although it doesn't actually change the current validation
significantly). The old method:
func (v Validator) Validate(src io.Reader) error
is now a new Validator type:
type Validator(blob io.Reader, descriptor *v1.Descriptor, strict bool) (err error)
and instead of instantiating an old Validator instance:
schema.MediaTypeImageConfig.Validate(reader)
there's a Validators registry mapping from the media type strings to
the appropriate Validator instance (which may or may not use JSON
Schema under the hood). And there's a Validate function (with the
same Validator interface) that looks up the appropriate entry in
Validators for you so you have:
schema.Validate(reader, descriptor, true)
By using a Validators map, we make it easy for library consumers to
register (or override) intra-blob validators for a particular type.
Locations that call Validate(...) will automatically pick up the new
validators without needing local changes.
All of the old validation was based on JSON Schema, so currently all
Validators values are ValidateJSONSchema. As the schema package grows
non-JSON-Schema validation, entries will start to look like:
var Validators = map[string]Validator{
v1.MediaTypeImageConfig: ValidateConfig,
...
}
although ValidateConfig will probably use ValidateJSONSchema
internally.
By passing through a descriptor, we get a chance to validate the
digest and size (which we were not doing before). Digest and size
validation for a byte array are also exposed directly (as
ValidateByteDigest and ValidateByteSize) for use in validators that
are not based on ValidateJSONSchema. Access to the digest also gives
us a way to print specific error messages on failures. In situations
where you don't know the blob digest, the new DigestByte will help you
calculate it (for a byte array).
There is also a new 'strict' parameter to distinguish between
compliant images (which should only pass when strict is false) and
images that only use features which the spec requires implementations
to support (which should pass regardless of strict). The current JSON
Schemas are not strict, and I expect we'll soon gain Go code to handle
the distinction (e.g. [4]). So the presence of 'strict' in the
Validator type is future-proofing our API and not exposing a
currently-implemented feature.
I've made the minimal sane changes to cmd/ and image/, because we're
dropping them from this repository [2] (and continuing them in
runtime-tools).
[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-10-12-21.01.log.html#l-71
[2]: opencontainers#337
[3]: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.5
[4]: opencontainers#341
Signed-off-by: W. Trevor King <[email protected]>
With image-tools split off into its own repository, the plan seems to
be to keep all intra-blob JSON validation in this repository and to
move all other validation (e.g. for layers or for walking Merkle
trees) in image-tools [1]. All the non-validation logic currently in
image/ is moving into image-tools as well [2].
Some requirements (e.g. multi-parameter checks like allowed OS/arch
pairs [3]) are difficult to handle in JSON Schema but easy to handle
in Go. And callers won't care if we're using JSON Schema or not; they
just want to know if their blob is valid.
This commit restructures intra-blob validation to ease the path going
forward (although it doesn't actually change the current validation
significantly). The old method:
func (v Validator) Validate(src io.Reader) error
is now a new Validator type:
type Validator(blob io.Reader, descriptor *v1.Descriptor, strict bool) (err error)
and instead of instantiating an old Validator instance:
schema.MediaTypeImageConfig.Validate(reader)
there's a Validators registry mapping from the media type strings to
the appropriate Validator instance (which may or may not use JSON
Schema under the hood). And there's a Validate function (with the
same Validator interface) that looks up the appropriate entry in
Validators for you so you have:
schema.Validate(reader, descriptor, true)
By using a Validators map, we make it easy for library consumers to
register (or override) intra-blob validators for a particular type.
Locations that call Validate(...) will automatically pick up the new
validators without needing local changes.
All of the old validation was based on JSON Schema, so currently all
Validators values are ValidateJSONSchema. As the schema package grows
non-JSON-Schema validation, entries will start to look like:
var Validators = map[string]Validator{
v1.MediaTypeImageConfig: ValidateConfig,
...
}
although ValidateConfig will probably use ValidateJSONSchema
internally.
By passing through a descriptor, we get a chance to validate the
digest and size (which we were not doing before). Digest and size
validation for a byte array are also exposed directly (as
ValidateByteDigest and ValidateByteSize) for use in validators that
are not based on ValidateJSONSchema. Access to the digest also gives
us a way to print specific error messages on failures. In situations
where you don't know the blob digest, the new DigestByte will help you
calculate it (for a byte array).
There is also a new 'strict' parameter to distinguish between
compliant images (which should only pass when strict is false) and
images that only use features which the spec requires implementations
to support (which should pass regardless of strict). The current JSON
Schemas are not strict, and I expect we'll soon gain Go code to handle
the distinction (e.g. [4]). So the presence of 'strict' in the
Validator type is future-proofing our API and not exposing a
currently-implemented feature.
I've made the minimal sane changes to cmd/ and image/, because we're
dropping them from this repository [2] (and continuing them in
runtime-tools).
[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-10-12-21.01.log.html#l-71
[2]: opencontainers#337
[3]: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.5
[4]: opencontainers#341
Signed-off-by: W. Trevor King <[email protected]>
With image-tools split off into its own repository, the plan seems to
be to keep all intra-blob JSON validation in this repository and to
move all other validation (e.g. for layers or for walking Merkle
trees) in image-tools [1]. All the non-validation logic currently in
image/ is moving into image-tools as well [2].
Some requirements (e.g. multi-parameter checks like allowed OS/arch
pairs [3]) are difficult to handle in JSON Schema but easy to handle
in Go. And callers won't care if we're using JSON Schema or not; they
just want to know if their blob is valid.
This commit restructures intra-blob validation to ease the path going
forward (although it doesn't actually change the current validation
significantly). The old method:
func (v Validator) Validate(src io.Reader) error
is now a new Validator type:
type Validator(blob io.Reader, descriptor *v1.Descriptor, strict bool) (err error)
and instead of instantiating an old Validator instance:
schema.MediaTypeImageConfig.Validate(reader)
there's a Validators registry mapping from the media type strings to
the appropriate Validator instance (which may or may not use JSON
Schema under the hood). And there's a Validate function (with the
same Validator interface) that looks up the appropriate entry in
Validators for you so you have:
schema.Validate(reader, descriptor, true)
By using a Validators map, we make it easy for library consumers to
register (or override) intra-blob validators for a particular type.
Locations that call Validate(...) will automatically pick up the new
validators without needing local changes.
All of the old validation was based on JSON Schema, so currently all
Validators values are ValidateJSONSchema. As the schema package grows
non-JSON-Schema validation, entries will start to look like:
var Validators = map[string]Validator{
v1.MediaTypeImageConfig: ValidateConfig,
...
}
although ValidateConfig will probably use ValidateJSONSchema
internally.
By passing through a descriptor, we get a chance to validate the
digest and size (which we were not doing before). Digest and size
validation for a byte array are also exposed directly (as
ValidateByteDigest and ValidateByteSize) for use in validators that
are not based on ValidateJSONSchema. Access to the digest also gives
us a way to print specific error messages on failures. In situations
where you don't know the blob digest, the new DigestByte will help you
calculate it (for a byte array).
There is also a new 'strict' parameter to distinguish between
compliant images (which should only pass when strict is false) and
images that only use features which the spec requires implementations
to support (which should pass regardless of strict). The current JSON
Schemas are not strict, and I expect we'll soon gain Go code to handle
the distinction (e.g. [4]). So the presence of 'strict' in the
Validator type is future-proofing our API and not exposing a
currently-implemented feature.
I've made the minimal sane changes to cmd/ and image/, because we're
dropping them from this repository [2] (and continuing them in
runtime-tools).
[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-10-12-21.01.log.html#l-71
[2]: opencontainers#337
[3]: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.5
[4]: opencontainers#341
Signed-off-by: W. Trevor King <[email protected]>
|
LGTM |
1 similar comment
|
LGTM |
With image-tools split off into its own repository, the plan seems to
be to keep all intra-blob JSON validation in this repository and to
move all other validation (e.g. for layers or for walking Merkle
trees) in image-tools [1]. All the non-validation logic currently in
image/ is moving into image-tools as well [2].
Some requirements (e.g. multi-parameter checks like allowed OS/arch
pairs [3]) are difficult to handle in JSON Schema but easy to handle
in Go. And callers won't care if we're using JSON Schema or not; they
just want to know if their blob is valid.
This commit restructures intra-blob validation to ease the path going
forward (although it doesn't actually change the current validation
significantly). The old method:
func (v Validator) Validate(src io.Reader) error
is now a new Validator type:
type Validator(blob io.Reader, descriptor *v1.Descriptor, strict bool) (err error)
and instead of instantiating an old Validator instance:
schema.MediaTypeImageConfig.Validate(reader)
there's a Validators registry mapping from the media type strings to
the appropriate Validator instance (which may or may not use JSON
Schema under the hood). And there's a Validate function (with the
same Validator interface) that looks up the appropriate entry in
Validators for you so you have:
schema.Validate(reader, descriptor, true)
By using a Validators map, we make it easy for library consumers to
register (or override) intra-blob validators for a particular type.
Locations that call Validate(...) will automatically pick up the new
validators without needing local changes.
All of the old validation was based on JSON Schema, so currently all
Validators values are ValidateJSONSchema. As the schema package grows
non-JSON-Schema validation, entries will start to look like:
var Validators = map[string]Validator{
v1.MediaTypeImageConfig: ValidateConfig,
...
}
although ValidateConfig will probably use ValidateJSONSchema
internally.
By passing through a descriptor, we get a chance to validate the
digest and size (which we were not doing before). Digest and size
validation for a byte array are also exposed directly (as
ValidateByteDigest and ValidateByteSize) for use in validators that
are not based on ValidateJSONSchema. Access to the digest also gives
us a way to print specific error messages on failures. In situations
where you don't know the blob digest, the new DigestByte will help you
calculate it (for a byte array).
There is also a new 'strict' parameter to distinguish between
compliant images (which should only pass when strict is false) and
images that only use features which the spec requires implementations
to support (which should pass regardless of strict). The current JSON
Schemas are not strict, and I expect we'll soon gain Go code to handle
the distinction (e.g. [4]). So the presence of 'strict' in the
Validator type is future-proofing our API and not exposing a
currently-implemented feature.
[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-10-12-21.01.log.html#l-71
[2]: opencontainers#337
[3]: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.5
[4]: opencontainers#341
Signed-off-by: W. Trevor King <[email protected]>
With image-tools split off into its own repository, the plan seems to
be to keep all intra-blob JSON validation in this repository and to
move all other validation (e.g. for layers or for walking Merkle
trees) in image-tools [1]. All the non-validation logic currently in
image/ is moving into image-tools as well [2].
Some requirements (e.g. multi-parameter checks like allowed OS/arch
pairs [3]) are difficult to handle in JSON Schema but easy to handle
in Go. And callers won't care if we're using JSON Schema or not; they
just want to know if their blob is valid.
This commit restructures intra-blob validation to ease the path going
forward (although it doesn't actually change the current validation
significantly). The old method:
func (v Validator) Validate(src io.Reader) error
is now a new Validator type:
type Validator(blob io.Reader, descriptor *v1.Descriptor, strict bool) (err error)
and instead of instantiating an old Validator instance:
schema.MediaTypeImageConfig.Validate(reader)
there's a Validators registry mapping from the media type strings to
the appropriate Validator instance (which may or may not use JSON
Schema under the hood). And there's a Validate function (with the
same Validator interface) that looks up the appropriate entry in
Validators for you so you have:
schema.Validate(reader, descriptor, true)
By using a Validators map, we make it easy for library consumers to
register (or override) intra-blob validators for a particular type.
Locations that call Validate(...) will automatically pick up the new
validators without needing local changes.
All of the old validation was based on JSON Schema, so currently all
Validators values are ValidateJSONSchema. As the schema package grows
non-JSON-Schema validation, entries will start to look like:
var Validators = map[string]Validator{
v1.MediaTypeImageConfig: ValidateConfig,
...
}
although ValidateConfig will probably use ValidateJSONSchema
internally.
By passing through a descriptor, we get a chance to validate the
digest and size (which we were not doing before). Digest and size
validation for a byte array are also exposed directly (as
ValidateByteDigest and ValidateByteSize) for use in validators that
are not based on ValidateJSONSchema. Access to the digest also gives
us a way to print specific error messages on failures. In situations
where you don't know the blob digest, the new DigestByte will help you
calculate it (for a byte array).
There is also a new 'strict' parameter to distinguish between
compliant images (which should always pass when strict is false) and
images that only use features which the spec requires implementations
to support (which should only pass if strict is true). The current
JSON Schemas are not strict, but the config/layer media type checks in
ValidateManifest exercise this distinction.
[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-10-12-21.01.log.html#l-71
[2]: opencontainers#337
[3]: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.5
[4]: opencontainers#341
Signed-off-by: W. Trevor King <[email protected]>
| stderr.Println(err) | ||
| os.Exit(1) | ||
| } | ||
| } |
There was a problem hiding this comment.
You delete this file in this place , but in the image-tools and did not restore the relevant content, why not？ I think it is useful to use.
There was a problem hiding this comment.
image-tools has all of the previous subcommands, just promoted to independent binaries. What were you missing?
There was a problem hiding this comment.
I mean, like the file, the set of all sub commands into a whole.
There was a problem hiding this comment.
I think the image-tools maintainers made an explicit decision to split the former sub-commands out into their own binaries (#296, opencontainers/image-tools#1). I don't have strong feelings on that one way or the other. What this PR is about is that none of those commands belong in the spec ;). If you feel like the wrapping command is important, it's probably best to open an issue arguing your case in image-tools.
With image-tools split off into its own repository, the plan seems to
be to keep all intra-blob JSON validation in this repository and to
move all other validation (e.g. for layers or for walking Merkle
trees) in image-tools [1]. All the non-validation logic currently in
image/ is moving into image-tools as well [2].
Some requirements (e.g. multi-parameter checks like allowed OS/arch
pairs [3]) are difficult to handle in JSON Schema but easy to handle
in Go. And callers won't care if we're using JSON Schema or not; they
just want to know if their blob is valid.
This commit restructures intra-blob validation to ease the path going
forward (although it doesn't actually change the current validation
significantly). The old method:
func (v Validator) Validate(src io.Reader) error
is now a new Validator type:
type Validator(blob io.Reader, descriptor *v1.Descriptor, strict bool) (err error)
and instead of instantiating an old Validator instance:
schema.MediaTypeImageConfig.Validate(reader)
there's a Validators registry mapping from the media type strings to
the appropriate Validator instance (which may or may not use JSON
Schema under the hood). And there's a Validate function (with the
same Validator interface) that looks up the appropriate entry in
Validators for you so you have:
schema.Validate(reader, descriptor, true)
By using a Validators map, we make it easy for library consumers to
register (or override) intra-blob validators for a particular type.
Locations that call Validate(...) will automatically pick up the new
validators without needing local changes.
All of the old validation was based on JSON Schema, so currently all
Validators values are ValidateJSONSchema. As the schema package grows
non-JSON-Schema validation, entries will start to look like:
var Validators = map[string]Validator{
v1.MediaTypeImageConfig: ValidateConfig,
...
}
although ValidateConfig will probably use ValidateJSONSchema
internally.
By passing through a descriptor, we get a chance to validate the
digest and size (which we were not doing before). Digest and size
validation for a byte array are also exposed directly (as
ValidateByteDigest and ValidateByteSize) for use in validators that
are not based on ValidateJSONSchema. Access to the digest also gives
us a way to print specific error messages on failures.
There is also a new 'strict' parameter to distinguish between
compliant images (which should always pass when strict is false) and
images that only use features which the spec requires implementations
to support (which should only pass if strict is true). The current
JSON Schemas are not strict, but the config/layer media type checks in
ValidateManifest exercise this distinction.
Also use go-digest for local hashing now that we're vendoring it.
[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-10-12-21.01.log.html#l-71
[2]: opencontainers#337
[3]: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.5
[4]: opencontainers#341
Signed-off-by: W. Trevor King <[email protected]>
With image-tools split off into its own repository, the plan seems to
be to keep all intra-blob JSON validation in this repository and to
move all other validation (e.g. for layers or for walking Merkle
trees) in image-tools [1]. All the non-validation logic currently in
image/ is moving into image-tools as well [2].
Some requirements (e.g. multi-parameter checks like allowed OS/arch
pairs [3]) are difficult to handle in JSON Schema but easy to handle
in Go. And callers won't care if we're using JSON Schema or not; they
just want to know if their blob is valid.
This commit restructures intra-blob validation to ease the path going
forward (although it doesn't actually change the current validation
significantly). The old method:
func (v Validator) Validate(src io.Reader) error
is now a new Validator type:
type Validator(blob io.Reader, descriptor *v1.Descriptor, strict bool) (err error)
and instead of instantiating an old Validator instance:
schema.MediaTypeImageConfig.Validate(reader)
there's a Validators registry mapping from the media type strings to
the appropriate Validator instance (which may or may not use JSON
Schema under the hood). And there's a Validate function (with the
same Validator interface) that looks up the appropriate entry in
Validators for you so you have:
schema.Validate(reader, descriptor, true)
By using a Validators map, we make it easy for library consumers to
register (or override) intra-blob validators for a particular type.
Locations that call Validate(...) will automatically pick up the new
validators without needing local changes.
All of the old validation was based on JSON Schema, so currently all
Validators values are ValidateJSONSchema. As the schema package grows
non-JSON-Schema validation, entries will start to look like:
var Validators = map[string]Validator{
v1.MediaTypeImageConfig: ValidateConfig,
...
}
although ValidateConfig will probably use ValidateJSONSchema
internally.
By passing through a descriptor, we get a chance to validate the
digest and size (which we were not doing before). Digest and size
validation for a byte array are also exposed directly (as
ValidateByteDigest and ValidateByteSize) for use in validators that
are not based on ValidateJSONSchema. Access to the digest also gives
us a way to print specific error messages on failures.
There is also a new 'strict' parameter to distinguish between
compliant images (which should always pass when strict is false) and
images that only use features which the spec requires implementations
to support (which should only pass if strict is true). The current
JSON Schemas are not strict, but the config/layer media type checks in
ValidateManifest exercise this distinction.
Also use go-digest for local hashing now that we're vendoring it.
[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-10-12-21.01.log.html#l-71
[2]: opencontainers#337
[3]: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.5.5
[4]: opencontainers#341
Signed-off-by: W. Trevor King <[email protected]>
5 participants
They are now in opencontainers/image-tools. Also update the
dependencies. Details in the commit messages.