diff --git a/libpod/container_commit.go b/libpod/container_commit.go index f447816e36..df0a77bc3d 100644 --- a/libpod/container_commit.go +++ b/libpod/container_commit.go @@ -12,7 +12,6 @@ import ( "github.com/containers/image/v5/types" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" - libpodutil "github.com/containers/podman/v4/pkg/util" "github.com/sirupsen/logrus" ) @@ -148,7 +147,7 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai importBuilder.SetWorkDir(c.config.Spec.Process.Cwd) // Process user changes - newImageConfig, err := libpodutil.GetImageConfig(options.Changes) + newImageConfig, err := libimage.ImageConfigFromChanges(options.Changes) if err != nil { return nil, err } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index fe658c1af6..e7952a0bc3 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -1,7 +1,6 @@ package util import ( - "encoding/json" "errors" "fmt" "io/fs" @@ -28,7 +27,6 @@ import ( "github.com/containers/podman/v4/pkg/signal" "github.com/containers/storage/pkg/idtools" stypes "github.com/containers/storage/types" - v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "golang.org/x/term" @@ -97,236 +95,6 @@ func StringMatchRegexSlice(s string, re []string) bool { return false } -// 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 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{}, fmt.Errorf("invalid change %q - must be formatted as KEY VALUE", change) - } - } - - outerKey := strings.ToUpper(strings.TrimSpace(split[0])) - value := strings.TrimSpace(split[1]) - switch outerKey { - case "USER": - // Assume literal contents are the user. - if value == "" { - return ImageConfig{}, fmt.Errorf("invalid change %q - must provide a value to USER", change) - } - config.User = value - case "EXPOSE": - // EXPOSE is either [portnum] or - // [portnum]/[proto] - // Protocol must be "tcp" or "udp" - splitPort := strings.Split(value, "/") - if len(splitPort) > 2 { - return ImageConfig{}, fmt.Errorf("invalid change %q - EXPOSE port must be formatted as PORT[/PROTO]", change) - } - portNum, err := strconv.Atoi(splitPort[0]) - if err != nil { - return ImageConfig{}, fmt.Errorf("invalid change %q - EXPOSE port must be an integer: %w", change, err) - } - if portNum > 65535 || portNum <= 0 { - return ImageConfig{}, fmt.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{}, fmt.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": - // 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{}, fmt.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, `"`), `"`) - } - config.Env = append(config.Env, fmt.Sprintf("%s=%s", key, val)) - case "ENTRYPOINT": - // 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": - // 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": - // 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{}, fmt.Errorf("invalid change %q - must provide at least one argument to VOLUME", change) - } - for _, vol := range testUnmarshal { - if vol == "" { - return ImageConfig{}, fmt.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": - // 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{}, fmt.Errorf("invalid change %q - must provide a non-empty WORKDIR", change) - } - config.WorkingDir = filepath.Join(config.WorkingDir, value) - case "LABEL": - // 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{}, fmt.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{}, fmt.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": - // Check the provided signal for validity. - killSignal, err := ParseSignal(value) - if err != nil { - return ImageConfig{}, fmt.Errorf("invalid change %q - KILLSIGNAL must be given a valid signal: %w", change, err) - } - config.StopSignal = fmt.Sprintf("%d", killSignal) - case "ONBUILD": - // Onbuild always appends. - if value == "" { - return ImageConfig{}, fmt.Errorf("invalid change %q - ONBUILD must be given an argument", change) - } - config.OnBuild = append(config.OnBuild, value) - default: - return ImageConfig{}, fmt.Errorf("invalid change %q - invalid instruction %s", change, outerKey) - } - } - - return config, nil -} - // ParseSignal parses and validates a signal name or number. func ParseSignal(rawSignal string) (syscall.Signal, error) { // Strip off leading dash, to allow -1 or -HUP diff --git a/pkg/util/utils_linux_test.go b/pkg/util/utils_linux_test.go deleted file mode 100644 index aa193bbef8..0000000000 --- a/pkg/util/utils_linux_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package util - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestGetImageConfigStopSignal(t *testing.T) { - // Linux-only because parsing signal names is not supported on non-Linux systems by - // pkg/signal. - 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) -} diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go index 3d74d4c786..70ecf05a24 100644 --- a/pkg/util/utils_test.go +++ b/pkg/util/utils_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) var ( @@ -22,232 +21,6 @@ func TestStringInSlice(t *testing.T) { assert.False(t, StringInSlice("one", []string{})) } -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 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) -} - func TestValidateSysctls(t *testing.T) { strSlice := []string{"net.core.test1=4", "kernel.msgmax=2"} result, _ := ValidateSysctls(strSlice)