diff options
| -rw-r--r-- | libpod/container_commit.go | 169 | ||||
| -rw-r--r-- | libpod/runtime_img.go | 3 | ||||
| -rw-r--r-- | pkg/util/utils.go | 310 | ||||
| -rw-r--r-- | pkg/util/utils_test.go | 308 | ||||
| -rw-r--r-- | pkg/varlinkapi/images.go | 2 | ||||
| -rw-r--r-- | test/e2e/import_test.go | 8 |
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)) |