Merge pull request #20657 from nalind/commit-config

RHEL-14922: accept a config blob alongside the "changes" slice when committing
This commit is contained in:
openshift-merge-bot[bot]
2023-12-01 21:09:23 +00:00
committed by GitHub
19 changed files with 330 additions and 74 deletions

View File

@ -9,6 +9,7 @@ import (
"github.com/containers/common/pkg/completion"
"github.com/containers/podman/v4/cmd/podman/common"
"github.com/containers/podman/v4/cmd/podman/registry"
"github.com/containers/podman/v4/pkg/api/handlers"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/spf13/cobra"
)
@ -47,7 +48,7 @@ var (
commitOptions = entities.CommitOptions{
ImageName: "",
}
iidFile string
configFile, iidFile string
)
func commitFlags(cmd *cobra.Command) {
@ -57,6 +58,10 @@ func commitFlags(cmd *cobra.Command) {
flags.StringArrayVarP(&commitOptions.Changes, changeFlagName, "c", []string{}, "Apply the following possible instructions to the created image (default []): "+strings.Join(common.ChangeCmds, " | "))
_ = cmd.RegisterFlagCompletionFunc(changeFlagName, common.AutocompleteChangeInstructions)
configFileFlagName := "config"
flags.StringVar(&configFile, configFileFlagName, "", "`file` containing a container configuration to merge into the image")
_ = cmd.RegisterFlagCompletionFunc(configFileFlagName, completion.AutocompleteDefault)
formatFlagName := "format"
flags.StringVarP(&commitOptions.Format, formatFlagName, "f", "oci", "`Format` of the image manifest and metadata")
_ = cmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteImageFormat)
@ -100,7 +105,16 @@ func commit(cmd *cobra.Command, args []string) error {
if !commitOptions.Quiet {
commitOptions.Writer = os.Stderr
}
if len(commitOptions.Changes) > 0 {
commitOptions.Changes = handlers.DecodeChanges(commitOptions.Changes)
}
if len(configFile) > 0 {
cfg, err := os.ReadFile(configFile)
if err != nil {
return fmt.Errorf("--config: %w", err)
}
commitOptions.Config = cfg
}
response, err := registry.ContainerEngine().ContainerCommit(context.Background(), container, commitOptions)
if err != nil {
return err

View File

@ -400,7 +400,7 @@ func createPodIfNecessary(cmd *cobra.Command, s *specgen.SpecGenerator, netOpts
var err error
uns := specgen.Namespace{NSMode: specgen.Default}
if cliVals.UserNS != "" {
uns, err = specgen.ParseNamespace(cliVals.UserNS)
uns, err = specgen.ParseUserNamespace(cliVals.UserNS)
if err != nil {
return err
}

View File

@ -4,9 +4,8 @@
####> are applicable to all of those.
#### **--volume**, **-v**=*[HOST-DIR:CONTAINER-DIR[:OPTIONS]]*
Create a bind mount. Specifying the `-v /HOST-DIR:/CONTAINER-DIR` option, Podman
bind mounts `/HOST-DIR` from the host to `/CONTAINER-DIR` in the Podman
container.
Mount a host directory into containers when executing RUN instructions during
the build.
The `OPTIONS` are a comma-separated list and can be: <sup>[[1]](#Footnote1)</sup>
@ -17,12 +16,9 @@ The `OPTIONS` are a comma-separated list and can be: <sup>[[1]](#Footnote1)</sup
The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The `HOST-DIR`
must be an absolute path as well. Podman bind-mounts the `HOST-DIR` to the
specified path. For example, when specifying the host path `/foo`,
Podman copies the contents of `/foo` to the container filesystem on the host
and bind mounts that into the container.
specified path when processing RUN instructions.
You can specify multiple **-v** options to mount one or more mounts to a
container.
You can specify multiple **-v** options to mount one or more mounts.
You can add the `:ro` or `:rw` suffix to a volume to mount it read-only or
read-write mode, respectively. By default, the volumes are mounted read-write.

View File

@ -36,6 +36,13 @@ Apply the following possible instructions to the created image:
Can be set multiple times.
#### **--config**=*ConfigBlobFile*
Merge the container configuration from the specified file into the configuration for the image
as it is being committed. The file contents should be a JSON-encoded version of
a Schema2Config structure, which is defined at
https://github.com/containers/image/blob/v5.29.0/manifest/docker_schema2.go#L67.
#### **--format**, **-f**=**oci** | *docker*
Set the format of the image manifest and metadata. The currently supported formats are **oci** and *docker*.\

View File

@ -20,16 +20,16 @@ import (
// ContainerCommitOptions is a struct used to commit a container to an image
// It uses buildah's CommitOptions as a base. Long-term we might wish to
// add these to the buildah struct once buildah is more integrated with
// libpod
// decouple these because it includes duplicates of fields that are in, or
// could later be added, to buildah's CommitOptions, which gets confusing
type ContainerCommitOptions struct {
buildah.CommitOptions
Pause bool
IncludeVolumes bool
Author string
Message string
Changes []string
Squash bool
Changes []string // gets merged with CommitOptions.OverrideChanges
Squash bool // always used instead of CommitOptions.Squash
}
// Commit commits the changes between a container and its image, creating a new
@ -69,6 +69,8 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
Squash: options.Squash,
SystemContext: c.runtime.imageContext,
PreferredManifestType: options.PreferredManifestType,
OverrideChanges: append(append([]string{}, options.Changes...), options.CommitOptions.OverrideChanges...),
OverrideConfig: options.CommitOptions.OverrideConfig,
}
importBuilder, err := buildah.ImportBuilder(ctx, c.runtime.store, builderOptions)
importBuilder.Format = options.PreferredManifestType
@ -150,51 +152,6 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
// Workdir
importBuilder.SetWorkDir(c.config.Spec.Process.Cwd)
// Process user changes
newImageConfig, err := libimage.ImageConfigFromChanges(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)
}
var commitRef types.ImageReference
if destImage != "" {
// Now resolve the name.

View File

@ -0,0 +1,34 @@
package handlers
import (
"strings"
"unicode"
)
// DecodeChanges reads one or more changes from a slice and cleans them up,
// since what we've advertised as being acceptable in the past isn't really.
func DecodeChanges(changes []string) []string {
result := make([]string, 0, len(changes))
for _, possiblyMultilineChange := range changes {
for _, change := range strings.Split(possiblyMultilineChange, "\n") {
// In particular, we document that we accept values
// like "CMD=/bin/sh", which is not valid Dockerfile
// syntax, so we can't just pass such a value directly
// to a parser that's going to rightfully reject it.
// If we trim the string of whitespace at both ends,
// and the first occurrence of "=" is before the first
// whitespace, replace that "=" with whitespace.
change = strings.TrimSpace(change)
if change == "" {
continue
}
firstEqualIndex := strings.Index(change, "=")
firstSpaceIndex := strings.IndexFunc(change, unicode.IsSpace)
if firstEqualIndex != -1 && (firstSpaceIndex == -1 || firstEqualIndex < firstSpaceIndex) {
change = change[:firstEqualIndex] + " " + change[firstEqualIndex+1:]
}
result = append(result, change)
}
}
return result
}

View File

@ -0,0 +1,52 @@
package handlers
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestDecodeChanges(t *testing.T) {
testCases := []struct {
description string
input string
output []string
}{
{
description: "nothing",
input: "",
output: []string{},
},
{
description: "space",
input: `CMD=/bin/bash`,
output: []string{"CMD /bin/bash"},
},
{
description: "equal",
input: `CMD=/bin/bash`,
output: []string{"CMD /bin/bash"},
},
{
description: "both-but-right-first",
input: `LABEL A=B`,
output: []string{"LABEL A=B"},
},
{
description: "both-but-right-second",
input: `LABEL A=B C=D`,
output: []string{"LABEL A=B C=D"},
},
{
description: "both-but-wrong",
input: `LABEL=A=B C=D`,
output: []string{"LABEL A=B C=D"},
},
}
for _, testCase := range testCases {
t.Run(testCase.description, func(t *testing.T) {
output := DecodeChanges([]string{testCase.input})
assert.Equalf(t, testCase.output, output, "decoded value was not what we expected")
})
}
}

View File

@ -2,7 +2,6 @@ package compat
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
@ -133,18 +132,17 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
PreferredManifestType: manifest.DockerV2Schema2MediaType,
}
input := handlers.CreateContainerConfig{}
if err := json.NewDecoder(r.Body).Decode(&input); err != nil {
utils.Error(w, http.StatusInternalServerError, fmt.Errorf("Decode(): %w", err))
return
}
options.Message = query.Comment
options.Author = query.Author
options.Pause = query.Pause
options.Squash = query.Squash
for _, change := range query.Changes {
options.Changes = append(options.Changes, strings.Split(change, "\n")...)
options.Changes = handlers.DecodeChanges(query.Changes)
if r.Body != nil {
defer r.Body.Close()
if options.CommitOptions.OverrideConfig, err = abi.DecodeOverrideConfig(r.Body); err != nil {
utils.Error(w, http.StatusBadRequest, err)
return
}
}
ctr, err := runtime.LookupContainer(query.Container)
if err != nil {

View File

@ -483,6 +483,13 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
SystemContext: sc,
PreferredManifestType: mimeType,
}
if r.Body != nil {
defer r.Body.Close()
if options.CommitOptions.OverrideConfig, err = abi.DecodeOverrideConfig(r.Body); err != nil {
utils.Error(w, http.StatusBadRequest, err)
return
}
}
if len(query.Tag) > 0 {
tag = query.Tag
}
@ -490,7 +497,7 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
options.Author = query.Author
options.Pause = query.Pause
options.Squash = query.Squash
options.Changes = query.Changes
options.Changes = handlers.DecodeChanges(query.Changes)
ctr, err := runtime.LookupContainer(query.Container)
if err != nil {
utils.Error(w, http.StatusNotFound, err)

View File

@ -33,7 +33,11 @@ func Commit(ctx context.Context, nameOrID string, options *CommitOptions) (entit
return entities.IDResponse{}, err
}
params.Set("container", nameOrID)
response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/commit", params, nil)
var requestBody io.Reader
if options.Config != nil {
requestBody = *options.Config
}
response, err := conn.DoRequest(ctx, requestBody, http.MethodPost, "/commit", params, nil)
if err != nil {
return id, err
}

View File

@ -29,6 +29,7 @@ type LogOptions struct {
type CommitOptions struct {
Author *string
Changes []string
Config *io.Reader `schema:"-"`
Comment *string
Format *string
Pause *bool

View File

@ -2,6 +2,7 @@
package containers
import (
"io"
"net/url"
"github.com/containers/podman/v4/pkg/bindings/internal/util"
@ -47,6 +48,21 @@ func (o *CommitOptions) GetChanges() []string {
return o.Changes
}
// WithConfig set field Config to given value
func (o *CommitOptions) WithConfig(value io.Reader) *CommitOptions {
o.Config = &value
return o
}
// GetConfig returns value of field Config
func (o *CommitOptions) GetConfig() io.Reader {
if o.Config == nil {
var z io.Reader
return z
}
return *o.Config
}
// WithComment set field Comment to given value
func (o *CommitOptions) WithComment(value string) *CommitOptions {
o.Comment = &value

View File

@ -164,6 +164,7 @@ type ContainerStatReport struct {
type CommitOptions struct {
Author string
Changes []string
Config []byte
Format string
ImageName string
IncludeVolumes bool

View File

@ -0,0 +1,22 @@
package abi
import (
"encoding/json"
"errors"
"io"
"github.com/containers/image/v5/manifest"
)
// DecodeOverrideConfig reads a Schema2Config from a Reader, suppressing EOF
// errors.
func DecodeOverrideConfig(reader io.Reader) (*manifest.Schema2Config, error) {
config := manifest.Schema2Config{}
if reader != nil {
err := json.NewDecoder(reader).Decode(&config)
if err != nil && !errors.Is(err, io.EOF) {
return nil, err
}
}
return &config, nil
}

View File

@ -0,0 +1,56 @@
package abi
import (
"strings"
"testing"
"github.com/containers/image/v5/manifest"
"github.com/stretchr/testify/assert"
)
func TestDecodeOverrideConfig(t *testing.T) {
testCases := []struct {
description string
body string
expectedValue *manifest.Schema2Config
expectedError bool
}{
{
description: "nothing",
body: ``,
expectedValue: &manifest.Schema2Config{},
},
{
description: "empty",
body: `{}`,
expectedValue: &manifest.Schema2Config{},
},
{
description: "user",
body: `{"User":"0:0"}`,
expectedValue: &manifest.Schema2Config{User: "0:0"},
},
{
description: "malformed",
body: `{"User":`,
expectedError: true,
},
}
t.Run("no reader", func(t *testing.T) {
value, err := DecodeOverrideConfig(nil)
assert.NoErrorf(t, err, "decoding nothing")
assert.NotNilf(t, value, "decoded value was unexpectedly nil")
})
for _, testCase := range testCases {
t.Run(testCase.description, func(t *testing.T) {
value, err := DecodeOverrideConfig(strings.NewReader(testCase.body))
if testCase.expectedError {
assert.Errorf(t, err, "decoding sample data")
} else {
assert.NoErrorf(t, err, "decoding sample data")
assert.NotNilf(t, value, "decoded value was unexpectedly nil")
assert.Equalf(t, *testCase.expectedValue, *value, "decoded value was not what we expected")
}
})
}
}

View File

@ -1,6 +1,7 @@
package abi
import (
"bytes"
"context"
"errors"
"fmt"
@ -17,6 +18,7 @@ import (
"github.com/containers/podman/v4/libpod"
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/libpod/logs"
"github.com/containers/podman/v4/pkg/api/handlers"
"github.com/containers/podman/v4/pkg/checkpoint"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/domain/entities/reports"
@ -581,18 +583,29 @@ func (ic *ContainerEngine) ContainerCommit(ctx context.Context, nameOrID string,
}
sc := ic.Libpod.SystemContext()
var changes []string
if len(options.Changes) > 0 {
changes = handlers.DecodeChanges(options.Changes)
}
var overrideConfig *manifest.Schema2Config
if len(options.Config) > 0 {
if overrideConfig, err = DecodeOverrideConfig(bytes.NewReader(options.Config)); err != nil {
return nil, err
}
}
coptions := buildah.CommitOptions{
SignaturePolicyPath: rtc.Engine.SignaturePolicyPath,
ReportWriter: options.Writer,
SystemContext: sc,
PreferredManifestType: mimeType,
OverrideConfig: overrideConfig,
}
opts := libpod.ContainerCommitOptions{
CommitOptions: coptions,
Pause: options.Pause,
IncludeVolumes: options.IncludeVolumes,
Message: options.Message,
Changes: options.Changes,
Changes: changes,
Author: options.Author,
Squash: options.Squash,
}

View File

@ -1,6 +1,7 @@
package tunnel
import (
"bytes"
"context"
"errors"
"fmt"
@ -347,7 +348,15 @@ func (ic *ContainerEngine) ContainerCommit(ctx context.Context, nameOrID string,
return nil, fmt.Errorf("invalid image name %q", opts.ImageName)
}
}
options := new(containers.CommitOptions).WithAuthor(opts.Author).WithChanges(opts.Changes).WithComment(opts.Message).WithSquash(opts.Squash).WithStream(!opts.Quiet)
var changes []string
if len(opts.Changes) > 0 {
changes = handlers.DecodeChanges(opts.Changes)
}
var configReader io.Reader
if len(opts.Config) > 0 {
configReader = bytes.NewReader(opts.Config)
}
options := new(containers.CommitOptions).WithAuthor(opts.Author).WithChanges(changes).WithComment(opts.Message).WithConfig(configReader).WithSquash(opts.Squash).WithStream(!opts.Quiet)
options.WithFormat(opts.Format).WithPause(opts.Pause).WithRepo(repo).WithTag(tag)
response, err := containers.Commit(ic.ClientCtx, nameOrID, options)
if err != nil {

30
test/apiv2/14-commit.at Normal file
View File

@ -0,0 +1,30 @@
# Create a container for testing the container initializing later
podman create -t -i --name myctr $IMAGE ls
config=$(mktemp -t config.XXXXXXXXXX.json)
cat > "$config" <<- EOF
{
"Entrypoint": ["/bin/crash"],
"Cmd": ["and", "burn"],
"Labels": {"for": "ever", "and": "ever"}
}
EOF
# Create a new image based on the container
t POST 'libpod/commit?container=myctr&repo=nativeimage&tag=1' $config 200
# Check some things
t GET libpod/images/nativeimage:1/json 200 ".Config.Cmd=$(jq .Cmd $config)" ".Config.Entrypoint=$(jq .Entrypoint $config)"
# Create a new image based on the container
t POST 'commit?container=myctr&repo=compatimage&tag=1' $config 201
# Check some things
t GET images/compatimage:1/json 200 ".Config.Cmd=$(jq .Cmd $config)" ".Config.Entrypoint=$(jq .Entrypoint $config)"
# Clean up
t DELETE containers/myctr 204
t DELETE images/nativeimage:1 200
t DELETE images/compatimage:1 200
rm -f "$config"
unset config

View File

@ -21,7 +21,7 @@ var _ = Describe("Podman commit", func() {
session := podmanTest.Podman([]string{"commit", "test1", "--change", "BOGUS=foo", "foobar.com/test1-image:latest"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
Expect(session.ErrorToString()).To(Equal("Error: invalid change \"BOGUS=foo\" - invalid instruction BOGUS"))
Expect(session.ErrorToString()).To(HaveSuffix(`applying changes: processing change "BOGUS foo": did not understand change instruction "BOGUS foo"`))
session = podmanTest.Podman([]string{"commit", "test1", "foobar.com/test1-image:latest"})
session.WaitWithDefaultTimeout()
@ -127,6 +127,45 @@ var _ = Describe("Podman commit", func() {
Expect(inspectResults[0].Labels).To(HaveKeyWithValue("image", "blue"))
})
It("podman commit container with --config flag", func() {
test := podmanTest.Podman([]string{"run", "--name", "test1", "-d", ALPINE, "ls"})
test.WaitWithDefaultTimeout()
Expect(test).Should(ExitCleanly())
Expect(podmanTest.NumberOfContainers()).To(Equal(1))
configFile, err := os.CreateTemp(podmanTest.TempDir, "")
Expect(err).Should(Succeed())
_, err = configFile.WriteString(`{"Labels":{"image":"green"}}`)
Expect(err).Should(Succeed())
configFile.Close()
session := podmanTest.Podman([]string{"commit", "-q", "--config", configFile.Name(), "test1", "foobar.com/test1-image:latest"})
session.WaitWithDefaultTimeout()
Expect(session).Should(ExitCleanly())
check := podmanTest.Podman([]string{"inspect", "foobar.com/test1-image:latest"})
check.WaitWithDefaultTimeout()
inspectResults := check.InspectImageJSON()
Expect(inspectResults[0].Labels).To(HaveKeyWithValue("image", "green"))
})
It("podman commit container with --config pointing to trash", func() {
test := podmanTest.Podman([]string{"run", "--name", "test1", "-d", ALPINE, "ls"})
test.WaitWithDefaultTimeout()
Expect(test).Should(ExitCleanly())
Expect(podmanTest.NumberOfContainers()).To(Equal(1))
configFile, err := os.CreateTemp(podmanTest.TempDir, "")
Expect(err).Should(Succeed())
_, err = configFile.WriteString("this is not valid JSON\n")
Expect(err).Should(Succeed())
configFile.Close()
session := podmanTest.Podman([]string{"commit", "-q", "--config", configFile.Name(), "test1", "foobar.com/test1-image:latest"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Not(ExitCleanly()))
})
It("podman commit container with --squash", func() {
test := podmanTest.Podman([]string{"run", "--name", "test1", "-d", ALPINE, "ls"})
test.WaitWithDefaultTimeout()