layers: describe policy enforcement for foreign layer media type

[stevvooe]

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

Supersedes #216, Requires #169.

[philips]

@stephenrwalli Can you take a look at this language? I am still unclear if "MUST NOT distribute" is something we can define usefully in a spec.

I totally get it as a human being with context on copyrights for proprietary software about the usefullness. I just don't know how to codify this. It is like the evil bit RFC: https://www.ietf.org/rfc/rfc3514.txt

[wking]

[Descriptors][descriptor.md] → [Descriptors](descriptor.md).

[wking]

Pre-#212 media type pattern?

[stevvooe]

Yes, good catch!

[jlbutler]

Since a foreign layer must not be pushed per the layer spec, might this media type be better defined as "must not be pushed" rather than "should never be pushed"?

Possibly better to redefine "as a non-redistributable gzipped tar archive", or similar.

[jonboulle]

-1 on the underspecified use of the "push" verb, as discussed in #216 (comment)

[stevvooe]

@jonboulle It is sufficient to comment inline on the specific language that needs correcting rather than saying "-1". It is much more constructive, unless you intend to block the PR.

I've replaced the term "push" with the more generic "distribute".

[stephenrwalli]

@philips I understand your example. It's not necessarily how you would codify it, so much as it gives the certification process the language to revoke a certification based on non-compliance. While it might be difficult to write a test to prove the MUST NOT, if an implementation was demonstrated to distribute such a layer, it clearly doesn't do what the specification says, and would not be able to claim conformance/compliance. Likewise, for a certification process, one can design the process to manual test such a thing or to use the condition to revoke a certification until the implementation complies. Make sense? (Sorry for the tardy reply.)

[wking]

On Tue, Sep 06, 2016 at 01:58:50PM -0700, Stephen Walli wrote:

While it might be difficult to write a test to prove the MUST NOT,
if an implementation was demonstrated to distribute such a layer, it
clearly doesn't do what the specification says, and would not be
able to claim conformance/compliance. Likewise, for a certification
process, one can design the process to manual test such a thing or
to use the condition to revoke a certification until the
implementation complies.

“Yeah, automatically testing that would be hard, but we can manually
test it” is a marker for a wishy definition 1. I think the problem
here is that “distributing” is not something that one instance can do
by itself. If it's pushing a blob, someone else must be receiving it.
In some cases (e.g. pushing to a local disk on a single-user system),
that probably doesn't count as “distributing” in a legal sense. In
others (e.g. pushing to a public repo on hub.docker.com) it does.
Where does caching to disk on a multi-user system fall in that
continuum? Are you distributing it by saving to a system-readabale
cache? Or is it only distributing if another user on the system reads
it out of that cache? If that system-readable cache happens to live
under your home directory, are you being robbed (and not distributing)
when the other user reads the file?

I imagine there are consensus opinions on at least some of that
(although I'm not aware of them). I don't think we want to make it
the business of spec implementers and validators to independently
figure out what those opinions are. If there are conditions under
which you would certify (or not) an implementation, I'd like to see
those conditions spelled out in the spec in enough detail for them to
be objectively tested without appealing to “The average person,
applying local community standards, looking at the work in its
entirety” and such. In some cases, that wishyness is unavoidable.
But we need something more solid if we're expecting implementation
software to act consistently based on a ‘foreign’ boolean.

One option would be to have a CAS engine API (e.g. #159) where the
public/private-ness of the engine was part of its initialization.
E.g.:

casEngine, err := NewEngine(ctx, path, public)

Then the engine is non-compliant if you can successfully:

casEngine.Put(ctx, reader, mediaType)

with a foreign mediaType (this is testable by the OCI). And the
engine-initializer is breaking the law if they create a CAS engine
pointing at a public hub.docker.com repo and claiming it is not public
(this is between the CAS engine configurer and the copyright holders
of the pushed content, and not something the OCI has to certify).

[philips]

cc @runcom @vbatts ?

[runcom]

I'm not sure either on this one - is it an hard requirement for this spec to outline the legal matters of foreign layers? I (sorry I may be wrong) don't think so. I think as foreign layers just layers that are downloaded through a list of urls in a descriptor. Do we really need to clarify that those layers can't be distributed? I understand this was born in Docker for this exact legal reason but I don't think the spec should enforce this restriction (not to push) - if people end up pushing the layer it's them breaking any legal terms, I'm not sure what the spec has to do with this. (I believe it's perfectly possible to upload the Windows base layer by taking it and uploading it, implementers (Docker) enforce this, can we just add an implementor note? or add a SHOULD?)

(sorry if any of the above makes no sense...)

[stephenrwalli]

You make good sense, @runcom. So a way to call attention to such things, that I've seen in the past, is to explicitly call something out as implementation defined. This is basically a warning to spec implementers that are not part of the immediate spec community to pay attention in the space before naively doing something in their implementation. It also allows for the behaviors that already exist in implementations. So in this case, wording becomes: "It is implementation defined whether or not implementations distribute layers tagged with this media type."

[jonboulle]

Such a phrasing would definitely assuage my concerns about the vagueness of "pushing", "distributing", "distributable", etc. (My wariness is largely around combining such terms with the keyword MUST without being very explicit about what they mean.)

[runcom]

@stephenrwalli that sounds perfect to me if people agree

[wking]

@RobDolinMS is going to quibble with this non-SHOULD should (e.g. opencontainers/runtime-spec#536 ;). How about dropping this line, since the idea is covered in the rest of this section?

[stevvooe]

This is an important clarification and frames the stronger language below. I can capitalize the SHOULD or re-phrase it.

[wking]

On Wed, Sep 07, 2016 at 11:43:09AM -0700, Stephen Day wrote:

@@ -156,3 +156,13 @@ Note that this opaque file will apply to all children, including sub-directori
Implementations SHOULD generate layers using explicit whiteout files, but MUST accept both.

Any given image is likely to be composed of several of these Image Filesystem Changeset tar archives.
+
+# Foreign Layers
+
+Certain layers, due to legal requirements, may not be regularly distributable.
+Typically, one can download such layers but they should never be uploaded.

This is an important clarification and frames the stronger language
below.

What is this line adding beyond the earlier:

Certain layers, due to legal requirements, may not be regularly distributable.

and the planned 1:

It is implementation defined whether or not implementations
distribute layers tagged with this media type.

Can you pin down the information the other lines are missing that
you're trying to convey here?

[runcom]

I agree with @wking to drop this line and favor the new one proposed

[wking]

On Wed, Sep 07, 2016 at 10:23:08AM -0700, Stephen Walli wrote:

So in this case, wording becomes: "It is implementation defined
whether or not implementations distribute layers tagged with this
media type."

This works for me too.

[wking]

On Wed, Sep 07, 2016 at 10:23:08AM -0700, Stephen Walli wrote:

So in this case, wording becomes: "It is implementation defined
whether or not implementations distribute layers tagged with this
media type."

It feels awkard to repeat “implementation”. How about:

It is unspecified whether or not implementations distribute layers
tagged with this media type.

?

[stephenrwalli]

I believe the suggested text becomes:

Foreign Layers
Layers that have these restrictions SHOULD be tagged with an alternative mediatype of `application/vnd.oci.image.layer.foreign.tar+gzip`.
 [Descriptors](descriptor.md) referencing these layers MAY include `urls` for downloading these layers.
It is implementation defined whether or not implementations upload layers tagged with this media type.

[stephenrwalli]

@wking I appreciate the awkwardness of language, but I'm using very specific phrasing. "Unspecified" says you can do anything. "Implementation defined" is that bit stronger saying implementations may already do things here and you should pay attention to your choices.

[philips]

LGTM

[wking]

On Wed, Sep 07, 2016 at 01:56:09PM -0700, Stephen Walli wrote:

@wking I appreciate the awkwardness of language, but I'm using very
specific phrasing. "Unspecified" says you can do
anything. "Implementation defined" is that bit stronger saying
implementations may already do things here and you should pay
attention to your choices.

Interesting. It looks like C defines those terms [1,2]. If we are
going to split those hairs (and it sounds like a good idea to me), I'd
recommend we put something like that in our spec or reference another
spec to define them for us. Ideally the latter. Is there an
equivalent to RFC 2119 for these terms?

 Page 18 of the PDF, page 11 of the document.

[runcom]

Are we sticking with the current language?

[stevvooe]

Updated with the new language!

[runcom]

s/upload/distribute?

[stevvooe]

@runcom I think there were concerns about the meaning of distribute. I'll change it to that if we can re-obtain consensus.

@stephenrwalli CC

[runcom]

@stevvooe sorry for the noise, I re-read and you're right..

[wking]

On Wed, Sep 07, 2016 at 02:40:52PM -0700, Stephen Day wrote:

Updated with the new language!

I still think we want to drop the “should never be uploaded” line 1,
and would like to define “implementation defined” 2. But those can
be addressed in follow-up work if they're too contentious to handle
now, and the rest of fbc6ce4 looks good to me.

[stevvooe]

@jonboulle I fully agree with the goal of reducing cognitive burden but consistency is also a valid goal. Docker calls this application/vnd.docker.image.rootfs.foreign.diff.tar.gzip and the concepts are isomorphic. Do you think these having different names helps to reduce cognitive burden?

How about we get this merged based on content and concept do another PR where we discuss the name of the media type? Would that suffice?

[wking]

On Fri, Sep 09, 2016 at 02:07:49PM -0700, Stephen Day wrote:

Do you think these having different names helps to reduce cognitive
burden?

When the new OCI name is more clear? Yes. If we didn't think so, we
wouldn't have landed #212.

How about we get this merged based on content and concept do another
PR where we discuss the name of the media type? Would that suffice?

I don't see how splitting this discussion out into a new PR will help
resolve it (there's not a lot of other distracting stuff happing in
this one), but if folks feel like that's the case, I'm happy to
discuss foreign → nondistributable in a follow-up PR.

[stevvooe]

When the new OCI name is more clear? Yes. If we didn't think so, we
wouldn't have landed #212.

#212 actually brings it closer to https://github.com/docker/distribution/blob/master/docs/spec/manifest-v2-2.md#media-types. Before, we had an extra level of indirection in OCI.

I'll say that I am as much of a stickler for naming as anyone, but I'm really just trying to move things forward. I think we have agreement on the concept, but I don't want to get it held up on naming. Let's get the concept in. If we agree on a name change, then we can merge that, as well.

[wking]

On Fri, Sep 09, 2016 at 02:39:34PM -0700, Stephen Day wrote:

Let's get the concept in. If we agree on a name change, then we can
merge that, as well.

Fair. I'm just not sure why we don't already have an agreement on
changing the name ;).

[wking]

I still have the concerns from 1, which I'm ok ignoring or punting
on, but otherwise e48e595 looks good to me.

[jonboulle]

LGTM now, thanks @stevvooe

I do agree with wking's comments in #233 (comment) but we can follow up for that.

[philips]

LGTM

[stevvooe]

🎊

[runcom]

🎉