summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>2019-12-06 04:09:06 -0800
committerGitHub <noreply@github.com>2019-12-06 04:09:06 -0800
commit8924a302a2470abc56e7d9ef64e0b29ba5b6db0d (patch)
treecf6a96d812167f9488e5d4ea9d1fbe272df7acfd
parente9c48206019be6b95b5cc8700945b773a857ae26 (diff)
parent60bfa305a88c686c91ebc0f8f98ac14405ae10cf (diff)
downloadpodman-8924a302a2470abc56e7d9ef64e0b29ba5b6db0d.tar.gz
podman-8924a302a2470abc56e7d9ef64e0b29ba5b6db0d.tar.bz2
podman-8924a302a2470abc56e7d9ef64e0b29ba5b6db0d.zip
Merge pull request #4563 from mheon/fix_change_parsing
Fix parsing for arrays of values in image changes
-rw-r--r--libpod/container_commit.go169
-rw-r--r--libpod/runtime_img.go3
-rw-r--r--pkg/util/utils.go310
-rw-r--r--pkg/util/utils_test.go308
-rw-r--r--pkg/varlinkapi/images.go2
-rw-r--r--test/e2e/import_test.go8
6 files changed, 535 insertions, 265 deletions
diff --git a/libpod/container_commit.go b/libpod/container_commit.go
index 42f298a81..a0ba57f4f 100644
--- a/libpod/container_commit.go
+++ b/libpod/container_commit.go
@@ -3,7 +3,6 @@ package libpod
import (
"context"
"fmt"
- "os"
"strings"
"github.com/containers/buildah"
@@ -12,6 +11,7 @@ import (
"github.com/containers/libpod/libpod/define"
"github.com/containers/libpod/libpod/events"
"github.com/containers/libpod/libpod/image"
+ libpodutil "github.com/containers/libpod/pkg/util"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
@@ -32,10 +32,6 @@ type ContainerCommitOptions struct {
// Commit commits the changes between a container and its image, creating a new
// image
func (c *Container) Commit(ctx context.Context, destImage string, options ContainerCommitOptions) (*image.Image, error) {
- var (
- isEnvCleared, isLabelCleared, isExposeCleared, isVolumeCleared bool
- )
-
if c.config.Rootfs != "" {
return nil, errors.Errorf("cannot commit a container that uses an exploded rootfs")
}
@@ -51,7 +47,7 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
if c.state.State == define.ContainerStateRunning && options.Pause {
if err := c.pause(); err != nil {
- return nil, errors.Wrapf(err, "error pausing container %q", c.ID())
+ return nil, errors.Wrapf(err, "error pausing container %q to commit", c.ID())
}
defer func() {
if err := c.unpause(); err != nil {
@@ -103,7 +99,7 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
}
// Expose ports
for _, p := range c.config.PortMappings {
- importBuilder.SetPort(fmt.Sprintf("%d", p.ContainerPort))
+ importBuilder.SetPort(fmt.Sprintf("%d/%s", p.ContainerPort, p.Protocol))
}
// Labels
for k, v := range c.Labels() {
@@ -111,7 +107,9 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
}
// No stop signal
// User
- importBuilder.SetUser(c.User())
+ if c.config.User != "" {
+ importBuilder.SetUser(c.config.User)
+ }
// Volumes
if options.IncludeVolumes {
for _, v := range c.config.UserVolumes {
@@ -119,107 +117,76 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
importBuilder.AddVolume(v)
}
}
- }
- // Workdir
- importBuilder.SetWorkDir(c.Spec().Process.Cwd)
-
- genCmd := func(cmd string) []string {
- trim := func(cmd []string) []string {
- if len(cmd) == 0 {
- return cmd
+ } else {
+ // Only include anonymous named volumes added by the user by
+ // default.
+ for _, v := range c.config.NamedVolumes {
+ include := false
+ for _, userVol := range c.config.UserVolumes {
+ if userVol == v.Dest {
+ include = true
+ break
+ }
}
-
- retCmd := []string{}
- for _, c := range cmd {
- if len(c) >= 2 {
- if c[0] == '"' && c[len(c)-1] == '"' {
- retCmd = append(retCmd, c[1:len(c)-1])
- continue
- }
+ if include {
+ vol, err := c.runtime.GetVolume(v.Name)
+ if err != nil {
+ return nil, errors.Wrapf(err, "volume %s used in container %s has been removed", v.Name, c.ID())
+ }
+ if vol.IsCtrSpecific() {
+ importBuilder.AddVolume(v.Dest)
}
- retCmd = append(retCmd, c)
}
- return retCmd
}
- if strings.HasPrefix(cmd, "[") {
- cmd = strings.TrimPrefix(cmd, "[")
- cmd = strings.TrimSuffix(cmd, "]")
- return trim(strings.Split(cmd, ","))
- }
- return []string{"/bin/sh", "-c", cmd}
}
- // Process user changes
- for _, change := range options.Changes {
- splitChange := strings.SplitN(change, "=", 2)
- if len(splitChange) != 2 {
- splitChange = strings.SplitN(change, " ", 2)
- if len(splitChange) < 2 {
- return nil, errors.Errorf("invalid change %s format", change)
- }
- }
+ // Workdir
+ importBuilder.SetWorkDir(c.config.Spec.Process.Cwd)
- switch strings.ToUpper(splitChange[0]) {
- case "CMD":
- importBuilder.SetCmd(genCmd(splitChange[1]))
- case "ENTRYPOINT":
- importBuilder.SetEntrypoint(genCmd(splitChange[1]))
- case "ENV":
- change := strings.Split(splitChange[1], " ")
- name := change[0]
- val := ""
- if len(change) < 2 {
- change = strings.Split(change[0], "=")
- }
- if len(change) < 2 {
- var ok bool
- val, ok = os.LookupEnv(name)
- if !ok {
- return nil, errors.Errorf("invalid env variable %q: not defined in your environment", name)
- }
- } else {
- name = change[0]
- val = strings.Join(change[1:], " ")
- }
- if !isEnvCleared { // Multiple values are valid, only clear once.
- importBuilder.ClearEnv()
- isEnvCleared = true
- }
- importBuilder.SetEnv(name, val)
- case "EXPOSE":
- if !isExposeCleared { // Multiple values are valid, only clear once
- importBuilder.ClearPorts()
- isExposeCleared = true
- }
- importBuilder.SetPort(splitChange[1])
- case "LABEL":
- change := strings.Split(splitChange[1], " ")
- if len(change) < 2 {
- change = strings.Split(change[0], "=")
- }
- if len(change) < 2 {
- return nil, errors.Errorf("invalid label %s format, requires to NAME=VAL", splitChange[1])
- }
- if !isLabelCleared { // multiple values are valid, only clear once
- importBuilder.ClearLabels()
- isLabelCleared = true
- }
- importBuilder.SetLabel(change[0], strings.Join(change[1:], " "))
- case "ONBUILD":
- importBuilder.SetOnBuild(splitChange[1])
- case "STOPSIGNAL":
- // No Set StopSignal
- case "USER":
- importBuilder.SetUser(splitChange[1])
- case "VOLUME":
- if !isVolumeCleared { // multiple values are valid, only clear once
- importBuilder.ClearVolumes()
- isVolumeCleared = true
- }
- importBuilder.AddVolume(splitChange[1])
- case "WORKDIR":
- importBuilder.SetWorkDir(splitChange[1])
+ // Process user changes
+ newImageConfig, err := libpodutil.GetImageConfig(options.Changes)
+ if err != nil {
+ return nil, err
+ }
+ if newImageConfig.User != "" {
+ importBuilder.SetUser(newImageConfig.User)
+ }
+ // EXPOSE only appends
+ for port := range newImageConfig.ExposedPorts {
+ importBuilder.SetPort(port)
+ }
+ // ENV only appends
+ for _, env := range newImageConfig.Env {
+ splitEnv := strings.SplitN(env, "=", 2)
+ key := splitEnv[0]
+ value := ""
+ if len(splitEnv) == 2 {
+ value = splitEnv[1]
}
+ importBuilder.SetEnv(key, value)
}
+ if newImageConfig.Entrypoint != nil {
+ importBuilder.SetEntrypoint(newImageConfig.Entrypoint)
+ }
+ if newImageConfig.Cmd != nil {
+ importBuilder.SetCmd(newImageConfig.Cmd)
+ }
+ // VOLUME only appends
+ for vol := range newImageConfig.Volumes {
+ importBuilder.AddVolume(vol)
+ }
+ if newImageConfig.WorkingDir != "" {
+ importBuilder.SetWorkDir(newImageConfig.WorkingDir)
+ }
+ for k, v := range newImageConfig.Labels {
+ importBuilder.SetLabel(k, v)
+ }
+ if newImageConfig.StopSignal != "" {
+ importBuilder.SetStopSignal(newImageConfig.StopSignal)
+ }
+ for _, onbuild := range newImageConfig.OnBuild {
+ importBuilder.SetOnBuild(onbuild)
+ }
+
candidates, _, _, err := util.ResolveName(destImage, "", sc, c.runtime.store)
if err != nil {
return nil, errors.Wrapf(err, "error resolving name %q", destImage)
diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go
index f2784c07d..abd8b581d 100644
--- a/libpod/runtime_img.go
+++ b/libpod/runtime_img.go
@@ -160,10 +160,11 @@ func (r *Runtime) Import(ctx context.Context, source string, reference string, c
ic := v1.ImageConfig{}
if len(changes) > 0 {
- ic, err = util.GetImageConfig(changes)
+ config, err := util.GetImageConfig(changes)
if err != nil {
return "", errors.Wrapf(err, "error adding config changes to image %q", source)
}
+ ic = config.ImageConfig
}
hist := []v1.History{
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 6906b26d5..5b4dfe9fa 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -1,11 +1,12 @@
package util
import (
+ "encoding/json"
"fmt"
"os"
"os/user"
"path/filepath"
- "regexp"
+ "strconv"
"strings"
"sync"
"time"
@@ -18,6 +19,7 @@ import (
"github.com/containers/libpod/pkg/rootless"
"github.com/containers/storage"
"github.com/containers/storage/pkg/idtools"
+ "github.com/docker/docker/pkg/signal"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
@@ -71,118 +73,236 @@ func StringInSlice(s string, sl []string) bool {
return false
}
-// ParseChanges returns key, value(s) pair for given option.
-func ParseChanges(option string) (key string, vals []string, err error) {
- // Supported format as below
- // 1. key=value
- // 2. key value
- // 3. key ["value","value1"]
- if strings.Contains(option, " ") {
- // This handles 2 & 3 conditions.
- var val string
- tokens := strings.SplitAfterN(option, " ", 2)
- if len(tokens) < 2 {
- return "", []string{}, fmt.Errorf("invalid key value %s", option)
- }
- key = strings.Trim(tokens[0], " ") // Need to trim whitespace part of delimiter.
- val = tokens[1]
- if strings.Contains(tokens[1], "[") && strings.Contains(tokens[1], "]") {
- //Trim '[',']' if exist.
- val = strings.TrimLeft(strings.TrimRight(tokens[1], "]"), "[")
- }
- vals = strings.Split(val, ",")
- } else if strings.Contains(option, "=") {
- // handles condition 1.
- tokens := strings.Split(option, "=")
- key = tokens[0]
- vals = tokens[1:]
- } else {
- // either ` ` or `=` must be provided after command
- return "", []string{}, fmt.Errorf("invalid format %s", option)
- }
-
- if len(vals) == 0 {
- return "", []string{}, errors.Errorf("no value given for instruction %q", key)
- }
-
- for _, v := range vals {
- //each option must not have ' '., `[`` or `]` & empty strings
- whitespaces := regexp.MustCompile(`[\[\s\]]`)
- if whitespaces.MatchString(v) || len(v) == 0 {
- return "", []string{}, fmt.Errorf("invalid value %s", v)
- }
- }
- return key, vals, nil
+// ImageConfig is a wrapper around the OCIv1 Image Configuration struct exported
+// by containers/image, but containing additional fields that are not supported
+// by OCIv1 (but are by Docker v2) - notably OnBuild.
+type ImageConfig struct {
+ v1.ImageConfig
+ OnBuild []string
}
-// GetImageConfig converts the --change flag values in the format "CMD=/bin/bash USER=example"
-// to a type v1.ImageConfig
-func GetImageConfig(changes []string) (v1.ImageConfig, error) {
- // USER=value | EXPOSE=value | ENV=value | ENTRYPOINT=value |
- // CMD=value | VOLUME=value | WORKDIR=value | LABEL=key=value | STOPSIGNAL=value
-
- var (
- user string
- env []string
- entrypoint []string
- cmd []string
- workingDir string
- stopSignal string
- )
-
- exposedPorts := make(map[string]struct{})
- volumes := make(map[string]struct{})
- labels := make(map[string]string)
- for _, ch := range changes {
- key, vals, err := ParseChanges(ch)
- if err != nil {
- return v1.ImageConfig{}, err
+// GetImageConfig produces a v1.ImageConfig from the --change flag that is
+// accepted by several Podman commands. It accepts a (limited subset) of
+// Dockerfile instructions.
+func GetImageConfig(changes []string) (ImageConfig, error) {
+ // Valid changes:
+ // USER
+ // EXPOSE
+ // ENV
+ // ENTRYPOINT
+ // CMD
+ // VOLUME
+ // WORKDIR
+ // LABEL
+ // STOPSIGNAL
+ // ONBUILD
+
+ config := ImageConfig{}
+
+ for _, change := range changes {
+ // First, let's assume proper Dockerfile format - space
+ // separator between instruction and value
+ split := strings.SplitN(change, " ", 2)
+
+ if len(split) != 2 {
+ split = strings.SplitN(change, "=", 2)
+ if len(split) != 2 {
+ return ImageConfig{}, errors.Errorf("invalid change %q - must be formatted as KEY VALUE", change)
+ }
}
- switch key {
+ outerKey := strings.ToUpper(strings.TrimSpace(split[0]))
+ value := strings.TrimSpace(split[1])
+ switch outerKey {
case "USER":
- user = vals[0]
+ // Assume literal contents are the user.
+ if value == "" {
+ return ImageConfig{}, errors.Errorf("invalid change %q - must provide a value to USER", change)
+ }
+ config.User = value
case "EXPOSE":
- var st struct{}
- exposedPorts[vals[0]] = st
+ // EXPOSE is either [portnum] or
+ // [portnum]/[proto]
+ // Protocol must be "tcp" or "udp"
+ splitPort := strings.Split(value, "/")
+ if len(splitPort) > 2 {
+ return ImageConfig{}, errors.Errorf("invalid change %q - EXPOSE port must be formatted as PORT[/PROTO]", change)
+ }
+ portNum, err := strconv.Atoi(splitPort[0])
+ if err != nil {
+ return ImageConfig{}, errors.Wrapf(err, "invalid change %q - EXPOSE port must be an integer", change)
+ }
+ if portNum > 65535 || portNum <= 0 {
+ return ImageConfig{}, errors.Errorf("invalid change %q - EXPOSE port must be a valid port number", change)
+ }
+ proto := "tcp"
+ if len(splitPort) > 1 {
+ testProto := strings.ToLower(splitPort[1])
+ switch testProto {
+ case "tcp", "udp":
+ proto = testProto
+ default:
+ return ImageConfig{}, errors.Errorf("invalid change %q - EXPOSE protocol must be TCP or UDP", change)
+ }
+ }
+ if config.ExposedPorts == nil {
+ config.ExposedPorts = make(map[string]struct{})
+ }
+ config.ExposedPorts[fmt.Sprintf("%d/%s", portNum, proto)] = struct{}{}
case "ENV":
- if len(vals) < 2 {
- return v1.ImageConfig{}, errors.Errorf("no value given for environment variable %q", vals[0])
+ // Format is either:
+ // ENV key=value
+ // ENV key=value key=value ...
+ // ENV key value
+ // Both keys and values can be surrounded by quotes to group them.
+ // For now: we only support key=value
+ // We will attempt to strip quotation marks if present.
+
+ var (
+ key, val string
+ )
+
+ splitEnv := strings.SplitN(value, "=", 2)
+ key = splitEnv[0]
+ // We do need a key
+ if key == "" {
+ return ImageConfig{}, errors.Errorf("invalid change %q - ENV must have at least one argument", change)
+ }
+ // Perfectly valid to not have a value
+ if len(splitEnv) == 2 {
+ val = splitEnv[1]
+ }
+
+ if strings.HasPrefix(key, `"`) && strings.HasSuffix(key, `"`) {
+ key = strings.TrimPrefix(strings.TrimSuffix(key, `"`), `"`)
+ }
+ if strings.HasPrefix(val, `"`) && strings.HasSuffix(val, `"`) {
+ val = strings.TrimPrefix(strings.TrimSuffix(val, `"`), `"`)
}
- env = append(env, strings.Join(vals[0:], "="))
+ config.Env = append(config.Env, fmt.Sprintf("%s=%s", key, val))
case "ENTRYPOINT":
- // ENTRYPOINT and CMD can have array of strings
- entrypoint = append(entrypoint, vals...)
+ // Two valid forms.
+ // First, JSON array.
+ // Second, not a JSON array - we interpret this as an
+ // argument to `sh -c`, unless empty, in which case we
+ // just use a blank entrypoint.
+ testUnmarshal := []string{}
+ if err := json.Unmarshal([]byte(value), &testUnmarshal); err != nil {
+ // It ain't valid JSON, so assume it's an
+ // argument to sh -c if not empty.
+ if value != "" {
+ config.Entrypoint = []string{"/bin/sh", "-c", value}
+ } else {
+ config.Entrypoint = []string{}
+ }
+ } else {
+ // Valid JSON
+ config.Entrypoint = testUnmarshal
+ }
case "CMD":
- // ENTRYPOINT and CMD can have array of strings
- cmd = append(cmd, vals...)
+ // Same valid forms as entrypoint.
+ // However, where ENTRYPOINT assumes that 'ENTRYPOINT '
+ // means no entrypoint, CMD assumes it is 'sh -c' with
+ // no third argument.
+ testUnmarshal := []string{}
+ if err := json.Unmarshal([]byte(value), &testUnmarshal); err != nil {
+ // It ain't valid JSON, so assume it's an
+ // argument to sh -c.
+ // Only include volume if it's not ""
+ config.Cmd = []string{"/bin/sh", "-c"}
+ if value != "" {
+ config.Cmd = append(config.Cmd, value)
+ }
+ } else {
+ // Valid JSON
+ config.Cmd = testUnmarshal
+ }
case "VOLUME":
- var st struct{}
- volumes[vals[0]] = st
+ // Either a JSON array or a set of space-separated
+ // paths.
+ // Acts rather similar to ENTRYPOINT and CMD, but always
+ // appends rather than replacing, and no sh -c prepend.
+ testUnmarshal := []string{}
+ if err := json.Unmarshal([]byte(value), &testUnmarshal); err != nil {
+ // Not valid JSON, so split on spaces
+ testUnmarshal = strings.Split(value, " ")
+ }
+ if len(testUnmarshal) == 0 {
+ return ImageConfig{}, errors.Errorf("invalid change %q - must provide at least one argument to VOLUME", change)
+ }
+ for _, vol := range testUnmarshal {
+ if vol == "" {
+ return ImageConfig{}, errors.Errorf("invalid change %q - VOLUME paths must not be empty", change)
+ }
+ if config.Volumes == nil {
+ config.Volumes = make(map[string]struct{})
+ }
+ config.Volumes[vol] = struct{}{}
+ }
case "WORKDIR":
- workingDir = vals[0]
+ // This can be passed multiple times.
+ // Each successive invocation is treated as relative to
+ // the previous one - so WORKDIR /A, WORKDIR b,
+ // WORKDIR c results in /A/b/c
+ // Just need to check it's not empty...
+ if value == "" {
+ return ImageConfig{}, errors.Errorf("invalid change %q - must provide a non-empty WORKDIR", change)
+ }
+ config.WorkingDir = filepath.Join(config.WorkingDir, value)
case "LABEL":
- if len(vals) == 2 {
- labels[vals[0]] = vals[1]
- } else {
- labels[vals[0]] = ""
+ // Same general idea as ENV, but we no longer allow " "
+ // as a separator.
+ // We didn't do that for ENV either, so nice and easy.
+ // Potentially problematic: LABEL might theoretically
+ // allow an = in the key? If people really do this, we
+ // may need to investigate more advanced parsing.
+ var (
+ key, val string
+ )
+
+ splitLabel := strings.SplitN(value, "=", 2)
+ // Unlike ENV, LABEL must have a value
+ if len(splitLabel) != 2 {
+ return ImageConfig{}, errors.Errorf("invalid change %q - LABEL must be formatted key=value", change)
+ }
+ key = splitLabel[0]
+ val = splitLabel[1]
+
+ if strings.HasPrefix(key, `"`) && strings.HasSuffix(key, `"`) {
+ key = strings.TrimPrefix(strings.TrimSuffix(key, `"`), `"`)
+ }
+ if strings.HasPrefix(val, `"`) && strings.HasSuffix(val, `"`) {
+ val = strings.TrimPrefix(strings.TrimSuffix(val, `"`), `"`)
+ }
+ // Check key after we strip quotations
+ if key == "" {
+ return ImageConfig{}, errors.Errorf("invalid change %q - LABEL must have a non-empty key", change)
}
+ if config.Labels == nil {
+ config.Labels = make(map[string]string)
+ }
+ config.Labels[key] = val
case "STOPSIGNAL":
- stopSignal = vals[0]
+ // Check the provided signal for validity.
+ // TODO: Worth checking range? ParseSignal allows
+ // negative numbers.
+ killSignal, err := signal.ParseSignal(value)
+ if err != nil {
+ return ImageConfig{}, errors.Wrapf(err, "invalid change %q - KILLSIGNAL must be given a valid signal", change)
+ }
+ config.StopSignal = fmt.Sprintf("%d", killSignal)
+ case "ONBUILD":
+ // Onbuild always appends.
+ if value == "" {
+ return ImageConfig{}, errors.Errorf("invalid change %q - ONBUILD must be given an argument", change)
+ }
+ config.OnBuild = append(config.OnBuild, value)
+ default:
+ return ImageConfig{}, errors.Errorf("invalid change %q - invalid instruction %s", change, outerKey)
}
}
- return v1.ImageConfig{
- User: user,
- ExposedPorts: exposedPorts,
- Env: env,
- Entrypoint: entrypoint,
- Cmd: cmd,
- Volumes: volumes,
- WorkingDir: workingDir,
- Labels: labels,
- StopSignal: stopSignal,
- }, nil
+ return config, nil
}
// ParseIDMapping takes idmappings and subuid and subgid maps and returns a storage mapping
diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go
index c938dc592..f4b03599d 100644
--- a/pkg/util/utils_test.go
+++ b/pkg/util/utils_test.go
@@ -4,6 +4,7 @@ import (
"testing"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
var (
@@ -19,70 +20,247 @@ func TestStringInSlice(t *testing.T) {
assert.False(t, StringInSlice("one", []string{}))
}
-func TestParseChanges(t *testing.T) {
- // CMD=/bin/sh
- _, vals, err := ParseChanges("CMD=/bin/sh")
- assert.EqualValues(t, []string{"/bin/sh"}, vals)
- assert.NoError(t, err)
-
- // CMD [/bin/sh]
- _, vals, err = ParseChanges("CMD [/bin/sh]")
- assert.EqualValues(t, []string{"/bin/sh"}, vals)
- assert.NoError(t, err)
-
- // CMD ["/bin/sh"]
- _, vals, err = ParseChanges(`CMD ["/bin/sh"]`)
- assert.EqualValues(t, []string{`"/bin/sh"`}, vals)
- assert.NoError(t, err)
-
- // CMD ["/bin/sh","-c","ls"]
- _, vals, err = ParseChanges(`CMD ["/bin/sh","c","ls"]`)
- assert.EqualValues(t, []string{`"/bin/sh"`, `"c"`, `"ls"`}, vals)
- assert.NoError(t, err)
-
- // CMD ["/bin/sh","arg-with,comma"]
- _, vals, err = ParseChanges(`CMD ["/bin/sh","arg-with,comma"]`)
- assert.EqualValues(t, []string{`"/bin/sh"`, `"arg-with`, `comma"`}, vals)
- assert.NoError(t, err)
-
- // CMD "/bin/sh"]
- _, _, err = ParseChanges(`CMD "/bin/sh"]`)
- assert.Error(t, err)
- assert.Equal(t, `invalid value "/bin/sh"]`, err.Error())
-
- // CMD [bin/sh
- _, _, err = ParseChanges(`CMD "/bin/sh"]`)
- assert.Error(t, err)
- assert.Equal(t, `invalid value "/bin/sh"]`, err.Error())
-
- // CMD ["/bin /sh"]
- _, _, err = ParseChanges(`CMD ["/bin /sh"]`)
- assert.Error(t, err)
- assert.Equal(t, `invalid value "/bin /sh"`, err.Error())
-
- // CMD ["/bin/sh", "-c","ls"] whitespace between values
- _, vals, err = ParseChanges(`CMD ["/bin/sh", "c","ls"]`)
- assert.Error(t, err)
- assert.Equal(t, `invalid value "c"`, err.Error())
-
- // CMD?
- _, _, err = ParseChanges(`CMD?`)
- assert.Error(t, err)
- assert.Equal(t, `invalid format CMD?`, err.Error())
-
- // empty values for CMD
- _, _, err = ParseChanges(`CMD `)
- assert.Error(t, err)
- assert.Equal(t, `invalid value `, err.Error())
-
- // LABEL=blue=image
- _, vals, err = ParseChanges(`LABEL=blue=image`)
- assert.EqualValues(t, []string{"blue", "image"}, vals)
- assert.NoError(t, err)
-
- // LABEL = blue=image
- _, vals, err = ParseChanges(`LABEL = blue=image`)
- assert.Error(t, err)
- assert.Equal(t, `invalid value = blue=image`, err.Error())
+func TestGetImageConfigUser(t *testing.T) {
+ validUser, err := GetImageConfig([]string{"USER valid"})
+ require.Nil(t, err)
+ assert.Equal(t, validUser.User, "valid")
+ validUser2, err := GetImageConfig([]string{"USER test_user_2"})
+ require.Nil(t, err)
+ assert.Equal(t, validUser2.User, "test_user_2")
+
+ _, err = GetImageConfig([]string{"USER "})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigExpose(t *testing.T) {
+ validPortNoProto, err := GetImageConfig([]string{"EXPOSE 80"})
+ require.Nil(t, err)
+ _, exists := validPortNoProto.ExposedPorts["80/tcp"]
+ assert.True(t, exists)
+
+ validPortTCP, err := GetImageConfig([]string{"EXPOSE 80/tcp"})
+ require.Nil(t, err)
+ _, exists = validPortTCP.ExposedPorts["80/tcp"]
+ assert.True(t, exists)
+
+ validPortUDP, err := GetImageConfig([]string{"EXPOSE 80/udp"})
+ require.Nil(t, err)
+ _, exists = validPortUDP.ExposedPorts["80/udp"]
+ assert.True(t, exists)
+
+ _, err = GetImageConfig([]string{"EXPOSE 99999"})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"EXPOSE 80/notaproto"})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"EXPOSE "})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"EXPOSE thisisnotanumber"})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigEnv(t *testing.T) {
+ validEnvNoValue, err := GetImageConfig([]string{"ENV key"})
+ require.Nil(t, err)
+ assert.True(t, StringInSlice("key=", validEnvNoValue.Env))
+
+ validEnvBareEquals, err := GetImageConfig([]string{"ENV key="})
+ require.Nil(t, err)
+ assert.True(t, StringInSlice("key=", validEnvBareEquals.Env))
+
+ validEnvKeyValue, err := GetImageConfig([]string{"ENV key=value"})
+ require.Nil(t, err)
+ assert.True(t, StringInSlice("key=value", validEnvKeyValue.Env))
+
+ validEnvKeyMultiEntryValue, err := GetImageConfig([]string{`ENV key="value1 value2"`})
+ require.Nil(t, err)
+ assert.True(t, StringInSlice("key=value1 value2", validEnvKeyMultiEntryValue.Env))
+
+ _, err = GetImageConfig([]string{"ENV "})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigEntrypoint(t *testing.T) {
+ binShEntrypoint, err := GetImageConfig([]string{"ENTRYPOINT /bin/bash"})
+ require.Nil(t, err)
+ require.Equal(t, 3, len(binShEntrypoint.Entrypoint))
+ assert.Equal(t, binShEntrypoint.Entrypoint[0], "/bin/sh")
+ assert.Equal(t, binShEntrypoint.Entrypoint[1], "-c")
+ assert.Equal(t, binShEntrypoint.Entrypoint[2], "/bin/bash")
+
+ entrypointWithSpaces, err := GetImageConfig([]string{"ENTRYPOINT ls -al"})
+ require.Nil(t, err)
+ require.Equal(t, 3, len(entrypointWithSpaces.Entrypoint))
+ assert.Equal(t, entrypointWithSpaces.Entrypoint[0], "/bin/sh")
+ assert.Equal(t, entrypointWithSpaces.Entrypoint[1], "-c")
+ assert.Equal(t, entrypointWithSpaces.Entrypoint[2], "ls -al")
+
+ jsonArrayEntrypoint, err := GetImageConfig([]string{`ENTRYPOINT ["ls", "-al"]`})
+ require.Nil(t, err)
+ require.Equal(t, 2, len(jsonArrayEntrypoint.Entrypoint))
+ assert.Equal(t, jsonArrayEntrypoint.Entrypoint[0], "ls")
+ assert.Equal(t, jsonArrayEntrypoint.Entrypoint[1], "-al")
+
+ emptyEntrypoint, err := GetImageConfig([]string{"ENTRYPOINT "})
+ require.Nil(t, err)
+ assert.Equal(t, 0, len(emptyEntrypoint.Entrypoint))
+
+ emptyEntrypointArray, err := GetImageConfig([]string{"ENTRYPOINT []"})
+ require.Nil(t, err)
+ assert.Equal(t, 0, len(emptyEntrypointArray.Entrypoint))
+}
+
+func TestGetImageConfigCmd(t *testing.T) {
+ binShCmd, err := GetImageConfig([]string{"CMD /bin/bash"})
+ require.Nil(t, err)
+ require.Equal(t, 3, len(binShCmd.Cmd))
+ assert.Equal(t, binShCmd.Cmd[0], "/bin/sh")
+ assert.Equal(t, binShCmd.Cmd[1], "-c")
+ assert.Equal(t, binShCmd.Cmd[2], "/bin/bash")
+
+ cmdWithSpaces, err := GetImageConfig([]string{"CMD ls -al"})
+ require.Nil(t, err)
+ require.Equal(t, 3, len(cmdWithSpaces.Cmd))
+ assert.Equal(t, cmdWithSpaces.Cmd[0], "/bin/sh")
+ assert.Equal(t, cmdWithSpaces.Cmd[1], "-c")
+ assert.Equal(t, cmdWithSpaces.Cmd[2], "ls -al")
+
+ jsonArrayCmd, err := GetImageConfig([]string{`CMD ["ls", "-al"]`})
+ require.Nil(t, err)
+ require.Equal(t, 2, len(jsonArrayCmd.Cmd))
+ assert.Equal(t, jsonArrayCmd.Cmd[0], "ls")
+ assert.Equal(t, jsonArrayCmd.Cmd[1], "-al")
+
+ emptyCmd, err := GetImageConfig([]string{"CMD "})
+ require.Nil(t, err)
+ require.Equal(t, 2, len(emptyCmd.Cmd))
+ assert.Equal(t, emptyCmd.Cmd[0], "/bin/sh")
+ assert.Equal(t, emptyCmd.Cmd[1], "-c")
+
+ blankCmd, err := GetImageConfig([]string{"CMD []"})
+ require.Nil(t, err)
+ assert.Equal(t, 0, len(blankCmd.Cmd))
+}
+
+func TestGetImageConfigVolume(t *testing.T) {
+ oneLenJSONArrayVol, err := GetImageConfig([]string{`VOLUME ["/test1"]`})
+ require.Nil(t, err)
+ _, exists := oneLenJSONArrayVol.Volumes["/test1"]
+ assert.True(t, exists)
+ assert.Equal(t, 1, len(oneLenJSONArrayVol.Volumes))
+
+ twoLenJSONArrayVol, err := GetImageConfig([]string{`VOLUME ["/test1", "/test2"]`})
+ require.Nil(t, err)
+ assert.Equal(t, 2, len(twoLenJSONArrayVol.Volumes))
+ _, exists = twoLenJSONArrayVol.Volumes["/test1"]
+ assert.True(t, exists)
+ _, exists = twoLenJSONArrayVol.Volumes["/test2"]
+ assert.True(t, exists)
+
+ oneLenVol, err := GetImageConfig([]string{"VOLUME /test1"})
+ require.Nil(t, err)
+ _, exists = oneLenVol.Volumes["/test1"]
+ assert.True(t, exists)
+ assert.Equal(t, 1, len(oneLenVol.Volumes))
+
+ twoLenVol, err := GetImageConfig([]string{"VOLUME /test1 /test2"})
+ require.Nil(t, err)
+ assert.Equal(t, 2, len(twoLenVol.Volumes))
+ _, exists = twoLenVol.Volumes["/test1"]
+ assert.True(t, exists)
+ _, exists = twoLenVol.Volumes["/test2"]
+ assert.True(t, exists)
+
+ _, err = GetImageConfig([]string{"VOLUME []"})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"VOLUME "})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{`VOLUME [""]`})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigWorkdir(t *testing.T) {
+ singleWorkdir, err := GetImageConfig([]string{"WORKDIR /testdir"})
+ require.Nil(t, err)
+ assert.Equal(t, singleWorkdir.WorkingDir, "/testdir")
+
+ twoWorkdirs, err := GetImageConfig([]string{"WORKDIR /testdir", "WORKDIR a"})
+ require.Nil(t, err)
+ assert.Equal(t, twoWorkdirs.WorkingDir, "/testdir/a")
+
+ _, err = GetImageConfig([]string{"WORKDIR "})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigLabel(t *testing.T) {
+ labelNoQuotes, err := GetImageConfig([]string{"LABEL key1=value1"})
+ require.Nil(t, err)
+ assert.Equal(t, labelNoQuotes.Labels["key1"], "value1")
+
+ labelWithQuotes, err := GetImageConfig([]string{`LABEL "key 1"="value 2"`})
+ require.Nil(t, err)
+ assert.Equal(t, labelWithQuotes.Labels["key 1"], "value 2")
+
+ labelNoValue, err := GetImageConfig([]string{"LABEL key="})
+ require.Nil(t, err)
+ contents, exists := labelNoValue.Labels["key"]
+ assert.True(t, exists)
+ assert.Equal(t, contents, "")
+
+ _, err = GetImageConfig([]string{"LABEL key"})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"LABEL "})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigStopSignal(t *testing.T) {
+ stopSignalValidInt, err := GetImageConfig([]string{"STOPSIGNAL 9"})
+ require.Nil(t, err)
+ assert.Equal(t, stopSignalValidInt.StopSignal, "9")
+
+ stopSignalValidString, err := GetImageConfig([]string{"STOPSIGNAL SIGKILL"})
+ require.Nil(t, err)
+ assert.Equal(t, stopSignalValidString.StopSignal, "9")
+
+ _, err = GetImageConfig([]string{"STOPSIGNAL 0"})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"STOPSIGNAL garbage"})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"STOPSIGNAL "})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigOnBuild(t *testing.T) {
+ onBuildOne, err := GetImageConfig([]string{"ONBUILD ADD /testdir1"})
+ require.Nil(t, err)
+ require.Equal(t, 1, len(onBuildOne.OnBuild))
+ assert.Equal(t, onBuildOne.OnBuild[0], "ADD /testdir1")
+
+ onBuildTwo, err := GetImageConfig([]string{"ONBUILD ADD /testdir1", "ONBUILD ADD /testdir2"})
+ require.Nil(t, err)
+ require.Equal(t, 2, len(onBuildTwo.OnBuild))
+ assert.Equal(t, onBuildTwo.OnBuild[0], "ADD /testdir1")
+ assert.Equal(t, onBuildTwo.OnBuild[1], "ADD /testdir2")
+
+ _, err = GetImageConfig([]string{"ONBUILD "})
+ assert.NotNil(t, err)
+}
+
+func TestGetImageConfigMisc(t *testing.T) {
+ _, err := GetImageConfig([]string{""})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"USER"})
+ assert.NotNil(t, err)
+
+ _, err = GetImageConfig([]string{"BADINST testvalue"})
+ assert.NotNil(t, err)
}
diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go
index 7abffa42a..604a455a5 100644
--- a/pkg/varlinkapi/images.go
+++ b/pkg/varlinkapi/images.go
@@ -602,7 +602,7 @@ func (i *LibpodAPI) ImportImage(call iopodman.VarlinkCall, source, reference, me
{Comment: message},
}
config := v1.Image{
- Config: configChanges,
+ Config: configChanges.ImageConfig,
History: history,
}
newImage, err := i.Runtime.ImageRuntime().Import(getContext(), source, reference, nil, image.SigningOptions{}, config)
diff --git a/test/e2e/import_test.go b/test/e2e/import_test.go
index 979440a50..bceb30f7c 100644
--- a/test/e2e/import_test.go
+++ b/test/e2e/import_test.go
@@ -105,7 +105,9 @@ var _ = Describe("Podman import", func() {
results.WaitWithDefaultTimeout()
Expect(results.ExitCode()).To(Equal(0))
imageData := results.InspectImageJSON()
- Expect(imageData[0].Config.Cmd[0]).To(Equal("/bin/bash"))
+ Expect(imageData[0].Config.Cmd[0]).To(Equal("/bin/sh"))
+ Expect(imageData[0].Config.Cmd[1]).To(Equal("-c"))
+ Expect(imageData[0].Config.Cmd[2]).To(Equal("/bin/bash"))
})
It("podman import with change flag CMD <path>", func() {
@@ -126,6 +128,8 @@ var _ = Describe("Podman import", func() {
Expect(results.ExitCode()).To(Equal(0))
imageData := results.InspectImageJSON()
Expect(imageData[0].Config.Cmd[0]).To(Equal("/bin/sh"))
+ Expect(imageData[0].Config.Cmd[1]).To(Equal("-c"))
+ Expect(imageData[0].Config.Cmd[2]).To(Equal("/bin/sh"))
})
It("podman import with change flag CMD [\"path\",\"path'\"", func() {
@@ -137,7 +141,7 @@ var _ = Describe("Podman import", func() {
export.WaitWithDefaultTimeout()
Expect(export.ExitCode()).To(Equal(0))
- importImage := podmanTest.Podman([]string{"import", "--change", "CMD [/bin/bash]", outfile, "imported-image"})
+ importImage := podmanTest.Podman([]string{"import", "--change", "CMD [\"/bin/bash\"]", outfile, "imported-image"})
importImage.WaitWithDefaultTimeout()
Expect(importImage.ExitCode()).To(Equal(0))