cmd/oci-image-tool: validate all refs by default #279

runcom · 2016-09-08T13:52:44Z

This is taking care of #271 (comment) (left a TODO/FIXME for later in case).

I'll adapt everything to be same as per #266 later

$ oci-image-tool validate /home/amurdaca/src/github.com/projectatomic/skopeo/busybox-oci2
reference "12.04": OK
reference "3.1": OK
reference "a59906e33509d14c036c8678d687bd4eec81ed7c4b8ce907b888c607f6a1e0e6": OK
reference "latest": OK
reference "notlatest": OK
reference "tag": OK
/home/amurdaca/src/github.com/projectatomic/skopeo/busybox-oci2: OK

/cc @wking @vbatts @philips @stevvooe

Signed-off-by: Antonio Murdaca [email protected]

wking · 2016-09-08T15:27:10Z

image/descriptor.go

@@ -49,6 +49,30 @@ func (d *descriptor) hash() string {
 	return pts[1]
 }

+func findDescriptors(w walker) (map[string]*descriptor, error) {


Maybe listReferences? In #159, the analogous function is refs.Engine.List().

wking · 2016-09-08T15:39:51Z

On Thu, Sep 08, 2016 at 06:52:45AM -0700, Antonio Murdaca wrote:

-- File Changes --

M cmd/oci-image-tool/validate.go (2)
M image/descriptor.go (24)
M image/image.go (30)

Update the man page too.

stevvooe · 2016-09-08T19:47:35Z

@runcom If unknown types are encountered, what is the intended behavior?

I ask because we do need to be able to have references to unknown things but still pass validation. This will make it easier to extend in the future.

wking · 2016-09-08T19:58:34Z

On Thu, Sep 08, 2016 at 12:47:36PM -0700, Stephen Day wrote:

@runcom If unknown types are encountered, what is the intended behavior?

I think we need to error with “not implemented”-level errors.

I ask because we do need to be able to have references to unknown
things but still pass validation. This will make it easier to extend
in the future.

That makes sense. We might want an --ignore-unknown-media-types flag
to avoid erroring out if we hit an unrecognized type. There's some
more discussion of unrecognized types in 1.

What happens if you recognize the ref type
(e.g. application/vnd.oci.image.manifest.list.v1+json) but not an
-ancestor- [edit: descendant] type (e.g. application/vnd.oci.image.layer.zip) and
--ignore-unknown-media-types is set? Do you print your warning and
return zero? Or is the error→warning softening only for the root
refs? We can have a global error→warning softening and still error
for the application/vnd.oci.image.layer.zip case if the specs for our
objects explicitly list allowed descriptor types [2,3].

stevvooe · 2016-09-08T20:00:21Z

@wking I was asking @runcom. It's okay to let a conversation develop before commenting.

wking · 2016-09-08T20:03:32Z

On Thu, Sep 08, 2016 at 01:00:21PM -0700, Stephen Day wrote:

It's okay to let a conversation develop before commenting.

Sure. I was just linking in previous unrecognized-type discussion to
try and avoid a fragmented discussion of that issue.

runcom · 2016-09-08T20:58:18Z

@stevvooe right now, if the media type is unknown the tool errors out. We should at least soften that to just print a warning. This behavior is in the validate function though, I can address this in a follow up and discuss more about errors/warnings if it's ok for you, this PR just makes sure we "find" every reference under refs/ to shorten the command line flags.

wking · 2016-09-08T21:33:14Z

On Thu, Sep 08, 2016 at 01:58:18PM -0700, Antonio Murdaca wrote:

This behavior is in the validate function though, I can address
this in a follow up and discuss more about errors/warnings if it's
ok for you, this PR just makes sure we "find" every reference under
refs/ to shorten the command line flags.

I'm fine punting unrecognized media type issues to follow up work,
since that is an orthogonal problem (this PR just increases the
likelyhood that we find unrecognized types).

stevvooe · 2016-09-08T22:46:10Z

@runcom SGTM. Please open an issue.

runcom · 2016-09-09T09:10:05Z

@stevvooe @vbatts @philips PTAL

Signed-off-by: Antonio Murdaca <[email protected]>

philips · 2016-09-12T04:00:54Z

LGTM.

@s-urbaniak ?

stevvooe · 2016-09-12T22:47:30Z

LGTM

s-urbaniak · 2016-09-13T04:36:10Z

image/image.go

@@ -33,7 +34,7 @@ func ValidateLayout(src string, refs []string, out *log.Logger) error {

 // Validate walks through the given .tar file and
 // validates the manifest pointed to by the given refs
-// or returns an error if the validation failed.
+//iiii or returns an error if the validation failed.


s-urbaniak · 2016-09-13T04:39:59Z

LGTM, with one small nit

philips · 2016-09-13T05:23:48Z

LGTM

wking reviewed Sep 8, 2016
View reviewed changes

wking mentioned this pull request Sep 8, 2016

Add yaml config file to image tool command usage #271

Closed

runcom mentioned this pull request Sep 9, 2016

cmd/oci-image-tool: validation errors and warnings #286

Closed

runcom force-pushed the validate-all-refs branch from 5c9400d to fde1e4d Compare September 9, 2016 09:09

cmd/oci-image-tool: validate all refs by default

a480306

Signed-off-by: Antonio Murdaca <[email protected]>

runcom force-pushed the validate-all-refs branch from fde1e4d to a480306 Compare September 9, 2016 10:05

s-urbaniak reviewed Sep 13, 2016
View reviewed changes

philips merged commit c2ed5aa into opencontainers:master Sep 13, 2016

runcom deleted the validate-all-refs branch September 13, 2016 06:04

philips mentioned this pull request Sep 21, 2016

cmd/oci-image-tool: validation errors and warnings opencontainers/image-tools#16

Open

wking mentioned this pull request Nov 14, 2016

image.go:change out.Print to fmt.Errorf opencontainers/image-tools#83

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/oci-image-tool: validate all refs by default #279

cmd/oci-image-tool: validate all refs by default #279

runcom commented Sep 8, 2016 •

edited

Loading

wking Sep 8, 2016

wking commented Sep 8, 2016

stevvooe commented Sep 8, 2016

wking commented Sep 8, 2016 •

edited

Loading

stevvooe commented Sep 8, 2016

wking commented Sep 8, 2016

runcom commented Sep 8, 2016

wking commented Sep 8, 2016

stevvooe commented Sep 8, 2016

runcom commented Sep 9, 2016

philips commented Sep 12, 2016 •

edited by caniszczyk

Loading

stevvooe commented Sep 12, 2016 •

edited by caniszczyk

Loading

s-urbaniak Sep 13, 2016

runcom Sep 13, 2016

s-urbaniak commented Sep 13, 2016

philips commented Sep 13, 2016

cmd/oci-image-tool: validate all refs by default #279

cmd/oci-image-tool: validate all refs by default #279

Conversation

runcom commented Sep 8, 2016 • edited Loading

wking Sep 8, 2016

Choose a reason for hiding this comment

wking commented Sep 8, 2016

stevvooe commented Sep 8, 2016

wking commented Sep 8, 2016 • edited Loading

stevvooe commented Sep 8, 2016

wking commented Sep 8, 2016

runcom commented Sep 8, 2016

wking commented Sep 8, 2016

stevvooe commented Sep 8, 2016

runcom commented Sep 9, 2016

philips commented Sep 12, 2016 • edited by caniszczyk Loading

stevvooe commented Sep 12, 2016 • edited by caniszczyk Loading

s-urbaniak Sep 13, 2016

Choose a reason for hiding this comment

runcom Sep 13, 2016

Choose a reason for hiding this comment

s-urbaniak commented Sep 13, 2016

philips commented Sep 13, 2016

runcom commented Sep 8, 2016 •

edited

Loading

wking commented Sep 8, 2016 •

edited

Loading

philips commented Sep 12, 2016 •

edited by caniszczyk

Loading

stevvooe commented Sep 12, 2016 •

edited by caniszczyk

Loading