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.

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

spec: describe descriptors and digests #111

Merged
merged 2 commits into from
Jun 9, 2016

Conversation

stevvooe
Copy link
Contributor

@stevvooe stevvooe commented Jun 1, 2016

Add a definition for content descriptors and digests, which are used as
content identifiers in the OCI image specification.

Signed-off-by: Stephen J Day stephen.day@docker.com

This is a draft of the descriptor specification, which includes the digest calculation.

Before merging:

  • Add json schema definition
  • Add descriptor examples to unit test suite

Closes #92

cc @philips @vbatts

@stevvooe stevvooe added this to the v1.0.0-rc milestone Jun 1, 2016
A _Content Descriptor_ or _Descriptor_, describes the disposition of targeted content.
A _Descriptor_ includes the type of content, an independently-verifiable content identifier, known as a "digest" and the byte-size of the raw content.

Descriptors SHOULD be embedded in other formats to securely reference external content.
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 this should be turned around to get “Other formats SHOULD use descriptors to securely reference external content”.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on turning it around as this format will stand on its own in #94

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, +1 on this change but I see below it says they can be independent

Copy link
Contributor

Choose a reason for hiding this comment

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

On Wed, Jun 01, 2016 at 04:59:36PM -0700, Brandon Philips wrote:

+Descriptors SHOULD be embedded in other formats to securely reference external content.

Well, +1 on this change but I see below it says they can be independent

We got the turned-around “Other formats SHOULD use descriptors to
securely reference external content.” line, but this is still here.
Can we drop this line?

@philips
Copy link
Contributor

philips commented Jun 2, 2016

LGTM after minor nit cleanups.

Approved with PullApprove

Before consuming content targeted by a descriptor, the byte content MUST be verified against the _digest_.
Heavy processing of before calculating a hash SHOULD be avoided.
Implementations MAY employ some canonicalization to ensure stable content identifiers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Include mention of size verification to avoid Length Extension Attack.

@stevvooe stevvooe force-pushed the descriptors-specification branch 2 times, most recently from 47e94ac to 5bb899a Compare June 3, 2016 00:27
@stevvooe
Copy link
Contributor Author

stevvooe commented Jun 3, 2016

Most of the feedback has been serviced. Schema definitions and validation of examples have been added.

Note that the schema layout probably isn't ideal, but we may want to revisit in another PR.

@stevvooe stevvooe changed the title [WIP] spec: describe descriptors and digests spec: describe descriptors and digests Jun 3, 2016
+++
draft = true
+++
<![end-metadata]-->
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this stuff?

Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was in the top of the other spec. Figured we were using it.

Was this copied over from docker? This looks like part of Docker's docs build system.

Copy link
Member

Choose a reason for hiding this comment

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

perhaps. It's seems fine to drop.

@philips
Copy link
Contributor

philips commented Jun 3, 2016

LGTM

nit: There is some metadata at the header of this file. I don't know what it is for but otherwise looks great.

Approved with PullApprove

@philips
Copy link
Contributor

philips commented Jun 3, 2016

@vbatts do you know how we force a rebuild?

Also, @opencontainers/image-spec-maintainers can we get an LGTM? One more LGTM required to merge this.


### Reserved

The following are field keys that MUST NOT be used in descriptors specified in other OCI specifications:
Copy link
Contributor

Choose a reason for hiding this comment

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

You're restricting your reservation to “other OCI specifications”? I expect you can trust OCI maintainers to PR this spec if they want a new descriptor field. The folks you want to warn off are external implementations and specs.

Copy link
Contributor

Choose a reason for hiding this comment

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

And because it doesn't hurt to also warn off other OCI specs, I'd use something generic like “MUST NOT be defined outside this specificiation”.

@philips
Copy link
Contributor

philips commented Jun 7, 2016

OK, looks like that worked.

Need an additional LGTM from an @opencontainers/image-spec-maintainers

@wking
Copy link
Contributor

wking commented Jun 7, 2016

On Tue, Jun 07, 2016 at 04:30:06PM -0700, Brandon Philips wrote:

LGTM

Do I have to do this again?

After each push 1.

@philips
Copy link
Contributor

philips commented Jun 7, 2016

@wking ick. I will file an issue with @caniszczyk to see if we can only have it decrement by 1 or something. This makes reviewing this stuff super annoying.

@wking
Copy link
Contributor

wking commented Jun 7, 2016

On Tue, Jun 07, 2016 at 04:35:42PM -0700, Brandon Philips wrote:

This makes reviewing this stuff super annoying.

Just don't bother stamping approval on things until they settle down
;).

@philips
Copy link
Contributor

philips commented Jun 8, 2016

@wking I just don't like the system implied mistrust. Like I trust Stephen to just fix nits after I give an LGTM and not require a full re-review of the PR. There is some middle ground here that more accurately reflects the trust model of a collaborative project.

@wking
Copy link
Contributor

wking commented Jun 8, 2016

On Tue, Jun 07, 2016 at 08:29:13PM -0700, Brandon Philips wrote:

@wking I just don't like the system implied mistrust.

It's you're project, so you can obviously pick the rules that make
sense to you. But “$OTHER_MAINTAINER is probably still fine with this
PR after the changes” seems like something you'd only have to assume
if $OTHER_MAINTAINER wasn't paying enough attention to explicitly say
“yeah, that still LGTM”.

@philips
Copy link
Contributor

philips commented Jun 8, 2016

I think this is ready to merge. Can other @opencontainers/image-spec-maintainers take a look and given an LGTM?

@stevvooe
Copy link
Contributor Author

stevvooe commented Jun 9, 2016

LGTM

Apparently, we can LGTM ourselves. Feel free to wait for another maintainer to LGTM before merging.

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Jun 9, 2016

LGTM

Approved with PullApprove

@vbatts vbatts merged commit 355b19d into opencontainers:master Jun 9, 2016
@stevvooe stevvooe deleted the descriptors-specification branch June 9, 2016 01:37
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jul 26, 2016
There are other APIs described in this specification (e.g. the state
JSON format, and the in-flight command-line API [1]), but this string
covers the configuration file and referenced objects (e.g. the
filesystem at root.path).  As additional, backwards compatible
features are added to the spec (leading to 1.1, 1.2, etc. releases)
and supported by runtimes, those runtimes will *still* stupport 1.0
configs.  Once a 2.0 spec is cut, runtimes that only support 2.0 (and
nothing in the 1.0 line) will no longer support the 1.0 config.

My preferred approach here would be to use JSON-LD [2,3,4] to
explicitly document the intended semantics for each field, which would
allow us to drop the config-wide version and version each field
independently.  That would mean a breaking change on a particular
field would only break compatibility for folks who were using that
field.  Unfortunately, I haven't had much luck pushing the consensus
in that direction.

This commit does not add wording about how the runtime and other
consumers should handle an incompatible version.  We can address that
once the command-line API lands.

[1]: opencontainers#513
[2]: opencontainers#371 (comment)
[3]: opencontainers/image-spec#111 (comment)
[4]: opencontainers#510 (comment)
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jul 26, 2016
There are other APIs described in this specification (e.g. the state
JSON format, and the in-flight command-line API [1]), but this string
covers the configuration file and referenced objects (e.g. the
filesystem at root.path).  As additional, backwards compatible
features are added to the spec (leading to 1.1, 1.2, etc. releases)
and supported by runtimes, those runtimes will *still* stupport 1.0
configs.  Once a 2.0 spec is cut, runtimes that only support 2.0 (and
nothing in the 1.0 line) will no longer support the 1.0 config.

My preferred approach here would be to use JSON-LD [2,3,4] to
explicitly document the intended semantics for each field, which would
allow us to drop the config-wide version and version each field
independently.  That would mean a breaking change on a particular
field would only break compatibility for folks who were using that
field.  Unfortunately, I haven't had much luck pushing the consensus
in that direction.

This commit does not add wording about how the runtime and other
consumers should handle an incompatible version.  We can address that
once the command-line API lands.

[1]: opencontainers#513
[2]: opencontainers#371 (comment)
[3]: opencontainers/image-spec#111 (comment)
[4]: opencontainers#510 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Sep 8, 2016
There are other APIs described in this specification (e.g. the state
JSON format, and the in-flight command-line API [1]), but this string
covers the configuration file and referenced objects (e.g. the
filesystem at root.path).  As additional, backwards compatible
features are added to the spec (leading to 1.1, 1.2, etc. releases)
and supported by runtimes, those runtimes will *still* stupport 1.0
configs.  Once a 2.0 spec is cut, runtimes that only support 2.0 (and
nothing in the 1.0 line) will no longer support the 1.0 config.

My preferred approach here would be to use JSON-LD [2,3,4] to
explicitly document the intended semantics for each field, which would
allow us to drop the config-wide version and version each field
independently.  That would mean a breaking change on a particular
field would only break compatibility for folks who were using that
field.  Unfortunately, I haven't had much luck pushing the consensus
in that direction.

This commit does not add wording about how the runtime and other
consumers should handle an incompatible version.  We can address that
once the command-line API lands.

[1]: opencontainers#513
[2]: opencontainers#371 (comment)
[3]: opencontainers/image-spec#111 (comment)
[4]: opencontainers#510 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Sep 14, 2016
There are other APIs described in this specification (e.g. the state
JSON format, and the in-flight command-line API [1]), but this string
covers the configuration file and referenced objects (e.g. the
filesystem at root.path).  As additional, backwards compatible
features are added to the spec (leading to 1.1, 1.2, etc. releases)
and supported by runtimes, those runtimes will *still* stupport 1.0
configs.  Once a 2.0 spec is cut, runtimes that only support 2.0 (and
nothing in the 1.0 line) will no longer support the 1.0 config.

My preferred approach here would be to use JSON-LD [2,3,4] to
explicitly document the intended semantics for each field, which would
allow us to drop the config-wide version and version each field
independently.  That would mean a breaking change on a particular
field would only break compatibility for folks who were using that
field.  Unfortunately, I haven't had much luck pushing the consensus
in that direction.

This commit does not add wording about how the runtime and other
consumers should handle an incompatible version.  We can address that
once the command-line API lands.

[1]: opencontainers#513
[2]: opencontainers#371 (comment)
[3]: opencontainers/image-spec#111 (comment)
[4]: opencontainers#510 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/image-spec that referenced this pull request Sep 18, 2016
The Docker link landed in 7b39189 (media-types: show relations
between media types, 2016-05-02, opencontainers#55), but we've had a descriptor
definition locally since d49d055 (spec: describe descriptors and
digests, 2016-06-01, opencontainers#111).

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/image-spec that referenced this pull request Sep 29, 2016
We're not forbidding other OCI specs from using these fields, we're
forbidding all specs (except for future versions of image-spec) from
using these fields [1,2].  The wording I'm bringing in here matches
what 873b9b6 (Add some text about extensions, 2016-06-26, opencontainers#164)
landed for the org.opencontainers.* annotation namespace.

My personal preference would be to use JSON-LD to assign explicit
semantics to every field (after which you don't have to worry about
namespacing the fields themselves) [2], but that would be a larger
change ;).

[1]: opencontainers#111 (comment)
[2]: opencontainers#111 (comment)
[3]: opencontainers#111 (comment)
     And later comments in that sub-thread.

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/image-spec that referenced this pull request Sep 29, 2016
We're not forbidding other OCI specs from using these fields, we're
forbidding all specs (except for future versions of image-spec) from
using these fields [1,2].  The wording I'm bringing in here matches
what 873b9b6 (Add some text about extensions, 2016-06-26, opencontainers#164)
landed for the org.opencontainers.* annotation namespace.

My personal preference would be to use JSON-LD to assign explicit
semantics to every field (after which you don't have to worry about
namespacing the fields themselves) [3], but that would be a larger
change ;).

[1]: opencontainers#111 (comment)
[2]: opencontainers#111 (comment)
[3]: opencontainers#111 (comment)
     And later comments in that sub-thread.

Signed-off-by: W. Trevor King <wking@tremily.us>
dattgoswami9lk5g added a commit to dattgoswami9lk5g/bremlinr that referenced this pull request Jun 6, 2022
We're not forbidding other OCI specs from using these fields, we're
forbidding all specs (except for future versions of image-spec) from
using these fields [1,2].  The wording I'm bringing in here matches
what 873b9b64 (Add some text about extensions, 2016-06-26, #164)
landed for the org.opencontainers.* annotation namespace.

My personal preference would be to use JSON-LD to assign explicit
semantics to every field (after which you don't have to worry about
namespacing the fields themselves) [3], but that would be a larger
change ;).

[1]: opencontainers/image-spec#111 (comment)
[2]: opencontainers/image-spec#111 (comment)
[3]: opencontainers/image-spec#111 (comment)
     And later comments in that sub-thread.

Signed-off-by: W. Trevor King <wking@tremily.us>
7c00d pushed a commit to 7c00d/J1nHyeockKim that referenced this pull request Jun 6, 2022
We're not forbidding other OCI specs from using these fields, we're
forbidding all specs (except for future versions of image-spec) from
using these fields [1,2].  The wording I'm bringing in here matches
what 873b9b64 (Add some text about extensions, 2016-06-26, #164)
landed for the org.opencontainers.* annotation namespace.

My personal preference would be to use JSON-LD to assign explicit
semantics to every field (after which you don't have to worry about
namespacing the fields themselves) [3], but that would be a larger
change ;).

[1]: opencontainers/image-spec#111 (comment)
[2]: opencontainers/image-spec#111 (comment)
[3]: opencontainers/image-spec#111 (comment)
     And later comments in that sub-thread.

Signed-off-by: W. Trevor King <wking@tremily.us>
7c00d added a commit to 7c00d/J1nHyeockKim that referenced this pull request Jun 6, 2022
We're not forbidding other OCI specs from using these fields, we're
forbidding all specs (except for future versions of image-spec) from
using these fields [1,2].  The wording I'm bringing in here matches
what 873b9b64 (Add some text about extensions, 2016-06-26, #164)
landed for the org.opencontainers.* annotation namespace.

My personal preference would be to use JSON-LD to assign explicit
semantics to every field (after which you don't have to worry about
namespacing the fields themselves) [3], but that would be a larger
change ;).

[1]: opencontainers/image-spec#111 (comment)
[2]: opencontainers/image-spec#111 (comment)
[3]: opencontainers/image-spec#111 (comment)
     And later comments in that sub-thread.

Signed-off-by: W. Trevor King <wking@tremily.us>
laventuraw added a commit to laventuraw/Kihara-tony0 that referenced this pull request Jun 6, 2022
We're not forbidding other OCI specs from using these fields, we're
forbidding all specs (except for future versions of image-spec) from
using these fields [1,2].  The wording I'm bringing in here matches
what 873b9b64 (Add some text about extensions, 2016-06-26, #164)
landed for the org.opencontainers.* annotation namespace.

My personal preference would be to use JSON-LD to assign explicit
semantics to every field (after which you don't have to worry about
namespacing the fields themselves) [3], but that would be a larger
change ;).

[1]: opencontainers/image-spec#111 (comment)
[2]: opencontainers/image-spec#111 (comment)
[3]: opencontainers/image-spec#111 (comment)
     And later comments in that sub-thread.

Signed-off-by: W. Trevor King <wking@tremily.us>
tomalopbsr0tt added a commit to tomalopbsr0tt/fabiojosej that referenced this pull request Oct 6, 2022
We're not forbidding other OCI specs from using these fields, we're
forbidding all specs (except for future versions of image-spec) from
using these fields [1,2].  The wording I'm bringing in here matches
what 873b9b64 (Add some text about extensions, 2016-06-26, #164)
landed for the org.opencontainers.* annotation namespace.

My personal preference would be to use JSON-LD to assign explicit
semantics to every field (after which you don't have to worry about
namespacing the fields themselves) [3], but that would be a larger
change ;).

[1]: opencontainers/image-spec#111 (comment)
[2]: opencontainers/image-spec#111 (comment)
[3]: opencontainers/image-spec#111 (comment)
     And later comments in that sub-thread.

Signed-off-by: W. Trevor King <wking@tremily.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants