From 527ebbde444387a040d8797ad7d22378273fb630 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 6 Apr 2017 11:54:42 -0700 Subject: [PATCH 1/4] specs-go/round_trip_test: Add round-trip testing for the config To make sure the Go structs can accurately transport known-good configurations. Use Docker's canonical JSON implementation so we don't have to worry about false-positives due to JSON whitespace, key order, etc. Signed-off-by: W. Trevor King --- specs-go/round_trip_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 specs-go/round_trip_test.go diff --git a/specs-go/round_trip_test.go b/specs-go/round_trip_test.go new file mode 100644 index 000000000..722a0174f --- /dev/null +++ b/specs-go/round_trip_test.go @@ -0,0 +1,36 @@ +package specs_test + +import ( + "bytes" + "fmt" + "testing" + + "github.com/docker/go/canonical/json" + "github.com/opencontainers/runtime-spec/specs-go" +) + +func TestConfigRoundTrip(t *testing.T) { + for i, configString := range []string{ + // canonical version of the config.md example + `{"ociVersion":"0.5.0-dev","platform":{"os":"linux","arch":"amd64"},"process":{"terminal":true,"consoleSize":{"height":0,"width":0},"user":{"uid":1,"gid":1,"additionalGids":[5,6]},"args":["sh"],"env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin","TERM=xterm"],"cwd":"/","capabilities":{"bounding":["CAP_AUDIT_WRITE","CAP_KILL","CAP_NET_BIND_SERVICE"],"effective":["CAP_AUDIT_WRITE","CAP_KILL"],"inheritable":["CAP_AUDIT_WRITE","CAP_KILL","CAP_NET_BIND_SERVICE"],"permitted":["CAP_AUDIT_WRITE","CAP_KILL","CAP_NET_BIND_SERVICE"],"ambient":["CAP_NET_BIND_SERVICE"]},"rlimits":[{"type":"RLIMIT_CORE","hard":1024,"soft":1024},{"type":"RLIMIT_NOFILE","hard":1024,"soft":1024}],"noNewPrivileges":true,"apparmorProfile":"acme_secure_profile","selinuxLabel":"system_u:system_r:svirt_lxc_net_t:s0:c124,c675"},"root":{"path":"rootfs","readonly":true},"hostname":"slartibartfast","mounts":[{"destination":"/proc","type":"proc","source":"proc"},{"destination":"/dev","type":"tmpfs","source":"tmpfs","options":["nosuid","strictatime","mode=755","size=65536k"]},{"destination":"/dev/pts","type":"devpts","source":"devpts","options":["nosuid","noexec","newinstance","ptmxmode=0666","mode=0620","gid=5"]},{"destination":"/dev/shm","type":"tmpfs","source":"shm","options":["nosuid","noexec","nodev","mode=1777","size=65536k"]},{"destination":"/dev/mqueue","type":"mqueue","source":"mqueue","options":["nosuid","noexec","nodev"]},{"destination":"/sys","type":"sysfs","source":"sysfs","options":["nosuid","noexec","nodev"]},{"destination":"/sys/fs/cgroup","type":"cgroup","source":"cgroup","options":["nosuid","noexec","nodev","relatime","ro"]}],"hooks":{"prestart":[{"path":"/usr/bin/fix-mounts","args":["fix-mounts","arg1","arg2"],"env":["key1=value1"]},{"path":"/usr/bin/setup-network"}],"poststart":[{"path":"/usr/bin/notify-start","timeout":5}],"poststop":[{"path":"/usr/sbin/cleanup.sh","args":["cleanup.sh","-f"]}]},"annotations":{"com.example.key1":"value1","com.example.key2":"value2"},"linux":{"uidMappings":[{"hostID":1000,"containerID":0,"size":32000}],"gidMappings":[{"hostID":1000,"containerID":0,"size":32000}],"sysctl":{"net.core.somaxconn":"256","net.ipv4.ip_forward":"1"},"resources":{"devices":[{"allow":false,"access":"rwm"},{"allow":true,"type":"c","major":10,"minor":229,"access":"rw"},{"allow":true,"type":"b","major":8,"minor":0,"access":"r"}],"disableOOMKiller":false,"oomScoreAdj":100,"memory":{"limit":536870912,"reservation":536870912,"swap":536870912,"kernel":0,"kernelTCP":0,"swappiness":0},"cpu":{"shares":1024,"quota":1000000,"period":500000,"realtimeRuntime":950000,"realtimePeriod":1000000,"cpus":"2-3","mems":"0-7"},"pids":{"limit":32771},"blockIO":{"blkioWeight":10,"blkioLeafWeight":10,"blkioWeightDevice":[{"major":8,"minor":0,"weight":500,"leafWeight":300},{"major":8,"minor":16,"weight":500}],"blkioThrottleReadBpsDevice":[{"major":8,"minor":0,"rate":600}],"blkioThrottleWriteIOPSDevice":[{"major":8,"minor":16,"rate":300}]},"hugepageLimits":[{"pageSize":"2MB","limit":9223372036854772000}],"network":{"classID":1048577,"priorities":[{"name":"eth0","priority":500},{"name":"eth1","priority":1000}]}},"cgroupsPath":"/myRuntime/myContainer","namespaces":[{"type":"pid"},{"type":"network"},{"type":"ipc"},{"type":"uts"},{"type":"mount"},{"type":"user"},{"type":"cgroup"}],"devices":[{"path":"/dev/fuse","type":"c","major":10,"minor":229,"fileMode":438,"uid":0,"gid":0},{"path":"/dev/sda","type":"b","major":8,"minor":0,"fileMode":432,"uid":0,"gid":0}],"seccomp":{"defaultAction":"SCMP_ACT_ALLOW","architectures":["SCMP_ARCH_X86","SCMP_ARCH_X32"],"syscalls":[{"names":["getcwd","chmod"],"action":"SCMP_ACT_ERRNO","args":null}]},"rootfsPropagation":"slave","maskedPaths":["/proc/kcore","/proc/latency_stats","/proc/timer_stats","/proc/sched_debug"],"readonlyPaths":["/proc/asound","/proc/bus","/proc/fs","/proc/irq","/proc/sys","/proc/sysrq-trigger"],"mountLabel":"system_u:object_r:svirt_sandbox_file_t:s0:c715,c811"}}`, + + // minimal Linux example (removing optional fields from the config.md example) + `{"ociVersion":"1.0.0","platform":{"os":"linux","arch":"amd64"},"process":{"user":{"uid":1,"gid":1},"args":["sh"],"cwd":"/"},"root":{"path":"rootfs"}}`, + } { + t.Run(fmt.Sprintf("config %d", i), func(t *testing.T) { + configBytes := []byte(configString) + var configStruct specs.Spec + err := json.NewDecoder(bytes.NewReader(configBytes)).Decode(&configStruct) + if err != nil { + t.Fatalf("failed to decode: %v", err) + } + outBytes, err := json.Marshal(configStruct) + if err != nil { + t.Fatalf("failed to encode: %v", err) + } + if bytes.Compare(configBytes, outBytes) != 0 { + t.Fatalf("failed to round-trip:\n%s", string(outBytes)) + } + }) + } +} From 43e73690187f6a41f7ad0bd9b33f015156a2eb5d Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 6 Apr 2017 13:21:25 -0700 Subject: [PATCH 2/4] Makefile: Add Go test for schema-go And run them from Travis. Also replace some old Travis test commands with a single call to 'make test'. Now we die on the first error (e.g. we don't run the Go tests if lint fails), but I don't think that's a big problem, and this way we don't have to maintain two parallel lists of test targets. Signed-off-by: W. Trevor King --- .travis.yml | 4 +--- Makefile | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7c2de7e71..abe45bfa3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,9 +17,7 @@ install: true script: - env | grep TRAVIS_ - - make .govet - - make .golint - echo "${TRAVIS_COMMIT_RANGE} -> ${TRAVIS_COMMIT_RANGE/.../..} (travis-ci/travis-ci#4596)" - - TRAVIS_COMMIT_RANGE="${TRAVIS_COMMIT_RANGE/.../..}" make .gitvalidation + - TRAVIS_COMMIT_RANGE="${TRAVIS_COMMIT_RANGE/.../..}" make test - make docs - make -C schema test diff --git a/Makefile b/Makefile index 770834b7c..d50fc9e3e 100644 --- a/Makefile +++ b/Makefile @@ -55,7 +55,14 @@ HOST_GOLANG_VERSION = $(shell go version | cut -d ' ' -f3 | cut -c 3-) # this variable is used like a function. First arg is the minimum version, Second arg is the version to be checked. ALLOWED_GO_VERSION = $(shell test '$(shell /bin/echo -e "$(1)\n$(2)" | sort -V | head -n1)' = '$(1)' && echo 'true') -test: .govet .golint .gitvalidation +TESTS := .govet .golint .gitvalidation + +# (*testing.T).Run is new in 1.7, https://golang.org/doc/go1.7#testing +ifeq ($(call ALLOWED_GO_VERSION,1.7,$(HOST_GOLANG_VERSION)),true) + TESTS += .specs-go +endif + +test: $(TESTS) .govet: go vet -x ./... @@ -77,7 +84,10 @@ else git-validation -v -run DCO,short-subject,dangling-whitespace -range $(EPOCH_TEST_COMMIT)..HEAD endif -install.tools: .install.golint .install.gitvalidation +.specs-go: + go test ./specs-go + +install.tools: .install.golint .install.gitvalidation .install.canonicaljson # golint does not even build for Date: Thu, 6 Apr 2017 13:27:45 -0700 Subject: [PATCH 3/4] specs-go/round_trip_test: Add a minimal Windows config process.user.username is not strictly required yet, but it is intended to be [1], and it doesn't seem worth making Process.User a pointer in the meantime. [1]: https://github.com/opencontainers/runtime-spec/issues/618#issuecomment-277105273 Signed-off-by: W. Trevor King --- specs-go/round_trip_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/specs-go/round_trip_test.go b/specs-go/round_trip_test.go index 722a0174f..f60757573 100644 --- a/specs-go/round_trip_test.go +++ b/specs-go/round_trip_test.go @@ -16,6 +16,9 @@ func TestConfigRoundTrip(t *testing.T) { // minimal Linux example (removing optional fields from the config.md example) `{"ociVersion":"1.0.0","platform":{"os":"linux","arch":"amd64"},"process":{"user":{"uid":1,"gid":1},"args":["sh"],"cwd":"/"},"root":{"path":"rootfs"}}`, + + // minimal Windows example + `{"ociVersion":"1.0.0","platform":{"os":"windows","arch":"amd64"},"process":{"user":{"username":"containeradministrator"},"args":["sh"],"cwd":"C:\\"},"root":{"path":"rootfs"}}`, } { t.Run(fmt.Sprintf("config %d", i), func(t *testing.T) { configBytes := []byte(configString) From 05415d469dec65d51fe269112e9a3bff50d5c9ec Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 6 Apr 2017 13:36:53 -0700 Subject: [PATCH 4/4] specs-go/config: Make User.UID and User.GID pointers Avoiding: $ go test ./specs-go --- FAIL: TestConfigRoundTrip (0.00s) --- FAIL: TestConfigRoundTrip/config_2 (0.00s) round_trip_test.go:35: failed to round-trip: {"ociVersion":"1.0.0","platform":{"os":"windows","arch":"amd64"},"process":{"user":{"uid":0,"gid":0,"username":"containeradministrator"},"args":["sh"],"cwd":"C:\\"},"root":{"path":"rootfs"}} FAIL FAIL github.com/opencontainers/runtime-spec/specs-go 0.003s We can't just add omitempty (without also making them pointers), because 0 is a meaningful value for both properties. Signed-off-by: W. Trevor King --- specs-go/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs-go/config.go b/specs-go/config.go index f2016b04b..82964628b 100644 --- a/specs-go/config.go +++ b/specs-go/config.go @@ -84,9 +84,9 @@ type Box struct { // User specifies specific user (and group) information for the container process. type User struct { // UID is the user id. - UID uint32 `json:"uid" platform:"linux,solaris"` + UID *uint32 `json:"uid,omitempty" platform:"linux,solaris"` // GID is the group id. - GID uint32 `json:"gid" platform:"linux,solaris"` + GID *uint32 `json:"gid,omitempty" platform:"linux,solaris"` // AdditionalGids are additional group ids set for the container's process. AdditionalGids []uint32 `json:"additionalGids,omitempty" platform:"linux,solaris"` // Username is the user name.