opencontainers / image-tools

OCI Image Tooling
https://opencontainers.org
Apache License 2.0
266 stars 83 forks source link

oci-image-tool: Drop http.FileSystem? #24

Open philips opened 7 years ago

philips commented 7 years ago

From @wking on June 16, 2016 14:36

While reading #148, the http.FileSystem section struck me as the wrong approach. “I'd like to validate this content against this JSON Schema” is easy to support generically, and keeping the schemas themselves outside of that tool allows you to use an existing version of the tool to validate new content against new schemas. If you compile the schemas in, you have to rebuild the tool to get access to the new schemas.

For an alternative approach, see the in-flight opencontainers/runtime-spec#490. This approach allows a generic JSON-Schema validator to use whichever schema you point it at. It would be nice to put the schemas up on www.opencontainers.org/schema/…, but even without that you can use the raw GitHub URLs. For oci-image-tool, you'd need a method to convert the detected/configured media type / version to a schema URL, but you already do that for http.FileSystem. The URLs would just get a bit longer, and if we start removing schemas you'd have to add logic to pin to the last image-spec version which carried a schema for your media type.

If this sounds like a reasonable approach, I can work up a PR.

Copied from original issue: opencontainers/image-spec#150

philips commented 7 years ago

I would be fine with making this an optional feature. Requiring network traffic by default doesn't seem like the best default however.

philips commented 7 years ago

From @wking on June 16, 2016 22:10

On Thu, Jun 16, 2016 at 03:01:46PM -0700, Brandon Philips wrote:

I would be fine with making this an optional feature. Requiring network traffic by default doesn't seem like the best default however.

Does Go have defaults for storing associated data (like Python's 1)? Baking it into the binary seems like “here's my spell checker with a compiled in snapshot of today's dictionary” (and I hope nobody does that ;).

philips commented 7 years ago

@wking no, this is the "right" way os doing this for Go. Compiling in the data in a section via a const is the common practice. Not great but that is how it is.

We have a bunch of tools at CoreOS that use this fake filesystem thingie as default and then let people override in "dev" mode.

philips commented 7 years ago

From @s-urbaniak on June 17, 2016 13:48

Agreeing to all @philips said. I can work on a possibility to look up schema via http/s, or if you @wking would like to do a PR, that'd be fine too.

philips commented 7 years ago

From @wking on June 17, 2016 16:12

On Fri, Jun 17, 2016 at 06:48:30AM -0700, Sergiusz Urbaniak wrote:

I can work on a possibility to look up schema via http/s…

I'm happy to let you do the work ;).

philips commented 7 years ago

I am going to make this post-v1.0.0 as it feels like a nice to have. Also, labeling as help wanted as it is a fun Go project for someone :)

wking commented 7 years ago

On Tue, Sep 20, 2016 at 07:48:54PM -0700, Brandon Philips wrote:

Copied from original issue: opencontainers/image-spec#150

With opencontainers/image-spec#337 having landed, I think this is clearly not an image-tools issue. Can we close this one?