Skip to content

run: flag to include the docker socket #5858

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
thaJeztah merged 1 commit into from
Apr 16, 2025
Merged

run: flag to include the docker socket #5858

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 141 additions & 13 deletions cli/command/container/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
package container

import (
"archive/tar"
"bytes"
"context"
"fmt"
"io"
"net/netip"
"os"
"path"
"strings"

"github.com/containerd/platforms"
"github.com/distribution/reference"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/completion"
"github.com/docker/cli/cli/command/image"
"github.com/docker/cli/cli/config/configfile"
"github.com/docker/cli/cli/config/types"
"github.com/docker/cli/cli/internal/jsonstream"
"github.com/docker/cli/cli/streams"
"github.com/docker/cli/cli/trust"
"github.com/docker/cli/opts"
"github.com/docker/docker/api/types/container"
imagetypes "github.com/docker/docker/api/types/image"
"github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/client"
"github.com/docker/docker/errdefs"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
Expand All @@ -35,11 +43,12 @@ const (
)

type createOptions struct {
name string
platform string
untrusted bool
pull string // always, missing, never
quiet bool
name string
platform string
untrusted bool
pull string // always, missing, never
quiet bool
useAPISocket bool
}

// NewCreateCommand creates a new cobra.Command for `docker create`
Expand Down Expand Up @@ -70,6 +79,8 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {
flags.StringVar(&options.name, "name", "", "Assign a name to the container")
flags.StringVar(&options.pull, "pull", PullImageMissing, `Pull image before creating ("`+PullImageAlways+`", "|`+PullImageMissing+`", "`+PullImageNever+`")`)
flags.BoolVarP(&options.quiet, "quiet", "q", false, "Suppress the pull output")
flags.BoolVarP(&options.useAPISocket, "use-api-socket", "", false, "Bind mount Docker API socket and required auth")
flags.SetAnnotation("use-api-socket", "experimentalCLI", nil) // Marks flag as experimental for now.

// Add an explicit help that doesn't have a `-h` to prevent the conflict
// with hostname
Expand Down Expand Up @@ -179,20 +190,20 @@ func (cid *cidFile) Write(id string) error {
return nil
}

func newCIDFile(path string) (*cidFile, error) {
if path == "" {
func newCIDFile(cidPath string) (*cidFile, error) {
if cidPath == "" {
return &cidFile{}, nil
}
if _, err := os.Stat(path); err == nil {
return nil, errors.Errorf("container ID file found, make sure the other container isn't running or delete %s", path)
if _, err := os.Stat(cidPath); err == nil {
return nil, errors.Errorf("container ID file found, make sure the other container isn't running or delete %s", cidPath)
}

f, err := os.Create(path)
f, err := os.Create(cidPath)
if err != nil {
return nil, errors.Wrap(err, "failed to create the container ID file")
}

return &cidFile{path: path, file: f}, nil
return &cidFile{path: cidPath, file: f}, nil
}

//nolint:gocyclo
Expand Down Expand Up @@ -239,6 +250,73 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
return nil
}

const dockerConfigPathInContainer = "/run/secrets/docker/config.json"
var apiSocketCreds map[string]types.AuthConfig

if options.useAPISocket {
// We'll create two new mounts to handle this flag:
// 1. Mount the actual docker socket.
// 2. A synthezised ~/.docker/config.json with resolved tokens.

socket := dockerCli.DockerEndpoint().Host
if !strings.HasPrefix(socket, "unix://") {
return "", fmt.Errorf("flag --use-api-socket can only be used with unix sockets: docker endpoint %s incompatible", socket)
}
socket = strings.TrimPrefix(socket, "unix://") // should we confirm absolute path?

containerCfg.HostConfig.Mounts = append(containerCfg.HostConfig.Mounts, mount.Mount{
Type: mount.TypeBind,
Source: socket,
Target: "/var/run/docker.sock",
BindOptions: &mount.BindOptions{},
})

/*

Ideally, we'd like to copy the config into a tmpfs but unfortunately,
the mounts won't be in place until we start the container. This can
leave around the config if the container doesn't get deleted.

We are using the most compose-secret-compatible approach,
which is implemented at
https://github.com/docker/compose/blob/main/pkg/compose/convergence.go#L737

// Prepare a tmpfs mount for our credentials so they go away after the
// container exits. We'll copy into this mount after the container is
// created.
containerCfg.HostConfig.Mounts = append(containerCfg.HostConfig.Mounts, mount.Mount{
Type: mount.TypeTmpfs,
Target: "/docker/",
TmpfsOptions: &mount.TmpfsOptions{
SizeBytes: 1 << 20, // only need a small partition
Mode: 0o600,
},
})
*/

var envvarPresent bool
for _, envvar := range containerCfg.Config.Env {
if strings.HasPrefix(envvar, "DOCKER_CONFIG=") {
envvarPresent = true
}
}

// If the DOCKER_CONFIG env var is already present, we assume the client knows
// what they're doing and don't inject the creds.
if !envvarPresent {
// Set our special little location for the config file.
containerCfg.Config.Env = append(containerCfg.Config.Env,
"DOCKER_CONFIG="+path.Dir(dockerConfigPathInContainer))

// Resolve this here for later, ensuring we error our before we create the container.
creds, err := dockerCli.ConfigFile().GetAllCredentials()
if err != nil {
return "", fmt.Errorf("resolving credentials failed: %w", err)
}
apiSocketCreds = creds // inject these after container creation.
}
}

var platform *specs.Platform
// Engine API version 1.41 first introduced the option to specify platform on
// create. It will produce an error if you try to set a platform on older API
Expand Down Expand Up @@ -286,11 +364,25 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
if warn := localhostDNSWarning(*hostConfig); warn != "" {
response.Warnings = append(response.Warnings, warn)
}

containerID = response.ID
for _, w := range response.Warnings {
_, _ = fmt.Fprintln(dockerCli.Err(), "WARNING:", w)
}
err = containerIDFile.Write(response.ID)
return response.ID, err
err = containerIDFile.Write(containerID)
Copy link
Member

@thaJeztah thaJeztah Apr 15, 2025

Choose a reason for hiding this comment

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

Slightly related; looks like we may be discarding this error if failing to write to a container-id file failed, and something fails below.

Copy link
Contributor Author

@stevvooe stevvooe Apr 15, 2025

Choose a reason for hiding this comment

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

We return it below but first try to resolve and write the creds.

Copy link
Member

@thaJeztah thaJeztah Apr 15, 2025

Choose a reason for hiding this comment

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

Yeah, it would likely be a corner case; I was looking at these lines, where we don't return the containerID, and don't include the error about the container id-file being written.

if err := copyDockerConfigIntoContainer(ctx, dockerCli.Client(), containerID, dockerConfigPathInContainer, newConfig); err != nil {
	return "", fmt.Errorf("injecting docker config.json into container failed: %w", err)
}

We could do an errors.Join or something along those lines, but not sure if that's super great, so let's keep that for follow-ups.

Doing the "copy to container" as part of docker start was something that crossed my mind, but not sure if we want to / can update the files before start (that may open up possible race-conditions as well).

Copy link
Contributor Author

@stevvooe stevvooe Apr 15, 2025

Choose a reason for hiding this comment

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

Do you know why we write the containerID to a file on create? That's a bit unexpected. I tried to keep a light foot through here.

Copy link
Member

@thaJeztah thaJeztah Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Oh! It's not written to a file by default (that function's signature is a bit misleading); it's only written to a file if the uses passes the --cidfile=<some path> option.

If the flag isn't set, nothing is written and the function always returns nil

I'm not a huge fan of that feature, but I think it was added for some users that wanted to automate things and/or start containers through some init system (not a good fit)


if options.useAPISocket && apiSocketCreds != nil {
// Create a new config file with just the auth.
newConfig := &configfile.ConfigFile{
AuthConfigs: apiSocketCreds,
}

if err := copyDockerConfigIntoContainer(ctx, dockerCli.Client(), containerID, dockerConfigPathInContainer, newConfig); err != nil {
return "", fmt.Errorf("injecting docker config.json into container failed: %w", err)
}
}

return containerID, err
}

// check the DNS settings passed via --dns against localhost regexp to warn if
Expand Down Expand Up @@ -321,3 +413,39 @@ func validatePullOpt(val string) error {
)
}
}

// copyDockerConfigIntoContainer takes the client configuration and copies it
// into the container.
//
// The path should be an absolute path in the container, commonly
// /root/.docker/config.json.
func copyDockerConfigIntoContainer(ctx context.Context, dockerAPI client.APIClient, containerID string, configPath string, config *configfile.ConfigFile) error {
var configBuf bytes.Buffer
if err := config.SaveToWriter(&configBuf); err != nil {
return fmt.Errorf("saving creds: %w", err)
}

// We don't need to get super fancy with the tar creation.
var tarBuf bytes.Buffer
tarWriter := tar.NewWriter(&tarBuf)
tarWriter.WriteHeader(&tar.Header{
Name: configPath,
Size: int64(configBuf.Len()),
Mode: 0o600,
})

if _, err := io.Copy(tarWriter, &configBuf); err != nil {
return fmt.Errorf("writing config to tar file for config copy: %w", err)
}

if err := tarWriter.Close(); err != nil {
return fmt.Errorf("closing tar for config copy failed: %w", err)
}

if err := dockerAPI.CopyToContainer(ctx, containerID, "/",
&tarBuf, container.CopyToContainerOptions{}); err != nil {
return fmt.Errorf("copying config.json into container failed: %w", err)
}

return nil
}
1 change: 1 addition & 0 deletions cli/command/container/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command {
flags.StringVar(&options.detachKeys, "detach-keys", "", "Override the key sequence for detaching a container")
flags.StringVar(&options.pull, "pull", PullImageMissing, `Pull image before running ("`+PullImageAlways+`", "`+PullImageMissing+`", "`+PullImageNever+`")`)
flags.BoolVarP(&options.quiet, "quiet", "q", false, "Suppress the pull output")
flags.BoolVarP(&options.createOptions.useAPISocket, "use-api-socket", "", false, "Bind mount Docker API socket and required auth")

// Add an explicit help that doesn't have a `-h` to prevent the conflict
// with hostname
Expand Down
1 change: 1 addition & 0 deletions docs/reference/commandline/container_create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ Create a new container
| `--tmpfs` | `list` | | Mount a tmpfs directory |
| `-t`, `--tty` | `bool` | | Allocate a pseudo-TTY |
| `--ulimit` | `ulimit` | | Ulimit options |
| `--use-api-socket` | `bool` | | Bind mount Docker API socket and required auth |
| `-u`, `--user` | `string` | | Username or UID (format: <name\|uid>[:<group\|gid>]) |
| `--userns` | `string` | | User namespace to use |
| `--uts` | `string` | | UTS namespace to use |
Expand Down
1 change: 1 addition & 0 deletions docs/reference/commandline/container_run.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ Create and run a new container from an image
| [`--tmpfs`](#tmpfs) | `list` | | Mount a tmpfs directory |
| [`-t`](#tty), [`--tty`](#tty) | `bool` | | Allocate a pseudo-TTY |
| [`--ulimit`](#ulimit) | `ulimit` | | Ulimit options |
| `--use-api-socket` | `bool` | | Bind mount Docker API socket and required auth |
| `-u`, `--user` | `string` | | Username or UID (format: <name\|uid>[:<group\|gid>]) |
| [`--userns`](#userns) | `string` | | User namespace to use |
| [`--uts`](#uts) | `string` | | UTS namespace to use |
Expand Down
1 change: 1 addition & 0 deletions docs/reference/commandline/create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ Create a new container
| `--tmpfs` | `list` | | Mount a tmpfs directory |
| `-t`, `--tty` | `bool` | | Allocate a pseudo-TTY |
| `--ulimit` | `ulimit` | | Ulimit options |
| `--use-api-socket` | `bool` | | Bind mount Docker API socket and required auth |
| `-u`, `--user` | `string` | | Username or UID (format: <name\|uid>[:<group\|gid>]) |
| `--userns` | `string` | | User namespace to use |
| `--uts` | `string` | | UTS namespace to use |
Expand Down
1 change: 1 addition & 0 deletions docs/reference/commandline/run.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ Create and run a new container from an image
| `--tmpfs` | `list` | | Mount a tmpfs directory |
| `-t`, `--tty` | `bool` | | Allocate a pseudo-TTY |
| `--ulimit` | `ulimit` | | Ulimit options |
| `--use-api-socket` | `bool` | | Bind mount Docker API socket and required auth |
| `-u`, `--user` | `string` | | Username or UID (format: <name\|uid>[:<group\|gid>]) |
| `--userns` | `string` | | User namespace to use |
| `--uts` | `string` | | UTS namespace to use |
Expand Down
Loading