feat: add DOCKER_HOST environment (#875)

* start of work to add DOCKER_HOST environment this seems to be the basic to add support for a custom docker host as an envar or command line option. Some questions I have: 1. how/where to test 2. should the default value be set to something instead of empty string? Signed-off-by: vsoch <[email protected]> * adding support for bare DOCKER_ envars, and DOCKER_HOST tests Signed-off-by: vsoch <[email protected]> * trying to fix linting Signed-off-by: vsoch <[email protected]> * more linting interesting I did not see this output locally - I must have a different development environment than what is run in the tests! Signed-off-by: vsoch <[email protected]> * fixes from Dave review! Signed-off-by: vsoch <[email protected]> * fix parsing of docker host flag value Signed-off-by: vsoch <[email protected]> * typo in docker test and we do not need custom lookup for docker host flag Signed-off-by: vsoch <[email protected]> * undefined out Signed-off-by: vsoch <[email protected]> * simplify build command Signed-off-by: vsoch <[email protected]> * simplify build command Signed-off-by: vsoch <[email protected]> * try to fix docker build - scratch adding an empty file Signed-off-by: vsoch <[email protected]> * attempting to fix tests Signed-off-by: vsoch <[email protected]> * try triggering ci again? Signed-off-by: vsoch <[email protected]> Co-authored-by: vsoch <[email protected]>
sylabs · Jun 21, 2022 · fd7a3f2 · fd7a3f2
1 parent cd1aa5c
commit fd7a3f2
Show file tree

Hide file tree

Showing 12 changed files with 196 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,8 @@
   specified instance, running within a cgroup.
 - Add `--sparse` flag to `overlay create` command to allow generation of a
   sparse ext3 overlay image.
+- Support for `DOCKER_HOST` parsing when using `docker-daemon://`
+- `DOCKER_USERNAME` and `DOCKER_PASSWORD` supported without `SINGULARITY_` prefix.
 - The `--no-mount` flag now accepts the value `bind-paths` to disable mounting of
   all `bind path` entries in `singularity.conf.
 

diff --git a/cmd/internal/cli/action_flags.go b/cmd/internal/cli/action_flags.go
@@ -861,6 +861,7 @@ func init() {
 		cmdManager.RegisterFlagForCmd(&commonNoHTTPSFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&commonOldNoHTTPSFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&dockerLoginFlag, actionsInstanceCmd...)
+		cmdManager.RegisterFlagForCmd(&dockerHostFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&actionEnvFlag, actionsInstanceCmd...)

diff --git a/cmd/internal/cli/build.go b/cmd/internal/cli/build.go
@@ -300,6 +300,7 @@ func init() {
 		cmdManager.RegisterFlagForCmd(&commonNoHTTPSFlag, buildCmd)
 		cmdManager.RegisterFlagForCmd(&commonTmpDirFlag, buildCmd)
 
+		cmdManager.RegisterFlagForCmd(&dockerHostFlag, buildCmd)
 		cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, buildCmd)
 		cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, buildCmd)
 		cmdManager.RegisterFlagForCmd(&dockerLoginFlag, buildCmd)

diff --git a/cmd/internal/cli/build_linux.go b/cmd/internal/cli/build_linux.go
@@ -393,6 +393,7 @@ func runBuildLocal(ctx context.Context, cmd *cobra.Command, dst, spec string) {
 				LibraryAuthToken:  authToken,
 				KeyServerOpts:     ko,
 				DockerAuthConfig:  authConf,
+				DockerDaemonHost:  dockerHost,
 				EncryptionKeyInfo: keyInfo,
 				FixPerms:          buildArgs.fixPerms,
 				SandboxTarget:     sandboxTarget,

diff --git a/cmd/internal/cli/pull.go b/cmd/internal/cli/pull.go
@@ -141,6 +141,7 @@ func init() {
 		cmdManager.RegisterFlagForCmd(&pullDisableCacheFlag, PullCmd)
 		cmdManager.RegisterFlagForCmd(&pullDirFlag, PullCmd)
 
+		cmdManager.RegisterFlagForCmd(&dockerHostFlag, PullCmd)
 		cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, PullCmd)
 		cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, PullCmd)
 		cmdManager.RegisterFlagForCmd(&dockerLoginFlag, PullCmd)

diff --git a/cmd/internal/cli/push.go b/cmd/internal/cli/push.go
@@ -69,6 +69,7 @@ func init() {
 		cmdManager.RegisterFlagForCmd(&pushAllowUnsignedFlag, PushCmd)
 		cmdManager.RegisterFlagForCmd(&pushDescriptionFlag, PushCmd)
 
+		cmdManager.RegisterFlagForCmd(&dockerHostFlag, PushCmd)
 		cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, PushCmd)
 		cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, PushCmd)
 	})

diff --git a/cmd/internal/cli/singularity.go b/cmd/internal/cli/singularity.go
@@ -50,6 +50,7 @@ var currentRemoteEndpoint *endpoint.Config
 var (
 	dockerAuthConfig ocitypes.DockerAuthConfig
 	dockerLogin      bool
+	dockerHost       string
 
 	encryptionPEMPath   string
 	promptForPassphrase bool
@@ -124,24 +125,26 @@ var singVerboseFlag = cmdline.Flag{
 
 // --docker-username
 var dockerUsernameFlag = cmdline.Flag{
-	ID:           "dockerUsernameFlag",
-	Value:        &dockerAuthConfig.Username,
-	DefaultValue: "",
-	Name:         "docker-username",
-	Usage:        "specify a username for docker authentication",
-	Hidden:       true,
-	EnvKeys:      []string{"DOCKER_USERNAME"},
+	ID:            "dockerUsernameFlag",
+	Value:         &dockerAuthConfig.Username,
+	DefaultValue:  "",
+	Name:          "docker-username",
+	Usage:         "specify a username for docker authentication",
+	Hidden:        true,
+	EnvKeys:       []string{"DOCKER_USERNAME"},
+	WithoutPrefix: true,
 }
 
 // --docker-password
 var dockerPasswordFlag = cmdline.Flag{
-	ID:           "dockerPasswordFlag",
-	Value:        &dockerAuthConfig.Password,
-	DefaultValue: "",
-	Name:         "docker-password",
-	Usage:        "specify a password for docker authentication",
-	Hidden:       true,
-	EnvKeys:      []string{"DOCKER_PASSWORD"},
+	ID:            "dockerPasswordFlag",
+	Value:         &dockerAuthConfig.Password,
+	DefaultValue:  "",
+	Name:          "docker-password",
+	Usage:         "specify a password for docker authentication",
+	Hidden:        true,
+	EnvKeys:       []string{"DOCKER_PASSWORD"},
+	WithoutPrefix: true,
 }
 
 // --docker-login
@@ -154,6 +157,17 @@ var dockerLoginFlag = cmdline.Flag{
 	EnvKeys:      []string{"DOCKER_LOGIN"},
 }
 
+// --docker-host
+var dockerHostFlag = cmdline.Flag{
+	ID:            "dockerHostFlag",
+	Value:         &dockerHost,
+	DefaultValue:  "",
+	Name:          "docker-host",
+	Usage:         "specify a custom Docker daemon host",
+	EnvKeys:       []string{"DOCKER_HOST"},
+	WithoutPrefix: true,
+}
+
 // --passphrase
 var commonPromptForPassphraseFlag = cmdline.Flag{
 	ID:           "commonPromptForPassphraseFlag",

diff --git a/e2e/docker/docker.go b/e2e/docker/docker.go
@@ -8,10 +8,12 @@ package docker
 import (
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	dockerclient "github.com/docker/docker/client"
 	"github.com/pkg/errors"
 	"github.com/sylabs/singularity/e2e/internal/e2e"
 	"github.com/sylabs/singularity/e2e/internal/testhelper"
@@ -118,6 +120,138 @@ func (c ctx) testDockerPulls(t *testing.T) {
 	}
 }
 
+// Testing DOCKER_ host support (only if docker available)
+func (c ctx) testDockerHost(t *testing.T) {
+	require.Command(t, "docker")
+
+	// Write a temporary "empty" Dockerfile (from scratch)
+	tmpPath, err := fs.MakeTmpDir(c.env.TestDir, "docker-", 0o755)
+	err = errors.Wrapf(err, "creating temporary directory in %q for docker host test", c.env.TestDir)
+	if err != nil {
+		t.Fatalf("failed to create temporary directory: %+v", err)
+	}
+	defer os.RemoveAll(tmpPath)
+
+	// Here is the Dockerfile, and a temporary image path
+	dockerfile := filepath.Join(tmpPath, "Dockerfile")
+	emptyfile := filepath.Join(tmpPath, "file")
+	tmpImage := filepath.Join(tmpPath, "scratch-tmp.sif")
+
+	// Write Dockerfile and empty file to file
+	dockerfileContent := []byte("FROM scratch\nCOPY file /mrbigglesworth")
+	err = os.WriteFile(dockerfile, dockerfileContent, 0o644)
+	if err != nil {
+		t.Fatalf("failed to create temporary Dockerfile: %+v", err)
+	}
+
+	fileContent := []byte("")
+	err = os.WriteFile(emptyfile, fileContent, 0o644)
+	if err != nil {
+		t.Fatalf("failed to create empty file: %+v", err)
+	}
+
+	dockerURI := "dinosaur/test-image:latest"
+	pullURI := "docker-daemon:" + dockerURI
+
+	// Invoke docker build to build an empty scratch image in the docker daemon.
+	// Use os/exec because easier to generate a command with a working directory
+	e2e.Privileged(func(t *testing.T) {
+		cmd := exec.Command("docker", "build", "-t", dockerURI, ".")
+		cmd.Dir = tmpPath
+		out, err := cmd.CombinedOutput()
+		if err != nil {
+			t.Fatalf("Unexpected error while running command.\n%s: %s", err, string(out))
+		}
+	})
+
+	tests := []struct {
+		name       string
+		envarName  string
+		envarValue string
+		exit       int
+	}{
+		// Unset docker host should use default and succeed
+		{
+			name:       "singularityDockerHostEmpty",
+			envarName:  "SINGULARITY_DOCKER_HOST",
+			envarValue: "",
+			exit:       0,
+		},
+		{
+			name:       "dockerHostEmpty",
+			envarName:  "DOCKER_HOST",
+			envarValue: "",
+			exit:       0,
+		},
+
+		// bad Docker host should fail
+		{
+			name:       "singularityDockerHostInvalid",
+			envarName:  "SINGULARITY_DOCKER_HOST",
+			envarValue: "tcp://192.168.59.103:oops",
+			exit:       255,
+		},
+		{
+			name:       "dockerHostInvalid",
+			envarName:  "DOCKER_HOST",
+			envarValue: "tcp://192.168.59.103:oops",
+			exit:       255,
+		},
+
+		// Set to default should succeed
+		// The default host varies based on OS, so we use dockerclient default
+		{
+			name:       "singularityDockerHostValid",
+			envarName:  "SINGULARITY_DOCKER_HOST",
+			envarValue: dockerclient.DefaultDockerHost,
+			exit:       0,
+		},
+		{
+			name:       "dockerHostValid",
+			envarName:  "DOCKER_HOST",
+			envarValue: dockerclient.DefaultDockerHost,
+			exit:       0,
+		},
+	}
+
+	for _, tt := range tests {
+		// Export variable to environment if it's defined
+		if tt.envarValue != "" {
+			e2e.Privileged(func(t *testing.T) {
+				c.env.RunSingularity(
+					t,
+					e2e.WithEnv(append(os.Environ(), tt.envarValue)),
+					e2e.AsSubtest(tt.name),
+					e2e.WithProfile(e2e.UserProfile),
+					e2e.WithCommand("pull"),
+					e2e.WithArgs(tmpImage, pullURI),
+					e2e.ExpectExit(tt.exit),
+				)
+			})
+		} else {
+			e2e.Privileged(func(t *testing.T) {
+				c.env.RunSingularity(
+					t,
+					e2e.AsSubtest(tt.name),
+					e2e.WithProfile(e2e.UserProfile),
+					e2e.WithCommand("pull"),
+					e2e.WithArgs(tmpImage, pullURI),
+					e2e.ExpectExit(tt.exit),
+				)
+			})
+		}
+	}
+
+	// Clean up docker image
+	e2e.Privileged(func(t *testing.T) {
+		cmd := exec.Command("docker", "rmi", dockerURI)
+		_, err = cmd.Output()
+		if err != nil {
+			t.Fatalf("Unexpected error while cleaning up docker image.\n%s", err)
+		}
+	})
+}
+
 // AUFS sanity tests
 func (c ctx) testDockerAUFS(t *testing.T) {
 	imageDir, cleanup := e2e.MakeTempDir(t, c.env.TestDir, "aufs-", "")
@@ -731,6 +865,7 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests {
 	return testhelper.Tests{
 		"AUFS":             c.testDockerAUFS,
 		"def file":         c.testDockerDefFile,
+		"docker host":      c.testDockerHost,
 		"permissions":      c.testDockerPermissions,
 		"pulls":            c.testDockerPulls,
 		"registry":         c.testDockerRegistry,

diff --git a/internal/pkg/build/sources/conveyorPacker_oci.go b/internal/pkg/build/sources/conveyorPacker_oci.go
@@ -144,11 +144,13 @@ func (cp *OCIConveyorPacker) Get(ctx context.Context, b *sytypes.Bundle) (err er
 	cp.sysCtx = &types.SystemContext{
 		OCIInsecureSkipTLSVerify: cp.b.Opts.NoHTTPS,
 		DockerAuthConfig:         cp.b.Opts.DockerAuthConfig,
+		DockerDaemonHost:         cp.b.Opts.DockerDaemonHost,
 		OSChoice:                 "linux",
 		AuthFilePath:             syfs.DockerConf(),
 		DockerRegistryUserAgent:  useragent.Value(),
 		BigFilesTemporaryDir:     b.TmpDir,
 	}
+
 	if cp.b.Opts.NoHTTPS {
 		cp.sysCtx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(true)
 	}

diff --git a/pkg/build/types/bundle.go b/pkg/build/types/bundle.go
@@ -49,6 +49,8 @@ type Options struct {
 	KeyServerOpts []scskeyclient.Option
 	// contains docker credentials if specified.
 	DockerAuthConfig *ocitypes.DockerAuthConfig
+	// Custom docker Daemon host
+	DockerDaemonHost string
 	// EncryptionKeyInfo specifies the key used for filesystem
 	// encryption if applicable.
 	// A nil value indicates encryption should not occur.

diff --git a/pkg/cmdline/flag.go b/pkg/cmdline/flag.go
@@ -27,6 +27,8 @@ type Flag struct {
 	Required     bool
 	EnvKeys      []string
 	EnvHandler   EnvHandler
+	// Export envar also without prefix
+	WithoutPrefix bool
 	// When Value is a []String:
 	// If true, will use pFlag StringArrayVar(P) type, where values are not split on comma.
 	// If false, will use pFlag StringSliceVar(P) type, where a single value is split on commas.
@@ -52,6 +54,11 @@ func (m *flagManager) setFlagOptions(flag *Flag, cmd *cobra.Command) {
 
 	if len(flag.EnvKeys) > 0 {
 		cmd.Flags().SetAnnotation(flag.Name, "envkey", flag.EnvKeys)
+
+		// Environment flags can also be exported without a prefix (e.g. DOCKER_*)
+		if flag.WithoutPrefix {
+			cmd.Flags().SetAnnotation(flag.Name, "withoutPrefix", []string{"true"})
+		}
 	}
 	if flag.Deprecated != "" {
 		cmd.Flags().MarkDeprecated(flag.Name, flag.Deprecated)
@@ -202,9 +209,22 @@ func (m *flagManager) updateCmdFlagFromEnv(cmd *cobra.Command, prefix string) er
 			return
 		}
 		for _, key := range envKeys {
+
+			// First priority goes to prefixed variable
 			val, set := os.LookupEnv(prefix + key)
 			if !set {
-				continue
+
+				// Determine if environment keys should be looked for without prefix
+				// This annotation just needs to be present, period
+				_, withoutPrefix := flag.Annotations["withoutPrefix"]
+				if !withoutPrefix {
+					continue
+				}
+				// Second try - looking for the same without prefix!
+				val, set = os.LookupEnv(key)
+				if !set {
+					continue
+				}
 			}
 			if mflag.EnvHandler != nil {
 				if err := mflag.EnvHandler(flag, val); err != nil {

diff --git a/pkg/sylog/sylog.go b/pkg/sylog/sylog.go
@@ -28,7 +28,7 @@ var messageColors = map[messageLevel]string{
 
 var (
 	noColorLevel messageLevel = 90
-	loggerLevel  messageLevel = InfoLevel
+	loggerLevel               = InfoLevel
 )
 
 var logWriter = (io.Writer)(os.Stderr)