add `umoci insert` command #237

tych0 · 2018-04-17T18:07:48Z

There are cases where you only want to insert files into an image. Right
now the procedure is: unpack the image, add the files, and repack the
image. However, the OCI format lends itself exactly to this kind of
addition -- we can just add a new layer with the files we wish to insert.
Additional motivation is in the case of large layers, we can avoid a lot of
i/o, CPU time to compress, and disk space requirements.

So, let's add a command to generate this new import layer.

Signed-off-by: Tycho Andersen tycho@tycho.ws

cyphar · 2018-04-18T12:32:15Z

oci/layer/generate.go

+
+		tg := newTarGenerator(writer, mapOptions)
+
+		err = filepath.Walk(root, func(curPath string, info os.FileInfo, err error) error {


I think this should be using unpriv.Walk (with the regular checks for which fs we should use). One of the things people want is for umoci to use it's magic CAP_DAC_OVERRIDE emulation to be able to handle annoying paths.

And once we add that, we'll need another integration test to make sure that the path modes are included correctly.

tych0 · 2018-04-23T15:06:38Z

Hey, sorry for the delay, I updated it and added a stat test.

cyphar

This also needs documentation. I can deal with the site documentation, but you can just copy the existing man page templates and modify them to fit (after you've added the mapping options of course).

cyphar · 2018-04-23T16:12:14Z

test/insert.bats

+
+	# ...and too many
+	umoci insert --image "${IMAGE}:${TAG}" asdf 123 456
+	[ "$status" -ne 0 ]


I prefer this to be in a separate test (to match the other "are the arguments handled correctly" tests).

Ok, might be worth changing unpack/repack too, that's where I got this from, and they're just inlined there.

Oh, then scratch that (I'll double-check why I thought that was the case...).

cyphar · 2018-04-23T16:14:45Z

test/insert.bats

+	[ "$status" -eq 0 ]
+
+	umoci insert --image "${IMAGE}:${TAG}" "${ROOT}/umoci" /tester
+	[ "$status" -eq 0 ]


I would also recommend having a rootfs with something like

mkdir -p $BUNDLE_A/rootfs/some/path chmod 000 $BUNDLE_A/rootfs/some/path umoci repack # whatever umoci insert blah /some/path/blah

(This should make no difference to the actual generation, but it would ensure that rootless is "handled" in that case. Also a similar case should be handled from the host side of things to make sure rootless is properly obeyed and the added directory is actually reconstructed correctly.)

Also this test should do an unpack after the first insertion to ensure that the file actually changes between the two insertions IMO.

If all of that is a bit too much to change, I can push them on top of your branch once you're done developing.

Nope, I'm happy to do that too.

cyphar · 2018-04-23T16:17:39Z

oci/layer/generate.go

+func GenerateInsertLayer(root string, prefix string, opt *MapOptions) io.ReadCloser {
+	root = path.Clean(root)
+	rootPrefixLen := len(root) - len(path.Base(root))
+	prefix = strings.TrimPrefix(prefix, "/")


This should be some mix of utils.CleanPath or similar. I'm not sure which. I get that you want to strip leading '/' but you can do that in the Join call. In addition, I'm not really sure what you're doing with rootPrefixLen -- why not use path.Rel?

rootPrefixLen is basically some fancy math so that if the user does: umoci insert --image ... /thing/to/insert /usr/bin, we end up with insert in /usr/bin, and not /usr/bin/thing/to/insert; path.Rel doesn't quite do the right thing here.

The TrimPrefix bit is because of this hunk: https://github.com/openSUSE/umoci/blob/master/oci/layer/tar_generate.go#L94 which causes a failure if there's an absolute path passed to the tar generator. It's not really clear why that's there, but I assume you had a reason, so I just trimmed the prefix. I'm not sure any path cleaning will do that, though.

umoci insert --image ... /thing/to/insert /usr/bin, we end up with insert in /usr/bin, and not /usr/bin/thing/to/insert; path.Rel doesn't quite do the right thing here.

Shouldn't be able to just do filepath.Base?

The TrimPrefix bit is because of this hunk: https://github.com/openSUSE/umoci/blob/master/oci/layer/tar_generate.go#L94 which causes a failure if there's an absolute path passed to the tar generator. It's not really clear why that's there, but I assume you had a reason, so I just trimmed the prefix.

It's for security reasons, to double-check that the path lexically doesn't have any un-clean components (so something like /../../../../../../../blah). But you're right that I didn't consider the "/"+"/abc" != filepath.Join("/", "/abc") edge-case (because generally tar archives don't have leading / for their entries).

We could fix this by removing leading / when normalising the paths.

no, because filepath.Base("/thing/to/insert") == "insert", not "/thing/to", which is what we want to know the length of. I think filepath.Dir() is the right thing though, I can change it to that.

R.e. the tar issue, you do a CleanPath() before that check, so shouldn't all the ..s be cleaned out? I suppose extra paranoia is ok, but if you don't trust your own code...

I'm happy to just trim the leading / in normalise(), though.

Sorry, should've been clearer with filepath.Base -- I was suggesting that we remove all of the rootPrefixLen code and just use filepath.Base to get the basename and append it to the target with filepath.Join. Unless I'm misunderstanding something, that should do the same thing (and it's more clear what you're actually trying to do with the paths).

R.e. the tar issue, you do a CleanPath() before that check, so shouldn't all the ..s be cleaned out? I suppose extra paranoia is ok, but if you don't trust your own code...

Yeah, we do a CleanPath. I'll be honest, I'm more than a little bit paranoid about the path handling code in umoci because it's the most dangerous thing we're dealing with (and there have been Docker CVEs because of missing trivial checks like the above -- some of my first Docker contributions were fixing security bugs in path sanitisation).

I'm happy to just trim the leading / in normalise(), though.

👍 Just do something like if filepath.IsAbs(path) { ... } (personally I would use filepath.Rel for it for clarity, even though the / case is a bit too trivial to justify it -- but feel free to just use strings.TrimPrefix).

cyphar · 2018-04-23T16:21:33Z

cmd/umoci/insert.go

@@ -0,0 +1,140 @@
+/*
+ * umoci: Umoci Modifies Open Containers' Images
+ * Copyright (C) 2018 Cisco


A lot of this file is copied from the other cmd/umoci/*.go files -- can you add an additional copyright line from the original?

cyphar · 2018-04-23T16:22:58Z

cmd/umoci/insert.go

+	}
+
+	// TODO: add some way to specify these from the cli
+	reader := layer.GenerateInsertLayer(ctx.Args()[0], ctx.Args()[1], nil)


You always pass nil for MapOptions here. That doesn't look right to me -- I would copy the unpack code for map options (perhaps even refactor them out and use the same code). I'm also not a huge fan of the ctx.Args usage here -- I would use ctx.App.Metadata (as ugly of a hack it might be) to make it more consistent.

Not only is it ugly, I think it's also unecessary. You can use https://godoc.org/github.com/urfave/cli#Context.GlobalBool and friends to access global stuff from a local context.

The way ctx.App.Metadata is used is so that we can do parsing and other modifications to the options in the BeforeFunc (so that the errors are done at the right time and so that we can have utils_ux refactor it). I don't think Global* would help for this case. Also unless I'm mistaken you cannot place global flags after the subcommand name (though I might be completely mistaken).

cyphar · 2018-04-23T16:24:40Z

cmd/umoci/insert.go

+		}
+		if ctx.Args()[1] == "" {
+			return errors.Errorf("path cannot be empty")
+		}


I would specify which path cannot be empty. Also, I'm not sure why the prefix cannot be empty -- an empty prefix just means it should be added to the root, no?

cyphar · 2018-04-23T16:26:06Z

cmd/umoci/insert.go

+Where "<image-path>" is the path to the OCI image, "<tag>" is the name of the
+tag that the content wil be inserted into (if not specified, defaults to
+"latest"), "<file>" is the file or folder to insert, and "<path>" is the prefix
+inside the image to insert it into.`,


I would mention that the prefix is not like tar prefixes -- where you can make prefixes that aren't entire path components. I assume this decision was intentional, so I would mention it.

I'm not sure what you mean by this, can you elaborate?

Sorry, I got confused. For a minute I though that tar cvf had an option to add a string prefix, and thought that it would be worthwhile to mention this is not the same semantics. tar has transform functions but the only relevant one is sed-like transforms.

But personally I would not use the word prefix in the documentation -- say that it's the target directory. Also I have a comment about the semantics of file insertion (you cannot change the filename when you insert at the moment, which doesn't feel ideal -- I would prefer if we match cp or rsync's semantics as close as we possibly can).

Ok. What do you want to see, then? I personally detest rsync's whole thing about trailing slash vs not, so I'd like to avoid that if possible.

cyphar · 2018-04-24T09:24:44Z

I have a concern that the semantics of umoci insert don't match cp (or rsync) when it comes to inserting files. As far as I can tell, you cannot insert a file into a path where the basename is different (the path is taken to be the directory the file will be placed into -- unlike the target semantics of most tools).

Of course, we cannot nicely just stat the target to figure out if it's a directory or a file, so we should do something like "if the target ends with a / we add it as a child of the target path with the same basename as the source, otherwise we use the target path verbatim".

Also, one other problem is how do we handle something like adding a directory over the top of another directory in the image (which could either result in a merged directory, or a directory only containing the inserted contents). I get the feeling you'd probably want to be able to delete any underlying directories on insertion (maybe as a --replace option or something) where we would add an opaque whiteout.

tych0 · 2018-04-24T14:47:30Z

IMO any weirdness about the user specifying a trailing / is basically terrible, and we should never do that. The semantics aren't the same as cp or rsync, but rsync's interface sucks so I'll ignore that. I could imagine something like umoci insert /path/to/insert /path/to/copy/to, which would be identical to cp, and thus require you to specify whatever prefix you want. FWIW, that's exactly what happens now, it's just that the documentation describes it in terms of prefix instead of destination path.

cyphar · 2018-04-24T16:27:25Z

In short, there are two things that I think should be doable with umoci insert. If you like we can work on these separate to this PR or we can work on them now. Here they are:

% # inserts a file (or directory) *at* the given path -- not as a child of the directory
% umoci insert --image some_image my_new_etc_passwd /etc/passwd
% # inserts a directory, overwriting any pre-existing directory -- not just merging the new with the old
% umoci insert --image some_image --whiteout some_dir /var/existing_directory

The first is just a matter of the path we generate, and making it handle that case. The second requires generating an opaque whiteout for /var/existing_directory.

IMO any weirdness about the user specifying a trailing / is basically terrible, and we should never do that. The semantics aren't the same as cp or rsync, but rsync's interface sucks so I'll ignore that.

FWIW, I agree that rsync's interface is pretty annoying, but I can't think of a simpler way of disambiguating between "I want you to insert this file at this path" versus "I want you to insert this file under this path". If you have one that doesn't require replicating the lovely / confusion, I'd be happy to take it -- I just couldn't think of a better example. 😖

But in my view the above two operations are pretty important (though the first is mainly a quality-of-life thing) in order to be able to use this in all of the situations I can think of. But as I said above, we can merge these changes earlier and I can work on the other things before the next release -- whichever works for you.

cyphar · 2018-04-24T16:32:05Z

FWIW, I agree that rsync's interface is pretty annoying, but I can't think of a simpler way of disambiguating between "I want you to insert this file at this path" versus "I want you to insert this file under this path".

Sorry, it should also be noted that we don't have to deal with the most confusing case with rsync -- "what if the target path already exists". Since we don't have the ability to (easily and efficiently) figure out if a path exists, that case is entirely ignored. Really then it boils down to

% umoci insert --image ABC /some/path /target/path  # /target/path = /same/path
% umoci insert --image ABC /some/path /target/path/ # /target/path/path = /same/path

Which is just a different way of making a boolean flag, like this:

% umoci insert --image ABC --verbatim /some/path /target/path # /target/path = /same/path
% umoci insert --image ABC            /some/path /target/path # /target/path/path = /same/path

Now, --verbatim is more explicit and less likely to be triggered by accident. But you could argue it's more cumbersome. But I don't really mind either way to be honest -- and I do share your opinion that changing semantics on trailing / is basically awful (though it should be noted that both cp and Linux do this in some situations -- not that it makes it right though).

tych0 · 2018-04-24T16:47:08Z

On Tue, Apr 24, 2018 at 09:32:08AM -0700, Aleksa Sarai wrote: > FWIW, I agree that `rsync`'s interface is pretty annoying, but I can't think of a simpler way of disambiguating between "I want you to insert this file **at** this path" versus "I want you to insert this file **under** this path". Sorry, it should also be noted that we don't have to deal with the most confusing case with `rsync` -- "what if the target path already exists". Since we don't have the ability to (easily and efficiently) figure out if a path exists, that case is entirely ignored. Really then it boils down to ``` % umoci insert --image ABC /some/path /target/path # /target/path = /same/path % umoci insert --image ABC /some/path /target/path/ # /target/path/path = /same/path ``` Which is just a different way of making a boolean flag, like this: ``` % umoci insert --image ABC --verbatim /some/path /target/path # /target/path = /same/path % umoci insert --image ABC /some/path /target/path # /target/path/path = /same/path ``` Now, `--verbatim` is more explicit and less likely to be triggered by accident. But you could argue it's more cumbersome. But I don't really mind either way to be honest.

I guess my argument would be that you basically never want /target/path/path, and if you do, you can just say that instead. So it should always be: target := path.Join(targetPath, path.Base(sourcePath)) of course, it's slightly more complicated, since when someone gives a directory, we'll end up walking the path, and inside the walk we'll do something like: target := path.Join(targetPath, current[path.Dir(sourcePath):]) That's basically what it does now, except with slightly uglier math on the paths. Tycho

cyphar · 2018-04-24T18:23:53Z

I guess my argument would be that you basically never want /target/path/path, and if you do, you can just say that instead.

So it should always be: target := path.Join(targetPath, path.Base(sourcePath))

But the current implementation does /target/path/path AFAICS:

% umoci insert --image opensuse:latest COPYING /test # I would expect /test = COPYING
% sudo ./umoci unpack --image opensuse:latest bundle
% sudo ls bundle/rootfs/test # it's a directory -- /target/path would be that bundle/rootfs/test is COPYING
COPYING

Which is why I mentioned it. If you think we don't need to support /target/path/path (I think it would be okay to have that option personally -- but not supporting it definitely makes it simpler), then we need to change the current semantics to match that. Or maybe I didn't explain what I meant by /target/path/path properly?

tych0 · 2018-04-24T18:50:00Z

On Tue, Apr 24, 2018 at 06:23:54PM +0000, Aleksa Sarai wrote: > I guess my argument would be that you basically never want /target/path/path, and if you do, you can just say that instead. > > So it should always be: target := path.Join(targetPath, path.Base(sourcePath)) But the current implementation does `/target/path/path` AFAICS: ``` % umoci insert --image opensuse:latest COPYING /test # I would expect /test = COPYING % sudo ./umoci unpack --image opensuse:latest bundle % sudo ls bundle/rootfs/test # it's a directory -- /target/path would be that bundle/rootfs/test is COPYING COPYING ``` Which is why I mentioned it. If you think we don't need to support `/target/path/path` (I think it would be okay to have that option personally -- but not supporting it definitely makes it simpler), then we need to change the current semantics to match that. Or maybe I didn't explain what I meant by `/target/path/path` properly?

Oh, I guess I was thinking of the case where the source is a directory, but I suppose it's the same thing. I'll see if I can get it to work this way.

cyphar · 2018-04-27T11:35:00Z

This will now need a rebase on top of #240, to use the refactored --uid-map parsing code (it makes it easier).

tych0 · 2018-04-30T14:27:48Z

Thanks for the heads up. Also, I had another thought about the /target/path/path issue, which was that in fact the current version doesn't. Your contrived example repeats path, but in the real world you would not:

umoci insert myexecutable /usr/bin
umoci insert myconfig /etc

Whereas with what you're proposing, we need to do,

umoci insert myexecutable /usr/bin/myexecutable
umoci insert myconfig /etc/myconfig

Again I'm happy to switch it, but it does result in more typing for everyone, although it also allows you to rename things on the fly.

tych0 · 2018-04-30T17:10:12Z

Ok, I think I've addressed everything; let me know if I missed something.

tych0 · 2018-04-30T20:03:08Z

Ah yes, the best kind of bug, where it passes locally and fails on travis :)

cyphar · 2018-04-30T23:02:47Z

I'll review this later today, or I can wait until you've fixed up the Travis failure (it's failing during rootless tests with bad argument errors, which is a bit odd).

Again I'm happy to switch it, but it does result in more typing for everyone, although it also allows you to rename things on the fly.

That's why I suggested having both options -- similar to how cp works (though with cp you can stat the target path to figure out if it's a directory or not -- doing that in an image would make it much more expensive). Personally I think it's necessary to have the ability to "rename things on the fly" because that's something that I think most people would expect to be able to do (we could add a -T or -t option that matches the cp semantics if we didn't want to do it using the trailing-/ semantics I proposed earlier).

tych0 · 2018-04-30T23:15:01Z

On Mon, Apr 30, 2018 at 04:02:48PM -0700, Aleksa Sarai wrote: I'll review this later today, or I can wait until you've fixed up the Travis failure (it's failing during rootless tests with bad argument errors, which is a bit odd).

yeah, I don't know what that's about. it works for me locally.

> Again I'm happy to switch it, but it does result in more typing for everyone, although it also allows you to rename things on the fly. That's why I suggested having both options -- similar to how `cp` works (though with `cp` you can `stat` the target path to figure out if it's a directory or not -- doing that in an image would make it much more expensive). Personally I think it's necessary to have the ability to "rename things on the fly" (we could add a `-T` or `-t` option that matches the `cp` semantics if we didn't want to do it using the trailing-`/` semantics I proposed earlier).

yeah, check out the way it works now. it's cp without the stat, exactly as in one of your proposals.

cyphar · 2018-05-01T00:45:03Z

Awesome. I'll play around with it today, and I can try to figure out why the tests are failing as well.

tych0 · 2018-05-04T00:29:04Z

Ok, I think I figured out what was wrong with the tests. This should be good to go now.

tych0 · 2018-05-08T23:14:24Z

Ping, any thoughts on this?

cyphar · 2018-05-09T16:30:30Z

This is looking pretty good. I'm going to play around with this and do a proper review soon. This will need a rebase. Documentation would also be good, but if you prefer I can do that in a follow-up.

tych0 · 2018-05-09T22:55:57Z

I rebased it. Let's get a version in before we bikeshed on any docs.

cyphar · 2018-05-09T23:05:03Z

Sure. 😉

cyphar · 2018-05-09T23:53:45Z

One thing that I mentioned above is that we don't have a way of overwriting an entire directory -- so if you insert a directory over an existing directory you end up with a merge of the two (with no nice way of only getting the new directory).

% mkdir dir1 && touch dir1/should_not_exist
% mkdir dir2 && touch dir2/should_exist
% umoci insert --image image dir1 /path
% umoci insert --image image dir2 /path
% umoci unpack --image image bundle && ls bundle/rootfs/path
should_exist  should_not_exist

I will work on this feature once this is merged as well. I will probably just make it something like a --whiteout flag (since I imagine that people will be interested in both ways of doing it).

cyphar · 2018-05-10T06:59:22Z

Tests are timing out on Travis (but only for rootless and only for Fedora). I can reproduce the test taking a long time -- it might even block indefinitely actually. Not sure why the test is taking so much longer than it used to..

cyphar · 2018-05-10T07:34:40Z

Okay, there's now a test failure in rootless (you can't see it on Travis for some reason):

not ok 60 umoci repack [replace]
# (in test file test/repack.bats, line 204)
#   `ln -s "a \\really //weird _00..:=path " "$BUNDLE_A/rootfs/usr/bin/env"' failed
# 21M   /tmp/umoci-coverage.7NhRrl
# 0     /tmp/umoci.YCR12A
# 83M   /tmp/image/blobs
# 83M   /tmp/image
# 103M  /tmp
# oci-image-tool: reference "latest": OK
# /tmp/image: OK
# Validation succeeded
# umoci -test.coverprofile=/tmp/umoci-coverage.7NhRrl/umoci.cov.vtQiqE __DEVEL--i-heard-you-like-tests unpack --rootless --image /tmp/image:latest /tmp/umoci-integration-tmpdir.8q2KHYm5 (status=0)
# PASS
# coverage: 31.9% of statements in ./...
# Bundle validation succeeded.
# rm: remove write-protected regular file '/tmp/umoci-integration-tmpdir.8q2KHYm5/rootfs/usr/bin/env'? ln: failed to create symbolic link '/tmp/umoci-integration-tmpdir.8q2KHYm5/rootfs/usr/bin/env': File exists

tych0 · 2018-05-10T15:21:04Z

Ok, this seems unrelated -- presumably /usr/bin/env is not +w, and since you're not passing -f to the rm of it, it's blocking. Why it works on other platforms I have no idea :)

FWIW, I also see failures on current master on ubuntu of the same umoci repack test,

# /tmp/bats.8942.src: line 165: /tmp/umoci-integration-tmpdir.4dWrFtb7/rootfs/whiteout_test/.wh. THIS IS A TEST : Operation not permitted

I've been ignoring these as well.

@cyphar

use alternate directory for tests remove umoci.cov* files on clean use umoci.cover instead of umoci for stat test fix up TODO about supporting MapOptions for insert add rootless insertion check change the way inserted file paths are calculated use CleanPath instead of path.Clean move TrimPrefix call into normalise add additional copyright to insert header insert: use unpriv.Walk instead add `umoci insert` command LGTMs: @cyphar Closes opencontainers#237

cyphar · 2018-05-15T08:55:57Z

#243 fixed the test failures. If you rebase I'll merge it, thanks!

There are cases where you only want to insert files into an image. Right now the procedure is: unpack the image, add the files, and repack the image. However, the OCI format lends itself exactly to this kind of addition -- we can just add a new layer with the files we wish to insert. Additional motivation is in the case of large layers, we can avoid a lot of i/o, CPU time to compress, and disk space requirements. So, let's add a command to generate this new import layer. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

This will make rootless insert potentially actually work? Signed-off-by: Tycho Andersen <tycho@tycho.ws>

Signed-off-by: Tycho Andersen <tycho@tycho.ws>

We really don't need this IsAbs test, but it's what was recommended, so here we are. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

Signed-off-by: Tycho Andersen <tycho@tycho.ws>

The test suite only builds umoci.cover, so this only worked if you had done `make` in pwd before running the test suite. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

umoci.cover and umoci.cov.* files are generated by the test suite, let's be sure to clean all of those up as well. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

Travis seems to have trouble when inserting files from ${ROOT}, so let's just make another directory with these files for testing purposes. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

tych0 · 2018-05-15T14:43:02Z

Cool, thanks. Looks like the tests passed this time.

cyphar · 2018-05-16T04:32:09Z

LGTM. Thanks @tych0!

@cyphar

use alternate directory for tests remove umoci.cov* files on clean use umoci.cover instead of umoci for stat test fix up TODO about supporting MapOptions for insert add rootless insertion check change the way inserted file paths are calculated use CleanPath instead of path.Clean move TrimPrefix call into normalise add additional copyright to insert header insert: use unpriv.Walk instead add `umoci insert` command LGTMs: @cyphar Closes #237

tych0 force-pushed the umoci-insert branch 3 times, most recently from 0207f91 to cc3fd50 Compare April 17, 2018 20:23

cyphar reviewed Apr 18, 2018

View reviewed changes

tych0 force-pushed the umoci-insert branch from 775e532 to 0893750 Compare April 23, 2018 15:06

cyphar reviewed Apr 23, 2018

View reviewed changes

cyphar mentioned this pull request Apr 24, 2018

umoci: add "raw unpack" #239

Merged

tych0 force-pushed the umoci-insert branch from 0893750 to 96cbc52 Compare April 30, 2018 17:09

tych0 force-pushed the umoci-insert branch 5 times, most recently from 7b237d9 to 77d57f8 Compare May 3, 2018 23:47

tych0 force-pushed the umoci-insert branch from 77d57f8 to b9ee563 Compare May 9, 2018 22:55

tych0 added 11 commits May 15, 2018 07:42

insert: use unpriv.Walk instead

fbe4d21

This will make rootless insert potentially actually work? Signed-off-by: Tycho Andersen <tycho@tycho.ws>

add additional copyright to insert header

e34dc14

Signed-off-by: Tycho Andersen <tycho@tycho.ws>

move TrimPrefix call into normalise

5ce60c3

We really don't need this IsAbs test, but it's what was recommended, so here we are. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

use CleanPath instead of path.Clean

89c47fe

Signed-off-by: Tycho Andersen <tycho@tycho.ws>

change the way inserted file paths are calculated

b9566f4

Signed-off-by: Tycho Andersen <tycho@tycho.ws>

add rootless insertion check

538c62e

Signed-off-by: Tycho Andersen <tycho@tycho.ws>

fix up TODO about supporting MapOptions for insert

b5da056

Signed-off-by: Tycho Andersen <tycho@tycho.ws>

use umoci.cover instead of umoci for stat test

ebb17dd

The test suite only builds umoci.cover, so this only worked if you had done `make` in pwd before running the test suite. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

remove umoci.cov* files on clean

045182a

umoci.cover and umoci.cov.* files are generated by the test suite, let's be sure to clean all of those up as well. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

use alternate directory for tests

9ba2e57

Travis seems to have trouble when inserting files from ${ROOT}, so let's just make another directory with these files for testing purposes. Signed-off-by: Tycho Andersen <tycho@tycho.ws>

tych0 force-pushed the umoci-insert branch from b9ee563 to 9ba2e57 Compare May 15, 2018 13:42

cyphar merged commit 9ba2e57 into opencontainers:master May 16, 2018


		tg := newTarGenerator(writer, mapOptions)

		err = filepath.Walk(root, func(curPath string, info os.FileInfo, err error) error {

add umoci insert command #237

add umoci insert command #237

Conversation

tych0 commented Apr 17, 2018

Choose a reason for hiding this comment

tych0 commented Apr 23, 2018

cyphar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar Apr 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

cyphar Apr 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar commented Apr 24, 2018 • edited Loading

tych0 commented Apr 24, 2018

cyphar commented Apr 24, 2018

cyphar commented Apr 24, 2018 • edited Loading

tych0 commented Apr 24, 2018 via email

cyphar commented Apr 24, 2018

tych0 commented Apr 24, 2018 via email

cyphar commented Apr 27, 2018 • edited Loading

tych0 commented Apr 30, 2018

tych0 commented Apr 30, 2018

tych0 commented Apr 30, 2018

cyphar commented Apr 30, 2018 • edited Loading

tych0 commented Apr 30, 2018 via email

cyphar commented May 1, 2018

tych0 commented May 4, 2018

tych0 commented May 8, 2018

cyphar commented May 9, 2018

tych0 commented May 9, 2018

cyphar commented May 9, 2018

cyphar commented May 9, 2018 • edited Loading

cyphar commented May 10, 2018 • edited Loading

cyphar commented May 10, 2018

tych0 commented May 10, 2018

cyphar commented May 15, 2018

tych0 commented May 15, 2018

cyphar commented May 16, 2018

add `umoci insert` command #237

add `umoci insert` command #237

cyphar Apr 23, 2018 •

edited

Loading

cyphar Apr 24, 2018 •

edited

Loading

cyphar Apr 24, 2018 •

edited

Loading

cyphar Apr 23, 2018 •

edited

Loading

cyphar commented Apr 24, 2018 •

edited

Loading

cyphar commented Apr 24, 2018 •

edited

Loading

cyphar commented Apr 27, 2018 •

edited

Loading

cyphar commented Apr 30, 2018 •

edited

Loading

cyphar commented May 9, 2018 •

edited

Loading

cyphar commented May 10, 2018 •

edited

Loading