Skip to content

Commit cad9bbc

Browse files
cypharcyphar
committed
merge branch 'pr-120'
LGTMs: @cyphar Closes #120
2 parents a5b3680 + 11cb2c6 commit cad9bbc

File tree

5 files changed

+70
-207
lines changed

5 files changed

+70
-207
lines changed

‎oci/config/convert/runtime.go‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,31 @@ package convert
1919

2020
import (
2121
"path/filepath"
22+
"sort"
2223
"strings"
2324

2425
"github.com/apex/log"
26+
gspec "github.com/openSUSE/umoci/oci/config/generate"
2527
"github.com/openSUSE/umoci/third_party/user"
2628
ispec "github.com/opencontainers/image-spec/specs-go/v1"
2729
rspec "github.com/opencontainers/runtime-spec/specs-go"
2830
rgen "github.com/opencontainers/runtime-tools/generate"
2931
"github.com/pkg/errors"
3032
)
3133

34+
const (
35+
authorAnnotation = "org.opencontainers.image.author"
36+
createdAnnotation = "org.opencontainers.image.created"
37+
stopSignalAnnotation = "org.opencontainers.image.StopSignal"
38+
)
39+
3240
// ToRuntimeSpec converts the given OCI image configuration to a runtime
3341
// configuration appropriate for use, which is templated on the default
3442
// configuration specified by the OCI runtime-tools. It is equivalent to
3543
// MutateRuntimeSpec("runtime-tools/generate".New(), image).Spec().
36-
func ToRuntimeSpec(rootfs string, image ispec.Image, manifest ispec.Manifest) (rspec.Spec, error) {
44+
func ToRuntimeSpec(rootfs string, image ispec.Image) (rspec.Spec, error) {
3745
g := rgen.New()
38-
if err := MutateRuntimeSpec(g, rootfs, image, manifest); err != nil {
46+
if err := MutateRuntimeSpec(g, rootfs, image); err != nil {
3947
return rspec.Spec{}, err
4048
}
4149
return *g.Spec(), nil
@@ -60,19 +68,17 @@ func parseEnv(env string) (string, string, error) {
6068
// MutateRuntimeSpec mutates a given runtime specification generator with the
6169
// image configuration provided. It returns the original generator, and does
6270
// not modify any fields directly (to allow for chaining).
63-
//
64-
// XXX: This conversion is not actually defined by the image-spec. There is a
65-
// proposal in-the-works though. https://github.com/opencontainers/image-spec/pull/492
66-
func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manifest ispec.Manifest) error {
71+
func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image) error {
6772
if image.OS != "linux" {
6873
return errors.Errorf("unsupported OS: %s", image.OS)
6974
}
7075

7176
// FIXME: We need to figure out if we're modifying an incompatible runtime spec.
7277
//g.SetVersion(rspec.Version)
73-
g.SetPlatformOS(image.OS)
74-
g.SetPlatformArch(image.Architecture)
7578

79+
// Set verbatim fields
80+
g.SetPlatformArch(image.Architecture)
81+
g.SetPlatformOS(image.OS)
7682
g.SetProcessTerminal(true)
7783
g.SetRootPath(filepath.Base(rootfs))
7884
g.SetRootReadonly(false)
@@ -82,7 +88,6 @@ func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manif
8288
g.SetProcessCwd(image.Config.WorkingDir)
8389
}
8490

85-
g.ClearProcessEnv()
8691
for _, env := range image.Config.Env {
8792
name, value, err := parseEnv(env)
8893
if err != nil {
@@ -91,17 +96,23 @@ func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manif
9196
g.AddProcessEnv(name, value)
9297
}
9398

94-
// We don't append to g.Spec().Process.Args because the default is non-zero.
95-
// FIXME: Should we make this instead only append to the pre-existing args
96-
// if Entrypoint != ""? I'm not really sure (Docker doesn't).
9799
args := []string{}
98100
args = append(args, image.Config.Entrypoint...)
99101
args = append(args, image.Config.Cmd...)
100-
if len(args) == 0 {
101-
args = []string{"sh"}
102+
if len(args) > 0 {
103+
g.SetProcessArgs(args)
104+
}
105+
106+
// Set annotations fields
107+
for key, value := range image.Config.Labels {
108+
g.AddAnnotation(key, value)
102109
}
103-
g.SetProcessArgs(args)
110+
g.AddAnnotation(authorAnnotation, image.Author)
111+
g.AddAnnotation(createdAnnotation, image.Created.Format(gspec.ISO8601))
112+
// FIXME: currently not supported! Uncomment when moving to image-spec rc5.
113+
// g.AddAnnotation(stopSignalAnnotation, image.Config.StopSignal)
104114

115+
// Set parsed fields
105116
// Get the *actual* uid and gid of the user. If the image doesn't contain
106117
// an /etc/passwd or /etc/group file then GetExecUserPath will just do a
107118
// numerical parsing.
@@ -124,30 +135,27 @@ func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manif
124135
g.SetProcessUID(uint32(execUser.Uid))
125136
g.SetProcessGID(uint32(execUser.Gid))
126137
g.ClearProcessAdditionalGids()
138+
127139
for _, gid := range execUser.Sgids {
128140
g.AddProcessAdditionalGid(uint32(gid))
129141
}
130142
if execUser.Home != "" {
131143
g.AddProcessEnv("HOME", execUser.Home)
132144
}
133145

146+
// Set optional fields
147+
var ports []string
148+
for port := range image.Config.ExposedPorts {
149+
ports = append(ports, port)
150+
}
151+
sort.Strings(ports)
152+
g.AddAnnotation("org.opencontainers.image.exposedPorts", strings.Join(ports, ","))
153+
134154
for vol := range image.Config.Volumes {
135155
// XXX: This is _fine_ but might cause some issues in the future.
136156
g.AddTmpfsMount(vol, []string{"rw"})
137157
}
138158

139-
// XXX: This order-of-addition is actually not codified in the spec.
140-
// However, this will be sorted once I write a proposal for it.
141-
// opencontainers/image-spec#479
142-
143-
g.ClearAnnotations()
144-
for key, value := range image.Config.Labels {
145-
g.AddAnnotation(key, value)
146-
}
147-
for key, value := range manifest.Annotations {
148-
g.AddAnnotation(key, value)
149-
}
150-
151159
// Remove all seccomp rules.
152160
g.Spec().Linux.Seccomp = nil
153161

‎oci/layer/unpack.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func UnpackRuntimeJSON(ctx context.Context, engine cas.Engine, configFile io.Wri
249249
}
250250

251251
g := rgen.New()
252-
if err := iconv.MutateRuntimeSpec(g, rootfs, config, manifest); err != nil {
252+
if err := iconv.MutateRuntimeSpec(g, rootfs, config); err != nil {
253253
return errors.Wrap(err, "generate config.json")
254254
}
255255

‎test/config.bats‎

Lines changed: 23 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,22 @@ function teardown() {
358358
[ "$status" -eq 0 ]
359359
bundle-verify "$BUNDLE"
360360

361-
# Make sure that only $HOME was set.
362-
sane_run jq -SMr '.process.env[]' "$BUNDLE/config.json"
361+
# Make sure that HOME, PATH and TERM are set
362+
sane_run jq -SMr '.process.env | length' "$BUNDLE/config.json"
363+
[ "$status" -eq 0 ]
364+
[[ "$output" == 3 ]]
365+
366+
sane_run jq -SMr '.process.env[] | startswith("HOME=")' "$BUNDLE/config.json"
367+
[ "$status" -eq 0 ]
368+
[[ "${lines[*]}" == *"true"* ]]
369+
370+
sane_run jq -SMr '.process.env[] | startswith("PATH=")' "$BUNDLE/config.json"
363371
[ "$status" -eq 0 ]
364-
[[ "${lines[0]}" == *"HOME="* ]]
365-
[ "${#lines[@]}" -eq 1 ]
372+
[[ "${lines[*]}" == *"true"* ]]
373+
374+
sane_run jq -SMr '.process.env[] | startswith("TERM=")' "$BUNDLE/config.json"
375+
[ "$status" -eq 0 ]
376+
[[ "${lines[*]}" == *"true"* ]]
366377

367378
image-verify "${IMAGE}"
368379
}
@@ -705,13 +716,14 @@ function teardown() {
705716
image-verify "${IMAGE}"
706717
}
707718

708-
@test "umoci config --manifest.annotation" {
719+
@test "umoci config --config.exposedports" {
709720
BUNDLE="$(setup_tmpdir)"
710721

711722
# Modify none of the configuration.
712723
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
713-
--clear=config.labels --clear=manifest.annotations \
714-
--manifest.annotation="com.cyphar.test=1" --manifest.annotation="com.cyphar.empty="
724+
--config.exposedports="2000" \
725+
--config.exposedports="8080/tcp" \
726+
--config.exposedports="1234/tcp"
715727
[ "$status" -eq 0 ]
716728
image-verify "${IMAGE}"
717729

@@ -720,82 +732,11 @@ function teardown() {
720732
[ "$status" -eq 0 ]
721733
bundle-verify "$BUNDLE"
722734

723-
sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
735+
sane_run jq -SMr '.annotations["org.opencontainers.image.exposedPorts"] | split(",")[]' "$BUNDLE/config.json"
724736
[ "$status" -eq 0 ]
725-
[[ "$output" == "1" ]]
726-
727-
sane_run jq -SMr '.annotations["com.cyphar.empty"]' "$BUNDLE/config.json"
728-
[ "$status" -eq 0 ]
729-
[[ "$output" == "" ]]
730-
731-
image-verify "${IMAGE}"
732-
}
733-
734-
@test "umoci config --config.label --manifest.annotation" {
735-
BUNDLE="$(setup_tmpdir)"
736-
737-
# Modify none of the configuration.
738-
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
739-
--clear=config.labels --clear=manifest.annotations \
740-
--config.label="com.cyphar.label_test={another value}" --config.label="com.cyphar.label_empty=" \
741-
--manifest.annotation="com.cyphar.manifest_test= another valu=e " --manifest.annotation="com.cyphar.manifest_empty="
742-
[ "$status" -eq 0 ]
743-
image-verify "${IMAGE}"
744-
745-
# Unpack the image again.
746-
umoci unpack --image "${IMAGE}:${TAG}-new" "$BUNDLE"
747-
[ "$status" -eq 0 ]
748-
bundle-verify "$BUNDLE"
749-
750-
sane_run jq -SMr '.annotations["com.cyphar.label_test"]' "$BUNDLE/config.json"
751-
[ "$status" -eq 0 ]
752-
[[ "$output" == "{another value}" ]]
753-
754-
sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
755-
[ "$status" -eq 0 ]
756-
[[ "$output" == "" ]]
757-
758-
sane_run jq -SMr '.annotations["com.cyphar.manifest_test"]' "$BUNDLE/config.json"
759-
[ "$status" -eq 0 ]
760-
[[ "$output" == " another valu=e " ]]
761-
762-
sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
763-
[ "$status" -eq 0 ]
764-
[[ "$output" == "" ]]
765-
766-
image-verify "${IMAGE}"
767-
}
768-
769-
# XXX: This is currently not in any spec. So we'll just test our own behaviour
770-
# here and we can fix it after opencontainers/image-spec#479 is fixed.
771-
@test "umoci config --config.label --manifest.annotation [clobber]" {
772-
BUNDLE="$(setup_tmpdir)"
773-
774-
# Modify none of the configuration.
775-
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
776-
--clear=config.labels --clear=manifest.annotations \
777-
--config.label="com.cyphar.test= this_is SOEM VALUE" --config.label="com.cyphar.label_empty=" \
778-
--manifest.annotation="com.cyphar.test== __ --a completely different VALuE " --manifest.annotation="com.cyphar.manifest_empty="
779-
[ "$status" -eq 0 ]
780-
image-verify "${IMAGE}"
781-
782-
# Unpack the image again.
783-
umoci unpack --image "${IMAGE}:${TAG}-new" "$BUNDLE"
784-
[ "$status" -eq 0 ]
785-
bundle-verify "$BUNDLE"
786-
787-
# Manifest beats config.
788-
sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
789-
[ "$status" -eq 0 ]
790-
[[ "$output" == "= __ --a completely different VALuE " ]]
791-
792-
sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
793-
[ "$status" -eq 0 ]
794-
[[ "$output" == "" ]]
795-
796-
sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
797-
[ "$status" -eq 0 ]
798-
[[ "$output" == "" ]]
737+
[[ "${lines[0]}" == "1234/tcp" ]]
738+
[[ "${lines[1]}" == "2000" ]]
739+
[[ "${lines[2]}" == "8080/tcp" ]]
799740

800741
image-verify "${IMAGE}"
801742
}

‎test/raw-config.bats‎

Lines changed: 10 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -301,10 +301,18 @@ function teardown() {
301301
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
302302
[ "$status" -eq 0 ]
303303

304-
# Make sure that nothing was set.
304+
# Make sure that PATH and TERM are set.
305305
sane_run jq -SMr '.process.env | length' "$BUNDLE/config.json"
306306
[ "$status" -eq 0 ]
307-
[[ "$output" == 0 ]]
307+
[[ "$output" == 2 ]]
308+
309+
sane_run jq -SMr '.process.env[] | startswith("PATH=")' "$BUNDLE/config.json"
310+
[ "$status" -eq 0 ]
311+
[[ "${lines[*]}" == *"true"* ]]
312+
313+
sane_run jq -SMr '.process.env[] | startswith("TERM=")' "$BUNDLE/config.json"
314+
[ "$status" -eq 0 ]
315+
[[ "${lines[*]}" == *"true"* ]]
308316

309317
image-verify "${IMAGE}"
310318
}
@@ -563,97 +571,3 @@ function teardown() {
563571

564572
image-verify "${IMAGE}"
565573
}
566-
567-
# XXX: This will probably become non-compliant with the spec once the whole
568-
# conversion logic is defined.
569-
@test "umoci raw runtime-config --manifest.annotation" {
570-
BUNDLE="$(setup_tmpdir)"
571-
572-
# Modify none of the configuration.
573-
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
574-
--clear=config.labels --clear=manifest.annotations \
575-
--manifest.annotation="com.cyphar.test=1" --manifest.annotation="com.cyphar.empty="
576-
[ "$status" -eq 0 ]
577-
image-verify "${IMAGE}"
578-
579-
# Unpack the image again.
580-
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
581-
[ "$status" -eq 0 ]
582-
583-
sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
584-
[ "$status" -eq 0 ]
585-
[[ "$output" == "1" ]]
586-
587-
sane_run jq -SMr '.annotations["com.cyphar.empty"]' "$BUNDLE/config.json"
588-
[ "$status" -eq 0 ]
589-
[[ "$output" == "" ]]
590-
591-
image-verify "${IMAGE}"
592-
}
593-
594-
@test "umoci raw runtime-config --config.label --manifest.annotation" {
595-
BUNDLE="$(setup_tmpdir)"
596-
597-
# Modify none of the configuration.
598-
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
599-
--clear=config.labels --clear=manifest.annotations \
600-
--config.label="com.cyphar.label_test={another value}" --config.label="com.cyphar.label_empty=" \
601-
--manifest.annotation="com.cyphar.manifest_test= another valu=e " --manifest.annotation="com.cyphar.manifest_empty="
602-
[ "$status" -eq 0 ]
603-
image-verify "${IMAGE}"
604-
605-
# Unpack the image again.
606-
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
607-
[ "$status" -eq 0 ]
608-
609-
sane_run jq -SMr '.annotations["com.cyphar.label_test"]' "$BUNDLE/config.json"
610-
[ "$status" -eq 0 ]
611-
[[ "$output" == "{another value}" ]]
612-
613-
sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
614-
[ "$status" -eq 0 ]
615-
[[ "$output" == "" ]]
616-
617-
sane_run jq -SMr '.annotations["com.cyphar.manifest_test"]' "$BUNDLE/config.json"
618-
[ "$status" -eq 0 ]
619-
[[ "$output" == " another valu=e " ]]
620-
621-
sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
622-
[ "$status" -eq 0 ]
623-
[[ "$output" == "" ]]
624-
625-
image-verify "${IMAGE}"
626-
}
627-
628-
# XXX: This is currently not in any spec. So we'll just test our own behaviour
629-
# here and we can fix it after opencontainers/image-spec#479 is fixed.
630-
@test "umoci raw runtime-config --config.label --manifest.annotation [clobber]" {
631-
BUNDLE="$(setup_tmpdir)"
632-
633-
# Modify none of the configuration.
634-
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
635-
--clear=config.labels --clear=manifest.annotations \
636-
--config.label="com.cyphar.test= this_is SOEM VALUE" --config.label="com.cyphar.label_empty=" \
637-
--manifest.annotation="com.cyphar.test== __ --a completely different VALuE " --manifest.annotation="com.cyphar.manifest_empty="
638-
[ "$status" -eq 0 ]
639-
image-verify "${IMAGE}"
640-
641-
# Unpack the image again.
642-
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
643-
[ "$status" -eq 0 ]
644-
645-
# Manifest beats config.
646-
sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
647-
[ "$status" -eq 0 ]
648-
[[ "$output" == "= __ --a completely different VALuE " ]]
649-
650-
sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
651-
[ "$status" -eq 0 ]
652-
[[ "$output" == "" ]]
653-
654-
sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
655-
[ "$status" -eq 0 ]
656-
[[ "$output" == "" ]]
657-
658-
image-verify "${IMAGE}"
659-
}

0 commit comments

Comments
 (0)