repack: do not include Volume paths in new layers

CHANGELOG.md

@@ -25,18 +25,29 @@ 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
   make sense now. openSUSE/umoci#121
 - `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

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")

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
+}

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())
+				}
+			}
+		}
+	}
+}