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

layer: make clear that diffs should not cross mountpoints #496

Closed
cyphar opened this issue Dec 14, 2016 · 9 comments
Closed

layer: make clear that diffs should not cross mountpoints #496

cyphar opened this issue Dec 14, 2016 · 9 comments

Comments

@cyphar
Copy link
Member

cyphar commented Dec 14, 2016

From #492, I noticed that there was a concern about whether Config.Volumes should be included in layer changesets. IMO the answer is clearly no, but we have to make a bunch of other decisions:

  1. Should layer changesets ever cross a mountpoint? My gut says no, but we need to make this explicit (and what mountpoint we should start at). There's also a platform-dependence issue.

  2. While conversion: add document about image -> runtime configuration #492 is defining the conversion of Volume -> mounts it's not a hard requirement, which means that either we have to i) talk about Volumes in layer.md b) make the requirement hard.

WDYT? Especially @vbatts who has had strong opinions about changesets in the past. 😉

@wking
Copy link
Contributor

wking commented Dec 14, 2016 via email

@stevvooe
Copy link
Contributor

@cyphar From the perspective of the image spec, Volumes is really just a changeset mask. This is an odd way of looking at it but from an image, that is all it is, practice. I am not sure if we need anything other than "Children of the the volume should not be included in the changeset". Requiring some sort of mount/volume duality may be overbearing for the runtime.

As far as crossing mountpoints, I am not sure what the implications are. There are going to issues in overlay, where files in sibling files in different layers may actually be a part of different devices (I think). It certainly makes creating the changesets much simpler, as you don't need to deal with problems that arise in these cases. How is crossing mountpoints represent in a tar file?

@wking
Copy link
Contributor

wking commented Dec 14, 2016 via email

@cyphar
Copy link
Member Author

cyphar commented Dec 15, 2016

From the perspective of the image spec, Volumes is really just a changeset mask. This is an odd way of looking at it but from an image, that is all it is, practice.

I agree with that way of wording it (mountpoints were not a good choice of wording) -- but from what I can tell there's nothing like that in the spec? Should I add it to config.md or layers.md or both?

As far as crossing mountpoints, I am not sure what the implications are.

Neither am I. Let's just drop it and focus on Volumes.

@stevvooe
Copy link
Contributor

@cyphar This probably belongs in config.md. Note that there is some behavior about how the volume content may be "copied up" from the layer. The masking effect is more about the next layer created from a previous image from the volume.

@cyphar
Copy link
Member Author

cyphar commented Dec 18, 2016

@stevvooe I've put forth some sample wording in #504.

@RobDolinMS
Copy link
Collaborator

@cyphar Does PR #504 satisfy this issue?

@cyphar
Copy link
Member Author

cyphar commented Apr 17, 2017

@RobDolinMS Yes, but the PR hasn't been merged yet so we should keep this open for the moment.

cyphar added a commit to opencontainers/umoci that referenced this issue Jun 3, 2017
This is (unfortunately) not mandated in the specification[1,2], but in
order to avoid accidentally spilling private information into published
layers (which is one use of Volumes) we must ignore all layers included
in Config.Volumes.

In future we should also add some flags or alernative ways of masking
paths.

[1]: opencontainers/image-spec#496
[2]: opencontainers/image-spec#504

Signed-off-by: Aleksa Sarai <asarai@suse.de>
cyphar added a commit to opencontainers/umoci that referenced this issue Jun 3, 2017
This is (unfortunately) not mandated in the specification[1,2], but in
order to avoid accidentally spilling private information into published
layers (which is one use of Volumes) we must ignore all layers included
in Config.Volumes.

In future we should also add some flags or alernative ways of masking
paths.

[1]: opencontainers/image-spec#496
[2]: opencontainers/image-spec#504

Signed-off-by: Aleksa Sarai <asarai@suse.de>
@cyphar
Copy link
Member Author

cyphar commented Mar 11, 2018

This has effectively been hanlded by #694.

@cyphar cyphar closed this as completed Mar 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants