Skip to content

Commit

Permalink
merge branch 'pr-114'
Browse files Browse the repository at this point in the history
LGTMs: @cyphar
Closes openSUSE/umoci#114
  • Loading branch information
cyphar committed Apr 11, 2017
2 parents 4aa2146 + 8680526 commit 6994e3c
Show file tree
Hide file tree
Showing 24 changed files with 1,099 additions and 2,036 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]
### Changed
- `umoci` now outputs configurations that are compliant with [`v1.0.0-rc5` of
the OCI runtime-spec][rspec-v1.0.0-rc5]. This means that now you can use runc
v1.0.0-rc3 with `umoci` (and rootless containers should work out of the box
if you use a development build of runc). openSUSE/umoci#114
- `umoci unpack` no longer adds a dummy linux.seccomp entry, and instead just
sets it to null. openSUSE/umoci#114

[rspec-v1.0.0-rc5]: https://github.com/opencontainers/runtime-spec/releases/tag/v1.0.0-rc5

## [0.2.0] - 2017-04-11
### Added
Expand Down
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ MAINTAINER "Aleksa Sarai <asarai@suse.com>"
# into openSUSE:Factory yet.
RUN zypper ar -f -p 10 -g obs://Virtualization:containers obs-vc && \
zypper ar -f -p 10 -g obs://devel:languages:go obs-dlg && \
zypper ar -f -p 10 -g obs://home:cyphar:containers obs-home-cyphar-containers && \
zypper ar -f -p 15 -g obs://home:cyphar obs-home-cyphar && \
zypper ar -f -p 15 -g obs://devel:languages:python3 obs-py3k && \
zypper --gpg-auto-import-keys -n ref && \
Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ install.static: $(GO_SRC)
.PHONY: update-deps
update-deps:
hack/vendor.sh
hack/patch.sh

.PHONY: clean
clean:
Expand Down
3 changes: 3 additions & 0 deletions hack/patch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ patch() {
# project is, so I'm just going to backport it here until I see that there's
# upstream activity.
patch github.com/pkg/errors errors-0001-errors-add-Debug-function.patch

# Backport https://github.com/opencontainers/runtime-tools/pull/359.
patch github.com/opencontainers/runtime-tools runtime-tools-0001-generate-remove-validate-dependency.patch
135 changes: 135 additions & 0 deletions hack/runtime-tools-0001-generate-remove-validate-dependency.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
From cc52997196f8bf7717c5efb5aacd647bc0977282 Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <asarai@suse.de>
Date: Tue, 11 Apr 2017 23:38:02 +1000
Subject: [PATCH] generate: remove validate dependency

The two modules {validate,generate} should be mutually exclusive and
should not depend on each other. In addition, it is not the job of
generate to carry out validation of any arguments provided (especially
system-specific arguments).

lastCap needs two copies because of the RHEL6 hack, which is a shame but
does not justify the import dependency (because that dependency pulls in
logrus and a few other libraries for no good reason).

Fixes: 1a899a6d893e ("validate: optimize capabilites check")
Signed-off-by: Aleksa Sarai <asarai@suse.de>
---
generate/generate.go | 27 +++++++++++++++------------
validate/validate.go | 13 +++++++------
2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/generate/generate.go b/generate/generate.go
index f8a6294ec796..36bb4785d9e3 100644
--- a/generate/generate.go
+++ b/generate/generate.go
@@ -11,7 +11,6 @@ import (

rspec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/runtime-tools/generate/seccomp"
- "github.com/opencontainers/runtime-tools/validate"
"github.com/syndtr/gocapability/capability"
)

@@ -819,12 +818,24 @@ func (g *Generator) AddBindMount(source, dest string, options []string) {
g.spec.Mounts = append(g.spec.Mounts, mnt)
}

+// lastCap return last cap of system, and is required to hack around RHEL6.
+// This is an exact copy of "validate/validate.go".lastCap.
+func lastCap() capability.Cap {
+ last := capability.CAP_LAST_CAP
+ // hack for RHEL6 which has no /proc/sys/kernel/cap_last_cap
+ if last == capability.Cap(63) {
+ last = capability.CAP_BLOCK_SUSPEND
+ }
+
+ return last
+}
+
// SetupPrivileged sets up the privilege-related fields inside g.spec.
func (g *Generator) SetupPrivileged(privileged bool) {
if privileged { // Add all capabilities in privileged mode.
var finalCapList []string
for _, cap := range capability.List() {
- if g.HostSpecific && cap > validate.LastCap() {
+ if g.HostSpecific && cap > lastCap() {
continue
}
finalCapList = append(finalCapList, fmt.Sprintf("CAP_%s", strings.ToUpper(cap.String())))
@@ -855,12 +866,8 @@ func (g *Generator) ClearProcessCapabilities() {

// AddProcessCapability adds a process capability into g.spec.Process.Capabilities.
func (g *Generator) AddProcessCapability(c string) error {
- cp := strings.ToUpper(c)
- if err := validate.CapValid(cp, g.HostSpecific); err != nil {
- return err
- }
-
g.initSpec()
+ cp := strings.ToUpper(c)

for _, cap := range g.spec.Process.Capabilities.Bounding {
if strings.ToUpper(cap) == cp {
@@ -902,12 +909,8 @@ func (g *Generator) AddProcessCapability(c string) error {

// DropProcessCapability drops a process capability from g.spec.Process.Capabilities.
func (g *Generator) DropProcessCapability(c string) error {
- cp := strings.ToUpper(c)
- if err := validate.CapValid(cp, g.HostSpecific); err != nil {
- return err
- }
-
g.initSpec()
+ cp := strings.ToUpper(c)

for i, cap := range g.spec.Process.Capabilities.Bounding {
if strings.ToUpper(cap) == cp {
diff --git a/validate/validate.go b/validate/validate.go
index 76c3ca6a7337..c834815260ae 100644
--- a/validate/validate.go
+++ b/validate/validate.go
@@ -312,7 +312,7 @@ func (v *Validator) CheckCapablities() (msgs []string) {
}

for _, capability := range caps {
- if err := CapValid(capability, v.HostSpecific); err != nil {
+ if err := capValid(capability, v.HostSpecific); err != nil {
msgs = append(msgs, fmt.Sprintf("capability %q is not valid, man capabilities(7)", capability))
}
}
@@ -614,8 +614,8 @@ func (v *Validator) CheckSeccomp() (msgs []string) {
return
}

-// CapValid checks whether a capability is valid
-func CapValid(c string, hostSpecific bool) error {
+// capValid checks whether a capability is valid
+func capValid(c string, hostSpecific bool) error {
isValid := false

if !strings.HasPrefix(c, "CAP_") {
@@ -623,7 +623,7 @@ func CapValid(c string, hostSpecific bool) error {
}
for _, cap := range capability.List() {
if c == fmt.Sprintf("CAP_%s", strings.ToUpper(cap.String())) {
- if hostSpecific && cap > LastCap() {
+ if hostSpecific && cap > lastCap() {
return fmt.Errorf("CAP_%s is not supported on the current host", c)
}
isValid = true
@@ -637,8 +637,9 @@ func CapValid(c string, hostSpecific bool) error {
return nil
}

-// LastCap return last cap of system
-func LastCap() capability.Cap {
+// lastCap return last cap of system, and is required to hack around RHEL6.
+// This is an exact copy of "generate/generate.go".lastCap.
+func lastCap() capability.Cap {
last := capability.CAP_LAST_CAP
// hack for RHEL6 which has no /proc/sys/kernel/cap_last_cap
if last == capability.Cap(63) {
--
2.12.2

14 changes: 9 additions & 5 deletions hack/vendor.sh
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,20 @@ clone() {
set -e
(
rm -rf "$vendorPath" && mkdir -p "$vendorPath"
git clone "$cloneURL" "$vendorPath" &>/dev/null
git clone "$cloneURL" "$vendorPath"
cd "$vendorPath"
git checkout --detach "$commit" &>/dev/null
)
git checkout --detach "$commit"
) &>/dev/null
}

# Update everything.
# TODO: Put this in a vendor.conf file or something like that (to be compatible
# with LK4D4/vndr). This setup is a bit unwieldy.
clone github.com/opencontainers/go-digest v1.0.0-rc0
clone github.com/opencontainers/image-spec v1.0.0-rc4
clone github.com/opencontainers/runtime-spec v1.0.0-rc2
clone github.com/opencontainers/runtime-spec v1.0.0-rc5
clone github.com/opencontainers/image-tools 421458f7e467ac86175408693a07da6d29817bf7
clone github.com/opencontainers/runtime-tools b61b44a71dafb8472bbc1e5eb0d68ed9ce8ba6ac
clone github.com/opencontainers/runtime-tools 2f0b832c731dcacc6ad44b03200a3a6a3e14393e
clone github.com/syndtr/gocapability 2c00daeb6c3b45114c80ac44119e7b8801fdd852
clone golang.org/x/crypto b8a2a83acfe6e6770b75de42d5ff4c67596675c0 https://github.com/golang/crypto
clone golang.org/x/sys d75a52659825e75fff6158388dddc6a5b04f9ba5 https://github.com/golang/sys
Expand All @@ -131,5 +131,9 @@ clone github.com/urfave/cli v1.18.1
clone github.com/vbatts/go-mtree 711a89aa4c4a8f148d87eb915456eba8ee7c6a0b
clone golang.org/x/net 45e771701b814666a7eb299e6c7a57d0b1799e91 https://github.com/golang/net

# Apply patches.
self="$(readlink -f "$(dirname "${BASH_SOURCE}")")"
$self/patch.sh

# Clean up the vendor directory.
clean
3 changes: 3 additions & 0 deletions oci/config/convert/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,5 +148,8 @@ func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manif
g.AddAnnotation(key, value)
}

// Remove all seccomp rules.
g.Spec().Linux.Seccomp = nil

return nil
}
14 changes: 7 additions & 7 deletions oci/layer/tar_extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,12 +471,12 @@ func TestUnpackEntryMap(t *testing.T) {
// TODO: Modify this to use subtests once Go 1.7 is in enough places.
func(t *testing.T) {
for _, test := range []struct {
uidMap rspec.IDMapping
gidMap rspec.IDMapping
uidMap rspec.LinuxIDMapping
gidMap rspec.LinuxIDMapping
}{
{rspec.IDMapping{HostID: 0, ContainerID: 0, Size: 100}, rspec.IDMapping{HostID: 0, ContainerID: 0, Size: 100}},
{rspec.IDMapping{HostID: uint32(os.Getuid()), ContainerID: 0, Size: 100}, rspec.IDMapping{HostID: uint32(os.Getgid()), ContainerID: 0, Size: 100}},
{rspec.IDMapping{HostID: uint32(os.Getuid() + 100), ContainerID: 0, Size: 100}, rspec.IDMapping{HostID: uint32(os.Getgid() + 200), ContainerID: 0, Size: 100}},
{rspec.LinuxIDMapping{HostID: 0, ContainerID: 0, Size: 100}, rspec.LinuxIDMapping{HostID: 0, ContainerID: 0, Size: 100}},
{rspec.LinuxIDMapping{HostID: uint32(os.Getuid()), ContainerID: 0, Size: 100}, rspec.LinuxIDMapping{HostID: uint32(os.Getgid()), ContainerID: 0, Size: 100}},
{rspec.LinuxIDMapping{HostID: uint32(os.Getuid() + 100), ContainerID: 0, Size: 100}, rspec.LinuxIDMapping{HostID: uint32(os.Getgid() + 200), ContainerID: 0, Size: 100}},
} {
// Create the files we're going to play with.
dir, err := ioutil.TempDir("", "umoci-TestUnpackEntryMap")
Expand All @@ -499,8 +499,8 @@ func TestUnpackEntryMap(t *testing.T) {
)

te := newTarExtractor(MapOptions{
UIDMappings: []rspec.IDMapping{test.uidMap},
GIDMappings: []rspec.IDMapping{test.gidMap},
UIDMappings: []rspec.LinuxIDMapping{test.uidMap},
GIDMappings: []rspec.LinuxIDMapping{test.gidMap},
})

// Regular file.
Expand Down
4 changes: 2 additions & 2 deletions oci/layer/unpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func UnpackRuntimeJSON(ctx context.Context, engine cas.Engine, configFile io.Wri
// containers. This is done by removing options and other settings that clash
// with unprivileged user namespaces.
func ToRootless(spec *rspec.Spec) {
var namespaces []rspec.Namespace
var namespaces []rspec.LinuxNamespace

// Remove additional groups.
spec.Process.User.AdditionalGids = nil
Expand All @@ -296,7 +296,7 @@ func ToRootless(spec *rspec.Spec) {
}
}
// Add userns to the spec.
namespaces = append(namespaces, rspec.Namespace{
namespaces = append(namespaces, rspec.LinuxNamespace{
Type: rspec.UserNamespace,
})
spec.Linux.Namespaces = namespaces
Expand Down
4 changes: 2 additions & 2 deletions oci/layer/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import (
type MapOptions struct {
// UIDMappings and GIDMappings are the UID and GID mappings to apply when
// packing and unpacking image rootfs layers.
UIDMappings []rspec.IDMapping `json:"uid_mappings"`
GIDMappings []rspec.IDMapping `json:"gid_mappings"`
UIDMappings []rspec.LinuxIDMapping `json:"uid_mappings"`
GIDMappings []rspec.LinuxIDMapping `json:"gid_mappings"`

// Rootless specifies whether any to error out if chown fails.
Rootless bool `json:"rootless"`
Expand Down
18 changes: 9 additions & 9 deletions pkg/idtools/idtools.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
// ToHost translates a remapped container ID to an unmapped host ID using the
// provided ID mapping. If no mapping is provided, then the mapping is a no-op.
// If there is no mapping for the given ID an error is returned.
func ToHost(contID int, idMap []rspec.IDMapping) (int, error) {
func ToHost(contID int, idMap []rspec.LinuxIDMapping) (int, error) {
if idMap == nil {
return contID, nil
}
Expand All @@ -46,7 +46,7 @@ func ToHost(contID int, idMap []rspec.IDMapping) (int, error) {
// container ID using the provided ID mapping. If no mapping is provided, then
// the mapping is a no-op. If there is no mapping for the given ID an error is
// returned.
func ToContainer(hostID int, idMap []rspec.IDMapping) (int, error) {
func ToContainer(hostID int, idMap []rspec.LinuxIDMapping) (int, error) {
if idMap == nil {
return hostID, nil
}
Expand All @@ -61,9 +61,9 @@ func ToContainer(hostID int, idMap []rspec.IDMapping) (int, error) {
}

// ParseMapping takes a mapping string of the form "host:container[:size]" and
// returns the corresponding rspec.IDMapping. An error is returned if not
// returns the corresponding rspec.LinuxIDMapping. An error is returned if not
// enough fields are provided or are otherwise invalid. The default size is 1.
func ParseMapping(spec string) (rspec.IDMapping, error) {
func ParseMapping(spec string) (rspec.LinuxIDMapping, error) {
parts := strings.Split(spec, ":")

var err error
Expand All @@ -72,25 +72,25 @@ func ParseMapping(spec string) (rspec.IDMapping, error) {
case 3:
size, err = strconv.Atoi(parts[2])
if err != nil {
return rspec.IDMapping{}, errors.Wrap(err, "invalid size in mapping")
return rspec.LinuxIDMapping{}, errors.Wrap(err, "invalid size in mapping")
}
case 2:
size = 1
default:
return rspec.IDMapping{}, errors.Errorf("invalid number of fields in mapping '%s': %d", spec, len(parts))
return rspec.LinuxIDMapping{}, errors.Errorf("invalid number of fields in mapping '%s': %d", spec, len(parts))
}

hostID, err = strconv.Atoi(parts[0])
if err != nil {
return rspec.IDMapping{}, errors.Wrap(err, "invalid hostID in mapping")
return rspec.LinuxIDMapping{}, errors.Wrap(err, "invalid hostID in mapping")
}

contID, err = strconv.Atoi(parts[1])
if err != nil {
return rspec.IDMapping{}, errors.Wrap(err, "invalid containerID in mapping")
return rspec.LinuxIDMapping{}, errors.Wrap(err, "invalid containerID in mapping")
}

return rspec.IDMapping{
return rspec.LinuxIDMapping{
HostID: uint32(hostID),
ContainerID: uint32(contID),
Size: uint32(size),
Expand Down
12 changes: 6 additions & 6 deletions pkg/idtools/idtools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
)

func TestToHost(t *testing.T) {
idMap := []rspec.IDMapping{
idMap := []rspec.LinuxIDMapping{
{
HostID: 1337,
ContainerID: 0,
Expand Down Expand Up @@ -74,7 +74,7 @@ func TestToHostNil(t *testing.T) {
}

func TestToHostLarger(t *testing.T) {
idMap := []rspec.IDMapping{
idMap := []rspec.LinuxIDMapping{
{
HostID: 8000,
ContainerID: 0,
Expand Down Expand Up @@ -109,7 +109,7 @@ func TestToHostLarger(t *testing.T) {
}

func TestToHostMultiple(t *testing.T) {
idMap := []rspec.IDMapping{
idMap := []rspec.LinuxIDMapping{
{
HostID: 2222,
ContainerID: 0,
Expand Down Expand Up @@ -157,7 +157,7 @@ func TestToHostMultiple(t *testing.T) {
}

func TestToContainer(t *testing.T) {
idMap := []rspec.IDMapping{
idMap := []rspec.LinuxIDMapping{
{
HostID: 1337,
ContainerID: 0,
Expand Down Expand Up @@ -207,7 +207,7 @@ func TestToContainerNil(t *testing.T) {
}

func TestToContainerLarger(t *testing.T) {
idMap := []rspec.IDMapping{
idMap := []rspec.LinuxIDMapping{
{
HostID: 8000,
ContainerID: 0,
Expand Down Expand Up @@ -242,7 +242,7 @@ func TestToContainerLarger(t *testing.T) {
}

func TestToContainerMultiple(t *testing.T) {
idMap := []rspec.IDMapping{
idMap := []rspec.LinuxIDMapping{
{
HostID: 2222,
ContainerID: 0,
Expand Down
Loading

0 comments on commit 6994e3c

Please sign in to comment.