Merge pull request #44642 from thaJeztah/remote_cli_package_step1

Remove "cli" package (step 1)

cli/config/configdir.go

@@ -1,27 +0,0 @@
-package config // import "github.com/docker/docker/cli/config"
-
-import (
-	"os"
-	"path/filepath"
-
-	"github.com/docker/docker/pkg/homedir"
-)
-
-var (
-	configDir     = os.Getenv("DOCKER_CONFIG")
-	configFileDir = ".docker"
-)
-
-// Dir returns the path to the configuration directory as specified by the DOCKER_CONFIG environment variable.
-// If DOCKER_CONFIG is unset, Dir returns ~/.docker .
-// Dir ignores XDG_CONFIG_HOME (same as the docker client).
-// TODO: this was copied from cli/config/configfile and should be removed once cmd/dockerd moves
-func Dir() string {
-	return configDir
-}
-
-func init() {
-	if configDir == "" {
-		configDir = filepath.Join(homedir.Get(), configFileDir)
-	}
-}

cli/error.go

@@ -1,33 +0,0 @@
-package cli // import "github.com/docker/docker/cli"
-
-import (
-	"fmt"
-	"strings"
-)
-
-// Errors is a list of errors.
-// Useful in a loop if you don't want to return the error right away and you want to display after the loop,
-// all the errors that happened during the loop.
-type Errors []error
-
-func (errList Errors) Error() string {
-	if len(errList) < 1 {
-		return ""
-	}
-
-	out := make([]string, len(errList))
-	for i := range errList {
-		out[i] = errList[i].Error()
-	}
-	return strings.Join(out, ", ")
-}
-
-// StatusError reports an unsuccessful exit by a command.
-type StatusError struct {
-	Status     string
-	StatusCode int
-}
-
-func (e StatusError) Error() string {
-	return fmt.Sprintf("Status: %s, Code: %d", e.Status, e.StatusCode)
-}

cli/cobra.go → cmd/dockerd/cobra.go

@@ -1,4 +1,4 @@
-package cli // import "github.com/docker/docker/cli"
+package main
 
 import (
 	"fmt"

cmd/dockerd/docker.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/docker/docker/cli"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/pkg/jsonmessage"
@@ -32,7 +31,7 @@ func newDaemonCommand() (*cobra.Command, error) {
 		Short:         "A self-sufficient runtime for containers.",
 		SilenceUsage:  true,
 		SilenceErrors: true,
-		Args:          cli.NoArgs,
+		Args:          NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.flags = cmd.Flags()
 			return runDaemon(opts)
@@ -40,7 +39,7 @@ func newDaemonCommand() (*cobra.Command, error) {
 		DisableFlagsInUseLine: true,
 		Version:               fmt.Sprintf("%s, build %s", dockerversion.Version, dockerversion.GitCommit),
 	}
-	cli.SetupRootCommand(cmd)
+	SetupRootCommand(cmd)
 
 	flags := cmd.Flags()
 	flags.BoolP("version", "v", false, "Print version information and quit")

cmd/dockerd/error.go

@@ -0,0 +1,15 @@
+package main
+
+import (
+	"fmt"
+)
+
+// StatusError reports an unsuccessful exit by a command.
+type StatusError struct {
+	Status     string
+	StatusCode int
+}
+
+func (e StatusError) Error() string {
+	return fmt.Sprintf("Status: %s, Code: %d", e.Status, e.StatusCode)
+}

cmd/dockerd/options.go

@@ -4,9 +4,9 @@ import (
 	"os"
 	"path/filepath"
 
-	cliconfig "github.com/docker/docker/cli/config"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/opts"
+	"github.com/docker/docker/pkg/homedir"
 	"github.com/docker/go-connections/tlsconfig"
 	"github.com/spf13/pflag"
 )
@@ -27,6 +27,40 @@ const (
 )
 
 var (
+	// The configDir (and "DOCKER_CONFIG" environment variable) is now only used
+	// for the default location for TLS certificates to secure the daemon API.
+	// It is a leftover from when the "docker" and "dockerd" CLI shared the
+	// same binary, allowing the DOCKER_CONFIG environment variable to set
+	// the location for certificates to be used by both.
+	//
+	// We need to change this, as there's various issues:
+	//
+	//   - DOCKER_CONFIG only affects TLS certificates, but does not change the
+	//     location for the actual *daemon configuration* (which defaults to
+	//     "/etc/docker/daemon.json").
+	//   - If no value is set, configDir uses "~/.docker/" as default, but does
+	//     not take $XDG_CONFIG_HOME into account (it uses pkg/homedir.Get, which
+	//     is not XDG_CONFIG_HOME-aware).
+	//   - Using the home directory can be problematic in cases where the CLI and
+	//     daemon actually live on the same host; if DOCKER_CONFIG is set to set
+	//     the "docker" CLI configuration path (and if the daemon shares that
+	//     environment variable, e.g. "sudo -E dockerd"), the daemon may create
+	//     the "~/.docker/" directory, but now the directory may be owned by "root".
+	//
+	// We should:
+	//
+	//   - deprecate DOCKER_CONFIG for the daemon
+	//   - decide where the TLS certs should live by default ("/etc/docker/"?)
+	//   - look at "when" (and when _not_) XDG_CONFIG_HOME should be used. Its
+	//     needed for rootless, but perhaps could be used for non-rootless(?)
+	//   - When changing  the location for TLS config, (ideally) they should
+	//     live in a directory separate from "non-sensitive" (configuration-)
+	//     files, so that general configuration can be shared (dotfiles repo
+	//     etc) separate from "sensitive" config (TLS certificates).
+	//
+	// TODO(thaJeztah): deprecate DOCKER_CONFIG and re-design daemon config locations. See https://github.com/moby/moby/issues/44640
+	configDir       = os.Getenv("DOCKER_CONFIG")
+	configFileDir   = ".docker"
 	dockerCertPath  = os.Getenv("DOCKER_CERT_PATH")
 	dockerTLSVerify = os.Getenv("DOCKER_TLS_VERIFY") != ""
 )
@@ -44,6 +78,16 @@ type daemonOptions struct {
 	Validate     bool
 }
 
+// defaultCertPath uses $DOCKER_CONFIG or ~/.docker, and does not look up
+// $XDG_CONFIG_HOME. See the comment on configDir above for further details.
+func defaultCertPath() string {
+	if configDir == "" {
+		// Set the default path if DOCKER_CONFIG is not set.
+		configDir = filepath.Join(homedir.Get(), configFileDir)
+	}
+	return configDir
+}
+
 // newDaemonOptions returns a new daemonFlags
 func newDaemonOptions(config *config.Config) *daemonOptions {
 	return &daemonOptions{
@@ -54,9 +98,7 @@ func newDaemonOptions(config *config.Config) *daemonOptions {
 // installFlags adds flags for the common options on the FlagSet
 func (o *daemonOptions) installFlags(flags *pflag.FlagSet) {
 	if dockerCertPath == "" {
-		// cliconfig.Dir returns $DOCKER_CONFIG or ~/.docker.
-		// cliconfig.Dir does not look up $XDG_CONFIG_HOME
-		dockerCertPath = cliconfig.Dir()
+		dockerCertPath = defaultCertPath()
 	}
 
 	flags.BoolVarP(&o.Debug, "debug", "D", false, "Enable debug mode")
@@ -65,6 +107,7 @@ func (o *daemonOptions) installFlags(flags *pflag.FlagSet) {
 	flags.BoolVar(&o.TLS, FlagTLS, DefaultTLSValue, "Use TLS; implied by --tlsverify")
 	flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify || DefaultTLSValue, "Use TLS and verify the remote")
 
+	// TODO(thaJeztah): set default TLSOptions in config.New()
 	o.TLSOptions = &tlsconfig.Options{}
 	tlsOptions := o.TLSOptions
 	flags.StringVar(&tlsOptions.CAFile, "tlscacert", filepath.Join(dockerCertPath, DefaultCaFile), "Trust certs signed only by this CA")

cmd/dockerd/options_test.go

@@ -4,7 +4,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	cliconfig "github.com/docker/docker/cli/config"
 	"github.com/docker/docker/daemon/config"
 	"github.com/spf13/pflag"
 	"gotest.tools/v3/assert"
@@ -27,10 +26,6 @@ func TestCommonOptionsInstallFlags(t *testing.T) {
 	assert.Check(t, is.Equal(opts.TLSOptions.KeyFile, "/foo/key"))
 }
 
-func defaultPath(filename string) string {
-	return filepath.Join(cliconfig.Dir(), filename)
-}
-
 func TestCommonOptionsInstallFlagsWithDefaults(t *testing.T) {
 	flags := pflag.NewFlagSet("testing", pflag.ContinueOnError)
 	opts := newDaemonOptions(&config.Config{})
@@ -38,7 +33,7 @@ func TestCommonOptionsInstallFlagsWithDefaults(t *testing.T) {
 
 	err := flags.Parse([]string{})
 	assert.Check(t, err)
-	assert.Check(t, is.Equal(defaultPath("ca.pem"), opts.TLSOptions.CAFile))
-	assert.Check(t, is.Equal(defaultPath("cert.pem"), opts.TLSOptions.CertFile))
-	assert.Check(t, is.Equal(defaultPath("key.pem"), opts.TLSOptions.KeyFile))
+	assert.Check(t, is.Equal(filepath.Join(defaultCertPath(), "ca.pem"), opts.TLSOptions.CAFile))
+	assert.Check(t, is.Equal(filepath.Join(defaultCertPath(), "cert.pem"), opts.TLSOptions.CertFile))
+	assert.Check(t, is.Equal(filepath.Join(defaultCertPath(), "key.pem"), opts.TLSOptions.KeyFile))
 }

cli/required.go → cmd/dockerd/required.go

@@ -1,4 +1,4 @@
-package cli // import "github.com/docker/docker/cli"
+package main
 
 import (
 	"strings"