layout: implement unpacking and validation by s-urbaniak · Pull Request #82 · opencontainers/image-spec

s-urbaniak · 2016-05-22T10:36:27Z

Fixes #75

Signed-off-by: Sergiusz Urbaniak sur@coreos.com

s-urbaniak · 2016-05-22T23:15:13Z

Blocked by #81: I need the config validator merged to thoroughly validate the image/image layout.

philips · 2016-05-24T19:03:56Z

@s-urbaniak #81 is merged, what is the next step?

s-urbaniak · 2016-05-24T20:09:37Z

@philips implementing image layout validation now, should be ready by EOD

s-urbaniak · 2016-05-25T00:24:06Z

@philips @vbatts @stevvooe PTAL; I implemented this using self-made image available here https://github.com/s-urbaniak/oci-image-test.

philips · 2016-05-25T01:34:15Z

@s-urbaniak Are you generating the OCI Runtime config.json? https://github.com/opencontainers/runtime-spec/blob/master/config.md We need to generate that from the serialization.config thingie.

s-urbaniak · 2016-05-25T01:35:15Z

@philips nope, as discussed OOB, this is just "pure" unpacking. The runtime config.json would have to be solved in a separate PR.

stevvooe · 2016-05-25T21:55:23Z

@s-urbaniak I'm looking at this today. Sorry for the delay.

philips · 2016-05-25T22:48:41Z

+		info := hdr.FileInfo()
+
+		if strings.HasPrefix(info.Name(), `.wh.`) {
+			path = strings.Replace(path, `.wh.`, "", 1)


any reason to use backtick instead of regular quotes?

no reason, just too fast typing ;-) fixed

stevvooe · 2016-05-26T01:55:58Z

@s-urbaniak This is looking very solid. Is there a corresponding specification for this format?

The only large change I'd suggest is that rather than manifests, this should be refs. These files should simply contain a descriptor pointing to the manifest blob. This will allow us to avoid the half-deserialized manifests.

stevvooe · 2016-05-26T01:58:16Z

+				return nil // ignore
+			}
+
+			if err := extractLayer(dest, r); err != nil {


If we unpack directly to disk, these layers really need to be verified against the manifest's digest.

+1 on that.

philips · 2016-05-26T02:12:15Z

@stevvooe what do you mean by "avoid the half-deserialized manifests"?

philips · 2016-05-26T02:15:27Z

@stevvooe are you thinking something like this for the refs schema:

$ cat refs/v1.0
{
  "media-types": {
    "application/vnd.oci.image.manifest.v1+json": {"size": "4096", "digest": "sha256:afff3928"
  }
}

s-urbaniak · 2016-05-26T10:12:04Z

@stevvooe @philips thanks a lot for the review! just fyi: I'm sitting between flights though so won't make to implement changes before tomorrow.

stevvooe · 2016-05-26T20:04:45Z

+
+// supported autodetection types
+const (
+	typeImageLayout  = "imageLayout"


Why not use the mediatypes directly?

this gives me finer grained control, i.e. typeImageLayout, and typeImage don't have media types at all and must be treated differently.

stevvooe · 2016-05-26T20:09:57Z

@philips I really referring to https://github.com/opencontainers/image-spec/pull/82/files#diff-1d4f0c730d731e154e77caf8c8ce36a4R80.

But, I was thinking about something like this:

$ cat refs/v1.0
{"size": 4096, "digest": "sha256:afff3928", "mediatype": "application/vnd.oci.image.manifest.v1+json"}

philips · 2016-05-26T21:14:35Z

@stevvooe I think we need an array of media types though as we need a way of emulating Accept with a filesystem. What if my ref is serving a v1 and v1.1 manifest? Let me file a new issue on this.

wking · 2016-05-26T21:43:17Z

On Thu, May 26, 2016 at 02:14:36PM -0700, Brandon Philips wrote:

What if my ref is serving a v1 and v1.1 manifest?

Make the root reference point to a manifest.list 1?

philips · 2016-05-26T23:16:10Z

@wking I think that is the approach we are going to take. Let me work up a proposal.

philips · 2016-05-27T02:38:14Z

@stevvooe here is a concrete format specification: #94

philips · 2016-05-27T02:38:40Z

+}
+
+func diffIDs(w walker, manifest string) ([]string, error) {
+	manifest = filepath.Join("manifests", manifest)


can you change this code to follow the spec here: #94

s-urbaniak · 2016-05-27T12:26:52Z

@stevvooe @philips refactored this towards the proposed layout spec in #94.

Fixes opencontainers#75 Signed-off-by: Sergiusz Urbaniak <sur@coreos.com>

philips · 2016-05-27T16:45:06Z

Overall LGTM for a starting point.

Anyone else give this an LGTM @opencontainers/image-spec-maintainers

jonboulle · 2016-05-31T09:20:27Z

LGTM

s-urbaniak force-pushed the layout branch 7 times, most recently from bd4afcd to 4aa9e1f Compare May 22, 2016 17:10

s-urbaniak force-pushed the layout branch from 4aa9e1f to 3dd62c3 Compare May 22, 2016 23:28

philips added this to the v0.2.0 milestone May 24, 2016

philips added priority/P0 component/oci-image-tool labels May 24, 2016

s-urbaniak force-pushed the layout branch from 3dd62c3 to 1090845 Compare May 25, 2016 00:16

s-urbaniak changed the title ~~[WIP] layout: unpacking initial commit~~ layout: implement unpacking and validation May 25, 2016

s-urbaniak mentioned this pull request May 25, 2016

Convert a serialization config JSON to a OCI runtime configuration. #87

Closed

philips reviewed May 25, 2016
View reviewed changes

stevvooe reviewed May 26, 2016
View reviewed changes

philips reviewed May 27, 2016
View reviewed changes

s-urbaniak force-pushed the layout branch from 1090845 to fc3c8ed Compare May 27, 2016 12:22

layout: implement unpacking and validation

d1874b6

Fixes opencontainers#75 Signed-off-by: Sergiusz Urbaniak <sur@coreos.com>

s-urbaniak force-pushed the layout branch from fc3c8ed to d1874b6 Compare May 27, 2016 12:30

jonboulle merged commit 3468157 into opencontainers:master May 31, 2016

cyphar mentioned this pull request Jun 8, 2016

[1.0.0] Need actual independent implementations #126

Closed

wking mentioned this pull request Jun 14, 2016

image-layout: clarification #136

Merged

wking mentioned this pull request Jul 22, 2016

Fix broken oci-image-tool #177

Merged

philips mentioned this pull request Sep 21, 2016

Convert a serialization config JSON to a OCI runtime configuration. opencontainers/image-tools#25

Open

Conversation

s-urbaniak commented May 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s-urbaniak commented May 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philips commented May 24, 2016

Uh oh!

s-urbaniak commented May 24, 2016

Uh oh!

s-urbaniak commented May 25, 2016

Uh oh!

philips commented May 25, 2016

Uh oh!

s-urbaniak commented May 25, 2016

Uh oh!

stevvooe commented May 25, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevvooe commented May 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philips commented May 26, 2016

Uh oh!

philips commented May 26, 2016

Uh oh!

s-urbaniak commented May 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevvooe commented May 26, 2016

Uh oh!

philips commented May 26, 2016

Uh oh!

wking commented May 26, 2016

Uh oh!

philips commented May 26, 2016

Uh oh!

philips commented May 27, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

s-urbaniak commented May 27, 2016

Uh oh!

philips commented May 27, 2016

Uh oh!

jonboulle commented May 31, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

s-urbaniak commented May 22, 2016 •

edited

Loading

s-urbaniak commented May 22, 2016 •

edited

Loading