Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate: match spec conversion #120

Merged
merged 1 commit into from
May 17, 2017
Merged

generate: match spec conversion #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 35 additions & 27 deletions oci/config/convert/runtime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,31 @@ package convert

import (
"path/filepath"
"sort"
"strings"

"github.com/apex/log"
gspec "github.com/openSUSE/umoci/oci/config/generate"
"github.com/openSUSE/umoci/third_party/user"
ispec "github.com/opencontainers/image-spec/specs-go/v1"
rspec "github.com/opencontainers/runtime-spec/specs-go"
rgen "github.com/opencontainers/runtime-tools/generate"
"github.com/pkg/errors"
)

const (
authorAnnotation = "org.opencontainers.image.author"
createdAnnotation = "org.opencontainers.image.created"
stopSignalAnnotation = "org.opencontainers.image.StopSignal"
)

// ToRuntimeSpec converts the given OCI image configuration to a runtime
// configuration appropriate for use, which is templated on the default
// configuration specified by the OCI runtime-tools. It is equivalent to
// MutateRuntimeSpec("runtime-tools/generate".New(), image).Spec().
func ToRuntimeSpec(rootfs string, image ispec.Image, manifest ispec.Manifest) (rspec.Spec, error) {
func ToRuntimeSpec(rootfs string, image ispec.Image) (rspec.Spec, error) {
g := rgen.New()
if err := MutateRuntimeSpec(g, rootfs, image, manifest); err != nil {
if err := MutateRuntimeSpec(g, rootfs, image); err != nil {
return rspec.Spec{}, err
}
return *g.Spec(), nil
Expand All @@ -60,19 +68,17 @@ func parseEnv(env string) (string, string, error) {
// MutateRuntimeSpec mutates a given runtime specification generator with the
// image configuration provided. It returns the original generator, and does
// not modify any fields directly (to allow for chaining).
//
// XXX: This conversion is not actually defined by the image-spec. There is a
// proposal in-the-works though. https://github.com/opencontainers/image-spec/pull/492
func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manifest ispec.Manifest) error {
func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image) error {
if image.OS != "linux" {
return errors.Errorf("unsupported OS: %s", image.OS)
}

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

// Set verbatim fields
g.SetPlatformArch(image.Architecture)
g.SetPlatformOS(image.OS)
g.SetProcessTerminal(true)
g.SetRootPath(filepath.Base(rootfs))
g.SetRootReadonly(false)
Expand All @@ -82,7 +88,6 @@ func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manif
g.SetProcessCwd(image.Config.WorkingDir)
}

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

// We don't append to g.Spec().Process.Args because the default is non-zero.
// FIXME: Should we make this instead only append to the pre-existing args
// if Entrypoint != ""? I'm not really sure (Docker doesn't).
args := []string{}
args = append(args, image.Config.Entrypoint...)
args = append(args, image.Config.Cmd...)
if len(args) == 0 {
args = []string{"sh"}
if len(args) > 0 {
g.SetProcessArgs(args)
}

// Set annotations fields
for key, value := range image.Config.Labels {
g.AddAnnotation(key, value)
}
g.SetProcessArgs(args)
g.AddAnnotation(authorAnnotation, image.Author)
g.AddAnnotation(createdAnnotation, image.Created.Format(gspec.ISO8601))
// FIXME: currently not supported! Uncomment when moving to image-spec rc5.
// g.AddAnnotation(stopSignalAnnotation, image.Config.StopSignal)

// Set parsed fields
// Get the *actual* uid and gid of the user. If the image doesn't contain
// an /etc/passwd or /etc/group file then GetExecUserPath will just do a
// numerical parsing.
Expand All @@ -124,30 +135,27 @@ func MutateRuntimeSpec(g rgen.Generator, rootfs string, image ispec.Image, manif
g.SetProcessUID(uint32(execUser.Uid))
g.SetProcessGID(uint32(execUser.Gid))
g.ClearProcessAdditionalGids()

for _, gid := range execUser.Sgids {
g.AddProcessAdditionalGid(uint32(gid))
}
if execUser.Home != "" {
g.AddProcessEnv("HOME", execUser.Home)
}

// Set optional fields
var ports []string
for port := range image.Config.ExposedPorts {
ports = append(ports, port)
}
Copy link
Member

@cyphar cyphar May 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed you'll need to sort ports after this because iteration over Go maps is random. You can use the sort package with sort.StringSlice (https://golang.org/pkg/sort/#StringSlice).

Though in future it would be nice if this was hidden behind the github.com/openSUSE/umoci/oci/config/generate API (we can do that after this is merged though).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func (g *Generator) ConfigExposedPortsToString() string { ... } ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer it to return the []string (the fact that internally it's implemented as a map is something that users shouldn't care about, but the fact it's a list of ports is important for implementations). In any case that's not important atm.

sort.Strings(ports)
g.AddAnnotation("org.opencontainers.image.exposedPorts", strings.Join(ports, ","))

for vol := range image.Config.Volumes {
// XXX: This is _fine_ but might cause some issues in the future.
g.AddTmpfsMount(vol, []string{"rw"})
}

// XXX: This order-of-addition is actually not codified in the spec.
// However, this will be sorted once I write a proposal for it.
// opencontainers/image-spec#479

g.ClearAnnotations()
for key, value := range image.Config.Labels {
g.AddAnnotation(key, value)
}
for key, value := range manifest.Annotations {
g.AddAnnotation(key, value)
}

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

Expand Down
2 changes: 1 addition & 1 deletion oci/layer/unpack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func UnpackRuntimeJSON(ctx context.Context, engine cas.Engine, configFile io.Wri
}

g := rgen.New()
if err := iconv.MutateRuntimeSpec(g, rootfs, config, manifest); err != nil {
if err := iconv.MutateRuntimeSpec(g, rootfs, config); err != nil {
return errors.Wrap(err, "generate config.json")
}

Expand Down
105 changes: 23 additions & 82 deletions test/config.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,22 @@ function teardown() {
[ "$status" -eq 0 ]
bundle-verify "$BUNDLE"

# Make sure that only $HOME was set.
sane_run jq -SMr '.process.env[]' "$BUNDLE/config.json"
# Make sure that HOME, PATH and TERM are set
sane_run jq -SMr '.process.env | length' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == 3 ]]

sane_run jq -SMr '.process.env[] | startswith("HOME=")' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "${lines[*]}" == *"true"* ]]

sane_run jq -SMr '.process.env[] | startswith("PATH=")' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "${lines[0]}" == *"HOME="* ]]
[ "${#lines[@]}" -eq 1 ]
[[ "${lines[*]}" == *"true"* ]]

sane_run jq -SMr '.process.env[] | startswith("TERM=")' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "${lines[*]}" == *"true"* ]]

image-verify "${IMAGE}"
}
Expand Down Expand Up @@ -705,13 +716,14 @@ function teardown() {
image-verify "${IMAGE}"
}

@test "umoci config --manifest.annotation" {
@test "umoci config --config.exposedports" {
BUNDLE="$(setup_tmpdir)"

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

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

sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
sane_run jq -SMr '.annotations["org.opencontainers.image.exposedPorts"] | split(",")[]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "1" ]]

sane_run jq -SMr '.annotations["com.cyphar.empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

image-verify "${IMAGE}"
}

@test "umoci config --config.label --manifest.annotation" {
BUNDLE="$(setup_tmpdir)"

# Modify none of the configuration.
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
--clear=config.labels --clear=manifest.annotations \
--config.label="com.cyphar.label_test={another value}" --config.label="com.cyphar.label_empty=" \
--manifest.annotation="com.cyphar.manifest_test= another valu=e " --manifest.annotation="com.cyphar.manifest_empty="
[ "$status" -eq 0 ]
image-verify "${IMAGE}"

# Unpack the image again.
umoci unpack --image "${IMAGE}:${TAG}-new" "$BUNDLE"
[ "$status" -eq 0 ]
bundle-verify "$BUNDLE"

sane_run jq -SMr '.annotations["com.cyphar.label_test"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "{another value}" ]]

sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

sane_run jq -SMr '.annotations["com.cyphar.manifest_test"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == " another valu=e " ]]

sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

image-verify "${IMAGE}"
}

# XXX: This is currently not in any spec. So we'll just test our own behaviour
# here and we can fix it after opencontainers/image-spec#479 is fixed.
@test "umoci config --config.label --manifest.annotation [clobber]" {
BUNDLE="$(setup_tmpdir)"

# Modify none of the configuration.
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
--clear=config.labels --clear=manifest.annotations \
--config.label="com.cyphar.test= this_is SOEM VALUE" --config.label="com.cyphar.label_empty=" \
--manifest.annotation="com.cyphar.test== __ --a completely different VALuE " --manifest.annotation="com.cyphar.manifest_empty="
[ "$status" -eq 0 ]
image-verify "${IMAGE}"

# Unpack the image again.
umoci unpack --image "${IMAGE}:${TAG}-new" "$BUNDLE"
[ "$status" -eq 0 ]
bundle-verify "$BUNDLE"

# Manifest beats config.
sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "= __ --a completely different VALuE " ]]

sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]
[[ "${lines[0]}" == "1234/tcp" ]]
[[ "${lines[1]}" == "2000" ]]
[[ "${lines[2]}" == "8080/tcp" ]]

image-verify "${IMAGE}"
}
106 changes: 10 additions & 96 deletions test/raw-config.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,18 @@ function teardown() {
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
[ "$status" -eq 0 ]

# Make sure that nothing was set.
# Make sure that PATH and TERM are set.
sane_run jq -SMr '.process.env | length' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == 0 ]]
[[ "$output" == 2 ]]

sane_run jq -SMr '.process.env[] | startswith("PATH=")' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "${lines[*]}" == *"true"* ]]

sane_run jq -SMr '.process.env[] | startswith("TERM=")' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "${lines[*]}" == *"true"* ]]

image-verify "${IMAGE}"
}
Expand Down Expand Up @@ -563,97 +571,3 @@ function teardown() {

image-verify "${IMAGE}"
}

# XXX: This will probably become non-compliant with the spec once the whole
# conversion logic is defined.
@test "umoci raw runtime-config --manifest.annotation" {
BUNDLE="$(setup_tmpdir)"

# Modify none of the configuration.
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
--clear=config.labels --clear=manifest.annotations \
--manifest.annotation="com.cyphar.test=1" --manifest.annotation="com.cyphar.empty="
[ "$status" -eq 0 ]
image-verify "${IMAGE}"

# Unpack the image again.
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
[ "$status" -eq 0 ]

sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "1" ]]

sane_run jq -SMr '.annotations["com.cyphar.empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

image-verify "${IMAGE}"
}

@test "umoci raw runtime-config --config.label --manifest.annotation" {
BUNDLE="$(setup_tmpdir)"

# Modify none of the configuration.
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
--clear=config.labels --clear=manifest.annotations \
--config.label="com.cyphar.label_test={another value}" --config.label="com.cyphar.label_empty=" \
--manifest.annotation="com.cyphar.manifest_test= another valu=e " --manifest.annotation="com.cyphar.manifest_empty="
[ "$status" -eq 0 ]
image-verify "${IMAGE}"

# Unpack the image again.
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
[ "$status" -eq 0 ]

sane_run jq -SMr '.annotations["com.cyphar.label_test"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "{another value}" ]]

sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

sane_run jq -SMr '.annotations["com.cyphar.manifest_test"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == " another valu=e " ]]

sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

image-verify "${IMAGE}"
}

# XXX: This is currently not in any spec. So we'll just test our own behaviour
# here and we can fix it after opencontainers/image-spec#479 is fixed.
@test "umoci raw runtime-config --config.label --manifest.annotation [clobber]" {
BUNDLE="$(setup_tmpdir)"

# Modify none of the configuration.
umoci config --image "${IMAGE}:${TAG}" --tag "${TAG}-new" \
--clear=config.labels --clear=manifest.annotations \
--config.label="com.cyphar.test= this_is SOEM VALUE" --config.label="com.cyphar.label_empty=" \
--manifest.annotation="com.cyphar.test== __ --a completely different VALuE " --manifest.annotation="com.cyphar.manifest_empty="
[ "$status" -eq 0 ]
image-verify "${IMAGE}"

# Unpack the image again.
umoci raw runtime-config --image "${IMAGE}:${TAG}-new" "$BUNDLE/config.json"
[ "$status" -eq 0 ]

# Manifest beats config.
sane_run jq -SMr '.annotations["com.cyphar.test"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "= __ --a completely different VALuE " ]]

sane_run jq -SMr '.annotations["com.cyphar.label_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

sane_run jq -SMr '.annotations["com.cyphar.manifest_empty"]' "$BUNDLE/config.json"
[ "$status" -eq 0 ]
[[ "$output" == "" ]]

image-verify "${IMAGE}"
}
Loading