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

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

Open
philips opened this issue Sep 21, 2016 · 7 comments
Open

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

philips opened this issue Sep 21, 2016 · 7 comments

Comments

@philips
Copy link
Contributor

philips commented Sep 21, 2016

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
Copy link
Contributor Author

philips commented Sep 21, 2016

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

@philips
Copy link
Contributor Author

philips commented Sep 21, 2016

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
Copy link
Contributor Author

philips commented Sep 21, 2016

@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
Copy link
Contributor Author

philips commented Sep 21, 2016

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
Copy link
Contributor Author

philips commented Sep 21, 2016

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
Copy link
Contributor Author

philips commented Sep 21, 2016

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
Copy link
Contributor

wking commented Nov 1, 2016

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants