Skip to content

Commit

Permalink
merge branch 'pr-127'
Browse files Browse the repository at this point in the history
LGTMs: @cyphar
Closes #127
  • Loading branch information
cyphar committed Jul 19, 2017
2 parents 7077cc7 + 1e52a94 commit 6982793
Show file tree
Hide file tree
Showing 5 changed files with 362 additions and 0 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions cmd/umoci/repack.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down
76 changes: 76 additions & 0 deletions 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
}
144 changes: 144 additions & 0 deletions 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())
}
}
}
}
}

0 comments on commit 6982793

Please sign in to comment.