diff --git a/CHANGELOG.md b/CHANGELOG.md index 048a26a4d..e1618b5dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). functionality has not been used "in the wild" and `umoci` doesn't have the UX to create such structures (yet) but these will be implemented in future versions. openSUSE/umoci#145 +- `umoci repack` now supports `--mask-path` to ignore changes in the rootfs + that are in a child of at least one of the provided masks when generating new + layers. openSUSE/umoci#127 ### Changed - Error messages from `github.com/openSUSE/umoci/oci/cas/drivers/dir` actually @@ -32,11 +35,19 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `umoci unpack` now generates `config.json` blobs according to the [still proposed][ispec-pr492] OCI image specification conversion document. openSUSE/umoci#120 +- `umoci repack` also now automatically adding `Config.Volumes` from the image + configuration to the set of masked paths. This matches recently added + [recommendations by the spec][ispec-pr694], but is a backwards-incompatible + change because the new default is that `Config.Volumes` **will** be masked. + If you wish to retain the old semantics, use `--no-mask-volumes` (though make + sure to be aware of the reasoning behind `Config.Volume` masking). + openSUSE/umoci#127 [cii]: https://bestpractices.coreinfrastructure.org/projects/1084 [rspec-v1.0.0-rc6]: https://github.com/opencontainers/runtime-spec/releases/tag/v1.0.0-rc6 [ispec-v1.0.0-rc7]: https://github.com/opencontainers/image-spec/releases/tag/v1.0.0-rc7 [ispec-pr492]: https://github.com/opencontainers/image-spec/pull/492 +[ispec-pr694]: https://github.com/opencontainers/image-spec/pull/694 ## [0.2.1] - 2017-04-12 ### Added diff --git a/cmd/umoci/repack.go b/cmd/umoci/repack.go index b6681d010..a23aff6f1 100644 --- a/cmd/umoci/repack.go +++ b/cmd/umoci/repack.go @@ -31,6 +31,7 @@ import ( "github.com/openSUSE/umoci/oci/casext" igen "github.com/openSUSE/umoci/oci/config/generate" "github.com/openSUSE/umoci/oci/layer" + "github.com/openSUSE/umoci/pkg/mtreefilter" ispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/urfave/cli" @@ -65,6 +66,17 @@ manifest and configuration information uses the new diff atop the old manifest.` // repack creates a new image, with a given tag. Category: "image", + Flags: []cli.Flag{ + cli.StringSliceFlag{ + Name: "mask-path", + Usage: "set of path prefixes in which deltas will be ignored when generating new layers", + }, + cli.BoolFlag{ + Name: "no-mask-volumes", + Usage: "do not add the Config.Volumes of the image to the set of masked paths", + }, + }, + Action: repack, Before: func(ctx *cli.Context) error { @@ -156,6 +168,19 @@ func repack(ctx *cli.Context) error { "ndiff": len(diffs), }).Debugf("umoci: checked mtree spec") + // We need to mask config.Volumes. + config, err := mutator.Config(context.Background()) + if err != nil { + return errors.Wrap(err, "get config") + } + maskedPaths := ctx.StringSlice("mask-path") + if !ctx.Bool("no-mask-volumes") { + for v := range config.Volumes { + maskedPaths = append(maskedPaths, v) + } + } + diffs = mtreefilter.FilterDeltas(diffs, mtreefilter.MaskFilter(maskedPaths)) + reader, err := layer.GenerateLayer(fullRootfsPath, diffs, &meta.MapOptions) if err != nil { return errors.Wrap(err, "generate diff layer") diff --git a/pkg/mtreefilter/mask.go b/pkg/mtreefilter/mask.go new file mode 100644 index 000000000..7f878b801 --- /dev/null +++ b/pkg/mtreefilter/mask.go @@ -0,0 +1,76 @@ +/* + * umoci: Umoci Modifies Open Containers' Images + * Copyright (C) 2017 SUSE LLC. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package mtreefilter + +import ( + "path/filepath" + + "github.com/apex/log" + "github.com/vbatts/go-mtree" +) + +// FilterFunc is a function used when filtering deltas with FilterDeltas. +type FilterFunc func(path string) bool + +// isParent returns whether the path a is lexically an ancestor of the path b. +func isParent(a, b string) bool { + a = filepath.Clean(a) + b = filepath.Clean(b) + + for a != b && b != filepath.Dir(b) { + b = filepath.Dir(b) + } + return a == b +} + +// MaskFilter is a factory for FilterFuncs that will mask all InodeDelta paths +// that are lexical children of any path in the mask slice. All paths are +// considered to be relative to '/'. +func MaskFilter(masks []string) FilterFunc { + return func(path string) bool { + // Convert the path to be cleaned and relative-to-root. + path = filepath.Join("/", path) + + // Check that no masks are matched. + for _, mask := range masks { + // Mask also needs to be cleaned and relative-to-root. + mask = filepath.Join("/", mask) + + // Is it a parent? + if isParent(mask, path) { + log.Debugf("maskfilter: ignoring path %q matched by mask %q", path, mask) + return false + } + } + + return true + } +} + +// FilterDeltas is a helper function to easily filter []mtree.InodeDelta with a +// filter function. Only entries which have `filter(delta.Path()) == true` will +// be included in the returned slice. +func FilterDeltas(deltas []mtree.InodeDelta, filter FilterFunc) []mtree.InodeDelta { + var filtered []mtree.InodeDelta + for _, delta := range deltas { + if filter(delta.Path()) { + filtered = append(filtered, delta) + } + } + return filtered +} diff --git a/pkg/mtreefilter/mask_test.go b/pkg/mtreefilter/mask_test.go new file mode 100644 index 000000000..e05a6a017 --- /dev/null +++ b/pkg/mtreefilter/mask_test.go @@ -0,0 +1,144 @@ +/* + * umoci: Umoci Modifies Open Containers' Images + * Copyright (C) 2017 SUSE LLC. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package mtreefilter + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/vbatts/go-mtree" +) + +func TestIsParent(t *testing.T) { + for _, test := range []struct { + parent, path string + expected bool + }{ + {"/", "/a", true}, + {"/", "/a/b/c", true}, + {"/", "/", true}, + {"/a path/", "/a path", true}, + {"/a nother path", "/a nother path/test", true}, + {"/a nother path", "/a nother path/test/1 2/ 33 /", true}, + {"/path1", "/path2", false}, + {"/pathA", "/PATHA", false}, + {"/pathC", "/path", false}, + {"/path9", "/", false}, + // Make sure it's not the same as filepath.HasPrefix. + {"/a/b/c", "/a/b/c/d", true}, + {"/a/b/c", "/a/b/cd", false}, + {"/a/b/c", "/a/bcd", false}, + {"/a/bc", "/a/bcd", false}, + {"/abc", "/abcd", false}, + } { + got := isParent(test.parent, test.path) + if got != test.expected { + t.Errorf("isParent(%q, %q) got %v expected %v", test.parent, test.path, got, test.expected) + } + } +} + +func TestMaskDeltas(t *testing.T) { + dir, err := ioutil.TempDir("", "TestMaskDeltas-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + var mtreeKeywords []mtree.Keyword = append(mtree.DefaultKeywords, "sha256digest") + + // Create some files. + if err != ioutil.WriteFile(filepath.Join(dir, "file1"), []byte("contents"), 0644) { + t.Fatal(err) + } + if err != ioutil.WriteFile(filepath.Join(dir, "file2"), []byte("another content"), 0644) { + t.Fatal(err) + } + if err != os.MkdirAll(filepath.Join(dir, "dir", "child"), 0755) { + t.Fatal(err) + } + if err != os.MkdirAll(filepath.Join(dir, "dir", "child2"), 0755) { + t.Fatal(err) + } + if err != ioutil.WriteFile(filepath.Join(dir, "dir", "file 3"), []byte("more content"), 0644) { + t.Fatal(err) + } + if err != ioutil.WriteFile(filepath.Join(dir, "dir", "child2", "4 files"), []byte("very content"), 0644) { + t.Fatal(err) + } + + // Generate a diff. + originalDh, err := mtree.Walk(dir, nil, mtreeKeywords, nil) + if err != nil { + t.Fatal(err) + } + + // Modify the root. + if err := os.RemoveAll(filepath.Join(dir, "file2")); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(dir, "dir", "new"), []byte("more content"), 0755); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(dir, "file1"), []byte("different contents"), 0666); err != nil { + t.Fatal(err) + } + + // Generate the set of diffs. + newDh, err := mtree.Walk(dir, nil, mtreeKeywords, nil) + if err != nil { + t.Fatal(err) + } + diff, err := mtree.Compare(originalDh, newDh, mtreeKeywords) + if err != nil { + t.Fatal(err) + } + + for _, test := range []struct { + paths []string + }{ + {nil}, + {[]string{"/"}}, + {[]string{"dir"}}, + {[]string{filepath.Join("dir", "child2")}}, + {[]string{"file2"}}, + {[]string{"/", "file2"}}, + {[]string{"file2", filepath.Join("dir", "child2")}}, + } { + newDiff := FilterDeltas(diff, MaskFilter(test.paths)) + for _, delta := range newDiff { + if len(test.paths) == 0 { + if len(newDiff) != len(diff) { + t.Errorf("expected diff={} to give %d got %d", len(diff), len(newDiff)) + } + } else { + found := false + for _, path := range test.paths { + if !isParent(path, delta.Path()) { + found = true + } + } + if !found { + t.Errorf("expected one of %v to not be a parent of %q", test.paths, delta.Path()) + } + } + } + } +} diff --git a/test/repack.bats b/test/repack.bats index 638d20281..540860e8f 100644 --- a/test/repack.bats +++ b/test/repack.bats @@ -499,3 +499,109 @@ function teardown() { image-verify "${IMAGE}" } + +@test "umoci repack [--config.volumes]" { + BUNDLE_A="$(setup_tmpdir)" + BUNDLE_B="$(setup_tmpdir)" + BUNDLE_C="$(setup_tmpdir)" + BUNDLE_D="$(setup_tmpdir)" + + # Set some paths to be volumes. + umoci config --image "${IMAGE}:${TAG}" --config.volume /volume --config.volume "/some nutty/path name/ here" + [ "$status" -eq 0 ] + image-verify "${IMAGE}" + + # Unpack the image. + umoci unpack --image "${IMAGE}:${TAG}" "$BUNDLE_A" + [ "$status" -eq 0 ] + bundle-verify "$BUNDLE_A" + + # Create files in those volumes. + mkdir -p "$BUNDLE_A/rootfs/some nutty/path name/" + echo "this is a test" > "$BUNDLE_A/rootfs/some nutty/path name/ here" + mkdir -p "$BUNDLE_A/rootfs/volume" + echo "another test" > "$BUNDLE_A/rootfs/volume/test" + # ... and some outside. + echo "more tests" > "$BUNDLE_A/rootfs/some nutty/path " + mkdir -p "$BUNDLE_A/rootfs/some/volume" + echo "in a mirror directory" > "$BUNDLE_A/rootfs/some/volume/test" + echo "checking mirror" > "$BUNDLE_A/rootfs/volumetest" + + # Repack the image under a new tag. + umoci repack --image "${IMAGE}:${TAG}-new" "$BUNDLE_A" + [ "$status" -eq 0 ] + image-verify "${IMAGE}" + + # Re-extract to verify the volume changes weren't included. + umoci unpack --image "${IMAGE}:${TAG}-new" "$BUNDLE_B" + [ "$status" -eq 0 ] + bundle-verify "$BUNDLE_B" + + # Check the files. + [ -f "$BUNDLE_B/rootfs/some nutty/path " ] + [[ "$(cat "$BUNDLE_B/rootfs/some nutty/path ")" == "more tests" ]] + [ -d "$BUNDLE_B/rootfs/some/volume" ] + [ -f "$BUNDLE_B/rootfs/some/volume/test" ] + [[ "$(cat "$BUNDLE_B/rootfs/some/volume/test")" == "in a mirror directory" ]] + [ -f "$BUNDLE_B/rootfs/volumetest" ] + [[ "$(cat "$BUNDLE_B/rootfs/volumetest")" == "checking mirror" ]] + + # Volume paths must not be included. + ! [ -e "$BUNDLE_B/rootfs/volume" ] + ! [ -e "$BUNDLE_B/rootfs/volume/test" ] + ! [ -e "$BUNDLE_B/rootfs/some nutty/path name" ] + ! [ -e "$BUNDLE_B/rootfs/some nutty/path name/ here" ] + + # Repack a copy with volumes not masked. + umoci repack --image "${IMAGE}:${TAG}-nomask" --no-mask-volumes "$BUNDLE_A" + [ "$status" -eq 0 ] + image-verify "${IMAGE}" + + # Extract the no-mask variant to make sure that masked changes *were* included. + umoci unpack --image "${IMAGE}:${TAG}-nomask" "$BUNDLE_C" + [ "$status" -eq 0 ] + bundle-verify "$BUNDLE_C" + + # Check the files. + [ -f "$BUNDLE_C/rootfs/some nutty/path " ] + [[ "$(cat "$BUNDLE_C/rootfs/some nutty/path ")" == "more tests" ]] + [ -d "$BUNDLE_C/rootfs/some/volume" ] + [ -f "$BUNDLE_C/rootfs/some/volume/test" ] + [[ "$(cat "$BUNDLE_C/rootfs/some/volume/test")" == "in a mirror directory" ]] + [ -f "$BUNDLE_C/rootfs/volumetest" ] + [[ "$(cat "$BUNDLE_C/rootfs/volumetest")" == "checking mirror" ]] + + # Volume paths must be included, as well as their contents. + [ -e "$BUNDLE_C/rootfs/volume" ] + [ -f "$BUNDLE_C/rootfs/volume/test" ] + [[ "$(cat "$BUNDLE_C/rootfs/volume/test")" == "another test" ]] + [ -d "$BUNDLE_C/rootfs/some nutty/path name" ] + [ -f "$BUNDLE_C/rootfs/some nutty/path name/ here" ] + [[ "$(cat "$BUNDLE_C/rootfs/some nutty/path name/ here")" == "this is a test" ]] + + # Re-do everything but this time with --mask-path. + umoci repack --image "${IMAGE}:${TAG}-new" --mask-path /volume "$BUNDLE_A" + [ "$status" -eq 0 ] + image-verify "${IMAGE}" + + # Re-extract to verify the masked path changes weren't included. + umoci unpack --image "${IMAGE}:${TAG}-new" "$BUNDLE_D" + [ "$status" -eq 0 ] + bundle-verify "$BUNDLE_D" + + # Check the files. + [ -f "$BUNDLE_D/rootfs/some nutty/path " ] + [[ "$(cat "$BUNDLE_D/rootfs/some nutty/path ")" == "more tests" ]] + [ -d "$BUNDLE_D/rootfs/some/volume" ] + [ -f "$BUNDLE_D/rootfs/some/volume/test" ] + [[ "$(cat "$BUNDLE_D/rootfs/some/volume/test")" == "in a mirror directory" ]] + [ -f "$BUNDLE_D/rootfs/volumetest" ] + [[ "$(cat "$BUNDLE_D/rootfs/volumetest")" == "checking mirror" ]] + + # Masked paths must not be included. + ! [ -e "$BUNDLE_D/rootfs/volume" ] + ! [ -e "$BUNDLE_D/rootfs/volume/test" ] + # And volumes will also not be included. + ! [ -e "$BUNDLE_D/rootfs/some nutty/path name" ] + ! [ -e "$BUNDLE_D/rootfs/some nutty/path name/ here" ] +}